Skip to content
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

Numerous Improvements for Two Factor Authentication #223

Closed
wants to merge 28 commits into from
Closed

Numerous Improvements for Two Factor Authentication #223

wants to merge 28 commits into from

Conversation

denera
Copy link

@denera denera commented Dec 9, 2022

New features:

  • Existing users can enable/disable 2FA through their Control Hub.
  • Admins can enable/disable 2FA for users through the Authentication Area.
  • New users registering with 2FA and existing users activating 2FA are shown a QR code for their OTP URI link.
  • Password changes for users with 2FA require a valid 2FA code.
  • 2FA can be required as well as allowed. When required, existing users who don't already have 2FA automatically receive 2FA credentials during their next login while new users are registered with 2FA by default.
  • 2FA secrets can be generated using the Google Authenticator PAM module. This is particularly useful for system users to maintain a single 2FA secret that works for multiple different services hosted on the same system.

Changes:

  • Added a NativeAuthenticator.change_2fa() function
  • Added Change2FAHandler and Change2FAAdminHandler
  • Modified ChangePasswordHandler to include 2FA
  • Modified LoginHandler to check if 2FA is required, and if so, automatically enable it for users without 2FA during login
  • Modified SignUpHandler to check if 2FA is required, and if so, automatically enable it for all new users
  • Added UserInfo.get_otp_secret() to optionally use google-authenticator to generate the 2FA secret key, or inherit the existing one from /home/user/.google_authenticator
  • Swapped onetimepass with pyotp to support OTP URI links and improve compliance with newer Google Authenticator implementations that use an issuer parameter in the URI for internal disambiguation
  • Added new HTML templates and modified existing ones to support new 2FA features
  • Added documentation for new features in options.md

Related Issues: #152, #155 #167, #172, #185

Notes:
The Google Authenticator PAM Module interaction is a little bit hacky right now. When NativeAuthenticator is configured to use this module, UserInfo first attempts to pull an existing 2FA secret out of /home/user/.google_authenticator and if there is no existing secret file, then it uses an os.system() call to google-authenticator to generate a new one.

I took a swing at doing this more cleanly by using pamela to authenticate through a JupyterHub PAM service that defines auth required pam_google_authenticator.so nullok. Unfortunately I never managed to get it to authenticate correctly with credentials and 2FA codes that I knew were correct. If someone else has experience making this work, please let me know.

Looking forward to any feedback. Happy to discuss and modify any UI and/or functionality choices.

@welcome
Copy link

welcome bot commented Dec 9, 2022

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@denera
Copy link
Author

denera commented Dec 9, 2022

The failed tests seem to be related to hub.init_db() call in tests/test_authenticator.py. Not sure if that could be introduced by anything in this PR but please let me know if so.

I'll try to add some new unit tests for 2FA as well while I'm at it.

@denera denera closed this by deleting the head repository Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant