Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Optionally override Oauth Cookie Names #3274

Merged
merged 7 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions api/v1alpha1/oidc_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or even idTokenName

// 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
Expand Down Expand Up @@ -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 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"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name BearerToken is confusing in this context. Let's rename it AccessToken to keep consistent with #3423

Suggested change
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)"
// +optional
IDToken *string `json:"idToken,omitempty"`
}
30 changes: 30 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,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:
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.
Expand Down
27 changes: 15 additions & 12 deletions internal/gatewayapi/securitypolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,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
Expand All @@ -600,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
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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"
Original file line number Diff line number Diff line change
@@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something is off, Accepted=False, you may have forgotten to include the Secret, the security IR field is also empty

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: {}
3 changes: 3 additions & 0 deletions internal/ir/xds.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading
Loading