From 11fd0d0300ce136343f3a7c875a85d12f030c314 Mon Sep 17 00:00:00 2001 From: Todd Knight Date: Thu, 8 Oct 2020 12:36:01 -0700 Subject: [PATCH] Fixing some API error and comment copy/paste errors. (#580) * Fixing some error and comment copy/paste errors. * Fix roles in project scope error message. --- .../controller/handlers/accounts/account_service.go | 2 +- .../handlers/authenticate/authenticate_service.go | 11 ++--------- .../handlers/authtokens/authtoken_service.go | 4 ++-- .../controller/handlers/groups/group_service.go | 6 +++--- .../handlers/host_catalogs/host_catalog_service.go | 4 ++-- .../controller/handlers/host_sets/host_set_service.go | 6 +++--- .../servers/controller/handlers/hosts/host_service.go | 2 +- .../servers/controller/handlers/roles/role_service.go | 4 ++-- .../controller/handlers/scopes/scope_service.go | 2 +- .../controller/handlers/targets/target_service.go | 6 +++--- .../servers/controller/handlers/users/user_service.go | 4 ++-- 11 files changed, 22 insertions(+), 29 deletions(-) diff --git a/internal/servers/controller/handlers/accounts/account_service.go b/internal/servers/controller/handlers/accounts/account_service.go index 652968c046..1c85162185 100644 --- a/internal/servers/controller/handlers/accounts/account_service.go +++ b/internal/servers/controller/handlers/accounts/account_service.go @@ -260,7 +260,7 @@ func (s Service) updateInRepo(ctx context.Context, scopeId, authMethId, id strin return nil, fmt.Errorf("unable to update auth method: %w", err) } if rowsUpdated == 0 { - return nil, handlers.NotFoundErrorf("AuthMethod %q doesn't exist or incorrect version provided.", id) + return nil, handlers.NotFoundErrorf("Account %q doesn't exist or incorrect version provided.", id) } return toProto(out) } diff --git a/internal/servers/controller/handlers/authenticate/authenticate_service.go b/internal/servers/controller/handlers/authenticate/authenticate_service.go index b6f92f5f14..8a2a8d67f0 100644 --- a/internal/servers/controller/handlers/authenticate/authenticate_service.go +++ b/internal/servers/controller/handlers/authenticate/authenticate_service.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/hashicorp/boundary/internal/auth" + "github.com/hashicorp/boundary/internal/auth/password" "github.com/hashicorp/boundary/internal/authtoken" pba "github.com/hashicorp/boundary/internal/gen/controller/api/resources/authtokens" "github.com/hashicorp/boundary/internal/gen/controller/api/resources/scopes" @@ -178,7 +179,7 @@ func validateAuthenticateRequest(req *pbs.AuthenticateRequest) error { badFields := make(map[string]string) if strings.TrimSpace(req.GetAuthMethodId()) == "" { badFields["auth_method_id"] = "This is a required field." - } else if validId(req.GetAuthMethodId(), "am") { + } else if !handlers.ValidId(password.AuthMethodPrefix, req.GetAuthMethodId()) { badFields["auth_method_id"] = "Invalid formatted identifier." } // TODO: Update this when we enable different auth method types. @@ -201,11 +202,3 @@ func validateAuthenticateRequest(req *pbs.AuthenticateRequest) error { } return nil } - -func validId(id, prefix string) bool { - if !strings.HasPrefix(id, prefix) { - return false - } - id = strings.TrimPrefix(id, prefix) - return !reInvalidID.Match([]byte(id)) -} diff --git a/internal/servers/controller/handlers/authtokens/authtoken_service.go b/internal/servers/controller/handlers/authtokens/authtoken_service.go index 17b56e0cdd..c4ac38963b 100644 --- a/internal/servers/controller/handlers/authtokens/authtoken_service.go +++ b/internal/servers/controller/handlers/authtokens/authtoken_service.go @@ -98,7 +98,7 @@ func (s Service) getFromRepo(ctx context.Context, id string) (*pb.AuthToken, err if errors.Is(err, db.ErrRecordNotFound) { return nil, handlers.NotFoundErrorf("AuthToken %q doesn't exist.", id) } - return nil, fmt.Errorf("unable to list auth tokens: %w", err) + return nil, fmt.Errorf("unable to lookup auth token: %w", err) } if u == nil { return nil, handlers.NotFoundErrorf("AuthToken %q doesn't exist.", id) @@ -214,7 +214,7 @@ func validateListRequest(req *pbs.ListAuthTokensRequest) error { badFields := map[string]string{} if !handlers.ValidId(scope.Org.Prefix(), req.GetScopeId()) && req.GetScopeId() != scope.Global.String() { - badFields["scope_id"] = "Incorrectly formatted identifier." + badFields["scope_id"] = "This field must be 'global' or a valid org scope id." } if len(badFields) > 0 { return handlers.InvalidArgumentErrorf("Improperly formatted identifier.", badFields) diff --git a/internal/servers/controller/handlers/groups/group_service.go b/internal/servers/controller/handlers/groups/group_service.go index ed1cd6cde3..7ecd3be2ce 100644 --- a/internal/servers/controller/handlers/groups/group_service.go +++ b/internal/servers/controller/handlers/groups/group_service.go @@ -469,7 +469,7 @@ func validateAddGroupMembersRequest(req *pbs.AddGroupMembersRequest) error { } for _, id := range req.GetMemberIds() { if !handlers.ValidId(iam.UserPrefix, id) { - badFields["member_ids"] = "Must only contain valid user ids." + badFields["member_ids"] = fmt.Sprintf("Must only contain valid user ids but found %q.", id) break } if id == "u_recovery" { @@ -493,7 +493,7 @@ func validateSetGroupMembersRequest(req *pbs.SetGroupMembersRequest) error { } for _, id := range req.GetMemberIds() { if !handlers.ValidId(iam.UserPrefix, id) { - badFields["member_ids"] = "Must only contain valid user ids." + badFields["member_ids"] = fmt.Sprintf("Must only contain valid user ids but found %q.", id) break } if id == "u_recovery" { @@ -520,7 +520,7 @@ func validateRemoveGroupMembersRequest(req *pbs.RemoveGroupMembersRequest) error } for _, id := range req.GetMemberIds() { if !handlers.ValidId(iam.UserPrefix, id) { - badFields["member_ids"] = "Must only contain valid user ids." + badFields["member_ids"] = fmt.Sprintf("Must only contain valid user ids but found %q.", id) break } } diff --git a/internal/servers/controller/handlers/host_catalogs/host_catalog_service.go b/internal/servers/controller/handlers/host_catalogs/host_catalog_service.go index 8fb71fd93c..f125bc3965 100644 --- a/internal/servers/controller/handlers/host_catalogs/host_catalog_service.go +++ b/internal/servers/controller/handlers/host_catalogs/host_catalog_service.go @@ -147,7 +147,7 @@ func (s Service) getFromRepo(ctx context.Context, id string) (*pb.HostCatalog, e return nil, err } if hc == nil { - return nil, handlers.NotFoundErrorf("Host catalog %q doesn't exist.", id) + return nil, handlers.NotFoundErrorf("Host Catalog %q doesn't exist.", id) } return toProto(hc), nil } @@ -221,7 +221,7 @@ func (s Service) updateInRepo(ctx context.Context, projId, id string, mask []str return nil, fmt.Errorf("unable to update host catalog: %w", err) } if rowsUpdated == 0 { - return nil, handlers.NotFoundErrorf("Host catalog %q doesn't exist or incorrect version provided.", id) + return nil, handlers.NotFoundErrorf("Host Catalog %q doesn't exist or incorrect version provided.", id) } return toProto(out), nil } diff --git a/internal/servers/controller/handlers/host_sets/host_set_service.go b/internal/servers/controller/handlers/host_sets/host_set_service.go index 0b8389afc4..c62457695f 100644 --- a/internal/servers/controller/handlers/host_sets/host_set_service.go +++ b/internal/servers/controller/handlers/host_sets/host_set_service.go @@ -38,7 +38,7 @@ type Service struct { var _ pbs.HostSetServiceServer = Service{} -// NewService returns a host catalog Service which handles host catalog related requests to boundary and uses the provided +// NewService returns a host set Service which handles host set related requests to boundary and uses the provided // repositories for storage and retrieval. func NewService(repoFn common.StaticRepoFactory) (Service, error) { if repoFn == nil { @@ -196,7 +196,7 @@ func (s Service) getFromRepo(ctx context.Context, id string) (*pb.HostSet, error return nil, err } if h == nil { - return nil, handlers.NotFoundErrorf("Host set %q doesn't exist.", id) + return nil, handlers.NotFoundErrorf("Host Set %q doesn't exist.", id) } return toProto(h, m), nil } @@ -257,7 +257,7 @@ func (s Service) updateInRepo(ctx context.Context, scopeId, catalogId, id string return nil, fmt.Errorf("unable to update host set: %w.", err) } if rowsUpdated == 0 { - return nil, handlers.NotFoundErrorf("Host set %q doesn't exist or incorrect version provided.", id) + return nil, handlers.NotFoundErrorf("Host Set %q doesn't exist or incorrect version provided.", id) } return toProto(out, m), nil } diff --git a/internal/servers/controller/handlers/hosts/host_service.go b/internal/servers/controller/handlers/hosts/host_service.go index 5de84cf8cd..c4f115fdb1 100644 --- a/internal/servers/controller/handlers/hosts/host_service.go +++ b/internal/servers/controller/handlers/hosts/host_service.go @@ -39,7 +39,7 @@ type Service struct { var _ pbs.HostServiceServer = Service{} -// NewService returns a host catalog Service which handles host catalog related requests to boundary and uses the provided +// NewService returns a host Service which handles host related requests to boundary and uses the provided // repositories for storage and retrieval. func NewService(repoFn common.StaticRepoFactory) (Service, error) { if repoFn == nil { diff --git a/internal/servers/controller/handlers/roles/role_service.go b/internal/servers/controller/handlers/roles/role_service.go index fcfdeaf035..f5f99343fe 100644 --- a/internal/servers/controller/handlers/roles/role_service.go +++ b/internal/servers/controller/handlers/roles/role_service.go @@ -587,7 +587,7 @@ func validateCreateRequest(req *pbs.CreateRoleRequest) error { } if item.GetGrantScopeId() != nil && handlers.ValidId(scope.Project.Prefix(), item.GetScopeId()) { if item.GetGrantScopeId().GetValue() != item.GetScopeId() { - badFields["grant_scope_id"] = "Must be empty or set to the project_id when the scope type is project." + badFields["grant_scope_id"] = "When the role is in a project scope this value must be that project's scope ID." } } if item.GetPrincipals() != nil { @@ -617,7 +617,7 @@ func validateUpdateRequest(req *pbs.UpdateRoleRequest) error { } if req.GetItem().GetGrantScopeId() != nil && handlers.ValidId(scope.Project.Prefix(), req.GetItem().GetScopeId()) { if req.GetItem().GetGrantScopeId().GetValue() != req.GetItem().GetScopeId() { - badFields["grant_scope_id"] = "Must be empty or set to the project_id when the scope type is project." + badFields["grant_scope_id"] = "When the role is in a project scope this value must be that project's scope ID" } } return badFields diff --git a/internal/servers/controller/handlers/scopes/scope_service.go b/internal/servers/controller/handlers/scopes/scope_service.go index e597656c02..7352481e9a 100644 --- a/internal/servers/controller/handlers/scopes/scope_service.go +++ b/internal/servers/controller/handlers/scopes/scope_service.go @@ -265,7 +265,7 @@ func (s Service) listFromRepo(ctx context.Context, scopeId string) ([]*pb.Scope, scps, err = repo.ListProjects(ctx, scopeId) default: return nil, handlers.InvalidArgumentErrorf("Error in provided request.", - map[string]string{"scope_id": "Invalid id for listing."}) + map[string]string{"scope_id": "This field must be 'global' or a valid org scope id."}) } if err != nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to list scopes: %v", err) diff --git a/internal/servers/controller/handlers/targets/target_service.go b/internal/servers/controller/handlers/targets/target_service.go index d05a92cc5b..b2d95af44a 100644 --- a/internal/servers/controller/handlers/targets/target_service.go +++ b/internal/servers/controller/handlers/targets/target_service.go @@ -805,7 +805,7 @@ func validateAddRequest(req *pbs.AddTargetHostSetsRequest) error { } for _, id := range req.GetHostSetIds() { if !handlers.ValidId(static.HostSetPrefix, id) { - badFields["host_set_ids"] = "Incorrectly formatted host set identifier." + badFields["host_set_ids"] = fmt.Sprintf("Incorrectly formatted host set identifier %q.", id) break } } @@ -825,7 +825,7 @@ func validateSetRequest(req *pbs.SetTargetHostSetsRequest) error { } for _, id := range req.GetHostSetIds() { if !handlers.ValidId(static.HostSetPrefix, id) { - badFields["host_set_ids"] = "Incorrectly formatted host set identifier." + badFields["host_set_ids"] = fmt.Sprintf("Incorrectly formatted host set identifier %q.", id) break } } @@ -848,7 +848,7 @@ func validateRemoveRequest(req *pbs.RemoveTargetHostSetsRequest) error { } for _, id := range req.GetHostSetIds() { if !handlers.ValidId(static.HostSetPrefix, id) { - badFields["host_set_ids"] = "Incorrectly formatted host set identifier." + badFields["host_set_ids"] = fmt.Sprintf("Incorrectly formatted host set identifier %q.", id) break } } diff --git a/internal/servers/controller/handlers/users/user_service.go b/internal/servers/controller/handlers/users/user_service.go index 0217595eff..4b22cbeaac 100644 --- a/internal/servers/controller/handlers/users/user_service.go +++ b/internal/servers/controller/handlers/users/user_service.go @@ -427,7 +427,7 @@ func validateCreateRequest(req *pbs.CreateUserRequest) error { badFields := map[string]string{} if !handlers.ValidId(scope.Org.Prefix(), req.GetItem().GetScopeId()) && scope.Global.String() != req.GetItem().GetScopeId() { - badFields["scope_id"] = "This field is missing or improperly formatted." + badFields["scope_id"] = "Must be 'global' or a valid org scope id." } return badFields }) @@ -445,7 +445,7 @@ func validateListRequest(req *pbs.ListUsersRequest) error { badFields := map[string]string{} if !handlers.ValidId(scope.Org.Prefix(), req.GetScopeId()) && req.GetScopeId() != scope.Global.String() { - badFields["scope_id"] = "Invalidly formatted required identifer." + badFields["scope_id"] = "Must be 'global' or a valid org scope id when listing." } if len(badFields) > 0 { return handlers.InvalidArgumentErrorf("Improperly formatted identifier.", badFields)