diff --git a/server/logout.go b/server/logout.go index d48f7a59fe..cb95398d83 100644 --- a/server/logout.go +++ b/server/logout.go @@ -44,6 +44,17 @@ func (s *Server) handleLogout(w http.ResponseWriter, r *http.Request) { postLogoutRedirectURI := r.FormValue("post_logout_redirect_uri") state := r.FormValue("state") + // When no id_token_hint is provided and this is a GET request, + // show a confirmation page instead of logging out immediately. + // This follows the OIDC spec recommendation and prevents CSRF via + // cross-site image/link requests (e.g. ). + if idTokenHint == "" && r.Method == http.MethodGet { + if err := s.templates.logout(r, w, "", false, true); err != nil { + s.logger.ErrorContext(ctx, "server template error", "err", err) + } + return + } + var userID, connectorID, clientID string if idTokenHint != "" { @@ -226,7 +237,7 @@ func (s *Server) finishLogout(w http.ResponseWriter, r *http.Request, postLogout } } - if err := s.templates.logout(r, w, backURL, loggedOut); err != nil { + if err := s.templates.logout(r, w, backURL, loggedOut, false); err != nil { s.logger.ErrorContext(r.Context(), "server template error", "err", err) } } diff --git a/server/logout_test.go b/server/logout_test.go index 0554cb0ff1..b4363e9fb5 100644 --- a/server/logout_test.go +++ b/server/logout_test.go @@ -47,7 +47,7 @@ func TestHandleLogoutPOST(t *testing.T) { require.Contains(t, body, "No active session") } -func TestHandleLogoutNoHint(t *testing.T) { +func TestHandleLogoutNoHintGETShowsConfirmation(t *testing.T) { httpServer, server := newTestServerWithSessions(t, nil) defer httpServer.Close() @@ -56,10 +56,41 @@ func TestHandleLogoutNoHint(t *testing.T) { server.ServeHTTP(rr, req) require.Equal(t, http.StatusOK, rr.Code) body := rr.Body.String() - require.Contains(t, body, "No active session") + require.Contains(t, body, "Do you want to log out?") + require.Contains(t, body, `method="post"`) require.NotContains(t, body, "successfully logged out") } +func TestHandleLogoutNoHintPOSTPerformsLogout(t *testing.T) { + httpServer, server := newTestServerWithSessions(t, nil) + defer httpServer.Close() + + ctx := t.Context() + userID := "test-user" + connectorID := "mock" + nonce := "testnonce" + + require.NoError(t, server.storage.CreateAuthSession(ctx, storage.AuthSession{ + UserID: userID, ConnectorID: connectorID, Nonce: nonce, + CreatedAt: time.Now(), LastActivity: time.Now(), + })) + + rr := httptest.NewRecorder() + req := httptest.NewRequest("POST", "/logout", nil) + req.AddCookie(&http.Cookie{ + Name: "dex_session", + Value: sessionCookieValue(userID, connectorID, nonce, server.sessionConfig.CookieEncryptionKey), + }) + server.ServeHTTP(rr, req) + + require.Equal(t, http.StatusOK, rr.Code) + require.Contains(t, rr.Body.String(), "successfully logged out") + + // Session should be deleted. + _, err := server.storage.GetAuthSession(ctx, userID, connectorID) + require.ErrorIs(t, err, storage.ErrNotFound) +} + func TestHandleLogoutInvalidHint(t *testing.T) { httpServer, server := newTestServerWithSessions(t, nil) defer httpServer.Close() @@ -156,8 +187,16 @@ func TestHandleLogoutRedirectURIWithoutHint(t *testing.T) { httpServer, server := newTestServerWithSessions(t, nil) defer httpServer.Close() + // GET without hint shows confirmation page regardless of other params. rr := httptest.NewRecorder() server.ServeHTTP(rr, httptest.NewRequest("GET", "/logout?post_logout_redirect_uri=https://example.com/done", nil)) + require.Equal(t, http.StatusOK, rr.Code) + require.Contains(t, rr.Body.String(), "Do you want to log out?") + + // POST without hint but with post_logout_redirect_uri returns 400 + // because post_logout_redirect_uri requires client_id from id_token_hint. + rr = httptest.NewRecorder() + server.ServeHTTP(rr, httptest.NewRequest("POST", "/logout?post_logout_redirect_uri=https://example.com/done", nil)) require.Equal(t, http.StatusBadRequest, rr.Code) } @@ -309,8 +348,9 @@ func TestHandleLogoutFromCookie(t *testing.T) { CreatedAt: time.Now(), LastActivity: time.Now(), })) + // POST performs logout (GET without hint shows confirmation). rr := httptest.NewRecorder() - req := httptest.NewRequest("GET", "/logout", nil) + req := httptest.NewRequest("POST", "/logout", nil) req.AddCookie(&http.Cookie{ Name: "dex_session", Value: sessionCookieValue(userID, connectorID, nonce, server.sessionConfig.CookieEncryptionKey), diff --git a/server/templates.go b/server/templates.go index 81da3d4d19..b81a6ba5dd 100644 --- a/server/templates.go +++ b/server/templates.go @@ -396,12 +396,13 @@ func (t *templates) home(r *http.Request, w http.ResponseWriter, data homeData) return renderTemplate(w, t.homeTmpl, data) } -func (t *templates) logout(r *http.Request, w http.ResponseWriter, backURL string, loggedOut bool) error { +func (t *templates) logout(r *http.Request, w http.ResponseWriter, backURL string, loggedOut bool, showConfirmation bool) error { data := struct { - BackURL string - LoggedOut bool - ReqPath string - }{backURL, loggedOut, r.URL.Path} + BackURL string + LoggedOut bool + ShowConfirmation bool + ReqPath string + }{backURL, loggedOut, showConfirmation, r.URL.Path} return renderTemplate(w, t.logoutTmpl, data) } diff --git a/web/templates/logout.html b/web/templates/logout.html index eb2a2b74a5..b3d8d66b17 100644 --- a/web/templates/logout.html +++ b/web/templates/logout.html @@ -1,7 +1,17 @@ {{ template "header.html" . }}
- {{ if .LoggedOut }} + {{ if .ShowConfirmation }} +

Logging Out

+
+
Do you want to log out?
+
+
+
+ +
+
+ {{ else if .LoggedOut }}

Logged Out

You have been successfully logged out.