Skip to content

[Browser MFA] Add browser MFA path to MFA finish flow#64523

Merged
danielashare merged 1 commit into
masterfrom
danielashare/browser-mfa-login-finish
Mar 28, 2026
Merged

[Browser MFA] Add browser MFA path to MFA finish flow#64523
danielashare merged 1 commit into
masterfrom
danielashare/browser-mfa-login-finish

Conversation

@danielashare
Copy link
Copy Markdown
Contributor

@danielashare danielashare commented Mar 11, 2026

This PR adds the Browser MFA path to /webapi/mfa/login/finish to finish the authentication flow for tsh. The RFD for this feature can be found here. Tracking issue here.

These changes address this part of the flow from the above RFD:

sequenceDiagram
    participant tsh
    participant browser as Browser
    participant proxy as Proxy
    participant auth as Auth

    tsh->>proxy: POST /webapi/mfa/login/finish
    proxy->>auth: AuthenticateSSHUser
    auth->>auth: ValidateMFAAuthResponse()
    auth->>auth: Generate SSH certificates
    auth-->>proxy: SSH Login Response
    proxy-->>tsh: Return certificates
Loading

Manual Test Plan

Test Environment

Running branch locally that has this feature with PoC code to enable starting the Browser MFA flow. Per-session being tested by connecting back to my Mac through Teleport.

Test Cases

  • Webauthn unaffected via browser and tsh
  • OTP unaffected via browser and tsh
  • Browser MFA works for login
  • Browser MFA works for per-session MFA
  • Audit event mfa_via_browser logged for Browser MFA

@danielashare danielashare self-assigned this Mar 11, 2026
@danielashare danielashare added no-changelog Indicates that a PR does not require a changelog entry backport/branch/v18 labels Mar 11, 2026
@github-actions github-actions Bot requested review from rudream and ryanclark March 11, 2026 16:54
Comment thread lib/auth/auth.go
auditEvent.Code = events.ValidateMFAAuthResponseCode
auditEvent.Success = true
deviceMetadata := mfaDeviceEventMetadata(authData.Device)
deviceMetadata.MFAViaBrowser = authData.MFAViaBrowser
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.

I don't love that I set the MFAViaBrowser field here. But, to add this field to the event via mfaDeviceEventMetadata I would have to add proto to MFADevice that isn't really related to an MFA device, or I could change mfaDeviceEventMetadata to accept authData and change everywhere that uses this function. Any strong opinions on this?

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e3c7c2746d

ℹ️ 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".

Comment thread lib/auth/browser_mfa.go
Comment thread lib/auth/methods.go
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8ce7262614

ℹ️ 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".

Comment thread lib/auth/browser_mfa.go
Comment thread lib/auth/browser_mfa.go
@danielashare danielashare force-pushed the danielashare/browser-mfa-login-finish branch from 8ce7262 to 84589cd Compare March 13, 2026 21:17
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7dabf4b777

ℹ️ 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".

Comment thread lib/auth/browser_mfa.go
@tigrato tigrato self-requested a review March 16, 2026 16:26
@danielashare danielashare force-pushed the danielashare/browser-mfa-client-mfa-receive branch 2 times, most recently from 8d6f4eb to d8311c3 Compare March 23, 2026 15:06
@danielashare danielashare force-pushed the danielashare/browser-mfa-login-finish branch from 7dabf4b to c9a119c Compare March 23, 2026 16:50
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c9a119cc7c

ℹ️ 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".

Comment thread lib/auth/browser_mfa.go
Comment thread lib/auth/browser_mfa.go
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b33fdd2ee3

ℹ️ 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".

Comment thread lib/auth/auth.go
Comment thread api/proto/teleport/legacy/types/events/events.proto
Comment thread lib/auth/browser_mfa.go Outdated
Comment thread lib/auth/browser_mfa.go Outdated
Comment thread lib/auth/browser_mfa.go Outdated
Comment thread lib/services/local/users.go
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4fa72ee04b

ℹ️ 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".

Comment thread lib/auth/browser_mfa.go Outdated
@danielashare danielashare force-pushed the danielashare/browser-mfa-client-mfa-receive branch from f3b8de7 to f540b6f Compare March 25, 2026 15:24
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f9fa43310b

ℹ️ 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".

Comment thread lib/client/sso/ceremony.go Outdated
@danielashare danielashare force-pushed the danielashare/browser-mfa-client-mfa-receive branch 2 times, most recently from 8fc32c9 to c9fe8ce Compare March 26, 2026 22:04
Base automatically changed from danielashare/browser-mfa-client-mfa-receive to master March 27, 2026 07:59
@danielashare danielashare force-pushed the danielashare/browser-mfa-login-finish branch from f9fa433 to bdcba07 Compare March 27, 2026 12:28
Comment thread lib/auth/browser_mfa.go
}

// Verify this is a Browser MFA session and not an SSO MFA session.
if mfaSess.TSHRedirectURL == "" || mfaSess.ConnectorType != constants.BrowserMFA {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we add this same check in VerifySSOMFASession?

Comment thread e
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

accidental e ref update?

@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from rudream March 27, 2026 22:03
@danielashare danielashare force-pushed the danielashare/browser-mfa-login-finish branch from 5ff3627 to a12fb73 Compare March 27, 2026 22:28
@danielashare danielashare added this pull request to the merge queue Mar 28, 2026
Merged via the queue into master with commit 8812b53 Mar 28, 2026
45 checks passed
@danielashare danielashare deleted the danielashare/browser-mfa-login-finish branch March 28, 2026 11:58
@backport-bot-workflows
Copy link
Copy Markdown
Contributor

@danielashare See the table below for backport results.

Branch Result
branch/v18 Failed

danielashare added a commit that referenced this pull request May 6, 2026
[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
ivan-bax pushed a commit to ivan-bax/teleport that referenced this pull request May 22, 2026
[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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/branch/v18 no-changelog Indicates that a PR does not require a changelog entry size/md

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants