Skip to content

Require 2FA to accept an org invite #14485

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

Merged
merged 2 commits into from
Sep 5, 2023

Conversation

ristomcgehee
Copy link
Contributor

This PR will require a user to have 2FA enabled in order to accept an invitation to join an organization.

Here's how it looks when they click on the link to join:
image

Closes #13768

@ristomcgehee ristomcgehee requested a review from a team as a code owner September 4, 2023 03:54
@ristomcgehee ristomcgehee force-pushed the enforce-2fa-org-invite branch from f50fbf4 to d66bb0e Compare September 4, 2023 04:04
@ristomcgehee
Copy link
Contributor Author

The failed check appears to be just an intermittent build issue.

@miketheman miketheman added 2FA HTML requires change to HTML files labels Sep 5, 2023
Comment on lines +21 to +25
{% if request.user.has_two_factor %}
{{ super() }}
{% else %}
<!-- Content hidden -->
{% endif %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise: ‏I hadn't seen this pattern used, nice way to hide the confirmation.

@miketheman miketheman merged commit a9169ec into pypi:main Sep 5, 2023
@miketheman
Copy link
Member

Thanks @ristomcgehee !

@di
Copy link
Member

di commented Sep 5, 2023

I'm not sure this is sufficient, a user could still manually POST to the endpoint that corresponds to this form and bypass the restriction.

Instead, the pattern I think we should generally follow is: still show the form, but make the "submit" button disabled and have a secondary button/direction to perform the blocking action, and also add a similar check on the view to prevent manual posting from succeeding.

@ristomcgehee
Copy link
Contributor Author

I haven't been adding server-side checks for these "require 2fa" PRs because I don't see this as a security control but rather as a nudge to enable 2fa. If you like, I can add server-side checks to these places.

What do you mean by this part:

have a secondary button/direction to perform the blocking action

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2FA HTML requires change to HTML files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Require 2FA to accept an invite to join an organization
3 participants