Skip to content

Refactor reauthentication required functionality from subclass to module#8031

Merged
mitchellhenke merged 3 commits intomainfrom
mitchellhenke/reauthentication-refactored
Mar 21, 2023
Merged

Refactor reauthentication required functionality from subclass to module#8031
mitchellhenke merged 3 commits intomainfrom
mitchellhenke/reauthentication-refactored

Conversation

@mitchellhenke
Copy link
Contributor

🛠 Summary of changes

We have some upcoming changes around how to handle reauthentication for accounts, and this is a slight refactor to make it a module with a method that can be called rather than a subclass that automatically calls the internal method.

This should not result in a change in behavior.

changelog: Internal, Authentication, Refactor reauthentication required functionality from subclass to module
@mitchellhenke mitchellhenke force-pushed the mitchellhenke/reauthentication-refactored branch from 21b52ef to 466d8b1 Compare March 20, 2023 20:14
@@ -1,9 +1,12 @@
module Users
class EmailsController < ReauthnRequiredController
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see the controller implementation removed from code. Should it be?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

whoops, yep, thank you

@mitchellhenke mitchellhenke requested a review from a team March 20, 2023 20:41
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 👍

@mitchellhenke mitchellhenke force-pushed the mitchellhenke/reauthentication-refactored branch from 781f89d to 09b6d0e Compare March 20, 2023 21:04
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


before_action :confirm_two_factor_authenticated
before_action :prompt_for_password_if_pii_locked
before_action :confirm_recently_authenticated
Copy link
Contributor

Choose a reason for hiding this comment

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

thank you for doing these as explicit includes instead of included do ... magic!

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