Extsvcacc: Split permission scope (#111491)

* Extsvcacc: Split permission scope

* Fix integration test

* Trigger CI/CD pipeline

* Change extsvc permission comparing

* Recreate unsplit permissions
This commit is contained in:
Gabriel MABILLE
2025-09-24 13:25:44 +02:00
committed by GitHub
parent b63ba0269f
commit a98870f8f9
5 changed files with 125 additions and 12 deletions
@@ -148,7 +148,15 @@ func getRolePermissions(ctx context.Context, sess *db.Session, id int64) ([]acce
func permissionDiff(previous, new []accesscontrol.Permission) (added, removed []accesscontrol.Permission) {
type key struct{ Action, Scope string }
prevMap := map[key]int64{}
unSplit := map[key]int64{}
for i := range previous {
// FIXME: This can be removed after a few releases.
// Previously, external service accounts scopes weren't split.
// We need to remove any unsplit permissions.
if previous[i].Scope != "" && previous[i].Kind == "" {
unSplit[key{previous[i].Action, previous[i].Scope}] = previous[i].ID
continue
}
prevMap[key{previous[i].Action, previous[i].Scope}] = previous[i].ID
}
newMap := map[key]int64{}
@@ -168,6 +176,12 @@ func permissionDiff(previous, new []accesscontrol.Permission) (added, removed []
removed = append(removed, accesscontrol.Permission{ID: id, Action: p.Action, Scope: p.Scope})
}
// FIXME: This can be removed after a few releases.
// Remove any unsplit permissions
for p, id := range unSplit {
removed = append(removed, accesscontrol.Permission{ID: id, Action: p.Action, Scope: p.Scope})
}
return added, removed
}
@@ -204,16 +218,6 @@ func (*AccessControlStore) savePermissions(ctx context.Context, sess *db.Session
return err
}
added, removed := permissionDiff(storedPermissions, permissions)
if len(added) > 0 {
for i := range added {
added[i].RoleID = roleID
added[i].Created = now
added[i].Updated = now
}
if _, err := sess.Insert(&added); err != nil {
return err
}
}
if len(removed) > 0 {
ids := make([]int64, len(removed))
for i := range removed {
@@ -227,6 +231,16 @@ func (*AccessControlStore) savePermissions(ctx context.Context, sess *db.Session
return errors.New("failed to delete permissions that have been removed from role")
}
}
if len(added) > 0 {
for i := range added {
added[i].RoleID = roleID
added[i].Created = now
added[i].Updated = now
}
if _, err := sess.Insert(&added); err != nil {
return err
}
}
return nil
}
@@ -249,3 +249,95 @@ func TestIntegrationAccessControlStore_DeleteExternalServiceRole(t *testing.T) {
})
}
}
func Test_permissionDiff(t *testing.T) {
tests := []struct {
name string
previous []accesscontrol.Permission
new []accesscontrol.Permission
wantAdded []accesscontrol.Permission
wantRemoved []accesscontrol.Permission
}{
{
name: "no changes",
previous: []accesscontrol.Permission{
{ID: 1, Action: "users:read", Scope: "users:id:1", Kind: "users", Attribute: "id", Identifier: "1"},
{ID: 2, Action: "users:write", Scope: "users:id:2", Kind: "users", Attribute: "id", Identifier: "2"},
},
new: []accesscontrol.Permission{
{Action: "users:read", Scope: "users:id:1", Kind: "users", Attribute: "id", Identifier: "1"},
{Action: "users:write", Scope: "users:id:2", Kind: "users", Attribute: "id", Identifier: "2"},
},
wantAdded: []accesscontrol.Permission{},
wantRemoved: []accesscontrol.Permission{},
},
{
name: "add new permissions",
previous: []accesscontrol.Permission{
{ID: 1, Action: "users:read", Scope: "users:id:1", Kind: "users", Attribute: "id", Identifier: "1"},
},
new: []accesscontrol.Permission{
{Action: "users:read", Scope: "users:id:1", Kind: "users", Attribute: "id", Identifier: "1"},
{Action: "users:write", Scope: "users:id:2", Kind: "users", Attribute: "id", Identifier: "2"},
},
wantAdded: []accesscontrol.Permission{
{Action: "users:write", Scope: "users:id:2", Kind: "users", Attribute: "id", Identifier: "2"},
},
wantRemoved: []accesscontrol.Permission{},
},
{
name: "remove existing permissions",
previous: []accesscontrol.Permission{
{ID: 1, Action: "users:read", Scope: "users:id:1", Kind: "users", Attribute: "id", Identifier: "1"},
{ID: 2, Action: "users:write", Scope: "users:id:2", Kind: "users", Attribute: "id", Identifier: "2"},
},
new: []accesscontrol.Permission{
{Action: "users:read", Scope: "users:id:1", Kind: "users", Attribute: "id", Identifier: "1"},
},
wantAdded: []accesscontrol.Permission{},
wantRemoved: []accesscontrol.Permission{
{ID: 2, Action: "users:write", Scope: "users:id:2"},
},
},
{
name: "add and remove permissions",
previous: []accesscontrol.Permission{
{ID: 1, Action: "users:read", Scope: "users:id:1", Kind: "users", Attribute: "id", Identifier: "1"},
{ID: 2, Action: "users:write", Scope: "users:id:2", Kind: "users", Attribute: "id", Identifier: "2"},
},
new: []accesscontrol.Permission{
{Action: "users:read", Scope: "users:id:1", Kind: "users", Attribute: "id", Identifier: "1"},
{Action: "users:delete", Scope: "users:id:3", Kind: "users", Attribute: "id", Identifier: "3"},
},
wantAdded: []accesscontrol.Permission{
{Action: "users:delete", Scope: "users:id:3", Kind: "users", Attribute: "id", Identifier: "3"},
},
wantRemoved: []accesscontrol.Permission{
{ID: 2, Action: "users:write", Scope: "users:id:2"},
},
},
{
name: "recreate unsplit permissions",
previous: []accesscontrol.Permission{
{ID: 1, Action: "users:read", Scope: "users:id:1"},
},
new: []accesscontrol.Permission{
{Action: "users:read", Scope: "users:id:1", Kind: "users", Attribute: "id", Identifier: "1"},
},
wantAdded: []accesscontrol.Permission{
{Action: "users:read", Scope: "users:id:1", Kind: "users", Attribute: "id", Identifier: "1"},
},
wantRemoved: []accesscontrol.Permission{
{ID: 1, Action: "users:read", Scope: "users:id:1"},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gotAdded, gotRemoved := permissionDiff(tt.previous, tt.new)
require.ElementsMatch(t, tt.wantAdded, gotAdded, "added permissions do not match")
require.ElementsMatch(t, tt.wantRemoved, gotRemoved, "removed permissions do not match")
})
}
}