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.

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

feat: introduce mfa #1645

Merged
merged 50 commits into from
Nov 1, 2024
Merged

feat: introduce mfa #1645

merged 50 commits into from
Nov 1, 2024

Conversation

bjoern-m
Copy link
Contributor

@bjoern-m bjoern-m commented Sep 23, 2024

Description

This PR introduces the implementation of Multi-Factor Authentication (MFA) across various user flows, including MFA onboarding and profile management, along with numerous fixes and feature additions.

MFA Support

  • Added support for MFA onboarding and usage across different flows (e.g., registration, login, recovery).
  • Enabled the use of authenticator apps and security keys for MFA.

WebAuthn Enhancements

  • Updated AAGUID list and adjusted WebAuthn-related flows, including simplifying WebAuthn creation calls and adding all credentials to the exclude list on registration.
  • Improved error handling for WebAuthn creation and credential persistence.

UI/UX Improvements

  • Added MFA related pages
  • Added new icons and translations

lfleischmann and others added 30 commits October 22, 2024 16:19
* feat: add mfa-usage sub-flow

---------

Co-authored-by: Lennart Fleischmann <[email protected]>
The mfa_creation subflow sets an mfa_method stash value so that
when creating and persisting the credential the mfa_only flag can
be set correctly in the hook responsible for that. But the profile flow
never "ends" and and returns to the initial state so I can also
register a passkey afterwards. The mfa_method stash key remains on the
stash but is used in the hook nonetheless, so the passkey is incorrectly
recognized as a security key.

The mfa_method key is now deleted after successfully persisting the
credential/security_key. This should not have an effect on the login
flow because the mfa_creation subflow is the last subflow to be
executed. It also should not affect the registration flow, because the
hook is not applied in the registration flow (persistence of data is
all handled in the create_user hook).
* feat: add authenticator app management to profile
* feat: passkey counts as second factor
Renames MFA stash entry for indicating usage (login) method to make its
meaning more explicit. Also removes code persisting a webauthn credential
from the attestation verification action in the onboarding flow because
this is already done by a shared hook.
bjoern-m and others added 14 commits October 22, 2024 17:03
* chore: skip mfa prompt if the user only has a passkey

* chore: refactor and improve mfa onboarding

* fix: no mfa onboarding when passwords and passkeys are disabled

* fix: only show mfa onbooarding once

* feat: add a function to the flowpilot to check whether a state has been visited
* chore: improved error handling

* feat: add missing translations (#1681)
Do not suspend the `webauthn_verify_attestation_response` action when passkeys are disabled, but security keys and MFA are enabled.
Change texts regarding security creation to be more consistent across the flows and to be more precise.
* fix: loading spinner alignment corrected

* fix: auth app deletion link is shown while deletion is not allowed
* chore: remove deprecated test persister

* chore: replace test persister calls

* chore: add saml state fixtures
backend/config/config_mfa.go Show resolved Hide resolved
backend/dto/profile.go Outdated Show resolved Hide resolved
backend/flow_api/services/webauthn.go Outdated Show resolved Hide resolved
@FlxMgdnz FlxMgdnz merged commit bc04b72 into main Nov 1, 2024
8 checks passed
@FlxMgdnz FlxMgdnz deleted the feat-mfa branch November 1, 2024 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Recently closed
Development

Successfully merging this pull request may close these issues.

4 participants