Skip to content

Commit 8336e48

Browse files
bosorawisdkanney
bosorawis
and
dkanney
authored
Domain: iam: Repository: update list-grant-scope and test setup to use new model (#5679)
* implement getRoleScopeId * move query to query.go * improve notfound err message * improve other err messages * use named parameter and move getRoleScopeId implementation * moved getRoleScopeId test * rename getRoleScopeId to getRoleScopeType * fix public_id ambiguous error * undo unintended change to getUserWithAccount * fix the correct query * split iam_role_global_individual_grant_scope to have separate tables for org and project * small comment change * small comment change * WIP: add tests * remove grant_scope as immutable column * add trigger to delete individual grant scope when grant_scope changes * add a test that covers changing grant_scope * rename function and trigger in iam_role_global * improve assertion in sqltest for iam_role_global * update iam_role_org to delete redundant grants scope * minor comment fix * no longer handle individual grant scope deletion with triggers and rename some functions * rename test * add all subtype definitions * remove unnecessary baseRole subtype * add clone, setTableName, and GetScope tests * add ResourceType and Actions test * add create and delete tests for globalROle * finish create and delete tests * add trigger for deleting base role * add trigger to sync update_time back to base iam_role table * add update tests * fix missing err checks * fix iam_role delete subtype trigger function name and use new.update_time instead of now() * add struct documentation to role subtypes * add version update check * implement getRoleScopeId * implement getRoleScopeId * save * remove struct embedding from iam.Role * fix tests to use new iam.Role definition * repository_role_test.go move to new iam.Role model * repository_principal_role_test.go use new iam.Role model * repository_role_grant_test.go use new iam.Role model in test * add oplog info to sql schema * internal/iam/testing.go use new role schema in TestRole * add toRole helper function to all role subtype * remove tests that are no longer relevant * internal/iam/repository_scope.go use new iam model * internal/iam/repository_role_grant.go use new iam model * internal/iam/repository_principal_role.go use new iam model * internal/iam/repository_role_test.go add test case for global scoped role * internal/iam/repository_grant_scope.go use new iam model * fix query * make create and lookup role work and add tests * add role id to getRoleScopeId error message * make DeleteRole work with new model and add tests * fix update * ensure oplog.ReplayableMessage is implemented on all role subtypes * internal/iam/repository_role_grant.go fix slugging version properly * internal/iam/repository_role.go minor correction to error message saying org instead of scope * internal/iam/repository_role_test.go add more update tests * add immutable_fields tests * fix rebase * change error code to RecordNotFound * refactor to use getScopeType * fix delete test * add getRoleScope utility function * repository_principal_role.go: refactor to remove multiple switch statements * repository_role_grant.go: refactor to reduce LOC * repository_role.go small refactor to use alloc func * repository_grant_scope.go refactor * review comments * implement getRoleScopeId * move query to query.go * improve notfound err message * improve other err messages * use named parameter and move getRoleScopeId implementation * moved getRoleScopeId test * rename getRoleScopeId to getRoleScopeType * fix public_id ambiguous error * undo unintended change to getUserWithAccount * fix the correct query * rename test * change error code to RecordNotFound * Update internal/iam/repository_role.go Co-authored-by: David Kanney <[email protected]> * switch to slice instead of counter * fix merge mistakes * handling special scopes in test function * fix TestRoleWithGrants * fix minor typo * make gen * fix comment typos * Bosorawis domain iam role use new model list role (#5676) * add and use new list roles query * run make gen * tweaked returned error * replace tabs with spaces in query string * missed one tab * remove leading spaces * move ListRoleGrantScopes to repository_grant_scope.go * rename repository_grant_scope to repository_role_grant_scope * add proto definition for global role individual grant scope tables * fix test from removing embeded struct from RoleGrantScope * add grant_scope to proto definition * implement GlobalRoleIndividualOrgGrantScope and GlobalRoleIndividualProjectGrantScope * update comment * run make gen to update comment * implement OrgRoleIndividualGrantScope and add tests * implement part of ListRoleGrantScopes * Add more test * add more test cases and remove add-grants test * unexport listRoleGrantScopes * use reader from function parameter instead of struct method * rename test to match actual function * run make gen * unexport individual grants structs * unexport individual grants structs - missed one file * change TestRole and TestRoleGrantScope function to support new model * add validation for special scopes * add role_org_individual_grant_scope.pb.go to protobuild make target * remove dead code from listRoleGrantScopes * fix testRoleGrantScopeSpecial not handling org role special scope properly * add back query removed by rebase --------- Co-authored-by: David Kanney <[email protected]>
1 parent 9b9aa75 commit 8336e48

20 files changed

+1918
-183
lines changed

‎Makefile‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,9 @@ protobuild:
155155
@protoc-go-inject-tag -input=./internal/oplog/oplog_test/oplog_test.pb.go
156156
@protoc-go-inject-tag -input=./internal/iam/store/group_member.pb.go
157157
@protoc-go-inject-tag -input=./internal/iam/store/role.pb.go
158+
@protoc-go-inject-tag -input=./internal/iam/store/role_global_individual_org_grant_scope.pb.go
159+
@protoc-go-inject-tag -input=./internal/iam/store/role_global_individual_project_grant_scope.pb.go
160+
@protoc-go-inject-tag -input=./internal/iam/store/role_org_individual_grant_scope.pb.go
158161
@protoc-go-inject-tag -input=./internal/iam/store/role_global.pb.go
159162
@protoc-go-inject-tag -input=./internal/iam/store/role_org.pb.go
160163
@protoc-go-inject-tag -input=./internal/iam/store/role_project.pb.go

‎internal/daemon/controller/handlers/roles/role_service_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ func TestList(t *testing.T) {
220220
var totalRoles []*pb.Role
221221
for i := 0; i < 10; i++ {
222222
or := iam.TestRole(t, conn, oWithRoles.GetPublicId())
223-
_ = iam.TestRoleGrantScope(t, conn, or.GetPublicId(), globals.GrantScopeChildren)
223+
_ = iam.TestRoleGrantScope(t, conn, or, globals.GrantScopeChildren)
224224
wantOrgRoles = append(wantOrgRoles, &pb.Role{
225225
Id: or.GetPublicId(),
226226
ScopeId: or.GetScopeId(),
@@ -2791,7 +2791,7 @@ func TestAddGrantScopes(t *testing.T) {
27912791
assert, require := assert.New(t), require.New(t)
27922792
role := iam.TestRole(t, conn, tc.scopeId, iam.WithGrantScopeIds([]string{"testing-none"}))
27932793
for _, e := range tc.existing {
2794-
_ = iam.TestRoleGrantScope(t, conn, role.GetPublicId(), e)
2794+
_ = iam.TestRoleGrantScope(t, conn, role, e)
27952795
}
27962796
req := &pbs.AddRoleGrantScopesRequest{
27972797
Id: role.GetPublicId(),
@@ -3129,7 +3129,7 @@ func TestSetGrantScopes(t *testing.T) {
31293129
assert, require := assert.New(t), require.New(t)
31303130
role := iam.TestRole(t, conn, tc.scopeId, iam.WithGrantScopeIds([]string{"testing-none"}))
31313131
for _, e := range tc.existing {
3132-
_ = iam.TestRoleGrantScope(t, conn, role.GetPublicId(), e)
3132+
_ = iam.TestRoleGrantScope(t, conn, role, e)
31333133
}
31343134
req := &pbs.SetRoleGrantScopesRequest{
31353135
Id: role.GetPublicId(),
@@ -3326,7 +3326,7 @@ func TestRemoveGrantScopes(t *testing.T) {
33263326
assert, require := assert.New(t), require.New(t)
33273327
role := iam.TestRole(t, conn, tc.scopeId, iam.WithGrantScopeIds([]string{"testing-none"}))
33283328
for _, e := range tc.existing {
3329-
_ = iam.TestRoleGrantScope(t, conn, role.GetPublicId(), e)
3329+
_ = iam.TestRoleGrantScope(t, conn, role, e)
33303330
}
33313331
req := &pbs.RemoveRoleGrantScopesRequest{
33323332
Id: role.GetPublicId(),

‎internal/daemon/controller/handlers/targets/tcp/target_service_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1295,7 +1295,7 @@ func TestCreate(t *testing.T) {
12951295
r := iam.TestRole(t, conn, "global")
12961296
_ = iam.TestUserRole(t, conn, r.GetPublicId(), at.GetIamUserId())
12971297
_ = iam.TestRoleGrant(t, conn, r.GetPublicId(), "ids=*;type=*;actions=*")
1298-
_ = iam.TestRoleGrantScope(t, conn, r.GetPublicId(), globals.GrantScopeDescendants)
1298+
_ = iam.TestRoleGrantScope(t, conn, r, globals.GrantScopeDescendants)
12991299

13001300
// Ensure we are using the OSS worker filter function. This prevents us from
13011301
// running tests in parallel.

‎internal/db/schema/migrations/oss/postgres/100/02_iam_role_org.up.sql‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ begin;
9696
on delete cascade
9797
on update cascade,
9898
scope_id wt_scope_id not null
99-
constraint iam_scope_org_scope_id_fkey
99+
constraint iam_scope_project_fkey
100100
references iam_scope_project(scope_id)
101101
on delete cascade
102102
on update cascade,
@@ -119,7 +119,7 @@ begin;
119119
primary key(role_id, scope_id)
120120
);
121121
comment on table iam_role_org_individual_grant_scope is
122-
'iam_role_global_individual_grant_scope is the subtype table for the org role with grant_scope as individual.';
122+
'iam_role_org_individual_grant_scope is the subtype table for the org role with grant_scope as individual.';
123123

124124
create trigger default_create_time_column before insert on iam_role_org_individual_grant_scope
125125
for each row execute procedure default_create_time();

‎internal/iam/query.go‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,4 +267,112 @@ select public_id,
267267
where public_id = any(select role_id from combined_role_types)
268268
order by update_time desc, public_id desc
269269
`
270+
271+
roleGrantsScopeQuery = `
272+
with
273+
global_roles (role_id) as (
274+
select public_id as role_id
275+
from iam_role_global
276+
where public_id = any($1)
277+
),
278+
org_roles (role_id) as (
279+
select public_id as role_id
280+
from iam_role_org
281+
where public_id = any($1)
282+
),
283+
proj_roles (role_id) as (
284+
select public_id as role_id
285+
from iam_role_project
286+
where public_id = any($1)
287+
),
288+
global_role_this_grants (role_id, scope_id_or_special, create_time) as (
289+
select public_id as role_id,
290+
'this' as scope_id_or_special,
291+
grant_this_role_scope_update_time as create_time
292+
from iam_role_global
293+
where public_id = any (select role_id from global_roles)
294+
and grant_this_role_scope = true
295+
),
296+
org_role_this_grants (role_id, scope_id_or_special, create_time) as (
297+
select public_id as role_id,
298+
'this' as scope_id_or_special,
299+
grant_this_role_scope_update_time as create_time
300+
from iam_role_org
301+
where public_id = any (select role_id from org_roles)
302+
and grant_this_role_scope = true
303+
),
304+
proj_role_this_grants (role_id, scope_id_or_special, create_time) as (
305+
select public_id as role_id,
306+
'this' as scope_id_or_special,
307+
create_time as create_time
308+
from iam_role_project
309+
where public_id = any (select role_id from proj_roles)
310+
),
311+
global_role_special_grants (role_id, scope_id_or_special, create_time) as (
312+
select public_id as role_id,
313+
grant_scope as scope_id_or_special,
314+
grant_this_role_scope_update_time as create_time
315+
from iam_role_global
316+
where public_id = any (select role_id from global_roles)
317+
and grant_scope != 'individual'
318+
),
319+
org_role_special_grants (role_id, scope_id_or_special, create_time) as (
320+
select public_id as role_id,
321+
grant_scope as scope_id_or_special,
322+
grant_this_role_scope_update_time as create_time
323+
from iam_role_org
324+
where public_id = any (select role_id from org_roles)
325+
and grant_scope != 'individual'
326+
),
327+
global_role_individual_org_grants (role_id, scope_id_or_special, create_time) as (
328+
select role_id as role_id,
329+
scope_id as scope_id_or_special,
330+
create_time as create_time
331+
from iam_role_global_individual_org_grant_scope
332+
where role_id = any (select role_id from global_roles)
333+
),
334+
global_role_individual_proj_grants (role_id, scope_id_or_special, create_time) as (
335+
select role_id as role_id,
336+
scope_id as scope_id_or_special,
337+
create_time as create_time
338+
from iam_role_global_individual_project_grant_scope
339+
where role_id = any (select role_id from global_roles)
340+
),
341+
org_role_individual_grants (role_id, scope_id_or_special, create_time) as (
342+
select role_id as role_id,
343+
scope_id as scope_id_or_special,
344+
create_time as create_time
345+
from iam_role_org_individual_grant_scope
346+
where role_id = any (select role_id from org_roles)
347+
),
348+
final (role_id, scope_id_or_special, create_time) as (
349+
select role_id, scope_id_or_special, create_time
350+
from global_role_this_grants
351+
union
352+
select role_id, scope_id_or_special, create_time
353+
from org_role_this_grants
354+
union
355+
select role_id, scope_id_or_special, create_time
356+
from proj_role_this_grants
357+
union
358+
select role_id, scope_id_or_special, create_time
359+
from global_role_special_grants
360+
union
361+
select role_id, scope_id_or_special, create_time
362+
from org_role_special_grants
363+
union
364+
select role_id, scope_id_or_special, create_time
365+
from global_role_individual_org_grants
366+
union
367+
select role_id, scope_id_or_special, create_time
368+
from global_role_individual_proj_grants
369+
union
370+
select role_id, scope_id_or_special, create_time
371+
from org_role_individual_grants
372+
)
373+
select role_id,
374+
scope_id_or_special,
375+
create_time
376+
from final;
377+
`
270378
)

‎internal/iam/repository_role.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ func (r *Repository) LookupRole(ctx context.Context, withPublicId string, opt ..
279279
if err != nil {
280280
return errors.Wrap(ctx, err, op)
281281
}
282-
rgs, err = repo.ListRoleGrantScopes(ctx, []string{withPublicId})
282+
rgs, err = listRoleGrantScopes(ctx, read, []string{withPublicId})
283283
if err != nil {
284284
return errors.Wrap(ctx, err, op)
285285
}
@@ -441,7 +441,7 @@ func (r *Repository) queryRoles(ctx context.Context, whereClause string, args []
441441
for _, retRole := range retRoles {
442442
roleIds = append(roleIds, retRole.PublicId)
443443
}
444-
retRoleGrantScopes, err = r.ListRoleGrantScopes(ctx, roleIds, WithReaderWriter(rd, w))
444+
retRoleGrantScopes, err = listRoleGrantScopes(ctx, r.reader, roleIds)
445445
if err != nil {
446446
return errors.Wrap(ctx, err, op)
447447
}

‎internal/iam/repository_role_grant.go‎

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -429,31 +429,6 @@ func (r *Repository) ListRoleGrants(ctx context.Context, roleId string, opt ...O
429429
return roleGrants, nil
430430
}
431431

432-
// ListRoleGrantScopes returns the grant scopes for the roleId and supports the WithLimit
433-
// option.
434-
func (r *Repository) ListRoleGrantScopes(ctx context.Context, roleIds []string, opt ...Option) ([]*RoleGrantScope, error) {
435-
const op = "iam.(Repository).ListRoleGrantScopes"
436-
if len(roleIds) == 0 {
437-
return nil, errors.New(ctx, errors.InvalidParameter, op, "missing role ids")
438-
}
439-
query := "?"
440-
var args []any
441-
for i, roleId := range roleIds {
442-
if roleId == "" {
443-
return nil, errors.New(ctx, errors.InvalidParameter, op, "missing role ids")
444-
}
445-
if i > 0 {
446-
query = query + ", ?"
447-
}
448-
args = append(args, roleId)
449-
}
450-
var roleGrantScopes []*RoleGrantScope
451-
if err := r.list(ctx, &roleGrantScopes, fmt.Sprintf("role_id in (%s)", query), args, opt...); err != nil {
452-
return nil, errors.Wrap(ctx, err, op, errors.WithMsg("unable to lookup role grant scopes"))
453-
}
454-
return roleGrantScopes, nil
455-
}
456-
457432
type MultiGrantTuple struct {
458433
RoleId string
459434
RoleScopeId string

‎internal/iam/repository_grant_scope.go‎ renamed to ‎internal/iam/repository_role_grant_scope.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -425,3 +425,30 @@ func (r *Repository) SetRoleGrantScopes(ctx context.Context, roleId string, role
425425
}
426426
return currentRoleGrantScopes, totalRowsDeleted, nil
427427
}
428+
429+
// listRoleGrantScopes returns the grant scopes for the roleId
430+
func listRoleGrantScopes(ctx context.Context, reader db.Reader, roleIds []string) ([]*RoleGrantScope, error) {
431+
const op = "iam.(Repository).listRoleGrantScopes"
432+
if len(roleIds) == 0 {
433+
return nil, errors.New(ctx, errors.InvalidParameter, op, "missing role ids")
434+
}
435+
rows, err := reader.Query(ctx, roleGrantsScopeQuery, []any{roleIds})
436+
if err != nil {
437+
return nil, errors.Wrap(ctx, err, op, errors.WithMsg("failed to query role grant scopes"))
438+
}
439+
440+
if rows.Err() != nil {
441+
return nil, errors.Wrap(ctx, rows.Err(), op, errors.WithMsg("role grant scope rows error"))
442+
}
443+
var result []*RoleGrantScope
444+
for rows.Next() {
445+
if err := reader.ScanRows(ctx, rows, &result); err != nil {
446+
return nil, errors.Wrap(ctx, err, op, errors.WithMsg(fmt.Sprintf("failed scan results from querying role scope for: %s", roleIds)))
447+
}
448+
}
449+
if err := rows.Err(); err != nil {
450+
return nil, errors.Wrap(ctx, err, op, errors.WithMsg(fmt.Sprintf("unexpected error scanning results from querying role scope for: %s", roleIds)))
451+
}
452+
453+
return result, nil
454+
}

0 commit comments

Comments
 (0)