From f604deabac9dd1180ce3d198761ea57071b5615f Mon Sep 17 00:00:00 2001 From: OscarLlamas6 Date: Wed, 14 Jan 2026 13:26:56 -0600 Subject: [PATCH 01/12] feat(iam): restrict User email updates to verified identity providers Implement validating admission webhook to enforce that User email addresses can only be changed to emails verified by linked external identity providers (GitHub, Google, etc.). This prevents email spoofing and ensures data integrity between User records and OIDC providers. Changes: - Add ValidateUpdate to User webhook to validate email changes against UserIdentities - Add field indexer for UserIdentity.status.userUID for efficient lookups - Create UserIdentity webhook to block DELETE operations (not supported yet) - Update User webhook annotation to intercept both CREATE and UPDATE operations - Register UserIdentity webhook in controller-manager Validation logic: - Allow email updates only if new email exists in user's linked UserIdentities - Reject updates if user has no linked identity providers - Reject updates if email is not verified by any linked provider - Provide helpful error messages with list of available verified emails UserIdentity DELETE protection: - Block deletion of UserIdentity resources via webhook - Identity provider links must be managed through Zitadel - Prevents inconsistencies between Milo and external auth provider - Requires automatic email synchronization logic before deletion can be enabled Resolves: API Restrict User Email Updates to Linked Identities --- .../controller-manager/controllermanager.go | 5 ++ config/webhook/manifests.yaml | 22 ++++++ .../webhooks/iam/v1alpha1/user_webhook.go | 73 ++++++++++++++++++- .../identity/v1alpha1/useridentity_webhook.go | 48 ++++++++++++ 4 files changed, 146 insertions(+), 2 deletions(-) create mode 100644 internal/webhooks/identity/v1alpha1/useridentity_webhook.go diff --git a/cmd/milo/controller-manager/controllermanager.go b/cmd/milo/controller-manager/controllermanager.go index 11a8d56b..9ce12a38 100644 --- a/cmd/milo/controller-manager/controllermanager.go +++ b/cmd/milo/controller-manager/controllermanager.go @@ -86,6 +86,7 @@ import ( quotacontroller "go.miloapis.com/milo/internal/quota/controllers" crmv1alpha1webhook "go.miloapis.com/milo/internal/webhooks/crm/v1alpha1" iamv1alpha1webhook "go.miloapis.com/milo/internal/webhooks/iam/v1alpha1" + identityv1alpha1webhook "go.miloapis.com/milo/internal/webhooks/identity/v1alpha1" notificationv1alpha1webhook "go.miloapis.com/milo/internal/webhooks/notification/v1alpha1" resourcemanagerv1alpha1webhook "go.miloapis.com/milo/internal/webhooks/resourcemanager/v1alpha1" crmv1alpha1 "go.miloapis.com/milo/pkg/apis/crm/v1alpha1" @@ -495,6 +496,10 @@ func Run(ctx context.Context, c *config.CompletedConfig, opts *Options) error { logger.Error(err, "Error setting up userdeactivation webhook") klog.FlushAndExit(klog.ExitFlushTimeout, 1) } + if err := identityv1alpha1webhook.SetupUserIdentityWebhooksWithManager(ctrl); err != nil { + logger.Error(err, "Error setting up useridentity webhook") + klog.FlushAndExit(klog.ExitFlushTimeout, 1) + } if err := notificationv1alpha1webhook.SetupEmailTemplateWebhooksWithManager(ctrl, SystemNamespace); err != nil { logger.Error(err, "Error setting up emailtemplate webhook") klog.FlushAndExit(klog.ExitFlushTimeout, 1) diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 2ffeb58a..697ad5aa 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -281,6 +281,7 @@ webhooks: - v1alpha1 operations: - CREATE + - UPDATE resources: - users sideEffects: NoneOnDryRun @@ -326,6 +327,27 @@ webhooks: resources: - userinvitations sideEffects: None +- admissionReviewVersions: + - v1 + - v1beta1 + clientConfig: + service: + name: milo-controller-manager + namespace: milo-system + path: /validate-identity-miloapis-com-v1alpha1-useridentity + port: 9443 + failurePolicy: Fail + name: vuseridentity.identity.miloapis.com + rules: + - apiGroups: + - identity.miloapis.com + apiVersions: + - v1alpha1 + operations: + - DELETE + resources: + - useridentities + sideEffects: None - admissionReviewVersions: - v1 - v1beta1 diff --git a/internal/webhooks/iam/v1alpha1/user_webhook.go b/internal/webhooks/iam/v1alpha1/user_webhook.go index eeadf62e..cd0cdbd8 100644 --- a/internal/webhooks/iam/v1alpha1/user_webhook.go +++ b/internal/webhooks/iam/v1alpha1/user_webhook.go @@ -12,17 +12,26 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" iamv1alpha1 "go.miloapis.com/milo/pkg/apis/iam/v1alpha1" + identityv1alpha1 "go.miloapis.com/milo/pkg/apis/identity/v1alpha1" ) // log is for logging in this package. var userlog = logf.Log.WithName("user-resource") -// +kubebuilder:webhook:path=/validate-iam-miloapis-com-v1alpha1-user,mutating=false,failurePolicy=fail,sideEffects=NoneOnDryRun,groups=iam.miloapis.com,resources=users,verbs=create,versions=v1alpha1,name=vuser.iam.miloapis.com,admissionReviewVersions={v1,v1beta1},serviceName=milo-controller-manager,servicePort=9443,serviceNamespace=milo-system +// +kubebuilder:webhook:path=/validate-iam-miloapis-com-v1alpha1-user,mutating=false,failurePolicy=fail,sideEffects=NoneOnDryRun,groups=iam.miloapis.com,resources=users,verbs=create;update,versions=v1alpha1,name=vuser.iam.miloapis.com,admissionReviewVersions={v1,v1beta1},serviceName=milo-controller-manager,servicePort=9443,serviceNamespace=milo-system // SetupWebhooksWithManager sets up all iam.miloapis.com webhooks func SetupUserWebhooksWithManager(mgr ctrl.Manager, systemNamespace string, userSelfManageRoleName string) error { userlog.Info("Setting up iam.miloapis.com user webhooks") + // Index UserIdentity by userUID for efficient lookups during email validation + if err := mgr.GetFieldIndexer().IndexField(context.Background(), &identityv1alpha1.UserIdentity{}, "status.userUID", func(rawObj client.Object) []string { + identity := rawObj.(*identityv1alpha1.UserIdentity) + return []string{identity.Status.UserUID} + }); err != nil { + return err + } + return ctrl.NewWebhookManagedBy(mgr). For(&iamv1alpha1.User{}). WithValidator(&UserValidator{ @@ -74,7 +83,67 @@ func (v *UserValidator) ValidateCreate(ctx context.Context, obj runtime.Object) } func (v *UserValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { - return nil, nil + oldUser := oldObj.(*iamv1alpha1.User) + newUser := newObj.(*iamv1alpha1.User) + + // If email hasn't changed, allow the update + if oldUser.Spec.Email == newUser.Spec.Email { + return nil, nil + } + + userlog.Info("Email change detected", + "user", newUser.Name, + "oldEmail", oldUser.Spec.Email, + "newEmail", newUser.Spec.Email) + + // Get all UserIdentities for this user + identityList := &identityv1alpha1.UserIdentityList{} + err := v.client.List(ctx, identityList, client.MatchingFields{ + "status.userUID": string(newUser.GetUID()), + }) + if err != nil { + userlog.Error(err, "Failed to list user identities", "user", newUser.Name) + return nil, fmt.Errorf("failed to list user identities: %w", err) + } + + // If no identities are linked, reject the change + if len(identityList.Items) == 0 { + userlog.Info("User has no linked identities, rejecting email change", "user", newUser.Name) + return nil, fmt.Errorf( + "cannot change email: no verified identity providers linked to this account. " + + "Please link an identity provider (GitHub, Google, etc.) first") + } + + // Validate that the new email exists in any of the linked identities + // The email is stored in the Username field of UserIdentity + for _, identity := range identityList.Items { + if identity.Status.Username == newUser.Spec.Email { + userlog.Info("Email validated against identity provider", + "email", newUser.Spec.Email, + "provider", identity.Status.ProviderName, + "providerID", identity.Status.ProviderID) + return nil, nil // Email is valid + } + } + + // Build list of available emails for error message + availableEmails := make([]string, 0, len(identityList.Items)) + for _, identity := range identityList.Items { + if identity.Status.Username != "" { + availableEmails = append(availableEmails, identity.Status.Username) + } + } + + // Email not found in any linked identity + userlog.Info("Email not found in linked identities", + "requestedEmail", newUser.Spec.Email, + "availableEmails", availableEmails) + + return nil, fmt.Errorf( + "email %q is not linked to any verified identity provider. "+ + "Available verified emails: %v", + newUser.Spec.Email, + availableEmails) } func (v *UserValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { diff --git a/internal/webhooks/identity/v1alpha1/useridentity_webhook.go b/internal/webhooks/identity/v1alpha1/useridentity_webhook.go new file mode 100644 index 00000000..962f41b9 --- /dev/null +++ b/internal/webhooks/identity/v1alpha1/useridentity_webhook.go @@ -0,0 +1,48 @@ +package v1alpha1 + +import ( + "context" + "fmt" + + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + identityv1alpha1 "go.miloapis.com/milo/pkg/apis/identity/v1alpha1" +) + +var useridentitylog = logf.Log.WithName("useridentity-resource") + +// +kubebuilder:webhook:path=/validate-identity-miloapis-com-v1alpha1-useridentity,mutating=false,failurePolicy=fail,sideEffects=None,groups=identity.miloapis.com,resources=useridentities,verbs=delete,versions=v1alpha1,name=vuseridentity.identity.miloapis.com,admissionReviewVersions={v1,v1beta1},serviceName=milo-controller-manager,servicePort=9443,serviceNamespace=milo-system + +// SetupUserIdentityWebhooksWithManager sets up the webhooks for UserIdentity +func SetupUserIdentityWebhooksWithManager(mgr ctrl.Manager) error { + useridentitylog.Info("Setting up identity.miloapis.com useridentity webhooks") + + return ctrl.NewWebhookManagedBy(mgr). + For(&identityv1alpha1.UserIdentity{}). + WithValidator(&UserIdentityValidator{}). + Complete() +} + +// UserIdentityValidator validates UserIdentity resources +type UserIdentityValidator struct{} + +func (v *UserIdentityValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { + return nil, nil +} + +func (v *UserIdentityValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { + return nil, nil +} + +func (v *UserIdentityValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { + userIdentity := obj.(*identityv1alpha1.UserIdentity) + useridentitylog.Info("Blocking UserIdentity deletion", "name", userIdentity.Name) + + return nil, fmt.Errorf( + "deleting UserIdentity resources is not currently supported. " + + "Identity provider links must be managed through the authentication provider (e.g., Zitadel). " + + "Automatic email synchronization logic is required before deletion can be enabled") +} From cbed380fd15977570e3ee8eeeca9a759e8c7743f Mon Sep 17 00:00:00 2001 From: OscarLlamas6 Date: Fri, 16 Jan 2026 08:44:13 -0600 Subject: [PATCH 02/12] chore(iam): use manual filtering for UserIdentities in email validation Field indexers don't work with UserIdentity because it's a dynamic REST API that proxies to auth-provider-zitadel and is not cached in the controller-manager. Changes: - Remove field indexer setup for UserIdentity.status.userUID - Change from client.MatchingFields to manual in-memory filtering - Add comment explaining why field selectors don't work with dynamic APIs The performance impact is minimal since: - UserIdentities are typically small in number (1-3 per user) - Email updates are infrequent operations - The List operation is already proxied to auth-provider-zitadel regardless --- .../webhooks/iam/v1alpha1/user_webhook.go | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/internal/webhooks/iam/v1alpha1/user_webhook.go b/internal/webhooks/iam/v1alpha1/user_webhook.go index cd0cdbd8..58f06380 100644 --- a/internal/webhooks/iam/v1alpha1/user_webhook.go +++ b/internal/webhooks/iam/v1alpha1/user_webhook.go @@ -24,14 +24,6 @@ var userlog = logf.Log.WithName("user-resource") func SetupUserWebhooksWithManager(mgr ctrl.Manager, systemNamespace string, userSelfManageRoleName string) error { userlog.Info("Setting up iam.miloapis.com user webhooks") - // Index UserIdentity by userUID for efficient lookups during email validation - if err := mgr.GetFieldIndexer().IndexField(context.Background(), &identityv1alpha1.UserIdentity{}, "status.userUID", func(rawObj client.Object) []string { - identity := rawObj.(*identityv1alpha1.UserIdentity) - return []string{identity.Status.UserUID} - }); err != nil { - return err - } - return ctrl.NewWebhookManagedBy(mgr). For(&iamv1alpha1.User{}). WithValidator(&UserValidator{ @@ -96,16 +88,23 @@ func (v *UserValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runti "oldEmail", oldUser.Spec.Email, "newEmail", newUser.Spec.Email) - // Get all UserIdentities for this user - identityList := &identityv1alpha1.UserIdentityList{} - err := v.client.List(ctx, identityList, client.MatchingFields{ - "status.userUID": string(newUser.GetUID()), - }) - if err != nil { + // Get all UserIdentities and filter by userUID + // Note: UserIdentity is a dynamic REST API (not cached), so we cannot use field selectors + // We must list all and filter manually + allIdentities := &identityv1alpha1.UserIdentityList{} + if err := v.client.List(ctx, allIdentities); err != nil { userlog.Error(err, "Failed to list user identities", "user", newUser.Name) return nil, fmt.Errorf("failed to list user identities: %w", err) } + // Filter identities for this user + identityList := &identityv1alpha1.UserIdentityList{} + for _, identity := range allIdentities.Items { + if identity.Status.UserUID == string(newUser.GetUID()) { + identityList.Items = append(identityList.Items, identity) + } + } + // If no identities are linked, reject the change if len(identityList.Items) == 0 { userlog.Info("User has no linked identities, rejecting email change", "user", newUser.Name) From d1f62a0a732e5b357fee533c2505f23ba059a302 Mon Sep 17 00:00:00 2001 From: "joggrbot[bot]" <107281636+joggrbot[bot]@users.noreply.github.com> Date: Fri, 16 Jan 2026 14:45:57 +0000 Subject: [PATCH 03/12] [skip ci] docs: fix outdated docs --- internal/apiserver/identity/useridentities/README.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/internal/apiserver/identity/useridentities/README.md b/internal/apiserver/identity/useridentities/README.md index 1d85eea8..60a905f7 100644 --- a/internal/apiserver/identity/useridentities/README.md +++ b/internal/apiserver/identity/useridentities/README.md @@ -38,7 +38,13 @@ Naming & structure - internal/apiserver/identity/useridentities/rest.go — REST storage - internal/apiserver/identity/useridentities/dynamic.go — provider implementation -Read-only resource +Read-only resource and admission webhook for deletion Unlike sessions, useridentities is a read-only resource. Users cannot create, update, or delete user identities through the Kubernetes API. Identity linking and unlinking is managed through the external identity provider (e.g., Zitadel). + +If a user attempts to delete a UserIdentity via the Kubernetes API, the operation will be explicitly rejected by an admission webhook, which returns an error similar to: + + deleting UserIdentity resources is not currently supported. Identity provider links must be managed through the authentication provider (e.g., Zitadel). Automatic email synchronization logic is required before deletion can be enabled + +This error response ensures deletions are consistently blocked at the API layer, clarifying current support and intended usage. \ No newline at end of file From e3e4a35193c314282fc018a22b8fb556925d4e44 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Fri, 16 Jan 2026 14:48:17 +0000 Subject: [PATCH 04/12] chore: add missing newlines at end of files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Automatically added newlines to 1 file(s) Co-Authored-By: github-actions[bot] --- internal/apiserver/identity/useridentities/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/apiserver/identity/useridentities/README.md b/internal/apiserver/identity/useridentities/README.md index 60a905f7..aa7a065e 100644 --- a/internal/apiserver/identity/useridentities/README.md +++ b/internal/apiserver/identity/useridentities/README.md @@ -47,4 +47,4 @@ If a user attempts to delete a UserIdentity via the Kubernetes API, the operatio deleting UserIdentity resources is not currently supported. Identity provider links must be managed through the authentication provider (e.g., Zitadel). Automatic email synchronization logic is required before deletion can be enabled -This error response ensures deletions are consistently blocked at the API layer, clarifying current support and intended usage. \ No newline at end of file +This error response ensures deletions are consistently blocked at the API layer, clarifying current support and intended usage. From 5e507cfd2418834aede423deee56725c0fefe562 Mon Sep 17 00:00:00 2001 From: OscarLlamas6 Date: Fri, 16 Jan 2026 09:25:13 -0600 Subject: [PATCH 05/12] chore(iam): restrict email updates to self-service only Restrict User email updates to self-service only since the webhook currently does not support impersonation. When an admin attempts to update another user's email, the UserIdentity list would be scoped to the admin's identities instead of the target user's identities, causing incorrect validation. By restricting to self-service only, we ensure that the authenticated user in the context matches the target user, so the UserIdentity API correctly returns the user's own linked identity providers. Changes: - Add validation that req.UserInfo.UID matches the target user UID - Remove manual filtering logic (no longer needed with self-service restriction) - Return 403 Forbidden with descriptive error for cross-user update attempts - Simplify UserIdentity list call since context guarantees correct scope Future work: If admin-initiated email updates are needed, implement impersonation support to create a client scoped to the target user. --- .../webhooks/iam/v1alpha1/user_webhook.go | 34 ++++++++++++------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/internal/webhooks/iam/v1alpha1/user_webhook.go b/internal/webhooks/iam/v1alpha1/user_webhook.go index 58f06380..17f4109d 100644 --- a/internal/webhooks/iam/v1alpha1/user_webhook.go +++ b/internal/webhooks/iam/v1alpha1/user_webhook.go @@ -88,21 +88,31 @@ func (v *UserValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runti "oldEmail", oldUser.Spec.Email, "newEmail", newUser.Spec.Email) - // Get all UserIdentities and filter by userUID - // Note: UserIdentity is a dynamic REST API (not cached), so we cannot use field selectors - // We must list all and filter manually - allIdentities := &identityv1alpha1.UserIdentityList{} - if err := v.client.List(ctx, allIdentities); err != nil { - userlog.Error(err, "Failed to list user identities", "user", newUser.Name) - return nil, fmt.Errorf("failed to list user identities: %w", err) + // Get the requesting user from admission request + req, err := admission.RequestFromContext(ctx) + if err != nil { + userlog.Error(err, "Failed to get request from context") + return nil, fmt.Errorf("failed to get request from context: %w", err) } - // Filter identities for this user + // Only allow users to update their own email (self-service only) + // This ensures that the UserIdentity list will be scoped to the correct user + if req.UserInfo.UID != string(newUser.GetUID()) { + userlog.Info("Rejecting email update from different user", + "requestingUser", req.UserInfo.UID, + "targetUser", newUser.GetUID()) + return nil, fmt.Errorf( + "forbidden: cannot update email address of another user. " + + "Email updates are restricted to self-service only") + } + + // Get UserIdentities for the requesting user + // Note: UserIdentity is a dynamic REST API that automatically scopes results + // to the authenticated user (from context), so we don't need manual filtering identityList := &identityv1alpha1.UserIdentityList{} - for _, identity := range allIdentities.Items { - if identity.Status.UserUID == string(newUser.GetUID()) { - identityList.Items = append(identityList.Items, identity) - } + if err := v.client.List(ctx, identityList); err != nil { + userlog.Error(err, "Failed to list user identities", "user", newUser.Name) + return nil, fmt.Errorf("failed to list user identities: %w", err) } // If no identities are linked, reject the change From 11292a09cec0ffad0705d69f6bb829e84cea893f Mon Sep 17 00:00:00 2001 From: OscarLlamas6 Date: Fri, 16 Jan 2026 10:18:23 -0600 Subject: [PATCH 06/12] chore(iam): use proper Kubernetes error wrappers in User email validation Replace fmt.Errorf with appropriate Kubernetes error types for consistency with codebase standards. Changes: - Use errors.NewForbidden for cross-user email update attempts (403) - Use errors.NewBadRequest for validation failures (400) - Use errors.NewInternalError for internal errors (500) - Add imports for k8s.io/apimachinery/pkg/api/errors and runtime/schema --- .../webhooks/iam/v1alpha1/user_webhook.go | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/internal/webhooks/iam/v1alpha1/user_webhook.go b/internal/webhooks/iam/v1alpha1/user_webhook.go index 17f4109d..428871af 100644 --- a/internal/webhooks/iam/v1alpha1/user_webhook.go +++ b/internal/webhooks/iam/v1alpha1/user_webhook.go @@ -4,8 +4,10 @@ import ( "context" "fmt" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -92,7 +94,7 @@ func (v *UserValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runti req, err := admission.RequestFromContext(ctx) if err != nil { userlog.Error(err, "Failed to get request from context") - return nil, fmt.Errorf("failed to get request from context: %w", err) + return nil, errors.NewInternalError(fmt.Errorf("failed to get request from context: %w", err)) } // Only allow users to update their own email (self-service only) @@ -101,9 +103,10 @@ func (v *UserValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runti userlog.Info("Rejecting email update from different user", "requestingUser", req.UserInfo.UID, "targetUser", newUser.GetUID()) - return nil, fmt.Errorf( - "forbidden: cannot update email address of another user. " + - "Email updates are restricted to self-service only") + return nil, errors.NewForbidden( + schema.GroupResource{Group: iamv1alpha1.SchemeGroupVersion.Group, Resource: "users"}, + newUser.Name, + fmt.Errorf("cannot update email address of another user. Email updates are restricted to self-service only")) } // Get UserIdentities for the requesting user @@ -112,13 +115,13 @@ func (v *UserValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runti identityList := &identityv1alpha1.UserIdentityList{} if err := v.client.List(ctx, identityList); err != nil { userlog.Error(err, "Failed to list user identities", "user", newUser.Name) - return nil, fmt.Errorf("failed to list user identities: %w", err) + return nil, errors.NewInternalError(fmt.Errorf("failed to list user identities: %w", err)) } // If no identities are linked, reject the change if len(identityList.Items) == 0 { userlog.Info("User has no linked identities, rejecting email change", "user", newUser.Name) - return nil, fmt.Errorf( + return nil, errors.NewBadRequest( "cannot change email: no verified identity providers linked to this account. " + "Please link an identity provider (GitHub, Google, etc.) first") } @@ -148,11 +151,12 @@ func (v *UserValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runti "requestedEmail", newUser.Spec.Email, "availableEmails", availableEmails) - return nil, fmt.Errorf( - "email %q is not linked to any verified identity provider. "+ - "Available verified emails: %v", - newUser.Spec.Email, - availableEmails) + return nil, errors.NewBadRequest( + fmt.Sprintf( + "email %q is not linked to any verified identity provider. "+ + "Available verified emails: %v", + newUser.Spec.Email, + availableEmails)) } func (v *UserValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { From e098a42fa9d00a510a8e174915ff38c856872e38 Mon Sep 17 00:00:00 2001 From: OscarLlamas6 Date: Fri, 16 Jan 2026 11:22:59 -0600 Subject: [PATCH 07/12] chore(webhooks): register identity API types in controller-manager Scheme Add identityv1alpha1 to the controller-manager Scheme to enable UserIdentity webhook registration. Without this registration, the webhook server fails to start with 'no kind is registered' error. Changes: - Add identityv1alpha1 import to controller-manager - Register identityv1alpha1.AddToScheme in init() - Use proper Kubernetes error wrappers (NewForbidden, NewBadRequest, NewInternalError) - Restrict User email updates to self-service only This fixes the test failures where webhooks were returning 'connection refused' because the controller-manager was crashing during webhook setup. --- cmd/milo/controller-manager/controllermanager.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/milo/controller-manager/controllermanager.go b/cmd/milo/controller-manager/controllermanager.go index 9ce12a38..4807ac29 100644 --- a/cmd/milo/controller-manager/controllermanager.go +++ b/cmd/milo/controller-manager/controllermanager.go @@ -91,6 +91,7 @@ import ( resourcemanagerv1alpha1webhook "go.miloapis.com/milo/internal/webhooks/resourcemanager/v1alpha1" crmv1alpha1 "go.miloapis.com/milo/pkg/apis/crm/v1alpha1" iamv1alpha1 "go.miloapis.com/milo/pkg/apis/iam/v1alpha1" + identityv1alpha1 "go.miloapis.com/milo/pkg/apis/identity/v1alpha1" infrastructurev1alpha1 "go.miloapis.com/milo/pkg/apis/infrastructure/v1alpha1" notificationv1alpha1 "go.miloapis.com/milo/pkg/apis/notification/v1alpha1" quotav1alpha1 "go.miloapis.com/milo/pkg/apis/quota/v1alpha1" @@ -170,6 +171,7 @@ func init() { utilruntime.Must(resourcemanagerv1alpha1.AddToScheme(Scheme)) utilruntime.Must(infrastructurev1alpha1.AddToScheme(Scheme)) utilruntime.Must(iamv1alpha1.AddToScheme(Scheme)) + utilruntime.Must(identityv1alpha1.AddToScheme(Scheme)) utilruntime.Must(notificationv1alpha1.AddToScheme(Scheme)) utilruntime.Must(crmv1alpha1.AddToScheme(Scheme)) utilruntime.Must(quotav1alpha1.AddToScheme(Scheme)) From 96dc3a16ad0e66f1a48512832e5a425ece315ac5 Mon Sep 17 00:00:00 2001 From: OscarLlamas6 Date: Fri, 16 Jan 2026 13:28:33 -0600 Subject: [PATCH 08/12] chore: validation adjustment --- internal/webhooks/iam/v1alpha1/user_webhook.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/webhooks/iam/v1alpha1/user_webhook.go b/internal/webhooks/iam/v1alpha1/user_webhook.go index 428871af..3909d9a3 100644 --- a/internal/webhooks/iam/v1alpha1/user_webhook.go +++ b/internal/webhooks/iam/v1alpha1/user_webhook.go @@ -99,10 +99,11 @@ func (v *UserValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runti // Only allow users to update their own email (self-service only) // This ensures that the UserIdentity list will be scoped to the correct user - if req.UserInfo.UID != string(newUser.GetUID()) { + // Note: req.UserInfo.UID is the Milo user ID, which matches the User resource name + if req.UserInfo.UID != newUser.Name { userlog.Info("Rejecting email update from different user", "requestingUser", req.UserInfo.UID, - "targetUser", newUser.GetUID()) + "targetUser", newUser.Name) return nil, errors.NewForbidden( schema.GroupResource{Group: iamv1alpha1.SchemeGroupVersion.Group, Resource: "users"}, newUser.Name, From 90eaeefa7a22f884af858e2cb0b839f5c99a3217 Mon Sep 17 00:00:00 2001 From: OscarLlamas6 Date: Fri, 16 Jan 2026 14:12:37 -0600 Subject: [PATCH 09/12] chore: removing latest changes --- internal/webhooks/iam/v1alpha1/user_webhook.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/internal/webhooks/iam/v1alpha1/user_webhook.go b/internal/webhooks/iam/v1alpha1/user_webhook.go index 3909d9a3..428871af 100644 --- a/internal/webhooks/iam/v1alpha1/user_webhook.go +++ b/internal/webhooks/iam/v1alpha1/user_webhook.go @@ -99,11 +99,10 @@ func (v *UserValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runti // Only allow users to update their own email (self-service only) // This ensures that the UserIdentity list will be scoped to the correct user - // Note: req.UserInfo.UID is the Milo user ID, which matches the User resource name - if req.UserInfo.UID != newUser.Name { + if req.UserInfo.UID != string(newUser.GetUID()) { userlog.Info("Rejecting email update from different user", "requestingUser", req.UserInfo.UID, - "targetUser", newUser.Name) + "targetUser", newUser.GetUID()) return nil, errors.NewForbidden( schema.GroupResource{Group: iamv1alpha1.SchemeGroupVersion.Group, Resource: "users"}, newUser.Name, From 06e581e1504a4a030813c1f112829baa1e9cccd6 Mon Sep 17 00:00:00 2001 From: OscarLlamas6 Date: Fri, 16 Jan 2026 14:29:28 -0600 Subject: [PATCH 10/12] chore: validation adjustment --- internal/webhooks/iam/v1alpha1/user_webhook.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/webhooks/iam/v1alpha1/user_webhook.go b/internal/webhooks/iam/v1alpha1/user_webhook.go index 428871af..3909d9a3 100644 --- a/internal/webhooks/iam/v1alpha1/user_webhook.go +++ b/internal/webhooks/iam/v1alpha1/user_webhook.go @@ -99,10 +99,11 @@ func (v *UserValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runti // Only allow users to update their own email (self-service only) // This ensures that the UserIdentity list will be scoped to the correct user - if req.UserInfo.UID != string(newUser.GetUID()) { + // Note: req.UserInfo.UID is the Milo user ID, which matches the User resource name + if req.UserInfo.UID != newUser.Name { userlog.Info("Rejecting email update from different user", "requestingUser", req.UserInfo.UID, - "targetUser", newUser.GetUID()) + "targetUser", newUser.Name) return nil, errors.NewForbidden( schema.GroupResource{Group: iamv1alpha1.SchemeGroupVersion.Group, Resource: "users"}, newUser.Name, From e74b7d480c9a0ff5c08d7de77c9f7a590936fdc7 Mon Sep 17 00:00:00 2001 From: OscarLlamas6 Date: Fri, 16 Jan 2026 14:53:10 -0600 Subject: [PATCH 11/12] chore: using apireader to get users dinamically --- internal/webhooks/iam/v1alpha1/user_webhook.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/internal/webhooks/iam/v1alpha1/user_webhook.go b/internal/webhooks/iam/v1alpha1/user_webhook.go index 3909d9a3..d77e60a7 100644 --- a/internal/webhooks/iam/v1alpha1/user_webhook.go +++ b/internal/webhooks/iam/v1alpha1/user_webhook.go @@ -26,10 +26,19 @@ var userlog = logf.Log.WithName("user-resource") func SetupUserWebhooksWithManager(mgr ctrl.Manager, systemNamespace string, userSelfManageRoleName string) error { userlog.Info("Setting up iam.miloapis.com user webhooks") + // Create a direct API client for UserIdentity queries (bypasses cache) + apiReader, err := client.New(mgr.GetConfig(), client.Options{ + Scheme: mgr.GetScheme(), + }) + if err != nil { + return fmt.Errorf("failed to create API reader: %w", err) + } + return ctrl.NewWebhookManagedBy(mgr). For(&iamv1alpha1.User{}). WithValidator(&UserValidator{ client: mgr.GetClient(), + apiReader: apiReader, systemNamespace: systemNamespace, userSelfManageRoleName: userSelfManageRoleName, }). @@ -39,6 +48,7 @@ func SetupUserWebhooksWithManager(mgr ctrl.Manager, systemNamespace string, user // UserValidator validates Users type UserValidator struct { client client.Client + apiReader client.Reader // Direct API client (no cache) for UserIdentity queries decoder admission.Decoder systemNamespace string userSelfManageRoleName string @@ -110,11 +120,10 @@ func (v *UserValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runti fmt.Errorf("cannot update email address of another user. Email updates are restricted to self-service only")) } - // Get UserIdentities for the requesting user - // Note: UserIdentity is a dynamic REST API that automatically scopes results - // to the authenticated user (from context), so we don't need manual filtering + // Get UserIdentities for the requesting user using direct API client (no cache) + // UserIdentity is a dynamic REST API, so we use apiReader to bypass the cache identityList := &identityv1alpha1.UserIdentityList{} - if err := v.client.List(ctx, identityList); err != nil { + if err := v.apiReader.List(ctx, identityList); err != nil { userlog.Error(err, "Failed to list user identities", "user", newUser.Name) return nil, errors.NewInternalError(fmt.Errorf("failed to list user identities: %w", err)) } From 23846c9b68866b12ecfe8f37a0a0719f95cd05a6 Mon Sep 17 00:00:00 2001 From: OscarLlamas6 Date: Fri, 16 Jan 2026 15:28:24 -0600 Subject: [PATCH 12/12] feat(webhooks): validate User email against UserIdentity providers - Add User webhook to restrict email updates to verified identity provider emails - Implement impersonated client to query UserIdentity API with proper user context - Block UserIdentity deletion to prevent orphaned identity links - Register identityv1alpha1 in controller-manager Scheme --- .../webhooks/iam/v1alpha1/user_webhook.go | 52 ++++++++++++++----- 1 file changed, 39 insertions(+), 13 deletions(-) diff --git a/internal/webhooks/iam/v1alpha1/user_webhook.go b/internal/webhooks/iam/v1alpha1/user_webhook.go index d77e60a7..62eb5288 100644 --- a/internal/webhooks/iam/v1alpha1/user_webhook.go +++ b/internal/webhooks/iam/v1alpha1/user_webhook.go @@ -4,10 +4,12 @@ import ( "context" "fmt" + authenticationv1 "k8s.io/api/authentication/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/rest" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -26,19 +28,12 @@ var userlog = logf.Log.WithName("user-resource") func SetupUserWebhooksWithManager(mgr ctrl.Manager, systemNamespace string, userSelfManageRoleName string) error { userlog.Info("Setting up iam.miloapis.com user webhooks") - // Create a direct API client for UserIdentity queries (bypasses cache) - apiReader, err := client.New(mgr.GetConfig(), client.Options{ - Scheme: mgr.GetScheme(), - }) - if err != nil { - return fmt.Errorf("failed to create API reader: %w", err) - } - return ctrl.NewWebhookManagedBy(mgr). For(&iamv1alpha1.User{}). WithValidator(&UserValidator{ client: mgr.GetClient(), - apiReader: apiReader, + restConfig: mgr.GetConfig(), + scheme: mgr.GetScheme(), systemNamespace: systemNamespace, userSelfManageRoleName: userSelfManageRoleName, }). @@ -48,7 +43,8 @@ func SetupUserWebhooksWithManager(mgr ctrl.Manager, systemNamespace string, user // UserValidator validates Users type UserValidator struct { client client.Client - apiReader client.Reader // Direct API client (no cache) for UserIdentity queries + restConfig *rest.Config + scheme *runtime.Scheme decoder admission.Decoder systemNamespace string userSelfManageRoleName string @@ -120,10 +116,16 @@ func (v *UserValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runti fmt.Errorf("cannot update email address of another user. Email updates are restricted to self-service only")) } - // Get UserIdentities for the requesting user using direct API client (no cache) - // UserIdentity is a dynamic REST API, so we use apiReader to bypass the cache + // Get UserIdentities for the requesting user using impersonated client + // UserIdentity is a dynamic REST API that requires proper user context + impersonatedClient, err := v.createImpersonatedClient(req.UserInfo) + if err != nil { + userlog.Error(err, "Failed to create impersonated client", "user", newUser.Name) + return nil, errors.NewInternalError(fmt.Errorf("failed to create impersonated client: %w", err)) + } + identityList := &identityv1alpha1.UserIdentityList{} - if err := v.apiReader.List(ctx, identityList); err != nil { + if err := impersonatedClient.List(ctx, identityList); err != nil { userlog.Error(err, "Failed to list user identities", "user", newUser.Name) return nil, errors.NewInternalError(fmt.Errorf("failed to list user identities: %w", err)) } @@ -173,6 +175,30 @@ func (v *UserValidator) ValidateDelete(ctx context.Context, obj runtime.Object) return nil, nil } +// createImpersonatedClient creates a client that impersonates the requesting user +// This is necessary for UserIdentity API calls which require proper user context +func (v *UserValidator) createImpersonatedClient(userInfo authenticationv1.UserInfo) (client.Client, error) { + // Convert Extra from authenticationv1.ExtraValue to map[string][]string + extra := make(map[string][]string) + for k, v := range userInfo.Extra { + extra[k] = []string(v) + } + + // Create a copy of the REST config with impersonation + impersonatedConfig := rest.CopyConfig(v.restConfig) + impersonatedConfig.Impersonate = rest.ImpersonationConfig{ + UserName: userInfo.Username, + UID: userInfo.UID, + Groups: userInfo.Groups, + Extra: extra, + } + + // Create a new client with the impersonated config + return client.New(impersonatedConfig, client.Options{ + Scheme: v.scheme, + }) +} + // createSelfManagePolicyBinding creates a PolicyBinding for the organization owner func (v *UserValidator) createSelfManagePolicyBinding(ctx context.Context, user *iamv1alpha1.User) error { userlog.Info("Attempting to create PolicyBinding for new user", "user", user.Name)