Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: stateless logout #3938

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: stateless logout #3938

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 5 additions & 18 deletions consent/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -911,9 +911,7 @@ func (h *Handler) rejectOAuth2ConsentRequest(w http.ResponseWriter, r *http.Requ
// Accept OAuth 2.0 Logout Request
//
// swagger:parameters acceptOAuth2LogoutRequest
//
//lint:ignore U1000 Used to generate Swagger and OpenAPI definitions
type acceptOAuth2LogoutRequest struct {
type _ struct {
// OAuth 2.0 Logout Request Challenge
//
// in: query
Expand Down Expand Up @@ -943,23 +941,21 @@ func (h *Handler) acceptOAuth2LogoutRequest(w http.ResponseWriter, r *http.Reque
r.URL.Query().Get("challenge"),
)

c, err := h.r.ConsentManager().AcceptLogoutRequest(r.Context(), challenge)
verifier, err := h.r.ConsentManager().AcceptLogoutRequest(r.Context(), challenge)
if err != nil {
h.r.Writer().WriteError(w, r, err)
return
}

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

// Reject OAuth 2.0 Logout Request
//
// swagger:parameters rejectOAuth2LogoutRequest
//
//lint:ignore U1000 Used to generate Swagger and OpenAPI definitions
type rejectOAuth2LogoutRequest struct {
type _ struct {
// in: query
// required: true
Challenge string `json:"logout_challenge"`
Expand Down Expand Up @@ -999,9 +995,7 @@ func (h *Handler) rejectOAuth2LogoutRequest(w http.ResponseWriter, r *http.Reque
// Get OAuth 2.0 Logout Request
//
// swagger:parameters getOAuth2LogoutRequest
//
//lint:ignore U1000 Used to generate Swagger and OpenAPI definitions
type getOAuth2LogoutRequest struct {
type _ struct {
// in: query
// required: true
Challenge string `json:"logout_challenge"`
Expand Down Expand Up @@ -1039,13 +1033,6 @@ func (h *Handler) getOAuth2LogoutRequest(w http.ResponseWriter, r *http.Request,
request.Client.Secret = ""
}

if request.WasHandled {
h.r.Writer().WriteCode(w, r, http.StatusGone, &flow.OAuth2RedirectTo{
RedirectTo: request.RequestURL,
})
return
}

h.r.Writer().Write(w, r, request)
}

Expand Down
55 changes: 0 additions & 55 deletions consent/handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,61 +27,6 @@ import (
"github.com/ory/x/sqlxx"
)

func TestGetLogoutRequest(t *testing.T) {
for k, tc := range []struct {
exists bool
handled bool
status int
}{
{false, false, http.StatusNotFound},
{true, false, http.StatusOK},
{true, true, http.StatusGone},
} {
t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
ctx := context.Background()
key := fmt.Sprint(k)
challenge := "challenge" + key
requestURL := "http://192.0.2.1"

conf := testhelpers.NewConfigurationWithDefaults()
reg := testhelpers.NewRegistryMemory(t, conf, &contextx.Default{})

if tc.exists {
cl := &client.Client{ID: "client" + key}
require.NoError(t, reg.ClientManager().CreateClient(ctx, cl))
require.NoError(t, reg.ConsentManager().CreateLogoutRequest(context.TODO(), &flow.LogoutRequest{
Client: cl,
ID: challenge,
WasHandled: tc.handled,
RequestURL: requestURL,
}))
}

h := NewHandler(reg, conf)
r := x.NewRouterAdmin(conf.AdminURL)
h.SetRoutes(r)
ts := httptest.NewServer(r)
defer ts.Close()

c := &http.Client{}
resp, err := c.Get(ts.URL + "/admin" + LogoutPath + "?challenge=" + challenge)
require.NoError(t, err)
require.EqualValues(t, tc.status, resp.StatusCode)

if tc.handled {
var result flow.OAuth2RedirectTo
require.NoError(t, json.NewDecoder(resp.Body).Decode(&result))
require.Equal(t, requestURL, result.RedirectTo)
} else if tc.exists {
var result flow.LogoutRequest
require.NoError(t, json.NewDecoder(resp.Body).Decode(&result))
require.Equal(t, challenge, result.ID)
require.Equal(t, requestURL, result.RequestURL)
}
})
}
}

func TestGetLoginRequest(t *testing.T) {
for k, tc := range []struct {
exists bool
Expand Down
4 changes: 2 additions & 2 deletions consent/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ type (
ListUserAuthenticatedClientsWithFrontChannelLogout(ctx context.Context, subject, sid string) ([]client.Client, error)
ListUserAuthenticatedClientsWithBackChannelLogout(ctx context.Context, subject, sid string) ([]client.Client, error)

CreateLogoutRequest(ctx context.Context, request *flow.LogoutRequest) error
CreateLogoutChallenge(ctx context.Context, request *flow.LogoutRequest) (challenge string, err error)
GetLogoutRequest(ctx context.Context, challenge string) (*flow.LogoutRequest, error)
AcceptLogoutRequest(ctx context.Context, challenge string) (*flow.LogoutRequest, error)
AcceptLogoutRequest(ctx context.Context, challenge string) (verifier string, err error)
RejectLogoutRequest(ctx context.Context, challenge string) error
VerifyAndInvalidateLogoutRequest(ctx context.Context, verifier string) (*flow.LogoutRequest, error)
}
Expand Down
27 changes: 13 additions & 14 deletions consent/sdk_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,13 +147,6 @@ func TestSDK(t *testing.T) {
_, err = m.VerifyAndInvalidateConsentRequest(context.Background(), consentVerifier(cr4Flow))
require.NoError(t, err)

lur1 := test.MockLogoutRequest("testsdk-1", true, network)
require.NoError(t, reg.ClientManager().CreateClient(context.Background(), lur1.Client))
require.NoError(t, m.CreateLogoutRequest(context.Background(), lur1))

lur2 := test.MockLogoutRequest("testsdk-2", false, network)
require.NoError(t, m.CreateLogoutRequest(context.Background(), lur2))

cr1.ID = consentChallenge(cr1Flow)
crGot := execute[hydra.OAuth2ConsentRequest](t, sdk.OAuth2API.GetOAuth2ConsentRequest(ctx).ConsentChallenge(cr1.ID))
compareSDKConsentRequest(t, cr1, *crGot)
Expand Down Expand Up @@ -213,19 +206,25 @@ func TestSDK(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, 0, len(csGot))

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

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

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

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

func compareSDKLoginRequest(t *testing.T, expected *LoginRequest, got hydra.OAuth2LoginRequest) {
Expand Down
62 changes: 29 additions & 33 deletions consent/strategy_default.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,13 @@ import (
"github.com/sirupsen/logrus"
"go.opentelemetry.io/otel/trace"

"github.com/ory/hydra/v2/flow"
"github.com/ory/hydra/v2/oauth2/flowctx"

"github.com/ory/fosite"
"github.com/ory/fosite/handler/openid"
"github.com/ory/fosite/token/jwt"
"github.com/ory/hydra/v2/client"
"github.com/ory/hydra/v2/driver/config"
"github.com/ory/hydra/v2/flow"
"github.com/ory/hydra/v2/oauth2/flowctx"
"github.com/ory/hydra/v2/x"
"github.com/ory/x/errorsx"
"github.com/ory/x/mapx"
Expand Down Expand Up @@ -855,21 +854,18 @@ func (s *DefaultStrategy) issueLogoutVerifier(ctx context.Context, w http.Respon
return nil, err
}

challenge := uuid.New()
if err := s.r.ConsentManager().CreateLogoutRequest(r.Context(), &flow.LogoutRequest{
RequestURL: r.URL.String(),
ID: challenge,
Subject: session.Subject,
SessionID: session.ID,
Verifier: uuid.New(),
RequestedAt: sqlxx.NullTime(time.Now().UTC().Round(time.Second)),
ExpiresAt: sqlxx.NullTime(time.Now().UTC().Round(time.Second).Add(s.c.ConsentRequestMaxAge(ctx))),
RPInitiated: false,

// PostLogoutRedirectURI is set to the value from config.Provider().LogoutRedirectURL()
now := time.Now().UTC().Round(time.Second)
challenge, err := s.r.ConsentManager().CreateLogoutChallenge(ctx, &flow.LogoutRequest{
RequestURL: r.URL.String(),
Subject: session.Subject,
SessionID: session.ID,
RequestedAt: now,
ExpiresAt: now.Add(s.c.ConsentRequestMaxAge(ctx)),
RPInitiated: false,
PostLogoutRedirectURI: redir,
}); err != nil {
return nil, err
})
if err != nil {
return nil, errors.WithStack(err)
}

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

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

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

challenge := uuid.New()
if err := s.r.ConsentManager().CreateLogoutRequest(r.Context(), &flow.LogoutRequest{
RequestURL: r.URL.String(),
ID: challenge,
SessionID: hintSid,
Subject: session.Subject,
Verifier: uuid.New(),
Client: cl,
RPInitiated: true,

// PostLogoutRedirectURI is set to the value from config.Provider().LogoutRedirectURL()
now = time.Now().UTC().Round(time.Second)
challenge, err := s.r.ConsentManager().CreateLogoutChallenge(ctx, &flow.LogoutRequest{
RequestURL: r.URL.String(),
Subject: session.Subject,
SessionID: hintSid,
RequestedAt: now,
ExpiresAt: now.Add(s.c.ConsentRequestMaxAge(ctx)),
RPInitiated: true,
PostLogoutRedirectURI: redir,
}); err != nil {
return nil, err
Client: cl,
})
if err != nil {
return nil, errors.WithStack(err)
}

http.Redirect(w, r, urlx.SetQuery(s.c.LogoutURL(ctx), url.Values{"logout_challenge": {challenge}}).String(), http.StatusFound)
Expand Down
26 changes: 13 additions & 13 deletions consent/strategy_logout_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,20 +157,21 @@ func TestLogoutFlows(t *testing.T) {
return &wg
}

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

res, _, err := adminApi.OAuth2API.GetOAuth2LogoutRequest(ctx).LogoutChallenge(r.URL.Query().Get("logout_challenge")).Execute()
challenge := r.URL.Query().Get("logout_challenge")
res, _, err := adminApi.OAuth2API.GetOAuth2LogoutRequest(ctx).LogoutChallenge(challenge).Execute()
if cb != nil {
cb(t, res, err)
cb(t, challenge, res, err)
} else {
require.NoError(t, err)
}

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

wg := newWg(2)
setupCheckAndAcceptLogoutHandler(t, wg, func(t *testing.T, res *hydra.OAuth2LogoutRequest, err error) {
setupCheckAndAcceptLogoutHandler(t, wg, func(t *testing.T, challenge string, res *hydra.OAuth2LogoutRequest, err error) {
require.NoError(t, err)
assert.EqualValues(t, subject, *res.Subject)
assert.NotEmpty(t, subject, res.Sid)
Expand Down Expand Up @@ -277,20 +278,19 @@ func TestLogoutFlows(t *testing.T) {
acceptLoginAs(t, subject)
browser := createBrowserWithSession(t, createSampleClient(t))

var logoutReq *hydra.OAuth2LogoutRequest
setupCheckAndAcceptLogoutHandler(t, nil, func(t *testing.T, req *hydra.OAuth2LogoutRequest, err error) {
var logoutChallenge string
setupCheckAndAcceptLogoutHandler(t, nil, func(t *testing.T, challenge string, _ *hydra.OAuth2LogoutRequest, err error) {
require.NoError(t, err)
logoutReq = req
logoutChallenge = challenge
})

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

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

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

Expand Down Expand Up @@ -485,7 +485,7 @@ func TestLogoutFlows(t *testing.T) {
c := createSampleClient(t)
acceptLoginAs(t, subject)

setupCheckAndAcceptLogoutHandler(t, nil, func(t *testing.T, res *hydra.OAuth2LogoutRequest, err error) {
setupCheckAndAcceptLogoutHandler(t, nil, func(t *testing.T, _ string, _ *hydra.OAuth2LogoutRequest, _ error) {
t.Fatalf("Logout should not have been called")
})
browser := createBrowserWithSession(t, c)
Expand Down
2 changes: 1 addition & 1 deletion consent/strategy_oauth_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestStrategyLoginConsentNext(t *testing.T) {
adminClient := hydra.NewAPIClient(hydra.NewConfiguration())
adminClient.GetConfig().Servers = hydra.ServerConfigurations{{URL: adminTS.URL}}

oauth2Config := func(t *testing.T, c *client.Client) *oauth2.Config {
oauth2Config := func(_ *testing.T, c *client.Client) *oauth2.Config {
return &oauth2.Config{
ClientID: c.GetID(),
ClientSecret: c.Secret,
Expand Down
Loading
Loading