Access Control / Information Disclosure

PoC outline to demonstrate the vulnerability prior to the fix (information disclosure / incorrect subject resolution when listing permissions): Context: Grafana is deployed with Kubernetes-backed user subjects. A resource (e.g., a dashboard) has permissions that reference a set of subjects by UID (e.g., a Kubernetes User UID). Before the fix: When an editor or reader requests the resource permissions, Grafana resolves each subject’s display name by calling GetByUID. If the request context does not provide sufficient permissions to read user/team objects, the lookup would fail or fall back to the legacy SQL store. In multi-instance setups, this fallback could yield empty UserID/UserLogin fields or omit the subject entirely, causing misleading or incomplete permission data to be returned (and potentially leaking information about who has access). After the fix: The request context is propagated into the Kubernetes GetByUID path and a service identity is used to resolve subjects. This ensures that subject resolution does not depend on the caller’s limited permissions and aligns with expected behavior, preventing silent fallbacks and ensuring the subject’s identity is correctly presented in the permissions list. Steps to reproduce (conceptual): 1) Deploy Grafana with Kubernetes-based IAM user service enabled and multi-instance storage (unified storage backend with per-instance SQLite). 2) Create a resource with permissions referencing a Kubernetes user UID (e.g., some-uid) that exists only in the Kubernetes identity store. 3) Login as a non-admin who can read the resource’s permissions but cannot read user/team objects directly. 4) Request the resource permissions list via Grafana API and observe that subject data (UserID/UserLogin) may be empty or a 403/denied path may cause silent fallback to legacy data in older builds. 5) After applying the fix: The same request returns properly resolved subject names by using the service identity and propagating request context, with correct UserID/UserLogin populated. Code pointers (conceptual): - Before fix (vulnerable path): GetByUID would be invoked with a background context, causing Kubernetes-based lookups to fall back or fail to resolve subjects in restricted contexts. - After fix: GetByUID is invoked with a context that carries the request identity (service identity), ensuring Kubernetes lookups succeed and leverage the proper permissions. Note: Exact endpoints depend on Grafana’s internal API for resource permissions; the PoC focuses on the behavior difference between using a background context vs a request-scoped service identity when resolving permission subjects.

