Skip to content

Commit 78e5e50

Browse files
alnralnr
committed
feat: stateless logout
1 parent 1520260 commit 78e5e50

26 files changed

+123
-526
lines changed

‎consent/handler.go

+5-18
Original file line numberDiff line numberDiff line change
@@ -927,9 +927,7 @@ func (h *Handler) rejectOAuth2ConsentRequest(w http.ResponseWriter, r *http.Requ
927927
// Accept OAuth 2.0 Logout Request
928928
//
929929
// swagger:parameters acceptOAuth2LogoutRequest
930-
//
931-
//lint:ignore U1000 Used to generate Swagger and OpenAPI definitions
932-
type acceptOAuth2LogoutRequest struct {
930+
type _ struct {
933931
// OAuth 2.0 Logout Request Challenge
934932
//
935933
// in: query
@@ -959,23 +957,21 @@ func (h *Handler) acceptOAuth2LogoutRequest(w http.ResponseWriter, r *http.Reque
959957
r.URL.Query().Get("challenge"),
960958
)
961959

962-
c, err := h.r.ConsentManager().AcceptLogoutRequest(r.Context(), challenge)
960+
verifier, err := h.r.ConsentManager().AcceptLogoutRequest(r.Context(), challenge)
963961
if err != nil {
964962
h.r.Writer().WriteError(w, r, err)
965963
return
966964
}
967965

968966
h.r.Writer().Write(w, r, &flow.OAuth2RedirectTo{
969-
RedirectTo: urlx.SetQuery(urlx.AppendPaths(h.c.PublicURL(r.Context()), "/oauth2/sessions/logout"), url.Values{"logout_verifier": {c.Verifier}}).String(),
967+
RedirectTo: urlx.SetQuery(urlx.AppendPaths(h.c.PublicURL(r.Context()), "/oauth2/sessions/logout"), url.Values{"logout_verifier": {verifier}}).String(),
970968
})
971969
}
972970

973971
// Reject OAuth 2.0 Logout Request
974972
//
975973
// swagger:parameters rejectOAuth2LogoutRequest
976-
//
977-
//lint:ignore U1000 Used to generate Swagger and OpenAPI definitions
978-
type rejectOAuth2LogoutRequest struct {
974+
type _ struct {
979975
// in: query
980976
// required: true
981977
Challenge string `json:"logout_challenge"`
@@ -1015,9 +1011,7 @@ func (h *Handler) rejectOAuth2LogoutRequest(w http.ResponseWriter, r *http.Reque
10151011
// Get OAuth 2.0 Logout Request
10161012
//
10171013
// swagger:parameters getOAuth2LogoutRequest
1018-
//
1019-
//lint:ignore U1000 Used to generate Swagger and OpenAPI definitions
1020-
type getOAuth2LogoutRequest struct {
1014+
type _ struct {
10211015
// in: query
10221016
// required: true
10231017
Challenge string `json:"logout_challenge"`
@@ -1055,12 +1049,5 @@ func (h *Handler) getOAuth2LogoutRequest(w http.ResponseWriter, r *http.Request,
10551049
request.Client.Secret = ""
10561050
}
10571051

1058-
if request.WasHandled {
1059-
h.r.Writer().WriteCode(w, r, http.StatusGone, &flow.OAuth2RedirectTo{
1060-
RedirectTo: request.RequestURL,
1061-
})
1062-
return
1063-
}
1064-
10651052
h.r.Writer().Write(w, r, request)
10661053
}

‎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
@@ -56,9 +56,9 @@ type (
5656
ListUserAuthenticatedClientsWithFrontChannelLogout(ctx context.Context, subject, sid string) ([]client.Client, error)
5757
ListUserAuthenticatedClientsWithBackChannelLogout(ctx context.Context, subject, sid string) ([]client.Client, error)
5858

59-
CreateLogoutRequest(ctx context.Context, request *flow.LogoutRequest) error
59+
CreateLogoutChallenge(ctx context.Context, request *flow.LogoutRequest) (challenge string, err error)
6060
GetLogoutRequest(ctx context.Context, challenge string) (*flow.LogoutRequest, error)
61-
AcceptLogoutRequest(ctx context.Context, challenge string) (*flow.LogoutRequest, error)
61+
AcceptLogoutRequest(ctx context.Context, challenge string) (verifier string, err error)
6262
RejectLogoutRequest(ctx context.Context, challenge string) error
6363
VerifyAndInvalidateLogoutRequest(ctx context.Context, verifier string) (*flow.LogoutRequest, error)
6464
}

‎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().Round(time.Second)
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

+14-13
Original file line numberDiff line numberDiff line change
@@ -16,21 +16,19 @@ import (
1616
"testing"
1717
"time"
1818

19-
"github.com/ory/hydra/v2/internal/kratos"
20-
"github.com/ory/x/pointerx"
21-
2219
"github.com/stretchr/testify/assert"
2320
"github.com/stretchr/testify/require"
2421
"github.com/tidwall/gjson"
2522

2623
jwtgo "github.com/ory/fosite/token/jwt"
27-
2824
hydra "github.com/ory/hydra-client-go/v2"
2925
"github.com/ory/hydra/v2/client"
3026
"github.com/ory/hydra/v2/driver/config"
27+
"github.com/ory/hydra/v2/internal/kratos"
3128
"github.com/ory/hydra/v2/internal/testhelpers"
3229
"github.com/ory/x/contextx"
3330
"github.com/ory/x/ioutilx"
31+
"github.com/ory/x/pointerx"
3432
)
3533

3634
func TestLogoutFlows(t *testing.T) {
@@ -163,14 +161,17 @@ func TestLogoutFlows(t *testing.T) {
163161
defer wg.Done()
164162
}
165163

166-
res, _, err := adminApi.OAuth2API.GetOAuth2LogoutRequest(ctx).LogoutChallenge(r.URL.Query().Get("logout_challenge")).Execute()
164+
challenge := r.URL.Query().Get("logout_challenge")
165+
res, _, err := adminApi.OAuth2API.GetOAuth2LogoutRequest(ctx).LogoutChallenge(challenge).Execute()
167166
if cb != nil {
168167
cb(t, res, err)
169168
} else {
170169
require.NoError(t, err)
171170
}
171+
require.NotNil(t, res)
172+
require.NotNil(t, res.Challenge)
172173

173-
v, _, err := adminApi.OAuth2API.AcceptOAuth2LogoutRequest(ctx).LogoutChallenge(r.URL.Query().Get("logout_challenge")).Execute()
174+
v, _, err := adminApi.OAuth2API.AcceptOAuth2LogoutRequest(ctx).LogoutChallenge(*res.Challenge).Execute()
174175
require.NoError(t, err)
175176
require.NotEmpty(t, v.RedirectTo)
176177
http.Redirect(w, r, v.RedirectTo, http.StatusFound)
@@ -277,20 +278,20 @@ func TestLogoutFlows(t *testing.T) {
277278
acceptLoginAs(t, subject)
278279
browser := createBrowserWithSession(t, createSampleClient(t))
279280

280-
var logoutReq *hydra.OAuth2LogoutRequest
281+
var logoutChallenge string
281282
setupCheckAndAcceptLogoutHandler(t, nil, func(t *testing.T, req *hydra.OAuth2LogoutRequest, err error) {
282283
require.NoError(t, err)
283-
logoutReq = req
284+
require.NotNil(t, req.Challenge)
285+
logoutChallenge = *req.Challenge
284286
})
285287

286288
// run once to log out
287289
logoutAndExpectPostLogoutPage(t, browser, http.MethodGet, url.Values{}, defaultRedirectedMessage)
288290

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)
291+
require.NotZero(t, logoutChallenge)
292292

293-
v, _, err := adminApi.OAuth2API.AcceptOAuth2LogoutRequest(ctx).LogoutChallenge(logoutReq.GetChallenge()).Execute()
293+
// double-submit: still works
294+
v, _, err := adminApi.OAuth2API.AcceptOAuth2LogoutRequest(ctx).LogoutChallenge(logoutChallenge).Execute()
294295
require.NoError(t, err)
295296
require.NotEmpty(t, v.RedirectTo)
296297

@@ -485,7 +486,7 @@ func TestLogoutFlows(t *testing.T) {
485486
c := createSampleClient(t)
486487
acceptLoginAs(t, subject)
487488

488-
setupCheckAndAcceptLogoutHandler(t, nil, func(t *testing.T, res *hydra.OAuth2LogoutRequest, err error) {
489+
setupCheckAndAcceptLogoutHandler(t, nil, func(t *testing.T, _ *hydra.OAuth2LogoutRequest, _ error) {
489490
t.Fatalf("Logout should not have been called")
490491
})
491492
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)