-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add two-factor authentication #13704
Conversation
@jtkech you may me interested in reviewing this one. If you have time, a review is much appreciated. Hopefully @sebastienros can do a review during triage tomorrow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a quick review, suggesting few changes, I will revise the PR again
Great work Mike!!
src/OrchardCore/OrchardCore.Users.Core/Services/LoginSettingsExtensions.cs
Outdated
Show resolved
Hide resolved
src/OrchardCore/OrchardCore.Users.Core/Services/LoginSettingsExtensions.cs
Show resolved
Hide resolved
|
@MikeAlhayek could you please provide a screenshot for asking for the token and possibly how to recover the missing token? Also it would be nice if you can demo it in the OC steering meeting |
It was demonstrated yesterday during the meeting :). |
No. The specification of the Authenticator apps only support 6 or 8. |
I expect something like that :) unfortunately, I didn't attend last few meetings |
…xtensions.cs Co-authored-by: Hisham Bin Ateya <hishamco_2007@yahoo.com>
src/OrchardCore.Modules/OrchardCore.Users/Drivers/RoleLoginSettingsDisplayDriver.cs
Outdated
Show resolved
Hide resolved
src/OrchardCore.Modules/OrchardCore.Users/Services/UsersThemeSelector.cs
Outdated
Show resolved
Hide resolved
Looking at aspnetcore I don't think the user id/name is ever logged. |
I think I was looking at a code that was generated using the code generator. Either way, it be good news is that we don't log that private info also. |
Fix #13703