[client] Force interactive login when extending the auth session (#7216)

* [client] Force interactive login when extending the auth session

A session extend must be answered from the account the peer is registered
under. With a silent PKCE flow (DisablePromptLogin or max_age=0) the IdP
answers from whatever session it already holds, which need not be the
peer's account when several are signed in; the token then fails the
user match in ExtendAuthSession with no way to pick another account.

Mark the PKCE flow request as a session extend so the management server
can force prompt=login for it, overriding the configured silent flow.

* [client] Reduce cognitive complexity of Server.Login

Login sat at cognitive complexity 27, over the 25 the linter allows.

Extract the interactive SSO branch into startSSOLogin, and split the
nested in-flight-flow reuse check out of it into reuseOAuthFlow, which
flattens the original if/else into early returns: it returns the cached
auth info when the previous flow targets the same client and still has
more than 90s left, otherwise cancels the stale wait and returns nil so
the caller requests a fresh flow.

The helpers take the contextState through a small statusSetter
interface, since internal.contextState is unexported and re-deriving it
with CtxGetState inside the helper would resolve against callerCtx
rather than rootCtx.

No behavior change: same ordering of state transitions, same mutex scope
around the oauthAuthFlow write, same error paths. Login is now at 21.

* [client] Respect DisablePromptLogin when extending the auth session

Forcing prompt=login on a session extend overrode DisablePromptLogin, which
is set for IdPs that break on it: Authentik triggers a double authentication
and social logins fail outright. Overriding it there trades a recoverable
extend for a login that cannot complete at all.

Keep the LoginFlag override, which only replaces max_age=0 or none with
prompt=login so the IdP honours login_hint, and leave DisablePromptLogin as
configured. Those deployments keep the silent flow, and with several accounts
signed in an extend answered from the wrong one still fails the user match.

* [client] Guard the shared OAuth flow state with the server mutex

reuseOAuthFlow read flow, expiresAt, waitCancel and info without holding
s.mutex, while startSSOLogin and WaitSSOLogin write them under it. Reading the
fields one at a time could also answer with auth info from a flow that was
already replaced, or cancel a wait that no longer belongs to the flow just
judged stale. Take one snapshot under the lock and decide from it.

WaitSSOLogin read oauthAuthFlow.flow twice outside the lock; both now use a
value snapshotted in the critical section that already installs actCancel.

Its stale waitCancel was read and called in a separate section from the one
installing the new one, so two racing calls could read the same predecessor and
leave one wait uncancelled. Swap the two in a single critical section. Both
cancels run after unlocking: the displaced wait takes s.mutex as it unwinds.

* [client] Verify the SSO login came back for the hinted account

login_hint is a suggestion the IdP may ignore: with a silent flow configured
(DisablePromptLogin or max_age=0) and a live IdP session for another account,
the login completes with that account's token. On a registered peer the
management server rejects it as a user mismatch, but on a fresh profile the
peer silently registers under the wrong account and the profile is then bound
to it — every later login follows the stored hint straight back.

After the token exchange, compare the ID token's email against the hint the
flow was sent with. On a mismatch, do not log in to management with the token;
run one more round asking the IdP to re-decide the account (prompt=login, via
ForceAccountPrompt — DisablePromptLogin still wins there). If the prompted
round also comes back different, proceed with a warning: the address may
legitimately have changed, and refusing forever would lock the user out of the
profile while the management server still rejects a token that does not own
the peer. A token or profile with no email to compare is not judged.

The retry differs per platform because of who opens the browser:

- CLI (netbird login foreground) and Android run the whole flow in one
  process, so the mismatch retries automatically: the browser reopens with
  the account prompt within the same login attempt.
- On desktop the login is split between the daemon and the GUI: Login hands
  the authorize URL to the GUI, WaitSSOLogin blocks for the token, and only
  the GUI can open a browser. A new URL cannot be handed out from inside
  WaitSSOLogin (its response has no field for one, kept that way to avoid a
  proto change), so the daemon arms forceAccountPrompt, fails the round with
  "connect again to choose the account", and builds the next Login's flow
  with the prompt — the user's next connect is the retry.

The flag and the flow annotations live in daemon memory only; SwitchProfile
drops them so the previous profile's hint cannot judge the next profile's
token. The device code flow has no prompt parameter (RFC 8628), so a prompted
round there runs as-is and a repeated mismatch is let through with the
warning rather than looping.

* [client] Address review comments on PKCE session extend flow

Fail the PKCE authorization flow test on request error instead of
continuing into a nil dereference, and make the godoc comments on the
touched exported symbols identifier-leading full sentences.

* [client] Match accounts only on the email claim of the ID token

The name-claim fallback in the ID token parsing is kept for the login
hint and display, but account matching now only considers a value that
came from the email claim, so a token without one no longer produces a
false account mismatch.

* [client] Drop the pending session extend on a profile switch

The profile-switch cleanup dropped the pending login flow and the
account-prompt flag, but left extendAuthSessionFlow untouched. Its device
code was issued by the previous profile's IdP client, so a
WaitExtendAuthSession still parked on the browser leg would submit the
resulting token against the new profile's engine.

* [client] Judge the SSO account against the flow that produced the token

WaitSSOLogin snapshotted the flow on entry but re-read the info, hint and
accountPrompted from the live s.oauthAuthFlow afterwards, in separate
critical sections. WaitToken blocks for the whole browser leg, so a
concurrent Login or RequestJWTAuth could replace the flow meanwhile and
the mismatch check would compare this wait's token against another flow's
account: either arming the prompt spuriously or letting a wrong-account
token through against an unrelated profile's hint. Take all of it in the
entry snapshot.

* [client] Keep the forced account prompt from being lost to flow reuse

startSSOLogin consumed forceAccountPrompt and applied the prompt to the
freshly built flow, but reuseOAuthFlow could then answer from a cached
flow for the same client — one built without prompt=login, e.g. by
RequestJWTAuth. The user got the same silent authorization URL that
produced the mismatch, with the flag already spent, so no later round
asked either. Rule reuse out when the prompt is forced, while still
cancelling the predecessor's wait.

RequestJWTAuth also wrote the flow fields one by one, leaving the previous
login's hint and accountPrompted behind for WaitSSOLogin to judge a later
token against. Both sites now replace the whole record.

* [client] Consume the forced account prompt after the retry

forceAccountPrompt was never cleared, so a flow that outlived the retry it
was armed for kept sending prompt=login on every later authorization
request and re-authenticated the user each time. RequestAuthInfo now takes
the flag as it builds the request.

* [client] Cancel the caller context in the SSO login tests

WaitSSOLogin parks a goroutine on the caller's context for the whole
browser leg. The tests passed context.Background(), which never cancels,
so each left one goroutine behind for the lifetime of the test binary.

* [client] Cancel the wait displaced by an OAuth flow replacement

Replacing the shared record with a whole struct value dropped the previous
flow's waitCancel, so an SSO browser wait still parked on it lost its
cancel: nothing could preempt it, and it could go on to run attemptLogin
or mutate the record behind the new flow. Both replacement sites now take
the displaced cancel over in the same critical section, via a shared
replaceOAuthFlow, and invoke it after the unlock.

* [client] Guard OAuth flow mutations by the flow that owns the wait

* [client] Arm the account prompt only from the wait that owns the flow

* [client] Adopt the three-value parseEmailFromIDToken in the device flow

The main merge brought in the device flow's email extraction from #7193,
which still used the two-value signature this branch replaced when account
matching was narrowed to the email claim. Git merged the files without a
textual conflict, so the branch stopped compiling.

Take the fromEmailClaim result and fill EmailClaim from it, the same way the
PKCE path does, so device-flow clients get the same account matching.

* [client] Populate the pending extend flow in the test server helper

SwitchProfile cancels and clears the pending session extend flow
unconditionally, the same way it clears the SSH JWT cache. New always
populates the field, but the hand-assembled test server did not, so
TestSwitchProfile_ClearsJWTCache panicked on a nil PendingFlow.
This commit is contained in:
Zoltan Papp
2026-10-08 15:40:39 +02:00
committed by GitHub
parent 834b0f699c
commit 515a01dd11
23 changed files with 1591 additions and 802 deletions
+39 -2
View File
@@ -194,12 +194,49 @@ func (a *Auth) login(urlOpener URLOpener, isAndroidTV bool) error {
}
func (a *Auth) foregroundGetTokenInfo(authClient *auth.Auth, urlOpener URLOpener, isAndroidTV bool) (*auth.TokenInfo, error) {
oAuthFlow, err := authClient.GetOAuthFlow(a.ctx, isAndroidTV, profileLoginHint(a.cfgPath))
return a.foregroundGetTokenInfoFlow(authClient, urlOpener, isAndroidTV, false)
}
// foregroundGetTokenInfoFlow runs the interactive flow. sessionExtend tells the
// server the token will renew this peer's session rather than log a peer in, so
// it can rule out a silent authorization the IdP could answer from an unrelated
// account. See PKCEAuthorizationFlowRequest.
func (a *Auth) foregroundGetTokenInfoFlow(authClient *auth.Auth, urlOpener URLOpener, isAndroidTV bool, sessionExtend bool) (*auth.TokenInfo, error) {
hint := profileLoginHint(a.cfgPath)
oAuthFlow, err := authClient.GetOAuthFlow(a.ctx, isAndroidTV, sessionExtend, hint)
if err != nil {
return nil, fmt.Errorf("failed to get OAuth flow: %v", err)
}
return runOAuthFlow(a.ctx, oAuthFlow, urlOpener, nil)
tokenInfo, err := runOAuthFlow(a.ctx, oAuthFlow, urlOpener, nil)
if err != nil {
return nil, err
}
if tokenInfo.MatchesAccount(hint) {
return tokenInfo, nil
}
// The IdP answered from a session belonging to another account. Retrying is
// what makes this recoverable: on a peer already registered the server would
// reject the token, and on a fresh one it would silently register the peer
// under the wrong account and bind the profile to it.
log.Infof("login returned an account other than the one this profile is bound to, retrying with an account prompt")
retryFlow := auth.RetryFlowForAccount(oAuthFlow)
if retryFlow == nil {
return tokenInfo, nil
}
retryToken, err := runOAuthFlow(a.ctx, retryFlow, urlOpener, nil)
if err != nil {
return nil, err
}
if !retryToken.MatchesAccount(hint) {
log.Warnf("login still returned a different account after the prompt, continuing with it")
}
return retryToken, nil
}
// profileLoginHint returns the stored account email for the profile at cfgPath.
+6 -4
View File
@@ -222,11 +222,13 @@ func (c *Client) extendAuthSession(ctx context.Context, urlOpener URLOpener, isA
}
defer authClient.Close()
// Passing the config path makes the flow pick up the login_hint: an extend
// renews the session of the account already signed in, so it must not stop to
// offer a choice.
// Passing the config path makes the flow pick up the login_hint. That alone
// cannot keep the IdP on this profile's account though — a hint is only a
// suggestion, and a silent authorization is answered from whatever session the
// IdP already has, which need not be this peer's when several accounts are
// signed in. Marking the flow as an extend lets the server rule that out.
a := NewAuthWithConfig(ctx, cfg, cfgPath)
tokenInfo, err := a.foregroundGetTokenInfo(authClient, urlOpener, isAndroidTV)
tokenInfo, err := a.foregroundGetTokenInfoFlow(authClient, urlOpener, isAndroidTV, true)
if err != nil {
return fmt.Errorf("interactive sso login failed: %v", err)
}
+1 -1
View File
@@ -469,7 +469,7 @@ func (s *SSHClient) requestJWTToken(cfg *profilemanager.Config, cfgPath string)
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
defer cancel()
flow, err := auth.NewOAuthFlow(ctx, cfg, false, true, profileLoginHint(cfgPath))
flow, err := auth.NewOAuthFlow(ctx, cfg, false, true, profileLoginHint(cfgPath), false)
if err != nil {
return "", fmt.Errorf("create oauth flow: %w", err)
}
+34 -1
View File
@@ -425,11 +425,44 @@ func foregroundGetTokenInfo(ctx context.Context, cmd *cobra.Command, config *pro
hint = profileState.Email
}
oAuthFlow, err := auth.NewOAuthFlow(ctx, config, util.HasGraphicalSession(), false, hint)
oAuthFlow, err := auth.NewOAuthFlow(ctx, config, util.HasGraphicalSession(), false, hint, false)
if err != nil {
return nil, err
}
tokenInfo, err := runInteractiveFlow(cmd, oAuthFlow)
if err != nil {
return nil, err
}
if tokenInfo.MatchesAccount(hint) {
return tokenInfo, nil
}
// The IdP answered from a session belonging to another account. Retrying is
// what makes this recoverable: on a peer already registered the server would
// reject the token, and on a fresh one it would silently register the peer
// under the wrong account and bind the profile to it.
cmd.Println("The login returned a different account than this profile uses. Asking to sign in again.")
retryFlow := auth.RetryFlowForAccount(oAuthFlow)
if retryFlow == nil {
return tokenInfo, nil
}
retryToken, err := runInteractiveFlow(cmd, retryFlow)
if err != nil {
return nil, err
}
if !retryToken.MatchesAccount(hint) {
log.Warnf("login still returned a different account after the prompt, continuing with it")
}
return retryToken, nil
}
// runInteractiveFlow requests the authorization info, shows the URL to the user
// and blocks until the token comes back.
func runInteractiveFlow(cmd *cobra.Command, oAuthFlow auth.OAuthFlow) (*auth.TokenInfo, error) {
flowInfo, err := oAuthFlow.RequestAuthInfo(context.TODO())
if err != nil {
return nil, fmt.Errorf("getting a request OAuth flow info failed: %v", err)
+113
View File
@@ -0,0 +1,113 @@
package auth
import (
"encoding/base64"
"encoding/json"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestTokenInfoMatchesAccount(t *testing.T) {
tests := []struct {
name string
token TokenInfo
hint string
match bool
}{
{
name: "same account",
token: TokenInfo{EmailClaim: "user@example.com"},
hint: "user@example.com",
match: true,
},
{
name: "different account",
token: TokenInfo{EmailClaim: "other@example.com"},
hint: "user@example.com",
match: false,
},
{
name: "case differences are the same account",
token: TokenInfo{EmailClaim: "User@Example.com"},
hint: "user@example.com",
match: true,
},
{
name: "no hint leaves the choice to the IdP",
token: TokenInfo{EmailClaim: "other@example.com"},
hint: "",
match: true,
},
{
name: "token without an email claim is not judged",
token: TokenInfo{EmailClaim: ""},
hint: "user@example.com",
match: true,
},
{
name: "name fallback does not trigger matching",
token: TokenInfo{Email: "Some One"},
hint: "user@example.com",
match: true,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
assert.Equal(t, tc.match, tc.token.MatchesAccount(tc.hint))
})
}
}
func TestParseEmailFromIDToken(t *testing.T) {
tests := []struct {
name string
claims map[string]interface{}
wantValue string
wantFromEmail bool
wantErr bool
}{
{
name: "email claim",
claims: map[string]interface{}{"email": "user@example.com", "name": "Some One"},
wantValue: "user@example.com",
wantFromEmail: true,
},
{
name: "name fallback",
claims: map[string]interface{}{"name": "Some One"},
wantValue: "Some One",
},
{
name: "neither claim present",
claims: map[string]interface{}{"sub": "abc"},
wantErr: true,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
value, fromEmailClaim, err := parseEmailFromIDToken(idTokenWithClaims(t, tc.claims))
if tc.wantErr {
require.Error(t, err)
return
}
require.NoError(t, err)
assert.Equal(t, tc.wantValue, value)
assert.Equal(t, tc.wantFromEmail, fromEmailClaim)
})
}
}
func TestRetryFlowForAccountUnsupportedFlow(t *testing.T) {
assert.Nil(t, RetryFlowForAccount(&DeviceAuthorizationFlow{}))
}
func idTokenWithClaims(t *testing.T, claims map[string]interface{}) string {
t.Helper()
payload, err := json.Marshal(claims)
require.NoError(t, err)
return "header." + base64.RawURLEncoding.EncodeToString(payload) + ".signature"
}
+11 -7
View File
@@ -103,7 +103,7 @@ func (a *Auth) IsSSOSupported(ctx context.Context) (bool, error) {
err := a.withRetry(ctx, func(client *mgm.GrpcClient) error {
// Try PKCE flow first
_, err := a.getPKCEFlow(client)
_, err := a.getPKCEFlow(client, false)
if err == nil {
supportsSSO = true
return nil
@@ -136,9 +136,13 @@ func (a *Auth) IsSSOSupported(ctx context.Context) (bool, error) {
return supportsSSO, err
}
// GetOAuthFlow returns an OAuth flow (PKCE or Device) using the existing management connection
// This avoids creating a new connection to the management server
func (a *Auth) GetOAuthFlow(ctx context.Context, forceDeviceAuth bool, hint string) (OAuthFlow, error) {
// GetOAuthFlow returns an OAuth flow (PKCE or Device) using the existing management connection.
// This avoids creating a new connection to the management server.
//
// sessionExtend marks the flow as renewing an existing peer's session rather than
// logging one in; the server needs it to rule out a silent authorization that the
// IdP could answer from another account. See PKCEAuthorizationFlowRequest.
func (a *Auth) GetOAuthFlow(ctx context.Context, forceDeviceAuth bool, sessionExtend bool, hint string) (OAuthFlow, error) {
var flow OAuthFlow
err := a.withRetry(ctx, func(client *mgm.GrpcClient) error {
@@ -153,7 +157,7 @@ func (a *Auth) GetOAuthFlow(ctx context.Context, forceDeviceAuth bool, hint stri
}
// Try PKCE flow first
pkceFlow, err := a.getPKCEFlow(client)
pkceFlow, err := a.getPKCEFlow(client, sessionExtend)
if err != nil {
// If PKCE not supported, try Device flow
if s, ok := status.FromError(err); ok && (s.Code() == codes.NotFound || s.Code() == codes.Unimplemented) {
@@ -240,8 +244,8 @@ func (a *Auth) Login(ctx context.Context, setupKey string, jwtToken string) (err
}
// getPKCEFlow retrieves PKCE authorization flow configuration and creates a flow instance
func (a *Auth) getPKCEFlow(client *mgm.GrpcClient) (*PKCEAuthorizationFlow, error) {
protoFlow, err := client.GetPKCEAuthorizationFlow()
func (a *Auth) getPKCEFlow(client *mgm.GrpcClient, sessionExtend bool) (*PKCEAuthorizationFlow, error) {
protoFlow, err := client.GetPKCEAuthorizationFlow(sessionExtend)
if err != nil {
if s, ok := status.FromError(err); ok && s.Code() == codes.NotFound {
log.Warnf("server couldn't find pkce flow, contact admin: %v", err)
+4 -1
View File
@@ -308,10 +308,13 @@ func (d *DeviceAuthorizationFlow) WaitToken(ctx context.Context, info AuthFlowIn
// callers store to send back as the login_hint. Without it a client
// driven through the device flow — Android TV and tvOS — never binds
// an account to its profile and every later login goes out blind.
if email, err := parseEmailFromIDToken(tokenInfo.IDToken); err != nil {
if email, fromEmailClaim, err := parseEmailFromIDToken(tokenInfo.IDToken); err != nil {
log.Warnf("failed to parse email from ID token: %v", err)
} else {
tokenInfo.Email = email
if fromEmailClaim {
tokenInfo.EmailClaim = email
}
}
log.Infof("device flow: user authorization confirmed after %d polls in %s", polls, time.Since(start).Round(time.Second))
+54 -7
View File
@@ -5,6 +5,7 @@ import (
"fmt"
"net/http"
"runtime"
"strings"
log "github.com/sirupsen/logrus"
"google.golang.org/grpc/codes"
@@ -25,6 +26,14 @@ type HTTPClient interface {
Do(req *http.Request) (*http.Response, error)
}
// accountPromptForcer is implemented by the PKCE flow only. The device code
// flow has no equivalent: RFC 8628 defines no prompt parameter, and the user
// confirms the code on a page that shows which account signs in, so a silent
// wrong-account answer is not the failure mode there.
type accountPromptForcer interface {
ForceAccountPrompt()
}
// AuthFlowInfo holds information for the OAuth 2.0 authorization flow
type AuthFlowInfo struct { //nolint:revive
DeviceCode string `json:"device_code"`
@@ -49,6 +58,23 @@ type TokenInfo struct {
ExpiresIn int `json:"expires_in"`
UseIDToken bool `json:"-"`
Email string `json:"-"`
EmailClaim string `json:"-"`
}
// MatchesAccount reports whether the token belongs to the account a profile is
// bound to. A hint the IdP could not have acted on — no hint stored, or a token
// that carried no email claim — is reported as a match: the check exists to catch a
// login answered from the wrong account, not to block one it cannot judge.
//
// The comparison is case-insensitive. Local-parts are case-sensitive per RFC
// 5321, but no IdP in practice issues two accounts differing only in case, and
// an IdP that echoes a differently-cased address would otherwise fail every
// login.
func (t TokenInfo) MatchesAccount(hint string) bool {
if hint == "" || t.EmailClaim == "" {
return true
}
return strings.EqualFold(t.EmailClaim, hint)
}
// GetTokenToUse returns either the access or id token based on UseIDToken field
@@ -63,19 +89,22 @@ func shouldUseDeviceFlow(force bool, isUnixDesktopClient bool) bool {
return force || (runtime.GOOS == "linux" || runtime.GOOS == "freebsd") && !isUnixDesktopClient
}
// NewOAuthFlow initializes and returns the appropriate OAuth flow based on the management configuration
// NewOAuthFlow initializes and returns the appropriate OAuth flow based on the management configuration.
//
// It starts by initializing the PKCE.If this process fails, it resorts to the Device Code Flow,
// and if that also fails, the authentication process is deemed unsuccessful
//
// On Linux distros without desktop environment support, it only tries to initialize the Device Code Flow
// forceDeviceCodeFlow can be used to skip PKCE and go directly to Device Code Flow (e.g., for Android TV)
func NewOAuthFlow(ctx context.Context, config *profilemanager.Config, isUnixDesktopClient bool, forceDeviceCodeFlow bool, hint string) (OAuthFlow, error) {
// On Linux distros without desktop environment support, it only tries to initialize the Device Code Flow.
// forceDeviceCodeFlow can be used to skip PKCE and go directly to Device Code Flow (e.g., for Android TV).
//
// sessionExtend marks the flow as renewing an existing peer's session rather than
// logging one in. See PKCEAuthorizationFlowRequest for what the server makes of it.
func NewOAuthFlow(ctx context.Context, config *profilemanager.Config, isUnixDesktopClient bool, forceDeviceCodeFlow bool, hint string, sessionExtend bool) (OAuthFlow, error) {
if shouldUseDeviceFlow(forceDeviceCodeFlow, isUnixDesktopClient) {
return authenticateWithDeviceCodeFlow(ctx, config, hint)
}
pkceFlow, err := authenticateWithPKCEFlow(ctx, config, hint)
pkceFlow, err := authenticateWithPKCEFlow(ctx, config, hint, sessionExtend)
if err != nil {
log.Debugf("failed to initialize pkce authentication with error: %v\n", err)
log.Debug("falling back to device code flow")
@@ -85,14 +114,14 @@ func NewOAuthFlow(ctx context.Context, config *profilemanager.Config, isUnixDesk
}
// authenticateWithPKCEFlow initializes the Proof Key for Code Exchange flow auth flow
func authenticateWithPKCEFlow(ctx context.Context, config *profilemanager.Config, hint string) (OAuthFlow, error) {
func authenticateWithPKCEFlow(ctx context.Context, config *profilemanager.Config, hint string, sessionExtend bool) (OAuthFlow, error) {
authClient, err := NewAuth(ctx, config.PrivateKey, config.ManagementURL, config)
if err != nil {
return nil, fmt.Errorf("failed to create auth client: %v", err)
}
defer authClient.Close()
pkceFlowInfo, err := authClient.getPKCEFlow(authClient.client)
pkceFlowInfo, err := authClient.getPKCEFlow(authClient.client, sessionExtend)
if err != nil {
return nil, fmt.Errorf("getting pkce authorization flow info failed with error: %v", err)
}
@@ -129,3 +158,21 @@ func authenticateWithDeviceCodeFlow(ctx context.Context, config *profilemanager.
return deviceFlowInfo, nil
}
// RetryFlowForAccount returns a flow that asks the IdP to re-authenticate, for
// a login answered with an account other than the one hinted. Returns nil when
// the flow cannot ask — the caller then proceeds with the token it has.
//
// Proceeding rather than failing is deliberate. The hint is an email that may
// simply have changed since it was stored, and refusing the login would lock a
// user out of their own profile over a rename. The retry gives the account a
// chance to be corrected; the server still rejects a token that does not own
// the peer.
func RetryFlowForAccount(flow OAuthFlow) OAuthFlow {
forcer, ok := flow.(accountPromptForcer)
if !ok {
return nil
}
forcer.ForceAccountPrompt()
return flow
}
+46 -27
View File
@@ -87,10 +87,11 @@ func validatePKCEConfig(config *PKCEAuthProviderConfig) error {
// PKCEAuthorizationFlow implements the OAuthFlow interface for
// the Authorization Code Flow with PKCE.
type PKCEAuthorizationFlow struct {
providerConfig PKCEAuthProviderConfig
state string
codeVerifier string
oAuthConfig *oauth2.Config
providerConfig PKCEAuthProviderConfig
state string
codeVerifier string
oAuthConfig *oauth2.Config
forceAccountPrompt bool
}
// NewPKCEAuthorizationFlow returns new PKCE authorization code flow.
@@ -153,11 +154,16 @@ func (p *PKCEAuthorizationFlow) RequestAuthInfo(ctx context.Context) (AuthFlowIn
oauth2.SetAuthURLParam("code_challenge", codeChallenge),
oauth2.SetAuthURLParam("audience", p.providerConfig.Audience),
}
forceAccountPrompt := p.forceAccountPrompt
p.forceAccountPrompt = false
if !p.providerConfig.DisablePromptLogin {
switch p.providerConfig.LoginFlag {
case common.LoginFlagPromptLogin:
switch {
case forceAccountPrompt:
params = append(params, oauth2.SetAuthURLParam("prompt", "login"))
case common.LoginFlagMaxAge0:
case p.providerConfig.LoginFlag == common.LoginFlagPromptLogin:
params = append(params, oauth2.SetAuthURLParam("prompt", "login"))
case p.providerConfig.LoginFlag == common.LoginFlagMaxAge0:
params = append(params, oauth2.SetAuthURLParam("max_age", "0"))
}
}
@@ -178,6 +184,20 @@ func (p *PKCEAuthorizationFlow) SetLoginHint(hint string) {
p.providerConfig.LoginHint = hint
}
// ForceAccountPrompt makes the next authorization request ask the IdP to
// re-authenticate instead of answering from the session it already holds. Used
// to retry a login that came back for an account other than the one hinted.
//
// The next RequestAuthInfo consumes the flag, so a flow that outlives its retry
// goes back to the configured behaviour instead of re-authenticating forever.
//
// DisablePromptLogin still wins: it is set for IdPs that break on prompt=login,
// where retrying with it would replace a wrong-account login with one that
// cannot complete at all.
func (p *PKCEAuthorizationFlow) ForceAccountPrompt() {
p.forceAccountPrompt = true
}
// WaitToken waits for the OAuth token in the PKCE Authorization Flow.
// It starts an HTTP server to receive the OAuth token callback and waits for the token or an error.
// Once the token is received, it is converted to TokenInfo and validated before returning.
@@ -310,49 +330,48 @@ func (p *PKCEAuthorizationFlow) parseOAuthToken(token *oauth2.Token) (TokenInfo,
return TokenInfo{}, fmt.Errorf("authentication failed: invalid access token - %w", err)
}
email, err := parseEmailFromIDToken(tokenInfo.IDToken)
email, fromEmailClaim, err := parseEmailFromIDToken(tokenInfo.IDToken)
if err != nil {
log.Warnf("failed to parse email from ID token: %v", err)
} else {
tokenInfo.Email = email
if fromEmailClaim {
tokenInfo.EmailClaim = email
}
}
return tokenInfo, nil
}
// parseEmailFromIDToken extracts the email (or name) claim from an ID token
// without verifying its signature. The value is best-effort and used only as a
// UX convenience (login hint prefill and display); it never drives an
// authorization decision. The authoritative identity is established server-side
// from the signature-verified token.
func parseEmailFromIDToken(token string) (string, error) {
// without verifying its signature. The value is best-effort: it prefills the
// login hint and is displayed. Account matching (see MatchesAccount) only uses
// it when it came from the email claim, which fromEmailClaim reports. It never
// grants anything — the authoritative identity is established server-side from
// the signature-verified token.
func parseEmailFromIDToken(token string) (value string, fromEmailClaim bool, err error) {
parts := strings.Split(token, ".")
if len(parts) < 2 {
return "", fmt.Errorf("invalid token format")
return "", false, fmt.Errorf("invalid token format")
}
data, err := base64.RawURLEncoding.DecodeString(parts[1])
if err != nil {
return "", fmt.Errorf("failed to decode payload: %w", err)
return "", false, fmt.Errorf("failed to decode payload: %w", err)
}
var claims map[string]interface{}
if err := json.Unmarshal(data, &claims); err != nil {
return "", fmt.Errorf("json unmarshal error: %w", err)
return "", false, fmt.Errorf("json unmarshal error: %w", err)
}
var email string
if emailValue, ok := claims["email"].(string); ok {
email = emailValue
} else {
val, ok := claims["name"].(string)
if ok {
email = val
} else {
return "", fmt.Errorf("email or name field not found in token payload")
}
if email, ok := claims["email"].(string); ok {
return email, true, nil
}
if name, ok := claims["name"].(string); ok {
return name, false, nil
}
return email, nil
return "", false, fmt.Errorf("email or name field not found in token payload")
}
func createCodeChallenge(codeVerifier string) string {
+26
View File
@@ -76,6 +76,32 @@ func TestPromptLogin(t *testing.T) {
}
}
func TestForceAccountPromptAppliesOnlyToTheRetry(t *testing.T) {
config := PKCEAuthProviderConfig{
ClientID: "test-client-id",
Audience: "test-audience",
TokenEndpoint: "https://test-token-endpoint.com/token",
Scope: "openid email profile",
AuthorizationEndpoint: "https://test-auth-endpoint.com/authorize",
RedirectURLs: []string{"http://127.0.0.1:33992/"},
UseIDToken: true,
LoginFlag: mgm.LoginFlagNone,
}
pkce, err := NewPKCEAuthorizationFlow(config)
require.NoError(t, err)
pkce.ForceAccountPrompt()
retry, err := pkce.RequestAuthInfo(context.Background())
require.NoError(t, err)
require.Contains(t, retry.VerificationURIComplete, "prompt=login")
next, err := pkce.RequestAuthInfo(context.Background())
require.NoError(t, err)
require.NotContains(t, next.VerificationURIComplete, "prompt=login",
"the forced prompt outlived the retry it was armed for")
}
func TestIsPortInExcludedRange(t *testing.T) {
tests := []struct {
name string
+1 -1
View File
@@ -479,7 +479,7 @@ func (c *Client) LoginForMobile() string {
}
c.applyMDMOverlay(cfg)
oAuthFlow, err := auth.NewOAuthFlow(ctx, cfg, false, false, "")
oAuthFlow, err := auth.NewOAuthFlow(ctx, cfg, false, false, "", false)
if err != nil {
return err.Error()
}
+1 -1
View File
@@ -330,7 +330,7 @@ func profileLoginHint(cfgPath string) string {
const authInfoRequestTimeout = 30 * time.Second
func (a *Auth) foregroundGetTokenInfo(authClient *auth.Auth, urlOpener URLOpener, forceDeviceAuth bool) (*auth.TokenInfo, error) {
oAuthFlow, err := authClient.GetOAuthFlow(a.ctx, forceDeviceAuth, profileLoginHint(a.cfgPath))
oAuthFlow, err := authClient.GetOAuthFlow(a.ctx, forceDeviceAuth, false, profileLoginHint(a.cfgPath))
if err != nil {
return nil, fmt.Errorf("failed to get OAuth flow: %v", err)
}
+211
View File
@@ -0,0 +1,211 @@
package server
import (
"context"
"testing"
"time"
"github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/client/internal"
"github.com/netbirdio/netbird/client/internal/auth"
"github.com/netbirdio/netbird/client/proto"
)
type stubOAuthFlow struct {
token auth.TokenInfo
onWait func()
}
func (f *stubOAuthFlow) RequestAuthInfo(context.Context) (auth.AuthFlowInfo, error) {
return auth.AuthFlowInfo{}, nil
}
func (f *stubOAuthFlow) WaitToken(context.Context, auth.AuthFlowInfo) (auth.TokenInfo, error) {
if f.onWait != nil {
f.onWait()
}
return f.token, nil
}
func (f *stubOAuthFlow) GetClientID(context.Context) string {
return "stub-client"
}
func TestWaitSSOLogin_WrongAccountArmsPromptAndFails(t *testing.T) {
s := newSSOTestServer(t, "user@example.com", false, "other@example.com")
attempts := 0
s.loginAttemptFn = func(context.Context, string, string) (internal.StatusType, error) {
attempts++
return "", nil
}
resp, err := s.WaitSSOLogin(callerCtx(t), &proto.WaitSSOLoginRequest{UserCode: "code"})
require.Error(t, err)
require.Nil(t, resp)
require.Equal(t, 0, attempts, "the wrong account's token reached the management login")
require.True(t, s.forceAccountPrompt, "the next login was not armed to ask for the account")
require.Nil(t, s.oauthAuthFlow.flow, "the mismatched flow stayed cached for reuse")
status, stateErr := internal.CtxGetState(s.rootCtx).Status()
require.NoError(t, stateErr)
require.Equal(t, internal.StatusNeedsLogin, status, "the mismatch must stay retryable")
}
func TestWaitSSOLogin_WrongAccountAfterPromptProceeds(t *testing.T) {
s := newSSOTestServer(t, "user@example.com", true, "other@example.com")
attempts := 0
s.loginAttemptFn = func(context.Context, string, string) (internal.StatusType, error) {
attempts++
return "", nil
}
resp, err := s.WaitSSOLogin(callerCtx(t), &proto.WaitSSOLoginRequest{UserCode: "code"})
require.NoError(t, err, "a prompted round must not error again on a mismatch")
require.NotNil(t, resp)
require.Equal(t, "other@example.com", resp.Email)
require.Equal(t, 1, attempts)
require.False(t, s.forceAccountPrompt)
}
func TestWaitSSOLogin_MatchingAccountProceeds(t *testing.T) {
s := newSSOTestServer(t, "user@example.com", false, "User@Example.com")
attempts := 0
s.loginAttemptFn = func(context.Context, string, string) (internal.StatusType, error) {
attempts++
return "", nil
}
resp, err := s.WaitSSOLogin(callerCtx(t), &proto.WaitSSOLoginRequest{UserCode: "code"})
require.NoError(t, err)
require.NotNil(t, resp)
require.Equal(t, 1, attempts)
require.False(t, s.forceAccountPrompt)
}
func TestWaitSSOLogin_NoHintIsNotJudged(t *testing.T) {
s := newSSOTestServer(t, "", false, "whoever@example.com")
attempts := 0
s.loginAttemptFn = func(context.Context, string, string) (internal.StatusType, error) {
attempts++
return "", nil
}
_, err := s.WaitSSOLogin(callerCtx(t), &proto.WaitSSOLoginRequest{UserCode: "code"})
require.NoError(t, err)
require.Equal(t, 1, attempts)
require.False(t, s.forceAccountPrompt)
}
func TestSwitchProfile_DropsAccountPromptAndPendingFlow(t *testing.T) {
s, ctx, _, _, _ := setupServerWithProfile(t)
s.forceAccountPrompt = true
cancelled := false
s.oauthAuthFlow = oauthAuthFlow{
flow: &stubOAuthFlow{},
hint: "user@example.com",
waitCancel: func() { cancelled = true },
}
extendCancelled := false
s.extendAuthSessionFlow.Set(&stubOAuthFlow{}, auth.AuthFlowInfo{DeviceCode: "device"})
s.extendAuthSessionFlow.SetWaitCancel(func() { extendCancelled = true })
_, err := s.SwitchProfile(ctx, nil)
require.NoError(t, err)
require.False(t, s.forceAccountPrompt, "the prompt flag leaked across a profile switch")
require.Nil(t, s.oauthAuthFlow.flow, "the previous profile's flow leaked across a profile switch")
require.Empty(t, s.oauthAuthFlow.hint)
require.True(t, cancelled, "the pending wait was not cancelled")
require.True(t, extendCancelled, "the pending extend wait was not cancelled")
_, _, pending := s.extendAuthSessionFlow.Get()
require.False(t, pending, "the previous profile's extend flow leaked across a profile switch")
}
func TestWaitSSOLogin_JudgesTheFlowThatProducedTheToken(t *testing.T) {
s := newSSOTestServer(t, "user@example.com", false, "user@example.com")
attempts := 0
s.loginAttemptFn = func(context.Context, string, string) (internal.StatusType, error) {
attempts++
return "", nil
}
flow := s.oauthAuthFlow.flow.(*stubOAuthFlow)
flow.onWait = func() {
s.mutex.Lock()
defer s.mutex.Unlock()
s.oauthAuthFlow.hint = "someone-else@example.com"
}
resp, err := s.WaitSSOLogin(callerCtx(t), &proto.WaitSSOLoginRequest{UserCode: "code"})
require.NoError(t, err, "a flow replaced mid-wait must not decide this wait's verdict")
require.NotNil(t, resp)
require.Equal(t, 1, attempts)
require.False(t, s.forceAccountPrompt, "the prompt was armed off another flow's hint")
}
func TestReuseOAuthFlow_ForcedPromptRefusesTheCachedFlow(t *testing.T) {
s := New(internal.CtxInitState(context.Background()), "console", "", false, false, false, false)
cancelled := false
s.oauthAuthFlow = oauthAuthFlow{
flow: &stubOAuthFlow{},
info: auth.AuthFlowInfo{UserCode: "code"},
expiresAt: time.Now().Add(time.Hour),
waitCancel: func() { cancelled = true },
}
state := internal.CtxGetState(s.rootCtx)
resp := s.reuseOAuthFlow(context.Background(), &stubOAuthFlow{}, state, true)
require.Nil(t, resp, "a forced account prompt reused the flow that skipped it")
require.True(t, cancelled, "the predecessor wait was orphaned")
resp = s.reuseOAuthFlow(context.Background(), &stubOAuthFlow{}, state, false)
require.NotNil(t, resp, "an unforced login stopped reusing a live flow")
require.Equal(t, "code", resp.UserCode)
}
func TestReplaceOAuthFlow_CancelsTheDisplacedWait(t *testing.T) {
s := New(internal.CtxInitState(context.Background()), "console", "", false, false, false, false)
cancelled := false
s.oauthAuthFlow = oauthAuthFlow{
flow: &stubOAuthFlow{},
info: auth.AuthFlowInfo{UserCode: "code"},
expiresAt: time.Now().Add(time.Hour),
hint: "user@example.com",
accountPrompted: true,
waitCancel: func() { cancelled = true },
}
next := &stubOAuthFlow{}
s.replaceOAuthFlow(oauthAuthFlow{flow: next, info: auth.AuthFlowInfo{UserCode: "next"}})
require.True(t, cancelled, "the displaced wait was left without an owner")
require.Equal(t, next, s.oauthAuthFlow.flow)
require.Equal(t, "next", s.oauthAuthFlow.info.UserCode)
require.Empty(t, s.oauthAuthFlow.hint, "the previous flow's hint survived the replacement")
require.False(t, s.oauthAuthFlow.accountPrompted)
require.Nil(t, s.oauthAuthFlow.waitCancel, "the consumed cancel stayed on the record")
}
func newSSOTestServer(t *testing.T, hint string, accountPrompted bool, tokenEmail string) *Server {
t.Helper()
s := New(internal.CtxInitState(context.Background()), "console", "", false, false, false, false)
s.oauthAuthFlow = oauthAuthFlow{
flow: &stubOAuthFlow{token: auth.TokenInfo{Email: tokenEmail, EmailClaim: tokenEmail}},
info: auth.AuthFlowInfo{UserCode: "code"},
expiresAt: time.Now().Add(time.Minute),
hint: hint,
accountPrompted: accountPrompted,
}
return s
}
// callerCtx is the gRPC caller's context. WaitSSOLogin parks a goroutine on it
// for the whole browser leg, so a test that never cancels leaks one.
func callerCtx(t *testing.T) context.Context {
t.Helper()
ctx, cancel := context.WithCancel(context.Background())
t.Cleanup(cancel)
return ctx
}
+183 -57
View File
@@ -71,6 +71,10 @@ const (
var ErrServiceNotUp = errors.New("service is not up")
type statusSetter interface {
Set(update internal.StatusType)
}
// Server for service control.
type Server struct {
rootCtx context.Context
@@ -88,6 +92,12 @@ type Server struct {
uiLogPath string
oauthAuthFlow oauthAuthFlow
// forceAccountPrompt makes the next startSSOLogin build its flow with a
// forced account prompt. Armed when a login came back for an account other
// than the hinted one: that flow's browser is gone, so the correction has to
// ride on the user's next connect. Guarded by mutex; deliberately not
// persisted — a lost flag only costs one more mismatch round.
forceAccountPrompt bool
// extendAuthSessionFlow holds the pending PKCE flow created by
// RequestExtendAuthSession until WaitExtendAuthSession resolves it.
// Kept separate from oauthAuthFlow (which is reserved for the SSH
@@ -177,6 +187,14 @@ type oauthAuthFlow struct {
cacheGeneration uint64
waitCancel context.CancelFunc
// hint is the account the flow was asked to sign in (login_hint). The token
// that comes back is compared against it; empty means nothing to compare.
hint string
// accountPrompted records that this flow already asked the IdP to re-decide
// the account (or could not ask — the device flow has no way to). A token
// for the wrong account is then let through with a warning instead of
// erroring again, so the flow cannot loop.
accountPrompted bool
}
// New server instance constructor.
@@ -756,7 +774,7 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro
}
if msg.SetupKey == "" {
return s.beginSSOLogin(ctx, config, msg)
return s.startSSOLogin(ctx, msg, config, state)
}
// Setup-key path: we are about to dial Management with the key, so the
@@ -772,23 +790,31 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro
return &proto.LoginResponse{}, nil
}
// beginSSOLogin starts the browser leg of a login that carries no setup key and
// returns the response that parks the caller on it.
func (s *Server) beginSSOLogin(ctx context.Context, config *profilemanager.Config, msg *proto.LoginRequest) (*proto.LoginResponse, error) {
state := internal.CtxGetState(s.rootCtx)
// startSSOLogin opens the interactive leg of a login: it reuses the in-flight
// OAuth flow when one is still valid for the same client, and otherwise
// requests fresh auth info and parks the daemon on StatusNeedsLogin.
func (s *Server) startSSOLogin(ctx context.Context, msg *proto.LoginRequest, config *profilemanager.Config, state statusSetter) (*proto.LoginResponse, error) {
hint := ""
if msg.Hint != nil {
hint = *msg.Hint
}
oAuthFlow, err := auth.NewOAuthFlow(ctx, config, msg.IsUnixDesktopClient, false, hint)
oAuthFlow, err := auth.NewOAuthFlow(ctx, config, msg.IsUnixDesktopClient, false, hint, false)
if err != nil {
state.Set(internal.StatusLoginFailed)
return nil, err
}
if resp := s.pendingOAuthFlowResponse(ctx, oAuthFlow); resp != nil {
state.Set(internal.StatusNeedsLogin)
s.mutex.Lock()
promptForAccount := s.forceAccountPrompt
s.forceAccountPrompt = false
s.mutex.Unlock()
if promptForAccount && auth.RetryFlowForAccount(oAuthFlow) == nil {
// The device flow cannot ask; run it as-is. accountPrompted still goes
// true below so a second mismatch is let through instead of looping.
log.Warnf("the previous login returned a different account, but this flow cannot ask the IdP to choose one")
}
if resp := s.reuseOAuthFlow(ctx, oAuthFlow, state, promptForAccount); resp != nil {
return resp, nil
}
@@ -798,11 +824,13 @@ func (s *Server) beginSSOLogin(ctx context.Context, config *profilemanager.Confi
return nil, err
}
s.mutex.Lock()
s.oauthAuthFlow.flow = oAuthFlow
s.oauthAuthFlow.info = authInfo
s.oauthAuthFlow.expiresAt = time.Now().Add(time.Duration(authInfo.ExpiresIn) * time.Second)
s.mutex.Unlock()
s.replaceOAuthFlow(oauthAuthFlow{
flow: oAuthFlow,
info: authInfo,
expiresAt: time.Now().Add(time.Duration(authInfo.ExpiresIn) * time.Second),
hint: hint,
accountPrompted: promptForAccount,
})
state.Set(internal.StatusNeedsLogin)
@@ -814,32 +842,74 @@ func (s *Server) beginSSOLogin(ctx context.Context, config *profilemanager.Confi
}, nil
}
// pendingOAuthFlowResponse returns the in-flight flow's response when it
// targets the same IdP client and has enough time left for the user to finish
// the browser leg, so a second login joins the pending flow instead of opening
// a competing one. A flow too close to expiry has its waiter cancelled and nil
// returned, leaving the caller to start a fresh flow.
func (s *Server) pendingOAuthFlowResponse(ctx context.Context, oAuthFlow auth.OAuthFlow) *proto.LoginResponse {
if s.oauthAuthFlow.flow == nil || s.oauthAuthFlow.flow.GetClientID(ctx) != oAuthFlow.GetClientID(ctx) {
// replaceOAuthFlow installs next as the shared OAuth flow record and takes over
// the wait it displaces, so a WaitSSOLogin still parked on the old flow is not
// left without an owner: nothing would preempt it, and it could go on to run
// attemptLogin or mutate the record behind the new flow.
//
// The displaced cancel is read in the same critical section that replaces the
// record, so two callers racing here cannot both take the same predecessor. The
// cancel runs after the unlock — the displaced wait takes s.mutex as it unwinds.
func (s *Server) replaceOAuthFlow(next oauthAuthFlow) {
s.mutex.Lock()
staleCancel := s.oauthAuthFlow.waitCancel
s.oauthAuthFlow = next
s.mutex.Unlock()
if staleCancel != nil {
staleCancel()
}
}
func (s *Server) expireOAuthFlow(flow auth.OAuthFlow) {
s.mutex.Lock()
if s.oauthAuthFlow.flow == flow {
s.oauthAuthFlow.expiresAt = time.Now()
}
s.mutex.Unlock()
}
// reuseOAuthFlow returns the cached auth info when the previous flow targets
// the same client and still has enough life left, and otherwise cancels the
// stale wait and returns nil so the caller requests a fresh flow.
//
// promptForAccount rules reuse out: the cached flow was built without the
// account prompt, so handing its URL back would repeat the silent
// authorization that returned the wrong account — and with the flag already
// consumed, no later round would ask either. The predecessor's wait is still
// cancelled on the way out, so it is not orphaned on its device-code window.
//
// The whole decision runs off one snapshot taken under s.mutex: a concurrent
// WaitSSOLogin replaces waitCancel and expires the flow, so reading the fields
// one at a time could cancel a wait that no longer belongs to the flow just
// judged stale, or answer with auth info from a flow that was already replaced.
// The cancel itself is called after unlocking — it runs arbitrary teardown, and
// WaitSSOLogin takes s.mutex on the way out.
func (s *Server) reuseOAuthFlow(ctx context.Context, oAuthFlow auth.OAuthFlow, state statusSetter, promptForAccount bool) *proto.LoginResponse {
s.mutex.Lock()
current := s.oauthAuthFlow
s.mutex.Unlock()
if current.flow == nil || current.flow.GetClientID(ctx) != oAuthFlow.GetClientID(ctx) {
return nil
}
if s.oauthAuthFlow.expiresAt.After(time.Now().Add(90 * time.Second)) {
log.Debugf("using previous oauth flow info")
return &proto.LoginResponse{
NeedsSSOLogin: true,
VerificationURI: s.oauthAuthFlow.info.VerificationURI,
VerificationURIComplete: s.oauthAuthFlow.info.VerificationURIComplete,
UserCode: s.oauthAuthFlow.info.UserCode,
if promptForAccount || !current.expiresAt.After(time.Now().Add(90*time.Second)) {
log.Warnf("canceling previous waiting execution")
if current.waitCancel != nil {
current.waitCancel()
}
return nil
}
log.Warnf("canceling previous waiting execution")
if s.oauthAuthFlow.waitCancel != nil {
s.oauthAuthFlow.waitCancel()
log.Debugf("using previous oauth flow info")
state.Set(internal.StatusNeedsLogin)
return &proto.LoginResponse{
NeedsSSOLogin: true,
VerificationURI: current.info.VerificationURI,
VerificationURIComplete: current.info.VerificationURIComplete,
UserCode: current.info.UserCode,
}
return nil
}
// WaitSSOLogin validates the supplied userCode against the in-flight OAuth
@@ -912,9 +982,16 @@ func (s *Server) WaitSSOLogin(callerCtx context.Context, msg *proto.WaitSSOLogin
}
s.actCancel = cancel
// One snapshot of the flow this wait belongs to. hint and accountPrompted
// are judged against the token that comes back below, and WaitToken blocks
// for the whole browser leg: a concurrent Login or RequestJWTAuth replaces
// s.oauthAuthFlow meanwhile, so re-reading them after the wait would judge
// this flow's token against another flow's account.
pending := s.oauthAuthFlow
flow := pending.flow
s.mutex.Unlock()
if s.oauthAuthFlow.flow == nil {
if flow == nil {
return nil, gstatus.Errorf(codes.Internal, "oauth flow is not initialized")
}
@@ -932,31 +1009,36 @@ func (s *Server) WaitSSOLogin(callerCtx context.Context, msg *proto.WaitSSOLogin
// the affordance instead of a Connecting that never resolves.
state.Set(internal.StatusNeedsLogin)
s.mutex.Lock()
flowInfo := s.oauthAuthFlow.info
s.mutex.Unlock()
flowInfo := pending.info
if flowInfo.UserCode != msg.UserCode {
state.Set(internal.StatusLoginFailed)
return nil, gstatus.Errorf(codes.InvalidArgument, "sso user code is invalid")
}
if s.oauthAuthFlow.waitCancel != nil {
s.oauthAuthFlow.waitCancel()
}
waitCTX, cancel := context.WithCancel(ctx)
defer cancel()
// Swap in this wait's cancel and take over the one it displaces in a single
// critical section, so two WaitSSOLogin calls racing here cannot both read
// the same predecessor and leave one wait uncancelled. Cancelling happens
// after the unlock: the displaced wait takes s.mutex as it unwinds.
s.mutex.Lock()
if s.oauthAuthFlow.flow != flow {
s.mutex.Unlock()
return nil, gstatus.Errorf(codes.Canceled, "sso login was replaced by a newer login")
}
staleCancel := s.oauthAuthFlow.waitCancel
s.oauthAuthFlow.waitCancel = cancel
s.mutex.Unlock()
tokenInfo, err := s.oauthAuthFlow.flow.WaitToken(waitCTX, flowInfo)
if staleCancel != nil {
staleCancel()
}
tokenInfo, err := flow.WaitToken(waitCTX, flowInfo)
if err != nil {
s.mutex.Lock()
s.oauthAuthFlow.expiresAt = time.Now()
s.mutex.Unlock()
s.expireOAuthFlow(flow)
switch {
case errors.Is(err, context.Canceled):
// External abort. If our caller cancelled (the client closed
@@ -969,7 +1051,9 @@ func (s *Server) WaitSSOLogin(callerCtx context.Context, msg *proto.WaitSSOLogin
// the new owner — don't clobber it.
if callerCtx.Err() != nil {
s.mutex.Lock()
s.oauthAuthFlow = oauthAuthFlow{}
if s.oauthAuthFlow.flow == flow {
s.oauthAuthFlow = oauthAuthFlow{}
}
s.mutex.Unlock()
}
case errors.Is(err, context.DeadlineExceeded):
@@ -985,9 +1069,32 @@ func (s *Server) WaitSSOLogin(callerCtx context.Context, msg *proto.WaitSSOLogin
return nil, err
}
s.mutex.Lock()
s.oauthAuthFlow.expiresAt = time.Now()
s.mutex.Unlock()
s.expireOAuthFlow(flow)
if !tokenInfo.MatchesAccount(pending.hint) {
if !pending.accountPrompted {
// The IdP answered from a session belonging to another account. The
// browser for this flow is gone, so a new URL cannot be handed out
// here — arm the prompt for the user's next connect and fail this
// round. Never log in with the token: on a registered peer the
// server would reject it, and on a fresh one it would silently
// register the peer under the wrong account.
log.Warnf("login returned an account other than the one this profile is bound to; the next connect will ask the IdP to choose")
s.mutex.Lock()
if s.oauthAuthFlow.flow == flow {
s.oauthAuthFlow = oauthAuthFlow{}
s.forceAccountPrompt = true
}
s.mutex.Unlock()
state.Set(internal.StatusNeedsLogin)
return nil, gstatus.Errorf(codes.FailedPrecondition, "the login used a different account than this profile; connect again to choose the account")
}
// Already asked once; the account may legitimately differ (a changed
// email address). Refusing again would lock the user out of the profile,
// and the management server still rejects a token that does not own the
// peer.
log.Warnf("login still returned a different account after the prompt, continuing with it")
}
if loginStatus, err := s.attemptLogin(ctx, "", tokenInfo.GetTokenToUse()); err != nil {
state.Set(loginStatus)
@@ -1297,6 +1404,22 @@ func (s *Server) SwitchProfile(callerCtx context.Context, msg *proto.SwitchProfi
s.jwtCache.clear()
// A pending login flow and the account-prompt flag describe the previous
// profile's login; carried across a switch they would judge the new
// profile's token against the old profile's account. CancelFunc is
// non-blocking, so calling it under the mutex is safe.
if cancel := s.oauthAuthFlow.waitCancel; cancel != nil {
cancel()
}
s.oauthAuthFlow = oauthAuthFlow{}
s.forceAccountPrompt = false
// A pending session extend belongs to the previous profile too: its device
// code was issued by that profile's IdP client, and WaitExtendAuthSession
// would submit the resulting token against the new profile's engine.
s.extendAuthSessionFlow.CancelWait()
s.extendAuthSessionFlow.Clear()
if msg != nil && msg.ProfileName != nil {
s.publishProfileListChanged(*msg.ProfileName)
}
@@ -1940,7 +2063,7 @@ func (s *Server) RequestJWTAuth(
}
// the daemon has no graphical session of its own, only the caller can answer this
oAuthFlow, err := auth.NewOAuthFlow(ctx, config, msg.GetHasGraphicalSession(), false, hint)
oAuthFlow, err := auth.NewOAuthFlow(ctx, config, msg.GetHasGraphicalSession(), false, hint, false)
if err != nil {
return nil, gstatus.Errorf(codes.Internal, "failed to create OAuth flow: %v", err)
}
@@ -1950,12 +2073,15 @@ func (s *Server) RequestJWTAuth(
return nil, gstatus.Errorf(codes.Internal, "failed to request auth info: %v", err)
}
s.mutex.Lock()
s.oauthAuthFlow.flow = oAuthFlow
s.oauthAuthFlow.info = authInfo
s.oauthAuthFlow.expiresAt = time.Now().Add(time.Duration(authInfo.ExpiresIn) * time.Second)
s.oauthAuthFlow.cacheGeneration = cacheGeneration
s.mutex.Unlock()
// This flow carries no profile hint: leaving the previous login's hint and
// accountPrompted in place would have WaitSSOLogin judge a later token
// against them.
s.replaceOAuthFlow(oauthAuthFlow{
flow: oAuthFlow,
info: authInfo,
expiresAt: time.Now().Add(time.Duration(authInfo.ExpiresIn) * time.Second),
cacheGeneration: cacheGeneration,
})
return &proto.RequestJWTAuthResponse{
VerificationURI: authInfo.VerificationURI,
@@ -2055,7 +2181,7 @@ func (s *Server) RequestExtendAuthSession(
}
// the daemon has no graphical session of its own, only the caller can answer this
oAuthFlow, err := auth.NewOAuthFlow(ctx, config, msg.GetHasGraphicalSession(), false, hint)
oAuthFlow, err := auth.NewOAuthFlow(ctx, config, msg.GetHasGraphicalSession(), false, hint, true)
if err != nil {
return nil, gstatus.Errorf(codes.Internal, "failed to create OAuth flow: %v", err)
}
+3 -1
View File
@@ -10,6 +10,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/client/internal"
"github.com/netbirdio/netbird/client/internal/auth"
"github.com/netbirdio/netbird/client/internal/peer"
"github.com/netbirdio/netbird/client/proto"
)
@@ -21,7 +22,8 @@ func newTestServer() *Server {
// New always populates the SSH JWT cache and the logout and
// profile-switch paths call into it unconditionally, so a Server
// assembled field by field has to populate it too.
jwtCache: newJWTCache(),
jwtCache: newJWTCache(),
extendAuthSessionFlow: auth.NewPendingFlow(),
}
}