mirror of
https://github.com/netbirdio/netbird.git
synced 2026-08-30 19:41:30 +02:00
[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.
This commit is contained in:
125
client/server/login_account_test.go
Normal file
125
client/server/login_account_test.go
Normal file
@@ -0,0 +1,125 @@
|
||||
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
|
||||
}
|
||||
|
||||
func (f *stubOAuthFlow) RequestAuthInfo(context.Context) (auth.AuthFlowInfo, error) {
|
||||
return auth.AuthFlowInfo{}, nil
|
||||
}
|
||||
|
||||
func (f *stubOAuthFlow) WaitToken(context.Context, auth.AuthFlowInfo) (auth.TokenInfo, error) {
|
||||
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(context.Background(), &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(context.Background(), &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(context.Background(), &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(context.Background(), &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 },
|
||||
}
|
||||
|
||||
_, 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")
|
||||
}
|
||||
|
||||
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}},
|
||||
info: auth.AuthFlowInfo{UserCode: "code"},
|
||||
expiresAt: time.Now().Add(time.Minute),
|
||||
hint: hint,
|
||||
accountPrompted: accountPrompted,
|
||||
}
|
||||
return s
|
||||
}
|
||||
@@ -82,6 +82,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
|
||||
@@ -153,6 +159,14 @@ type oauthAuthFlow struct {
|
||||
flow auth.OAuthFlow
|
||||
info auth.AuthFlowInfo
|
||||
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.
|
||||
@@ -709,6 +723,16 @@ func (s *Server) startSSOLogin(ctx context.Context, msg *proto.LoginRequest, con
|
||||
return nil, err
|
||||
}
|
||||
|
||||
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); resp != nil {
|
||||
return resp, nil
|
||||
}
|
||||
@@ -723,6 +747,8 @@ func (s *Server) startSSOLogin(ctx context.Context, msg *proto.LoginRequest, con
|
||||
s.oauthAuthFlow.flow = oAuthFlow
|
||||
s.oauthAuthFlow.info = authInfo
|
||||
s.oauthAuthFlow.expiresAt = time.Now().Add(time.Duration(authInfo.ExpiresIn) * time.Second)
|
||||
s.oauthAuthFlow.hint = hint
|
||||
s.oauthAuthFlow.accountPrompted = promptForAccount
|
||||
s.mutex.Unlock()
|
||||
|
||||
state.Set(internal.StatusNeedsLogin)
|
||||
@@ -923,8 +949,33 @@ func (s *Server) WaitSSOLogin(callerCtx context.Context, msg *proto.WaitSSOLogin
|
||||
|
||||
s.mutex.Lock()
|
||||
s.oauthAuthFlow.expiresAt = time.Now()
|
||||
hint := s.oauthAuthFlow.hint
|
||||
accountPrompted := s.oauthAuthFlow.accountPrompted
|
||||
s.mutex.Unlock()
|
||||
|
||||
if !tokenInfo.MatchesAccount(hint) {
|
||||
if !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()
|
||||
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)
|
||||
return nil, err
|
||||
@@ -1225,6 +1276,16 @@ func (s *Server) SwitchProfile(callerCtx context.Context, msg *proto.SwitchProfi
|
||||
|
||||
s.config = config
|
||||
|
||||
// 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
|
||||
|
||||
if msg != nil && msg.ProfileName != nil {
|
||||
s.publishProfileListChanged(*msg.ProfileName)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user