Skip to content

Extract mixin for common MFA deletion behaviors#11796

Merged
aduth merged 1 commit intomainfrom
aduth-mfa-deletion-concern
Jan 28, 2025
Merged

Extract mixin for common MFA deletion behaviors#11796
aduth merged 1 commit intomainfrom
aduth-mfa-deletion-concern

Conversation

@aduth
Copy link
Copy Markdown
Contributor

@aduth aduth commented Jan 24, 2025

🎫 Ticket

Related to LG-14052

🛠 Summary of changes

Refactors to create a new MfaDeletionConcern#handle_successful_mfa_deletion method. This builds upon #11795, where the method was initially introduced to handle "Undo" behaviors associated with a mismatched WebAuthn configuration.

Deleting a multi-factor authentication method always incurs the following:

  • Creating a user history event
  • Revoking remembered device
  • Dispatching a RISC security event

Previously, these were duplicated across controllers for each method, which creates risk of human error for failing to account for one or more of these actions.

As an example, refactoring this behavior reveals that we don't have a user event associated with deleting backup codes, which became obvious when abstracting this method with a required event_type keyword argument. This will be addressed in a follow-up ticket.

📜 Testing Plan

Verify no regressions of impacted behavior, and that existing test coverage passes.

@aduth aduth requested a review from a team January 24, 2025 13:37
@aduth aduth force-pushed the aduth-mfa-deletion-concern branch from f1de60b to d79f40f Compare January 24, 2025 16:45
changelog: Internal, Code Quality, Extract mixin for common MFA deletion behaviors
@aduth aduth force-pushed the aduth-mfa-deletion-concern branch from d79f40f to 15f7bf7 Compare January 28, 2025 17:44
@aduth
Copy link
Copy Markdown
Contributor Author

aduth commented Jan 28, 2025

I've pushed a rebase of the branch after merging #11795. Since #11795 separately included the mixin method as part of its implementation, the scope of this branch is now limited to using the mixin consistently across other MFA method controllers.

@aduth
Copy link
Copy Markdown
Contributor Author

aduth commented Jan 28, 2025

As an example, refactoring this behavior reveals that we don't have a user event associated with deleting backup codes, which became obvious when abstracting this method with a required event_type keyword argument. This will be addressed in a follow-up ticket.

Ticket: LG-15602

@aduth aduth merged commit b3941b1 into main Jan 28, 2025
@aduth aduth deleted the aduth-mfa-deletion-concern branch January 28, 2025 21:58
n1zyy pushed a commit that referenced this pull request Jan 31, 2025
changelog: Internal, Code Quality, Extract mixin for common MFA deletion behaviors
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.

2 participants