CyberCoveLLC/threshold
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

ListResolvableAliasesPermissions Hotfix (#5673)

Browse Source 
* ICU-16921 Fix ListResolvableAliasesPermissions where function skips some permission building when all children scope is provided, this currently results in skipping subsequent direct grants. Also, update listResolvableAliasesRepo to return early with no data when splitting permissions yields no relevant permissions.

* Remove conditional split permission check for listResolvableAliasesRefresh and listRemovedResolvableAliasIds as condition is never reachable. Additionally remove unecessary grants being given to list targets as part of TestACL_ListResolvableAliasesPermissions.

* Remove additonal 'type=target;actions=list' grants as part of TestACL_ListResolvableAliasesPermissions

* Update Switch Statement Too Factor For Edge Case With Children Granted With Role Scope Global

* Resolve comment to refactor error return in listResolvableAliases to instead return no data and document why and when this can occur.

* Add unit tests to cover case where provided permissions to ListResolvableAliases service contains no grant scopes where targets can reside.
This commit is contained in:
Ryan Derr authored and GitHub committed 2025-05-02 13:12:32 -05:00
1 parent 3429014e70
commit 093226a057
4 files changed
+222 -23
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

internal/alias/target/repository_alias_list_resolvable.go
+5 -1
View File
@@ -115,8 +115,12 @@ func (r *Repository) listResolvableAliases(ctx context.Context, permissions []pe
destinationIdClauses = append(destinationIdClauses, "destination_id in (select public_id from target where project_id = any(@target_scope_ids))")
args = append(args, sql.Named("target_scope_ids", "{"+strings.Join(directScopeIds, ",")+"}"))
}
// This condition checks if there are no destinations target Ids and no child scopes,
// while also ensuring that the "allDescendants" flag is not set, if so we return no data.
// An example scenario of when this can happen is when a role on the global scope grants a user
// access to list aliases and read targets, but only within the global scope and its immediate children.
if len(destinationIdClauses) == 0 && len(childAllScopes) == 0 {
return nil, time.Time{}, errors.New(ctx, errors.InvalidParameter, op, "no target ids or scope ids provided")
return nil, time.Time{}, nil
}
whereClause = fmt.Sprintf("destination_id is not null and (%s)", strings.Join(destinationIdClauses, " or "))
}
internal/alias/target/service_list_resolvable_ext_test.go
+28
View File
@@ -88,6 +88,27 @@ func TestService_ListResolvableAliases(t *testing.T) {
All: true,
},
}
// Test the scenario where permissions are provided to the ListResolvableAliases service,
// but none of the permissions include grant scopes corresponding to scopes of which targets reside.
noTargetScopePerms := []perms.Permission{
{
RoleScopeId: scope.Global.String(),
GrantScopeId: globals.GrantScopeChildren,
Resource: resource.Target,
Action: action.ListResolvableAliases,
OnlySelf: false,
All: true,
},
{
RoleScopeId: scope.Global.String(),
GrantScopeId: scope.Global.String(),
Resource: resource.Target,
Action: action.ListResolvableAliases,
OnlySelf: false,
All: true,
},
}
// Reverse since we read items in descending order (newest first)
slices.Reverse(byScopeResources)
@@ -159,6 +180,13 @@ func TestService_ListResolvableAliases(t *testing.T) {
_, err := target.ListResolvableAliases(ctx, []byte("some hash"), 1, repo, nil)
require.ErrorContains(t, err, "missing target permissions")
})
t.Run("no valid target permissions", func(t *testing.T) {
t.Parallel()
res, err := target.ListResolvableAliases(ctx, []byte("some hash"), 1, repo, noTargetScopePerms)
require.NoError(t, err)
require.NotNil(t, res)
require.Empty(t, res.Items)
})
})
t.Run("ListPage validation", func(t *testing.T) {
t.Parallel()
internal/perms/acl.go
+8 -8
View File
@@ -468,21 +468,21 @@ func (a ACL) ListResolvableAliasesPermissions(requestedType resource.Type, actio
// so skip it
continue
}
default:
// Since direct grants must be the same scope or downstream, the
// only possibility left for a children grant is that the parent is
// global and the grant is on the org -- if it was for projects it
// would need to be a descendants grant
if _, ok := childrenScopes[scope.Global.String()]; ok {
// We already looked at this scope in the children grants, so skip it
case p.RoleScopeId == scope.Global.String() && strings.HasPrefix(grantScopeId, scope.Org.Prefix()):
// Handle the case where the parent scope is global, and the grant is at the org level.
// Direct grants must either match the parent scope or be downstream. This condition
// accounts for a scenario where a child grant exists at the org level while the parent
// is global. If the grant were for projects, it would require a descendants grant instead.
// Skip processing if the current scope is already accounted for in the childrenScopes map.
if _, ok := childrenScopes[p.RoleScopeId]; ok {
continue
}
}
if a.buildPermission(&scopes.ScopeInfo{Id: grantScopeId}, requestedType, actions, false, &p) {
perms = append(perms, p)
}
}
return perms
}
internal/perms/acl_test.go
+181 -14
View File
@@ -830,7 +830,6 @@ func TestACL_ListResolvableAliasesPermissions(t *testing.T) {
roleScope: "global",
grantScope: globals.GrantScopeDescendants,
grants: []string{
"type=target;actions=list",
"ids=ttcp_1234567890;actions=read",
},
},
@@ -856,7 +855,6 @@ func TestACL_ListResolvableAliasesPermissions(t *testing.T) {
roleScope: "global",
grantScope: globals.GrantScopeDescendants,
grants: []string{
"type=target;actions=list",
"ids=ttcp_1234567890;actions=read",
},
},
@@ -864,12 +862,20 @@ func TestACL_ListResolvableAliasesPermissions(t *testing.T) {
roleScope: "global",
grantScope: "global",
grants: []string{
"type=target;actions=list",
"ids=ttcp_1234567890;actions=read",
},
},
},
expPermissions: []Permission{
{
RoleScopeId: scope.Global.String(),
GrantScopeId: scope.Global.String(),
Resource: resource.Target,
Action: action.ListResolvableAliases,
ResourceIds: []string{"ttcp_1234567890"},
All: false,
OnlySelf: false,
},
{
RoleScopeId: scope.Global.String(),
GrantScopeId: globals.GrantScopeDescendants,
@@ -879,6 +885,22 @@ func TestACL_ListResolvableAliasesPermissions(t *testing.T) {
All: false,
OnlySelf: false,
},
},
},
{
name: "global_with_this_no_descendants",
resourceType: resource.Target,
actionSet: action.NewActionSet(action.Read, action.Cancel),
aclGrants: []scopeGrant{
{
roleScope: "global",
grantScope: "global",
grants: []string{
"ids=ttcp_1234567890;actions=read",
},
},
},
expPermissions: []Permission{
{
RoleScopeId: scope.Global.String(),
GrantScopeId: scope.Global.String(),
@@ -899,7 +921,6 @@ func TestACL_ListResolvableAliasesPermissions(t *testing.T) {
roleScope: "global",
grantScope: globals.GrantScopeChildren,
grants: []string{
"type=target;actions=list",
"ids=ttcp_1234567890;actions=read",
},
},
@@ -916,6 +937,161 @@ func TestACL_ListResolvableAliasesPermissions(t *testing.T) {
},
},
},
{
name: "global_with_this_with_valid_children_and_direct_grant",
resourceType: resource.Target,
actionSet: action.NewActionSet(action.Read, action.Cancel),
aclGrants: []scopeGrant{
{
roleScope: "global",
grantScope: "global",
grants: []string{
"ids=ttcp_1234567890;actions=read",
},
},
{
roleScope: "global",
grantScope: globals.GrantScopeChildren,
grants: []string{
"ids=ttcp_1234567890;actions=read",
},
},
{
roleScope: "global",
grantScope: "p_1",
grants: []string{
"ids=ttcp_1234567890;actions=read",
},
},
},
expPermissions: []Permission{
{
RoleScopeId: scope.Global.String(),
GrantScopeId: scope.Global.String(),
Resource: resource.Target,
Action: action.ListResolvableAliases,
ResourceIds: []string{"ttcp_1234567890"},
All: false,
OnlySelf: false,
},
{
RoleScopeId: scope.Global.String(),
GrantScopeId: globals.GrantScopeChildren,
Resource: resource.Target,
Action: action.ListResolvableAliases,
ResourceIds: []string{"ttcp_1234567890"},
All: false,
OnlySelf: false,
},
{
RoleScopeId: scope.Global.String(),
GrantScopeId: "p_1",
Resource: resource.Target,
Action: action.ListResolvableAliases,
ResourceIds: []string{"ttcp_1234567890"},
All: false,
OnlySelf: false,
},
},
},
{
name: "global_with_this_with_and_direct_grant",
resourceType: resource.Target,
actionSet: action.NewActionSet(action.Read, action.Cancel),
aclGrants: []scopeGrant{
{
roleScope: "global",
grantScope: "global",
grants: []string{
"ids=ttcp_1234567890;actions=read",
},
},
{
roleScope: "global",
grantScope: "p_1",
grants: []string{
"ids=ttcp_1234567890;actions=read",
},
},
},
expPermissions: []Permission{
{
RoleScopeId: scope.Global.String(),
GrantScopeId: scope.Global.String(),
Resource: resource.Target,
Action: action.ListResolvableAliases,
ResourceIds: []string{"ttcp_1234567890"},
All: false,
OnlySelf: false,
},
{
RoleScopeId: scope.Global.String(),
GrantScopeId: "p_1",
Resource: resource.Target,
Action: action.ListResolvableAliases,
ResourceIds: []string{"ttcp_1234567890"},
All: false,
OnlySelf: false,
},
},
},
{
name: "global_with_this_with_multiple_direct_grants",
resourceType: resource.Target,
actionSet: action.NewActionSet(action.Read, action.Cancel),
aclGrants: []scopeGrant{
{
roleScope: "global",
grantScope: "global",
grants: []string{
"ids=ttcp_1234567890;actions=read",
},
},
{
roleScope: "global",
grantScope: "o_1",
grants: []string{
"ids=ttcp_1234567890;actions=read",
},
},
{
roleScope: "global",
grantScope: "p_1",
grants: []string{
"ids=ttcp_1234567890;actions=read",
},
},
},
expPermissions: []Permission{
{
RoleScopeId: scope.Global.String(),
GrantScopeId: scope.Global.String(),
Resource: resource.Target,
Action: action.ListResolvableAliases,
ResourceIds: []string{"ttcp_1234567890"},
All: false,
OnlySelf: false,
},
{
RoleScopeId: scope.Global.String(),
GrantScopeId: "o_1",
Resource: resource.Target,
Action: action.ListResolvableAliases,
ResourceIds: []string{"ttcp_1234567890"},
All: false,
OnlySelf: false,
},
{
RoleScopeId: scope.Global.String(),
GrantScopeId: "p_1",
Resource: resource.Target,
Action: action.ListResolvableAliases,
ResourceIds: []string{"ttcp_1234567890"},
All: false,
OnlySelf: false,
},
},
},
{
name: "org_no_this_with_children_and_direct_grant",
resourceType: resource.Target,
@@ -926,7 +1102,6 @@ func TestACL_ListResolvableAliasesPermissions(t *testing.T) {
roleParentScopeId: scope.Global.String(),
grantScope: globals.GrantScopeChildren,
grants: []string{
"type=target;actions=list",
"ids=ttcp_1234567890;actions=read",
},
},
@@ -935,7 +1110,6 @@ func TestACL_ListResolvableAliasesPermissions(t *testing.T) {
roleParentScopeId: scope.Global.String(),
grantScope: "o_2",
grants: []string{
"type=target;actions=list",
"ids=ttcp_1234567890;actions=read",
},
},
@@ -973,7 +1147,6 @@ func TestACL_ListResolvableAliasesPermissions(t *testing.T) {
roleParentScopeId: scope.Global.String(),
grantScope: globals.GrantScopeChildren,
grants: []string{
"type=target;actions=list",
"ids=ttcp_1234567890;actions=read",
},
},
@@ -982,7 +1155,6 @@ func TestACL_ListResolvableAliasesPermissions(t *testing.T) {
roleParentScopeId: scope.Global.String(),
grantScope: "o_1",
grants: []string{
"type=target;actions=list",
"ids=ttcp_1234567890;actions=read",
},
},
@@ -1020,7 +1192,6 @@ func TestACL_ListResolvableAliasesPermissions(t *testing.T) {
roleParentScopeId: scope.Global.String(),
grantScope: globals.GrantScopeChildren,
grants: []string{
"type=target;actions=list",
"ids=ttcp_1234567890;actions=read",
},
},
@@ -1029,7 +1200,6 @@ func TestACL_ListResolvableAliasesPermissions(t *testing.T) {
roleParentScopeId: scope.Global.String(),
grantScope: "o_1",
grants: []string{
"type=target;actions=list",
"ids=ttcp_1234567890;actions=read",
},
},
@@ -1038,7 +1208,6 @@ func TestACL_ListResolvableAliasesPermissions(t *testing.T) {
roleParentScopeId: "o_1",
grantScope: "p_1a",
grants: []string{
"type=target;actions=list",
"ids=ttcp_1234567890;actions=read",
},
},
@@ -1047,7 +1216,6 @@ func TestACL_ListResolvableAliasesPermissions(t *testing.T) {
roleParentScopeId: "o_1",
grantScope: "p_1b",
grants: []string{
"type=target;actions=list",
"ids=ttcp_1234567890;actions=read",
},
},
@@ -1056,7 +1224,6 @@ func TestACL_ListResolvableAliasesPermissions(t *testing.T) {
roleParentScopeId: "o_2",
grantScope: "p_2",
grants: []string{
"type=target;actions=list",
"ids=ttcp_1234567890;actions=read",
},
},
@@ -1112,7 +1279,7 @@ func TestACL_ListResolvableAliasesPermissions(t *testing.T) {
acl := NewACL(grants...)
perms := acl.ListResolvableAliasesPermissions(tt.resourceType, tt.actionSet)
require.ElementsMatch(t, tt.expPermissions, perms)
assert.ElementsMatch(t, tt.expPermissions, perms)
})
}
}