Revert "AccessControl: Implement a way to register fixed roles (#35641)" (#37397)

This reverts commit 88c11f1cc0.

pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go

@@ -15,10 +15,9 @@ import (
 
 // OSSAccessControlService is the service implementing role based access control.
 type OSSAccessControlService struct {
-	Cfg           *setting.Cfg          `inject:""`
-	UsageStats    usagestats.UsageStats `inject:""`
-	Log           log.Logger
-	registrations accesscontrol.RegistrationList
+	Cfg        *setting.Cfg          `inject:""`
+	UsageStats usagestats.UsageStats `inject:""`
+	Log        log.Logger
 }
 
 // Init initializes the OSSAccessControlService.
@@ -70,11 +69,11 @@ func (ac *OSSAccessControlService) GetUserPermissions(ctx context.Context, user
 	for _, builtin := range builtinRoles {
 		if roleNames, ok := accesscontrol.FixedRoleGrants[builtin]; ok {
 			for _, name := range roleNames {
-				role, exists := accesscontrol.FixedRoles[name]
+				r, exists := accesscontrol.FixedRoles[name]
 				if !exists {
 					continue
 				}
-				for _, p := range role.Permissions {
+				for _, p := range r.Permissions {
 					permission := p
 					permissions = append(permissions, &permission)
 				}
@@ -96,82 +95,3 @@ func (ac *OSSAccessControlService) GetUserBuiltInRoles(user *models.SignedInUser
 
 	return roles
 }
-
-func (ac *OSSAccessControlService) saveFixedRole(role accesscontrol.RoleDTO) {
-	if storedRole, ok := accesscontrol.FixedRoles[role.Name]; ok {
-		// If a package wants to override another package's role, the version
-		// needs to be increased. Hence, we don't overwrite a role with a
-		// greater version.
-		if storedRole.Version >= role.Version {
-			log.Debugf("role %v has already been stored in a greater version, skipping registration", role.Name)
-			return
-		}
-	}
-	// Save role
-	accesscontrol.FixedRoles[role.Name] = role
-}
-
-func (ac *OSSAccessControlService) assignFixedRole(role accesscontrol.RoleDTO, builtInRoles []string) {
-	for _, builtInRole := range builtInRoles {
-		// Only record new assignments
-		alreadyAssigned := false
-		assignments, ok := accesscontrol.FixedRoleGrants[builtInRole]
-		if ok {
-			for _, assignedRole := range assignments {
-				if assignedRole == role.Name {
-					log.Debugf("role %v has already been assigned to %v", role.Name, builtInRole)
-					alreadyAssigned = true
-				}
-			}
-		}
-		if !alreadyAssigned {
-			assignments = append(assignments, role.Name)
-			accesscontrol.FixedRoleGrants[builtInRole] = assignments
-		}
-	}
-}
-
-// RegisterFixedRoles registers all declared roles in RAM
-func (ac *OSSAccessControlService) RegisterFixedRoles() error {
-	// If accesscontrol is disabled no need to register roles
-	if ac.IsDisabled() {
-		return nil
-	}
-	var err error
-	ac.registrations.Range(func(registration accesscontrol.RoleRegistration) bool {
-		ac.registerFixedRole(registration.Role, registration.Grants)
-		return true
-	})
-	return err
-}
-
-// RegisterFixedRole saves a fixed role and assigns it to built-in roles
-func (ac *OSSAccessControlService) registerFixedRole(role accesscontrol.RoleDTO, builtInRoles []string) {
-	ac.saveFixedRole(role)
-	ac.assignFixedRole(role, builtInRoles)
-}
-
-// DeclareFixedRoles allow the caller to declare, to the service, fixed roles and their assignments
-// to organization roles ("Viewer", "Editor", "Admin") or "Grafana Admin"
-func (ac *OSSAccessControlService) DeclareFixedRoles(registrations ...accesscontrol.RoleRegistration) error {
-	// If accesscontrol is disabled no need to register roles
-	if ac.IsDisabled() {
-		return nil
-	}
-
-	for _, r := range registrations {
-		err := accesscontrol.ValidateFixedRole(r.Role)
-		if err != nil {
-			return err
-		}
-
-		err = accesscontrol.ValidateBuiltInRoles(r.Grants)
-		if err != nil {
-			return err
-		}
-
-		ac.registrations.Append(r)
-	}
-
-	return nil
-}

pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol_test.go

@@ -2,7 +2,6 @@ package ossaccesscontrol
 
 import (
 	"context"
-	"fmt"
 	"testing"
 
 	"github.com/grafana/grafana/pkg/infra/log"
@@ -32,28 +31,6 @@ func setupTestEnv(t testing.TB) *OSSAccessControlService {
 	return &ac
 }
 
-func removeRoleHelper(role string) {
-	delete(accesscontrol.FixedRoles, role)
-
-	// Compute new grants removing any appearance of the role in the list
-	replaceGrants := map[string][]string{}
-
-	for builtInRole, grants := range accesscontrol.FixedRoleGrants {
-		newGrants := make([]string, len(grants))
-		for _, r := range grants {
-			if r != role {
-				newGrants = append(newGrants, r)
-			}
-		}
-		replaceGrants[builtInRole] = newGrants
-	}
-
-	// Replace grants
-	for br, grants := range replaceGrants {
-		accesscontrol.FixedRoleGrants[br] = grants
-	}
-}
-
 type usageStatsMock struct {
 	t            *testing.T
 	metricsFuncs []usagestats.MetricsFunc
@@ -189,349 +166,3 @@ func TestUsageMetrics(t *testing.T) {
 		})
 	}
 }
-
-type assignmentTestCase struct {
-	role         accesscontrol.RoleDTO
-	builtInRoles []string
-}
-
-func TestOSSAccessControlService_RegisterFixedRole(t *testing.T) {
-	tests := []struct {
-		name string
-		runs []assignmentTestCase
-	}{
-		{
-			name: "Successfully register role no assignments",
-			runs: []assignmentTestCase{
-				{
-					role: accesscontrol.RoleDTO{
-						Version: 1,
-						Name:    "fixed:test:test",
-					},
-				},
-			},
-		},
-		{
-			name: "Successfully ignore overwriting existing role",
-			runs: []assignmentTestCase{
-				{
-					role: accesscontrol.RoleDTO{
-						Version: 1,
-						Name:    "fixed:test:test",
-					},
-				},
-				{
-					role: accesscontrol.RoleDTO{
-						Version: 1,
-						Name:    "fixed:test:test",
-					},
-				},
-			},
-		},
-		{
-			name: "Successfully register and assign role",
-			runs: []assignmentTestCase{
-				{
-					role: accesscontrol.RoleDTO{
-						Version: 1,
-						Name:    "fixed:test:test",
-					},
-					builtInRoles: []string{"Viewer", "Editor", "Admin"},
-				},
-			},
-		},
-		{
-			name: "Successfully ignore unchanged assignment",
-			runs: []assignmentTestCase{
-				{
-					role: accesscontrol.RoleDTO{
-						Version: 1,
-						Name:    "fixed:test:test",
-					},
-					builtInRoles: []string{"Viewer"},
-				},
-				{
-					role: accesscontrol.RoleDTO{
-						Version: 2,
-						Name:    "fixed:test:test",
-					},
-					builtInRoles: []string{"Viewer"},
-				},
-			},
-		},
-		{
-			name: "Successfully add a new assignment",
-			runs: []assignmentTestCase{
-				{
-					role: accesscontrol.RoleDTO{
-						Version: 1,
-						Name:    "fixed:test:test",
-					},
-					builtInRoles: []string{"Viewer"},
-				},
-				{
-					role: accesscontrol.RoleDTO{
-						Version: 1,
-						Name:    "fixed:test:test",
-					},
-					builtInRoles: []string{"Editor"},
-				},
-			},
-		},
-	}
-
-	// Check all runs performed so far to get the number of assignments seeder
-	// should have recorded
-	getTotalAssignCount := func(curRunIdx int, runs []assignmentTestCase) int {
-		builtIns := map[string]struct{}{}
-		for i := 0; i < curRunIdx+1; i++ {
-			for _, br := range runs[i].builtInRoles {
-				builtIns[br] = struct{}{}
-			}
-		}
-		return len(builtIns)
-	}
-
-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			ac := &OSSAccessControlService{
-				Cfg:        setting.NewCfg(),
-				UsageStats: &usageStatsMock{t: t, metricsFuncs: make([]usagestats.MetricsFunc, 0)},
-				Log:        log.New("accesscontrol-test"),
-			}
-
-			for i, run := range tc.runs {
-				// Remove any inserted role after the test case has been run
-				t.Cleanup(func() { removeRoleHelper(run.role.Name) })
-
-				ac.registerFixedRole(run.role, run.builtInRoles)
-
-				// Check role has been registered
-				storedRole, ok := accesscontrol.FixedRoles[run.role.Name]
-				assert.True(t, ok, "role should have been registered")
-
-				// Check registered role has not been altered
-				assert.Equal(t, run.role, storedRole, "role should not have been altered")
-
-				// Check assignments
-				// Count number of times the role has been assigned
-				assignCnt := 0
-				for _, grants := range accesscontrol.FixedRoleGrants {
-					for _, r := range grants {
-						if r == run.role.Name {
-							assignCnt++
-						}
-					}
-				}
-				assert.Equal(t, getTotalAssignCount(i, tc.runs), assignCnt,
-					"assignments should only be added, never removed")
-
-				for _, br := range run.builtInRoles {
-					assigns, ok := accesscontrol.FixedRoleGrants[br]
-					assert.True(t, ok,
-						fmt.Sprintf("role %s should have been assigned to %s", run.role.Name, br))
-					assert.Contains(t, assigns, run.role.Name,
-						fmt.Sprintf("role %s should have been assigned to %s", run.role.Name, br))
-				}
-			}
-		})
-	}
-}
-
-func TestOSSAccessControlService_DeclareFixedRoles(t *testing.T) {
-	tests := []struct {
-		name          string
-		registrations []accesscontrol.RoleRegistration
-		wantErr       bool
-		err           error
-	}{
-		{
-			name:    "should work with empty list",
-			wantErr: false,
-		},
-		{
-			name: "should add registration",
-			registrations: []accesscontrol.RoleRegistration{
-				{
-					Role: accesscontrol.RoleDTO{
-						Version: 1,
-						Name:    "fixed:test:test",
-					},
-					Grants: []string{"Admin"},
-				},
-			},
-			wantErr: false,
-		},
-		{
-			name: "should fail registration invalid role name",
-			registrations: []accesscontrol.RoleRegistration{
-				{
-					Role: accesscontrol.RoleDTO{
-						Version: 1,
-						Name:    "custom:test:test",
-					},
-					Grants: []string{"Admin"},
-				},
-			},
-			wantErr: true,
-			err:     accesscontrol.ErrFixedRolePrefixMissing,
-		},
-		{
-			name: "should fail registration invalid builtin role assignment",
-			registrations: []accesscontrol.RoleRegistration{
-				{
-					Role: accesscontrol.RoleDTO{
-						Version: 1,
-						Name:    "fixed:test:test",
-					},
-					Grants: []string{"WrongAdmin"},
-				},
-			},
-			wantErr: true,
-			err:     accesscontrol.ErrInvalidBuiltinRole,
-		},
-		{
-			name: "should add multiple registrations at once",
-			registrations: []accesscontrol.RoleRegistration{
-				{
-					Role: accesscontrol.RoleDTO{
-						Version: 1,
-						Name:    "fixed:test:test",
-					},
-					Grants: []string{"Admin"},
-				},
-				{
-					Role: accesscontrol.RoleDTO{
-						Version: 1,
-						Name:    "fixed:test2:test2",
-					},
-					Grants: []string{"Admin"},
-				},
-			},
-			wantErr: false,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			ac := &OSSAccessControlService{
-				Cfg:           setting.NewCfg(),
-				UsageStats:    &usageStatsMock{t: t, metricsFuncs: make([]usagestats.MetricsFunc, 0)},
-				Log:           log.New("accesscontrol-test"),
-				registrations: accesscontrol.RegistrationList{},
-			}
-			ac.Cfg.FeatureToggles = map[string]bool{"accesscontrol": true}
-
-			// Test
-			err := ac.DeclareFixedRoles(tt.registrations...)
-			if tt.wantErr {
-				require.Error(t, err)
-				assert.ErrorIs(t, err, tt.err)
-				return
-			}
-			require.NoError(t, err)
-
-			registrationCnt := 0
-			ac.registrations.Range(func(registration accesscontrol.RoleRegistration) bool {
-				registrationCnt++
-				return true
-			})
-			assert.Equal(t, len(tt.registrations), registrationCnt,
-				"expected service registration list to contain all test registrations")
-		})
-	}
-}
-
-func TestOSSAccessControlService_RegisterFixedRoles(t *testing.T) {
-	tests := []struct {
-		name          string
-		token         models.Licensing
-		registrations []accesscontrol.RoleRegistration
-		wantErr       bool
-	}{
-		{
-			name: "should work with empty list",
-		},
-		{
-			name: "should register and assign role",
-			registrations: []accesscontrol.RoleRegistration{
-				{
-					Role: accesscontrol.RoleDTO{
-						Version: 1,
-						Name:    "fixed:test:test",
-					},
-					Grants: []string{"Admin"},
-				},
-			},
-			wantErr: false,
-		},
-		{
-			name: "should register and assign multiple roles",
-			registrations: []accesscontrol.RoleRegistration{
-				{
-					Role: accesscontrol.RoleDTO{
-						Version: 1,
-						Name:    "fixed:test:test",
-					},
-					Grants: []string{"Admin"},
-				},
-				{
-					Role: accesscontrol.RoleDTO{
-						Version: 1,
-						Name:    "fixed:test2:test2",
-					},
-					Grants: []string{"Admin"},
-				},
-			},
-			wantErr: false,
-		},
-	}
-
-	for _, tt := range tests {
-		cfg := setting.NewCfg()
-		cfg.FeatureToggles = map[string]bool{"accesscontrol": true}
-
-		t.Run(tt.name, func(t *testing.T) {
-			// Remove any inserted role after the test case has been run
-			t.Cleanup(func() {
-				for _, registration := range tt.registrations {
-					removeRoleHelper(registration.Role.Name)
-				}
-			})
-
-			// Setup
-			ac := &OSSAccessControlService{
-				Cfg:           setting.NewCfg(),
-				UsageStats:    &usageStatsMock{t: t, metricsFuncs: make([]usagestats.MetricsFunc, 0)},
-				Log:           log.New("accesscontrol-test"),
-				registrations: accesscontrol.RegistrationList{},
-			}
-			ac.Cfg.FeatureToggles = map[string]bool{"accesscontrol": true}
-			ac.registrations.Append(tt.registrations...)
-
-			// Test
-			err := ac.RegisterFixedRoles()
-			if tt.wantErr {
-				require.Error(t, err)
-				return
-			}
-			require.NoError(t, err)
-
-			// Check
-			for _, registration := range tt.registrations {
-				role, ok := accesscontrol.FixedRoles[registration.Role.Name]
-				assert.True(t, ok,
-					fmt.Sprintf("role %s should have been registered", registration.Role.Name))
-				assert.NotNil(t, role,
-					fmt.Sprintf("role %s should have been registered", registration.Role.Name))
-
-				for _, br := range registration.Grants {
-					rolesWithGrant, ok := accesscontrol.FixedRoleGrants[br]
-					assert.True(t, ok,
-						fmt.Sprintf("role %s should have been assigned to %s", registration.Role.Name, br))
-					assert.Contains(t, rolesWithGrant, registration.Role.Name,
-						fmt.Sprintf("role %s should have been assigned to %s", registration.Role.Name, br))
-				}
-			}
-		})
-	}
-}