Skip to content

Commit

Permalink
fix: remove session from store on logout
Browse files Browse the repository at this point in the history
This patch resolves an issue where the session would not be purged from the store when performing an RP-initiated logout request from a client, if said client does not purge the authentication session properly because the client does not have access to it or because the client misbehaves.
  • Loading branch information
aeneasr committed Dec 8, 2020
1 parent ef12c06 commit 4495f56
Showing 1 changed file with 11 additions and 1 deletion.
12 changes: 11 additions & 1 deletion consent/strategy_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -964,6 +964,8 @@ func (s *DefaultStrategy) completeLogout(w http.ResponseWriter, r *http.Request)
}
}

_, _ = s.revokeAuthenticationCookie(w, r, s.r.CookieStore()) // Cookie removal is optional

urls, err := s.generateFrontChannelLogoutURLs(r.Context(), lr.Subject, lr.SessionID)
if err != nil {
return nil, err
Expand All @@ -973,14 +975,22 @@ func (s *DefaultStrategy) completeLogout(w http.ResponseWriter, r *http.Request)
return nil, err
}

if err := s.revokeAuthenticationSession(w, r); err != nil {
// We delete the session after back channel log out has worked as the session is otherwise removed
// from the store which will break the query for finding all the channels.
//
// executeBackChannelLogout only fails on system errors so not on URL errors, so this should be fine
// even if an upstream URL fails!
if err := s.r.ConsentManager().DeleteLoginSession(r.Context(), lr.SessionID); errors.Is(err, sqlcon.ErrNoRows) {
// This is ok (session probably already revoked), do nothing!
} else if err != nil {
return nil, err
}

s.r.AuditLogger().
WithRequest(r).
WithField("subject", lr.Subject).
Info("User logout completed!")

return &LogoutResult{
RedirectTo: lr.PostLogoutRedirectURI,
FrontChannelLogoutURLs: urls,
Expand Down

0 comments on commit 4495f56

Please sign in to comment.