From fec1718f2056e3f53d64607561e9f0cd899ba632 Mon Sep 17 00:00:00 2001 From: huabing zhao Date: Thu, 28 Dec 2023 15:16:18 +0800 Subject: [PATCH] expose redirect uri to oidc API Signed-off-by: huabing zhao --- api/v1alpha1/oidc_types.go | 7 ++++ internal/gatewayapi/securitypolicy.go | 28 ++++++++++++-- internal/gatewayapi/securitypolicy_test.go | 37 +++++++++++++++++++ ...typolicy-with-jwt-and-invalid-oidc.in.yaml | 2 + ...ypolicy-with-jwt-and-invalid-oidc.out.yaml | 2 + ...itypolicy-with-oidc-invalid-issuer.in.yaml | 1 + ...typolicy-with-oidc-invalid-issuer.out.yaml | 1 + ...policy-with-oidc-invalid-secretref.in.yaml | 2 + ...olicy-with-oidc-invalid-secretref.out.yaml | 3 ++ .../testdata/securitypolicy-with-oidc.in.yaml | 2 + .../securitypolicy-with-oidc.out.yaml | 21 ++++++++--- internal/ir/xds.go | 8 +++- internal/xds/translator/oidc.go | 6 +-- .../translator/testdata/in/xds-ir/oidc.yaml | 6 +++ .../testdata/out/xds-ir/oidc.listeners.yaml | 12 +++--- site/content/en/latest/api/extension_types.md | 1 + 16 files changed, 118 insertions(+), 21 deletions(-) create mode 100644 internal/gatewayapi/securitypolicy_test.go diff --git a/api/v1alpha1/oidc_types.go b/api/v1alpha1/oidc_types.go index f690cfd7c56..cf8e0f2e547 100644 --- a/api/v1alpha1/oidc_types.go +++ b/api/v1alpha1/oidc_types.go @@ -30,6 +30,13 @@ type OIDC struct { // +kubebuilder:validation:Required ClientSecret gwapiv1b1.SecretObjectReference `json:"clientSecret"` + // The redirect URI to be used in the OIDC + // [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). + // + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=253 + RedirectURI string `json:"redirectURI"` + // The OIDC scopes to be used in the // [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). // The "openid" scope is always added to the list of scopes if not already diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go index f3c3e47930a..3e16b6c09d7 100644 --- a/internal/gatewayapi/securitypolicy.go +++ b/internal/gatewayapi/securitypolicy.go @@ -467,14 +467,34 @@ func (t *Translator) buildOIDC( } scopes := appendOpenidScopeIfNotExist(oidc.Scopes) + redirectPathMatcher := redirectPath(oidc.RedirectURI) + if redirectPathMatcher == "" { + return nil, fmt.Errorf("invalid redirect URI: %s", oidc.RedirectURI) + } + return &ir.OIDC{ - Provider: *provider, - ClientID: oidc.ClientID, - ClientSecret: clientSecretBytes, - Scopes: scopes, + Provider: *provider, + ClientID: oidc.ClientID, + ClientSecret: clientSecretBytes, + RedirectURI: oidc.RedirectURI, + RedirectPathMatcher: redirectPathMatcher, + Scopes: scopes, }, nil } +func redirectPath(redirectURI string) string { + count := 0 + for i, char := range redirectURI { + if char == '/' { + count++ + if count == 3 { + return redirectURI[i:] + } + } + } + return "" +} + // appendOpenidScopeIfNotExist appends the openid scope to the provided scopes // if it is not already present. // `openid` is a required scope for OIDC. diff --git a/internal/gatewayapi/securitypolicy_test.go b/internal/gatewayapi/securitypolicy_test.go new file mode 100644 index 00000000000..4b364973b37 --- /dev/null +++ b/internal/gatewayapi/securitypolicy_test.go @@ -0,0 +1,37 @@ +// Copyright Envoy Gateway Authors +// SPDX-License-Identifier: Apache-2.0 +// The full text of the Apache license is available in the LICENSE file at +// the root of the repo. + +package gatewayapi + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test_redirectPathMatcher(t *testing.T) { + + tests := []struct { + name string + redirectURI string + want string + }{ + { + name: "redirectURI with path", + redirectURI: "https://example.com/test/oauth2/callback", + want: "/test/oauth2/callback", + }, + { + name: "redirectURI with header tokens", + redirectURI: "%REQ(x-forwarded-proto)%://%REQ(:authority)%/test/oauth2/callback", + want: "/test/oauth2/callback", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equalf(t, tt.want, redirectPath(tt.redirectURI), "redirectPathMatcher(%v)", tt.redirectURI) + }) + } +} diff --git a/internal/gatewayapi/testdata/securitypolicy-with-jwt-and-invalid-oidc.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-jwt-and-invalid-oidc.in.yaml index 833c2b70c13..9e70e38a87c 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-jwt-and-invalid-oidc.in.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-jwt-and-invalid-oidc.in.yaml @@ -81,6 +81,7 @@ securityPolicies: clientID: "client1.apps.googleusercontent.com" clientSecret: name: "client1-secret" + redirectURI: "https://www.example.com/oauth2/callback" - apiVersion: gateway.envoyproxy.io/v1alpha1 kind: SecurityPolicy metadata: @@ -109,3 +110,4 @@ securityPolicies: clientID: "client1.apps.googleusercontent.com" clientSecret: name: "client2-secret" + redirectURI: "https://www.example.com/oauth2/callback" diff --git a/internal/gatewayapi/testdata/securitypolicy-with-jwt-and-invalid-oidc.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-jwt-and-invalid-oidc.out.yaml index 0af09853aab..4983d318c49 100755 --- a/internal/gatewayapi/testdata/securitypolicy-with-jwt-and-invalid-oidc.out.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-jwt-and-invalid-oidc.out.yaml @@ -159,6 +159,7 @@ securityPolicies: name: client2-secret provider: issuer: https://accounts.google.com + redirectURI: https://www.example.com/oauth2/callback targetRef: group: gateway.networking.k8s.io kind: HTTPRoute @@ -197,6 +198,7 @@ securityPolicies: name: client1-secret provider: issuer: https://accounts.google.com + redirectURI: https://www.example.com/oauth2/callback targetRef: group: gateway.networking.k8s.io kind: Gateway diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.in.yaml index 9f49012c528..eda4aaa98f6 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.in.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.in.yaml @@ -38,3 +38,4 @@ securityPolicies: clientID: "client1.apps.foo.bar.com" clientSecret: name: "client1-secret" + redirectURI: "https://www.example.com/oauth2/callback" diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.out.yaml index 14e0c27ca1e..728154229e2 100755 --- a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.out.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.out.yaml @@ -71,6 +71,7 @@ securityPolicies: name: client1-secret provider: issuer: https://httpbin.org/ + redirectURI: https://www.example.com/oauth2/callback targetRef: group: gateway.networking.k8s.io kind: Gateway diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.in.yaml index c3c86142e0b..0d92c45b832 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.in.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.in.yaml @@ -76,6 +76,7 @@ securityPolicies: clientID: "client1.apps.googleusercontent.com" clientSecret: name: "client1-secret" + redirectURI: "https://www.example.com/oauth2/callback" - apiVersion: gateway.envoyproxy.io/v1alpha1 kind: SecurityPolicy metadata: @@ -95,6 +96,7 @@ securityPolicies: clientSecret: namespace: envoy-gateway name: "client2-secret" + redirectURI: "https://www.example.com/oauth2/callback" - apiVersion: gateway.envoyproxy.io/v1alpha1 kind: SecurityPolicy metadata: diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.out.yaml index daec348c156..7890a9eb0a3 100755 --- a/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.out.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.out.yaml @@ -183,6 +183,7 @@ securityPolicies: authorizationEndpoint: https://accounts.google.com/o/oauth2/v2/auth issuer: https://accounts.google.com tokenEndpoint: https://oauth2.googleapis.com/token + redirectURI: https://www.example.com/oauth2/callback targetRef: group: gateway.networking.k8s.io kind: Gateway @@ -212,6 +213,7 @@ securityPolicies: authorizationEndpoint: https://accounts.google.com/o/oauth2/v2/auth issuer: https://accounts.google.com tokenEndpoint: https://oauth2.googleapis.com/token + redirectURI: https://www.example.com/oauth2/callback targetRef: group: gateway.networking.k8s.io kind: Gateway @@ -241,6 +243,7 @@ securityPolicies: authorizationEndpoint: https://accounts.google.com/o/oauth2/v2/auth issuer: https://accounts.google.com tokenEndpoint: https://oauth2.googleapis.com/token + redirectURI: "" targetRef: group: gateway.networking.k8s.io kind: Gateway diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc.in.yaml index 889fbfbfc8e..15dddbbe3e5 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-oidc.in.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc.in.yaml @@ -107,6 +107,7 @@ securityPolicies: clientID: "client1.apps.googleusercontent.com" clientSecret: name: "client1-secret" + redirectURI: "https://www.example.com/bar/oauth2/callback" - apiVersion: gateway.envoyproxy.io/v1alpha1 kind: SecurityPolicy metadata: @@ -126,6 +127,7 @@ securityPolicies: clientID: "client2.oauth.foo.com" clientSecret: name: "client2-secret" + redirectURI: "https://www.example.com/foo/oauth2/callback" scopes: ["openid", "email", "profile"] - apiVersion: gateway.envoyproxy.io/v1alpha1 kind: SecurityPolicy diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml index 57b031b3e3f..ef4170d71fe 100755 --- a/internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml @@ -184,6 +184,7 @@ securityPolicies: authorizationEndpoint: https://oauth.foo.com/oauth2/v2/auth issuer: https://oauth.foo.com tokenEndpoint: https://oauth.foo.com/token + redirectURI: https://www.example.com/foo/oauth2/callback scopes: - openid - email @@ -218,6 +219,7 @@ securityPolicies: authorizationEndpoint: https://oauth.bar.com/oauth2/v2/auth issuer: https://oauth.bar.com tokenEndpoint: https://oauth.bar.com/token + redirectURI: "" targetRef: group: gateway.networking.k8s.io kind: GRPCRoute @@ -225,9 +227,9 @@ securityPolicies: status: conditions: - lastTransitionTime: null - message: SecurityPolicy has been accepted. - reason: Accepted - status: "True" + message: 'Invalid redirect URI: ' + reason: Invalid + status: "False" type: Accepted - apiVersion: gateway.envoyproxy.io/v1alpha1 kind: SecurityPolicy @@ -244,6 +246,7 @@ securityPolicies: name: client1-secret provider: issuer: https://accounts.google.com + redirectURI: https://www.example.com/bar/oauth2/callback targetRef: group: gateway.networking.k8s.io kind: Gateway @@ -289,6 +292,8 @@ xdsIR: provider: authorizationEndpoint: https://oauth.foo.com/oauth2/v2/auth tokenEndpoint: https://oauth.foo.com/token + redirectPathMatcher: /foo/oauth2/callback + redirectURI: https://www.example.com/foo/oauth2/callback scopes: - openid - email @@ -317,6 +322,8 @@ xdsIR: provider: authorizationEndpoint: https://accounts.google.com/o/oauth2/v2/auth tokenEndpoint: https://oauth2.googleapis.com/token + redirectPathMatcher: /bar/oauth2/callback + redirectURI: https://www.example.com/bar/oauth2/callback scopes: - openid pathMatch: @@ -338,10 +345,12 @@ xdsIR: hostname: '*' name: grpcroute/default/grpcroute-1/rule/0/match/-1/* oidc: - clientID: client3.bar.foo.com + clientID: client1.apps.googleusercontent.com clientSecret: Y2xpZW50MTpzZWNyZXQK provider: - authorizationEndpoint: https://oauth.bar.com/oauth2/v2/auth - tokenEndpoint: https://oauth.bar.com/token + authorizationEndpoint: https://accounts.google.com/o/oauth2/v2/auth + tokenEndpoint: https://oauth2.googleapis.com/token + redirectPathMatcher: /bar/oauth2/callback + redirectURI: https://www.example.com/bar/oauth2/callback scopes: - openid diff --git a/internal/ir/xds.go b/internal/ir/xds.go index 54403633764..b521d2a4e24 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -365,8 +365,14 @@ type OIDC struct { // [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). // // This is an Opaque secret. The client secret should be stored in the key "client-secret". + ClientSecret []byte `json:"clientSecret" yaml:"clientSecret"` - ClientSecret []byte `json:"clientSecret,omitempty" yaml:"clientSecret,omitempty"` + // The redirect URI to be used in the + // [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). + RedirectURI string `json:"redirectURI" yaml:"redirectURI"` + + // Matching rule for the redirect path. + RedirectPathMatcher string `json:"redirectPathMatcher" yaml:"redirectPathMatcher"` // The OIDC scopes to be used in the // [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). diff --git a/internal/xds/translator/oidc.go b/internal/xds/translator/oidc.go index 018ad0a120b..cd786fc962f 100644 --- a/internal/xds/translator/oidc.go +++ b/internal/xds/translator/oidc.go @@ -28,8 +28,6 @@ import ( const ( oauth2Filter = "envoy.filters.http.oauth2" defaultTokenEndpointTimeout = 10 - redirectURL = "%REQ(x-forwarded-proto)%://%REQ(:authority)%/oauth2/callback" - redirectPathMatcher = "/oauth2/callback" defaultSignoutPath = "/signout" ) @@ -131,12 +129,12 @@ func oauth2Config(route *ir.HTTPRoute) (*oauth2v3.OAuth2, error) { }, }, AuthorizationEndpoint: route.OIDC.Provider.AuthorizationEndpoint, - RedirectUri: redirectURL, + RedirectUri: route.OIDC.RedirectURI, RedirectPathMatcher: &matcherv3.PathMatcher{ Rule: &matcherv3.PathMatcher_Path{ Path: &matcherv3.StringMatcher{ MatchPattern: &matcherv3.StringMatcher_Exact{ - Exact: redirectPathMatcher, + Exact: route.OIDC.RedirectPathMatcher, }, }, }, diff --git a/internal/xds/translator/testdata/in/xds-ir/oidc.yaml b/internal/xds/translator/testdata/in/xds-ir/oidc.yaml index efbd8b9ea69..ba9301ec0e6 100644 --- a/internal/xds/translator/testdata/in/xds-ir/oidc.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/oidc.yaml @@ -18,6 +18,8 @@ http: oidc: clientID: client.oauth.foo.com clientSecret: Y2xpZW50MTpzZWNyZXQK + redirectURI: "https://www.example.com/foo/oauth2/callback" + redirectPathMatcher: "/foo/oauth2/callback" provider: authorizationEndpoint: https://oauth.foo.com/oauth2/v2/auth tokenEndpoint: https://oauth.foo.com/token @@ -38,6 +40,8 @@ http: oidc: clientID: client.oauth.bar.com clientSecret: Y2xpZW50MTpzZWNyZXQK + redirectURI: "https://www.example.com/bar/oauth2/callback" + redirectPathMatcher: "/bar/oauth2/callback" provider: authorizationEndpoint: https://oauth.bar.com/oauth2/v2/auth tokenEndpoint: https://oauth.bar.com/token @@ -58,6 +62,8 @@ http: oidc: clientID: test.oauth.bar.com clientSecret: Y2xpZW50MTpzZWNyZXQK + redirectURI: "https://www.example.com/test/oauth2/callback" + redirectPathMatcher: "/test/oauth2/callback" provider: authorizationEndpoint: https://oauth.bar.com/oauth2/v2/auth tokenEndpoint: https://oauth.bar.com/token diff --git a/internal/xds/translator/testdata/out/xds-ir/oidc.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/oidc.listeners.yaml index 2a001af7866..49b541ef135 100755 --- a/internal/xds/translator/testdata/out/xds-ir/oidc.listeners.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/oidc.listeners.yaml @@ -39,8 +39,8 @@ forwardBearerToken: true redirectPathMatcher: path: - exact: /oauth2/callback - redirectUri: '%REQ(x-forwarded-proto)%://%REQ(:authority)%/oauth2/callback' + exact: /foo/oauth2/callback + redirectUri: https://www.example.com/foo/oauth2/callback signoutPath: path: exact: /signout @@ -73,8 +73,8 @@ forwardBearerToken: true redirectPathMatcher: path: - exact: /oauth2/callback - redirectUri: '%REQ(x-forwarded-proto)%://%REQ(:authority)%/oauth2/callback' + exact: /bar/oauth2/callback + redirectUri: https://www.example.com/bar/oauth2/callback signoutPath: path: exact: /signout @@ -107,8 +107,8 @@ forwardBearerToken: true redirectPathMatcher: path: - exact: /oauth2/callback - redirectUri: '%REQ(x-forwarded-proto)%://%REQ(:authority)%/oauth2/callback' + exact: /test/oauth2/callback + redirectUri: https://www.example.com/test/oauth2/callback signoutPath: path: exact: /signout diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index e5ec70e86e4..36d239a4399 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -1245,6 +1245,7 @@ _Appears in:_ | `clientID` _string_ | The client ID to be used in the OIDC [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). | | `clientSecret` _[SecretObjectReference](https://gateway-api.sigs.k8s.io/references/spec/#gateway.networking.k8s.io/v1.SecretObjectReference)_ | The Kubernetes secret which contains the OIDC client secret to be used in the [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). This is an Opaque secret. The client secret should be stored in the key "client-secret". | +| `redirectURI` _string_ | The redirect URI to be used in the OIDC [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). | | `scopes` _string array_ | The OIDC scopes to be used in the [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). The "openid" scope is always added to the list of scopes if not already specified. |