diff --git a/server/handlers.go b/server/handlers.go index 8db9b05f..7bc04c48 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -224,6 +224,50 @@ func (s *Server) handleAuthorization(w http.ResponseWriter, r *http.Request) { if len(connectors) == 1 && !s.alwaysShowLogin { connURL.Path = s.absPath("/auth", url.PathEscape(connectors[0].ID)) http.Redirect(w, r, connURL.String(), http.StatusFound) + return + } + + // Skip connector selection if a valid session exists, unless prompt=select_account or alwaysShowLogin. + if s.sessionConfig != nil { + authReq, _, err := s.parseAuthorizationRequest(r) + if err != nil { + s.logger.ErrorContext(r.Context(), "failed to parse authorization request", "err", err) + + switch authErr := err.(type) { + case *redirectedAuthErr: + authErr.Handler().ServeHTTP(w, r) + case *displayedAuthErr: + s.renderError(r, w, authErr.Status, err.Error()) + default: + panic("unsupported error type") + } + } + prompt, err := ParsePrompt(authReq.Prompt) + if err != nil { + // Server error because authReq was validated before saving it to database. + s.redirectWithError(w, r, authReq, errServerError, "Invalid authentication request") + return + } + + // Invalid prompts will be validated and properly redirected later + if !s.alwaysShowLogin && !prompt.SelectAccount() { + session := s.getValidSession(ctx, w, r) + if session != nil { + for _, c := range connectors { + if c.ID != session.ConnectorID { + continue + } + connURL.Path = s.absPath("/auth", url.PathEscape(session.ConnectorID)) + http.Redirect(w, r, connURL.String(), http.StatusFound) + return + } + } + } + if prompt.None() { + // Cannot authenticate silently with prompt=none. + s.redirectWithError(w, r, authReq, errLoginRequired, "id_token_hint does not match authenticated user") + return + } } connectorInfos := make([]connectorInfo, 0, len(connectors)) diff --git a/server/handlers_test.go b/server/handlers_test.go index dad1ab2a..c25aeaf5 100644 --- a/server/handlers_test.go +++ b/server/handlers_test.go @@ -1913,6 +1913,143 @@ func TestHandleAuthorizationWithNoMatchingConnectors(t *testing.T) { require.Equal(t, http.StatusBadRequest, rr.Code) } +func TestHandleAuthorizationSessionSkipsConnectorSelection(t *testing.T) { + ctx := t.Context() + + sessionConfig := &SessionConfig{ + CookieName: "dex_session", + AbsoluteLifetime: 24 * time.Hour, + ValidIfNotUsedFor: 1 * time.Hour, + } + + client := storage.Client{ + ID: "test-client", + Secret: "secret", + RedirectURIs: []string{"https://example.com/callback"}, + Name: "Test Client", + } + + authURL := fmt.Sprintf("/auth?client_id=%s&redirect_uri=%s&response_type=code&scope=openid", + client.ID, url.QueryEscape("https://example.com/callback")) + + createSession := func(t *testing.T, s *Server, connectorID string) *http.Cookie { + t.Helper() + now := time.Now() + nonce := "test-nonce" + session := storage.AuthSession{ + UserID: "user1", + ConnectorID: connectorID, + Nonce: nonce, + ClientStates: map[string]*storage.ClientAuthState{}, + CreatedAt: now.Add(-30 * time.Minute), + LastActivity: now.Add(-5 * time.Minute), + IPAddress: "127.0.0.1", + UserAgent: "test", + AbsoluteExpiry: now.Add(24 * time.Hour), + IdleExpiry: now.Add(1 * time.Hour), + } + require.NoError(t, s.storage.CreateAuthSession(ctx, session)) + return &http.Cookie{ + Name: "dex_session", + Value: sessionCookieValue("user1", connectorID, nonce), + } + } + + t.Run("valid session redirects to session connector", func(t *testing.T) { + httpServer, s := newTestServerMultipleConnectors(t, func(c *Config) { + c.SessionConfig = sessionConfig + }) + defer httpServer.Close() + require.NoError(t, s.storage.CreateClient(ctx, client)) + + cookie := createSession(t, s, "mock") + + rr := httptest.NewRecorder() + req := httptest.NewRequest("GET", authURL, nil) + req.AddCookie(cookie) + s.ServeHTTP(rr, req) + + require.Equal(t, http.StatusFound, rr.Code) + require.Contains(t, rr.Header().Get("Location"), "/auth/mock") + }) + + t.Run("prompt=select_account shows connector selection despite session", func(t *testing.T) { + httpServer, s := newTestServerMultipleConnectors(t, func(c *Config) { + c.SessionConfig = sessionConfig + }) + defer httpServer.Close() + require.NoError(t, s.storage.CreateClient(ctx, client)) + + cookie := createSession(t, s, "mock") + + rr := httptest.NewRecorder() + req := httptest.NewRequest("GET", authURL+"&prompt=select_account", nil) + req.AddCookie(cookie) + s.ServeHTTP(rr, req) + + require.Equal(t, http.StatusOK, rr.Code) + }) + + t.Run("no session shows connector selection", func(t *testing.T) { + httpServer, s := newTestServerMultipleConnectors(t, func(c *Config) { + c.SessionConfig = sessionConfig + }) + defer httpServer.Close() + require.NoError(t, s.storage.CreateClient(ctx, client)) + + rr := httptest.NewRecorder() + req := httptest.NewRequest("GET", authURL, nil) + s.ServeHTTP(rr, req) + + require.Equal(t, http.StatusOK, rr.Code) + }) + + t.Run("alwaysShowLogin shows connector selection despite session", func(t *testing.T) { + httpServer, s := newTestServerMultipleConnectors(t, func(c *Config) { + c.SessionConfig = sessionConfig + c.AlwaysShowLoginScreen = true + }) + defer httpServer.Close() + require.NoError(t, s.storage.CreateClient(ctx, client)) + + cookie := createSession(t, s, "mock") + + rr := httptest.NewRecorder() + req := httptest.NewRequest("GET", authURL, nil) + req.AddCookie(cookie) + s.ServeHTTP(rr, req) + + require.Equal(t, http.StatusOK, rr.Code) + }) + + t.Run("session connector not in filtered list shows connector selection", func(t *testing.T) { + httpServer, s := newTestServerMultipleConnectors(t, func(c *Config) { + c.SessionConfig = sessionConfig + }) + defer httpServer.Close() + + filteredClient := storage.Client{ + ID: "filtered-client", + Secret: "secret", + RedirectURIs: []string{"https://example.com/callback"}, + Name: "Filtered Client", + AllowedConnectors: []string{"mock", "mock2"}, + } + require.NoError(t, s.storage.CreateClient(ctx, filteredClient)) + + // Session is for "other-connector" which is not in the allowed list. + cookie := createSession(t, s, "other-connector") + + rr := httptest.NewRecorder() + req := httptest.NewRequest("GET", fmt.Sprintf("/auth?client_id=%s&redirect_uri=%s&response_type=code&scope=openid", + filteredClient.ID, url.QueryEscape("https://example.com/callback")), nil) + req.AddCookie(cookie) + s.ServeHTTP(rr, req) + + require.Equal(t, http.StatusOK, rr.Code) + }) +} + func TestHandleAuthorizationWithoutAllowedConnectors(t *testing.T) { ctx := t.Context() diff --git a/server/prompt.go b/server/prompt.go index f4941745..c225a270 100644 --- a/server/prompt.go +++ b/server/prompt.go @@ -9,9 +9,10 @@ import ( // The parameter is space-separated and may contain: "none", "login", "consent", "select_account". // "none" must not be combined with any other value. type Prompt struct { - none bool - login bool - consent bool + none bool + login bool + consent bool + selectAccount bool } // ParsePrompt parses and validates the raw prompt query parameter. @@ -39,13 +40,13 @@ func ParsePrompt(raw string) (Prompt, error) { case "consent": p.consent = true case "select_account": - // Dex does not support account selection; ignore per spec recommendation. + p.selectAccount = true default: return Prompt{}, fmt.Errorf("invalid prompt value %q", v) } } - if p.none && (p.login || p.consent) { + if p.none && (p.login || p.consent || p.selectAccount) { return Prompt{}, fmt.Errorf("prompt=none must not be combined with other values") } @@ -61,6 +62,9 @@ func (p Prompt) Login() bool { return p.login } // Consent returns true if the caller requested forced consent screen. func (p Prompt) Consent() bool { return p.consent } +// SelectAccount returns true if the caller requested account/connector selection. +func (p Prompt) SelectAccount() bool { return p.selectAccount } + // String returns the canonical space-separated representation stored in the database. func (p Prompt) String() string { var parts []string @@ -73,5 +77,8 @@ func (p Prompt) String() string { if p.consent { parts = append(parts, "consent") } + if p.selectAccount { + parts = append(parts, "select_account") + } return strings.Join(parts, " ") } diff --git a/server/prompt_test.go b/server/prompt_test.go index 00820ce8..e02b78b3 100644 --- a/server/prompt_test.go +++ b/server/prompt_test.go @@ -20,14 +20,16 @@ func TestParsePrompt(t *testing.T) { {name: "consent", raw: "consent", want: Prompt{consent: true}}, {name: "login consent", raw: "login consent", want: Prompt{login: true, consent: true}}, {name: "consent login", raw: "consent login", want: Prompt{login: true, consent: true}}, - {name: "select_account ignored", raw: "select_account", want: Prompt{}}, - {name: "login select_account", raw: "login select_account", want: Prompt{login: true}}, + {name: "select_account", raw: "select_account", want: Prompt{selectAccount: true}}, + {name: "login select_account", raw: "login select_account", want: Prompt{login: true, selectAccount: true}}, + {name: "consent select_account", raw: "consent select_account", want: Prompt{consent: true, selectAccount: true}}, {name: "duplicate values", raw: "login login", want: Prompt{login: true}}, {name: "whitespace padding", raw: " login ", want: Prompt{login: true}}, // Errors. {name: "none with login", raw: "none login", wantErr: true}, {name: "none with consent", raw: "none consent", wantErr: true}, + {name: "none with select_account", raw: "none select_account", wantErr: true}, {name: "unknown value", raw: "bogus", wantErr: true}, } @@ -54,6 +56,8 @@ func TestPromptString(t *testing.T) { {Prompt{login: true}, "login"}, {Prompt{consent: true}, "consent"}, {Prompt{login: true, consent: true}, "login consent"}, + {Prompt{selectAccount: true}, "select_account"}, + {Prompt{login: true, selectAccount: true}, "login select_account"}, } for _, tc := range tests { diff --git a/server/session.go b/server/session.go index 9640400e..3d0e0ec2 100644 --- a/server/session.go +++ b/server/session.go @@ -150,11 +150,11 @@ func (s *Server) clearSessionCookie(w http.ResponseWriter) { }) } -// getValidAuthSession returns a valid, non-expired session or nil. +// getValidSession returns a valid, non-expired session or nil. // It parses the session cookie to extract (userID, connectorID, nonce), // looks up the session by composite key, and verifies the nonce. // Invalid or expired session cookies are cleared automatically. -func (s *Server) getValidAuthSession(ctx context.Context, w http.ResponseWriter, r *http.Request, authReq *storage.AuthRequest) *storage.AuthSession { +func (s *Server) getValidSession(ctx context.Context, w http.ResponseWriter, r *http.Request) *storage.AuthSession { if s.sessionConfig == nil { return nil } @@ -211,12 +211,22 @@ func (s *Server) getValidAuthSession(ctx context.Context, w http.ResponseWriter, return nil } + return &session +} + +// getValidAuthSession returns a valid session matching the auth request's connector, or nil. +func (s *Server) getValidAuthSession(ctx context.Context, w http.ResponseWriter, r *http.Request, authReq *storage.AuthRequest) *storage.AuthSession { + session := s.getValidSession(ctx, w, r) + if session == nil { + return nil + } + // Only reuse sessions from the same connector. if session.ConnectorID != authReq.ConnectorID { return nil } - return &session + return session } // createOrUpdateAuthSession creates a new session or updates an existing one