From 0160f4f72ce5d770a1ab4dd2fd2a433eec45628b Mon Sep 17 00:00:00 2001 From: Karl Persson Date: Fri, 27 Sep 2024 15:53:11 +0200 Subject: [PATCH] RBAC: Add legacy authorization checks to service accounts (#93753) * Extract a helper funtion to perform list with authorization checks * Add k8s verb to utils package * Construct default mapping when no custom mapping is passed * Configure authorization checks for service accounts * Fix helper and add filtering to service accounts --- pkg/apimachinery/utils/verbs.go | 22 ++++ .../iam/v0alpha1/types_servier_account.go | 8 ++ pkg/apis/iam/v0alpha1/types_user.go | 12 +- pkg/registry/apis/iam/authorizer.go | 22 +++- pkg/registry/apis/iam/common/common.go | 94 +++++++++++++++ pkg/registry/apis/iam/common/common_test.go | 105 +++++++++++++++++ .../apis/iam/legacy/service_account.go | 78 +++++++++++++ .../legacy/service_account_internal_id.sql | 7 ++ pkg/registry/apis/iam/legacy/sql.go | 1 + pkg/registry/apis/iam/register.go | 2 +- pkg/registry/apis/iam/serviceaccount/store.go | 52 +++++---- pkg/registry/apis/iam/user/store.go | 107 ++++-------------- pkg/services/accesscontrol/authorizer.go | 21 +++- pkg/services/accesscontrol/authorizer_test.go | 11 +- 14 files changed, 424 insertions(+), 118 deletions(-) create mode 100644 pkg/apimachinery/utils/verbs.go create mode 100644 pkg/registry/apis/iam/common/common_test.go create mode 100644 pkg/registry/apis/iam/legacy/service_account_internal_id.sql diff --git a/pkg/apimachinery/utils/verbs.go b/pkg/apimachinery/utils/verbs.go new file mode 100644 index 00000000000..7ab5086332f --- /dev/null +++ b/pkg/apimachinery/utils/verbs.go @@ -0,0 +1,22 @@ +package utils + +// Kubernetes request verbs +// http://kubernetes.io/docs/reference/access-authn-authz/authorization/#request-verb-resource +const ( + // VerbGet is mapped from HTTP GET for individual resource + VerbGet = "get" + // VerbList is mapped from HTTP GET for collections + VerbList = "list" + // VerbWatch is mapped from HTTP GET for watching an individual resource or collection of resources + VerbWatch = "watch" + // VerbCreate is mapped from HTTP POST + VerbCreate = "create" + // VerbUpdate is mapped from HTTP PUT + VerbUpdate = "update" + // VerbPatch is mapped from HTTP PATCH + VerbPatch = "patch" + // VerbDelete is mapped from HTTP DELETE for individual resources + VerbDelete = "delete" + // VerbDelete is mapped from HTTP DELETE for collections + VerbDeleteCollection = "deletecollection" +) diff --git a/pkg/apis/iam/v0alpha1/types_servier_account.go b/pkg/apis/iam/v0alpha1/types_servier_account.go index 8451095c04d..04e75eda7a2 100644 --- a/pkg/apis/iam/v0alpha1/types_servier_account.go +++ b/pkg/apis/iam/v0alpha1/types_servier_account.go @@ -1,6 +1,8 @@ package v0alpha1 import ( + "fmt" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -12,9 +14,15 @@ type ServiceAccount struct { Spec ServiceAccountSpec `json:"spec,omitempty"` } +func (s ServiceAccount) AuthID() string { + return fmt.Sprintf("%d", s.Spec.InternalID) +} + type ServiceAccountSpec struct { Title string `json:"title,omitempty"` Disabled bool `json:"disabled,omitempty"` + // This is currently used for authorization checks but we don't want to expose it + InternalID int64 `json:"-"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/pkg/apis/iam/v0alpha1/types_user.go b/pkg/apis/iam/v0alpha1/types_user.go index 713b2041876..2ea79d2ecdb 100644 --- a/pkg/apis/iam/v0alpha1/types_user.go +++ b/pkg/apis/iam/v0alpha1/types_user.go @@ -1,6 +1,10 @@ package v0alpha1 -import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +import ( + "fmt" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object type User struct { @@ -10,12 +14,18 @@ type User struct { Spec UserSpec `json:"spec,omitempty"` } +func (u User) AuthID() string { + return fmt.Sprintf("%d", u.Spec.InternalID) +} + type UserSpec struct { Name string `json:"name,omitempty"` Login string `json:"login,omitempty"` Email string `json:"email,omitempty"` EmailVerified bool `json:"emailVerified,omitempty"` Disabled bool `json:"disabled,omitempty"` + // This is currently used for authorization checks but we don't want to expose it + InternalID int64 `json:"-"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/pkg/registry/apis/iam/authorizer.go b/pkg/registry/apis/iam/authorizer.go index 81dc1ee0187..8cc8cb68aba 100644 --- a/pkg/registry/apis/iam/authorizer.go +++ b/pkg/registry/apis/iam/authorizer.go @@ -7,6 +7,7 @@ import ( "github.com/grafana/authlib/claims" "k8s.io/apiserver/pkg/authorization/authorizer" + "github.com/grafana/grafana/pkg/apimachinery/utils" iamv0 "github.com/grafana/grafana/pkg/apis/iam/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/iam/legacy" "github.com/grafana/grafana/pkg/services/accesscontrol" @@ -20,8 +21,8 @@ func newLegacyAuthorizer(ac accesscontrol.AccessControl, store legacy.LegacyIden Resource: iamv0.UserResourceInfo.GetName(), Attr: "id", Mapping: map[string]string{ - "get": accesscontrol.ActionOrgUsersRead, - "list": accesscontrol.ActionOrgUsersRead, + utils.VerbGet: accesscontrol.ActionOrgUsersRead, + utils.VerbList: accesscontrol.ActionOrgUsersRead, }, Resolver: accesscontrol.ResourceResolverFunc(func(ctx context.Context, ns claims.NamespaceInfo, name string) ([]string, error) { res, err := store.GetUserInternalID(ctx, ns, legacy.GetUserInternalIDQuery{ @@ -36,10 +37,23 @@ func newLegacyAuthorizer(ac accesscontrol.AccessControl, store legacy.LegacyIden accesscontrol.ResourceAuthorizerOptions{ Resource: "display", Unchecked: map[string]bool{ - "get": true, - "list": true, + utils.VerbGet: true, + utils.VerbList: true, }, }, + accesscontrol.ResourceAuthorizerOptions{ + Resource: iamv0.ServiceAccountResourceInfo.GetName(), + Attr: "id", + Resolver: accesscontrol.ResourceResolverFunc(func(ctx context.Context, ns claims.NamespaceInfo, name string) ([]string, error) { + res, err := store.GetServiceAccountInternalID(ctx, ns, legacy.GetServiceAccountInternalIDQuery{ + UID: name, + }) + if err != nil { + return nil, err + } + return []string{fmt.Sprintf("serviceaccounts:id:%d", res.ID)}, nil + }), + }, ) return gfauthorizer.NewResourceAuthorizer(client), client diff --git a/pkg/registry/apis/iam/common/common.go b/pkg/registry/apis/iam/common/common.go index 1e3533f3b73..ab6cf73bef5 100644 --- a/pkg/registry/apis/iam/common/common.go +++ b/pkg/registry/apis/iam/common/common.go @@ -1,9 +1,14 @@ package common import ( + "context" "strconv" + "github.com/grafana/authlib/claims" + "github.com/grafana/grafana/pkg/apimachinery/identity" + "github.com/grafana/grafana/pkg/apimachinery/utils" iamv0 "github.com/grafana/grafana/pkg/apis/iam/v0alpha1" + "github.com/grafana/grafana/pkg/services/apiserver/endpoints/request" "github.com/grafana/grafana/pkg/services/team" ) @@ -23,3 +28,92 @@ func MapTeamPermission(p team.PermissionType) iamv0.TeamPermission { return iamv0.TeamPermissionMember } } + +// 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 { + Items []T + RV int64 + Continue int64 +} + +type ListFunc[T Resource] func(ctx context.Context, ns claims.NamespaceInfo, p Pagination) (*ListResponse[T], error) + +// List is a helper function that will perform access check on resources if +// prvovided with a claims.AccessClient. +func List[T Resource]( + ctx context.Context, + resourceName string, + ac claims.AccessClient, + p Pagination, + fn ListFunc[T], +) (*ListResponse[T], error) { + ns, err := request.NamespaceInfoFrom(ctx, true) + if err != nil { + return nil, err + } + + ident, err := identity.GetRequester(ctx) + if err != nil { + return nil, err + } + + check := func(_ string, _ string) bool { return true } + if ac != nil { + var err error + check, err = ac.Compile(ctx, ident, claims.AccessRequest{ + Verb: utils.VerbList, + Resource: resourceName, + Namespace: ns.Value, + }) + + if err != nil { + return nil, err + } + } + + res := &ListResponse[T]{Items: make([]T, 0, p.Limit)} + + first, err := fn(ctx, ns, p) + if err != nil { + return nil, err + } + + for _, item := range first.Items { + if !check(ns.Value, item.AuthID()) { + continue + } + res.Items = append(res.Items, item) + } + res.Continue = first.Continue + res.RV = first.RV + +outer: + for len(res.Items) < int(p.Limit) && res.Continue != 0 { + // FIXME: it is not optimal to reduce the amout we look for here but it is the easiest way to + // correctly handle pagination and continue tokens + r, err := fn(ctx, ns, Pagination{Limit: p.Limit - int64(len(res.Items)), Continue: res.Continue}) + if err != nil { + return nil, err + } + + for _, item := range r.Items { + if len(res.Items) == int(p.Limit) { + res.Continue = r.Continue + break outer + } + + if !check(ns.Value, item.AuthID()) { + continue + } + + res.Items = append(res.Items, item) + } + } + + return res, nil +} diff --git a/pkg/registry/apis/iam/common/common_test.go b/pkg/registry/apis/iam/common/common_test.go new file mode 100644 index 00000000000..0f1d359842f --- /dev/null +++ b/pkg/registry/apis/iam/common/common_test.go @@ -0,0 +1,105 @@ +package common + +import ( + "context" + "testing" + + "github.com/grafana/authlib/claims" + "github.com/stretchr/testify/assert" + "k8s.io/apiserver/pkg/endpoints/request" + + "github.com/grafana/grafana/pkg/apimachinery/identity" + "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" + "github.com/grafana/grafana/pkg/services/authz/zanzana" + "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(), zanzana.NewNoopClient()) + + t.Run("should allow all items if no access client is passed", func(t *testing.T) { + ctx := newContext("stacks-1", newIdent()) + + res, err := List(ctx, "items", nil, Pagination{Limit: 2}, func(ctx context.Context, ns claims.NamespaceInfo, p Pagination) (*ListResponse[item], error) { + return &ListResponse[item]{ + Items: []item{item{"1"}, item{"2"}}, + }, nil + }) + assert.NoError(t, err) + assert.Len(t, res.Items, 2) + }) + + t.Run("should filter out items that are allowed", func(t *testing.T) { + ctx := newContext("stacks-1", newIdent(accesscontrol.Permission{Action: "items:read", Scope: "items:uid:1"})) + + a := accesscontrol.NewLegacyAccessClient(ac, accesscontrol.ResourceAuthorizerOptions{ + Resource: "items", + Attr: "uid", + }) + res, err := List(ctx, "items", a, Pagination{Limit: 2}, func(ctx context.Context, ns claims.NamespaceInfo, p Pagination) (*ListResponse[item], error) { + return &ListResponse[item]{ + Items: []item{item{"1"}, item{"2"}}, + }, nil + }) + assert.NoError(t, err) + assert.Len(t, res.Items, 1) + }) + + t.Run("should fetch more for partial response with continue token", func(t *testing.T) { + ctx := newContext("stacks-1", newIdent( + accesscontrol.Permission{Action: "items:read", Scope: "items:uid:1"}, + accesscontrol.Permission{Action: "items:read", Scope: "items:uid:3"}, + )) + + a := accesscontrol.NewLegacyAccessClient(ac, accesscontrol.ResourceAuthorizerOptions{ + Resource: "items", + Attr: "uid", + }) + + var called bool + + res, err := List(ctx, "items", a, Pagination{Limit: 2}, func(ctx context.Context, ns claims.NamespaceInfo, p Pagination) (*ListResponse[item], error) { + if called { + return &ListResponse[item]{ + Items: []item{item{"3"}}, + }, nil + } + + called = true + return &ListResponse[item]{ + Items: []item{item{"1"}, item{"2"}}, + Continue: 3, + }, nil + }) + 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()) + }) +} + +func newContext(namespace string, ident *identity.StaticRequester) context.Context { + return request.WithNamespace(identity.WithRequester(context.Background(), ident), namespace) +} + +func newIdent(permissions ...accesscontrol.Permission) *identity.StaticRequester { + pmap := map[string][]string{} + for _, p := range permissions { + pmap[p.Action] = append(pmap[p.Action], p.Scope) + } + + return &identity.StaticRequester{ + OrgID: 1, + Permissions: map[int64]map[string][]string{1: pmap}, + } +} diff --git a/pkg/registry/apis/iam/legacy/service_account.go b/pkg/registry/apis/iam/legacy/service_account.go index 03610e2a3f2..6e87be7f13d 100644 --- a/pkg/registry/apis/iam/legacy/service_account.go +++ b/pkg/registry/apis/iam/legacy/service_account.go @@ -2,6 +2,7 @@ package legacy import ( "context" + "errors" "fmt" "time" @@ -11,6 +12,83 @@ import ( "github.com/grafana/grafana/pkg/storage/unified/sql/sqltemplate" ) +type GetServiceAccountInternalIDQuery struct { + OrgID int64 + UID string +} + +type GetServiceAccountInternalIDResult struct { + ID int64 +} + +var sqlQueryServiceAccountInternalIDTemplate = mustTemplate("service_account_internal_id.sql") + +func newGetServiceAccountInternalID(sql *legacysql.LegacyDatabaseHelper, q *GetServiceAccountInternalIDQuery) getServiceAccountInternalIDQuery { + return getServiceAccountInternalIDQuery{ + SQLTemplate: sqltemplate.New(sql.DialectForDriver()), + UserTable: sql.Table("user"), + OrgUserTable: sql.Table("org_user"), + Query: q, + } +} + +type getServiceAccountInternalIDQuery struct { + sqltemplate.SQLTemplate + UserTable string + OrgUserTable string + Query *GetServiceAccountInternalIDQuery +} + +func (r getServiceAccountInternalIDQuery) Validate() error { + return nil // TODO +} + +func (s *legacySQLStore) GetServiceAccountInternalID( + ctx context.Context, + ns claims.NamespaceInfo, + query GetServiceAccountInternalIDQuery, +) (*GetServiceAccountInternalIDResult, error) { + query.OrgID = ns.OrgID + if query.OrgID == 0 { + return nil, fmt.Errorf("expected non zero org id") + } + + sql, err := s.sql(ctx) + if err != nil { + return nil, err + } + + req := newGetServiceAccountInternalID(sql, &query) + q, err := sqltemplate.Execute(sqlQueryServiceAccountInternalIDTemplate, req) + if err != nil { + return nil, fmt.Errorf("execute template %q: %w", sqlQueryServiceAccountInternalIDTemplate.Name(), err) + } + + rows, err := sql.DB.GetSqlxSession().Query(ctx, q, req.GetArgs()...) + defer func() { + if rows != nil { + _ = rows.Close() + } + }() + + if err != nil { + return nil, err + } + + if !rows.Next() { + return nil, errors.New("service account not found") + } + + var id int64 + if err := rows.Scan(&id); err != nil { + return nil, err + } + + return &GetServiceAccountInternalIDResult{ + id, + }, nil +} + type ListServiceAccountsQuery struct { UID string OrgID int64 diff --git a/pkg/registry/apis/iam/legacy/service_account_internal_id.sql b/pkg/registry/apis/iam/legacy/service_account_internal_id.sql new file mode 100644 index 00000000000..f6c468bf577 --- /dev/null +++ b/pkg/registry/apis/iam/legacy/service_account_internal_id.sql @@ -0,0 +1,7 @@ +SELECT u.id +FROM {{ .Ident .UserTable }} as u +INNER JOIN {{ .Ident .OrgUserTable }} as o ON u.id = o.user_id +WHERE o.org_id = {{ .Arg .Query.OrgID }} +AND u.uid = {{ .Arg .Query.UID }} +AND u.is_service_account +LIMIT 1; diff --git a/pkg/registry/apis/iam/legacy/sql.go b/pkg/registry/apis/iam/legacy/sql.go index 47b29e65e82..5e0e3f470ed 100644 --- a/pkg/registry/apis/iam/legacy/sql.go +++ b/pkg/registry/apis/iam/legacy/sql.go @@ -18,6 +18,7 @@ type LegacyIdentityStore interface { ListUsers(ctx context.Context, ns claims.NamespaceInfo, query ListUserQuery) (*ListUserResult, error) ListUserTeams(ctx context.Context, ns claims.NamespaceInfo, query ListUserTeamsQuery) (*ListUserTeamsResult, error) + GetServiceAccountInternalID(ctx context.Context, ns claims.NamespaceInfo, query GetServiceAccountInternalIDQuery) (*GetServiceAccountInternalIDResult, error) ListServiceAccounts(ctx context.Context, ns claims.NamespaceInfo, query ListServiceAccountsQuery) (*ListServiceAccountResult, error) ListServiceAccountTokens(ctx context.Context, ns claims.NamespaceInfo, query ListServiceAccountTokenQuery) (*ListServiceAccountTokenResult, error) diff --git a/pkg/registry/apis/iam/register.go b/pkg/registry/apis/iam/register.go index 46bbd06b920..1a69dcf136c 100644 --- a/pkg/registry/apis/iam/register.go +++ b/pkg/registry/apis/iam/register.go @@ -108,7 +108,7 @@ func (b *IdentityAccessManagementAPIBuilder) UpdateAPIGroupInfo(apiGroupInfo *ge storage[userResource.StoragePath("teams")] = user.NewLegacyTeamMemberREST(b.store) serviceAccountResource := iamv0.ServiceAccountResourceInfo - storage[serviceAccountResource.StoragePath()] = serviceaccount.NewLegacyStore(b.store) + storage[serviceAccountResource.StoragePath()] = serviceaccount.NewLegacyStore(b.store, b.accessClient) storage[serviceAccountResource.StoragePath("tokens")] = serviceaccount.NewLegacyTokenREST(b.store) if b.sso != nil { diff --git a/pkg/registry/apis/iam/serviceaccount/store.go b/pkg/registry/apis/iam/serviceaccount/store.go index 618f2089793..fdc3e2f826e 100644 --- a/pkg/registry/apis/iam/serviceaccount/store.go +++ b/pkg/registry/apis/iam/serviceaccount/store.go @@ -10,6 +10,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/registry/rest" + "github.com/grafana/authlib/claims" "github.com/grafana/grafana/pkg/apimachinery/utils" iamv0 "github.com/grafana/grafana/pkg/apis/iam/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/iam/common" @@ -27,12 +28,13 @@ var ( var resource = iamv0.ServiceAccountResourceInfo -func NewLegacyStore(store legacy.LegacyIdentityStore) *LegacyStore { - return &LegacyStore{store} +func NewLegacyStore(store legacy.LegacyIdentityStore, ac claims.AccessClient) *LegacyStore { + return &LegacyStore{store, ac} } type LegacyStore struct { store legacy.LegacyIdentityStore + ac claims.AccessClient } func (s *LegacyStore) New() runtime.Object { @@ -58,28 +60,38 @@ func (s *LegacyStore) ConvertToTable(ctx context.Context, object runtime.Object, } func (s *LegacyStore) List(ctx context.Context, options *internalversion.ListOptions) (runtime.Object, error) { - ns, err := request.NamespaceInfoFrom(ctx, true) + res, err := common.List( + ctx, resource.GetName(), s.ac, common.PaginationFromListOptions(options), + func(ctx context.Context, ns claims.NamespaceInfo, p common.Pagination) (*common.ListResponse[iamv0.ServiceAccount], error) { + found, err := s.store.ListServiceAccounts(ctx, ns, legacy.ListServiceAccountsQuery{ + Pagination: p, + }) + + if err != nil { + return nil, err + } + + items := make([]iamv0.ServiceAccount, 0, len(found.Items)) + for _, sa := range found.Items { + items = append(items, toSAItem(sa, ns.Value)) + } + + return &common.ListResponse[iamv0.ServiceAccount]{ + Items: items, + RV: found.RV, + Continue: found.Continue, + }, nil + }, + ) + if err != nil { return nil, err } - found, err := s.store.ListServiceAccounts(ctx, ns, legacy.ListServiceAccountsQuery{ - OrgID: ns.OrgID, - Pagination: common.PaginationFromListOptions(options), - }) - if err != nil { - return nil, err - } - - list := &iamv0.ServiceAccountList{} - for _, item := range found.Items { - list.Items = append(list.Items, toSAItem(item, ns.Value)) - } - - list.ListMeta.Continue = common.OptionalFormatInt(found.Continue) - list.ListMeta.ResourceVersion = common.OptionalFormatInt(found.RV) - - return list, err + obj := &iamv0.ServiceAccountList{Items: res.Items} + obj.ListMeta.Continue = common.OptionalFormatInt(res.Continue) + obj.ListMeta.ResourceVersion = common.OptionalFormatInt(res.RV) + return obj, nil } func toSAItem(sa legacy.ServiceAccount, ns string) iamv0.ServiceAccount { diff --git a/pkg/registry/apis/iam/user/store.go b/pkg/registry/apis/iam/user/store.go index 26104cf9638..86e1f4669e4 100644 --- a/pkg/registry/apis/iam/user/store.go +++ b/pkg/registry/apis/iam/user/store.go @@ -11,7 +11,6 @@ import ( "k8s.io/apiserver/pkg/registry/rest" "github.com/grafana/authlib/claims" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/apimachinery/utils" iamv0 "github.com/grafana/grafana/pkg/apis/iam/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/iam/common" @@ -62,99 +61,40 @@ func (s *LegacyStore) ConvertToTable(ctx context.Context, object runtime.Object, } func (s *LegacyStore) List(ctx context.Context, options *internalversion.ListOptions) (runtime.Object, error) { - ns, err := request.NamespaceInfoFrom(ctx, true) - if err != nil { - return nil, err - } + res, err := common.List( + ctx, resource.GetName(), s.ac, common.PaginationFromListOptions(options), + func(ctx context.Context, ns claims.NamespaceInfo, p common.Pagination) (*common.ListResponse[iamv0.User], error) { + found, err := s.store.ListUsers(ctx, ns, legacy.ListUserQuery{ + Pagination: p, + }) - if s.ac != nil { - return s.listWithCheck(ctx, ns, common.PaginationFromListOptions(options)) - } - - return s.listWithoutCheck(ctx, ns, common.PaginationFromListOptions(options)) -} - -func (s *LegacyStore) listWithCheck(ctx context.Context, ns claims.NamespaceInfo, p common.Pagination) (runtime.Object, error) { - ident, err := identity.GetRequester(ctx) - if err != nil { - return nil, err - } - - check, err := s.ac.Compile(ctx, ident, claims.AccessRequest{ - Verb: "list", - Resource: resource.GetName(), - Namespace: ns.Value, - }) - - if err != nil { - return nil, err - } - - list := func(p common.Pagination) ([]iamv0.User, int64, int64, error) { - found, err := s.store.ListUsers(ctx, ns, legacy.ListUserQuery{ - Pagination: p, - }) - - if err != nil { - return nil, 0, 0, err - } - - out := make([]iamv0.User, 0, len(found.Users)) - for _, u := range found.Users { - if check(ns.Value, strconv.FormatInt(u.ID, 10)) { - out = append(out, toUserItem(&u, ns.Value)) + if err != nil { + return nil, err } - } - return out, found.Continue, found.RV, nil - } + users := make([]iamv0.User, 0, len(found.Users)) + for _, u := range found.Users { + users = append(users, toUserItem(&u, ns.Value)) + } + + return &common.ListResponse[iamv0.User]{ + Items: users, + RV: found.RV, + Continue: found.Continue, + }, nil + }, + ) - items, c, rv, err := list(p) if err != nil { return nil, err } -outer: - for len(items) < int(p.Limit) && c != 0 { - var more []iamv0.User - more, c, _, err = list(common.Pagination{Limit: p.Limit, Continue: c}) - if err != nil { - return nil, err - } - - for _, u := range more { - if len(items) == int(p.Limit) { - break outer - } - items = append(items, u) - } - } - - obj := &iamv0.UserList{Items: items} - obj.ListMeta.Continue = common.OptionalFormatInt(c) - obj.ListMeta.ResourceVersion = common.OptionalFormatInt(rv) + obj := &iamv0.UserList{Items: res.Items} + obj.ListMeta.Continue = common.OptionalFormatInt(res.Continue) + obj.ListMeta.ResourceVersion = common.OptionalFormatInt(res.RV) return obj, nil } -func (s *LegacyStore) listWithoutCheck(ctx context.Context, ns claims.NamespaceInfo, p common.Pagination) (runtime.Object, error) { - found, err := s.store.ListUsers(ctx, ns, legacy.ListUserQuery{ - Pagination: p, - }) - - if err != nil { - return nil, err - } - - list := &iamv0.UserList{} - for _, item := range found.Users { - list.Items = append(list.Items, toUserItem(&item, ns.Value)) - } - - list.ListMeta.Continue = common.OptionalFormatInt(found.Continue) - list.ListMeta.ResourceVersion = common.OptionalFormatInt(found.RV) - return list, nil -} - func (s *LegacyStore) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) { ns, err := request.NamespaceInfoFrom(ctx, true) if err != nil { @@ -191,6 +131,7 @@ func toUserItem(u *user.User, ns string) iamv0.User { Email: u.Email, EmailVerified: u.EmailVerified, Disabled: u.IsDisabled, + InternalID: u.ID, }, } obj, _ := utils.MetaAccessor(item) diff --git a/pkg/services/accesscontrol/authorizer.go b/pkg/services/accesscontrol/authorizer.go index 9134166f0f3..5d400b06dc9 100644 --- a/pkg/services/accesscontrol/authorizer.go +++ b/pkg/services/accesscontrol/authorizer.go @@ -7,6 +7,7 @@ import ( "github.com/grafana/authlib/claims" "github.com/grafana/grafana/pkg/apimachinery/identity" + "github.com/grafana/grafana/pkg/apimachinery/utils" ) // ResourceResolver is called before authorization is performed. @@ -35,6 +36,7 @@ type ResourceAuthorizerOptions struct { Attr string // Mapping is used to translate k8s verb to rbac action. // Key is the desired verb and value the rbac action it should be translated into. + // If no mapping is provided it will get a "default" mapping. Mapping map[string]string // Resolver if passed can translate into one or more scopes used to authorize resource. // This is useful when stored scopes are based on something else than k8s name or @@ -47,13 +49,28 @@ var _ claims.AccessClient = (*LegacyAccessClient)(nil) func NewLegacyAccessClient(ac AccessControl, opts ...ResourceAuthorizerOptions) *LegacyAccessClient { stored := map[string]ResourceAuthorizerOptions{} + defaultMapping := func(r string) map[string]string { + return map[string]string{ + utils.VerbGet: fmt.Sprintf("%s:read", r), + utils.VerbList: fmt.Sprintf("%s:read", r), + utils.VerbWatch: fmt.Sprintf("%s:read", r), + utils.VerbCreate: fmt.Sprintf("%s:create", r), + utils.VerbUpdate: fmt.Sprintf("%s:write", r), + utils.VerbPatch: fmt.Sprintf("%s:write", r), + utils.VerbDelete: fmt.Sprintf("%s:delete", r), + utils.VerbDeleteCollection: fmt.Sprintf("%s:delete", r), + } + } + for _, o := range opts { if o.Unchecked == nil { o.Unchecked = map[string]bool{} } + if o.Mapping == nil { - o.Mapping = map[string]string{} + o.Mapping = defaultMapping(o.Resource) } + stored[o.Resource] = o } @@ -107,7 +124,7 @@ func (c *LegacyAccessClient) HasAccess(ctx context.Context, id claims.AuthInfo, } else { eval = EvalPermission(action, fmt.Sprintf("%s:%s:%s", opts.Resource, opts.Attr, req.Name)) } - } else if req.Verb == "list" { + } else if req.Verb == utils.VerbList { // For list request we need to filter out in storage layer. eval = EvalPermission(action) } else { diff --git a/pkg/services/accesscontrol/authorizer_test.go b/pkg/services/accesscontrol/authorizer_test.go index d602d105cf2..5d7e7f28f71 100644 --- a/pkg/services/accesscontrol/authorizer_test.go +++ b/pkg/services/accesscontrol/authorizer_test.go @@ -14,14 +14,11 @@ import ( "github.com/grafana/grafana/pkg/services/featuremgmt" ) -func TestResourceAuthorizer_HasAccess(t *testing.T) { +func TestLegacyAccessClient_HasAccess(t *testing.T) { ac := acimpl.ProvideAccessControl(featuremgmt.WithFeatures(), zanzana.NewNoopClient()) - t.Run("should have no opinion for non resource requests", func(t *testing.T) { - a := accesscontrol.NewLegacyAccessClient(ac, accesscontrol.ResourceAuthorizerOptions{ - Resource: "dashboards", - Attr: "uid", - }) + t.Run("should reject when when no configuration for resource exist", func(t *testing.T) { + a := accesscontrol.NewLegacyAccessClient(ac) ok, err := a.HasAccess(context.Background(), &identity.StaticRequester{}, claims.AccessRequest{ Verb: "get", @@ -29,7 +26,7 @@ func TestResourceAuthorizer_HasAccess(t *testing.T) { Namespace: "default", Name: "1", }) - assert.Error(t, err) + assert.NoError(t, err) assert.Equal(t, false, ok) })