Skip to content

feat: add email for 2fa not yet enabled on upload #14444

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

Conversation

miketheman
Copy link
Member

As a way to create an activity-based campaign, enable the email on file upload.
Use the repeat_window to only send the email again within the 14 (or 90 in the event of failure) day window.

Resolves #13761

Includes a test commit refactor to supply totp_secret by default.

As preparation for adding 2FA requirement for everyone,
update the base `UserFactory` to have a random `totp_secret` value, which
then makes `User.has_2fa` return `True` during tests.

Make explicit `None` in test cases that are asserting about specifics.

This is simpler than updating the ~50 test cases in
forklift/test_legacy.py, not to mention the other 400+ test locations
that use `UserFactory`.

Signed-off-by: Mike Fiedler <[email protected]>
As a way to create an activity-based campaign, enable the email on file
upload.
Use the `repeat_window` to only send the email again within the 14 (or
90 in the event of failure) day window.

Resolves pypi#13761

Signed-off-by: Mike Fiedler <[email protected]>
@miketheman miketheman added the 2FA label Aug 28, 2023
@miketheman miketheman requested a review from a team as a code owner August 28, 2023 20:45
@miketheman
Copy link
Member Author

An example of the HTML email, for easier viewing:

Screenshot 2023-08-28 at 16 46 34

@@ -1493,6 +1494,11 @@ def file_upload(request):
},
)

# Check if the user has any 2FA methods enabled, and if not, email them.
if request.user and not request.user.has_two_factor:
Copy link
Member

Choose a reason for hiding this comment

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

Two questions: under what conditions would request.user be falsy rather than None here? should we be doing a request.user is None instead of implicit falsy check?

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like it's either a sqlalchemy object or None.

I followed the pattern that others had implemented in this 700+ line function where others use if request.user.

Signed-off-by: Mike Fiedler <[email protected]>
@miketheman miketheman merged commit 90cf722 into pypi:main Aug 29, 2023
@miketheman miketheman deleted the miketheman/13761-email-campaign-2fa-on-upload branch August 29, 2023 15:55
@miketheman miketheman mentioned this pull request Jun 9, 2023
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Begin 2FA email campaign for 2024 requirement.
2 participants