From 08a6f31733d0f8b42b79c77645e7231fba318eb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Jim=C3=A9nez=20S=C3=A1nchez?= Date: Mon, 15 Dec 2025 16:39:07 +0100 Subject: [PATCH] Provisioning: allow editors to POST jobs in provisioning API (#115351) fix: allow editors to POST jobs in provisioning API Editors should be able to post jobs in the 'jobs' endpoint for syncing repositories. This aligns with the requirement that syncing a repository requires editor privileges. - Separated 'jobs' subresource authorization from repository/test - Allow both admins and editors to POST jobs - Added integration tests to verify permissions Fixes authorization bug where editors were incorrectly denied access. --- pkg/registry/apis/provisioning/register.go | 197 ++++++++++-------- .../apis/provisioning/repository_test.go | 83 ++++++++ 2 files changed, 198 insertions(+), 82 deletions(-) diff --git a/pkg/registry/apis/provisioning/register.go b/pkg/registry/apis/provisioning/register.go index d18fc1156a8..cbf50a027f4 100644 --- a/pkg/registry/apis/provisioning/register.go +++ b/pkg/registry/apis/provisioning/register.go @@ -328,91 +328,124 @@ func (b *APIBuilder) GetAuthorizer() authorizer.Authorizer { return authorizer.DecisionDeny, "failed to find requester", err } - // Different routes may need different permissions. - // * Reading and modifying a repository's configuration requires administrator privileges. - // * Reading a repository's limited configuration (/stats & /settings) requires viewer privileges. - // * Reading a repository's files requires viewer privileges. - // * Reading a repository's refs requires viewer privileges. - // * Editing a repository's files requires editor privileges. - // * Syncing a repository requires editor privileges. - // * Exporting a repository requires administrator privileges. - // * Migrating a repository requires administrator privileges. - // * Testing a repository configuration requires administrator privileges. - // * Viewing a repository's history requires editor privileges. - - switch a.GetResource() { - case provisioning.RepositoryResourceInfo.GetName(): - // TODO: Support more fine-grained permissions than the basic roles. Especially on Enterprise. - switch a.GetSubresource() { - case "", "test", "jobs": - // Doing something with the repository itself. - if id.GetOrgRole().Includes(identity.RoleAdmin) { - return authorizer.DecisionAllow, "", nil - } - return authorizer.DecisionDeny, "admin role is required", nil - - case "refs": - // This is strictly a read operation. It is handy on the frontend for viewers. - if id.GetOrgRole().Includes(identity.RoleViewer) { - return authorizer.DecisionAllow, "", nil - } - return authorizer.DecisionDeny, "viewer role is required", nil - case "files": - // Access to files is controlled by the AccessClient - return authorizer.DecisionAllow, "", nil - - case "resources", "sync", "history": - // These are strictly read operations. - // Sync can also be somewhat destructive, but it's expected to be fine to import changes. - if id.GetOrgRole().Includes(identity.RoleEditor) { - return authorizer.DecisionAllow, "", nil - } else { - return authorizer.DecisionDeny, "editor role is required", nil - } - case "status": - if id.GetOrgRole().Includes(identity.RoleViewer) && a.GetVerb() == apiutils.VerbGet { - return authorizer.DecisionAllow, "", nil - } - return authorizer.DecisionDeny, "users cannot update the status of a repository", nil - default: - if id.GetIsGrafanaAdmin() { - return authorizer.DecisionAllow, "", nil - } - return authorizer.DecisionDeny, "unmapped subresource defaults to no access", nil - } - - case "stats": - // This can leak information one shouldn't necessarily have access to. - if id.GetOrgRole().Includes(identity.RoleAdmin) { - return authorizer.DecisionAllow, "", nil - } - return authorizer.DecisionDeny, "admin role is required", nil - - case "settings": - // This is strictly a read operation. It is handy on the frontend for viewers. - if id.GetOrgRole().Includes(identity.RoleViewer) { - return authorizer.DecisionAllow, "", nil - } - return authorizer.DecisionDeny, "viewer role is required", nil - - case provisioning.JobResourceInfo.GetName(), - provisioning.HistoricJobResourceInfo.GetName(): - // Jobs are shown on the configuration page. - if id.GetOrgRole().Includes(identity.RoleAdmin) { - return authorizer.DecisionAllow, "", nil - } - return authorizer.DecisionDeny, "admin role is required", nil - - default: - // We haven't bothered with this kind yet. - if id.GetIsGrafanaAdmin() { - return authorizer.DecisionAllow, "", nil - } - return authorizer.DecisionDeny, "unmapped kind defaults to no access", nil - } + return b.authorizeResource(ctx, a, id) }) } +// authorizeResource handles authorization for different resources. +// Different routes may need different permissions. +// * Reading and modifying a repository's configuration requires administrator privileges. +// * Reading a repository's limited configuration (/stats & /settings) requires viewer privileges. +// * Reading a repository's files requires viewer privileges. +// * Reading a repository's refs requires viewer privileges. +// * Editing a repository's files requires editor privileges. +// * Syncing a repository requires editor privileges. +// * Exporting a repository requires administrator privileges. +// * Migrating a repository requires administrator privileges. +// * Testing a repository configuration requires administrator privileges. +// * Viewing a repository's history requires editor privileges. +func (b *APIBuilder) authorizeResource(ctx context.Context, a authorizer.Attributes, id identity.Requester) (authorizer.Decision, string, error) { + switch a.GetResource() { + case provisioning.RepositoryResourceInfo.GetName(): + return b.authorizeRepositorySubresource(a, id) + case "stats": + return b.authorizeStats(id) + case "settings": + return b.authorizeSettings(id) + case provisioning.JobResourceInfo.GetName(), provisioning.HistoricJobResourceInfo.GetName(): + return b.authorizeJobs(id) + default: + return b.authorizeDefault(id) + } +} + +// authorizeRepositorySubresource handles authorization for repository subresources. +func (b *APIBuilder) authorizeRepositorySubresource(a authorizer.Attributes, id identity.Requester) (authorizer.Decision, string, error) { + // TODO: Support more fine-grained permissions than the basic roles. Especially on Enterprise. + switch a.GetSubresource() { + case "", "test": + // Doing something with the repository itself. + if id.GetOrgRole().Includes(identity.RoleAdmin) { + return authorizer.DecisionAllow, "", nil + } + return authorizer.DecisionDeny, "admin role is required", nil + + case "jobs": + // Posting jobs requires editor privileges (for syncing). + if id.GetOrgRole().Includes(identity.RoleAdmin) || id.GetOrgRole().Includes(identity.RoleEditor) { + return authorizer.DecisionAllow, "", nil + } + return authorizer.DecisionDeny, "editor role is required", nil + + case "refs": + // This is strictly a read operation. It is handy on the frontend for viewers. + if id.GetOrgRole().Includes(identity.RoleViewer) { + return authorizer.DecisionAllow, "", nil + } + return authorizer.DecisionDeny, "viewer role is required", nil + + case "files": + // Access to files is controlled by the AccessClient + return authorizer.DecisionAllow, "", nil + + case "resources", "sync", "history": + // These are strictly read operations. + // Sync can also be somewhat destructive, but it's expected to be fine to import changes. + if id.GetOrgRole().Includes(identity.RoleEditor) { + return authorizer.DecisionAllow, "", nil + } + return authorizer.DecisionDeny, "editor role is required", nil + + case "status": + if id.GetOrgRole().Includes(identity.RoleViewer) && a.GetVerb() == apiutils.VerbGet { + return authorizer.DecisionAllow, "", nil + } + return authorizer.DecisionDeny, "users cannot update the status of a repository", nil + + default: + if id.GetIsGrafanaAdmin() { + return authorizer.DecisionAllow, "", nil + } + return authorizer.DecisionDeny, "unmapped subresource defaults to no access", nil + } +} + +// authorizeStats handles authorization for stats resource. +func (b *APIBuilder) authorizeStats(id identity.Requester) (authorizer.Decision, string, error) { + // This can leak information one shouldn't necessarily have access to. + if id.GetOrgRole().Includes(identity.RoleAdmin) { + return authorizer.DecisionAllow, "", nil + } + return authorizer.DecisionDeny, "admin role is required", nil +} + +// authorizeSettings handles authorization for settings resource. +func (b *APIBuilder) authorizeSettings(id identity.Requester) (authorizer.Decision, string, error) { + // This is strictly a read operation. It is handy on the frontend for viewers. + if id.GetOrgRole().Includes(identity.RoleViewer) { + return authorizer.DecisionAllow, "", nil + } + return authorizer.DecisionDeny, "viewer role is required", nil +} + +// authorizeJobs handles authorization for job resources. +func (b *APIBuilder) authorizeJobs(id identity.Requester) (authorizer.Decision, string, error) { + // Jobs are shown on the configuration page. + if id.GetOrgRole().Includes(identity.RoleAdmin) { + return authorizer.DecisionAllow, "", nil + } + return authorizer.DecisionDeny, "admin role is required", nil +} + +// authorizeDefault handles authorization for unmapped resources. +func (b *APIBuilder) authorizeDefault(id identity.Requester) (authorizer.Decision, string, error) { + // We haven't bothered with this kind yet. + if id.GetIsGrafanaAdmin() { + return authorizer.DecisionAllow, "", nil + } + return authorizer.DecisionDeny, "unmapped kind defaults to no access", nil +} + func (b *APIBuilder) GetGroupVersion() schema.GroupVersion { return provisioning.SchemeGroupVersion } diff --git a/pkg/tests/apis/provisioning/repository_test.go b/pkg/tests/apis/provisioning/repository_test.go index 37796da7b3d..877ed0d5f98 100644 --- a/pkg/tests/apis/provisioning/repository_test.go +++ b/pkg/tests/apis/provisioning/repository_test.go @@ -867,3 +867,86 @@ func TestIntegrationProvisioning_DeleteRepositoryAndReleaseResources(t *testing. } }, time.Second*20, time.Millisecond*10, "Expected folders to be released") } + +func TestIntegrationProvisioning_JobPermissions(t *testing.T) { + testutil.SkipIntegrationTestInShortMode(t) + + helper := runGrafana(t) + ctx := context.Background() + + const repo = "job-permissions-test" + testRepo := TestRepo{ + Name: repo, + Target: "folder", + Copies: map[string]string{}, // No files needed for this test + ExpectedDashboards: 0, + ExpectedFolders: 1, // Repository creates a folder + } + helper.CreateRepo(t, testRepo) + + jobSpec := provisioning.JobSpec{ + Action: provisioning.JobActionPull, + Pull: &provisioning.SyncJobOptions{}, + } + body := asJSON(jobSpec) + + t.Run("editor can POST jobs", func(t *testing.T) { + var statusCode int + result := helper.EditorREST.Post(). + Namespace("default"). + Resource("repositories"). + Name(repo). + SubResource("jobs"). + Body(body). + SetHeader("Content-Type", "application/json"). + Do(ctx).StatusCode(&statusCode) + + require.NoError(t, result.Error(), "editor should be able to POST jobs") + require.Equal(t, http.StatusAccepted, statusCode, "should return 202 Accepted") + + // Verify the job was created + obj, err := result.Get() + require.NoError(t, err, "should get job object") + unstruct, ok := obj.(*unstructured.Unstructured) + require.True(t, ok, "expecting unstructured object") + require.NotEmpty(t, unstruct.GetName(), "job should have a name") + }) + + t.Run("viewer cannot POST jobs", func(t *testing.T) { + var statusCode int + result := helper.ViewerREST.Post(). + Namespace("default"). + Resource("repositories"). + Name(repo). + SubResource("jobs"). + Body(body). + SetHeader("Content-Type", "application/json"). + Do(ctx).StatusCode(&statusCode) + + require.Error(t, result.Error(), "viewer should not be able to POST jobs") + require.Equal(t, http.StatusForbidden, statusCode, "should return 403 Forbidden") + require.True(t, apierrors.IsForbidden(result.Error()), "error should be forbidden") + }) + + t.Run("admin can POST jobs", func(t *testing.T) { + var statusCode int + result := helper.AdminREST.Post(). + Namespace("default"). + Resource("repositories"). + Name(repo). + SubResource("jobs"). + Body(body). + SetHeader("Content-Type", "application/json"). + Do(ctx).StatusCode(&statusCode) + + // Job might already exist from previous test, which is acceptable + if apierrors.IsAlreadyExists(result.Error()) { + // Wait for the existing job to complete + helper.AwaitJobs(t, repo) + return + } + + require.NoError(t, result.Error(), "admin should be able to POST jobs") + require.Equal(t, http.StatusAccepted, statusCode, "should return 202 Accepted") + }) +}