diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/kas/auth_test.go b/control-plane-operator/controllers/hostedcontrolplane/v2/kas/auth_test.go index fb9e592dc1a5..ff2d54a41312 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/v2/kas/auth_test.go +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/kas/auth_test.go @@ -1733,6 +1733,1004 @@ func TestAdaptAuthConfig(t *testing.T) { }, shouldError: true, }, + { + name: "When discovery URL has a different host from issuer URL, it should generate valid authentication configuration", + client: fake.NewClientBuilder().Build(), + featureGates: []featuregate.Feature{ + featuregates.ExternalOIDCWithUpstreamParity, + }, + expectedAuthenticationConfiguration: &AuthenticationConfiguration{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiserver.config.k8s.io/v1alpha1", + Kind: "AuthenticationConfiguration", + }, + JWT: []JWTAuthenticator{ + { + Issuer: Issuer{ + URL: "https://issuer.example.com", + DiscoveryURL: "https://discovery.example.com/.well-known/openid-configuration", + AudienceMatchPolicy: AudienceMatchPolicyMatchAny, + Audiences: []string{"one", "two"}, + }, + ClaimMappings: ClaimMappings{ + Username: PrefixedClaimOrExpression{ + Prefix: ptr.To("https://issuer.example.com#"), + Claim: "username", + }, + Groups: PrefixedClaimOrExpression{ + Prefix: ptr.To(""), + Claim: "", + }, + UID: ClaimOrExpression{Claim: "sub"}, + Extra: []ExtraMapping{}, + }, + ClaimValidationRules: []ClaimValidationRule{}, + UserValidationRules: []UserValidationRule{}, + }, + }, + }, + hcpAuthenticationSpec: &configv1.AuthenticationSpec{ + OIDCProviders: []configv1.OIDCProvider{ + { + Name: "test", + Issuer: configv1.TokenIssuer{ + URL: "https://issuer.example.com", + DiscoveryURL: "https://discovery.example.com/.well-known/openid-configuration", + Audiences: []configv1.TokenAudience{"one", "two"}, + }, + ClaimMappings: configv1.TokenClaimMappings{ + Username: configv1.UsernameClaimMapping{ + PrefixPolicy: configv1.NoOpinion, + Claim: "username", + }, + }, + }, + }, + }, + shouldError: false, + }, + { + name: "When discovery URL has a different path from issuer URL, it should generate valid authentication configuration", + client: fake.NewClientBuilder().Build(), + featureGates: []featuregate.Feature{ + featuregates.ExternalOIDCWithUpstreamParity, + }, + expectedAuthenticationConfiguration: &AuthenticationConfiguration{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiserver.config.k8s.io/v1alpha1", + Kind: "AuthenticationConfiguration", + }, + JWT: []JWTAuthenticator{ + { + Issuer: Issuer{ + URL: "https://example.com/issuer", + DiscoveryURL: "https://example.com/discovery/.well-known/openid-configuration", + AudienceMatchPolicy: AudienceMatchPolicyMatchAny, + Audiences: []string{"one", "two"}, + }, + ClaimMappings: ClaimMappings{ + Username: PrefixedClaimOrExpression{ + Prefix: ptr.To("https://example.com/issuer#"), + Claim: "username", + }, + Groups: PrefixedClaimOrExpression{ + Prefix: ptr.To(""), + Claim: "", + }, + UID: ClaimOrExpression{Claim: "sub"}, + Extra: []ExtraMapping{}, + }, + ClaimValidationRules: []ClaimValidationRule{}, + UserValidationRules: []UserValidationRule{}, + }, + }, + }, + hcpAuthenticationSpec: &configv1.AuthenticationSpec{ + OIDCProviders: []configv1.OIDCProvider{ + { + Name: "test", + Issuer: configv1.TokenIssuer{ + URL: "https://example.com/issuer", + DiscoveryURL: "https://example.com/discovery/.well-known/openid-configuration", + Audiences: []configv1.TokenAudience{"one", "two"}, + }, + ClaimMappings: configv1.TokenClaimMappings{ + Username: configv1.UsernameClaimMapping{ + PrefixPolicy: configv1.NoOpinion, + Claim: "username", + }, + }, + }, + }, + }, + shouldError: false, + }, + { + name: "When a valid userValidationRule with single expression is provided, it should generate valid authentication configuration", + client: fake.NewClientBuilder().Build(), + featureGates: []featuregate.Feature{ + featuregates.ExternalOIDCWithUpstreamParity, + }, + expectedAuthenticationConfiguration: &AuthenticationConfiguration{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiserver.config.k8s.io/v1alpha1", + Kind: "AuthenticationConfiguration", + }, + JWT: []JWTAuthenticator{ + { + Issuer: Issuer{ + URL: "https://test.com", + AudienceMatchPolicy: AudienceMatchPolicyMatchAny, + Audiences: []string{"one", "two"}, + }, + ClaimMappings: ClaimMappings{ + Username: PrefixedClaimOrExpression{ + Prefix: ptr.To("https://test.com#"), + Claim: "username", + }, + Groups: PrefixedClaimOrExpression{ + Prefix: ptr.To(""), + Claim: "", + }, + UID: ClaimOrExpression{Claim: "sub"}, + Extra: []ExtraMapping{}, + }, + ClaimValidationRules: []ClaimValidationRule{}, + UserValidationRules: []UserValidationRule{ + { + Expression: "!user.username.startsWith('system:')", + Message: "username cannot use reserved system: prefix", + }, + }, + }, + }, + }, + hcpAuthenticationSpec: &configv1.AuthenticationSpec{ + OIDCProviders: []configv1.OIDCProvider{ + { + Name: "test", + Issuer: configv1.TokenIssuer{ + URL: "https://test.com", + Audiences: []configv1.TokenAudience{"one", "two"}, + }, + ClaimMappings: configv1.TokenClaimMappings{ + Username: configv1.UsernameClaimMapping{ + PrefixPolicy: configv1.NoOpinion, + Claim: "username", + }, + }, + UserValidationRules: []configv1.TokenUserValidationRule{ + { + Expression: "!user.username.startsWith('system:')", + Message: "username cannot use reserved system: prefix", + }, + }, + }, + }, + }, + shouldError: false, + }, + { + name: "When valid userValidationRules with multiple expressions ANDed together are provided, it should generate valid authentication configuration", + client: fake.NewClientBuilder().Build(), + featureGates: []featuregate.Feature{ + featuregates.ExternalOIDCWithUpstreamParity, + }, + expectedAuthenticationConfiguration: &AuthenticationConfiguration{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiserver.config.k8s.io/v1alpha1", + Kind: "AuthenticationConfiguration", + }, + JWT: []JWTAuthenticator{ + { + Issuer: Issuer{ + URL: "https://test.com", + AudienceMatchPolicy: AudienceMatchPolicyMatchAny, + Audiences: []string{"one", "two"}, + }, + ClaimMappings: ClaimMappings{ + Username: PrefixedClaimOrExpression{ + Prefix: ptr.To("https://test.com#"), + Claim: "username", + }, + Groups: PrefixedClaimOrExpression{ + Prefix: ptr.To(""), + Claim: "", + }, + UID: ClaimOrExpression{Claim: "sub"}, + Extra: []ExtraMapping{}, + }, + ClaimValidationRules: []ClaimValidationRule{}, + UserValidationRules: []UserValidationRule{ + { + Expression: "!user.username.startsWith('system:')", + Message: "username cannot use reserved system: prefix", + }, + { + Expression: "user.groups.size() > 0", + Message: "user must have at least one group", + }, + { + Expression: "user.username.contains('@')", + Message: "username must be an email address", + }, + }, + }, + }, + }, + hcpAuthenticationSpec: &configv1.AuthenticationSpec{ + OIDCProviders: []configv1.OIDCProvider{ + { + Name: "test", + Issuer: configv1.TokenIssuer{ + URL: "https://test.com", + Audiences: []configv1.TokenAudience{"one", "two"}, + }, + ClaimMappings: configv1.TokenClaimMappings{ + Username: configv1.UsernameClaimMapping{ + PrefixPolicy: configv1.NoOpinion, + Claim: "username", + }, + }, + UserValidationRules: []configv1.TokenUserValidationRule{ + { + Expression: "!user.username.startsWith('system:')", + Message: "username cannot use reserved system: prefix", + }, + { + Expression: "user.groups.size() > 0", + Message: "user must have at least one group", + }, + { + Expression: "user.username.contains('@')", + Message: "username must be an email address", + }, + }, + }, + }, + }, + shouldError: false, + }, + { + name: "When valid claimValidationRules with CEL and multiple expressions are provided, it should generate valid authentication configuration", + client: fake.NewClientBuilder().Build(), + featureGates: []featuregate.Feature{ + featuregates.ExternalOIDCWithUpstreamParity, + }, + expectedAuthenticationConfiguration: &AuthenticationConfiguration{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiserver.config.k8s.io/v1alpha1", + Kind: "AuthenticationConfiguration", + }, + JWT: []JWTAuthenticator{ + { + Issuer: Issuer{ + URL: "https://test.com", + AudienceMatchPolicy: AudienceMatchPolicyMatchAny, + Audiences: []string{"one", "two"}, + }, + ClaimMappings: ClaimMappings{ + Username: PrefixedClaimOrExpression{ + Prefix: ptr.To("https://test.com#"), + Claim: "username", + }, + Groups: PrefixedClaimOrExpression{ + Prefix: ptr.To(""), + Claim: "", + }, + UID: ClaimOrExpression{Claim: "sub"}, + Extra: []ExtraMapping{}, + }, + ClaimValidationRules: []ClaimValidationRule{ + { + Expression: "has(claims.email) && claims.email.endsWith('@example.com')", + Message: "email must be from example.com domain", + }, + { + Expression: "claims.email_verified == true", + Message: "email must be verified", + }, + }, + UserValidationRules: []UserValidationRule{}, + }, + }, + }, + hcpAuthenticationSpec: &configv1.AuthenticationSpec{ + OIDCProviders: []configv1.OIDCProvider{ + { + Name: "test", + Issuer: configv1.TokenIssuer{ + URL: "https://test.com", + Audiences: []configv1.TokenAudience{"one", "two"}, + }, + ClaimMappings: configv1.TokenClaimMappings{ + Username: configv1.UsernameClaimMapping{ + PrefixPolicy: configv1.NoOpinion, + Claim: "username", + }, + }, + ClaimValidationRules: []configv1.TokenClaimValidationRule{ + { + Type: configv1.TokenValidationRuleTypeCEL, + CEL: configv1.TokenClaimValidationCELRule{ + Expression: "has(claims.email) && claims.email.endsWith('@example.com')", + Message: "email must be from example.com domain", + }, + }, + { + Type: configv1.TokenValidationRuleTypeCEL, + CEL: configv1.TokenClaimValidationCELRule{ + Expression: "claims.email_verified == true", + Message: "email must be verified", + }, + }, + }, + }, + }, + }, + shouldError: false, + }, + { + name: "When full feature parity with discoveryURL, CEL claim mappings, claim validation, and user validation is configured, it should generate valid authentication configuration", + client: fake.NewClientBuilder().Build(), + featureGates: []featuregate.Feature{ + featuregates.ExternalOIDCWithUpstreamParity, + }, + expectedAuthenticationConfiguration: &AuthenticationConfiguration{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiserver.config.k8s.io/v1alpha1", + Kind: "AuthenticationConfiguration", + }, + JWT: []JWTAuthenticator{ + { + Issuer: Issuer{ + URL: "https://issuer.example.com", + DiscoveryURL: "https://discovery.example.com/.well-known/openid-configuration", + AudienceMatchPolicy: AudienceMatchPolicyMatchAny, + Audiences: []string{"my-app"}, + }, + ClaimMappings: ClaimMappings{ + Username: PrefixedClaimOrExpression{ + Expression: "claims.email.split('@')[0]", + }, + Groups: PrefixedClaimOrExpression{ + Expression: "type(claims.groups) == list ? claims.groups : []", + }, + UID: ClaimOrExpression{Claim: "sub"}, + Extra: []ExtraMapping{}, + }, + ClaimValidationRules: []ClaimValidationRule{ + { + Expression: "has(claims.email) && claims.email.endsWith('@example.com')", + Message: "email must be from example.com domain", + }, + { + Expression: "claims.email_verified == true", + Message: "email must be verified", + }, + }, + UserValidationRules: []UserValidationRule{ + { + Expression: "!user.username.startsWith('system:')", + Message: "username cannot use reserved system: prefix", + }, + { + Expression: "user.groups.size() > 0", + Message: "user must have at least one group", + }, + }, + }, + }, + }, + hcpAuthenticationSpec: &configv1.AuthenticationSpec{ + OIDCProviders: []configv1.OIDCProvider{ + { + Name: "test", + Issuer: configv1.TokenIssuer{ + URL: "https://issuer.example.com", + DiscoveryURL: "https://discovery.example.com/.well-known/openid-configuration", + Audiences: []configv1.TokenAudience{"my-app"}, + }, + ClaimMappings: configv1.TokenClaimMappings{ + Username: configv1.UsernameClaimMapping{ + Expression: "claims.email.split('@')[0]", + }, + Groups: configv1.PrefixedClaimMapping{ + TokenClaimMapping: configv1.TokenClaimMapping{ + Expression: "type(claims.groups) == list ? claims.groups : []", + }, + }, + }, + ClaimValidationRules: []configv1.TokenClaimValidationRule{ + { + Type: configv1.TokenValidationRuleTypeCEL, + CEL: configv1.TokenClaimValidationCELRule{ + Expression: "has(claims.email) && claims.email.endsWith('@example.com')", + Message: "email must be from example.com domain", + }, + }, + { + Type: configv1.TokenValidationRuleTypeCEL, + CEL: configv1.TokenClaimValidationCELRule{ + Expression: "claims.email_verified == true", + Message: "email must be verified", + }, + }, + }, + UserValidationRules: []configv1.TokenUserValidationRule{ + { + Expression: "!user.username.startsWith('system:')", + Message: "username cannot use reserved system: prefix", + }, + { + Expression: "user.groups.size() > 0", + Message: "user must have at least one group", + }, + }, + }, + }, + }, + shouldError: false, + }, + { + name: "When claimValidationRule with CEL has an empty expression, it should return an error", + client: fake.NewClientBuilder().Build(), + featureGates: []featuregate.Feature{ + featuregates.ExternalOIDCWithUpstreamParity, + }, + hcpAuthenticationSpec: &configv1.AuthenticationSpec{ + OIDCProviders: []configv1.OIDCProvider{ + { + Name: "test", + Issuer: configv1.TokenIssuer{ + URL: "https://test.com", + Audiences: []configv1.TokenAudience{"one", "two"}, + }, + ClaimMappings: configv1.TokenClaimMappings{ + Username: configv1.UsernameClaimMapping{ + PrefixPolicy: configv1.NoOpinion, + Claim: "username", + }, + }, + ClaimValidationRules: []configv1.TokenClaimValidationRule{ + { + Type: configv1.TokenValidationRuleTypeCEL, + CEL: configv1.TokenClaimValidationCELRule{ + Expression: "", // empty expression + Message: "validation failed", + }, + }, + }, + }, + }, + }, + shouldError: true, + }, + { + name: "When username expression uses complex CEL to extract from nested claims, it should generate valid authentication configuration", + client: fake.NewClientBuilder().Build(), + featureGates: []featuregate.Feature{ + featuregates.ExternalOIDCWithUpstreamParity, + }, + expectedAuthenticationConfiguration: &AuthenticationConfiguration{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiserver.config.k8s.io/v1alpha1", + Kind: "AuthenticationConfiguration", + }, + JWT: []JWTAuthenticator{ + { + Issuer: Issuer{ + URL: "https://test.com", + AudienceMatchPolicy: AudienceMatchPolicyMatchAny, + Audiences: []string{"one", "two"}, + }, + ClaimMappings: ClaimMappings{ + Username: PrefixedClaimOrExpression{ + Expression: "has(claims.preferred_username) ? claims.preferred_username : claims.sub", + }, + Groups: PrefixedClaimOrExpression{ + Prefix: ptr.To(""), + Claim: "", + }, + UID: ClaimOrExpression{Claim: "sub"}, + Extra: []ExtraMapping{}, + }, + ClaimValidationRules: []ClaimValidationRule{}, + UserValidationRules: []UserValidationRule{}, + }, + }, + }, + hcpAuthenticationSpec: &configv1.AuthenticationSpec{ + OIDCProviders: []configv1.OIDCProvider{ + { + Name: "test", + Issuer: configv1.TokenIssuer{ + URL: "https://test.com", + Audiences: []configv1.TokenAudience{"one", "two"}, + }, + ClaimMappings: configv1.TokenClaimMappings{ + Username: configv1.UsernameClaimMapping{ + Expression: "has(claims.preferred_username) ? claims.preferred_username : claims.sub", + }, + }, + }, + }, + }, + shouldError: false, + }, + { + name: "When groups expression uses complex CEL with conditionals based on claim type, it should generate valid authentication configuration", + client: fake.NewClientBuilder().Build(), + featureGates: []featuregate.Feature{ + featuregates.ExternalOIDCWithUpstreamParity, + }, + expectedAuthenticationConfiguration: &AuthenticationConfiguration{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiserver.config.k8s.io/v1alpha1", + Kind: "AuthenticationConfiguration", + }, + JWT: []JWTAuthenticator{ + { + Issuer: Issuer{ + URL: "https://test.com", + AudienceMatchPolicy: AudienceMatchPolicyMatchAny, + Audiences: []string{"one", "two"}, + }, + ClaimMappings: ClaimMappings{ + Username: PrefixedClaimOrExpression{ + Prefix: ptr.To("https://test.com#"), + Claim: "username", + }, + Groups: PrefixedClaimOrExpression{ + Expression: "claims.?groups.orValue([])", + }, + UID: ClaimOrExpression{Claim: "sub"}, + Extra: []ExtraMapping{}, + }, + ClaimValidationRules: []ClaimValidationRule{}, + UserValidationRules: []UserValidationRule{}, + }, + }, + }, + hcpAuthenticationSpec: &configv1.AuthenticationSpec{ + OIDCProviders: []configv1.OIDCProvider{ + { + Name: "test", + Issuer: configv1.TokenIssuer{ + URL: "https://test.com", + Audiences: []configv1.TokenAudience{"one", "two"}, + }, + ClaimMappings: configv1.TokenClaimMappings{ + Username: configv1.UsernameClaimMapping{ + PrefixPolicy: configv1.NoOpinion, + Claim: "username", + }, + Groups: configv1.PrefixedClaimMapping{ + TokenClaimMapping: configv1.TokenClaimMapping{ + Expression: "claims.?groups.orValue([])", + }, + }, + }, + }, + }, + }, + shouldError: false, + }, + { + name: "When multiple claimValidationRules with CEL type and complex expressions are provided, it should generate valid authentication configuration", + client: fake.NewClientBuilder().Build(), + featureGates: []featuregate.Feature{ + featuregates.ExternalOIDCWithUpstreamParity, + }, + expectedAuthenticationConfiguration: &AuthenticationConfiguration{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiserver.config.k8s.io/v1alpha1", + Kind: "AuthenticationConfiguration", + }, + JWT: []JWTAuthenticator{ + { + Issuer: Issuer{ + URL: "https://test.com", + AudienceMatchPolicy: AudienceMatchPolicyMatchAny, + Audiences: []string{"one", "two"}, + }, + ClaimMappings: ClaimMappings{ + Username: PrefixedClaimOrExpression{ + Prefix: ptr.To("https://test.com#"), + Claim: "username", + }, + Groups: PrefixedClaimOrExpression{ + Prefix: ptr.To(""), + Claim: "", + }, + UID: ClaimOrExpression{Claim: "sub"}, + Extra: []ExtraMapping{}, + }, + ClaimValidationRules: []ClaimValidationRule{ + { + Expression: "has(claims.email) && claims.email.contains('@')", + Message: "token must have valid email claim", + }, + { + Expression: "claims.email_verified == true", + Message: "email must be verified", + }, + { + Expression: "has(claims.groups) && type(claims.groups) == list", + Message: "groups claim must be a list", + }, + }, + UserValidationRules: []UserValidationRule{}, + }, + }, + }, + hcpAuthenticationSpec: &configv1.AuthenticationSpec{ + OIDCProviders: []configv1.OIDCProvider{ + { + Name: "test", + Issuer: configv1.TokenIssuer{ + URL: "https://test.com", + Audiences: []configv1.TokenAudience{"one", "two"}, + }, + ClaimMappings: configv1.TokenClaimMappings{ + Username: configv1.UsernameClaimMapping{ + PrefixPolicy: configv1.NoOpinion, + Claim: "username", + }, + }, + ClaimValidationRules: []configv1.TokenClaimValidationRule{ + { + Type: configv1.TokenValidationRuleTypeCEL, + CEL: configv1.TokenClaimValidationCELRule{ + Expression: "has(claims.email) && claims.email.contains('@')", + Message: "token must have valid email claim", + }, + }, + { + Type: configv1.TokenValidationRuleTypeCEL, + CEL: configv1.TokenClaimValidationCELRule{ + Expression: "claims.email_verified == true", + Message: "email must be verified", + }, + }, + { + Type: configv1.TokenValidationRuleTypeCEL, + CEL: configv1.TokenClaimValidationCELRule{ + Expression: "has(claims.groups) && type(claims.groups) == list", + Message: "groups claim must be a list", + }, + }, + }, + }, + }, + }, + shouldError: false, + }, + { + name: "When CEL expression for username and groups with filtering omits prefix and prefixPolicy, it should generate valid authentication configuration", + client: fake.NewClientBuilder().Build(), + featureGates: []featuregate.Feature{ + featuregates.ExternalOIDCWithUpstreamParity, + }, + expectedAuthenticationConfiguration: &AuthenticationConfiguration{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiserver.config.k8s.io/v1alpha1", + Kind: "AuthenticationConfiguration", + }, + JWT: []JWTAuthenticator{ + { + Issuer: Issuer{ + URL: "https://test.com", + AudienceMatchPolicy: AudienceMatchPolicyMatchAny, + Audiences: []string{"one", "two"}, + }, + ClaimMappings: ClaimMappings{ + Username: PrefixedClaimOrExpression{ + Expression: "claims.email.split('@')[0]", + }, + Groups: PrefixedClaimOrExpression{ + Expression: "claims.?groups.orValue(dyn([])).filter(g, g.startsWith('ocp-'))", + }, + UID: ClaimOrExpression{Claim: "sub"}, + Extra: []ExtraMapping{}, + }, + ClaimValidationRules: []ClaimValidationRule{ + { + Expression: "claims.email_verified == true", + Message: "email must be verified", + }, + }, + UserValidationRules: []UserValidationRule{ + { + Expression: "user.username.size() > 5", + Message: "username must be longer than 5 characters", + }, + { + Expression: "user.groups.size() > 0", + Message: "user must belong to at least one group after filtering", + }, + }, + }, + }, + }, + hcpAuthenticationSpec: &configv1.AuthenticationSpec{ + OIDCProviders: []configv1.OIDCProvider{ + { + Name: "test", + Issuer: configv1.TokenIssuer{ + URL: "https://test.com", + Audiences: []configv1.TokenAudience{"one", "two"}, + }, + ClaimMappings: configv1.TokenClaimMappings{ + Username: configv1.UsernameClaimMapping{ + // Omitting prefixPolicy when using expression - should be allowed + Expression: "claims.email.split('@')[0]", + }, + Groups: configv1.PrefixedClaimMapping{ + TokenClaimMapping: configv1.TokenClaimMapping{ + // Omitting prefix when using expression - should be allowed + Expression: "claims.?groups.orValue(dyn([])).filter(g, g.startsWith('ocp-'))", + }, + }, + }, + ClaimValidationRules: []configv1.TokenClaimValidationRule{ + { + Type: configv1.TokenValidationRuleTypeCEL, + CEL: configv1.TokenClaimValidationCELRule{ + Expression: "claims.email_verified == true", + Message: "email must be verified", + }, + }, + }, + UserValidationRules: []configv1.TokenUserValidationRule{ + { + Expression: "user.username.size() > 5", + Message: "username must be longer than 5 characters", + }, + { + Expression: "user.groups.size() > 0", + Message: "user must belong to at least one group after filtering", + }, + }, + }, + }, + }, + shouldError: false, + }, + { + name: "When combined claim and user validation with CEL expressions is configured, it should generate valid authentication configuration", + client: fake.NewClientBuilder().Build(), + featureGates: []featuregate.Feature{ + featuregates.ExternalOIDCWithUpstreamParity, + }, + expectedAuthenticationConfiguration: &AuthenticationConfiguration{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiserver.config.k8s.io/v1alpha1", + Kind: "AuthenticationConfiguration", + }, + JWT: []JWTAuthenticator{ + { + Issuer: Issuer{ + URL: "https://test.com", + AudienceMatchPolicy: AudienceMatchPolicyMatchAny, + Audiences: []string{"one", "two"}, + }, + ClaimMappings: ClaimMappings{ + Username: PrefixedClaimOrExpression{ + Expression: "claims.email.split('@')[0]", + }, + Groups: PrefixedClaimOrExpression{ + Expression: "claims.?groups.orValue(dyn([])).filter(g, g.startsWith('ocp-'))", + }, + UID: ClaimOrExpression{Claim: "sub"}, + Extra: []ExtraMapping{}, + }, + ClaimValidationRules: []ClaimValidationRule{ + { + Expression: "has(claims.email) && claims.email.contains('@')", + Message: "token must have valid email claim", + }, + { + Expression: "claims.email_verified == true", + Message: "email must be verified", + }, + }, + UserValidationRules: []UserValidationRule{ + { + Expression: "user.username.size() > 5", + Message: "mapped username must be longer than 5 characters", + }, + { + Expression: "user.groups.size() > 0", + Message: "user must have at least one group after filtering", + }, + { + Expression: "!user.username.startsWith('system:')", + Message: "username cannot use reserved system: prefix", + }, + }, + }, + }, + }, + hcpAuthenticationSpec: &configv1.AuthenticationSpec{ + OIDCProviders: []configv1.OIDCProvider{ + { + Name: "test", + Issuer: configv1.TokenIssuer{ + URL: "https://test.com", + Audiences: []configv1.TokenAudience{"one", "two"}, + }, + ClaimMappings: configv1.TokenClaimMappings{ + Username: configv1.UsernameClaimMapping{ + Expression: "claims.email.split('@')[0]", + }, + Groups: configv1.PrefixedClaimMapping{ + TokenClaimMapping: configv1.TokenClaimMapping{ + Expression: "claims.?groups.orValue(dyn([])).filter(g, g.startsWith('ocp-'))", + }, + }, + }, + ClaimValidationRules: []configv1.TokenClaimValidationRule{ + { + Type: configv1.TokenValidationRuleTypeCEL, + CEL: configv1.TokenClaimValidationCELRule{ + Expression: "has(claims.email) && claims.email.contains('@')", + Message: "token must have valid email claim", + }, + }, + { + Type: configv1.TokenValidationRuleTypeCEL, + CEL: configv1.TokenClaimValidationCELRule{ + Expression: "claims.email_verified == true", + Message: "email must be verified", + }, + }, + }, + UserValidationRules: []configv1.TokenUserValidationRule{ + { + Expression: "user.username.size() > 5", + Message: "mapped username must be longer than 5 characters", + }, + { + Expression: "user.groups.size() > 0", + Message: "user must have at least one group after filtering", + }, + { + Expression: "!user.username.startsWith('system:')", + Message: "username cannot use reserved system: prefix", + }, + }, + }, + }, + }, + shouldError: false, + }, + { + name: "When username expression uses conditional logic with fallback, it should generate valid authentication configuration", + client: fake.NewClientBuilder().Build(), + featureGates: []featuregate.Feature{ + featuregates.ExternalOIDCWithUpstreamParity, + }, + expectedAuthenticationConfiguration: &AuthenticationConfiguration{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiserver.config.k8s.io/v1alpha1", + Kind: "AuthenticationConfiguration", + }, + JWT: []JWTAuthenticator{ + { + Issuer: Issuer{ + URL: "https://test.com", + AudienceMatchPolicy: AudienceMatchPolicyMatchAny, + Audiences: []string{"one", "two"}, + }, + ClaimMappings: ClaimMappings{ + Username: PrefixedClaimOrExpression{ + Expression: "has(claims.preferred_username) && claims.preferred_username != '' ? claims.preferred_username : claims.email.split('@')[0]", + }, + Groups: PrefixedClaimOrExpression{ + Prefix: ptr.To(""), + Claim: "", + }, + UID: ClaimOrExpression{Claim: "sub"}, + Extra: []ExtraMapping{}, + }, + ClaimValidationRules: []ClaimValidationRule{ + { + Expression: "claims.email_verified == true", + Message: "email must be verified when used for username", + }, + }, + UserValidationRules: []UserValidationRule{}, + }, + }, + }, + hcpAuthenticationSpec: &configv1.AuthenticationSpec{ + OIDCProviders: []configv1.OIDCProvider{ + { + Name: "test", + Issuer: configv1.TokenIssuer{ + URL: "https://test.com", + Audiences: []configv1.TokenAudience{"one", "two"}, + }, + ClaimMappings: configv1.TokenClaimMappings{ + Username: configv1.UsernameClaimMapping{ + Expression: "has(claims.preferred_username) && claims.preferred_username != '' ? claims.preferred_username : claims.email.split('@')[0]", + }, + }, + ClaimValidationRules: []configv1.TokenClaimValidationRule{ + { + Type: configv1.TokenValidationRuleTypeCEL, + CEL: configv1.TokenClaimValidationCELRule{ + Expression: "claims.email_verified == true", + Message: "email must be verified when used for username", + }, + }, + }, + }, + }, + }, + shouldError: false, + }, + { + name: "When groups expression uses map and filter operations with orValue for type safety, it should generate valid authentication configuration", + client: fake.NewClientBuilder().Build(), + featureGates: []featuregate.Feature{ + featuregates.ExternalOIDCWithUpstreamParity, + }, + expectedAuthenticationConfiguration: &AuthenticationConfiguration{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiserver.config.k8s.io/v1alpha1", + Kind: "AuthenticationConfiguration", + }, + JWT: []JWTAuthenticator{ + { + Issuer: Issuer{ + URL: "https://test.com", + AudienceMatchPolicy: AudienceMatchPolicyMatchAny, + Audiences: []string{"one", "two"}, + }, + ClaimMappings: ClaimMappings{ + Username: PrefixedClaimOrExpression{ + Prefix: ptr.To("https://test.com#"), + Claim: "username", + }, + Groups: PrefixedClaimOrExpression{ + // Use optional access (?) and dyn([]) to handle optional 'roles' claim and provide type-safe default for filter/map + Expression: "claims.?roles.orValue(dyn([])).filter(r, r.startsWith('openshift-')).map(r, r.substring(10))", + }, + UID: ClaimOrExpression{Claim: "sub"}, + Extra: []ExtraMapping{}, + }, + ClaimValidationRules: []ClaimValidationRule{}, + UserValidationRules: []UserValidationRule{}, + }, + }, + }, + hcpAuthenticationSpec: &configv1.AuthenticationSpec{ + OIDCProviders: []configv1.OIDCProvider{ + { + Name: "test", + Issuer: configv1.TokenIssuer{ + URL: "https://test.com", + Audiences: []configv1.TokenAudience{"one", "two"}, + }, + ClaimMappings: configv1.TokenClaimMappings{ + Username: configv1.UsernameClaimMapping{ + PrefixPolicy: configv1.NoOpinion, + Claim: "username", + }, + Groups: configv1.PrefixedClaimMapping{ + TokenClaimMapping: configv1.TokenClaimMapping{ + // Use optional access (?) and dyn([]) to handle optional 'roles' claim and provide type-safe default for filter/map + Expression: "claims.?roles.orValue(dyn([])).filter(r, r.startsWith('openshift-')).map(r, r.substring(10))", + }, + }, + }, + }, + }, + }, + shouldError: false, + }, } for _, tc := range testCases { diff --git a/hypershift-operator/main.go b/hypershift-operator/main.go index a663242bf0f8..01e81643b04c 100644 --- a/hypershift-operator/main.go +++ b/hypershift-operator/main.go @@ -25,6 +25,7 @@ import ( hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" awsutil "github.com/openshift/hypershift/cmd/infra/aws/util" "github.com/openshift/hypershift/cmd/install/assets" + cpofeaturegate "github.com/openshift/hypershift/control-plane-operator/featuregates" pkiconfig "github.com/openshift/hypershift/control-plane-pki-operator/config" etcdrecovery "github.com/openshift/hypershift/etcd-recovery" "github.com/openshift/hypershift/hypershift-operator/controllers/auditlogpersistence" @@ -209,6 +210,9 @@ func NewStartCommand() *cobra.Command { featuregate.ConfigureFeatureSet(featureSet) featuregate.Gate().AddFlag(cmd.Flags()) + // Configure feature set from CPO (needed to propagate feature gates like TechPreviewNoUpgrade) + cpofeaturegate.ConfigureFeatureSet(featureSet) + cmd.Run = func(cmd *cobra.Command, args []string) { ctx, cancel := context.WithCancel(ctrl.SetupSignalHandler()) defer cancel() diff --git a/test/e2e/external_oidc_test.go b/test/e2e/external_oidc_test.go index 3f186089655b..34917e71b886 100644 --- a/test/e2e/external_oidc_test.go +++ b/test/e2e/external_oidc_test.go @@ -6,6 +6,7 @@ import ( "context" "os" "testing" + "time" . "github.com/onsi/gomega" @@ -13,16 +14,60 @@ import ( configv1client "github.com/openshift/client-go/config/clientset/versioned" hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" e2eutil "github.com/openshift/hypershift/test/e2e/util" - kauthnv1 "k8s.io/api/authentication/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" kauthnv1typedclient "k8s.io/client-go/kubernetes/typed/authentication/v1" - + "k8s.io/client-go/rest" crclient "sigs.k8s.io/controller-runtime/pkg/client" "github.com/openshift/hypershift/control-plane-operator/featuregates" ) +// createTestUserWithGroupAndCustomUsername creates a Keycloak user with a custom username pattern, group, and password. +func createTestUserWithGroup(ctx context.Context, kc *e2eutil.KeycloakAdminClient, usernamePrefix string, emailVerified bool) (string, string, string, string, error) { + // create group + groupName := e2eutil.GenerateRandomPassword(16) + + // create user with custom username + username := e2eutil.GenerateRandomPassword(16) + if len(usernamePrefix) > 0 { + username = usernamePrefix + username + } + email := username + "@test.example.com" + + password := e2eutil.GenerateRandomPassword(16) + + user := e2eutil.KeycloakUser{ + Username: username, + Enabled: true, + FirstName: username, + LastName: "Test", + Email: email, + EmailVerified: emailVerified, + Groups: []string{groupName}, + Credentials: []e2eutil.KeycloakCredential{ + { + Type: "password", + Value: password, + Temporary: false, + }, + }, + } + + _, err := kc.CreateGroup(ctx, groupName) + if err != nil { + return "", "", "", "", err + } + + _, err = kc.CreateUser(ctx, user) + if err != nil { + return "", "", "", "", err + } + + return username, email, password, groupName, nil +} + func TestExternalOIDC(t *testing.T) { e2eutil.AtLeast(t, e2eutil.Version419) @@ -50,28 +95,56 @@ func TestExternalOIDC(t *testing.T) { g.Expect(hostedCluster.Spec.Configuration.Authentication).NotTo(BeNil()) g.Expect(hostedCluster.Spec.Configuration.Authentication.OIDCProviders).NotTo(BeEmpty()) clientCfg := e2eutil.WaitForGuestRestConfig(t, ctx, mgtClient, hostedCluster) - authKubeConfig := e2eutil.ChangeUserForKeycloakExtOIDC(t, ctx, clientCfg, clusterOpts.ExtOIDCConfig) - authClient, err := kauthnv1typedclient.NewForConfig(authKubeConfig) - g.Expect(err).NotTo(HaveOccurred()) - selfSubjectReview, err := authClient.SelfSubjectReviews().Create(ctx, &kauthnv1.SelfSubjectReview{}, metav1.CreateOptions{}) - g.Expect(err).NotTo(HaveOccurred()) - t.Logf("selfSubjectReview %+v", selfSubjectReview) + + // Setup Keycloak admin client + kc, err := e2eutil.SetupKeycloakAdminClientFromCluster(ctx, t, mgtClient, clusterOpts.ExtOIDCConfig) + if err != nil { + t.Skipf("Could not setup Keycloak admin client: %v", err) + } t.Run("[OCPFeatureGate:ExternalOIDC] test keycloak external OIDC", func(t *testing.T) { // No gates exist for ExternalOIDC as it has already been enabled by default. g := NewWithT(t) - t.Logf("begin to test external OIDC %s", globalOpts.ExternalOIDCProvider) - g.Expect(hostedCluster.Spec.Configuration).NotTo(BeNil()) - g.Expect(hostedCluster.Spec.Configuration.Authentication).NotTo(BeNil()) - g.Expect(hostedCluster.Spec.Configuration.Authentication.OIDCProviders).NotTo(BeEmpty()) - clientCfg := e2eutil.WaitForGuestRestConfig(t, ctx, mgtClient, hostedCluster) - e2eutil.ChangeClientForKeycloakExtOIDC(t, ctx, clientCfg, clusterOpts.ExtOIDCConfig) + username, email, password, groupName, err := createTestUserWithGroup(ctx, kc, "", true) + g.Expect(err).NotTo(HaveOccurred()) + + testAuthConfig := *clusterOpts.ExtOIDCConfig + testAuthConfig.TestUsers = username + ":" + password + testUserKubeConfig := e2eutil.ChangeUserForKeycloakExtOIDC(t, ctx, clientCfg, &testAuthConfig) + testAuthClient, err := kauthnv1typedclient.NewForConfig(testUserKubeConfig) + g.Expect(err).NotTo(HaveOccurred()) + + selfSubjectReview, err := testAuthClient.SelfSubjectReviews().Create(ctx, &kauthnv1.SelfSubjectReview{}, metav1.CreateOptions{}) + g.Expect(err).NotTo(HaveOccurred(), "user should be authenticated + able to do SelfSubjectReview") + g.Expect(selfSubjectReview.Status.UserInfo.Username).To(Equal(clusterOpts.ExtOIDCConfig.UserPrefix+email), "username should be mapped correctly") + g.Expect(selfSubjectReview.Status.UserInfo.Groups).To(HaveLen(2), "user should have groups system:authenticated and IdP group") + g.Expect(selfSubjectReview.Status.UserInfo.Groups).To(ContainElements("system:authenticated", clusterOpts.ExtOIDCConfig.GroupPrefix+groupName)) + t.Logf("successfully get oidc user client") + }) if featuregates.Gate().Enabled(featuregates.ExternalOIDCWithUIDAndExtraClaimMappings) { + // ExternalOIDCWithUIDAndExtraClaimMappings has graduated to Default feature set + // Auth config includes: UID expression + Extra claim mappings + // Auth config uses: Static claim-based username/groups WITH prefixes (legacy behavior) t.Run("[OCPFeatureGate:ExternalOIDCWithUIDAndExtraClaimMappings] Test external OIDC userInfo username", func(t *testing.T) { g := NewWithT(t) + username, email, password, groupName, err := createTestUserWithGroup(ctx, kc, "", true) + g.Expect(err).NotTo(HaveOccurred()) + + testAuthConfig := *clusterOpts.ExtOIDCConfig + testAuthConfig.TestUsers = username + ":" + password + testUserKubeConfig := e2eutil.ChangeUserForKeycloakExtOIDC(t, ctx, clientCfg, &testAuthConfig) + testAuthClient, err := kauthnv1typedclient.NewForConfig(testUserKubeConfig) + g.Expect(err).NotTo(HaveOccurred()) + + selfSubjectReview, err := testAuthClient.SelfSubjectReviews().Create(ctx, &kauthnv1.SelfSubjectReview{}, metav1.CreateOptions{}) + g.Expect(err).NotTo(HaveOccurred(), "user should be authenticated + able to do SelfSubjectReview") + g.Expect(selfSubjectReview.Status.UserInfo.Username).To(Equal(clusterOpts.ExtOIDCConfig.UserPrefix+email), "username should be mapped correctly") + g.Expect(selfSubjectReview.Status.UserInfo.Groups).To(HaveLen(2), "user should have groups system:authenticated and IdP group") + g.Expect(selfSubjectReview.Status.UserInfo.Groups).To(ContainElements("system:authenticated", clusterOpts.ExtOIDCConfig.GroupPrefix+groupName)) + t.Logf("begin to test external OIDC with external OIDC userInfo username") g.Expect(selfSubjectReview.Status.UserInfo.Username).NotTo(BeEmpty()) g.Expect(selfSubjectReview.Status.UserInfo.Username).Should(ContainSubstring(clusterOpts.ExtOIDCConfig.UserPrefix)) @@ -79,13 +152,45 @@ func TestExternalOIDC(t *testing.T) { t.Run("[OCPFeatureGate:ExternalOIDCWithUIDAndExtraClaimMappings] Test external OIDC userInfo Groups", func(t *testing.T) { g := NewWithT(t) + username, email, password, groupName, err := createTestUserWithGroup(ctx, kc, "", true) + g.Expect(err).NotTo(HaveOccurred()) + + testAuthConfig := *clusterOpts.ExtOIDCConfig + testAuthConfig.TestUsers = username + ":" + password + testUserKubeConfig := e2eutil.ChangeUserForKeycloakExtOIDC(t, ctx, clientCfg, &testAuthConfig) + testAuthClient, err := kauthnv1typedclient.NewForConfig(testUserKubeConfig) + g.Expect(err).NotTo(HaveOccurred()) + + selfSubjectReview, err := testAuthClient.SelfSubjectReviews().Create(ctx, &kauthnv1.SelfSubjectReview{}, metav1.CreateOptions{}) + g.Expect(err).NotTo(HaveOccurred(), "user should be authenticated + able to do SelfSubjectReview") + g.Expect(selfSubjectReview.Status.UserInfo.Username).To(Equal(clusterOpts.ExtOIDCConfig.UserPrefix+email), "username should be mapped correctly") + g.Expect(selfSubjectReview.Status.UserInfo.Groups).To(HaveLen(2), "user should have groups system:authenticated and IdP group") + g.Expect(selfSubjectReview.Status.UserInfo.Groups).To(ContainElements("system:authenticated", clusterOpts.ExtOIDCConfig.GroupPrefix+groupName)) + t.Logf("begin to test external OIDC userInfo Groups") g.Expect(selfSubjectReview.Status.UserInfo.Groups).NotTo(BeEmpty()) g.Expect(selfSubjectReview.Status.UserInfo.Groups).Should(ContainElements(ContainSubstring(clusterOpts.ExtOIDCConfig.GroupPrefix))) }) + // UID and Extra mappings are present in Default feature set + // Config: UID expression ("testuid-" + claims.sub + "-uidtest") + 2 Extra mappings t.Run("[OCPFeatureGate:ExternalOIDCWithUIDAndExtraClaimMappings] Test external OIDC userInfo UID", func(t *testing.T) { g := NewWithT(t) + username, email, password, groupName, err := createTestUserWithGroup(ctx, kc, "", true) + g.Expect(err).NotTo(HaveOccurred()) + + testAuthConfig := *clusterOpts.ExtOIDCConfig + testAuthConfig.TestUsers = username + ":" + password + testUserKubeConfig := e2eutil.ChangeUserForKeycloakExtOIDC(t, ctx, clientCfg, &testAuthConfig) + testAuthClient, err := kauthnv1typedclient.NewForConfig(testUserKubeConfig) + g.Expect(err).NotTo(HaveOccurred()) + + selfSubjectReview, err := testAuthClient.SelfSubjectReviews().Create(ctx, &kauthnv1.SelfSubjectReview{}, metav1.CreateOptions{}) + g.Expect(err).NotTo(HaveOccurred(), "user should be authenticated + able to do SelfSubjectReview") + g.Expect(selfSubjectReview.Status.UserInfo.Username).To(Equal(clusterOpts.ExtOIDCConfig.UserPrefix+email), "username should be mapped correctly") + g.Expect(selfSubjectReview.Status.UserInfo.Groups).To(HaveLen(2), "user should have groups system:authenticated and IdP group") + g.Expect(selfSubjectReview.Status.UserInfo.Groups).To(ContainElements("system:authenticated", clusterOpts.ExtOIDCConfig.GroupPrefix+groupName)) + t.Logf("begin to test external OIDC userInfo UID") g.Expect(selfSubjectReview.Status.UserInfo.UID).NotTo(BeEmpty()) g.Expect(selfSubjectReview.Status.UserInfo.UID).Should(ContainSubstring(e2eutil.ExternalOIDCUIDExpressionPrefix)) @@ -94,6 +199,21 @@ func TestExternalOIDC(t *testing.T) { t.Run("[OCPFeatureGate:ExternalOIDCWithUIDAndExtraClaimMappings] Test external OIDC userInfo Extra", func(t *testing.T) { g := NewWithT(t) + username, email, password, groupName, err := createTestUserWithGroup(ctx, kc, "", true) + g.Expect(err).NotTo(HaveOccurred()) + + testAuthConfig := *clusterOpts.ExtOIDCConfig + testAuthConfig.TestUsers = username + ":" + password + testUserKubeConfig := e2eutil.ChangeUserForKeycloakExtOIDC(t, ctx, clientCfg, &testAuthConfig) + testAuthClient, err := kauthnv1typedclient.NewForConfig(testUserKubeConfig) + g.Expect(err).NotTo(HaveOccurred()) + + selfSubjectReview, err := testAuthClient.SelfSubjectReviews().Create(ctx, &kauthnv1.SelfSubjectReview{}, metav1.CreateOptions{}) + g.Expect(err).NotTo(HaveOccurred(), "user should be authenticated + able to do SelfSubjectReview") + g.Expect(selfSubjectReview.Status.UserInfo.Username).To(Equal(clusterOpts.ExtOIDCConfig.UserPrefix+email), "username should be mapped correctly") + g.Expect(selfSubjectReview.Status.UserInfo.Groups).To(HaveLen(2), "user should have groups system:authenticated and IdP group") + g.Expect(selfSubjectReview.Status.UserInfo.Groups).To(ContainElements("system:authenticated", clusterOpts.ExtOIDCConfig.GroupPrefix+groupName)) + t.Logf("begin to test external OIDC userInfo Extra") g.Expect(selfSubjectReview.Status.UserInfo.Extra).NotTo(BeEmpty()) g.Expect(selfSubjectReview.Status.UserInfo.Extra).Should(HaveKey(e2eutil.ExternalOIDCExtraKeyBar)) @@ -102,12 +222,196 @@ func TestExternalOIDC(t *testing.T) { t.Run("[OCPFeatureGate:ExternalOIDCWithUIDAndExtraClaimMappings] Test external OIDC: check co status using oauth client", func(t *testing.T) { g := NewWithT(t) - t.Logf("begin to test for checking co status") - client, err := configv1client.NewForConfig(authKubeConfig) + username, email, password, groupName, err := createTestUserWithGroup(ctx, kc, "", true) + g.Expect(err).NotTo(HaveOccurred()) + + testAuthConfig := *clusterOpts.ExtOIDCConfig + testAuthConfig.TestUsers = username + ":" + password + testUserKubeConfig := e2eutil.ChangeUserForKeycloakExtOIDC(t, ctx, clientCfg, &testAuthConfig) + testAuthClient, err := kauthnv1typedclient.NewForConfig(testUserKubeConfig) + g.Expect(err).NotTo(HaveOccurred()) + + selfSubjectReview, err := testAuthClient.SelfSubjectReviews().Create(ctx, &kauthnv1.SelfSubjectReview{}, metav1.CreateOptions{}) + g.Expect(err).NotTo(HaveOccurred(), "user should be authenticated + able to do SelfSubjectReview") + g.Expect(selfSubjectReview.Status.UserInfo.Username).To(Equal(clusterOpts.ExtOIDCConfig.UserPrefix+email), "username should be mapped correctly") + g.Expect(selfSubjectReview.Status.UserInfo.Groups).To(HaveLen(2), "user should have groups system:authenticated and IdP group") + g.Expect(selfSubjectReview.Status.UserInfo.Groups).To(ContainElements("system:authenticated", clusterOpts.ExtOIDCConfig.GroupPrefix+groupName)) + + t.Logf("begin to test for checking cluster operator status") + client, err := configv1client.NewForConfig(testUserKubeConfig) g.Expect(err).NotTo(HaveOccurred()) _, err = client.ConfigV1().ClusterOperators().Get(ctx, "image-registry", metav1.GetOptions{}) g.Expect(err).To(HaveOccurred()) }) } + + // ExternalOIDCWithUpstreamParity tests - Tests CEL expressions and validation rules + // Auth config adds: CEL expressions for username/groups (NO prefixes) + // Auth config adds: Claim validation rules (email exists, email_verified) + // Auth config adds: User validation rules (no system: prefix, no 'forbidden' word) + if featuregates.Gate().Enabled(featuregates.ExternalOIDCWithUpstreamParity) { + upstreamParityOpts := clusterOpts + upstreamParityOpts.FeatureSet = string(configv1.TechPreviewNoUpgrade) + upstreamParityOpts.ExtOIDCConfig.CustomizeAuthSpec = func(spec *configv1.AuthenticationSpec) { + // Use CEL expression for username mapping instead of static claim + spec.OIDCProviders[0].ClaimMappings.Username = configv1.UsernameClaimMapping{ + Expression: "claims.email.split('@')[0]", + } + + // Use CEL expression for groups mapping instead of static claim + spec.OIDCProviders[0].ClaimMappings.Groups = configv1.PrefixedClaimMapping{ + TokenClaimMapping: configv1.TokenClaimMapping{ + Expression: "claims.?groups.orValue([])", + }, + } + + // Add claim validation rules + spec.OIDCProviders[0].ClaimValidationRules = []configv1.TokenClaimValidationRule{ + { + Type: configv1.TokenValidationRuleTypeCEL, + CEL: configv1.TokenClaimValidationCELRule{ + Expression: "claims.email_verified == true", + Message: "email_verified claim must be true", + }, + }, + } + + // Add user validation rules + spec.OIDCProviders[0].UserValidationRules = []configv1.TokenUserValidationRule{ + { + Expression: "!user.username.contains('forbidden')", + Message: "username cannot contain the word 'forbidden'", + }, + } + } + + featuregates.ConfigureFeatureSet(upstreamParityOpts.FeatureSet) + + t.Run("[OCPFeatureGate:ExternalOIDCWithUpstreamParity] Patch and validate upstream parity config", func(t *testing.T) { + g := NewWithT(t) + + t.Logf("Patching HostedCluster %s/%s with upstream parity OIDC config", hostedCluster.Namespace, hostedCluster.Name) + + // Build the new auth spec using CustomizeAuthSpec pattern (already exists!) + // Use upstreamParityOpts which has the CustomizeAuthSpec callback set + newAuthSpec := upstreamParityOpts.ExtOIDCConfig.GetAuthenticationConfig() + + // Patch using the same pattern as postCreateExternalOIDC + patchHostedClusterAuth(ctx, g, mgtClient, hostedCluster, newAuthSpec) + + // Wait for KAS to reload - reuse the Eventually pattern from v2 tests + waitForKASAuthReload(ctx, t, g, clientCfg, upstreamParityOpts.ExtOIDCConfig, kc) + + t.Logf("Successfully patched and validated upstream parity OIDC config") + }) + + t.Run("[OCPFeatureGate:ExternalOIDCWithUpstreamParity] Token is valid + authn'd, username/groups mapped correctly", func(t *testing.T) { + g := NewWithT(t) + username, _, password, groupName, err := createTestUserWithGroup(ctx, kc, "", true) + g.Expect(err).NotTo(HaveOccurred()) + + testAuthConfig := *upstreamParityOpts.ExtOIDCConfig + testAuthConfig.TestUsers = username + ":" + password + testUserKubeConfig := e2eutil.ChangeUserForKeycloakExtOIDC(t, ctx, clientCfg, &testAuthConfig) + testAuthClient, err := kauthnv1typedclient.NewForConfig(testUserKubeConfig) + g.Expect(err).NotTo(HaveOccurred()) + + selfSubjectReview, err := testAuthClient.SelfSubjectReviews().Create(ctx, &kauthnv1.SelfSubjectReview{}, metav1.CreateOptions{}) + g.Expect(err).NotTo(HaveOccurred(), "user should be authenticated + able to do SelfSubjectReview") + g.Expect(selfSubjectReview.Status.UserInfo.Username).To(Equal(username), "username should be mapped correctly") + g.Expect(selfSubjectReview.Status.UserInfo.Groups).To(HaveLen(2), "user should have groups system:authenticated and IdP group") + g.Expect(selfSubjectReview.Status.UserInfo.Groups).To(ContainElements("system:authenticated", groupName)) + }) + + t.Run("[OCPFeatureGate:ExternalOIDCWithUpstreamParity] Token is valid, user not authn'd, claim validations not passed", func(t *testing.T) { + g := NewWithT(t) + username, _, password, _, err := createTestUserWithGroup(ctx, kc, "", false) + + testAuthConfig := *upstreamParityOpts.ExtOIDCConfig + testAuthConfig.TestUsers = username + ":" + password + testUserKubeConfig := e2eutil.ChangeUserForKeycloakExtOIDC(t, ctx, clientCfg, &testAuthConfig) + testAuthClient, err := kauthnv1typedclient.NewForConfig(testUserKubeConfig) + g.Expect(err).NotTo(HaveOccurred()) + + _, err = testAuthClient.SelfSubjectReviews().Create(ctx, &kauthnv1.SelfSubjectReview{}, metav1.CreateOptions{}) + + g.Expect(err).To(HaveOccurred(), "user should not be authenticated + able to do SelfSubjectReview") + g.Expect(apierrors.IsUnauthorized(err)).To(BeTrue(), "should receive an unauthorized error when trying to create SelfSubjectReview") + }) + + t.Run("[OCPFeatureGate:ExternalOIDCWithUpstreamParity] Token is valid, user not authn'd, user validations not passed", func(t *testing.T) { + g := NewWithT(t) + username, _, password, _, err := createTestUserWithGroup(ctx, kc, "cel-test-user-forbidden", true) + g.Expect(err).NotTo(HaveOccurred()) + + testAuthConfig := *upstreamParityOpts.ExtOIDCConfig + testAuthConfig.TestUsers = username + ":" + password + testUserKubeConfig := e2eutil.ChangeUserForKeycloakExtOIDC(t, ctx, clientCfg, &testAuthConfig) + testAuthClient, err := kauthnv1typedclient.NewForConfig(testUserKubeConfig) + g.Expect(err).NotTo(HaveOccurred()) + + _, err = testAuthClient.SelfSubjectReviews().Create(ctx, &kauthnv1.SelfSubjectReview{}, metav1.CreateOptions{}) + + g.Expect(err).To(HaveOccurred(), "user should not be authenticated + able to do SelfSubjectReview") + g.Expect(apierrors.IsUnauthorized(err)).To(BeTrue(), "should receive an unauthorized error when trying to create SelfSubjectReview") + }) + } }).Execute(&clusterOpts, globalOpts.Platform, globalOpts.ArtifactDir, "external-oidc", globalOpts.ServiceAccountSigningKey) } + +// patchHostedClusterAuth patches the HostedCluster's authentication configuration +// Reuses the exact same pattern as azure.go:postCreateExternalOIDC (lines 305-313) +func patchHostedClusterAuth(ctx context.Context, g Gomega, mgtClient crclient.Client, hc *hyperv1.HostedCluster, newAuthSpec *configv1.AuthenticationSpec) { + // Get the latest version + current := &hyperv1.HostedCluster{} + err := mgtClient.Get(ctx, crclient.ObjectKey{Namespace: hc.Namespace, Name: hc.Name}, current) + g.Expect(err).NotTo(HaveOccurred(), "should be able to get HostedCluster") + + // Create patch + patch := crclient.MergeFrom(current.DeepCopy()) + + // Mutate + if current.Spec.Configuration == nil { + current.Spec.Configuration = &hyperv1.ClusterConfiguration{} + } + current.Spec.Configuration.Authentication = newAuthSpec + + // Apply + err = mgtClient.Patch(ctx, current, patch) + g.Expect(err).NotTo(HaveOccurred(), "should be able to patch HostedCluster authentication config") +} + +// waitForKASAuthReload waits for KAS to pick up the new authentication config +// Reuses the Eventually pattern from hosted_cluster_external_oidc_test.go:310-325 +func waitForKASAuthReload(ctx context.Context, t *testing.T, g Gomega, clientCfg *rest.Config, authConfig *e2eutil.ExtOIDCConfig, kc *e2eutil.KeycloakAdminClient) { + t.Logf("Waiting for KAS to reload authentication config (timeout: 5 minutes)") + + // Create a test user to validate the new config + username, _, password, _, err := createTestUserWithGroup(ctx, kc, "", true) + g.Expect(err).NotTo(HaveOccurred()) + + testAuthConfig := *authConfig + testAuthConfig.TestUsers = username + ":" + password + + // This is the SAME pattern as v2 tests - Eventually + fresh token per attempt + g.Eventually(func(g Gomega) { + t.Logf("Attempting authentication with new OIDC config (user: %s)", username) + + // Get fresh token each attempt (Keycloak tokens have short TTL) + testUserKubeConfig := e2eutil.ChangeUserForKeycloakExtOIDC(t, ctx, clientCfg, &testAuthConfig) + testAuthClient, err := kauthnv1typedclient.NewForConfig(testUserKubeConfig) + g.Expect(err).NotTo(HaveOccurred()) + + // Try SelfSubjectReview - fails until KAS reloads + selfSubjectReview, err := testAuthClient.SelfSubjectReviews().Create(ctx, &kauthnv1.SelfSubjectReview{}, metav1.CreateOptions{}) + if err != nil { + t.Logf("Authentication attempt failed (expected during reload): %v", err) + } + g.Expect(err).NotTo(HaveOccurred(), "KAS should accept OIDC token after reload") + + // Verify new config is active (username = email local part, NO prefix) + g.Expect(selfSubjectReview.Status.UserInfo.Username).To(Equal(username), "username should use CEL expression (no prefix)") + + t.Logf("KAS has successfully reloaded authentication config") + }).WithTimeout(5 * time.Minute).WithPolling(15 * time.Second).Should(Succeed()) +} diff --git a/test/e2e/util/external_oidc.go b/test/e2e/util/external_oidc.go index 8cfb466145a4..62cb37620511 100644 --- a/test/e2e/util/external_oidc.go +++ b/test/e2e/util/external_oidc.go @@ -27,10 +27,8 @@ import ( configv1 "github.com/openshift/api/config/v1" configv1typedclient "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1" - kauthnv1 "k8s.io/api/authentication/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - kauthnv1typedclient "k8s.io/client-go/kubernetes/typed/authentication/v1" "k8s.io/client-go/rest" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" @@ -68,6 +66,10 @@ type ExtOIDCConfig struct { // for oidcProviders.issuer.issuerCertificateAuthority IssuerCAConfigmapName string IssuerCABundleFile string + + // CustomizeAuthSpec allows tests to modify the baseline auth configuration + // The function receives the generated baseline spec and can modify it in place + CustomizeAuthSpec func(*configv1.AuthenticationSpec) } func GetExtOIDCConfig(provider, cliClientID, consoleClientID, issuerURL, consoleSecret, issuerCABundleFile, testUsers string) *ExtOIDCConfig { @@ -156,6 +158,11 @@ func (config *ExtOIDCConfig) GetAuthenticationConfig() *configv1.AuthenticationS ) } + // Apply custom modifications if provided + if config.CustomizeAuthSpec != nil { + config.CustomizeAuthSpec(authnSpec) + } + return authnSpec } @@ -220,15 +227,6 @@ func IsExternalOIDCCluster(t testing.TB, ctx context.Context, clientCfg *rest.Co return authConfig.Spec.Type == configv1.AuthenticationTypeOIDC, nil } -// ChangeClientForKeycloakExtOIDC changes the guest client using a keycloak user config -func ChangeClientForKeycloakExtOIDC(t testing.TB, ctx context.Context, clientCfg *rest.Config, authConfig *ExtOIDCConfig) crclient.Client { - g := NewWithT(t) - newConfig := ChangeUserForKeycloakExtOIDC(t, ctx, clientCfg, authConfig) - client, err := crclient.New(newConfig, crclient.Options{Scheme: scheme}) - g.Expect(err).NotTo(HaveOccurred(), "could not create guest client using the new config") - return client -} - // ChangeUserForKeycloakExtOIDC changes the user of current CLI session for a Keycloak external OIDC cluster func ChangeUserForKeycloakExtOIDC(t testing.TB, ctx context.Context, clientCfg *rest.Config, authConfig *ExtOIDCConfig) *rest.Config { g := NewWithT(t) @@ -282,7 +280,7 @@ func ChangeUserForKeycloakExtOIDC(t testing.TB, ctx context.Context, clientCfg * body, err := io.ReadAll(response.Body) g.Expect(err).NotTo(HaveOccurred()) - var respMap map[string]interface{} + var respMap map[string]any err = json.Unmarshal(body, &respMap) g.Expect(err).NotTo(HaveOccurred()) idToken, ok := respMap["id_token"].(string) @@ -317,15 +315,7 @@ func ChangeUserForKeycloakExtOIDC(t testing.TB, ctx context.Context, clientCfg * err = os.WriteFile(filepath.Join(tokenCacheDir, "oc", tokenCacheFile), []byte(tokenCache), 0600) g.Expect(err).NotTo(HaveOccurred()) - clientConfigForExtOIDCUser := GetClientConfigForKeycloakOIDCUser(clientCfg, authConfig, tokenCacheDir) - authClient, err := kauthnv1typedclient.NewForConfig(clientConfigForExtOIDCUser) - g.Expect(err).NotTo(HaveOccurred()) - - selfSubjectReview, err := authClient.SelfSubjectReviews().Create(ctx, &kauthnv1.SelfSubjectReview{}, metav1.CreateOptions{}) - g.Expect(err).NotTo(HaveOccurred()) - - t.Logf("Detected external OIDC cluster using Keycloak as the provider. The user is now %q", selfSubjectReview.Status.UserInfo.Username) - return clientConfigForExtOIDCUser + return GetClientConfigForKeycloakOIDCUser(clientCfg, authConfig, tokenCacheDir) } // GetClientConfigForKeycloakOIDCUser gets a client config for an external OIDC cluster diff --git a/test/e2e/util/keycloak.go b/test/e2e/util/keycloak.go new file mode 100644 index 000000000000..54f3099bc4f0 --- /dev/null +++ b/test/e2e/util/keycloak.go @@ -0,0 +1,291 @@ +package util + +import ( + "context" + "crypto/tls" + "encoding/json" + "fmt" + "io" + "math/rand" + "net/http" + "net/url" + "strings" + "testing" + + . "github.com/onsi/gomega" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + + crclient "sigs.k8s.io/controller-runtime/pkg/client" +) + +// KeycloakAdminClient provides methods to interact with Keycloak Admin REST API +type KeycloakAdminClient struct { + BaseURL string + AdminToken string + HTTPClient *http.Client + AdminUser string + AdminPass string +} + +// KeycloakUser represents a Keycloak user +type KeycloakUser struct { + Username string `json:"username"` + Enabled bool `json:"enabled"` + FirstName string `json:"firstName,omitempty"` + LastName string `json:"lastName,omitempty"` + Email string `json:"email,omitempty"` + EmailVerified bool `json:"emailVerified,omitempty"` + Groups []string `json:"groups,omitempty"` + Credentials []KeycloakCredential `json:"credentials,omitempty"` +} + +// KeycloakGroup represents a Keycloak group +type KeycloakGroup struct { + Name string `json:"name"` +} + +// KeycloakCredential represents a user password credential +type KeycloakCredential struct { + Type string `json:"type"` + Value string `json:"value"` + Temporary bool `json:"temporary"` +} + +// NewKeycloakAdminClient creates a new Keycloak admin client +func NewKeycloakAdminClient(baseURL, adminUser, adminPass, caCertFile string) *KeycloakAdminClient { + return &KeycloakAdminClient{ + BaseURL: baseURL, + AdminUser: adminUser, + AdminPass: adminPass, + HTTPClient: &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: true, + }, + }, + }, + } +} + +// GetAdminToken obtains an admin access token +func (kc *KeycloakAdminClient) GetAdminToken(ctx context.Context) error { + tokenURL := fmt.Sprintf("%s/realms/master/protocol/openid-connect/token", kc.BaseURL) + + formData := url.Values{ + "client_id": []string{"admin-cli"}, + "grant_type": []string{"password"}, + "username": []string{kc.AdminUser}, + "password": []string{kc.AdminPass}, + } + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, tokenURL, nil) + if err != nil { + return fmt.Errorf("failed to create token request: %w", err) + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.Body = io.NopCloser(strings.NewReader(formData.Encode())) + + resp, err := kc.HTTPClient.Do(req) + if err != nil { + return fmt.Errorf("failed to get admin token: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + body, _ := io.ReadAll(resp.Body) + return fmt.Errorf("failed to get admin token, status: %d, body: %s", resp.StatusCode, string(body)) + } + + var tokenResp map[string]any + if err := json.NewDecoder(resp.Body).Decode(&tokenResp); err != nil { + return fmt.Errorf("failed to decode token response: %w", err) + } + + accessToken, ok := tokenResp["access_token"].(string) + if !ok { + return fmt.Errorf("access_token not found in response") + } + + kc.AdminToken = accessToken + return nil +} + +// CreateGroup creates a new group in Keycloak +func (kc *KeycloakAdminClient) CreateGroup(ctx context.Context, groupName string) (string, error) { + groupURL := fmt.Sprintf("%s/admin/realms/master/groups", kc.BaseURL) + + group := KeycloakGroup{Name: groupName} + groupJSON, err := json.Marshal(group) + if err != nil { + return "", fmt.Errorf("failed to marshal group: %w", err) + } + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, groupURL, strings.NewReader(string(groupJSON))) + if err != nil { + return "", fmt.Errorf("failed to create group request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+kc.AdminToken) + req.Header.Set("Content-Type", "application/json") + + resp, err := kc.HTTPClient.Do(req) + if err != nil { + return "", fmt.Errorf("failed to create group: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusCreated { + body, _ := io.ReadAll(resp.Body) + return "", fmt.Errorf("failed to create group, status: %d, body: %s", resp.StatusCode, string(body)) + } + + // Extract group ID from Location header + location := resp.Header.Get("Location") + if location == "" { + return "", fmt.Errorf("location header not found in response") + } + + // Location format: https://host/admin/realms/master/groups/{groupId} + parts := strings.Split(location, "/") + if len(parts) == 0 { + return "", fmt.Errorf("failed to parse group ID from location: %s", location) + } + groupID := parts[len(parts)-1] + + return groupID, nil +} + +// CreateUser creates a new user in Keycloak +func (kc *KeycloakAdminClient) CreateUser(ctx context.Context, user KeycloakUser) (string, error) { + userURL := fmt.Sprintf("%s/admin/realms/master/users", kc.BaseURL) + + userJSON, err := json.Marshal(user) + if err != nil { + return "", fmt.Errorf("failed to marshal user: %w", err) + } + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, userURL, strings.NewReader(string(userJSON))) + if err != nil { + return "", fmt.Errorf("failed to create user request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+kc.AdminToken) + req.Header.Set("Content-Type", "application/json") + + resp, err := kc.HTTPClient.Do(req) + if err != nil { + return "", fmt.Errorf("failed to create user: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusCreated { + body, _ := io.ReadAll(resp.Body) + return "", fmt.Errorf("failed to create user, status: %d, body: %s", resp.StatusCode, string(body)) + } + + // Extract user ID from Location header + location := resp.Header.Get("Location") + if location == "" { + return "", fmt.Errorf("location header not found in response") + } + + // Location format: https://host/admin/realms/master/users/{userId} + parts := strings.Split(location, "/") + if len(parts) == 0 { + return "", fmt.Errorf("failed to parse user ID from location: %s", location) + } + userID := parts[len(parts)-1] + + return userID, nil +} + +// GenerateRandomPassword generates a random password +func GenerateRandomPassword(length int) string { + const charset = "abcdefghijklmnopqrstuvwxyz0123456789" + b := make([]byte, length) + for i := range b { + b[i] = charset[rand.Intn(len(charset))] + } + return string(b) +} + +// SetupKeycloakAdminClientFromCluster retrieves Keycloak admin credentials from the cluster and creates an admin client +func SetupKeycloakAdminClientFromCluster(ctx context.Context, t *testing.T, mgtClient crclient.Client, config *ExtOIDCConfig) (*KeycloakAdminClient, error) { + g := NewWithT(t) + + // Tests are ran on both AWS and Azure AKS clusters respectively. + // However, Keycloak credentials are stored differently on both. + + // On AWS, both admin username and password credentials are stored + // via a StatefulSet called 'keycloak' in the 'keycloak' namespace. + + // On AKS, the admin username is stored in a config map called + // 'keycloak-env-vars' in 'keycloak' namespace via data.KC_BOOTSTAP_ADMIN_USERNAME, + // and the admin password is stored in a secret called 'keycloak' + // in the 'keycloak' namespace via data.admin-password . + // https://github.com/bitnami/charts/tree/main/bitnami/keycloak/templates + + adminUser, adminPass := "", "" + + // Try AWS approach first: read from StatefulSet environment variables + t.Logf("Retrieving Keycloak admin credentials from StatefulSet (AWS approach)") + sts := &appsv1.StatefulSet{} + err := mgtClient.Get(ctx, crclient.ObjectKey{ + Namespace: "keycloak", + Name: "keycloak", + }, sts) + if err == nil { + // StatefulSet exists, try to read credentials from environment variables + for _, env := range sts.Spec.Template.Spec.Containers[0].Env { + if env.Name == "KC_BOOTSTRAP_ADMIN_USERNAME" { + adminUser = env.Value + } + if env.Name == "KC_BOOTSTRAP_ADMIN_PASSWORD" { + adminPass = env.Value + } + } + } + + // If credentials not found in StatefulSet, try AKS approach: ConfigMap + Secret + if adminUser == "" || adminPass == "" { + t.Logf("Credentials not found in StatefulSet, trying AKS approach (ConfigMap + Secret)") + + // Get admin username from ConfigMap + cm := &corev1.ConfigMap{} + err = mgtClient.Get(ctx, crclient.ObjectKey{ + Namespace: "keycloak", + Name: "keycloak-env-vars", + }, cm) + if err == nil && cm.Data != nil { + adminUser = cm.Data["KC_BOOTSTRAP_ADMIN_USERNAME"] + } + + // Get admin password from Secret + secret := &corev1.Secret{} + err = mgtClient.Get(ctx, crclient.ObjectKey{ + Namespace: "keycloak", + Name: "keycloak", + }, secret) + if err == nil && secret.Data != nil { + adminPass = string(secret.Data["admin-password"]) + } + } + + // Verify we found both credentials + if adminUser == "" || adminPass == "" { + return nil, fmt.Errorf("could not find Keycloak admin credentials in StatefulSet (AWS) or ConfigMap+Secret (AKS)") + } + + t.Logf("Successfully retrieved Keycloak admin credentials (username: %s)", adminUser) + + // Trim /realms/master from issuerURL + baseURL := strings.TrimSuffix(config.IssuerURL, "/realms/master") + kc := NewKeycloakAdminClient(baseURL, adminUser, adminPass, config.IssuerCABundleFile) + + // Verify access by getting admin token + err = kc.GetAdminToken(ctx) + g.Expect(err).NotTo(HaveOccurred(), "failed to get admin token") + + t.Logf("Successfully created Keycloak admin client") + return kc, nil +}