From 48518d5ec5e90c6bb5f3747afb453704ae03f6cb Mon Sep 17 00:00:00 2001 From: Jonas Hungershausen Date: Fri, 10 Nov 2023 15:13:08 +0100 Subject: [PATCH 1/4] feat: allow additional id token audiences --- embedx/config.schema.json | 13 ++++-- selfservice/strategy/oidc/provider_apple.go | 17 +++----- .../strategy/oidc/provider_apple_test.go | 16 ++++++- selfservice/strategy/oidc/provider_config.go | 6 ++- selfservice/strategy/oidc/provider_google.go | 16 ++----- .../strategy/oidc/provider_google_test.go | 19 ++++++-- selfservice/strategy/oidc/token_verifier.go | 43 +++++++++++++++++++ 7 files changed, 96 insertions(+), 34 deletions(-) create mode 100644 selfservice/strategy/oidc/token_verifier.go diff --git a/embedx/config.schema.json b/embedx/config.schema.json index 0f1cfd03546d..70ac3a54c9d0 100644 --- a/embedx/config.schema.json +++ b/embedx/config.schema.json @@ -531,6 +531,14 @@ "description": "The ID of the organization that this provider belongs to. Only effective in the Ory Network.", "type": "string", "examples": ["12345678-1234-1234-1234-123456789012"] + }, + "allowed_id_token_audiences": { + "title": "Additional client ids allowed when using ID token submission", + "type": "array", + "items": { + "type": "string", + "examples": ["12345678-1234-1234-1234-123456789012"] + } } }, "additionalProperties": false, @@ -2004,10 +2012,7 @@ "type": "string", "title": "Default Read Consistency Level", "description": "The default consistency level to use when reading from the database. Defaults to `strong` to not break existing API contracts. Only set this to `eventual` if you can accept that other read APIs will suddenly return eventually consistent results. It is only effective in Ory Network.", - "enum": [ - "strong", - "eventual" - ], + "enum": ["strong", "eventual"], "default": "strong" } } diff --git a/selfservice/strategy/oidc/provider_apple.go b/selfservice/strategy/oidc/provider_apple.go index 27dd2b7e6635..d680d0c1faae 100644 --- a/selfservice/strategy/oidc/provider_apple.go +++ b/selfservice/strategy/oidc/provider_apple.go @@ -154,20 +154,13 @@ func (a *ProviderApple) DecodeQuery(query url.Values, claims *Claims) { var _ IDTokenVerifier = new(ProviderApple) +const issuerUrlApple = "https://appleid.apple.com" + func (a *ProviderApple) Verify(ctx context.Context, rawIDToken string) (*Claims, error) { keySet := oidc.NewRemoteKeySet(ctx, a.JWKSUrl) - verifier := oidc.NewVerifier("https://appleid.apple.com", keySet, &oidc.Config{ - ClientID: a.config.ClientID, - }) - token, err := verifier.Verify(oidc.ClientContext(ctx, a.reg.HTTPClient(ctx).HTTPClient), rawIDToken) - if err != nil { - return nil, err - } - claims := &Claims{} - if err := token.Claims(claims); err != nil { - return nil, err - } - return claims, nil + + ctx = oidc.ClientContext(ctx, a.reg.HTTPClient(ctx).HTTPClient) + return verifyToken(ctx, keySet, a.config, rawIDToken, issuerUrlApple) } var _ NonceValidationSkipper = new(ProviderApple) diff --git a/selfservice/strategy/oidc/provider_apple_test.go b/selfservice/strategy/oidc/provider_apple_test.go index 69d5dfd44b69..3aff8b901eed 100644 --- a/selfservice/strategy/oidc/provider_apple_test.go +++ b/selfservice/strategy/oidc/provider_apple_test.go @@ -48,7 +48,6 @@ func TestDecodeQuery(t *testing.T) { assert.Empty(t, tc.claims.Email) }) } - } func TestAppleVerify(t *testing.T) { @@ -94,7 +93,7 @@ func TestAppleVerify(t *testing.T) { _, err := apple.Verify(context.Background(), token) require.Error(t, err) - assert.Equal(t, `oidc: expected audience "com.example.app" got ["com.different-example.app"]`, err.Error()) + assert.Equal(t, `token audience didn't match allowed audiences: [com.example.app] oidc: expected audience "com.example.app" got ["com.different-example.app"]`, err.Error()) }) t.Run("case=fails due to jwks mismatch", func(t *testing.T) { @@ -109,4 +108,17 @@ func TestAppleVerify(t *testing.T) { require.Error(t, err) assert.Equal(t, "failed to verify signature: failed to verify id token signature", err.Error()) }) + + t.Run("case=succeedes with additional id token audience", func(t *testing.T) { + _, reg := internal.NewFastRegistryWithMocks(t) + apple := oidc.NewProviderApple(&oidc.Configuration{ + ClientID: "something.else.app", + AdditionalIDTokenAudiences: []string{"com.example.app"}, + }, reg).(*oidc.ProviderApple) + apple.JWKSUrl = ts.URL + token := createIdToken(t, makeClaims("com.example.app")) + + _, err := apple.Verify(context.Background(), token) + require.NoError(t, err) + }) } diff --git a/selfservice/strategy/oidc/provider_config.go b/selfservice/strategy/oidc/provider_config.go index 4c8275c4f709..c6fc84f762de 100644 --- a/selfservice/strategy/oidc/provider_config.go +++ b/selfservice/strategy/oidc/provider_config.go @@ -99,7 +99,7 @@ type Configuration struct { // It can be either a URL (file://, http(s)://, base64://) or an inline JSONNet code snippet. Mapper string `json:"mapper_url"` - // RequestedClaims string encoded json object that specifies claims and optionally their properties which should be + // RequestedClaims is a string encoded json object that specifies claims and optionally their properties that should be // included in the id_token or returned from the UserInfo Endpoint. // // More information: https://openid.net/specs/openid-connect-core-1_0.html#ClaimsParameter @@ -108,6 +108,10 @@ type Configuration struct { // An optional organization ID that this provider belongs to. // This parameter is only effective in the Ory Network. OrganizationID string `json:"organization_id"` + + // AdditionalIDTokenAudiences is a list of additional audiences allowed in the ID Token. + // This is only relevant in OIDC flows that submit an IDToken instead of using the callback from the OIDC provider. + AdditionalIDTokenAudiences []string `json:"additional_id_token_audiences"` } func (p Configuration) Redir(public *url.URL) string { diff --git a/selfservice/strategy/oidc/provider_google.go b/selfservice/strategy/oidc/provider_google.go index c1ccdf505383..db7b4fdd14e5 100644 --- a/selfservice/strategy/oidc/provider_google.go +++ b/selfservice/strategy/oidc/provider_google.go @@ -72,20 +72,12 @@ func (g *ProviderGoogle) AuthCodeURLOptions(r ider) []oauth2.AuthCodeOption { var _ IDTokenVerifier = new(ProviderGoogle) +const issuerUrlGoogle = "https://accounts.google.com" + func (p *ProviderGoogle) Verify(ctx context.Context, rawIDToken string) (*Claims, error) { keySet := oidc.NewRemoteKeySet(ctx, p.JWKSUrl) - verifier := oidc.NewVerifier("https://accounts.google.com", keySet, &oidc.Config{ - ClientID: p.config.ClientID, - }) - token, err := verifier.Verify(oidc.ClientContext(ctx, p.reg.HTTPClient(ctx).HTTPClient), rawIDToken) - if err != nil { - return nil, err - } - claims := &Claims{} - if err := token.Claims(claims); err != nil { - return nil, err - } - return claims, nil + ctx = oidc.ClientContext(ctx, p.reg.HTTPClient(ctx).HTTPClient) + return verifyToken(ctx, keySet, p.config, rawIDToken, issuerUrlGoogle) } var _ NonceValidationSkipper = new(ProviderGoogle) diff --git a/selfservice/strategy/oidc/provider_google_test.go b/selfservice/strategy/oidc/provider_google_test.go index 2d849948d391..34b4d46e73b2 100644 --- a/selfservice/strategy/oidc/provider_google_test.go +++ b/selfservice/strategy/oidc/provider_google_test.go @@ -98,8 +98,8 @@ func TestVerify(t *testing.T) { c, err := p.Verify(context.Background(), token) require.NoError(t, err) - assert.Equal(t, "apple@ory.sh", c.Email) - assert.Equal(t, "apple@ory.sh", c.Subject) + assert.Equal(t, "google@ory.sh", c.Email) + assert.Equal(t, "google@ory.sh", c.Subject) assert.Equal(t, "https://accounts.google.com", c.Issuer) }) @@ -109,7 +109,7 @@ func TestVerify(t *testing.T) { _, err := p.Verify(context.Background(), token) require.Error(t, err) - assert.Equal(t, `oidc: expected audience "com.example.app" got ["com.different-example.app"]`, err.Error()) + assert.Equal(t, `token audience didn't match allowed audiences: [com.example.app] oidc: expected audience "com.example.app" got ["com.different-example.app"]`, err.Error()) }) t.Run("case=fails due to jwks mismatch", func(t *testing.T) { @@ -120,4 +120,17 @@ func TestVerify(t *testing.T) { require.Error(t, err) assert.Equal(t, "failed to verify signature: failed to verify id token signature", err.Error()) }) + + t.Run("case=succeedes with additional id token audience", func(t *testing.T) { + _, reg := internal.NewFastRegistryWithMocks(t) + google := oidc.NewProviderGoogle(&oidc.Configuration{ + ClientID: "something.else.app", + AdditionalIDTokenAudiences: []string{"com.example.app"}, + }, reg).(*oidc.ProviderGoogle) + google.JWKSUrl = ts.URL + token := createIdToken(t, makeClaims("com.example.app")) + + _, err := google.Verify(context.Background(), token) + require.NoError(t, err) + }) } diff --git a/selfservice/strategy/oidc/token_verifier.go b/selfservice/strategy/oidc/token_verifier.go new file mode 100644 index 000000000000..076273019fee --- /dev/null +++ b/selfservice/strategy/oidc/token_verifier.go @@ -0,0 +1,43 @@ +// Copyright © 2023 Ory Corp +// SPDX-License-Identifier: Apache-2.0 + +package oidc + +import ( + "context" + "fmt" + "strings" + + "github.com/coreos/go-oidc" +) + +func verifyToken(ctx context.Context, keySet oidc.KeySet, config *Configuration, rawIDToken, issuerURL string) (*Claims, error) { + // keySet := oidc.NewRemoteKeySet(ctx, a.JWKSUrl) + tokenAudiences := append([]string{config.ClientID}, config.AdditionalIDTokenAudiences...) + var token *oidc.IDToken + var err error + for _, aud := range tokenAudiences { + verifier := oidc.NewVerifier(issuerURL, keySet, &oidc.Config{ + ClientID: aud, + }) + token, err = verifier.Verify(ctx, rawIDToken) + if err != nil && strings.Contains(err.Error(), "oidc: expected audience") { + // The audience is not the one we expect, try the next one + continue + } else if err != nil { + // Something else went wrong + return nil, err + } + // The token was verified successfully + break + } + if err != nil { + // None of the allowed audiences matched the audience in the token + return nil, fmt.Errorf("token audience didn't match allowed audiences: %+v %w", tokenAudiences, err) + } + claims := &Claims{} + if err := token.Claims(claims); err != nil { + return nil, err + } + return claims, nil +} From 4592016f60072ab461472ad96140c39fb72ba41c Mon Sep 17 00:00:00 2001 From: Jonas Hungershausen Date: Fri, 10 Nov 2023 15:15:34 +0100 Subject: [PATCH 2/4] chore: u --- selfservice/strategy/oidc/token_verifier.go | 1 - 1 file changed, 1 deletion(-) diff --git a/selfservice/strategy/oidc/token_verifier.go b/selfservice/strategy/oidc/token_verifier.go index 076273019fee..f74a95291c02 100644 --- a/selfservice/strategy/oidc/token_verifier.go +++ b/selfservice/strategy/oidc/token_verifier.go @@ -12,7 +12,6 @@ import ( ) func verifyToken(ctx context.Context, keySet oidc.KeySet, config *Configuration, rawIDToken, issuerURL string) (*Claims, error) { - // keySet := oidc.NewRemoteKeySet(ctx, a.JWKSUrl) tokenAudiences := append([]string{config.ClientID}, config.AdditionalIDTokenAudiences...) var token *oidc.IDToken var err error From 7a9425f373783656d816cfa1ca49455bce3505b9 Mon Sep 17 00:00:00 2001 From: Jonas Hungershausen Date: Sun, 12 Nov 2023 12:58:36 +0100 Subject: [PATCH 3/4] chore: fix tests --- selfservice/strategy/oidc/provider_apple_test.go | 6 +++--- selfservice/strategy/oidc/provider_google_test.go | 8 ++++---- selfservice/strategy/oidc/strategy_helper_test.go | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/selfservice/strategy/oidc/provider_apple_test.go b/selfservice/strategy/oidc/provider_apple_test.go index 3aff8b901eed..422ae643708a 100644 --- a/selfservice/strategy/oidc/provider_apple_test.go +++ b/selfservice/strategy/oidc/provider_apple_test.go @@ -63,7 +63,7 @@ func TestAppleVerify(t *testing.T) { makeClaims := func(aud string) jwt.RegisteredClaims { return jwt.RegisteredClaims{ Issuer: "https://appleid.apple.com", - Subject: "apple@ory.sh", + Subject: "acme@ory.sh", Audience: jwt.ClaimStrings{aud}, ExpiresAt: jwt.NewNumericDate(time.Now().Add(24 * time.Hour)), } @@ -78,8 +78,8 @@ func TestAppleVerify(t *testing.T) { c, err := apple.Verify(context.Background(), token) require.NoError(t, err) - assert.Equal(t, "apple@ory.sh", c.Email) - assert.Equal(t, "apple@ory.sh", c.Subject) + assert.Equal(t, "acme@ory.sh", c.Email) + assert.Equal(t, "acme@ory.sh", c.Subject) assert.Equal(t, "https://appleid.apple.com", c.Issuer) }) diff --git a/selfservice/strategy/oidc/provider_google_test.go b/selfservice/strategy/oidc/provider_google_test.go index 34b4d46e73b2..d900b088d358 100644 --- a/selfservice/strategy/oidc/provider_google_test.go +++ b/selfservice/strategy/oidc/provider_google_test.go @@ -59,7 +59,7 @@ func TestProviderGoogle_AccessType(t *testing.T) { assert.Contains(t, options, oauth2.AccessTypeOffline) } -func TestVerify(t *testing.T) { +func TestGoogleVerify(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(200) w.Write(publicJWKS) @@ -73,7 +73,7 @@ func TestVerify(t *testing.T) { makeClaims := func(aud string) jwt.RegisteredClaims { return jwt.RegisteredClaims{ Issuer: "https://accounts.google.com", - Subject: "apple@ory.sh", + Subject: "acme@ory.sh", Audience: jwt.ClaimStrings{aud}, ExpiresAt: jwt.NewNumericDate(time.Now().Add(24 * time.Hour)), } @@ -98,8 +98,8 @@ func TestVerify(t *testing.T) { c, err := p.Verify(context.Background(), token) require.NoError(t, err) - assert.Equal(t, "google@ory.sh", c.Email) - assert.Equal(t, "google@ory.sh", c.Subject) + assert.Equal(t, "acme@ory.sh", c.Email) + assert.Equal(t, "acme@ory.sh", c.Subject) assert.Equal(t, "https://accounts.google.com", c.Issuer) }) diff --git a/selfservice/strategy/oidc/strategy_helper_test.go b/selfservice/strategy/oidc/strategy_helper_test.go index 0c92de45d0c7..51df94357bda 100644 --- a/selfservice/strategy/oidc/strategy_helper_test.go +++ b/selfservice/strategy/oidc/strategy_helper_test.go @@ -119,7 +119,7 @@ func newHydraIntegration(t *testing.T, remote *string, subject *string, claims * GrantScope []string `json:"grant_scope,omitempty"` } - var do = func(w http.ResponseWriter, r *http.Request, href string, payload io.Reader) { + do := func(w http.ResponseWriter, r *http.Request, href string, payload io.Reader) { req, err := http.NewRequest("PUT", href, payload) require.NoError(t, err) req.Header.Set("Content-Type", "application/json") @@ -370,7 +370,7 @@ func createIdToken(t *testing.T, cl jwt.RegisteredClaims) string { require.NoError(t, json.Unmarshal(rawKey, key)) token := jwt.NewWithClaims(jwt.SigningMethodRS256, &claims{ RegisteredClaims: &cl, - Email: "apple@ory.sh", + Email: "acme@ory.sh", }) token.Header["kid"] = key.KeyID s, err := token.SignedString(key.Key) From 6035910b05b9869019bee6fa626e08ebf66b4c92 Mon Sep 17 00:00:00 2001 From: Jonas Hungershausen Date: Mon, 13 Nov 2023 12:20:32 +0100 Subject: [PATCH 4/4] chore: add fallback error --- selfservice/strategy/oidc/token_verifier.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/selfservice/strategy/oidc/token_verifier.go b/selfservice/strategy/oidc/token_verifier.go index f74a95291c02..e30a529bc6cc 100644 --- a/selfservice/strategy/oidc/token_verifier.go +++ b/selfservice/strategy/oidc/token_verifier.go @@ -14,7 +14,7 @@ import ( func verifyToken(ctx context.Context, keySet oidc.KeySet, config *Configuration, rawIDToken, issuerURL string) (*Claims, error) { tokenAudiences := append([]string{config.ClientID}, config.AdditionalIDTokenAudiences...) var token *oidc.IDToken - var err error + err := fmt.Errorf("no audience matched the token's audience") for _, aud := range tokenAudiences { verifier := oidc.NewVerifier(issuerURL, keySet, &oidc.Config{ ClientID: aud,