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.

Sign up for GitHub

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

TWO_FACTOR_SKIP_WELCOME setting to skip welcome page. #150

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

TWO_FACTOR_SKIP_WELCOME setting to skip welcome page. #150

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
123bea3
TWO_FACTOR_SKIP_WELCOME setting to skip welcome page.
stewbawka Jun 8, 2016
4f13c24
Using condition_dict to skip welcome page according to settings
stewbawka Jul 7, 2016
0e74905
Fixed pep8 linting complaint
stewbawka Jul 7, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions two_factor/views/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link

@pbassut pbassut Oct 3, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

why not just not getattr(settings, 'TWO_FACTOR_SKIP_WELCOME', False)?

Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. because a definition of None in the settings is not the same as False.
  2. because it will still be executed at import time, not when it should be evaluated.

Copy link

@pbassut pbassut Oct 3, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

  1. Change False to None then.
  2. What's the problem of being executed at import time? (don't know. actually curious)

Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. It's only evaluated once which makes testing harder. When you want to check for different values of TWO_FACTOR_SKIP_WELCOME you'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.

condition_dict = {
'welcome': lambda self: show_welcome,
'generator': lambda self: self.get_method() == 'generator',
'call': lambda self: self.get_method() == 'call',
'sms': lambda self: self.get_method() == 'sms',
Expand Down