refactor: OIDC validation and defaulting (#157)

* fix oidc config validation

* loosen restrictions on default oidc provider name and change default to 'openmcp'

* validate oidc provider name uniqueness

* adapt docs

Author: Diaphteiros

api/common/oidc_types.go

@@ -1,23 +1,29 @@
 package common
 
 import (
-	"strings"
-
 	rbacv1 "k8s.io/api/rbac/v1"
 )
 
 type OIDCProviderConfig struct {
 	// Name is the name of the OIDC provider.
 	// May be used in k8s resources, therefore has to be a valid k8s name.
+	// It is also used (with a ':' suffix) as prefix in k8s resources referencing users or groups from this OIDC provider.
+	// E.g. if the name is 'example', the username 'alice' from this provider will be referenced as 'example:alice' in k8s resources.
+	// Must be unique among all OIDC providers configured in the same environment.
 	// +kubebuilder:validation:MinLength=1
 	// +kubebuilder:validation:MaxLength=253
-	// +kubebuilder:validation:Pattern=`[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*`
+	// +kubebuilder:validation:Pattern=`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`
+	// +kubebuilder:validation:XValidation:rule=`self != "system"`, message="'system' is a reserved string and may not be used as OIDC provider name"
 	Name string `json:"name"`
 
 	// Issuer is the issuer URL of the OIDC provider.
+	// Must be a valid URL.
+	// +kubebuilder:validation:Pattern=`^https?://[^\s/$.?#].[^\s]*$`
+	// +kubebuilder:validation:MinLength=1
 	Issuer string `json:"issuer"`
 
 	// ClientID is the client ID to use for the OIDC provider.
+	// +kubebuilder:validation:MinLength=1
 	ClientID string `json:"clientID"`
 
 	// GroupsClaim is the claim in the OIDC token that contains the groups.
@@ -26,24 +32,12 @@ type OIDCProviderConfig struct {
 	// +optional
 	GroupsClaim string `json:"groupsClaim"`
 
-	// GroupsPrefix is a prefix that will be added to all group names when referenced in RBAC rules.
-	// This is required to avoid conflicts with Kubernetes built-in groups.
-	// If the prefix does not end with a colon (:), it will be added automatically.
-	// +kubebuilder:validation:MinLength=1
-	GroupsPrefix string `json:"groupsPrefix"`
-
 	// UsernameClaim is the claim in the OIDC token that contains the username.
 	// If empty, the default claim "sub" will be used.
 	// +kubebuilder:default="sub"
 	// +optional
 	UsernameClaim string `json:"usernameClaim"`
 
-	// UsernamePrefix is a prefix that will be added to all usernames when referenced in RBAC rules.
-	// This is required to avoid conflicts with Kubernetes built-in users.
-	// If the prefix does not end with a colon (:), it will be added automatically.
-	// +kubebuilder:validation:MinLength=1
-	UsernamePrefix string `json:"usernamePrefix"`
-
 	// ExtraScopes is a list of extra scopes that should be requested from the OIDC provider.
 	// +optional
 	ExtraScopes []string `json:"extraScopes,omitempty"`
@@ -90,14 +84,14 @@ func (o *OIDCProviderConfig) Default() *OIDCProviderConfig {
 	if o.GroupsClaim == "" {
 		o.GroupsClaim = "groups"
 	}
-	if !strings.HasSuffix(o.GroupsPrefix, ":") {
-		o.GroupsPrefix += ":"
-	}
 	if o.UsernameClaim == "" {
 		o.UsernameClaim = "sub"
 	}
-	if !strings.HasSuffix(o.UsernamePrefix, ":") {
-		o.UsernamePrefix += ":"
-	}
 	return o
 }
+
+// UsernameGroupsPrefix returns the prefix for usernames and groups for this OIDC provider.
+// It is equivalent to <provider_name> + ":".
+func (o *OIDCProviderConfig) UsernameGroupsPrefix() string {
+	return o.Name + ":"
+}

api/core/v2alpha1/constants.go

@@ -2,7 +2,7 @@ package v2alpha1
 
 const (
 	// DefaultOIDCProviderName is the identifier for the default OIDC provider.
-	DefaultOIDCProviderName = "default"
+	DefaultOIDCProviderName = "openmcp"
 	// DefaultMCPClusterPurpose is the default purpose for ManagedControlPlane clusters.
 	DefaultMCPClusterPurpose = "mcp"
 )
@@ -15,6 +15,8 @@ const (
 
 	// ManagedPurposeMCPPurposeOverride is used as value for the managed purpose label. It must not be modified.
 	ManagedPurposeMCPPurposeOverride = "mcp-purpose-override"
+	// ManagedPurposeOIDCProviderNameUniqueness is used as value for the managed purpose label. It must not be modified.
+	ManagedPurposeOIDCProviderNameUniqueness = "oidc-provider-name-uniqueness"
 
 	MCPFinalizer = GroupName + "/mcp"
 

api/core/v2alpha1/managedcontrolplane_types.go

@@ -31,7 +31,6 @@ type IAMConfig struct {
 
 	// OIDCProviders is a list of OIDC providers that should be configured for the ManagedControlPlaneV2.
 	// They are independent of the standard OIDC provider and in addition to it, unless it has been disabled by not specifying any role bindings.
-	// +kubebuilder:validation:items:XValidation:rule="self.name != 'default'", message="OIDC provider name must not be 'default' as this is reserved for the standard OIDC provider"
 	// +optional
 	OIDCProviders []*commonapi.OIDCProviderConfig `json:"oidcProviders,omitempty"`
 }

api/crds/manifests/clusters.openmcp.cloud_accessrequests.yaml

@@ -73,6 +73,7 @@ spec:
                 properties:
                   clientID:
                     description: ClientID is the client ID to use for the OIDC provider.
+                    minLength: 1
                     type: string
                   extraScopes:
                     description: ExtraScopes is a list of extra scopes that should
@@ -86,24 +87,28 @@ spec:
                       GroupsClaim is the claim in the OIDC token that contains the groups.
                       If empty, the default claim "groups" will be used.
                     type: string
-                  groupsPrefix:
+                  issuer:
                     description: |-
-                      GroupsPrefix is a prefix that will be added to all group names when referenced in RBAC rules.
-                      This is required to avoid conflicts with Kubernetes built-in groups.
-                      If the prefix does not end with a colon (:), it will be added automatically.
+                      Issuer is the issuer URL of the OIDC provider.
+                      Must be a valid URL.
                     minLength: 1
-                    type: string
-                  issuer:
-                    description: Issuer is the issuer URL of the OIDC provider.
+                    pattern: ^https?://[^\s/$.?#].[^\s]*$
                     type: string
                   name:
                     description: |-
                       Name is the name of the OIDC provider.
                       May be used in k8s resources, therefore has to be a valid k8s name.
+                      It is also used (with a ':' suffix) as prefix in k8s resources referencing users or groups from this OIDC provider.
+                      E.g. if the name is 'example', the username 'alice' from this provider will be referenced as 'example:alice' in k8s resources.
+                      Must be unique among all OIDC providers configured in the same environment.
                     maxLength: 253
                     minLength: 1
-                    pattern: '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*'
+                    pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
                     type: string
+                    x-kubernetes-validations:
+                    - message: '''system'' is a reserved string and may not be used
+                        as OIDC provider name'
+                      rule: self != "system"
                   roleBindings:
                     description: |-
                       RoleBindings is a list of subjects with (cluster) role bindings that should be created for them.
@@ -260,20 +265,11 @@ spec:
                       UsernameClaim is the claim in the OIDC token that contains the username.
                       If empty, the default claim "sub" will be used.
                     type: string
-                  usernamePrefix:
-                    description: |-
-                      UsernamePrefix is a prefix that will be added to all usernames when referenced in RBAC rules.
-                      This is required to avoid conflicts with Kubernetes built-in users.
-                      If the prefix does not end with a colon (:), it will be added automatically.
-                    minLength: 1
-                    type: string
                 required:
                 - clientID
-                - groupsPrefix
                 - issuer
                 - name
                 - roleBindings
-                - usernamePrefix
                 type: object
               requestRef:
                 description: |-

api/crds/manifests/core.openmcp.cloud_managedcontrolplanev2s.yaml

@@ -58,6 +58,7 @@ spec:
                         clientID:
                           description: ClientID is the client ID to use for the OIDC
                             provider.
+                          minLength: 1
                           type: string
                         extraScopes:
                           description: ExtraScopes is a list of extra scopes that
@@ -71,24 +72,28 @@ spec:
                             GroupsClaim is the claim in the OIDC token that contains the groups.
                             If empty, the default claim "groups" will be used.
                           type: string
-                        groupsPrefix:
+                        issuer:
                           description: |-
-                            GroupsPrefix is a prefix that will be added to all group names when referenced in RBAC rules.
-                            This is required to avoid conflicts with Kubernetes built-in groups.
-                            If the prefix does not end with a colon (:), it will be added automatically.
+                            Issuer is the issuer URL of the OIDC provider.
+                            Must be a valid URL.
                           minLength: 1
-                          type: string
-                        issuer:
-                          description: Issuer is the issuer URL of the OIDC provider.
+                          pattern: ^https?://[^\s/$.?#].[^\s]*$
                           type: string
                         name:
                           description: |-
                             Name is the name of the OIDC provider.
                             May be used in k8s resources, therefore has to be a valid k8s name.
+                            It is also used (with a ':' suffix) as prefix in k8s resources referencing users or groups from this OIDC provider.
+                            E.g. if the name is 'example', the username 'alice' from this provider will be referenced as 'example:alice' in k8s resources.
+                            Must be unique among all OIDC providers configured in the same environment.
                           maxLength: 253
                           minLength: 1
-                          pattern: '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*'
+                          pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
                           type: string
+                          x-kubernetes-validations:
+                          - message: '''system'' is a reserved string and may not
+                              be used as OIDC provider name'
+                            rule: self != "system"
                         roleBindings:
                           description: |-
                             RoleBindings is a list of subjects with (cluster) role bindings that should be created for them.
@@ -171,25 +176,12 @@ spec:
                             UsernameClaim is the claim in the OIDC token that contains the username.
                             If empty, the default claim "sub" will be used.
                           type: string
-                        usernamePrefix:
-                          description: |-
-                            UsernamePrefix is a prefix that will be added to all usernames when referenced in RBAC rules.
-                            This is required to avoid conflicts with Kubernetes built-in users.
-                            If the prefix does not end with a colon (:), it will be added automatically.
-                          minLength: 1
-                          type: string
                       required:
                       - clientID
-                      - groupsPrefix
                       - issuer
                       - name
                       - roleBindings
-                      - usernamePrefix
                       type: object
-                      x-kubernetes-validations:
-                      - message: OIDC provider name must not be 'default' as this
-                          is reserved for the standard OIDC provider
-                        rule: self.name != 'default'
                     type: array
                   roleBindings:
                     description: |-