RBAC: generated prefixed uids for external service role (#76601)

* Replace FixedRoleUID function with a common function to generate these prefixes

* Use common function to generate prefixed uid for external service accounts

Co-authored-by: Gabriel MABILLE <gabriel.mabille@grafana.com>

---------

Co-authored-by: Gabriel MABILLE <gabriel.mabille@grafana.com>

pkg/services/accesscontrol/database/externalservices.go

@@ -2,9 +2,7 @@ package database
 
 import (
 	"context"
-	// #nosec G505 Used only for generating a 160 bit hash, it's not used for security purposes
+
-	"crypto/sha1"
-	"encoding/hex"
 	"errors"
 	"fmt"
 	"time"
@@ -13,24 +11,13 @@ import (
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 )
 
-// extServiceRoleUID generates a 160 bit unique ID using SHA-1 that fits within the 40 characters limit of the role UID.
-func extServiceRoleUID(externalServiceID string) string {
-	uid := fmt.Sprintf("%s%s_permissions", accesscontrol.ExternalServiceRoleUIDPrefix, externalServiceID)
-	// #nosec G505 Used only for generating a 160 bit hash, it's not used for security purposes
-	hasher := sha1.New()
-	hasher.Write([]byte(uid))
-
-	return hex.EncodeToString(hasher.Sum(nil))
-}
-
 func extServiceRoleName(externalServiceID string) string {
 	name := fmt.Sprintf("%s%s:permissions", accesscontrol.ExternalServiceRolePrefix, externalServiceID)
 	return name
 }
 
 func (s *AccessControlStore) DeleteExternalServiceRole(ctx context.Context, externalServiceID string) error {
-	uid := extServiceRoleUID(externalServiceID)
+	uid := accesscontrol.PrefixedRoleUID(extServiceRoleName(externalServiceID))
-
 	return s.sql.WithDbSession(ctx, func(sess *db.Session) error {
 		stored, errGet := getRoleByUID(ctx, sess, uid)
 		if errGet != nil {
@@ -90,11 +77,12 @@ func (s *AccessControlStore) SaveExternalServiceRole(ctx context.Context, cmd ac
 }
 
 func genExternalServiceRole(cmd accesscontrol.SaveExternalServiceRoleCommand) accesscontrol.Role {
+	name := extServiceRoleName(cmd.ExternalServiceID)
 	role := accesscontrol.Role{
 		OrgID:       cmd.OrgID,
 		Version:     1,
-		Name:        extServiceRoleName(cmd.ExternalServiceID),
+		Name:        name,
-		UID:         extServiceRoleUID(cmd.ExternalServiceID),
+		UID:         accesscontrol.PrefixedRoleUID(name),
 		DisplayName: fmt.Sprintf("External Service %s Permissions", cmd.ExternalServiceID),
 		Description: fmt.Sprintf("External Service %s permissions", cmd.ExternalServiceID),
 		Group:       "External Service",

pkg/services/accesscontrol/database/externalservices_test.go

@@ -3,10 +3,8 @@ package database
 import (
 	"context"
 	// #nosec G505 Used only for generating a 160 bit hash, it's not used for security purposes
-	"crypto/sha1"
+
-	"encoding/hex"
 	"errors"
-	"fmt"
 	"testing"
 
 	"github.com/grafana/grafana/pkg/infra/db"
@@ -166,7 +164,7 @@ func TestAccessControlStore_SaveExternalServiceRole(t *testing.T) {
 				require.NoError(t, err)
 
 				errDBSession := s.sql.WithDbSession(ctx, func(sess *db.Session) error {
-					storedRole, err := getRoleByUID(ctx, sess, sha1Hash(fmt.Sprintf("externalservice_%s_permissions", tt.runs[i].cmd.ExternalServiceID)))
+					storedRole, err := getRoleByUID(ctx, sess, accesscontrol.PrefixedRoleUID(extServiceRoleName(tt.runs[i].cmd.ExternalServiceID)))
 					require.NoError(t, err)
 					require.NotNil(t, storedRole)
 					require.Equal(t, tt.runs[i].cmd.Global, storedRole.Global(), "Incorrect global state of the role")
@@ -253,7 +251,7 @@ func TestAccessControlStore_DeleteExternalServiceRole(t *testing.T) {
 			}
 			roleID := int64(-1)
 			err := s.sql.WithDbSession(ctx, func(sess *db.Session) error {
-				role, err := getRoleByUID(ctx, sess, extServiceRoleUID(tt.id))
+				role, err := getRoleByUID(ctx, sess, accesscontrol.PrefixedRoleUID(extServiceRoleName(tt.id)))
 				if err != nil && !errors.Is(err, accesscontrol.ErrRoleNotFound) {
 					return err
 				}
@@ -295,7 +293,7 @@ func TestAccessControlStore_DeleteExternalServiceRole(t *testing.T) {
 
 			// Role should be deleted
 			_ = s.sql.WithDbSession(ctx, func(sess *db.Session) error {
-				storedRole, err := getRoleByUID(ctx, sess, extServiceRoleUID(tt.id))
+				storedRole, err := getRoleByUID(ctx, sess, accesscontrol.PrefixedRoleUID(extServiceRoleName(tt.id)))
 				require.ErrorIs(t, err, accesscontrol.ErrRoleNotFound)
 				require.Nil(t, storedRole)
 				return nil
@@ -303,9 +301,3 @@ func TestAccessControlStore_DeleteExternalServiceRole(t *testing.T) {
 		})
 	}
 }
-
-func sha1Hash(text string) string {
-	h := sha1.New()
-	_, _ = h.Write([]byte(text))
-	return hex.EncodeToString(h.Sum(nil))
-}

pkg/services/accesscontrol/models.go

@@ -319,22 +319,9 @@ func (cmd *SaveExternalServiceRoleCommand) Validate() error {
 }
 
 const (
-	GlobalOrgID                  = 0
+	GlobalOrgID      = 0
-	FixedRolePrefix              = "fixed:"
-	FixedRoleUIDPrefix           = "fixed_"
-	ManagedRolePrefix            = "managed:"
-	BasicRolePrefix              = "basic:"
-	PluginRolePrefix             = "plugins:"
-	ExternalServiceRolePrefix    = "externalservice:"
-	BasicRoleUIDPrefix           = "basic_"
-	ExternalServiceRoleUIDPrefix = "externalservice_"
-	RoleGrafanaAdmin             = "Grafana Admin"
-
 	GeneralFolderUID = "general"
-
+	RoleGrafanaAdmin = "Grafana Admin"
-	// Basic Role None
-	BasicRoleNoneUID  = "basic_none"
-	BasicRoleNoneName = "basic:none"
 
 	// Permission actions
 

pkg/services/accesscontrol/roles.go

@@ -12,6 +12,24 @@ import (
 	"github.com/grafana/grafana/pkg/setting"
 )
 
+const (
+	BasicRolePrefix    = "basic:"
+	BasicRoleUIDPrefix = "basic_"
+
+	ExternalServiceRolePrefix    = "extsvc:"
+	ExternalServiceRoleUIDPrefix = "extsvc_"
+
+	FixedRolePrefix    = "fixed:"
+	FixedRoleUIDPrefix = "fixed_"
+
+	ManagedRolePrefix = "managed:"
+
+	PluginRolePrefix = "plugins:"
+
+	BasicRoleNoneUID  = "basic_none"
+	BasicRoleNoneName = "basic:none"
+)
+
 // Roles definition
 var (
 	ldapReaderRole = RoleDTO{
@@ -256,13 +274,16 @@ func ConcatPermissions(permissions ...[]Permission) []Permission {
 	return perms
 }
 
-// FixedRoleUID generates a UID of 34 bytes: "fixed_" + base64(sha1(roleName))
+// PrefixedRoleUID generates a uid from name with the same prefix.
-func FixedRoleUID(roleName string) string {
+// Generated uid is 28 bytes + length of prefix: <prefix>_base64(sha1(roleName))
+func PrefixedRoleUID(roleName string) string {
+	prefix := strings.Split(roleName, ":")[0] + "_"
+
 	// #nosec G505 Used only for generating a 160 bit hash, it's not used for security purposes
 	hasher := sha1.New()
 	hasher.Write([]byte(roleName))
 
-	return fmt.Sprintf("%s%s", FixedRoleUIDPrefix, base64.RawURLEncoding.EncodeToString(hasher.Sum(nil)))
+	return fmt.Sprintf("%s%s", prefix, base64.RawURLEncoding.EncodeToString(hasher.Sum(nil)))
 }
 
 // ValidateFixedRole errors when a fixed role does not match expected pattern