[Browser MFA] Add Browser MFA to challenge request flow by danielashare · Pull Request #63936 · gravitational/teleport

danielashare · 2026-02-18T16:48:37Z

This PR adds Browser MFA fields to the /webapi/login/mfa/begin flow. 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 tsh
    participant proxy as Proxy
    participant auth as Auth

    tsh->>proxy: POST /webapi/mfa/login/begin<br/>w/ redirect /callback?secret_key=xxx
    proxy->>auth: Forward login request
    auth->>auth: Generate request_id<br/>Upsert SSOMFASession
    auth-->>proxy: MFA Challenge
    proxy-->>tsh: Return MFA Challenge

Manual tests:

Test new tsh client receives Browser MFA challenge
Confirm Teleport Web UI login page is unchanged
Test regular user/pass with MFA (YubiKey, OTP, TouchID) still works
Test passwordless still works (TouchID and YubiKey)

bl-nero · 2026-03-04T12:52:16Z

+	BrowserMFATSHRedirectURL string
+	ProxyAddress             string
+	Ext                      *mfav1.ChallengeExtensions
+	SIP                      *mfav1.SessionIdentifyingPayload


Just to clarify if I understand the purpose correctly: does this field have any purpose outside of per-session MFA?

Yes, this field is only for in-band per-session MFA

bl-nero · 2026-03-04T12:57:32Z

+	// If the user has a WebAuthn device and no SSO MFA configured, return a Browser
+	// MFA challenge. This challenge is useful in cases where a user only has a
+	// browser-associated WebAuthn device, but is trying to MFA via a CLI tool (tsh, tctl etc.)
+	if groupedDevs.Browser != nil && browserMFATSHRedirectURL != "" {


Is this feature unconditionally turned on? I'm asking because the previous conditional has enableSSO as a part of the condition.

It isn't unconditionally enabled, I checked if it was enabled inside this if statement because I didn't want to query the auth preferences if the MFA device or redirect URL wasn't set. However, I realise that authPrefs are checked further up and I can just reuse that response, so I've modified this to match the other MFA methods

bl-nero · 2026-03-04T13:05:39Z

+		return nil, trace.Wrap(err, InvalidClientRedirectErrorMessage)
+	}
+
+	proxyAddr := params.ProxyAddress


So ProxyAddress is expected to be filled by the proxy service, and it's not a user-provided value, right? Can you explain in which case this value will be empty and will need to be filled here?

The SSO MFA challenge request flow, which this is heavily inspired by, passes the proxy address and a redirect URL. Browser MFA just has the client send the redirect URL. So, unless some large changes are made to SSO MFA, the proxy address is always going to be present because it is passed as a param to the mfaAuthChallenge function that calls either BeginSSOMFAChallenge or BeginBrowserMFAChallenge. Just in case changes are made, I have a fallback here to get the proxy address. I'll add a comment to explain that.

bl-nero · 2026-03-04T13:42:27Z

 	// TargetCluster is the optional cluster where the authentication is targeted.
 	TargetCluster string `json:"target_cluster,omitempty"`
+	// TshRedirectURL is the redirect URL used to return a WebAuthn response back to tsh
+	TshRedirectURL string `json:"tsh_redirect_url,omitempty"`


This should be TSHRedirectURL to conform to the naming conventions. Also: I've seen this field written to, but I don't think I've seen it being actually used anywhere. Will it be used in a future PR?

Renamed that. Yeah, there are other PRs that will read this value

bl-nero · 2026-03-04T13:48:32Z

 			Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN,
 		}
+
+		if req.BrowserMFATSHRedirectURL != "" {


I'd argue that this conditional is unnecessary, as the URL will be initialized to an empty string anyway.

Makes it look cleaner 👍

bl-nero · 2026-03-04T13:50:49Z

 }

+// BrowserMFAChallenge contains browser challenge details.
+message BrowserMFAChallenge {


Will this message be used in a future PR? I don't see it used here.

Apologies, that commit snuck in to this branch, I've removed it now

Joerger

LGTM, just a small test request

Joerger · 2026-03-20T17:04:48Z

+			},
+		},
+		{
+			name:     "NOK SSO MFA user should not get Browser MFA",


Let's add another test case for an SSO MFA user w/ a webauthn device getting both challenges.

zmb3 · 2026-03-20T21:17:14Z

@@ -4459,7 +4444,7 @@ func (a *Server) CreateAuthenticateChallenge(ctx context.Context, req *proto.Cre
 		}
 	}

-	challenges, err := a.mfaAuthChallenge(ctx, username, req.SSOClientRedirectURL, req.ProxyAddress, challengeExtensions)
+	challenges, err := a.mfaAuthChallenge(ctx, username, req.SSOClientRedirectURL, req.BrowserMFATSHRedirectURL, req.ProxyAddress, challengeExtensions)


Yeah that's exactly what I was thinking.

I'm not sure if we'll end up liking the change or not. I guess it depends how big of a change it is and how clear the final code is, but give it a shot and see if you like it better with a single redirect URL.

zmb3 · 2026-03-20T21:20:48Z

+		Webauthn: &types.Webauthn{
+			RPID: "localhost",
+		},
+		AllowBrowserAuthentication: types.NewBoolOption(true),


Is this the new option that enables browser MFA?

If so, I think the name should mention "MFA" - we don't want people to think this is some sort of setting for disabling regular web UI login.

Yes it is, and I agree we should change the name. However, the change to types.proto has already been merged to master with this name. Is there any way we can change this field?

