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

[v17] Add SSO MFA prompt for WebUI MFA flows #50529

Open
wants to merge 1 commit into
base: branch/v17
Choose a base branch
from

Conversation

Joerger
Copy link
Contributor

@Joerger Joerger commented Dec 20, 2024

Changelog: Add full SSO MFA support for the WebUI.

Backport #49794 to branch/v17

* Include sso channel ID in web mfa challenges.

* Handle SSO MFA challenges.

* Handle sso response in backend.

* Handle non-webauthn mfa response for file transfer, admin actions, and app session.

* Simplify useMfa with new helpers.

* Fix lint.

* Use AuthnDialog for file transfers; Fix json backend logic for file transfers.

* Make useMfa and AuthnDialog more reusable and error proof.

* Use AuthnDialog for App sessions.

* Resolve comments.

* Fix broken app launcher; improve mfaRequired logic in useMfa.

* Fix AuthnDialog test.

* Fix merge conflict with Db web access.

* fix stories.

* Refactor mfa required logic.

* Address bl-nero's comments.

* Address Ryan's comments.

* Add useMfa unit test.

* Fix story lint.

* Replace Promise.withResolvers for compatiblity with older browers; Fix bug where MFA couldn't be retried after a failed attempt; Add extra tests.
Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-50529.d212ksyjt6y4yg.amplifyapp.com

@@ -22,7 +22,7 @@ import { useParams } from 'react-router';
import useAttempt from 'shared/hooks/useAttemptNext';

import { ButtonState } from 'teleport/lib/tdp';
import { useMfa } from 'teleport/lib/useMfa';
import { useMfaTty } from 'teleport/lib/useMfa';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pretty sure this breaks per-session MFA for desktops: see #50557

Requesting changes and adding a do-not-merge label to prevent this from getting in a release.

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.

3 participants