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

Fix useMfa error handling #50844

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

Joerger
Copy link
Contributor

@Joerger Joerger commented Jan 7, 2025

The original error is now displayed in the MFA dialog, and the MFA canclled by user error is displayed at the top level after the MFA dialog is closed:

Screenshot 2025-01-07 at 12 22 32 PM
Screenshot 2025-01-07 at 12 23 37 PM

Fixes #50582

This will be backported with #50529

@Joerger Joerger added no-changelog Indicates that a PR does not require a changelog entry backport/branch/v17 labels Jan 7, 2025
@github-actions github-actions bot requested review from kimlisa and kiosion January 7, 2025 20:25
@Joerger Joerger requested a review from gzdunek January 7, 2025 20:26
@Joerger
Copy link
Contributor Author

Joerger commented Jan 7, 2025

@gzdunek This is the same issue you pointed out here. I just separated the fix 93ec6b7 into this new PR, in case #50373 is scrapped.

@zmb3
Copy link
Collaborator

zmb3 commented Jan 8, 2025

Can we use the American-English spelling (canceled) instead of the British-English spelling (cancelled)?

@Joerger Joerger force-pushed the joerger/fix-useMfa-error-state branch from 01d553d to dd33f6a Compare January 8, 2025 17:47
@Joerger Joerger force-pushed the joerger/fix-useMfa-error-state branch from dd33f6a to 6de6710 Compare January 8, 2025 20:27
@Joerger Joerger requested a review from gzdunek January 13, 2025 19:37
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 requested a review from gzdunek January 14, 2025 18:15
Comment on lines +239 to +241
// If the user cancels the MFA attempt and closes the dialog, the mfa status
// should be 'success', or else the dialog would remain open to display the error.
// This error is meant to be handled by the caller.
Copy link
Contributor

@gzdunek gzdunek Jan 15, 2025

Choose a reason for hiding this comment

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

This comment is no longer true (the part about a 'success' status).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/branch/v17 no-changelog Indicates that a PR does not require a changelog entry size/sm ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

web: canceling MFA dialog shows same error twice
3 participants