diff --git a/client/server/login_account_test.go b/client/server/login_account_test.go index f9a7fd09b..ea138ae02 100644 --- a/client/server/login_account_test.go +++ b/client/server/login_account_test.go @@ -13,7 +13,8 @@ import ( ) type stubOAuthFlow struct { - token auth.TokenInfo + token auth.TokenInfo + onWait func() } func (f *stubOAuthFlow) RequestAuthInfo(context.Context) (auth.AuthFlowInfo, error) { @@ -21,6 +22,9 @@ func (f *stubOAuthFlow) RequestAuthInfo(context.Context) (auth.AuthFlowInfo, err } func (f *stubOAuthFlow) WaitToken(context.Context, auth.AuthFlowInfo) (auth.TokenInfo, error) { + if f.onWait != nil { + f.onWait() + } return f.token, nil } @@ -119,6 +123,31 @@ func TestSwitchProfile_DropsAccountPromptAndPendingFlow(t *testing.T) { 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" + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + resp, err := s.WaitSSOLogin(ctx, &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 newSSOTestServer(t *testing.T, hint string, accountPrompted bool, tokenEmail string) *Server { t.Helper() s := New(internal.CtxInitState(context.Background()), "console", "", false, false, false, false) diff --git a/client/server/server.go b/client/server/server.go index 8084c157f..ddee9226b 100644 --- a/client/server/server.go +++ b/client/server/server.go @@ -868,7 +868,13 @@ func (s *Server) WaitSSOLogin(callerCtx context.Context, msg *proto.WaitSSOLogin } s.actCancel = cancel - flow := s.oauthAuthFlow.flow + // 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 flow == nil { @@ -889,9 +895,7 @@ 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) @@ -949,12 +953,10 @@ 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 { + 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