Permissions: Fix team and role permissions on folders/dashboards not displayed for non Grafana Admin users (#31132)

* Cfg: fix hidden users initialization

* add tests

* do not call isHiddenUser function for non-user permission

* do not call isHiddenUser function for non-user permission

pkg/api/dashboard_permission.go

@@ -32,7 +32,7 @@ func (hs *HTTPServer) GetDashboardPermissionList(c *models.ReqContext) response.
 
 	filteredAcls := make([]*models.DashboardAclInfoDTO, 0, len(acl))
 	for _, perm := range acl {
-		if dtos.IsHiddenUser(perm.UserLogin, c.SignedInUser, hs.Cfg) {
+		if perm.UserId > 0 && dtos.IsHiddenUser(perm.UserLogin, c.SignedInUser, hs.Cfg) {
 			continue
 		}
 

pkg/api/dtos/models.go

@@ -77,7 +77,7 @@ func GetGravatarUrlWithDefault(text string, defaultText string) string {
 }
 
 func IsHiddenUser(userLogin string, signedInUser *models.SignedInUser, cfg *setting.Cfg) bool {
-	if signedInUser.IsGrafanaAdmin || userLogin == signedInUser.Login {
+	if userLogin == "" || signedInUser.IsGrafanaAdmin || userLogin == signedInUser.Login {
 		return false
 	}
 

pkg/api/dtos/models_test.go

@@ -0,0 +1,86 @@
+package dtos
+
+import (
+	"testing"
+
+	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/setting"
+	"gotest.tools/assert"
+)
+
+func TestIsHiddenUser(t *testing.T) {
+	emptyHiddenUsers := map[string]struct{}{}
+	hiddenUser := map[string]struct{}{
+		"user": {},
+	}
+
+	testcases := []struct {
+		desc         string
+		userLogin    string
+		signedInUser *models.SignedInUser
+		hiddenUsers  map[string]struct{}
+		expected     bool
+	}{
+		{
+			desc:      "non-server admin user should see non-hidden user",
+			userLogin: "user",
+			signedInUser: &models.SignedInUser{
+				IsGrafanaAdmin: false,
+				Login:          "admin",
+			},
+			hiddenUsers: emptyHiddenUsers,
+			expected:    false,
+		},
+		{
+			desc:      "non-server admin user should not see hidden user",
+			userLogin: "user",
+			signedInUser: &models.SignedInUser{
+				IsGrafanaAdmin: false,
+				Login:          "admin",
+			},
+			hiddenUsers: hiddenUser,
+			expected:    true,
+		},
+		{
+			desc:      "non-server admin user should see himself, even if he's hidden",
+			userLogin: "admin",
+			signedInUser: &models.SignedInUser{
+				IsGrafanaAdmin: false,
+				Login:          "admin",
+			},
+			hiddenUsers: map[string]struct{}{
+				"admin": {},
+			},
+			expected: false,
+		},
+		{
+			desc:      "server admin user should see hidden user",
+			userLogin: "user",
+			signedInUser: &models.SignedInUser{
+				IsGrafanaAdmin: true,
+				Login:          "admin",
+			},
+			hiddenUsers: hiddenUser,
+			expected:    false,
+		},
+		{
+			desc:      "server admin user should see non-hidden user",
+			userLogin: "user",
+			signedInUser: &models.SignedInUser{
+				IsGrafanaAdmin: true,
+				Login:          "admin",
+			},
+			hiddenUsers: emptyHiddenUsers,
+			expected:    false,
+		},
+	}
+
+	for _, c := range testcases {
+		t.Run(c.desc, func(t *testing.T) {
+			isHidden := IsHiddenUser(c.userLogin, c.signedInUser, &setting.Cfg{
+				HiddenUsers: c.hiddenUsers,
+			})
+			assert.Equal(t, c.expected, isHidden)
+		})
+	}
+}

pkg/api/folder_permission.go

@@ -34,7 +34,7 @@ func (hs *HTTPServer) GetFolderPermissionList(c *models.ReqContext) response.Res
 
 	filteredAcls := make([]*models.DashboardAclInfoDTO, 0, len(acl))
 	for _, perm := range acl {
-		if dtos.IsHiddenUser(perm.UserLogin, c.SignedInUser, hs.Cfg) {
+		if perm.UserId > 0 && dtos.IsHiddenUser(perm.UserLogin, c.SignedInUser, hs.Cfg) {
 			continue
 		}
 

pkg/setting/setting.go

@@ -1196,7 +1196,9 @@ func readUserSettings(iniFile *ini.File, cfg *Cfg) error {
 	hiddenUsers := users.Key("hidden_users").MustString("")
 	for _, user := range strings.Split(hiddenUsers, ",") {
 		user = strings.TrimSpace(user)
-		cfg.HiddenUsers[user] = struct{}{}
+		if user != "" {
+			cfg.HiddenUsers[user] = struct{}{}
+		}
 	}
 
 	return nil