Skip to content

Commit f46ed94

Browse files
alnralnr
committed
feat: stateless logout
1 parent a7579b8 commit f46ed94

29 files changed

+128
-617
lines changed

‎consent/handler.go

+5-18
Original file line numberDiff line numberDiff line change
@@ -911,9 +911,7 @@ func (h *Handler) rejectOAuth2ConsentRequest(w http.ResponseWriter, r *http.Requ
911911
// Accept OAuth 2.0 Logout Request
912912
//
913913
// swagger:parameters acceptOAuth2LogoutRequest
914-
//
915-
//lint:ignore U1000 Used to generate Swagger and OpenAPI definitions
916-
type acceptOAuth2LogoutRequest struct {
914+
type _ struct {
917915
// OAuth 2.0 Logout Request Challenge
918916
//
919917
// in: query
@@ -943,23 +941,21 @@ func (h *Handler) acceptOAuth2LogoutRequest(w http.ResponseWriter, r *http.Reque
943941
r.URL.Query().Get("challenge"),
944942
)
945943

946-
c, err := h.r.ConsentManager().AcceptLogoutRequest(r.Context(), challenge)
944+
verifier, err := h.r.ConsentManager().AcceptLogoutRequest(r.Context(), challenge)
947945
if err != nil {
948946
h.r.Writer().WriteError(w, r, err)
949947
return
950948
}
951949

952950
h.r.Writer().Write(w, r, &flow.OAuth2RedirectTo{
953-
RedirectTo: urlx.SetQuery(urlx.AppendPaths(h.c.PublicURL(r.Context()), "/oauth2/sessions/logout"), url.Values{"logout_verifier": {c.Verifier}}).String(),
951+
RedirectTo: urlx.SetQuery(urlx.AppendPaths(h.c.PublicURL(r.Context()), "/oauth2/sessions/logout"), url.Values{"logout_verifier": {verifier}}).String(),
954952
})
955953
}
956954

957955
// Reject OAuth 2.0 Logout Request
958956
//
959957
// swagger:parameters rejectOAuth2LogoutRequest
960-
//
961-
//lint:ignore U1000 Used to generate Swagger and OpenAPI definitions
962-
type rejectOAuth2LogoutRequest struct {
958+
type _ struct {
963959
// in: query
964960
// required: true
965961
Challenge string `json:"logout_challenge"`
@@ -999,9 +995,7 @@ func (h *Handler) rejectOAuth2LogoutRequest(w http.ResponseWriter, r *http.Reque
999995
// Get OAuth 2.0 Logout Request
1000996
//
1001997
// swagger:parameters getOAuth2LogoutRequest
1002-
//
1003-
//lint:ignore U1000 Used to generate Swagger and OpenAPI definitions
1004-
type getOAuth2LogoutRequest struct {
998+
type _ struct {
1005999
// in: query
10061000
// required: true
10071001
Challenge string `json:"logout_challenge"`
@@ -1039,13 +1033,6 @@ func (h *Handler) getOAuth2LogoutRequest(w http.ResponseWriter, r *http.Request,
10391033
request.Client.Secret = ""
10401034
}
10411035

1042-
if request.WasHandled {
1043-
h.r.Writer().WriteCode(w, r, http.StatusGone, &flow.OAuth2RedirectTo{
1044-
RedirectTo: request.RequestURL,
1045-
})
1046-
return
1047-
}
1048-
10491036
h.r.Writer().Write(w, r, request)
10501037
}
10511038

‎consent/handler_test.go

-55
Original file line numberDiff line numberDiff line change
@@ -27,61 +27,6 @@ import (
2727
"github.com/ory/x/sqlxx"
2828
)
2929

30-
func TestGetLogoutRequest(t *testing.T) {
31-
for k, tc := range []struct {
32-
exists bool
33-
handled bool
34-
status int
35-
}{
36-
{false, false, http.StatusNotFound},
37-
{true, false, http.StatusOK},
38-
{true, true, http.StatusGone},
39-
} {
40-
t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
41-
ctx := context.Background()
42-
key := fmt.Sprint(k)
43-
challenge := "challenge" + key
44-
requestURL := "http://192.0.2.1"
45-
46-
conf := testhelpers.NewConfigurationWithDefaults()
47-
reg := testhelpers.NewRegistryMemory(t, conf, &contextx.Default{})
48-
49-
if tc.exists {
50-
cl := &client.Client{ID: "client" + key}
51-
require.NoError(t, reg.ClientManager().CreateClient(ctx, cl))
52-
require.NoError(t, reg.ConsentManager().CreateLogoutRequest(context.TODO(), &flow.LogoutRequest{
53-
Client: cl,
54-
ID: challenge,
55-
WasHandled: tc.handled,
56-
RequestURL: requestURL,
57-
}))
58-
}
59-
60-
h := NewHandler(reg, conf)
61-
r := x.NewRouterAdmin(conf.AdminURL)
62-
h.SetRoutes(r)
63-
ts := httptest.NewServer(r)
64-
defer ts.Close()
65-
66-
c := &http.Client{}
67-
resp, err := c.Get(ts.URL + "/admin" + LogoutPath + "?challenge=" + challenge)
68-
require.NoError(t, err)
69-
require.EqualValues(t, tc.status, resp.StatusCode)
70-
71-
if tc.handled {
72-
var result flow.OAuth2RedirectTo
73-
require.NoError(t, json.NewDecoder(resp.Body).Decode(&result))
74-
require.Equal(t, requestURL, result.RedirectTo)
75-
} else if tc.exists {
76-
var result flow.LogoutRequest
77-
require.NoError(t, json.NewDecoder(resp.Body).Decode(&result))
78-
require.Equal(t, challenge, result.ID)
79-
require.Equal(t, requestURL, result.RequestURL)
80-
}
81-
})
82-
}
83-
}
84-
8530
func TestGetLoginRequest(t *testing.T) {
8631
for k, tc := range []struct {
8732
exists bool

‎consent/manager.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,9 @@ type (
5555
ListUserAuthenticatedClientsWithFrontChannelLogout(ctx context.Context, subject, sid string) ([]client.Client, error)
5656
ListUserAuthenticatedClientsWithBackChannelLogout(ctx context.Context, subject, sid string) ([]client.Client, error)
5757

58-
CreateLogoutRequest(ctx context.Context, request *flow.LogoutRequest) error
58+
CreateLogoutChallenge(ctx context.Context, request *flow.LogoutRequest) (challenge string, err error)
5959
GetLogoutRequest(ctx context.Context, challenge string) (*flow.LogoutRequest, error)
60-
AcceptLogoutRequest(ctx context.Context, challenge string) (*flow.LogoutRequest, error)
60+
AcceptLogoutRequest(ctx context.Context, challenge string) (verifier string, err error)
6161
RejectLogoutRequest(ctx context.Context, challenge string) error
6262
VerifyAndInvalidateLogoutRequest(ctx context.Context, verifier string) (*flow.LogoutRequest, error)
6363
}

‎consent/sdk_test.go

+13-14
Original file line numberDiff line numberDiff line change
@@ -147,13 +147,6 @@ func TestSDK(t *testing.T) {
147147
_, err = m.VerifyAndInvalidateConsentRequest(context.Background(), consentVerifier(cr4Flow))
148148
require.NoError(t, err)
149149

150-
lur1 := test.MockLogoutRequest("testsdk-1", true, network)
151-
require.NoError(t, reg.ClientManager().CreateClient(context.Background(), lur1.Client))
152-
require.NoError(t, m.CreateLogoutRequest(context.Background(), lur1))
153-
154-
lur2 := test.MockLogoutRequest("testsdk-2", false, network)
155-
require.NoError(t, m.CreateLogoutRequest(context.Background(), lur2))
156-
157150
cr1.ID = consentChallenge(cr1Flow)
158151
crGot := execute[hydra.OAuth2ConsentRequest](t, sdk.OAuth2API.GetOAuth2ConsentRequest(ctx).ConsentChallenge(cr1.ID))
159152
compareSDKConsentRequest(t, cr1, *crGot)
@@ -213,19 +206,25 @@ func TestSDK(t *testing.T) {
213206
require.NoError(t, err)
214207
assert.Equal(t, 0, len(csGot))
215208

216-
luGot, _, err := sdk.OAuth2API.GetOAuth2LogoutRequest(ctx).LogoutChallenge(makeID("challenge", network, "testsdk-1")).Execute()
209+
lur1 := test.MockLogoutRequest("testsdk-1", true, network)
210+
require.NoError(t, reg.ClientManager().CreateClient(context.Background(), lur1.Client))
211+
loc1, err := m.CreateLogoutChallenge(context.Background(), lur1)
217212
require.NoError(t, err)
218-
compareSDKLogoutRequest(t, lur1, luGot)
219213

220-
luaGot, _, err := sdk.OAuth2API.AcceptOAuth2LogoutRequest(ctx).LogoutChallenge(makeID("challenge", network, "testsdk-1")).Execute()
214+
luGot, _, err := sdk.OAuth2API.GetOAuth2LogoutRequest(ctx).LogoutChallenge(loc1).Execute()
221215
require.NoError(t, err)
222-
assert.EqualValues(t, "https://www.ory.sh/oauth2/sessions/logout?logout_verifier="+makeID("verifier", network, "testsdk-1"), luaGot.RedirectTo)
216+
compareSDKLogoutRequest(t, lur1, luGot)
223217

224-
_, err = sdk.OAuth2API.RejectOAuth2LogoutRequest(ctx).LogoutChallenge(lur2.ID).Execute()
218+
luaGot, _, err := sdk.OAuth2API.AcceptOAuth2LogoutRequest(ctx).LogoutChallenge(loc1).Execute()
225219
require.NoError(t, err)
220+
assert.Contains(t, luaGot.RedirectTo, "https://www.ory.sh/oauth2/sessions/logout?logout_verifier=")
226221

227-
_, _, err = sdk.OAuth2API.GetOAuth2LogoutRequest(ctx).LogoutChallenge(lur2.ID).Execute()
228-
require.Error(t, err)
222+
lur2 := test.MockLogoutRequest("testsdk-2", false, network)
223+
loc2, err := m.CreateLogoutChallenge(context.Background(), lur2)
224+
require.NoError(t, err)
225+
lur2Got, err := sdk.OAuth2API.RejectOAuth2LogoutRequest(ctx).LogoutChallenge(loc2).Execute()
226+
require.NoError(t, err)
227+
assert.Equal(t, http.StatusNoContent, lur2Got.StatusCode)
229228
}
230229

231230
func compareSDKLoginRequest(t *testing.T, expected *LoginRequest, got hydra.OAuth2LoginRequest) {

‎consent/strategy_default.go

+29-33
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,13 @@ import (
2121
"github.com/sirupsen/logrus"
2222
"go.opentelemetry.io/otel/trace"
2323

24-
"github.com/ory/hydra/v2/flow"
25-
"github.com/ory/hydra/v2/oauth2/flowctx"
26-
2724
"github.com/ory/fosite"
2825
"github.com/ory/fosite/handler/openid"
2926
"github.com/ory/fosite/token/jwt"
3027
"github.com/ory/hydra/v2/client"
3128
"github.com/ory/hydra/v2/driver/config"
29+
"github.com/ory/hydra/v2/flow"
30+
"github.com/ory/hydra/v2/oauth2/flowctx"
3231
"github.com/ory/hydra/v2/x"
3332
"github.com/ory/x/errorsx"
3433
"github.com/ory/x/mapx"
@@ -855,21 +854,18 @@ func (s *DefaultStrategy) issueLogoutVerifier(ctx context.Context, w http.Respon
855854
return nil, err
856855
}
857856

858-
challenge := uuid.New()
859-
if err := s.r.ConsentManager().CreateLogoutRequest(r.Context(), &flow.LogoutRequest{
860-
RequestURL: r.URL.String(),
861-
ID: challenge,
862-
Subject: session.Subject,
863-
SessionID: session.ID,
864-
Verifier: uuid.New(),
865-
RequestedAt: sqlxx.NullTime(time.Now().UTC().Round(time.Second)),
866-
ExpiresAt: sqlxx.NullTime(time.Now().UTC().Round(time.Second).Add(s.c.ConsentRequestMaxAge(ctx))),
867-
RPInitiated: false,
868-
869-
// PostLogoutRedirectURI is set to the value from config.Provider().LogoutRedirectURL()
857+
now := time.Now().UTC().Round(time.Second)
858+
challenge, err := s.r.ConsentManager().CreateLogoutChallenge(ctx, &flow.LogoutRequest{
859+
RequestURL: r.URL.String(),
860+
Subject: session.Subject,
861+
SessionID: session.ID,
862+
RequestedAt: now,
863+
ExpiresAt: now.Add(s.c.ConsentRequestMaxAge(ctx)),
864+
RPInitiated: false,
870865
PostLogoutRedirectURI: redir,
871-
}); err != nil {
872-
return nil, err
866+
})
867+
if err != nil {
868+
return nil, errors.WithStack(err)
873869
}
874870

875871
s.r.AuditLogger().
@@ -895,13 +891,13 @@ func (s *DefaultStrategy) issueLogoutVerifier(ctx context.Context, w http.Respon
895891
)
896892
}
897893

898-
now := time.Now().UTC().Unix()
899-
if !claims.VerifyIssuedAt(now, true) {
894+
now := time.Now().UTC()
895+
if !claims.VerifyIssuedAt(now.Unix(), true) {
900896
return nil, errorsx.WithStack(fosite.ErrInvalidRequest.
901897
WithHintf(
902898
`Logout failed because iat claim value '%.0f' from query parameter id_token_hint is before now ('%d').`,
903899
mapx.GetFloat64Default(mksi, "iat", float64(0)),
904-
now,
900+
now.Unix(),
905901
),
906902
)
907903
}
@@ -939,6 +935,7 @@ func (s *DefaultStrategy) issueLogoutVerifier(ctx context.Context, w http.Respon
939935
return nil, errorsx.WithStack(fosite.ErrInvalidRequest.
940936
WithHint("Logout failed because none of the listed audiences is a registered OAuth 2.0 Client."))
941937
}
938+
cl.Secret = "" // We don't want to expose the client secret.
942939

943940
if len(requestedRedir) > 0 {
944941
var f *url.URL
@@ -979,20 +976,19 @@ func (s *DefaultStrategy) issueLogoutVerifier(ctx context.Context, w http.Respon
979976
return nil, err
980977
}
981978

982-
challenge := uuid.New()
983-
if err := s.r.ConsentManager().CreateLogoutRequest(r.Context(), &flow.LogoutRequest{
984-
RequestURL: r.URL.String(),
985-
ID: challenge,
986-
SessionID: hintSid,
987-
Subject: session.Subject,
988-
Verifier: uuid.New(),
989-
Client: cl,
990-
RPInitiated: true,
991-
992-
// PostLogoutRedirectURI is set to the value from config.Provider().LogoutRedirectURL()
979+
now = time.Now().UTC().Round(time.Second)
980+
challenge, err := s.r.ConsentManager().CreateLogoutChallenge(ctx, &flow.LogoutRequest{
981+
RequestURL: r.URL.String(),
982+
Subject: session.Subject,
983+
SessionID: hintSid,
984+
RequestedAt: now,
985+
ExpiresAt: now.Add(s.c.ConsentRequestMaxAge(ctx)),
986+
RPInitiated: true,
993987
PostLogoutRedirectURI: redir,
994-
}); err != nil {
995-
return nil, err
988+
Client: cl,
989+
})
990+
if err != nil {
991+
return nil, errors.WithStack(err)
996992
}
997993

998994
http.Redirect(w, r, urlx.SetQuery(s.c.LogoutURL(ctx), url.Values{"logout_challenge": {challenge}}).String(), http.StatusFound)

‎consent/strategy_logout_test.go

+13-13
Original file line numberDiff line numberDiff line change
@@ -157,20 +157,21 @@ func TestLogoutFlows(t *testing.T) {
157157
return &wg
158158
}
159159

160-
setupCheckAndAcceptLogoutHandler := func(t *testing.T, wg *sync.WaitGroup, cb func(*testing.T, *hydra.OAuth2LogoutRequest, error)) {
160+
setupCheckAndAcceptLogoutHandler := func(t *testing.T, wg *sync.WaitGroup, cb func(_ *testing.T, challenge string, _ *hydra.OAuth2LogoutRequest, _ error)) {
161161
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
162162
if wg != nil {
163163
defer wg.Done()
164164
}
165165

166-
res, _, err := adminApi.OAuth2API.GetOAuth2LogoutRequest(ctx).LogoutChallenge(r.URL.Query().Get("logout_challenge")).Execute()
166+
challenge := r.URL.Query().Get("logout_challenge")
167+
res, _, err := adminApi.OAuth2API.GetOAuth2LogoutRequest(ctx).LogoutChallenge(challenge).Execute()
167168
if cb != nil {
168-
cb(t, res, err)
169+
cb(t, challenge, res, err)
169170
} else {
170171
require.NoError(t, err)
171172
}
172173

173-
v, _, err := adminApi.OAuth2API.AcceptOAuth2LogoutRequest(ctx).LogoutChallenge(r.URL.Query().Get("logout_challenge")).Execute()
174+
v, _, err := adminApi.OAuth2API.AcceptOAuth2LogoutRequest(ctx).LogoutChallenge(challenge).Execute()
174175
require.NoError(t, err)
175176
require.NotEmpty(t, v.RedirectTo)
176177
http.Redirect(w, r, v.RedirectTo, http.StatusFound)
@@ -244,7 +245,7 @@ func TestLogoutFlows(t *testing.T) {
244245
acceptLoginAs(t, subject)
245246

246247
wg := newWg(2)
247-
setupCheckAndAcceptLogoutHandler(t, wg, func(t *testing.T, res *hydra.OAuth2LogoutRequest, err error) {
248+
setupCheckAndAcceptLogoutHandler(t, wg, func(t *testing.T, challenge string, res *hydra.OAuth2LogoutRequest, err error) {
248249
require.NoError(t, err)
249250
assert.EqualValues(t, subject, *res.Subject)
250251
assert.NotEmpty(t, subject, res.Sid)
@@ -277,20 +278,19 @@ func TestLogoutFlows(t *testing.T) {
277278
acceptLoginAs(t, subject)
278279
browser := createBrowserWithSession(t, createSampleClient(t))
279280

280-
var logoutReq *hydra.OAuth2LogoutRequest
281-
setupCheckAndAcceptLogoutHandler(t, nil, func(t *testing.T, req *hydra.OAuth2LogoutRequest, err error) {
281+
var logoutChallenge string
282+
setupCheckAndAcceptLogoutHandler(t, nil, func(t *testing.T, challenge string, _ *hydra.OAuth2LogoutRequest, err error) {
282283
require.NoError(t, err)
283-
logoutReq = req
284+
logoutChallenge = challenge
284285
})
285286

286287
// run once to log out
287288
logoutAndExpectPostLogoutPage(t, browser, http.MethodGet, url.Values{}, defaultRedirectedMessage)
288289

289-
// run again to ensure that the logout challenge is invalid
290-
_, _, err := adminApi.OAuth2API.GetOAuth2LogoutRequest(ctx).LogoutChallenge(logoutReq.GetChallenge()).Execute()
291-
assert.Error(t, err)
290+
require.NotZero(t, logoutChallenge)
292291

293-
v, _, err := adminApi.OAuth2API.AcceptOAuth2LogoutRequest(ctx).LogoutChallenge(logoutReq.GetChallenge()).Execute()
292+
// double-submit: still works
293+
v, _, err := adminApi.OAuth2API.AcceptOAuth2LogoutRequest(ctx).LogoutChallenge(logoutChallenge).Execute()
294294
require.NoError(t, err)
295295
require.NotEmpty(t, v.RedirectTo)
296296

@@ -485,7 +485,7 @@ func TestLogoutFlows(t *testing.T) {
485485
c := createSampleClient(t)
486486
acceptLoginAs(t, subject)
487487

488-
setupCheckAndAcceptLogoutHandler(t, nil, func(t *testing.T, res *hydra.OAuth2LogoutRequest, err error) {
488+
setupCheckAndAcceptLogoutHandler(t, nil, func(t *testing.T, _ string, _ *hydra.OAuth2LogoutRequest, _ error) {
489489
t.Fatalf("Logout should not have been called")
490490
})
491491
browser := createBrowserWithSession(t, c)

‎consent/strategy_oauth_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func TestStrategyLoginConsentNext(t *testing.T) {
5353
adminClient := hydra.NewAPIClient(hydra.NewConfiguration())
5454
adminClient.GetConfig().Servers = hydra.ServerConfigurations{{URL: adminTS.URL}}
5555

56-
oauth2Config := func(t *testing.T, c *client.Client) *oauth2.Config {
56+
oauth2Config := func(_ *testing.T, c *client.Client) *oauth2.Config {
5757
return &oauth2.Config{
5858
ClientID: c.GetID(),
5959
ClientSecret: c.Secret,

0 commit comments

Comments
 (0)