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

api: remove deprecated grant_scope_id field (#4886)

Browse Source 
* api: remove deprecated grant_scope_id field

The grant_scope_id field was deprecated in 0.15.0, so we can remove
it for 0.17.0.

* Fix tests

* Fix more tests

* Remove an option and some related code that stuck around

* Fix bats tests

---------

Co-authored-by: Jeff Mitchell <jeffrey.mitchell@gmail.com>
This commit is contained in:
Johan Brandhorst-SatzkornandJeff Mitchell authored and GitHub committed 2024-06-17 12:09:33 -07:00
1 parent d547e0d459
commit c6f3c375f3
21 files changed
+107 -297
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

internal/daemon/controller/handlers/roles/role_service.go
+5 -52
View File
@@ -739,9 +739,6 @@ func (s Service) createInRepo(ctx context.Context, scopeId string, item *pb.Role
if item.GetDescription() != nil {
opts = append(opts, iam.WithDescription(item.GetDescription().GetValue()))
}
if item.GetGrantScopeId() != nil {
opts = append(opts, iam.WithGrantScopeId(item.GetGrantScopeId().GetValue()))
}
u, err := iam.NewRole(ctx, scopeId, opts...)
if err != nil {
return nil, nil, nil, nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to build role for creation: %v.", err)
@@ -782,27 +779,8 @@ func (s Service) updateInRepo(ctx context.Context, scopeId, id string, mask []st
return nil, nil, nil, nil, err
}
// This has to be separate from Translate since the field only exists on the
// API side now.
var hasGrantScopeIdMask bool
if grantScopeId := item.GetGrantScopeId(); grantScopeId != nil {
// Have to manually check the masks, which could be concatenated strings or slices
for _, v := range mask {
if strutil.StrListContains(strings.Split(v, ","), "grant_scope_id") {
opts = append(opts, iam.WithGrantScopeId(grantScopeId.GetValue()))
hasGrantScopeIdMask = true
break
}
}
if hasGrantScopeIdMask {
if err := validateRoleGrantScopesHierarchy(ctx, repo, id, []string{item.GetGrantScopeId().GetValue()}); err != nil {
return nil, nil, nil, nil, err
}
}
}
dbMask := maskManager.Translate(mask)
if len(dbMask) == 0 && !hasGrantScopeIdMask {
if len(dbMask) == 0 {
return nil, nil, nil, nil, handlers.InvalidArgumentErrorf("No valid fields provided in the update mask.", map[string]string{"update_mask": "No valid fields provided in the update mask."})
}
@@ -813,7 +791,7 @@ func (s Service) updateInRepo(ctx context.Context, scopeId, id string, mask []st
// This is slightly problematic but it's a very unlikely error case and when
// we remove the ability to update grant scope ID via here in 0.17 it will
// go away.
if rowsUpdated == 0 && !hasGrantScopeIdMask {
if rowsUpdated == 0 {
return nil, nil, nil, nil, handlers.NotFoundErrorf("Role %q doesn't exist or incorrect version provided.", id)
}
return out, pr, gr, grantScopes, nil
@@ -1128,9 +1106,6 @@ func toProto(ctx context.Context, in *iam.Role, principals []*iam.PrincipalRole,
}
sort.Strings(out.GrantScopeIds)
}
if outputFields.Has(globals.GrantScopeIdField) && len(grantScopes) == 1 {
out.GrantScopeId = &wrapperspb.StringValue{Value: grantScopes[0].ScopeIdOrSpecial}
}
if outputFields.Has(globals.PrincipalsField) {
for _, p := range principals {
principal := &pb.Principal{
@@ -1194,17 +1169,6 @@ func validateCreateRequest(req *pbs.CreateRoleRequest) error {
scope.Global.String() != item.GetScopeId() {
badFields["scope_id"] = "This field is missing or improperly formatted."
}
if item.GetGrantScopeId() != nil {
switch {
case handlers.ValidId(handlers.Id(item.GetScopeId()), scope.Project.Prefix()):
switch item.GetGrantScopeId().GetValue() {
case item.GetScopeId(),
globals.GrantScopeThis:
default:
badFields["grant_scope_id"] = "When the role is in a project scope this value must be that project's scope ID or \"this\"."
}
}
}
if item.GetPrincipals() != nil {
badFields["principals"] = "This is a read only field."
}
@@ -1230,17 +1194,6 @@ func validateUpdateRequest(req *pbs.UpdateRoleRequest) error {
if req.GetItem().GetGrantStrings() != nil {
badFields["grant_strings"] = "This is a read only field and cannot be specified in an update request."
}
if item := req.GetItem(); item != nil {
switch {
case handlers.ValidId(handlers.Id(item.GetScopeId()), scope.Project.Prefix()):
switch item.GetGrantScopeId().GetValue() {
case item.GetScopeId(),
globals.GrantScopeThis:
default:
badFields["grant_scope_id"] = "When the role is in a project scope this value must be that project's scope ID or \"this\"."
}
}
}
return badFields
}, globals.RolePrefix)
}
@@ -1531,7 +1484,7 @@ func validateRoleGrantScopesHierarchy(ctx context.Context, repo *iam.Repository,
return handlers.InvalidArgumentErrorf(
"Invalid grant scope.",
map[string]string{
"grant_scope_id": `Project scopes can only have their own scope ID or "this" as a grant scope ID.`,
"grant_scope_ids": `Project scopes can only have their own scope ID or "this" as a grant scope ID.`,
})
}
}
@@ -1547,7 +1500,7 @@ func validateRoleGrantScopesHierarchy(ctx context.Context, repo *iam.Repository,
return handlers.InvalidArgumentErrorf(
"Invalid grant scope.",
map[string]string{
"grant_scope_id": fmt.Sprintf("Grant scope ID %q is not valid to set on an organization role.", grantScope),
"grant_scope_ids": fmt.Sprintf("Grant scope ID %q is not valid to set on an organization role.", grantScope),
})
}
}
@@ -1556,7 +1509,7 @@ func validateRoleGrantScopesHierarchy(ctx context.Context, repo *iam.Repository,
return handlers.InvalidArgumentErrorf(
"Improperly formatted identifier.",
map[string]string{
"grant_scope_id": `Unknown scope prefix type.`,
"grant_scope_ids": `Unknown scope prefix type.`,
})
}
return nil
internal/daemon/controller/handlers/roles/role_service_test.go
+16 -44
View File
@@ -59,7 +59,7 @@ func createDefaultRolesAndRepo(t *testing.T) (*iam.Role, *iam.Role, func() (*iam
}
o, p := iam.TestScopes(t, iamRepo)
or := iam.TestRole(t, conn, o.GetPublicId(), iam.WithDescription("default"), iam.WithName("default"), iam.WithGrantScopeId(p.GetPublicId()))
or := iam.TestRole(t, conn, o.GetPublicId(), iam.WithDescription("default"), iam.WithName("default"), iam.WithGrantScopeIds([]string{p.GetPublicId()}))
pr := iam.TestRole(t, conn, p.GetPublicId(), iam.WithDescription("default"), iam.WithName("default"))
return or, pr, repoFn
}
@@ -101,7 +101,6 @@ func TestGet(t *testing.T) {
Scope: &scopes.ScopeInfo{Id: or.GetScopeId(), Type: scope.Org.String(), ParentScopeId: scope.Global.String()},
Name: &wrapperspb.StringValue{Value: or.GetName()},
Description: &wrapperspb.StringValue{Value: or.GetDescription()},
GrantScopeId: &wrapperspb.StringValue{Value: pr.GetScopeId()},
GrantScopeIds: []string{pr.GetScopeId()},
CreatedTime: or.CreateTime.GetTimestamp(),
UpdatedTime: or.UpdateTime.GetTimestamp(),
@@ -115,7 +114,6 @@ func TestGet(t *testing.T) {
Scope: &scopes.ScopeInfo{Id: pr.GetScopeId(), Type: scope.Project.String(), ParentScopeId: or.GetScopeId()},
Name: &wrapperspb.StringValue{Value: pr.GetName()},
Description: &wrapperspb.StringValue{Value: pr.GetDescription()},
GrantScopeId: &wrapperspb.StringValue{Value: globals.GrantScopeThis},
GrantScopeIds: []string{globals.GrantScopeThis},
CreatedTime: pr.CreateTime.GetTimestamp(),
UpdatedTime: pr.UpdateTime.GetTimestamp(),
@@ -891,10 +889,9 @@ func TestCreate(t *testing.T) {
{
name: "Create a valid Role",
req: &pbs.CreateRoleRequest{Item: &pb.Role{
ScopeId: defaultOrgRole.GetScopeId(),
Name: &wrapperspb.StringValue{Value: "name"},
Description: &wrapperspb.StringValue{Value: "desc"},
GrantScopeId: &wrapperspb.StringValue{Value: defaultProjRole.ScopeId},
ScopeId: defaultOrgRole.GetScopeId(),
Name: &wrapperspb.StringValue{Value: "name"},
Description: &wrapperspb.StringValue{Value: "desc"},
}},
res: &pbs.CreateRoleResponse{
Uri: fmt.Sprintf("roles/%s_", globals.RolePrefix),
@@ -903,8 +900,7 @@ func TestCreate(t *testing.T) {
Scope: &scopes.ScopeInfo{Id: defaultOrgRole.GetScopeId(), Type: scope.Org.String(), ParentScopeId: scope.Global.String()},
Name: &wrapperspb.StringValue{Value: "name"},
Description: &wrapperspb.StringValue{Value: "desc"},
GrantScopeId: &wrapperspb.StringValue{Value: defaultProjRole.ScopeId},
GrantScopeIds: []string{defaultProjRole.ScopeId},
GrantScopeIds: []string{globals.GrantScopeThis},
Version: 2,
AuthorizedActions: testAuthorizedActions,
},
@@ -913,10 +909,9 @@ func TestCreate(t *testing.T) {
{
name: "Create a valid Global Role",
req: &pbs.CreateRoleRequest{Item: &pb.Role{
ScopeId: scope.Global.String(),
Name: &wrapperspb.StringValue{Value: "name"},
Description: &wrapperspb.StringValue{Value: "desc"},
GrantScopeId: &wrapperspb.StringValue{Value: defaultProjRole.ScopeId},
ScopeId: scope.Global.String(),
Name: &wrapperspb.StringValue{Value: "name"},
Description: &wrapperspb.StringValue{Value: "desc"},
}},
res: &pbs.CreateRoleResponse{
Uri: fmt.Sprintf("roles/%s_", globals.RolePrefix),
@@ -925,8 +920,7 @@ func TestCreate(t *testing.T) {
Scope: &scopes.ScopeInfo{Id: scope.Global.String(), Type: scope.Global.String(), Name: scope.Global.String(), Description: "Global Scope"},
Name: &wrapperspb.StringValue{Value: "name"},
Description: &wrapperspb.StringValue{Value: "desc"},
GrantScopeId: &wrapperspb.StringValue{Value: defaultProjRole.ScopeId},
GrantScopeIds: []string{defaultProjRole.ScopeId},
GrantScopeIds: []string{globals.GrantScopeThis},
Version: 2,
AuthorizedActions: testAuthorizedActions,
},
@@ -948,26 +942,12 @@ func TestCreate(t *testing.T) {
Scope: &scopes.ScopeInfo{Id: defaultProjRole.GetScopeId(), Type: scope.Project.String(), ParentScopeId: defaultOrgRole.GetScopeId()},
Name: &wrapperspb.StringValue{Value: "name"},
Description: &wrapperspb.StringValue{Value: "desc"},
GrantScopeId: &wrapperspb.StringValue{Value: globals.GrantScopeThis},
GrantScopeIds: []string{globals.GrantScopeThis},
Version: 2,
AuthorizedActions: testAuthorizedActions,
},
},
},
{
name: "Invalid grant scope ID",
req: &pbs.CreateRoleRequest{
Item: &pb.Role{
ScopeId: defaultProjRole.GetScopeId(),
Name: &wrapperspb.StringValue{Value: "name"},
Description: &wrapperspb.StringValue{Value: "desc"},
GrantScopeId: &wrapperspb.StringValue{Value: defaultOrgRole.GetScopeId()},
},
},
res: nil,
err: handlers.ApiErrorWithCode(codes.InvalidArgument),
},
{
name: "Can't specify Id",
req: &pbs.CreateRoleRequest{Item: &pb.Role{
@@ -1128,7 +1108,6 @@ func TestUpdate(t *testing.T) {
Name: &wrapperspb.StringValue{Value: "new"},
Description: &wrapperspb.StringValue{Value: "desc"},
CreatedTime: or.GetCreateTime().GetTimestamp(),
GrantScopeId: &wrapperspb.StringValue{Value: p.GetPublicId()},
GrantScopeIds: []string{p.GetPublicId()},
GrantStrings: []string{grant.GetRaw()},
Grants: []*pb.Grant{grant},
@@ -1143,12 +1122,11 @@ func TestUpdate(t *testing.T) {
scopeId: or.GetScopeId(),
req: &pbs.UpdateRoleRequest{
UpdateMask: &field_mask.FieldMask{
Paths: []string{"name,description,grant_scope_id"},
Paths: []string{"name,description"},
},
Item: &pb.Role{
Name: &wrapperspb.StringValue{Value: "new"},
Description: &wrapperspb.StringValue{Value: "desc"},
GrantScopeId: &wrapperspb.StringValue{Value: globals.GrantScopeThis},
Name: &wrapperspb.StringValue{Value: "new"},
Description: &wrapperspb.StringValue{Value: "desc"},
},
},
res: &pbs.UpdateRoleResponse{
@@ -1159,8 +1137,7 @@ func TestUpdate(t *testing.T) {
Name: &wrapperspb.StringValue{Value: "new"},
Description: &wrapperspb.StringValue{Value: "desc"},
CreatedTime: or.GetCreateTime().GetTimestamp(),
GrantScopeId: &wrapperspb.StringValue{Value: globals.GrantScopeThis},
GrantScopeIds: []string{globals.GrantScopeThis},
GrantScopeIds: []string{p.GetPublicId()},
GrantStrings: []string{grant.GetRaw()},
Grants: []*pb.Grant{grant},
PrincipalIds: []string{u.GetPublicId()},
@@ -1190,7 +1167,6 @@ func TestUpdate(t *testing.T) {
Name: &wrapperspb.StringValue{Value: "new"},
Description: &wrapperspb.StringValue{Value: "desc"},
CreatedTime: pr.GetCreateTime().GetTimestamp(),
GrantScopeId: &wrapperspb.StringValue{Value: globals.GrantScopeThis},
GrantScopeIds: []string{globals.GrantScopeThis},
GrantStrings: []string{grant.GetRaw()},
Grants: []*pb.Grant{grant},
@@ -1221,7 +1197,6 @@ func TestUpdate(t *testing.T) {
Name: &wrapperspb.StringValue{Value: "new"},
Description: &wrapperspb.StringValue{Value: "desc"},
CreatedTime: pr.GetCreateTime().GetTimestamp(),
GrantScopeId: &wrapperspb.StringValue{Value: globals.GrantScopeThis},
GrantScopeIds: []string{globals.GrantScopeThis},
GrantStrings: []string{grant.GetRaw()},
Grants: []*pb.Grant{grant},
@@ -1283,8 +1258,7 @@ func TestUpdate(t *testing.T) {
Scope: &scopes.ScopeInfo{Id: or.GetScopeId(), Type: scope.Org.String(), ParentScopeId: scope.Global.String()},
Description: &wrapperspb.StringValue{Value: "default"},
CreatedTime: or.GetCreateTime().GetTimestamp(),
GrantScopeId: &wrapperspb.StringValue{Value: globals.GrantScopeThis},
GrantScopeIds: []string{globals.GrantScopeThis},
GrantScopeIds: []string{p.GetPublicId()},
GrantStrings: []string{grant.GetRaw()},
Grants: []*pb.Grant{grant},
PrincipalIds: []string{u.GetPublicId()},
@@ -1313,8 +1287,7 @@ func TestUpdate(t *testing.T) {
Name: &wrapperspb.StringValue{Value: "updated"},
Description: &wrapperspb.StringValue{Value: "default"},
CreatedTime: or.GetCreateTime().GetTimestamp(),
GrantScopeId: &wrapperspb.StringValue{Value: globals.GrantScopeThis},
GrantScopeIds: []string{globals.GrantScopeThis},
GrantScopeIds: []string{p.GetPublicId()},
GrantStrings: []string{grant.GetRaw()},
Grants: []*pb.Grant{grant},
PrincipalIds: []string{u.GetPublicId()},
@@ -1343,8 +1316,7 @@ func TestUpdate(t *testing.T) {
Name: &wrapperspb.StringValue{Value: "default"},
Description: &wrapperspb.StringValue{Value: "notignored"},
CreatedTime: or.GetCreateTime().GetTimestamp(),
GrantScopeId: &wrapperspb.StringValue{Value: globals.GrantScopeThis},
GrantScopeIds: []string{globals.GrantScopeThis},
GrantScopeIds: []string{p.GetPublicId()},
GrantStrings: []string{grant.GetRaw()},
Grants: []*pb.Grant{grant},
PrincipalIds: []string{u.GetPublicId()},