diff --git a/pkg/services/accesscontrol/resourcepermissions/api_adapter.go b/pkg/services/accesscontrol/resourcepermissions/api_adapter.go index 108548fe9849a..1eae5c9167ac8 100644 --- a/pkg/services/accesscontrol/resourcepermissions/api_adapter.go +++ b/pkg/services/accesscontrol/resourcepermissions/api_adapter.go @@ -24,6 +24,7 @@ import ( folderv1 "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1" iamv0 "github.com/grafana/grafana/apps/iam/pkg/apis/iam/v0alpha1" "github.com/grafana/grafana/pkg/api/dtos" + "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/accesscontrol" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" @@ -73,7 +74,7 @@ func (a *api) getResourcePermissionsFromK8s(c *contextmodel.ReqContext, namespac return nil, fmt.Errorf("failed to convert to typed resource permission: %w", err) } - directDTO, err := a.convertK8sResourcePermissionToDTO(&resourcePerm, namespace, false) + directDTO, err := a.convertK8sResourcePermissionToDTO(ctx, &resourcePerm, namespace, false) if err != nil { return nil, err } @@ -114,13 +115,18 @@ func (a *api) getResourcePermissionsFromK8s(c *contextmodel.ReqContext, namespac return dto, nil } -func (a *api) convertK8sResourcePermissionToDTO(resourcePerm *iamv0.ResourcePermission, namespace string, isInherited bool) (getResourcePermissionsResponse, error) { +func (a *api) convertK8sResourcePermissionToDTO(ctx context.Context, resourcePerm *iamv0.ResourcePermission, namespace string, isInherited bool) (getResourcePermissionsResponse, error) { namespaceInfo, err := types.ParseNamespace(namespace) if err != nil { return nil, fmt.Errorf("failed to parse namespace %q: %w", namespace, err) } orgID := namespaceInfo.OrgID + // Resolve subject names with a service identity: the caller is already authorized + // to read this resource's permissions but may lack users:read (e.g. an editor), + // which would otherwise leave the subject unnamed. Mirrors the legacy SQL join. + lookupCtx, _ := identity.WithServiceIdentity(ctx, orgID) + permissions := resourcePerm.Spec.Permissions dto := make(getResourcePermissionsResponse, 0, len(permissions)) @@ -154,7 +160,7 @@ func (a *api) convertK8sResourcePermissionToDTO(resourcePerm *iamv0.ResourcePerm switch kind { case iamv0.ResourcePermissionSpecPermissionKindUser, iamv0.ResourcePermissionSpecPermissionKindServiceAccount: - userDetails, err := a.service.userService.GetByUID(context.Background(), &user.GetUserByUIDQuery{UID: name}) + userDetails, err := a.service.userService.GetByUID(lookupCtx, &user.GetUserByUIDQuery{UID: name}) if err == nil { permDTO.UserID = userDetails.ID permDTO.UserUID = userDetails.UID @@ -165,7 +171,7 @@ func (a *api) convertK8sResourcePermissionToDTO(resourcePerm *iamv0.ResourcePerm permDTO.ID = a.getRoleIDFromK8sObject(permDTO.RoleName, orgID) } case iamv0.ResourcePermissionSpecPermissionKindTeam: - teamDetails, err := a.service.teamService.GetTeamByID(context.Background(), &team.GetTeamByIDQuery{ + teamDetails, err := a.service.teamService.GetTeamByID(lookupCtx, &team.GetTeamByIDQuery{ UID: name, OrgID: orgID, }) @@ -297,7 +303,7 @@ func (a *api) getFolderHierarchyPermissions(ctx context.Context, namespace strin continue } - inheritedDTO, err := a.convertK8sResourcePermissionToDTO(&parentResourcePerm, namespace, true) + inheritedDTO, err := a.convertK8sResourcePermissionToDTO(ctx, &parentResourcePerm, namespace, true) if err != nil { a.logger.Warn("Failed to convert parent folder permissions to DTO", "error", err, "parentFolder", parentFolder.Name) continue diff --git a/pkg/services/accesscontrol/resourcepermissions/api_adapter_test.go b/pkg/services/accesscontrol/resourcepermissions/api_adapter_test.go index 1cd66dabfa9df..4a170e4137c4b 100644 --- a/pkg/services/accesscontrol/resourcepermissions/api_adapter_test.go +++ b/pkg/services/accesscontrol/resourcepermissions/api_adapter_test.go @@ -280,7 +280,7 @@ func TestConvertK8sResourcePermissionToDTO(t *testing.T) { }, } - inheritedPerms, err := api.convertK8sResourcePermissionToDTO(folderPermission, "stack-123-org-1", true) + inheritedPerms, err := api.convertK8sResourcePermissionToDTO(context.Background(), folderPermission, "stack-123-org-1", true) require.NoError(t, err) require.Len(t, inheritedPerms, 2, "should have 2 inherited permissions (Editor and Viewer)") diff --git a/pkg/services/user/userimpl/user.go b/pkg/services/user/userimpl/user.go index 215a6961c7350..c6a5789772c1a 100644 --- a/pkg/services/user/userimpl/user.go +++ b/pkg/services/user/userimpl/user.go @@ -102,6 +102,23 @@ func (s *Service) GetByID(ctx context.Context, cmd *user.GetUserByIDQuery) (*use } func (s *Service) GetByUID(ctx context.Context, cmd *user.GetUserByUIDQuery) (*user.User, error) { + ctx, span := s.tracer.Start(ctx, "user.wrapper.GetByUID", trace.WithAttributes( + attribute.String("userUID", cmd.UID), + )) + defer span.End() + + ctxLogger := s.logger.FromContext(ctx) + + if s.isKubernetesUserServiceEnabled(ctx) && !s.shouldFallbackToLegacy(ctx) { + result, err := s.k8sService.GetByUID(s.k8sCtxWithIdentity(ctx), cmd) + if err == nil { + span.SetAttributes(attribute.Bool("fallback_to_legacy", false)) + return result, nil + } + ctxLogger.Warn("k8s GetByUID failed, falling back to legacy", "userUID", cmd.UID, "err", err) + } + + span.SetAttributes(attribute.Bool("fallback_to_legacy", true)) return s.legacyService.GetByUID(ctx, cmd) } diff --git a/pkg/services/user/userk8s/user.go b/pkg/services/user/userk8s/user.go index 42e8b3f1fae0f..6b080da6d427b 100644 --- a/pkg/services/user/userk8s/user.go +++ b/pkg/services/user/userk8s/user.go @@ -13,6 +13,7 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/trace" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/dynamic" "k8s.io/client-go/rest" @@ -246,7 +247,44 @@ func (s *UserK8sService) GetByID(ctx context.Context, cmd *user.GetUserByIDQuery } func (s *UserK8sService) GetByUID(ctx context.Context, cmd *user.GetUserByUIDQuery) (*user.User, error) { - return nil, errors.New("not implemented") + ctx, span := s.tracer.Start(ctx, "user.getByUID", trace.WithAttributes( + attribute.String("userUID", cmd.UID), + )) + defer span.End() + + ctxLogger := s.logger.FromContext(ctx) + + orgID, err := s.getOrgID(ctx, ctxLogger) + if err != nil { + ctxLogger.Error("failed to get orgID from context in GetByUID", "err", err) + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + return nil, err + } + + namespace := s.namespaceMapper(orgID) + span.SetAttributes(attribute.Int64("orgID", orgID)) + + client, err := s.getUserClient(ctx) + if err != nil { + ctxLogger.Error("failed to get k8s client", "namespace", namespace, "err", err) + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + return nil, err + } + + found, err := client.Get(ctx, resource.Identifier{Namespace: namespace, Name: cmd.UID}) + if err != nil { + if apierrors.IsNotFound(err) { + return nil, user.ErrUserNotFound + } + ctxLogger.Error("k8s user get by UID failed", "namespace", namespace, "userUID", cmd.UID, "err", err) + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + return nil, err + } + + return toUser(found, orgID), nil } func (s *UserK8sService) ListByIdOrUID(ctx context.Context, ids []string, intIDs []int64) ([]*user.User, error) { diff --git a/pkg/services/user/userk8s/user_test.go b/pkg/services/user/userk8s/user_test.go index ecb3c8e5b98fb..1604a2d22b6c9 100644 --- a/pkg/services/user/userk8s/user_test.go +++ b/pkg/services/user/userk8s/user_test.go @@ -486,6 +486,124 @@ func TestUserK8sService_GetByID(t *testing.T) { } } +func TestUserK8sService_GetByUID(t *testing.T) { + makeGetResponse := func(u v0alpha1.User) func(http.ResponseWriter, *http.Request) { + return func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(u) + } + } + + tests := []struct { + name string + cmd *user.GetUserByUIDQuery + requesterOrgID int64 + serverResponse func(w http.ResponseWriter, r *http.Request) + nilProvider bool + noRequester bool + expectErr bool + expectErrIs error + expectUser *user.User + }{ + { + name: "successfully retrieves a user by UID", + requesterOrgID: 1, + cmd: &user.GetUserByUIDQuery{UID: "some-uid"}, + serverResponse: func(w http.ResponseWriter, r *http.Request) { + assert.Contains(t, r.URL.Path, "some-uid") + makeGetResponse(newTestK8sUser("some-uid", "org-1", "jdoe", "jdoe@example.com"))(w, r) + }, + expectUser: &user.User{ + ID: 42, + UID: "some-uid", + OrgID: 1, + Login: "jdoe", + Email: "jdoe@example.com", + Name: "John Doe", + IsAdmin: true, + EmailVerified: true, + LastSeenAt: time.Date(2025, 6, 1, 10, 0, 0, 0, time.UTC), + }, + }, + { + name: "returns ErrUserNotFound when user does not exist", + requesterOrgID: 1, + cmd: &user.GetUserByUIDQuery{UID: "missing-uid"}, + serverResponse: func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotFound) + _ = json.NewEncoder(w).Encode(metav1.Status{ + TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: "Status"}, + Status: metav1.StatusFailure, + Reason: metav1.StatusReasonNotFound, + Code: http.StatusNotFound, + }) + }, + expectErr: true, + expectErrIs: user.ErrUserNotFound, + }, + { + name: "propagates non-not-found error from k8s client", + requesterOrgID: 1, + cmd: &user.GetUserByUIDQuery{UID: "some-uid"}, + serverResponse: func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusInternalServerError) + _ = json.NewEncoder(w).Encode(metav1.Status{ + TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: "Status"}, + Status: metav1.StatusFailure, + Message: "k8s error", + Code: http.StatusInternalServerError, + }) + }, + expectErr: true, + }, + { + name: "returns error when config provider not initialized", + cmd: &user.GetUserByUIDQuery{UID: "some-uid"}, + nilProvider: true, + expectErr: true, + }, + { + name: "returns error when no requester in context", + cmd: &user.GetUserByUIDQuery{UID: "some-uid"}, + noRequester: true, + expectErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + svc, ctx := setupServiceAndCtx(t, svcTestSetup{ + nilProvider: tt.nilProvider, + noRequester: tt.noRequester, + requesterOrgID: tt.requesterOrgID, + serverResponse: tt.serverResponse, + }) + + result, err := svc.GetByUID(ctx, tt.cmd) + + if tt.expectErr { + require.Error(t, err) + if tt.expectErrIs != nil { + require.ErrorIs(t, err, tt.expectErrIs) + } + return + } + + require.NoError(t, err) + assert.Equal(t, tt.expectUser.ID, result.ID) + assert.Equal(t, tt.expectUser.UID, result.UID) + assert.Equal(t, tt.expectUser.OrgID, result.OrgID) + assert.Equal(t, tt.expectUser.Login, result.Login) + assert.Equal(t, tt.expectUser.Email, result.Email) + assert.Equal(t, tt.expectUser.Name, result.Name) + assert.Equal(t, tt.expectUser.IsAdmin, result.IsAdmin) + assert.Equal(t, tt.expectUser.EmailVerified, result.EmailVerified) + }) + } +} + func TestUserK8sService_GetByEmail(t *testing.T) { tests := []struct { name string