[Browser MFA] Add BrowserMFARequestID to CreateAuthenticateChallenge#63945
Conversation
0cb377a to
8a5561b
Compare
3d30faf to
e9e925e
Compare
nicholasmarais1158
left a comment
There was a problem hiding this comment.
I didn't manual test, but code looks good.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3dd7938fa2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
codingllama
left a comment
There was a problem hiding this comment.
Please resolve merge conflicts and address the codex review.
|
|
||
| // Replace the challenge extensions with the ones found in the SSO MFA object. | ||
| // These are the ones from the original tsh request. | ||
| challengeExtensions = mfatypes.ChallengeExtensionsToProto(chalExts) |
There was a problem hiding this comment.
Should we make sure that SSO MFA sessions record all ChallengeExtensions fields as well? It appears we do not record UserVerificationRequirement:
Lines 158 to 161 in 6382ef5
OK if you want to follow up separately.
There was a problem hiding this comment.
Yes, I'll include this in an upstream PR, thanks
3dd7938 to
d0542e9
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0542e9d88
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@danielashare See the table below for backport results.
|
[Browser MFA] Add protobuf and config (#63831) [Browser MFA] Add proto for Browser MFA feature (#64048) [Browser MFA] Add CompleteBrowserMFAChallenge gRPC (#63873) [Browser MFA] Rename browser mfa config name (#64980) [Browser MFA] Add BrowserMFARequestID to CreateAuthenticateChallenge (#63945) [Browser MFA] Add Browser MFA to challenge request flow (#63936) [Browser MFA] Add initial requests for browser MFA process to client tools (#64301) [Browser MFA] Add tsh callback handling for webauthn response (#64461) [Browser MFA] Add Browser MFA to presence checks (#65052) [Browser MFA] Add browser MFA path to MFA finish flow (#64523) [Browser MFA] Add Browser MFA to Connect (#64887) [Browser MFA] Add Browser MFA UI (#64692) [Browser MFA] Fix formatting in moderated sessions (#65236) [Browser MFA] Add Browser MFA ceremony tests
[Browser MFA] Add protobuf and config (gravitational#63831) [Browser MFA] Add proto for Browser MFA feature (gravitational#64048) [Browser MFA] Add CompleteBrowserMFAChallenge gRPC (gravitational#63873) [Browser MFA] Rename browser mfa config name (gravitational#64980) [Browser MFA] Add BrowserMFARequestID to CreateAuthenticateChallenge (gravitational#63945) [Browser MFA] Add Browser MFA to challenge request flow (gravitational#63936) [Browser MFA] Add initial requests for browser MFA process to client tools (gravitational#64301) [Browser MFA] Add tsh callback handling for webauthn response (gravitational#64461) [Browser MFA] Add Browser MFA to presence checks (gravitational#65052) [Browser MFA] Add browser MFA path to MFA finish flow (gravitational#64523) [Browser MFA] Add Browser MFA to Connect (gravitational#64887) [Browser MFA] Add Browser MFA UI (gravitational#64692) [Browser MFA] Fix formatting in moderated sessions (gravitational#65236) [Browser MFA] Add Browser MFA ceremony tests
This PR adds the
BrowserMFARequestIDfield to thePOST /webapi/mfa/authenticatechallengecall so that the correct challenge extensions can be set when requesting an MFA challenge through the browser. The RFD for this addition can be found here. Needs to be merged after #63831.These changes address this part of the flow from the above RFD:
sequenceDiagram participant browser as Browser participant proxy as Proxy participant auth as Auth browser->>proxy: POST /webapi/mfa/authenticatechallenge<br/>w/ browser_mfa_request_id proxy->>auth: rpc CreateAuthenticateChallenge<br/>w/ browser_mfa_request_id auth->>auth: Lookup SSOMFASession<br/>Get challenge extensions auth->>proxy: MFA Challenge proxy->>browser: MFA ChallengeManual tests: