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

feature: Improve 2FA #101

Merged
merged 1 commit into from
Jun 10, 2019
Merged

feature: Improve 2FA #101

merged 1 commit into from
Jun 10, 2019

Conversation

jwag956
Copy link
Member

@jwag956 jwag956 commented Jun 9, 2019

A rather large refactor with 2 main goals:

  1. remove sending personal info as part of initialization during signup
  2. implement opt-in in addition to 2FA required mode.

While the basic flow and forms didn't change - there were many changes:

  • A new configuration variable SECURITY_TWO_FACTOR_REQUIRED (default False)
    If you want old behavior of requiring 2FA - this must be set.
  • Contents of session cookie are completely different.
  • CHANGED: Signal names:
    • 'user-two-factored' -> 'tf_code_confirmed'
    • 'two_factor_method_changed' -> 'tf_profile_changed'
  • 2 New signals introduced: 'tf_disabled' and 'tf_security_token_sent'
  • Code for most 2FA views changed dramatically, however the actual flow
    should be compatible.
  • CHANGE: if call /tf-setup and haven't re-confirmed password - now redirect to
    two_factor_confirm_url rather than login_url.
  • CHANGED: as part of naming rationalization - the context processor names changed:
    • two_factor_change_method_password_confirmation_context_processor -> tf_password_verify_context_processor
    • two_factor_setup_context_processor -> tf_setup_context_processor
    • two_factor_token_validation_context_processor -> tf_token_validation_context_processor

Various bugs and doc improvements:

  • The two factor _VALIDITY configuration variables were fixed to reflect prior changes
    which meant these values are now in seconds.
  • A new message TWO_FACTOR_DISABLED was introduced

Testing Improvements:

  • view_scaffold.py was introduced that makes it easy to test forms. This is a real
    Flask application that can be run, and using a normal browser can interact with
    various workflows.

Closes #95

@jwag956 jwag956 self-assigned this Jun 9, 2019
@jwag956 jwag956 requested a review from TillerBurr June 9, 2019 04:44
@jwag956 jwag956 added this to the 3.2.0 milestone Jun 9, 2019
flask_security/forms.py Outdated Show resolved Hide resolved
TillerBurr
TillerBurr previously approved these changes Jun 10, 2019
Copy link
Collaborator

@TillerBurr TillerBurr left a comment

Choose a reason for hiding this comment

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

The views did change quite a bit, glad I didn't start writing tests. I think this is good to merge.

@jwag956
Copy link
Member Author

jwag956 commented Jun 10, 2019

@baurt thanks for your review. I incorporated your ideas. Will merge after that.
Just one more refactor to do - as laid out in issue #102

thanks for sticking with this and hopefully this isn't causing you too much work.
With 102 in I will cut an rc2 and see if you and maybe some other can try it out - then release it - hopefully by end of june.

A rather large refactor with 2 main goals:

1) remove sending personal info as part of initialization during signup
2) implement opt-in in addition to 2FA required mode.

While the basic flow and forms didn't change - there were many changes:

- A new configuration variable SECURITY_TWO_FACTOR_REQUIRED (default False)
  If you want old behavior of requiring 2FA - this must be set.
- Contents of session cookie are completely different.
- CHANGED: Signal names:
   - 'user-two-factored' -> 'tf_code_confirmed'
   - 'two_factor_method_changed' -> 'tf_profile_changed'
- 2 New signals introduced: 'tf_disabled' and 'tf_security_token_sent'
- Code for most 2FA views changed dramatically, however the actual flow
  should be compatible.
- CHANGE: if call /tf-setup and haven't re-confirmed password - now redirect to
  two_factor_confirm_url rather than login_url.
- CHANGED: as part of naming rationalization - the context processor names changed:
   - two_factor_change_method_password_confirmation_context_processor -> tf_password_verify_context_processor
   - two_factor_setup_context_processor -> tf_setup_context_processor
   - two_factor_token_validation_context_processor -> tf_token_validation_context_processor

Various bugs and doc improvements:

- The two factor _VALIDITY configuration variables were fixed to reflect prior changes
  which meant these values are now in seconds.
- A new message TWO_FACTOR_DISABLED was introduced

Testing Improvements:

- view_scaffold.py was introduced that makes it easy to test forms. This is a real
  Flask application that can be run, and using a normal browser can interact with
  various workflows.
@TillerBurr
Copy link
Collaborator

@jwag956 No worries. I enjoy looking at it and helping improve the library.

@jwag956 jwag956 merged commit c8d7adb into master Jun 10, 2019
@jwag956 jwag956 deleted the peruser95 branch June 13, 2019 02:40
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.

2FA - enable per-user 2FA
2 participants