AccessControl: Add safety valve truncation for long user defined scopes (#79854)

* fix migrator bootloop by invalidating permissions

* add test for scope truncation

* lint

* fix max size scope

pkg/services/accesscontrol/migrator/migrator.go

@@ -15,6 +15,10 @@ var (
 	batchSize = 1000
 )
 
+const (
+	maxLen = 40
+)
+
 func MigrateScopeSplit(db db.DB, log log.Logger) error {
 	t := time.Now()
 	ctx := context.Background()
@@ -49,6 +53,12 @@ func MigrateScopeSplit(db db.DB, log log.Logger) error {
 		for i := start; i < end; i++ {
 			kind, attribute, identifier := permissions[i].SplitScope()
 
+			// Trim to max length to avoid bootloop.
+			// too long scopes will be truncated and the permission will become invalid.
+			kind = trimToMaxLen(kind, maxLen)
+			attribute = trimToMaxLen(attribute, maxLen)
+			identifier = trimToMaxLen(identifier, maxLen)
+
 			delQuery += "?,"
 			delArgs = append(delArgs, permissions[i].ID)
 
@@ -109,3 +119,10 @@ func batch(count, batchSize int, eachFn func(start, end int) error) error {
 
 	return nil
 }
+
+func trimToMaxLen(s string, maxLen int) string {
+	if len(s) > maxLen {
+		return s[:maxLen]
+	}
+	return s
+}

pkg/services/accesscontrol/migrator/migrator_test.go

@@ -3,14 +3,17 @@ package migrator
 import (
 	"context"
 	"fmt"
+	"strings"
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
 	"github.com/grafana/grafana/pkg/infra/db"
 	"github.com/grafana/grafana/pkg/infra/log"
 	ac "github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
-	"github.com/stretchr/testify/require"
 )
 
 func batchInsertPermissions(cnt int, sqlStore db.DB) error {
@@ -35,27 +38,45 @@ func batchInsertPermissions(cnt int, sqlStore db.DB) error {
 	})
 }
 
-func TestMigrateScopeSplit(t *testing.T) {
+// TestIntegrationMigrateScopeSplit tests the scope split migration
+// also tests the scope split truncation logic
+func TestIntegrationMigrateScopeSplitTruncation(t *testing.T) {
 	sqlStore := db.InitTestDB(t)
 	logger := log.New("accesscontrol.migrator.test")
 
+	batchSize = 20
 	// Populate permissions
 	require.NoError(t, batchInsertPermissions(3*batchSize, sqlStore), "could not insert permissions")
 
+	// Insert a permission with a scope longer than 240 characters
+	longScope := strings.Repeat("a", 60) + ":" + strings.Repeat("b", 60) + ":" + strings.Repeat("c", 60)
+	permission := ac.Permission{
+		RoleID:  1,
+		Action:  "action",
+		Scope:   longScope,
+		Created: time.Now(),
+		Updated: time.Now(),
+	}
+	require.NoError(t, sqlStore.WithDbSession(context.Background(), func(sess *db.Session) error {
+		_, err := sess.Insert(permission)
+		return err
+	}), "could not insert permission with long scope")
+
 	// Migrate
 	require.NoError(t, MigrateScopeSplit(sqlStore, logger))
 
 	// Check migration result
-	permissions := make([]ac.Permission, 0, 3*batchSize)
+	permissions := make([]ac.Permission, 0, 3*batchSize+1)
 	errFind := sqlStore.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
 		return sess.Find(&permissions)
 	})
 	require.NoError(t, errFind, "could not find permissions in store")
 
 	for i := range permissions {
-		require.Equal(t, fmt.Sprintf("resource:uid:%v", i+1), permissions[i].Scope, "scope should have been preserved")
-		require.Equal(t, "resource", permissions[i].Kind)
-		require.Equal(t, "uid", permissions[i].Attribute)
-		require.Equal(t, fmt.Sprintf("%v", i+1), permissions[i].Identifier)
+		if permissions[i].Scope == longScope {
+			assert.Equal(t, strings.Repeat("a", 40), permissions[i].Kind)
+			assert.Equal(t, strings.Repeat("b", 40), permissions[i].Attribute)
+			assert.Equal(t, strings.Repeat("c", 40), permissions[i].Identifier)
+		}
 	}
 }