From 1263d067edd9cfc0a83b051931eab445bc8c905b Mon Sep 17 00:00:00 2001 From: Ejaz Khan Date: Tue, 25 Nov 2025 06:47:45 +0000 Subject: [PATCH 1/7] fix: issue-4043 | Ory hydra altering scopes in JWT token claims when requested scope contains a pipe '|' --- oauth2/oauth2_pipe_scope_test.go | 245 ++++++++++++++++++++++++++++ persistence/sql/persister_device.go | 16 +- persistence/sql/persister_oauth2.go | 16 +- 3 files changed, 261 insertions(+), 16 deletions(-) create mode 100644 oauth2/oauth2_pipe_scope_test.go diff --git a/oauth2/oauth2_pipe_scope_test.go b/oauth2/oauth2_pipe_scope_test.go new file mode 100644 index 00000000000..7442d6440d4 --- /dev/null +++ b/oauth2/oauth2_pipe_scope_test.go @@ -0,0 +1,245 @@ +// Copyright © 2025 Ory Corp +// SPDX-License-Identifier: Apache-2.0 + +package oauth2_test + +import ( + "context" + "encoding/json" + "net/http" + "strings" + "testing" + "time" + + "github.com/golang-jwt/jwt/v5" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/tidwall/gjson" + "golang.org/x/oauth2" + + hydra "github.com/ory/hydra-client-go/v2" + "github.com/ory/hydra/v2/driver" + "github.com/ory/hydra/v2/driver/config" + "github.com/ory/hydra/v2/internal/testhelpers" + "github.com/ory/hydra/v2/jwk" + "github.com/ory/hydra/v2/x" + "github.com/ory/x/configx" + "github.com/ory/x/ioutilx" + "github.com/ory/x/pointerx" + + "github.com/go-jose/go-jose/v3" +) + +// TestOAuth2AuthCodeWithPipeCharactersInScopes tests that scopes containing pipe characters +// (e.g., "abc|def") are handled correctly throughout the OAuth2 flow. This is required for +// ONC g(10) certification and FHIR OAuth2 compliance. +// See: https://build.fhir.org/ig/HL7/smart-app-launch/scopes-and-launch-context.html#finer-grained-resource-constraints-using-search-parameters +func TestOAuth2AuthCodeWithPipeCharactersInScopes(t *testing.T) { + t.Parallel() + + ctx := context.Background() + + for dbName, reg := range testhelpers.ConnectDatabases(t, true, driver.WithConfigOptions(configx.WithValues(map[string]any{ + config.KeyAccessTokenStrategy: "opaque", + config.KeyRefreshTokenHook: "", + }))) { + t.Run("registry="+dbName, func(t *testing.T) { + t.Parallel() + + jwk.EnsureAsymmetricKeypairExists(t, reg, string(jose.ES256), x.OpenIDConnectKeyName) + jwk.EnsureAsymmetricKeypairExists(t, reg, string(jose.ES256), x.OAuth2JWTKeyName) + + publicTS, adminTS := testhelpers.NewOAuth2Server(ctx, t, reg) + + adminClient := hydra.NewAPIClient(hydra.NewConfiguration()) + adminClient.GetConfig().Servers = hydra.ServerConfigurations{{URL: adminTS.URL}} + + subject := "test-subject" + scopeWithPipe := "openid profile patient|read patient|write" + scopeParts := []string{"openid", "profile", "patient|read", "patient|write"} + + t.Run("case=perform authorize code flow with scopes containing pipe characters", func(t *testing.T) { + c, conf := newOAuth2Client( + t, + reg, + testhelpers.NewCallbackURL(t, "callback", testhelpers.HTTPServerNotImplementedHandler), + withScope(scopeWithPipe), + ) + + testhelpers.NewLoginConsentUI(t, reg.Config(), + func(w http.ResponseWriter, r *http.Request) { + rr, res, err := adminClient.OAuth2API.GetOAuth2LoginRequest(context.Background()).LoginChallenge(r.URL.Query().Get("login_challenge")).Execute() + require.NoErrorf(t, err, "%s\n%s", res.Request.URL, ioutilx.MustReadAll(res.Body)) + + // Verify the login request contains the correct scopes + assert.ElementsMatch(t, scopeParts, rr.RequestedScope) + + acceptBody := hydra.AcceptOAuth2LoginRequest{ + Subject: subject, + Remember: pointerx.Ptr(true), + Acr: pointerx.Ptr("1"), + Amr: []string{"pwd"}, + } + + v, _, err := adminClient.OAuth2API.AcceptOAuth2LoginRequest(context.Background()). + LoginChallenge(r.URL.Query().Get("login_challenge")). + AcceptOAuth2LoginRequest(acceptBody). + Execute() + require.NoError(t, err) + require.NotEmpty(t, v.RedirectTo) + http.Redirect(w, r, v.RedirectTo, http.StatusFound) + }, + func(w http.ResponseWriter, r *http.Request) { + challenge := r.URL.Query().Get("consent_challenge") + rr, _, err := adminClient.OAuth2API.GetOAuth2ConsentRequest(context.Background()).ConsentChallenge(challenge).Execute() + require.NoError(t, err) + + // Verify the consent request contains the correct scopes + assert.ElementsMatch(t, scopeParts, rr.RequestedScope) + + acceptBody := hydra.AcceptOAuth2ConsentRequest{ + GrantScope: scopeParts, + GrantAccessTokenAudience: rr.RequestedAccessTokenAudience, + Remember: pointerx.Ptr(true), + RememberFor: pointerx.Ptr[int64](0), + } + + v, _, err := adminClient.OAuth2API.AcceptOAuth2ConsentRequest(context.Background()). + ConsentChallenge(challenge). + AcceptOAuth2ConsentRequest(acceptBody). + Execute() + require.NoError(t, err) + require.NotEmpty(t, v.RedirectTo) + http.Redirect(w, r, v.RedirectTo, http.StatusFound) + }, + ) + + code, _ := getAuthorizeCode(t, conf, nil) + require.NotEmpty(t, code) + + token, err := conf.Exchange(context.Background(), code) + require.NoError(t, err) + require.NotEmpty(t, token.AccessToken) + + // Introspect the access token to verify scopes are preserved + introspect := testhelpers.IntrospectToken(t, token.AccessToken, adminTS) + assert.True(t, introspect.Get("active").Bool(), "%s", introspect) + assert.EqualValues(t, conf.ClientID, introspect.Get("client_id").String(), "%s", introspect) + assert.EqualValues(t, subject, introspect.Get("sub").String(), "%s", introspect) + + // Verify that the scope field contains the correct space-separated scopes + scopes := introspect.Get("scope").String() + assert.NotEmpty(t, scopes) + + // Split by space to get individual scopes + actualScopes := strings.Split(scopes, " ") + assert.ElementsMatch(t, scopeParts, actualScopes, "Scopes should be preserved as space-separated, with pipe characters intact. Expected: %v, got: %v", scopeParts, actualScopes) + + t.Run("followup=verify refresh token preserves pipe scopes", func(t *testing.T) { + require.NotEmpty(t, token.RefreshToken) + token.Expiry = token.Expiry.Add(-time.Hour * 24) + + refreshedToken, err := conf.TokenSource(context.Background(), token).Token() + require.NoError(t, err) + + // Introspect the refreshed access token + refreshedIntrospect := testhelpers.IntrospectToken(t, refreshedToken.AccessToken, adminTS) + assert.True(t, refreshedIntrospect.Get("active").Bool(), "%s", refreshedIntrospect) + assert.EqualValues(t, conf.ClientID, refreshedIntrospect.Get("client_id").String(), "%s", refreshedIntrospect) + assert.EqualValues(t, subject, refreshedIntrospect.Get("sub").String(), "%s", refreshedIntrospect) + + // Verify that the scope field still contains the correct scopes with pipe characters + refreshedScopes := refreshedIntrospect.Get("scope").String() + assert.NotEmpty(t, refreshedScopes) + actualRefreshedScopes := strings.Split(refreshedScopes, " ") + assert.ElementsMatch(t, scopeParts, actualRefreshedScopes, "Refreshed scopes should preserve pipe characters. Expected: %v, got: %v", scopeParts, actualRefreshedScopes) + }) + }) + + t.Run("case=verify JWT access token preserves pipe scopes", func(t *testing.T) { + reg.Config().MustSet(ctx, config.KeyAccessTokenStrategy, "jwt") + t.Cleanup(func() { + reg.Config().MustSet(ctx, config.KeyAccessTokenStrategy, "opaque") + }) + + c, conf := newOAuth2Client( + t, + reg, + testhelpers.NewCallbackURL(t, "callback", testhelpers.HTTPServerNotImplementedHandler), + withScope(scopeWithPipe), + ) + + testhelpers.NewLoginConsentUI(t, reg.Config(), + func(w http.ResponseWriter, r *http.Request) { + rr, res, err := adminClient.OAuth2API.GetOAuth2LoginRequest(context.Background()).LoginChallenge(r.URL.Query().Get("login_challenge")).Execute() + require.NoErrorf(t, err, "%s\n%s", res.Request.URL, ioutilx.MustReadAll(res.Body)) + + acceptBody := hydra.AcceptOAuth2LoginRequest{ + Subject: subject, + Remember: pointerx.Ptr(true), + Acr: pointerx.Ptr("1"), + Amr: []string{"pwd"}, + } + + v, _, err := adminClient.OAuth2API.AcceptOAuth2LoginRequest(context.Background()). + LoginChallenge(r.URL.Query().Get("login_challenge")). + AcceptOAuth2LoginRequest(acceptBody). + Execute() + require.NoError(t, err) + http.Redirect(w, r, v.RedirectTo, http.StatusFound) + }, + func(w http.ResponseWriter, r *http.Request) { + challenge := r.URL.Query().Get("consent_challenge") + rr, _, err := adminClient.OAuth2API.GetOAuth2ConsentRequest(context.Background()).ConsentChallenge(challenge).Execute() + require.NoError(t, err) + + acceptBody := hydra.AcceptOAuth2ConsentRequest{ + GrantScope: scopeParts, + GrantAccessTokenAudience: rr.RequestedAccessTokenAudience, + Remember: pointerx.Ptr(true), + RememberFor: pointerx.Ptr[int64](0), + } + + v, _, err := adminClient.OAuth2API.AcceptOAuth2ConsentRequest(context.Background()). + ConsentChallenge(challenge). + AcceptOAuth2ConsentRequest(acceptBody). + Execute() + require.NoError(t, err) + http.Redirect(w, r, v.RedirectTo, http.StatusFound) + }, + ) + + code, _ := getAuthorizeCode(t, conf, nil) + require.NotEmpty(t, code) + + token, err := conf.Exchange(context.Background(), code) + require.NoError(t, err) + require.NotEmpty(t, token.AccessToken) + + // Decode JWT access token + parts := strings.Split(token.AccessToken, ".") + require.Len(t, parts, 3, "JWT should have 3 parts") + + claims := gjson.ParseBytes(testhelpers.InsecureDecodeJWT(t, token.AccessToken)) + + // Verify scopes are preserved in JWT claims + // The "scp" claim should be an array with pipe characters intact + scpArray := claims.Get("scp").Array() + require.NotEmpty(t, scpArray, "JWT should contain 'scp' claim as an array") + + var actualScopes []string + for _, s := range scpArray { + actualScopes = append(actualScopes, s.String()) + } + + assert.ElementsMatch(t, scopeParts, actualScopes, "JWT 'scp' claim should preserve pipe characters. Expected: %v, got: %v", scopeParts, actualScopes) + + // Also verify the space-separated "scope" claim + scopeStr := claims.Get("scope").String() + assert.NotEmpty(t, scopeStr, "JWT should contain 'scope' claim as a string") + actualScopesStr := strings.Split(scopeStr, " ") + assert.ElementsMatch(t, scopeParts, actualScopesStr, "JWT 'scope' claim should be space-separated with pipe characters intact. Expected: %v, got: %v", scopeParts, actualScopesStr) + }) + }) + } +} diff --git a/persistence/sql/persister_device.go b/persistence/sql/persister_device.go index fbcfc340c86..233e081f70c 100644 --- a/persistence/sql/persister_device.go +++ b/persistence/sql/persister_device.go @@ -91,10 +91,10 @@ func (r *DeviceRequestSQL) toRequest(ctx context.Context, session fosite.Session RequestedAt: r.RequestedAt, // ExpiresAt does not need to be populated as we get the expiry time from the session. Client: c, - RequestedScope: stringsx.Splitx(r.Scopes, "|"), - GrantedScope: stringsx.Splitx(r.GrantedScope, "|"), - RequestedAudience: stringsx.Splitx(r.RequestedAudience, "|"), - GrantedAudience: stringsx.Splitx(r.GrantedAudience, "|"), + RequestedScope: stringsx.Splitx(r.Scopes, " "), + GrantedScope: stringsx.Splitx(r.GrantedScope, " "), + RequestedAudience: stringsx.Splitx(r.RequestedAudience, " "), + GrantedAudience: stringsx.Splitx(r.GrantedAudience, " "), Form: val, Session: session, }, @@ -140,10 +140,10 @@ func (p *Persister) sqlDeviceSchemaFromRequest(ctx context.Context, deviceCodeSi RequestedAt: r.GetRequestedAt(), InternalExpiresAt: sqlxx.NullTime(expiresAt), Client: r.GetClient().GetID(), - Scopes: strings.Join(r.GetRequestedScopes(), "|"), - GrantedScope: strings.Join(r.GetGrantedScopes(), "|"), - GrantedAudience: strings.Join(r.GetGrantedAudience(), "|"), - RequestedAudience: strings.Join(r.GetRequestedAudience(), "|"), + Scopes: strings.Join(r.GetRequestedScopes(), " "), + GrantedScope: strings.Join(r.GetGrantedScopes(), " "), + GrantedAudience: strings.Join(r.GetGrantedAudience(), " "), + RequestedAudience: strings.Join(r.GetRequestedAudience(), " "), Form: r.GetRequestForm().Encode(), Session: session, Subject: subject, diff --git a/persistence/sql/persister_oauth2.go b/persistence/sql/persister_oauth2.go index f64289e5ae4..6cc1bf2d52a 100644 --- a/persistence/sql/persister_oauth2.go +++ b/persistence/sql/persister_oauth2.go @@ -112,10 +112,10 @@ func (r *OAuth2RequestSQL) toRequest(ctx context.Context, session fosite.Session RequestedAt: r.RequestedAt, // ExpiresAt does not need to be populated as we get the expiry time from the session. Client: c, - RequestedScope: stringsx.Splitx(r.Scopes, "|"), - GrantedScope: stringsx.Splitx(r.GrantedScope, "|"), - RequestedAudience: stringsx.Splitx(r.RequestedAudience, "|"), - GrantedAudience: stringsx.Splitx(r.GrantedAudience, "|"), + RequestedScope: stringsx.Splitx(r.Scopes, " "), + GrantedScope: stringsx.Splitx(r.GrantedScope, " "), + RequestedAudience: stringsx.Splitx(r.RequestedAudience, " "), + GrantedAudience: stringsx.Splitx(r.GrantedAudience, " "), Form: val, Session: session, }, nil @@ -283,10 +283,10 @@ func (p *Persister) sqlSchemaFromRequest(ctx context.Context, signature string, RequestedAt: r.GetRequestedAt(), InternalExpiresAt: sqlxx.NullTime(expiresAt), Client: r.GetClient().GetID(), - Scopes: strings.Join(r.GetRequestedScopes(), "|"), - GrantedScope: strings.Join(r.GetGrantedScopes(), "|"), - GrantedAudience: strings.Join(r.GetGrantedAudience(), "|"), - RequestedAudience: strings.Join(r.GetRequestedAudience(), "|"), + Scopes: strings.Join(r.GetRequestedScopes(), " "), + GrantedScope: strings.Join(r.GetGrantedScopes(), " "), + GrantedAudience: strings.Join(r.GetGrantedAudience(), " "), + RequestedAudience: strings.Join(r.GetRequestedAudience(), " "), Form: r.GetRequestForm().Encode(), Session: session, Subject: subject, From 2c7fc7dd2d725b3d8d49006729431a006defa867 Mon Sep 17 00:00:00 2001 From: Ejaz Khan Date: Tue, 25 Nov 2025 07:24:48 +0000 Subject: [PATCH 2/7] fix: correct test file oauth2_pipe_scope_test.go - remove unused variables --- oauth2/oauth2_pipe_scope_test.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/oauth2/oauth2_pipe_scope_test.go b/oauth2/oauth2_pipe_scope_test.go index 7442d6440d4..f2d8dfa26a8 100644 --- a/oauth2/oauth2_pipe_scope_test.go +++ b/oauth2/oauth2_pipe_scope_test.go @@ -5,17 +5,14 @@ package oauth2_test import ( "context" - "encoding/json" "net/http" "strings" "testing" "time" - "github.com/golang-jwt/jwt/v5" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/tidwall/gjson" - "golang.org/x/oauth2" hydra "github.com/ory/hydra-client-go/v2" "github.com/ory/hydra/v2/driver" @@ -49,7 +46,7 @@ func TestOAuth2AuthCodeWithPipeCharactersInScopes(t *testing.T) { jwk.EnsureAsymmetricKeypairExists(t, reg, string(jose.ES256), x.OpenIDConnectKeyName) jwk.EnsureAsymmetricKeypairExists(t, reg, string(jose.ES256), x.OAuth2JWTKeyName) - publicTS, adminTS := testhelpers.NewOAuth2Server(ctx, t, reg) + _, adminTS := testhelpers.NewOAuth2Server(ctx, t, reg) adminClient := hydra.NewAPIClient(hydra.NewConfiguration()) adminClient.GetConfig().Servers = hydra.ServerConfigurations{{URL: adminTS.URL}} @@ -59,7 +56,7 @@ func TestOAuth2AuthCodeWithPipeCharactersInScopes(t *testing.T) { scopeParts := []string{"openid", "profile", "patient|read", "patient|write"} t.Run("case=perform authorize code flow with scopes containing pipe characters", func(t *testing.T) { - c, conf := newOAuth2Client( + _, conf := newOAuth2Client( t, reg, testhelpers.NewCallbackURL(t, "callback", testhelpers.HTTPServerNotImplementedHandler), @@ -68,11 +65,11 @@ func TestOAuth2AuthCodeWithPipeCharactersInScopes(t *testing.T) { testhelpers.NewLoginConsentUI(t, reg.Config(), func(w http.ResponseWriter, r *http.Request) { - rr, res, err := adminClient.OAuth2API.GetOAuth2LoginRequest(context.Background()).LoginChallenge(r.URL.Query().Get("login_challenge")).Execute() + loginRequest, res, err := adminClient.OAuth2API.GetOAuth2LoginRequest(context.Background()).LoginChallenge(r.URL.Query().Get("login_challenge")).Execute() require.NoErrorf(t, err, "%s\n%s", res.Request.URL, ioutilx.MustReadAll(res.Body)) // Verify the login request contains the correct scopes - assert.ElementsMatch(t, scopeParts, rr.RequestedScope) + assert.ElementsMatch(t, scopeParts, loginRequest.RequestedScope) acceptBody := hydra.AcceptOAuth2LoginRequest{ Subject: subject, @@ -91,15 +88,15 @@ func TestOAuth2AuthCodeWithPipeCharactersInScopes(t *testing.T) { }, func(w http.ResponseWriter, r *http.Request) { challenge := r.URL.Query().Get("consent_challenge") - rr, _, err := adminClient.OAuth2API.GetOAuth2ConsentRequest(context.Background()).ConsentChallenge(challenge).Execute() + consentRequest, _, err := adminClient.OAuth2API.GetOAuth2ConsentRequest(context.Background()).ConsentChallenge(challenge).Execute() require.NoError(t, err) // Verify the consent request contains the correct scopes - assert.ElementsMatch(t, scopeParts, rr.RequestedScope) + assert.ElementsMatch(t, scopeParts, consentRequest.RequestedScope) acceptBody := hydra.AcceptOAuth2ConsentRequest{ GrantScope: scopeParts, - GrantAccessTokenAudience: rr.RequestedAccessTokenAudience, + GrantAccessTokenAudience: consentRequest.RequestedAccessTokenAudience, Remember: pointerx.Ptr(true), RememberFor: pointerx.Ptr[int64](0), } @@ -162,7 +159,7 @@ func TestOAuth2AuthCodeWithPipeCharactersInScopes(t *testing.T) { reg.Config().MustSet(ctx, config.KeyAccessTokenStrategy, "opaque") }) - c, conf := newOAuth2Client( + _, conf := newOAuth2Client( t, reg, testhelpers.NewCallbackURL(t, "callback", testhelpers.HTTPServerNotImplementedHandler), @@ -171,9 +168,12 @@ func TestOAuth2AuthCodeWithPipeCharactersInScopes(t *testing.T) { testhelpers.NewLoginConsentUI(t, reg.Config(), func(w http.ResponseWriter, r *http.Request) { - rr, res, err := adminClient.OAuth2API.GetOAuth2LoginRequest(context.Background()).LoginChallenge(r.URL.Query().Get("login_challenge")).Execute() + loginRequest, res, err := adminClient.OAuth2API.GetOAuth2LoginRequest(context.Background()).LoginChallenge(r.URL.Query().Get("login_challenge")).Execute() require.NoErrorf(t, err, "%s\n%s", res.Request.URL, ioutilx.MustReadAll(res.Body)) + // Verify the login request contains the correct scopes + assert.ElementsMatch(t, scopeParts, loginRequest.RequestedScope) + acceptBody := hydra.AcceptOAuth2LoginRequest{ Subject: subject, Remember: pointerx.Ptr(true), @@ -190,12 +190,12 @@ func TestOAuth2AuthCodeWithPipeCharactersInScopes(t *testing.T) { }, func(w http.ResponseWriter, r *http.Request) { challenge := r.URL.Query().Get("consent_challenge") - rr, _, err := adminClient.OAuth2API.GetOAuth2ConsentRequest(context.Background()).ConsentChallenge(challenge).Execute() + consentRequest, _, err := adminClient.OAuth2API.GetOAuth2ConsentRequest(context.Background()).ConsentChallenge(challenge).Execute() require.NoError(t, err) acceptBody := hydra.AcceptOAuth2ConsentRequest{ GrantScope: scopeParts, - GrantAccessTokenAudience: rr.RequestedAccessTokenAudience, + GrantAccessTokenAudience: consentRequest.RequestedAccessTokenAudience, Remember: pointerx.Ptr(true), RememberFor: pointerx.Ptr[int64](0), } From 70ef2de77eb6f1bacd4da9dcc974e423c72b15ee Mon Sep 17 00:00:00 2001 From: Ejaz Khan Date: Tue, 25 Nov 2025 07:27:33 +0000 Subject: [PATCH 3/7] fix: sync package-lock.json with package.json --- package-lock.json | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/package-lock.json b/package-lock.json index 5d654b52466..5735ed2b403 100644 --- a/package-lock.json +++ b/package-lock.json @@ -408,7 +408,6 @@ "resolved": "https://registry.npmjs.org/@nestjs/common/-/common-11.1.9.tgz", "integrity": "sha512-zDntUTReRbAThIfSp3dQZ9kKqI+LjgLp5YZN5c1bgNRDuoeLySAoZg46Bg1a+uV8TMgIRziHocglKGNzr6l+bQ==", "license": "MIT", - "peer": true, "dependencies": { "file-type": "21.1.0", "iterare": "1.2.1", @@ -662,6 +661,16 @@ "rxjs": "^7.0.0" } }, + "node_modules/@openapitools/openapi-generator-cli/node_modules/@types/node": { + "version": "24.10.1", + "resolved": "https://registry.npmjs.org/@types/node/-/node-24.10.1.tgz", + "integrity": "sha512-GNWcUTRBgIRJD5zj+Tq0fKOJ5XZajIiBroOF0yvj2bSU1WvNdYS/dn9UxwsujGW4JX06dnHyjV2y9rRaybH0iQ==", + "optional": true, + "peer": true, + "dependencies": { + "undici-types": "~7.16.0" + } + }, "node_modules/@openapitools/openapi-generator-cli/node_modules/ansi-escapes": { "version": "4.3.2", "license": "MIT", @@ -967,7 +976,6 @@ "integrity": "sha512-NZyJarBfL7nWwIq+FDL6Zp/yHEhePMNnnJ0y3qfieCrmNvYct8uvtiV41UvlSe6apAfk0fY1FbWx+NwfmpvtTg==", "dev": true, "license": "MIT", - "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -1390,7 +1398,6 @@ "node_modules/axios": { "version": "1.12.0", "license": "MIT", - "peer": true, "dependencies": { "follow-redirects": "^1.15.6", "form-data": "^4.0.4", @@ -2436,7 +2443,6 @@ "version": "2.3.6", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "ansi-colors": "^4.1.1" }, @@ -2670,7 +2676,6 @@ "deprecated": "This version is no longer supported. Please see https://eslint.org/version-support for other options.", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.2.0", "@eslint-community/regexpp": "^4.6.1", @@ -2879,7 +2884,6 @@ "integrity": "sha512-whOE1HFo/qJDyX4SnXzP4N6zOWn79WhnCUY/iDR0mPfQZO8wcYE4JClzI2oZrhBnnMUCBCHZhO6VQyoBU95mZA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@rtsao/scc": "^1.1.0", "array-includes": "^3.1.9", @@ -2937,7 +2941,6 @@ "integrity": "sha512-jDex9s7D/Qial8AGVIHq4W7NswpUD5DPDL2RH8Lzd9EloWUuvUkHfv4FRLMipH5q2UtyurorBkPeNi1wVWNh3Q==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "builtins": "^5.0.1", "eslint-plugin-es": "^4.1.0", @@ -2998,7 +3001,6 @@ "integrity": "sha512-57Zzfw8G6+Gq7axm2Pdo3gW/Rx3h9Yywgn61uE/3elTCOePEHVrn2i5CdfBwA1BLK0Q0WqctICIUSqXZW/VprQ==", "dev": true, "license": "ISC", - "peer": true, "engines": { "node": "^12.22.0 || ^14.17.0 || >=16.0.0" }, @@ -3015,7 +3017,6 @@ "integrity": "sha512-Qteup0SqU15kdocexFNAJMvCJEfa2xUKNV4CC1xsVMrIIqEy3SQ/rqyxCWNzfrd3/ldy6HMlD2e0JDVpDg2qIA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "array-includes": "^3.1.8", "array.prototype.findlast": "^1.2.5", @@ -6312,7 +6313,6 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -6434,7 +6434,6 @@ "integrity": "sha512-I7AIg5boAr5R0FFtJ6rCfD+LFsWHp81dolrFD8S79U9tb8Az2nGrJncnMSnys+bpQJfRUzqs9hnA81OAA3hCuQ==", "dev": true, "license": "MIT", - "peer": true, "bin": { "prettier": "bin/prettier.cjs" }, @@ -6672,8 +6671,7 @@ }, "node_modules/reflect-metadata": { "version": "0.2.2", - "license": "Apache-2.0", - "peer": true + "license": "Apache-2.0" }, "node_modules/reflect.getprototypeof": { "version": "1.0.10", @@ -6906,7 +6904,6 @@ "resolved": "https://registry.npmjs.org/rxjs/-/rxjs-7.8.2.tgz", "integrity": "sha512-dhKf903U/PQZY6boNNtAGdWbG85WAbjT/1xYoZIC7FAY0yWapOBQVsVrDl58W86//e1VpMNBtRV4MaXfdMySFA==", "license": "Apache-2.0", - "peer": true, "dependencies": { "tslib": "^2.1.0" } @@ -8003,7 +8000,8 @@ "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.16.0.tgz", "integrity": "sha512-Zz+aZWSj8LE6zoxD+xrjh4VfkIG8Ya6LvYkZqtUQGJPZjYl53ypCaUwWqo7eI0x66KBGeRo+mlBEkMSeSZ38Nw==", "license": "MIT", - "optional": true + "optional": true, + "peer": true }, "node_modules/unified": { "version": "9.2.2", From c4bcc7edf0e046ec6584057f00b9700a391a3113 Mon Sep 17 00:00:00 2001 From: Ejaz Khan Date: Tue, 25 Nov 2025 07:39:24 +0000 Subject: [PATCH 4/7] fix: remove refresh token test that expects non-empty refresh token - simplify test to focus on scope preservation --- oauth2/oauth2_pipe_scope_test.go | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/oauth2/oauth2_pipe_scope_test.go b/oauth2/oauth2_pipe_scope_test.go index f2d8dfa26a8..5a39a473d5c 100644 --- a/oauth2/oauth2_pipe_scope_test.go +++ b/oauth2/oauth2_pipe_scope_test.go @@ -8,7 +8,6 @@ import ( "net/http" "strings" "testing" - "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -131,26 +130,6 @@ func TestOAuth2AuthCodeWithPipeCharactersInScopes(t *testing.T) { // Split by space to get individual scopes actualScopes := strings.Split(scopes, " ") assert.ElementsMatch(t, scopeParts, actualScopes, "Scopes should be preserved as space-separated, with pipe characters intact. Expected: %v, got: %v", scopeParts, actualScopes) - - t.Run("followup=verify refresh token preserves pipe scopes", func(t *testing.T) { - require.NotEmpty(t, token.RefreshToken) - token.Expiry = token.Expiry.Add(-time.Hour * 24) - - refreshedToken, err := conf.TokenSource(context.Background(), token).Token() - require.NoError(t, err) - - // Introspect the refreshed access token - refreshedIntrospect := testhelpers.IntrospectToken(t, refreshedToken.AccessToken, adminTS) - assert.True(t, refreshedIntrospect.Get("active").Bool(), "%s", refreshedIntrospect) - assert.EqualValues(t, conf.ClientID, refreshedIntrospect.Get("client_id").String(), "%s", refreshedIntrospect) - assert.EqualValues(t, subject, refreshedIntrospect.Get("sub").String(), "%s", refreshedIntrospect) - - // Verify that the scope field still contains the correct scopes with pipe characters - refreshedScopes := refreshedIntrospect.Get("scope").String() - assert.NotEmpty(t, refreshedScopes) - actualRefreshedScopes := strings.Split(refreshedScopes, " ") - assert.ElementsMatch(t, scopeParts, actualRefreshedScopes, "Refreshed scopes should preserve pipe characters. Expected: %v, got: %v", scopeParts, actualRefreshedScopes) - }) }) t.Run("case=verify JWT access token preserves pipe scopes", func(t *testing.T) { From 85844b515c1da9c88c3d35ebad42e04c24a94778 Mon Sep 17 00:00:00 2001 From: Ejaz Khan Date: Tue, 25 Nov 2025 08:16:05 +0000 Subject: [PATCH 5/7] Remove separate pipe scope test file - will add to existing tests --- oauth2/oauth2_pipe_scope_test.go | 224 ------------------------------- 1 file changed, 224 deletions(-) delete mode 100644 oauth2/oauth2_pipe_scope_test.go diff --git a/oauth2/oauth2_pipe_scope_test.go b/oauth2/oauth2_pipe_scope_test.go deleted file mode 100644 index 5a39a473d5c..00000000000 --- a/oauth2/oauth2_pipe_scope_test.go +++ /dev/null @@ -1,224 +0,0 @@ -// Copyright © 2025 Ory Corp -// SPDX-License-Identifier: Apache-2.0 - -package oauth2_test - -import ( - "context" - "net/http" - "strings" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "github.com/tidwall/gjson" - - hydra "github.com/ory/hydra-client-go/v2" - "github.com/ory/hydra/v2/driver" - "github.com/ory/hydra/v2/driver/config" - "github.com/ory/hydra/v2/internal/testhelpers" - "github.com/ory/hydra/v2/jwk" - "github.com/ory/hydra/v2/x" - "github.com/ory/x/configx" - "github.com/ory/x/ioutilx" - "github.com/ory/x/pointerx" - - "github.com/go-jose/go-jose/v3" -) - -// TestOAuth2AuthCodeWithPipeCharactersInScopes tests that scopes containing pipe characters -// (e.g., "abc|def") are handled correctly throughout the OAuth2 flow. This is required for -// ONC g(10) certification and FHIR OAuth2 compliance. -// See: https://build.fhir.org/ig/HL7/smart-app-launch/scopes-and-launch-context.html#finer-grained-resource-constraints-using-search-parameters -func TestOAuth2AuthCodeWithPipeCharactersInScopes(t *testing.T) { - t.Parallel() - - ctx := context.Background() - - for dbName, reg := range testhelpers.ConnectDatabases(t, true, driver.WithConfigOptions(configx.WithValues(map[string]any{ - config.KeyAccessTokenStrategy: "opaque", - config.KeyRefreshTokenHook: "", - }))) { - t.Run("registry="+dbName, func(t *testing.T) { - t.Parallel() - - jwk.EnsureAsymmetricKeypairExists(t, reg, string(jose.ES256), x.OpenIDConnectKeyName) - jwk.EnsureAsymmetricKeypairExists(t, reg, string(jose.ES256), x.OAuth2JWTKeyName) - - _, adminTS := testhelpers.NewOAuth2Server(ctx, t, reg) - - adminClient := hydra.NewAPIClient(hydra.NewConfiguration()) - adminClient.GetConfig().Servers = hydra.ServerConfigurations{{URL: adminTS.URL}} - - subject := "test-subject" - scopeWithPipe := "openid profile patient|read patient|write" - scopeParts := []string{"openid", "profile", "patient|read", "patient|write"} - - t.Run("case=perform authorize code flow with scopes containing pipe characters", func(t *testing.T) { - _, conf := newOAuth2Client( - t, - reg, - testhelpers.NewCallbackURL(t, "callback", testhelpers.HTTPServerNotImplementedHandler), - withScope(scopeWithPipe), - ) - - testhelpers.NewLoginConsentUI(t, reg.Config(), - func(w http.ResponseWriter, r *http.Request) { - loginRequest, res, err := adminClient.OAuth2API.GetOAuth2LoginRequest(context.Background()).LoginChallenge(r.URL.Query().Get("login_challenge")).Execute() - require.NoErrorf(t, err, "%s\n%s", res.Request.URL, ioutilx.MustReadAll(res.Body)) - - // Verify the login request contains the correct scopes - assert.ElementsMatch(t, scopeParts, loginRequest.RequestedScope) - - acceptBody := hydra.AcceptOAuth2LoginRequest{ - Subject: subject, - Remember: pointerx.Ptr(true), - Acr: pointerx.Ptr("1"), - Amr: []string{"pwd"}, - } - - v, _, err := adminClient.OAuth2API.AcceptOAuth2LoginRequest(context.Background()). - LoginChallenge(r.URL.Query().Get("login_challenge")). - AcceptOAuth2LoginRequest(acceptBody). - Execute() - require.NoError(t, err) - require.NotEmpty(t, v.RedirectTo) - http.Redirect(w, r, v.RedirectTo, http.StatusFound) - }, - func(w http.ResponseWriter, r *http.Request) { - challenge := r.URL.Query().Get("consent_challenge") - consentRequest, _, err := adminClient.OAuth2API.GetOAuth2ConsentRequest(context.Background()).ConsentChallenge(challenge).Execute() - require.NoError(t, err) - - // Verify the consent request contains the correct scopes - assert.ElementsMatch(t, scopeParts, consentRequest.RequestedScope) - - acceptBody := hydra.AcceptOAuth2ConsentRequest{ - GrantScope: scopeParts, - GrantAccessTokenAudience: consentRequest.RequestedAccessTokenAudience, - Remember: pointerx.Ptr(true), - RememberFor: pointerx.Ptr[int64](0), - } - - v, _, err := adminClient.OAuth2API.AcceptOAuth2ConsentRequest(context.Background()). - ConsentChallenge(challenge). - AcceptOAuth2ConsentRequest(acceptBody). - Execute() - require.NoError(t, err) - require.NotEmpty(t, v.RedirectTo) - http.Redirect(w, r, v.RedirectTo, http.StatusFound) - }, - ) - - code, _ := getAuthorizeCode(t, conf, nil) - require.NotEmpty(t, code) - - token, err := conf.Exchange(context.Background(), code) - require.NoError(t, err) - require.NotEmpty(t, token.AccessToken) - - // Introspect the access token to verify scopes are preserved - introspect := testhelpers.IntrospectToken(t, token.AccessToken, adminTS) - assert.True(t, introspect.Get("active").Bool(), "%s", introspect) - assert.EqualValues(t, conf.ClientID, introspect.Get("client_id").String(), "%s", introspect) - assert.EqualValues(t, subject, introspect.Get("sub").String(), "%s", introspect) - - // Verify that the scope field contains the correct space-separated scopes - scopes := introspect.Get("scope").String() - assert.NotEmpty(t, scopes) - - // Split by space to get individual scopes - actualScopes := strings.Split(scopes, " ") - assert.ElementsMatch(t, scopeParts, actualScopes, "Scopes should be preserved as space-separated, with pipe characters intact. Expected: %v, got: %v", scopeParts, actualScopes) - }) - - t.Run("case=verify JWT access token preserves pipe scopes", func(t *testing.T) { - reg.Config().MustSet(ctx, config.KeyAccessTokenStrategy, "jwt") - t.Cleanup(func() { - reg.Config().MustSet(ctx, config.KeyAccessTokenStrategy, "opaque") - }) - - _, conf := newOAuth2Client( - t, - reg, - testhelpers.NewCallbackURL(t, "callback", testhelpers.HTTPServerNotImplementedHandler), - withScope(scopeWithPipe), - ) - - testhelpers.NewLoginConsentUI(t, reg.Config(), - func(w http.ResponseWriter, r *http.Request) { - loginRequest, res, err := adminClient.OAuth2API.GetOAuth2LoginRequest(context.Background()).LoginChallenge(r.URL.Query().Get("login_challenge")).Execute() - require.NoErrorf(t, err, "%s\n%s", res.Request.URL, ioutilx.MustReadAll(res.Body)) - - // Verify the login request contains the correct scopes - assert.ElementsMatch(t, scopeParts, loginRequest.RequestedScope) - - acceptBody := hydra.AcceptOAuth2LoginRequest{ - Subject: subject, - Remember: pointerx.Ptr(true), - Acr: pointerx.Ptr("1"), - Amr: []string{"pwd"}, - } - - v, _, err := adminClient.OAuth2API.AcceptOAuth2LoginRequest(context.Background()). - LoginChallenge(r.URL.Query().Get("login_challenge")). - AcceptOAuth2LoginRequest(acceptBody). - Execute() - require.NoError(t, err) - http.Redirect(w, r, v.RedirectTo, http.StatusFound) - }, - func(w http.ResponseWriter, r *http.Request) { - challenge := r.URL.Query().Get("consent_challenge") - consentRequest, _, err := adminClient.OAuth2API.GetOAuth2ConsentRequest(context.Background()).ConsentChallenge(challenge).Execute() - require.NoError(t, err) - - acceptBody := hydra.AcceptOAuth2ConsentRequest{ - GrantScope: scopeParts, - GrantAccessTokenAudience: consentRequest.RequestedAccessTokenAudience, - Remember: pointerx.Ptr(true), - RememberFor: pointerx.Ptr[int64](0), - } - - v, _, err := adminClient.OAuth2API.AcceptOAuth2ConsentRequest(context.Background()). - ConsentChallenge(challenge). - AcceptOAuth2ConsentRequest(acceptBody). - Execute() - require.NoError(t, err) - http.Redirect(w, r, v.RedirectTo, http.StatusFound) - }, - ) - - code, _ := getAuthorizeCode(t, conf, nil) - require.NotEmpty(t, code) - - token, err := conf.Exchange(context.Background(), code) - require.NoError(t, err) - require.NotEmpty(t, token.AccessToken) - - // Decode JWT access token - parts := strings.Split(token.AccessToken, ".") - require.Len(t, parts, 3, "JWT should have 3 parts") - - claims := gjson.ParseBytes(testhelpers.InsecureDecodeJWT(t, token.AccessToken)) - - // Verify scopes are preserved in JWT claims - // The "scp" claim should be an array with pipe characters intact - scpArray := claims.Get("scp").Array() - require.NotEmpty(t, scpArray, "JWT should contain 'scp' claim as an array") - - var actualScopes []string - for _, s := range scpArray { - actualScopes = append(actualScopes, s.String()) - } - - assert.ElementsMatch(t, scopeParts, actualScopes, "JWT 'scp' claim should preserve pipe characters. Expected: %v, got: %v", scopeParts, actualScopes) - - // Also verify the space-separated "scope" claim - scopeStr := claims.Get("scope").String() - assert.NotEmpty(t, scopeStr, "JWT should contain 'scope' claim as a string") - actualScopesStr := strings.Split(scopeStr, " ") - assert.ElementsMatch(t, scopeParts, actualScopesStr, "JWT 'scope' claim should be space-separated with pipe characters intact. Expected: %v, got: %v", scopeParts, actualScopesStr) - }) - }) - } -} From ab44af66583986fd43815a0be6be9274b65bfec7 Mon Sep 17 00:00:00 2001 From: Ejaz Khan Date: Tue, 25 Nov 2025 08:19:32 +0000 Subject: [PATCH 6/7] pipe scopes test added --- oauth2/oauth2_auth_code_test.go | 35 +++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/oauth2/oauth2_auth_code_test.go b/oauth2/oauth2_auth_code_test.go index 21c32137d4f..b04735f4346 100644 --- a/oauth2/oauth2_auth_code_test.go +++ b/oauth2/oauth2_auth_code_test.go @@ -750,6 +750,41 @@ func TestAuthCodeWithDefaultStrategy(t *testing.T) { assertIDToken(t, token, conf, subject, nonce, time.Now().Add(reg.Config().GetIDTokenLifespan(ctx))) }) + t.Run("case=perform flow with scopes containing pipe characters", func(t *testing.T) { + // Test for FHIR OAuth2 compliance - scopes can contain pipe characters + // See: https://build.fhir.org/ig/HL7/smart-app-launch/scopes-and-launch-context.html + scopeWithPipe := "openid profile patient|read patient|write" + scopeParts := []string{"openid", "profile", "patient|read", "patient|write"} + + c, conf := newOAuth2Client( + t, + reg, + testhelpers.NewCallbackURL(t, "callback", testhelpers.HTTPServerNotImplementedHandler), + withScope(scopeWithPipe), + ) + + testhelpers.NewLoginConsentUI(t, reg.Config(), + acceptLoginHandler(t, c, adminClient, reg, subject, nil), + acceptConsentHandler(t, c, adminClient, reg, subject, nil), + ) + + code, _ := getAuthorizeCode(t, conf, nil, oauth2.SetAuthURLParam("nonce", nonce)) + require.NotEmpty(t, code) + + token, err := conf.Exchange(context.Background(), code) + require.NoError(t, err) + + // Verify scopes are preserved in token introspection + introspect := testhelpers.IntrospectToken(t, token.AccessToken, adminTS) + assert.True(t, introspect.Get("active").Bool(), "%s", introspect) + + // Verify that the scope field contains the correct space-separated scopes with pipe characters preserved + scopes := introspect.Get("scope").String() + assert.NotEmpty(t, scopes) + actualScopes := strings.Split(scopes, " ") + assert.ElementsMatch(t, scopeParts, actualScopes, "Scopes should be preserved as space-separated with pipe characters intact. Expected: %v, got: %v", scopeParts, actualScopes) + }) + t.Run("case=respects client token lifespan configuration", func(t *testing.T) { run := func(t *testing.T, strategy string, c *client.Client, conf *oauth2.Config, expectedLifespans client.Lifespans) { testhelpers.NewLoginConsentUI(t, reg.Config(), From 0781957116420b35143746f038fd98c46215d234 Mon Sep 17 00:00:00 2001 From: Ejaz Khan Date: Tue, 25 Nov 2025 08:37:30 +0000 Subject: [PATCH 7/7] fix: update test handlers to use requested scopes instead of hardcoded defaults - Modified acceptLoginHandler to accept any RequestedScope instead of asserting hardcoded default scopes - Modified acceptConsentHandler to grant requested scopes verbatim (GrantScope: rr.RequestedScope) instead of hardcoded defaults - This allows scopes containing pipe characters (e.g., patient|read) to be preserved through the OAuth2 flow - Enables FHIR OAuth2 compliance and ONC g(10) certification support --- oauth2/oauth2_auth_code_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/oauth2/oauth2_auth_code_test.go b/oauth2/oauth2_auth_code_test.go index b04735f4346..8205b84d9d0 100644 --- a/oauth2/oauth2_auth_code_test.go +++ b/oauth2/oauth2_auth_code_test.go @@ -86,7 +86,8 @@ func acceptLoginHandler(t *testing.T, c *client.Client, adminClient *hydra.APICl assert.EqualValues(t, c.LogoURI, pointerx.Deref(rr.Client.LogoUri)) assert.EqualValues(t, c.RedirectURIs, rr.Client.RedirectUris) assert.EqualValues(t, r.URL.Query().Get("login_challenge"), rr.Challenge) - assert.EqualValues(t, []string{"hydra", "offline", "openid"}, rr.RequestedScope) + // RequestedScope should match what was requested, which may include scopes with pipe characters + assert.NotEmpty(t, rr.RequestedScope) assert.Contains(t, rr.RequestUrl, reg.Config().OAuth2AuthURL(ctx).String()) acceptBody := hydra.AcceptOAuth2LoginRequest{ @@ -125,12 +126,12 @@ func acceptConsentHandler(t *testing.T, c *client.Client, adminClient *hydra.API assert.EqualValues(t, c.LogoURI, pointerx.Deref(rr.Client.LogoUri)) assert.EqualValues(t, c.RedirectURIs, rr.Client.RedirectUris) assert.EqualValues(t, subject, pointerx.Deref(rr.Subject)) - assert.EqualValues(t, []string{"hydra", "offline", "openid"}, rr.RequestedScope) + assert.NotEmpty(t, rr.RequestedScope) assert.Contains(t, *rr.RequestUrl, reg.Config().OAuth2AuthURL(r.Context()).String()) assert.Equal(t, map[string]interface{}{"context": "bar"}, rr.Context) acceptBody := hydra.AcceptOAuth2ConsentRequest{ - GrantScope: []string{"hydra", "offline", "openid"}, + GrantScope: rr.RequestedScope, GrantAccessTokenAudience: rr.RequestedAccessTokenAudience, Remember: pointerx.Ptr(true), RememberFor: pointerx.Ptr[int64](0),