RBAC: Add option to skip rbac check for specified verbs (#93654)

* Add option to skip rbac check for specified verbs
This commit is contained in:
Karl Persson 2024-09-24 15:13:04 +02:00 committed by GitHub
parent b04799dab9
commit c28b37a67b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 149 additions and 109 deletions

View File

@ -166,7 +166,7 @@ func AddKnownTypes(scheme *runtime.Scheme, version string) {
&ServiceAccountTokenList{},
&Team{},
&TeamList{},
&IdentityDisplayResults{},
&DisplayList{},
&SSOSetting{},
&SSOSettingList{},
&TeamBinding{},

View File

@ -7,23 +7,24 @@ import (
)
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
type IdentityDisplayResults struct {
type DisplayList struct {
metav1.TypeMeta `json:",inline"`
metav1.ListMeta `json:"metadata,omitempty"`
// Request keys used to lookup the display value
// +listType=set
Keys []string `json:"keys"`
// Matching items (the caller may need to remap from keys to results)
// +listType=atomic
Display []IdentityDisplay `json:"display"`
// Input keys that were not useable
// +listType=set
InvalidKeys []string `json:"invalidKeys,omitempty"`
// Matching items (the caller may need to remap from keys to results)
// +listType=atomic
Items []Display `json:"display"`
}
type IdentityDisplay struct {
type Display struct {
Identity IdentityRef `json:"identity"`
// Display name for identity.

View File

@ -68,7 +68,7 @@ type TeamMemberList struct {
}
type TeamMember struct {
IdentityDisplay `json:",inline"`
Display `json:",inline"`
// External is set if member ship was synced from external IDP.
External bool `json:"external,omitempty"`

View File

@ -12,56 +12,57 @@ import (
)
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
func (in *IdentityDisplay) DeepCopyInto(out *IdentityDisplay) {
func (in *Display) DeepCopyInto(out *Display) {
*out = *in
out.Identity = in.Identity
return
}
// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IdentityDisplay.
func (in *IdentityDisplay) DeepCopy() *IdentityDisplay {
// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Display.
func (in *Display) DeepCopy() *Display {
if in == nil {
return nil
}
out := new(IdentityDisplay)
out := new(Display)
in.DeepCopyInto(out)
return out
}
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
func (in *IdentityDisplayResults) DeepCopyInto(out *IdentityDisplayResults) {
func (in *DisplayList) DeepCopyInto(out *DisplayList) {
*out = *in
out.TypeMeta = in.TypeMeta
in.ListMeta.DeepCopyInto(&out.ListMeta)
if in.Keys != nil {
in, out := &in.Keys, &out.Keys
*out = make([]string, len(*in))
copy(*out, *in)
}
if in.Display != nil {
in, out := &in.Display, &out.Display
*out = make([]IdentityDisplay, len(*in))
copy(*out, *in)
}
if in.InvalidKeys != nil {
in, out := &in.InvalidKeys, &out.InvalidKeys
*out = make([]string, len(*in))
copy(*out, *in)
}
if in.Items != nil {
in, out := &in.Items, &out.Items
*out = make([]Display, len(*in))
copy(*out, *in)
}
return
}
// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IdentityDisplayResults.
func (in *IdentityDisplayResults) DeepCopy() *IdentityDisplayResults {
// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DisplayList.
func (in *DisplayList) DeepCopy() *DisplayList {
if in == nil {
return nil
}
out := new(IdentityDisplayResults)
out := new(DisplayList)
in.DeepCopyInto(out)
return out
}
// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object.
func (in *IdentityDisplayResults) DeepCopyObject() runtime.Object {
func (in *DisplayList) DeepCopyObject() runtime.Object {
if c := in.DeepCopy(); c != nil {
return c
}
@ -440,7 +441,7 @@ func (in *TeamList) DeepCopyObject() runtime.Object {
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
func (in *TeamMember) DeepCopyInto(out *TeamMember) {
*out = *in
out.IdentityDisplay = in.IdentityDisplay
out.Display = in.Display
return
}

View File

@ -14,8 +14,8 @@ import (
func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenAPIDefinition {
return map[string]common.OpenAPIDefinition{
"github.com/grafana/grafana/pkg/apis/iam/v0alpha1.IdentityDisplay": schema_pkg_apis_iam_v0alpha1_IdentityDisplay(ref),
"github.com/grafana/grafana/pkg/apis/iam/v0alpha1.IdentityDisplayResults": schema_pkg_apis_iam_v0alpha1_IdentityDisplayResults(ref),
"github.com/grafana/grafana/pkg/apis/iam/v0alpha1.Display": schema_pkg_apis_iam_v0alpha1_Display(ref),
"github.com/grafana/grafana/pkg/apis/iam/v0alpha1.DisplayList": schema_pkg_apis_iam_v0alpha1_DisplayList(ref),
"github.com/grafana/grafana/pkg/apis/iam/v0alpha1.IdentityRef": schema_pkg_apis_iam_v0alpha1_IdentityRef(ref),
"github.com/grafana/grafana/pkg/apis/iam/v0alpha1.SSOSetting": schema_pkg_apis_iam_v0alpha1_SSOSetting(ref),
"github.com/grafana/grafana/pkg/apis/iam/v0alpha1.SSOSettingList": schema_pkg_apis_iam_v0alpha1_SSOSettingList(ref),
@ -43,7 +43,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA
}
}
func schema_pkg_apis_iam_v0alpha1_IdentityDisplay(ref common.ReferenceCallback) common.OpenAPIDefinition {
func schema_pkg_apis_iam_v0alpha1_Display(ref common.ReferenceCallback) common.OpenAPIDefinition {
return common.OpenAPIDefinition{
Schema: spec.Schema{
SchemaProps: spec.SchemaProps{
@ -86,7 +86,7 @@ func schema_pkg_apis_iam_v0alpha1_IdentityDisplay(ref common.ReferenceCallback)
}
}
func schema_pkg_apis_iam_v0alpha1_IdentityDisplayResults(ref common.ReferenceCallback) common.OpenAPIDefinition {
func schema_pkg_apis_iam_v0alpha1_DisplayList(ref common.ReferenceCallback) common.OpenAPIDefinition {
return common.OpenAPIDefinition{
Schema: spec.Schema{
SchemaProps: spec.SchemaProps{
@ -106,6 +106,12 @@ func schema_pkg_apis_iam_v0alpha1_IdentityDisplayResults(ref common.ReferenceCal
Format: "",
},
},
"metadata": {
SchemaProps: spec.SchemaProps{
Default: map[string]interface{}{},
Ref: ref("k8s.io/apimachinery/pkg/apis/meta/v1.ListMeta"),
},
},
"keys": {
VendorExtensible: spec.VendorExtensible{
Extensions: spec.Extensions{
@ -126,25 +132,6 @@ func schema_pkg_apis_iam_v0alpha1_IdentityDisplayResults(ref common.ReferenceCal
},
},
},
"display": {
VendorExtensible: spec.VendorExtensible{
Extensions: spec.Extensions{
"x-kubernetes-list-type": "atomic",
},
},
SchemaProps: spec.SchemaProps{
Description: "Matching items (the caller may need to remap from keys to results)",
Type: []string{"array"},
Items: &spec.SchemaOrArray{
Schema: &spec.Schema{
SchemaProps: spec.SchemaProps{
Default: map[string]interface{}{},
Ref: ref("github.com/grafana/grafana/pkg/apis/iam/v0alpha1.IdentityDisplay"),
},
},
},
},
},
"invalidKeys": {
VendorExtensible: spec.VendorExtensible{
Extensions: spec.Extensions{
@ -165,12 +152,31 @@ func schema_pkg_apis_iam_v0alpha1_IdentityDisplayResults(ref common.ReferenceCal
},
},
},
"display": {
VendorExtensible: spec.VendorExtensible{
Extensions: spec.Extensions{
"x-kubernetes-list-type": "atomic",
},
},
SchemaProps: spec.SchemaProps{
Description: "Matching items (the caller may need to remap from keys to results)",
Type: []string{"array"},
Items: &spec.SchemaOrArray{
Schema: &spec.Schema{
SchemaProps: spec.SchemaProps{
Default: map[string]interface{}{},
Ref: ref("github.com/grafana/grafana/pkg/apis/iam/v0alpha1.Display"),
},
},
},
},
},
},
Required: []string{"keys", "display"},
},
},
Dependencies: []string{
"github.com/grafana/grafana/pkg/apis/iam/v0alpha1.IdentityDisplay"},
"github.com/grafana/grafana/pkg/apis/iam/v0alpha1.Display", "k8s.io/apimachinery/pkg/apis/meta/v1.ListMeta"},
}
}

View File

@ -1,2 +1,4 @@
API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/iam/v0alpha1,DisplayList,Items
API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/iam/v0alpha1,TeamBindingSpec,Subjects
API rule violation: names_match,github.com/grafana/grafana/pkg/apis/iam/v0alpha1,IdentityDisplay,InternalID
API rule violation: names_match,github.com/grafana/grafana/pkg/apis/iam/v0alpha1,Display,InternalID
API rule violation: names_match,github.com/grafana/grafana/pkg/apis/iam/v0alpha1,DisplayList,Items

View File

@ -7,29 +7,40 @@ import (
"github.com/grafana/authlib/claims"
"k8s.io/apiserver/pkg/authorization/authorizer"
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"
gfauthorizer "github.com/grafana/grafana/pkg/services/apiserver/auth/authorizer"
)
func newLegacyAuthorizer(ac accesscontrol.AccessControl, store legacy.LegacyIdentityStore) (authorizer.Authorizer, claims.AccessClient) {
client := accesscontrol.NewLegacyAccessClient(ac, accesscontrol.ResourceAuthorizerOptions{
Resource: "users",
Attr: "id",
Mapping: map[string]string{
"get": accesscontrol.ActionOrgUsersRead,
"list": accesscontrol.ActionOrgUsersRead,
client := accesscontrol.NewLegacyAccessClient(
ac,
accesscontrol.ResourceAuthorizerOptions{
Resource: iamv0.UserResourceInfo.GetName(),
Attr: "id",
Mapping: map[string]string{
"get": accesscontrol.ActionOrgUsersRead,
"list": accesscontrol.ActionOrgUsersRead,
},
Resolver: accesscontrol.ResourceResolverFunc(func(ctx context.Context, ns claims.NamespaceInfo, name string) ([]string, error) {
res, err := store.GetUserInternalID(ctx, ns, legacy.GetUserInternalIDQuery{
UID: name,
})
if err != nil {
return nil, err
}
return []string{fmt.Sprintf("users:id:%d", res.ID)}, nil
}),
},
Resolver: accesscontrol.ResourceResolverFunc(func(ctx context.Context, ns claims.NamespaceInfo, name string) ([]string, error) {
res, err := store.GetUserInternalID(ctx, ns, legacy.GetUserInternalIDQuery{
UID: name,
})
if err != nil {
return nil, err
}
return []string{fmt.Sprintf("users:id:%d", res.ID)}, nil
}),
})
accesscontrol.ResourceAuthorizerOptions{
Resource: "display",
Unchecked: map[string]bool{
"get": true,
"list": true,
},
},
)
return gfauthorizer.NewResourceAuthorizer(client), client
}

View File

@ -24,7 +24,6 @@ import (
"github.com/grafana/grafana/pkg/registry/apis/iam/user"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/apiserver/builder"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/ssosettings"
"github.com/grafana/grafana/pkg/storage/legacysql"
)
@ -42,17 +41,11 @@ type IdentityAccessManagementAPIBuilder struct {
}
func RegisterAPIService(
features featuremgmt.FeatureToggles,
apiregistration builder.APIRegistrar,
ssoService ssosettings.Service,
sql db.DB,
ac accesscontrol.AccessControl,
) (*IdentityAccessManagementAPIBuilder, error) {
if !features.IsEnabledGlobally(featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs) {
// skip registration unless opting into experimental apis
return nil, nil
}
store := legacy.NewLegacySQLStores(legacysql.NewDatabaseProvider(sql))
authorizer, client := newLegacyAuthorizer(ac, store)
@ -114,9 +107,9 @@ func (b *IdentityAccessManagementAPIBuilder) UpdateAPIGroupInfo(apiGroupInfo *ge
storage[userResource.StoragePath()] = user.NewLegacyStore(b.store, b.accessClient)
storage[userResource.StoragePath("teams")] = user.NewLegacyTeamMemberREST(b.store)
serviceaccountResource := iamv0.ServiceAccountResourceInfo
storage[serviceaccountResource.StoragePath()] = serviceaccount.NewLegacyStore(b.store)
storage[serviceaccountResource.StoragePath("tokens")] = serviceaccount.NewLegacyTokenREST(b.store)
serviceAccountResource := iamv0.ServiceAccountResourceInfo
storage[serviceAccountResource.StoragePath()] = serviceaccount.NewLegacyStore(b.store)
storage[serviceAccountResource.StoragePath("tokens")] = serviceaccount.NewLegacyTokenREST(b.store)
if b.sso != nil {
ssoResource := iamv0.SSOSettingResourceInfo

View File

@ -97,7 +97,7 @@ var cfg = &setting.Cfg{}
func mapToTeamMember(m legacy.TeamMember) iamv0.TeamMember {
return iamv0.TeamMember{
IdentityDisplay: iamv0.IdentityDisplay{
Display: iamv0.Display{
Identity: iamv0.IdentityRef{
Type: claims.TypeUser,
Name: m.UserUID,

View File

@ -35,7 +35,7 @@ func NewLegacyDisplayREST(store legacy.LegacyIdentityStore) *LegacyDisplayREST {
}
func (r *LegacyDisplayREST) New() runtime.Object {
return &iamv0.IdentityDisplayResults{}
return &iamv0.DisplayList{}
}
func (r *LegacyDisplayREST) Destroy() {}
@ -45,8 +45,7 @@ func (r *LegacyDisplayREST) NamespaceScoped() bool {
}
func (r *LegacyDisplayREST) GetSingularName() string {
// not actually used anywhere, but required by SingularNameProvider
return "identitydisplay"
return "display"
}
func (r *LegacyDisplayREST) ProducesMIMETypes(verb string) []string {
@ -54,11 +53,11 @@ func (r *LegacyDisplayREST) ProducesMIMETypes(verb string) []string {
}
func (r *LegacyDisplayREST) ProducesObject(verb string) any {
return &iamv0.IdentityDisplayResults{}
return &iamv0.DisplayList{}
}
func (r *LegacyDisplayREST) ConnectMethods() []string {
return []string{"GET"}
return []string{http.MethodGet}
}
func (r *LegacyDisplayREST) NewConnectOptions() (runtime.Object, bool, string) {
@ -91,13 +90,13 @@ func (r *LegacyDisplayREST) Connect(ctx context.Context, name string, _ runtime.
return
}
rsp := &iamv0.IdentityDisplayResults{
rsp := &iamv0.DisplayList{
Keys: keys.keys,
InvalidKeys: keys.invalid,
Display: make([]iamv0.IdentityDisplay, 0, len(users.Users)+len(keys.disp)+1),
Items: make([]iamv0.Display, 0, len(users.Users)+len(keys.disp)+1),
}
for _, user := range users.Users {
disp := iamv0.IdentityDisplay{
disp := iamv0.Display{
Identity: iamv0.IdentityRef{
Type: claims.TypeUser,
Name: user.UID,
@ -109,12 +108,12 @@ func (r *LegacyDisplayREST) Connect(ctx context.Context, name string, _ runtime.
disp.Identity.Type = claims.TypeServiceAccount
}
disp.AvatarURL = dtos.GetGravatarUrlWithDefault(fakeCfgForGravatar, user.Email, disp.DisplayName)
rsp.Display = append(rsp.Display, disp)
rsp.Items = append(rsp.Items, disp)
}
// Append the constants here
if len(keys.disp) > 0 {
rsp.Display = append(rsp.Display, keys.disp...)
rsp.Items = append(rsp.Items, keys.disp...)
}
responder.Object(200, rsp)
}), nil
@ -127,7 +126,7 @@ type dispKeys struct {
invalid []string
// For terminal keys, this is a constant
disp []iamv0.IdentityDisplay
disp []iamv0.Display
}
func parseKeys(req []string) dispKeys {
@ -148,7 +147,7 @@ func parseKeys(req []string) dispKeys {
switch t {
case claims.TypeAnonymous:
keys.disp = append(keys.disp, iamv0.IdentityDisplay{
keys.disp = append(keys.disp, iamv0.Display{
Identity: iamv0.IdentityRef{
Type: t,
},
@ -157,7 +156,7 @@ func parseKeys(req []string) dispKeys {
})
continue
case claims.TypeAPIKey:
keys.disp = append(keys.disp, iamv0.IdentityDisplay{
keys.disp = append(keys.disp, iamv0.Display{
Identity: iamv0.IdentityRef{
Type: t,
Name: key,
@ -167,7 +166,7 @@ func parseKeys(req []string) dispKeys {
})
continue
case claims.TypeProvisioning:
keys.disp = append(keys.disp, iamv0.IdentityDisplay{
keys.disp = append(keys.disp, iamv0.Display{
Identity: iamv0.IdentityRef{
Type: t,
},
@ -184,7 +183,7 @@ func parseKeys(req []string) dispKeys {
id, err := strconv.ParseInt(key, 10, 64)
if err == nil {
if id == 0 {
keys.disp = append(keys.disp, iamv0.IdentityDisplay{
keys.disp = append(keys.disp, iamv0.Display{
Identity: iamv0.IdentityRef{
Type: claims.TypeUser,
Name: key,

View File

@ -27,6 +27,9 @@ func (r ResourceResolverFunc) Resolve(ctx context.Context, ns claims.NamespaceIn
type ResourceAuthorizerOptions struct {
// Resource is the resource name in plural.
Resource string
// Unchecked is used to skip authorization checks for specified verbs.
// This takes precedence over configured Mapping
Unchecked map[string]bool
// Attr is attribute used for resource scope. It's usually 'id' or 'uid'
// depending on what is stored for the resource.
Attr string
@ -45,6 +48,9 @@ func NewLegacyAccessClient(ac AccessControl, opts ...ResourceAuthorizerOptions)
stored := map[string]ResourceAuthorizerOptions{}
for _, o := range opts {
if o.Unchecked == nil {
o.Unchecked = map[string]bool{}
}
if o.Mapping == nil {
o.Mapping = map[string]string{}
}
@ -75,6 +81,11 @@ func (c *LegacyAccessClient) HasAccess(ctx context.Context, id claims.AuthInfo,
return false, nil
}
skip := opts.Unchecked[req.Verb]
if skip {
return true, nil
}
action, ok := opts.Mapping[req.Verb]
if !ok {
return false, fmt.Errorf("missing action for %s %s", req.Verb, req.Resource)

View File

@ -103,6 +103,41 @@ func TestResourceAuthorizer_HasAccess(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, true, ok)
})
t.Run("should skip authorization for configured verb", func(t *testing.T) {
a := accesscontrol.NewLegacyAccessClient(ac, accesscontrol.ResourceAuthorizerOptions{
Resource: "dashboards",
Attr: "uid",
Unchecked: map[string]bool{
"get": true,
},
Mapping: map[string]string{
"create": "dashboards:create",
},
})
ident := newIdent(accesscontrol.Permission{})
ok, err := a.HasAccess(context.Background(), ident, claims.AccessRequest{
Verb: "get",
Namespace: "default",
Resource: "dashboards",
Name: "1",
})
assert.NoError(t, err)
assert.Equal(t, true, ok)
ok, err = a.HasAccess(context.Background(), ident, claims.AccessRequest{
Verb: "create",
Namespace: "default",
Resource: "dashboards",
Name: "1",
})
assert.NoError(t, err)
assert.Equal(t, false, ok)
})
}
func newIdent(permissions ...accesscontrol.Permission) *identity.StaticRequester {

View File

@ -2,7 +2,6 @@ package identity
import (
"context"
"fmt"
"testing"
"github.com/grafana/grafana/pkg/services/featuremgmt"
@ -30,22 +29,6 @@ func TestMain(m *testing.M) {
testsuite.Run(m)
}
func TestIntegrationRequiresDevMode(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test")
}
helper := apis.NewK8sTestHelper(t, testinfra.GrafanaOpts{
AppModeProduction: true, // should fail
DisableAnonymous: true,
EnableFeatureToggles: []string{
featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs, // Required to start the example service
},
})
_, err := helper.NewDiscoveryClient().ServerResourcesForGroupVersion("iam.grafana.app/v0alpha1")
require.Error(t, err)
}
func TestIntegrationIdentity(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test")
@ -102,7 +85,6 @@ func TestIntegrationIdentity(t *testing.T) {
// Get just the specs (avoids values that change with each deployment)
found = teamClient.SpecJSON(rsp)
// fmt.Printf("%s", found) // NOTE the first value does not have an email or login
require.JSONEq(t, `[
{},
{
@ -130,7 +112,6 @@ func TestIntegrationIdentity(t *testing.T) {
// Get just the specs (avoids values that change with each deployment)
found = teamClient.SpecJSON(rsp)
fmt.Printf("%s", found) // NOTE the first value does not have an email or login
require.JSONEq(t, `[
{
"email": "admin-3",