Folders: simplify guardian permissions checks (#63183)

simplify code

pkg/services/folder/folderimpl/folder.go

@@ -95,17 +95,17 @@ func (s *Service) Get(ctx context.Context, cmd *folder.GetFolderQuery) (*folder.
 	var err error
 	switch {
 	case cmd.UID != nil:
-		dashFolder, err = s.getFolderByUID(ctx, cmd.SignedInUser, cmd.OrgID, *cmd.UID)
+		dashFolder, err = s.getFolderByUID(ctx, cmd.OrgID, *cmd.UID)
 		if err != nil {
 			return nil, err
 		}
 	case cmd.ID != nil:
-		dashFolder, err = s.getFolderByID(ctx, cmd.SignedInUser, *cmd.ID, cmd.OrgID)
+		dashFolder, err = s.getFolderByID(ctx, *cmd.ID, cmd.OrgID)
 		if err != nil {
 			return nil, err
 		}
 	case cmd.Title != nil:
-		dashFolder, err = s.getFolderByTitle(ctx, cmd.SignedInUser, cmd.OrgID, *cmd.Title)
+		dashFolder, err = s.getFolderByTitle(ctx, cmd.OrgID, *cmd.Title)
 		if err != nil {
 			return nil, err
 		}
@@ -113,28 +113,14 @@ func (s *Service) Get(ctx context.Context, cmd *folder.GetFolderQuery) (*folder.
 		return nil, folder.ErrBadRequest.Errorf("either on of UID, ID, Title fields must be present")
 	}
 
-	if !s.features.IsEnabled(featuremgmt.FlagNestedFolders) {
-		return dashFolder, nil
-	}
-
-	if cmd.ID != nil {
-		cmd.ID = nil
-		cmd.UID = &dashFolder.UID
-	}
-
 	if dashFolder.IsGeneral() {
 		return dashFolder, nil
 	}
 
-	f, err := s.store.Get(ctx, *cmd)
-	if err != nil {
-		return nil, err
-	}
-
 	// do not get guardian by the folder ID because it differs from the nested folder ID
 	// and the legacy folder ID has been associated with the permissions:
 	// use the folde UID instead that is the same for both
-	g, err := guardian.NewByUID(ctx, f.UID, f.OrgID, cmd.SignedInUser)
+	g, err := guardian.NewByUID(ctx, dashFolder.UID, dashFolder.OrgID, cmd.SignedInUser)
 	if err != nil {
 		return nil, err
 	}
@@ -146,6 +132,20 @@ func (s *Service) Get(ctx context.Context, cmd *folder.GetFolderQuery) (*folder.
 		return nil, dashboards.ErrFolderAccessDenied
 	}
 
+	if !s.features.IsEnabled(featuremgmt.FlagNestedFolders) {
+		return dashFolder, nil
+	}
+
+	if cmd.ID != nil {
+		cmd.ID = nil
+		cmd.UID = &dashFolder.UID
+	}
+
+	f, err := s.store.Get(ctx, *cmd)
+	if err != nil {
+		return nil, err
+	}
+
 	// always expose the dashboard store sequential ID
 	f.ID = dashFolder.ID
 
@@ -193,77 +193,20 @@ func (s *Service) GetParents(ctx context.Context, q folder.GetParentsQuery) ([]*
 	return s.store.GetParents(ctx, q)
 }
 
-func (s *Service) getFolderByID(ctx context.Context, user *user.SignedInUser, id int64, orgID int64) (*folder.Folder, error) {
+func (s *Service) getFolderByID(ctx context.Context, id int64, orgID int64) (*folder.Folder, error) {
 	if id == 0 {
 		return &folder.GeneralFolder, nil
 	}
 
-	dashFolder, err := s.dashboardFolderStore.GetFolderByID(ctx, orgID, id)
-	if err != nil {
-		return nil, err
-	}
-
-	// do not get guardian by the folder ID because it differs from the nested folder ID
-	// and the legacy folder ID has been associated with the permissions:
-	// use the folde UID instead that is the same for both
-	g, err := guardian.NewByUID(ctx, dashFolder.UID, orgID, user)
-	if err != nil {
-		return nil, err
-	}
-
-	if canView, err := g.CanView(); err != nil || !canView {
-		if err != nil {
-			return nil, toFolderError(err)
-		}
-		return nil, dashboards.ErrFolderAccessDenied
-	}
-
-	return dashFolder, nil
+	return s.dashboardFolderStore.GetFolderByID(ctx, orgID, id)
 }
 
-func (s *Service) getFolderByUID(ctx context.Context, user *user.SignedInUser, orgID int64, uid string) (*folder.Folder, error) {
-	dashFolder, err := s.dashboardFolderStore.GetFolderByUID(ctx, orgID, uid)
-	if err != nil {
-		return nil, err
-	}
-
-	// do not get guardian by the folder ID because it differs from the nested folder ID
-	// and the legacy folder ID has been associated with the permissions:
-	// use the folde UID instead that is the same for both
-	g, err := guardian.NewByUID(ctx, dashFolder.UID, orgID, user)
-	if err != nil {
-		return nil, err
-	}
-
-	if canView, err := g.CanView(); err != nil || !canView {
-		if err != nil {
-			return nil, toFolderError(err)
-		}
-		return nil, dashboards.ErrFolderAccessDenied
-	}
-
-	return dashFolder, nil
+func (s *Service) getFolderByUID(ctx context.Context, orgID int64, uid string) (*folder.Folder, error) {
+	return s.dashboardFolderStore.GetFolderByUID(ctx, orgID, uid)
 }
 
-func (s *Service) getFolderByTitle(ctx context.Context, user *user.SignedInUser, orgID int64, title string) (*folder.Folder, error) {
-	dashFolder, err := s.dashboardFolderStore.GetFolderByTitle(ctx, orgID, title)
-	if err != nil {
-		return nil, err
-	}
-
-	g, err := guardian.NewByUID(ctx, dashFolder.UID, orgID, user)
-	if err != nil {
-		return nil, err
-	}
-
-	if canView, err := g.CanView(); err != nil || !canView {
-		if err != nil {
-			return nil, toFolderError(err)
-		}
-		return nil, dashboards.ErrFolderAccessDenied
-	}
-
-	return dashFolder, nil
+func (s *Service) getFolderByTitle(ctx context.Context, orgID int64, title string) (*folder.Folder, error) {
+	return s.dashboardFolderStore.GetFolderByTitle(ctx, orgID, title)
 }
 
 func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) (*folder.Folder, error) {

pkg/services/folder/folderimpl/folder_test.go

@@ -258,7 +258,7 @@ func TestIntegrationFolderService(t *testing.T) {
 
 				folderStore.On("GetFolderByID", mock.Anything, orgID, expected.ID).Return(expected, nil)
 
-				actual, err := service.getFolderByID(context.Background(), usr, expected.ID, orgID)
+				actual, err := service.getFolderByID(context.Background(), expected.ID, orgID)
 				require.Equal(t, expected, actual)
 				require.NoError(t, err)
 			})
@@ -269,7 +269,7 @@ func TestIntegrationFolderService(t *testing.T) {
 
 				folderStore.On("GetFolderByUID", mock.Anything, orgID, expected.UID).Return(expected, nil)
 
-				actual, err := service.getFolderByUID(context.Background(), usr, orgID, expected.UID)
+				actual, err := service.getFolderByUID(context.Background(), orgID, expected.UID)
 				require.Equal(t, expected, actual)
 				require.NoError(t, err)
 			})
@@ -279,7 +279,7 @@ func TestIntegrationFolderService(t *testing.T) {
 
 				folderStore.On("GetFolderByTitle", mock.Anything, orgID, expected.Title).Return(expected, nil)
 
-				actual, err := service.getFolderByTitle(context.Background(), usr, orgID, expected.Title)
+				actual, err := service.getFolderByTitle(context.Background(), orgID, expected.Title)
 				require.Equal(t, expected, actual)
 				require.NoError(t, err)
 			})