Skip to content

Change re-authentication to only require a second factor rather than password and second factor and require authentication for more account management actions#8037

Merged
mitchellhenke merged 11 commits intomainfrom
mitchellhenke/reauthentication-required-2
Mar 24, 2023
Merged

Change re-authentication to only require a second factor rather than password and second factor and require authentication for more account management actions#8037
mitchellhenke merged 11 commits intomainfrom
mitchellhenke/reauthentication-required-2

Conversation

@mitchellhenke
Copy link
Contributor

🛠 Summary of changes

Follow-up to #8031

This PR intends to simplify some of the re-authentication requirements and improve account security within the IDP in two ways:

  • Require only a second factor authentication
    • We currently require the user to enter their password and then 2FA, and this flow can be a bit fickle since it has to manage state across a few requests
    • This also includes not allowing a remembered device cookie for the second factor re-authentication
  • Add re-authentication requirements to authentication-related account management actions like managing backup codes, etc.

It is behind a feature flag where the behavior should not change when it is disabled and it should be backwards and forwards compatible.

@mitchellhenke mitchellhenke requested a review from a team March 21, 2023 15:31
Copy link
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, small questions, no big objections

@mitchellhenke mitchellhenke force-pushed the mitchellhenke/reauthentication-required-2 branch 4 times, most recently from 390f05d to ae66066 Compare March 22, 2023 15:17
Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

I know it's a hold-over, but... is this value used for anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not that I can find. It's added in the original PR for this behavior, but I can't see that it's used for anything in it: https://github.com/18F/identity-idp/pull/507/files#diff-21b23b276c9228c6a6d1393e4f1ef35a90a49d7c9776cae93f395381990dea36R5

@mitchellhenke mitchellhenke force-pushed the mitchellhenke/reauthentication-required-2 branch from 4cef28b to b68b10c Compare March 24, 2023 19:19
Mitchell Henke added 10 commits March 24, 2023 15:18
…password and second factor and require authentication for more account management actions

changelog: User-Facing Improvements, Authentication, Change re-authentication to only require a second factor rather than password and second factor and require authentication for more account management actions
@mitchellhenke mitchellhenke force-pushed the mitchellhenke/reauthentication-required-2 branch from b68b10c to 5e5fb18 Compare March 24, 2023 20:18
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.

3 participants