From 2bce57571374f68857064e0356da7a476fe8440f Mon Sep 17 00:00:00 2001 From: sam-burrell Date: Thu, 25 Apr 2024 17:08:45 +0100 Subject: [PATCH 1/6] Added ability to specifiy a cookieSuffix Signed-off-by: sam-burrell Signed-off-by: Connor Rogers <23215294+coro@users.noreply.github.com> --- api/v1alpha1/oidc_types.go | 5 +++++ .../generated/gateway.envoyproxy.io_securitypolicies.yaml | 6 ++++++ internal/gatewayapi/securitypolicy.go | 7 ++++++- site/content/en/latest/api/extension_types.md | 1 + 4 files changed, 18 insertions(+), 1 deletion(-) diff --git a/api/v1alpha1/oidc_types.go b/api/v1alpha1/oidc_types.go index ecce7957627..00a6e485e2c 100644 --- a/api/v1alpha1/oidc_types.go +++ b/api/v1alpha1/oidc_types.go @@ -30,6 +30,11 @@ type OIDC struct { // +kubebuilder:validation:Required ClientSecret gwapiv1b1.SecretObjectReference `json:"clientSecret"` + // The optional cookie suffix to be added to Bearer and IdToken cookies in the + // [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). + // If not specified, uses a randomly generated suffix + CookieSuffix *string `json:"cookieSuffix,omitempty"` + // 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/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml index 919d272cf89..3c880161445 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml @@ -671,6 +671,12 @@ spec: required: - name type: object + cookieSuffix: + description: |- + The optional cookie suffix to be added to Bearer and IdToken cookies in the + [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). + If not specified, uses a randomly generated suffix + type: string logoutPath: description: |- The path to log a user out, clearing their credential cookies. diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go index e68cf3fabae..ce8d5cca841 100644 --- a/internal/gatewayapi/securitypolicy.go +++ b/internal/gatewayapi/securitypolicy.go @@ -582,8 +582,13 @@ func (t *Translator) buildOIDC( logoutPath = *oidc.LogoutPath } - // Generate a unique cookie suffix for oauth filters + // Generate a unique cookie suffix for oauth filters if CookieSuffix is not provided + // This is to avoid cookie name collision when multiple security policies are applied + // to the same route. suffix := utils.Digest32(string(policy.UID)) + if oidc.CookieSuffix != nil { + suffix = *oidc.CookieSuffix + } // Get the HMAC secret // HMAC secret is generated by the CertGen job and stored in a secret diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index b7dd708a308..28fff44a958 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -2278,6 +2278,7 @@ _Appears in:_ | `provider` | _[OIDCProvider](#oidcprovider)_ | true | The OIDC Provider configuration. | | `clientID` | _string_ | true | 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)_ | true | 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". | +| `cookieSuffix` | _string_ | false | The optional cookie suffix to be added to Bearer and IdToken cookies in the [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). If not specified, uses a randomly generated suffix | | `scopes` | _string array_ | false | 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. | | `resources` | _string array_ | false | The OIDC resources to be used in the
[Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). | | `redirectURL` | _string_ | true | The redirect URL to be used in the OIDC
[Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).
If not specified, uses the default redirect URI "%REQ(x-forwarded-proto)%://%REQ(:authority)%/oauth2/callback" | From 1077aefc70182401c3461d99a176a48c8f84c82b Mon Sep 17 00:00:00 2001 From: Connor Rogers <23215294+coro@users.noreply.github.com> Date: Wed, 22 May 2024 13:49:24 +0100 Subject: [PATCH 2/6] Allow override of OAuth cookie names Co-authored-by: Sam Burrell Signed-off-by: Connor Rogers <23215294+coro@users.noreply.github.com> --- api/v1alpha1/oidc_types.go | 19 ++++++++++-- api/v1alpha1/zz_generated.deepcopy.go | 30 +++++++++++++++++++ internal/gatewayapi/securitypolicy.go | 28 ++++++++--------- internal/ir/xds.go | 3 ++ internal/ir/zz_generated.deepcopy.go | 5 ++++ internal/xds/translator/oidc.go | 11 +++++++ site/content/en/latest/api/extension_types.md | 17 ++++++++++- 7 files changed, 95 insertions(+), 18 deletions(-) diff --git a/api/v1alpha1/oidc_types.go b/api/v1alpha1/oidc_types.go index 00a6e485e2c..d49552d05ee 100644 --- a/api/v1alpha1/oidc_types.go +++ b/api/v1alpha1/oidc_types.go @@ -30,10 +30,11 @@ type OIDC struct { // +kubebuilder:validation:Required ClientSecret gwapiv1b1.SecretObjectReference `json:"clientSecret"` - // The optional cookie suffix to be added to Bearer and IdToken cookies in the + // The optional cookie name overrides to be used for Bearer and IdToken cookies in the // [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). // If not specified, uses a randomly generated suffix - CookieSuffix *string `json:"cookieSuffix,omitempty"` + // +optional + CookieNames *OIDCCookieNames `json:"cookieNames,omitempty"` // The OIDC scopes to be used in the // [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). @@ -80,3 +81,17 @@ type OIDCProvider struct { // +optional TokenEndpoint *string `json:"tokenEndpoint,omitempty"` } + +// OIDCCookieNames defines the names of cookies to use in the Envoy OIDC filter. +type OIDCCookieNames struct { + // The name of the cookie used to store the BearerToken in the + // [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). + // If not specified, defaults to "BearerToken-(randomly generated uid)" + // +optional + BearerToken *string `json:"bearerToken,omitempty"` + // The name of the cookie used to store the IdToken in the + // [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). + // If not specified, defaults to "IdToken-(randomly generated uid)" + // +optional + IDToken *string `json:"idToken,omitempty"` +} diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index e2d32b0df8e..c93c94bab73 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -3176,6 +3176,11 @@ func (in *OIDC) DeepCopyInto(out *OIDC) { *out = *in in.Provider.DeepCopyInto(&out.Provider) in.ClientSecret.DeepCopyInto(&out.ClientSecret) + if in.CookieNames != nil { + in, out := &in.CookieNames, &out.CookieNames + *out = new(OIDCCookieNames) + (*in).DeepCopyInto(*out) + } if in.Scopes != nil { in, out := &in.Scopes, &out.Scopes *out = make([]string, len(*in)) @@ -3208,6 +3213,31 @@ func (in *OIDC) DeepCopy() *OIDC { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OIDCCookieNames) DeepCopyInto(out *OIDCCookieNames) { + *out = *in + if in.BearerToken != nil { + in, out := &in.BearerToken, &out.BearerToken + *out = new(string) + **out = **in + } + if in.IDToken != nil { + in, out := &in.IDToken, &out.IDToken + *out = new(string) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OIDCCookieNames. +func (in *OIDCCookieNames) DeepCopy() *OIDCCookieNames { + if in == nil { + return nil + } + out := new(OIDCCookieNames) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OIDCProvider) DeepCopyInto(out *OIDCProvider) { *out = *in diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go index ce8d5cca841..b3eeda8dff5 100644 --- a/internal/gatewayapi/securitypolicy.go +++ b/internal/gatewayapi/securitypolicy.go @@ -582,13 +582,10 @@ func (t *Translator) buildOIDC( logoutPath = *oidc.LogoutPath } - // Generate a unique cookie suffix for oauth filters if CookieSuffix is not provided + // Generate a unique cookie suffix for oauth filters. // This is to avoid cookie name collision when multiple security policies are applied // to the same route. suffix := utils.Digest32(string(policy.UID)) - if oidc.CookieSuffix != nil { - suffix = *oidc.CookieSuffix - } // Get the HMAC secret // HMAC secret is generated by the CertGen job and stored in a secret @@ -605,17 +602,18 @@ func (t *Translator) buildOIDC( } return &ir.OIDC{ - Name: irConfigName(policy), - Provider: *provider, - ClientID: oidc.ClientID, - ClientSecret: clientSecretBytes, - Scopes: scopes, - Resources: oidc.Resources, - RedirectURL: redirectURL, - RedirectPath: redirectPath, - LogoutPath: logoutPath, - CookieSuffix: suffix, - HMACSecret: hmacData, + Name: irConfigName(policy), + Provider: *provider, + ClientID: oidc.ClientID, + ClientSecret: clientSecretBytes, + Scopes: scopes, + Resources: oidc.Resources, + RedirectURL: redirectURL, + RedirectPath: redirectPath, + LogoutPath: logoutPath, + CookieSuffix: suffix, + CookieNameOverrides: policy.Spec.OIDC.CookieNames, + HMACSecret: hmacData, }, nil } diff --git a/internal/ir/xds.go b/internal/ir/xds.go index 3dc44daefd8..6985564cb49 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -676,6 +676,9 @@ type OIDC struct { // These cookies are set by the oauth filter, including: BearerToken, // OauthHMAC, OauthExpires, IdToken, and RefreshToken. CookieSuffix string `json:"cookieSuffix,omitempty"` + + // CookieNameOverrides can optionally override the generated name of the cookies set by the oauth filter. + CookieNameOverrides *egv1a1.OIDCCookieNames `json:"cookieNameOverrides,omitempty"` } type OIDCProvider struct { diff --git a/internal/ir/zz_generated.deepcopy.go b/internal/ir/zz_generated.deepcopy.go index 662f0d7f721..8a8abfcdf72 100644 --- a/internal/ir/zz_generated.deepcopy.go +++ b/internal/ir/zz_generated.deepcopy.go @@ -1482,6 +1482,11 @@ func (in *OIDC) DeepCopyInto(out *OIDC) { *out = make([]string, len(*in)) copy(*out, *in) } + if in.CookieNameOverrides != nil { + in, out := &in.CookieNameOverrides, &out.CookieNameOverrides + *out = new(v1alpha1.OIDCCookieNames) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OIDC. diff --git a/internal/xds/translator/oidc.go b/internal/xds/translator/oidc.go index 183fb7944aa..9be0bab06de 100644 --- a/internal/xds/translator/oidc.go +++ b/internal/xds/translator/oidc.go @@ -172,6 +172,17 @@ func oauth2Config(oidc *ir.OIDC) (*oauth2v3.OAuth2, error) { Resources: oidc.Resources, }, } + + if oidc.CookieNameOverrides != nil && + oidc.CookieNameOverrides.BearerToken != nil { + oauth2.Config.Credentials.CookieNames.BearerToken = *oidc.CookieNameOverrides.BearerToken + } + + if oidc.CookieNameOverrides != nil && + oidc.CookieNameOverrides.IDToken != nil { + oauth2.Config.Credentials.CookieNames.IdToken = *oidc.CookieNameOverrides.IDToken + } + return oauth2, nil } diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index 28fff44a958..b3019b03b6b 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -2278,13 +2278,28 @@ _Appears in:_ | `provider` | _[OIDCProvider](#oidcprovider)_ | true | The OIDC Provider configuration. | | `clientID` | _string_ | true | 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)_ | true | 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". | -| `cookieSuffix` | _string_ | false | The optional cookie suffix to be added to Bearer and IdToken cookies in the [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). If not specified, uses a randomly generated suffix | +| `cookieNames` | _[OIDCCookieNames](#oidccookienames)_ | false | The optional cookie name overrides to be used for Bearer and IdToken cookies in the
[Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).
If not specified, uses a randomly generated suffix | | `scopes` | _string array_ | false | 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. | | `resources` | _string array_ | false | The OIDC resources to be used in the
[Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). | | `redirectURL` | _string_ | true | The redirect URL to be used in the OIDC
[Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).
If not specified, uses the default redirect URI "%REQ(x-forwarded-proto)%://%REQ(:authority)%/oauth2/callback" | | `logoutPath` | _string_ | true | The path to log a user out, clearing their credential cookies.
If not specified, uses a default logout path "/logout" | +#### OIDCCookieNames + + + +OIDCCookieNames defines the names of cookies to use in the Envoy OIDC filter. + +_Appears in:_ +- [OIDC](#oidc) + +| Field | Type | Required | Description | +| --- | --- | --- | --- | +| `bearerToken` | _string_ | false | The name of the cookie used to store the BearerToken in the
[Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).
If not specified, defaults to "BearerToken-(randomly generated uid)" | +| `idToken` | _string_ | false | The name of the cookie used to store the IdToken in the
[Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).
If not specified, defaults to "IdToken-(randomly generated uid)" | + + #### OIDCProvider From e58c466a60cedb7a37daaaad5a556397c9720050 Mon Sep 17 00:00:00 2001 From: Connor Rogers <23215294+coro@users.noreply.github.com> Date: Wed, 22 May 2024 14:03:28 +0100 Subject: [PATCH 3/6] Update Helm CRD docs Co-authored-by: Sam Burrell Signed-off-by: Connor Rogers <23215294+coro@users.noreply.github.com> --- ...ateway.envoyproxy.io_securitypolicies.yaml | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml index 3c880161445..1360633e378 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml @@ -671,12 +671,25 @@ spec: required: - name type: object - cookieSuffix: + cookieNames: description: |- - The optional cookie suffix to be added to Bearer and IdToken cookies in the + The optional cookie name overrides to be used for Bearer and IdToken cookies in the [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). If not specified, uses a randomly generated suffix - type: string + properties: + bearerToken: + description: |- + The name of the cookie used to store the BearerToken in the + [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). + If not specified, defaults to "BearerToken-(randomly generated uid)" + type: string + idToken: + description: |- + The name of the cookie used to store the IdToken in the + [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). + If not specified, defaults to "IdToken-(randomly generated uid)" + type: string + type: object logoutPath: description: |- The path to log a user out, clearing their credential cookies. From 728ac577d8b7ef4287e8a04e28081c54937ac150 Mon Sep 17 00:00:00 2001 From: Connor Rogers <23215294+coro@users.noreply.github.com> Date: Thu, 23 May 2024 10:24:47 +0100 Subject: [PATCH 4/6] Add testdata for custom cookie name overrides Co-authored-by: Sam Burrell Signed-off-by: Connor Rogers <23215294+coro@users.noreply.github.com> --- ...itypolicy-with-oidc-custom-cookies.in.yaml | 66 +++++++ ...typolicy-with-oidc-custom-cookies.out.yaml | 169 ++++++++++++++++++ .../translator/testdata/in/xds-ir/oidc.yaml | 3 + .../testdata/out/xds-ir/oidc.listeners.yaml | 4 +- 4 files changed, 240 insertions(+), 2 deletions(-) create mode 100644 internal/gatewayapi/testdata/securitypolicy-with-oidc-custom-cookies.in.yaml create mode 100755 internal/gatewayapi/testdata/securitypolicy-with-oidc-custom-cookies.out.yaml diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc-custom-cookies.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc-custom-cookies.in.yaml new file mode 100644 index 00000000000..cd1c39f9677 --- /dev/null +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc-custom-cookies.in.yaml @@ -0,0 +1,66 @@ +secrets: +- apiVersion: v1 + kind: Secret + metadata: + namespace: envoy-gateway + name: client1-secret + data: + client-secret: Y2xpZW50MTpzZWNyZXQK +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + namespace: envoy-gateway + name: gateway-1 + spec: + gatewayClassName: envoy-gateway-class + listeners: + - name: http + protocol: HTTP + port: 80 + allowedRoutes: + namespaces: + from: All +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-1 + spec: + hostnames: + - www.example.com + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: http + rules: + - matches: + - path: + value: "/foo" + backendRefs: + - name: service-1 + port: 8080 +securityPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + namespace: envoy-gateway + name: policy-for-gateway # This policy should attach httproute-2 + uid: b8284d0f-de82-4c65-b204-96a0d3f258a1 + spec: + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + oidc: + provider: + issuer: "https://accounts.google.com" + clientID: "client1.apps.googleusercontent.com" + clientSecret: + name: "client1-secret" + redirectURL: "https://www.example.com/bar/oauth2/callback" + logoutPath: "/bar/logout" + cookieNames: + idToken: "CustomIdTokenCookie" + bearerToken: "CustomBearerTokenCookie" diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc-custom-cookies.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc-custom-cookies.out.yaml new file mode 100755 index 00000000000..ea01108758f --- /dev/null +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc-custom-cookies.out.yaml @@ -0,0 +1,169 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + creationTimestamp: null + name: gateway-1 + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + name: http + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 1 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: http + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + name: httproute-1 + namespace: default + spec: + hostnames: + - www.example.com + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + matches: + - path: + value: /foo + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway + sectionName: http +infraIR: + envoy-gateway/gateway-1: + proxy: + listeners: + - address: null + name: envoy-gateway/gateway-1/http + ports: + - containerPort: 10080 + name: http-80 + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-1 + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + name: envoy-gateway/gateway-1 +securityPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + creationTimestamp: null + name: policy-for-gateway + namespace: envoy-gateway + uid: b8284d0f-de82-4c65-b204-96a0d3f258a1 + spec: + oidc: + clientID: client1.apps.googleusercontent.com + clientSecret: + group: null + kind: null + name: client1-secret + cookieNames: + bearerToken: CustomBearerTokenCookie + idToken: CustomIdTokenCookie + logoutPath: /bar/logout + provider: + issuer: https://accounts.google.com + redirectURL: https://www.example.com/bar/oauth2/callback + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: HMAC secret envoy-gateway-system/envoy-oidc-hmac not found + reason: Invalid + status: "False" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller +xdsIR: + envoy-gateway/gateway-1: + accessLog: + text: + - path: /dev/stdout + http: + - address: 0.0.0.0 + hostnames: + - '*' + isHTTP2: false + name: envoy-gateway/gateway-1/http + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 10080 + routes: + - destination: + name: httproute/default/httproute-1/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + protocol: HTTP + weight: 1 + hostname: www.example.com + isHTTP2: false + name: httproute/default/httproute-1/rule/0/match/0/www_example_com + pathMatch: + distinct: false + name: "" + prefix: /foo + security: {} diff --git a/internal/xds/translator/testdata/in/xds-ir/oidc.yaml b/internal/xds/translator/testdata/in/xds-ir/oidc.yaml index b2e395f8320..ab09dd501b3 100644 --- a/internal/xds/translator/testdata/in/xds-ir/oidc.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/oidc.yaml @@ -66,3 +66,6 @@ http: redirectPath: "/bar/oauth2/callback" logoutPath: "/bar/logout" cookieSuffix: 5f93c2e4 + cookieNameOverrides: + idToken: "CustomIdTokenOverride" + bearerToken: "CustomBearerTokenOverride" 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 95e075f047e..7077a5f987e 100644 --- a/internal/xds/translator/testdata/out/xds-ir/oidc.listeners.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/oidc.listeners.yaml @@ -71,8 +71,8 @@ credentials: clientId: client.oauth.bar.com cookieNames: - bearerToken: BearerToken-5f93c2e4 - idToken: IdToken-5f93c2e4 + bearerToken: CustomBearerTokenOverride + idToken: CustomIdTokenOverride oauthExpires: OauthExpires-5f93c2e4 oauthHmac: OauthHMAC-5f93c2e4 refreshToken: RefreshToken-5f93c2e4 From 24ad396174c242990430d1e642d914401dfe1c3d Mon Sep 17 00:00:00 2001 From: Connor Rogers <23215294+coro@users.noreply.github.com> Date: Fri, 24 May 2024 10:14:04 +0100 Subject: [PATCH 5/6] Fix regression tests to have accepted gateway Signed-off-by: Connor Rogers <23215294+coro@users.noreply.github.com> --- ...itypolicy-with-oidc-custom-cookies.in.yaml | 7 ++++++ ...typolicy-with-oidc-custom-cookies.out.yaml | 25 ++++++++++++++++--- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc-custom-cookies.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc-custom-cookies.in.yaml index cd1c39f9677..8dcc03fdb46 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-oidc-custom-cookies.in.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc-custom-cookies.in.yaml @@ -6,6 +6,13 @@ secrets: name: client1-secret data: client-secret: Y2xpZW50MTpzZWNyZXQK +- apiVersion: v1 + kind: Secret + metadata: + namespace: envoy-gateway-system + name: envoy-oidc-hmac + data: + hmac-secret: qrOYACHXoe7UEDI/raOjNSx+Z9ufXSc/22C3T6X/zPY= gateways: - apiVersion: gateway.networking.k8s.io/v1 kind: Gateway diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc-custom-cookies.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc-custom-cookies.out.yaml index ea01108758f..3bfd96b0b96 100755 --- a/internal/gatewayapi/testdata/securitypolicy-with-oidc-custom-cookies.out.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc-custom-cookies.out.yaml @@ -129,9 +129,9 @@ securityPolicies: namespace: envoy-gateway conditions: - lastTransitionTime: null - message: HMAC secret envoy-gateway-system/envoy-oidc-hmac not found - reason: Invalid - status: "False" + message: Policy has been accepted. + reason: Accepted + status: "True" type: Accepted controllerName: gateway.envoyproxy.io/gatewayclass-controller xdsIR: @@ -166,4 +166,21 @@ xdsIR: distinct: false name: "" prefix: /foo - security: {} + security: + oidc: + clientID: client1.apps.googleusercontent.com + clientSecret: Y2xpZW50MTpzZWNyZXQK + cookieNameOverrides: + bearerToken: CustomBearerTokenCookie + idToken: CustomIdTokenCookie + cookieSuffix: b0a1b740 + hmacSecret: qrOYACHXoe7UEDI/raOjNSx+Z9ufXSc/22C3T6X/zPY= + logoutPath: /bar/logout + name: securitypolicy/envoy-gateway/policy-for-gateway + provider: + authorizationEndpoint: https://accounts.google.com/o/oauth2/v2/auth + tokenEndpoint: https://oauth2.googleapis.com/token + redirectPath: /bar/oauth2/callback + redirectURL: https://www.example.com/bar/oauth2/callback + scopes: + - openid From 14e216ce04328ba2497684b931118c0f6044838f Mon Sep 17 00:00:00 2001 From: Connor Rogers <23215294+coro@users.noreply.github.com> Date: Fri, 24 May 2024 10:23:50 +0100 Subject: [PATCH 6/6] Rename BearerToken to AccessToken Signed-off-by: Connor Rogers <23215294+coro@users.noreply.github.com> --- api/v1alpha1/oidc_types.go | 6 +++--- api/v1alpha1/zz_generated.deepcopy.go | 4 ++-- .../generated/gateway.envoyproxy.io_securitypolicies.yaml | 6 +++--- .../securitypolicy-with-oidc-custom-cookies.in.yaml | 2 +- .../securitypolicy-with-oidc-custom-cookies.out.yaml | 4 ++-- internal/ir/xds.go | 2 +- internal/xds/translator/oidc.go | 6 +++--- internal/xds/translator/testdata/in/xds-ir/oidc.yaml | 2 +- ...isteners-same-port-with-different-filters.listeners.yaml | 2 +- .../xds/translator/testdata/out/xds-ir/oidc.listeners.yaml | 4 ++-- site/content/en/latest/api/extension_types.md | 2 +- 11 files changed, 20 insertions(+), 20 deletions(-) diff --git a/api/v1alpha1/oidc_types.go b/api/v1alpha1/oidc_types.go index d49552d05ee..b1de7097f86 100644 --- a/api/v1alpha1/oidc_types.go +++ b/api/v1alpha1/oidc_types.go @@ -84,11 +84,11 @@ type OIDCProvider struct { // OIDCCookieNames defines the names of cookies to use in the Envoy OIDC filter. type OIDCCookieNames struct { - // The name of the cookie used to store the BearerToken in the + // The name of the cookie used to store the AccessToken in the // [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). - // If not specified, defaults to "BearerToken-(randomly generated uid)" + // If not specified, defaults to "AccessToken-(randomly generated uid)" // +optional - BearerToken *string `json:"bearerToken,omitempty"` + AccessToken *string `json:"accessToken,omitempty"` // The name of the cookie used to store the IdToken in the // [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). // If not specified, defaults to "IdToken-(randomly generated uid)" diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index c93c94bab73..13641e00080 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -3216,8 +3216,8 @@ func (in *OIDC) DeepCopy() *OIDC { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OIDCCookieNames) DeepCopyInto(out *OIDCCookieNames) { *out = *in - if in.BearerToken != nil { - in, out := &in.BearerToken, &out.BearerToken + if in.AccessToken != nil { + in, out := &in.AccessToken, &out.AccessToken *out = new(string) **out = **in } diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml index 1360633e378..ff88f6332d1 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml @@ -677,11 +677,11 @@ spec: [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). If not specified, uses a randomly generated suffix properties: - bearerToken: + accessToken: description: |- - The name of the cookie used to store the BearerToken in the + The name of the cookie used to store the AccessToken in the [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). - If not specified, defaults to "BearerToken-(randomly generated uid)" + If not specified, defaults to "AccessToken-(randomly generated uid)" type: string idToken: description: |- diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc-custom-cookies.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc-custom-cookies.in.yaml index 8dcc03fdb46..13ce562de3b 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-oidc-custom-cookies.in.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc-custom-cookies.in.yaml @@ -70,4 +70,4 @@ securityPolicies: logoutPath: "/bar/logout" cookieNames: idToken: "CustomIdTokenCookie" - bearerToken: "CustomBearerTokenCookie" + accessToken: "CustomAccessTokenCookie" diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc-custom-cookies.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc-custom-cookies.out.yaml index 3bfd96b0b96..07ceea24fae 100755 --- a/internal/gatewayapi/testdata/securitypolicy-with-oidc-custom-cookies.out.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-oidc-custom-cookies.out.yaml @@ -110,7 +110,7 @@ securityPolicies: kind: null name: client1-secret cookieNames: - bearerToken: CustomBearerTokenCookie + accessToken: CustomAccessTokenCookie idToken: CustomIdTokenCookie logoutPath: /bar/logout provider: @@ -171,7 +171,7 @@ xdsIR: clientID: client1.apps.googleusercontent.com clientSecret: Y2xpZW50MTpzZWNyZXQK cookieNameOverrides: - bearerToken: CustomBearerTokenCookie + accessToken: CustomAccessTokenCookie idToken: CustomIdTokenCookie cookieSuffix: b0a1b740 hmacSecret: qrOYACHXoe7UEDI/raOjNSx+Z9ufXSc/22C3T6X/zPY= diff --git a/internal/ir/xds.go b/internal/ir/xds.go index 6985564cb49..b32da13e782 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -673,7 +673,7 @@ type OIDC struct { // CookieSuffix will be added to the name of the cookies set by the oauth filter. // Adding a suffix avoids multiple oauth filters from overwriting each other's cookies. - // These cookies are set by the oauth filter, including: BearerToken, + // These cookies are set by the oauth filter, including: AccessToken, // OauthHMAC, OauthExpires, IdToken, and RefreshToken. CookieSuffix string `json:"cookieSuffix,omitempty"` diff --git a/internal/xds/translator/oidc.go b/internal/xds/translator/oidc.go index 9be0bab06de..ae54603f57b 100644 --- a/internal/xds/translator/oidc.go +++ b/internal/xds/translator/oidc.go @@ -159,7 +159,7 @@ func oauth2Config(oidc *ir.OIDC) (*oauth2v3.OAuth2, error) { }, }, CookieNames: &oauth2v3.OAuth2Credentials_CookieNames{ - BearerToken: fmt.Sprintf("BearerToken-%s", oidc.CookieSuffix), + BearerToken: fmt.Sprintf("AccessToken-%s", oidc.CookieSuffix), OauthHmac: fmt.Sprintf("OauthHMAC-%s", oidc.CookieSuffix), OauthExpires: fmt.Sprintf("OauthExpires-%s", oidc.CookieSuffix), IdToken: fmt.Sprintf("IdToken-%s", oidc.CookieSuffix), @@ -174,8 +174,8 @@ func oauth2Config(oidc *ir.OIDC) (*oauth2v3.OAuth2, error) { } if oidc.CookieNameOverrides != nil && - oidc.CookieNameOverrides.BearerToken != nil { - oauth2.Config.Credentials.CookieNames.BearerToken = *oidc.CookieNameOverrides.BearerToken + oidc.CookieNameOverrides.AccessToken != nil { + oauth2.Config.Credentials.CookieNames.BearerToken = *oidc.CookieNameOverrides.AccessToken } if oidc.CookieNameOverrides != nil && diff --git a/internal/xds/translator/testdata/in/xds-ir/oidc.yaml b/internal/xds/translator/testdata/in/xds-ir/oidc.yaml index ab09dd501b3..68e36bffab1 100644 --- a/internal/xds/translator/testdata/in/xds-ir/oidc.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/oidc.yaml @@ -68,4 +68,4 @@ http: cookieSuffix: 5f93c2e4 cookieNameOverrides: idToken: "CustomIdTokenOverride" - bearerToken: "CustomBearerTokenOverride" + accessToken: "CustomAccessTokenOverride" diff --git a/internal/xds/translator/testdata/out/xds-ir/multiple-listeners-same-port-with-different-filters.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/multiple-listeners-same-port-with-different-filters.listeners.yaml index 1365c290cb7..a06b7265316 100755 --- a/internal/xds/translator/testdata/out/xds-ir/multiple-listeners-same-port-with-different-filters.listeners.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/multiple-listeners-same-port-with-different-filters.listeners.yaml @@ -117,7 +117,7 @@ credentials: clientId: client.oauth.foo.com cookieNames: - bearerToken: BearerToken-5F93C2E4 + bearerToken: AccessToken-5F93C2E4 idToken: IdToken-5F93C2E4 oauthExpires: OauthExpires-5F93C2E4 oauthHmac: OauthHMAC-5F93C2E4 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 7077a5f987e..ab8556ba154 100644 --- a/internal/xds/translator/testdata/out/xds-ir/oidc.listeners.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/oidc.listeners.yaml @@ -28,7 +28,7 @@ credentials: clientId: client.oauth.foo.com cookieNames: - bearerToken: BearerToken-5F93C2E4 + bearerToken: AccessToken-5F93C2E4 idToken: IdToken-5F93C2E4 oauthExpires: OauthExpires-5F93C2E4 oauthHmac: OauthHMAC-5F93C2E4 @@ -71,7 +71,7 @@ credentials: clientId: client.oauth.bar.com cookieNames: - bearerToken: CustomBearerTokenOverride + bearerToken: CustomAccessTokenOverride idToken: CustomIdTokenOverride oauthExpires: OauthExpires-5f93c2e4 oauthHmac: OauthHMAC-5f93c2e4 diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index b3019b03b6b..76a947af036 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -2296,7 +2296,7 @@ _Appears in:_ | Field | Type | Required | Description | | --- | --- | --- | --- | -| `bearerToken` | _string_ | false | The name of the cookie used to store the BearerToken in the
[Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).
If not specified, defaults to "BearerToken-(randomly generated uid)" | +| `accessToken` | _string_ | false | The name of the cookie used to store the AccessToken in the
[Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).
If not specified, defaults to "AccessToken-(randomly generated uid)" | | `idToken` | _string_ | false | The name of the cookie used to store the IdToken in the
[Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).
If not specified, defaults to "IdToken-(randomly generated uid)" |