From e0dbb966fc067e109264a7e7923d2fce58eaebac Mon Sep 17 00:00:00 2001 From: Misi Date: Thu, 12 Mar 2026 13:42:22 +0100 Subject: [PATCH] IAM: Add users filtering and improved RBAC mapper for users API (#119100) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * IAM: Add hidden users filtering and improved RBAC mapper for users API - Add StoreWrapper for user resource that filters hidden users on Get/List - Wire up StoreWrapper in the users API group registration - Expand RBAC verb mapping for users to use explicit action translations - Add integration tests for hidden users filtering behavior Co-Authored-By: Claude Sonnet 4.6 * IAM: Fix duplicate user validation and storewrapper context propagation The storewrapper replaced the request context with a service identity (OrgID=0) before invoking createValidation/updateValidation callbacks. Since these callbacks wrap k8s admission webhooks (including the duplicate email/login checks), the validation ran with OrgID=0 causing SearchOrgUsers to return no results, silently passing duplicates through to the DB which then returned a 500 instead of 409. Fix 1 (storewrapper): Add validationWithUserContext and updateValidationWithUserContext helpers that rebind validation callbacks to the original user context before passing them to the inner store. Fix 2 (legacy store): Add toUserConflictError as defense-in-depth that converts SQLite UNIQUE constraint failures on user.email/user.login into proper 409 Conflict API errors in CreateUser and UpdateUser. Co-Authored-By: Claude Sonnet 4.6 * Regen * Use configprovider.ConfigProvider instead of setting.Cfg * Enforce hidden-users restrictions on write operations BeforeCreate, BeforeUpdate, and BeforeDelete in the user StoreWrapper now return HTTP 403 when the target user's login is in the hidden-users list, returning a generic "operation not permitted" message to callers and logging the hidden-user detail server-side via a structured logger. Integration tests are updated to create the user before marking it hidden (so BeforeCreate does not block setup), then verify all four guarded paths (get→404, list filtered, update→403, delete→403) and add a dedicated sub-test that confirms create is blocked once a login is in the hidden list. Co-Authored-By: Claude Sonnet 4.6 * IAM: Add WithPreserveIdentity option to storewrapper Introduces a WithPreserveIdentity() functional option on storewrapper.New() so the users storage path passes the original caller identity through to the inner store instead of replacing it with a service identity. This ensures admission validation (e.g. duplicate email/login checks) runs with the correct OrgID. Adds unit tests for the new option. Co-Authored-By: Claude Sonnet 4.6 * Address feedback * Fix some minor issues * Update pkg/registry/apis/iam/register.go Co-authored-by: Gabriel MABILLE * Address feedback --------- Co-authored-by: Claude Sonnet 4.6 Co-authored-by: Gabriel MABILLE --- go.work.sum | 1 + .../src/apis/iam.grafana.app-v0alpha1.json | 2 +- .../iam/authorizer/external_group_mapping.go | 2 +- .../authorizer/external_group_mapping_test.go | 2 +- .../iam/authorizer/resource_permissions.go | 2 +- .../iam/authorizer/rolebinding_authorizer.go | 4 +- .../iam/authorizer/team_binding_authorizer.go | 2 +- .../team_binding_authorizer_test.go | 2 +- pkg/registry/apis/iam/legacy/user.go | 32 +++- pkg/registry/apis/iam/models.go | 10 + pkg/registry/apis/iam/register.go | 31 +++- pkg/registry/apis/iam/user/store_wrapper.go | 171 ++++++++++++++++++ pkg/registry/apis/iam/user/validate.go | 35 ++++ pkg/registry/apis/iam/user/validate_test.go | 55 +++++- pkg/server/wire_gen.go | 4 +- .../auth/authorizer/storewrapper/wrapper.go | 43 ++++- .../authorizer/storewrapper/wrapper_test.go | 91 +++++++++- pkg/services/authz/rbac/mapper.go | 19 +- pkg/tests/apis/iam/iam_test.go | 5 +- .../apis/iam/team_binding_integration_test.go | 7 +- .../team_members_search_integration_test.go | 7 +- pkg/tests/apis/iam/user_integration_test.go | 107 ++++++++++- .../apis/iam/user_search_integration_test.go | 35 ++-- .../iam/user_teams_search_integration_test.go | 7 +- .../iam.grafana.app-v0alpha1.json | 2 +- pkg/tests/apis/openapi_test.go | 3 +- pkg/tests/testinfra/testinfra.go | 5 + 27 files changed, 618 insertions(+), 68 deletions(-) create mode 100644 pkg/registry/apis/iam/user/store_wrapper.go diff --git a/go.work.sum b/go.work.sum index 7a03469731c..18e82797042 100644 --- a/go.work.sum +++ b/go.work.sum @@ -271,6 +271,7 @@ contrib.go.opencensus.io/exporter/ocagent v0.6.0 h1:Z1n6UAyr0QwM284yUuh5Zd8JlvxU contrib.go.opencensus.io/exporter/stackdriver v0.13.15-0.20230702191903-2de6d2748484 h1:xRc46S76eyn4ZF3jWX8I+aUSKVLw5EQ1aDvHwfV5W1o= contrib.go.opencensus.io/exporter/stackdriver v0.13.15-0.20230702191903-2de6d2748484/go.mod h1:uxw+4/0SiKbbVSD/F2tk5pJTdVcfIBBcsQ8gwcu4X+E= dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9 h1:VpgP7xuJadIUuKccphEpTJnWhS2jkQyMt6Y7pJCD7fY= +filippo.io/edwards25519 v1.2.0/go.mod h1:xzAOLCNug/yB62zG1bQ8uziwrIqIuxhctzJT18Q77mc= gioui.org v0.2.0 h1:RbzDn1h/pCVf/q44ImQSa/J3MIFpY3OWphzT/Tyei+w= gioui.org v0.2.0/go.mod h1:1H72sKEk/fNFV+l0JNeM2Dt3co3Y4uaQcD+I+/GQ0e4= gioui.org/cpu v0.0.0-20220412190645-f1e9e8c3b1f7 h1:tNJdnP5CgM39PRc+KWmBRRYX/zJ+rd5XaYxY5d5veqA= diff --git a/packages/grafana-openapi/src/apis/iam.grafana.app-v0alpha1.json b/packages/grafana-openapi/src/apis/iam.grafana.app-v0alpha1.json index 772832075c2..70febfe11e8 100644 --- a/packages/grafana-openapi/src/apis/iam.grafana.app-v0alpha1.json +++ b/packages/grafana-openapi/src/apis/iam.grafana.app-v0alpha1.json @@ -3855,7 +3855,7 @@ "/users": { "get": { "tags": ["User"], - "description": "list objects of kind User", + "description": "list or watch objects of kind User", "operationId": "listUser", "parameters": [ { diff --git a/pkg/registry/apis/iam/authorizer/external_group_mapping.go b/pkg/registry/apis/iam/authorizer/external_group_mapping.go index 9535d220089..587c0d38622 100644 --- a/pkg/registry/apis/iam/authorizer/external_group_mapping.go +++ b/pkg/registry/apis/iam/authorizer/external_group_mapping.go @@ -73,7 +73,7 @@ func (r *ExternalGroupMappingAuthorizer) BeforeDelete(ctx context.Context, obj r } // BeforeUpdate implements ResourceStorageAuthorizer. -func (r *ExternalGroupMappingAuthorizer) BeforeUpdate(ctx context.Context, obj runtime.Object) error { +func (r *ExternalGroupMappingAuthorizer) BeforeUpdate(ctx context.Context, oldObj, obj runtime.Object) error { // Update is not supported for ExternalGroupMapping resources and update attempts are blocked at a lower level, // so this is just a safeguard. return apierrors.NewMethodNotSupported(iamv0.ExternalGroupMappingResourceInfo.GroupResource(), "PUT/PATCH") diff --git a/pkg/registry/apis/iam/authorizer/external_group_mapping_test.go b/pkg/registry/apis/iam/authorizer/external_group_mapping_test.go index 69ee7e5fed9..d948de09e93 100644 --- a/pkg/registry/apis/iam/authorizer/external_group_mapping_test.go +++ b/pkg/registry/apis/iam/authorizer/external_group_mapping_test.go @@ -173,7 +173,7 @@ func TestExternalGroupMapping_BeforeUpdate(t *testing.T) { authz := NewExternalGroupMappingAuthorizer(accessClient) ctx := types.WithAuthInfo(context.Background(), user) - err := authz.BeforeUpdate(ctx, mapping) + err := authz.BeforeUpdate(ctx, mapping, mapping) require.Error(t, err) require.True(t, apierrors.IsMethodNotSupported(err)) require.Contains(t, err.Error(), "PUT/PATCH") diff --git a/pkg/registry/apis/iam/authorizer/resource_permissions.go b/pkg/registry/apis/iam/authorizer/resource_permissions.go index 3cbf5baf371..5a0cde73817 100644 --- a/pkg/registry/apis/iam/authorizer/resource_permissions.go +++ b/pkg/registry/apis/iam/authorizer/resource_permissions.go @@ -160,7 +160,7 @@ func (r *ResourcePermissionsAuthorizer) BeforeDelete(ctx context.Context, obj ru } // BeforeUpdate implements ResourceStorageAuthorizer. -func (r *ResourcePermissionsAuthorizer) BeforeUpdate(ctx context.Context, obj runtime.Object) error { +func (r *ResourcePermissionsAuthorizer) BeforeUpdate(ctx context.Context, oldObj, obj runtime.Object) error { return r.beforeWrite(ctx, obj) } diff --git a/pkg/registry/apis/iam/authorizer/rolebinding_authorizer.go b/pkg/registry/apis/iam/authorizer/rolebinding_authorizer.go index 4de4efee6cf..a883992ad4b 100644 --- a/pkg/registry/apis/iam/authorizer/rolebinding_authorizer.go +++ b/pkg/registry/apis/iam/authorizer/rolebinding_authorizer.go @@ -38,7 +38,7 @@ func (a *RoleBindingAuthorizer) BeforeCreate(ctx context.Context, obj runtime.Ob return a.beforeWrite(ctx, obj) } -func (a *RoleBindingAuthorizer) BeforeUpdate(ctx context.Context, obj runtime.Object) error { +func (a *RoleBindingAuthorizer) BeforeUpdate(ctx context.Context, oldObj, obj runtime.Object) error { return a.beforeWrite(ctx, obj) } @@ -96,7 +96,7 @@ func (d *DenyCustomRoleRefsAuthorizer) BeforeCreate(ctx context.Context, obj run return d.beforeWrite(ctx, obj) } -func (d *DenyCustomRoleRefsAuthorizer) BeforeUpdate(ctx context.Context, obj runtime.Object) error { +func (d *DenyCustomRoleRefsAuthorizer) BeforeUpdate(ctx context.Context, oldObj, obj runtime.Object) error { return d.beforeWrite(ctx, obj) } diff --git a/pkg/registry/apis/iam/authorizer/team_binding_authorizer.go b/pkg/registry/apis/iam/authorizer/team_binding_authorizer.go index 58bb157dbaf..6e8be225420 100644 --- a/pkg/registry/apis/iam/authorizer/team_binding_authorizer.go +++ b/pkg/registry/apis/iam/authorizer/team_binding_authorizer.go @@ -79,7 +79,7 @@ func (r *TeamBindingAuthorizer) BeforeDelete(ctx context.Context, obj runtime.Ob } // BeforeUpdate implements ResourceStorageAuthorizer. -func (r *TeamBindingAuthorizer) BeforeUpdate(ctx context.Context, obj runtime.Object) error { +func (r *TeamBindingAuthorizer) BeforeUpdate(ctx context.Context, oldObj, obj runtime.Object) error { return r.beforeWrite(ctx, obj) } diff --git a/pkg/registry/apis/iam/authorizer/team_binding_authorizer_test.go b/pkg/registry/apis/iam/authorizer/team_binding_authorizer_test.go index 8faddf481e8..4f4778f9974 100644 --- a/pkg/registry/apis/iam/authorizer/team_binding_authorizer_test.go +++ b/pkg/registry/apis/iam/authorizer/team_binding_authorizer_test.go @@ -199,7 +199,7 @@ func TestTeamBinding_BeforeUpdate(t *testing.T) { authz := NewTeamBindingAuthorizer(accessClient) ctx := types.WithAuthInfo(context.Background(), user) - err := authz.BeforeUpdate(ctx, binding) + err := authz.BeforeUpdate(ctx, binding, binding) if tt.shouldAllow { require.NoError(t, err) } else { diff --git a/pkg/registry/apis/iam/legacy/user.go b/pkg/registry/apis/iam/legacy/user.go index cad93d0e221..651c866b874 100644 --- a/pkg/registry/apis/iam/legacy/user.go +++ b/pkg/registry/apis/iam/legacy/user.go @@ -5,17 +5,21 @@ import ( stdsql "database/sql" "errors" "fmt" + "strings" "text/template" "time" claims "github.com/grafana/authlib/types" + iamv0 "github.com/grafana/grafana/apps/iam/pkg/apis/iam/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/iam/common" + "github.com/grafana/grafana/pkg/services/sqlstore/migrator" "github.com/grafana/grafana/pkg/services/sqlstore/session" "github.com/grafana/grafana/pkg/services/team" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/storage/legacysql" "github.com/grafana/grafana/pkg/storage/unified/sql/sqltemplate" "github.com/grafana/grafana/pkg/util" + apierrors "k8s.io/apimachinery/pkg/api/errors" ) type GetUserInternalIDQuery struct { @@ -472,12 +476,36 @@ func (s *legacySQLStore) CreateUser(ctx context.Context, ns claims.NamespaceInfo }) if err != nil { - return nil, err + return nil, toUserConflictError(sql.DB.GetDialect(), err, cmd.UID) } return &CreateUserResult{User: createdUser}, nil } +// toUserConflictError converts a UNIQUE constraint violation on user.email or +// user.login into a 409 Conflict API error. All other errors are returned as-is. +// It uses the DB dialect to detect the violation across SQLite, MySQL and PostgreSQL. +func toUserConflictError(dialect migrator.Dialect, err error, uid string) error { + if err == nil { + return nil + } + if dialect != nil && dialect.IsUniqueConstraintViolation(err) { + msg := dialect.ErrorMessage(err) + switch { + case strings.Contains(msg, "email"): + return apierrors.NewConflict(iamv0.UserResourceInfo.GroupResource(), uid, + fmt.Errorf("email is already taken")) + case strings.Contains(msg, "login"): + return apierrors.NewConflict(iamv0.UserResourceInfo.GroupResource(), uid, + fmt.Errorf("login is already taken")) + default: + return apierrors.NewConflict(iamv0.UserResourceInfo.GroupResource(), uid, + fmt.Errorf("user already exists")) + } + } + return err +} + func newDeleteUser(sql *legacysql.LegacyDatabaseHelper, cmd *DeleteUserCommand) deleteUserQuery { return deleteUserQuery{ SQLTemplate: sqltemplate.New(sql.DialectForDriver()), @@ -733,7 +761,7 @@ func (s *legacySQLStore) UpdateUser(ctx context.Context, ns claims.NamespaceInfo }) if err != nil { - return nil, err + return nil, toUserConflictError(sql.DB.GetDialect(), err, cmd.UID) } return &UpdateUserResult{User: updatedUser}, nil diff --git a/pkg/registry/apis/iam/models.go b/pkg/registry/apis/iam/models.go index c82ed4fe341..677567e9d96 100644 --- a/pkg/registry/apis/iam/models.go +++ b/pkg/registry/apis/iam/models.go @@ -6,6 +6,7 @@ import ( "github.com/grafana/authlib/types" + "github.com/grafana/grafana/pkg/configprovider" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" iamauthorizer "github.com/grafana/grafana/pkg/registry/apis/iam/authorizer" @@ -108,4 +109,13 @@ type IdentityAccessManagementAPIBuilder struct { features featuremgmt.FeatureToggles tracing tracing.Tracer + + cfgProvider configprovider.ConfigProvider + + apiConfig Config +} + +// Config holds IAM-specific configuration +type Config struct { + SingleOrganization bool } diff --git a/pkg/registry/apis/iam/register.go b/pkg/registry/apis/iam/register.go index 0ac4763fb07..653666f6098 100644 --- a/pkg/registry/apis/iam/register.go +++ b/pkg/registry/apis/iam/register.go @@ -30,6 +30,7 @@ import ( "github.com/grafana/grafana/pkg/apimachinery/utils" legacyiamv0 "github.com/grafana/grafana/pkg/apis/iam/v0alpha1" grafanaregistry "github.com/grafana/grafana/pkg/apiserver/registry/generic" + "github.com/grafana/grafana/pkg/configprovider" "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" @@ -65,6 +66,7 @@ const MaxConcurrentZanzanaWrites = 20 func RegisterAPIService( cfg *setting.Cfg, + cfgProvider configprovider.ConfigProvider, features featuremgmt.FeatureToggles, apiregistration builder.APIRegistrar, ssoService ssosettings.Service, @@ -134,8 +136,12 @@ func RegisterAPIService( unified: unified, userSearchClient: resource.NewSearchClient(dualwrite.NewSearchAdapter(dual), iamv0.UserResourceInfo.GroupResource(), unified, user.NewUserLegacySearchClient(orgService, tracing, cfg), features), - teamSearch: NewTeamSearchHandler(tracing, dual, team.NewLegacyTeamSearchClient(teamService, tracing), unified, features, accessClient), - tracing: tracing, + teamSearch: NewTeamSearchHandler(tracing, dual, team.NewLegacyTeamSearchClient(teamService, tracing), unified, features, accessClient), + tracing: tracing, + cfgProvider: cfgProvider, + apiConfig: Config{ + SingleOrganization: cfg.RBAC.SingleOrganization, + }, } builder.userSearchHandler = user.NewSearchHandler(tracing, builder.userSearchClient, features, cfg, accessClient) @@ -192,6 +198,7 @@ func NewAPIService( reg: reg, roleApiInstaller: roleApiInstaller, globalRoleApiInstaller: globalRoleApiInstaller, + apiConfig: Config{SingleOrganization: true}, teamLBACApiInstaller: teamLBACApiInstaller, authorizer: authorizer.AuthorizerFunc( func(ctx context.Context, a authorizer.Attributes) (authorizer.Decision, string, error) { @@ -323,7 +330,8 @@ func (b *IdentityAccessManagementAPIBuilder) UpdateAPIGroupInfo(apiGroupInfo *ge enableRoleBindingsApi := client.Boolean(ctx, featuremgmt.FlagKubernetesAuthzRoleBindingsApi, false, openfeature.TransactionContext(ctx)) enableGlobalRolesApi := client.Boolean(ctx, featuremgmt.FlagKubernetesAuthzGlobalRolesApi, false, openfeature.TransactionContext(ctx)) enableTeamLBACRuleApi := client.Boolean(ctx, featuremgmt.FlagKubernetesAuthzTeamLBACRuleApi, false, openfeature.TransactionContext(ctx)) - enableUserApi := client.Boolean(ctx, featuremgmt.FlagKubernetesUsersApi, false, openfeature.TransactionContext(ctx)) + // Until users have been fully migrated to namespaces (no global users), we only enable the users api in single organization setups. + enableUserApi := b.isSingleOrgSetup() && client.Boolean(ctx, featuremgmt.FlagKubernetesUsersApi, false, openfeature.TransactionContext(ctx)) enableServiceAccountsApi := client.Boolean(ctx, featuremgmt.FlagKubernetesServiceAccountsApi, false, openfeature.TransactionContext(ctx)) enableServiceAccountTokensApi := client.Boolean(ctx, featuremgmt.FlagKubernetesServiceAccountTokensApi, false, openfeature.TransactionContext(ctx)) enableExternalGroupMappingsApi := client.Boolean(ctx, featuremgmt.FlagKubernetesExternalGroupMappingsApi, false, openfeature.TransactionContext(ctx)) @@ -494,7 +502,6 @@ func (b *IdentityAccessManagementAPIBuilder) UpdateUsersAPIGroup(opts builder.AP if err != nil { return err } - storage[userResource.StoragePath()] = userUniStore if enableZanzanaSync { b.logger.Info("Enabling hooks for User to sync basic role assignments to Zanzana") @@ -503,15 +510,23 @@ func (b *IdentityAccessManagementAPIBuilder) UpdateUsersAPIGroup(opts builder.AP userUniStore.AfterDelete = b.AfterUserDelete } + var userStore storewrapper.K8sStorage = userUniStore + if b.userLegacyStore != nil { dw, err := opts.DualWriteBuilder(userResource.GroupResource(), b.userLegacyStore, userUniStore) if err != nil { return err } - storage[userResource.StoragePath()] = dw + var ok bool + userStore, ok = dw.(storewrapper.K8sStorage) + if !ok { + return fmt.Errorf("expected storewrapper.K8sStorage, got %T", dw) + } } + storage[userResource.StoragePath()] = storewrapper.New(userStore, user.NewStoreWrapper(b.cfgProvider), storewrapper.WithPreserveIdentity()) + if b.dual != nil && b.unified != nil { legacyTeamBindingSearchClient := teambinding.NewLegacyTeamBindingSearchClient(b.store, b.tracing) @@ -766,7 +781,7 @@ func (b *IdentityAccessManagementAPIBuilder) GetAPIRoutes(gv schema.GroupVersion ctx, cancelFn := context.WithTimeout(context.Background(), time.Second*5) defer cancelFn() - enableUserApi := client.Boolean(ctx, featuremgmt.FlagKubernetesUsersApi, false, openfeature.TransactionContext(ctx)) + enableUserApi := b.isSingleOrgSetup() && client.Boolean(ctx, featuremgmt.FlagKubernetesUsersApi, false, openfeature.TransactionContext(ctx)) enableExternalGroupMappingsApi := client.Boolean(ctx, featuremgmt.FlagKubernetesExternalGroupMappingsApi, false, openfeature.TransactionContext(ctx)) searchRoutes := make([]*builder.APIRoutes, 0, 3) @@ -971,6 +986,10 @@ func NewLocalStore(resourceInfo utils.ResourceInfo, scheme *runtime.Scheme, defa return store, err } +func (b *IdentityAccessManagementAPIBuilder) isSingleOrgSetup() bool { + return b.apiConfig.SingleOrganization +} + func mergeAPIRoutes(routes ...*builder.APIRoutes) *builder.APIRoutes { merged := &builder.APIRoutes{} for _, r := range routes { diff --git a/pkg/registry/apis/iam/user/store_wrapper.go b/pkg/registry/apis/iam/user/store_wrapper.go new file mode 100644 index 00000000000..af0422c57d6 --- /dev/null +++ b/pkg/registry/apis/iam/user/store_wrapper.go @@ -0,0 +1,171 @@ +package user + +import ( + "context" + "errors" + + claims "github.com/grafana/authlib/types" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + + iamv0 "github.com/grafana/grafana/apps/iam/pkg/apis/iam/v0alpha1" + "github.com/grafana/grafana/pkg/configprovider" + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/apiserver/auth/authorizer/storewrapper" +) + +// StoreWrapper filters users based on the hidden users configuration. +// It does not enforce any write authorization — those are handled at the API level. +type StoreWrapper struct { + cfgProvider configprovider.ConfigProvider + logger log.Logger +} + +var _ storewrapper.ResourceStorageAuthorizer = (*StoreWrapper)(nil) + +func NewStoreWrapper(cfgProvider configprovider.ConfigProvider) *StoreWrapper { + return &StoreWrapper{ + cfgProvider: cfgProvider, + logger: log.New("grafana-apiserver.users.storewrapper"), + } +} + +// AfterGet returns NotFound if the user's login is in the hidden users list +// and the requester is not the user themselves. +func (f *StoreWrapper) AfterGet(ctx context.Context, obj runtime.Object) error { + cfg, err := f.cfgProvider.Get(ctx) + if err != nil { + return err + } + if len(cfg.HiddenUsers) == 0 { + return nil + } + + authInfo, ok := claims.AuthInfoFrom(ctx) + if !ok { + return storewrapper.ErrUnauthenticated + } + + u, ok := obj.(*iamv0.User) + if !ok { + return nil + } + + login := u.Spec.Login + if _, isHidden := cfg.HiddenUsers[login]; isHidden && login != authInfo.GetUsername() { + return apierrors.NewNotFound(iamv0.UserResourceInfo.GroupResource(), u.Name) + } + + return nil +} + +// FilterList removes hidden users from the list, except for the requester themselves. +func (f *StoreWrapper) FilterList(ctx context.Context, list runtime.Object) (runtime.Object, error) { + cfg, err := f.cfgProvider.Get(ctx) + if err != nil { + return nil, err + } + if len(cfg.HiddenUsers) == 0 { + return list, nil + } + + authInfo, ok := claims.AuthInfoFrom(ctx) + if !ok { + return nil, storewrapper.ErrUnauthenticated + } + + userList, ok := list.(*iamv0.UserList) + if !ok { + return list, nil + } + + requesterLogin := authInfo.GetUsername() + filtered := make([]iamv0.User, 0, len(userList.Items)) + for _, u := range userList.Items { + if _, isHidden := cfg.HiddenUsers[u.Spec.Login]; !isHidden || u.Spec.Login == requesterLogin { + filtered = append(filtered, u) + } + } + userList.Items = filtered + return userList, nil +} + +// BeforeCreate returns Forbidden if the new user's login is in the hidden users list. +func (f *StoreWrapper) BeforeCreate(ctx context.Context, obj runtime.Object) error { + cfg, err := f.cfgProvider.Get(ctx) + if err != nil { + return err + } + if len(cfg.HiddenUsers) == 0 { + return nil + } + + u, ok := obj.(*iamv0.User) + if !ok { + return nil + } + + if _, isHidden := cfg.HiddenUsers[u.Spec.Login]; isHidden { + f.logger.Info("blocked create for hidden user", "login", u.Spec.Login, "name", u.Name) + return apierrors.NewForbidden(iamv0.UserResourceInfo.GroupResource(), u.Name, errors.New("operation not permitted")) + } + + return nil +} + +// BeforeUpdate returns Forbidden if the target user (old object) or the new login is in the hidden users list. +func (f *StoreWrapper) BeforeUpdate(ctx context.Context, oldObj, obj runtime.Object) error { + cfg, err := f.cfgProvider.Get(ctx) + if err != nil { + return err + } + if len(cfg.HiddenUsers) == 0 { + return nil + } + + oldUser, ok := oldObj.(*iamv0.User) + if !ok { + return nil + } + + if _, isHidden := cfg.HiddenUsers[oldUser.Spec.Login]; isHidden { + f.logger.Info("blocked update for hidden user", "login", oldUser.Spec.Login, "name", oldUser.Name) + return apierrors.NewForbidden(iamv0.UserResourceInfo.GroupResource(), oldUser.Name, errors.New("operation not permitted")) + } + + // Also check the new object in case the login is being changed to a hidden user's login. + newUser, ok := obj.(*iamv0.User) + if !ok { + return nil + } + + if _, isHidden := cfg.HiddenUsers[newUser.Spec.Login]; isHidden { + f.logger.Info("blocked update to hidden user login", "login", newUser.Spec.Login, "name", newUser.Name) + return apierrors.NewForbidden(iamv0.UserResourceInfo.GroupResource(), newUser.Name, errors.New("operation not permitted")) + } + + return nil +} + +// BeforeDelete returns Forbidden if the target user is in the hidden users list. +func (f *StoreWrapper) BeforeDelete(ctx context.Context, obj runtime.Object) error { + cfg, err := f.cfgProvider.Get(ctx) + if err != nil { + return err + } + if len(cfg.HiddenUsers) == 0 { + return nil + } + + u, ok := obj.(*iamv0.User) + if !ok { + return nil + } + + if _, isHidden := cfg.HiddenUsers[u.Spec.Login]; isHidden { + f.logger.Info("blocked delete for hidden user", "login", u.Spec.Login, "name", u.Name) + return apierrors.NewForbidden(iamv0.UserResourceInfo.GroupResource(), u.Name, errors.New("operation not permitted")) + } + + return nil +} diff --git a/pkg/registry/apis/iam/user/validate.go b/pkg/registry/apis/iam/user/validate.go index 1fa1d28a875..d91790916b4 100644 --- a/pkg/registry/apis/iam/user/validate.go +++ b/pkg/registry/apis/iam/user/validate.go @@ -3,6 +3,7 @@ package user import ( "context" "fmt" + "reflect" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/selection" @@ -86,6 +87,15 @@ func ValidateOnUpdate(ctx context.Context, userSearchClient resourcepb.ResourceI } } + // Only service identities or Grafana admins may update profile fields (login, email, title, etc.). + // OrgAdmins are allowed through the RBAC gate with "org.users:write" but + // are restricted to only role updates. + if !isServiceUser && !isGrafanaAdmin && !onlyAllowedFieldsChanged(oldObj.Spec, newObj.Spec) { + return apierrors.NewForbidden(iamv0alpha1.UserResourceInfo.GroupResource(), + newObj.Name, + fmt.Errorf("updating fields beyond org role requires service identity or grafana admin")) + } + if newObj.Spec.Login == "" && newObj.Spec.Email == "" { return apierrors.NewBadRequest("user must have either login or email") } @@ -109,6 +119,31 @@ func ValidateOnUpdate(ctx context.Context, userSearchClient resourcepb.ResourceI return nil } +// allowedFieldsForNonServiceUsers lists the UserSpec fields that non-service users +// are allowed to change. Role is the only one for now. Any field NOT in this set requires a service identity to modify. +var allowedFieldsForNonServiceUsers = map[string]bool{ + "Role": true, +} + +// onlyAllowedFieldsChanged iterates over all fields of UserSpec via reflection and +// returns true only if every field that changed is in the allowed set. This is +// forward-compatible: any new field added to UserSpec is automatically restricted +// to service identities without needing to update this function. +func onlyAllowedFieldsChanged(oldSpec, newSpec iamv0alpha1.UserSpec) bool { + oldVal := reflect.ValueOf(oldSpec) + newVal := reflect.ValueOf(newSpec) + + for i := range oldVal.NumField() { + if allowedFieldsForNonServiceUsers[oldVal.Type().Field(i).Name] { + continue + } + if !reflect.DeepEqual(oldVal.Field(i).Interface(), newVal.Field(i).Interface()) { + return false + } + } + return true +} + func validateRole(obj *iamv0alpha1.User) error { if obj.Spec.Role == "" { return apierrors.NewBadRequest("role is required") diff --git a/pkg/registry/apis/iam/user/validate_test.go b/pkg/registry/apis/iam/user/validate_test.go index 17909f2ca7a..acfa134d59d 100644 --- a/pkg/registry/apis/iam/user/validate_test.go +++ b/pkg/registry/apis/iam/user/validate_test.go @@ -307,7 +307,7 @@ func TestValidateOnUpdate(t *testing.T) { Spec: iamv0alpha1.UserSpec{Login: "", Email: "", Role: "Viewer"}, }, requester: &identity.StaticRequester{ - Type: types.TypeUser, + Type: types.TypeAccessPolicy, }, expectError: true, errorContains: "user must have either login or email", @@ -321,8 +321,7 @@ func TestValidateOnUpdate(t *testing.T) { Spec: iamv0alpha1.UserSpec{Login: "testuser", Email: "", Role: "Viewer"}, }, requester: &identity.StaticRequester{ - Type: types.TypeUser, - IsGrafanaAdmin: true, + Type: types.TypeAccessPolicy, }, searchClient: &FakeUserLegacySearchClient{}, expectError: false, @@ -336,8 +335,7 @@ func TestValidateOnUpdate(t *testing.T) { Spec: iamv0alpha1.UserSpec{Login: "", Email: "test@example", Role: "Viewer"}, }, requester: &identity.StaticRequester{ - Type: types.TypeUser, - IsGrafanaAdmin: true, + Type: types.TypeAccessPolicy, }, searchClient: &FakeUserLegacySearchClient{}, expectError: false, @@ -471,6 +469,49 @@ func TestValidateOnUpdate(t *testing.T) { }, expectError: false, }, + { + name: "non-service non-admin user updating role field is allowed", + oldUser: &iamv0alpha1.User{ + Spec: iamv0alpha1.UserSpec{Login: "testuser", Email: "user@example", Role: "Viewer"}, + }, + newUser: &iamv0alpha1.User{ + Spec: iamv0alpha1.UserSpec{Login: "testuser", Email: "user@example", Role: "Editor"}, + }, + requester: &identity.StaticRequester{ + Type: types.TypeUser, + IsGrafanaAdmin: false, + }, + expectError: false, + }, + { + name: "non-service non-admin user updating non-role fields is forbidden", + oldUser: &iamv0alpha1.User{ + Spec: iamv0alpha1.UserSpec{Login: "testuser", Email: "old@example", Role: "Viewer"}, + }, + newUser: &iamv0alpha1.User{ + Spec: iamv0alpha1.UserSpec{Login: "testuser", Email: "new@example", Role: "Viewer"}, + }, + requester: &identity.StaticRequester{ + Type: types.TypeUser, + IsGrafanaAdmin: false, + }, + expectError: true, + errorContains: "updating fields beyond org role requires service identity or grafana admin", + }, + { + name: "grafana admin updating non-role fields is allowed", + oldUser: &iamv0alpha1.User{ + Spec: iamv0alpha1.UserSpec{Login: "testuser", Email: "user@example", Title: "Old Title", Role: "Viewer"}, + }, + newUser: &iamv0alpha1.User{ + Spec: iamv0alpha1.UserSpec{Login: "testuser", Email: "user@example", Title: "New Title", Role: "Viewer"}, + }, + requester: &identity.StaticRequester{ + Type: types.TypeUser, + IsGrafanaAdmin: true, + }, + expectError: false, + }, { name: "update with existing email", oldUser: &iamv0alpha1.User{ @@ -486,7 +527,7 @@ func TestValidateOnUpdate(t *testing.T) { Spec: iamv0alpha1.UserSpec{Email: "two@example", Role: "Viewer"}, }, requester: &identity.StaticRequester{ - Type: types.TypeUser, + Type: types.TypeAccessPolicy, IsGrafanaAdmin: true, }, searchClient: &FakeUserLegacySearchClient{ @@ -512,7 +553,7 @@ func TestValidateOnUpdate(t *testing.T) { Spec: iamv0alpha1.UserSpec{Login: "two", Role: "Viewer"}, }, requester: &identity.StaticRequester{ - Type: types.TypeUser, + Type: types.TypeAccessPolicy, IsGrafanaAdmin: true, }, searchClient: &FakeUserLegacySearchClient{ diff --git a/pkg/server/wire_gen.go b/pkg/server/wire_gen.go index 1871ec1329e..ad232c40736 100644 --- a/pkg/server/wire_gen.go +++ b/pkg/server/wire_gen.go @@ -910,7 +910,7 @@ func Initialize(ctx context.Context, cfg *setting.Cfg, opts Options, apiOpts api storageBackendImpl := noopstorage.ProvideStorageBackend() noopTeamGroupsREST := externalgroupmapping.ProvideNoopTeamGroupsREST() noopSearchREST := externalgroupmapping.ProvideNoopSearchREST() - identityAccessManagementAPIBuilder, err := iam.RegisterAPIService(cfg, featureToggles, apiserverService, ssosettingsimplService, sqlStore, accessControl, accessClient, zanzanaClient, registerer, roleApiInstaller, globalRoleApiInstaller, teamLBACApiInstaller, externalGroupMappingApiInstaller, tracingService, storageBackendImpl, noopTeamGroupsREST, noopSearchREST, dualwriteService, resourceClient, orgService, userService, teamService, eventualRestConfigProvider) + identityAccessManagementAPIBuilder, err := iam.RegisterAPIService(cfg, configProvider, featureToggles, apiserverService, ssosettingsimplService, sqlStore, accessControl, accessClient, zanzanaClient, registerer, roleApiInstaller, globalRoleApiInstaller, teamLBACApiInstaller, externalGroupMappingApiInstaller, tracingService, storageBackendImpl, noopTeamGroupsREST, noopSearchREST, dualwriteService, resourceClient, orgService, userService, teamService, eventualRestConfigProvider) if err != nil { return nil, err } @@ -1605,7 +1605,7 @@ func InitializeForTest(ctx context.Context, t sqlutil.ITestDB, testingT interfac storageBackendImpl := noopstorage.ProvideStorageBackend() noopTeamGroupsREST := externalgroupmapping.ProvideNoopTeamGroupsREST() noopSearchREST := externalgroupmapping.ProvideNoopSearchREST() - identityAccessManagementAPIBuilder, err := iam.RegisterAPIService(cfg, featureToggles, apiserverService, ssosettingsimplService, sqlStore, accessControl, accessClient, zanzanaClient, registerer, roleApiInstaller, globalRoleApiInstaller, teamLBACApiInstaller, externalGroupMappingApiInstaller, tracingService, storageBackendImpl, noopTeamGroupsREST, noopSearchREST, dualwriteService, resourceClient, orgService, userService, teamService, eventualRestConfigProvider) + identityAccessManagementAPIBuilder, err := iam.RegisterAPIService(cfg, configProvider, featureToggles, apiserverService, ssosettingsimplService, sqlStore, accessControl, accessClient, zanzanaClient, registerer, roleApiInstaller, globalRoleApiInstaller, teamLBACApiInstaller, externalGroupMappingApiInstaller, tracingService, storageBackendImpl, noopTeamGroupsREST, noopSearchREST, dualwriteService, resourceClient, orgService, userService, teamService, eventualRestConfigProvider) if err != nil { return nil, err } diff --git a/pkg/services/apiserver/auth/authorizer/storewrapper/wrapper.go b/pkg/services/apiserver/auth/authorizer/storewrapper/wrapper.go index 35139e9e07b..9082e1709ac 100644 --- a/pkg/services/apiserver/auth/authorizer/storewrapper/wrapper.go +++ b/pkg/services/apiserver/auth/authorizer/storewrapper/wrapper.go @@ -24,7 +24,7 @@ var ( // ResourceStorageAuthorizer defines authorization hooks for resource storage operations. type ResourceStorageAuthorizer interface { BeforeCreate(ctx context.Context, obj runtime.Object) error - BeforeUpdate(ctx context.Context, obj runtime.Object) error + BeforeUpdate(ctx context.Context, oldObj, obj runtime.Object) error BeforeDelete(ctx context.Context, obj runtime.Object) error AfterGet(ctx context.Context, obj runtime.Object) error FilterList(ctx context.Context, list runtime.Object) (runtime.Object, error) @@ -34,9 +34,25 @@ type ResourceStorageAuthorizer interface { // It overrides the identity in the context to use service identity for the underlying store operations so the // store's authorization always succeeds and the wrapper enforces authorization. The wrapper injects the original // user's UID as metadata identity so unistore can set createdBy/updatedBy correctly (see identity.WithOriginalIdentityUID). +// The wrapper also supports an option to preserve the original caller's identity in the context for inner store calls instead of replacing it with a service identity. +// Use this when the inner store does not perform its own RBAC checks and the caller's identity is needed downstream (e.g. for admission webhooks). type Wrapper struct { - inner K8sStorage - authorizer ResourceStorageAuthorizer + inner K8sStorage + authorizer ResourceStorageAuthorizer + preserveIdentity bool +} + +// Option configures a Wrapper. +type Option func(*Wrapper) + +// WithPreserveIdentity instructs the Wrapper to leave the caller's identity in the context when +// calling the inner store, instead of replacing it with a service identity. Use this when the inner +// store does not perform its own RBAC checks and the caller's identity is needed downstream +// (e.g. for admission webhooks). +func WithPreserveIdentity() Option { + return func(w *Wrapper) { + w.preserveIdentity = true + } } type K8sStorage interface { @@ -54,17 +70,27 @@ var _ k8srest.Watcher = (*Wrapper)(nil) // New returns a Wrapper that enforces authorization and uses service identity for inner store calls, // injecting the original user's UID for createdBy/updatedBy annotations. -func New(store K8sStorage, authz ResourceStorageAuthorizer) *Wrapper { - return &Wrapper{inner: store, authorizer: authz} +func New(store K8sStorage, authz ResourceStorageAuthorizer, opts ...Option) *Wrapper { + w := &Wrapper{inner: store, authorizer: authz} + for _, opt := range opts { + opt(w) + } + return w } // storeCtx returns the context for inner store calls: service identity so the store's authorization // succeeds, with the original user's UID injected as metadata identity for createdBy/updatedBy (see identity.WithOriginalIdentityUID). +// When preserveIdentity is true the original caller context is returned unchanged; func (w *Wrapper) storeCtx(ctx context.Context) context.Context { + if w.preserveIdentity { + return ctx + } + srvCtx, _ := identity.WithServiceIdentity(ctx, 0) if user, err := identity.GetRequester(ctx); err == nil && user.GetUID() != "" { srvCtx = identity.WithOriginalIdentityUID(srvCtx, user.GetUID()) } + return srvCtx } @@ -78,6 +104,7 @@ func (w *Wrapper) Create(ctx context.Context, obj runtime.Object, createValidati if err != nil { return nil, err } + return w.inner.Create(w.storeCtx(ctx), obj, createValidation, options) } @@ -188,7 +215,7 @@ func (a *authorizedUpdateInfo) UpdatedObject(ctx context.Context, oldObj runtime } // Enforce authorization using the original user context - if err := a.authorizer.BeforeUpdate(a.userCtx, updatedObj); err != nil { + if err := a.authorizer.BeforeUpdate(a.userCtx, oldObj, updatedObj); err != nil { return nil, err } @@ -212,7 +239,7 @@ func (b *NoopAuthorizer) BeforeCreate(ctx context.Context, obj runtime.Object) e return nil } -func (b *NoopAuthorizer) BeforeUpdate(ctx context.Context, obj runtime.Object) error { +func (b *NoopAuthorizer) BeforeUpdate(ctx context.Context, oldObj, obj runtime.Object) error { return nil } @@ -237,7 +264,7 @@ func (d *DenyAuthorizer) BeforeCreate(ctx context.Context, obj runtime.Object) e return ErrUnauthorized } -func (d *DenyAuthorizer) BeforeUpdate(ctx context.Context, obj runtime.Object) error { +func (d *DenyAuthorizer) BeforeUpdate(ctx context.Context, oldObj, obj runtime.Object) error { return ErrUnauthorized } diff --git a/pkg/services/apiserver/auth/authorizer/storewrapper/wrapper_test.go b/pkg/services/apiserver/auth/authorizer/storewrapper/wrapper_test.go index d1dd3bb5e4e..7a1dca7e895 100644 --- a/pkg/services/apiserver/auth/authorizer/storewrapper/wrapper_test.go +++ b/pkg/services/apiserver/auth/authorizer/storewrapper/wrapper_test.go @@ -35,6 +35,19 @@ func newTestSetup(t *testing.T) *testSetup { return &testSetup{mockStore: mockStore, mockAuth: mockAuth, wrapper: wrapper, ctx: ctx} } +func newTestSetupWithPreserveIdentity(t *testing.T) *testSetup { + mockStore := rest.NewMockStorage(t) + mockAuth := &FakeAuthorizer{} + wrapper := New(mockStore, mockAuth, WithPreserveIdentity()) + + ctx := identity.WithRequester( + context.Background(), + &identity.StaticRequester{UserUID: "u001", Type: types.TypeUser}, + ) + + return &testSetup{mockStore: mockStore, mockAuth: mockAuth, wrapper: wrapper, ctx: ctx} +} + func matchesOriginalUser() func(context.Context) bool { return func(ctx context.Context) bool { user, err := identity.GetRequester(ctx) @@ -274,7 +287,7 @@ func TestWrapper_Update(t *testing.T) { assert.True(t, updated) // Now verify that the authorization is performed inside UpdatedObject - setup.mockAuth.On("BeforeUpdate", mock.MatchedBy(matchesOriginalUser()), oldObj).Return(nil) + setup.mockAuth.On("BeforeUpdate", mock.MatchedBy(matchesOriginalUser()), oldObj, oldObj).Return(nil) obj, err := authzInfo.UpdatedObject(context.Background(), oldObj) require.NoError(t, err) assert.Equal(t, oldObj, obj) @@ -336,6 +349,78 @@ func TestWrapper_PassthroughMethods(t *testing.T) { setup.mockStore.AssertExpectations(t) } +func TestWrapper_WithPreserveIdentity(t *testing.T) { + t.Run("Create passes original user identity to inner store", func(t *testing.T) { + setup := newTestSetupWithPreserveIdentity(t) + + obj := &fakeObject{} + createOpts := &metaV1.CreateOptions{} + expectedObj := &fakeObject{ObjectMeta: metaV1.ObjectMeta{Name: "created"}} + + setup.mockAuth.On("BeforeCreate", mock.MatchedBy(matchesOriginalUser()), obj).Return(nil) + // Inner store must receive original user identity, not service identity. + setup.mockStore.On("Create", mock.MatchedBy(matchesOriginalUser()), obj, mock.Anything, createOpts).Return(expectedObj, nil) + + result, err := setup.wrapper.Create(setup.ctx, obj, nil, createOpts) + + require.NoError(t, err) + assert.Equal(t, expectedObj, result) + setup.mockAuth.AssertExpectations(t) + setup.mockStore.AssertExpectations(t) + }) + + t.Run("Get passes original user identity to inner store", func(t *testing.T) { + setup := newTestSetupWithPreserveIdentity(t) + + obj := &fakeObject{ObjectMeta: metaV1.ObjectMeta{Name: "fetched"}} + + setup.mockStore.On("Get", mock.MatchedBy(matchesOriginalUser()), "fetched", mock.Anything).Return(obj, nil) + setup.mockAuth.On("AfterGet", mock.MatchedBy(matchesOriginalUser()), obj).Return(nil) + + result, err := setup.wrapper.Get(setup.ctx, "fetched", &metaV1.GetOptions{}) + + require.NoError(t, err) + assert.Equal(t, obj, result) + setup.mockAuth.AssertExpectations(t) + setup.mockStore.AssertExpectations(t) + }) + + t.Run("List passes original user identity to inner store", func(t *testing.T) { + setup := newTestSetupWithPreserveIdentity(t) + + listObj := &metaV1.List{} + + setup.mockStore.On("List", mock.MatchedBy(matchesOriginalUser()), mock.Anything).Return(listObj, nil) + setup.mockAuth.On("FilterList", mock.MatchedBy(matchesOriginalUser()), listObj).Return(listObj, nil) + + result, err := setup.wrapper.List(setup.ctx, &internalversion.ListOptions{}) + + require.NoError(t, err) + assert.Equal(t, listObj, result) + setup.mockAuth.AssertExpectations(t) + setup.mockStore.AssertExpectations(t) + }) + + t.Run("Delete passes original user identity to inner store", func(t *testing.T) { + setup := newTestSetupWithPreserveIdentity(t) + + obj := &fakeObject{ObjectMeta: metaV1.ObjectMeta{Name: "to-delete"}} + deleteOpts := &metaV1.DeleteOptions{} + + setup.mockStore.On("Get", mock.MatchedBy(matchesOriginalUser()), "to-delete", mock.Anything).Return(obj, nil) + setup.mockAuth.On("BeforeDelete", mock.MatchedBy(matchesOriginalUser()), obj).Return(nil) + setup.mockStore.On("Delete", mock.MatchedBy(matchesOriginalUser()), "to-delete", mock.Anything, deleteOpts).Return(obj, true, nil) + + result, deleted, err := setup.wrapper.Delete(setup.ctx, "to-delete", nil, deleteOpts) + + require.NoError(t, err) + assert.Equal(t, obj, result) + assert.True(t, deleted) + setup.mockAuth.AssertExpectations(t) + setup.mockStore.AssertExpectations(t) + }) +} + // ----- // Fakes // ----- @@ -349,8 +434,8 @@ func (f *FakeAuthorizer) BeforeCreate(ctx context.Context, obj runtime.Object) e return args.Error(0) } -func (f *FakeAuthorizer) BeforeUpdate(ctx context.Context, obj runtime.Object) error { - args := f.Called(ctx, obj) +func (f *FakeAuthorizer) BeforeUpdate(ctx context.Context, oldObj, obj runtime.Object) error { + args := f.Called(ctx, oldObj, obj) return args.Error(0) } diff --git a/pkg/services/authz/rbac/mapper.go b/pkg/services/authz/rbac/mapper.go index 6ffa0b28b74..d51bb446b22 100644 --- a/pkg/services/authz/rbac/mapper.go +++ b/pkg/services/authz/rbac/mapper.go @@ -219,7 +219,24 @@ func NewMapperRegistry() MapperRegistry { }, "iam.grafana.app": { // Users is a special case. We translate user permissions from id to uid based. - "users": newResourceTranslation("users", "uid", false, map[string]bool{utils.VerbCreate: true}), + "users": translation{ + resource: "users", + attribute: "uid", + verbMapping: map[string]string{ + utils.VerbCreate: "users:create", + utils.VerbGet: "org.users:read", + utils.VerbUpdate: "org.users:write", + utils.VerbPatch: "org.users:write", + utils.VerbDelete: "org.users:remove", + utils.VerbDeleteCollection: "users:delete", + utils.VerbList: "org.users:read", + utils.VerbWatch: "org.users:read", + utils.VerbGetPermissions: "users.permissions:read", + utils.VerbSetPermissions: "users.permissions:write", + }, + folderSupport: false, + skipScopeOnVerb: map[string]bool{utils.VerbCreate: true}, + }, "serviceaccounts": newResourceTranslation("serviceaccounts", "uid", false, map[string]bool{utils.VerbCreate: true}), // Teams is a special case. We translate user permissions from id to uid based. "teams": newResourceTranslation("teams", "uid", false, map[string]bool{utils.VerbCreate: true}), diff --git a/pkg/tests/apis/iam/iam_test.go b/pkg/tests/apis/iam/iam_test.go index eb397ebd9c9..7fdc5d66f6f 100644 --- a/pkg/tests/apis/iam/iam_test.go +++ b/pkg/tests/apis/iam/iam_test.go @@ -41,8 +41,9 @@ func TestIntegrationIdentity(t *testing.T) { testutil.SkipIntegrationTestInShortMode(t) helper := apis.NewK8sTestHelper(t, testinfra.GrafanaOpts{ - AppModeProduction: false, // required for experimental APIs - DisableAnonymous: true, + AppModeProduction: false, // required for experimental APIs + DisableAnonymous: true, + RBACSingleOrganization: true, EnableFeatureToggles: []string{ featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs, // Required to start the example service featuremgmt.FlagKubernetesUsersApi, diff --git a/pkg/tests/apis/iam/team_binding_integration_test.go b/pkg/tests/apis/iam/team_binding_integration_test.go index 2bde3531982..4efb586a713 100644 --- a/pkg/tests/apis/iam/team_binding_integration_test.go +++ b/pkg/tests/apis/iam/team_binding_integration_test.go @@ -28,9 +28,10 @@ func TestIntegrationTeamBindings(t *testing.T) { for _, mode := range modes { t.Run(fmt.Sprintf("Team binding CRUD operations with dual writer mode %d", mode), func(t *testing.T) { helper := apis.NewK8sTestHelper(t, testinfra.GrafanaOpts{ - AppModeProduction: false, - DisableAnonymous: true, - APIServerStorageType: "unified", + AppModeProduction: false, + DisableAnonymous: true, + RBACSingleOrganization: true, + APIServerStorageType: "unified", UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{ "teambindings.iam.grafana.app": { DualWriterMode: mode, diff --git a/pkg/tests/apis/iam/team_members_search_integration_test.go b/pkg/tests/apis/iam/team_members_search_integration_test.go index f04fe7452a0..c73958e3f29 100644 --- a/pkg/tests/apis/iam/team_members_search_integration_test.go +++ b/pkg/tests/apis/iam/team_members_search_integration_test.go @@ -35,9 +35,10 @@ func TestIntegrationTeamMembers(t *testing.T) { for _, mode := range modes { t.Run(fmt.Sprintf("With dual writer mode %d", mode), func(t *testing.T) { helper := apis.NewK8sTestHelper(t, testinfra.GrafanaOpts{ - AppModeProduction: false, - DisableAnonymous: true, - APIServerStorageType: "unified", + AppModeProduction: false, + DisableAnonymous: true, + RBACSingleOrganization: true, + APIServerStorageType: "unified", UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{ "users.iam.grafana.app": { DualWriterMode: mode, diff --git a/pkg/tests/apis/iam/user_integration_test.go b/pkg/tests/apis/iam/user_integration_test.go index f4de86d477a..ad08cacfdb9 100644 --- a/pkg/tests/apis/iam/user_integration_test.go +++ b/pkg/tests/apis/iam/user_integration_test.go @@ -24,9 +24,10 @@ func TestIntegrationUsers(t *testing.T) { for _, mode := range modes { t.Run(fmt.Sprintf("DualWriterMode %d", mode), func(t *testing.T) { helper := apis.NewK8sTestHelper(t, testinfra.GrafanaOpts{ - AppModeProduction: false, - DisableAnonymous: true, - APIServerStorageType: "unified", + AppModeProduction: false, + DisableAnonymous: true, + RBACSingleOrganization: true, + APIServerStorageType: "unified", UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{ "users.iam.grafana.app": { DualWriterMode: mode, @@ -44,6 +45,7 @@ func TestIntegrationUsers(t *testing.T) { }) doUserCRUDTestsUsingTheNewAPIs(t, helper) + doHiddenUsersTests(t, helper) doUserFieldSelectorTests(t, helper) if mode < 3 { @@ -119,6 +121,9 @@ func doUserCRUDTestsUsingTheNewAPIs(t *testing.T, helper *apis.K8sTestHelper) { created, err := userClient.Resource.Create(ctx, helper.LoadYAMLOrJSONFile("testdata/user-test-create-v1.yaml"), metav1.CreateOptions{}) require.NoError(t, err) require.NotNil(t, created) + t.Cleanup(func() { + _ = userClient.Resource.Delete(context.Background(), created.GetName(), metav1.DeleteOptions{}) + }) // Get the user to update createdUID := created.GetName() @@ -373,6 +378,102 @@ func doUserCRUDTestsUsingTheLegacyAPIs(t *testing.T, helper *apis.K8sTestHelper) }) } +func doHiddenUsersTests(t *testing.T, helper *apis.K8sTestHelper) { + t.Run("should hide users from the hidden users list on Get and List", func(t *testing.T) { + ctx := context.Background() + + const hiddenLogin = "hidden-integration-user" + + adminClient := helper.GetResourceClient(apis.ResourceClientArgs{ + User: helper.Org1.Admin, + Namespace: helper.Namespacer(helper.Org1.Admin.Identity.GetOrgID()), + GVR: gvrUsers, + }) + + // Create the user before marking it as hidden so BeforeCreate does not block it. + obj := helper.LoadYAMLOrJSONFile("testdata/user-test-create-v0.yaml") + spec := obj.Object["spec"].(map[string]interface{}) + spec["login"] = hiddenLogin + spec["email"] = hiddenLogin + "@example.com" + obj.Object["spec"] = spec + + created, err := adminClient.Resource.Create(ctx, obj, metav1.CreateOptions{}) + require.NoError(t, err) + createdUID := created.GetName() + + // Register the hidden login in the live Cfg so UserFilter picks it up. + helper.GetEnv().Cfg.HiddenUsers[hiddenLogin] = struct{}{} + // Cleanup: remove from hidden list first so that Delete can succeed. + t.Cleanup(func() { + delete(helper.GetEnv().Cfg.HiddenUsers, hiddenLogin) + _ = adminClient.Resource.Delete(context.Background(), createdUID, metav1.DeleteOptions{}) + }) + + // Get should return 404 when the requester is not the hidden user. + _, err = adminClient.Resource.Get(ctx, createdUID, metav1.GetOptions{}) + require.Error(t, err) + var statusErr *errors.StatusError + require.ErrorAs(t, err, &statusErr) + require.Equal(t, int32(404), statusErr.ErrStatus.Code) + + // List should not include the hidden user. + list, err := adminClient.Resource.List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + for _, item := range list.Items { + itemSpec := item.Object["spec"].(map[string]interface{}) + require.NotEqual(t, hiddenLogin, itemSpec["login"]) + } + + // Update should return 403 for a hidden user. + userToUpdate := created.DeepCopy() + updateSpec := userToUpdate.Object["spec"].(map[string]interface{}) + updateSpec["title"] = "Updated Title" + userToUpdate.Object["spec"] = updateSpec + _, err = adminClient.Resource.Update(ctx, userToUpdate, metav1.UpdateOptions{}) + require.Error(t, err) + require.ErrorAs(t, err, &statusErr) + require.Equal(t, int32(403), statusErr.ErrStatus.Code) + require.Contains(t, statusErr.ErrStatus.Message, "operation not permitted") + + // Delete should return 403 for a hidden user. + err = adminClient.Resource.Delete(ctx, createdUID, metav1.DeleteOptions{}) + require.Error(t, err) + require.ErrorAs(t, err, &statusErr) + require.Equal(t, int32(403), statusErr.ErrStatus.Code) + require.Contains(t, statusErr.ErrStatus.Message, "operation not permitted") + }) + + t.Run("should not be able to create a user whose login is in the hidden users list", func(t *testing.T) { + ctx := context.Background() + + const hiddenLogin = "hidden-create-blocked-user" + + adminClient := helper.GetResourceClient(apis.ResourceClientArgs{ + User: helper.Org1.Admin, + Namespace: helper.Namespacer(helper.Org1.Admin.Identity.GetOrgID()), + GVR: gvrUsers, + }) + + helper.GetEnv().Cfg.HiddenUsers[hiddenLogin] = struct{}{} + t.Cleanup(func() { + delete(helper.GetEnv().Cfg.HiddenUsers, hiddenLogin) + }) + + obj := helper.LoadYAMLOrJSONFile("testdata/user-test-create-v0.yaml") + spec := obj.Object["spec"].(map[string]interface{}) + spec["login"] = hiddenLogin + spec["email"] = hiddenLogin + "@example.com" + obj.Object["spec"] = spec + + _, err := adminClient.Resource.Create(ctx, obj, metav1.CreateOptions{}) + require.Error(t, err) + var statusErr *errors.StatusError + require.ErrorAs(t, err, &statusErr) + require.Equal(t, int32(403), statusErr.ErrStatus.Code) + require.Contains(t, statusErr.ErrStatus.Message, "operation not permitted") + }) +} + func doUserFieldSelectorTests(t *testing.T, helper *apis.K8sTestHelper) { t.Run("should list users using field selectors", func(t *testing.T) { ctx := context.Background() diff --git a/pkg/tests/apis/iam/user_search_integration_test.go b/pkg/tests/apis/iam/user_search_integration_test.go index 55c63a6cc7d..2e82f838073 100644 --- a/pkg/tests/apis/iam/user_search_integration_test.go +++ b/pkg/tests/apis/iam/user_search_integration_test.go @@ -30,9 +30,10 @@ func TestIntegrationUserSearch(t *testing.T) { for _, mode := range modes { t.Run(fmt.Sprintf("DualWriterMode %d", mode), func(t *testing.T) { helper := apis.NewK8sTestHelper(t, testinfra.GrafanaOpts{ - AppModeProduction: false, - DisableAnonymous: true, - APIServerStorageType: "unified", + AppModeProduction: false, + DisableAnonymous: true, + RBACSingleOrganization: true, + APIServerStorageType: "unified", UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{ "users.iam.grafana.app": { DualWriterMode: mode, @@ -79,9 +80,10 @@ func TestIntegrationUserSearch_WithSorting(t *testing.T) { for _, mode := range modes { t.Run(fmt.Sprintf("DualWriterMode %d", mode), func(t *testing.T) { helper := apis.NewK8sTestHelper(t, testinfra.GrafanaOpts{ - AppModeProduction: false, - DisableAnonymous: true, - APIServerStorageType: "unified", + AppModeProduction: false, + DisableAnonymous: true, + RBACSingleOrganization: true, + APIServerStorageType: "unified", UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{ "users.iam.grafana.app": { DualWriterMode: mode, @@ -178,9 +180,10 @@ func TestIntegrationUserSearch_SortCompareLegacy(t *testing.T) { for _, mode := range modes { t.Run(fmt.Sprintf("DualWriterMode %d", mode), func(t *testing.T) { helper := apis.NewK8sTestHelper(t, testinfra.GrafanaOpts{ - AppModeProduction: false, - DisableAnonymous: true, - APIServerStorageType: "unified", + AppModeProduction: false, + DisableAnonymous: true, + RBACSingleOrganization: true, + APIServerStorageType: "unified", UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{ "users.iam.grafana.app": { DualWriterMode: mode, @@ -245,9 +248,10 @@ func TestIntegrationUserSearch_Paging(t *testing.T) { for _, mode := range modes { t.Run(fmt.Sprintf("DualWriterMode %d", mode), func(t *testing.T) { helper := apis.NewK8sTestHelper(t, testinfra.GrafanaOpts{ - AppModeProduction: false, - DisableAnonymous: true, - APIServerStorageType: "unified", + AppModeProduction: false, + DisableAnonymous: true, + RBACSingleOrganization: true, + APIServerStorageType: "unified", UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{ "users.iam.grafana.app": { DualWriterMode: mode, @@ -345,9 +349,10 @@ func TestIntegrationUserSearch_AccessControl(t *testing.T) { for _, mode := range modes { t.Run(fmt.Sprintf("DualWriterMode %d", mode), func(t *testing.T) { helper := apis.NewK8sTestHelper(t, testinfra.GrafanaOpts{ - AppModeProduction: false, - DisableAnonymous: true, - APIServerStorageType: "unified", + AppModeProduction: false, + DisableAnonymous: true, + RBACSingleOrganization: true, + APIServerStorageType: "unified", UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{ "users.iam.grafana.app": { DualWriterMode: mode, diff --git a/pkg/tests/apis/iam/user_teams_search_integration_test.go b/pkg/tests/apis/iam/user_teams_search_integration_test.go index fed50067795..e250b021e97 100644 --- a/pkg/tests/apis/iam/user_teams_search_integration_test.go +++ b/pkg/tests/apis/iam/user_teams_search_integration_test.go @@ -35,9 +35,10 @@ func TestIntegrationUserTeams(t *testing.T) { for _, mode := range modes { t.Run(fmt.Sprintf("With dual writer mode %d", mode), func(t *testing.T) { helper := apis.NewK8sTestHelper(t, testinfra.GrafanaOpts{ - AppModeProduction: false, - DisableAnonymous: true, - APIServerStorageType: "unified", + AppModeProduction: false, + DisableAnonymous: true, + RBACSingleOrganization: true, + APIServerStorageType: "unified", UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{ "users.iam.grafana.app": { DualWriterMode: mode, diff --git a/pkg/tests/apis/openapi_snapshots/iam.grafana.app-v0alpha1.json b/pkg/tests/apis/openapi_snapshots/iam.grafana.app-v0alpha1.json index 5f5ed01a6e0..f3197277948 100644 --- a/pkg/tests/apis/openapi_snapshots/iam.grafana.app-v0alpha1.json +++ b/pkg/tests/apis/openapi_snapshots/iam.grafana.app-v0alpha1.json @@ -4109,7 +4109,7 @@ "tags": [ "User" ], - "description": "list objects of kind User", + "description": "list or watch objects of kind User", "operationId": "listUser", "parameters": [ { diff --git a/pkg/tests/apis/openapi_test.go b/pkg/tests/apis/openapi_test.go index 6f4a7d56a35..93887c57d24 100644 --- a/pkg/tests/apis/openapi_test.go +++ b/pkg/tests/apis/openapi_test.go @@ -26,7 +26,8 @@ func TestIntegrationOpenAPIs(t *testing.T) { testutil.SkipIntegrationTestInShortMode(t) h := NewK8sTestHelper(t, testinfra.GrafanaOpts{ - AppModeProduction: false, // required for experimental APIs + AppModeProduction: false, // required for experimental APIs + RBACSingleOrganization: true, // required for the Users API EnableFeatureToggles: []string{ featuremgmt.FlagQueryService, // Query Library featuremgmt.FlagProvisioning, diff --git a/pkg/tests/testinfra/testinfra.go b/pkg/tests/testinfra/testinfra.go index 5ba9c988b0d..351f53abc62 100644 --- a/pkg/tests/testinfra/testinfra.go +++ b/pkg/tests/testinfra/testinfra.go @@ -402,6 +402,10 @@ func CreateGrafDir(t *testing.T, opts GrafanaOpts) (string, string) { require.NoError(t, err) _, err = rbacSect.NewKey("permission_cache", "false") require.NoError(t, err) + if opts.RBACSingleOrganization { + _, err = rbacSect.NewKey("single_organization", "true") + require.NoError(t, err) + } if opts.DisableAuthZClientCache { authzSect, err := cfg.NewSection("authorization") @@ -833,6 +837,7 @@ type GrafanaOpts struct { LicensePath string EnableRecordingRules bool EnableSCIM bool + RBACSingleOrganization bool APIServerRuntimeConfig string DisableControllers bool DisableDBCleanup bool