ExtSvcAuth: Assign roles locally (#78669)

* ExtSvcAuth: Assign roles locally

* Fix test

* HandlePluginStateChanged in the OrgID

* Remove Global from command

* Use AssignmentOrgID instead of OrgID

* Remove unecessary test case
This commit is contained in:
Gabriel MABILLE
2023-11-29 12:12:30 +01:00
committed by GitHub
parent 5a6ac44902
commit 72d32eed27
8 changed files with 44 additions and 119 deletions
@@ -762,7 +762,7 @@ func TestService_SaveExternalServiceRole(t *testing.T) {
runs: []run{
{
cmd: accesscontrol.SaveExternalServiceRoleCommand{
OrgID: 2,
AssignmentOrgID: 2,
ServiceAccountID: 2,
ExternalServiceID: "App 1",
Permissions: []accesscontrol.Permission{{Action: "users:read", Scope: "users:id:1"}},
@@ -776,7 +776,7 @@ func TestService_SaveExternalServiceRole(t *testing.T) {
runs: []run{
{
cmd: accesscontrol.SaveExternalServiceRoleCommand{
Global: true,
AssignmentOrgID: 1,
ServiceAccountID: 2,
ExternalServiceID: "App 1",
Permissions: []accesscontrol.Permission{{Action: "users:read", Scope: "users:id:1"}},
@@ -785,7 +785,7 @@ func TestService_SaveExternalServiceRole(t *testing.T) {
},
{
cmd: accesscontrol.SaveExternalServiceRoleCommand{
Global: true,
AssignmentOrgID: 1,
ServiceAccountID: 2,
ExternalServiceID: "App 1",
Permissions: []accesscontrol.Permission{
@@ -802,7 +802,7 @@ func TestService_SaveExternalServiceRole(t *testing.T) {
runs: []run{
{
cmd: accesscontrol.SaveExternalServiceRoleCommand{
OrgID: 2,
AssignmentOrgID: 2,
ExternalServiceID: "App 1",
Permissions: []accesscontrol.Permission{{Action: "users:read", Scope: "users:id:1"}},
},
@@ -825,7 +825,7 @@ func TestService_SaveExternalServiceRole(t *testing.T) {
require.NoError(t, err)
// Check that the permissions and assignment are stored correctly
perms, errGetPerms := ac.getUserPermissions(ctx, &user.SignedInUser{OrgID: r.cmd.OrgID, UserID: 2}, accesscontrol.Options{})
perms, errGetPerms := ac.getUserPermissions(ctx, &user.SignedInUser{OrgID: r.cmd.AssignmentOrgID, UserID: 2}, accesscontrol.Options{})
require.NoError(t, errGetPerms)
assert.ElementsMatch(t, r.cmd.Permissions, perms)
}
@@ -848,7 +848,7 @@ func TestService_DeleteExternalServiceRole(t *testing.T) {
{
name: "handles deleting role that exists",
initCmd: &accesscontrol.SaveExternalServiceRoleCommand{
Global: true,
AssignmentOrgID: 1,
ServiceAccountID: 2,
ExternalServiceID: "App 1",
Permissions: []accesscontrol.Permission{{Action: "users:read", Scope: "users:id:1"}},
@@ -877,7 +877,7 @@ func TestService_DeleteExternalServiceRole(t *testing.T) {
if tt.initCmd != nil {
// Check that the permissions and assignment are removed correctly
perms, errGetPerms := ac.getUserPermissions(ctx, &user.SignedInUser{OrgID: tt.initCmd.OrgID, UserID: 2}, accesscontrol.Options{})
perms, errGetPerms := ac.getUserPermissions(ctx, &user.SignedInUser{OrgID: tt.initCmd.AssignmentOrgID, UserID: 2}, accesscontrol.Options{})
require.NoError(t, errGetPerms)
assert.Empty(t, perms)
}
@@ -79,7 +79,7 @@ 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,
OrgID: accesscontrol.GlobalOrgID, // External Service Roles are global
Version: 1,
Name: name,
UID: accesscontrol.PrefixedRoleUID(name),
@@ -90,21 +90,15 @@ func genExternalServiceRole(cmd accesscontrol.SaveExternalServiceRoleCommand) ac
Created: time.Now(),
Updated: time.Now(),
}
if cmd.Global {
role.OrgID = accesscontrol.GlobalOrgID
}
return role
}
func genExternalServiceAssignment(cmd accesscontrol.SaveExternalServiceRoleCommand) accesscontrol.UserRole {
assignment := accesscontrol.UserRole{
OrgID: cmd.OrgID,
OrgID: cmd.AssignmentOrgID,
UserID: cmd.ServiceAccountID,
Created: time.Now(),
}
if cmd.Global {
assignment.OrgID = accesscontrol.GlobalOrgID
}
return assignment
}
@@ -27,7 +27,7 @@ func TestAccessControlStore_SaveExternalServiceRole(t *testing.T) {
{
cmd: accesscontrol.SaveExternalServiceRoleCommand{
ExternalServiceID: "app1",
Global: true,
AssignmentOrgID: 1,
ServiceAccountID: 1,
Permissions: []accesscontrol.Permission{
{Action: "users:read", Scope: "users:id:1"},
@@ -44,7 +44,7 @@ func TestAccessControlStore_SaveExternalServiceRole(t *testing.T) {
{
cmd: accesscontrol.SaveExternalServiceRoleCommand{
ExternalServiceID: "app1",
Global: true,
AssignmentOrgID: 1,
ServiceAccountID: 1,
Permissions: []accesscontrol.Permission{
{Action: "users:read", Scope: "users:id:1"},
@@ -55,7 +55,7 @@ func TestAccessControlStore_SaveExternalServiceRole(t *testing.T) {
{
cmd: accesscontrol.SaveExternalServiceRoleCommand{
ExternalServiceID: "app1",
Global: true,
AssignmentOrgID: 1,
ServiceAccountID: 1,
Permissions: []accesscontrol.Permission{
{Action: "users:write", Scope: "users:id:1"},
@@ -65,51 +65,13 @@ func TestAccessControlStore_SaveExternalServiceRole(t *testing.T) {
},
},
},
{
name: "allow switching role from local to global and back",
runs: []run{
{
cmd: accesscontrol.SaveExternalServiceRoleCommand{
ExternalServiceID: "app1",
OrgID: 1,
ServiceAccountID: 1,
Permissions: []accesscontrol.Permission{
{Action: "users:read", Scope: "users:id:1"},
{Action: "users:read", Scope: "users:id:2"},
},
},
},
{
cmd: accesscontrol.SaveExternalServiceRoleCommand{
ExternalServiceID: "app1",
Global: true,
ServiceAccountID: 1,
Permissions: []accesscontrol.Permission{
{Action: "users:read", Scope: "users:id:1"},
{Action: "users:read", Scope: "users:id:2"},
},
},
},
{
cmd: accesscontrol.SaveExternalServiceRoleCommand{
ExternalServiceID: "app1",
OrgID: 1,
ServiceAccountID: 1,
Permissions: []accesscontrol.Permission{
{Action: "users:read", Scope: "users:id:1"},
{Action: "users:read", Scope: "users:id:2"},
},
},
},
},
},
{
name: "edge case - remove all permissions",
runs: []run{
{
cmd: accesscontrol.SaveExternalServiceRoleCommand{
ExternalServiceID: "app1",
Global: true,
AssignmentOrgID: 1,
ServiceAccountID: 1,
Permissions: []accesscontrol.Permission{
{Action: "users:read", Scope: "users:id:1"},
@@ -120,7 +82,7 @@ func TestAccessControlStore_SaveExternalServiceRole(t *testing.T) {
{
cmd: accesscontrol.SaveExternalServiceRoleCommand{
ExternalServiceID: "app1",
Global: true,
AssignmentOrgID: 1,
ServiceAccountID: 1,
Permissions: []accesscontrol.Permission{},
},
@@ -133,14 +95,14 @@ func TestAccessControlStore_SaveExternalServiceRole(t *testing.T) {
{
cmd: accesscontrol.SaveExternalServiceRoleCommand{
ExternalServiceID: "app1",
Global: true,
AssignmentOrgID: 1,
ServiceAccountID: 1,
},
},
{
cmd: accesscontrol.SaveExternalServiceRoleCommand{
ExternalServiceID: "app1",
Global: true,
AssignmentOrgID: 1,
ServiceAccountID: 2,
},
wantErr: true,
@@ -167,8 +129,7 @@ func TestAccessControlStore_SaveExternalServiceRole(t *testing.T) {
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")
require.Equal(t, tt.runs[i].cmd.OrgID, storedRole.OrgID, "Incorrect OrgID of the role")
require.True(t, storedRole.Global(), "Incorrect global state of the role")
storedPerm, err := getRolePermissions(ctx, sess, storedRole.ID)
require.NoError(t, err)
@@ -181,8 +142,7 @@ func TestAccessControlStore_SaveExternalServiceRole(t *testing.T) {
has, err := sess.Where("role_id = ? AND user_id = ?", storedRole.ID, tt.runs[i].cmd.ServiceAccountID).Get(&assignment)
require.NoError(t, err)
require.True(t, has)
require.Equal(t, tt.runs[i].cmd.Global, assignment.OrgID == accesscontrol.GlobalOrgID, "Incorrect global state of the assignment")
require.Equal(t, tt.runs[i].cmd.OrgID, assignment.OrgID, "Incorrect OrgID for the role assignment")
require.Equal(t, tt.runs[i].cmd.AssignmentOrgID, assignment.OrgID, "Incorrect OrgID for the role assignment")
return nil
})
@@ -206,27 +166,10 @@ func TestAccessControlStore_DeleteExternalServiceRole(t *testing.T) {
wantErr: false,
},
{
name: "delete local role",
name: "delete role",
init: func(t *testing.T, ctx context.Context, s *AccessControlStore) {
errSave := s.SaveExternalServiceRole(ctx, accesscontrol.SaveExternalServiceRoleCommand{
OrgID: 2,
ExternalServiceID: extID,
ServiceAccountID: 3,
Permissions: []accesscontrol.Permission{
{Action: "users:read", Scope: "users:id:1"},
{Action: "users:write", Scope: "users:id:1"},
},
})
require.NoError(t, errSave)
},
id: extID,
wantErr: false,
},
{
name: "delete global role",
init: func(t *testing.T, ctx context.Context, s *AccessControlStore) {
errSave := s.SaveExternalServiceRole(ctx, accesscontrol.SaveExternalServiceRoleCommand{
Global: true,
AssignmentOrgID: 2,
ExternalServiceID: extID,
ServiceAccountID: 3,
Permissions: []accesscontrol.Permission{
+1 -6
View File
@@ -274,8 +274,7 @@ type SetResourcePermissionCommand struct {
}
type SaveExternalServiceRoleCommand struct {
OrgID int64
Global bool
AssignmentOrgID int64
ExternalServiceID string
ServiceAccountID int64
Permissions []Permission
@@ -289,10 +288,6 @@ func (cmd *SaveExternalServiceRoleCommand) Validate() error {
// slugify the external service id ID for the role to have correct name and uid
cmd.ExternalServiceID = slugify.Slugify(cmd.ExternalServiceID)
if (cmd.OrgID == GlobalOrgID) != cmd.Global {
return fmt.Errorf("invalid org id %d for global role %t", cmd.OrgID, cmd.Global)
}
// Check and deduplicate permissions
if cmd.Permissions == nil || len(cmd.Permissions) == 0 {
return errors.New("no permissions provided")
+6 -17
View File
@@ -15,21 +15,10 @@ func TestSaveExternalServiceRoleCommand_Validate(t *testing.T) {
wantPermissions []Permission
wantErr bool
}{
{
name: "invalid global statement",
cmd: SaveExternalServiceRoleCommand{
OrgID: 1,
Global: true,
ExternalServiceID: "app 1",
ServiceAccountID: 2,
Permissions: []Permission{{Action: "users:read", Scope: "users:id:1"}},
},
wantErr: true,
},
{
name: "invalid no permissions",
cmd: SaveExternalServiceRoleCommand{
OrgID: 1,
AssignmentOrgID: 1,
ExternalServiceID: "app 1",
ServiceAccountID: 2,
Permissions: []Permission{},
@@ -39,7 +28,7 @@ func TestSaveExternalServiceRoleCommand_Validate(t *testing.T) {
{
name: "invalid service account id",
cmd: SaveExternalServiceRoleCommand{
OrgID: 1,
AssignmentOrgID: 1,
ExternalServiceID: "app 1",
ServiceAccountID: -1,
Permissions: []Permission{{Action: "users:read", Scope: "users:id:1"}},
@@ -49,7 +38,7 @@ func TestSaveExternalServiceRoleCommand_Validate(t *testing.T) {
{
name: "invalid no Ext Service ID",
cmd: SaveExternalServiceRoleCommand{
OrgID: 1,
AssignmentOrgID: 1,
ServiceAccountID: 2,
Permissions: []Permission{{Action: "users:read", Scope: "users:id:1"}},
},
@@ -59,7 +48,7 @@ func TestSaveExternalServiceRoleCommand_Validate(t *testing.T) {
name: "slugify the external service ID correctly",
cmd: SaveExternalServiceRoleCommand{
ExternalServiceID: "ThisIs a Very Strange ___ App Name?",
Global: true,
AssignmentOrgID: 1,
ServiceAccountID: 2,
Permissions: []Permission{{Action: "users:read", Scope: "users:id:1"}},
},
@@ -69,7 +58,7 @@ func TestSaveExternalServiceRoleCommand_Validate(t *testing.T) {
{
name: "invalid empty Action",
cmd: SaveExternalServiceRoleCommand{
OrgID: 1,
AssignmentOrgID: 1,
ExternalServiceID: "app 1",
ServiceAccountID: 2,
Permissions: []Permission{{Action: "", Scope: "users:id:1"}},
@@ -80,7 +69,7 @@ func TestSaveExternalServiceRoleCommand_Validate(t *testing.T) {
{
name: "permission deduplication",
cmd: SaveExternalServiceRoleCommand{
OrgID: 1,
AssignmentOrgID: 1,
ExternalServiceID: "app 1",
ServiceAccountID: 2,
Permissions: []Permission{