From 98453fbcffe9f445ca5e75d3bf4fa75f066c53a2 Mon Sep 17 00:00:00 2001 From: Misi Date: Fri, 9 Jan 2026 14:21:20 +0100 Subject: [PATCH] IAM: Use the new way to authorize resources (#116061) * Use name for authz for User, SA, Team * Use VerbList --- apps/iam/pkg/apis/iam/v0alpha1/extensions.go | 43 ----- pkg/registry/apis/iam/common/common.go | 19 +- pkg/registry/apis/iam/common/common_test.go | 169 ++++++++++++++++-- pkg/registry/apis/iam/register.go | 4 +- pkg/registry/apis/iam/serviceaccount/store.go | 16 +- pkg/registry/apis/iam/team/rest_members.go | 35 +++- pkg/registry/apis/iam/team/store.go | 16 +- pkg/registry/apis/iam/user/store.go | 16 +- 8 files changed, 234 insertions(+), 84 deletions(-) delete mode 100644 apps/iam/pkg/apis/iam/v0alpha1/extensions.go diff --git a/apps/iam/pkg/apis/iam/v0alpha1/extensions.go b/apps/iam/pkg/apis/iam/v0alpha1/extensions.go deleted file mode 100644 index c0c5a11f7e3..00000000000 --- a/apps/iam/pkg/apis/iam/v0alpha1/extensions.go +++ /dev/null @@ -1,43 +0,0 @@ -package v0alpha1 - -import ( - "fmt" - - "github.com/grafana/grafana/pkg/apimachinery/utils" -) - -func (u User) AuthID() string { - meta, err := utils.MetaAccessor(&u) - if err != nil { - return "" - } - // TODO: Workaround until we move all definitions - // After having all resource definitions here in the app, we can remove this - // and we need to change the List authorization to use the MetaAccessor and the GetDeprecatedInternalID method - //nolint:staticcheck - return fmt.Sprintf("%d", meta.GetDeprecatedInternalID()) -} - -func (s ServiceAccount) AuthID() string { - meta, err := utils.MetaAccessor(&s) - if err != nil { - return "" - } - // TODO: Workaround until we move all definitions - // After having all resource definitions here in the app, we can remove this - // and we need to change the List authorization to use the MetaAccessor and the GetDeprecatedInternalID method - //nolint:staticcheck - return fmt.Sprintf("%d", meta.GetDeprecatedInternalID()) -} - -func (t Team) AuthID() string { - meta, err := utils.MetaAccessor(&t) - if err != nil { - return "" - } - // TODO: Workaround until we move all definitions - // After having all resource definitions here in the app, we can remove this - // and we need to change the List authorization to use the MetaAccessor and the GetDeprecatedInternalID method - //nolint:staticcheck - return fmt.Sprintf("%d", meta.GetDeprecatedInternalID()) -} diff --git a/pkg/registry/apis/iam/common/common.go b/pkg/registry/apis/iam/common/common.go index b508084bcae..a5409ce5ac4 100644 --- a/pkg/registry/apis/iam/common/common.go +++ b/pkg/registry/apis/iam/common/common.go @@ -12,6 +12,7 @@ import ( legacyiamv0 "github.com/grafana/grafana/pkg/apis/iam/v0alpha1" "github.com/grafana/grafana/pkg/services/apiserver/endpoints/request" "github.com/grafana/grafana/pkg/services/team" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // OptonalFormatInt formats num as a string. If num is less or equal than 0 @@ -39,23 +40,17 @@ func MapUserTeamPermission(p team.PermissionType) legacyiamv0.TeamPermission { } } -// Resource is required to be implemented for list return types so we can -// perform authorization. -type Resource interface { - AuthID() string -} - -type ListResponse[T Resource] struct { +type ListResponse[T metav1.Object] struct { Items []T RV int64 Continue int64 } -type ListFunc[T Resource] func(ctx context.Context, ns authlib.NamespaceInfo, p Pagination) (*ListResponse[T], error) +type ListFunc[T metav1.Object] func(ctx context.Context, ns authlib.NamespaceInfo, p Pagination) (*ListResponse[T], error) // List is a helper function that will perform access check on resources if // prvovided with a authlib.AccessClient. -func List[T Resource]( +func List[T metav1.Object]( ctx context.Context, resource utils.ResourceInfo, ac authlib.AccessClient, @@ -78,7 +73,7 @@ func List[T Resource]( check, _, err = ac.Compile(ctx, ident, authlib.ListRequest{ Resource: resource.GroupResource().Resource, Group: resource.GroupResource().Group, - Verb: "list", + Verb: utils.VerbList, Namespace: ns.Value, }) @@ -95,7 +90,7 @@ func List[T Resource]( } for _, item := range first.Items { - if !check(item.AuthID(), "") { + if !check(item.GetName(), "") { continue } res.Items = append(res.Items, item) @@ -118,7 +113,7 @@ outer: break outer } - if !check(item.AuthID(), "") { + if !check(item.GetName(), "") { continue } diff --git a/pkg/registry/apis/iam/common/common_test.go b/pkg/registry/apis/iam/common/common_test.go index d835c238b8c..ea079043fd1 100644 --- a/pkg/registry/apis/iam/common/common_test.go +++ b/pkg/registry/apis/iam/common/common_test.go @@ -5,6 +5,8 @@ import ( "testing" "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/apiserver/pkg/endpoints/request" authlib "github.com/grafana/authlib/types" @@ -15,14 +17,6 @@ import ( "github.com/grafana/grafana/pkg/services/featuremgmt" ) -type item struct { - id string -} - -func (i item) AuthID() string { - return i.id -} - func TestList(t *testing.T) { ac := acimpl.ProvideAccessControl(featuremgmt.WithFeatures()) @@ -83,8 +77,8 @@ func TestList(t *testing.T) { assert.NoError(t, err) assert.Len(t, res.Items, 2) - assert.Equal(t, "1", res.Items[0].AuthID()) - assert.Equal(t, "3", res.Items[1].AuthID()) + assert.Equal(t, "1", res.Items[0].GetName()) + assert.Equal(t, "3", res.Items[1].GetName()) }) } @@ -103,3 +97,158 @@ func newIdent(permissions ...accesscontrol.Permission) *identity.StaticRequester Permissions: map[int64]map[string][]string{1: pmap}, } } + +var _ metav1.Object = (*item)(nil) + +type item struct { + id string +} + +// GetAnnotations implements v1.Object. +func (i item) GetAnnotations() map[string]string { + panic("unimplemented") +} + +// GetCreationTimestamp implements v1.Object. +func (i item) GetCreationTimestamp() metav1.Time { + panic("unimplemented") +} + +// GetDeletionGracePeriodSeconds implements v1.Object. +func (i item) GetDeletionGracePeriodSeconds() *int64 { + panic("unimplemented") +} + +// GetDeletionTimestamp implements v1.Object. +func (i item) GetDeletionTimestamp() *metav1.Time { + panic("unimplemented") +} + +// GetFinalizers implements v1.Object. +func (i item) GetFinalizers() []string { + panic("unimplemented") +} + +// GetGenerateName implements v1.Object. +func (i item) GetGenerateName() string { + panic("unimplemented") +} + +// GetGeneration implements v1.Object. +func (i item) GetGeneration() int64 { + panic("unimplemented") +} + +// GetLabels implements v1.Object. +func (i item) GetLabels() map[string]string { + panic("unimplemented") +} + +// GetManagedFields implements v1.Object. +func (i item) GetManagedFields() []metav1.ManagedFieldsEntry { + panic("unimplemented") +} + +// GetNamespace implements v1.Object. +func (i item) GetNamespace() string { + panic("unimplemented") +} + +// GetOwnerReferences implements v1.Object. +func (i item) GetOwnerReferences() []metav1.OwnerReference { + panic("unimplemented") +} + +// GetResourceVersion implements v1.Object. +func (i item) GetResourceVersion() string { + panic("unimplemented") +} + +// GetSelfLink implements v1.Object. +func (i item) GetSelfLink() string { + panic("unimplemented") +} + +// GetUID implements v1.Object. +func (i item) GetUID() types.UID { + panic("unimplemented") +} + +// SetAnnotations implements v1.Object. +func (i item) SetAnnotations(annotations map[string]string) { + panic("unimplemented") +} + +// SetCreationTimestamp implements v1.Object. +func (i item) SetCreationTimestamp(timestamp metav1.Time) { + panic("unimplemented") +} + +// SetDeletionGracePeriodSeconds implements v1.Object. +func (i item) SetDeletionGracePeriodSeconds(*int64) { + panic("unimplemented") +} + +// SetDeletionTimestamp implements v1.Object. +func (i item) SetDeletionTimestamp(timestamp *metav1.Time) { + panic("unimplemented") +} + +// SetFinalizers implements v1.Object. +func (i item) SetFinalizers(finalizers []string) { + panic("unimplemented") +} + +// SetGenerateName implements v1.Object. +func (i item) SetGenerateName(name string) { + panic("unimplemented") +} + +// SetGeneration implements v1.Object. +func (i item) SetGeneration(generation int64) { + panic("unimplemented") +} + +// SetLabels implements v1.Object. +func (i item) SetLabels(labels map[string]string) { + panic("unimplemented") +} + +// SetManagedFields implements v1.Object. +func (i item) SetManagedFields(managedFields []metav1.ManagedFieldsEntry) { + panic("unimplemented") +} + +// SetName implements v1.Object. +func (i item) SetName(name string) { + panic("unimplemented") +} + +// SetNamespace implements v1.Object. +func (i item) SetNamespace(namespace string) { + panic("unimplemented") +} + +// SetOwnerReferences implements v1.Object. +func (i item) SetOwnerReferences([]metav1.OwnerReference) { + panic("unimplemented") +} + +// SetResourceVersion implements v1.Object. +func (i item) SetResourceVersion(version string) { + panic("unimplemented") +} + +// SetSelfLink implements v1.Object. +func (i item) SetSelfLink(selfLink string) { + panic("unimplemented") +} + +// SetUID implements v1.Object. +func (i item) SetUID(uid types.UID) { + panic("unimplemented") +} + +func (i item) GetName() string { + return i.id +} diff --git a/pkg/registry/apis/iam/register.go b/pkg/registry/apis/iam/register.go index 63fc7253e8a..7c9d8c558c4 100644 --- a/pkg/registry/apis/iam/register.go +++ b/pkg/registry/apis/iam/register.go @@ -102,7 +102,7 @@ func RegisterAPIService( store: store, userLegacyStore: user.NewLegacyStore(store, accessClient, enableAuthnMutation, tracing), saLegacyStore: serviceaccount.NewLegacyStore(store, accessClient, enableAuthnMutation, tracing), - legacyTeamStore: team.NewLegacyStore(store, legacyAccessClient, enableAuthnMutation, tracing), + legacyTeamStore: team.NewLegacyStore(store, accessClient, enableAuthnMutation, tracing), teamBindingLegacyStore: teambinding.NewLegacyBindingStore(store, enableAuthnMutation, tracing), ssoLegacyStore: sso.NewLegacyStore(ssoService, tracing), coreRolesStorage: coreRolesStorage, @@ -327,7 +327,7 @@ func (b *IdentityAccessManagementAPIBuilder) UpdateTeamsAPIGroup(opts builder.AP storage[teamResource.StoragePath()] = dw } - storage[teamResource.StoragePath("members")] = team.NewLegacyTeamMemberREST(b.store) + storage[teamResource.StoragePath("members")] = team.NewLegacyTeamMemberREST(b.store, b.accessClient) if b.teamGroupsHandler != nil { storage[teamResource.StoragePath("groups")] = b.teamGroupsHandler } diff --git a/pkg/registry/apis/iam/serviceaccount/store.go b/pkg/registry/apis/iam/serviceaccount/store.go index db4709a4e2e..6d1ac774174 100644 --- a/pkg/registry/apis/iam/serviceaccount/store.go +++ b/pkg/registry/apis/iam/serviceaccount/store.go @@ -178,7 +178,7 @@ func (s *LegacyStore) List(ctx context.Context, options *internalversion.ListOpt res, err := common.List( ctx, resource, s.ac, common.PaginationFromListOptions(options), - func(ctx context.Context, ns claims.NamespaceInfo, p common.Pagination) (*common.ListResponse[iamv0alpha1.ServiceAccount], error) { + func(ctx context.Context, ns claims.NamespaceInfo, p common.Pagination) (*common.ListResponse[*iamv0alpha1.ServiceAccount], error) { found, err := s.store.ListServiceAccounts(ctx, ns, legacy.ListServiceAccountsQuery{ Pagination: p, }) @@ -187,12 +187,13 @@ func (s *LegacyStore) List(ctx context.Context, options *internalversion.ListOpt return nil, err } - items := make([]iamv0alpha1.ServiceAccount, 0, len(found.Items)) + items := make([]*iamv0alpha1.ServiceAccount, 0, len(found.Items)) for _, sa := range found.Items { - items = append(items, s.toSAItem(sa, ns.Value)) + saItem := s.toSAItem(sa, ns.Value) + items = append(items, &saItem) } - return &common.ListResponse[iamv0alpha1.ServiceAccount]{ + return &common.ListResponse[*iamv0alpha1.ServiceAccount]{ Items: items, RV: found.RV, Continue: found.Continue, @@ -204,7 +205,12 @@ func (s *LegacyStore) List(ctx context.Context, options *internalversion.ListOpt return nil, err } - obj := &iamv0alpha1.ServiceAccountList{Items: res.Items} + items := make([]iamv0alpha1.ServiceAccount, len(res.Items)) + for i, sa := range res.Items { + items[i] = *sa + } + + obj := &iamv0alpha1.ServiceAccountList{Items: items} obj.Continue = common.OptionalFormatInt(res.Continue) obj.ResourceVersion = common.OptionalFormatInt(res.RV) return obj, nil diff --git a/pkg/registry/apis/iam/team/rest_members.go b/pkg/registry/apis/iam/team/rest_members.go index 6659823b9fa..586b074d710 100644 --- a/pkg/registry/apis/iam/team/rest_members.go +++ b/pkg/registry/apis/iam/team/rest_members.go @@ -2,14 +2,20 @@ package team import ( "context" + "fmt" "net/http" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/registry/rest" claims "github.com/grafana/authlib/types" + iamv0alpha1 "github.com/grafana/grafana/apps/iam/pkg/apis/iam/v0alpha1" "github.com/grafana/grafana/pkg/api/dtos" + "github.com/grafana/grafana/pkg/apimachinery/identity" + "github.com/grafana/grafana/pkg/apimachinery/utils" iamv0 "github.com/grafana/grafana/pkg/apis/iam/v0alpha1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "github.com/grafana/grafana/pkg/registry/apis/iam/common" "github.com/grafana/grafana/pkg/registry/apis/iam/legacy" "github.com/grafana/grafana/pkg/services/apiserver/endpoints/request" @@ -23,12 +29,13 @@ var ( _ rest.Connecter = (*LegacyTeamMemberREST)(nil) ) -func NewLegacyTeamMemberREST(store legacy.LegacyIdentityStore) *LegacyTeamMemberREST { - return &LegacyTeamMemberREST{store} +func NewLegacyTeamMemberREST(store legacy.LegacyIdentityStore, ac claims.AccessClient) *LegacyTeamMemberREST { + return &LegacyTeamMemberREST{store: store, ac: ac} } type LegacyTeamMemberREST struct { store legacy.LegacyIdentityStore + ac claims.AccessClient } // New implements rest.Storage. @@ -62,6 +69,30 @@ func (s *LegacyTeamMemberREST) Connect(ctx context.Context, name string, options } return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ident, err := identity.GetRequester(ctx) + if err != nil { + responder.Error(err) + return + } + + checkResp, err := s.ac.Check(ctx, ident, claims.CheckRequest{ + Group: iamv0alpha1.TeamResourceInfo.GroupResource().Group, + Resource: iamv0alpha1.TeamResourceInfo.GroupResource().Resource, + Name: name, + Namespace: ns.Value, + Verb: utils.VerbGetPermissions, + }, "") + + if err != nil { + responder.Error(err) + return + } + + if !checkResp.Allowed { + responder.Error(apierrors.NewForbidden(iamv0alpha1.TeamResourceInfo.GroupResource(), name, fmt.Errorf("permission denied"))) + return + } + res, err := s.store.ListTeamMembers(ctx, ns, legacy.ListTeamMembersQuery{ UID: name, Pagination: common.PaginationFromListQuery(r.URL.Query()), diff --git a/pkg/registry/apis/iam/team/store.go b/pkg/registry/apis/iam/team/store.go index c667c8ec374..834bba00335 100644 --- a/pkg/registry/apis/iam/team/store.go +++ b/pkg/registry/apis/iam/team/store.go @@ -174,7 +174,7 @@ func (s *LegacyStore) List(ctx context.Context, options *internalversion.ListOpt res, err := common.List( ctx, resource, s.ac, common.PaginationFromListOptions(options), - func(ctx context.Context, ns claims.NamespaceInfo, p common.Pagination) (*common.ListResponse[iamv0alpha1.Team], error) { + func(ctx context.Context, ns claims.NamespaceInfo, p common.Pagination) (*common.ListResponse[*iamv0alpha1.Team], error) { found, err := s.store.ListTeams(ctx, ns, legacy.ListTeamQuery{ Pagination: p, }) @@ -183,12 +183,13 @@ func (s *LegacyStore) List(ctx context.Context, options *internalversion.ListOpt return nil, err } - teams := make([]iamv0alpha1.Team, 0, len(found.Teams)) + teams := make([]*iamv0alpha1.Team, 0, len(found.Teams)) for _, t := range found.Teams { - teams = append(teams, toTeamObject(t, ns)) + team := toTeamObject(t, ns) + teams = append(teams, &team) } - return &common.ListResponse[iamv0alpha1.Team]{ + return &common.ListResponse[*iamv0alpha1.Team]{ Items: teams, RV: found.RV, Continue: found.Continue, @@ -200,7 +201,12 @@ func (s *LegacyStore) List(ctx context.Context, options *internalversion.ListOpt return nil, fmt.Errorf("failed to list teams: %w", err) } - list := &iamv0alpha1.TeamList{Items: res.Items} + items := make([]iamv0alpha1.Team, len(res.Items)) + for i, t := range res.Items { + items[i] = *t + } + + list := &iamv0alpha1.TeamList{Items: items} list.Continue = common.OptionalFormatInt(res.Continue) list.ResourceVersion = common.OptionalFormatInt(res.RV) diff --git a/pkg/registry/apis/iam/user/store.go b/pkg/registry/apis/iam/user/store.go index 53a146412ea..5cc9343111a 100644 --- a/pkg/registry/apis/iam/user/store.go +++ b/pkg/registry/apis/iam/user/store.go @@ -183,7 +183,7 @@ func (s *LegacyStore) List(ctx context.Context, options *internalversion.ListOpt res, err := common.List( ctx, userResource, s.ac, common.PaginationFromListOptions(options), - func(ctx context.Context, ns claims.NamespaceInfo, p common.Pagination) (*common.ListResponse[iamv0alpha1.User], error) { + func(ctx context.Context, ns claims.NamespaceInfo, p common.Pagination) (*common.ListResponse[*iamv0alpha1.User], error) { found, err := s.store.ListUsers(ctx, ns, legacy.ListUserQuery{ Pagination: p, }) @@ -192,12 +192,13 @@ func (s *LegacyStore) List(ctx context.Context, options *internalversion.ListOpt return nil, err } - users := make([]iamv0alpha1.User, 0, len(found.Items)) + users := make([]*iamv0alpha1.User, 0, len(found.Items)) for _, u := range found.Items { - users = append(users, toUserItem(&u, ns.Value)) + user := toUserItem(&u, ns.Value) + users = append(users, &user) } - return &common.ListResponse[iamv0alpha1.User]{ + return &common.ListResponse[*iamv0alpha1.User]{ Items: users, RV: found.RV, Continue: found.Continue, @@ -209,7 +210,12 @@ func (s *LegacyStore) List(ctx context.Context, options *internalversion.ListOpt return nil, err } - obj := &iamv0alpha1.UserList{Items: res.Items} + items := make([]iamv0alpha1.User, len(res.Items)) + for i, u := range res.Items { + items[i] = *u + } + + obj := &iamv0alpha1.UserList{Items: items} obj.Continue = common.OptionalFormatInt(res.Continue) obj.ResourceVersion = common.OptionalFormatInt(res.RV) return obj, nil