feat: Optionally override Oauth Cookie Names (envoyproxy#3274)

* Added ability to specifiy a cookieSuffix Signed-off-by: sam-burrell <sam.burrell@gmail.com> Signed-off-by: Connor Rogers <23215294+coro@users.noreply.github.com> * Allow override of OAuth cookie names Co-authored-by: Sam Burrell <sam.burrell@gmail.com> Signed-off-by: Connor Rogers <23215294+coro@users.noreply.github.com> * Update Helm CRD docs Co-authored-by: Sam Burrell <sam.burrell@gmail.com> Signed-off-by: Connor Rogers <23215294+coro@users.noreply.github.com> * Add testdata for custom cookie name overrides Co-authored-by: Sam Burrell <sam.burrell@gmail.com> Signed-off-by: Connor Rogers <23215294+coro@users.noreply.github.com> * Fix regression tests to have accepted gateway Signed-off-by: Connor Rogers <23215294+coro@users.noreply.github.com> * Rename BearerToken to AccessToken Signed-off-by: Connor Rogers <23215294+coro@users.noreply.github.com> --------- Signed-off-by: sam-burrell <sam.burrell@gmail.com> Signed-off-by: Connor Rogers <23215294+coro@users.noreply.github.com> Co-authored-by: Connor Rogers <23215294+coro@users.noreply.github.com>
guydc · May 29, 2024 · 44330ef · 44330ef
1 parent 04e2f05
commit 44330ef
Show file tree

Hide file tree

Showing 13 changed files with 387 additions and 18 deletions.
diff --git a/api/v1alpha1/oidc_types.go b/api/v1alpha1/oidc_types.go
@@ -30,6 +30,12 @@ type OIDC struct {
 	// +kubebuilder:validation:Required
 	ClientSecret gwapiv1b1.SecretObjectReference `json:"clientSecret"`
 
+	// 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
+	// +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).
 	// The "openid" scope is always added to the list of scopes if not already
@@ -75,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 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)"
+	// +optional
+	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)"
+	// +optional
+	IDToken *string `json:"idToken,omitempty"`
+}
diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml
@@ -862,6 +862,25 @@ spec:
                     required:
                     - name
                     type: object
+                  cookieNames:
+                    description: |-
+                      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
+                    properties:
+                      accessToken:
+                        description: |-
+                          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)"
+                        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.

diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go
@@ -604,7 +604,9 @@ func (t *Translator) buildOIDC(
 		logoutPath = *oidc.LogoutPath
 	}
 
-	// Generate a unique cookie suffix for oauth filters
+	// 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))
 
 	// Get the HMAC secret.
@@ -622,17 +624,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/gatewayapi/testdata/securitypolicy-with-oidc-custom-cookies.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc-custom-cookies.in.yaml
@@ -0,0 +1,73 @@
+secrets:
+- apiVersion: v1
+  kind: Secret
+  metadata:
+    namespace: envoy-gateway
+    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
+  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"
+        accessToken: "CustomAccessTokenCookie"