RBAC: Make RBAC action names more consistent (#49730) (#50083)

* update action names

* correctly retrieve teams for signed in user

* remove test

* undo swagger changes

* undo swagger changes pt2

* add migration from old action names to the new ones

* rename from list to read

* linting

* also update alertign actions

* fix migration

(cherry picked from commit 5dbea9996b)

Co-authored-by: Ieva <ieva.vasiljeva@grafana.com>

pkg/services/accesscontrol/filter.go

@@ -11,7 +11,7 @@ import (
 var sqlIDAcceptList = map[string]struct{}{
 	"id":               {},
 	"org_user.user_id": {},
-	"role.id":          {},
+	"role.uid":         {},
 	"t.id":             {},
 	"team.id":          {},
 	"u.id":             {},

pkg/services/accesscontrol/models.go

@@ -285,32 +285,31 @@ const (
 	ActionAPIKeyDelete = "apikeys:delete"
 
 	// Users actions
-	ActionUsersRead     = "users:read"
-	ActionUsersWrite    = "users:write"
-	ActionUsersTeamRead = "users.teams:read"
+	ActionUsersRead  = "users:read"
+	ActionUsersWrite = "users:write"
 	// We can ignore gosec G101 since this does not contain any credentials.
 	// nolint:gosec
-	ActionUsersAuthTokenList = "users.authtoken:list"
+	ActionUsersAuthTokenList = "users.authtoken:read"
 	// We can ignore gosec G101 since this does not contain any credentials.
 	// nolint:gosec
-	ActionUsersAuthTokenUpdate = "users.authtoken:update"
+	ActionUsersAuthTokenUpdate = "users.authtoken:write"
 	// We can ignore gosec G101 since this does not contain any credentials.
 	// nolint:gosec
-	ActionUsersPasswordUpdate    = "users.password:update"
+	ActionUsersPasswordUpdate    = "users.password:write"
 	ActionUsersDelete            = "users:delete"
 	ActionUsersCreate            = "users:create"
 	ActionUsersEnable            = "users:enable"
 	ActionUsersDisable           = "users:disable"
-	ActionUsersPermissionsUpdate = "users.permissions:update"
+	ActionUsersPermissionsUpdate = "users.permissions:write"
 	ActionUsersLogout            = "users:logout"
-	ActionUsersQuotasList        = "users.quotas:list"
-	ActionUsersQuotasUpdate      = "users.quotas:update"
+	ActionUsersQuotasList        = "users.quotas:read"
+	ActionUsersQuotasUpdate      = "users.quotas:write"
 
 	// Org actions
-	ActionOrgUsersRead       = "org.users:read"
-	ActionOrgUsersAdd        = "org.users:add"
-	ActionOrgUsersRemove     = "org.users:remove"
-	ActionOrgUsersRoleUpdate = "org.users.role:update"
+	ActionOrgUsersRead   = "org.users:read"
+	ActionOrgUsersAdd    = "org.users:add"
+	ActionOrgUsersRemove = "org.users:remove"
+	ActionOrgUsersWrite  = "org.users:write"
 
 	// LDAP actions
 	ActionLDAPUsersRead    = "ldap.user:read"
@@ -363,12 +362,12 @@ const (
 	// Alerting rules actions
 	ActionAlertingRuleCreate = "alert.rules:create"
 	ActionAlertingRuleRead   = "alert.rules:read"
-	ActionAlertingRuleUpdate = "alert.rules:update"
+	ActionAlertingRuleUpdate = "alert.rules:write"
 	ActionAlertingRuleDelete = "alert.rules:delete"
 
 	// Alerting instances (+silences) actions
 	ActionAlertingInstanceCreate = "alert.instances:create"
-	ActionAlertingInstanceUpdate = "alert.instances:update"
+	ActionAlertingInstanceUpdate = "alert.instances:write"
 	ActionAlertingInstanceRead   = "alert.instances:read"
 
 	// Alerting Notification policies actions

pkg/services/accesscontrol/roles.go

@@ -53,14 +53,14 @@ var (
 		DisplayName: "Organization user writer",
 		Description: "Within a single organization, add a user, invite a user, read information about a user and their role, remove a user from that organization, or change the role of a user.",
 		Group:       "User administration (organizational)",
-		Version:     3,
+		Version:     4,
 		Permissions: ConcatPermissions(orgUsersReaderRole.Permissions, []Permission{
 			{
 				Action: ActionOrgUsersAdd,
 				Scope:  ScopeUsersAll,
 			},
 			{
-				Action: ActionOrgUsersRoleUpdate,
+				Action: ActionOrgUsersWrite,
 				Scope:  ScopeUsersAll,
 			},
 			{
@@ -116,16 +116,12 @@ var (
 		DisplayName: "User reader",
 		Description: "Read all users and their information, such as team memberships, authentication tokens, and quotas.",
 		Group:       "User administration (global)",
-		Version:     4,
+		Version:     6,
 		Permissions: []Permission{
 			{
 				Action: ActionUsersRead,
 				Scope:  ScopeGlobalUsersAll,
 			},
-			{
-				Action: ActionUsersTeamRead,
-				Scope:  ScopeGlobalUsersAll,
-			},
 			{
 				Action: ActionUsersAuthTokenList,
 				Scope:  ScopeGlobalUsersAll,
@@ -142,7 +138,7 @@ var (
 		DisplayName: "User writer",
 		Description: "Read and update all attributes and settings for all users in Grafana: update user information, read user information, create or enable or disable a user, make a user a Grafana administrator, sign out a user, update a user’s authentication token, or update quotas for all users.",
 		Group:       "User administration (global)",
-		Version:     4,
+		Version:     5,
 		Permissions: ConcatPermissions(usersReaderRole.Permissions, []Permission{
 			{
 				Action: ActionUsersPasswordUpdate,

pkg/services/guardian/guardian.go

@@ -270,7 +270,7 @@ func (g *dashboardGuardianImpl) getTeams() ([]*models.TeamDTO, error) {
 		return g.teams, nil
 	}
 
-	query := models.GetTeamsByUserQuery{OrgId: g.orgId, UserId: g.user.UserId}
+	query := models.GetTeamsByUserQuery{OrgId: g.orgId, UserId: g.user.UserId, SignedInUser: g.user}
 	err := g.store.GetTeamsByUser(g.ctx, &query)
 
 	g.teams = query.Result

pkg/services/licensing/accesscontrol.go

@@ -4,7 +4,7 @@ import "github.com/grafana/grafana/pkg/services/accesscontrol"
 
 const (
 	ActionRead        = "licensing:read"
-	ActionUpdate      = "licensing:update"
+	ActionUpdate      = "licensing:write"
 	ActionDelete      = "licensing:delete"
 	ActionReportsRead = "licensing.reports:read"
 )

pkg/services/ngalert/accesscontrol.go

@@ -36,7 +36,7 @@ var (
 			DisplayName: "Rules Editor",
 			Description: "Can add, update, and delete rules in any Grafana folder and external providers",
 			Group:       AlertRolesGroup,
-			Version:     2,
+			Version:     3,
 			Permissions: accesscontrol.ConcatPermissions(rulesReaderRole.Role.Permissions, []accesscontrol.Permission{
 				{
 					Action: accesscontrol.ActionAlertingRuleCreate,
@@ -84,7 +84,7 @@ var (
 			DisplayName: "Silences Editor",
 			Description: "Can add and update silences in Grafana and external providers",
 			Group:       AlertRolesGroup,
-			Version:     1,
+			Version:     2,
 			Permissions: accesscontrol.ConcatPermissions(instancesReaderRole.Role.Permissions, []accesscontrol.Permission{
 				{
 					Action: accesscontrol.ActionAlertingInstanceCreate,

pkg/services/serviceaccounts/database/database.go

@@ -209,25 +209,7 @@ func (s *ServiceAccountsStoreImpl) RetrieveServiceAccount(ctx context.Context, o
 		return nil
 	})
 
-	if err != nil {
-		return nil, err
-	}
-
-	// Get Teams of service account. Can be optimized by combining with the query above
-	// in refactor
-	getTeamQuery := models.GetTeamsByUserQuery{UserId: serviceAccountID, OrgId: orgID}
-	if err := s.sqlStore.GetTeamsByUser(ctx, &getTeamQuery); err != nil {
-		return nil, err
-	}
-	teams := make([]string, len(getTeamQuery.Result))
-
-	for i := range getTeamQuery.Result {
-		teams[i] = getTeamQuery.Result[i].Name
-	}
-
-	serviceAccount.Teams = teams
-
-	return serviceAccount, nil
+	return serviceAccount, err
 }
 
 func (s *ServiceAccountsStoreImpl) RetrieveServiceAccountIdByName(ctx context.Context, orgID int64, name string) (int64, error) {

pkg/services/serviceaccounts/database/database_test.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"testing"
 
-	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts/tests"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
@@ -107,20 +106,3 @@ func TestStore_RetrieveServiceAccount(t *testing.T) {
 		})
 	}
 }
-func TestStore_RetrieveServiceAccountWithTeams(t *testing.T) {
-	userToCreate := tests.TestUser{Login: "servicetestwithTeam@admin", IsServiceAccount: true}
-	db, store := setupTestDatabase(t)
-	user := tests.SetupUserServiceAccount(t, db, userToCreate)
-
-	team, err := store.sqlStore.CreateTeam("serviceTeam", "serviceTeam", user.OrgId)
-	require.NoError(t, err)
-
-	err = store.sqlStore.AddTeamMember(user.Id, user.OrgId, team.Id, false, models.PERMISSION_VIEW)
-	require.NoError(t, err)
-
-	dto, err := store.RetrieveServiceAccount(context.Background(), user.OrgId, user.Id)
-	require.NoError(t, err)
-	require.Equal(t, userToCreate.Login, dto.Login)
-	require.Len(t, dto.Teams, 1)
-	require.Equal(t, "serviceTeam", dto.Teams[0])
-}

pkg/services/sqlstore/migrations/accesscontrol/action_migrator.go

@@ -0,0 +1,68 @@
+package accesscontrol
+
+import (
+	"fmt"
+
+	"github.com/grafana/grafana/pkg/services/accesscontrol"
+	"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
+
+	"xorm.io/xorm"
+)
+
+func AddActionNameMigrator(mg *migrator.Migrator) {
+	mg.AddMigration("RBAC action name migrator", &actionNameMigrator{})
+}
+
+type actionNameMigrator struct {
+	sess     *xorm.Session
+	migrator *migrator.Migrator
+	migrator.MigrationBase
+}
+
+var _ migrator.CodeMigration = new(actionNameMigrator)
+
+func (m *actionNameMigrator) SQL(migrator.Dialect) string {
+	return CodeMigrationSQL
+}
+
+func (m *actionNameMigrator) Exec(sess *xorm.Session, migrator *migrator.Migrator) error {
+	m.sess = sess
+	m.migrator = migrator
+	return m.migrateActionNames()
+}
+
+func (m *actionNameMigrator) migrateActionNames() error {
+	actionNameMapping := map[string]string{
+		"licensing:update":         "licensing:write",
+		"reports.admin:create":     "reports:create",
+		"reports.admin:write":      "reports:write",
+		"org.users.role:update":    accesscontrol.ActionOrgUsersWrite,
+		"users.authtoken:update":   accesscontrol.ActionUsersAuthTokenUpdate,
+		"users.password:update":    accesscontrol.ActionUsersPasswordUpdate,
+		"users.permissions:update": accesscontrol.ActionUsersPermissionsUpdate,
+		"users.quotas:update":      accesscontrol.ActionUsersQuotasUpdate,
+		"teams.roles:list":         "teams.roles:read",
+		"users.roles:list":         "users.roles:read",
+		"users.authtoken:list":     accesscontrol.ActionUsersAuthTokenList,
+		"users.quotas:list":        accesscontrol.ActionUsersQuotasList,
+		"users.permissions:list":   "users.permissions:read",
+		"alert.instances:update":   accesscontrol.ActionAlertingInstanceUpdate,
+		"alert.rules:update":       accesscontrol.ActionAlertingRuleUpdate,
+	}
+	for oldName, newName := range actionNameMapping {
+		_, err := m.sess.Table(&accesscontrol.Permission{}).Where("action = ?", oldName).Update(&accesscontrol.Permission{Action: newName})
+		if err != nil {
+			return fmt.Errorf("failed to update permission table for action %s: %w", oldName, err)
+		}
+	}
+
+	actionsToDelete := []string{"users.teams:read", "roles:list"}
+	for _, action := range actionsToDelete {
+		_, err := m.sess.Table(&accesscontrol.Permission{}).Where("action = ?", action).Delete(accesscontrol.Permission{})
+		if err != nil {
+			return fmt.Errorf("failed to update permission table for action %s: %w", action, err)
+		}
+	}
+
+	return nil
+}

pkg/services/sqlstore/migrations/migrations.go

@@ -90,6 +90,7 @@ func (*OSSMigrations) AddMigration(mg *Migrator) {
 
 	accesscontrol.AddManagedPermissionsMigration(mg)
 	accesscontrol.AddManagedFolderAlertActionsMigration(mg)
+	accesscontrol.AddActionNameMigrator(mg)
 }
 
 func addMigrationLogMigrations(mg *Migrator) {

pkg/services/sqlstore/team.go

@@ -310,12 +310,23 @@ func (ss *SQLStore) GetTeamsByUser(ctx context.Context, query *models.GetTeamsBy
 		query.Result = make([]*models.TeamDTO, 0)
 
 		var sql bytes.Buffer
+		var params []interface{}
+		params = append(params, query.OrgId, query.UserId)
 
 		sql.WriteString(getTeamSelectSQLBase([]string{}))
 		sql.WriteString(` INNER JOIN team_member on team.id = team_member.team_id`)
 		sql.WriteString(` WHERE team.org_id = ? and team_member.user_id = ?`)
 
-		err := sess.SQL(sql.String(), query.OrgId, query.UserId).Find(&query.Result)
+		if !ac.IsDisabled(ss.Cfg) {
+			acFilter, err := ac.Filter(query.SignedInUser, "team.id", "teams:id:", ac.ActionTeamsRead)
+			if err != nil {
+				return err
+			}
+			sql.WriteString(` and` + acFilter.Where)
+			params = append(params, acFilter.Args...)
+		}
+
+		err := sess.SQL(sql.String(), params...).Find(&query.Result)
 		return err
 	})
 }

pkg/services/sqlstore/team_test.go

@@ -222,7 +222,14 @@ func TestIntegrationTeamCommandsAndQueries(t *testing.T) {
 				err := sqlStore.AddTeamMember(userIds[0], testOrgID, groupId, false, 0)
 				require.NoError(t, err)
 
-				query := &models.GetTeamsByUserQuery{OrgId: testOrgID, UserId: userIds[0]}
+				query := &models.GetTeamsByUserQuery{
+					OrgId:  testOrgID,
+					UserId: userIds[0],
+					SignedInUser: &models.SignedInUser{
+						OrgId:       testOrgID,
+						Permissions: map[int64]map[string][]string{testOrgID: {ac.ActionOrgUsersRead: {ac.ScopeUsersAll}, ac.ActionTeamsRead: {ac.ScopeTeamsAll}}},
+					},
+				}
 				err = sqlStore.GetTeamsByUser(context.Background(), query)
 				require.NoError(t, err)
 				require.Equal(t, len(query.Result), 1)

pkg/services/sqlstore/user.go

@@ -577,7 +577,20 @@ func (ss *SQLStore) GetSignedInUser(ctx context.Context, query *models.GetSigned
 			user.ExternalAuthId = ""
 		}
 
-		getTeamsByUserQuery := &models.GetTeamsByUserQuery{OrgId: user.OrgId, UserId: user.UserId}
+		// tempUser is used to retrieve the teams for the signed in user for internal use.
+		tempUser := &models.SignedInUser{
+			OrgId: user.OrgId,
+			Permissions: map[int64]map[string][]string{
+				user.OrgId: {
+					ac.ActionTeamsRead: {ac.ScopeTeamsAll},
+				},
+			},
+		}
+		getTeamsByUserQuery := &models.GetTeamsByUserQuery{
+			OrgId:        user.OrgId,
+			UserId:       user.UserId,
+			SignedInUser: tempUser,
+		}
 		err = ss.GetTeamsByUser(ctx, getTeamsByUserQuery)
 		if err != nil {
 			return err