From 92b7ad5a0f5965ac6897e678ab348bd619ca4640 Mon Sep 17 00:00:00 2001 From: Nick Cao Date: Tue, 18 Feb 2025 12:09:17 -0500 Subject: [PATCH 1/3] Always allow clients to use the internal authenticator --- api/v1alpha1/client_helpers.go | 14 ++++++++++---- api/v1alpha1/exporter_helpers.go | 14 ++++++++++---- internal/authorization/basic.go | 5 +++-- internal/controller/client_controller.go | 2 +- internal/controller/exporter_controller.go | 2 +- internal/controller/secret_helpers.go | 8 ++++---- internal/oidc/op.go | 15 +++------------ 7 files changed, 32 insertions(+), 28 deletions(-) diff --git a/api/v1alpha1/client_helpers.go b/api/v1alpha1/client_helpers.go index ab2a1356..06be70c0 100644 --- a/api/v1alpha1/client_helpers.go +++ b/api/v1alpha1/client_helpers.go @@ -2,10 +2,16 @@ package v1alpha1 import "strings" -func (c *Client) Username(prefix string) string { +func (c *Client) InternalSubject() string { + return strings.Join([]string{"client", c.Namespace, c.Name, string(c.UID)}, ":") +} + +func (c *Client) Usernames(prefix string) []string { + usernames := []string{prefix + c.InternalSubject()} + if c.Spec.Username != nil { - return *c.Spec.Username - } else { - return prefix + strings.Join([]string{"client", c.Namespace, c.Name, string(c.UID)}, ":") + usernames = append(usernames, *c.Spec.Username) } + + return usernames } diff --git a/api/v1alpha1/exporter_helpers.go b/api/v1alpha1/exporter_helpers.go index 4448e038..946b0182 100644 --- a/api/v1alpha1/exporter_helpers.go +++ b/api/v1alpha1/exporter_helpers.go @@ -2,10 +2,16 @@ package v1alpha1 import "strings" -func (e *Exporter) Username(prefix string) string { +func (e *Exporter) InternalSubject() string { + return strings.Join([]string{"exporter", e.Namespace, e.Name, string(e.UID)}, ":") +} + +func (e *Exporter) Usernames(prefix string) []string { + usernames := []string{prefix + e.InternalSubject()} + if e.Spec.Username != nil { - return *e.Spec.Username - } else { - return prefix + strings.Join([]string{"exporter", e.Namespace, e.Name, string(e.UID)}, ":") + usernames = append(usernames, *e.Spec.Username) } + + return usernames } diff --git a/internal/authorization/basic.go b/internal/authorization/basic.go index e11061a4..638a1c35 100644 --- a/internal/authorization/basic.go +++ b/internal/authorization/basic.go @@ -2,6 +2,7 @@ package authorization import ( "context" + "slices" jumpstarterdevv1alpha1 "github.com/jumpstarter-dev/jumpstarter-controller/api/v1alpha1" "k8s.io/apiserver/pkg/authorization/authorizer" @@ -30,7 +31,7 @@ func (b *BasicAuthorizer) Authorize( }, &e); err != nil { return authorizer.DecisionDeny, "failed to get exporter", err } - if e.Username(b.prefix) == attributes.GetUser().GetName() { + if slices.Contains(e.Usernames(b.prefix), attributes.GetUser().GetName()) { return authorizer.DecisionAllow, "", nil } else { return authorizer.DecisionDeny, "", nil @@ -43,7 +44,7 @@ func (b *BasicAuthorizer) Authorize( }, &c); err != nil { return authorizer.DecisionDeny, "failed to get client", err } - if c.Username(b.prefix) == attributes.GetUser().GetName() { + if slices.Contains(c.Usernames(b.prefix), attributes.GetUser().GetName()) { return authorizer.DecisionAllow, "", nil } else { return authorizer.DecisionDeny, "", nil diff --git a/internal/controller/client_controller.go b/internal/controller/client_controller.go index 47f987a7..fb5bb359 100644 --- a/internal/controller/client_controller.go +++ b/internal/controller/client_controller.go @@ -79,7 +79,7 @@ func (r *ClientReconciler) reconcileStatusCredential( secret, err := ensureSecret(ctx, kclient.ObjectKey{ Name: client.Name + "-client", Namespace: client.Namespace, - }, r.Client, r.Signer, client.Username(r.Signer.Prefix())) + }, r.Client, r.Signer, client.InternalSubject()) if err != nil { return fmt.Errorf("reconcileStatusCredential: failed to prepare credential for client: %w", err) } diff --git a/internal/controller/exporter_controller.go b/internal/controller/exporter_controller.go index bed30a24..d237b3ef 100644 --- a/internal/controller/exporter_controller.go +++ b/internal/controller/exporter_controller.go @@ -91,7 +91,7 @@ func (r *ExporterReconciler) reconcileStatusCredential( secret, err := ensureSecret(ctx, client.ObjectKey{ Name: exporter.Name + "-exporter", Namespace: exporter.Namespace, - }, r.Client, r.Signer, exporter.Username(r.Signer.Prefix())) + }, r.Client, r.Signer, exporter.InternalSubject()) if err != nil { return fmt.Errorf("reconcileStatusCredential: failed to prepare credential for exporter: %w", err) } diff --git a/internal/controller/secret_helpers.go b/internal/controller/secret_helpers.go index 25050814..1bb3ddce 100644 --- a/internal/controller/secret_helpers.go +++ b/internal/controller/secret_helpers.go @@ -18,7 +18,7 @@ func ensureSecret( key client.ObjectKey, kclient client.Client, signer *oidc.Signer, - username string, + subject string, ) (*corev1.Secret, error) { logger := log.FromContext(ctx).WithName("ensureSecret") var secret corev1.Secret @@ -29,7 +29,7 @@ func ensureSecret( } // Secret not present logger.Info("secret not present, creating") - token, err := signer.Token(username) + token, err := signer.Token(subject) if err != nil { logger.Error(err, "failed to sign token") return nil, err @@ -51,11 +51,11 @@ func ensureSecret( return &secret, nil } else { token, ok := secret.Data[TokenKey] - if !ok || signer.UnsafeValidate(string(token)) != nil { + if !ok || signer.Validate(string(token)) != nil { // Secret present but invalid logger.Info("secret present but invalid, updating") original := client.MergeFrom(secret.DeepCopy()) - token, err := signer.Token(username) + token, err := signer.Token(subject) if err != nil { logger.Error(err, "failed to sign token") return nil, err diff --git a/internal/oidc/op.go b/internal/oidc/op.go index 7589898a..523c2ca1 100644 --- a/internal/oidc/op.go +++ b/internal/oidc/op.go @@ -7,7 +7,6 @@ import ( "crypto/sha256" "encoding/binary" "math/rand" - "strings" "time" "filippo.io/keygen" @@ -18,8 +17,6 @@ import ( "github.com/zitadel/oidc/v3/pkg/op" ) -const tokenPlaceholder string = "placeholder for external OIDC provider access token" - type Signer struct { privatekey *ecdsa.PrivateKey issuer string @@ -92,10 +89,7 @@ func (k *Signer) Register(group gin.IRoutes) { }) } -func (k *Signer) UnsafeValidate(token string) error { - if token == tokenPlaceholder { - return nil - } +func (k *Signer) Validate(token string) error { _, err := jwt.Parse(token, func(t *jwt.Token) (interface{}, error) { return &k.privatekey.PublicKey, nil }, @@ -109,14 +103,11 @@ func (k *Signer) UnsafeValidate(token string) error { } func (k *Signer) Token( - username string, + subject string, ) (string, error) { - if !strings.HasPrefix(username, k.prefix) { - return tokenPlaceholder, nil - } return jwt.NewWithClaims(jwt.SigningMethodES256, jwt.RegisteredClaims{ Issuer: k.issuer, - Subject: strings.TrimPrefix(username, k.prefix), + Subject: subject, Audience: []string{k.audience}, IssuedAt: jwt.NewNumericDate(time.Now()), ExpiresAt: jwt.NewNumericDate(time.Now().Add(365 * 24 * time.Hour)), // FIXME: rotate keys on expiration From adcce56c7547bca199e0600bac86196c05d6b2ff Mon Sep 17 00:00:00 2001 From: Nick Cao Date: Tue, 18 Feb 2025 12:32:24 -0500 Subject: [PATCH 2/3] Drop Signer.Prefix method and field --- cmd/main.go | 3 +-- internal/config/config.go | 1 + internal/controller/client_controller_test.go | 6 +++--- internal/controller/exporter_controller_test.go | 4 ++-- internal/controller/lease_controller_test.go | 2 +- internal/controller/suite_test.go | 2 +- internal/oidc/config.go | 4 ++-- internal/oidc/op.go | 12 +++--------- 8 files changed, 14 insertions(+), 20 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index bfe1c8b0..de66b1ca 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -145,7 +145,6 @@ func main() { []byte(os.Getenv("CONTROLLER_KEY")), "https://localhost:8085", "jumpstarter", - "internal:", ) if err != nil { setupLog.Error(err, "unable to create internal oidc signer") @@ -206,7 +205,7 @@ func main() { Client: watchClient, Scheme: mgr.GetScheme(), Authn: authentication.NewBearerTokenAuthenticator(authenticator), - Authz: authorization.NewBasicAuthorizer(watchClient, oidcSigner.Prefix()), + Authz: authorization.NewBasicAuthorizer(watchClient, "internal:"), Attr: authorization.NewMetadataAttributesGetter(authorization.MetadataAttributesGetterConfig{ NamespaceKey: "jumpstarter-namespace", ResourceKey: "jumpstarter-kind", diff --git a/internal/config/config.go b/internal/config/config.go index f16255c0..ab3c4500 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -34,6 +34,7 @@ func LoadConfiguration( scheme, []byte(rawAuthenticationConfiguration), signer, + "internal:", // TODO: read this from configuration certificateAuthority, ) if err != nil { diff --git a/internal/controller/client_controller_test.go b/internal/controller/client_controller_test.go index 21aac28d..e704f705 100644 --- a/internal/controller/client_controller_test.go +++ b/internal/controller/client_controller_test.go @@ -79,7 +79,7 @@ var _ = Describe("Identity Controller", func() { }) It("should successfully reconcile the resource", func() { By("Reconciling the created resource") - signer, err := oidc.NewSignerFromSeed([]byte{}, "https://example.com", "dummy", "dummy:") + signer, err := oidc.NewSignerFromSeed([]byte{}, "https://example.com", "dummy") Expect(err).NotTo(HaveOccurred()) controllerReconciler := &ClientReconciler{ @@ -98,7 +98,7 @@ var _ = Describe("Identity Controller", func() { It("should reconcile a missing token secret", func() { By("recreating the secret") - signer, err := oidc.NewSignerFromSeed([]byte{}, "https://example.com", "dummy", "dummy:") + signer, err := oidc.NewSignerFromSeed([]byte{}, "https://example.com", "dummy") Expect(err).NotTo(HaveOccurred()) controllerReconciler := &ClientReconciler{ @@ -129,7 +129,7 @@ var _ = Describe("Identity Controller", func() { It("should reconcile an invalid token secret", func() { By("recreating the secret") - signer, err := oidc.NewSignerFromSeed([]byte{}, "https://example.com", "dummy", "dummy:") + signer, err := oidc.NewSignerFromSeed([]byte{}, "https://example.com", "dummy") Expect(err).NotTo(HaveOccurred()) controllerReconciler := &ClientReconciler{ diff --git a/internal/controller/exporter_controller_test.go b/internal/controller/exporter_controller_test.go index 1d19c93b..9449993a 100644 --- a/internal/controller/exporter_controller_test.go +++ b/internal/controller/exporter_controller_test.go @@ -79,7 +79,7 @@ var _ = Describe("Exporter Controller", func() { }) It("should successfully reconcile the resource", func() { By("Reconciling the created resource") - signer, err := oidc.NewSignerFromSeed([]byte{}, "https://example.com", "dummy", "dummy:") + signer, err := oidc.NewSignerFromSeed([]byte{}, "https://example.com", "dummy") Expect(err).NotTo(HaveOccurred()) controllerReconciler := &ExporterReconciler{ @@ -97,7 +97,7 @@ var _ = Describe("Exporter Controller", func() { }) It("should reconcile a missing token secret", func() { By("recreating the secret") - signer, err := oidc.NewSignerFromSeed([]byte{}, "https://example.com", "dummy", "dummy:") + signer, err := oidc.NewSignerFromSeed([]byte{}, "https://example.com", "dummy") Expect(err).NotTo(HaveOccurred()) controllerReconciler := &ExporterReconciler{ diff --git a/internal/controller/lease_controller_test.go b/internal/controller/lease_controller_test.go index 2d9a8233..4cd12c75 100644 --- a/internal/controller/lease_controller_test.go +++ b/internal/controller/lease_controller_test.go @@ -355,7 +355,7 @@ func reconcileLease(ctx context.Context, lease *jumpstarterdevv1alpha1.Lease) re Scheme: k8sClient.Scheme(), } - signer, err := oidc.NewSignerFromSeed([]byte{}, "https://example.com", "dummy", "dummy:") + signer, err := oidc.NewSignerFromSeed([]byte{}, "https://example.com", "dummy") Expect(err).NotTo(HaveOccurred()) exporterReconciler := &ExporterReconciler{ diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 22a2e32d..0613e465 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -122,7 +122,7 @@ func createExporters(ctx context.Context, exporters ...*jumpstarterdevv1alpha1.E Namespace: "default", // TODO(user):Modify as needed } - signer, err := oidc.NewSignerFromSeed([]byte{}, "https://example.com", "dummy", "dummy:") + signer, err := oidc.NewSignerFromSeed([]byte{}, "https://example.com", "dummy") Expect(err).NotTo(HaveOccurred()) controllerReconciler := &ExporterReconciler{ diff --git a/internal/oidc/config.go b/internal/oidc/config.go index f0c53b09..aa8608e8 100644 --- a/internal/oidc/config.go +++ b/internal/oidc/config.go @@ -12,7 +12,6 @@ import ( tokenunion "k8s.io/apiserver/pkg/authentication/token/union" "k8s.io/apiserver/pkg/server/dynamiccertificates" "k8s.io/apiserver/plugin/pkg/authenticator/token/oidc" - "k8s.io/utils/ptr" ) func LoadAuthenticationConfiguration( @@ -20,6 +19,7 @@ func LoadAuthenticationConfiguration( scheme *runtime.Scheme, configuration []byte, signer *Signer, + prefix string, certificateAuthority string, ) (authenticator.Token, error) { var authenticationConfiguration jumpstarterdevv1alpha1.AuthenticationConfiguration @@ -41,7 +41,7 @@ func LoadAuthenticationConfiguration( ClaimMappings: apiserverv1beta1.ClaimMappings{ Username: apiserverv1beta1.PrefixedClaimOrExpression{ Claim: "sub", - Prefix: ptr.To(signer.Prefix()), + Prefix: &prefix, }, }, }) diff --git a/internal/oidc/op.go b/internal/oidc/op.go index 523c2ca1..072b8594 100644 --- a/internal/oidc/op.go +++ b/internal/oidc/op.go @@ -21,19 +21,17 @@ type Signer struct { privatekey *ecdsa.PrivateKey issuer string audience string - prefix string } -func NewSigner(privateKey *ecdsa.PrivateKey, issuer, audience, prefix string) *Signer { +func NewSigner(privateKey *ecdsa.PrivateKey, issuer, audience string) *Signer { return &Signer{ privatekey: privateKey, issuer: issuer, audience: audience, - prefix: prefix, } } -func NewSignerFromSeed(seed []byte, issuer, audience, prefix string) (*Signer, error) { +func NewSignerFromSeed(seed []byte, issuer, audience string) (*Signer, error) { hash := sha256.Sum256(seed) source := rand.NewSource(int64(binary.BigEndian.Uint64(hash[:8]))) reader := rand.New(source) @@ -41,7 +39,7 @@ func NewSignerFromSeed(seed []byte, issuer, audience, prefix string) (*Signer, e if err != nil { return nil, err } - return NewSigner(key, issuer, audience, prefix), nil + return NewSigner(key, issuer, audience), nil } func (k *Signer) Issuer() string { @@ -52,10 +50,6 @@ func (k *Signer) Audience() string { return k.audience } -func (k *Signer) Prefix() string { - return k.prefix -} - func (k *Signer) ID() string { return "default" } From 6815c862de4d64d26fa13fb953fc3614a00d613d Mon Sep 17 00:00:00 2001 From: Nick Cao Date: Fri, 7 Mar 2025 09:24:18 -0500 Subject: [PATCH 3/3] Load internal authentication username prefix from configmap --- .../authenticationconfiguration_types.go | 7 ++++++- api/v1alpha1/zz_generated.deepcopy.go | 16 ++++++++++++++++ cmd/main.go | 4 ++-- internal/config/config.go | 13 ++++++------- internal/oidc/config.go | 17 ++++++++++++----- 5 files changed, 42 insertions(+), 15 deletions(-) diff --git a/api/v1alpha1/authenticationconfiguration_types.go b/api/v1alpha1/authenticationconfiguration_types.go index 8088b27f..3ad2c09b 100644 --- a/api/v1alpha1/authenticationconfiguration_types.go +++ b/api/v1alpha1/authenticationconfiguration_types.go @@ -11,7 +11,12 @@ import ( type AuthenticationConfiguration struct { metav1.TypeMeta - JWT []apiserverv1beta1.JWTAuthenticator `json:"jwt"` + Internal Internal `json:"internal"` + JWT []apiserverv1beta1.JWTAuthenticator `json:"jwt"` +} + +type Internal struct { + Prefix string `json:"prefix"` } func init() { diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 7f21fd38..847691b9 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -31,6 +31,7 @@ import ( func (in *AuthenticationConfiguration) DeepCopyInto(out *AuthenticationConfiguration) { *out = *in out.TypeMeta = in.TypeMeta + out.Internal = in.Internal if in.JWT != nil { in, out := &in.JWT, &out.JWT *out = make([]v1beta1.JWTAuthenticator, len(*in)) @@ -415,6 +416,21 @@ func (in *From) DeepCopy() *From { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Internal) DeepCopyInto(out *Internal) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Internal. +func (in *Internal) DeepCopy() *Internal { + if in == nil { + return nil + } + out := new(Internal) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Lease) DeepCopyInto(out *Lease) { *out = *in diff --git a/cmd/main.go b/cmd/main.go index de66b1ca..d05f2920 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -151,7 +151,7 @@ func main() { os.Exit(1) } - authenticator, err := config.LoadConfiguration( + authenticator, prefix, err := config.LoadConfiguration( context.Background(), mgr.GetAPIReader(), mgr.GetScheme(), @@ -205,7 +205,7 @@ func main() { Client: watchClient, Scheme: mgr.GetScheme(), Authn: authentication.NewBearerTokenAuthenticator(authenticator), - Authz: authorization.NewBasicAuthorizer(watchClient, "internal:"), + Authz: authorization.NewBasicAuthorizer(watchClient, prefix), Attr: authorization.NewMetadataAttributesGetter(authorization.MetadataAttributesGetterConfig{ NamespaceKey: "jumpstarter-namespace", ResourceKey: "jumpstarter-kind", diff --git a/internal/config/config.go b/internal/config/config.go index ab3c4500..c3d6b315 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -18,28 +18,27 @@ func LoadConfiguration( key client.ObjectKey, signer *oidc.Signer, certificateAuthority string, -) (authenticator.Token, error) { +) (authenticator.Token, string, error) { var configmap corev1.ConfigMap if err := client.Get(ctx, key, &configmap); err != nil { - return nil, err + return nil, "", err } rawAuthenticationConfiguration, ok := configmap.Data["authentication"] if !ok { - return nil, fmt.Errorf("LoadConfiguration: missing authentication section") + return nil, "", fmt.Errorf("LoadConfiguration: missing authentication section") } - authenticator, err := oidc.LoadAuthenticationConfiguration( + authenticator, prefix, err := oidc.LoadAuthenticationConfiguration( ctx, scheme, []byte(rawAuthenticationConfiguration), signer, - "internal:", // TODO: read this from configuration certificateAuthority, ) if err != nil { - return nil, err + return nil, "", err } - return authenticator, nil + return authenticator, prefix, nil } diff --git a/internal/oidc/config.go b/internal/oidc/config.go index aa8608e8..f8cbd7b8 100644 --- a/internal/oidc/config.go +++ b/internal/oidc/config.go @@ -19,9 +19,8 @@ func LoadAuthenticationConfiguration( scheme *runtime.Scheme, configuration []byte, signer *Signer, - prefix string, certificateAuthority string, -) (authenticator.Token, error) { +) (authenticator.Token, string, error) { var authenticationConfiguration jumpstarterdevv1alpha1.AuthenticationConfiguration if err := runtime.DecodeInto( serializer.NewCodecFactory(scheme, serializer.EnableStrict). @@ -29,7 +28,11 @@ func LoadAuthenticationConfiguration( configuration, &authenticationConfiguration, ); err != nil { - return nil, err + return nil, "", err + } + + if authenticationConfiguration.Internal.Prefix == "" { + authenticationConfiguration.Internal.Prefix = "internal:" } authenticationConfiguration.JWT = append(authenticationConfiguration.JWT, apiserverv1beta1.JWTAuthenticator{ @@ -41,16 +44,20 @@ func LoadAuthenticationConfiguration( ClaimMappings: apiserverv1beta1.ClaimMappings{ Username: apiserverv1beta1.PrefixedClaimOrExpression{ Claim: "sub", - Prefix: &prefix, + Prefix: &authenticationConfiguration.Internal.Prefix, }, }, }) - return newJWTAuthenticator( + authn, err := newJWTAuthenticator( ctx, scheme, authenticationConfiguration, ) + if err != nil { + return nil, "", err + } + return authn, authenticationConfiguration.Internal.Prefix, nil } // Reference: https://github.com/kubernetes/kubernetes/blob/v1.32.1/pkg/kubeapiserver/authenticator/config.go#L244