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

Add SSO MFA prompt for WebUI MFA flows #49794

Merged
merged 20 commits into from
Dec 20, 2024
Merged

Add SSO MFA prompt for WebUI MFA flows #49794

merged 20 commits into from
Dec 20, 2024

Conversation

Joerger
Copy link
Contributor

@Joerger Joerger commented Dec 4, 2024

Changelog: Add full SSO MFA support for the WebUI.

Extends SSO MFA support for:

Note: per-session SSO MFA for Node, Kube, and Desktop was already supported.

Depends on #49680

@Joerger Joerger requested a review from avatus December 4, 2024 21:19
@Joerger Joerger mentioned this pull request Dec 4, 2024
@Joerger Joerger force-pushed the joerger/refactor-reauthenticate branch 4 times, most recently from 4abf873 to 42ba430 Compare December 6, 2024 00:11
@Joerger Joerger force-pushed the joerger/sso-mfa-method branch 2 times, most recently from 5911b85 to 672dddb Compare December 9, 2024 22:11
@Joerger Joerger force-pushed the joerger/refactor-reauthenticate branch from ea8e8eb to 40336ed Compare December 9, 2024 22:26
@Joerger Joerger force-pushed the joerger/refactor-reauthenticate branch 2 times, most recently from af41a4e to bdd69ca Compare December 10, 2024 01:04
@Joerger Joerger force-pushed the joerger/sso-mfa-method branch from 672dddb to 9ad6250 Compare December 10, 2024 01:18
@Joerger Joerger force-pushed the joerger/refactor-reauthenticate branch from e60ec3d to 428e745 Compare December 11, 2024 02:41
@Joerger Joerger force-pushed the joerger/sso-mfa-method branch from 9ad6250 to 213322c Compare December 11, 2024 02:42
@Joerger Joerger marked this pull request as ready for review December 11, 2024 02:42
@github-actions github-actions bot requested a review from kiosion December 11, 2024 02:43
@avatus avatus requested a review from bl-nero December 12, 2024 17:06
@Joerger Joerger force-pushed the joerger/refactor-reauthenticate branch 2 times, most recently from 8e2fb35 to 9e8ad10 Compare December 14, 2024 01:18
@Joerger Joerger force-pushed the joerger/sso-mfa-method branch from 213322c to fb8c772 Compare December 14, 2024 02:16
lib/client/weblogin.go Outdated Show resolved Hide resolved
lib/client/weblogin.go Outdated Show resolved Hide resolved
lib/web/mfa.go Outdated Show resolved Hide resolved
submitAttempt={ft.submitMfaAttempt}
onCancel={ft.clearMfaChallenge}
/>
)}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a scenario where we can get an mfaChallenge on both mfa and ft?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, the mfa authndialog will always be handled and complete before we get to the ft authndialog. In the new updated code it would be easy to reuse the same mfa state if you're concerned about it though.

web/packages/teleport/src/lib/useMfa.ts Outdated Show resolved Hide resolved
web/packages/teleport/src/lib/useMfa.ts Outdated Show resolved Hide resolved
web/packages/teleport/src/lib/useMfa.ts Outdated Show resolved Hide resolved
@Joerger Joerger force-pushed the joerger/sso-mfa-method branch from 4182ae3 to e3c7edf Compare December 16, 2024 22:12
@Joerger Joerger force-pushed the joerger/refactor-reauthenticate branch from 5d0f08d to 89e4639 Compare December 16, 2024 22:17
@Joerger Joerger force-pushed the joerger/sso-mfa-method branch from e3c7edf to ff21791 Compare December 16, 2024 22:33
@Joerger Joerger requested review from bl-nero and ryanclark December 16, 2024 22:39
@Joerger Joerger force-pushed the joerger/sso-mfa-method branch from ff21791 to 99729ea Compare December 17, 2024 01:49
@Joerger Joerger force-pushed the joerger/sso-mfa-method branch from 5ee3d03 to e103a1f Compare December 19, 2024 20:28
@Joerger Joerger added this pull request to the merge queue Dec 20, 2024
Merged via the queue into master with commit 7269273 Dec 20, 2024
44 checks passed
@Joerger Joerger deleted the joerger/sso-mfa-method branch December 20, 2024 04:10
@public-teleport-github-review-bot

@Joerger See the table below for backport results.

Branch Result
branch/v17 Failed

Joerger added a commit that referenced this pull request Dec 20, 2024
* 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.
@@ -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.

@Joerger why is desktop session calling useMfaTty? There is no TTY for desktop sessions - this is SSH-only code, is it not?

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.

4 participants