Skip to content

Allow resending OTP in reauthentication context#998

Merged
monfresh merged 1 commit intomasterfrom
mb-allow-resending-sms
Jan 26, 2017
Merged

Allow resending OTP in reauthentication context#998
monfresh merged 1 commit intomasterfrom
mb-allow-resending-sms

Conversation

@monfresh
Copy link
Copy Markdown
Contributor

Why: It would be broken functionality otherwise.

How: Define a new context called reauthentication in the change
factor flow because TwoFactorAuthenticable#check_already_authenticated
should only be called in the initial authentication context.

**Why**: It would be broken functionality otherwise.

**How**: Define a new context called `reauthentication` in the change
factor flow because `TwoFactorAuthenticable#check_already_authenticated`
should only be called in the initial authentication context.
Copy link
Copy Markdown
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM


def authentication_context?
context == 'authentication'
context == 'authentication' || context == 'reauthentication'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe now is still not the time...but I think it would be great if we could make these auth contexts into constants? Making them constants helps remind that they're finite and enumerable

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

agreed. I was planning on making another pass at this module to see how to improve it.

@monfresh monfresh merged commit 8c6d0b7 into master Jan 26, 2017
@monfresh monfresh deleted the mb-allow-resending-sms branch January 26, 2017 21:19
amoose pushed a commit that referenced this pull request Mar 7, 2017
**Why**: It would be broken functionality otherwise.

**How**: Define a new context called `reauthentication` in the change
factor flow because `TwoFactorAuthenticable#check_already_authenticated`
should only be called in the initial authentication context.
amoose pushed a commit that referenced this pull request Mar 8, 2017
**Why**: It would be broken functionality otherwise.

**How**: Define a new context called `reauthentication` in the change
factor flow because `TwoFactorAuthenticable#check_already_authenticated`
should only be called in the initial authentication context.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants