Skip to content

MFA for Admin Actions: CreateResetPasswordToken#35465

Merged
Joerger merged 3 commits intomasterfrom
joerger/admin-actions-password-token
Dec 15, 2023
Merged

MFA for Admin Actions: CreateResetPasswordToken#35465
Joerger merged 3 commits intomasterfrom
joerger/admin-actions-password-token

Conversation

@Joerger
Copy link
Copy Markdown
Contributor

@Joerger Joerger commented Dec 7, 2023

Require MFA for CreateResetPasswordToken.

Part of RFD 131.

Based off #35386 to use the same test helpers.

@Joerger Joerger added the no-changelog Indicates that a PR does not require a changelog entry label Dec 7, 2023
@github-actions github-actions Bot added size/sm tctl tctl - Teleport admin tool labels Dec 7, 2023
@Joerger Joerger force-pushed the joerger/admin-actions-users branch 2 times, most recently from b9a1850 to d7b206d Compare December 8, 2023 03:06
@Joerger Joerger force-pushed the joerger/admin-actions-password-token branch 2 times, most recently from 29f6ced to b873854 Compare December 8, 2023 03:49
Copy link
Copy Markdown
Contributor

@kimlisa kimlisa left a comment

Choose a reason for hiding this comment

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

i was trying to test this in the web UI and i get this, am i missing something? (also i had otp setup)

image

@Joerger
Copy link
Copy Markdown
Contributor Author

Joerger commented Dec 8, 2023

@kimlisa It's expected that OTP wouldn't work on the WebUI. This is something I hope to add after the initial v15 release if it can be prioritized.

@Joerger Joerger requested a review from kimlisa December 8, 2023 17:26
@kimlisa
Copy link
Copy Markdown
Contributor

kimlisa commented Dec 9, 2023

@kimlisa It's expected that OTP wouldn't work on the WebUI. This is something I hope to add after the initial v15 release if it can be prioritized.

hrm, i think we should render an actionable message that the user can take to fix this problem, like telling them that this action requires re-auth and OTP is not supported so they should add a hardware key (and maybe link them to the https://localhost:3080/web/account/twofactor)? idk about CLI, but in the web UI we can tell what kind of second factor the cluster has, so even before the user performs the action, we can tell them it won't work (eg: clusters that only have otp enabled?)

@Joerger
Copy link
Copy Markdown
Contributor Author

Joerger commented Dec 11, 2023

@kimlisa I added an actionable error message. If you think we should link them to their account page directly could you share a commit/diff to make that work?

Also note that we are reducing the scope to only clusters that support webauthn, so TOTP only clusters will avoid this MFA flow entirely.

@Joerger Joerger force-pushed the joerger/admin-actions-users branch 2 times, most recently from 16de23f to 8bccc53 Compare December 12, 2023 00:54
@Joerger Joerger force-pushed the joerger/admin-actions-password-token branch 4 times, most recently from c82806f to 7d2b847 Compare December 12, 2023 03:16
Copy link
Copy Markdown
Contributor

@kimlisa kimlisa left a comment

Choose a reason for hiding this comment

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

this is much better, thanks 👍

@Joerger Joerger force-pushed the joerger/admin-actions-users branch from 3884f1f to 9ca0d37 Compare December 12, 2023 18:28
Base automatically changed from joerger/admin-actions-users to master December 14, 2023 21:59
@Joerger Joerger force-pushed the joerger/admin-actions-password-token branch from 7d2b847 to 764c3d2 Compare December 15, 2023 18:35
@Joerger Joerger enabled auto-merge December 15, 2023 18:35
@Joerger Joerger added this pull request to the merge queue Dec 15, 2023
Merged via the queue into master with commit ddddf8f Dec 15, 2023
@Joerger Joerger deleted the joerger/admin-actions-password-token branch December 15, 2023 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog Indicates that a PR does not require a changelog entry size/sm tctl tctl - Teleport admin tool

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants