Skip to content

Commit f4d4840

Browse files
committed
feat: add grant_this_role_scope column to iam_role_project (#5738)
* add grant_this_role_scope and sqltes for iam_project_role * update role_project.proto to add grant_this_role_scope field * fix missing fields in role_project.proto * support GrantThisRoleScope field in iam/testing.go * update repository_scope to set GrantThisRoleScope to true * add GrantThisRoleScope to projec role tests to role_test.go * add grant_this_role_scope check to query.go * update roleGrantScopeUpdater to split setGrantScope to setHierarchicalGrantScope and removeHierarchicalGrantScope * update create_role to set default vaule for iam_role_project.grant_this_role_scope * update set, add, delete, list role grant scopes to support grant_this_role_scope in project_role * rename methods * run make gen * make TestRole not bump version when creating roles with default 'this' grants * address pr comments * fix variable names
1 parent bf1336f commit f4d4840

File tree

12 files changed

+492
-124
lines changed

12 files changed

+492
-124
lines changed

internal/db/schema/migrations/oss/postgres/100/03_iam_role_project.up.sql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@ begin;
1616
on update cascade,
1717
name text,
1818
description text,
19+
grant_this_role_scope boolean not null default false,
1920
version wt_version,
21+
grant_this_role_scope_update_time wt_timestamp,
2022
create_time wt_timestamp,
2123
update_time wt_timestamp,
2224
constraint iam_role_project_name_scope_id_uq
@@ -43,6 +45,12 @@ begin;
4345
create trigger delete_iam_role_subtype after delete on iam_role_project
4446
for each row execute procedure delete_iam_role_subtype();
4547

48+
create trigger insert_iam_role_project_grant_this_role_scope_update_time before insert on iam_role_project
49+
for each row execute procedure insert_grant_this_role_scope_update_time();
50+
51+
create trigger update_iam_role_project_grant_this_role_scope_update_time before update on iam_role_project
52+
for each row execute procedure insert_grant_this_role_scope_update_time();
53+
4654
create trigger immutable_columns before update on iam_role_project
4755
for each row execute procedure immutable_columns('scope_id', 'create_time');
4856

internal/db/sqltest/tests/iam/iam_role_project.sql

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
-- SPDX-License-Identifier: BUSL-1.1
33

44
begin;
5-
select plan(8);
5+
select plan(14);
66
select wtt_load('widgets', 'iam');
77

88
--------------------------------------------------------------------------------
@@ -90,5 +90,56 @@ select throws_like(
9090
'immutable_columns trigger prevents changing create_time'
9191
);
9292

93+
------------------------------------------------------------------------------
94+
-- 4) testing grant_scope_update_time trigger
95+
------------------------------------------------------------------------------
96+
97+
-- 4a) insert a row -> expect it to set grant_scope_update_time (if triggered on insert)
98+
prepare insert_with_grant_this_role_scope_update_time as
99+
insert into iam_role_project
100+
(public_id, scope_id, grant_this_role_scope, grant_this_role_scope_update_time)
101+
values
102+
('r_proj_22222222', 'p____bwidget', false, null);
103+
select lives_ok('insert_with_grant_this_role_scope_update_time');
104+
105+
select is(
106+
(select grant_this_role_scope_update_time is not null
107+
from iam_role_project
108+
where public_id = 'r_proj_22222222'),
109+
true,
110+
'grant_this_role_scope_update_time should be set right after insert if the trigger sets it'
111+
);
112+
113+
-- 4b) update grant_this_role_scope => trigger should update grant_this_role_scope_update_time
114+
115+
-- update grant_this_role_scope_update_time to default
116+
prepare reset_grant_this_role_scope_update_time as
117+
update iam_role_project
118+
set grant_this_role_scope_update_time = '1970-01-01 00:00:00'
119+
where public_id = 'r_proj_22222222';
120+
select lives_ok('reset_grant_this_role_scope_update_time');
121+
122+
prepare update_grant_this_role_scope as
123+
update iam_role_project
124+
set grant_this_role_scope = true
125+
where public_id = 'r_proj_22222222';
126+
select lives_ok('update_grant_this_role_scope');
127+
128+
select is(
129+
(select grant_this_role_scope_update_time is not null
130+
from iam_role_project
131+
where public_id = 'r_proj_22222222'),
132+
true,
133+
'grant_this_role_scope_update_time should be updated after changing grant_this_role_scope'
134+
);
135+
136+
select is(
137+
(select grant_this_role_scope_update_time = now()
138+
from iam_role_project
139+
where public_id = 'r_proj_22222222'),
140+
true,
141+
'grant_this_role_scope_update_time should be updated after changing grant_this_role_scope'
142+
);
143+
93144
select * from finish();
94145
rollback;

internal/iam/query.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,7 @@ proj_role_this_grants (role_id, scope_id_or_special, create_time) as (
410410
create_time as create_time
411411
from iam_role_project
412412
where public_id = any (select role_id from proj_roles)
413+
and grant_this_role_scope = true
413414
),
414415
global_role_special_grants (role_id, scope_id_or_special, create_time) as (
415416
select public_id as role_id,

internal/iam/repository_role.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,11 @@ func (r *Repository) CreateRole(ctx context.Context, role *Role, opt ...Option)
6666
case strings.HasPrefix(role.GetScopeId(), globals.ProjectPrefix):
6767
roleToCreate = &projectRole{
6868
ProjectRole: &store.ProjectRole{
69-
PublicId: id,
70-
ScopeId: role.ScopeId,
71-
Name: role.Name,
72-
Description: role.Description,
73-
// GrantThisRoleScope: true, will be set in https://github.com/hashicorp/boundary/pull/5738 since the field doesn't exist yet
69+
PublicId: id,
70+
ScopeId: role.ScopeId,
71+
Name: role.Name,
72+
Description: role.Description,
73+
GrantThisRoleScope: true,
7474
},
7575
}
7676
default:

internal/iam/repository_role_grant_scope.go

Lines changed: 56 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,6 @@ func (r *Repository) AddRoleGrantScopes(ctx context.Context, roleId string, role
3939
if err != nil {
4040
return nil, errors.Wrap(ctx, err, op, errors.WithMsg(fmt.Sprintf("unable to get role %s scope id", roleId)))
4141
}
42-
if scp.Type == scope.Project.String() {
43-
return nil, errors.New(ctx, errors.InvalidParameter, op, "grant scope cannot be added to project roles")
44-
}
4542

4643
// Find existing grant scopes to find duplicate grants
4744
originalGrantScopes, err := listRoleGrantScopes(ctx, r.reader, []string{roleId})
@@ -99,15 +96,22 @@ func (r *Repository) AddRoleGrantScopes(ctx context.Context, roleId string, role
9996
if _, ok := originalGrantScopeMap[globals.GrantScopeChildren]; ok {
10097
return nil, errors.New(ctx, errors.InvalidParameter, op, "grant scope children already exists, only one of descendants or children grant scope can be specified")
10198
}
102-
updateRole.setGrantScope(globals.GrantScopeDescendants)
99+
err = updateRole.setGrantScope(ctx, globals.GrantScopeDescendants)
100+
if err != nil {
101+
return nil, errors.Wrap(ctx, err, op)
102+
}
103103
updateMask = append(updateMask, "GrantScope")
104104
finalGrantScope = globals.GrantScopeDescendants
105105
case addChildren:
106106
// cannot add 'children' when descendant is already set, only one hierarchical grant scope can be set
107107
if _, ok := originalGrantScopeMap[globals.GrantScopeDescendants]; ok {
108108
return nil, errors.New(ctx, errors.InvalidParameter, op, "grant scope descendants already exists, only one of descendants or children grant scope can be specified")
109109
}
110-
updateRole.setGrantScope(globals.GrantScopeChildren)
110+
err = updateRole.setGrantScope(ctx, globals.GrantScopeChildren)
111+
if err != nil {
112+
return nil, errors.Wrap(ctx, err, op)
113+
}
114+
111115
updateMask = append(updateMask, "GrantScope")
112116
finalGrantScope = globals.GrantScopeChildren
113117
default:
@@ -155,7 +159,10 @@ func (r *Repository) AddRoleGrantScopes(ctx context.Context, roleId string, role
155159
return nil, errors.Wrap(ctx, err, op, errors.WithMsg("unable to split org roles grant scopes"))
156160
}
157161
default:
158-
return nil, errors.New(ctx, errors.InvalidParameter, op, "invalid role scope")
162+
// granting individual grant scope to roles is only allowed for roles in global and org scopes
163+
if len(individualScopesToAdd) > 0 {
164+
return nil, errors.New(ctx, errors.InvalidParameter, op, "individual role grant scope can only be set for global roles or org roles")
165+
}
159166
}
160167

161168
oplogWrapper, err := r.kms.GetWrapper(ctx, scp.GetPublicId(), kms.KeyPurposeOplog)
@@ -179,7 +186,9 @@ func (r *Repository) AddRoleGrantScopes(ctx context.Context, roleId string, role
179186
return errors.Wrap(ctx, err, op, errors.WithMsg("unable to update role"))
180187
}
181188
if addThis {
182-
retRoleGrantScopes = append(retRoleGrantScopes, updateRole.grantThisRoleScope())
189+
if g, ok := updateRole.grantThisRoleScope(); ok {
190+
retRoleGrantScopes = append(retRoleGrantScopes, g)
191+
}
183192
}
184193
if addDescendants || addChildren {
185194
if g, ok := updateRole.grantScope(); ok {
@@ -256,9 +265,6 @@ func (r *Repository) DeleteRoleGrantScopes(ctx context.Context, roleId string, r
256265
if err != nil {
257266
return db.NoRowsAffected, errors.Wrap(ctx, err, op, errors.WithMsg(fmt.Sprintf("unable to get role %s scope id", roleId)))
258267
}
259-
if scp.Type == scope.Project.String() {
260-
return db.NoRowsAffected, errors.New(ctx, errors.InvalidParameter, op, "grant scope cannot be deleted from a project role")
261-
}
262268

263269
oplogWrapper, err := r.kms.GetWrapper(ctx, scp.GetPublicId(), kms.KeyPurposeOplog)
264270
if err != nil {
@@ -316,8 +322,8 @@ func (r *Repository) DeleteRoleGrantScopes(ctx context.Context, roleId string, r
316322

317323
// handle case where hierarchical grant scope ['children', 'descendants'] is removed
318324
// these grants are mutually exclusive so an OR operation is safe here
319-
if removeChildren || removeDescendants {
320-
updateRole.setGrantScope(globals.GrantScopeIndividual)
325+
if (removeChildren || removeDescendants) && scp.Type != scope.Project.String() {
326+
updateRole.removeGrantScope()
321327
updateMask = append(updateMask, "GrantScope")
322328
// manually bump rows deleted when for deleting hierarchical grant scope since this is now
323329
// a DB row update instead of deleting a row.
@@ -354,7 +360,9 @@ func (r *Repository) DeleteRoleGrantScopes(ctx context.Context, roleId string, r
354360
return db.NoRowsAffected, errors.Wrap(ctx, err, op, errors.WithMsg("unable to split org roles grant scopes"))
355361
}
356362
default:
357-
return db.NoRowsAffected, errors.New(ctx, errors.InvalidParameter, op, "invalid role scope")
363+
// granting individual grant scope to roles is only allowed for roles in global and org scopes
364+
// but deleting individual grant scopes when the grant scope doesn't exist on a role is allowed
365+
// so we don't return an error here and treat this as a no-op
358366
}
359367

360368
_, err = r.writer.DoTx(
@@ -453,9 +461,6 @@ func (r *Repository) SetRoleGrantScopes(ctx context.Context, roleId string, role
453461
if err != nil {
454462
return nil, db.NoRowsAffected, errors.Wrap(ctx, err, op, errors.WithMsg(fmt.Sprintf("unable to get role %s scope id", roleId)))
455463
}
456-
if scp.Type == scope.Project.String() {
457-
return nil, db.NoRowsAffected, errors.New(ctx, errors.InvalidParameter, op, "cannot modify grant scopes of a project scoped role")
458-
}
459464

460465
// NOTE: Set calculation can safely take place out of the transaction since
461466
// we are using roleVersion to ensure that we end up operating on the same
@@ -521,7 +526,6 @@ func (r *Repository) SetRoleGrantScopes(ctx context.Context, roleId string, role
521526
}
522527

523528
// return early if the there's a conflict in grant_scopes we're trying to add
524-
finalGrantScope := globals.GrantScopeIndividual
525529
_, addDescendants := toAdd[globals.GrantScopeDescendants]
526530
_, addChildren := toAdd[globals.GrantScopeChildren]
527531
if addDescendants && addChildren {
@@ -538,19 +542,37 @@ func (r *Repository) SetRoleGrantScopes(ctx context.Context, roleId string, role
538542
totalRowsDeleted++
539543
}
540544

541-
// determine the final hierarchical grant scopes stored in 'grant_scope' column ['descendants', 'children']
545+
// finalGrantScopeForIndividualGrantScope is the final value of what 'grant_scope' column of the role will be.
546+
// this is only important for global roles with individual scopes IDs granted to the role.
547+
// A foreign key between iam_role_global.grant_scope and iam_role_global_individual_project_grant_scope.grant_scope
548+
// can either be ['children', 'individual'] which will prevents inserting if the values don't match.
549+
// Assuming that the value is 'individual' and only set it to 'children' if we're adding a 'children' grant
550+
// the current value in the database does not matter and will be overridden by this method
551+
finalGrantScopeForIndividualGrantScope := globals.GrantScopeIndividual
552+
553+
// determine the final hierarchical grant scopes stored in `grant_scope` column [`descendants`, `children`]
542554
// depending on if we're adding or removing grants
555+
// if descendants or children is being added, set finalGrantScope to the grant-to-be-added
556+
// to resolve the
543557
switch {
544558
case addDescendants:
545-
updateRole.setGrantScope(globals.GrantScopeDescendants)
546-
finalGrantScope = globals.GrantScopeDescendants
559+
err := updateRole.setGrantScope(ctx, globals.GrantScopeDescendants)
560+
if err != nil {
561+
return nil, db.NoRowsAffected, errors.Wrap(ctx, err, op)
562+
}
547563
updateMask = append(updateMask, "GrantScope")
548564
case addChildren:
549-
updateRole.setGrantScope(globals.GrantScopeChildren)
550-
finalGrantScope = globals.GrantScopeChildren
565+
err := updateRole.setGrantScope(ctx, globals.GrantScopeChildren)
566+
if err != nil {
567+
return nil, db.NoRowsAffected, errors.Wrap(ctx, err, op)
568+
}
551569
updateMask = append(updateMask, "GrantScope")
570+
// set final grant scope to 'children'. finalGrantScopeForIndividualGrantScope will be used later
571+
// when constructing entries for individual project grant scope
572+
finalGrantScopeForIndividualGrantScope = globals.GrantScopeChildren
573+
552574
case removeDescendants || removeChildren:
553-
updateRole.setGrantScope(globals.GrantScopeIndividual)
575+
updateRole.removeGrantScope()
554576
updateMask = append(updateMask, "GrantScope")
555577
}
556578

@@ -591,11 +613,11 @@ func (r *Repository) SetRoleGrantScopes(ctx context.Context, roleId string, role
591613

592614
switch scp.Type {
593615
case scope.Global.String():
594-
globalRoleIndividualOrgToAdd, globalRoleIndividualProjToAdd, err = individualGlobalRoleGrantScope(ctx, roleId, finalGrantScope, individualScopesToAdd)
616+
globalRoleIndividualOrgToAdd, globalRoleIndividualProjToAdd, err = individualGlobalRoleGrantScope(ctx, roleId, finalGrantScopeForIndividualGrantScope, individualScopesToAdd)
595617
if err != nil {
596618
return nil, db.NoRowsAffected, errors.Wrap(ctx, err, op, errors.WithMsg("failed to convert global role individual org scopes to scope specific scope object for grant scope addition"))
597619
}
598-
globalRoleIndividualOrgToRemove, globalRoleIndividualProjToRemove, err = individualGlobalRoleGrantScope(ctx, roleId, finalGrantScope, individualScopesToRemove)
620+
globalRoleIndividualOrgToRemove, globalRoleIndividualProjToRemove, err = individualGlobalRoleGrantScope(ctx, roleId, finalGrantScopeForIndividualGrantScope, individualScopesToRemove)
599621
if err != nil {
600622
return nil, db.NoRowsAffected, errors.Wrap(ctx, err, op, errors.WithMsg("failed to convert global role individual proj scopes to scope specific scope object for grant scope removal"))
601623
}
@@ -609,14 +631,16 @@ func (r *Repository) SetRoleGrantScopes(ctx context.Context, roleId string, role
609631
if err != nil {
610632
return nil, db.NoRowsAffected, errors.Wrap(ctx, err, op, errors.WithMsg("failed to convert org role individual proj scopes to scope specific scope object for grant scope removal"))
611633
}
612-
613634
default:
614-
return nil, db.NoRowsAffected, errors.New(ctx, errors.InvalidParameter, op, fmt.Sprintf("invalid role scope type %s", scp.Type))
635+
if len(individualScopesToRemove) > 0 || len(individualScopesToAdd) > 0 {
636+
return nil, db.NoRowsAffected, errors.New(ctx, errors.InvalidParameter, op,
637+
fmt.Sprintf("roles in scope type %s does not allow individual role grant scope", scp.Type))
638+
}
615639
}
616640

617641
oplogWrapper, err := r.kms.GetWrapper(ctx, scp.GetPublicId(), kms.KeyPurposeOplog)
618642
if err != nil {
619-
return nil, db.NoRowsAffected, errors.Wrap(ctx, err, op, errors.WithMsg("unable to get oplog wrapper"))
643+
return nil, db.NoRowsAffected, errors.Wrap(ctx, err, op, errors.WithMsg("unable to get oplog wra pper"))
620644
}
621645

622646
var retGrantScopes []*RoleGrantScope
@@ -823,6 +847,10 @@ func allocRoleScopeGranter(ctx context.Context, roleId string, scopeType string)
823847
o := allocOrgRole()
824848
o.PublicId = roleId
825849
res = &o
850+
case scope.Project.String():
851+
p := allocProjectRole()
852+
p.PublicId = roleId
853+
res = &p
826854
default:
827855
return nil, errors.New(ctx, errors.InvalidParameter, op, "invalid role scope")
828856
}

0 commit comments

Comments
 (0)