From a403122101fcfd7281bffc32c5d874f757488065 Mon Sep 17 00:00:00 2001 From: Evan Hearne Date: Mon, 20 Apr 2026 13:34:49 +0100 Subject: [PATCH 1/9] test(CNTRLPLANE-3306): add ExternalOIDCWithUpstreamParity e2e tests This change: + Adds extensive testing for the ExternalOIDCWithUpstreamParity feature gate in `control-plane-operator/controllers/hostedcontrolplane/v2/kas/auth_test.go`. + includes testing the discoveryUrl, userValidationRules, claimValidationRules, and claimMappings. + Tests the feature gate in a HyperShift cluster in `test/e2e/external_oidc_test.go`. + Provides basic auth config testing of the feature gate in `test/e2e/util/external_oidc.go` . This should allow us to progress in promotion of the feature from TechPreview to GA. --- .../hostedcontrolplane/v2/kas/auth_test.go | 998 ++++++++++++++++++ test/e2e/external_oidc_test.go | 75 ++ test/e2e/util/external_oidc.go | 63 +- 3 files changed, 1135 insertions(+), 1 deletion(-) 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..7dae9fe33788 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: "valid discovery URL - different host from issuer URL", + 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: "valid discovery URL - different path from issuer URL", + 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: "valid userValidationRule - single expression", + 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: "valid userValidationRule - multiple expressions ANDed together", + 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: "valid claimValidationRule with CEL - multiple expressions", + 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: "full feature parity - discoveryURL, CEL claim mappings, claim validation, and user validation", + 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: "claimValidationRule with CEL - empty expression, 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: "username expression with complex CEL - extracting from nested claims", + 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: "groups expression with complex CEL - conditional based on claim type", + 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: "multiple claimValidationRules - CEL type with complex expressions", + 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: "CEL expression username and groups with filtering - omitting prefix/prefixPolicy", + 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: "combined claim and user validation with CEL expressions", + 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: "username expression with conditional logic and fallback", + 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: "groups expression with map and filter operations - using orValue for type safety", + 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/test/e2e/external_oidc_test.go b/test/e2e/external_oidc_test.go index 3f186089655b..e9240fd4ced3 100644 --- a/test/e2e/external_oidc_test.go +++ b/test/e2e/external_oidc_test.go @@ -5,6 +5,7 @@ package e2e import ( "context" "os" + "strings" "testing" . "github.com/onsi/gomega" @@ -109,5 +110,79 @@ func TestExternalOIDC(t *testing.T) { g.Expect(err).To(HaveOccurred()) }) } + + if featuregates.Gate().Enabled(featuregates.ExternalOIDCWithUpstreamParity) { + t.Run("[OCPFeatureGate:ExternalOIDCWithUpstreamParity] Test CEL username expression mapping", func(t *testing.T) { + g := NewWithT(t) + t.Logf("begin to test CEL username expression mapping") + g.Expect(selfSubjectReview.Status.UserInfo.Username).NotTo(BeEmpty()) + // Username should be the email prefix (before @) since we configured expression: claims.email.split('@')[0] + // doesn't contain substring + g.Expect(selfSubjectReview.Status.UserInfo.Username).NotTo(ContainSubstring("@")) + // equals the actual email prefix + // e2eutil.ExternalOIDCExtraKeyFoo --> claims.email expression + emailValues := selfSubjectReview.Status.UserInfo.Extra[e2eutil.ExternalOIDCExtraKeyFoo] + g.Expect(emailValues).NotTo(BeEmpty()) + email := emailValues[0] + expectedUserName := strings.Split(email, "@")[0] + g.Expect(selfSubjectReview.Status.UserInfo.Username).Should(Equal(expectedUserName)) + t.Logf("CEL username expression successfully mapped to: %s", selfSubjectReview.Status.UserInfo.Username) + }) + + t.Run("[OCPFeatureGate:ExternalOIDCWithUpstreamParity] Test CEL groups expression mapping", func(t *testing.T) { + g := NewWithT(t) + t.Logf("begin to test CEL groups expression mapping") + // Groups expression uses: claims.?groups.orValue([]) + // If the token has groups, they should be present without prefix (no prefix in CEL expression) + g.Expect(hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].ClaimMappings.Groups.Expression).NotTo(BeEmpty()) + t.Logf("CEL groups expression configured: %s", hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].ClaimMappings.Groups.Expression) + // Verify the groups are actually mapped correctly - should exist and have no prefix + g.Expect(selfSubjectReview.Status.UserInfo.Groups).NotTo(BeEmpty()) + // Groups should NOT contain the prefix since we're using CEL expression without prefix + for _, group := range selfSubjectReview.Status.UserInfo.Groups { + g.Expect(group).NotTo(HavePrefix(clusterOpts.ExtOIDCConfig.GroupPrefix)) + } + t.Logf("CEL groups expression successfully mapped groups without prefix: %v", selfSubjectReview.Status.UserInfo.Groups) + }) + + t.Run("[OCPFeatureGate:ExternalOIDCWithUpstreamParity] Test claim validation rules", func(t *testing.T) { + g := NewWithT(t) + t.Logf("begin to test claim validation rules") + g.Expect(hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].ClaimValidationRules).NotTo(BeEmpty()) + claimRules := hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].ClaimValidationRules + g.Expect(claimRules).Should(HaveLen(2)) + + // Verify configuration - rules should be CEL type with expected expressions + g.Expect(claimRules[0].Type).Should(Equal(configv1.TokenValidationRuleTypeCEL)) + g.Expect(claimRules[0].CEL.Expression).Should(Equal(e2eutil.ClaimValidationExprEmailExists)) + + g.Expect(claimRules[1].Type).Should(Equal(configv1.TokenValidationRuleTypeCEL)) + g.Expect(claimRules[1].CEL.Expression).Should(Equal(e2eutil.ClaimValidationExprEmailVerified)) + + // Verify behavior - authentication succeeded, proving claim validation rules passed + // Rule 1 validates email exists and is non-empty + emailValues := selfSubjectReview.Status.UserInfo.Extra[e2eutil.ExternalOIDCExtraKeyFoo] + g.Expect(emailValues).NotTo(BeEmpty(), "email claim should exist (validated by rule 1)") + g.Expect(emailValues[0]).NotTo(BeEmpty(), "email claim should be non-empty (validated by rule 1)") + // Rule 2 validates email_verified == true (implicit - authentication succeeded) + t.Logf("Claim validation rules successfully validated token with email: %s", emailValues[0]) + }) + + t.Run("[OCPFeatureGate:ExternalOIDCWithUpstreamParity] Test user validation rules", func(t *testing.T) { + g := NewWithT(t) + t.Logf("begin to test user validation rules") + g.Expect(hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].UserValidationRules).NotTo(BeEmpty()) + userRules := hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].UserValidationRules + g.Expect(userRules).Should(HaveLen(1)) + + // Verify configuration - rule should use expected CEL expression + g.Expect(userRules[0].Expression).Should(Equal(e2eutil.UserValidationExprNoSystemPrefix)) + + // Verify behavior - authentication succeeded, proving user validation rule passed + // Rule validates username doesn't start with 'system:' + g.Expect(selfSubjectReview.Status.UserInfo.Username).NotTo(HavePrefix("system:"), "username should not have system: prefix (validated by user rule)") + t.Logf("User validation rules successfully validated user: %s", selfSubjectReview.Status.UserInfo.Username) + }) + } }).Execute(&clusterOpts, globalOpts.Platform, globalOpts.ArtifactDir, "external-oidc", globalOpts.ServiceAccountSigningKey) } diff --git a/test/e2e/util/external_oidc.go b/test/e2e/util/external_oidc.go index 8cfb466145a4..6bcc68ab3d1e 100644 --- a/test/e2e/util/external_oidc.go +++ b/test/e2e/util/external_oidc.go @@ -49,6 +49,13 @@ const ( ExternalOIDCExtraKeyBarValueExpression = "extra-test-mark" ExternalOIDCExtraKeyFoo = "extratest.openshift.com/foo" ExternalOIDCExtraKeyFooValueExpression = "claims.email" // This is a variable, not a string literal + + // CEL expressions for claim validation rules + ClaimValidationExprEmailExists = "has(claims.email) && claims.email != ''" + ClaimValidationExprEmailVerified = "claims.email_verified == true" + + // CEL expression for user validation rule + UserValidationExprNoSystemPrefix = "!user.username.startsWith('system:')" ) type ExtOIDCConfig struct { @@ -156,6 +163,57 @@ func (config *ExtOIDCConfig) GetAuthenticationConfig() *configv1.AuthenticationS ) } + if featuregates.Gate().Enabled(featuregates.ExternalOIDCWithUpstreamParity) { + // Check if ExternalOIDCWithUIDAndExtraClaimMappings feature gate is enabled. + // If not, we will need to add extra mapping to access email for username + // verification later. + if !featuregates.Gate().Enabled(featuregates.ExternalOIDCWithUIDAndExtraClaimMappings) { + authnSpec.OIDCProviders[0].ClaimMappings.Extra = append(authnSpec.OIDCProviders[0].ClaimMappings.Extra, + configv1.ExtraMapping{ + Key: ExternalOIDCExtraKeyFoo, + ValueExpression: ExternalOIDCExtraKeyFooValueExpression, + }, + ) + } + // Use CEL expression for username mapping instead of static claim + authnSpec.OIDCProviders[0].ClaimMappings.Username = configv1.UsernameClaimMapping{ + Expression: "claims.email.split('@')[0]", + } + + // Use CEL expression for groups mapping instead of static claim + authnSpec.OIDCProviders[0].ClaimMappings.Groups = configv1.PrefixedClaimMapping{ + TokenClaimMapping: configv1.TokenClaimMapping{ + Expression: "claims.?groups.orValue([])", + }, + } + + // Add claim validation rules + authnSpec.OIDCProviders[0].ClaimValidationRules = []configv1.TokenClaimValidationRule{ + { + Type: configv1.TokenValidationRuleTypeCEL, + CEL: configv1.TokenClaimValidationCELRule{ + Expression: ClaimValidationExprEmailExists, + Message: "email claim must be present and non-empty", + }, + }, + { + Type: configv1.TokenValidationRuleTypeCEL, + CEL: configv1.TokenClaimValidationCELRule{ + Expression: ClaimValidationExprEmailVerified, + Message: "email_verified claim must be true", + }, + }, + } + + // Add user validation rules + authnSpec.OIDCProviders[0].UserValidationRules = []configv1.TokenUserValidationRule{ + { + Expression: UserValidationExprNoSystemPrefix, + Message: "username cannot use reserved system: prefix", + }, + } + } + return authnSpec } @@ -180,8 +238,11 @@ func ValidateAuthenticationSpec(t testing.TB, ctx context.Context, client crclie actualAuth := hostedCluster.Spec.Configuration.Authentication g.Expect(actualAuth.OIDCProviders).NotTo(BeEmpty()) - if featuregates.Gate().Enabled(featuregates.ExternalOIDCWithUIDAndExtraClaimMappings) { + if featuregates.Gate().Enabled(featuregates.ExternalOIDCWithUpstreamParity) || featuregates.Gate().Enabled(featuregates.ExternalOIDCWithUIDAndExtraClaimMappings) { g.Expect(actualAuth.OIDCProviders[0].ClaimMappings.Extra).NotTo(BeEmpty()) + } + + if featuregates.Gate().Enabled(featuregates.ExternalOIDCWithUIDAndExtraClaimMappings) { g.Expect(actualAuth.OIDCProviders[0].ClaimMappings.UID).NotTo(BeNil()) } From 22b0c7f4eaf7f56d091aa58c74aabf33c8529af9 Mon Sep 17 00:00:00 2001 From: Evan Hearne Date: Thu, 30 Apr 2026 11:32:45 +0100 Subject: [PATCH 2/9] test(CNTRLPLANE-3306): add funcs/tests for interacting with keycloak instance This commit adds the necessary functions to interact with the externally deployed Keycloak instance. The Keycloak instance is deployed at the link below. https://github.com/openshift/release/blob/main/ci-operator/step-registry/idp /external-oidc/keycloak/server/idp-external-oidc-keycloak-server-commands.sh Also adds user/group CRUD functionality to tests. --- test/e2e/external_oidc_test.go | 102 ++++++ test/e2e/util/external_oidc.go | 645 ++++++++++++++++++++++++++++++++- 2 files changed, 746 insertions(+), 1 deletion(-) diff --git a/test/e2e/external_oidc_test.go b/test/e2e/external_oidc_test.go index e9240fd4ced3..5e0f8c2d27d4 100644 --- a/test/e2e/external_oidc_test.go +++ b/test/e2e/external_oidc_test.go @@ -129,6 +129,108 @@ func TestExternalOIDC(t *testing.T) { t.Logf("CEL username expression successfully mapped to: %s", selfSubjectReview.Status.UserInfo.Username) }) + t.Run("[OCPFeatureGate:ExternalOIDCWithUpstreamParity] Test CEL username expression mapping 1", func(t *testing.T) { + g := NewWithT(t) + t.Logf("begin to test CEL username expression mapping with manually created user/group") + + // Get admin credentials from environment variables + adminUser := os.Getenv("KEYCLOAK_ADMIN_USER") + adminPass := os.Getenv("KEYCLOAK_ADMIN_PASS") + if adminUser == "" || adminPass == "" { + t.Skip("KEYCLOAK_ADMIN_USER and KEYCLOAK_ADMIN_PASS environment variables must be set") + } + + // Create admin client + kc := e2eutil.NewKeycloakAdminClient(clusterOpts.ExtOIDCConfig.IssuerURL, adminUser, adminPass, clusterOpts.ExtOIDCConfig.IssuerCABundleFile) + err := kc.GetAdminToken(ctx) + g.Expect(err).NotTo(HaveOccurred(), "failed to get admin token") + + // Create test resources tracker for automatic cleanup + testResources := e2eutil.NewTestResources(kc) + defer testResources.Cleanup(ctx, t) + + // Create a test group + testGroupName := "cel-test-group-" + e2eutil.GenerateRandomPassword(8) + groupID, err := testResources.CreateTestGroup(ctx, t, testGroupName) + g.Expect(err).NotTo(HaveOccurred(), "failed to create test group") + t.Logf("Created test group: %s (ID: %s)", testGroupName, groupID) + + // Create a test user with specific email + testUsername := "cel-test-user-" + e2eutil.GenerateRandomPassword(8) + testEmail := testUsername + "@cel-test.example.com" + testPassword := e2eutil.GenerateRandomPassword(16) + userID, err := testResources.CreateTestUser(ctx, t, testUsername, testEmail, testPassword) + g.Expect(err).NotTo(HaveOccurred(), "failed to create test user") + t.Logf("Created test user: %s (email: %s, ID: %s)", testUsername, testEmail, userID) + + // Add user to group + err = kc.AddUserToGroup(ctx, userID, groupID) + g.Expect(err).NotTo(HaveOccurred(), "failed to add user to group") + t.Logf("Added user %s to group %s", testUsername, testGroupName) + + // Create a temporary auth config for this test user + testAuthConfig := *clusterOpts.ExtOIDCConfig + testAuthConfig.TestUsers = testUsername + ":" + testPassword + + // Authenticate as the test user + testUserKubeConfig := e2eutil.ChangeUserForKeycloakExtOIDC(t, ctx, clientCfg, &testAuthConfig) + testAuthClient, err := kauthnv1typedclient.NewForConfig(testUserKubeConfig) + g.Expect(err).NotTo(HaveOccurred(), "failed to create auth client for test user") + + // Verify username expression mapping + testSelfSubjectReview, err := testAuthClient.SelfSubjectReviews().Create(ctx, &kauthnv1.SelfSubjectReview{}, metav1.CreateOptions{}) + g.Expect(err).NotTo(HaveOccurred(), "failed to get self subject review") + t.Logf("Test user self subject review: %+v", testSelfSubjectReview.Status.UserInfo) + + // Verify username is the email prefix (before @) + expectedUsername := strings.Split(testEmail, "@")[0] + g.Expect(testSelfSubjectReview.Status.UserInfo.Username).Should(Equal(expectedUsername), + "username should be email prefix from CEL expression: claims.email.split('@')[0]") + g.Expect(testSelfSubjectReview.Status.UserInfo.Username).NotTo(ContainSubstring("@"), + "username should not contain @ symbol") + t.Logf("CEL username expression correctly mapped '%s' to '%s'", testEmail, testSelfSubjectReview.Status.UserInfo.Username) + + // Verify groups are mapped without prefix (due to CEL expression) + g.Expect(testSelfSubjectReview.Status.UserInfo.Groups).NotTo(BeEmpty(), + "user should have groups from Keycloak") + hasTestGroup := false + for _, group := range testSelfSubjectReview.Status.UserInfo.Groups { + if group == testGroupName { + hasTestGroup = true + } + // Groups should not have prefix when using CEL expression + g.Expect(group).NotTo(HavePrefix(clusterOpts.ExtOIDCConfig.GroupPrefix), + "groups should not have prefix when using CEL expression") + } + g.Expect(hasTestGroup).To(BeTrue(), "user should be member of test group: %s", testGroupName) + t.Logf("CEL groups expression correctly mapped groups: %v", testSelfSubjectReview.Status.UserInfo.Groups) + + // Clean up resources (will be called by defer, but we'll do it explicitly to verify deletion) + t.Logf("Deleting test user and group") + err = kc.DeleteUser(ctx, userID) + g.Expect(err).NotTo(HaveOccurred(), "failed to delete test user") + t.Logf("✓ Deleted user: %s", userID) + + err = kc.DeleteGroup(ctx, groupID) + g.Expect(err).NotTo(HaveOccurred(), "failed to delete test group") + t.Logf("✓ Deleted group: %s", groupID) + + // Verify deletion - trying to get user/group should fail + t.Logf("Verifying user deletion") + _, err = kc.GetUserByUsername(ctx, testUsername) + g.Expect(err).To(HaveOccurred(), "user should not exist after deletion") + g.Expect(err.Error()).To(ContainSubstring("user not found"), "error should indicate user not found") + t.Logf("Verified user deletion: user %s not found", testUsername) + + t.Logf("Verifying group deletion") + _, err = kc.GetGroupByName(ctx, testGroupName) + g.Expect(err).To(HaveOccurred(), "group should not exist after deletion") + g.Expect(err.Error()).To(ContainSubstring("group not found"), "error should indicate group not found") + t.Logf("Verified group deletion: group %s not found", testGroupName) + + t.Logf("Test completed successfully: CEL username expression mapping verified with manual user/group creation and cleanup") + }) + t.Run("[OCPFeatureGate:ExternalOIDCWithUpstreamParity] Test CEL groups expression mapping", func(t *testing.T) { g := NewWithT(t) t.Logf("begin to test CEL groups expression mapping") diff --git a/test/e2e/util/external_oidc.go b/test/e2e/util/external_oidc.go index 6bcc68ab3d1e..ac7dd1bd7cbe 100644 --- a/test/e2e/util/external_oidc.go +++ b/test/e2e/util/external_oidc.go @@ -343,7 +343,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) @@ -418,3 +418,646 @@ func GetClientConfigForKeycloakOIDCUser(clientCfg *rest.Config, authConfig *ExtO return userClientConfig } + +// KeycloakAdminClient provides methods to interact with Keycloak Admin REST API +type KeycloakAdminClient struct { + BaseURL string + AdminToken string + HTTPClient *http.Client + AdminUser string + AdminPass string + CACertFile 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"` +} + +// 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"` +} + +// KeycloakClient represents a Keycloak client +type KeycloakClient struct { + ID string `json:"id"` + ClientID string `json:"clientId"` +} + +// KeycloakProtocolMapper represents a protocol mapper +type KeycloakProtocolMapper struct { + Name string `json:"name"` + Protocol string `json:"protocol"` + ProtocolMapper string `json:"protocolMapper"` + ConsentRequired bool `json:"consentRequired"` + Config map[string]string `json:"config"` +} + +// NewKeycloakAdminClient creates a new Keycloak admin client +func NewKeycloakAdminClient(baseURL, adminUser, adminPass, caCertFile string) *KeycloakAdminClient { + return &KeycloakAdminClient{ + BaseURL: baseURL, + AdminUser: adminUser, + AdminPass: adminPass, + CACertFile: caCertFile, + 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, "POST", 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, "POST", 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, "POST", 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 +} + +// SetUserPassword sets a user's password +func (kc *KeycloakAdminClient) SetUserPassword(ctx context.Context, userID, password string, temporary bool) error { + passwordURL := fmt.Sprintf("%s/admin/realms/master/users/%s/reset-password", kc.BaseURL, userID) + + credential := KeycloakCredential{ + Type: "password", + Value: password, + Temporary: temporary, + } + + credJSON, err := json.Marshal(credential) + if err != nil { + return fmt.Errorf("failed to marshal credential: %w", err) + } + + req, err := http.NewRequestWithContext(ctx, "PUT", passwordURL, strings.NewReader(string(credJSON))) + if err != nil { + return fmt.Errorf("failed to create password 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 set password: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusNoContent { + body, _ := io.ReadAll(resp.Body) + return fmt.Errorf("failed to set password, status: %d, body: %s", resp.StatusCode, string(body)) + } + + return nil +} + +// AddUserToGroup adds a user to a group +func (kc *KeycloakAdminClient) AddUserToGroup(ctx context.Context, userID, groupID string) error { + groupURL := fmt.Sprintf("%s/admin/realms/master/users/%s/groups/%s", kc.BaseURL, userID, groupID) + + req, err := http.NewRequestWithContext(ctx, "PUT", groupURL, nil) + if err != nil { + return fmt.Errorf("failed to create add-to-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 add user to group: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusNoContent { + body, _ := io.ReadAll(resp.Body) + return fmt.Errorf("failed to add user to group, status: %d, body: %s", resp.StatusCode, string(body)) + } + + return nil +} + +// GetClientByClientID retrieves a client's internal ID by its clientId +func (kc *KeycloakAdminClient) GetClientByClientID(ctx context.Context, clientID string) (string, error) { + clientsURL := fmt.Sprintf("%s/admin/realms/master/clients?clientId=%s", kc.BaseURL, url.QueryEscape(clientID)) + + req, err := http.NewRequestWithContext(ctx, "GET", clientsURL, nil) + if err != nil { + return "", fmt.Errorf("failed to create get-client request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+kc.AdminToken) + + resp, err := kc.HTTPClient.Do(req) + if err != nil { + return "", fmt.Errorf("failed to get client: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + body, _ := io.ReadAll(resp.Body) + return "", fmt.Errorf("failed to get client, status: %d, body: %s", resp.StatusCode, string(body)) + } + + var clients []KeycloakClient + if err := json.NewDecoder(resp.Body).Decode(&clients); err != nil { + return "", fmt.Errorf("failed to decode clients response: %w", err) + } + + if len(clients) == 0 { + return "", fmt.Errorf("client not found: %s", clientID) + } + + return clients[0].ID, nil +} + +// DeleteUser deletes a user from Keycloak +func (kc *KeycloakAdminClient) DeleteUser(ctx context.Context, userID string) error { + userURL := fmt.Sprintf("%s/admin/realms/master/users/%s", kc.BaseURL, userID) + + req, err := http.NewRequestWithContext(ctx, "DELETE", userURL, nil) + if err != nil { + return fmt.Errorf("failed to create delete-user request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+kc.AdminToken) + + resp, err := kc.HTTPClient.Do(req) + if err != nil { + return fmt.Errorf("failed to delete user: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusNoContent { + body, _ := io.ReadAll(resp.Body) + return fmt.Errorf("failed to delete user, status: %d, body: %s", resp.StatusCode, string(body)) + } + + return nil +} + +// DeleteGroup deletes a group from Keycloak +func (kc *KeycloakAdminClient) DeleteGroup(ctx context.Context, groupID string) error { + groupURL := fmt.Sprintf("%s/admin/realms/master/groups/%s", kc.BaseURL, groupID) + + req, err := http.NewRequestWithContext(ctx, "DELETE", groupURL, nil) + if err != nil { + return fmt.Errorf("failed to create delete-group request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+kc.AdminToken) + + resp, err := kc.HTTPClient.Do(req) + if err != nil { + return fmt.Errorf("failed to delete group: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusNoContent { + body, _ := io.ReadAll(resp.Body) + return fmt.Errorf("failed to delete group, status: %d, body: %s", resp.StatusCode, string(body)) + } + + return nil +} + +// GetUserByUsername retrieves a user ID by username +func (kc *KeycloakAdminClient) GetUserByUsername(ctx context.Context, username string) (string, error) { + usersURL := fmt.Sprintf("%s/admin/realms/master/users?username=%s&exact=true", kc.BaseURL, url.QueryEscape(username)) + + req, err := http.NewRequestWithContext(ctx, "GET", usersURL, nil) + if err != nil { + return "", fmt.Errorf("failed to create get-user request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+kc.AdminToken) + + resp, err := kc.HTTPClient.Do(req) + if err != nil { + return "", fmt.Errorf("failed to get user: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + body, _ := io.ReadAll(resp.Body) + return "", fmt.Errorf("failed to get user, status: %d, body: %s", resp.StatusCode, string(body)) + } + + var users []struct { + ID string `json:"id"` + Username string `json:"username"` + } + if err := json.NewDecoder(resp.Body).Decode(&users); err != nil { + return "", fmt.Errorf("failed to decode users response: %w", err) + } + + if len(users) == 0 { + return "", fmt.Errorf("user not found: %s", username) + } + + return users[0].ID, nil +} + +// GetGroupByName retrieves a group ID by name +func (kc *KeycloakAdminClient) GetGroupByName(ctx context.Context, groupName string) (string, error) { + groupsURL := fmt.Sprintf("%s/admin/realms/master/groups", kc.BaseURL) + + req, err := http.NewRequestWithContext(ctx, "GET", groupsURL, nil) + if err != nil { + return "", fmt.Errorf("failed to create get-groups request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+kc.AdminToken) + + resp, err := kc.HTTPClient.Do(req) + if err != nil { + return "", fmt.Errorf("failed to get groups: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + body, _ := io.ReadAll(resp.Body) + return "", fmt.Errorf("failed to get groups, status: %d, body: %s", resp.StatusCode, string(body)) + } + + var groups []struct { + ID string `json:"id"` + Name string `json:"name"` + } + if err := json.NewDecoder(resp.Body).Decode(&groups); err != nil { + return "", fmt.Errorf("failed to decode groups response: %w", err) + } + + for _, group := range groups { + if group.Name == groupName { + return group.ID, nil + } + } + + return "", fmt.Errorf("group not found: %s", groupName) +} + +// CreateProtocolMapper creates a protocol mapper for a client +func (kc *KeycloakAdminClient) CreateProtocolMapper(ctx context.Context, clientID string, mapper KeycloakProtocolMapper) error { + mapperURL := fmt.Sprintf("%s/admin/realms/master/clients/%s/protocol-mappers/models", kc.BaseURL, clientID) + + mapperJSON, err := json.Marshal(mapper) + if err != nil { + return fmt.Errorf("failed to marshal mapper: %w", err) + } + + req, err := http.NewRequestWithContext(ctx, "POST", mapperURL, strings.NewReader(string(mapperJSON))) + if err != nil { + return fmt.Errorf("failed to create mapper 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 mapper: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusCreated { + body, _ := io.ReadAll(resp.Body) + return fmt.Errorf("failed to create mapper, status: %d, body: %s", resp.StatusCode, string(body)) + } + + return nil +} + +// SetupKeycloakTestEnvironment creates test users, groups, and protocol mappers +func SetupKeycloakTestEnvironment(t *testing.T, ctx context.Context, config *ExtOIDCConfig, adminUser, adminPass string, numUsers int) (string, error) { + g := NewWithT(t) + + // Create admin client + kc := NewKeycloakAdminClient(config.IssuerURL, adminUser, adminPass, config.IssuerCABundleFile) + + // Get admin token + err := kc.GetAdminToken(ctx) + if err != nil { + return "", fmt.Errorf("failed to get admin token: %w", err) + } + + // Create group + t.Logf("Creating Keycloak group: keycloak-testgroup-1") + groupID, err := kc.CreateGroup(ctx, "keycloak-testgroup-1") + if err != nil { + return "", fmt.Errorf("failed to create group: %w", err) + } + t.Logf("Created group with ID: %s", groupID) + + // Create users and add to group + var users []string + for i := 1; i <= numUsers; i++ { + username := fmt.Sprintf("keycloak-testuser-%d", i) + password := GenerateRandomPassword(12) + + user := KeycloakUser{ + Username: username, + Enabled: true, + FirstName: username, + LastName: "KC", + Email: fmt.Sprintf("%s@example.com", username), + EmailVerified: true, + } + + t.Logf("Creating user: %s", username) + userID, err := kc.CreateUser(ctx, user) + if err != nil { + return "", fmt.Errorf("failed to create user %s: %w", username, err) + } + + // Set password + err = kc.SetUserPassword(ctx, userID, password, false) + if err != nil { + return "", fmt.Errorf("failed to set password for user %s: %w", username, err) + } + + // Add to group + err = kc.AddUserToGroup(ctx, userID, groupID) + if err != nil { + return "", fmt.Errorf("failed to add user %s to group: %w", username, err) + } + + users = append(users, fmt.Sprintf("%s:%s", username, password)) + } + + // Create protocol mappers for clients + groupMapper := KeycloakProtocolMapper{ + Name: "groupmapper", + Protocol: "openid-connect", + ProtocolMapper: "oidc-group-membership-mapper", + ConsentRequired: false, + Config: map[string]string{ + "full.path": "false", + "userinfo.token.claim": "true", + "id.token.claim": "true", + "access.token.claim": "false", + "claim.name": "groups", + }, + } + + // Add group mapper to CLI client + t.Logf("Adding group mapper to CLI client: %s", config.CliClientID) + cliClientID, err := kc.GetClientByClientID(ctx, config.CliClientID) + if err != nil { + return "", fmt.Errorf("failed to get CLI client ID: %w", err) + } + err = kc.CreateProtocolMapper(ctx, cliClientID, groupMapper) + g.Expect(err).NotTo(HaveOccurred(), "failed to create protocol mapper for CLI client") + + // Add group mapper to console client + t.Logf("Adding group mapper to console client: %s", config.ConsoleClientID) + consoleClientID, err := kc.GetClientByClientID(ctx, config.ConsoleClientID) + if err != nil { + return "", fmt.Errorf("failed to get console client ID: %w", err) + } + err = kc.CreateProtocolMapper(ctx, consoleClientID, groupMapper) + g.Expect(err).NotTo(HaveOccurred(), "failed to create protocol mapper for console client") + + // Return users in format "user1:pass1,user2:pass2,..." + return strings.Join(users, ","), 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) +} + +// TestResources tracks resources created during a test for cleanup +type TestResources struct { + AdminClient *KeycloakAdminClient + UserIDs []string + GroupIDs []string + UserCreds map[string]string // username -> password +} + +// NewTestResources creates a new TestResources tracker +func NewTestResources(adminClient *KeycloakAdminClient) *TestResources { + return &TestResources{ + AdminClient: adminClient, + UserIDs: []string{}, + GroupIDs: []string{}, + UserCreds: make(map[string]string), + } +} + +// CreateTestUser creates a user and tracks it for cleanup +func (tr *TestResources) CreateTestUser(ctx context.Context, t *testing.T, username, email, password string) (string, error) { + user := KeycloakUser{ + Username: username, + Enabled: true, + FirstName: username, + LastName: "Test", + Email: email, + EmailVerified: true, + } + + userID, err := tr.AdminClient.CreateUser(ctx, user) + if err != nil { + return "", err + } + + // Set password + err = tr.AdminClient.SetUserPassword(ctx, userID, password, false) + if err != nil { + return "", err + } + + // Track for cleanup + tr.UserIDs = append(tr.UserIDs, userID) + tr.UserCreds[username] = password + + t.Logf("Created test user: %s (ID: %s)", username, userID) + return userID, nil +} + +// CreateTestGroup creates a group and tracks it for cleanup +func (tr *TestResources) CreateTestGroup(ctx context.Context, t *testing.T, groupName string) (string, error) { + groupID, err := tr.AdminClient.CreateGroup(ctx, groupName) + if err != nil { + return "", err + } + + // Track for cleanup + tr.GroupIDs = append(tr.GroupIDs, groupID) + + t.Logf("Created test group: %s (ID: %s)", groupName, groupID) + return groupID, nil +} + +// GetTestUsersString returns users in format "user1:pass1,user2:pass2,..." +func (tr *TestResources) GetTestUsersString() string { + var users []string + for username, password := range tr.UserCreds { + users = append(users, fmt.Sprintf("%s:%s", username, password)) + } + return strings.Join(users, ",") +} + +// Cleanup deletes all tracked resources +func (tr *TestResources) Cleanup(ctx context.Context, t *testing.T) { + t.Logf("Cleaning up test resources: %d users, %d groups", len(tr.UserIDs), len(tr.GroupIDs)) + + // Delete users + for _, userID := range tr.UserIDs { + if err := tr.AdminClient.DeleteUser(ctx, userID); err != nil { + t.Logf("Warning: failed to delete user %s: %v", userID, err) + } else { + t.Logf("Deleted user: %s", userID) + } + } + + // Delete groups + for _, groupID := range tr.GroupIDs { + if err := tr.AdminClient.DeleteGroup(ctx, groupID); err != nil { + t.Logf("Warning: failed to delete group %s: %v", groupID, err) + } else { + t.Logf("Deleted group: %s", groupID) + } + } + + // Clear tracking + tr.UserIDs = []string{} + tr.GroupIDs = []string{} + tr.UserCreds = make(map[string]string) +} From 0dc8a03c9f54c66bec28d43d6dc84ea7714e04d7 Mon Sep 17 00:00:00 2001 From: Evan Hearne Date: Thu, 14 May 2026 13:18:46 +0100 Subject: [PATCH 3/9] test(CNTRLPLANE-3306): configure cpo feature sets in hypershift-operator This commit configures the control plane operator feature sets in hypershift-operator. This change is required as hypershift-operator re-uses control-plane-operator auth config validation code, which checks specifically for control-plane-operator feature set enablement. Without this change, if we try to enable feature set such as TechPreviewNoUpgrade, validation will fail as hypershift-operator does not configure control-plane-operator feature sets. This change will allow us to test feature gates behind TechPreviewNoUpgrade and others that are present in control-plane-operator, throughout HyperShift. --- hypershift-operator/main.go | 4 ++++ 1 file changed, 4 insertions(+) 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() From a8998f483715b946a3a40366c348cd0c90747f87 Mon Sep 17 00:00:00 2001 From: Evan Hearne Date: Thu, 14 May 2026 17:00:27 +0100 Subject: [PATCH 4/9] test(CNTRLPLANE-3306): rework tests + add support for aws/aks e2e This commit fixes failing tests for ExternalOIDCWithUpstreamParity and ExternalOIDCWithUIDAndExtraClaimMappings feature gates by wrapping feature gate specific test expectations behind if-statements. Also reworks ExternalOIDCWithUpstreamParity tests with new user/group CRUD framework. It also adds negative testing to existing tests. Additionally, AWS and AKS e2e support is added. Keycloak is deployed differently on both clusters and credentials are also stored differently. TryAuthenticateUser() now uses AnonymousClientConfig() instead of using clientcfg on its own, which effectively copied over admin token that was able to authenticate with kube-apiserver whenever it was used. This caused KAS logs to show selfsubjectreviews being successful for system:admin, because these weren't cleared. Now, only the specified user token should be used to identify the user for validation rule testing. It also adds check for `Unauthorized` to be present in error message, rather than relying on a more detailed error message to be thrown. --- test/e2e/external_oidc_test.go | 382 ++++++++++++++++++++------------- test/e2e/util/external_oidc.go | 287 ++++++++++++++++++++++++- 2 files changed, 515 insertions(+), 154 deletions(-) diff --git a/test/e2e/external_oidc_test.go b/test/e2e/external_oidc_test.go index 5e0f8c2d27d4..2cf8da0c7773 100644 --- a/test/e2e/external_oidc_test.go +++ b/test/e2e/external_oidc_test.go @@ -5,6 +5,7 @@ package e2e import ( "context" "os" + "slices" "strings" "testing" @@ -14,7 +15,6 @@ 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" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" kauthnv1typedclient "k8s.io/client-go/kubernetes/typed/authentication/v1" @@ -58,32 +58,39 @@ func TestExternalOIDC(t *testing.T) { 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) t.Logf("successfully get oidc user client") }) if featuregates.Gate().Enabled(featuregates.ExternalOIDCWithUIDAndExtraClaimMappings) { - t.Run("[OCPFeatureGate:ExternalOIDCWithUIDAndExtraClaimMappings] Test external OIDC userInfo username", func(t *testing.T) { - g := NewWithT(t) - 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)) - }) - t.Run("[OCPFeatureGate:ExternalOIDCWithUIDAndExtraClaimMappings] Test external OIDC userInfo Groups", func(t *testing.T) { - g := NewWithT(t) - 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))) - }) + // Since this username/group behavior differs between ExternalODICWithUIDandExtraClaimMappings and + // ExternalOIDCWithUpstreamParity feature gates, we should put this test behind a feature + // gate check. + if !featuregates.Gate().Enabled(featuregates.ExternalOIDCWithUpstreamParity) { + t.Run("[OCPFeatureGate:ExternalOIDCWithUIDAndExtraClaimMappings] Test external OIDC userInfo username", func(t *testing.T) { + g := NewWithT(t) + 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)) + }) + + t.Run("[OCPFeatureGate:ExternalOIDCWithUIDAndExtraClaimMappings] Test external OIDC userInfo Groups", func(t *testing.T) { + g := NewWithT(t) + 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))) + }) + } t.Run("[OCPFeatureGate:ExternalOIDCWithUIDAndExtraClaimMappings] Test external OIDC userInfo UID", func(t *testing.T) { g := NewWithT(t) @@ -115,175 +122,252 @@ func TestExternalOIDC(t *testing.T) { t.Run("[OCPFeatureGate:ExternalOIDCWithUpstreamParity] Test CEL username expression mapping", func(t *testing.T) { g := NewWithT(t) t.Logf("begin to test CEL username expression mapping") - g.Expect(selfSubjectReview.Status.UserInfo.Username).NotTo(BeEmpty()) - // Username should be the email prefix (before @) since we configured expression: claims.email.split('@')[0] - // doesn't contain substring - g.Expect(selfSubjectReview.Status.UserInfo.Username).NotTo(ContainSubstring("@")) - // equals the actual email prefix - // e2eutil.ExternalOIDCExtraKeyFoo --> claims.email expression - emailValues := selfSubjectReview.Status.UserInfo.Extra[e2eutil.ExternalOIDCExtraKeyFoo] - g.Expect(emailValues).NotTo(BeEmpty()) - email := emailValues[0] - expectedUserName := strings.Split(email, "@")[0] - g.Expect(selfSubjectReview.Status.UserInfo.Username).Should(Equal(expectedUserName)) - t.Logf("CEL username expression successfully mapped to: %s", selfSubjectReview.Status.UserInfo.Username) - }) - - t.Run("[OCPFeatureGate:ExternalOIDCWithUpstreamParity] Test CEL username expression mapping 1", func(t *testing.T) { - g := NewWithT(t) - t.Logf("begin to test CEL username expression mapping with manually created user/group") - - // Get admin credentials from environment variables - adminUser := os.Getenv("KEYCLOAK_ADMIN_USER") - adminPass := os.Getenv("KEYCLOAK_ADMIN_PASS") - if adminUser == "" || adminPass == "" { - t.Skip("KEYCLOAK_ADMIN_USER and KEYCLOAK_ADMIN_PASS environment variables must be set") - } - - // Create admin client - kc := e2eutil.NewKeycloakAdminClient(clusterOpts.ExtOIDCConfig.IssuerURL, adminUser, adminPass, clusterOpts.ExtOIDCConfig.IssuerCABundleFile) - err := kc.GetAdminToken(ctx) - g.Expect(err).NotTo(HaveOccurred(), "failed to get admin token") - // Create test resources tracker for automatic cleanup + // Setup: Create test resources with automatic cleanup testResources := e2eutil.NewTestResources(kc) defer testResources.Cleanup(ctx, t) - // Create a test group - testGroupName := "cel-test-group-" + e2eutil.GenerateRandomPassword(8) - groupID, err := testResources.CreateTestGroup(ctx, t, testGroupName) - g.Expect(err).NotTo(HaveOccurred(), "failed to create test group") - t.Logf("Created test group: %s (ID: %s)", testGroupName, groupID) - - // Create a test user with specific email - testUsername := "cel-test-user-" + e2eutil.GenerateRandomPassword(8) - testEmail := testUsername + "@cel-test.example.com" - testPassword := e2eutil.GenerateRandomPassword(16) - userID, err := testResources.CreateTestUser(ctx, t, testUsername, testEmail, testPassword) - g.Expect(err).NotTo(HaveOccurred(), "failed to create test user") - t.Logf("Created test user: %s (email: %s, ID: %s)", testUsername, testEmail, userID) - - // Add user to group - err = kc.AddUserToGroup(ctx, userID, groupID) - g.Expect(err).NotTo(HaveOccurred(), "failed to add user to group") - t.Logf("Added user %s to group %s", testUsername, testGroupName) - - // Create a temporary auth config for this test user - testAuthConfig := *clusterOpts.ExtOIDCConfig - testAuthConfig.TestUsers = testUsername + ":" + testPassword - - // Authenticate as the test user - testUserKubeConfig := e2eutil.ChangeUserForKeycloakExtOIDC(t, ctx, clientCfg, &testAuthConfig) - testAuthClient, err := kauthnv1typedclient.NewForConfig(testUserKubeConfig) - g.Expect(err).NotTo(HaveOccurred(), "failed to create auth client for test user") - - // Verify username expression mapping - testSelfSubjectReview, err := testAuthClient.SelfSubjectReviews().Create(ctx, &kauthnv1.SelfSubjectReview{}, metav1.CreateOptions{}) - g.Expect(err).NotTo(HaveOccurred(), "failed to get self subject review") - t.Logf("Test user self subject review: %+v", testSelfSubjectReview.Status.UserInfo) - - // Verify username is the email prefix (before @) - expectedUsername := strings.Split(testEmail, "@")[0] - g.Expect(testSelfSubjectReview.Status.UserInfo.Username).Should(Equal(expectedUsername), + // Setup: Create authenticated test user with group + testUser, err := testResources.SetupAuthenticatedUserWithGroup(ctx, t, "cel-test-user", "cel-test-group", clientCfg, clusterOpts.ExtOIDCConfig) + g.Expect(err).NotTo(HaveOccurred()) + + // Verify: Username is email prefix (before @) + expectedUsername := strings.Split(testUser.Email, "@")[0] + g.Expect(testUser.SelfSubjectReview.Status.UserInfo.Username).Should(Equal(expectedUsername), "username should be email prefix from CEL expression: claims.email.split('@')[0]") - g.Expect(testSelfSubjectReview.Status.UserInfo.Username).NotTo(ContainSubstring("@"), + g.Expect(testUser.SelfSubjectReview.Status.UserInfo.Username).NotTo(ContainSubstring("@"), "username should not contain @ symbol") - t.Logf("CEL username expression correctly mapped '%s' to '%s'", testEmail, testSelfSubjectReview.Status.UserInfo.Username) + t.Logf("CEL username expression correctly mapped '%s' to '%s'", testUser.Email, testUser.SelfSubjectReview.Status.UserInfo.Username) + + // Edge case test: preferred_username vs email-derived username mismatch + // Tests that CEL expression uses claims.email, not claims.preferred_username + t.Logf("Edge case test: Creating user with preferred_username different from email local part") + preferredUsername := "cel-preferred-" + e2eutil.GenerateRandomPassword(8) + actualEmail := "cel-email-" + e2eutil.GenerateRandomPassword(8) + "@test.example.com" + preferredPassword := e2eutil.GenerateRandomPassword(16) + + // In Keycloak, the 'username' field becomes the 'preferred_username' claim + // So we create a user where username != email local part + _, err = testResources.CreateTestUser(ctx, t, preferredUsername, actualEmail, preferredPassword) + g.Expect(err).NotTo(HaveOccurred()) + t.Logf("Created user: preferred_username='%s', email='%s'", preferredUsername, actualEmail) + + // Authenticate as this user + preferredAuthConfig := *clusterOpts.ExtOIDCConfig + preferredAuthConfig.TestUsers = preferredUsername + ":" + preferredPassword + preferredKubeConfig := e2eutil.ChangeUserForKeycloakExtOIDC(t, ctx, clientCfg, &preferredAuthConfig) + preferredAuthClient, err := kauthnv1typedclient.NewForConfig(preferredKubeConfig) + g.Expect(err).NotTo(HaveOccurred()) + + preferredReview, err := preferredAuthClient.SelfSubjectReviews().Create(ctx, &kauthnv1.SelfSubjectReview{}, metav1.CreateOptions{}) + g.Expect(err).NotTo(HaveOccurred()) - // Verify groups are mapped without prefix (due to CEL expression) - g.Expect(testSelfSubjectReview.Status.UserInfo.Groups).NotTo(BeEmpty(), + // Verify: K8s username should come from email claim, NOT preferred_username claim + expectedUsername = strings.Split(actualEmail, "@")[0] + g.Expect(preferredReview.Status.UserInfo.Username).Should(Equal(expectedUsername), + "username should be derived from email claim via CEL expression, not from preferred_username claim") + g.Expect(preferredReview.Status.UserInfo.Username).NotTo(Equal(preferredUsername), + "username should NOT equal preferred_username when they differ") + t.Logf("✓ CEL expression correctly used email claim over preferred_username: K8s username='%s' (from email='%s'), preferred_username='%s'", + preferredReview.Status.UserInfo.Username, actualEmail, preferredUsername) + + // Verify: Groups are mapped without prefix + g.Expect(testUser.SelfSubjectReview.Status.UserInfo.Groups).NotTo(BeEmpty(), "user should have groups from Keycloak") hasTestGroup := false - for _, group := range testSelfSubjectReview.Status.UserInfo.Groups { - if group == testGroupName { + for _, group := range testUser.SelfSubjectReview.Status.UserInfo.Groups { + if group == testUser.GroupName { hasTestGroup = true } - // Groups should not have prefix when using CEL expression g.Expect(group).NotTo(HavePrefix(clusterOpts.ExtOIDCConfig.GroupPrefix), "groups should not have prefix when using CEL expression") } - g.Expect(hasTestGroup).To(BeTrue(), "user should be member of test group: %s", testGroupName) - t.Logf("CEL groups expression correctly mapped groups: %v", testSelfSubjectReview.Status.UserInfo.Groups) - - // Clean up resources (will be called by defer, but we'll do it explicitly to verify deletion) - t.Logf("Deleting test user and group") - err = kc.DeleteUser(ctx, userID) - g.Expect(err).NotTo(HaveOccurred(), "failed to delete test user") - t.Logf("✓ Deleted user: %s", userID) - - err = kc.DeleteGroup(ctx, groupID) - g.Expect(err).NotTo(HaveOccurred(), "failed to delete test group") - t.Logf("✓ Deleted group: %s", groupID) - - // Verify deletion - trying to get user/group should fail - t.Logf("Verifying user deletion") - _, err = kc.GetUserByUsername(ctx, testUsername) - g.Expect(err).To(HaveOccurred(), "user should not exist after deletion") - g.Expect(err.Error()).To(ContainSubstring("user not found"), "error should indicate user not found") - t.Logf("Verified user deletion: user %s not found", testUsername) - - t.Logf("Verifying group deletion") - _, err = kc.GetGroupByName(ctx, testGroupName) - g.Expect(err).To(HaveOccurred(), "group should not exist after deletion") - g.Expect(err.Error()).To(ContainSubstring("group not found"), "error should indicate group not found") - t.Logf("Verified group deletion: group %s not found", testGroupName) - - t.Logf("Test completed successfully: CEL username expression mapping verified with manual user/group creation and cleanup") + g.Expect(hasTestGroup).To(BeTrue(), "user should be member of test group: %s", testUser.GroupName) + t.Logf("CEL groups expression correctly mapped groups: %v", testUser.SelfSubjectReview.Status.UserInfo.Groups) }) t.Run("[OCPFeatureGate:ExternalOIDCWithUpstreamParity] Test CEL groups expression mapping", func(t *testing.T) { g := NewWithT(t) t.Logf("begin to test CEL groups expression mapping") - // Groups expression uses: claims.?groups.orValue([]) - // If the token has groups, they should be present without prefix (no prefix in CEL expression) + + // Setup: Create test resources with automatic cleanup + testResources := e2eutil.NewTestResources(kc) + defer testResources.Cleanup(ctx, t) + + // Verify: Groups expression is configured g.Expect(hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].ClaimMappings.Groups.Expression).NotTo(BeEmpty()) t.Logf("CEL groups expression configured: %s", hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].ClaimMappings.Groups.Expression) - // Verify the groups are actually mapped correctly - should exist and have no prefix - g.Expect(selfSubjectReview.Status.UserInfo.Groups).NotTo(BeEmpty()) - // Groups should NOT contain the prefix since we're using CEL expression without prefix - for _, group := range selfSubjectReview.Status.UserInfo.Groups { - g.Expect(group).NotTo(HavePrefix(clusterOpts.ExtOIDCConfig.GroupPrefix)) + + // Setup: Create authenticated test user with group + testUser, err := testResources.SetupAuthenticatedUserWithGroup(ctx, t, "cel-groups-test-user", "cel-groups-test-group", clientCfg, clusterOpts.ExtOIDCConfig) + g.Expect(err).NotTo(HaveOccurred()) + + // Verify: User has groups from Keycloak + g.Expect(testUser.SelfSubjectReview.Status.UserInfo.Groups).NotTo(BeEmpty(), + "user should have groups from Keycloak") + + // Verify: Groups are mapped without prefix (CEL expression removes prefix) + for _, group := range testUser.SelfSubjectReview.Status.UserInfo.Groups { + g.Expect(group).NotTo(HavePrefix(clusterOpts.ExtOIDCConfig.GroupPrefix), + "groups should not have prefix when using CEL expression") } - t.Logf("CEL groups expression successfully mapped groups without prefix: %v", selfSubjectReview.Status.UserInfo.Groups) + + // Verify: User is member of the test group + hasTestGroup := slices.Contains(testUser.SelfSubjectReview.Status.UserInfo.Groups, testUser.GroupName) + g.Expect(hasTestGroup).To(BeTrue(), "user should be member of test group: %s", testUser.GroupName) + t.Logf("CEL groups expression successfully mapped groups without prefix: %v", testUser.SelfSubjectReview.Status.UserInfo.Groups) + + // Negative test: User without groups should still authenticate + // The CEL expression claims.?groups.orValue([]) handles missing groups claim gracefully + t.Logf("Negative test: Creating user without group membership") + noGroupUsername := "cel-no-groups-" + e2eutil.GenerateRandomPassword(8) + noGroupEmail := noGroupUsername + "@test.example.com" + noGroupPassword := e2eutil.GenerateRandomPassword(16) + _, err = testResources.CreateTestUser(ctx, t, noGroupUsername, noGroupEmail, noGroupPassword) + g.Expect(err).NotTo(HaveOccurred()) + + // Authenticate user without groups - should SUCCEED with empty groups + noGroupAuthConfig := *clusterOpts.ExtOIDCConfig + noGroupAuthConfig.TestUsers = noGroupUsername + ":" + noGroupPassword + noGroupKubeConfig := e2eutil.ChangeUserForKeycloakExtOIDC(t, ctx, clientCfg, &noGroupAuthConfig) + noGroupAuthClient, err := kauthnv1typedclient.NewForConfig(noGroupKubeConfig) + g.Expect(err).NotTo(HaveOccurred()) + + noGroupReview, err := noGroupAuthClient.SelfSubjectReviews().Create(ctx, &kauthnv1.SelfSubjectReview{}, metav1.CreateOptions{}) + g.Expect(err).NotTo(HaveOccurred(), "authentication should succeed even when user has no groups") + t.Logf("✓ User without groups authenticated successfully with groups: %v", noGroupReview.Status.UserInfo.Groups) }) t.Run("[OCPFeatureGate:ExternalOIDCWithUpstreamParity] Test claim validation rules", func(t *testing.T) { g := NewWithT(t) t.Logf("begin to test claim validation rules") + + // Refresh admin token before creating test resources + err := kc.GetAdminToken(ctx) + g.Expect(err).NotTo(HaveOccurred(), "failed to refresh Keycloak admin token") + + // Setup: Create test resources with automatic cleanup + testResources := e2eutil.NewTestResources(kc) + defer testResources.Cleanup(ctx, t) + + // Verify: Claim validation rules are configured g.Expect(hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].ClaimValidationRules).NotTo(BeEmpty()) claimRules := hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].ClaimValidationRules g.Expect(claimRules).Should(HaveLen(2)) - // Verify configuration - rules should be CEL type with expected expressions - g.Expect(claimRules[0].Type).Should(Equal(configv1.TokenValidationRuleTypeCEL)) - g.Expect(claimRules[0].CEL.Expression).Should(Equal(e2eutil.ClaimValidationExprEmailExists)) - - g.Expect(claimRules[1].Type).Should(Equal(configv1.TokenValidationRuleTypeCEL)) - g.Expect(claimRules[1].CEL.Expression).Should(Equal(e2eutil.ClaimValidationExprEmailVerified)) - - // Verify behavior - authentication succeeded, proving claim validation rules passed - // Rule 1 validates email exists and is non-empty - emailValues := selfSubjectReview.Status.UserInfo.Extra[e2eutil.ExternalOIDCExtraKeyFoo] - g.Expect(emailValues).NotTo(BeEmpty(), "email claim should exist (validated by rule 1)") - g.Expect(emailValues[0]).NotTo(BeEmpty(), "email claim should be non-empty (validated by rule 1)") - // Rule 2 validates email_verified == true (implicit - authentication succeeded) - t.Logf("Claim validation rules successfully validated token with email: %s", emailValues[0]) + // Verify: Rules are CEL type with expected expressions + g.Expect(claimRules[0].Type).Should(BeEquivalentTo(configv1.TokenValidationRuleTypeCEL)) + g.Expect(claimRules[0].CEL.Expression).Should(BeEquivalentTo(e2eutil.ClaimValidationExprEmailExists)) + g.Expect(claimRules[1].Type).Should(BeEquivalentTo(configv1.TokenValidationRuleTypeCEL)) + g.Expect(claimRules[1].CEL.Expression).Should(BeEquivalentTo(e2eutil.ClaimValidationExprEmailVerified)) + + // Test 1: Valid user - email exists and email_verified=true + // Should PASS validation and authenticate successfully + t.Logf("Test 1: Creating user with valid claims (email exists, email_verified=true)") + validUser, err := testResources.SetupAuthenticatedUserWithGroup(ctx, t, "claim-valid-user", "claim-valid-group", clientCfg, clusterOpts.ExtOIDCConfig) + g.Expect(err).NotTo(HaveOccurred(), "authentication must succeed when all claim validation rules pass") + g.Expect(validUser.Email).NotTo(BeEmpty(), "test user email must be non-empty") + t.Logf("✓ User with email='%s' and email_verified=true authenticated successfully", validUser.Email) + + // Test 2: Invalid user - email_verified=false + // Demonstrates rule 2 requirement: claims.email_verified == true + t.Logf("Test 2: Creating user with email_verified=false (violates rule 2)") + invalidUsername := "claim-invalid-user-" + e2eutil.GenerateRandomPassword(8) + invalidEmail := invalidUsername + "@test.example.com" + invalidPassword := e2eutil.GenerateRandomPassword(16) + invalidUserID, err := testResources.CreateTestUserWithEmailVerification(ctx, t, invalidUsername, invalidEmail, invalidPassword, false) + g.Expect(err).NotTo(HaveOccurred(), "creating user in Keycloak should succeed") + g.Expect(invalidEmail).NotTo(BeEmpty(), "test user has non-empty email but email_verified=false") + t.Logf("✓ Created user '%s' with email='%s' and email_verified=false (ID: %s)", invalidUsername, invalidEmail, invalidUserID) + + // Attempt to authenticate - should FAIL due to claim validation rule 2 + err = testResources.TryAuthenticateUser(ctx, t, invalidUsername, invalidPassword, clientCfg, clusterOpts.ExtOIDCConfig) + g.Expect(err).To(HaveOccurred(), "authentication must fail when email_verified=false") + t.Logf("✓ User with email_verified=false correctly rejected: %v", err) + + // refresh token here so we don't get a 401 + err = kc.GetAdminToken(ctx) + g.Expect(err).NotTo(HaveOccurred(), "failed to refresh Keycloak admin token") + + // Test 3: Invalid - empty email (violates rule 1) + // Demonstrates rule 1 requirement: has(claims.email) && claims.email != '' + t.Logf("Test 3: Creating user with empty email (violates rule 1)") + emptyEmailUsername := "claim-empty-email-" + e2eutil.GenerateRandomPassword(8) + emptyEmailPassword := e2eutil.GenerateRandomPassword(16) + emptyEmailUserID, err := testResources.CreateTestUserWithEmailVerification(ctx, t, emptyEmailUsername, "", emptyEmailPassword, true) + g.Expect(err).NotTo(HaveOccurred(), "creating user in Keycloak should succeed") + t.Logf("✓ Created user '%s' with email='' and email_verified=true (ID: %s)", emptyEmailUsername, emptyEmailUserID) + + // Attempt to authenticate - should FAIL due to claim validation rule 1 + err = testResources.TryAuthenticateUser(ctx, t, emptyEmailUsername, emptyEmailPassword, clientCfg, clusterOpts.ExtOIDCConfig) + g.Expect(err).To(HaveOccurred(), "authentication must fail when email is empty") + g.Expect(err.Error()).Should(ContainSubstring("Unauthorized"), + "empty email user cannot authenticate as it violates user validation rule") + t.Logf("✓ User with empty email correctly rejected: %v", err) + + t.Logf("Claim validation rules successfully validated: only users with non-empty email and email_verified=true can authenticate") }) t.Run("[OCPFeatureGate:ExternalOIDCWithUpstreamParity] Test user validation rules", func(t *testing.T) { g := NewWithT(t) t.Logf("begin to test user validation rules") - g.Expect(hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].UserValidationRules).NotTo(BeEmpty()) - userRules := hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].UserValidationRules - g.Expect(userRules).Should(HaveLen(1)) - // Verify configuration - rule should use expected CEL expression - g.Expect(userRules[0].Expression).Should(Equal(e2eutil.UserValidationExprNoSystemPrefix)) + // Refresh admin token before creating test resources + err := kc.GetAdminToken(ctx) + g.Expect(err).NotTo(HaveOccurred(), "failed to refresh Keycloak admin token") + + // Setup: Create test resources with automatic cleanup + testResources := e2eutil.NewTestResources(kc) + defer testResources.Cleanup(ctx, t) - // Verify behavior - authentication succeeded, proving user validation rule passed - // Rule validates username doesn't start with 'system:' - g.Expect(selfSubjectReview.Status.UserInfo.Username).NotTo(HavePrefix("system:"), "username should not have system: prefix (validated by user rule)") - t.Logf("User validation rules successfully validated user: %s", selfSubjectReview.Status.UserInfo.Username) + // Verify: User validation rules are configured + g.Expect(hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].UserValidationRules).NotTo(BeEmpty()) + userRules := hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].UserValidationRules + g.Expect(userRules).Should(HaveLen(2), "should have two user validation rules") + + // Verify: Rules use expected CEL expressions + expressions := []string{userRules[0].Expression, userRules[1].Expression} + g.Expect(expressions).Should(ContainElement(e2eutil.UserValidationExprNoSystemPrefix)) + g.Expect(expressions).Should(ContainElement(e2eutil.UserValidationExprNoForbiddenWord)) + t.Logf("User validation rules configured: %v", expressions) + + // Test 1: Valid user - passes all validation rules + // Should PASS validation and authenticate successfully + t.Logf("Test 1: Creating user with valid username (no system: prefix, no 'forbidden' word)") + validUser, err := testResources.SetupAuthenticatedUserWithGroup(ctx, t, "user-valid", "user-valid-group", clientCfg, clusterOpts.ExtOIDCConfig) + g.Expect(err).NotTo(HaveOccurred(), "authentication must succeed when all validation rules pass") + + // Username is derived from email via CEL: claims.email.split('@')[0] + expectedUsername := strings.Split(validUser.Email, "@")[0] + g.Expect(validUser.SelfSubjectReview.Status.UserInfo.Username).Should(Equal(expectedUsername)) + g.Expect(validUser.SelfSubjectReview.Status.UserInfo.Username).NotTo(HavePrefix("system:")) + g.Expect(validUser.SelfSubjectReview.Status.UserInfo.Username).NotTo(ContainSubstring("forbidden")) + t.Logf("✓ User with username='%s' authenticated successfully", validUser.SelfSubjectReview.Status.UserInfo.Username) + + // Test 2: Invalid user - username contains "forbidden" + // Demonstrates the testable user validation rule: !user.username.contains('forbidden') + t.Logf("Test 2: Creating user with 'forbidden' in username (violates user validation rule)") + forbiddenUsername := "user-forbidden-" + e2eutil.GenerateRandomPassword(8) + forbiddenEmail := forbiddenUsername + "@test.example.com" + forbiddenPassword := e2eutil.GenerateRandomPassword(16) + forbiddenUserID, err := testResources.CreateTestUser(ctx, t, forbiddenUsername, forbiddenEmail, forbiddenPassword) + g.Expect(err).NotTo(HaveOccurred(), "creating user in Keycloak should succeed") + t.Logf("Created user with email='%s', mapped username will be '%s' (ID: %s)", forbiddenEmail, forbiddenUsername, forbiddenUserID) + + // Try to authenticate - should FAIL due to user validation rule + err = testResources.TryAuthenticateUser(ctx, t, forbiddenUsername, forbiddenPassword, clientCfg, clusterOpts.ExtOIDCConfig) + g.Expect(err).To(HaveOccurred(), "authentication must fail when username contains 'forbidden'") + g.Expect(err.Error()).Should(ContainSubstring("Unauthorized"), + "forbidden user cannot authenticate as it violates user validation rule") + t.Logf("✓ User with 'forbidden' in username correctly rejected with error: %v", err) + + // NOTE: We cannot test the negative case for the system: prefix rule via Keycloak + // because RFC 5322 email addresses do not allow colons in the local part. + // Since username = claims.email.split('@')[0], we would need an email like + // "system:admin@test.example.com", which is invalid per email standards. + // The system: prefix rule should be tested via unit tests or envtest where claims can be mocked. + + t.Logf("User validation rules successfully validated: users with 'forbidden' in username are rejected") }) } }).Execute(&clusterOpts, globalOpts.Platform, globalOpts.ArtifactDir, "external-oidc", globalOpts.ServiceAccountSigningKey) diff --git a/test/e2e/util/external_oidc.go b/test/e2e/util/external_oidc.go index ac7dd1bd7cbe..96bbfdf1f466 100644 --- a/test/e2e/util/external_oidc.go +++ b/test/e2e/util/external_oidc.go @@ -27,6 +27,7 @@ import ( configv1 "github.com/openshift/api/config/v1" configv1typedclient "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1" + appsv1 "k8s.io/api/apps/v1" kauthnv1 "k8s.io/api/authentication/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -54,8 +55,9 @@ const ( ClaimValidationExprEmailExists = "has(claims.email) && claims.email != ''" ClaimValidationExprEmailVerified = "claims.email_verified == true" - // CEL expression for user validation rule - UserValidationExprNoSystemPrefix = "!user.username.startsWith('system:')" + // CEL expressions for user validation rules + UserValidationExprNoSystemPrefix = "!user.username.startsWith('system:')" + UserValidationExprNoForbiddenWord = "!user.username.contains('forbidden')" ) type ExtOIDCConfig struct { @@ -211,6 +213,10 @@ func (config *ExtOIDCConfig) GetAuthenticationConfig() *configv1.AuthenticationS Expression: UserValidationExprNoSystemPrefix, Message: "username cannot use reserved system: prefix", }, + { + Expression: UserValidationExprNoForbiddenWord, + Message: "username cannot contain the word 'forbidden'", + }, } } @@ -397,7 +403,7 @@ func GetClientConfigForKeycloakOIDCUser(clientCfg *rest.Config, authConfig *ExtO "--issuer-url=" + authConfig.IssuerURL, "--client-id=" + authConfig.CliClientID, "--extra-scopes=email,profile", - "--callback-address=127.0.0.1:8080", + "--callback-address=127.0.0.1:0", "--certificate-authority=" + authConfig.IssuerCABundleFile, } @@ -963,6 +969,87 @@ func GenerateRandomPassword(length int) string { 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 +} + // TestResources tracks resources created during a test for cleanup type TestResources struct { AdminClient *KeycloakAdminClient @@ -983,13 +1070,18 @@ func NewTestResources(adminClient *KeycloakAdminClient) *TestResources { // CreateTestUser creates a user and tracks it for cleanup func (tr *TestResources) CreateTestUser(ctx context.Context, t *testing.T, username, email, password string) (string, error) { + return tr.CreateTestUserWithEmailVerification(ctx, t, username, email, password, true) +} + +// CreateTestUserWithEmailVerification creates a user with specific email verification status and tracks it for cleanup +func (tr *TestResources) CreateTestUserWithEmailVerification(ctx context.Context, t *testing.T, username, email, password string, emailVerified bool) (string, error) { user := KeycloakUser{ Username: username, Enabled: true, FirstName: username, LastName: "Test", Email: email, - EmailVerified: true, + EmailVerified: emailVerified, } userID, err := tr.AdminClient.CreateUser(ctx, user) @@ -1007,7 +1099,7 @@ func (tr *TestResources) CreateTestUser(ctx context.Context, t *testing.T, usern tr.UserIDs = append(tr.UserIDs, userID) tr.UserCreds[username] = password - t.Logf("Created test user: %s (ID: %s)", username, userID) + t.Logf("Created test user: %s (ID: %s, email_verified: %v)", username, userID, emailVerified) return userID, nil } @@ -1025,6 +1117,16 @@ func (tr *TestResources) CreateTestGroup(ctx context.Context, t *testing.T, grou return groupID, nil } +// CreateTestUserWithRandomCredentials creates a user with generated credentials and tracks it for cleanup +func (tr *TestResources) CreateTestUserWithRandomCredentials(ctx context.Context, t *testing.T, usernamePrefix string) (userID, username, email, password string, err error) { + username = usernamePrefix + "-" + GenerateRandomPassword(8) + email = username + "@test.example.com" + password = GenerateRandomPassword(16) + + userID, err = tr.CreateTestUser(ctx, t, username, email, password) + return userID, username, email, password, err +} + // GetTestUsersString returns users in format "user1:pass1,user2:pass2,..." func (tr *TestResources) GetTestUsersString() string { var users []string @@ -1038,6 +1140,10 @@ func (tr *TestResources) GetTestUsersString() string { func (tr *TestResources) Cleanup(ctx context.Context, t *testing.T) { t.Logf("Cleaning up test resources: %d users, %d groups", len(tr.UserIDs), len(tr.GroupIDs)) + if err := tr.AdminClient.GetAdminToken(ctx); err != nil { + t.Logf("Warning: failed to refresh admin token: %v", err) + } + // Delete users for _, userID := range tr.UserIDs { if err := tr.AdminClient.DeleteUser(ctx, userID); err != nil { @@ -1061,3 +1167,174 @@ func (tr *TestResources) Cleanup(ctx context.Context, t *testing.T) { tr.GroupIDs = []string{} tr.UserCreds = make(map[string]string) } + +// AuthenticatedTestUser contains the results of setting up an authenticated test user +type AuthenticatedTestUser struct { + Username string + Email string + Password string + UserID string + GroupID string + GroupName string + KubeConfig *rest.Config + AuthClient kauthnv1typedclient.AuthenticationV1Interface + SelfSubjectReview *kauthnv1.SelfSubjectReview +} + +// TryAuthenticateUser attempts to authenticate a user and returns error if authentication fails +// This is useful for negative testing where you expect authentication to fail +func (tr *TestResources) TryAuthenticateUser( + ctx context.Context, + t *testing.T, + username string, + password string, + clientCfg *rest.Config, + extOIDCConfig *ExtOIDCConfig, +) error { + // This function replicates ChangeUserForKeycloakExtOIDC but returns errors + // instead of using gomega assertions, allowing negative testing scenarios + + if extOIDCConfig == nil { + return fmt.Errorf("extOIDCConfig is nil") + } + if extOIDCConfig.ExternalOIDCProvider != ProviderKeycloak { + return fmt.Errorf("expected Keycloak provider, got %s", extOIDCConfig.ExternalOIDCProvider) + } + + // Step 1: Get OIDC token from Keycloak + httpClient := &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: true, + }, + }, + } + + requestURL := extOIDCConfig.IssuerURL + "/protocol/openid-connect/token" + oidcClientID := extOIDCConfig.CliClientID + if oidcClientID == "" { + return fmt.Errorf("oidcClientID is empty") + } + + formData := url.Values{ + "client_id": []string{oidcClientID}, + "grant_type": []string{"password"}, + "password": []string{password}, + "scope": []string{"openid email profile"}, + "username": []string{username}, + } + + response, err := httpClient.PostForm(requestURL, formData) + if err != nil { + return fmt.Errorf("failed to POST to token endpoint: %w", err) + } + defer response.Body.Close() + + // Authentication can fail at Keycloak level (e.g., email_verified=false) + if response.StatusCode != http.StatusOK { + body, _ := io.ReadAll(response.Body) + return fmt.Errorf("keycloak authentication failed with status %d: %s", response.StatusCode, string(body)) + } + + body, err := io.ReadAll(response.Body) + if err != nil { + return fmt.Errorf("failed to read response body: %w", err) + } + + var respMap map[string]any + err = json.Unmarshal(body, &respMap) + if err != nil { + return fmt.Errorf("failed to unmarshal token response: %w", err) + } + + idToken, ok := respMap["id_token"].(string) + if !ok { + return fmt.Errorf("id_token not found or not a string in response") + } + + // Step 2: Try to authenticate with K8s using the ID token directly + // We use the token as a bearer token instead of going through the exec plugin, + // which would attempt an authorization code flow (browser-based) that requires + // user interaction. Using the token directly allows us to capture validation + // errors from the Kubernetes API server. + // Use AnonymousClientConfig to clear all auth credentials (certs, tokens) from the admin config + clientConfigForExtOIDCUser := rest.AnonymousClientConfig(rest.CopyConfig(clientCfg)) + clientConfigForExtOIDCUser.BearerToken = idToken + + authClient, err := kauthnv1typedclient.NewForConfig(clientConfigForExtOIDCUser) + if err != nil { + return fmt.Errorf("failed to create auth client: %w", err) + } + + // Authentication can fail at K8s level due to claim validation rules or user validation rules + _, err = authClient.SelfSubjectReviews().Create(ctx, &kauthnv1.SelfSubjectReview{}, metav1.CreateOptions{}) + if err != nil { + return fmt.Errorf("kubernetes authentication failed: %w", err) + } + + return nil +} + +// SetupAuthenticatedUserWithGroup creates a test user, group, adds user to group, authenticates, and gets self subject review +// This is a complete setup for testing authentication and authorization scenarios +func (tr *TestResources) SetupAuthenticatedUserWithGroup( + ctx context.Context, + t *testing.T, + usernamePrefix string, + groupNamePrefix string, + clientCfg *rest.Config, + extOIDCConfig *ExtOIDCConfig, +) (*AuthenticatedTestUser, error) { + g := NewWithT(t) + + // Create test group + groupName := groupNamePrefix + "-" + GenerateRandomPassword(8) + groupID, err := tr.CreateTestGroup(ctx, t, groupName) + if err != nil { + return nil, fmt.Errorf("failed to create test group: %w", err) + } + t.Logf("Created test group: %s (ID: %s)", groupName, groupID) + + // Create test user with specific email + username := usernamePrefix + "-" + GenerateRandomPassword(8) + email := username + "@test.example.com" + password := GenerateRandomPassword(16) + userID, err := tr.CreateTestUser(ctx, t, username, email, password) + if err != nil { + return nil, fmt.Errorf("failed to create test user: %w", err) + } + t.Logf("Created test user: %s (email: %s, ID: %s)", username, email, userID) + + // Add user to group + err = tr.AdminClient.AddUserToGroup(ctx, userID, groupID) + if err != nil { + return nil, fmt.Errorf("failed to add user to group: %w", err) + } + t.Logf("Added user %s to group %s", username, groupName) + + // Authenticate as test user + testAuthConfig := *extOIDCConfig + testAuthConfig.TestUsers = username + ":" + password + testUserKubeConfig := ChangeUserForKeycloakExtOIDC(t, ctx, clientCfg, &testAuthConfig) + testAuthClient, err := kauthnv1typedclient.NewForConfig(testUserKubeConfig) + g.Expect(err).NotTo(HaveOccurred()) + + // Get self subject review + selfSubjectReview, err := testAuthClient.SelfSubjectReviews().Create(ctx, &kauthnv1.SelfSubjectReview{}, metav1.CreateOptions{}) + if err != nil { + return nil, fmt.Errorf("failed to get self subject review: %w", err) + } + t.Logf("Test user self subject review: %+v", selfSubjectReview.Status.UserInfo) + + return &AuthenticatedTestUser{ + Username: username, + Email: email, + Password: password, + UserID: userID, + GroupID: groupID, + GroupName: groupName, + KubeConfig: testUserKubeConfig, + AuthClient: testAuthClient, + SelfSubjectReview: selfSubjectReview, + }, nil +} From ad6c8feba5da2936da5a3dc87e8d58536416c6d2 Mon Sep 17 00:00:00 2001 From: Evan Hearne Date: Wed, 20 May 2026 17:31:16 +0100 Subject: [PATCH 5/9] test(CNTRLPLANE-3306): migrate keycloak api code to separate file This change moves keycloak api code from `external_oidc.go` to its own file, `keycloak.go` . Also adds check for admin token expiry + refreshes if needed. --- test/e2e/external_oidc_test.go | 24 +- test/e2e/util/external_oidc.go | 916 ----------------------------- test/e2e/util/keycloak.go | 1000 ++++++++++++++++++++++++++++++++ 3 files changed, 1008 insertions(+), 932 deletions(-) create mode 100644 test/e2e/util/keycloak.go diff --git a/test/e2e/external_oidc_test.go b/test/e2e/external_oidc_test.go index 2cf8da0c7773..5af4b0c6cfba 100644 --- a/test/e2e/external_oidc_test.go +++ b/test/e2e/external_oidc_test.go @@ -124,7 +124,8 @@ func TestExternalOIDC(t *testing.T) { t.Logf("begin to test CEL username expression mapping") // Setup: Create test resources with automatic cleanup - testResources := e2eutil.NewTestResources(kc) + testResources, err := e2eutil.NewTestResources(ctx, kc, clusterOpts.ExtOIDCConfig) + g.Expect(err).NotTo(HaveOccurred()) defer testResources.Cleanup(ctx, t) // Setup: Create authenticated test user with group @@ -191,7 +192,8 @@ func TestExternalOIDC(t *testing.T) { t.Logf("begin to test CEL groups expression mapping") // Setup: Create test resources with automatic cleanup - testResources := e2eutil.NewTestResources(kc) + testResources, err := e2eutil.NewTestResources(ctx, kc, clusterOpts.ExtOIDCConfig) + g.Expect(err).NotTo(HaveOccurred()) defer testResources.Cleanup(ctx, t) // Verify: Groups expression is configured @@ -242,12 +244,9 @@ func TestExternalOIDC(t *testing.T) { g := NewWithT(t) t.Logf("begin to test claim validation rules") - // Refresh admin token before creating test resources - err := kc.GetAdminToken(ctx) - g.Expect(err).NotTo(HaveOccurred(), "failed to refresh Keycloak admin token") - // Setup: Create test resources with automatic cleanup - testResources := e2eutil.NewTestResources(kc) + testResources, err := e2eutil.NewTestResources(ctx, kc, clusterOpts.ExtOIDCConfig) + g.Expect(err).NotTo(HaveOccurred()) defer testResources.Cleanup(ctx, t) // Verify: Claim validation rules are configured @@ -285,10 +284,6 @@ func TestExternalOIDC(t *testing.T) { g.Expect(err).To(HaveOccurred(), "authentication must fail when email_verified=false") t.Logf("✓ User with email_verified=false correctly rejected: %v", err) - // refresh token here so we don't get a 401 - err = kc.GetAdminToken(ctx) - g.Expect(err).NotTo(HaveOccurred(), "failed to refresh Keycloak admin token") - // Test 3: Invalid - empty email (violates rule 1) // Demonstrates rule 1 requirement: has(claims.email) && claims.email != '' t.Logf("Test 3: Creating user with empty email (violates rule 1)") @@ -312,12 +307,9 @@ func TestExternalOIDC(t *testing.T) { g := NewWithT(t) t.Logf("begin to test user validation rules") - // Refresh admin token before creating test resources - err := kc.GetAdminToken(ctx) - g.Expect(err).NotTo(HaveOccurred(), "failed to refresh Keycloak admin token") - // Setup: Create test resources with automatic cleanup - testResources := e2eutil.NewTestResources(kc) + testResources, err := e2eutil.NewTestResources(ctx, kc, clusterOpts.ExtOIDCConfig) + g.Expect(err).NotTo(HaveOccurred()) defer testResources.Cleanup(ctx, t) // Verify: User validation rules are configured diff --git a/test/e2e/util/external_oidc.go b/test/e2e/util/external_oidc.go index 96bbfdf1f466..42f9eaff6c7d 100644 --- a/test/e2e/util/external_oidc.go +++ b/test/e2e/util/external_oidc.go @@ -15,7 +15,6 @@ import ( "os" "path/filepath" "regexp" - "strings" "testing" "time" @@ -27,7 +26,6 @@ import ( configv1 "github.com/openshift/api/config/v1" configv1typedclient "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1" - appsv1 "k8s.io/api/apps/v1" kauthnv1 "k8s.io/api/authentication/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -424,917 +422,3 @@ func GetClientConfigForKeycloakOIDCUser(clientCfg *rest.Config, authConfig *ExtO return userClientConfig } - -// KeycloakAdminClient provides methods to interact with Keycloak Admin REST API -type KeycloakAdminClient struct { - BaseURL string - AdminToken string - HTTPClient *http.Client - AdminUser string - AdminPass string - CACertFile 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"` -} - -// 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"` -} - -// KeycloakClient represents a Keycloak client -type KeycloakClient struct { - ID string `json:"id"` - ClientID string `json:"clientId"` -} - -// KeycloakProtocolMapper represents a protocol mapper -type KeycloakProtocolMapper struct { - Name string `json:"name"` - Protocol string `json:"protocol"` - ProtocolMapper string `json:"protocolMapper"` - ConsentRequired bool `json:"consentRequired"` - Config map[string]string `json:"config"` -} - -// NewKeycloakAdminClient creates a new Keycloak admin client -func NewKeycloakAdminClient(baseURL, adminUser, adminPass, caCertFile string) *KeycloakAdminClient { - return &KeycloakAdminClient{ - BaseURL: baseURL, - AdminUser: adminUser, - AdminPass: adminPass, - CACertFile: caCertFile, - 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, "POST", 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, "POST", 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, "POST", 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 -} - -// SetUserPassword sets a user's password -func (kc *KeycloakAdminClient) SetUserPassword(ctx context.Context, userID, password string, temporary bool) error { - passwordURL := fmt.Sprintf("%s/admin/realms/master/users/%s/reset-password", kc.BaseURL, userID) - - credential := KeycloakCredential{ - Type: "password", - Value: password, - Temporary: temporary, - } - - credJSON, err := json.Marshal(credential) - if err != nil { - return fmt.Errorf("failed to marshal credential: %w", err) - } - - req, err := http.NewRequestWithContext(ctx, "PUT", passwordURL, strings.NewReader(string(credJSON))) - if err != nil { - return fmt.Errorf("failed to create password 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 set password: %w", err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusNoContent { - body, _ := io.ReadAll(resp.Body) - return fmt.Errorf("failed to set password, status: %d, body: %s", resp.StatusCode, string(body)) - } - - return nil -} - -// AddUserToGroup adds a user to a group -func (kc *KeycloakAdminClient) AddUserToGroup(ctx context.Context, userID, groupID string) error { - groupURL := fmt.Sprintf("%s/admin/realms/master/users/%s/groups/%s", kc.BaseURL, userID, groupID) - - req, err := http.NewRequestWithContext(ctx, "PUT", groupURL, nil) - if err != nil { - return fmt.Errorf("failed to create add-to-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 add user to group: %w", err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusNoContent { - body, _ := io.ReadAll(resp.Body) - return fmt.Errorf("failed to add user to group, status: %d, body: %s", resp.StatusCode, string(body)) - } - - return nil -} - -// GetClientByClientID retrieves a client's internal ID by its clientId -func (kc *KeycloakAdminClient) GetClientByClientID(ctx context.Context, clientID string) (string, error) { - clientsURL := fmt.Sprintf("%s/admin/realms/master/clients?clientId=%s", kc.BaseURL, url.QueryEscape(clientID)) - - req, err := http.NewRequestWithContext(ctx, "GET", clientsURL, nil) - if err != nil { - return "", fmt.Errorf("failed to create get-client request: %w", err) - } - req.Header.Set("Authorization", "Bearer "+kc.AdminToken) - - resp, err := kc.HTTPClient.Do(req) - if err != nil { - return "", fmt.Errorf("failed to get client: %w", err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - body, _ := io.ReadAll(resp.Body) - return "", fmt.Errorf("failed to get client, status: %d, body: %s", resp.StatusCode, string(body)) - } - - var clients []KeycloakClient - if err := json.NewDecoder(resp.Body).Decode(&clients); err != nil { - return "", fmt.Errorf("failed to decode clients response: %w", err) - } - - if len(clients) == 0 { - return "", fmt.Errorf("client not found: %s", clientID) - } - - return clients[0].ID, nil -} - -// DeleteUser deletes a user from Keycloak -func (kc *KeycloakAdminClient) DeleteUser(ctx context.Context, userID string) error { - userURL := fmt.Sprintf("%s/admin/realms/master/users/%s", kc.BaseURL, userID) - - req, err := http.NewRequestWithContext(ctx, "DELETE", userURL, nil) - if err != nil { - return fmt.Errorf("failed to create delete-user request: %w", err) - } - req.Header.Set("Authorization", "Bearer "+kc.AdminToken) - - resp, err := kc.HTTPClient.Do(req) - if err != nil { - return fmt.Errorf("failed to delete user: %w", err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusNoContent { - body, _ := io.ReadAll(resp.Body) - return fmt.Errorf("failed to delete user, status: %d, body: %s", resp.StatusCode, string(body)) - } - - return nil -} - -// DeleteGroup deletes a group from Keycloak -func (kc *KeycloakAdminClient) DeleteGroup(ctx context.Context, groupID string) error { - groupURL := fmt.Sprintf("%s/admin/realms/master/groups/%s", kc.BaseURL, groupID) - - req, err := http.NewRequestWithContext(ctx, "DELETE", groupURL, nil) - if err != nil { - return fmt.Errorf("failed to create delete-group request: %w", err) - } - req.Header.Set("Authorization", "Bearer "+kc.AdminToken) - - resp, err := kc.HTTPClient.Do(req) - if err != nil { - return fmt.Errorf("failed to delete group: %w", err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusNoContent { - body, _ := io.ReadAll(resp.Body) - return fmt.Errorf("failed to delete group, status: %d, body: %s", resp.StatusCode, string(body)) - } - - return nil -} - -// GetUserByUsername retrieves a user ID by username -func (kc *KeycloakAdminClient) GetUserByUsername(ctx context.Context, username string) (string, error) { - usersURL := fmt.Sprintf("%s/admin/realms/master/users?username=%s&exact=true", kc.BaseURL, url.QueryEscape(username)) - - req, err := http.NewRequestWithContext(ctx, "GET", usersURL, nil) - if err != nil { - return "", fmt.Errorf("failed to create get-user request: %w", err) - } - req.Header.Set("Authorization", "Bearer "+kc.AdminToken) - - resp, err := kc.HTTPClient.Do(req) - if err != nil { - return "", fmt.Errorf("failed to get user: %w", err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - body, _ := io.ReadAll(resp.Body) - return "", fmt.Errorf("failed to get user, status: %d, body: %s", resp.StatusCode, string(body)) - } - - var users []struct { - ID string `json:"id"` - Username string `json:"username"` - } - if err := json.NewDecoder(resp.Body).Decode(&users); err != nil { - return "", fmt.Errorf("failed to decode users response: %w", err) - } - - if len(users) == 0 { - return "", fmt.Errorf("user not found: %s", username) - } - - return users[0].ID, nil -} - -// GetGroupByName retrieves a group ID by name -func (kc *KeycloakAdminClient) GetGroupByName(ctx context.Context, groupName string) (string, error) { - groupsURL := fmt.Sprintf("%s/admin/realms/master/groups", kc.BaseURL) - - req, err := http.NewRequestWithContext(ctx, "GET", groupsURL, nil) - if err != nil { - return "", fmt.Errorf("failed to create get-groups request: %w", err) - } - req.Header.Set("Authorization", "Bearer "+kc.AdminToken) - - resp, err := kc.HTTPClient.Do(req) - if err != nil { - return "", fmt.Errorf("failed to get groups: %w", err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - body, _ := io.ReadAll(resp.Body) - return "", fmt.Errorf("failed to get groups, status: %d, body: %s", resp.StatusCode, string(body)) - } - - var groups []struct { - ID string `json:"id"` - Name string `json:"name"` - } - if err := json.NewDecoder(resp.Body).Decode(&groups); err != nil { - return "", fmt.Errorf("failed to decode groups response: %w", err) - } - - for _, group := range groups { - if group.Name == groupName { - return group.ID, nil - } - } - - return "", fmt.Errorf("group not found: %s", groupName) -} - -// CreateProtocolMapper creates a protocol mapper for a client -func (kc *KeycloakAdminClient) CreateProtocolMapper(ctx context.Context, clientID string, mapper KeycloakProtocolMapper) error { - mapperURL := fmt.Sprintf("%s/admin/realms/master/clients/%s/protocol-mappers/models", kc.BaseURL, clientID) - - mapperJSON, err := json.Marshal(mapper) - if err != nil { - return fmt.Errorf("failed to marshal mapper: %w", err) - } - - req, err := http.NewRequestWithContext(ctx, "POST", mapperURL, strings.NewReader(string(mapperJSON))) - if err != nil { - return fmt.Errorf("failed to create mapper 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 mapper: %w", err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusCreated { - body, _ := io.ReadAll(resp.Body) - return fmt.Errorf("failed to create mapper, status: %d, body: %s", resp.StatusCode, string(body)) - } - - return nil -} - -// SetupKeycloakTestEnvironment creates test users, groups, and protocol mappers -func SetupKeycloakTestEnvironment(t *testing.T, ctx context.Context, config *ExtOIDCConfig, adminUser, adminPass string, numUsers int) (string, error) { - g := NewWithT(t) - - // Create admin client - kc := NewKeycloakAdminClient(config.IssuerURL, adminUser, adminPass, config.IssuerCABundleFile) - - // Get admin token - err := kc.GetAdminToken(ctx) - if err != nil { - return "", fmt.Errorf("failed to get admin token: %w", err) - } - - // Create group - t.Logf("Creating Keycloak group: keycloak-testgroup-1") - groupID, err := kc.CreateGroup(ctx, "keycloak-testgroup-1") - if err != nil { - return "", fmt.Errorf("failed to create group: %w", err) - } - t.Logf("Created group with ID: %s", groupID) - - // Create users and add to group - var users []string - for i := 1; i <= numUsers; i++ { - username := fmt.Sprintf("keycloak-testuser-%d", i) - password := GenerateRandomPassword(12) - - user := KeycloakUser{ - Username: username, - Enabled: true, - FirstName: username, - LastName: "KC", - Email: fmt.Sprintf("%s@example.com", username), - EmailVerified: true, - } - - t.Logf("Creating user: %s", username) - userID, err := kc.CreateUser(ctx, user) - if err != nil { - return "", fmt.Errorf("failed to create user %s: %w", username, err) - } - - // Set password - err = kc.SetUserPassword(ctx, userID, password, false) - if err != nil { - return "", fmt.Errorf("failed to set password for user %s: %w", username, err) - } - - // Add to group - err = kc.AddUserToGroup(ctx, userID, groupID) - if err != nil { - return "", fmt.Errorf("failed to add user %s to group: %w", username, err) - } - - users = append(users, fmt.Sprintf("%s:%s", username, password)) - } - - // Create protocol mappers for clients - groupMapper := KeycloakProtocolMapper{ - Name: "groupmapper", - Protocol: "openid-connect", - ProtocolMapper: "oidc-group-membership-mapper", - ConsentRequired: false, - Config: map[string]string{ - "full.path": "false", - "userinfo.token.claim": "true", - "id.token.claim": "true", - "access.token.claim": "false", - "claim.name": "groups", - }, - } - - // Add group mapper to CLI client - t.Logf("Adding group mapper to CLI client: %s", config.CliClientID) - cliClientID, err := kc.GetClientByClientID(ctx, config.CliClientID) - if err != nil { - return "", fmt.Errorf("failed to get CLI client ID: %w", err) - } - err = kc.CreateProtocolMapper(ctx, cliClientID, groupMapper) - g.Expect(err).NotTo(HaveOccurred(), "failed to create protocol mapper for CLI client") - - // Add group mapper to console client - t.Logf("Adding group mapper to console client: %s", config.ConsoleClientID) - consoleClientID, err := kc.GetClientByClientID(ctx, config.ConsoleClientID) - if err != nil { - return "", fmt.Errorf("failed to get console client ID: %w", err) - } - err = kc.CreateProtocolMapper(ctx, consoleClientID, groupMapper) - g.Expect(err).NotTo(HaveOccurred(), "failed to create protocol mapper for console client") - - // Return users in format "user1:pass1,user2:pass2,..." - return strings.Join(users, ","), 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 -} - -// TestResources tracks resources created during a test for cleanup -type TestResources struct { - AdminClient *KeycloakAdminClient - UserIDs []string - GroupIDs []string - UserCreds map[string]string // username -> password -} - -// NewTestResources creates a new TestResources tracker -func NewTestResources(adminClient *KeycloakAdminClient) *TestResources { - return &TestResources{ - AdminClient: adminClient, - UserIDs: []string{}, - GroupIDs: []string{}, - UserCreds: make(map[string]string), - } -} - -// CreateTestUser creates a user and tracks it for cleanup -func (tr *TestResources) CreateTestUser(ctx context.Context, t *testing.T, username, email, password string) (string, error) { - return tr.CreateTestUserWithEmailVerification(ctx, t, username, email, password, true) -} - -// CreateTestUserWithEmailVerification creates a user with specific email verification status and tracks it for cleanup -func (tr *TestResources) CreateTestUserWithEmailVerification(ctx context.Context, t *testing.T, username, email, password string, emailVerified bool) (string, error) { - user := KeycloakUser{ - Username: username, - Enabled: true, - FirstName: username, - LastName: "Test", - Email: email, - EmailVerified: emailVerified, - } - - userID, err := tr.AdminClient.CreateUser(ctx, user) - if err != nil { - return "", err - } - - // Set password - err = tr.AdminClient.SetUserPassword(ctx, userID, password, false) - if err != nil { - return "", err - } - - // Track for cleanup - tr.UserIDs = append(tr.UserIDs, userID) - tr.UserCreds[username] = password - - t.Logf("Created test user: %s (ID: %s, email_verified: %v)", username, userID, emailVerified) - return userID, nil -} - -// CreateTestGroup creates a group and tracks it for cleanup -func (tr *TestResources) CreateTestGroup(ctx context.Context, t *testing.T, groupName string) (string, error) { - groupID, err := tr.AdminClient.CreateGroup(ctx, groupName) - if err != nil { - return "", err - } - - // Track for cleanup - tr.GroupIDs = append(tr.GroupIDs, groupID) - - t.Logf("Created test group: %s (ID: %s)", groupName, groupID) - return groupID, nil -} - -// CreateTestUserWithRandomCredentials creates a user with generated credentials and tracks it for cleanup -func (tr *TestResources) CreateTestUserWithRandomCredentials(ctx context.Context, t *testing.T, usernamePrefix string) (userID, username, email, password string, err error) { - username = usernamePrefix + "-" + GenerateRandomPassword(8) - email = username + "@test.example.com" - password = GenerateRandomPassword(16) - - userID, err = tr.CreateTestUser(ctx, t, username, email, password) - return userID, username, email, password, err -} - -// GetTestUsersString returns users in format "user1:pass1,user2:pass2,..." -func (tr *TestResources) GetTestUsersString() string { - var users []string - for username, password := range tr.UserCreds { - users = append(users, fmt.Sprintf("%s:%s", username, password)) - } - return strings.Join(users, ",") -} - -// Cleanup deletes all tracked resources -func (tr *TestResources) Cleanup(ctx context.Context, t *testing.T) { - t.Logf("Cleaning up test resources: %d users, %d groups", len(tr.UserIDs), len(tr.GroupIDs)) - - if err := tr.AdminClient.GetAdminToken(ctx); err != nil { - t.Logf("Warning: failed to refresh admin token: %v", err) - } - - // Delete users - for _, userID := range tr.UserIDs { - if err := tr.AdminClient.DeleteUser(ctx, userID); err != nil { - t.Logf("Warning: failed to delete user %s: %v", userID, err) - } else { - t.Logf("Deleted user: %s", userID) - } - } - - // Delete groups - for _, groupID := range tr.GroupIDs { - if err := tr.AdminClient.DeleteGroup(ctx, groupID); err != nil { - t.Logf("Warning: failed to delete group %s: %v", groupID, err) - } else { - t.Logf("Deleted group: %s", groupID) - } - } - - // Clear tracking - tr.UserIDs = []string{} - tr.GroupIDs = []string{} - tr.UserCreds = make(map[string]string) -} - -// AuthenticatedTestUser contains the results of setting up an authenticated test user -type AuthenticatedTestUser struct { - Username string - Email string - Password string - UserID string - GroupID string - GroupName string - KubeConfig *rest.Config - AuthClient kauthnv1typedclient.AuthenticationV1Interface - SelfSubjectReview *kauthnv1.SelfSubjectReview -} - -// TryAuthenticateUser attempts to authenticate a user and returns error if authentication fails -// This is useful for negative testing where you expect authentication to fail -func (tr *TestResources) TryAuthenticateUser( - ctx context.Context, - t *testing.T, - username string, - password string, - clientCfg *rest.Config, - extOIDCConfig *ExtOIDCConfig, -) error { - // This function replicates ChangeUserForKeycloakExtOIDC but returns errors - // instead of using gomega assertions, allowing negative testing scenarios - - if extOIDCConfig == nil { - return fmt.Errorf("extOIDCConfig is nil") - } - if extOIDCConfig.ExternalOIDCProvider != ProviderKeycloak { - return fmt.Errorf("expected Keycloak provider, got %s", extOIDCConfig.ExternalOIDCProvider) - } - - // Step 1: Get OIDC token from Keycloak - httpClient := &http.Client{ - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{ - InsecureSkipVerify: true, - }, - }, - } - - requestURL := extOIDCConfig.IssuerURL + "/protocol/openid-connect/token" - oidcClientID := extOIDCConfig.CliClientID - if oidcClientID == "" { - return fmt.Errorf("oidcClientID is empty") - } - - formData := url.Values{ - "client_id": []string{oidcClientID}, - "grant_type": []string{"password"}, - "password": []string{password}, - "scope": []string{"openid email profile"}, - "username": []string{username}, - } - - response, err := httpClient.PostForm(requestURL, formData) - if err != nil { - return fmt.Errorf("failed to POST to token endpoint: %w", err) - } - defer response.Body.Close() - - // Authentication can fail at Keycloak level (e.g., email_verified=false) - if response.StatusCode != http.StatusOK { - body, _ := io.ReadAll(response.Body) - return fmt.Errorf("keycloak authentication failed with status %d: %s", response.StatusCode, string(body)) - } - - body, err := io.ReadAll(response.Body) - if err != nil { - return fmt.Errorf("failed to read response body: %w", err) - } - - var respMap map[string]any - err = json.Unmarshal(body, &respMap) - if err != nil { - return fmt.Errorf("failed to unmarshal token response: %w", err) - } - - idToken, ok := respMap["id_token"].(string) - if !ok { - return fmt.Errorf("id_token not found or not a string in response") - } - - // Step 2: Try to authenticate with K8s using the ID token directly - // We use the token as a bearer token instead of going through the exec plugin, - // which would attempt an authorization code flow (browser-based) that requires - // user interaction. Using the token directly allows us to capture validation - // errors from the Kubernetes API server. - // Use AnonymousClientConfig to clear all auth credentials (certs, tokens) from the admin config - clientConfigForExtOIDCUser := rest.AnonymousClientConfig(rest.CopyConfig(clientCfg)) - clientConfigForExtOIDCUser.BearerToken = idToken - - authClient, err := kauthnv1typedclient.NewForConfig(clientConfigForExtOIDCUser) - if err != nil { - return fmt.Errorf("failed to create auth client: %w", err) - } - - // Authentication can fail at K8s level due to claim validation rules or user validation rules - _, err = authClient.SelfSubjectReviews().Create(ctx, &kauthnv1.SelfSubjectReview{}, metav1.CreateOptions{}) - if err != nil { - return fmt.Errorf("kubernetes authentication failed: %w", err) - } - - return nil -} - -// SetupAuthenticatedUserWithGroup creates a test user, group, adds user to group, authenticates, and gets self subject review -// This is a complete setup for testing authentication and authorization scenarios -func (tr *TestResources) SetupAuthenticatedUserWithGroup( - ctx context.Context, - t *testing.T, - usernamePrefix string, - groupNamePrefix string, - clientCfg *rest.Config, - extOIDCConfig *ExtOIDCConfig, -) (*AuthenticatedTestUser, error) { - g := NewWithT(t) - - // Create test group - groupName := groupNamePrefix + "-" + GenerateRandomPassword(8) - groupID, err := tr.CreateTestGroup(ctx, t, groupName) - if err != nil { - return nil, fmt.Errorf("failed to create test group: %w", err) - } - t.Logf("Created test group: %s (ID: %s)", groupName, groupID) - - // Create test user with specific email - username := usernamePrefix + "-" + GenerateRandomPassword(8) - email := username + "@test.example.com" - password := GenerateRandomPassword(16) - userID, err := tr.CreateTestUser(ctx, t, username, email, password) - if err != nil { - return nil, fmt.Errorf("failed to create test user: %w", err) - } - t.Logf("Created test user: %s (email: %s, ID: %s)", username, email, userID) - - // Add user to group - err = tr.AdminClient.AddUserToGroup(ctx, userID, groupID) - if err != nil { - return nil, fmt.Errorf("failed to add user to group: %w", err) - } - t.Logf("Added user %s to group %s", username, groupName) - - // Authenticate as test user - testAuthConfig := *extOIDCConfig - testAuthConfig.TestUsers = username + ":" + password - testUserKubeConfig := ChangeUserForKeycloakExtOIDC(t, ctx, clientCfg, &testAuthConfig) - testAuthClient, err := kauthnv1typedclient.NewForConfig(testUserKubeConfig) - g.Expect(err).NotTo(HaveOccurred()) - - // Get self subject review - selfSubjectReview, err := testAuthClient.SelfSubjectReviews().Create(ctx, &kauthnv1.SelfSubjectReview{}, metav1.CreateOptions{}) - if err != nil { - return nil, fmt.Errorf("failed to get self subject review: %w", err) - } - t.Logf("Test user self subject review: %+v", selfSubjectReview.Status.UserInfo) - - return &AuthenticatedTestUser{ - Username: username, - Email: email, - Password: password, - UserID: userID, - GroupID: groupID, - GroupName: groupName, - KubeConfig: testUserKubeConfig, - AuthClient: testAuthClient, - SelfSubjectReview: selfSubjectReview, - }, nil -} diff --git a/test/e2e/util/keycloak.go b/test/e2e/util/keycloak.go new file mode 100644 index 000000000000..2e1fbba040e7 --- /dev/null +++ b/test/e2e/util/keycloak.go @@ -0,0 +1,1000 @@ +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" + 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" + 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"` +} + +// 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"` +} + +// KeycloakClient represents a Keycloak client +type KeycloakClient struct { + ID string `json:"id"` + ClientID string `json:"clientId"` +} + +// KeycloakProtocolMapper represents a protocol mapper +type KeycloakProtocolMapper struct { + Name string `json:"name"` + Protocol string `json:"protocol"` + ProtocolMapper string `json:"protocolMapper"` + ConsentRequired bool `json:"consentRequired"` + Config map[string]string `json:"config"` +} + +// 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, "POST", 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, "POST", 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, "POST", 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 +} + +// SetUserPassword sets a user's password +func (kc *KeycloakAdminClient) SetUserPassword(ctx context.Context, userID, password string, temporary bool) error { + passwordURL := fmt.Sprintf("%s/admin/realms/master/users/%s/reset-password", kc.BaseURL, userID) + + credential := KeycloakCredential{ + Type: "password", + Value: password, + Temporary: temporary, + } + + credJSON, err := json.Marshal(credential) + if err != nil { + return fmt.Errorf("failed to marshal credential: %w", err) + } + + req, err := http.NewRequestWithContext(ctx, "PUT", passwordURL, strings.NewReader(string(credJSON))) + if err != nil { + return fmt.Errorf("failed to create password 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 set password: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusNoContent { + body, _ := io.ReadAll(resp.Body) + return fmt.Errorf("failed to set password, status: %d, body: %s", resp.StatusCode, string(body)) + } + + return nil +} + +// AddUserToGroup adds a user to a group +func (kc *KeycloakAdminClient) AddUserToGroup(ctx context.Context, userID, groupID string) error { + groupURL := fmt.Sprintf("%s/admin/realms/master/users/%s/groups/%s", kc.BaseURL, userID, groupID) + + req, err := http.NewRequestWithContext(ctx, "PUT", groupURL, nil) + if err != nil { + return fmt.Errorf("failed to create add-to-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 add user to group: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusNoContent { + body, _ := io.ReadAll(resp.Body) + return fmt.Errorf("failed to add user to group, status: %d, body: %s", resp.StatusCode, string(body)) + } + + return nil +} + +// GetClientByClientID retrieves a client's internal ID by its clientId +func (kc *KeycloakAdminClient) GetClientByClientID(ctx context.Context, clientID string) (string, error) { + clientsURL := fmt.Sprintf("%s/admin/realms/master/clients?clientId=%s", kc.BaseURL, url.QueryEscape(clientID)) + + req, err := http.NewRequestWithContext(ctx, "GET", clientsURL, nil) + if err != nil { + return "", fmt.Errorf("failed to create get-client request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+kc.AdminToken) + + resp, err := kc.HTTPClient.Do(req) + if err != nil { + return "", fmt.Errorf("failed to get client: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + body, _ := io.ReadAll(resp.Body) + return "", fmt.Errorf("failed to get client, status: %d, body: %s", resp.StatusCode, string(body)) + } + + var clients []KeycloakClient + if err := json.NewDecoder(resp.Body).Decode(&clients); err != nil { + return "", fmt.Errorf("failed to decode clients response: %w", err) + } + + if len(clients) == 0 { + return "", fmt.Errorf("client not found: %s", clientID) + } + + return clients[0].ID, nil +} + +// DeleteUser deletes a user from Keycloak +func (kc *KeycloakAdminClient) DeleteUser(ctx context.Context, userID string) error { + userURL := fmt.Sprintf("%s/admin/realms/master/users/%s", kc.BaseURL, userID) + + req, err := http.NewRequestWithContext(ctx, "DELETE", userURL, nil) + if err != nil { + return fmt.Errorf("failed to create delete-user request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+kc.AdminToken) + + resp, err := kc.HTTPClient.Do(req) + if err != nil { + return fmt.Errorf("failed to delete user: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusNoContent { + body, _ := io.ReadAll(resp.Body) + return fmt.Errorf("failed to delete user, status: %d, body: %s", resp.StatusCode, string(body)) + } + + return nil +} + +// DeleteGroup deletes a group from Keycloak +func (kc *KeycloakAdminClient) DeleteGroup(ctx context.Context, groupID string) error { + groupURL := fmt.Sprintf("%s/admin/realms/master/groups/%s", kc.BaseURL, groupID) + + req, err := http.NewRequestWithContext(ctx, "DELETE", groupURL, nil) + if err != nil { + return fmt.Errorf("failed to create delete-group request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+kc.AdminToken) + + resp, err := kc.HTTPClient.Do(req) + if err != nil { + return fmt.Errorf("failed to delete group: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusNoContent { + body, _ := io.ReadAll(resp.Body) + return fmt.Errorf("failed to delete group, status: %d, body: %s", resp.StatusCode, string(body)) + } + + return nil +} + +// GetUserByUsername retrieves a user ID by username +func (kc *KeycloakAdminClient) GetUserByUsername(ctx context.Context, username string) (string, error) { + usersURL := fmt.Sprintf("%s/admin/realms/master/users?username=%s&exact=true", kc.BaseURL, url.QueryEscape(username)) + + req, err := http.NewRequestWithContext(ctx, "GET", usersURL, nil) + if err != nil { + return "", fmt.Errorf("failed to create get-user request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+kc.AdminToken) + + resp, err := kc.HTTPClient.Do(req) + if err != nil { + return "", fmt.Errorf("failed to get user: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + body, _ := io.ReadAll(resp.Body) + return "", fmt.Errorf("failed to get user, status: %d, body: %s", resp.StatusCode, string(body)) + } + + var users []struct { + ID string `json:"id"` + Username string `json:"username"` + } + if err := json.NewDecoder(resp.Body).Decode(&users); err != nil { + return "", fmt.Errorf("failed to decode users response: %w", err) + } + + if len(users) == 0 { + return "", fmt.Errorf("user not found: %s", username) + } + + return users[0].ID, nil +} + +// GetGroupByName retrieves a group ID by name +func (kc *KeycloakAdminClient) GetGroupByName(ctx context.Context, groupName string) (string, error) { + groupsURL := fmt.Sprintf("%s/admin/realms/master/groups", kc.BaseURL) + + req, err := http.NewRequestWithContext(ctx, "GET", groupsURL, nil) + if err != nil { + return "", fmt.Errorf("failed to create get-groups request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+kc.AdminToken) + + resp, err := kc.HTTPClient.Do(req) + if err != nil { + return "", fmt.Errorf("failed to get groups: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + body, _ := io.ReadAll(resp.Body) + return "", fmt.Errorf("failed to get groups, status: %d, body: %s", resp.StatusCode, string(body)) + } + + var groups []struct { + ID string `json:"id"` + Name string `json:"name"` + } + if err := json.NewDecoder(resp.Body).Decode(&groups); err != nil { + return "", fmt.Errorf("failed to decode groups response: %w", err) + } + + for _, group := range groups { + if group.Name == groupName { + return group.ID, nil + } + } + + return "", fmt.Errorf("group not found: %s", groupName) +} + +// CreateProtocolMapper creates a protocol mapper for a client +func (kc *KeycloakAdminClient) CreateProtocolMapper(ctx context.Context, clientID string, mapper KeycloakProtocolMapper) error { + mapperURL := fmt.Sprintf("%s/admin/realms/master/clients/%s/protocol-mappers/models", kc.BaseURL, clientID) + + mapperJSON, err := json.Marshal(mapper) + if err != nil { + return fmt.Errorf("failed to marshal mapper: %w", err) + } + + req, err := http.NewRequestWithContext(ctx, "POST", mapperURL, strings.NewReader(string(mapperJSON))) + if err != nil { + return fmt.Errorf("failed to create mapper 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 mapper: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusCreated { + body, _ := io.ReadAll(resp.Body) + return fmt.Errorf("failed to create mapper, status: %d, body: %s", resp.StatusCode, string(body)) + } + + return nil +} + +// SetupKeycloakTestEnvironment creates test users, groups, and protocol mappers +func SetupKeycloakTestEnvironment(t *testing.T, ctx context.Context, config *ExtOIDCConfig, adminUser, adminPass string, numUsers int) (string, error) { + g := NewWithT(t) + + // Create admin client + kc := NewKeycloakAdminClient(config.IssuerURL, adminUser, adminPass, config.IssuerCABundleFile) + + // Get admin token + err := kc.GetAdminToken(ctx) + if err != nil { + return "", fmt.Errorf("failed to get admin token: %w", err) + } + + // Create group + t.Logf("Creating Keycloak group: keycloak-testgroup-1") + groupID, err := kc.CreateGroup(ctx, "keycloak-testgroup-1") + if err != nil { + return "", fmt.Errorf("failed to create group: %w", err) + } + t.Logf("Created group with ID: %s", groupID) + + // Create users and add to group + var users []string + for i := 1; i <= numUsers; i++ { + username := fmt.Sprintf("keycloak-testuser-%d", i) + password := GenerateRandomPassword(12) + + user := KeycloakUser{ + Username: username, + Enabled: true, + FirstName: username, + LastName: "KC", + Email: fmt.Sprintf("%s@example.com", username), + EmailVerified: true, + } + + t.Logf("Creating user: %s", username) + userID, err := kc.CreateUser(ctx, user) + if err != nil { + return "", fmt.Errorf("failed to create user %s: %w", username, err) + } + + // Set password + err = kc.SetUserPassword(ctx, userID, password, false) + if err != nil { + return "", fmt.Errorf("failed to set password for user %s: %w", username, err) + } + + // Add to group + err = kc.AddUserToGroup(ctx, userID, groupID) + if err != nil { + return "", fmt.Errorf("failed to add user %s to group: %w", username, err) + } + + users = append(users, fmt.Sprintf("%s:%s", username, password)) + } + + // Create protocol mappers for clients + groupMapper := KeycloakProtocolMapper{ + Name: "groupmapper", + Protocol: "openid-connect", + ProtocolMapper: "oidc-group-membership-mapper", + ConsentRequired: false, + Config: map[string]string{ + "full.path": "false", + "userinfo.token.claim": "true", + "id.token.claim": "true", + "access.token.claim": "false", + "claim.name": "groups", + }, + } + + // Add group mapper to CLI client + t.Logf("Adding group mapper to CLI client: %s", config.CliClientID) + cliClientID, err := kc.GetClientByClientID(ctx, config.CliClientID) + if err != nil { + return "", fmt.Errorf("failed to get CLI client ID: %w", err) + } + err = kc.CreateProtocolMapper(ctx, cliClientID, groupMapper) + g.Expect(err).NotTo(HaveOccurred(), "failed to create protocol mapper for CLI client") + + // Add group mapper to console client + t.Logf("Adding group mapper to console client: %s", config.ConsoleClientID) + consoleClientID, err := kc.GetClientByClientID(ctx, config.ConsoleClientID) + if err != nil { + return "", fmt.Errorf("failed to get console client ID: %w", err) + } + err = kc.CreateProtocolMapper(ctx, consoleClientID, groupMapper) + g.Expect(err).NotTo(HaveOccurred(), "failed to create protocol mapper for console client") + + // Return users in format "user1:pass1,user2:pass2,..." + return strings.Join(users, ","), 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 +} + +// TestResources tracks resources created during a test for cleanup +type TestResources struct { + AdminClient *KeycloakAdminClient + UserIDs []string + GroupIDs []string + UserCreds map[string]string // username -> password +} + +func renewAdminTokenIfExpired(ctx context.Context, adminClient *KeycloakAdminClient, externalOIDCConfig *ExtOIDCConfig) error { + clientID := externalOIDCConfig.ConsoleClientID + clientSecret := externalOIDCConfig.ConsoleClientSecretValue + accessToken := adminClient.AdminToken + + requestURL := externalOIDCConfig.IssuerURL + "/protocol/openid-connect/token/introspect" + + formData := url.Values{ + "client_id": []string{clientID}, + "client_secret": []string{clientSecret}, + "token": []string{accessToken}, + } + + httpClient := &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: true, + }, + }, + } + + response, err := httpClient.PostForm( + requestURL, + formData, + ) + + if err != nil { + return fmt.Errorf("failed to POST to token introspect endpoint: %w", err) + } + + defer response.Body.Close() + + body, err := io.ReadAll(response.Body) + if err != nil { + return fmt.Errorf("failed to read response body: %w", err) + } + + var respMap map[string]any + + err = json.Unmarshal(body, &respMap) + if err != nil { + return fmt.Errorf("failed to unmarshal response: %w", err) + } + + active, ok := respMap["active"].(bool) + if !ok { + return fmt.Errorf("active not found or not a bool in response") + } + + if !active { + // refresh admin token as it has expired + adminClient.GetAdminToken(ctx) + } + + return nil +} + +// NewTestResources creates a new TestResources tracker +func NewTestResources(ctx context.Context, adminClient *KeycloakAdminClient, externalOIDCConfig *ExtOIDCConfig) (*TestResources, error) { + // renew admin token if expired + err := renewAdminTokenIfExpired(ctx, adminClient, externalOIDCConfig) + + if err != nil { + return nil, fmt.Errorf("an error occurred while checking for token renewal: %w", err) + } + + return &TestResources{ + AdminClient: adminClient, + UserIDs: []string{}, + GroupIDs: []string{}, + UserCreds: make(map[string]string), + }, nil +} + +// CreateTestUser creates a user and tracks it for cleanup +func (tr *TestResources) CreateTestUser(ctx context.Context, t *testing.T, username, email, password string) (string, error) { + return tr.CreateTestUserWithEmailVerification(ctx, t, username, email, password, true) +} + +// CreateTestUserWithEmailVerification creates a user with specific email verification status and tracks it for cleanup +func (tr *TestResources) CreateTestUserWithEmailVerification(ctx context.Context, t *testing.T, username, email, password string, emailVerified bool) (string, error) { + user := KeycloakUser{ + Username: username, + Enabled: true, + FirstName: username, + LastName: "Test", + Email: email, + EmailVerified: emailVerified, + } + + userID, err := tr.AdminClient.CreateUser(ctx, user) + if err != nil { + return "", err + } + + // Set password + err = tr.AdminClient.SetUserPassword(ctx, userID, password, false) + if err != nil { + return "", err + } + + // Track for cleanup + tr.UserIDs = append(tr.UserIDs, userID) + tr.UserCreds[username] = password + + t.Logf("Created test user: %s (ID: %s, email_verified: %v)", username, userID, emailVerified) + return userID, nil +} + +// CreateTestGroup creates a group and tracks it for cleanup +func (tr *TestResources) CreateTestGroup(ctx context.Context, t *testing.T, groupName string) (string, error) { + groupID, err := tr.AdminClient.CreateGroup(ctx, groupName) + if err != nil { + return "", err + } + + // Track for cleanup + tr.GroupIDs = append(tr.GroupIDs, groupID) + + t.Logf("Created test group: %s (ID: %s)", groupName, groupID) + return groupID, nil +} + +// CreateTestUserWithRandomCredentials creates a user with generated credentials and tracks it for cleanup +func (tr *TestResources) CreateTestUserWithRandomCredentials(ctx context.Context, t *testing.T, usernamePrefix string) (userID, username, email, password string, err error) { + username = usernamePrefix + "-" + GenerateRandomPassword(8) + email = username + "@test.example.com" + password = GenerateRandomPassword(16) + + userID, err = tr.CreateTestUser(ctx, t, username, email, password) + return userID, username, email, password, err +} + +// GetTestUsersString returns users in format "user1:pass1,user2:pass2,..." +func (tr *TestResources) GetTestUsersString() string { + var users []string + for username, password := range tr.UserCreds { + users = append(users, fmt.Sprintf("%s:%s", username, password)) + } + return strings.Join(users, ",") +} + +// Cleanup deletes all tracked resources +func (tr *TestResources) Cleanup(ctx context.Context, t *testing.T) { + t.Logf("Cleaning up test resources: %d users, %d groups", len(tr.UserIDs), len(tr.GroupIDs)) + + if err := tr.AdminClient.GetAdminToken(ctx); err != nil { + t.Logf("Warning: failed to refresh admin token: %v", err) + } + + // Delete users + for _, userID := range tr.UserIDs { + if err := tr.AdminClient.DeleteUser(ctx, userID); err != nil { + t.Logf("Warning: failed to delete user %s: %v", userID, err) + } else { + t.Logf("Deleted user: %s", userID) + } + } + + // Delete groups + for _, groupID := range tr.GroupIDs { + if err := tr.AdminClient.DeleteGroup(ctx, groupID); err != nil { + t.Logf("Warning: failed to delete group %s: %v", groupID, err) + } else { + t.Logf("Deleted group: %s", groupID) + } + } + + // Clear tracking + tr.UserIDs = []string{} + tr.GroupIDs = []string{} + tr.UserCreds = make(map[string]string) +} + +// AuthenticatedTestUser contains the results of setting up an authenticated test user +type AuthenticatedTestUser struct { + Username string + Email string + Password string + UserID string + GroupID string + GroupName string + KubeConfig *rest.Config + AuthClient kauthnv1typedclient.AuthenticationV1Interface + SelfSubjectReview *kauthnv1.SelfSubjectReview +} + +// TryAuthenticateUser attempts to authenticate a user and returns error if authentication fails +// This is useful for negative testing where you expect authentication to fail +func (tr *TestResources) TryAuthenticateUser( + ctx context.Context, + t *testing.T, + username string, + password string, + clientCfg *rest.Config, + extOIDCConfig *ExtOIDCConfig, +) error { + // This function replicates ChangeUserForKeycloakExtOIDC but returns errors + // instead of using gomega assertions, allowing negative testing scenarios + + if extOIDCConfig == nil { + return fmt.Errorf("extOIDCConfig is nil") + } + if extOIDCConfig.ExternalOIDCProvider != ProviderKeycloak { + return fmt.Errorf("expected Keycloak provider, got %s", extOIDCConfig.ExternalOIDCProvider) + } + + // Step 1: Get OIDC token from Keycloak + httpClient := &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: true, + }, + }, + } + + requestURL := extOIDCConfig.IssuerURL + "/protocol/openid-connect/token" + oidcClientID := extOIDCConfig.CliClientID + if oidcClientID == "" { + return fmt.Errorf("oidcClientID is empty") + } + + formData := url.Values{ + "client_id": []string{oidcClientID}, + "grant_type": []string{"password"}, + "password": []string{password}, + "scope": []string{"openid email profile"}, + "username": []string{username}, + } + + response, err := httpClient.PostForm(requestURL, formData) + if err != nil { + return fmt.Errorf("failed to POST to token endpoint: %w", err) + } + defer response.Body.Close() + + // Authentication can fail at Keycloak level (e.g., email_verified=false) + if response.StatusCode != http.StatusOK { + body, _ := io.ReadAll(response.Body) + return fmt.Errorf("keycloak authentication failed with status %d: %s", response.StatusCode, string(body)) + } + + body, err := io.ReadAll(response.Body) + if err != nil { + return fmt.Errorf("failed to read response body: %w", err) + } + + var respMap map[string]any + err = json.Unmarshal(body, &respMap) + if err != nil { + return fmt.Errorf("failed to unmarshal token response: %w", err) + } + + idToken, ok := respMap["id_token"].(string) + if !ok { + return fmt.Errorf("id_token not found or not a string in response") + } + + // Step 2: Try to authenticate with K8s using the ID token directly + // We use the token as a bearer token instead of going through the exec plugin, + // which would attempt an authorization code flow (browser-based) that requires + // user interaction. Using the token directly allows us to capture validation + // errors from the Kubernetes API server. + // Use AnonymousClientConfig to clear all auth credentials (certs, tokens) from the admin config + clientConfigForExtOIDCUser := rest.AnonymousClientConfig(rest.CopyConfig(clientCfg)) + clientConfigForExtOIDCUser.BearerToken = idToken + + authClient, err := kauthnv1typedclient.NewForConfig(clientConfigForExtOIDCUser) + if err != nil { + return fmt.Errorf("failed to create auth client: %w", err) + } + + // Authentication can fail at K8s level due to claim validation rules or user validation rules + _, err = authClient.SelfSubjectReviews().Create(ctx, &kauthnv1.SelfSubjectReview{}, metav1.CreateOptions{}) + if err != nil { + return fmt.Errorf("kubernetes authentication failed: %w", err) + } + + return nil +} + +// SetupAuthenticatedUserWithGroup creates a test user, group, adds user to group, authenticates, and gets self subject review +// This is a complete setup for testing authentication and authorization scenarios +func (tr *TestResources) SetupAuthenticatedUserWithGroup( + ctx context.Context, + t *testing.T, + usernamePrefix string, + groupNamePrefix string, + clientCfg *rest.Config, + extOIDCConfig *ExtOIDCConfig, +) (*AuthenticatedTestUser, error) { + g := NewWithT(t) + + // Create test group + groupName := groupNamePrefix + "-" + GenerateRandomPassword(8) + groupID, err := tr.CreateTestGroup(ctx, t, groupName) + if err != nil { + return nil, fmt.Errorf("failed to create test group: %w", err) + } + t.Logf("Created test group: %s (ID: %s)", groupName, groupID) + + // Create test user with specific email + username := usernamePrefix + "-" + GenerateRandomPassword(8) + email := username + "@test.example.com" + password := GenerateRandomPassword(16) + userID, err := tr.CreateTestUser(ctx, t, username, email, password) + if err != nil { + return nil, fmt.Errorf("failed to create test user: %w", err) + } + t.Logf("Created test user: %s (email: %s, ID: %s)", username, email, userID) + + // Add user to group + err = tr.AdminClient.AddUserToGroup(ctx, userID, groupID) + if err != nil { + return nil, fmt.Errorf("failed to add user to group: %w", err) + } + t.Logf("Added user %s to group %s", username, groupName) + + // Authenticate as test user + testAuthConfig := *extOIDCConfig + testAuthConfig.TestUsers = username + ":" + password + testUserKubeConfig := ChangeUserForKeycloakExtOIDC(t, ctx, clientCfg, &testAuthConfig) + testAuthClient, err := kauthnv1typedclient.NewForConfig(testUserKubeConfig) + g.Expect(err).NotTo(HaveOccurred()) + + // Get self subject review + selfSubjectReview, err := testAuthClient.SelfSubjectReviews().Create(ctx, &kauthnv1.SelfSubjectReview{}, metav1.CreateOptions{}) + if err != nil { + return nil, fmt.Errorf("failed to get self subject review: %w", err) + } + t.Logf("Test user self subject review: %+v", selfSubjectReview.Status.UserInfo) + + return &AuthenticatedTestUser{ + Username: username, + Email: email, + Password: password, + UserID: userID, + GroupID: groupID, + GroupName: groupName, + KubeConfig: testUserKubeConfig, + AuthClient: testAuthClient, + SelfSubjectReview: selfSubjectReview, + }, nil +} From 40888bf520737d1d8c62244c7ed84865fd7242b9 Mon Sep 17 00:00:00 2001 From: Evan Hearne Date: Thu, 21 May 2026 13:52:02 +0100 Subject: [PATCH 6/9] test(CNTRLPLANE-3306): add custom auth spec functionality for testing This change adds the ability to specify custom auth config for testing ExternalOIDC functionality, which will prove useful for when features behind feature gates graduate to default feature set. --- test/e2e/util/external_oidc.go | 3 +++ test/e2e/util/hypershift_framework.go | 6 +++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/test/e2e/util/external_oidc.go b/test/e2e/util/external_oidc.go index 42f9eaff6c7d..4358b8ec8ec0 100644 --- a/test/e2e/util/external_oidc.go +++ b/test/e2e/util/external_oidc.go @@ -75,6 +75,9 @@ type ExtOIDCConfig struct { // for oidcProviders.issuer.issuerCertificateAuthority IssuerCAConfigmapName string IssuerCABundleFile string + + // custom field for adding custom auth configuration + CustomAuthSpec *configv1.AuthenticationSpec } func GetExtOIDCConfig(provider, cliClientID, consoleClientID, issuerURL, consoleSecret, issuerCABundleFile, testUsers string) *ExtOIDCConfig { diff --git a/test/e2e/util/hypershift_framework.go b/test/e2e/util/hypershift_framework.go index 4781067822c8..24bfee0bbefd 100644 --- a/test/e2e/util/hypershift_framework.go +++ b/test/e2e/util/hypershift_framework.go @@ -483,7 +483,11 @@ func (h *hypershiftTest) createHostedCluster(opts *PlatformAgnosticOptions, plat if v.Spec.Configuration == nil { v.Spec.Configuration = &hyperv1.ClusterConfiguration{} } - v.Spec.Configuration.Authentication = opts.ExtOIDCConfig.GetAuthenticationConfig() + if opts.ExtOIDCConfig.CustomAuthSpec != nil { + v.Spec.Configuration.Authentication = opts.ExtOIDCConfig.CustomAuthSpec + } else { + v.Spec.Configuration.Authentication = opts.ExtOIDCConfig.GetAuthenticationConfig() + } } } } From b3b2a8463ba417c02f0b2f1282b2e6307bc88fd2 Mon Sep 17 00:00:00 2001 From: Evan Hearne Date: Thu, 21 May 2026 13:52:56 +0100 Subject: [PATCH 7/9] test(CNTRLPLANE-3306): test custom auth spec config This change experiments with custom auth config chage from previous commit by adding new NHT.Execute() with custom auth config specified. --- test/e2e/external_oidc_test.go | 166 +++++++++++++++++++++++++++++++++ test/e2e/util/external_oidc.go | 1 + test/e2e/util/keycloak.go | 45 +++++---- 3 files changed, 195 insertions(+), 17 deletions(-) diff --git a/test/e2e/external_oidc_test.go b/test/e2e/external_oidc_test.go index 5af4b0c6cfba..1983f3877bee 100644 --- a/test/e2e/external_oidc_test.go +++ b/test/e2e/external_oidc_test.go @@ -363,4 +363,170 @@ func TestExternalOIDC(t *testing.T) { }) } }).Execute(&clusterOpts, globalOpts.Platform, globalOpts.ArtifactDir, "external-oidc", globalOpts.ServiceAccountSigningKey) + + // experiment: see if we can pass custom auth config to test here + customClusterOpts := clusterOpts + config := customClusterOpts.ExtOIDCConfig + customClusterOpts.ExtOIDCConfig.CustomAuthSpec = &configv1.AuthenticationSpec{ + Type: configv1.AuthenticationTypeOIDC, + OIDCProviders: []configv1.OIDCProvider{ + { + Name: config.OIDCProviderName, + Issuer: configv1.TokenIssuer{ + Audiences: []configv1.TokenAudience{ + configv1.TokenAudience(config.CliClientID), + configv1.TokenAudience(config.ConsoleClientID), + }, + URL: config.IssuerURL, + CertificateAuthority: configv1.ConfigMapNameReference{ + Name: config.IssuerCAConfigmapName, + }, + }, + OIDCClients: []configv1.OIDCClientConfig{ + { + ClientID: config.CliClientID, + ComponentName: "cli", + ComponentNamespace: "openshift-console", + ExtraScopes: []string{"email"}, + }, + { + ClientID: config.ConsoleClientID, + ClientSecret: configv1.SecretNameReference{ + Name: config.ConsoleClientSecretName, + }, + ComponentName: "console", + ComponentNamespace: "openshift-console", + ExtraScopes: []string{"email"}, + }, + }, + ClaimMappings: configv1.TokenClaimMappings{ + UID: &configv1.TokenClaimOrExpressionMapping{ + Claim: "sub", + }, + Username: configv1.UsernameClaimMapping{ + Expression: "claims.email.split('@')[0]", + }, + Groups: configv1.PrefixedClaimMapping{ + TokenClaimMapping: configv1.TokenClaimMapping{ + Expression: "claims.?groups.orValue([])", + }, + }, + Extra: []configv1.ExtraMapping{ + { + Key: "extratest.openshift.com/foo", + ValueExpression: "claims.email", + }, + }, + }, + ClaimValidationRules: []configv1.TokenClaimValidationRule{ + { + Type: configv1.TokenValidationRuleTypeCEL, + CEL: configv1.TokenClaimValidationCELRule{ + Expression: "has(claims.email) && claims.email != ''", + Message: "email claim must be present and non-empty", + }, + }, + { + Type: configv1.TokenValidationRuleTypeCEL, + CEL: configv1.TokenClaimValidationCELRule{ + Expression: "claims.email_verified == true", + Message: "email_verified claim must be true", + }, + }, + }, + UserValidationRules: []configv1.TokenUserValidationRule{ + { + Expression: "!user.username.startsWith('system:')", + Message: "username cannot use reserved system: prefix", + }, + { + Expression: "!user.username.contains('forbidden')", + Message: "username cannot contain the word 'forbidden'", + }, + }, + }, + }, + } + // currently is still under feature gate - having the tests + // run like this mean easy removal of feature gate check + // when they graduate to default feature set. + if featuregates.Gate().Enabled(featuregates.ExternalOIDCWithUpstreamParity) { + e2eutil.NewHypershiftTest(t, ctx, func(t *testing.T, g Gomega, mgtClient crclient.Client, hostedCluster *hyperv1.HostedCluster) { + 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) + 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:ExternalOIDCWithUpstreamParity] Test user validation rules 1", func(t *testing.T) { + g := NewWithT(t) + t.Logf("begin to test user validation rules") + + // Setup: Create test resources with automatic cleanup + testResources, err := e2eutil.NewTestResources(ctx, kc, clusterOpts.ExtOIDCConfig) + g.Expect(err).NotTo(HaveOccurred()) + defer testResources.Cleanup(ctx, t) + + // Verify: User validation rules are configured + g.Expect(hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].UserValidationRules).NotTo(BeEmpty()) + userRules := hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].UserValidationRules + g.Expect(userRules).Should(HaveLen(2), "should have two user validation rules") + + // Verify: Rules use expected CEL expressions + expressions := []string{userRules[0].Expression, userRules[1].Expression} + g.Expect(expressions).Should(ContainElement(e2eutil.UserValidationExprNoSystemPrefix)) + g.Expect(expressions).Should(ContainElement(e2eutil.UserValidationExprNoForbiddenWord)) + t.Logf("User validation rules configured: %v", expressions) + + // Test 1: Valid user - passes all validation rules + // Should PASS validation and authenticate successfully + t.Logf("Test 1: Creating user with valid username (no system: prefix, no 'forbidden' word)") + validUser, err := testResources.SetupAuthenticatedUserWithGroup(ctx, t, "user-valid", "user-valid-group", clientCfg, clusterOpts.ExtOIDCConfig) + g.Expect(err).NotTo(HaveOccurred(), "authentication must succeed when all validation rules pass") + + // Username is derived from email via CEL: claims.email.split('@')[0] + expectedUsername := strings.Split(validUser.Email, "@")[0] + g.Expect(validUser.SelfSubjectReview.Status.UserInfo.Username).Should(Equal(expectedUsername)) + g.Expect(validUser.SelfSubjectReview.Status.UserInfo.Username).NotTo(HavePrefix("system:")) + g.Expect(validUser.SelfSubjectReview.Status.UserInfo.Username).NotTo(ContainSubstring("forbidden")) + t.Logf("✓ User with username='%s' authenticated successfully", validUser.SelfSubjectReview.Status.UserInfo.Username) + + // Test 2: Invalid user - username contains "forbidden" + // Demonstrates the testable user validation rule: !user.username.contains('forbidden') + t.Logf("Test 2: Creating user with 'forbidden' in username (violates user validation rule)") + forbiddenUsername := "user-forbidden-" + e2eutil.GenerateRandomPassword(8) + forbiddenEmail := forbiddenUsername + "@test.example.com" + forbiddenPassword := e2eutil.GenerateRandomPassword(16) + forbiddenUserID, err := testResources.CreateTestUser(ctx, t, forbiddenUsername, forbiddenEmail, forbiddenPassword) + g.Expect(err).NotTo(HaveOccurred(), "creating user in Keycloak should succeed") + t.Logf("Created user with email='%s', mapped username will be '%s' (ID: %s)", forbiddenEmail, forbiddenUsername, forbiddenUserID) + + // Try to authenticate - should FAIL due to user validation rule + err = testResources.TryAuthenticateUser(ctx, t, forbiddenUsername, forbiddenPassword, clientCfg, clusterOpts.ExtOIDCConfig) + g.Expect(err).To(HaveOccurred(), "authentication must fail when username contains 'forbidden'") + g.Expect(err.Error()).Should(ContainSubstring("Unauthorized"), + "forbidden user cannot authenticate as it violates user validation rule") + t.Logf("✓ User with 'forbidden' in username correctly rejected with error: %v", err) + + // NOTE: We cannot test the negative case for the system: prefix rule via Keycloak + // because RFC 5322 email addresses do not allow colons in the local part. + // Since username = claims.email.split('@')[0], we would need an email like + // "system:admin@test.example.com", which is invalid per email standards. + // The system: prefix rule should be tested via unit tests or envtest where claims can be mocked. + + t.Logf("User validation rules successfully validated: users with 'forbidden' in username are rejected") + }) + }).Execute(&customClusterOpts, globalOpts.Platform, globalOpts.ArtifactDir, "ext-oidc-usr-rules", globalOpts.ServiceAccountSigningKey) + } + } diff --git a/test/e2e/util/external_oidc.go b/test/e2e/util/external_oidc.go index 4358b8ec8ec0..23be67982d77 100644 --- a/test/e2e/util/external_oidc.go +++ b/test/e2e/util/external_oidc.go @@ -15,6 +15,7 @@ import ( "os" "path/filepath" "regexp" + "strings" "testing" "time" diff --git a/test/e2e/util/keycloak.go b/test/e2e/util/keycloak.go index 2e1fbba040e7..e78c82ebe366 100644 --- a/test/e2e/util/keycloak.go +++ b/test/e2e/util/keycloak.go @@ -20,6 +20,7 @@ import ( 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" ) @@ -96,7 +97,7 @@ func (kc *KeycloakAdminClient) GetAdminToken(ctx context.Context) error { "password": []string{kc.AdminPass}, } - req, err := http.NewRequestWithContext(ctx, "POST", tokenURL, nil) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, tokenURL, nil) if err != nil { return fmt.Errorf("failed to create token request: %w", err) } @@ -138,7 +139,7 @@ func (kc *KeycloakAdminClient) CreateGroup(ctx context.Context, groupName string return "", fmt.Errorf("failed to marshal group: %w", err) } - req, err := http.NewRequestWithContext(ctx, "POST", groupURL, strings.NewReader(string(groupJSON))) + 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) } @@ -181,7 +182,7 @@ func (kc *KeycloakAdminClient) CreateUser(ctx context.Context, user KeycloakUser return "", fmt.Errorf("failed to marshal user: %w", err) } - req, err := http.NewRequestWithContext(ctx, "POST", userURL, strings.NewReader(string(userJSON))) + 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) } @@ -230,7 +231,7 @@ func (kc *KeycloakAdminClient) SetUserPassword(ctx context.Context, userID, pass return fmt.Errorf("failed to marshal credential: %w", err) } - req, err := http.NewRequestWithContext(ctx, "PUT", passwordURL, strings.NewReader(string(credJSON))) + req, err := http.NewRequestWithContext(ctx, http.MethodPut, passwordURL, strings.NewReader(string(credJSON))) if err != nil { return fmt.Errorf("failed to create password request: %w", err) } @@ -255,7 +256,7 @@ func (kc *KeycloakAdminClient) SetUserPassword(ctx context.Context, userID, pass func (kc *KeycloakAdminClient) AddUserToGroup(ctx context.Context, userID, groupID string) error { groupURL := fmt.Sprintf("%s/admin/realms/master/users/%s/groups/%s", kc.BaseURL, userID, groupID) - req, err := http.NewRequestWithContext(ctx, "PUT", groupURL, nil) + req, err := http.NewRequestWithContext(ctx, http.MethodPut, groupURL, nil) if err != nil { return fmt.Errorf("failed to create add-to-group request: %w", err) } @@ -280,7 +281,7 @@ func (kc *KeycloakAdminClient) AddUserToGroup(ctx context.Context, userID, group func (kc *KeycloakAdminClient) GetClientByClientID(ctx context.Context, clientID string) (string, error) { clientsURL := fmt.Sprintf("%s/admin/realms/master/clients?clientId=%s", kc.BaseURL, url.QueryEscape(clientID)) - req, err := http.NewRequestWithContext(ctx, "GET", clientsURL, nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, clientsURL, nil) if err != nil { return "", fmt.Errorf("failed to create get-client request: %w", err) } @@ -313,7 +314,7 @@ func (kc *KeycloakAdminClient) GetClientByClientID(ctx context.Context, clientID func (kc *KeycloakAdminClient) DeleteUser(ctx context.Context, userID string) error { userURL := fmt.Sprintf("%s/admin/realms/master/users/%s", kc.BaseURL, userID) - req, err := http.NewRequestWithContext(ctx, "DELETE", userURL, nil) + req, err := http.NewRequestWithContext(ctx, http.MethodDelete, userURL, nil) if err != nil { return fmt.Errorf("failed to create delete-user request: %w", err) } @@ -337,7 +338,7 @@ func (kc *KeycloakAdminClient) DeleteUser(ctx context.Context, userID string) er func (kc *KeycloakAdminClient) DeleteGroup(ctx context.Context, groupID string) error { groupURL := fmt.Sprintf("%s/admin/realms/master/groups/%s", kc.BaseURL, groupID) - req, err := http.NewRequestWithContext(ctx, "DELETE", groupURL, nil) + req, err := http.NewRequestWithContext(ctx, http.MethodDelete, groupURL, nil) if err != nil { return fmt.Errorf("failed to create delete-group request: %w", err) } @@ -361,7 +362,7 @@ func (kc *KeycloakAdminClient) DeleteGroup(ctx context.Context, groupID string) func (kc *KeycloakAdminClient) GetUserByUsername(ctx context.Context, username string) (string, error) { usersURL := fmt.Sprintf("%s/admin/realms/master/users?username=%s&exact=true", kc.BaseURL, url.QueryEscape(username)) - req, err := http.NewRequestWithContext(ctx, "GET", usersURL, nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, usersURL, nil) if err != nil { return "", fmt.Errorf("failed to create get-user request: %w", err) } @@ -397,7 +398,7 @@ func (kc *KeycloakAdminClient) GetUserByUsername(ctx context.Context, username s func (kc *KeycloakAdminClient) GetGroupByName(ctx context.Context, groupName string) (string, error) { groupsURL := fmt.Sprintf("%s/admin/realms/master/groups", kc.BaseURL) - req, err := http.NewRequestWithContext(ctx, "GET", groupsURL, nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, groupsURL, nil) if err != nil { return "", fmt.Errorf("failed to create get-groups request: %w", err) } @@ -440,7 +441,7 @@ func (kc *KeycloakAdminClient) CreateProtocolMapper(ctx context.Context, clientI return fmt.Errorf("failed to marshal mapper: %w", err) } - req, err := http.NewRequestWithContext(ctx, "POST", mapperURL, strings.NewReader(string(mapperJSON))) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, mapperURL, strings.NewReader(string(mapperJSON))) if err != nil { return fmt.Errorf("failed to create mapper request: %w", err) } @@ -675,11 +676,13 @@ func renewAdminTokenIfExpired(ctx context.Context, adminClient *KeycloakAdminCli }, } - response, err := httpClient.PostForm( - requestURL, - formData, - ) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, requestURL, strings.NewReader(formData.Encode())) + if err != nil { + return fmt.Errorf("failed to create token introspect request: %w", err) + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + response, err := httpClient.Do(req) if err != nil { return fmt.Errorf("failed to POST to token introspect endpoint: %w", err) } @@ -705,7 +708,9 @@ func renewAdminTokenIfExpired(ctx context.Context, adminClient *KeycloakAdminCli if !active { // refresh admin token as it has expired - adminClient.GetAdminToken(ctx) + if err := adminClient.GetAdminToken(ctx); err != nil { + return fmt.Errorf("failed to refresh admin token: %w", err) + } } return nil @@ -884,7 +889,13 @@ func (tr *TestResources) TryAuthenticateUser( "username": []string{username}, } - response, err := httpClient.PostForm(requestURL, formData) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, requestURL, strings.NewReader(formData.Encode())) + if err != nil { + return fmt.Errorf("failed to create token request: %w", err) + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + + response, err := httpClient.Do(req) if err != nil { return fmt.Errorf("failed to POST to token endpoint: %w", err) } From 84572405c66bf7dddc7aa8ef40f92e28c3424786 Mon Sep 17 00:00:00 2001 From: Evan Hearne Date: Wed, 17 Jun 2026 09:46:31 +0100 Subject: [PATCH 8/9] test(CNTRLPLANE-3306): customize auth spec as needed This change moves from having to write out a complete auth spec per test, instead only modifying a baseline auth spec as needed per test. This is currently being tested on one test. If successful, next commits will look to migrate over all tests to custom auth spec pattern. --- test/e2e/external_oidc_test.go | 88 +++------------------------ test/e2e/util/external_oidc.go | 23 +++---- test/e2e/util/hypershift_framework.go | 6 +- 3 files changed, 20 insertions(+), 97 deletions(-) diff --git a/test/e2e/external_oidc_test.go b/test/e2e/external_oidc_test.go index 1983f3877bee..02144d24b68d 100644 --- a/test/e2e/external_oidc_test.go +++ b/test/e2e/external_oidc_test.go @@ -366,86 +366,18 @@ func TestExternalOIDC(t *testing.T) { // experiment: see if we can pass custom auth config to test here customClusterOpts := clusterOpts - config := customClusterOpts.ExtOIDCConfig - customClusterOpts.ExtOIDCConfig.CustomAuthSpec = &configv1.AuthenticationSpec{ - Type: configv1.AuthenticationTypeOIDC, - OIDCProviders: []configv1.OIDCProvider{ + customClusterOpts.ExtOIDCConfig.CustomizeAuthSpec = func(spec *configv1.AuthenticationSpec) { + // Customize UID mapping to use claim instead of expression + spec.OIDCProviders[0].ClaimMappings.UID = &configv1.TokenClaimOrExpressionMapping{ + Claim: "sub", + } + // Keep only one extra mapping instead of two + spec.OIDCProviders[0].ClaimMappings.Extra = []configv1.ExtraMapping{ { - Name: config.OIDCProviderName, - Issuer: configv1.TokenIssuer{ - Audiences: []configv1.TokenAudience{ - configv1.TokenAudience(config.CliClientID), - configv1.TokenAudience(config.ConsoleClientID), - }, - URL: config.IssuerURL, - CertificateAuthority: configv1.ConfigMapNameReference{ - Name: config.IssuerCAConfigmapName, - }, - }, - OIDCClients: []configv1.OIDCClientConfig{ - { - ClientID: config.CliClientID, - ComponentName: "cli", - ComponentNamespace: "openshift-console", - ExtraScopes: []string{"email"}, - }, - { - ClientID: config.ConsoleClientID, - ClientSecret: configv1.SecretNameReference{ - Name: config.ConsoleClientSecretName, - }, - ComponentName: "console", - ComponentNamespace: "openshift-console", - ExtraScopes: []string{"email"}, - }, - }, - ClaimMappings: configv1.TokenClaimMappings{ - UID: &configv1.TokenClaimOrExpressionMapping{ - Claim: "sub", - }, - Username: configv1.UsernameClaimMapping{ - Expression: "claims.email.split('@')[0]", - }, - Groups: configv1.PrefixedClaimMapping{ - TokenClaimMapping: configv1.TokenClaimMapping{ - Expression: "claims.?groups.orValue([])", - }, - }, - Extra: []configv1.ExtraMapping{ - { - Key: "extratest.openshift.com/foo", - ValueExpression: "claims.email", - }, - }, - }, - ClaimValidationRules: []configv1.TokenClaimValidationRule{ - { - Type: configv1.TokenValidationRuleTypeCEL, - CEL: configv1.TokenClaimValidationCELRule{ - Expression: "has(claims.email) && claims.email != ''", - Message: "email claim must be present and non-empty", - }, - }, - { - Type: configv1.TokenValidationRuleTypeCEL, - CEL: configv1.TokenClaimValidationCELRule{ - Expression: "claims.email_verified == true", - Message: "email_verified claim must be true", - }, - }, - }, - UserValidationRules: []configv1.TokenUserValidationRule{ - { - Expression: "!user.username.startsWith('system:')", - Message: "username cannot use reserved system: prefix", - }, - { - Expression: "!user.username.contains('forbidden')", - Message: "username cannot contain the word 'forbidden'", - }, - }, + Key: "extratest.openshift.com/foo", + ValueExpression: "claims.email", }, - }, + } } // currently is still under feature gate - having the tests // run like this mean easy removal of feature gate check diff --git a/test/e2e/util/external_oidc.go b/test/e2e/util/external_oidc.go index 23be67982d77..f71597ab3439 100644 --- a/test/e2e/util/external_oidc.go +++ b/test/e2e/util/external_oidc.go @@ -77,8 +77,9 @@ type ExtOIDCConfig struct { IssuerCAConfigmapName string IssuerCABundleFile string - // custom field for adding custom auth configuration - CustomAuthSpec *configv1.AuthenticationSpec + // 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 { @@ -168,17 +169,6 @@ func (config *ExtOIDCConfig) GetAuthenticationConfig() *configv1.AuthenticationS } if featuregates.Gate().Enabled(featuregates.ExternalOIDCWithUpstreamParity) { - // Check if ExternalOIDCWithUIDAndExtraClaimMappings feature gate is enabled. - // If not, we will need to add extra mapping to access email for username - // verification later. - if !featuregates.Gate().Enabled(featuregates.ExternalOIDCWithUIDAndExtraClaimMappings) { - authnSpec.OIDCProviders[0].ClaimMappings.Extra = append(authnSpec.OIDCProviders[0].ClaimMappings.Extra, - configv1.ExtraMapping{ - Key: ExternalOIDCExtraKeyFoo, - ValueExpression: ExternalOIDCExtraKeyFooValueExpression, - }, - ) - } // Use CEL expression for username mapping instead of static claim authnSpec.OIDCProviders[0].ClaimMappings.Username = configv1.UsernameClaimMapping{ Expression: "claims.email.split('@')[0]", @@ -222,6 +212,11 @@ func (config *ExtOIDCConfig) GetAuthenticationConfig() *configv1.AuthenticationS } } + // Apply custom modifications if provided + if config.CustomizeAuthSpec != nil { + config.CustomizeAuthSpec(authnSpec) + } + return authnSpec } @@ -246,7 +241,7 @@ func ValidateAuthenticationSpec(t testing.TB, ctx context.Context, client crclie actualAuth := hostedCluster.Spec.Configuration.Authentication g.Expect(actualAuth.OIDCProviders).NotTo(BeEmpty()) - if featuregates.Gate().Enabled(featuregates.ExternalOIDCWithUpstreamParity) || featuregates.Gate().Enabled(featuregates.ExternalOIDCWithUIDAndExtraClaimMappings) { + if featuregates.Gate().Enabled(featuregates.ExternalOIDCWithUIDAndExtraClaimMappings) { g.Expect(actualAuth.OIDCProviders[0].ClaimMappings.Extra).NotTo(BeEmpty()) } diff --git a/test/e2e/util/hypershift_framework.go b/test/e2e/util/hypershift_framework.go index 24bfee0bbefd..4781067822c8 100644 --- a/test/e2e/util/hypershift_framework.go +++ b/test/e2e/util/hypershift_framework.go @@ -483,11 +483,7 @@ func (h *hypershiftTest) createHostedCluster(opts *PlatformAgnosticOptions, plat if v.Spec.Configuration == nil { v.Spec.Configuration = &hyperv1.ClusterConfiguration{} } - if opts.ExtOIDCConfig.CustomAuthSpec != nil { - v.Spec.Configuration.Authentication = opts.ExtOIDCConfig.CustomAuthSpec - } else { - v.Spec.Configuration.Authentication = opts.ExtOIDCConfig.GetAuthenticationConfig() - } + v.Spec.Configuration.Authentication = opts.ExtOIDCConfig.GetAuthenticationConfig() } } } From 72b017b7008e757ec82e4f75c715cd740ee83189 Mon Sep 17 00:00:00 2001 From: Evan Hearne Date: Wed, 17 Jun 2026 13:23:07 +0100 Subject: [PATCH 9/9] test(CNTRLPLANE-3306): refactor ExternalOIDC tests This change refactors the external oidc tests so that they use the auth config spec, modifying as needed for their use case. The change also lifts feature gate checks for feature sets that have graduated to default, and allows for easier modification of default auth config when this occurs in the future. When feature gate checks are needed for feature sets, we can now specify auth config modifications to be used in test execution. --- hypershift-operator/main_test.go | 24 + test/e2e/external_oidc_test.go | 459 +++++--------------- test/e2e/util/external_oidc.go | 41 ++ test/e2e/util/external_oidc_test_helpers.go | 310 +++++++++++++ test/e2e/util/keycloak.go | 30 ++ 5 files changed, 507 insertions(+), 357 deletions(-) create mode 100644 hypershift-operator/main_test.go create mode 100644 test/e2e/util/external_oidc_test_helpers.go diff --git a/hypershift-operator/main_test.go b/hypershift-operator/main_test.go new file mode 100644 index 000000000000..4801d526fc3f --- /dev/null +++ b/hypershift-operator/main_test.go @@ -0,0 +1,24 @@ +package main + +import ( + "testing" + + "github.com/spf13/cobra" +) + +// When NewStartCommand is called, it should return a valid cobra command +func TestNewStartCommand(t *testing.T) { + cmd := NewStartCommand() + if cmd == nil { + t.Fatal("NewStartCommand() returned nil") + } + if _, ok := any(cmd).(*cobra.Command); !ok { + t.Fatalf("NewStartCommand() did not return a *cobra.Command, got %T", cmd) + } + if cmd.Use != "run" { + t.Errorf("expected Use='run', got %q", cmd.Use) + } + if cmd.Short == "" { + t.Error("expected non-empty Short description") + } +} diff --git a/test/e2e/external_oidc_test.go b/test/e2e/external_oidc_test.go index 02144d24b68d..fb881da6eacb 100644 --- a/test/e2e/external_oidc_test.go +++ b/test/e2e/external_oidc_test.go @@ -5,8 +5,6 @@ package e2e import ( "context" "os" - "slices" - "strings" "testing" . "github.com/onsi/gomega" @@ -24,6 +22,52 @@ import ( "github.com/openshift/hypershift/control-plane-operator/featuregates" ) +// TestExternalOIDC validates external OIDC authentication across feature gate configurations. +// +// Feature Gate Evolution: +// - ExternalOIDC: Base OIDC support (graduated to Default) +// - ExternalOIDCWithUIDAndExtraClaimMappings: Adds UID expression + Extra mappings (graduated to Default) +// - ExternalOIDCWithUpstreamParity: Adds CEL expressions, validation rules (currently TechPreviewNoUpgrade) +// +// Test Execution Matrix: +// +// 1. Main test execution (lines 49-120): +// - Always runs with feature-gate-driven auth config +// - Default feature set: Static claim mappings with prefixes + UID/Extra +// - TechPreviewNoUpgrade: CEL expressions (no prefixes) + validation rules + UID/Extra +// +// 2. Custom config test execution (lines 122-154): +// - Only runs when ExternalOIDCWithUpstreamParity enabled +// - Uses CustomizeAuthSpec to override UID (claim) and Extra (1 mapping) +// - Preserves CEL expressions and validation rules from feature gate +// - Demonstrates configuration customization for testing variants +// +// Auth Config by Feature Set: +// +// Default: +// +// Username: Static claim "email" WITH prefix +// Groups: Static claim "groups" WITH prefix +// UID: Expression ("testuid-" + claims.sub + "-uidtest") +// Extra: 2 mappings (bar, foo) +// +// TechPreviewNoUpgrade: +// +// Username: CEL expression (claims.email.split('@')[0]) - NO prefix +// Groups: CEL expression (claims.?groups.orValue([])) - NO prefix +// UID: Expression (same as Default) +// Extra: 2 mappings (same as Default) +// ClaimValidationRules: email exists, email_verified +// UserValidationRules: no system: prefix, no 'forbidden' word +// +// Custom Config (TechPreviewNoUpgrade + CustomizeAuthSpec): +// +// Username: CEL expression (preserved from feature gate) +// Groups: CEL expression (preserved from feature gate) +// UID: Claim "sub" (OVERRIDDEN by CustomizeAuthSpec) +// Extra: 1 mapping (REPLACED by CustomizeAuthSpec) +// ClaimValidationRules: (preserved from feature gate) +// UserValidationRules: (preserved from feature gate) func TestExternalOIDC(t *testing.T) { e2eutil.AtLeast(t, e2eutil.Version419) @@ -71,394 +115,95 @@ func TestExternalOIDC(t *testing.T) { 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) - // Since this username/group behavior differs between ExternalODICWithUIDandExtraClaimMappings and - // ExternalOIDCWithUpstreamParity feature gates, we should put this test behind a feature - // gate check. - if !featuregates.Gate().Enabled(featuregates.ExternalOIDCWithUpstreamParity) { - t.Run("[OCPFeatureGate:ExternalOIDCWithUIDAndExtraClaimMappings] Test external OIDC userInfo username", func(t *testing.T) { - g := NewWithT(t) - 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)) - }) - - t.Run("[OCPFeatureGate:ExternalOIDCWithUIDAndExtraClaimMappings] Test external OIDC userInfo Groups", func(t *testing.T) { - g := NewWithT(t) - 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))) - }) - } - - t.Run("[OCPFeatureGate:ExternalOIDCWithUIDAndExtraClaimMappings] Test external OIDC userInfo UID", func(t *testing.T) { - g := NewWithT(t) - 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)) - g.Expect(selfSubjectReview.Status.UserInfo.UID).Should(ContainSubstring(e2eutil.ExternalOIDCUIDExpressionSubfix)) + // Legacy prefixed mappings test only runs when ExternalOIDCWithUpstreamParity is NOT enabled + // Config: Username/Groups use static claims with prefixes (claim: "email", prefix: "prefix-") + // When ExternalOIDCWithUpstreamParity IS enabled, it overwrites with CEL expressions (no prefixes) + if !featuregates.Gate().Enabled(featuregates.ExternalOIDCWithUpstreamParity) { + t.Run("[OCPFeatureGate:ExternalOIDCWithUIDAndExtraClaimMappings] Test external OIDC userInfo username and groups", func(t *testing.T) { + e2eutil.TestLegacyPrefixedMappings(t, ctx, hostedCluster, kc, clientCfg, clusterOpts.ExtOIDCConfig, selfSubjectReview) }) + } - t.Run("[OCPFeatureGate:ExternalOIDCWithUIDAndExtraClaimMappings] Test external OIDC userInfo Extra", func(t *testing.T) { - g := NewWithT(t) - 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)) - g.Expect(selfSubjectReview.Status.UserInfo.Extra).Should(HaveKey(e2eutil.ExternalOIDCExtraKeyFoo)) - }) + // 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 and Extra", func(t *testing.T) { + e2eutil.TestUIDAndExtraMappings(t, ctx, selfSubjectReview) + }) - 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) - g.Expect(err).NotTo(HaveOccurred()) - _, err = client.ConfigV1().ClusterOperators().Get(ctx, "image-registry", metav1.GetOptions{}) - g.Expect(err).To(HaveOccurred()) - }) - } + 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) + g.Expect(err).NotTo(HaveOccurred()) + _, err = client.ConfigV1().ClusterOperators().Get(ctx, "image-registry", metav1.GetOptions{}) + g.Expect(err).To(HaveOccurred()) + }) + // ExternalOIDCWithUpstreamParity is currently TechPreviewNoUpgrade only + // 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) { t.Run("[OCPFeatureGate:ExternalOIDCWithUpstreamParity] Test CEL username expression mapping", func(t *testing.T) { - g := NewWithT(t) - t.Logf("begin to test CEL username expression mapping") - - // Setup: Create test resources with automatic cleanup - testResources, err := e2eutil.NewTestResources(ctx, kc, clusterOpts.ExtOIDCConfig) - g.Expect(err).NotTo(HaveOccurred()) - defer testResources.Cleanup(ctx, t) - - // Setup: Create authenticated test user with group - testUser, err := testResources.SetupAuthenticatedUserWithGroup(ctx, t, "cel-test-user", "cel-test-group", clientCfg, clusterOpts.ExtOIDCConfig) - g.Expect(err).NotTo(HaveOccurred()) - - // Verify: Username is email prefix (before @) - expectedUsername := strings.Split(testUser.Email, "@")[0] - g.Expect(testUser.SelfSubjectReview.Status.UserInfo.Username).Should(Equal(expectedUsername), - "username should be email prefix from CEL expression: claims.email.split('@')[0]") - g.Expect(testUser.SelfSubjectReview.Status.UserInfo.Username).NotTo(ContainSubstring("@"), - "username should not contain @ symbol") - t.Logf("CEL username expression correctly mapped '%s' to '%s'", testUser.Email, testUser.SelfSubjectReview.Status.UserInfo.Username) - - // Edge case test: preferred_username vs email-derived username mismatch - // Tests that CEL expression uses claims.email, not claims.preferred_username - t.Logf("Edge case test: Creating user with preferred_username different from email local part") - preferredUsername := "cel-preferred-" + e2eutil.GenerateRandomPassword(8) - actualEmail := "cel-email-" + e2eutil.GenerateRandomPassword(8) + "@test.example.com" - preferredPassword := e2eutil.GenerateRandomPassword(16) - - // In Keycloak, the 'username' field becomes the 'preferred_username' claim - // So we create a user where username != email local part - _, err = testResources.CreateTestUser(ctx, t, preferredUsername, actualEmail, preferredPassword) - g.Expect(err).NotTo(HaveOccurred()) - t.Logf("Created user: preferred_username='%s', email='%s'", preferredUsername, actualEmail) - - // Authenticate as this user - preferredAuthConfig := *clusterOpts.ExtOIDCConfig - preferredAuthConfig.TestUsers = preferredUsername + ":" + preferredPassword - preferredKubeConfig := e2eutil.ChangeUserForKeycloakExtOIDC(t, ctx, clientCfg, &preferredAuthConfig) - preferredAuthClient, err := kauthnv1typedclient.NewForConfig(preferredKubeConfig) - g.Expect(err).NotTo(HaveOccurred()) - - preferredReview, err := preferredAuthClient.SelfSubjectReviews().Create(ctx, &kauthnv1.SelfSubjectReview{}, metav1.CreateOptions{}) - g.Expect(err).NotTo(HaveOccurred()) - - // Verify: K8s username should come from email claim, NOT preferred_username claim - expectedUsername = strings.Split(actualEmail, "@")[0] - g.Expect(preferredReview.Status.UserInfo.Username).Should(Equal(expectedUsername), - "username should be derived from email claim via CEL expression, not from preferred_username claim") - g.Expect(preferredReview.Status.UserInfo.Username).NotTo(Equal(preferredUsername), - "username should NOT equal preferred_username when they differ") - t.Logf("✓ CEL expression correctly used email claim over preferred_username: K8s username='%s' (from email='%s'), preferred_username='%s'", - preferredReview.Status.UserInfo.Username, actualEmail, preferredUsername) - - // Verify: Groups are mapped without prefix - g.Expect(testUser.SelfSubjectReview.Status.UserInfo.Groups).NotTo(BeEmpty(), - "user should have groups from Keycloak") - hasTestGroup := false - for _, group := range testUser.SelfSubjectReview.Status.UserInfo.Groups { - if group == testUser.GroupName { - hasTestGroup = true - } - g.Expect(group).NotTo(HavePrefix(clusterOpts.ExtOIDCConfig.GroupPrefix), - "groups should not have prefix when using CEL expression") - } - g.Expect(hasTestGroup).To(BeTrue(), "user should be member of test group: %s", testUser.GroupName) - t.Logf("CEL groups expression correctly mapped groups: %v", testUser.SelfSubjectReview.Status.UserInfo.Groups) + e2eutil.TestCELUsernameMapping(t, ctx, hostedCluster, kc, clientCfg, clusterOpts.ExtOIDCConfig) }) t.Run("[OCPFeatureGate:ExternalOIDCWithUpstreamParity] Test CEL groups expression mapping", func(t *testing.T) { - g := NewWithT(t) - t.Logf("begin to test CEL groups expression mapping") - - // Setup: Create test resources with automatic cleanup - testResources, err := e2eutil.NewTestResources(ctx, kc, clusterOpts.ExtOIDCConfig) - g.Expect(err).NotTo(HaveOccurred()) - defer testResources.Cleanup(ctx, t) - - // Verify: Groups expression is configured - g.Expect(hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].ClaimMappings.Groups.Expression).NotTo(BeEmpty()) - t.Logf("CEL groups expression configured: %s", hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].ClaimMappings.Groups.Expression) - - // Setup: Create authenticated test user with group - testUser, err := testResources.SetupAuthenticatedUserWithGroup(ctx, t, "cel-groups-test-user", "cel-groups-test-group", clientCfg, clusterOpts.ExtOIDCConfig) - g.Expect(err).NotTo(HaveOccurred()) - - // Verify: User has groups from Keycloak - g.Expect(testUser.SelfSubjectReview.Status.UserInfo.Groups).NotTo(BeEmpty(), - "user should have groups from Keycloak") - - // Verify: Groups are mapped without prefix (CEL expression removes prefix) - for _, group := range testUser.SelfSubjectReview.Status.UserInfo.Groups { - g.Expect(group).NotTo(HavePrefix(clusterOpts.ExtOIDCConfig.GroupPrefix), - "groups should not have prefix when using CEL expression") - } - - // Verify: User is member of the test group - hasTestGroup := slices.Contains(testUser.SelfSubjectReview.Status.UserInfo.Groups, testUser.GroupName) - g.Expect(hasTestGroup).To(BeTrue(), "user should be member of test group: %s", testUser.GroupName) - t.Logf("CEL groups expression successfully mapped groups without prefix: %v", testUser.SelfSubjectReview.Status.UserInfo.Groups) - - // Negative test: User without groups should still authenticate - // The CEL expression claims.?groups.orValue([]) handles missing groups claim gracefully - t.Logf("Negative test: Creating user without group membership") - noGroupUsername := "cel-no-groups-" + e2eutil.GenerateRandomPassword(8) - noGroupEmail := noGroupUsername + "@test.example.com" - noGroupPassword := e2eutil.GenerateRandomPassword(16) - _, err = testResources.CreateTestUser(ctx, t, noGroupUsername, noGroupEmail, noGroupPassword) - g.Expect(err).NotTo(HaveOccurred()) - - // Authenticate user without groups - should SUCCEED with empty groups - noGroupAuthConfig := *clusterOpts.ExtOIDCConfig - noGroupAuthConfig.TestUsers = noGroupUsername + ":" + noGroupPassword - noGroupKubeConfig := e2eutil.ChangeUserForKeycloakExtOIDC(t, ctx, clientCfg, &noGroupAuthConfig) - noGroupAuthClient, err := kauthnv1typedclient.NewForConfig(noGroupKubeConfig) - g.Expect(err).NotTo(HaveOccurred()) - - noGroupReview, err := noGroupAuthClient.SelfSubjectReviews().Create(ctx, &kauthnv1.SelfSubjectReview{}, metav1.CreateOptions{}) - g.Expect(err).NotTo(HaveOccurred(), "authentication should succeed even when user has no groups") - t.Logf("✓ User without groups authenticated successfully with groups: %v", noGroupReview.Status.UserInfo.Groups) + e2eutil.TestCELGroupsMapping(t, ctx, hostedCluster, kc, clientCfg, clusterOpts.ExtOIDCConfig) }) t.Run("[OCPFeatureGate:ExternalOIDCWithUpstreamParity] Test claim validation rules", func(t *testing.T) { - g := NewWithT(t) - t.Logf("begin to test claim validation rules") - - // Setup: Create test resources with automatic cleanup - testResources, err := e2eutil.NewTestResources(ctx, kc, clusterOpts.ExtOIDCConfig) - g.Expect(err).NotTo(HaveOccurred()) - defer testResources.Cleanup(ctx, t) - - // Verify: Claim validation rules are configured - g.Expect(hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].ClaimValidationRules).NotTo(BeEmpty()) - claimRules := hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].ClaimValidationRules - g.Expect(claimRules).Should(HaveLen(2)) - - // Verify: Rules are CEL type with expected expressions - g.Expect(claimRules[0].Type).Should(BeEquivalentTo(configv1.TokenValidationRuleTypeCEL)) - g.Expect(claimRules[0].CEL.Expression).Should(BeEquivalentTo(e2eutil.ClaimValidationExprEmailExists)) - g.Expect(claimRules[1].Type).Should(BeEquivalentTo(configv1.TokenValidationRuleTypeCEL)) - g.Expect(claimRules[1].CEL.Expression).Should(BeEquivalentTo(e2eutil.ClaimValidationExprEmailVerified)) - - // Test 1: Valid user - email exists and email_verified=true - // Should PASS validation and authenticate successfully - t.Logf("Test 1: Creating user with valid claims (email exists, email_verified=true)") - validUser, err := testResources.SetupAuthenticatedUserWithGroup(ctx, t, "claim-valid-user", "claim-valid-group", clientCfg, clusterOpts.ExtOIDCConfig) - g.Expect(err).NotTo(HaveOccurred(), "authentication must succeed when all claim validation rules pass") - g.Expect(validUser.Email).NotTo(BeEmpty(), "test user email must be non-empty") - t.Logf("✓ User with email='%s' and email_verified=true authenticated successfully", validUser.Email) - - // Test 2: Invalid user - email_verified=false - // Demonstrates rule 2 requirement: claims.email_verified == true - t.Logf("Test 2: Creating user with email_verified=false (violates rule 2)") - invalidUsername := "claim-invalid-user-" + e2eutil.GenerateRandomPassword(8) - invalidEmail := invalidUsername + "@test.example.com" - invalidPassword := e2eutil.GenerateRandomPassword(16) - invalidUserID, err := testResources.CreateTestUserWithEmailVerification(ctx, t, invalidUsername, invalidEmail, invalidPassword, false) - g.Expect(err).NotTo(HaveOccurred(), "creating user in Keycloak should succeed") - g.Expect(invalidEmail).NotTo(BeEmpty(), "test user has non-empty email but email_verified=false") - t.Logf("✓ Created user '%s' with email='%s' and email_verified=false (ID: %s)", invalidUsername, invalidEmail, invalidUserID) - - // Attempt to authenticate - should FAIL due to claim validation rule 2 - err = testResources.TryAuthenticateUser(ctx, t, invalidUsername, invalidPassword, clientCfg, clusterOpts.ExtOIDCConfig) - g.Expect(err).To(HaveOccurred(), "authentication must fail when email_verified=false") - t.Logf("✓ User with email_verified=false correctly rejected: %v", err) - - // Test 3: Invalid - empty email (violates rule 1) - // Demonstrates rule 1 requirement: has(claims.email) && claims.email != '' - t.Logf("Test 3: Creating user with empty email (violates rule 1)") - emptyEmailUsername := "claim-empty-email-" + e2eutil.GenerateRandomPassword(8) - emptyEmailPassword := e2eutil.GenerateRandomPassword(16) - emptyEmailUserID, err := testResources.CreateTestUserWithEmailVerification(ctx, t, emptyEmailUsername, "", emptyEmailPassword, true) - g.Expect(err).NotTo(HaveOccurred(), "creating user in Keycloak should succeed") - t.Logf("✓ Created user '%s' with email='' and email_verified=true (ID: %s)", emptyEmailUsername, emptyEmailUserID) - - // Attempt to authenticate - should FAIL due to claim validation rule 1 - err = testResources.TryAuthenticateUser(ctx, t, emptyEmailUsername, emptyEmailPassword, clientCfg, clusterOpts.ExtOIDCConfig) - g.Expect(err).To(HaveOccurred(), "authentication must fail when email is empty") - g.Expect(err.Error()).Should(ContainSubstring("Unauthorized"), - "empty email user cannot authenticate as it violates user validation rule") - t.Logf("✓ User with empty email correctly rejected: %v", err) - - t.Logf("Claim validation rules successfully validated: only users with non-empty email and email_verified=true can authenticate") + e2eutil.TestClaimValidationRules(t, ctx, hostedCluster, kc, clientCfg, clusterOpts.ExtOIDCConfig) }) t.Run("[OCPFeatureGate:ExternalOIDCWithUpstreamParity] Test user validation rules", func(t *testing.T) { - g := NewWithT(t) - t.Logf("begin to test user validation rules") - - // Setup: Create test resources with automatic cleanup - testResources, err := e2eutil.NewTestResources(ctx, kc, clusterOpts.ExtOIDCConfig) - g.Expect(err).NotTo(HaveOccurred()) - defer testResources.Cleanup(ctx, t) - - // Verify: User validation rules are configured - g.Expect(hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].UserValidationRules).NotTo(BeEmpty()) - userRules := hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].UserValidationRules - g.Expect(userRules).Should(HaveLen(2), "should have two user validation rules") - - // Verify: Rules use expected CEL expressions - expressions := []string{userRules[0].Expression, userRules[1].Expression} - g.Expect(expressions).Should(ContainElement(e2eutil.UserValidationExprNoSystemPrefix)) - g.Expect(expressions).Should(ContainElement(e2eutil.UserValidationExprNoForbiddenWord)) - t.Logf("User validation rules configured: %v", expressions) - - // Test 1: Valid user - passes all validation rules - // Should PASS validation and authenticate successfully - t.Logf("Test 1: Creating user with valid username (no system: prefix, no 'forbidden' word)") - validUser, err := testResources.SetupAuthenticatedUserWithGroup(ctx, t, "user-valid", "user-valid-group", clientCfg, clusterOpts.ExtOIDCConfig) - g.Expect(err).NotTo(HaveOccurred(), "authentication must succeed when all validation rules pass") - - // Username is derived from email via CEL: claims.email.split('@')[0] - expectedUsername := strings.Split(validUser.Email, "@")[0] - g.Expect(validUser.SelfSubjectReview.Status.UserInfo.Username).Should(Equal(expectedUsername)) - g.Expect(validUser.SelfSubjectReview.Status.UserInfo.Username).NotTo(HavePrefix("system:")) - g.Expect(validUser.SelfSubjectReview.Status.UserInfo.Username).NotTo(ContainSubstring("forbidden")) - t.Logf("✓ User with username='%s' authenticated successfully", validUser.SelfSubjectReview.Status.UserInfo.Username) - - // Test 2: Invalid user - username contains "forbidden" - // Demonstrates the testable user validation rule: !user.username.contains('forbidden') - t.Logf("Test 2: Creating user with 'forbidden' in username (violates user validation rule)") - forbiddenUsername := "user-forbidden-" + e2eutil.GenerateRandomPassword(8) - forbiddenEmail := forbiddenUsername + "@test.example.com" - forbiddenPassword := e2eutil.GenerateRandomPassword(16) - forbiddenUserID, err := testResources.CreateTestUser(ctx, t, forbiddenUsername, forbiddenEmail, forbiddenPassword) - g.Expect(err).NotTo(HaveOccurred(), "creating user in Keycloak should succeed") - t.Logf("Created user with email='%s', mapped username will be '%s' (ID: %s)", forbiddenEmail, forbiddenUsername, forbiddenUserID) - - // Try to authenticate - should FAIL due to user validation rule - err = testResources.TryAuthenticateUser(ctx, t, forbiddenUsername, forbiddenPassword, clientCfg, clusterOpts.ExtOIDCConfig) - g.Expect(err).To(HaveOccurred(), "authentication must fail when username contains 'forbidden'") - g.Expect(err.Error()).Should(ContainSubstring("Unauthorized"), - "forbidden user cannot authenticate as it violates user validation rule") - t.Logf("✓ User with 'forbidden' in username correctly rejected with error: %v", err) - - // NOTE: We cannot test the negative case for the system: prefix rule via Keycloak - // because RFC 5322 email addresses do not allow colons in the local part. - // Since username = claims.email.split('@')[0], we would need an email like - // "system:admin@test.example.com", which is invalid per email standards. - // The system: prefix rule should be tested via unit tests or envtest where claims can be mocked. - - t.Logf("User validation rules successfully validated: users with 'forbidden' in username are rejected") + e2eutil.TestUserValidationRules(t, ctx, hostedCluster, kc, clientCfg, clusterOpts.ExtOIDCConfig) }) } }).Execute(&clusterOpts, globalOpts.Platform, globalOpts.ArtifactDir, "external-oidc", globalOpts.ServiceAccountSigningKey) - // experiment: see if we can pass custom auth config to test here - customClusterOpts := clusterOpts - customClusterOpts.ExtOIDCConfig.CustomizeAuthSpec = func(spec *configv1.AuthenticationSpec) { - // Customize UID mapping to use claim instead of expression - spec.OIDCProviders[0].ClaimMappings.UID = &configv1.TokenClaimOrExpressionMapping{ - Claim: "sub", - } - // Keep only one extra mapping instead of two - spec.OIDCProviders[0].ClaimMappings.Extra = []configv1.ExtraMapping{ - { - Key: "extratest.openshift.com/foo", - ValueExpression: "claims.email", - }, - } - } - // currently is still under feature gate - having the tests - // run like this mean easy removal of feature gate check - // when they graduate to default feature set. + // Custom config test - Tests configuration customization via CustomizeAuthSpec + // Demonstrates that CustomizeAuthSpec can override specific fields while keeping others + // Base config from ExternalOIDCWithUpstreamParity feature gate includes: + // - CEL username/groups expressions + // - Claim validation rules + // - User validation rules + // CustomizeAuthSpec OVERRIDES: + // - UID: Changes from expression to claim-based (Claim: "sub") + // - Extra: Reduces from 2 mappings to 1 mapping + // CustomizeAuthSpec PRESERVES (does not touch): + // - Username/Groups CEL expressions + // - Claim validation rules (still present, that's why TestUserValidationRules works) + // - User validation rules (still present, that's why TestUserValidationRules works) if featuregates.Gate().Enabled(featuregates.ExternalOIDCWithUpstreamParity) { + customClusterOpts := clusterOpts + customClusterOpts.ExtOIDCConfig.CustomizeAuthSpec = e2eutil.AuthConfigCombined( + e2eutil.AuthConfigUIDFromClaim(), + e2eutil.AuthConfigMinimalExtraMappings(), + ) + e2eutil.NewHypershiftTest(t, ctx, func(t *testing.T, g Gomega, mgtClient crclient.Client, hostedCluster *hyperv1.HostedCluster) { 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) - 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) + kc, err := e2eutil.SetupKeycloakAdminClientFromCluster(ctx, t, mgtClient, customClusterOpts.ExtOIDCConfig) if err != nil { t.Skipf("Could not setup Keycloak admin client: %v", err) } - t.Run("[OCPFeatureGate:ExternalOIDCWithUpstreamParity] Test user validation rules 1", func(t *testing.T) { - g := NewWithT(t) - t.Logf("begin to test user validation rules") - - // Setup: Create test resources with automatic cleanup - testResources, err := e2eutil.NewTestResources(ctx, kc, clusterOpts.ExtOIDCConfig) - g.Expect(err).NotTo(HaveOccurred()) - defer testResources.Cleanup(ctx, t) - - // Verify: User validation rules are configured - g.Expect(hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].UserValidationRules).NotTo(BeEmpty()) - userRules := hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].UserValidationRules - g.Expect(userRules).Should(HaveLen(2), "should have two user validation rules") - - // Verify: Rules use expected CEL expressions - expressions := []string{userRules[0].Expression, userRules[1].Expression} - g.Expect(expressions).Should(ContainElement(e2eutil.UserValidationExprNoSystemPrefix)) - g.Expect(expressions).Should(ContainElement(e2eutil.UserValidationExprNoForbiddenWord)) - t.Logf("User validation rules configured: %v", expressions) - - // Test 1: Valid user - passes all validation rules - // Should PASS validation and authenticate successfully - t.Logf("Test 1: Creating user with valid username (no system: prefix, no 'forbidden' word)") - validUser, err := testResources.SetupAuthenticatedUserWithGroup(ctx, t, "user-valid", "user-valid-group", clientCfg, clusterOpts.ExtOIDCConfig) - g.Expect(err).NotTo(HaveOccurred(), "authentication must succeed when all validation rules pass") - - // Username is derived from email via CEL: claims.email.split('@')[0] - expectedUsername := strings.Split(validUser.Email, "@")[0] - g.Expect(validUser.SelfSubjectReview.Status.UserInfo.Username).Should(Equal(expectedUsername)) - g.Expect(validUser.SelfSubjectReview.Status.UserInfo.Username).NotTo(HavePrefix("system:")) - g.Expect(validUser.SelfSubjectReview.Status.UserInfo.Username).NotTo(ContainSubstring("forbidden")) - t.Logf("✓ User with username='%s' authenticated successfully", validUser.SelfSubjectReview.Status.UserInfo.Username) - - // Test 2: Invalid user - username contains "forbidden" - // Demonstrates the testable user validation rule: !user.username.contains('forbidden') - t.Logf("Test 2: Creating user with 'forbidden' in username (violates user validation rule)") - forbiddenUsername := "user-forbidden-" + e2eutil.GenerateRandomPassword(8) - forbiddenEmail := forbiddenUsername + "@test.example.com" - forbiddenPassword := e2eutil.GenerateRandomPassword(16) - forbiddenUserID, err := testResources.CreateTestUser(ctx, t, forbiddenUsername, forbiddenEmail, forbiddenPassword) - g.Expect(err).NotTo(HaveOccurred(), "creating user in Keycloak should succeed") - t.Logf("Created user with email='%s', mapped username will be '%s' (ID: %s)", forbiddenEmail, forbiddenUsername, forbiddenUserID) - - // Try to authenticate - should FAIL due to user validation rule - err = testResources.TryAuthenticateUser(ctx, t, forbiddenUsername, forbiddenPassword, clientCfg, clusterOpts.ExtOIDCConfig) - g.Expect(err).To(HaveOccurred(), "authentication must fail when username contains 'forbidden'") - g.Expect(err.Error()).Should(ContainSubstring("Unauthorized"), - "forbidden user cannot authenticate as it violates user validation rule") - t.Logf("✓ User with 'forbidden' in username correctly rejected with error: %v", err) - - // NOTE: We cannot test the negative case for the system: prefix rule via Keycloak - // because RFC 5322 email addresses do not allow colons in the local part. - // Since username = claims.email.split('@')[0], we would need an email like - // "system:admin@test.example.com", which is invalid per email standards. - // The system: prefix rule should be tested via unit tests or envtest where claims can be mocked. - t.Logf("User validation rules successfully validated: users with 'forbidden' in username are rejected") + t.Run("[OCPFeatureGate:ExternalOIDCWithUpstreamParity] Test user validation rules (custom UID config)", func(t *testing.T) { + // This test works because validation rules are preserved from feature gate config + // Only UID and Extra mappings were customized + e2eutil.TestUserValidationRules(t, ctx, hostedCluster, kc, clientCfg, customClusterOpts.ExtOIDCConfig) }) - }).Execute(&customClusterOpts, globalOpts.Platform, globalOpts.ArtifactDir, "ext-oidc-usr-rules", globalOpts.ServiceAccountSigningKey) + }).Execute(&customClusterOpts, globalOpts.Platform, globalOpts.ArtifactDir, "ext-oidc-uid-claim", globalOpts.ServiceAccountSigningKey) } } diff --git a/test/e2e/util/external_oidc.go b/test/e2e/util/external_oidc.go index f71597ab3439..77bd2cd2588b 100644 --- a/test/e2e/util/external_oidc.go +++ b/test/e2e/util/external_oidc.go @@ -220,6 +220,47 @@ func (config *ExtOIDCConfig) GetAuthenticationConfig() *configv1.AuthenticationS return authnSpec } +// AuthConfigDefault returns nil (no customization - uses feature-gate-driven defaults). +// When it should use default auth configuration without customization +func AuthConfigDefault() func(*configv1.AuthenticationSpec) { + return nil +} + +// AuthConfigUIDFromClaim customizes auth spec to use claim-based UID instead of expression. +// When it should customize UID mapping to use claim instead of expression +func AuthConfigUIDFromClaim() func(*configv1.AuthenticationSpec) { + return func(spec *configv1.AuthenticationSpec) { + spec.OIDCProviders[0].ClaimMappings.UID = &configv1.TokenClaimOrExpressionMapping{ + Claim: "sub", + } + } +} + +// AuthConfigMinimalExtraMappings reduces extra mappings to a single test mapping. +// When it should customize extra mappings to use minimal configuration +func AuthConfigMinimalExtraMappings() func(*configv1.AuthenticationSpec) { + return func(spec *configv1.AuthenticationSpec) { + spec.OIDCProviders[0].ClaimMappings.Extra = []configv1.ExtraMapping{ + { + Key: "extratest.openshift.com/foo", + ValueExpression: "claims.email", + }, + } + } +} + +// AuthConfigCombined combines multiple customization functions. +// When it should combine multiple auth spec customizations +func AuthConfigCombined(customizers ...func(*configv1.AuthenticationSpec)) func(*configv1.AuthenticationSpec) { + return func(spec *configv1.AuthenticationSpec) { + for _, customize := range customizers { + if customize != nil { + customize(spec) + } + } + } +} + // ValidateAuthenticationSpec validates the external OIDC configuration and the expected HostedCluster authentication configuration before running the test func ValidateAuthenticationSpec(t testing.TB, ctx context.Context, client crclient.Client, hostedCluster *hyperv1.HostedCluster, config *ExtOIDCConfig) { g := NewWithT(t) diff --git a/test/e2e/util/external_oidc_test_helpers.go b/test/e2e/util/external_oidc_test_helpers.go new file mode 100644 index 000000000000..49f360043196 --- /dev/null +++ b/test/e2e/util/external_oidc_test_helpers.go @@ -0,0 +1,310 @@ +package util + +import ( + "context" + "slices" + "strings" + "testing" + + . "github.com/onsi/gomega" + + hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" + + kauthnv1 "k8s.io/api/authentication/v1" + "k8s.io/client-go/rest" +) + +// TestCELUsernameMapping validates that CEL username expression correctly maps email to username. +// When it should use CEL username expression mapping +func TestCELUsernameMapping(t *testing.T, ctx context.Context, + hostedCluster *hyperv1.HostedCluster, + kc *KeycloakAdminClient, + clientCfg *rest.Config, + extOIDCConfig *ExtOIDCConfig) { + + g := NewWithT(t) + t.Logf("begin to test CEL username expression mapping") + + // Setup: Create test resources with automatic cleanup + testResources, err := NewTestResources(ctx, kc, extOIDCConfig) + g.Expect(err).NotTo(HaveOccurred()) + defer testResources.Cleanup(ctx, t) + + // Setup: Create authenticated test user with group + testUser, err := testResources.SetupAuthenticatedUserWithGroup(ctx, t, "cel-test-user", "cel-test-group", clientCfg, extOIDCConfig) + g.Expect(err).NotTo(HaveOccurred()) + + // Verify: Username is email prefix (before @) + expectedUsername := strings.Split(testUser.Email, "@")[0] + g.Expect(testUser.SelfSubjectReview.Status.UserInfo.Username).Should(Equal(expectedUsername), + "username should be email prefix from CEL expression: claims.email.split('@')[0]") + g.Expect(testUser.SelfSubjectReview.Status.UserInfo.Username).NotTo(ContainSubstring("@"), + "username should not contain @ symbol") + t.Logf("CEL username expression correctly mapped '%s' to '%s'", testUser.Email, testUser.SelfSubjectReview.Status.UserInfo.Username) + + // Edge case test: preferred_username vs email-derived username mismatch + // Tests that CEL expression uses claims.email, not claims.preferred_username + t.Logf("Edge case test: Creating user with preferred_username different from email local part") + preferredUsername := "cel-preferred-" + GenerateRandomPassword(8) + actualEmail := "cel-email-" + GenerateRandomPassword(8) + "@test.example.com" + preferredPassword := GenerateRandomPassword(16) + + // In Keycloak, the 'username' field becomes the 'preferred_username' claim + // So we create a user where username != email local part + _, err = testResources.CreateTestUser(ctx, t, preferredUsername, actualEmail, preferredPassword) + g.Expect(err).NotTo(HaveOccurred()) + t.Logf("Created user: preferred_username='%s', email='%s'", preferredUsername, actualEmail) + + // Authenticate as this user + preferredAuthConfig := *extOIDCConfig + preferredAuthConfig.TestUsers = preferredUsername + ":" + preferredPassword + preferredReview, err := testResources.AuthenticateAndGetSelfSubjectReview(ctx, t, preferredUsername, preferredPassword, clientCfg, &preferredAuthConfig) + g.Expect(err).NotTo(HaveOccurred()) + + // Verify: K8s username should come from email claim, NOT preferred_username claim + expectedUsername = strings.Split(actualEmail, "@")[0] + g.Expect(preferredReview.Status.UserInfo.Username).Should(Equal(expectedUsername), + "username should be derived from email claim via CEL expression, not from preferred_username claim") + g.Expect(preferredReview.Status.UserInfo.Username).NotTo(Equal(preferredUsername), + "username should NOT equal preferred_username when they differ") + t.Logf("✓ CEL expression correctly used email claim over preferred_username: K8s username='%s' (from email='%s'), preferred_username='%s'", + preferredReview.Status.UserInfo.Username, actualEmail, preferredUsername) + + // Verify: Groups are mapped without prefix + g.Expect(testUser.SelfSubjectReview.Status.UserInfo.Groups).NotTo(BeEmpty(), + "user should have groups from Keycloak") + hasTestGroup := false + for _, group := range testUser.SelfSubjectReview.Status.UserInfo.Groups { + if group == testUser.GroupName { + hasTestGroup = true + } + g.Expect(group).NotTo(HavePrefix(extOIDCConfig.GroupPrefix), + "groups should not have prefix when using CEL expression") + } + g.Expect(hasTestGroup).To(BeTrue(), "user should be member of test group: %s", testUser.GroupName) + t.Logf("CEL groups expression correctly mapped groups: %v", testUser.SelfSubjectReview.Status.UserInfo.Groups) +} + +// TestCELGroupsMapping validates that CEL groups expression correctly maps groups without prefix. +// When it should test CEL groups expression mapping +func TestCELGroupsMapping(t *testing.T, ctx context.Context, + hostedCluster *hyperv1.HostedCluster, + kc *KeycloakAdminClient, + clientCfg *rest.Config, + extOIDCConfig *ExtOIDCConfig) { + + g := NewWithT(t) + t.Logf("begin to test CEL groups expression mapping") + + // Setup: Create test resources with automatic cleanup + testResources, err := NewTestResources(ctx, kc, extOIDCConfig) + g.Expect(err).NotTo(HaveOccurred()) + defer testResources.Cleanup(ctx, t) + + // Verify: Groups expression is configured + g.Expect(hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].ClaimMappings.Groups.Expression).NotTo(BeEmpty()) + t.Logf("CEL groups expression configured: %s", hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].ClaimMappings.Groups.Expression) + + // Setup: Create authenticated test user with group + testUser, err := testResources.SetupAuthenticatedUserWithGroup(ctx, t, "cel-groups-test-user", "cel-groups-test-group", clientCfg, extOIDCConfig) + g.Expect(err).NotTo(HaveOccurred()) + + // Verify: User has groups from Keycloak + g.Expect(testUser.SelfSubjectReview.Status.UserInfo.Groups).NotTo(BeEmpty(), + "user should have groups from Keycloak") + + // Verify: Groups are mapped without prefix (CEL expression removes prefix) + for _, group := range testUser.SelfSubjectReview.Status.UserInfo.Groups { + g.Expect(group).NotTo(HavePrefix(extOIDCConfig.GroupPrefix), + "groups should not have prefix when using CEL expression") + } + + // Verify: User is member of the test group + hasTestGroup := slices.Contains(testUser.SelfSubjectReview.Status.UserInfo.Groups, testUser.GroupName) + g.Expect(hasTestGroup).To(BeTrue(), "user should be member of test group: %s", testUser.GroupName) + t.Logf("CEL groups expression successfully mapped groups without prefix: %v", testUser.SelfSubjectReview.Status.UserInfo.Groups) + + // Negative test: User without groups should still authenticate + // The CEL expression claims.?groups.orValue([]) handles missing groups claim gracefully + t.Logf("Negative test: Creating user without group membership") + noGroupUsername := "cel-no-groups-" + GenerateRandomPassword(8) + noGroupEmail := noGroupUsername + "@test.example.com" + noGroupPassword := GenerateRandomPassword(16) + _, err = testResources.CreateTestUser(ctx, t, noGroupUsername, noGroupEmail, noGroupPassword) + g.Expect(err).NotTo(HaveOccurred()) + + // Authenticate user without groups - should SUCCEED with empty groups + noGroupReview, err := testResources.AuthenticateAndGetSelfSubjectReview(ctx, t, noGroupUsername, noGroupPassword, clientCfg, extOIDCConfig) + g.Expect(err).NotTo(HaveOccurred(), "authentication should succeed even when user has no groups") + t.Logf("✓ User without groups authenticated successfully with groups: %v", noGroupReview.Status.UserInfo.Groups) +} + +// TestClaimValidationRules validates that claim validation rules reject invalid tokens. +// When it should test claim validation rules +func TestClaimValidationRules(t *testing.T, ctx context.Context, + hostedCluster *hyperv1.HostedCluster, + kc *KeycloakAdminClient, + clientCfg *rest.Config, + extOIDCConfig *ExtOIDCConfig) { + + g := NewWithT(t) + t.Logf("begin to test claim validation rules") + + // Setup: Create test resources with automatic cleanup + testResources, err := NewTestResources(ctx, kc, extOIDCConfig) + g.Expect(err).NotTo(HaveOccurred()) + defer testResources.Cleanup(ctx, t) + + // Verify: Claim validation rules are configured + g.Expect(hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].ClaimValidationRules).NotTo(BeEmpty()) + claimRules := hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].ClaimValidationRules + g.Expect(claimRules).Should(HaveLen(2)) + + // Verify: Rules are CEL type with expected expressions + g.Expect(claimRules[0].Type).Should(BeEquivalentTo("CEL")) + g.Expect(claimRules[0].CEL.Expression).Should(BeEquivalentTo(ClaimValidationExprEmailExists)) + g.Expect(claimRules[1].Type).Should(BeEquivalentTo("CEL")) + g.Expect(claimRules[1].CEL.Expression).Should(BeEquivalentTo(ClaimValidationExprEmailVerified)) + + // Test 1: Valid user - email exists and email_verified=true + // Should PASS validation and authenticate successfully + t.Logf("Test 1: Creating user with valid claims (email exists, email_verified=true)") + validUser, err := testResources.SetupAuthenticatedUserWithGroup(ctx, t, "claim-valid-user", "claim-valid-group", clientCfg, extOIDCConfig) + g.Expect(err).NotTo(HaveOccurred(), "authentication must succeed when all claim validation rules pass") + g.Expect(validUser.Email).NotTo(BeEmpty(), "test user email must be non-empty") + t.Logf("✓ User with email='%s' and email_verified=true authenticated successfully", validUser.Email) + + // Test 2: Invalid user - email_verified=false + // Demonstrates rule 2 requirement: claims.email_verified == true + t.Logf("Test 2: Creating user with email_verified=false (violates rule 2)") + invalidUsername := "claim-invalid-user-" + GenerateRandomPassword(8) + invalidEmail := invalidUsername + "@test.example.com" + invalidPassword := GenerateRandomPassword(16) + invalidUserID, err := testResources.CreateTestUserWithEmailVerification(ctx, t, invalidUsername, invalidEmail, invalidPassword, false) + g.Expect(err).NotTo(HaveOccurred(), "creating user in Keycloak should succeed") + g.Expect(invalidEmail).NotTo(BeEmpty(), "test user has non-empty email but email_verified=false") + t.Logf("✓ Created user '%s' with email='%s' and email_verified=false (ID: %s)", invalidUsername, invalidEmail, invalidUserID) + + // Attempt to authenticate - should FAIL due to claim validation rule 2 + err = testResources.TryAuthenticateUser(ctx, t, invalidUsername, invalidPassword, clientCfg, extOIDCConfig) + g.Expect(err).To(HaveOccurred(), "authentication must fail when email_verified=false") + t.Logf("✓ User with email_verified=false correctly rejected: %v", err) + + // Test 3: Invalid - empty email (violates rule 1) + // Demonstrates rule 1 requirement: has(claims.email) && claims.email != '' + t.Logf("Test 3: Creating user with empty email (violates rule 1)") + emptyEmailUsername := "claim-empty-email-" + GenerateRandomPassword(8) + emptyEmailPassword := GenerateRandomPassword(16) + emptyEmailUserID, err := testResources.CreateTestUserWithEmailVerification(ctx, t, emptyEmailUsername, "", emptyEmailPassword, true) + g.Expect(err).NotTo(HaveOccurred(), "creating user in Keycloak should succeed") + t.Logf("✓ Created user '%s' with email='' and email_verified=true (ID: %s)", emptyEmailUsername, emptyEmailUserID) + + // Attempt to authenticate - should FAIL due to claim validation rule 1 + err = testResources.TryAuthenticateUser(ctx, t, emptyEmailUsername, emptyEmailPassword, clientCfg, extOIDCConfig) + g.Expect(err).To(HaveOccurred(), "authentication must fail when email is empty") + g.Expect(err.Error()).Should(ContainSubstring("Unauthorized"), + "empty email user cannot authenticate as it violates user validation rule") + t.Logf("✓ User with empty email correctly rejected: %v", err) + + t.Logf("Claim validation rules successfully validated: only users with non-empty email and email_verified=true can authenticate") +} + +// TestUserValidationRules validates that user validation rules reject invalid usernames. +// When it should test user validation rules +func TestUserValidationRules(t *testing.T, ctx context.Context, + hostedCluster *hyperv1.HostedCluster, + kc *KeycloakAdminClient, + clientCfg *rest.Config, + extOIDCConfig *ExtOIDCConfig) { + + g := NewWithT(t) + t.Logf("begin to test user validation rules") + + // Setup: Create test resources with automatic cleanup + testResources, err := NewTestResources(ctx, kc, extOIDCConfig) + g.Expect(err).NotTo(HaveOccurred()) + defer testResources.Cleanup(ctx, t) + + // Verify: User validation rules are configured + g.Expect(hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].UserValidationRules).NotTo(BeEmpty()) + userRules := hostedCluster.Spec.Configuration.Authentication.OIDCProviders[0].UserValidationRules + g.Expect(userRules).Should(HaveLen(2), "should have two user validation rules") + + // Verify: Rules use expected CEL expressions + expressions := []string{userRules[0].Expression, userRules[1].Expression} + g.Expect(expressions).Should(ContainElement(UserValidationExprNoSystemPrefix)) + g.Expect(expressions).Should(ContainElement(UserValidationExprNoForbiddenWord)) + t.Logf("User validation rules configured: %v", expressions) + + // Test 1: Valid user - passes all validation rules + // Should PASS validation and authenticate successfully + t.Logf("Test 1: Creating user with valid username (no system: prefix, no 'forbidden' word)") + validUser, err := testResources.SetupAuthenticatedUserWithGroup(ctx, t, "user-valid", "user-valid-group", clientCfg, extOIDCConfig) + g.Expect(err).NotTo(HaveOccurred(), "authentication must succeed when all validation rules pass") + + // Username is derived from email via CEL: claims.email.split('@')[0] + expectedUsername := strings.Split(validUser.Email, "@")[0] + g.Expect(validUser.SelfSubjectReview.Status.UserInfo.Username).Should(Equal(expectedUsername)) + g.Expect(validUser.SelfSubjectReview.Status.UserInfo.Username).NotTo(HavePrefix("system:")) + g.Expect(validUser.SelfSubjectReview.Status.UserInfo.Username).NotTo(ContainSubstring("forbidden")) + t.Logf("✓ User with username='%s' authenticated successfully", validUser.SelfSubjectReview.Status.UserInfo.Username) + + // Test 2: Invalid user - username contains "forbidden" + // Demonstrates the testable user validation rule: !user.username.contains('forbidden') + t.Logf("Test 2: Creating user with 'forbidden' in username (violates user validation rule)") + forbiddenUsername := "user-forbidden-" + GenerateRandomPassword(8) + forbiddenEmail := forbiddenUsername + "@test.example.com" + forbiddenPassword := GenerateRandomPassword(16) + forbiddenUserID, err := testResources.CreateTestUser(ctx, t, forbiddenUsername, forbiddenEmail, forbiddenPassword) + g.Expect(err).NotTo(HaveOccurred(), "creating user in Keycloak should succeed") + t.Logf("Created user with email='%s', mapped username will be '%s' (ID: %s)", forbiddenEmail, forbiddenUsername, forbiddenUserID) + + // Try to authenticate - should FAIL due to user validation rule + err = testResources.TryAuthenticateUser(ctx, t, forbiddenUsername, forbiddenPassword, clientCfg, extOIDCConfig) + g.Expect(err).To(HaveOccurred(), "authentication must fail when username contains 'forbidden'") + g.Expect(err.Error()).Should(ContainSubstring("Unauthorized"), + "forbidden user cannot authenticate as it violates user validation rule") + t.Logf("✓ User with 'forbidden' in username correctly rejected with error: %v", err) + + // NOTE: We cannot test the negative case for the system: prefix rule via Keycloak + // because RFC 5322 email addresses do not allow colons in the local part. + // Since username = claims.email.split('@')[0], we would need an email like + // "system:admin@test.example.com", which is invalid per email standards. + // The system: prefix rule should be tested via unit tests or envtest where claims can be mocked. + + t.Logf("User validation rules successfully validated: users with 'forbidden' in username are rejected") +} + +// TestLegacyPrefixedMappings validates username/group prefix behavior from the legacy feature gate. +// When it should test legacy prefixed mappings (ExternalOIDCWithUIDAndExtraClaimMappings without ExternalOIDCWithUpstreamParity) +func TestLegacyPrefixedMappings(t *testing.T, ctx context.Context, + hostedCluster *hyperv1.HostedCluster, + kc *KeycloakAdminClient, + clientCfg *rest.Config, + extOIDCConfig *ExtOIDCConfig, + selfSubjectReview *kauthnv1.SelfSubjectReview) { + + g := NewWithT(t) + 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(extOIDCConfig.UserPrefix)) + + 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(extOIDCConfig.GroupPrefix))) +} + +// TestUIDAndExtraMappings validates UID expression and extra claim mappings. +// When it should test UID and extra claim mappings (ExternalOIDCWithUIDAndExtraClaimMappings) +func TestUIDAndExtraMappings(t *testing.T, ctx context.Context, selfSubjectReview *kauthnv1.SelfSubjectReview) { + g := NewWithT(t) + 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(ExternalOIDCUIDExpressionPrefix)) + g.Expect(selfSubjectReview.Status.UserInfo.UID).Should(ContainSubstring(ExternalOIDCUIDExpressionSubfix)) + + 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(ExternalOIDCExtraKeyBar)) + g.Expect(selfSubjectReview.Status.UserInfo.Extra).Should(HaveKey(ExternalOIDCExtraKeyFoo)) +} diff --git a/test/e2e/util/keycloak.go b/test/e2e/util/keycloak.go index e78c82ebe366..fc2bee0ae3a0 100644 --- a/test/e2e/util/keycloak.go +++ b/test/e2e/util/keycloak.go @@ -1009,3 +1009,33 @@ func (tr *TestResources) SetupAuthenticatedUserWithGroup( SelfSubjectReview: selfSubjectReview, }, nil } + +// AuthenticateAndGetSelfSubjectReview authenticates a user and returns their SelfSubjectReview +// When it should authenticate and get self subject review for a specific user +func (tr *TestResources) AuthenticateAndGetSelfSubjectReview( + ctx context.Context, + t *testing.T, + username string, + password string, + clientCfg *rest.Config, + extOIDCConfig *ExtOIDCConfig, +) (*kauthnv1.SelfSubjectReview, error) { + g := NewWithT(t) + + // Create a temporary auth config with these credentials + tempConfig := *extOIDCConfig + tempConfig.TestUsers = username + ":" + password + + // Authenticate as this user + authKubeConfig := ChangeUserForKeycloakExtOIDC(t, ctx, clientCfg, &tempConfig) + authClient, err := kauthnv1typedclient.NewForConfig(authKubeConfig) + g.Expect(err).NotTo(HaveOccurred()) + + // Get self subject review + selfSubjectReview, err := authClient.SelfSubjectReviews().Create(ctx, &kauthnv1.SelfSubjectReview{}, metav1.CreateOptions{}) + if err != nil { + return nil, fmt.Errorf("failed to get self subject review: %w", err) + } + + return selfSubjectReview, nil +}