Skip to content

OCPBUGS-81686: fix(authentication): use v2 auth validation for CEL and expression support. #8246

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-bot merged 1 commit into from
May 11, 2026
Merged

OCPBUGS-81686: fix(authentication): use v2 auth validation for CEL and expression support. #8246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions control-plane-operator/controllers/hostedcontrolplane/v2/kas/auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"net/url"
"strings"

hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1"
"github.com/openshift/hypershift/control-plane-operator/featuregates"
component "github.com/openshift/hypershift/support/controlplane-component"
"github.com/openshift/hypershift/support/supportedversion"
Expand Down Expand Up @@ -37,7 +36,7 @@ func adaptAuthConfig(cpContext component.WorkloadContext, config *corev1.ConfigM
return nil
}

authConfig, err := generateAuthConfig(cpContext, cpContext.Client, cpContext.HCP)
authConfig, err := GenerateAuthConfig(cpContext, cpContext.HCP.Spec.Configuration.Authentication, cpContext.Client, cpContext.HCP.Namespace)
if err != nil {
return fmt.Errorf("failed to generate authentication config: %w", err)
}
Expand All @@ -59,7 +58,11 @@ func adaptAuthConfig(cpContext component.WorkloadContext, config *corev1.ConfigM
return nil
}

func generateAuthConfig(ctx context.Context, c crclient.Reader, hcp *hyperv1.HostedControlPlane) (*AuthenticationConfiguration, error) {
func GenerateAuthConfig(ctx context.Context, spec *configv1.AuthenticationSpec, c crclient.Reader, namespace string) (*AuthenticationConfiguration, error) {
if spec == nil {
return nil, fmt.Errorf("authentication spec cannot be nil")
}

config := &AuthenticationConfiguration{
TypeMeta: metav1.TypeMeta{
Kind: "AuthenticationConfiguration",
Expand All @@ -68,8 +71,8 @@ func generateAuthConfig(ctx context.Context, c crclient.Reader, hcp *hyperv1.Hos
JWT: []JWTAuthenticator{},
}

for _, provider := range hcp.Spec.Configuration.Authentication.OIDCProviders {
jwt, err := generateJWTForProvider(ctx, provider, c, hcp.Namespace)
for _, provider := range spec.OIDCProviders {
jwt, err := generateJWTForProvider(ctx, provider, c, namespace)
if err != nil {
return nil, fmt.Errorf("generating JWT authenticator for provider %q: %v", provider.Name, err)
}
Expand Down
179 changes: 179 additions & 0 deletions control-plane-operator/controllers/hostedcontrolplane/v2/kas/auth_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package kas

import (
"context"
"encoding/json"
"testing"

Expand Down Expand Up @@ -60,6 +61,184 @@ MlubpsoEK2bYQDZskgDGCHI=
-----END CERTIFICATE-----
`

func TestGenerateAuthConfig(t *testing.T) {
Comment thread
ShazaAldawamneh marked this conversation as resolved.
type testCase struct {
name string
ctx context.Context
spec *configv1.AuthenticationSpec
client crclient.Reader
namespace string
expectedAuthenticationConfiguration *AuthenticationConfiguration
shouldError bool
featureGates []featuregate.Feature
}

testCases := []testCase{
{
name: "When authentication spec is nil, it should return an error",
ctx: context.Background(),
spec: nil,
client: nil,
namespace: "test",
shouldError: true,
},
{
name: "When valid OIDC provider is provided, it should generate valid authentication configuration",
ctx: context.Background(),
spec: &configv1.AuthenticationSpec{
OIDCProviders: []configv1.OIDCProvider{
{
Name: "test-provider",
Issuer: configv1.TokenIssuer{
URL: "https://test.example.com",
Audiences: []configv1.TokenAudience{
"test-audience",
},
},
ClaimMappings: configv1.TokenClaimMappings{
Username: configv1.UsernameClaimMapping{
Claim: "email",
PrefixPolicy: configv1.NoPrefix,
},
},
},
},
},
client: fake.NewClientBuilder().Build(),
namespace: "test-namespace",
expectedAuthenticationConfiguration: &AuthenticationConfiguration{
TypeMeta: metav1.TypeMeta{
APIVersion: "apiserver.config.k8s.io/v1alpha1",
Kind: "AuthenticationConfiguration",
},
JWT: []JWTAuthenticator{
{
Issuer: Issuer{
URL: "https://test.example.com",
AudienceMatchPolicy: AudienceMatchPolicyMatchAny,
Audiences: []string{"test-audience"},
},
ClaimMappings: ClaimMappings{
Username: PrefixedClaimOrExpression{
Prefix: ptr.To(""),
Claim: "email",
},
Groups: PrefixedClaimOrExpression{
Prefix: ptr.To(""),
Claim: "",
},
UID: ClaimOrExpression{
Claim: "sub",
},
Extra: []ExtraMapping{},
},
ClaimValidationRules: []ClaimValidationRule{},
},
},
},
shouldError: false,
},
{
name: "When OIDC provider with CEL validation is provided, it should generate configuration with validation rules",
ctx: context.Background(),
spec: &configv1.AuthenticationSpec{
OIDCProviders: []configv1.OIDCProvider{
{
Name: "test-provider",
Issuer: configv1.TokenIssuer{
URL: "https://test.example.com",
Audiences: []configv1.TokenAudience{
"test-audience",
},
},
ClaimValidationRules: []configv1.TokenClaimValidationRule{
{
Type: configv1.TokenValidationRuleTypeCEL,
CEL: configv1.TokenClaimValidationCELRule{
Expression: "claims.email_verified == true",
Message: "email must be verified",
},
},
},
ClaimMappings: configv1.TokenClaimMappings{
Username: configv1.UsernameClaimMapping{
Expression: "claims.email",
},
},
},
},
},
client: fake.NewClientBuilder().Build(),
namespace: "test-namespace",
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.example.com",
AudienceMatchPolicy: AudienceMatchPolicyMatchAny,
Audiences: []string{"test-audience"},
},
ClaimMappings: ClaimMappings{
Username: PrefixedClaimOrExpression{
Expression: "claims.email",
},
Groups: PrefixedClaimOrExpression{
Prefix: ptr.To(""),
Claim: "",
},
UID: ClaimOrExpression{
Claim: "sub",
},
Extra: []ExtraMapping{},
},
ClaimValidationRules: []ClaimValidationRule{
{
Expression: "claims.email_verified == true",
Message: "email must be verified",
},
},
UserValidationRules: []UserValidationRule{},
},
},
},
shouldError: false,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
if len(tc.featureGates) > 0 {
for _, feature := range tc.featureGates {
fgtesting.SetFeatureGateDuringTest(t, featuregates.Gate(), feature, true)
}
}

actualConfig, err := GenerateAuthConfig(tc.ctx, tc.spec, tc.client, tc.namespace)

switch {
case tc.shouldError && err == nil:
t.Fatal("expected an error to have occurred but got none")
case !tc.shouldError && err != nil:
t.Fatalf("unexpected error: %v", err)
case tc.shouldError && err != nil:
// as expected
return
}

if diff := cmp.Diff(tc.expectedAuthenticationConfiguration, actualConfig); diff != "" {
t.Fatalf("actual authentication configuration does not match expected (-want +got):\n%s", diff)
}
})
}
}

func TestAdaptAuthConfig(t *testing.T) {
type testCase struct {
name string
Expand Down
4 changes: 2 additions & 2 deletions support/validations/authentication.go
View file
Open in desktop
Comment thread
ShazaAldawamneh marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"context"
"fmt"

"github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/kas"
"github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/v2/kas"
"github.com/openshift/hypershift/support/supportedversion"

configv1 "github.com/openshift/api/config/v1"
Expand Down Expand Up @@ -43,7 +43,7 @@ func ValidateAuthenticationSpecForTypeOIDC(ctx context.Context, client crclient.
return nil
}

authConfig, err := kas.GenerateAuthConfig(authn, ctx, client, namespace)
authConfig, err := kas.GenerateAuthConfig(ctx, authn, client, namespace)
if err != nil {
return fmt.Errorf("generating structured authentication configuration: %w", err)
}
Expand Down
86 changes: 84 additions & 2 deletions support/validations/authentication_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,13 @@ package validations
import (
"testing"

"github.com/openshift/hypershift/control-plane-operator/featuregates"

configv1 "github.com/openshift/api/config/v1"

"k8s.io/component-base/featuregate"
fgtesting "k8s.io/component-base/featuregate/testing"

"github.com/stretchr/testify/require"
)

Expand All @@ -13,11 +18,12 @@ func TestValidateAuthenticationSpec(t *testing.T) {
name string
authentication *configv1.AuthenticationSpec
shouldError bool
featureGates []featuregate.Feature
}

testcases := []testcase{
{
name: "valid OIDC authentication config",
name: "When valid OIDC authentication config is provided, it should succeed",
authentication: &configv1.AuthenticationSpec{
Type: configv1.AuthenticationTypeOIDC,
OIDCProviders: []configv1.OIDCProvider{
Expand Down Expand Up @@ -60,7 +66,7 @@ func TestValidateAuthenticationSpec(t *testing.T) {
shouldError: false,
},
{
name: "invalid OIDC authentication config",
name: "When invalid OIDC authentication config is provided, it should return an error",
authentication: &configv1.AuthenticationSpec{
Type: configv1.AuthenticationTypeOIDC,
OIDCProviders: []configv1.OIDCProvider{
Expand Down Expand Up @@ -105,10 +111,86 @@ func TestValidateAuthenticationSpec(t *testing.T) {
},
shouldError: true,
},
{
name: "When OIDC provider has CEL validation and username expression, it should succeed",
featureGates: []featuregate.Feature{
featuregates.ExternalOIDCWithUpstreamParity,
},
authentication: &configv1.AuthenticationSpec{
Type: configv1.AuthenticationTypeOIDC,
OIDCProviders: []configv1.OIDCProvider{
{
Name: "test-provider",
Issuer: configv1.TokenIssuer{
URL: "https://test.example.com",
Audiences: []configv1.TokenAudience{
"test-audience",
},
},
ClaimValidationRules: []configv1.TokenClaimValidationRule{
{
Type: configv1.TokenValidationRuleTypeCEL,
CEL: configv1.TokenClaimValidationCELRule{
Expression: "claims.email_verified == true",
Message: "email must be verified",
},
},
},
ClaimMappings: configv1.TokenClaimMappings{
Username: configv1.UsernameClaimMapping{
Expression: "has(claims.email) ? claims.email : claims.sub",
},
},
},
},
},
shouldError: false,
},
{
name: "When OIDC provider has invalid CEL expression syntax, it should return an error",
featureGates: []featuregate.Feature{
featuregates.ExternalOIDCWithUpstreamParity,
},
authentication: &configv1.AuthenticationSpec{
Type: configv1.AuthenticationTypeOIDC,
OIDCProviders: []configv1.OIDCProvider{
{
Name: "test-provider",
Issuer: configv1.TokenIssuer{
URL: "https://test.example.com",
Audiences: []configv1.TokenAudience{
"test-audience",
},
},
ClaimValidationRules: []configv1.TokenClaimValidationRule{
{
Type: configv1.TokenValidationRuleTypeCEL,
CEL: configv1.TokenClaimValidationCELRule{
Expression: "invalid CEL syntax!!!",
Message: "this should fail",
},
},
},
ClaimMappings: configv1.TokenClaimMappings{
Username: configv1.UsernameClaimMapping{
Claim: "email",
PrefixPolicy: configv1.NoPrefix,
},
},
},
},
},
shouldError: true,
},
}

for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
if len(tc.featureGates) > 0 {
for _, feature := range tc.featureGates {
fgtesting.SetFeatureGateDuringTest(t, featuregates.Gate(), feature, true)
}
}
err := ValidateAuthenticationSpec(t.Context(), nil, tc.authentication, "foo", []string{})
require.Equal(t, err != nil, tc.shouldError, "expected error state mismatch", "expected an error?", tc.shouldError, "received", err)
})
Expand Down