Skip to content

Commit 3f9e01b

Browse files
authored
Provisioning: Use AccessChecker to verify if request has access to the parsed object (#103646)
1 parent 5634ca4 commit 3f9e01b

File tree

6 files changed

+164
-40
lines changed

6 files changed

+164
-40
lines changed

pkg/registry/apis/provisioning/files.go

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ import (
1010
"k8s.io/apimachinery/pkg/runtime"
1111
"k8s.io/apiserver/pkg/registry/rest"
1212

13+
authlib "github.com/grafana/authlib/types"
1314
"github.com/grafana/grafana-app-sdk/logging"
15+
"github.com/grafana/grafana/pkg/apimachinery/identity"
1416
provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1"
1517
"github.com/grafana/grafana/pkg/registry/apis/provisioning/repository"
1618
"github.com/grafana/grafana/pkg/registry/apis/provisioning/resources"
@@ -24,12 +26,13 @@ const (
2426

2527
type filesConnector struct {
2628
getter RepoGetter
29+
access authlib.AccessChecker
2730
parsers resources.ParserFactory
2831
clients resources.ClientFactory
2932
}
3033

31-
func NewFilesConnector(getter RepoGetter, parsers resources.ParserFactory, clients resources.ClientFactory) *filesConnector {
32-
return &filesConnector{getter: getter, parsers: parsers, clients: clients}
34+
func NewFilesConnector(getter RepoGetter, parsers resources.ParserFactory, clients resources.ClientFactory, access authlib.AccessChecker) *filesConnector {
35+
return &filesConnector{getter: getter, parsers: parsers, clients: clients, access: access}
3336
}
3437

3538
func (*filesConnector) New() runtime.Object {
@@ -56,10 +59,10 @@ func (*filesConnector) NewConnectOptions() (runtime.Object, bool, string) {
5659
}
5760

5861
// TODO: document the synchronous write and delete on the API Spec
59-
func (s *filesConnector) Connect(ctx context.Context, name string, opts runtime.Object, responder rest.Responder) (http.Handler, error) {
62+
func (c *filesConnector) Connect(ctx context.Context, name string, opts runtime.Object, responder rest.Responder) (http.Handler, error) {
6063
logger := logging.FromContext(ctx).With("logger", "files-connector", "repository_name", name)
6164
ctx = logging.Context(ctx, logger)
62-
repo, err := s.getter.GetHealthyRepository(ctx, name)
65+
repo, err := c.getter.GetHealthyRepository(ctx, name)
6366
if err != nil {
6467
logger.Debug("failed to find repository", "error", err)
6568
return nil, err
@@ -70,12 +73,12 @@ func (s *filesConnector) Connect(ctx context.Context, name string, opts runtime.
7073
return nil, apierrors.NewBadRequest("repository does not support read-writing")
7174
}
7275

73-
parser, err := s.parsers.GetParser(ctx, readWriter)
76+
parser, err := c.parsers.GetParser(ctx, readWriter)
7477
if err != nil {
7578
return nil, fmt.Errorf("failed to get parser: %w", err)
7679
}
7780

78-
clients, err := s.clients.Clients(ctx, repo.Config().Namespace)
81+
clients, err := c.clients.Clients(ctx, repo.Config().Namespace)
7982
if err != nil {
8083
return nil, fmt.Errorf("failed to get clients: %w", err)
8184
}
@@ -85,7 +88,7 @@ func (s *filesConnector) Connect(ctx context.Context, name string, opts runtime.
8588
return nil, fmt.Errorf("failed to get folder client: %w", err)
8689
}
8790
folders := resources.NewFolderManager(readWriter, folderClient, resources.NewEmptyFolderTree())
88-
dualReadWriter := resources.NewDualReadWriter(readWriter, parser, folders)
91+
dualReadWriter := resources.NewDualReadWriter(readWriter, parser, folders, c.access)
8992

9093
return withTimeout(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
9194
query := r.URL.Query()
@@ -107,7 +110,7 @@ func (s *filesConnector) Connect(ctx context.Context, name string, opts runtime.
107110

108111
isDir := safepath.IsDir(filePath)
109112
if r.Method == http.MethodGet && isDir {
110-
files, err := s.listFolderFiles(ctx, filePath, ref, readWriter)
113+
files, err := c.listFolderFiles(ctx, filePath, ref, readWriter)
111114
if err != nil {
112115
responder.Error(err)
113116
return
@@ -200,7 +203,18 @@ func (s *filesConnector) Connect(ctx context.Context, name string, opts runtime.
200203
}
201204

202205
// listFolderFiles returns a list of files in a folder
203-
func (s *filesConnector) listFolderFiles(ctx context.Context, filePath string, ref string, readWriter repository.ReaderWriter) (*provisioning.FileList, error) {
206+
func (c *filesConnector) listFolderFiles(ctx context.Context, filePath string, ref string, readWriter repository.ReaderWriter) (*provisioning.FileList, error) {
207+
id, err := identity.GetRequester(ctx)
208+
if err != nil {
209+
return nil, fmt.Errorf("missing auth info in context")
210+
}
211+
212+
// TODO: replace with access check on the repo itself
213+
if !id.GetOrgRole().Includes(identity.RoleAdmin) {
214+
return nil, apierrors.NewForbidden(resources.DashboardResource.GroupResource(), "",
215+
fmt.Errorf("requires admin role"))
216+
}
217+
204218
// TODO: Implement folder navigation
205219
if len(filePath) > 0 {
206220
return nil, apierrors.NewBadRequest("folder navigation not yet supported")

pkg/registry/apis/provisioning/register.go

Lines changed: 10 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"k8s.io/kube-openapi/pkg/spec3"
2525
"k8s.io/kube-openapi/pkg/validation/spec"
2626

27+
authlib "github.com/grafana/authlib/types"
2728
"github.com/grafana/grafana-app-sdk/logging"
2829
dashboard "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v0alpha1"
2930
"github.com/grafana/grafana/pkg/apimachinery/identity"
@@ -98,6 +99,7 @@ type APIBuilder struct {
9899
unified resource.ResourceClient
99100
secrets secrets.Service
100101
client client.ProvisioningV0alpha1Interface
102+
access authlib.AccessChecker
101103
}
102104

103105
// NewAPIBuilder creates an API builder.
@@ -116,6 +118,7 @@ func NewAPIBuilder(
116118
legacyMigrator legacy.LegacyMigrator,
117119
storageStatus dualwrite.Service,
118120
secrets secrets.Service,
121+
access authlib.AccessChecker,
119122
) *APIBuilder {
120123
// HACK: Assume is only public if it is HTTPS
121124
isPublic := strings.HasPrefix(urlProvider(""), "https://")
@@ -140,6 +143,7 @@ func NewAPIBuilder(
140143
storageStatus: storageStatus,
141144
unified: unified,
142145
secrets: secrets,
146+
access: access,
143147
jobHistory: jobs.NewJobHistoryCache(),
144148
}
145149
}
@@ -156,6 +160,7 @@ func RegisterAPIService(
156160
client resource.ResourceClient, // implements resource.RepositoryClient
157161
configProvider apiserver.RestConfigProvider,
158162
ghFactory *github.Factory,
163+
access authlib.AccessClient,
159164
legacyMigrator legacy.LegacyMigrator,
160165
storageStatus dualwrite.Service,
161166
usageStatsService usagestats.Service,
@@ -180,7 +185,7 @@ func RegisterAPIService(
180185
filepath.Join(cfg.DataPath, "clone"), // where repositories are cloned (temporarialy for now)
181186
configProvider, ghFactory,
182187
legacyMigrator, storageStatus,
183-
secrets.NewSingleTenant(secretsSvc),
188+
secrets.NewSingleTenant(secretsSvc), access,
184189
)
185190
apiregistration.RegisterAPI(builder)
186191
usageStatsService.RegisterMetricsFunc(builder.collectProvisioningStats)
@@ -217,41 +222,22 @@ func (b *APIBuilder) GetAuthorizer() authorizer.Authorizer {
217222
case provisioning.RepositoryResourceInfo.GetName():
218223
// TODO: Support more fine-grained permissions than the basic roles. Especially on Enterprise.
219224
switch a.GetSubresource() {
220-
case "":
225+
case "", "test", "jobs":
221226
// Doing something with the repository itself.
222227
if id.GetOrgRole().Includes(identity.RoleAdmin) {
223228
return authorizer.DecisionAllow, "", nil
224229
}
225230
return authorizer.DecisionDeny, "admin role is required", nil
226231

227-
case "test", "export", "migrate":
228-
// Testing doesn't make sense for non-admins.
229-
// Exporting and migrating are potentially dangerous.
230-
if id.GetOrgRole().Includes(identity.RoleAdmin) {
231-
return authorizer.DecisionAllow, "", nil
232-
}
233-
return authorizer.DecisionDeny, "admin role is required", nil
234-
235232
case "webhook":
236233
// When the resource is a webhook, we'll deal with permissions manually by checking signatures or similar in the webhook handler.
237234
// The user in this context is usually an anonymous user, but may also be an authenticated synthetic check by the Grafana instance's operator as well.
238235
// For context on the anonymous user, check the authn/clients/provisioning.go file.
239236
return authorizer.DecisionAllow, "", nil
240237

241238
case "files":
242-
// Reading files is allowed for everyone, so as to allow code reviews and similar.
243-
// Writing files is only allowed fo Editor and higher.
244-
isViewer := id.GetOrgRole().Includes(identity.RoleViewer)
245-
isEditor := id.GetOrgRole().Includes(identity.RoleEditor)
246-
isReadOperation := a.GetVerb() == apiutils.VerbGet
247-
248-
if isEditor || (isViewer && isReadOperation) {
249-
return authorizer.DecisionAllow, "", nil
250-
} else if isReadOperation {
251-
return authorizer.DecisionDeny, "viewer role is required for reads", nil
252-
} else {
253-
return authorizer.DecisionDeny, "editor role is required for edits", nil
254-
}
239+
// Access to files is controlled by the AccessClient
240+
return authorizer.DecisionAllow, "", nil
255241

256242
case "render":
257243
// This is used to read a blob from unified storage, for GitHub PR comments.
@@ -362,7 +348,7 @@ func (b *APIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserver.APIGroupI
362348
storage[provisioning.RepositoryResourceInfo.StoragePath("test")] = &testConnector{
363349
getter: b,
364350
}
365-
storage[provisioning.RepositoryResourceInfo.StoragePath("files")] = NewFilesConnector(b, b.parsers, b.clients)
351+
storage[provisioning.RepositoryResourceInfo.StoragePath("files")] = NewFilesConnector(b, b.parsers, b.clients, b.access)
366352
storage[provisioning.RepositoryResourceInfo.StoragePath("resources")] = &listConnector{
367353
getter: b,
368354
lister: b.resourceLister,

pkg/registry/apis/provisioning/resources/dualwriter.go

Lines changed: 76 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,13 @@ import (
77
apierrors "k8s.io/apimachinery/pkg/api/errors"
88
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
99

10+
"github.com/google/uuid"
11+
12+
authlib "github.com/grafana/authlib/types"
1013
"github.com/grafana/grafana-app-sdk/logging"
1114
"github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1"
1215
"github.com/grafana/grafana/pkg/apimachinery/identity"
16+
"github.com/grafana/grafana/pkg/apimachinery/utils"
1317
provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1"
1418
"github.com/grafana/grafana/pkg/registry/apis/provisioning/repository"
1519
"github.com/grafana/grafana/pkg/registry/apis/provisioning/safepath"
@@ -21,10 +25,11 @@ type DualReadWriter struct {
2125
repo repository.ReaderWriter
2226
parser Parser
2327
folders *FolderManager
28+
access authlib.AccessChecker
2429
}
2530

26-
func NewDualReadWriter(repo repository.ReaderWriter, parser Parser, folders *FolderManager) *DualReadWriter {
27-
return &DualReadWriter{repo: repo, parser: parser, folders: folders}
31+
func NewDualReadWriter(repo repository.ReaderWriter, parser Parser, folders *FolderManager, access authlib.AccessChecker) *DualReadWriter {
32+
return &DualReadWriter{repo: repo, parser: parser, folders: folders, access: access}
2833
}
2934

3035
func (r *DualReadWriter) Read(ctx context.Context, path string, ref string) (*ParsedResource, error) {
@@ -43,6 +48,11 @@ func (r *DualReadWriter) Read(ctx context.Context, path string, ref string) (*Pa
4348
return nil, fmt.Errorf("parse file: %w", err)
4449
}
4550

51+
// Authorize the parsed resource
52+
if err = r.authorize(ctx, parsed, utils.VerbGet); err != nil {
53+
return nil, err
54+
}
55+
4656
// Fail as we use the dry run for this response and it's not about updating the resource
4757
if err := parsed.DryRun(ctx); err != nil {
4858
return nil, fmt.Errorf("run dry run: %w", err)
@@ -73,6 +83,10 @@ func (r *DualReadWriter) Delete(ctx context.Context, path string, ref string, me
7383
return nil, fmt.Errorf("parse file: %w", err)
7484
}
7585

86+
if err = r.authorize(ctx, parsed, utils.VerbDelete); err != nil {
87+
return nil, err
88+
}
89+
7690
parsed.Action = provisioning.ResourceActionDelete
7791
err = r.repo.Delete(ctx, path, ref, message)
7892
if err != nil {
@@ -112,6 +126,10 @@ func (r *DualReadWriter) CreateFolder(ctx context.Context, path string, ref stri
112126
return nil, fmt.Errorf("not a folder path")
113127
}
114128

129+
if err := r.authorizeCreateFolder(ctx, path); err != nil {
130+
return nil, err
131+
}
132+
115133
// Now actually create the folder
116134
if err := r.repo.Create(ctx, path, ref, nil, message); err != nil {
117135
return nil, fmt.Errorf("failed to create folder: %w", err)
@@ -167,6 +185,10 @@ func (r *DualReadWriter) CreateResource(ctx context.Context, path string, ref st
167185
return nil, fmt.Errorf("parse file: %w", err)
168186
}
169187

188+
if err = r.authorize(ctx, parsed, utils.VerbCreate); err != nil {
189+
return nil, err
190+
}
191+
170192
data, err = parsed.ToSaveBytes()
171193
if err != nil {
172194
return nil, err
@@ -218,6 +240,10 @@ func (r *DualReadWriter) UpdateResource(ctx context.Context, path string, ref st
218240
return nil, fmt.Errorf("parse file: %w", err)
219241
}
220242

243+
if err = r.authorize(ctx, parsed, utils.VerbUpdate); err != nil {
244+
return nil, err
245+
}
246+
221247
data, err = parsed.ToSaveBytes()
222248
if err != nil {
223249
return nil, err
@@ -250,3 +276,51 @@ func (r *DualReadWriter) UpdateResource(ctx context.Context, path string, ref st
250276

251277
return parsed, nil
252278
}
279+
280+
func (r *DualReadWriter) authorize(ctx context.Context, parsed *ParsedResource, verb string) error {
281+
auth, ok := authlib.AuthInfoFrom(ctx)
282+
if !ok {
283+
return fmt.Errorf("missing auth info in context")
284+
}
285+
rsp, err := r.access.Check(ctx, auth, authlib.CheckRequest{
286+
Group: parsed.GVR.Group,
287+
Resource: parsed.GVR.Resource,
288+
Namespace: parsed.Obj.GetNamespace(),
289+
Name: parsed.Obj.GetName(),
290+
Folder: parsed.Meta.GetFolder(),
291+
Verb: verb,
292+
})
293+
if err != nil {
294+
return err
295+
}
296+
if !rsp.Allowed {
297+
return apierrors.NewForbidden(parsed.GVR.GroupResource(), parsed.Obj.GetName(),
298+
fmt.Errorf("no access to see embedded file"))
299+
}
300+
return nil
301+
}
302+
303+
func (r *DualReadWriter) authorizeCreateFolder(ctx context.Context, _ string) error {
304+
auth, ok := authlib.AuthInfoFrom(ctx)
305+
if !ok {
306+
return fmt.Errorf("missing auth info in context")
307+
}
308+
rsp, err := r.access.Check(ctx, auth, authlib.CheckRequest{
309+
Group: FolderResource.Group,
310+
Resource: FolderResource.Resource,
311+
Namespace: r.repo.Config().GetNamespace(),
312+
Verb: utils.VerbCreate,
313+
314+
// TODO: Currently this checks if you can create a new folder in root
315+
// Ideally we should check the path and use the explicit parent and new id
316+
Name: "f" + uuid.NewString(),
317+
})
318+
if err != nil {
319+
return err
320+
}
321+
if !rsp.Allowed {
322+
return apierrors.NewForbidden(FolderResource.GroupResource(), "",
323+
fmt.Errorf("unable to create folder resource"))
324+
}
325+
return nil
326+
}

pkg/registry/apis/provisioning/resources/folders.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
1111
"k8s.io/client-go/dynamic"
1212

13+
"github.com/grafana/grafana/pkg/apimachinery/identity"
1314
"github.com/grafana/grafana/pkg/apimachinery/utils"
1415
"github.com/grafana/grafana/pkg/apis/folder/v0alpha1"
1516
"github.com/grafana/grafana/pkg/registry/apis/provisioning/repository"
@@ -105,6 +106,12 @@ func (fm *FolderManager) EnsureFolderExists(ctx context.Context, folder Folder,
105106
return fmt.Errorf("failed to check if folder exists: %w", err)
106107
}
107108

109+
// Always use the provisioning identity when writing
110+
ctx, _, err = identity.WithProvisioningIdentity(ctx, cfg.GetNamespace())
111+
if err != nil {
112+
return fmt.Errorf("unable to use provisioning identity %w", err)
113+
}
114+
108115
obj = &unstructured.Unstructured{
109116
Object: map[string]interface{}{
110117
"spec": map[string]any{

pkg/registry/apis/provisioning/resources/parser.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,12 @@ func (f *ParsedResource) DryRun(ctx context.Context) error {
217217
return fmt.Errorf("no client configured")
218218
}
219219

220-
var err error
220+
// Use the same identity that would eventually write the resource (via Run)
221+
ctx, _, err := identity.WithProvisioningIdentity(ctx, f.Obj.GetNamespace())
222+
if err != nil {
223+
return err
224+
}
225+
221226
// FIXME: shouldn't we check for the specific error?
222227
// Dry run CREATE or UPDATE
223228
f.Existing, _ = f.Client.Get(ctx, f.Obj.GetName(), metav1.GetOptions{})

0 commit comments

Comments
 (0)