Renamed and in review here

backport-bot-workflows · 2026-03-26T12:47:01Z

@danielashare See the table below for backport results.

Branch	Result
branch/v18	Failed

danielashare self-assigned this Feb 18, 2026

danielashare requested review from Joerger and codingllama February 18, 2026 16:49

danielashare added no-changelog Indicates that a PR does not require a changelog entry backport/branch/v18 labels Feb 18, 2026

github-actions Bot requested review from kimlisa and okraport February 18, 2026 16:49

github-actions Bot added the size/lg label Feb 18, 2026

danielashare commented Feb 18, 2026

View reviewed changes

Comment thread lib/auth/auth.go Outdated

Comment thread lib/auth/sso_mfa.go Outdated

danielashare mentioned this pull request Feb 19, 2026

Browser MFA Feature #63987

Open

danielashare marked this pull request as draft February 20, 2026 14:15

danielashare force-pushed the danielashare/browser-mfa-challenge-request branch from f9d5208 to 9bd9b9e Compare February 23, 2026 15:52

danielashare changed the base branch from danielashare/browser-mfa-proto to danielashare/browser-mfa-proto-2 February 23, 2026 15:54

danielashare force-pushed the danielashare/browser-mfa-challenge-request branch 3 times, most recently from e8d3f57 to 66deea0 Compare February 23, 2026 17:40

danielashare marked this pull request as ready for review February 23, 2026 17:42

github-actions Bot requested review from kopiczko and smallinsky February 23, 2026 17:42

github-actions Bot added the size/md label Feb 23, 2026

danielashare removed request for kopiczko and smallinsky February 23, 2026 18:19

danielashare force-pushed the danielashare/browser-mfa-proto-2 branch from 68a5f0c to 7ac607e Compare March 2, 2026 11:08

danielashare force-pushed the danielashare/browser-mfa-challenge-request branch from 3e63a80 to 09a1356 Compare March 3, 2026 16:06

danielashare requested review from hugoShaka, r0mant, rosstimothy, russjones, tigrato and zmb3 as code owners March 3, 2026 16:06

bl-nero reviewed Mar 4, 2026

View reviewed changes

danielashare mentioned this pull request Mar 5, 2026

[Browser MFA] Add initial requests for browser MFA process to client tools #64301

Merged

8 tasks

danielashare force-pushed the danielashare/browser-mfa-challenge-request branch from adae9ae to 2ce5b66 Compare March 5, 2026 20:59

Joerger reviewed Mar 6, 2026

View reviewed changes

Base automatically changed from danielashare/browser-mfa-proto-2 to master March 6, 2026 07:23

danielashare force-pushed the danielashare/browser-mfa-challenge-request branch from acc34c1 to 655a559 Compare March 6, 2026 07:29

Joerger reviewed Mar 14, 2026

View reviewed changes

Comment thread lib/auth/auth.go Outdated

Comment thread lib/auth/browser_mfa.go Outdated

Comment thread lib/auth/browser_mfa.go Outdated

Comment thread lib/auth/sso_mfa.go Outdated

Comment thread e

danielashare force-pushed the danielashare/browser-mfa-challenge-request branch from 6e6e08b to 80601ac Compare March 20, 2026 16:02

Joerger reviewed Mar 20, 2026

View reviewed changes

zmb3 reviewed Mar 20, 2026

View reviewed changes

Joerger reviewed Mar 25, 2026

View reviewed changes

Comment thread lib/auth/auth.go

Comment thread lib/auth/browser_mfa.go Outdated

Joerger approved these changes Mar 25, 2026

View reviewed changes

zmb3 approved these changes Mar 25, 2026

View reviewed changes

public-teleport-github-review-bot Bot removed request for codingllama, kimlisa and okraport March 25, 2026 21:27

danielashare force-pushed the danielashare/browser-mfa-challenge-request branch from 6784a31 to 6143668 Compare March 26, 2026 10:57

[Browser MFA] Add Browser MFA to challenge request flow

c039f40

danielashare force-pushed the danielashare/browser-mfa-challenge-request branch from 6143668 to c039f40 Compare March 26, 2026 11:21

danielashare added this pull request to the merge queue Mar 26, 2026

Merged via the queue into master with commit a9fbf8b Mar 26, 2026
46 checks passed

danielashare deleted the danielashare/browser-mfa-challenge-request branch March 26, 2026 12:45

danielashare added a commit that referenced this pull request Mar 31, 2026

[Browser MFA] Add Browser MFA to challenge request flow (#63936)

1304a33

danielashare added a commit that referenced this pull request Apr 2, 2026

[Browser MFA] Add Browser MFA to challenge request flow (#63936)

dee5940

danielashare added a commit that referenced this pull request Apr 7, 2026

[Browser MFA] Add Browser MFA to challenge request flow (#63936)

bdcc603

danielashare added a commit that referenced this pull request Apr 17, 2026

[Browser MFA] Add Browser MFA to challenge request flow (#63936)

ed042e8

mmcallister pushed a commit that referenced this pull request Apr 28, 2026

[Browser MFA] Add Browser MFA to challenge request flow (#63936)

3100eff

danielashare added a commit that referenced this pull request Apr 30, 2026

[Browser MFA] Add Browser MFA to challenge request flow (#63936)

9ee4e1b

Conversation

danielashare commented Feb 18, 2026

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Joerger left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielashare Mar 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

backport-bot-workflows Bot commented Mar 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

danielashare Mar 21, 2026 •

edited

Loading