Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion server/logout.go
Original file line number Diff line number Diff line change
Expand Up @@ -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. <img src="/logout">).
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 != "" {
Expand Down Expand Up @@ -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)
}
}
Expand Down
46 changes: 43 additions & 3 deletions server/logout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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()
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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),
Expand Down
11 changes: 6 additions & 5 deletions server/templates.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
12 changes: 11 additions & 1 deletion web/templates/logout.html
Original file line number Diff line number Diff line change
@@ -1,7 +1,17 @@
{{ template "header.html" . }}

<div class="theme-panel">
{{ if .LoggedOut }}
{{ if .ShowConfirmation }}
<h2 class="theme-heading">Logging Out</h2>
<div>
<div class="dex-subtle-text">Do you want to log out?</div>
</div>
<div class="theme-form-row">
<form method="post" action="{{ .ReqPath }}">
<button type="submit" class="dex-btn theme-btn--primary">Log Out</button>
</form>
</div>
{{ else if .LoggedOut }}
<h2 class="theme-heading">Logged Out</h2>
<div>
<div class="dex-subtle-text">You have been successfully logged out.</div>
Expand Down
Loading