From efa5d5ac95abcfb915491dafc899b14f8843fbbf Mon Sep 17 00:00:00 2001 From: Henning Perl Date: Mon, 28 Oct 2024 14:23:32 +0100 Subject: [PATCH 1/9] feat: support more claims in password grant --- go.mod | 5 +- go.sum | 4 +- internal/kratos/fake_kratos.go | 14 +-- internal/kratos/kratos.go | 14 +-- oauth2/handler.go | 9 +- oauth2/oauth2_rop_test.go | 103 +++++++++++++++++++++- oauth2/session.go | 14 +++ persistence/sql/persister_authenticate.go | 12 ++- 8 files changed, 148 insertions(+), 27 deletions(-) diff --git a/go.mod b/go.mod index 08033c6b508..2b7dadd7de0 100644 --- a/go.mod +++ b/go.mod @@ -33,7 +33,7 @@ require ( github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 github.com/oleiade/reflections v1.0.1 github.com/ory/analytics-go/v5 v5.0.1 - github.com/ory/fosite v0.47.0 + github.com/ory/fosite v0.47.1-0.20241028132122-b35b62fed16e github.com/ory/go-acc v0.2.9-0.20230103102148-6b1c9a70dbbe github.com/ory/graceful v0.1.3 github.com/ory/herodot v0.10.3-0.20230626083119-d7e5192f0d88 @@ -69,8 +69,6 @@ require ( golang.org/x/tools v0.23.0 ) -require github.com/hashicorp/go-cleanhttp v0.5.2 // indirect - require ( code.dny.dev/ssrf v0.2.0 // indirect dario.cat/mergo v1.0.0 // indirect @@ -147,6 +145,7 @@ require ( github.com/gorilla/websocket v1.5.0 // indirect github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 // indirect github.com/grpc-ecosystem/grpc-gateway/v2 v2.20.0 // indirect + github.com/hashicorp/go-cleanhttp v0.5.2 // indirect github.com/hashicorp/hcl v1.0.0 // indirect github.com/huandu/xstrings v1.4.0 // indirect github.com/imdario/mergo v0.3.16 // indirect diff --git a/go.sum b/go.sum index 53e07d5b53d..d0132d1d821 100644 --- a/go.sum +++ b/go.sum @@ -387,8 +387,8 @@ github.com/ory/analytics-go/v5 v5.0.1 h1:LX8T5B9FN8KZXOtxgN+R3I4THRRVB6+28IKgKBp github.com/ory/analytics-go/v5 v5.0.1/go.mod h1:lWCiCjAaJkKfgR/BN5DCLMol8BjKS1x+4jxBxff/FF0= github.com/ory/dockertest/v3 v3.10.1-0.20240704115616-d229e74b748d h1:By96ZSVuH5LyjXLVVMfvJoLVGHaT96LdOnwgFSLVf0E= github.com/ory/dockertest/v3 v3.10.1-0.20240704115616-d229e74b748d/go.mod h1:F2FIjwwAk6CsNAs//B8+aPFQF0t84pbM8oliyNXwQrk= -github.com/ory/fosite v0.47.0 h1:Iqu5uhx54JqZQPn2hRhqjESrmRRyQb00uJjfEi1a1QI= -github.com/ory/fosite v0.47.0/go.mod h1:5U6c9nOLxyTdD/qrFv7N88TSxkdk5Wq8NzvB7UViDP0= +github.com/ory/fosite v0.47.1-0.20241028132122-b35b62fed16e h1:aRhPoQt0QFrjQVrNDGiGQcT9cY/o8iqqBkMdoiyznjI= +github.com/ory/fosite v0.47.1-0.20241028132122-b35b62fed16e/go.mod h1:5U6c9nOLxyTdD/qrFv7N88TSxkdk5Wq8NzvB7UViDP0= github.com/ory/go-acc v0.2.9-0.20230103102148-6b1c9a70dbbe h1:rvu4obdvqR0fkSIJ8IfgzKOWwZ5kOT2UNfLq81Qk7rc= github.com/ory/go-acc v0.2.9-0.20230103102148-6b1c9a70dbbe/go.mod h1:z4n3u6as84LbV4YmgjHhnwtccQqzf4cZlSk9f1FhygI= github.com/ory/go-convenience v0.1.0 h1:zouLKfF2GoSGnJwGq+PE/nJAE6dj2Zj5QlTgmMTsTS8= diff --git a/internal/kratos/fake_kratos.go b/internal/kratos/fake_kratos.go index bffad0696f0..2303320c305 100644 --- a/internal/kratos/fake_kratos.go +++ b/internal/kratos/fake_kratos.go @@ -7,6 +7,7 @@ import ( "context" "github.com/ory/fosite" + client "github.com/ory/kratos-client-go" ) type ( @@ -17,9 +18,10 @@ type ( ) const ( - FakeSessionID = "fake-kratos-session-id" - FakeUsername = "fake-kratos-username" - FakePassword = "fake-kratos-password" // nolint: gosec + FakeSessionID = "fake-kratos-session-id" + FakeUsername = "fake-kratos-username" + FakePassword = "fake-kratos-password" // nolint: gosec + FakeIdentityID = "fake-kratos-identity-id" ) var _ Client = new(FakeKratos) @@ -35,11 +37,11 @@ func (f *FakeKratos) DisableSession(_ context.Context, identityProviderSessionID return nil } -func (f *FakeKratos) Authenticate(_ context.Context, username, password string) error { +func (f *FakeKratos) Authenticate(_ context.Context, username, password string) (*client.Session, error) { if username == FakeUsername && password == FakePassword { - return nil + return &client.Session{Identity: &client.Identity{Id: FakeIdentityID}}, nil } - return fosite.ErrNotFound + return nil, fosite.ErrNotFound } func (f *FakeKratos) Reset() { diff --git a/internal/kratos/kratos.go b/internal/kratos/kratos.go index c6d7be034ce..04e8fbfcdfb 100644 --- a/internal/kratos/kratos.go +++ b/internal/kratos/kratos.go @@ -31,7 +31,7 @@ type ( } Client interface { DisableSession(ctx context.Context, identityProviderSessionID string) error - Authenticate(ctx context.Context, name, secret string) error + Authenticate(ctx context.Context, name, secret string) (*client.Session, error) } Default struct { dependencies @@ -42,7 +42,7 @@ func New(d dependencies) Client { return &Default{dependencies: d} } -func (k *Default) Authenticate(ctx context.Context, name, secret string) (err error) { +func (k *Default) Authenticate(ctx context.Context, name, secret string) (session *client.Session, err error) { ctx, span := k.Tracer(ctx).Tracer().Start(ctx, "kratos.Authenticate") otelx.End(span, &err) @@ -52,17 +52,17 @@ func (k *Default) Authenticate(ctx context.Context, name, secret string) (err er span.SetAttributes(attribute.Bool("skipped", true)) span.SetAttributes(attribute.String("reason", "kratos public url not set")) - return errors.New("kratos public url not set") + return nil, errors.New("kratos public url not set") } kratos := k.newKratosClient(ctx, publicURL) flow, _, err := kratos.FrontendAPI.CreateNativeLoginFlow(ctx).Execute() if err != nil { - return err + return nil, err } - _, _, err = kratos.FrontendAPI.UpdateLoginFlow(ctx).Flow(flow.Id).UpdateLoginFlowBody(client.UpdateLoginFlowBody{ + res, _, err := kratos.FrontendAPI.UpdateLoginFlow(ctx).Flow(flow.Id).UpdateLoginFlowBody(client.UpdateLoginFlowBody{ UpdateLoginFlowWithPasswordMethod: &client.UpdateLoginFlowWithPasswordMethod{ Method: "password", Identifier: name, @@ -70,10 +70,10 @@ func (k *Default) Authenticate(ctx context.Context, name, secret string) (err er }, }).Execute() if err != nil { - return fosite.ErrNotFound.WithWrap(err) + return nil, fosite.ErrNotFound.WithWrap(err) } - return nil + return &res.Session, nil } func (k *Default) DisableSession(ctx context.Context, identityProviderSessionID string) (err error) { diff --git a/oauth2/handler.go b/oauth2/handler.go index e4b63d5a0ce..bdb97e2a5be 100644 --- a/oauth2/handler.go +++ b/oauth2/handler.go @@ -962,7 +962,8 @@ func (h *Handler) oauth2TokenExchange(w http.ResponseWriter, r *http.Request) { } if accessRequest.GetGrantTypes().ExactOne(string(fosite.GrantTypeClientCredentials)) || - accessRequest.GetGrantTypes().ExactOne(string(fosite.GrantTypeJWTBearer)) { + accessRequest.GetGrantTypes().ExactOne(string(fosite.GrantTypeJWTBearer)) || + accessRequest.GetGrantTypes().ExactOne(string(fosite.GrantTypePassword)) { var accessTokenKeyID string if h.c.AccessTokenStrategy(ctx, client.AccessTokenStrategySource(accessRequest.GetClient())) == "jwt" { accessTokenKeyID, err = h.r.AccessTokenJWTStrategy().GetPublicKeyID(ctx) @@ -975,9 +976,13 @@ func (h *Handler) oauth2TokenExchange(w http.ResponseWriter, r *http.Request) { } // only for client_credentials, otherwise Authentication is included in session - if accessRequest.GetGrantTypes().ExactOne("client_credentials") { + if accessRequest.GetGrantTypes().ExactOne(string(fosite.GrantTypeClientCredentials)) { session.Subject = accessRequest.GetClient().GetID() } + // only for password grant, otherwise Authentication is included in session + if accessRequest.GetGrantTypes().ExactOne(string(fosite.GrantTypePassword)) { + session.Subject = accessRequest.GetRequestForm().Get("username") + } session.ClientID = accessRequest.GetClient().GetID() session.KID = accessTokenKeyID session.DefaultSession.Claims.Issuer = h.c.IssuerURL(ctx).String() diff --git a/oauth2/oauth2_rop_test.go b/oauth2/oauth2_rop_test.go index 73946a25539..d2fcc6a3b7c 100644 --- a/oauth2/oauth2_rop_test.go +++ b/oauth2/oauth2_rop_test.go @@ -5,7 +5,11 @@ package oauth2_test import ( "context" + "encoding/json" + "net/http" + "net/http/httptest" "testing" + "time" "github.com/google/uuid" "github.com/stretchr/testify/assert" @@ -13,11 +17,16 @@ import ( "golang.org/x/oauth2" "github.com/ory/fosite/compose" + "github.com/ory/fosite/token/jwt" hydra "github.com/ory/hydra/v2/client" + "github.com/ory/hydra/v2/driver/config" + "github.com/ory/hydra/v2/flow" "github.com/ory/hydra/v2/fositex" "github.com/ory/hydra/v2/internal" "github.com/ory/hydra/v2/internal/kratos" "github.com/ory/hydra/v2/internal/testhelpers" + hydraoauth2 "github.com/ory/hydra/v2/oauth2" + "github.com/ory/hydra/v2/x" "github.com/ory/x/contextx" ) @@ -32,7 +41,12 @@ func TestResourceOwnerPasswordGrant(t *testing.T) { secret := uuid.New().String() client := &hydra.Client{ Secret: secret, - GrantTypes: []string{"password"}, + GrantTypes: []string{"password", "refresh_token"}, + Scope: "offline", + Lifespans: hydra.Lifespans{ + PasswordGrantAccessTokenLifespan: x.NullDuration{Duration: 1 * time.Hour, Valid: true}, + PasswordGrantRefreshTokenLifespan: x.NullDuration{Duration: 1 * time.Hour, Valid: true}, + }, } require.NoError(t, reg.ClientManager().CreateClient(ctx, client)) @@ -44,15 +58,96 @@ func TestResourceOwnerPasswordGrant(t *testing.T) { TokenURL: reg.Config().OAuth2TokenURL(ctx).String(), AuthStyle: oauth2.AuthStyleInHeader, }, + Scopes: []string{"offline"}, } + hs := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, r.Header.Get("Content-Type"), "application/json; charset=UTF-8") + assert.Equal(t, r.Header.Get("Authorization"), "Bearer secret value") + + var hookReq hydraoauth2.TokenHookRequest + require.NoError(t, json.NewDecoder(r.Body).Decode(&hookReq)) + require.NotEmpty(t, hookReq.Session) + require.Equal(t, kratos.FakeIdentityID, hookReq.Session.Extra["identity_id"]) + require.NotEmpty(t, hookReq.Request) + require.ElementsMatch(t, []string{}, hookReq.Request.GrantedAudience) + + claims := map[string]interface{}{ + "hooked": true, + "identity_id": kratos.FakeIdentityID, + } + if hookReq.Request.GrantTypes[0] == "refresh_token" { + claims["refreshed"] = true + } + + hookResp := hydraoauth2.TokenHookResponse{ + Session: flow.AcceptOAuth2ConsentRequestSession{ + AccessToken: claims, + IDToken: claims, + }, + } + + w.WriteHeader(http.StatusOK) + require.NoError(t, json.NewEncoder(w).Encode(&hookResp)) + })) + defer hs.Close() + + reg.Config().MustSet(ctx, config.KeyTokenHook, &config.HookConfig{ + URL: hs.URL, + Auth: &config.Auth{ + Type: "api_key", + Config: config.AuthConfig{ + In: "header", + Name: "Authorization", + Value: "Bearer secret value", + }, + }, + }) + reg.Config().MustSet(ctx, config.KeyAccessTokenStrategy, "jwt") + t.Run("case=get ROP grant token with valid username and password", func(t *testing.T) { token, err := oauth2Config.PasswordCredentialsToken(ctx, kratos.FakeUsername, kratos.FakePassword) require.NoError(t, err) require.NotEmpty(t, token.AccessToken) - i := testhelpers.IntrospectToken(t, oauth2Config, token.AccessToken, adminTS) - assert.True(t, i.Get("active").Bool(), "%s", i) - assert.EqualValues(t, oauth2Config.ClientID, i.Get("client_id").String(), "%s", i) + + // Access token should have hook and identity_id claims + jwtAT, err := jwt.Parse(token.AccessToken, func(token *jwt.Token) (interface{}, error) { + return reg.AccessTokenJWTStrategy().GetPublicKey(ctx) + }) + require.NoError(t, err) + assert.Equal(t, kratos.FakeUsername, jwtAT.Claims["sub"]) + assert.Equal(t, kratos.FakeIdentityID, jwtAT.Claims["ext"].(map[string]any)["identity_id"].(string)) + assert.True(t, jwtAT.Claims["ext"].(map[string]any)["hooked"].(bool)) + + t.Run("case=introspect token", func(t *testing.T) { + // Introspected token should have hook and identity_id claims + i := testhelpers.IntrospectToken(t, oauth2Config, token.AccessToken, adminTS) + assert.True(t, i.Get("active").Bool(), "%s", i) + assert.Equal(t, kratos.FakeUsername, i.Get("sub").String(), "%s", i) + assert.Equal(t, kratos.FakeIdentityID, i.Get("ext.identity_id").String(), "%s", i) + assert.True(t, i.Get("ext.hooked").Bool(), "%s", i) + assert.EqualValues(t, oauth2Config.ClientID, i.Get("client_id").String(), "%s", i) + }) + + t.Run("case=refresh token", func(t *testing.T) { + // Refreshed access token should have hook and identity_id claims + require.NotEmpty(t, token.RefreshToken) + token.Expiry = token.Expiry.Add(-time.Hour * 24) + refreshedToken, err := oauth2Config.TokenSource(context.Background(), token).Token() + require.NoError(t, err) + + require.NotEqual(t, token.AccessToken, refreshedToken.AccessToken) + require.NotEqual(t, token.RefreshToken, refreshedToken.RefreshToken) + + jwtAT, err := jwt.Parse(refreshedToken.AccessToken, func(token *jwt.Token) (interface{}, error) { + return reg.AccessTokenJWTStrategy().GetPublicKey(ctx) + }) + require.NoError(t, err) + assert.Equal(t, kratos.FakeUsername, jwtAT.Claims["sub"]) + assert.Equal(t, kratos.FakeIdentityID, jwtAT.Claims["ext"].(map[string]any)["identity_id"].(string)) + assert.True(t, jwtAT.Claims["ext"].(map[string]any)["hooked"].(bool)) + assert.True(t, jwtAT.Claims["ext"].(map[string]any)["refreshed"].(bool)) + }) }) t.Run("case=access denied for invalid password", func(t *testing.T) { diff --git a/oauth2/session.go b/oauth2/session.go index 6dc9039eb22..0630cb09142 100644 --- a/oauth2/session.go +++ b/oauth2/session.go @@ -194,3 +194,17 @@ func (s *Session) UnmarshalJSON(original []byte) (err error) { return nil } + +// GetExtraClaims implements ExtraClaimsSession for Session. +// The returned value can be modified in-place. +func (s *Session) GetExtraClaims() map[string]interface{} { + if s == nil { + return nil + } + + if s.Extra == nil { + s.Extra = make(map[string]interface{}) + } + + return s.Extra +} diff --git a/persistence/sql/persister_authenticate.go b/persistence/sql/persister_authenticate.go index 4fdc7eff0ae..2bc53858be3 100644 --- a/persistence/sql/persister_authenticate.go +++ b/persistence/sql/persister_authenticate.go @@ -3,8 +3,14 @@ package sql -import "context" +import ( + "context" +) -func (p *Persister) Authenticate(ctx context.Context, name, secret string) error { - return p.r.Kratos().Authenticate(ctx, name, secret) +func (p *Persister) Authenticate(ctx context.Context, name, secret string) (string, error) { + session, err := p.r.Kratos().Authenticate(ctx, name, secret) + if err != nil { + return "", err + } + return session.Identity.Id, nil } From 3c007fe99045fd2194fe92bca327f0db9699b324 Mon Sep 17 00:00:00 2001 From: Henning Perl Date: Tue, 29 Oct 2024 10:56:48 +0100 Subject: [PATCH 2/9] code review --- go.mod | 2 ++ oauth2/handler.go | 8 +++++++- oauth2/oauth2_rop_test.go | 30 ++++++++++++++---------------- 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/go.mod b/go.mod index 2b7dadd7de0..14bbe355481 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,8 @@ go 1.22 toolchain go1.22.5 +replace github.com/ory/fosite => ../fosite + replace github.com/ory/hydra-client-go/v2 => ./internal/httpclient replace github.com/gobuffalo/pop/v6 => github.com/ory/pop/v6 v6.2.0 diff --git a/oauth2/handler.go b/oauth2/handler.go index bdb97e2a5be..5e659d09c5e 100644 --- a/oauth2/handler.go +++ b/oauth2/handler.go @@ -981,7 +981,13 @@ func (h *Handler) oauth2TokenExchange(w http.ResponseWriter, r *http.Request) { } // only for password grant, otherwise Authentication is included in session if accessRequest.GetGrantTypes().ExactOne(string(fosite.GrantTypePassword)) { - session.Subject = accessRequest.GetRequestForm().Get("username") + if sess, ok := accessRequest.GetSession().(fosite.ExtraClaimsSession); ok { + sess.GetExtraClaims()["username"] = accessRequest.GetRequestForm().Get("username") + session.Subject = sess.GetExtraClaims()["identity_id"].(string) + delete(sess.GetExtraClaims(), "identity_id") + session.DefaultSession.Username = accessRequest.GetRequestForm().Get("username") + } + } session.ClientID = accessRequest.GetClient().GetID() session.KID = accessTokenKeyID diff --git a/oauth2/oauth2_rop_test.go b/oauth2/oauth2_rop_test.go index d2fcc6a3b7c..a07d142793f 100644 --- a/oauth2/oauth2_rop_test.go +++ b/oauth2/oauth2_rop_test.go @@ -36,7 +36,7 @@ func TestResourceOwnerPasswordGrant(t *testing.T) { reg := internal.NewMockedRegistry(t, &contextx.Default{}) reg.WithKratos(fakeKratos) reg.WithExtraFositeFactories([]fositex.Factory{compose.OAuth2ResourceOwnerPasswordCredentialsFactory}) - _, adminTS := testhelpers.NewOAuth2Server(ctx, t, reg) + publicTS, adminTS := testhelpers.NewOAuth2Server(ctx, t, reg) secret := uuid.New().String() client := &hydra.Client{ @@ -67,15 +67,12 @@ func TestResourceOwnerPasswordGrant(t *testing.T) { var hookReq hydraoauth2.TokenHookRequest require.NoError(t, json.NewDecoder(r.Body).Decode(&hookReq)) - require.NotEmpty(t, hookReq.Session) - require.Equal(t, kratos.FakeIdentityID, hookReq.Session.Extra["identity_id"]) - require.NotEmpty(t, hookReq.Request) - require.ElementsMatch(t, []string{}, hookReq.Request.GrantedAudience) - - claims := map[string]interface{}{ - "hooked": true, - "identity_id": kratos.FakeIdentityID, - } + assert.NotEmpty(t, hookReq.Session) + assert.NotEmpty(t, hookReq.Request) + assert.ElementsMatch(t, []string{}, hookReq.Request.GrantedAudience) + + claims := hookReq.Session.Extra + claims["hooked"] = true if hookReq.Request.GrantTypes[0] == "refresh_token" { claims["refreshed"] = true } @@ -115,16 +112,17 @@ func TestResourceOwnerPasswordGrant(t *testing.T) { return reg.AccessTokenJWTStrategy().GetPublicKey(ctx) }) require.NoError(t, err) - assert.Equal(t, kratos.FakeUsername, jwtAT.Claims["sub"]) - assert.Equal(t, kratos.FakeIdentityID, jwtAT.Claims["ext"].(map[string]any)["identity_id"].(string)) + assert.Equal(t, kratos.FakeUsername, jwtAT.Claims["ext"].(map[string]any)["username"]) + assert.Equal(t, kratos.FakeIdentityID, jwtAT.Claims["sub"]) + assert.Equal(t, publicTS.URL, jwtAT.Claims["iss"]) assert.True(t, jwtAT.Claims["ext"].(map[string]any)["hooked"].(bool)) t.Run("case=introspect token", func(t *testing.T) { // Introspected token should have hook and identity_id claims i := testhelpers.IntrospectToken(t, oauth2Config, token.AccessToken, adminTS) assert.True(t, i.Get("active").Bool(), "%s", i) - assert.Equal(t, kratos.FakeUsername, i.Get("sub").String(), "%s", i) - assert.Equal(t, kratos.FakeIdentityID, i.Get("ext.identity_id").String(), "%s", i) + assert.Equal(t, kratos.FakeUsername, i.Get("ext.username").String(), "%s", i) + assert.Equal(t, kratos.FakeIdentityID, i.Get("sub").String(), "%s", i) assert.True(t, i.Get("ext.hooked").Bool(), "%s", i) assert.EqualValues(t, oauth2Config.ClientID, i.Get("client_id").String(), "%s", i) }) @@ -143,8 +141,8 @@ func TestResourceOwnerPasswordGrant(t *testing.T) { return reg.AccessTokenJWTStrategy().GetPublicKey(ctx) }) require.NoError(t, err) - assert.Equal(t, kratos.FakeUsername, jwtAT.Claims["sub"]) - assert.Equal(t, kratos.FakeIdentityID, jwtAT.Claims["ext"].(map[string]any)["identity_id"].(string)) + assert.Equal(t, kratos.FakeIdentityID, jwtAT.Claims["sub"]) + assert.Equal(t, kratos.FakeUsername, jwtAT.Claims["ext"].(map[string]any)["username"]) assert.True(t, jwtAT.Claims["ext"].(map[string]any)["hooked"].(bool)) assert.True(t, jwtAT.Claims["ext"].(map[string]any)["refreshed"].(bool)) }) From ef93cb9e3cfa637780219e35c82d1f88b524e107 Mon Sep 17 00:00:00 2001 From: Henning Perl Date: Tue, 29 Oct 2024 11:05:11 +0100 Subject: [PATCH 3/9] chore: update snapshots --- oauth2/.snapshots/TestHandlerWellKnown-hsm_enabled=false.json | 3 ++- oauth2/.snapshots/TestHandlerWellKnown-hsm_enabled=true.json | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/oauth2/.snapshots/TestHandlerWellKnown-hsm_enabled=false.json b/oauth2/.snapshots/TestHandlerWellKnown-hsm_enabled=false.json index 215fa018214..5bc92ec79a5 100644 --- a/oauth2/.snapshots/TestHandlerWellKnown-hsm_enabled=false.json +++ b/oauth2/.snapshots/TestHandlerWellKnown-hsm_enabled=false.json @@ -63,7 +63,8 @@ "require_request_uri_registration": true, "response_modes_supported": [ "query", - "fragment" + "fragment", + "form_post" ], "response_types_supported": [ "code", diff --git a/oauth2/.snapshots/TestHandlerWellKnown-hsm_enabled=true.json b/oauth2/.snapshots/TestHandlerWellKnown-hsm_enabled=true.json index 215fa018214..5bc92ec79a5 100644 --- a/oauth2/.snapshots/TestHandlerWellKnown-hsm_enabled=true.json +++ b/oauth2/.snapshots/TestHandlerWellKnown-hsm_enabled=true.json @@ -63,7 +63,8 @@ "require_request_uri_registration": true, "response_modes_supported": [ "query", - "fragment" + "fragment", + "form_post" ], "response_types_supported": [ "code", From 39e1c61906eee6a45f36b72d7dda023557314355 Mon Sep 17 00:00:00 2001 From: Henning Perl Date: Tue, 29 Oct 2024 11:13:38 +0100 Subject: [PATCH 4/9] support audience claim --- oauth2/handler.go | 4 ++++ oauth2/oauth2_rop_test.go | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/oauth2/handler.go b/oauth2/handler.go index 5e659d09c5e..0e820ec3f48 100644 --- a/oauth2/handler.go +++ b/oauth2/handler.go @@ -988,6 +988,10 @@ func (h *Handler) oauth2TokenExchange(w http.ResponseWriter, r *http.Request) { session.DefaultSession.Username = accessRequest.GetRequestForm().Get("username") } + // Also add audience claims + for _, aud := range accessRequest.GetClient().GetAudience() { + accessRequest.GrantAudience(aud) + } } session.ClientID = accessRequest.GetClient().GetID() session.KID = accessTokenKeyID diff --git a/oauth2/oauth2_rop_test.go b/oauth2/oauth2_rop_test.go index a07d142793f..ba113b36c2a 100644 --- a/oauth2/oauth2_rop_test.go +++ b/oauth2/oauth2_rop_test.go @@ -28,6 +28,7 @@ import ( hydraoauth2 "github.com/ory/hydra/v2/oauth2" "github.com/ory/hydra/v2/x" "github.com/ory/x/contextx" + "github.com/ory/x/sqlxx" ) func TestResourceOwnerPasswordGrant(t *testing.T) { @@ -39,10 +40,12 @@ func TestResourceOwnerPasswordGrant(t *testing.T) { publicTS, adminTS := testhelpers.NewOAuth2Server(ctx, t, reg) secret := uuid.New().String() + audience := "https://aud.example.com" client := &hydra.Client{ Secret: secret, GrantTypes: []string{"password", "refresh_token"}, Scope: "offline", + Audience: sqlxx.StringSliceJSONFormat{audience}, Lifespans: hydra.Lifespans{ PasswordGrantAccessTokenLifespan: x.NullDuration{Duration: 1 * time.Hour, Valid: true}, PasswordGrantRefreshTokenLifespan: x.NullDuration{Duration: 1 * time.Hour, Valid: true}, @@ -116,6 +119,7 @@ func TestResourceOwnerPasswordGrant(t *testing.T) { assert.Equal(t, kratos.FakeIdentityID, jwtAT.Claims["sub"]) assert.Equal(t, publicTS.URL, jwtAT.Claims["iss"]) assert.True(t, jwtAT.Claims["ext"].(map[string]any)["hooked"].(bool)) + assert.Equal(t, oauth2Config.ClientID, audience, jwtAT.Claims["aud"]) t.Run("case=introspect token", func(t *testing.T) { // Introspected token should have hook and identity_id claims From c7864d613add19f82131b66492992aa4158249f1 Mon Sep 17 00:00:00 2001 From: Henning Perl Date: Tue, 29 Oct 2024 11:24:41 +0100 Subject: [PATCH 5/9] fix --- go.mod | 4 +--- go.sum | 2 ++ oauth2/oauth2_rop_test.go | 7 +++---- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/go.mod b/go.mod index 14bbe355481..54875b14f34 100644 --- a/go.mod +++ b/go.mod @@ -4,8 +4,6 @@ go 1.22 toolchain go1.22.5 -replace github.com/ory/fosite => ../fosite - replace github.com/ory/hydra-client-go/v2 => ./internal/httpclient replace github.com/gobuffalo/pop/v6 => github.com/ory/pop/v6 v6.2.0 @@ -35,7 +33,7 @@ require ( github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 github.com/oleiade/reflections v1.0.1 github.com/ory/analytics-go/v5 v5.0.1 - github.com/ory/fosite v0.47.1-0.20241028132122-b35b62fed16e + github.com/ory/fosite v0.47.1-0.20241028135452-199f8ab66014 github.com/ory/go-acc v0.2.9-0.20230103102148-6b1c9a70dbbe github.com/ory/graceful v0.1.3 github.com/ory/herodot v0.10.3-0.20230626083119-d7e5192f0d88 diff --git a/go.sum b/go.sum index d0132d1d821..b17281f7842 100644 --- a/go.sum +++ b/go.sum @@ -389,6 +389,8 @@ github.com/ory/dockertest/v3 v3.10.1-0.20240704115616-d229e74b748d h1:By96ZSVuH5 github.com/ory/dockertest/v3 v3.10.1-0.20240704115616-d229e74b748d/go.mod h1:F2FIjwwAk6CsNAs//B8+aPFQF0t84pbM8oliyNXwQrk= github.com/ory/fosite v0.47.1-0.20241028132122-b35b62fed16e h1:aRhPoQt0QFrjQVrNDGiGQcT9cY/o8iqqBkMdoiyznjI= github.com/ory/fosite v0.47.1-0.20241028132122-b35b62fed16e/go.mod h1:5U6c9nOLxyTdD/qrFv7N88TSxkdk5Wq8NzvB7UViDP0= +github.com/ory/fosite v0.47.1-0.20241028135452-199f8ab66014 h1:vtcEvjeArxH3CayG8Gn5y3mCzo1gibu74rZhvvveANE= +github.com/ory/fosite v0.47.1-0.20241028135452-199f8ab66014/go.mod h1:5U6c9nOLxyTdD/qrFv7N88TSxkdk5Wq8NzvB7UViDP0= github.com/ory/go-acc v0.2.9-0.20230103102148-6b1c9a70dbbe h1:rvu4obdvqR0fkSIJ8IfgzKOWwZ5kOT2UNfLq81Qk7rc= github.com/ory/go-acc v0.2.9-0.20230103102148-6b1c9a70dbbe/go.mod h1:z4n3u6as84LbV4YmgjHhnwtccQqzf4cZlSk9f1FhygI= github.com/ory/go-convenience v0.1.0 h1:zouLKfF2GoSGnJwGq+PE/nJAE6dj2Zj5QlTgmMTsTS8= diff --git a/oauth2/oauth2_rop_test.go b/oauth2/oauth2_rop_test.go index ba113b36c2a..4adb4904452 100644 --- a/oauth2/oauth2_rop_test.go +++ b/oauth2/oauth2_rop_test.go @@ -40,12 +40,12 @@ func TestResourceOwnerPasswordGrant(t *testing.T) { publicTS, adminTS := testhelpers.NewOAuth2Server(ctx, t, reg) secret := uuid.New().String() - audience := "https://aud.example.com" + audience := sqlxx.StringSliceJSONFormat{"https://aud.example.com"} client := &hydra.Client{ Secret: secret, GrantTypes: []string{"password", "refresh_token"}, Scope: "offline", - Audience: sqlxx.StringSliceJSONFormat{audience}, + Audience: audience, Lifespans: hydra.Lifespans{ PasswordGrantAccessTokenLifespan: x.NullDuration{Duration: 1 * time.Hour, Valid: true}, PasswordGrantRefreshTokenLifespan: x.NullDuration{Duration: 1 * time.Hour, Valid: true}, @@ -72,7 +72,6 @@ func TestResourceOwnerPasswordGrant(t *testing.T) { require.NoError(t, json.NewDecoder(r.Body).Decode(&hookReq)) assert.NotEmpty(t, hookReq.Session) assert.NotEmpty(t, hookReq.Request) - assert.ElementsMatch(t, []string{}, hookReq.Request.GrantedAudience) claims := hookReq.Session.Extra claims["hooked"] = true @@ -119,7 +118,7 @@ func TestResourceOwnerPasswordGrant(t *testing.T) { assert.Equal(t, kratos.FakeIdentityID, jwtAT.Claims["sub"]) assert.Equal(t, publicTS.URL, jwtAT.Claims["iss"]) assert.True(t, jwtAT.Claims["ext"].(map[string]any)["hooked"].(bool)) - assert.Equal(t, oauth2Config.ClientID, audience, jwtAT.Claims["aud"]) + assert.ElementsMatch(t, audience, jwtAT.Claims["aud"]) t.Run("case=introspect token", func(t *testing.T) { // Introspected token should have hook and identity_id claims From fd1e1ff0e37f1dbd36a0d9ee9cf333042680f275 Mon Sep 17 00:00:00 2001 From: Henning Perl Date: Tue, 29 Oct 2024 11:44:15 +0100 Subject: [PATCH 6/9] document return params Co-authored-by: Arne Luenser --- persistence/sql/persister_authenticate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/persistence/sql/persister_authenticate.go b/persistence/sql/persister_authenticate.go index 2bc53858be3..013ccc30051 100644 --- a/persistence/sql/persister_authenticate.go +++ b/persistence/sql/persister_authenticate.go @@ -7,7 +7,7 @@ import ( "context" ) -func (p *Persister) Authenticate(ctx context.Context, name, secret string) (string, error) { +func (p *Persister) Authenticate(ctx context.Context, name, secret string) (subject string, err error) { session, err := p.r.Kratos().Authenticate(ctx, name, secret) if err != nil { return "", err From 98cc7780d5d3a9b972ec7925edf9add4bcc2cb91 Mon Sep 17 00:00:00 2001 From: Henning Perl Date: Tue, 29 Oct 2024 12:03:06 +0100 Subject: [PATCH 7/9] code review --- go.mod | 2 +- go.sum | 1 + oauth2/handler.go | 2 -- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 54875b14f34..c39ef4d53b5 100644 --- a/go.mod +++ b/go.mod @@ -33,7 +33,7 @@ require ( github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 github.com/oleiade/reflections v1.0.1 github.com/ory/analytics-go/v5 v5.0.1 - github.com/ory/fosite v0.47.1-0.20241028135452-199f8ab66014 + github.com/ory/fosite v0.47.1-0.20241029110151-7f6e7908e349 github.com/ory/go-acc v0.2.9-0.20230103102148-6b1c9a70dbbe github.com/ory/graceful v0.1.3 github.com/ory/herodot v0.10.3-0.20230626083119-d7e5192f0d88 diff --git a/go.sum b/go.sum index b17281f7842..bbca6bfc965 100644 --- a/go.sum +++ b/go.sum @@ -391,6 +391,7 @@ github.com/ory/fosite v0.47.1-0.20241028132122-b35b62fed16e h1:aRhPoQt0QFrjQVrND github.com/ory/fosite v0.47.1-0.20241028132122-b35b62fed16e/go.mod h1:5U6c9nOLxyTdD/qrFv7N88TSxkdk5Wq8NzvB7UViDP0= github.com/ory/fosite v0.47.1-0.20241028135452-199f8ab66014 h1:vtcEvjeArxH3CayG8Gn5y3mCzo1gibu74rZhvvveANE= github.com/ory/fosite v0.47.1-0.20241028135452-199f8ab66014/go.mod h1:5U6c9nOLxyTdD/qrFv7N88TSxkdk5Wq8NzvB7UViDP0= +github.com/ory/fosite v0.47.1-0.20241029110151-7f6e7908e349/go.mod h1:5U6c9nOLxyTdD/qrFv7N88TSxkdk5Wq8NzvB7UViDP0= github.com/ory/go-acc v0.2.9-0.20230103102148-6b1c9a70dbbe h1:rvu4obdvqR0fkSIJ8IfgzKOWwZ5kOT2UNfLq81Qk7rc= github.com/ory/go-acc v0.2.9-0.20230103102148-6b1c9a70dbbe/go.mod h1:z4n3u6as84LbV4YmgjHhnwtccQqzf4cZlSk9f1FhygI= github.com/ory/go-convenience v0.1.0 h1:zouLKfF2GoSGnJwGq+PE/nJAE6dj2Zj5QlTgmMTsTS8= diff --git a/oauth2/handler.go b/oauth2/handler.go index 0e820ec3f48..288ed1f16f0 100644 --- a/oauth2/handler.go +++ b/oauth2/handler.go @@ -983,8 +983,6 @@ func (h *Handler) oauth2TokenExchange(w http.ResponseWriter, r *http.Request) { if accessRequest.GetGrantTypes().ExactOne(string(fosite.GrantTypePassword)) { if sess, ok := accessRequest.GetSession().(fosite.ExtraClaimsSession); ok { sess.GetExtraClaims()["username"] = accessRequest.GetRequestForm().Get("username") - session.Subject = sess.GetExtraClaims()["identity_id"].(string) - delete(sess.GetExtraClaims(), "identity_id") session.DefaultSession.Username = accessRequest.GetRequestForm().Get("username") } From e9f1336a266e602468e925f5ceae2cb7046a52db Mon Sep 17 00:00:00 2001 From: Henning Perl Date: Tue, 29 Oct 2024 14:41:12 +0100 Subject: [PATCH 8/9] bump fosite --- go.mod | 2 +- go.sum | 7 ++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index c39ef4d53b5..15639fe7bda 100644 --- a/go.mod +++ b/go.mod @@ -33,7 +33,7 @@ require ( github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 github.com/oleiade/reflections v1.0.1 github.com/ory/analytics-go/v5 v5.0.1 - github.com/ory/fosite v0.47.1-0.20241029110151-7f6e7908e349 + github.com/ory/fosite v0.47.1-0.20241029134014-168636ff33c7 github.com/ory/go-acc v0.2.9-0.20230103102148-6b1c9a70dbbe github.com/ory/graceful v0.1.3 github.com/ory/herodot v0.10.3-0.20230626083119-d7e5192f0d88 diff --git a/go.sum b/go.sum index bbca6bfc965..1faa2f5f293 100644 --- a/go.sum +++ b/go.sum @@ -387,11 +387,8 @@ github.com/ory/analytics-go/v5 v5.0.1 h1:LX8T5B9FN8KZXOtxgN+R3I4THRRVB6+28IKgKBp github.com/ory/analytics-go/v5 v5.0.1/go.mod h1:lWCiCjAaJkKfgR/BN5DCLMol8BjKS1x+4jxBxff/FF0= github.com/ory/dockertest/v3 v3.10.1-0.20240704115616-d229e74b748d h1:By96ZSVuH5LyjXLVVMfvJoLVGHaT96LdOnwgFSLVf0E= github.com/ory/dockertest/v3 v3.10.1-0.20240704115616-d229e74b748d/go.mod h1:F2FIjwwAk6CsNAs//B8+aPFQF0t84pbM8oliyNXwQrk= -github.com/ory/fosite v0.47.1-0.20241028132122-b35b62fed16e h1:aRhPoQt0QFrjQVrNDGiGQcT9cY/o8iqqBkMdoiyznjI= -github.com/ory/fosite v0.47.1-0.20241028132122-b35b62fed16e/go.mod h1:5U6c9nOLxyTdD/qrFv7N88TSxkdk5Wq8NzvB7UViDP0= -github.com/ory/fosite v0.47.1-0.20241028135452-199f8ab66014 h1:vtcEvjeArxH3CayG8Gn5y3mCzo1gibu74rZhvvveANE= -github.com/ory/fosite v0.47.1-0.20241028135452-199f8ab66014/go.mod h1:5U6c9nOLxyTdD/qrFv7N88TSxkdk5Wq8NzvB7UViDP0= -github.com/ory/fosite v0.47.1-0.20241029110151-7f6e7908e349/go.mod h1:5U6c9nOLxyTdD/qrFv7N88TSxkdk5Wq8NzvB7UViDP0= +github.com/ory/fosite v0.47.1-0.20241029134014-168636ff33c7 h1:QyLWLIUgC32pPrHoeW82xlkDiIL2j2o2vq64y5SsLRM= +github.com/ory/fosite v0.47.1-0.20241029134014-168636ff33c7/go.mod h1:LC+0FyghTTjdSAznGVbtj0yK2nq0LAElh6TbMck8diA= github.com/ory/go-acc v0.2.9-0.20230103102148-6b1c9a70dbbe h1:rvu4obdvqR0fkSIJ8IfgzKOWwZ5kOT2UNfLq81Qk7rc= github.com/ory/go-acc v0.2.9-0.20230103102148-6b1c9a70dbbe/go.mod h1:z4n3u6as84LbV4YmgjHhnwtccQqzf4cZlSk9f1FhygI= github.com/ory/go-convenience v0.1.0 h1:zouLKfF2GoSGnJwGq+PE/nJAE6dj2Zj5QlTgmMTsTS8= From cee989cad767ebbcdd406ff9ecd6740467a989a5 Mon Sep 17 00:00:00 2001 From: Henning Perl Date: Wed, 30 Oct 2024 10:50:24 +0100 Subject: [PATCH 9/9] chore: bump fosite to a cherry-picked branch --- consent/strategy_oauth_test.go | 8 ++++---- go.mod | 6 +++++- go.sum | 6 ++++++ 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/consent/strategy_oauth_test.go b/consent/strategy_oauth_test.go index f1827fffd50..370a3378074 100644 --- a/consent/strategy_oauth_test.go +++ b/consent/strategy_oauth_test.go @@ -823,7 +823,7 @@ func TestStrategyLoginConsentNext(t *testing.T) { makeRequestAndExpectCode(t, hc, c, url.Values{}) // Make request with additional scope and prompt none, which fails - makeRequestAndExpectError(t, hc, c, url.Values{"prompt": {"none"}, "scope": {"openid"}}, + makeRequestAndExpectError(t, hc, c, url.Values{"prompt": {"none"}, "scope": {"openid"}, "redirect_uri": {c.RedirectURIs[0]}}, "Prompt 'none' was requested, but no previous consent was found") }) @@ -930,11 +930,11 @@ func TestStrategyLoginConsentNext(t *testing.T) { }{ { d: "check all the sub claims", - values: url.Values{"scope": {"openid"}}, + values: url.Values{"scope": {"openid"}, "redirect_uri": {c.RedirectURIs[0]}}, }, { d: "works with id_token_hint", - values: url.Values{"scope": {"openid"}, "id_token_hint": {testhelpers.NewIDToken(t, reg, hash)}}, + values: url.Values{"scope": {"openid"}, "redirect_uri": {c.RedirectURIs[0]}, "id_token_hint": {testhelpers.NewIDToken(t, reg, hash)}}, }, } { t.Run("case="+tc.d, func(t *testing.T) { @@ -974,7 +974,7 @@ func TestStrategyLoginConsentNext(t *testing.T) { }), acceptConsentHandler(t, &hydra.AcceptOAuth2ConsentRequest{GrantScope: []string{"openid"}})) - code := makeRequestAndExpectCode(t, nil, c, url.Values{}) + code := makeRequestAndExpectCode(t, nil, c, url.Values{"redirect_uri": {c.RedirectURIs[0]}}) conf := oauth2Config(t, c) token, err := conf.Exchange(context.Background(), code) diff --git a/go.mod b/go.mod index 15639fe7bda..b499154156a 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,10 @@ replace github.com/ory/hydra-client-go/v2 => ./internal/httpclient replace github.com/gobuffalo/pop/v6 => github.com/ory/pop/v6 v6.2.0 +// Bump Fosite to https://github.com/ory/fosite/tree/hperl/v0.47.0%2B168636f, which contains +// https://github.com/ory/fosite/commit/b40b1cbb1997e2160eaaf97fb6f73960db4c6118 on top of the latest release. +replace github.com/ory/fosite => github.com/ory/fosite v0.47.1-0.20241030092116-b40b1cbb1997 + require ( github.com/ThalesIgnite/crypto11 v1.2.5 github.com/bradleyjkemp/cupaloy/v2 v2.8.0 @@ -33,7 +37,7 @@ require ( github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 github.com/oleiade/reflections v1.0.1 github.com/ory/analytics-go/v5 v5.0.1 - github.com/ory/fosite v0.47.1-0.20241029134014-168636ff33c7 + github.com/ory/fosite v0.47.0 github.com/ory/go-acc v0.2.9-0.20230103102148-6b1c9a70dbbe github.com/ory/graceful v0.1.3 github.com/ory/herodot v0.10.3-0.20230626083119-d7e5192f0d88 diff --git a/go.sum b/go.sum index 1faa2f5f293..fa41fd2756a 100644 --- a/go.sum +++ b/go.sum @@ -387,8 +387,14 @@ github.com/ory/analytics-go/v5 v5.0.1 h1:LX8T5B9FN8KZXOtxgN+R3I4THRRVB6+28IKgKBp github.com/ory/analytics-go/v5 v5.0.1/go.mod h1:lWCiCjAaJkKfgR/BN5DCLMol8BjKS1x+4jxBxff/FF0= github.com/ory/dockertest/v3 v3.10.1-0.20240704115616-d229e74b748d h1:By96ZSVuH5LyjXLVVMfvJoLVGHaT96LdOnwgFSLVf0E= github.com/ory/dockertest/v3 v3.10.1-0.20240704115616-d229e74b748d/go.mod h1:F2FIjwwAk6CsNAs//B8+aPFQF0t84pbM8oliyNXwQrk= +github.com/ory/fosite v0.47.0 h1:Iqu5uhx54JqZQPn2hRhqjESrmRRyQb00uJjfEi1a1QI= +github.com/ory/fosite v0.47.0/go.mod h1:5U6c9nOLxyTdD/qrFv7N88TSxkdk5Wq8NzvB7UViDP0= +github.com/ory/fosite v0.47.1-0.20241029112424-62f07ce22e57 h1:/eMox8UstN3u1r6YfVpIdiXhuz9y+ESPBUzlEHsK4AU= +github.com/ory/fosite v0.47.1-0.20241029112424-62f07ce22e57/go.mod h1:LC+0FyghTTjdSAznGVbtj0yK2nq0LAElh6TbMck8diA= github.com/ory/fosite v0.47.1-0.20241029134014-168636ff33c7 h1:QyLWLIUgC32pPrHoeW82xlkDiIL2j2o2vq64y5SsLRM= github.com/ory/fosite v0.47.1-0.20241029134014-168636ff33c7/go.mod h1:LC+0FyghTTjdSAznGVbtj0yK2nq0LAElh6TbMck8diA= +github.com/ory/fosite v0.47.1-0.20241030092116-b40b1cbb1997 h1:dryAvfoAFa1hYn6C0SPmISglYn+S775XOZgCCm54tbw= +github.com/ory/fosite v0.47.1-0.20241030092116-b40b1cbb1997/go.mod h1:5U6c9nOLxyTdD/qrFv7N88TSxkdk5Wq8NzvB7UViDP0= github.com/ory/go-acc v0.2.9-0.20230103102148-6b1c9a70dbbe h1:rvu4obdvqR0fkSIJ8IfgzKOWwZ5kOT2UNfLq81Qk7rc= github.com/ory/go-acc v0.2.9-0.20230103102148-6b1c9a70dbbe/go.mod h1:z4n3u6as84LbV4YmgjHhnwtccQqzf4cZlSk9f1FhygI= github.com/ory/go-convenience v0.1.0 h1:zouLKfF2GoSGnJwGq+PE/nJAE6dj2Zj5QlTgmMTsTS8=