diff --git a/internal/alias/target/repository_alias_list_resolvable.go b/internal/alias/target/repository_alias_list_resolvable.go index a9b144b8d..bec1bcdff 100644 --- a/internal/alias/target/repository_alias_list_resolvable.go +++ b/internal/alias/target/repository_alias_list_resolvable.go @@ -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 ")) } diff --git a/internal/alias/target/service_list_resolvable_ext_test.go b/internal/alias/target/service_list_resolvable_ext_test.go index 48d6be51c..ec687fd0d 100644 --- a/internal/alias/target/service_list_resolvable_ext_test.go +++ b/internal/alias/target/service_list_resolvable_ext_test.go @@ -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() diff --git a/internal/perms/acl.go b/internal/perms/acl.go index 694c6cff8..d5c65a230 100644 --- a/internal/perms/acl.go +++ b/internal/perms/acl.go @@ -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 } diff --git a/internal/perms/acl_test.go b/internal/perms/acl_test.go index 02cebf3cd..889216458 100644 --- a/internal/perms/acl_test.go +++ b/internal/perms/acl_test.go @@ -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) }) } }