-
Notifications
You must be signed in to change notification settings - Fork 415
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
feat: Allow organisation admins to mandate 2fa for their organisation #2877
feat: Allow organisation admins to mandate 2fa for their organisation #2877
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Uffizzi Preview |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2877 +/- ##
=======================================
Coverage 95.60% 95.61%
=======================================
Files 1009 1010 +1
Lines 28904 28914 +10
=======================================
+ Hits 27634 27645 +11
+ Misses 1270 1269 -1
☔ View full report in Codecov by Sentry. |
@@ -275,7 +275,7 @@ const App = class extends Component { | |||
) | |||
} | |||
if (AccountStore.forced2Factor()) { |
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 think instead of making all of the isLoginChanges, just adjust this function to return false if 2fa has been set. It should remove the need for any other frontend change in this PR.
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.
This change was made to handle the case where you are already logged in, your organization forces 2factor and you disable your 2factor, it prevents the page from reloading, which does happen if 2factor is enabled on login.
@@ -381,8 +385,12 @@ const store = Object.assign({}, BaseStore, { | |||
return false | |||
} | |||
|
|||
if (store.samlOrOauth) { |
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.
Can be replaced with my other suggestion.
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.
This prevents the 2factor from being forced on Oauth or Saml accounts
@kyle-ssg Can you re-review, please? |
Thanks for submitting a PR! Please check the boxes below:
pre-commit
to check lintingChanges
How did you test this code?