Skip to content

Commit

Permalink
feat: add ability to revoke login sessions by SessionID (#3450)
Browse files Browse the repository at this point in the history
API `revokeOAuth2LoginSessions` can now revoke a single session by a SessionID (`sid` claim in the id_token) and execute an OpenID Connect Back-channel logout.

Closes #3448
  • Loading branch information
sgal authored Mar 13, 2023
1 parent d455bf6 commit b42482b
Show file tree
Hide file tree
Showing 5 changed files with 149 additions and 24 deletions.
38 changes: 31 additions & 7 deletions consent/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ type listOAuth2ConsentSessions struct {
// in: query
// required: true
Subject string `json:"subject"`

// The login session id to list the consent sessions for.
//
// in: query
Expand Down Expand Up @@ -229,17 +230,28 @@ type revokeOAuth2LoginSessions struct {
// The subject to revoke authentication sessions for.
//
// in: query
// required: true
Subject string `json:"subject"`

// OAuth 2.0 Subject
//
// The subject to revoke authentication sessions for.
//
// in: query
SessionID string `json:"sid"`
}

// swagger:route DELETE /admin/oauth2/auth/sessions/login oAuth2 revokeOAuth2LoginSessions
//
// # Revokes All OAuth 2.0 Login Sessions of a Subject
// # Revokes OAuth 2.0 Login Sessions by either a Subject or a SessionID
//
// This endpoint invalidates authentication sessions. After revoking the authentication session(s), the subject
// has to re-authenticate at the Ory OAuth2 Provider. This endpoint does not invalidate any tokens.
//
// If you send the subject in a query param, all authentication sessions that belong to that subject are revoked.
// No OpennID Connect Front- or Back-channel logout is performed in this case.
//
// This endpoint invalidates a subject's authentication session. After revoking the authentication session, the subject
// has to re-authenticate at the Ory OAuth2 Provider. This endpoint does not invalidate any tokens and
// does not work with OpenID Connect Front- or Back-channel logout.
// Alternatively, you can send a SessionID via `sid` query param, in which case, only the session that is connected
// to that SessionID is revoked. OpenID Connect Back-channel logout is performed in this case.
//
// Consumes:
// - application/json
Expand All @@ -253,9 +265,21 @@ type revokeOAuth2LoginSessions struct {
// 204: emptyResponse
// default: errorOAuth2
func (h *Handler) revokeOAuth2LoginSessions(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
sid := r.URL.Query().Get("sid")
subject := r.URL.Query().Get("subject")
if subject == "" {
h.r.Writer().WriteError(w, r, errorsx.WithStack(fosite.ErrInvalidRequest.WithHint(`Query parameter 'subject' is not defined but should have been.`)))

if sid == "" && subject == "" {
h.r.Writer().WriteError(w, r, errorsx.WithStack(fosite.ErrInvalidRequest.WithHint(`Either 'subject' or 'sid' query parameters need to be defined.`)))
return
}

if sid != "" {
if err := h.r.ConsentStrategy().HandleHeadlessLogout(r.Context(), w, r, sid); err != nil {
h.r.Writer().WriteError(w, r, err)
return
}

w.WriteHeader(http.StatusNoContent)
return
}

Expand Down
1 change: 1 addition & 0 deletions consent/strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@ var _ Strategy = new(DefaultStrategy)
type Strategy interface {
HandleOAuth2AuthorizationRequest(ctx context.Context, w http.ResponseWriter, r *http.Request, req fosite.AuthorizeRequester) (*AcceptOAuth2ConsentRequest, error)
HandleOpenIDConnectLogout(ctx context.Context, w http.ResponseWriter, r *http.Request) (*LogoutResult, error)
HandleHeadlessLogout(ctx context.Context, w http.ResponseWriter, r *http.Request, sid string) error
ObfuscateSubjectIdentifier(ctx context.Context, cl fosite.Client, subject, forcedIdentifier string) (string, error)
}
57 changes: 45 additions & 12 deletions consent/strategy_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -899,6 +899,25 @@ func (s *DefaultStrategy) issueLogoutVerifier(ctx context.Context, w http.Respon
return nil, errorsx.WithStack(ErrAbortOAuth2Request)
}

func (s *DefaultStrategy) performBackChannelLogoutAndDeleteSession(ctx context.Context, r *http.Request, subject string, sid string) error {
if err := s.executeBackChannelLogout(r.Context(), r, subject, sid); err != nil {
return err
}

// 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(), sid); errors.Is(err, sqlcon.ErrNoRows) {
// This is ok (session probably already revoked), do nothing!
} else if err != nil {
return err
}

return nil
}

func (s *DefaultStrategy) completeLogout(ctx context.Context, w http.ResponseWriter, r *http.Request) (*LogoutResult, error) {
verifier := r.URL.Query().Get("logout_verifier")

Expand Down Expand Up @@ -948,18 +967,7 @@ func (s *DefaultStrategy) completeLogout(ctx context.Context, w http.ResponseWri
return nil, err
}

if err := s.executeBackChannelLogout(r.Context(), r, lr.Subject, lr.SessionID); err != nil {
return nil, err
}

// 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 {
if err := s.performBackChannelLogoutAndDeleteSession(r.Context(), r, lr.Subject, lr.SessionID); err != nil {
return nil, err
}

Expand All @@ -983,6 +991,31 @@ func (s *DefaultStrategy) HandleOpenIDConnectLogout(ctx context.Context, w http.
return s.completeLogout(ctx, w, r)
}

func (s *DefaultStrategy) HandleHeadlessLogout(ctx context.Context, w http.ResponseWriter, r *http.Request, sid string) error {
loginSession, lsErr := s.r.ConsentManager().GetRememberedLoginSession(ctx, sid)

if errors.Is(lsErr, x.ErrNotFound) {
// This is ok (session probably already revoked), do nothing!
// Not triggering the back-channel logout because subject is not available
// See https://github.com/ory/hydra/pull/3450#discussion_r1127798485
return nil
} else if lsErr != nil {
return lsErr
}

if err := s.performBackChannelLogoutAndDeleteSession(r.Context(), r, loginSession.Subject, sid); err != nil {
return err
}

s.r.AuditLogger().
WithRequest(r).
WithField("subject", loginSession.Subject).
WithField("sid", sid).
Info("User logout completed via headless flow!")

return nil
}

func (s *DefaultStrategy) HandleOAuth2AuthorizationRequest(ctx context.Context, w http.ResponseWriter, r *http.Request, req fosite.AuthorizeRequester) (*AcceptOAuth2ConsentRequest, error) {
authenticationVerifier := strings.TrimSpace(req.GetRequestForm().Get("login_verifier"))
consentVerifier := strings.TrimSpace(req.GetRequestForm().Get("consent_verifier"))
Expand Down
73 changes: 68 additions & 5 deletions consent/strategy_logout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,29 @@ func TestLogoutFlows(t *testing.T) {
return string(ioutilx.MustReadAll(resp.Body)), resp
}

makeHeadlessLogoutRequest := func(t *testing.T, hc *http.Client, values url.Values) (body string, resp *http.Response) {
var err error
req, err := http.NewRequest(http.MethodDelete, adminTS.URL+"/admin/oauth2/auth/sessions/login?"+values.Encode(), nil)
require.NoError(t, err)

resp, err = hc.Do(req)

require.NoError(t, err)
defer resp.Body.Close()
return string(ioutilx.MustReadAll(resp.Body)), resp
}

logoutViaHeadlessAndExpectNoContent := func(t *testing.T, browser *http.Client, values url.Values) {
_, res := makeHeadlessLogoutRequest(t, browser, values)
assert.EqualValues(t, http.StatusNoContent, res.StatusCode)
}

logoutViaHeadlessAndExpectError := func(t *testing.T, browser *http.Client, values url.Values, expectedErrorMessage string) {
body, res := makeHeadlessLogoutRequest(t, browser, values)
assert.EqualValues(t, http.StatusBadRequest, res.StatusCode)
assert.Contains(t, body, expectedErrorMessage)
}

logoutAndExpectErrorPage := func(t *testing.T, browser *http.Client, method string, values url.Values, expectedErrorMessage string) {
body, res := makeLogoutRequest(t, browser, method, values)
assert.EqualValues(t, http.StatusInternalServerError, res.StatusCode)
Expand Down Expand Up @@ -153,7 +176,7 @@ func TestLogoutFlows(t *testing.T) {
reg.Config().MustSet(ctx, config.KeyLogoutURL, server.URL)
}

acceptLoginAsAndWatchSid := func(t *testing.T, subject string, sid chan string) {
acceptLoginAsAndWatchSidForConsumers := func(t *testing.T, subject string, sid chan string, remember bool, numSidConsumers int) {
testhelpers.NewLoginConsentUI(t, reg.Config(),
checkAndAcceptLoginHandler(t, adminApi, subject, func(t *testing.T, res *hydra.OAuth2LoginRequest, err error) hydra.AcceptOAuth2LoginRequest {
require.NoError(t, err)
Expand All @@ -164,16 +187,22 @@ func TestLogoutFlows(t *testing.T) {
require.NoError(t, err)
if sid != nil {
go func() {
sid <- *res.LoginSessionId
for i := 0; i < numSidConsumers; i++ {
sid <- *res.LoginSessionId
}
}()
}
return hydra.AcceptOAuth2ConsentRequest{Remember: pointerx.Bool(true)}
return hydra.AcceptOAuth2ConsentRequest{Remember: pointerx.Bool(remember)}
}))

}

acceptLoginAsAndWatchSid := func(t *testing.T, subject string, sid chan string) {
acceptLoginAsAndWatchSidForConsumers(t, subject, sid, true, 1)
}

acceptLoginAs := func(t *testing.T, subject string) {
acceptLoginAsAndWatchSid(t, subject, nil)
acceptLoginAsAndWatchSidForConsumers(t, subject, nil, true, 0)
}

subject := "aeneas-rekkas"
Expand Down Expand Up @@ -399,7 +428,7 @@ func TestLogoutFlows(t *testing.T) {

t.Run("case=should pass rp-inititated flow without any action because SID is unknown", func(t *testing.T) {
c := createSampleClient(t)
acceptLoginAsAndWatchSid(t, subject, nil)
acceptLoginAs(t, subject)

checkAndAcceptLogout(t, nil, func(t *testing.T, res *hydra.OAuth2LogoutRequest, err error) {
t.Fatalf("Logout should not have been called")
Expand Down Expand Up @@ -490,4 +519,38 @@ func TestLogoutFlows(t *testing.T) {

wg.Wait()
})

t.Run("case=should execute backchannel logout in headless flow with sid", func(t *testing.T) {
numSidConsumers := 2
sid := make(chan string, numSidConsumers)
acceptLoginAsAndWatchSidForConsumers(t, subject, sid, true, numSidConsumers)

backChannelWG := newWg(1)
c := createClientWithBackchannelLogout(t, backChannelWG, func(t *testing.T, logoutToken gjson.Result) {
assert.EqualValues(t, <-sid, logoutToken.Get("sid").String(), logoutToken.Raw)
assert.Empty(t, logoutToken.Get("sub").String(), logoutToken.Raw) // The sub claim should be empty because it doesn't work with forced obfuscation and thus we can't easily recover it.
assert.Empty(t, logoutToken.Get("nonce").String(), logoutToken.Raw)
})

logoutViaHeadlessAndExpectNoContent(t, createBrowserWithSession(t, c), url.Values{"sid": {<-sid}})

backChannelWG.Wait() // we want to ensure that all back channels have been called!
})

t.Run("case=should logout in headless flow with non-existing sid", func(t *testing.T) {
logoutViaHeadlessAndExpectNoContent(t, browserWithoutSession, url.Values{"sid": {"non-existing-sid"}})
})

t.Run("case=should logout in headless flow with session that has remember=false", func(t *testing.T) {
sid := make(chan string)
acceptLoginAsAndWatchSidForConsumers(t, subject, sid, false, 1)

c := createSampleClient(t)

logoutViaHeadlessAndExpectNoContent(t, createBrowserWithSession(t, c), url.Values{"sid": {<-sid}})
})

t.Run("case=should fail headless logout because neither sid nor subject were provided", func(t *testing.T) {
logoutViaHeadlessAndExpectError(t, browserWithoutSession, url.Values{}, `Either 'subject' or 'sid' query parameters need to be defined.`)
})
}
4 changes: 4 additions & 0 deletions oauth2/oauth2_helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ func (c *consentMock) HandleOpenIDConnectLogout(ctx context.Context, w http.Resp
panic("not implemented")
}

func (c *consentMock) HandleHeadlessLogout(ctx context.Context, w http.ResponseWriter, r *http.Request, sid string) error {
panic("not implemented")
}

func (c *consentMock) ObfuscateSubjectIdentifier(ctx context.Context, cl fosite.Client, subject, forcedIdentifier string) (string, error) {
if c, ok := cl.(*client.Client); ok && c.SubjectType == "pairwise" {
panic("not implemented")
Expand Down

0 comments on commit b42482b

Please sign in to comment.