public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "Folders: Use authlib.AccessClient in authorizer" (#110812)

Browse Source 
Revert "Folders: Use authlib.AccessClient in authorizer (#110602)"

This reverts commit 0cb52b8be0.
This commit is contained in:
Gabriel MABILLE
2025-09-09 15:45:37 +02:00
committed by GitHub
parent b30916c917
commit d0f25b0cd7
7 changed files with 317 additions and 162 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/registry/apis/dashboard/register.go
+6 -6
View File
@@ -19,7 +19,7 @@ import (
"k8s.io/kube-openapi/pkg/spec3"
"k8s.io/kube-openapi/pkg/validation/spec"
authlib "github.com/grafana/authlib/types"
claims "github.com/grafana/authlib/types"
internal "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard"
dashv0 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1"
dashv1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v1beta1"
@@ -79,7 +79,7 @@ type DashboardsAPIBuilder struct {
authorizer authorizer.Authorizer
accessControl accesscontrol.AccessControl
accessClient authlib.AccessClient
accessClient claims.AccessClient
legacy *DashboardStorage
unified resource.ResourceClient
dashboardProvisioningService dashboards.DashboardProvisioningService
@@ -113,7 +113,7 @@ func RegisterAPIService(
dashboardPermissions dashboards.PermissionsRegistrationService,
dashboardPermissionsSvc accesscontrol.DashboardPermissionsService,
accessControl accesscontrol.AccessControl,
accessClient authlib.AccessClient,
accessClient claims.AccessClient,
provisioning provisioning.ProvisioningService,
dashStore dashboards.Store,
reg prometheus.Registerer,
@@ -167,7 +167,7 @@ func RegisterAPIService(
return builder
}
func NewAPIService(ac authlib.AccessClient, features featuremgmt.FeatureToggles, folderClientProvider client.K8sHandlerProvider, datasourceProvider schemaversion.DataSourceInfoProvider, pluginStore *pluginstore.Service) *DashboardsAPIBuilder {
func NewAPIService(ac claims.AccessClient, features featuremgmt.FeatureToggles, folderClientProvider client.K8sHandlerProvider, datasourceProvider schemaversion.DataSourceInfoProvider, pluginStore *pluginstore.Service) *DashboardsAPIBuilder {
// TODO: Plugin store will soon be removed,
// as the cases for plugin fetching is not needed. Keeping it now to not break implementation
if pluginStore == nil {
@@ -274,7 +274,7 @@ func (b *DashboardsAPIBuilder) validateDelete(ctx context.Context, a admission.A
return nil
}
nsInfo, err := authlib.ParseNamespace(a.GetNamespace())
nsInfo, err := claims.ParseNamespace(a.GetNamespace())
if err != nil {
return fmt.Errorf("%v: %w", "failed to parse namespace", err)
}
@@ -380,7 +380,7 @@ func (b *DashboardsAPIBuilder) validateUpdate(ctx context.Context, a admission.A
}
// Parse namespace for old dashboard
nsInfo, err := authlib.ParseNamespace(oldAccessor.GetNamespace())
nsInfo, err := claims.ParseNamespace(oldAccessor.GetNamespace())
if err != nil {
return fmt.Errorf("failed to parse namespace: %w", err)
}
pkg/registry/apis/folders/authorizer.go
+74 -5
View File
@@ -2,20 +2,89 @@ package folders
import (
"context"
"errors"
"slices"
"k8s.io/apiserver/pkg/authorization/authorizer"
authlib "github.com/grafana/authlib/types"
"github.com/grafana/authlib/types"
"github.com/grafana/grafana/pkg/apimachinery/identity"
"github.com/grafana/grafana/pkg/apimachinery/utils"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/dashboards"
)
func newAuthorizer(ac authlib.AccessChecker) authorizer.Authorizer {
// newLegacyAuthorizer creates an authorizer using legacy access control, this is only usable for single tenant api.
func newLegacyAuthorizer(ac accesscontrol.AccessControl) authorizer.Authorizer {
return authorizer.AuthorizerFunc(func(ctx context.Context, attr authorizer.Attributes) (authorizer.Decision, string, error) {
in, err := authorizerFunc(ctx, attr)
if err != nil {
if errors.Is(err, errNoUser) {
return authorizer.DecisionDeny, "", nil
}
return authorizer.DecisionNoOpinion, "", nil
}
ok, err := ac.Evaluate(ctx, in.user, in.evaluator)
if ok {
return authorizer.DecisionAllow, "", nil
}
return authorizer.DecisionDeny, "folder", err
})
}
func authorizerFunc(ctx context.Context, attr authorizer.Attributes) (*authorizerParams, error) {
allowedVerbs := []string{utils.VerbCreate, utils.VerbDelete, utils.VerbList}
verb := attr.GetVerb()
name := attr.GetName()
if (!attr.IsResourceRequest()) || (name == "" && verb != utils.VerbCreate && slices.Contains(allowedVerbs, verb)) {
return nil, errNoResource
}
// require a user
user, err := identity.GetRequester(ctx)
if err != nil {
return nil, errNoUser
}
scope := dashboards.ScopeFoldersProvider.GetResourceScopeUID(name)
var eval accesscontrol.Evaluator
// "get" is used for sub-resources with GET http (parents, access, count)
switch verb {
case utils.VerbCreate:
eval = accesscontrol.EvalPermission(dashboards.ActionFoldersCreate)
case utils.VerbPatch:
fallthrough
case utils.VerbUpdate:
eval = accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, scope)
case utils.VerbDeleteCollection:
fallthrough
case utils.VerbDelete:
eval = accesscontrol.EvalPermission(dashboards.ActionFoldersDelete, scope)
case utils.VerbList:
eval = accesscontrol.EvalPermission(dashboards.ActionFoldersRead)
default:
eval = accesscontrol.EvalPermission(dashboards.ActionFoldersRead, scope)
}
return &authorizerParams{evaluator: eval, user: user}, nil
}
// newMultiTenantAuthorizer creates an authorizer suitable to multi-tenant setup.
// For now it only allow authorization of access tokens.
func newMultiTenantAuthorizer(ac types.AccessClient) authorizer.Authorizer {
return authorizer.AuthorizerFunc(func(ctx context.Context, a authorizer.Attributes) (authorizer.Decision, string, error) {
info, ok := authlib.AuthInfoFrom(ctx)
info, ok := types.AuthInfoFrom(ctx)
if !ok {
return authorizer.DecisionDeny, "missing auth info", nil
}
res, err := ac.Check(ctx, info, authlib.CheckRequest{
// For now we only allow access policy to authorize with multi-tenant setup
if !types.IsIdentityType(info.GetIdentityType(), types.TypeAccessPolicy) {
return authorizer.DecisionDeny, "permission denied", nil
}
res, err := ac.Check(ctx, info, types.CheckRequest{
Verb: a.GetVerb(),
Group: a.GetAPIGroup(),
Resource: a.GetResource(),
@@ -25,7 +94,7 @@ func newAuthorizer(ac authlib.AccessChecker) authorizer.Authorizer {
})
if err != nil {
return authorizer.DecisionDeny, "failed to perform authorization", err
return authorizer.DecisionDeny, "faild to perform authorization", err
}
if !res.Allowed {
pkg/registry/apis/folders/authorizer_test.go
+188 -42
View File
@@ -2,7 +2,6 @@ package folders
import (
"context"
"fmt"
"testing"
"github.com/go-jose/go-jose/v3/jwt"
@@ -11,16 +10,183 @@ import (
"github.com/grafana/authlib/authn"
"github.com/grafana/authlib/authz"
authlib "github.com/grafana/authlib/types"
"github.com/grafana/authlib/types"
folders "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1beta1"
"github.com/grafana/grafana/pkg/apimachinery/identity"
"github.com/grafana/grafana/pkg/apimachinery/utils"
"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/user"
)
func TestFolderAuthorizer(t *testing.T) {
func TestLegacyAuthorizer(t *testing.T) {
type input struct {
user identity.Requester
verb string
}
type expect struct {
authorized authorizer.Decision
err error
}
var orgID int64 = 1
tests := []struct {
name string
input input
expect expect
}{
{
name: "user with create permissions should be able to create a folder",
input: input{
user: &user.SignedInUser{
UserID: 1,
OrgID: orgID,
Name: "123",
Permissions: map[int64]map[string][]string{
orgID: {dashboards.ActionFoldersCreate: {}, dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersAll}},
},
},
verb: string(utils.VerbCreate),
},
expect: expect{
authorized: authorizer.DecisionAllow,
},
},
{
name: "not possible to create a folder without a user",
input: input{
user: nil,
verb: string(utils.VerbCreate),
},
expect: expect{authorized: authorizer.DecisionDeny},
},
{
name: "user without permissions should not be able to create a folder",
input: input{
user: &user.SignedInUser{},
verb: string(utils.VerbCreate),
},
expect: expect{authorized: authorizer.DecisionDeny},
},
{
name: "user in another orgId should not be able to create a folder ",
input: input{
user: &user.SignedInUser{
UserID: 1,
OrgID: 2,
Name: "123",
Permissions: map[int64]map[string][]string{
orgID: {dashboards.ActionFoldersCreate: {}, dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersAll}},
},
},
verb: string(utils.VerbCreate),
},
expect: expect{authorized: authorizer.DecisionDeny},
},
{
name: "user with read permissions should be able to list folders",
input: input{
user: &user.SignedInUser{
UserID: 1,
OrgID: orgID,
Name: "123",
Permissions: map[int64]map[string][]string{
orgID: {},
},
},
verb: string(utils.VerbList),
},
expect: expect{authorized: authorizer.DecisionDeny},
},
{
name: "user with delete permissions should be able to delete a folder",
input: input{
user: &user.SignedInUser{
UserID: 1,
OrgID: orgID,
Name: "123",
Permissions: map[int64]map[string][]string{
orgID: {dashboards.ActionFoldersDelete: {dashboards.ScopeFoldersAll}, dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersAll}},
},
},
verb: string(utils.VerbDelete),
},
expect: expect{authorized: authorizer.DecisionAllow},
},
{
name: "user without delete permissions should NOT be able to delete a folder",
input: input{
user: &user.SignedInUser{
UserID: 1,
OrgID: orgID,
Name: "123",
Permissions: map[int64]map[string][]string{
orgID: {},
},
},
verb: string(utils.VerbDelete),
},
expect: expect{authorized: authorizer.DecisionDeny},
},
{
name: "user with write permissions should be able to update a folder",
input: input{
user: &user.SignedInUser{
UserID: 1,
OrgID: orgID,
Name: "123",
Permissions: map[int64]map[string][]string{
orgID: {dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersAll}},
},
},
verb: string(utils.VerbUpdate),
},
expect: expect{authorized: authorizer.DecisionAllow},
},
{
name: "user without write permissions should NOT be able to update a folder",
input: input{
user: &user.SignedInUser{
UserID: 1,
OrgID: orgID,
Name: "123",
Permissions: map[int64]map[string][]string{
orgID: {},
},
},
verb: string(utils.VerbUpdate),
},
expect: expect{authorized: authorizer.DecisionDeny},
},
}
authz := newLegacyAuthorizer(acimpl.ProvideAccessControl(featuremgmt.WithFeatures()))
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
authorized, _, err := authz.Authorize(
identity.WithRequester(context.Background(), tt.input.user),
authorizer.AttributesRecord{User: tt.input.user, Verb: tt.input.verb, Resource: "folders", ResourceRequest: true, Name: "123"},
)
if tt.expect.err != nil {
require.Error(t, err)
require.Equal(t, authorizer.DecisionDeny, authorized)
return
}
require.NoError(t, err)
require.Equal(t, tt.expect.authorized, authorized)
})
}
}
func TestMultiTenantAuthorizer(t *testing.T) {
type input struct {
verb string
info authlib.AuthInfo
client authlib.AccessChecker
info types.AuthInfo
client types.AccessClient
}
type expected struct {
@@ -29,17 +195,21 @@ func TestFolderAuthorizer(t *testing.T) {
}
tests := []struct {
name string
input input
expected expected
name string
input input
expeted expected
}{
{
name: "missing auth info",
name: "non access policy idenity should not be able to authorize",
input: input{
verb: utils.VerbGet,
client: authz.NewClient(nil),
verb: utils.VerbGet,
info: &identity.StaticRequester{
Type: types.TypeUser,
UserID: 1,
UserUID: "1",
},
},
expected: expected{
expeted: expected{
authorized: authorizer.DecisionDeny,
},
},
@@ -60,7 +230,7 @@ func TestFolderAuthorizer(t *testing.T) {
}),
client: authz.NewClient(nil),
},
expected: expected{
expeted: expected{
authorized: authorizer.DecisionAllow,
},
},
@@ -81,52 +251,28 @@ func TestFolderAuthorizer(t *testing.T) {
}),
client: authz.NewClient(nil),
},
expected: expected{
expeted: expected{
authorized: authorizer.DecisionDeny,
},
},
{
name: "with client error",
input: input{
verb: utils.VerbGet,
info: authn.NewIDTokenAuthInfo(authn.Claims[authn.AccessTokenClaims]{}, &authn.Claims[authn.IDTokenClaims]{}),
client: &dummyClient{
check: func(ctx context.Context, info authlib.AuthInfo, req authlib.CheckRequest) (authlib.CheckResponse, error) {
return authlib.CheckResponse{}, fmt.Errorf("nope")
},
},
},
expected: expected{
authorized: authorizer.DecisionDeny,
err: true,
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
authz := newAuthorizer(tt.input.client)
authz := newMultiTenantAuthorizer(tt.input.client)
authorized, _, err := authz.Authorize(
authlib.WithAuthInfo(context.Background(), tt.input.info),
types.WithAuthInfo(context.Background(), tt.input.info),
authorizer.AttributesRecord{User: tt.input.info, Verb: tt.input.verb, APIGroup: folders.GROUP, Resource: "folders", ResourceRequest: true, Name: "123", Namespace: "stacks-1"},
)
if tt.expected.err {
if tt.expeted.err {
require.Error(t, err)
require.Equal(t, authorizer.DecisionDeny, authorized)
return
}
require.NoError(t, err)
require.Equal(t, tt.expected.authorized, authorized)
require.Equal(t, tt.expeted.authorized, authorized)
})
}
}
type dummyClient struct {
check func(ctx context.Context, info authlib.AuthInfo, req authlib.CheckRequest) (authlib.CheckResponse, error)
}
func (c *dummyClient) Check(ctx context.Context, info authlib.AuthInfo, req authlib.CheckRequest) (authlib.CheckResponse, error) {
return c.check(ctx, info, req)
}
pkg/registry/apis/folders/register.go
+12 -2
View File
@@ -2,6 +2,7 @@ package folders
import (
"context"
"errors"
"fmt"
"strings"
@@ -19,6 +20,7 @@ import (
authlib "github.com/grafana/authlib/types"
folders "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1beta1"
"github.com/grafana/grafana/apps/iam/pkg/reconcilers"
"github.com/grafana/grafana/pkg/apimachinery/identity"
grafanaregistry "github.com/grafana/grafana/pkg/apiserver/registry/generic"
grafanarest "github.com/grafana/grafana/pkg/apiserver/rest"
"github.com/grafana/grafana/pkg/services/accesscontrol"
@@ -38,6 +40,9 @@ var _ builder.APIGroupValidation = (*FolderAPIBuilder)(nil)
var resourceInfo = folders.FolderResourceInfo
var errNoUser = errors.New("valid user is required")
var errNoResource = errors.New("resource name is required")
// This is used just so wire has something unique to return
type FolderAPIBuilder struct {
features featuremgmt.FeatureToggles
@@ -77,7 +82,7 @@ func RegisterAPIService(cfg *setting.Cfg,
acService: acService,
ac: accessControl,
permissionsOnCreate: cfg.RBAC.PermissionsOnCreation("folder"),
authorizer: newAuthorizer(accessClient),
authorizer: newLegacyAuthorizer(accessControl),
searcher: unified,
permissionStore: reconcilers.NewZanzanaPermissionStore(zanzanaClient),
}
@@ -87,7 +92,7 @@ func RegisterAPIService(cfg *setting.Cfg,
func NewAPIService(ac authlib.AccessClient) *FolderAPIBuilder {
return &FolderAPIBuilder{
authorizer: newAuthorizer(ac),
authorizer: newMultiTenantAuthorizer(ac),
ignoreLegacy: true,
}
}
@@ -211,6 +216,11 @@ func (b *FolderAPIBuilder) PostProcessOpenAPI(oas *spec3.OpenAPI) (*spec3.OpenAP
return oas, nil
}
type authorizerParams struct {
user identity.Requester
evaluator accesscontrol.Evaluator
}
func (b *FolderAPIBuilder) GetAuthorizer() authorizer.Authorizer {
return b.authorizer
}