Skip to content

Commit bef930f

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

File tree

15 files changed

+174
-24
lines changed

15 files changed

+174
-24
lines changed

‎api/v1alpha1/oidc_types.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,28 +10,33 @@ import (
1010
gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"
1111
)
1212

13-
const OIDCClientSecretKey = "client-secret"
13+
const (
14+
OIDCClientSecretKey = "client-secret"
15+
OIDCClientIDKey = "client-id"
16+
)
1417

1518
// OIDC defines the configuration for the OpenID Connect (OIDC) authentication.
19+
// +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"
1620
type OIDC struct {
1721
// The OIDC Provider configuration.
1822
Provider OIDCProvider `json:"provider"`
1923

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

2832
// The Kubernetes secret which contains the client ID to be used in the
2933
// [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).
3034
// Exactly one of clientID or clientIDRef must be set.
3135
// This is an Opaque secret. The client ID should be stored in the key "client-id".
3236
//
37+
// Only one of clientID or clientIDRef must be set.
38+
//
3339
// +optional
34-
// +notImplementedHide
3540
ClientIDRef *gwapiv1.SecretObjectReference `json:"clientIDRef,omitempty"`
3641

3742
// 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: 21 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,25 @@ 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+
switch {
1016+
case oidc.ClientID != nil:
1017+
clientID = *oidc.ClientID
1018+
case oidc.ClientIDRef != nil:
1019+
var clientIDSecret *corev1.Secret
1020+
if clientIDSecret, err = t.validateSecretRef(false, from, *oidc.ClientIDRef, resources); err != nil {
1021+
return nil, err
1022+
}
1023+
clientIDBytes, ok := clientIDSecret.Data[egv1a1.OIDCClientIDKey]
1024+
if !ok || len(clientIDBytes) == 0 {
1025+
return nil, fmt.Errorf("client ID not found in secret %s/%s", clientIDSecret.Namespace, clientIDSecret.Name)
1026+
}
1027+
clientID = string(clientIDBytes)
1028+
default:
1029+
// This is just a sanity check - the CRD validation should have caught this.
1030+
return nil, fmt.Errorf("client ID must be specified in OIDC policy %s/%s", policy.Namespace, policy.Name)
1031+
}
1032+
10131033
if clientSecret, err = t.validateSecretRef(
10141034
false, from, oidc.ClientSecret, resources); err != nil {
10151035
return nil, err
@@ -1068,7 +1088,7 @@ func (t *Translator) buildOIDC(
10681088
return &ir.OIDC{
10691089
Name: irConfigName(policy),
10701090
Provider: *provider,
1071-
ClientID: oidc.ClientID,
1091+
ClientID: clientID,
10721092
ClientSecret: clientSecretBytes,
10731093
Scopes: scopes,
10741094
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: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,10 @@ securityPolicies:
146146
uid: 08335a80-83ba-4592-888f-6ac0bba44ce4
147147
spec:
148148
oidc:
149-
clientID: client2.oauth.foo.com
149+
clientIDRef:
150+
group: null
151+
kind: null
152+
name: client2-secret
150153
clientSecret:
151154
group: null
152155
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...)

‎internal/provider/kubernetes/predicates_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ func TestValidateSecretForReconcile(t *testing.T) {
371371
AuthorizationEndpoint: ptr.To("https://accounts.google.com/o/oauth2/v2/auth"),
372372
TokenEndpoint: ptr.To("https://oauth2.googleapis.com/token"),
373373
},
374-
ClientID: "client-id",
374+
ClientID: ptr.To("client-id"),
375375
ClientSecret: gwapiv1.SecretObjectReference{
376376
Name: "secret",
377377
},

0 commit comments

Comments
 (0)