From fa700dc00278bd62675e4f79f200b385b067f61e Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Thu, 1 Oct 2020 10:51:54 -0400 Subject: [PATCH] Add account ID templating (#518) * Remove default role and create via the same mechanism as other initial resources --- internal/auth/auth.go | 9 ++- internal/cmd/base/initial_resources.go | 54 ++++++++++++++++- internal/cmd/base/servers.go | 4 ++ internal/cmd/commands/database/funcs.go | 33 ++++++++++- internal/cmd/commands/database/init.go | 59 ++++++++++++++----- internal/cmd/commands/roles/role.go | 2 +- internal/db/domains_test.go | 4 -- internal/db/migrations/postgres.gen.go | 32 +--------- .../postgres/01_domain_types.up.sql | 4 +- internal/db/migrations/postgres/06_iam.up.sql | 28 --------- .../iam/repository_principal_role_test.go | 2 +- internal/iam/repository_role_grant.go | 4 +- internal/iam/repository_role_grant_test.go | 2 +- internal/iam/repository_role_test.go | 14 +---- internal/iam/repository_scope.go | 11 +++- internal/iam/role_grant.go | 4 +- internal/perms/acl_test.go | 25 +++++++- internal/perms/grants.go | 18 ++++-- internal/perms/grants_test.go | 29 ++++++++- internal/perms/option.go | 47 +++++++++++++++ .../controller/handlers/roles/role_service.go | 7 +-- .../handlers/roles/role_service_test.go | 12 +--- .../handlers/scopes/scope_service_test.go | 4 +- internal/servers/controller/testing.go | 25 +++++--- 24 files changed, 291 insertions(+), 142 deletions(-) create mode 100644 internal/perms/option.go diff --git a/internal/auth/auth.go b/internal/auth/auth.go index bce094e3c..02fae261e 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -292,6 +292,7 @@ func (v verifier) performAuthCheck() (aclResults perms.ACLResults, userId string _ = retErr scopeInfo = new(scopes.ScopeInfo) userId = "u_anon" + var accountId string // Validate the token and fetch the corresponding user ID switch v.requestInfo.TokenFormat { @@ -321,6 +322,7 @@ func (v verifier) performAuthCheck() (aclResults perms.ACLResults, userId string break } if at != nil { + accountId = at.GetAuthAccountId() userId = at.GetIamUserId() if userId == "" { v.logger.Warn("perform auth check: valid token did not map to a user, likely because no account is associated with the user any longer; continuing as u_anon", "token_id", at.GetPublicId()) @@ -385,7 +387,12 @@ func (v verifier) performAuthCheck() (aclResults perms.ACLResults, userId string } parsedGrants = make([]perms.Grant, 0, len(grantPairs)) for _, pair := range grantPairs { - parsed, err := perms.Parse(pair.ScopeId, userId, pair.Grant, true) + parsed, err := perms.Parse( + pair.ScopeId, + pair.Grant, + perms.WithUserId(userId), + perms.WithAccountId(accountId), + perms.WithSkipFinalValidation(true)) if err != nil { retErr = fmt.Errorf("perform auth check: failed to parse grant %#v: %w", pair.Grant, err) return diff --git a/internal/cmd/base/initial_resources.go b/internal/cmd/base/initial_resources.go index 145339457..536163449 100644 --- a/internal/cmd/base/initial_resources.go +++ b/internal/cmd/base/initial_resources.go @@ -15,6 +15,58 @@ import ( "github.com/hashicorp/vault/sdk/helper/base62" ) +func (b *Server) CreateInitialLoginRole(ctx context.Context) (*iam.Role, error) { + rw := db.New(b.Database) + + kmsRepo, err := kms.NewRepository(rw, rw) + if err != nil { + return nil, fmt.Errorf("error creating kms repository: %w", err) + } + kmsCache, err := kms.NewKms(kmsRepo, kms.WithLogger(b.Logger.Named("kms"))) + if err != nil { + return nil, fmt.Errorf("error creating kms cache: %w", err) + } + if err := kmsCache.AddExternalWrappers( + kms.WithRootWrapper(b.RootKms), + ); err != nil { + return nil, fmt.Errorf("error adding config keys to kms: %w", err) + } + cancelCtx, cancel := context.WithCancel(ctx) + go func() { + <-b.ShutdownCh + cancel() + }() + + iamRepo, err := iam.NewRepository(rw, rw, kmsCache, iam.WithRandomReader(b.SecureRandomReader)) + if err != nil { + return nil, fmt.Errorf("unable to create repo for initial login role: %w", err) + } + + pr, err := iam.NewRole(scope.Global.String(), + iam.WithName("Login and Default Grants"), + iam.WithDescription(`Role created for login capability and account self-management for users of the global scope at its creation time`), + ) + if err != nil { + return nil, fmt.Errorf("error creating in memory role for generated grants: %w", err) + } + role, err := iamRepo.CreateRole(cancelCtx, pr) + if err != nil { + return nil, fmt.Errorf("error creating role for default generated grants: %w", err) + } + if _, err := iamRepo.AddRoleGrants(cancelCtx, role.PublicId, role.Version, []string{ + "type=scope;actions=list", + "id=*;type=auth-method;actions=authenticate,list", + "id={{account.id}};actions=read,change-password", + }); err != nil { + return nil, fmt.Errorf("error creating grant for default generated grants: %w", err) + } + if _, err := iamRepo.AddPrincipalRoles(cancelCtx, role.PublicId, role.Version+1, []string{"u_anon"}, nil); err != nil { + return nil, fmt.Errorf("error adding principal to role for default generated grants: %w", err) + } + + return role, nil +} + func (b *Server) CreateInitialAuthMethod(ctx context.Context) (*password.AuthMethod, *iam.User, error) { rw := db.New(b.Database) @@ -122,7 +174,7 @@ func (b *Server) CreateInitialAuthMethod(ctx context.Context) (*password.AuthMet } // Create a role tying them together pr, err := iam.NewRole(scope.Global.String(), - iam.WithName("Generated global scope admin role"), + iam.WithName("Administration"), iam.WithDescription(`Provides admin grants within the "global" scope to the initial user`), ) if err != nil { diff --git a/internal/cmd/base/servers.go b/internal/cmd/base/servers.go index 5561b7d41..ca7e6b2aa 100644 --- a/internal/cmd/base/servers.go +++ b/internal/cmd/base/servers.go @@ -498,6 +498,10 @@ func (b *Server) CreateDevDatabase(dialect string, opt ...Option) error { return err } + if _, err := b.CreateInitialLoginRole(context.Background()); err != nil { + return err + } + if opts.withSkipAuthMethodCreation { // now that we have passed all the error cases, reset c to be a noop so the // defer doesn't do anything. diff --git a/internal/cmd/commands/database/funcs.go b/internal/cmd/commands/database/funcs.go index 9d0f139a8..a5714b7f2 100644 --- a/internal/cmd/commands/database/funcs.go +++ b/internal/cmd/commands/database/funcs.go @@ -6,7 +6,34 @@ import ( "github.com/hashicorp/boundary/internal/cmd/base" ) -type AuthMethodInfo struct { +type RoleInfo struct { + RoleId string `json:"scope_id"` + Name string `json:"name"` +} + +func generateInitialRoleTableOutput(in *RoleInfo) string { + nonAttributeMap := map[string]interface{}{ + "Role ID": in.RoleId, + "Name": in.Name, + } + + maxLength := 0 + for k := range nonAttributeMap { + if len(k) > maxLength { + maxLength = len(k) + } + } + + ret := []string{ + "", + "Initial login role information:", + base.WrapMap(2, maxLength+2, nonAttributeMap), + } + + return base.WrapForHelpText(ret) +} + +type AuthInfo struct { AuthMethodId string `json:"auth_method_id"` AuthMethodName string `json:"auth_method_name"` LoginName string `json:"login_name"` @@ -16,7 +43,7 @@ type AuthMethodInfo struct { UserName string `json:"user_name"` } -func generateInitialAuthMethodTableOutput(in *AuthMethodInfo) string { +func generateInitialAuthTableOutput(in *AuthInfo) string { nonAttributeMap := map[string]interface{}{ "Scope ID": in.ScopeId, "Auth Method ID": in.AuthMethodId, @@ -36,7 +63,7 @@ func generateInitialAuthMethodTableOutput(in *AuthMethodInfo) string { ret := []string{ "", - "Initial auth method information:", + "Initial auth information:", base.WrapMap(2, maxLength+2, nonAttributeMap), } diff --git a/internal/cmd/commands/database/init.go b/internal/cmd/commands/database/init.go index 4d210c576..b1b076afd 100644 --- a/internal/cmd/commands/database/init.go +++ b/internal/cmd/commands/database/init.go @@ -31,15 +31,16 @@ type InitCommand struct { configWrapper wrapping.Wrapper - flagConfig string - flagConfigKms string - flagLogLevel string - flagLogFormat string - flagMigrationUrl string - flagSkipAuthMethodCreation bool - flagSkipScopesCreation bool - flagSkipHostResourcesCreation bool - flagSkipTargetCreation bool + flagConfig string + flagConfigKms string + flagLogLevel string + flagLogFormat string + flagMigrationUrl string + flagSkipInitialLoginRoleCreation bool + flagSkipAuthMethodCreation bool + flagSkipScopesCreation bool + flagSkipHostResourcesCreation bool + flagSkipTargetCreation bool } func (c *InitCommand) Synopsis() string { @@ -56,6 +57,7 @@ func (c *InitCommand) Help() string { "", " Unless told not to via flags, some initial resources will be created, in the following order and in the indicated scopes:", "", + " Initial Login Role (global)", " Password-Type Auth Method (global)", " Org Scope (global)", " Project Scope (org)", @@ -113,6 +115,12 @@ func (c *InitCommand) Flags() *base.FlagSets { f = set.NewFlagSet("Init Options") + f.BoolVar(&base.BoolVar{ + Name: "skip-initial-login-role-creation", + Target: &c.flagSkipInitialLoginRoleCreation, + Usage: "If not set, a default role allowing necessary grants for logging in will not be created as part of initialization. If set, the recovery KMS will be needed to perform any actions.", + }) + f.BoolVar(&base.BoolVar{ Name: "skip-auth-method-creation", Target: &c.flagSkipAuthMethodCreation, @@ -283,10 +291,6 @@ func (c *InitCommand) Run(args []string) (retCode int) { c.UI.Info("Global-scope KMS keys successfully created.") } - if c.flagSkipAuthMethodCreation { - return 0 - } - var jsonMap map[string]interface{} if base.Format(c.UI) == "json" { jsonMap = make(map[string]interface{}) @@ -301,6 +305,31 @@ func (c *InitCommand) Run(args []string) (retCode int) { }() } + if c.flagSkipInitialLoginRoleCreation { + return 0 + } + + role, err := c.srv.CreateInitialLoginRole(c.Context) + if err != nil { + c.UI.Error(fmt.Errorf("Error creating initial global-scoped login role: %w", err).Error()) + return 1 + } + + roleInfo := &RoleInfo{ + RoleId: role.PublicId, + Name: role.Name, + } + switch base.Format(c.UI) { + case "table": + c.UI.Output(generateInitialRoleTableOutput(roleInfo)) + case "json": + jsonMap["login_role"] = roleInfo + } + + if c.flagSkipAuthMethodCreation { + return 0 + } + // Use an easy name, at least c.srv.DevLoginName = "admin" am, user, err := c.srv.CreateInitialAuthMethod(c.Context) @@ -309,7 +338,7 @@ func (c *InitCommand) Run(args []string) (retCode int) { return 1 } - authMethodInfo := &AuthMethodInfo{ + authMethodInfo := &AuthInfo{ AuthMethodId: c.srv.DevAuthMethodId, AuthMethodName: am.Name, LoginName: c.srv.DevLoginName, @@ -320,7 +349,7 @@ func (c *InitCommand) Run(args []string) (retCode int) { } switch base.Format(c.UI) { case "table": - c.UI.Output(generateInitialAuthMethodTableOutput(authMethodInfo)) + c.UI.Output(generateInitialAuthTableOutput(authMethodInfo)) case "json": jsonMap["auth_method"] = authMethodInfo } diff --git a/internal/cmd/commands/roles/role.go b/internal/cmd/commands/roles/role.go index adf63b0f7..377f094e2 100644 --- a/internal/cmd/commands/roles/role.go +++ b/internal/cmd/commands/roles/role.go @@ -182,7 +182,7 @@ func (c *Command) Run(args []string) int { if len(grants) > 0 { for _, grant := range grants { - _, err := perms.Parse("global", "", grant, false) + _, err := perms.Parse("global", grant) if err != nil { c.UI.Error(fmt.Errorf("Grant %q could not be parsed successfully: %w", grant, err).Error()) return 1 diff --git a/internal/db/domains_test.go b/internal/db/domains_test.go index cdc3e2a77..5427e9f36 100644 --- a/internal/db/domains_test.go +++ b/internal/db/domains_test.go @@ -53,10 +53,6 @@ returning id; "00000009", "", }, - { - "r_default", - "role", - }, { "global", "scope", diff --git a/internal/db/migrations/postgres.gen.go b/internal/db/migrations/postgres.gen.go index 37f49d209..7b803c897 100644 --- a/internal/db/migrations/postgres.gen.go +++ b/internal/db/migrations/postgres.gen.go @@ -64,10 +64,10 @@ comment on domain wt_scope_id is create domain wt_role_id as text not null check( - length(trim(value)) > 10 or value = 'r_default' + length(trim(value)) > 10 ); comment on domain wt_scope_id is -'"r_default", or random ID generated with github.com/hashicorp/vault/sdk/helper/base62'; +'Random ID generated with github.com/hashicorp/vault/sdk/helper/base62'; create domain wt_timestamp as timestamp with time zone @@ -945,24 +945,6 @@ before insert on iam_role_grant for each row execute procedure default_create_time(); -create or replace function - disallow_r_default_deletion() - returns trigger -as $$ -begin - if old.public_id = 'r_default' then - raise exception 'deletion of r_default not allowed'; - end if; - return old; -end; -$$ language plpgsql; - -create trigger - iam_role_disallow_global_deletion -before -delete on iam_role - for each row execute procedure disallow_r_default_deletion(); - create trigger update_version_column after update on iam_role @@ -1099,16 +1081,6 @@ begin end; $$ language plpgsql; -insert into iam_role (public_id, name, description, scope_id) - values('r_default', 'Default Grants', 'Default role created on first instantiation of Boundary. It is meant to provide enough permissions for users to successfully authenticate via various client types.', 'global'); -insert into iam_role_grant (role_id, canonical_grant, raw_grant) - values - ('r_default', 'type=scope;actions=list', 'type=scope;actions=list'), - ('r_default', 'id=*;type=auth-method;actions=authenticate,list', 'id=*;type=auth-method;actions=authenticate,list'); -insert into iam_user_role (role_id, principal_id) - values - ('r_default', 'u_anon'); - -- iam_principle_role provides a consolidated view all principal roles assigned -- (user and group roles). create view iam_principal_role as diff --git a/internal/db/migrations/postgres/01_domain_types.up.sql b/internal/db/migrations/postgres/01_domain_types.up.sql index cdbc0a64e..9fa5ba7d0 100644 --- a/internal/db/migrations/postgres/01_domain_types.up.sql +++ b/internal/db/migrations/postgres/01_domain_types.up.sql @@ -33,10 +33,10 @@ comment on domain wt_scope_id is create domain wt_role_id as text not null check( - length(trim(value)) > 10 or value = 'r_default' + length(trim(value)) > 10 ); comment on domain wt_scope_id is -'"r_default", or random ID generated with github.com/hashicorp/vault/sdk/helper/base62'; +'Random ID generated with github.com/hashicorp/vault/sdk/helper/base62'; create domain wt_timestamp as timestamp with time zone diff --git a/internal/db/migrations/postgres/06_iam.up.sql b/internal/db/migrations/postgres/06_iam.up.sql index 76a05aee7..10b32924b 100644 --- a/internal/db/migrations/postgres/06_iam.up.sql +++ b/internal/db/migrations/postgres/06_iam.up.sql @@ -427,24 +427,6 @@ before insert on iam_role_grant for each row execute procedure default_create_time(); -create or replace function - disallow_r_default_deletion() - returns trigger -as $$ -begin - if old.public_id = 'r_default' then - raise exception 'deletion of r_default not allowed'; - end if; - return old; -end; -$$ language plpgsql; - -create trigger - iam_role_disallow_global_deletion -before -delete on iam_role - for each row execute procedure disallow_r_default_deletion(); - create trigger update_version_column after update on iam_role @@ -581,16 +563,6 @@ begin end; $$ language plpgsql; -insert into iam_role (public_id, name, description, scope_id) - values('r_default', 'Default Grants', 'Default role created on first instantiation of Boundary. It is meant to provide enough permissions for users to successfully authenticate via various client types.', 'global'); -insert into iam_role_grant (role_id, canonical_grant, raw_grant) - values - ('r_default', 'type=scope;actions=list', 'type=scope;actions=list'), - ('r_default', 'id=*;type=auth-method;actions=authenticate,list', 'id=*;type=auth-method;actions=authenticate,list'); -insert into iam_user_role (role_id, principal_id) - values - ('r_default', 'u_anon'); - -- iam_principle_role provides a consolidated view all principal roles assigned -- (user and group roles). create view iam_principal_role as diff --git a/internal/iam/repository_principal_role_test.go b/internal/iam/repository_principal_role_test.go index e444fe07d..2d5b33656 100644 --- a/internal/iam/repository_principal_role_test.go +++ b/internal/iam/repository_principal_role_test.go @@ -264,7 +264,7 @@ func TestRepository_ListPrincipalRoles(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) - require.NoError(conn.Where("public_id != ?", "r_default").Delete(allocRole()).Error) + require.NoError(conn.Where("1=1").Delete(allocRole()).Error) role := TestRole(t, conn, tt.createScopeId) userRoles := make([]string, 0, tt.createCnt) groupRoles := make([]string, 0, tt.createCnt) diff --git a/internal/iam/repository_role_grant.go b/internal/iam/repository_role_grant.go index 5ee317d75..21221bc28 100644 --- a/internal/iam/repository_role_grant.go +++ b/internal/iam/repository_role_grant.go @@ -162,7 +162,7 @@ func (r *Repository) DeleteRoleGrants(ctx context.Context, roleId string, roleVe deleteRoleGrants := make([]interface{}, 0, len(grants)) for _, grant := range grants { // Use a fake scope, just want to get out a canonical string - perm, err := perms.Parse("o_abcd1234", "", grant, true) + perm, err := perms.Parse("o_abcd1234", grant, perms.WithSkipFinalValidation(true)) if err != nil { return fmt.Errorf("delete role grants: error parsing grant string: %w", err) } @@ -253,7 +253,7 @@ func (r *Repository) SetRoleGrants(ctx context.Context, roleId string, roleVersi deleteRoleGrants := make([]interface{}, 0, len(grants)) for _, grant := range grants { // Use a fake scope, just want to get out a canonical string - perm, err := perms.Parse("o_abcd1234", "", grant, true) + perm, err := perms.Parse("o_abcd1234", grant, perms.WithSkipFinalValidation(true)) if err != nil { return nil, db.NoRowsAffected, fmt.Errorf("set role grants: error parsing grant string: %w", err) } diff --git a/internal/iam/repository_role_grant_test.go b/internal/iam/repository_role_grant_test.go index 07dfc2594..33a6f7608 100644 --- a/internal/iam/repository_role_grant_test.go +++ b/internal/iam/repository_role_grant_test.go @@ -190,7 +190,7 @@ func TestRepository_ListRoleGrants(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) - require.NoError(conn.Where("public_id != ?", "r_default").Delete(allocRole()).Error) + require.NoError(conn.Where("1=1").Delete(allocRole()).Error) role := TestRole(t, conn, tt.createScopeId) roleGrants := make([]string, 0, tt.createCnt) for i := 0; i < tt.createCnt; i++ { diff --git a/internal/iam/repository_role_test.go b/internal/iam/repository_role_test.go index 0a17729f8..47f2ea858 100644 --- a/internal/iam/repository_role_test.go +++ b/internal/iam/repository_role_test.go @@ -502,19 +502,7 @@ func TestRepository_DeleteRole(t *testing.T) { wantErr: true, wantErrMsg: "delete role: missing public id invalid parameter", }, - { - name: "r_default", - args: args{ - role: func() *Role { - r := allocRole() - r.PublicId = "r_default" - return &r - }(), - }, - wantRowsDeleted: 0, - wantErr: true, - wantErrMsg: `deletion of r_default not allowed`, - }, + { name: "not-found", args: args{ diff --git a/internal/iam/repository_scope.go b/internal/iam/repository_scope.go index 388c84102..c8e8f89d6 100644 --- a/internal/iam/repository_scope.go +++ b/internal/iam/repository_scope.go @@ -135,8 +135,8 @@ func (r *Repository) CreateScope(ctx context.Context, s *Scope, userId string, o return nil, fmt.Errorf("create scope: error generating public id for new default role: %w", err) } defaultRole.PublicId = defaultRolePublicId - defaultRole.Name = "Default Grants" - defaultRole.Description = fmt.Sprintf("Role created for authentication to and listing of some resources of scope %s at its creation time", scopePublicId) + defaultRole.Name = "Login and Default Grants" + defaultRole.Description = fmt.Sprintf("Role created for login capability and account self-management for users of scope %s at its creation time", scopePublicId) defaultRoleRaw = defaultRole defaultRoleMetadata = oplog.Metadata{ "resource-public-id": []string{defaultRolePublicId}, @@ -290,8 +290,13 @@ func (r *Repository) CreateScope(ctx context.Context, s *Scope, userId string, o return fmt.Errorf("unable to create in memory role grant: %w", err) } grants = append(grants, roleGrant) + roleGrant, err = NewRoleGrant(defaultRolePublicId, "id={{account.id}};actions=read,change-password") + if err != nil { + return fmt.Errorf("unable to create in memory role grant: %w", err) + } + grants = append(grants, roleGrant) - roleGrantOplogMsgs := make([]*oplog.Message, 0, 2) + roleGrantOplogMsgs := make([]*oplog.Message, 0, 3) if err := w.CreateItems(ctx, grants, db.NewOplogMsgs(&roleGrantOplogMsgs)); err != nil { return fmt.Errorf("unable to add grants: %w", err) } diff --git a/internal/iam/role_grant.go b/internal/iam/role_grant.go index 0de8d18ef..4c44f2cdc 100644 --- a/internal/iam/role_grant.go +++ b/internal/iam/role_grant.go @@ -34,7 +34,7 @@ func NewRoleGrant(roleId string, grant string, opt ...Option) (*RoleGrant, error // Validate that the grant parses successfully. Note that we fake the scope // here to avoid a lookup as the scope is only relevant at actual ACL // checking time and we just care that it parses correctly. - perm, err := perms.Parse("o_abcd1234", "", grant, false) + perm, err := perms.Parse("o_abcd1234", grant) if err != nil { return nil, fmt.Errorf("new role grant: error parsing grant string: %w", err) } @@ -73,7 +73,7 @@ func (g *RoleGrant) VetForWrite(ctx context.Context, r db.Reader, opType db.OpTy // checking time and we just care that it parses correctly. We may have // already done this in NewRoleGrant, but we re-check and set it here // anyways because it should still be part of the vetting process. - perm, err := perms.Parse("o_abcd1234", "", g.RawGrant, false) + perm, err := perms.Parse("o_abcd1234", g.RawGrant) if err != nil { return fmt.Errorf("vet role grant for writing: error parsing grant string: %w", err) } diff --git a/internal/perms/acl_test.go b/internal/perms/acl_test.go index 54fe40219..8cac46049 100644 --- a/internal/perms/acl_test.go +++ b/internal/perms/acl_test.go @@ -26,6 +26,7 @@ func Test_ACLAllowed(t *testing.T) { resource Resource actionsAllowed []actionAllowed userId string + accountId string } // A set of common grants to use in the following tests @@ -51,6 +52,7 @@ func Test_ACLAllowed(t *testing.T) { scope: "o_c", grants: []string{ "id={{user.id }};actions=read,update", + "id={{ account.id}};actions=change-password", }, }, { @@ -219,6 +221,27 @@ func Test_ACLAllowed(t *testing.T) { }, userId: "u_abcd1234", }, + { + name: "bad templated account id", + resource: Resource{ScopeId: "o_c"}, + scopeGrants: commonGrants, + actionsAllowed: []actionAllowed{ + {action: action.List}, + {action: action.Authenticate}, + {action: action.Delete}, + }, + accountId: "apw_1234567890", + }, + { + name: "good templated user id", + resource: Resource{ScopeId: "o_c", Id: "apw_1234567890"}, + scopeGrants: commonGrants, + actionsAllowed: []actionAllowed{ + {action: action.ChangePassword, allowed: true}, + {action: action.Update}, + }, + accountId: "apw_1234567890", + }, { name: "all type", resource: Resource{ScopeId: "o_d", Type: resource.Account}, @@ -236,7 +259,7 @@ func Test_ACLAllowed(t *testing.T) { var grants []Grant for _, sg := range test.scopeGrants { for _, g := range sg.grants { - grant, err := Parse(sg.scope, test.userId, g, false) + grant, err := Parse(sg.scope, g, WithAccountId(test.accountId), WithUserId(test.userId)) require.NoError(t, err) grants = append(grants, grant) } diff --git a/internal/perms/grants.go b/internal/perms/grants.go index 8dcf8f8bd..92ac9320d 100644 --- a/internal/perms/grants.go +++ b/internal/perms/grants.go @@ -230,7 +230,7 @@ func (g *Grant) unmarshalText(grantString string) error { // // The scope must be the org and project where this grant originated, not the // request. -func Parse(scopeId, userId, grantString string, skipFinalValidation bool) (Grant, error) { +func Parse(scopeId, grantString string, opt ...Option) (Grant, error) { if len(grantString) == 0 { return Grant{}, errors.New("grant string is empty") } @@ -266,14 +266,22 @@ func Parse(scopeId, userId, grantString string, skipFinalValidation bool) (Grant } } - // Check for templated user ID, and subtitute in with the authenticated user + opts := getOpts(opt...) + + // Check for templated values ID, and subtitute in with the authenticated values // if so - if grant.id != "" && userId != "" && strings.HasPrefix(grant.id, "{{") { + if grant.id != "" && strings.HasPrefix(grant.id, "{{") { id := strings.TrimSuffix(strings.TrimPrefix(grant.id, "{{"), "}}") id = strings.ToLower(strings.TrimSpace(id)) switch id { case "user.id": - grant.id = userId + if opts.withUserId != "" { + grant.id = opts.withUserId + } + case "account.id": + if opts.withAccountId != "" { + grant.id = opts.withAccountId + } default: return Grant{}, fmt.Errorf("unknown template %q in grant %q value", grant.id, "id") } @@ -287,7 +295,7 @@ func Parse(scopeId, userId, grantString string, skipFinalValidation bool) (Grant return Grant{}, err } - if !skipFinalValidation { + if !opts.withSkipFinalValidation { // Validate the grant. Create a dummy resource and pass it through // Allowed and ensure that we get allowed. acl := NewACL(grant) diff --git a/internal/perms/grants_test.go b/internal/perms/grants_test.go index e01b20858..3dfc49f3e 100644 --- a/internal/perms/grants_test.go +++ b/internal/perms/grants_test.go @@ -316,6 +316,7 @@ func Test_Parse(t *testing.T) { name string input string userId string + accountId string err string scopeOverride string expected Grant @@ -479,13 +480,35 @@ func Test_Parse(t *testing.T) { }, }, }, + { + name: "bad account id template", + input: `id={{superman}};actions=create,read`, + accountId: "apw_1234567890", + err: `unknown template "{{superman}}" in grant "id" value`, + }, + { + name: "good account id template", + input: `id={{ account.id}};actions=create,read`, + accountId: "apw_1234567890", + expected: Grant{ + scope: Scope{ + Id: "o_scope", + Type: scope.Org, + }, + id: "apw_1234567890", + actions: map[action.Type]bool{ + action.Create: true, + action.Read: true, + }, + }, + }, } - _, err := Parse("", "", "", false) + _, err := Parse("", "") require.Error(t, err) assert.Equal(t, "grant string is empty", err.Error()) - _, err = Parse("", "", "{}", false) + _, err = Parse("", "{}") require.Error(t, err) assert.Equal(t, "no scope ID provided", err.Error()) @@ -497,7 +520,7 @@ func Test_Parse(t *testing.T) { if test.scopeOverride != "" { scope = test.scopeOverride } - grant, err := Parse(scope, test.userId, test.input, false) + grant, err := Parse(scope, test.input, WithUserId(test.userId), WithAccountId(test.accountId)) if test.err != "" { require.Error(err) assert.True(strings.Contains(err.Error(), test.err), err.Error()) diff --git a/internal/perms/option.go b/internal/perms/option.go new file mode 100644 index 000000000..ab779ddee --- /dev/null +++ b/internal/perms/option.go @@ -0,0 +1,47 @@ +package perms + +// getOpts - iterate the inbound Options and return a struct +func getOpts(opt ...Option) options { + opts := getDefaultOptions() + for _, o := range opt { + o(&opts) + } + return opts +} + +// Option - how Options are passed as arguments +type Option func(*options) + +// options = how options are represented +type options struct { + withUserId string + withAccountId string + withSkipFinalValidation bool +} + +func getDefaultOptions() options { + return options{} +} + +// WithUserId provides a user ID to be used for any templating in grant strings +func WithUserId(userId string) Option { + return func(o *options) { + o.withUserId = userId + } +} + +// WithAccountId provides an account ID to be used for any templating in grant +// strings +func WithAccountId(accountId string) Option { + return func(o *options) { + o.withAccountId = accountId + } +} + +// WithSkipFinalValidation allows skipping the validity step where we ensure we +// can run a resource described by the grant successfully through the ACL check +func WithSkipFinalValidation(skipFinalValidation bool) Option { + return func(o *options) { + o.withSkipFinalValidation = skipFinalValidation + } +} diff --git a/internal/servers/controller/handlers/roles/role_service.go b/internal/servers/controller/handlers/roles/role_service.go index 16a2b0ffe..5d790f064 100644 --- a/internal/servers/controller/handlers/roles/role_service.go +++ b/internal/servers/controller/handlers/roles/role_service.go @@ -538,7 +538,7 @@ func toProto(in *iam.Role, principals []iam.PrincipalRole, grants []*iam.RoleGra } for _, g := range grants { out.GrantStrings = append(out.GrantStrings, g.GetRawGrant()) - parsed, err := perms.Parse(in.GetGrantScopeId(), "", g.GetRawGrant(), false) + parsed, err := perms.Parse(in.GetGrantScopeId(), g.GetRawGrant()) if err != nil { // This should never happen as we validate on the way in, but let's // return what we can since we are still returning the raw grant @@ -625,11 +625,6 @@ func validateUpdateRequest(req *pbs.UpdateRoleRequest) error { func validateDeleteRequest(req *pbs.DeleteRoleRequest) error { return handlers.ValidateDeleteRequest(iam.RolePrefix, req, func() map[string]string { - if req.GetId() == "r_default" { - return map[string]string{ - "id": `Deleting "r_default" is not allowed`, - } - } return nil }) } diff --git a/internal/servers/controller/handlers/roles/role_service_test.go b/internal/servers/controller/handlers/roles/role_service_test.go index b21cb1cf1..5d91e7e27 100644 --- a/internal/servers/controller/handlers/roles/role_service_test.go +++ b/internal/servers/controller/handlers/roles/role_service_test.go @@ -279,14 +279,6 @@ func TestDelete(t *testing.T) { }, err: handlers.ApiErrorWithCode(codes.NotFound), }, - { - name: "Delete default role", - scopeId: "global", - req: &pbs.DeleteRoleRequest{ - Id: "r_default", - }, - err: handlers.ApiErrorWithCode(codes.InvalidArgument), - }, { name: "Bad Role Id formatting", scopeId: or.GetPublicId(), @@ -504,7 +496,7 @@ func TestCreate(t *testing.T) { func TestUpdate(t *testing.T) { grantString := "id=*;type=*;actions=*" - g, err := perms.Parse("global", "", grantString, false) + g, err := perms.Parse("global", grantString) require.NoError(t, err) _, actions := g.Actions() grant := &pb.Grant{ @@ -1339,7 +1331,7 @@ func checkEqualGrants(t *testing.T, expected []string, got *pb.Role) { require.Equal(len(expected), len(got.GrantStrings)) require.Equal(len(expected), len(got.Grants)) for i, v := range expected { - parsed, err := perms.Parse("o_abc123", "", v, false) + parsed, err := perms.Parse("o_abc123", v) require.NoError(err) assert.Equal(expected[i], got.GrantStrings[i]) assert.Equal(expected[i], got.Grants[i].GetRaw()) diff --git a/internal/servers/controller/handlers/scopes/scope_service_test.go b/internal/servers/controller/handlers/scopes/scope_service_test.go index 5dc933d8e..b07dde17b 100644 --- a/internal/servers/controller/handlers/scopes/scope_service_test.go +++ b/internal/servers/controller/handlers/scopes/scope_service_test.go @@ -614,8 +614,8 @@ func TestCreate(t *testing.T) { switch role.GetName() { case "Administration": assert.Equal(fmt.Sprintf("Role created for administration of scope %s by user %s at its creation time", got.GetItem().GetId(), userId), role.GetDescription()) - case "Default Grants": - assert.Equal(fmt.Sprintf("Role created for authentication to and listing of some resources of scope %s at its creation time", got.GetItem().GetId()), role.GetDescription()) + case "Login and Default Grants": + assert.Equal(fmt.Sprintf("Role created for login capability and account self-management for users of scope %s at its creation time", got.GetItem().GetId()), role.GetDescription()) default: t.Fatal("unexpected role name", role.GetName()) } diff --git a/internal/servers/controller/testing.go b/internal/servers/controller/testing.go index f22aeb122..d8b6c8241 100644 --- a/internal/servers/controller/testing.go +++ b/internal/servers/controller/testing.go @@ -230,6 +230,10 @@ type TestControllerOpts struct { // DefaultPassword is the password used when creating the default account. DefaultPassword string + // DisableInitialLoginRoleCreation can be set true to disable creating the + // global scope login role automatically. + DisableInitialLoginRoleCreation bool + // DisableAuthMethodCreation can be set true to disable creating an auth // method automatically. DisableAuthMethodCreation bool @@ -389,22 +393,27 @@ func NewTestController(t *testing.T, opts *TestControllerOpts) *TestController { if err := tc.b.CreateGlobalKmsKeys(ctx); err != nil { t.Fatal(err) } - if !opts.DisableAuthMethodCreation { - if _, _, err := tc.b.CreateInitialAuthMethod(ctx); err != nil { + if !opts.DisableInitialLoginRoleCreation { + if _, err := tc.b.CreateInitialLoginRole(context.Background()); err != nil { t.Fatal(err) } - if !opts.DisableScopesCreation { - if _, _, err := tc.b.CreateInitialScopes(ctx); err != nil { + if !opts.DisableAuthMethodCreation { + if _, _, err := tc.b.CreateInitialAuthMethod(ctx); err != nil { t.Fatal(err) } - if !opts.DisableHostResourcesCreation { - if _, _, _, err := tc.b.CreateInitialHostResources(ctx); err != nil { + if !opts.DisableScopesCreation { + if _, _, err := tc.b.CreateInitialScopes(ctx); err != nil { t.Fatal(err) } - if !opts.DisableTargetCreation { - if _, err := tc.b.CreateInitialTarget(ctx); err != nil { + if !opts.DisableHostResourcesCreation { + if _, _, _, err := tc.b.CreateInitialHostResources(ctx); err != nil { t.Fatal(err) } + if !opts.DisableTargetCreation { + if _, err := tc.b.CreateInitialTarget(ctx); err != nil { + t.Fatal(err) + } + } } } }