TWO_FACTOR_SKIP_WELCOME setting to skip welcome page. #150
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
two_factor/views/core.py
Outdated
| ('welcome', Form), | ||
| ('method', MethodForm), | ||
| ('generator', TOTPDeviceForm), | ||
| ('sms', PhoneNumberForm), | ||
| ('call', PhoneNumberForm), | ||
| ('validation', DeviceValidationForm), | ||
| ('yubikey', YubiKeyDeviceForm), | ||
| ) | ||
| ] | ||
| if hasattr(settings, 'TWO_FACTOR_SKIP_WELCOME') and settings.TWO_FACTOR_SKIP_WELCOME: |
There was a problem hiding this comment.
Use condition_dict to skip certain wizard pages
| @@ -213,7 +213,10 @@ class SetupView(IdempotentSessionWizardView): | |||
| ('validation', DeviceValidationForm), | |||
| ('yubikey', YubiKeyDeviceForm), | |||
| ) | |||
|
|
|||
| show_welcome = not hasattr(settings, 'TWO_FACTOR_SKIP_WELCOME') or not settings.TWO_FACTOR_SKIP_WELCOME | |||
There was a problem hiding this comment.
This will be executed at import time, not when the lambda function in the condition_dict is being called. Can you either define the lambda expression here or move this into the lambda function's body, please.
There was a problem hiding this comment.
why not just not getattr(settings, 'TWO_FACTOR_SKIP_WELCOME', False)?
There was a problem hiding this comment.
- because a definition of
Nonein the settings is not the same asFalse. - because it will still be executed at import time, not when it should be evaluated.
There was a problem hiding this comment.
- Change
FalsetoNonethen. - What's the problem of being executed at import time? (don't know. actually curious)
There was a problem hiding this comment.
- It's only evaluated once which makes testing harder. When you want to check for different values of
TWO_FACTOR_SKIP_WELCOMEyou'd need to take care of that yourself. Using dynamic evaluation at runtime isn't much slower within this simple situation but makes testing easier.
|
Apart from that, can you please add the settings to the documentation and add a test for this. Thank you 😄 |
MarkusH
requested changes
Oct 3, 2016
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR for issue #149
TWO_FACTOR_SKIP_WELCOME = True in settings will drop welcome page from wizard.
If setting is left out it will continue to work as normal.
Unit tests all passing.