Skip to content

Make Web Add User prompt for MFA just once#37067

Merged
Joerger merged 8 commits intomasterfrom
joerger/reuse-mfa-web-add-user
Jan 24, 2024
Merged

Make Web Add User prompt for MFA just once#37067
Joerger merged 8 commits intomasterfrom
joerger/reuse-mfa-web-add-user

Conversation

@Joerger
Copy link
Copy Markdown
Contributor

@Joerger Joerger commented Jan 23, 2024

Similar to #36997, this PR updates adding a user from the WebUI to perform the admin action MFA ceremony upfront, allowing reuse so CreateUser and CreateResetPasswordToken only require one prompt.

This PR is also a pre-req for other similar PRs since it adds the webauthnResponse argument to post/put/delete requests.

@Joerger Joerger added no-changelog Indicates that a PR does not require a changelog entry backport/branch/v15 labels Jan 23, 2024
@github-actions github-actions Bot requested a review from rudream January 23, 2024 01:33
@github-actions github-actions Bot requested a review from ryanclark January 23, 2024 01:33
@Joerger Joerger force-pushed the joerger/reuse-mfa-web-add-user branch from a6ed028 to 9a057f0 Compare January 23, 2024 01:38
@Joerger Joerger force-pushed the joerger/reuse-mfa-web-add-user branch from 9a057f0 to 9055d54 Compare January 23, 2024 01:55
Comment thread web/packages/teleport/src/services/auth/auth.ts Outdated
Comment thread web/packages/teleport/src/Users/useUsers.ts Outdated
@Joerger Joerger requested a review from ryanclark January 23, 2024 20:26
@Joerger
Copy link
Copy Markdown
Contributor Author

Joerger commented Jan 24, 2024

@rudream @ryanclark Sorry for the short timeline but could you please review today or tomorrow? I have multiple fixes depending on this change that ought to get into v15.

Comment thread web/packages/teleport/src/config.ts Outdated
Comment thread web/packages/teleport/src/services/auth/auth.ts Outdated
} catch (err) {
if (
err?.response?.status === 400 &&
err?.message.includes('missing target for MFA check')
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I would put a comment on the Go side where this error is defined that says that the UI depends on this not changing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The UI only depends on this for old versions, new versions will know how to handle the IsMfaRequiredAdminAction request.

Comment thread web/packages/teleport/src/services/auth/auth.ts Outdated
@Joerger Joerger force-pushed the joerger/reuse-mfa-web-add-user branch from cdd6f40 to 808e339 Compare January 24, 2024 20:22
@Joerger Joerger enabled auto-merge January 24, 2024 20:23
@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from rudream January 24, 2024 20:23
@Joerger Joerger added this pull request to the merge queue Jan 24, 2024
Merged via the queue into master with commit 20a0b93 Jan 24, 2024
@Joerger Joerger deleted the joerger/reuse-mfa-web-add-user branch January 24, 2024 20:49
@public-teleport-github-review-bot
Copy link
Copy Markdown

@Joerger See the table below for backport results.

Branch Result
branch/v15 Failed

Joerger added a commit that referenced this pull request Jan 24, 2024
* Allow webauthnResponse to be passed into API requests; Prompt for MFA before creating a user in the WebUI.

* Skip MFA check if webauthn is not configured.

* Add getWebauthnResponseForAdminAction helper function.

* Remove unused admin action name.

* Fix lint.

* Check for specific error.

* Address comments.

* Relax admin action mfa enforcement expectation, in line with #37136.
github-merge-queue Bot pushed a commit that referenced this pull request Jan 24, 2024
* Allow webauthnResponse to be passed into API requests; Prompt for MFA before creating a user in the WebUI.

* Skip MFA check if webauthn is not configured.

* Add getWebauthnResponseForAdminAction helper function.

* Remove unused admin action name.

* Fix lint.

* Check for specific error.

* Address comments.

* Relax admin action mfa enforcement expectation, in line with #37136.
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 ui

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"Access denied" when inviting users to Teleport v15 Cloud instance

4 participants