Skip to content

Commit 03d93a4

Browse files
zhaohuabingzhaohuabing
committed
impl getting OIDC client ID from secret
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
1 parent fc2766b commit 03d93a4

File tree

10 files changed

+80
-17
lines changed

10 files changed

+80
-17
lines changed

‎api/v1alpha1/oidc_types.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,27 +11,30 @@ import (
1111
)
1212

1313
const OIDCClientSecretKey = "client-secret"
14+
const OIDCClientIDKey = "client-id"
1415

1516
// OIDC defines the configuration for the OpenID Connect (OIDC) authentication.
17+
// +kubebuilder:validation:XValidation:rule="(has(self.clientID) && !has(self.clientIDRef)) || (!has(self.clientID) && has(self.clientIDRef))", message="only one of clientID or clientIDRef must be set"
1618
type OIDC struct {
1719
// The OIDC Provider configuration.
1820
Provider OIDCProvider `json:"provider"`
1921

2022
// The client ID to be used in the OIDC
2123
// [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).
2224
//
25+
// Only one of clientID or clientIDRef must be set.
26+
// +optional
2327
// +kubebuilder:validation:MinLength=1
24-
ClientID string `json:"clientID"`
25-
26-
// TODO zhaohuabing make ClientID optional in the implementation PR
28+
ClientID *string `json:"clientID, omitempty"`
2729

2830
// The Kubernetes secret which contains the client ID to be used in the
2931
// [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).
3032
// Exactly one of clientID or clientIDRef must be set.
3133
// This is an Opaque secret. The client ID should be stored in the key "client-id".
3234
//
35+
// Only one of clientID or clientIDRef must be set.
36+
//
3337
// +optional
34-
// +notImplementedHide
3538
ClientIDRef *gwapiv1.SecretObjectReference `json:"clientIDRef,omitempty"`
3639

3740
// The Kubernetes secret which contains the OIDC client secret to be used in the

‎api/v1alpha1/zz_generated.deepcopy.go

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_securitypolicies.yaml

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3676,6 +3676,8 @@ spec:
36763676
description: |-
36773677
The client ID to be used in the OIDC
36783678
[Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).
3679+
3680+
Only one of clientID or clientIDRef must be set.
36793681
minLength: 1
36803682
type: string
36813683
clientIDRef:
@@ -3684,6 +3686,8 @@ spec:
36843686
[Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).
36853687
Exactly one of clientID or clientIDRef must be set.
36863688
This is an Opaque secret. The client ID should be stored in the key "client-id".
3689+
3690+
Only one of clientID or clientIDRef must be set.
36873691
properties:
36883692
group:
36893693
default: ""
@@ -4900,10 +4904,13 @@ spec:
49004904
type: string
49014905
type: array
49024906
required:
4903-
- clientID
49044907
- clientSecret
49054908
- provider
49064909
type: object
4910+
x-kubernetes-validations:
4911+
- message: only one of clientID or clientIDRef must be set
4912+
rule: (has(self.clientID) && !has(self.clientIDRef)) || (!has(self.clientID)
4913+
&& has(self.clientIDRef))
49074914
targetRef:
49084915
description: |-
49094916
TargetRef is the name of the resource this policy is being attached to.

‎charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3675,6 +3675,8 @@ spec:
36753675
description: |-
36763676
The client ID to be used in the OIDC
36773677
[Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).
3678+
3679+
Only one of clientID or clientIDRef must be set.
36783680
minLength: 1
36793681
type: string
36803682
clientIDRef:
@@ -3683,6 +3685,8 @@ spec:
36833685
[Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).
36843686
Exactly one of clientID or clientIDRef must be set.
36853687
This is an Opaque secret. The client ID should be stored in the key "client-id".
3688+
3689+
Only one of clientID or clientIDRef must be set.
36863690
properties:
36873691
group:
36883692
default: ""
@@ -4899,10 +4903,13 @@ spec:
48994903
type: string
49004904
type: array
49014905
required:
4902-
- clientID
49034906
- clientSecret
49044907
- provider
49054908
type: object
4909+
x-kubernetes-validations:
4910+
- message: only one of clientID or clientIDRef must be set
4911+
rule: (has(self.clientID) && !has(self.clientIDRef)) || (!has(self.clientID)
4912+
&& has(self.clientIDRef))
49064913
targetRef:
49074914
description: |-
49084915
TargetRef is the name of the resource this policy is being attached to.

‎internal/gatewayapi/securitypolicy.go

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -990,6 +990,7 @@ func (t *Translator) buildOIDC(
990990
var (
991991
oidc = policy.Spec.OIDC
992992
provider *ir.OIDCProvider
993+
clientID string
993994
clientSecret *corev1.Secret
994995
redirectURL = defaultRedirectURL
995996
redirectPath = defaultRedirectPath
@@ -1010,6 +1011,24 @@ func (t *Translator) buildOIDC(
10101011
namespace: policy.Namespace,
10111012
}
10121013

1014+
// Client ID can be specified either as a string or as a reference to a secret.
1015+
if oidc.ClientID != nil {
1016+
clientID = *oidc.ClientID
1017+
} else if oidc.ClientIDRef != nil {
1018+
var clientIDSecret *corev1.Secret
1019+
if clientIDSecret, err = t.validateSecretRef(false, from, *oidc.ClientIDRef, resources); err != nil {
1020+
return nil, err
1021+
}
1022+
clientIDBytes, ok := clientIDSecret.Data[egv1a1.OIDCClientIDKey]
1023+
if !ok || len(clientIDBytes) == 0 {
1024+
return nil, fmt.Errorf("client ID not found in secret %s/%s",clientIDSecret.Namespace, clientIDSecret.Name)
1025+
}
1026+
clientID = string(clientIDBytes)
1027+
} else {
1028+
// This is just a sanity check - the CRD validation should have caught this.
1029+
return nil, fmt.Errorf("client ID must be specified in OIDC policy %s/%s", policy.Namespace, policy.Name)
1030+
}
1031+
10131032
if clientSecret, err = t.validateSecretRef(
10141033
false, from, oidc.ClientSecret, resources); err != nil {
10151034
return nil, err
@@ -1068,7 +1087,7 @@ func (t *Translator) buildOIDC(
10681087
return &ir.OIDC{
10691088
Name: irConfigName(policy),
10701089
Provider: *provider,
1071-
ClientID: oidc.ClientID,
1090+
ClientID: clientID,
10721091
ClientSecret: clientSecretBytes,
10731092
Scopes: scopes,
10741093
Resources: oidc.Resources,

‎internal/gatewayapi/testdata/securitypolicy-with-oidc.in.yaml

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,7 @@ secrets:
1313
name: client2-secret
1414
data:
1515
client-secret: Y2xpZW50MTpzZWNyZXQK
16-
- apiVersion: v1
17-
kind: Secret
18-
metadata:
19-
namespace: default
20-
name: client3-secret
21-
data:
22-
client-secret: Y2xpZW50MTpzZWNyZXQK
16+
client-id: Y2xpZW50Mi5vYXV0aC5mb28uY29t
2317
- apiVersion: v1
2418
kind: Secret
2519
metadata:
@@ -121,7 +115,8 @@ securityPolicies:
121115
issuer: "https://oauth.foo.com"
122116
authorizationEndpoint: "https://oauth.foo.com/oauth2/v2/auth"
123117
tokenEndpoint: "https://oauth.foo.com/token"
124-
clientID: "client2.oauth.foo.com"
118+
clientIDRef:
119+
name: "client2-secret"
125120
clientSecret:
126121
name: "client2-secret"
127122
scopes: ["openid", "email", "profile"]

‎internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,11 @@ securityPolicies:
146146
uid: 08335a80-83ba-4592-888f-6ac0bba44ce4
147147
spec:
148148
oidc:
149-
clientID: client2.oauth.foo.com
149+
clientID: null
150+
clientIDRef:
151+
group: null
152+
kind: null
153+
name: client2-secret
150154
clientSecret:
151155
group: null
152156
kind: null

‎internal/provider/kubernetes/controller.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -745,6 +745,25 @@ func (r *gatewayAPIReconciler) processSecurityPolicyObjectRefs(
745745
"policy", policy, "secretRef", oidc.ClientSecret)
746746
}
747747

748+
if oidc.ClientIDRef != nil {
749+
if err := r.processSecretRef(
750+
ctx,
751+
resourceMap,
752+
resourceTree,
753+
resource.KindSecurityPolicy,
754+
policy.Namespace,
755+
policy.Name,
756+
*oidc.ClientIDRef); err != nil {
757+
// If the error is transient, we return it to allow Reconcile to retry.
758+
if isTransientError(err) {
759+
return err
760+
}
761+
r.log.Error(err,
762+
"failed to process OIDC ClientIDRef for SecurityPolicy",
763+
"policy", policy, "secretRef", oidc.ClientIDRef)
764+
}
765+
}
766+
748767
var backendRefs []gwapiv1.BackendObjectReference
749768
if oidc.Provider.BackendRef != nil {
750769
backendRefs = append(backendRefs, *oidc.Provider.BackendRef)

‎internal/provider/kubernetes/indexers.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -589,6 +589,9 @@ func secretSecurityPolicyIndexFunc(rawObj client.Object) []string {
589589

590590
if securityPolicy.Spec.OIDC != nil {
591591
secretReferences = append(secretReferences, securityPolicy.Spec.OIDC.ClientSecret)
592+
if securityPolicy.Spec.OIDC.ClientIDRef != nil {
593+
secretReferences = append(secretReferences, *securityPolicy.Spec.OIDC.ClientIDRef)
594+
}
592595
}
593596
if securityPolicy.Spec.APIKeyAuth != nil {
594597
secretReferences = append(secretReferences, securityPolicy.Spec.APIKeyAuth.CredentialRefs...)

‎site/content/en/latest/api/extension_types.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3229,7 +3229,8 @@ _Appears in:_
32293229
| Field | Type | Required | Default | Description |
32303230
| --- | --- | --- | --- | --- |
32313231
| `provider` | _[OIDCProvider](#oidcprovider)_ | true | | The OIDC Provider configuration. |
3232-
| `clientID` | _string_ | true | | The client ID to be used in the OIDC<br />[Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). |
3232+
| `clientID` | _string_ | false | | The client ID to be used in the OIDC<br />[Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).<br />Only one of clientID or clientIDRef must be set. |
3233+
| `clientIDRef` | _[SecretObjectReference](https://gateway-api.sigs.k8s.io/references/spec/#gateway.networking.k8s.io/v1.SecretObjectReference)_ | false | | The Kubernetes secret which contains the client ID to be used in the<br />[Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).<br />Exactly one of clientID or clientIDRef must be set.<br />This is an Opaque secret. The client ID should be stored in the key "client-id".<br />Only one of clientID or clientIDRef must be set. |
32333234
| `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<br />[Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).<br />This is an Opaque secret. The client secret should be stored in the key<br />"client-secret". |
32343235
| `cookieNames` | _[OIDCCookieNames](#oidccookienames)_ | false | | The optional cookie name overrides to be used for Bearer and IdToken cookies in the<br />[Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).<br />If not specified, uses a randomly generated suffix |
32353236
| `cookieConfig` | _[OIDCCookieConfig](#oidccookieconfig)_ | false | | CookieConfigs allows setting the SameSite attribute for OIDC cookies.<br />By default, its unset. |

0 commit comments

Comments
 (0)