From 482bb6a2fb4d9fd310faf8c12b1da5793df8638f Mon Sep 17 00:00:00 2001 From: mohammad-hamid Date: Tue, 16 Dec 2025 11:43:03 -0500 Subject: [PATCH] AuthZ: Redirect legacy resource permissions handler to k8s (part II) (#114356) * move restconfig to options * Add K8s API redirects for write operations * Revert restConfigProvider changes to receivers, service accounts, and teams * discard changing team permissions * lint * cleanup * trigger build * address feedback * improve test coverage * lint * trigger build * refactor --- .../accesscontrol/resourcepermissions/api.go | 72 +++++- .../resourcepermissions/api_adapter.go | 231 ++++++++++++++++++ .../resourcepermissions/api_adapter_test.go | 200 +++++++++++++++ 3 files changed, 499 insertions(+), 4 deletions(-) create mode 100644 pkg/services/accesscontrol/resourcepermissions/api_adapter_test.go diff --git a/pkg/services/accesscontrol/resourcepermissions/api.go b/pkg/services/accesscontrol/resourcepermissions/api.go index 981a99f5189..3c6d1da2038 100644 --- a/pkg/services/accesscontrol/resourcepermissions/api.go +++ b/pkg/services/accesscontrol/resourcepermissions/api.go @@ -33,6 +33,7 @@ type api struct { permissions []string features featuremgmt.FeatureToggles restConfigProvider apiserver.RestConfigProvider + logger log.Logger } func newApi(cfg *setting.Cfg, ac accesscontrol.AccessControl, router routing.RouteRegister, manager *Service, features featuremgmt.FeatureToggles, restConfigProvider apiserver.RestConfigProvider) *api { @@ -41,7 +42,23 @@ func newApi(cfg *setting.Cfg, ac accesscontrol.AccessControl, router routing.Rou for i := len(manager.permissions) - 1; i >= 0; i-- { permissions = append(permissions, manager.permissions[i]) } - return &api{cfg, ac, router, manager, permissions, features, restConfigProvider} + return &api{ + cfg: cfg, + ac: ac, + router: router, + service: manager, + permissions: permissions, + features: features, + restConfigProvider: restConfigProvider, + logger: log.New("resource-permissions-api"), + } +} + +// shouldUseK8sAPIs returns true if both feature flags for K8s API redirect are enabled +func (a *api) shouldUseK8sAPIs() bool { + //nolint:staticcheck // not yet migrated to OpenFeature + return a.features.IsEnabledGlobally(featuremgmt.FlagKubernetesAuthZHandlerRedirect) && + a.features.IsEnabledGlobally(featuremgmt.FlagKubernetesAuthzResourcePermissionApis) } func (a *api) registerEndpoints() { @@ -189,11 +206,10 @@ func (a *api) getPermissions(c *contextmodel.ReqContext) response.Response { return response.JSON(http.StatusOK, k8sPermissions) } span.RecordError(err) - logger := log.New("resource-permissions-api") if errors.Is(err, ErrRestConfigNotAvailable) { - logger.Debug("k8s API not available for resource permissions, falling back to legacy", "error", err, "resourceID", resourceID, "resource", a.service.options.Resource) + a.logger.Debug("k8s API not available for resource permissions, falling back to legacy", "error", err, "resourceID", resourceID, "resource", a.service.options.Resource) } else { - logger.Warn("Failed to get resource permissions from k8s API, falling back to legacy", "error", err, "resourceID", resourceID, "resource", a.service.options.Resource) + a.logger.Warn("Failed to get resource permissions from k8s API, falling back to legacy", "error", err, "resourceID", resourceID, "resource", a.service.options.Resource) } } @@ -304,6 +320,19 @@ func (a *api) setUserPermission(c *contextmodel.ReqContext) response.Response { return response.Error(http.StatusBadRequest, "bad request data", err) } + if a.shouldUseK8sAPIs() { + err := a.setUserPermissionToK8s(c.Req.Context(), c.Namespace, resourceID, userID, cmd.Permission) + if err == nil { + return permissionSetResponse(cmd) + } + span.RecordError(err) + if errors.Is(err, ErrRestConfigNotAvailable) { + a.logger.Debug("k8s API not available for resource permissions, falling back to legacy", "error", err, "resourceID", resourceID, "resource", a.service.options.Resource) + } else { + a.logger.Warn("Failed to set user permission in k8s API, falling back to legacy", "error", err, "resourceID", resourceID, "resource", a.service.options.Resource) + } + } + _, err = a.service.SetUserPermission(c.Req.Context(), c.GetOrgID(), accesscontrol.User{ID: userID}, resourceID, cmd.Permission) if err != nil { return response.Err(err) @@ -361,6 +390,19 @@ func (a *api) setTeamPermission(c *contextmodel.ReqContext) response.Response { return response.Error(http.StatusBadRequest, "bad request data", err) } + if a.shouldUseK8sAPIs() { + err := a.setTeamPermissionToK8s(c.Req.Context(), c.Namespace, resourceID, teamID, cmd.Permission) + if err == nil { + return permissionSetResponse(cmd) + } + span.RecordError(err) + if errors.Is(err, ErrRestConfigNotAvailable) { + a.logger.Debug("k8s API not available for resource permissions, falling back to legacy", "error", err, "resourceID", resourceID, "resource", a.service.options.Resource) + } else { + a.logger.Warn("Failed to set team permission in k8s API, falling back to legacy", "error", err, "resourceID", resourceID, "resource", a.service.options.Resource) + } + } + _, err = a.service.SetTeamPermission(c.Req.Context(), c.GetOrgID(), teamID, resourceID, cmd.Permission) if err != nil { return response.Err(err) @@ -415,6 +457,17 @@ func (a *api) setBuiltinRolePermission(c *contextmodel.ReqContext) response.Resp return response.Error(http.StatusBadRequest, "bad request data", err) } + if a.shouldUseK8sAPIs() { + err := a.setBuiltInRolePermissionToK8s(c.Req.Context(), c.Namespace, resourceID, builtInRole, cmd.Permission) + if err == nil { + return permissionSetResponse(cmd) + } + span.RecordError(err) + if errors.Is(err, ErrRestConfigNotAvailable) { + a.logger.Debug("k8s API not available for resource permissions, falling back to legacy", "error", err, "resourceID", resourceID, "resource", a.service.options.Resource) + } + } + _, err := a.service.SetBuiltInRolePermission(c.Req.Context(), c.GetOrgID(), builtInRole, resourceID, cmd.Permission) if err != nil { return response.Err(err) @@ -463,6 +516,17 @@ func (a *api) setPermissions(c *contextmodel.ReqContext) response.Response { return response.Error(http.StatusBadRequest, "Bad request data: "+err.Error(), err) } + if a.shouldUseK8sAPIs() { + err := a.setResourcePermissionsToK8s(c.Req.Context(), c.Namespace, resourceID, cmd.Permissions) + if err == nil { + return response.Success("Permissions updated") + } + span.RecordError(err) + if errors.Is(err, ErrRestConfigNotAvailable) { + a.logger.Debug("k8s API not available for resource permissions, falling back to legacy", "error", err, "resourceID", resourceID, "resource", a.service.options.Resource) + } + } + _, err := a.service.SetPermissions(ctx, c.GetOrgID(), resourceID, cmd.Permissions...) if err != nil { return response.Err(err) diff --git a/pkg/services/accesscontrol/resourcepermissions/api_adapter.go b/pkg/services/accesscontrol/resourcepermissions/api_adapter.go index 868ae1a32b5..578111368e5 100644 --- a/pkg/services/accesscontrol/resourcepermissions/api_adapter.go +++ b/pkg/services/accesscontrol/resourcepermissions/api_adapter.go @@ -13,9 +13,12 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/dynamic" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + iamv0 "github.com/grafana/grafana/apps/iam/pkg/apis/iam/v0alpha1" "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/team" "github.com/grafana/grafana/pkg/services/user" ) @@ -162,3 +165,231 @@ func getMapKeys(m map[string][]string) []string { func (a *api) buildResourcePermissionName(resourceID string) string { return fmt.Sprintf("%s-%s-%s", a.getAPIGroup(), a.service.options.Resource, resourceID) } + +// Write operations + +func (a *api) setResourcePermissionsToK8s(ctx context.Context, namespace string, resourceID string, permissions []accesscontrol.SetResourcePermissionCommand) error { + dynamicClient, err := a.getDynamicClient(ctx) + if err != nil { + return err + } + + resourcePermName := a.buildResourcePermissionName(resourceID) + resourcePermResource := dynamicClient.Resource(iamv0.ResourcePermissionInfo.GroupVersionResource()).Namespace(namespace) + + _, existingResourceVersion, err := a.getExistingResourcePermission(ctx, resourcePermResource, resourcePermName) + if err != nil { + return err + } + + k8sPermissions := make([]iamv0.ResourcePermissionspecPermission, 0, len(permissions)) + for _, perm := range permissions { + if perm.Permission == "" { + continue + } + + kind := a.getPermissionKind(perm) + name, err := a.getPermissionName(ctx, perm) + if err != nil { + return fmt.Errorf("failed to get permission name: %w", err) + } + + k8sPermissions = append(k8sPermissions, iamv0.ResourcePermissionspecPermission{ + Kind: iamv0.ResourcePermissionSpecPermissionKind(kind), + Name: name, + Verb: cases.Lower(language.Und).String(perm.Permission), + }) + } + + if len(k8sPermissions) == 0 { + if existingResourceVersion != "" { + err = resourcePermResource.Delete(ctx, resourcePermName, metav1.DeleteOptions{}) + if err != nil && !k8serrors.IsNotFound(err) { + return fmt.Errorf("failed to delete resource permission in k8s: %w", err) + } + } + return nil + } + + resourcePerm := &iamv0.ResourcePermission{ + TypeMeta: metav1.TypeMeta{ + APIVersion: iamv0.ResourcePermissionInfo.GroupVersion().String(), + Kind: iamv0.ResourcePermissionInfo.TypeMeta().Kind, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: resourcePermName, + Namespace: namespace, + ResourceVersion: existingResourceVersion, + }, + Spec: iamv0.ResourcePermissionSpec{ + Resource: iamv0.ResourcePermissionspecResource{ + ApiGroup: a.getAPIGroup(), + Resource: a.service.options.Resource, + Name: resourceID, + }, + Permissions: k8sPermissions, + }, + } + + return a.createOrUpdateResourcePermission(ctx, resourcePermResource, resourcePerm, existingResourceVersion != "") +} + +func (a *api) setUserPermissionToK8s(ctx context.Context, namespace string, resourceID string, userID int64, permission string) error { + userDetails, err := a.service.userService.GetByID(ctx, &user.GetUserByIDQuery{ID: userID}) + if err != nil { + return fmt.Errorf("failed to get user details: %w", err) + } + + return a.setSinglePermissionToK8s(ctx, namespace, resourceID, string(iamv0.ResourcePermissionSpecPermissionKindUser), userDetails.UID, permission) +} + +func (a *api) setTeamPermissionToK8s(ctx context.Context, namespace string, resourceID string, teamID int64, permission string) error { + teamDetails, err := a.service.teamService.GetTeamByID(ctx, &team.GetTeamByIDQuery{ID: teamID}) + if err != nil { + return fmt.Errorf("failed to get team details: %w", err) + } + + return a.setSinglePermissionToK8s(ctx, namespace, resourceID, string(iamv0.ResourcePermissionSpecPermissionKindTeam), teamDetails.UID, permission) +} + +func (a *api) setBuiltInRolePermissionToK8s(ctx context.Context, namespace string, resourceID string, builtInRole string, permission string) error { + return a.setSinglePermissionToK8s(ctx, namespace, resourceID, string(iamv0.ResourcePermissionSpecPermissionKindBasicRole), builtInRole, permission) +} + +func (a *api) setSinglePermissionToK8s(ctx context.Context, namespace string, resourceID string, kind string, name string, permission string) error { + dynamicClient, err := a.getDynamicClient(ctx) + if err != nil { + return err + } + + resourcePermName := a.buildResourcePermissionName(resourceID) + resourcePermResource := dynamicClient.Resource(iamv0.ResourcePermissionInfo.GroupVersionResource()).Namespace(namespace) + + existingResourcePerm, existingResourceVersion, err := a.getExistingResourcePermission(ctx, resourcePermResource, resourcePermName) + if err != nil { + return err + } + + newPermissions := make([]iamv0.ResourcePermissionspecPermission, 0) + for _, perm := range existingResourcePerm.Spec.Permissions { + if string(perm.Kind) == kind && perm.Name == name { + continue + } + newPermissions = append(newPermissions, perm) + } + + if permission != "" { + newPermissions = append(newPermissions, iamv0.ResourcePermissionspecPermission{ + Kind: iamv0.ResourcePermissionSpecPermissionKind(kind), + Name: name, + Verb: cases.Lower(language.Und).String(permission), + }) + } + + if len(newPermissions) == 0 { + if existingResourceVersion != "" { + err = resourcePermResource.Delete(ctx, resourcePermName, metav1.DeleteOptions{}) + if err != nil && !k8serrors.IsNotFound(err) { + return fmt.Errorf("failed to delete resource permission in k8s: %w", err) + } + } + return nil + } + + resourcePerm := &iamv0.ResourcePermission{ + TypeMeta: metav1.TypeMeta{ + APIVersion: iamv0.ResourcePermissionInfo.GroupVersion().String(), + Kind: iamv0.ResourcePermissionInfo.TypeMeta().Kind, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: resourcePermName, + Namespace: namespace, + ResourceVersion: existingResourceVersion, + }, + Spec: iamv0.ResourcePermissionSpec{ + Resource: iamv0.ResourcePermissionspecResource{ + ApiGroup: a.getAPIGroup(), + Resource: a.service.options.Resource, + Name: resourceID, + }, + Permissions: newPermissions, + }, + } + + return a.createOrUpdateResourcePermission(ctx, resourcePermResource, resourcePerm, existingResourceVersion != "") +} + +func (a *api) getPermissionKind(perm accesscontrol.SetResourcePermissionCommand) string { + if perm.UserID != 0 { + return string(iamv0.ResourcePermissionSpecPermissionKindUser) + } + if perm.TeamID != 0 { + return string(iamv0.ResourcePermissionSpecPermissionKindTeam) + } + if perm.BuiltinRole != "" { + return string(iamv0.ResourcePermissionSpecPermissionKindBasicRole) + } + return "" +} + +func (a *api) getExistingResourcePermission(ctx context.Context, resourcePermResource dynamic.ResourceInterface, resourcePermName string) (*iamv0.ResourcePermission, string, error) { + unstructuredObj, err := resourcePermResource.Get(ctx, resourcePermName, metav1.GetOptions{}) + if err != nil { + if k8serrors.IsNotFound(err) { + return &iamv0.ResourcePermission{}, "", nil + } + return nil, "", fmt.Errorf("failed to get existing resource permission: %w", err) + } + + var resourcePerm iamv0.ResourcePermission + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(unstructuredObj.Object, &resourcePerm); err != nil { + return nil, "", fmt.Errorf("failed to convert existing resource permission: %w", err) + } + + return &resourcePerm, unstructuredObj.GetResourceVersion(), nil +} + +func (a *api) createOrUpdateResourcePermission(ctx context.Context, resourcePermResource dynamic.ResourceInterface, resourcePerm *iamv0.ResourcePermission, isUpdate bool) error { + unstructuredObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(resourcePerm) + if err != nil { + return fmt.Errorf("failed to convert resource permission to unstructured: %w", err) + } + unstructuredPerm := &unstructured.Unstructured{Object: unstructuredObj} + + if isUpdate { + _, err = resourcePermResource.Update(ctx, unstructuredPerm, metav1.UpdateOptions{}) + if err != nil { + return fmt.Errorf("failed to update resource permission in k8s: %w", err) + } + } else { + _, err = resourcePermResource.Create(ctx, unstructuredPerm, metav1.CreateOptions{}) + if err != nil { + return fmt.Errorf("failed to create resource permission in k8s: %w", err) + } + } + + return nil +} + +func (a *api) getPermissionName(ctx context.Context, perm accesscontrol.SetResourcePermissionCommand) (string, error) { + if perm.UserID != 0 { + userDetails, err := a.service.userService.GetByID(ctx, &user.GetUserByIDQuery{ID: perm.UserID}) + if err != nil { + return "", fmt.Errorf("failed to get user details for user ID %d: %w", perm.UserID, err) + } + return userDetails.UID, nil + } + if perm.TeamID != 0 { + teamDetails, err := a.service.teamService.GetTeamByID(ctx, &team.GetTeamByIDQuery{ + ID: perm.TeamID, + }) + if err != nil { + return "", fmt.Errorf("failed to get team details for team ID %d: %w", perm.TeamID, err) + } + return teamDetails.UID, nil + } + if perm.BuiltinRole != "" { + return perm.BuiltinRole, nil + } + return "", fmt.Errorf("no valid permission subject found") +} diff --git a/pkg/services/accesscontrol/resourcepermissions/api_adapter_test.go b/pkg/services/accesscontrol/resourcepermissions/api_adapter_test.go new file mode 100644 index 00000000000..ca0ca214c9a --- /dev/null +++ b/pkg/services/accesscontrol/resourcepermissions/api_adapter_test.go @@ -0,0 +1,200 @@ +package resourcepermissions + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + + iamv0 "github.com/grafana/grafana/apps/iam/pkg/apis/iam/v0alpha1" + "github.com/grafana/grafana/pkg/services/accesscontrol" +) + +// TestGetPermissionKind tests the permission kind mapping logic +func TestGetPermissionKind(t *testing.T) { + api := &api{ + service: &Service{ + options: Options{ + Resource: "dashboards", + ResourceAttribute: "uid", + }, + }, + } + + tests := []struct { + name string + perm accesscontrol.SetResourcePermissionCommand + expected string + }{ + { + name: "user permission", + perm: accesscontrol.SetResourcePermissionCommand{UserID: 123}, + expected: string(iamv0.ResourcePermissionSpecPermissionKindUser), + }, + { + name: "team permission", + perm: accesscontrol.SetResourcePermissionCommand{TeamID: 456}, + expected: string(iamv0.ResourcePermissionSpecPermissionKindTeam), + }, + { + name: "builtin role permission", + perm: accesscontrol.SetResourcePermissionCommand{BuiltinRole: "Editor"}, + expected: string(iamv0.ResourcePermissionSpecPermissionKindBasicRole), + }, + { + name: "empty permission returns empty kind", + perm: accesscontrol.SetResourcePermissionCommand{}, + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + kind := api.getPermissionKind(tt.perm) + assert.Equal(t, tt.expected, kind) + }) + } +} + +// TestGetDynamicClient_RestConfigNotAvailable tests error handling when rest config is not available +func TestGetDynamicClient_RestConfigNotAvailable(t *testing.T) { + ctx := context.Background() + + api := &api{ + service: &Service{ + options: Options{ + Resource: "dashboards", + }, + }, + restConfigProvider: nil, + } + + client, err := api.getDynamicClient(ctx) + + assert.Error(t, err) + assert.Nil(t, client) + assert.Equal(t, ErrRestConfigNotAvailable, err) +} + +// TestBuildResourcePermissionName tests resource permission name building +func TestBuildResourcePermissionName(t *testing.T) { + tests := []struct { + name string + apiGroup string + resource string + resourceID string + expectedName string + }{ + { + name: "with custom API group", + apiGroup: "dashboard.grafana.app", + resource: "dashboards", + resourceID: "dashboard-uid-123", + expectedName: "dashboard.grafana.app-dashboards-dashboard-uid-123", + }, + { + name: "with default API group", + apiGroup: "", + resource: "folders", + resourceID: "folder-uid-456", + expectedName: "folders.grafana.app-folders-folder-uid-456", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + api := &api{ + service: &Service{ + options: Options{ + Resource: tt.resource, + APIGroup: tt.apiGroup, + }, + }, + } + + name := api.buildResourcePermissionName(tt.resourceID) + assert.Equal(t, tt.expectedName, name) + }) + } +} + +// TestGetAPIGroup tests API group resolution +func TestGetAPIGroup(t *testing.T) { + t.Run("returns custom API group when set", func(t *testing.T) { + api := &api{ + service: &Service{ + options: Options{ + Resource: "dashboards", + APIGroup: "custom.grafana.app", + }, + }, + } + + group := api.getAPIGroup() + assert.Equal(t, "custom.grafana.app", group) + }) + + t.Run("returns default API group when not set", func(t *testing.T) { + api := &api{ + service: &Service{ + options: Options{ + Resource: "dashboards", + APIGroup: "", + }, + }, + } + + group := api.getAPIGroup() + assert.Equal(t, "dashboards.grafana.app", group) + }) + + t.Run("default group for folders", func(t *testing.T) { + api := &api{ + service: &Service{ + options: Options{ + Resource: "folders", + APIGroup: "", + }, + }, + } + + group := api.getAPIGroup() + assert.Equal(t, "folders.grafana.app", group) + }) +} + +// TestResourcePermissionKindConstants verifies the kind constants match expected values +func TestResourcePermissionKindConstants(t *testing.T) { + tests := []struct { + name string + kind iamv0.ResourcePermissionSpecPermissionKind + expected string + }{ + { + name: "User kind", + kind: iamv0.ResourcePermissionSpecPermissionKindUser, + expected: "User", + }, + { + name: "Team kind", + kind: iamv0.ResourcePermissionSpecPermissionKindTeam, + expected: "Team", + }, + { + name: "ServiceAccount kind", + kind: iamv0.ResourcePermissionSpecPermissionKindServiceAccount, + expected: "ServiceAccount", + }, + { + name: "BasicRole kind", + kind: iamv0.ResourcePermissionSpecPermissionKindBasicRole, + expected: "BasicRole", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, string(tt.kind)) + }) + } +}