From 3cd20882fa110f3d38607aff53474fda4163d648 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zolt=C3=A1n=20Papp?= Date: Thu, 27 Aug 2026 11:33:22 +0200 Subject: [PATCH] [client] Keep the forced account prompt from being lost to flow reuse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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/server/login_account_test.go | 20 +++++++++++++++++ client/server/server.go | 35 ++++++++++++++++++++--------- 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/client/server/login_account_test.go b/client/server/login_account_test.go index ea138ae02..7fda8f43c 100644 --- a/client/server/login_account_test.go +++ b/client/server/login_account_test.go @@ -148,6 +148,26 @@ func TestWaitSSOLogin_JudgesTheFlowThatProducedTheToken(t *testing.T) { 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 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 ddee9226b..a121e1a98 100644 --- a/client/server/server.go +++ b/client/server/server.go @@ -733,7 +733,7 @@ func (s *Server) startSSOLogin(ctx context.Context, msg *proto.LoginRequest, con 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 { + if resp := s.reuseOAuthFlow(ctx, oAuthFlow, state, promptForAccount); resp != nil { return resp, nil } @@ -744,11 +744,13 @@ func (s *Server) startSSOLogin(ctx context.Context, msg *proto.LoginRequest, con } 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.hint = hint - s.oauthAuthFlow.accountPrompted = promptForAccount + s.oauthAuthFlow = oauthAuthFlow{ + flow: oAuthFlow, + info: authInfo, + expiresAt: time.Now().Add(time.Duration(authInfo.ExpiresIn) * time.Second), + hint: hint, + accountPrompted: promptForAccount, + } s.mutex.Unlock() state.Set(internal.StatusNeedsLogin) @@ -765,13 +767,19 @@ func (s *Server) startSSOLogin(ctx context.Context, msg *proto.LoginRequest, con // 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) *proto.LoginResponse { +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() @@ -780,7 +788,7 @@ func (s *Server) reuseOAuthFlow(ctx context.Context, oAuthFlow auth.OAuthFlow, s return nil } - if !current.expiresAt.After(time.Now().Add(90 * time.Second)) { + if promptForAccount || !current.expiresAt.After(time.Now().Add(90*time.Second)) { log.Warnf("canceling previous waiting execution") if current.waitCancel != nil { current.waitCancel() @@ -1843,10 +1851,15 @@ func (s *Server) RequestJWTAuth( return nil, gstatus.Errorf(codes.Internal, "failed to request auth info: %v", err) } + // Replace the whole record: this flow carries no profile hint, and leaving + // the previous login's hint and accountPrompted in place would have + // WaitSSOLogin judge a later token against them. 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 = oauthAuthFlow{ + flow: oAuthFlow, + info: authInfo, + expiresAt: time.Now().Add(time.Duration(authInfo.ExpiresIn) * time.Second), + } s.mutex.Unlock() return &proto.RequestJWTAuthResponse{