From b16ba07a334c6fd915f7a71949ef982d35ae8ad1 Mon Sep 17 00:00:00 2001 From: Emily Ye Date: Wed, 27 Nov 2019 22:32:01 -0800 Subject: [PATCH 01/35] initial static account changes Co-authored-by: emily --- plugin/backend.go | 16 +- plugin/field_data_utils.go | 104 +++++++ plugin/gcp_account_resources.go | 161 ++++++++++- plugin/iamutil/iam_policy.go | 6 +- plugin/path_role_set.go | 25 +- plugin/path_roleset_secrets.go | 145 ++++++++++ plugin/path_static_account.go | 345 +++++++++++++++++++++++ plugin/path_static_account_rotate_key.go | 112 ++++++++ plugin/path_static_account_secrets.go | 109 +++++++ plugin/role_set.go | 80 ++---- plugin/rollback.go | 213 +++++++------- plugin/secrets_access_token.go | 53 +--- plugin/secrets_service_account_key.go | 149 ++++------ plugin/secrets_test.go | 186 +++--------- plugin/static_account.go | 343 ++++++++++++++++++++++ 15 files changed, 1575 insertions(+), 472 deletions(-) create mode 100644 plugin/field_data_utils.go create mode 100644 plugin/path_roleset_secrets.go create mode 100644 plugin/path_static_account.go create mode 100644 plugin/path_static_account_rotate_key.go create mode 100644 plugin/path_static_account_secrets.go create mode 100644 plugin/static_account.go diff --git a/plugin/backend.go b/plugin/backend.go index 114f61b4..7f039938 100644 --- a/plugin/backend.go +++ b/plugin/backend.go @@ -35,7 +35,8 @@ type backend struct { resources iamutil.ResourceParser - rolesetLock sync.Mutex + rolesetLock sync.Mutex + staticAccountLock sync.Mutex } // Factory returns a new backend as logical.Backend. @@ -69,12 +70,21 @@ func Backend() *backend { []*framework.Path{ pathConfig(b), pathConfigRotateRoot(b), + // Roleset pathRoleSet(b), pathRoleSetList(b), pathRoleSetRotateAccount(b), pathRoleSetRotateKey(b), - pathSecretAccessToken(b), - pathSecretServiceAccountKey(b), + pathRoleSetSecretAccessToken(b), + pathRoleSetSecretServiceAccountKey(b), + deprecatedPathRoleSetSecretAccessToken(b), + deprecatedPathRoleSetSecretServiceAccountKey(b), + // Static Account + pathStaticAccount(b), + pathStaticAccountList(b), + pathStaticAccountRotateKey(b), + pathStaticAccountSecretAccessToken(b), + pathStaticAccountSecretServiceAccountKey(b), }, ), Secrets: []*framework.Secret{ diff --git a/plugin/field_data_utils.go b/plugin/field_data_utils.go new file mode 100644 index 00000000..0c9255d6 --- /dev/null +++ b/plugin/field_data_utils.go @@ -0,0 +1,104 @@ +package gcpsecrets + +import ( + "fmt" + "github.com/hashicorp/errwrap" + "github.com/hashicorp/vault-plugin-secrets-gcp/plugin/util" + "github.com/hashicorp/vault/sdk/framework" +) + +type inputParams struct { + name string + secretType string + + hasBindings bool + rawBindings string + bindings ResourceBindings + + project string + serviceAccountEmail string + + scopes []string +} + +func (input *inputParams) parseOkInputSecretType(d *framework.FieldData) (warnings []string, err error) { + secretType := d.Get("secret_type").(string) + if secretType == "" && input.secretType == "" { + return nil, fmt.Errorf("secret_type is required") + } + if input.secretType != "" && secretType != "" && input.secretType != secretType { + return nil, fmt.Errorf("cannot update secret_type - currently %q", input.secretType) + } + + switch secretType { + case SecretTypeKey, SecretTypeAccessToken: + input.secretType = secretType + return nil, nil + default: + return nil, fmt.Errorf("invalid secret_type %q", secretType) + } +} + +func (input *inputParams) parseOkInputEmail(d *framework.FieldData) (warnings []string, err error) { + email := d.Get("email").(string) + if email == "" && input.serviceAccountEmail == "" { + return nil, fmt.Errorf("email is required") + } + if input.serviceAccountEmail != "" && email != "" && input.serviceAccountEmail != email { + return nil, fmt.Errorf("cannot update email - currently %q", input.serviceAccountEmail) + } + + input.serviceAccountEmail = email + return nil, nil +} + +func (input *inputParams) parseOkInputTokenScopes(d *framework.FieldData) (warnings []string, err error) { + // Parse secretType if not yet parsed + if input.secretType == "" { + warnings, err = input.parseOkInputTokenScopes(d) + if err != nil { + return nil, err + } + } + + v, ok := d.GetOk("token_scopes") + if ok { + scopes, castOk := v.([]string) + if !castOk { + return nil, fmt.Errorf("scopes unexpected type %T, expected []string", v) + } + input.scopes = scopes + } + + if input.secretType == SecretTypeAccessToken && len(input.scopes) == 0 { + return nil, fmt.Errorf("non-empty token_scopes must be provided for generating access token secrets") + } + + if input.secretType != SecretTypeAccessToken && ok && len(input.scopes) > 0 { + warnings = append(warnings, "ignoring non-empty token scopes, secret type not access_token") + } + return +} + +func (input *inputParams) parseOkInputBindings(d *framework.FieldData) (warnings []string, err error) { + bRaw, ok := d.GetOk("bindings") + if !ok { + input.hasBindings = false + return nil, nil + } + + rawBindings, castok := bRaw.(string) + if !castok { + return nil, fmt.Errorf("bindings are not a string") + } + + bindings, err := util.ParseBindings(bRaw.(string)) + if err != nil { + return nil, errwrap.Wrapf("unable to parse bindings: {{err}}", err) + } + + input.hasBindings = true + input.rawBindings = rawBindings + input.bindings = bindings + return nil, nil +} diff --git a/plugin/gcp_account_resources.go b/plugin/gcp_account_resources.go index 99d7aa2a..6a8d3ff6 100644 --- a/plugin/gcp_account_resources.go +++ b/plugin/gcp_account_resources.go @@ -8,13 +8,20 @@ import ( "github.com/hashicorp/errwrap" "github.com/hashicorp/go-gcp-common/gcputil" + "github.com/hashicorp/go-multierror" "github.com/hashicorp/vault-plugin-secrets-gcp/plugin/iamutil" "github.com/hashicorp/vault-plugin-secrets-gcp/plugin/util" "github.com/hashicorp/vault/sdk/helper/useragent" "github.com/hashicorp/vault/sdk/logical" + "google.golang.org/api/googleapi" "google.golang.org/api/iam/v1" ) +const ( + flagCanDeleteServiceAccount = true + flagMustKeepServiceAccount = false +) + type ( // gcpAccountResources is a wrapper around the GCP resources Vault creates to generate credentials. // This includes a Vault-managed GCP service account (required), IAM bindings, and/or key via TokenGenerator @@ -44,6 +51,18 @@ func (rb ResourceBindings) asOutput() map[string][]string { return out } +func (rb ResourceBindings) sub(toRemove ResourceBindings) ResourceBindings { + subbed := make(ResourceBindings) + for r, iamRoles := range rb { + toRemoveIamRoles, ok := toRemove[r] + if ok { + iamRoles = iamRoles.Sub(toRemoveIamRoles) + } + subbed[r] = iamRoles + } + return subbed +} + func getStringHash(bindingsRaw string) string { ssum := sha256.Sum256([]byte(bindingsRaw)) return base64.StdEncoding.EncodeToString(ssum[:]) @@ -73,7 +92,7 @@ func (b *backend) createNewTokenGen(ctx context.Context, req *logical.Request, p } func (b *backend) createIamBindings(ctx context.Context, req *logical.Request, saEmail string, binds ResourceBindings) error { - b.Logger().Debug("creating IAM bindings", "account_email", saEmail) + b.Logger().Debug("creating IAM bindings", "account_email", saEmail, "bindings", binds) httpC, err := b.HTTPClient(req.Storage) if err != nil { return err @@ -131,6 +150,146 @@ func (b *backend) createServiceAccount(ctx context.Context, req *logical.Request return iamAdmin.Projects.ServiceAccounts.Create(fmt.Sprintf("projects/%s", project), createSaReq).Do() } +// tryDeleteGcpAccountResources creates WALs to clean up a service account's +// bindings, key, and account (if removeServiceAccount is true) +func (b *backend) tryDeleteGcpAccountResources(ctx context.Context, req *logical.Request, boundResources *gcpAccountResources, removeServiceAccount bool, walIds []string) []string { + if boundResources == nil { + b.Logger().Debug("skip deletion for nil roleset resources") + return nil + } + + b.Logger().Debug("try to delete GCP account resources", "bound resources", boundResources, "remove service account", removeServiceAccount) + + iamAdmin, err := b.IAMAdminClient(req.Storage) + if err != nil { + return []string{err.Error()} + } + + warnings := make([]string, 0) + if boundResources.tokenGen != nil { + if err := b.deleteTokenGenKey(ctx, iamAdmin, boundResources.tokenGen); err != nil { + w := fmt.Sprintf("unable to delete key under service account %q (WAL entry to clean-up later has been added): %v", boundResources.accountId.ResourceName(), err) + warnings = append(warnings, w) + } + } + + if merr := b.removeBindings(ctx, req, boundResources.accountId.EmailOrId, boundResources.bindings); merr != nil { + for _, err := range merr.Errors { + w := fmt.Sprintf("unable to delete IAM policy bindings for service account %q (WAL entry to clean-up later has been added): %v", boundResources.accountId.EmailOrId, err) + warnings = append(warnings, w) + } + } + + if removeServiceAccount { + if err := b.deleteServiceAccount(ctx, iamAdmin, boundResources.accountId); err != nil { + w := fmt.Sprintf("unable to delete service account %q (WAL entry to clean-up later has been added): %v", boundResources.accountId.ResourceName(), err) + warnings = append(warnings, w) + } + } + + // If resources were deleted, we don't need the WAL rollbacks we created for these resources. + if len(warnings) == 0 { + b.tryDeleteWALs(ctx, req.Storage, walIds...) + } + + return nil +} + +func (b *backend) deleteTokenGenKey(ctx context.Context, iamAdmin *iam.Service, tgen *TokenGenerator) error { + if tgen == nil || tgen.KeyName == "" { + return nil + } + + _, err := iamAdmin.Projects.ServiceAccounts.Keys.Delete(tgen.KeyName).Do() + if err != nil && !isGoogleAccountKeyNotFoundErr(err) { + return errwrap.Wrapf("unable to delete service account key: {{err}}", err) + } + return nil +} + +func (b *backend) removeBindings(ctx context.Context, req *logical.Request, email string, bindings ResourceBindings) (allErr *multierror.Error) { + httpC, err := b.HTTPClient(req.Storage) + if err != nil { + return &multierror.Error{Errors: []error{err}} + } + + apiHandle := iamutil.GetApiHandle(httpC, useragent.String()) + + for resName, roles := range bindings { + resource, err := b.resources.Parse(resName) + if err != nil { + allErr = multierror.Append(allErr, errwrap.Wrapf(fmt.Sprintf("unable to delete role binding for resource '%s': {{err}}", resName), err)) + continue + } + + p, err := resource.GetIamPolicy(ctx, apiHandle) + if err != nil { + allErr = multierror.Append(allErr, errwrap.Wrapf(fmt.Sprintf("unable to delete role binding for resource '%s': {{err}}", resName), err)) + continue + } + + changed, newP := p.RemoveBindings(&iamutil.PolicyDelta{ + Email: email, + Roles: roles, + }) + if !changed { + continue + } + if _, err = resource.SetIamPolicy(ctx, apiHandle, newP); err != nil { + allErr = multierror.Append(allErr, errwrap.Wrapf(fmt.Sprintf("unable to delete role binding for resource '%s': {{err}}", resName), err)) + continue + } + } + return +} + +func (b *backend) deleteServiceAccount(ctx context.Context, iamAdmin *iam.Service, account gcputil.ServiceAccountId) error { + if account.EmailOrId == "" { + return nil + } + + _, err := iamAdmin.Projects.ServiceAccounts.Delete(account.ResourceName()).Do() + if err != nil && !isGoogleAccountNotFoundErr(err) { + return errwrap.Wrapf("unable to delete service account: {{err}}", err) + } + return nil +} + +func isGoogleAccountNotFoundErr(err error) bool { + return isGoogleApiErrorWithCodes(err, 404) +} + +func isGoogleAccountKeyNotFoundErr(err error) bool { + return isGoogleApiErrorWithCodes(err, 403, 404) +} + +func isGoogleAccountUnauthorizedErr(err error) bool { + return isGoogleApiErrorWithCodes(err, 403) +} + +func isGoogleApiErrorWithCodes(err error, validErrCodes ...int) bool { + if err == nil { + return false + } + + gErr, ok := err.(*googleapi.Error) + if !ok { + wrapErrV := errwrap.GetType(err, &googleapi.Error{}) + if wrapErrV == nil { + return false + } + gErr = wrapErrV.(*googleapi.Error) + } + + for _, code := range validErrCodes { + if gErr.Code == code { + return true + } + } + + return false +} + func emailForServiceAccountName(project, accountName string) string { return fmt.Sprintf(serviceAccountEmailTemplate, accountName, project) } diff --git a/plugin/iamutil/iam_policy.go b/plugin/iamutil/iam_policy.go index 73c9299c..ad10a2ff 100644 --- a/plugin/iamutil/iam_policy.go +++ b/plugin/iamutil/iam_policy.go @@ -34,14 +34,14 @@ type PolicyDelta struct { } func (p *Policy) AddBindings(toAdd *PolicyDelta) (changed bool, updated *Policy) { - return p.ChangedBindings(toAdd, nil) + return p.ChangeBindings(toAdd, nil) } func (p *Policy) RemoveBindings(toRemove *PolicyDelta) (changed bool, updated *Policy) { - return p.ChangedBindings(nil, toRemove) + return p.ChangeBindings(nil, toRemove) } -func (p *Policy) ChangedBindings(toAdd *PolicyDelta, toRemove *PolicyDelta) (changed bool, updated *Policy) { +func (p *Policy) ChangeBindings(toAdd *PolicyDelta, toRemove *PolicyDelta) (changed bool, updated *Policy) { if toAdd == nil && toRemove == nil { return false, p } diff --git a/plugin/path_role_set.go b/plugin/path_role_set.go index 469bb3be..75db344a 100644 --- a/plugin/path_role_set.go +++ b/plugin/path_role_set.go @@ -157,7 +157,11 @@ func (b *backend) pathRoleSetRead(ctx context.Context, req *logical.Request, d * } func (b *backend) pathRoleSetDelete(ctx context.Context, req *logical.Request, d *framework.FieldData) (resp *logical.Response, err error) { - rsName := d.Get("name").(string) + nameRaw, ok := d.GetOk("name") + if !ok { + return logical.ErrorResponse("name is required"), nil + } + rsName := nameRaw.(string) b.rolesetLock.Lock() defer b.rolesetLock.Unlock() @@ -185,11 +189,11 @@ func (b *backend) pathRoleSetDelete(ctx context.Context, req *logical.Request, d } // Try to clean up resources. - if cleanupErr := b.tryDeleteRoleSetResources(ctx, req, resources, walIds); cleanupErr != nil { - b.Logger().Warn( - "unable to clean up unused GCP resources from deleted roleset. WALs exist to clean up but ignoring error", - "roleset", rsName, "errors", cleanupErr) - return &logical.Response{Warnings: []string{cleanupErr.Error()}}, nil + if warnings := b.tryDeleteRoleSetResources(ctx, req, resources, walIds); len(warnings) > 0 { + b.Logger().Debug( + "unable to delete GCP resources for deleted roleset but WALs exist to clean up, ignoring errors", + "roleset", rsName, "errors", warnings) + return &logical.Response{Warnings: warnings}, nil } b.Logger().Debug("successfully deleted roleset and GCP resources", "name", rsName) @@ -203,6 +207,9 @@ func (b *backend) pathRoleSetCreateUpdate(ctx context.Context, req *logical.Requ b.rolesetLock.Lock() defer b.rolesetLock.Unlock() + b.rolesetLock.Lock() + defer b.rolesetLock.Unlock() + rs, err := getRoleSet(name, ctx, req.Storage) if err != nil { return nil, err @@ -339,6 +346,9 @@ func (b *backend) pathRoleSetRotateAccount(ctx context.Context, req *logical.Req b.rolesetLock.Lock() defer b.rolesetLock.Unlock() + b.rolesetLock.Lock() + defer b.rolesetLock.Unlock() + rs, err := getRoleSet(name, ctx, req.Storage) if err != nil { return nil, err @@ -367,6 +377,9 @@ func (b *backend) pathRoleSetRotateKey(ctx context.Context, req *logical.Request b.rolesetLock.Lock() defer b.rolesetLock.Unlock() + b.rolesetLock.Lock() + defer b.rolesetLock.Unlock() + rs, err := getRoleSet(name, ctx, req.Storage) if err != nil { return nil, err diff --git a/plugin/path_roleset_secrets.go b/plugin/path_roleset_secrets.go new file mode 100644 index 00000000..d6283000 --- /dev/null +++ b/plugin/path_roleset_secrets.go @@ -0,0 +1,145 @@ +package gcpsecrets + +import ( + "context" + "fmt" + + "github.com/hashicorp/vault/sdk/framework" + "github.com/hashicorp/vault/sdk/logical" +) + +var fieldSchemaRoleSetServiceAccountKey = map[string]*framework.FieldSchema{ + "roleset": { + Type: framework.TypeString, + Description: "Required. Name of the role set.", + }, + "key_algorithm": { + Type: framework.TypeString, + Description: fmt.Sprintf(`Private key algorithm for service account key - defaults to %s"`, keyAlgorithmRSA2k), + Default: keyAlgorithmRSA2k, + }, + "key_type": { + Type: framework.TypeString, + Description: fmt.Sprintf(`Private key type for service account key - defaults to %s"`, privateKeyTypeJson), + Default: privateKeyTypeJson, + }, + "ttl": { + Type: framework.TypeDurationSecond, + Description: "Lifetime of the service account key", + }, +} + +var fieldSchemaRoleSetAccessToken = map[string]*framework.FieldSchema{ + "roleset": { + Type: framework.TypeString, + Description: "Required. Name of the role set.", + }, +} + +func pathRoleSetSecretServiceAccountKey(b *backend) *framework.Path { + return &framework.Path{ + Pattern: fmt.Sprintf("roleset/%s/key", framework.GenericNameRegex("roleset")), + Deprecated: true, + Fields: fieldSchemaRoleSetServiceAccountKey, + ExistenceCheck: b.pathRoleSetExistenceCheck("roleset"), + Operations: map[logical.Operation]framework.OperationHandler{ + logical.ReadOperation: &framework.PathOperation{Callback: b.pathRoleSetSecretKey}, + logical.UpdateOperation: &framework.PathOperation{Callback: b.pathRoleSetSecretKey}, + }, + HelpSynopsis: pathServiceAccountKeySyn, + HelpDescription: pathServiceAccountKeyDesc, + } +} + +func deprecatedPathRoleSetSecretServiceAccountKey(b *backend) *framework.Path { + return &framework.Path{ + Pattern: fmt.Sprintf("key/%s", framework.GenericNameRegex("roleset")), + Deprecated: true, + Fields: fieldSchemaRoleSetServiceAccountKey, + ExistenceCheck: b.pathRoleSetExistenceCheck("roleset"), + Operations: map[logical.Operation]framework.OperationHandler{ + logical.ReadOperation: &framework.PathOperation{Callback: b.pathRoleSetSecretKey}, + logical.UpdateOperation: &framework.PathOperation{Callback: b.pathRoleSetSecretKey}, + }, + HelpSynopsis: pathServiceAccountKeySyn, + HelpDescription: pathServiceAccountKeyDesc, + } +} + +func pathRoleSetSecretAccessToken(b *backend) *framework.Path { + return &framework.Path{ + Pattern: fmt.Sprintf("roleset/%s/token", framework.GenericNameRegex("roleset")), + Fields: fieldSchemaRoleSetAccessToken, + ExistenceCheck: b.pathRoleSetExistenceCheck("roleset"), + Operations: map[logical.Operation]framework.OperationHandler{ + logical.ReadOperation: &framework.PathOperation{Callback: b.pathRoleSetSecretAccessToken}, + logical.UpdateOperation: &framework.PathOperation{Callback: b.pathRoleSetSecretAccessToken}, + }, + HelpSynopsis: pathTokenHelpSyn, + HelpDescription: pathTokenHelpDesc, + } +} + +func deprecatedPathRoleSetSecretAccessToken(b *backend) *framework.Path { + return &framework.Path{ + Pattern: fmt.Sprintf("token/%s", framework.GenericNameRegex("roleset")), + Deprecated: true, + Fields: fieldSchemaRoleSetAccessToken, + ExistenceCheck: b.pathRoleSetExistenceCheck("roleset"), + Operations: map[logical.Operation]framework.OperationHandler{ + logical.ReadOperation: &framework.PathOperation{Callback: b.pathRoleSetSecretAccessToken}, + logical.UpdateOperation: &framework.PathOperation{Callback: b.pathRoleSetSecretAccessToken}, + }, + HelpSynopsis: pathTokenHelpSyn, + HelpDescription: pathTokenHelpDesc, + } +} + +func (b *backend) pathRoleSetSecretKey(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + rsName := d.Get("roleset").(string) + keyType := d.Get("key_type").(string) + keyAlg := d.Get("key_algorithm").(string) + ttl := d.Get("ttl").(int) + + rs, err := getRoleSet(rsName, ctx, req.Storage) + if err != nil { + return nil, err + } + if rs == nil { + return logical.ErrorResponse("role set %q does not exists", rsName), nil + } + + if rs.SecretType != SecretTypeKey { + return logical.ErrorResponse("role set %q cannot generate service account keys (has secret type %s)", rsName, rs.SecretType), nil + } + + params := secretKeyParams{ + keyType: keyType, + keyAlgorithm: keyAlg, + ttl: ttl, + extraInternalData: map[string]interface{}{ + "role_set": rs.Name, + "role_set_bindings": rs.bindingHash(), + }, + } + + return b.createServiceAccountKeySecret(ctx, req.Storage, rs.AccountId, params) +} + +func (b *backend) pathRoleSetSecretAccessToken(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + rsName := d.Get("roleset").(string) + + rs, err := getRoleSet(rsName, ctx, req.Storage) + if err != nil { + return nil, err + } + if rs == nil { + return logical.ErrorResponse("role set '%s' does not exists", rsName), nil + } + + if rs.SecretType != SecretTypeAccessToken { + return logical.ErrorResponse("role set '%s' cannot generate access tokens (has secret type %s)", rsName, rs.SecretType), nil + } + + return b.secretAccessTokenResponse(ctx, req.Storage, rs.TokenGen) +} diff --git a/plugin/path_static_account.go b/plugin/path_static_account.go new file mode 100644 index 00000000..b894cdd5 --- /dev/null +++ b/plugin/path_static_account.go @@ -0,0 +1,345 @@ +package gcpsecrets + +import ( + "context" + "errors" + "fmt" + "github.com/hashicorp/errwrap" + "github.com/hashicorp/vault/sdk/framework" + "github.com/hashicorp/vault/sdk/logical" +) + +const ( + staticAccountStoragePrefix = "static" + staticAccountPathPrefix = "static" + gcpServiceAccountInferredProject = "-" +) + +func pathStaticAccount(b *backend) *framework.Path { + return &framework.Path{ + Pattern: fmt.Sprintf("%s/%s", staticAccountPathPrefix, framework.GenericNameRegex("name")), + Fields: map[string]*framework.FieldSchema{ + "name": { + Type: framework.TypeString, + Description: "Required. Name to refer to this static account in Vault.", + }, + "secret_type": { + Type: framework.TypeString, + Description: fmt.Sprintf("Type of secret generated for this account. Defaults to %q", SecretTypeAccessToken), + Default: SecretTypeAccessToken, + }, + "email": { + Type: framework.TypeString, + Description: "Name of the GCP service account to manage.", + }, + "bindings": { + Type: framework.TypeString, + Description: "Bindings configuration string.", + }, + "token_scopes": { + Type: framework.TypeCommaStringSlice, + Description: fmt.Sprintf(`List of OAuth scopes to assign to access tokens generated under this account. Ignored if secret_type not %q`, SecretTypeKey), + }, + }, + ExistenceCheck: b.pathStaticAccountExistenceCheck, + Operations: map[logical.Operation]framework.OperationHandler{ + logical.DeleteOperation: &framework.PathOperation{ + Callback: b.pathStaticAccountDelete, + }, + logical.ReadOperation: &framework.PathOperation{ + Callback: b.pathStaticAccountRead, + }, + logical.CreateOperation: &framework.PathOperation{ + Callback: b.pathStaticAccountCreate, + }, + logical.UpdateOperation: &framework.PathOperation{ + Callback: b.pathStaticAccountUpdate, + }, + }, + HelpSynopsis: pathStaticAccountHelpSyn, + HelpDescription: pathStaticAccountHelpDesc, + } +} + +func pathStaticAccountList(b *backend) *framework.Path { + // Paths for listing role sets + return &framework.Path{ + Pattern: fmt.Sprintf("%ss?/?", staticAccountPathPrefix), + Operations: map[logical.Operation]framework.OperationHandler{ + logical.ListOperation: &framework.PathOperation{ + Callback: b.pathStaticAccountList, + }, + }, + HelpSynopsis: pathListStaticAccountHelpSyn, + HelpDescription: pathListStaticAccountHelpDesc, + } +} + +func (b *backend) pathStaticAccountExistenceCheck(ctx context.Context, req *logical.Request, d *framework.FieldData) (bool, error) { + nameRaw, ok := d.GetOk("name") + if !ok { + return false, errors.New("roleset name is required") + } + + acct, err := b.getStaticAccount(nameRaw.(string), ctx, req.Storage) + if err != nil { + return false, err + } + + return acct != nil, nil +} + +func (b *backend) pathStaticAccountRead(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + nameRaw, ok := d.GetOk("name") + if !ok { + return logical.ErrorResponse("name is required"), nil + } + + acct, err := b.getStaticAccount(nameRaw.(string), ctx, req.Storage) + if err != nil { + return nil, err + } + if acct == nil { + return nil, nil + } + + data := map[string]interface{}{ + "service_account_project": acct.Project, + "service_account_email": acct.EmailOrId, + "secret_type": acct.SecretType, + } + + if len(acct.Bindings) > 0 { + data["bindings"] = acct.Bindings.asOutput() + } + if acct.TokenGen != nil && acct.SecretType == SecretTypeAccessToken { + data["token_scopes"] = acct.TokenGen.Scopes + } + + return &logical.Response{ + Data: data, + }, nil +} + +func (b *backend) pathStaticAccountDelete(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + nameRaw, ok := d.GetOk("name") + if !ok { + return logical.ErrorResponse("name is required"), nil + } + name := nameRaw.(string) + + acct, err := b.getStaticAccount(name, ctx, req.Storage) + if err != nil { + return nil, errwrap.Wrapf(fmt.Sprintf("unable to get static account %q: {{err}}", name), err) + } + if acct == nil { + return nil, nil + } + + resources := acct.boundResources() + + // Add WALs + walIds, err := b.addWalsForStaticAccountResources(ctx, req, name, resources) + if err != nil { + return nil, errwrap.Wrapf(fmt.Sprintf("unable to create WALs for static account GCP resources %s: {{err}}", name), err) + } + + // Delete roleset + b.Logger().Debug("deleting roleset from storage", "name", name) + if err := req.Storage.Delete(ctx, fmt.Sprintf("%s/%s", staticAccountStoragePrefix, name)); err != nil { + return nil, err + } + + // Try to clean up resources. + if warnings := b.tryDeleteStaticAccountResources(ctx, req, resources, walIds); len(warnings) > 0 { + b.Logger().Debug( + "unable to delete GCP resources for deleted roleset but WALs exist to clean up, ignoring errors", + "roleset", name, "errors", warnings) + return &logical.Response{Warnings: warnings}, nil + } + + b.Logger().Debug("finished deleting static account from storage", "name", name) + return nil, nil +} + +func (b *backend) pathStaticAccountCreate(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + input, warnings, err := b.parseStaticAccountInformation(nil, d) + if err != nil { + return logical.ErrorResponse(err.Error()), nil + } + if input == nil { + return nil, fmt.Errorf("plugin error - parse returned unexpected nil input") + } + + b.staticAccountLock.Lock() + defer b.staticAccountLock.Unlock() + + // Create and save roleset with new resources. + if err := b.createStaticAccount(ctx, req, input); err != nil { + return logical.ErrorResponse(err.Error()), nil + } + if len(warnings) > 0 { + return &logical.Response{Warnings: warnings}, nil + } + return nil, nil +} + +func (b *backend) pathStaticAccountUpdate(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + nameRaw, ok := d.GetOk("name") + if !ok { + return logical.ErrorResponse("name is required"), nil + } + name := nameRaw.(string) + + b.staticAccountLock.Lock() + defer b.staticAccountLock.Unlock() + + acct, err := b.getStaticAccount(name, ctx, req.Storage) + if err != nil { + return nil, err + } + if acct == nil { + return nil, fmt.Errorf("unable to find static account %s to update", name) + } + + initialInput := &inputParams{ + name: acct.Name, + secretType: acct.SecretType, + rawBindings: acct.RawBindings, + bindings: acct.Bindings, + project: acct.Project, + serviceAccountEmail: acct.EmailOrId, + } + if acct.TokenGen != nil { + initialInput.scopes = acct.TokenGen.Scopes + } + + updateInput, warnings, err := b.parseStaticAccountInformation(initialInput, d) + if err != nil { + return logical.ErrorResponse(err.Error()), nil + } + if updateInput == nil { + return nil, fmt.Errorf("plugin error - parse returned unexpected nil input") + } + + updateWarns, err := b.updateStaticAccount(ctx, req, acct, updateInput) + if err != nil { + return logical.ErrorResponse("unable to update: %s", err), nil + } + warnings = append(warnings, updateWarns...) + if len(warnings) > 0 { + return &logical.Response{Warnings: warnings}, nil + } + return nil, nil +} + +func (b *backend) pathStaticAccountList(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + accounts, err := req.Storage.List(ctx, fmt.Sprintf("%s/", staticAccountStoragePrefix)) + if err != nil { + return nil, err + } + return logical.ListResponse(accounts), nil +} + +func (b *backend) parseStaticAccountInformation(prevValues *inputParams, d *framework.FieldData) (*inputParams, []string, error) { + var warnings []string + + input := prevValues + if prevValues == nil { + input = &inputParams{} + } + + nameRaw, ok := d.GetOk("name") + if !ok { + return nil, nil, fmt.Errorf("name is required") + } + input.name = nameRaw.(string) + + ws, err := input.parseOkInputSecretType(d) + if err != nil { + return nil, nil, err + } else if len(ws) > 0 { + warnings = append(warnings, ws...) + } + + ws, err = input.parseOkInputEmail(d) + if err != nil { + return nil, nil, err + } else if len(ws) > 0 { + warnings = append(warnings, ws...) + } + + ws, err = input.parseOkInputTokenScopes(d) + if err != nil { + return nil, nil, err + } else if len(ws) > 0 { + warnings = append(warnings, ws...) + } + + ws, err = input.parseOkInputBindings(d) + if err != nil { + return nil, nil, err + } else if len(ws) > 0 { + warnings = append(warnings, ws...) + } + + return input, warnings, nil +} + +const pathStaticAccountHelpSyn = `Register and manage a GCP service account to generate credentials under` +const pathStaticAccountHelpDesc = ` +This path allows you to register a static GCP service account that you want to generate secrets against. +This creates sets of IAM roles to specific GCP resources. Secrets (either service account keys or +access tokens) are generated under this account. The account must exist at creation of static account creation. + +If bindings are specified, Vault will assign IAM permissions to the given service account. Bindings +can be given as a HCL (or JSON) string with the following format: + +resource "some/gcp/resource/uri" { + roles = [ + "roles/role1", + "roles/role2", + "roles/role3", + ... + ] +} + +The given resource can have the following + +* Project-level self link + Self-link for a resource under a given project + (i.e. resource name starts with 'projects/...') + Use if you need to provide a versioned object or + are directly using resource.self_link. + + Example (Compute instance): + http://www.googleapis.com/compute/v1/projects/$PROJECT/zones/$ZONE/instances/$INSTANCE_NAME + +* Full Resource Name + A scheme-less URI consisting of a DNS-compatible + API service name and a resource path (i.e. the + relative resource name). Useful if you need to + specify what service this resource is under + but just want the preferred supported API version. + Note that if the resource you are using is for + a non-preferred API with multiple service versions, + you MUST specify the version. + + Example (IAM service account): + //$SERVICE.googleapis.com/projects/my-project/serviceAccounts/myserviceaccount@... + +* Relative Resource Name: + A URI path (path-noscheme) without the leading "/". + It identifies a resource within the API service. + Use if there is only one service that your + resource could belong to. If there are multiple + API versions that support the resource, we will + attempt to use the preferred version and ask + for more specific format otherwise. + + Example (Pubsub subscription): + projects/myproject/subscriptions/mysub +` + +const pathListStaticAccountHelpSyn = `List created static accounts.` +const pathListStaticAccountHelpDesc = `List created static accounts.` diff --git a/plugin/path_static_account_rotate_key.go b/plugin/path_static_account_rotate_key.go new file mode 100644 index 00000000..9ead9524 --- /dev/null +++ b/plugin/path_static_account_rotate_key.go @@ -0,0 +1,112 @@ +package gcpsecrets + +import ( + "context" + "fmt" + "github.com/hashicorp/vault/sdk/framework" + "github.com/hashicorp/vault/sdk/logical" +) + +func pathStaticAccountRotateKey(b *backend) *framework.Path { + return &framework.Path{ + Pattern: fmt.Sprintf("%s/%s/rotate-key", staticAccountPathPrefix, framework.GenericNameRegex("name")), + Fields: map[string]*framework.FieldSchema{ + "name": { + Type: framework.TypeString, + Description: "Name of the account.", + }, + }, + ExistenceCheck: b.pathStaticAccountExistenceCheck, + Operations: map[logical.Operation]framework.OperationHandler{ + logical.UpdateOperation: &framework.PathOperation{ + Callback: b.pathStaticAccountRotateKey, + }, + }, + HelpSynopsis: pathStaticAccountRotateKeyHelpSyn, + HelpDescription: pathStaticAccountRotateKeyHelpDesc, + } +} + +func (b *backend) pathStaticAccountRotateKey(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + nameRaw, ok := d.GetOk("name") + if !ok { + return logical.ErrorResponse("name is required"), nil + } + name := nameRaw.(string) + + b.staticAccountLock.Lock() + defer b.staticAccountLock.Unlock() + + acct, err := b.getStaticAccount(name, ctx, req.Storage) + if err != nil { + return nil, err + } + if acct == nil { + return logical.ErrorResponse("account '%s' not found", name), nil + } + + if acct.SecretType != SecretTypeAccessToken { + return logical.ErrorResponse("cannot rotate key for non-access-token static account"), nil + } + + if acct.TokenGen == nil { + return nil, fmt.Errorf("unexpected invalid account has no TokenGen") + } + + scopes := acct.TokenGen.Scopes + oldTokenGen := acct.TokenGen + oldWalId, err := b.addWalRoleSetServiceAccountKey(ctx, req, acct.Name, &acct.ServiceAccountId, oldTokenGen.KeyName) + if err != nil { + return nil, err + } + + // Add WALs for new TokenGen - since we don't have a key ID yet, give an empty key name so WAL + // will know to just clear keys that aren't being used. This also covers up cleaning up + // the old token generator, so we don't add a separate WAL for that. + newWalId, err := b.addWalRoleSetServiceAccountKey(ctx, req, acct.Name, &acct.ServiceAccountId, "") + if err != nil { + return nil, err + } + + newTokenGen, err := b.createNewTokenGen(ctx, req, acct.ResourceName(), scopes) + if err != nil { + return nil, err + } + + // Edit roleset with new key and save to storage. + acct.TokenGen = newTokenGen + if err := acct.save(ctx, req.Storage); err != nil { + return nil, err + } + + // Try deleting the old key. + iamAdmin, err := b.IAMAdminClient(req.Storage) + if err != nil { + return nil, err + } + + b.tryDeleteWALs(ctx, req.Storage, newWalId) + + if oldTokenGen != nil { + if err := b.deleteTokenGenKey(ctx, iamAdmin, oldTokenGen); err != nil { + return &logical.Response{ + Warnings: []string{ + fmt.Sprintf("saved static account with new token generator service account key but failed to delete old key (covered by WAL): %v", err), + }, + }, nil + } + b.tryDeleteWALs(ctx, req.Storage, oldWalId) + } + return nil, nil +} + +const pathStaticAccountRotateKeyHelpSyn = `Rotate the key used to generate access tokens for a static account` +const pathStaticAccountRotateKeyHelpDesc = ` +This path allows you to manually rotate the service account key +created by Vault for a static account that generates access tokens secrets. +This path only applies to static accounts that generate access tokens. +It will not delete the associated service account or change bindings. + +Note that this will not invalidate access tokens created with the old key. +The only way to do so is to delete the service account. +` diff --git a/plugin/path_static_account_secrets.go b/plugin/path_static_account_secrets.go new file mode 100644 index 00000000..38f858ce --- /dev/null +++ b/plugin/path_static_account_secrets.go @@ -0,0 +1,109 @@ +package gcpsecrets + +import ( + "context" + "fmt" + "github.com/hashicorp/vault/sdk/framework" + "github.com/hashicorp/vault/sdk/logical" +) + +func pathStaticAccountSecretServiceAccountKey(b *backend) *framework.Path { + return &framework.Path{ + Pattern: fmt.Sprintf("%s/%s/key", staticAccountPathPrefix, framework.GenericNameRegex("static_account")), + Deprecated: true, + Fields: map[string]*framework.FieldSchema{ + "static_account": { + Type: framework.TypeString, + Description: "Required. Name of the static_account.", + }, + "key_algorithm": { + Type: framework.TypeString, + Description: fmt.Sprintf(`Private key algorithm for service account key - defaults to %s"`, keyAlgorithmRSA2k), + Default: keyAlgorithmRSA2k, + }, + "key_type": { + Type: framework.TypeString, + Description: fmt.Sprintf(`Private key type for service account key - defaults to %s"`, privateKeyTypeJson), + Default: privateKeyTypeJson, + }, + "ttl": { + Type: framework.TypeDurationSecond, + Description: "Lifetime of the service account key", + }, + }, + ExistenceCheck: b.pathStaticAccountExistenceCheck, + Operations: map[logical.Operation]framework.OperationHandler{ + logical.ReadOperation: &framework.PathOperation{Callback: b.pathStaticAccountSecretKey}, + logical.UpdateOperation: &framework.PathOperation{Callback: b.pathStaticAccountSecretKey}, + }, + HelpSynopsis: pathServiceAccountKeySyn, + HelpDescription: pathServiceAccountKeyDesc, + } +} + +func pathStaticAccountSecretAccessToken(b *backend) *framework.Path { + return &framework.Path{ + Pattern: fmt.Sprintf("%s/%s/token", staticAccountPathPrefix, framework.GenericNameRegex("static_account")), + Deprecated: true, + Fields: map[string]*framework.FieldSchema{ + "static_account": { + Type: framework.TypeString, + Description: "Required. Name of the static_account.", + }, + }, + ExistenceCheck: b.pathStaticAccountExistenceCheck, + Operations: map[logical.Operation]framework.OperationHandler{ + logical.ReadOperation: &framework.PathOperation{Callback: b.pathStaticAccountAccessToken}, + logical.UpdateOperation: &framework.PathOperation{Callback: b.pathStaticAccountAccessToken}, + }, + HelpSynopsis: pathServiceAccountKeySyn, + HelpDescription: pathServiceAccountKeyDesc, + } +} + +func (b *backend) pathStaticAccountSecretKey(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + acctName := d.Get("static_account").(string) + keyType := d.Get("key_type").(string) + keyAlg := d.Get("key_algorithm").(string) + ttl := d.Get("ttl").(int) + + acct, err := b.getStaticAccount(acctName, ctx, req.Storage) + if err != nil { + return nil, err + } + if acct == nil { + return logical.ErrorResponse("static account %q does not exists", acctName), nil + } + if acct.SecretType != SecretTypeKey { + return logical.ErrorResponse("static account %q cannot generate service account keys (has secret type %q)", acctName, acct.SecretType), nil + } + + params := secretKeyParams{ + keyType: keyType, + keyAlgorithm: keyAlg, + ttl: ttl, + extraInternalData: map[string]interface{}{ + "static_account": acct.Name, + "static_account_bindings": acct.bindingHash(), + }, + } + + return b.createServiceAccountKeySecret(ctx, req.Storage, &acct.ServiceAccountId, params) +} + +func (b *backend) pathStaticAccountAccessToken(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + acctName := d.Get("static_account").(string) + + acct, err := b.getStaticAccount(acctName, ctx, req.Storage) + if err != nil { + return nil, err + } + if acct == nil { + return logical.ErrorResponse("static account %q does not exists", acctName), nil + } + if acct.SecretType != SecretTypeAccessToken { + return logical.ErrorResponse("static account %q cannot generate access tokens (has secret type %q)", acctName, acct.SecretType), nil + } + + return b.secretAccessTokenResponse(ctx, req.Storage, acct.TokenGen) +} diff --git a/plugin/role_set.go b/plugin/role_set.go index 03e522d2..1bb6607b 100644 --- a/plugin/role_set.go +++ b/plugin/role_set.go @@ -10,9 +10,7 @@ import ( "github.com/hashicorp/errwrap" "github.com/hashicorp/go-gcp-common/gcputil" "github.com/hashicorp/go-multierror" - "github.com/hashicorp/vault-plugin-secrets-gcp/plugin/iamutil" "github.com/hashicorp/vault/sdk/framework" - "github.com/hashicorp/vault/sdk/helper/useragent" "github.com/hashicorp/vault/sdk/logical" "google.golang.org/api/iam/v1" ) @@ -115,10 +113,7 @@ func (b *backend) getServiceAccount(iamAdmin *iam.Service, accountId *gcputil.Se account, err := iamAdmin.Projects.ServiceAccounts.Get(accountId.ResourceName()).Do() if err != nil { return nil, errwrap.Wrapf(fmt.Sprintf("could not find service account %q: {{err}}", accountId.ResourceName()), err) - } else if account == nil { - return nil, fmt.Errorf("couldn't fetch service account %q: {{err}}", accountId.ResourceName()) } - return account, nil } @@ -193,13 +188,8 @@ func (b *backend) saveRoleSetWithNewAccount(ctx context.Context, req *logical.Re // We successfully saved the new roleset with new resources, so try cleaning up WALs // that would rollback the roleset resources (will no-op if still in use by roleset) b.tryDeleteWALs(ctx, req.Storage, newWalIds...) - if cleanupErr := b.tryDeleteRoleSetResources(ctx, req, oldResources, oldWalIds); cleanupErr != nil { - b.Logger().Warn( - "unable to clean up unused old GCP resources for roleset. WALs exist to clean up but ignoring error", - "roleset", rs.Name, "errors", cleanupErr) - return []string{cleanupErr.Error()}, nil - } - return nil, nil + + return b.tryDeleteRoleSetResources(ctx, req, oldResources, oldWalIds), nil } // saveRoleSetWithNewTokenKey rotates the role set access_token key and saves it to storage. @@ -213,12 +203,21 @@ func (b *backend) saveRoleSetWithNewTokenKey(ctx context.Context, req *logical.R b.Logger().Debug("updating roleset with new account key") - oldTokenGen := rs.TokenGen + var oldTokenGen *TokenGenerator + var oldWalId string + if rs.TokenGen != nil { + scopes = rs.TokenGen.Scopes + oldTokenGen = rs.TokenGen + oldWalId, err = b.addWalRoleSetServiceAccountKey(ctx, req, rs.Name, rs.AccountId, oldTokenGen.KeyName) + if err != nil { + return "", err + } + } // Add WALs for new TokenGen - since we don't have a key ID yet, give an empty key name so WAL // will know to just clear keys that aren't being used. This also covers up cleaning up // the old token generator, so we don't add a separate WAL for that. - walId, err := b.addWalRoleSetServiceAccountKey(ctx, req, rs.Name, rs.AccountId, "") + newWalId, err := b.addWalRoleSetServiceAccountKey(ctx, req, rs.Name, rs.AccountId, "") if err != nil { return "", err } @@ -239,11 +238,14 @@ func (b *backend) saveRoleSetWithNewTokenKey(ctx context.Context, req *logical.R if err != nil { return "", err } - if err := b.deleteTokenGenKey(ctx, iamAdmin, oldTokenGen); err != nil { - return errwrap.Wrapf("roleset update succeeded but got error while trying to delete old key - will be cleaned up later by WAL: {{err}}", err).Error(), nil - } - b.tryDeleteWALs(ctx, req.Storage, walId) + b.tryDeleteWALs(ctx, req.Storage, newWalId) + if oldTokenGen != nil { + if err := b.deleteTokenGenKey(ctx, iamAdmin, oldTokenGen); err != nil { + return errwrap.Wrapf("roleset update succeeded but got error while trying to delete old key - will be cleaned up later by WAL: {{err}}", err).Error(), nil + } + b.tryDeleteWALs(ctx, req.Storage, oldWalId) + } return "", nil } @@ -309,46 +311,8 @@ func (b *backend) addWalRoleSetServiceAccountKey(ctx context.Context, req *logic // tryDeleteRoleSetResources tries to delete GCP resources previously managed by a roleset. // This assumes that deletion of these resources will already be guaranteed by WAL rollbacks (referred to by the walIds) // and will return errors as a list of warnings instead. -func (b *backend) tryDeleteRoleSetResources(ctx context.Context, req *logical.Request, boundResources *gcpAccountResources, walIds []string) error { - if boundResources == nil { - b.Logger().Debug("skip deletion for nil roleset resources") - return nil - } - - httpC, err := b.HTTPClient(req.Storage) - if err != nil { - return err - } - - iamAdmin, err := b.IAMAdminClient(req.Storage) - if err != nil { - return err - } - - iamHandle := iamutil.GetApiHandle(httpC, useragent.String()) - - var merr *multierror.Error - if boundResources.tokenGen != nil { - if err := b.deleteTokenGenKey(ctx, iamAdmin, boundResources.tokenGen); err != nil { - merr = multierror.Append(merr, fmt.Errorf("unable to delete key under service account %q (WAL entry to clean-up later has been added): %v", boundResources.accountId.ResourceName(), err)) - } - } - - if merr := b.removeBindings(ctx, iamHandle, boundResources.accountId.EmailOrId, boundResources.bindings); merr != nil { - for _, err := range merr.Errors { - merr = multierror.Append(merr, fmt.Errorf("unable to delete IAM policy bindings for service account %q (WAL entry to clean-up later has been added): %v", boundResources.accountId.EmailOrId, err)) - } - } - - if err := b.deleteServiceAccount(ctx, iamAdmin, boundResources.accountId); err != nil { - merr = multierror.Append(merr, fmt.Errorf("unable to delete service account %q (WAL entry to clean-up later has been added): %v", boundResources.accountId.ResourceName(), err)) - } - - // If resources were deleted, we don't need the WAL rollbacks we created for these resources. - if merr == nil || merr.Len() == 0 { - b.tryDeleteWALs(ctx, req.Storage, walIds...) - } - return merr.ErrorOrNil() +func (b *backend) tryDeleteRoleSetResources(ctx context.Context, req *logical.Request, boundResources *gcpAccountResources, walIds []string) []string { + return b.tryDeleteGcpAccountResources(ctx, req, boundResources, flagCanDeleteServiceAccount, walIds) } // generateAccountNameForRoleSet returns a new random name for a Vault service account based off roleset name and time. diff --git a/plugin/rollback.go b/plugin/rollback.go index 04cf6e64..c4947d31 100644 --- a/plugin/rollback.go +++ b/plugin/rollback.go @@ -4,23 +4,20 @@ import ( "context" "fmt" - "github.com/hashicorp/errwrap" "github.com/hashicorp/go-gcp-common/gcputil" - "github.com/hashicorp/go-multierror" "github.com/hashicorp/vault-plugin-secrets-gcp/plugin/iamutil" "github.com/hashicorp/vault-plugin-secrets-gcp/plugin/util" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/helper/useragent" "github.com/hashicorp/vault/sdk/logical" "github.com/mitchellh/mapstructure" - "google.golang.org/api/googleapi" - "google.golang.org/api/iam/v1" ) const ( - walTypeAccount = "account" - walTypeAccountKey = "account_key" - walTypeIamPolicy = "iam_policy" + walTypeAccount = "account" + walTypeAccountKey = "account_key" + walTypeIamPolicy = "iam_policy" + walTypeIamPolicyDiff = "iam_policy_diff" ) func (b *backend) walRollback(ctx context.Context, req *logical.Request, kind string, data interface{}) error { @@ -31,6 +28,8 @@ func (b *backend) walRollback(ctx context.Context, req *logical.Request, kind st return b.serviceAccountKeyRollback(ctx, req, data) case walTypeIamPolicy: return b.serviceAccountPolicyRollback(ctx, req, data) + case walTypeIamPolicyDiff: + return b.serviceAccountPolicyDiffRollback(ctx, req, data) default: return fmt.Errorf("unknown type to rollback") } @@ -43,6 +42,7 @@ type walAccount struct { type walAccountKey struct { RoleSet string + StaticAccount string ServiceAccountName string KeyName string } @@ -54,6 +54,14 @@ type walIamPolicy struct { Roles []string } +type walIamPolicyStaticAccount struct { + StaticAccount string + AccountId gcputil.ServiceAccountId + Resource string + RolesAdded []string + RolesRemoved []string +} + func (b *backend) serviceAccountRollback(ctx context.Context, req *logical.Request, data interface{}) error { b.rolesetLock.Lock() defer b.rolesetLock.Unlock() @@ -63,13 +71,13 @@ func (b *backend) serviceAccountRollback(ctx context.Context, req *logical.Reque return err } - // If account is still being used, WAL entry was not - // deleted properly after a successful operation. - // Remove WAL entry. rs, err := getRoleSet(entry.RoleSet, ctx, req.Storage) if err != nil { return err } + + // If account is still being used, WAL entry was not deleted properly after a successful operation. + // Remove WAL entry. if rs != nil && entry.Id.ResourceName() == rs.AccountId.ResourceName() { // Still being used - don't delete this service account. return nil @@ -93,26 +101,46 @@ func (b *backend) serviceAccountKeyRollback(ctx context.Context, req *logical.Re return err } - b.Logger().Debug("checking roleset listed in WAL is access_token roleset") - + b.Logger().Debug("checking parent listed in WAL generates access_token secret") var keyInUse string - // Get roleset for entry - rs, err := getRoleSet(entry.RoleSet, ctx, req.Storage) - if err != nil { - return err - } + switch { + case entry.RoleSet != "": + rs, err := getRoleSet(entry.RoleSet, ctx, req.Storage) + if err != nil { + return err + } - // If roleset is not nil, get key in use. - if rs != nil { - if rs.SecretType != SecretTypeAccessToken { - // Don't clean keys if roleset doesn't create access_tokens (i.e. creates keys). - return nil + // If roleset is not nil, get key in use. + if rs != nil { + if rs.SecretType != SecretTypeAccessToken { + // Remove WAL entry - we don't clean keys if roleset generates key secrets. + return nil + } + + if rs.TokenGen != nil { + keyInUse = rs.TokenGen.KeyName + } + } + case entry.StaticAccount != "": + sa, err := b.getStaticAccount(entry.StaticAccount, ctx, req.Storage) + if err != nil { + return err } - if rs.TokenGen != nil { - keyInUse = rs.TokenGen.KeyName + // If roleset is not nil, get key in use. + if sa != nil { + if sa.SecretType != SecretTypeAccessToken { + // Remove WAL entry - we don't clean keys if roleset generates key secrets. + return nil + } + if sa.TokenGen != nil { + keyInUse = sa.TokenGen.KeyName + } } + default: + b.Logger().Error("removing invalid walAccountKey with empty RoleSet and empty StaticAccount - may need manual cleanup: %v", entry) + return nil } iamC, err := b.IAMAdminClient(req.Storage) @@ -167,19 +195,21 @@ func (b *backend) serviceAccountPolicyRollback(ctx context.Context, req *logical return err } + var rolesInUse util.StringSet + // Try to verify service account not being used by roleset rs, err := getRoleSet(entry.RoleSet, ctx, req.Storage) if err != nil { return err } + if rs.AccountId != nil && rs.AccountId.ResourceName() == entry.AccountId.ResourceName() { + rolesInUse = rs.Bindings[entry.Resource] + } // Take out any bindings still being used by this role set from roles being removed. rolesToRemove := util.ToSet(entry.Roles) - if rs != nil && rs.AccountId.ResourceName() == entry.AccountId.ResourceName() { - currRoles, ok := rs.Bindings[entry.Resource] - if ok { - rolesToRemove = rolesToRemove.Sub(currRoles) - } + if len(rolesInUse) > 0 { + rolesToRemove = rolesToRemove.Sub(rolesInUse) } r, err := b.resources.Parse(entry.Resource) @@ -193,10 +223,6 @@ func (b *backend) serviceAccountPolicyRollback(ctx context.Context, req *logical } apiHandle := iamutil.GetApiHandle(httpC, useragent.String()) - if err != nil { - return err - } - p, err := r.GetIamPolicy(ctx, apiHandle) if err != nil { if isGoogleAccountNotFoundErr(err) || isGoogleAccountUnauthorizedErr(err) { @@ -210,7 +236,6 @@ func (b *backend) serviceAccountPolicyRollback(ctx context.Context, req *logical Email: entry.AccountId.EmailOrId, Roles: rolesToRemove, }) - if !changed { return nil } @@ -219,59 +244,65 @@ func (b *backend) serviceAccountPolicyRollback(ctx context.Context, req *logical return err } -func (b *backend) deleteServiceAccount(ctx context.Context, iamAdmin *iam.Service, account gcputil.ServiceAccountId) error { - if account.EmailOrId == "" { - return nil +func (b *backend) serviceAccountPolicyDiffRollback(ctx context.Context, req *logical.Request, data interface{}) error { + b.rolesetLock.Lock() + defer b.rolesetLock.Unlock() + + var entry walIamPolicyStaticAccount + if err := mapstructure.Decode(data, &entry); err != nil { + return err } - _, err := iamAdmin.Projects.ServiceAccounts.Delete(account.ResourceName()).Do() + var rolesInUse util.StringSet + + // Try to verify service account not being used by roleset + sa, err := b.getStaticAccount(entry.StaticAccount, ctx, req.Storage) if err != nil { - if !isGoogleAccountNotFoundErr(err) && !isGoogleAccountUnauthorizedErr(err) { - return errwrap.Wrapf("unable to delete service account: {{err}}", err) - } + return err } - return nil -} - -func (b *backend) deleteTokenGenKey(ctx context.Context, iamAdmin *iam.Service, tgen *TokenGenerator) error { - if tgen == nil || tgen.KeyName == "" { - return nil + if sa.ResourceName() == entry.AccountId.ResourceName() { + rolesInUse = sa.Bindings[entry.Resource] } - _, err := iamAdmin.Projects.ServiceAccounts.Keys.Delete(tgen.KeyName).Do() - if err != nil && !isGoogleAccountKeyNotFoundErr(err) { - return errwrap.Wrapf("unable to delete service account key: {{err}}", err) + // We added roles that are not actually in use + addedRolesToRemove := util.ToSet(entry.RolesAdded).Sub(rolesInUse) + + // We removed roles that are still saved + removedRolesToAdd := util.ToSet(entry.RolesRemoved).Intersection(rolesInUse) + + r, err := b.resources.Parse(entry.Resource) + if err != nil { + return err } - return nil -} -func (b *backend) removeBindings(ctx context.Context, apiHandle *iamutil.ApiHandle, email string, bindings ResourceBindings) (allErr *multierror.Error) { - for resName, roles := range bindings { - resource, err := b.resources.Parse(resName) - if err != nil { - allErr = multierror.Append(allErr, errwrap.Wrapf(fmt.Sprintf("unable to delete role binding for resource '%s': {{err}}", resName), err)) - continue - } + httpC, err := b.HTTPClient(req.Storage) + if err != nil { + return err + } - p, err := resource.GetIamPolicy(ctx, apiHandle) - if err != nil { - allErr = multierror.Append(allErr, errwrap.Wrapf(fmt.Sprintf("unable to delete role binding for resource '%s': {{err}}", resName), err)) - continue - } + apiHandle := iamutil.GetApiHandle(httpC, useragent.String()) + p, err := r.GetIamPolicy(ctx, apiHandle) + if err != nil { + return err + } - changed, newP := p.RemoveBindings(&iamutil.PolicyDelta{ - Email: email, - Roles: roles, + changed, newP := p.ChangeBindings( + // toAdd + &iamutil.PolicyDelta{ + Email: entry.AccountId.EmailOrId, + Roles: removedRolesToAdd, + }, + // toRemove + &iamutil.PolicyDelta{ + Email: entry.AccountId.EmailOrId, + Roles: addedRolesToRemove, }) - if !changed { - continue - } - if _, err = resource.SetIamPolicy(ctx, apiHandle, newP); err != nil { - allErr = multierror.Append(allErr, errwrap.Wrapf(fmt.Sprintf("unable to delete role binding for resource '%s': {{err}}", resName), err)) - continue - } + if !changed { + return nil } - return + + _, err = r.SetIamPolicy(ctx, apiHandle, newP) + return err } // This tries to clean up WALs that are no longer needed. @@ -286,35 +317,3 @@ func (b *backend) tryDeleteWALs(ctx context.Context, s logical.Storage, walIds . } } } - -func isGoogleAccountNotFoundErr(err error) bool { - return isGoogleApiErrorWithCodes(err, 404) -} - -func isGoogleAccountUnauthorizedErr(err error) bool { - return isGoogleApiErrorWithCodes(err, 403) -} - -func isGoogleAccountKeyNotFoundErr(err error) bool { - return isGoogleApiErrorWithCodes(err, 403, 404) -} - -func isGoogleApiErrorWithCodes(err error, validErrCodes ...int) bool { - if err == nil { - return false - } - ok := errwrap.ContainsType(err, new(googleapi.Error)) - if !ok { - return false - } - - gErr := errwrap.GetType(err, new(googleapi.Error)).(*googleapi.Error) - - for _, code := range validErrCodes { - if gErr.Code == code { - return true - } - } - - return false -} diff --git a/plugin/secrets_access_token.go b/plugin/secrets_access_token.go index d5fddb36..50727c86 100644 --- a/plugin/secrets_access_token.go +++ b/plugin/secrets_access_token.go @@ -3,7 +3,6 @@ package gcpsecrets import ( "context" "encoding/base64" - "fmt" "time" "github.com/hashicorp/errwrap" @@ -13,49 +12,12 @@ import ( "golang.org/x/oauth2/google" ) -func pathSecretAccessToken(b *backend) *framework.Path { - return &framework.Path{ - Pattern: fmt.Sprintf("token/%s", framework.GenericNameRegex("roleset")), - Fields: map[string]*framework.FieldSchema{ - "roleset": { - Type: framework.TypeString, - Description: "Required. Name of the role set.", - }, - }, - ExistenceCheck: b.pathRoleSetExistenceCheck("roleset"), - Operations: map[logical.Operation]framework.OperationHandler{ - logical.ReadOperation: &framework.PathOperation{Callback: b.pathAccessToken}, - logical.UpdateOperation: &framework.PathOperation{Callback: b.pathAccessToken}, - }, - HelpSynopsis: pathTokenHelpSyn, - HelpDescription: pathTokenHelpDesc, - } -} - -func (b *backend) pathAccessToken(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - rsName := d.Get("roleset").(string) - - rs, err := getRoleSet(rsName, ctx, req.Storage) - if err != nil { - return nil, err - } - if rs == nil { - return logical.ErrorResponse("role set '%s' does not exist", rsName), nil - } - - if rs.SecretType != SecretTypeAccessToken { - return logical.ErrorResponse("role set '%s' cannot generate access tokens (has secret type %s)", rsName, rs.SecretType), nil - } - - return b.secretAccessTokenResponse(ctx, req.Storage, rs) -} - -func (b *backend) secretAccessTokenResponse(ctx context.Context, s logical.Storage, rs *RoleSet) (*logical.Response, error) { - if rs.TokenGen == nil || rs.TokenGen.KeyName == "" { - return logical.ErrorResponse("invalid role set has no service account key, must be updated (path roleset/%s/rotate-key) before generating new secrets", rs.Name), nil +func (b *backend) secretAccessTokenResponse(ctx context.Context, s logical.Storage, tokenGen *TokenGenerator) (*logical.Response, error) { + if tokenGen == nil || tokenGen.KeyName == "" { + return logical.ErrorResponse("invalid token generator has no service account key"), nil } - token, err := rs.TokenGen.getAccessToken(ctx) + token, err := tokenGen.getAccessToken(ctx) if err != nil { return logical.ErrorResponse("unable to generate token - make sure your roleset service account and key are still valid: %v", err), nil } @@ -109,9 +71,10 @@ Please see backend documentation for more information: https://www.vaultproject.io/docs/secrets/gcp/index.html ` -// EVERYTHING USING THIS SECRET TYPE IS CURRENTLY DEPRECATED. -// We keep it to allow for clean up of access_token secrets/leases that may have be left over -// by older versions of Vault. +// THIS SECRET TYPE IS DEPRECATED - future secret requests returns a response with no framework.Secret +// We are keeping them as part of the created framework.Secret +// to allow for clean up of access_token secrets and leases +// from older versions of Vault. const SecretTypeAccessToken = "access_token" func secretAccessToken(b *backend) *framework.Secret { diff --git a/plugin/secrets_service_account_key.go b/plugin/secrets_service_account_key.go index d56fa3b8..35648df7 100644 --- a/plugin/secrets_service_account_key.go +++ b/plugin/secrets_service_account_key.go @@ -5,6 +5,8 @@ import ( "fmt" "time" + "github.com/hashicorp/go-gcp-common/gcputil" + "github.com/hashicorp/errwrap" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/logical" @@ -17,6 +19,12 @@ const ( privateKeyTypeJson = "TYPE_GOOGLE_CREDENTIALS_FILE" ) +type secretKeyParams struct { + keyType, keyAlgorithm string + ttl int + extraInternalData map[string]interface{} +} + func secretServiceAccountKey(b *backend) *framework.Secret { return &framework.Secret{ Type: SecretTypeKey, @@ -34,66 +42,11 @@ func secretServiceAccountKey(b *backend) *framework.Secret { Description: "Type of the private key (i.e. whether it is JSON or P12). Valid values are GCP enum(ServiceAccountPrivateKeyType)", }, }, - Renew: b.secretKeyRenew, Revoke: b.secretKeyRevoke, } } -func pathSecretServiceAccountKey(b *backend) *framework.Path { - return &framework.Path{ - Pattern: fmt.Sprintf("key/%s", framework.GenericNameRegex("roleset")), - Fields: map[string]*framework.FieldSchema{ - "roleset": { - Type: framework.TypeString, - Description: "Required. Name of the role set.", - }, - "key_algorithm": { - Type: framework.TypeString, - Description: fmt.Sprintf(`Private key algorithm for service account key - defaults to %s"`, keyAlgorithmRSA2k), - Default: keyAlgorithmRSA2k, - }, - "key_type": { - Type: framework.TypeString, - Description: fmt.Sprintf(`Private key type for service account key - defaults to %s"`, privateKeyTypeJson), - Default: privateKeyTypeJson, - }, - "ttl": { - Type: framework.TypeDurationSecond, - Description: "Lifetime of the service account key", - }, - }, - ExistenceCheck: b.pathRoleSetExistenceCheck("roleset"), - Operations: map[logical.Operation]framework.OperationHandler{ - logical.ReadOperation: &framework.PathOperation{Callback: b.pathServiceAccountKey}, - logical.UpdateOperation: &framework.PathOperation{Callback: b.pathServiceAccountKey}, - }, - HelpSynopsis: pathServiceAccountKeySyn, - HelpDescription: pathServiceAccountKeyDesc, - } -} - -func (b *backend) pathServiceAccountKey(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - rsName := d.Get("roleset").(string) - keyType := d.Get("key_type").(string) - keyAlg := d.Get("key_algorithm").(string) - ttl := d.Get("ttl").(int) - - rs, err := getRoleSet(rsName, ctx, req.Storage) - if err != nil { - return nil, err - } - if rs == nil { - return logical.ErrorResponse(fmt.Sprintf("role set '%s' does not exist", rsName)), nil - } - - if rs.SecretType != SecretTypeKey { - return logical.ErrorResponse(fmt.Sprintf("role set '%s' cannot generate service account keys (has secret type %s)", rsName, rs.SecretType)), nil - } - - return b.getSecretKey(ctx, req.Storage, rs, keyType, keyAlg, ttl) -} - func (b *backend) secretKeyRenew(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { resp, err := b.verifySecretServiceKeyExists(ctx, req) if err != nil { @@ -116,31 +69,38 @@ func (b *backend) secretKeyRenew(ctx context.Context, req *logical.Request, d *f return resp, nil } -func (b *backend) verifySecretServiceKeyExists(ctx context.Context, req *logical.Request) (*logical.Response, error) { - keyName, ok := req.Secret.InternalData["key_name"] - if !ok { - return nil, fmt.Errorf("invalid secret, internal data is missing key name") - } +func (b *backend) verifyBindingsNotUpdatedForSecret(ctx context.Context, req *logical.Request) error { + if v, ok := req.Secret.InternalData["role_set"]; ok { + bindingSum, ok := req.Secret.InternalData["role_set_bindings"] + if !ok { + return fmt.Errorf("invalid secret, internal data is missing role set bindings checksum") + } - rsName, ok := req.Secret.InternalData["role_set"] - if !ok { - return nil, fmt.Errorf("invalid secret, internal data is missing role set name") - } + // Verify role set was not deleted. + rs, err := getRoleSet(v.(string), ctx, req.Storage) + if err != nil { + return fmt.Errorf("could not find role set %q to verify secret", v) + } - bindingSum, ok := req.Secret.InternalData["role_set_bindings"] - if !ok { - return nil, fmt.Errorf("invalid secret, internal data is missing role set checksum") + // Verify role set bindings have not changed since secret was generated. + if rs.bindingHash() != bindingSum.(string) { + return fmt.Errorf("role set '%v' bindings were updated since secret was generated, cannot renew", v) + } + } else { + return fmt.Errorf("invalid secret, internal data is missing role set or static account name") } - // Verify role set was not deleted. - rs, err := getRoleSet(rsName.(string), ctx, req.Storage) - if err != nil { - return logical.ErrorResponse(fmt.Sprintf("could not find role set '%v' for secret", rsName)), nil + return nil +} + +func (b *backend) verifySecretServiceKeyExists(ctx context.Context, req *logical.Request) (*logical.Response, error) { + keyName, ok := req.Secret.InternalData["key_name"] + if !ok { + return nil, fmt.Errorf("invalid secret, internal data is missing key name") } - // Verify role set bindings have not changed since secret was generated. - if rs.bindingHash() != bindingSum.(string) { - return logical.ErrorResponse(fmt.Sprintf("role set '%v' bindings were updated since secret was generated, cannot renew", rsName)), nil + if err := b.verifyBindingsNotUpdatedForSecret(ctx, req); err != nil { + return logical.ErrorResponse(err.Error()), err } // Verify service account key still exists. @@ -148,9 +108,11 @@ func (b *backend) verifySecretServiceKeyExists(ctx context.Context, req *logical if err != nil { return logical.ErrorResponse("could not confirm key still exists in GCP"), nil } + if k, err := iamAdmin.Projects.ServiceAccounts.Keys.Get(keyName.(string)).Do(); err != nil || k == nil { - return logical.ErrorResponse(fmt.Sprintf("could not confirm key still exists in GCP: %v", err)), nil + return logical.ErrorResponse("could not confirm key still exists in GCP: %v", err), nil } + return nil, nil } @@ -167,13 +129,13 @@ func (b *backend) secretKeyRevoke(ctx context.Context, req *logical.Request, d * _, err = iamAdmin.Projects.ServiceAccounts.Keys.Delete(keyNameRaw.(string)).Do() if err != nil && !isGoogleAccountKeyNotFoundErr(err) { - return logical.ErrorResponse(fmt.Sprintf("unable to delete service account key: %v", err)), nil + return logical.ErrorResponse("unable to delete service account key: %v", err), nil } return nil, nil } -func (b *backend) getSecretKey(ctx context.Context, s logical.Storage, rs *RoleSet, keyType, keyAlgorithm string, ttl int) (*logical.Response, error) { +func (b *backend) createServiceAccountKeySecret(ctx context.Context, s logical.Storage, id *gcputil.ServiceAccountId, params secretKeyParams) (*logical.Response, error) { cfg, err := getConfig(ctx, s) if err != nil { return nil, errwrap.Wrapf("could not read backend config: {{err}}", err) @@ -181,9 +143,6 @@ func (b *backend) getSecretKey(ctx context.Context, s logical.Storage, rs *RoleS if cfg == nil { cfg = &config{} } - if rs.AccountId == nil { - return nil, fmt.Errorf("unable to create secret (service account key), roleset has nil account ID") - } iamC, err := b.IAMAdminClient(s) if err != nil { @@ -191,9 +150,9 @@ func (b *backend) getSecretKey(ctx context.Context, s logical.Storage, rs *RoleS } key, err := iamC.Projects.ServiceAccounts.Keys.Create( - rs.AccountId.ResourceName(), &iam.CreateServiceAccountKeyRequest{ - KeyAlgorithm: keyAlgorithm, - PrivateKeyType: keyType, + id.ResourceName(), &iam.CreateServiceAccountKeyRequest{ + KeyAlgorithm: params.keyAlgorithm, + PrivateKeyType: params.keyType, }).Do() if err != nil { return logical.ErrorResponse(err.Error()), nil @@ -205,9 +164,11 @@ func (b *backend) getSecretKey(ctx context.Context, s logical.Storage, rs *RoleS "key_type": key.PrivateKeyType, } internalD := map[string]interface{}{ - "key_name": key.Name, - "role_set": rs.Name, - "role_set_bindings": rs.bindingHash(), + "key_name": key.Name, + } + + for k, v := range params.extraInternalData { + internalD[k] = v } resp := b.Secret(SecretTypeKey).Response(secretD, internalD) @@ -217,20 +178,16 @@ func (b *backend) getSecretKey(ctx context.Context, s logical.Storage, rs *RoleS resp.Secret.TTL = cfg.TTL // If the request came with a TTL value, overwrite the config default - if ttl > 0 { - resp.Secret.TTL = time.Duration(ttl) * time.Second + if params.ttl > 0 { + resp.Secret.TTL = time.Duration(params.ttl) * time.Second } return resp, nil } -const pathServiceAccountKeySyn = `Generate an service account private key under a specific role set.` +const pathServiceAccountKeySyn = `Generate an service account private key secret.` const pathServiceAccountKeyDesc = ` -This path will generate a new service account private key for accessing GCP APIs. -A role set, binding IAM roles to specific GCP resources, will be specified -by name - for example, if this backend is mounted at "gcp", then "gcp/key/deploy" -would generate service account keys for the "deploy" role set. - -On the backend, each roleset is associated with a service account under -which secrets/keys are created. +This path will generate a new service account key for accessing GCP APIs. +Either specify "roleset/my-roleset" or "static/my-account" to generate a key corresponding +to a roleset or static account respectively. ` diff --git a/plugin/secrets_test.go b/plugin/secrets_test.go index b5cf46cb..a9afdad3 100644 --- a/plugin/secrets_test.go +++ b/plugin/secrets_test.go @@ -29,9 +29,32 @@ var testRoles = util.StringSet{ "roles/iam.roleViewer": struct{}{}, } -func TestSecrets_GenerateAccessToken(t *testing.T) { - secretType := SecretTypeAccessToken +// Test deprecated path still works +func TestSecrets_getRoleSetAccessToken(t *testing.T) { rsName := "test-gentoken" + testGetRoleSetAccessToken(t, rsName, fmt.Sprintf("roleset/%s/token", rsName)) +} + +// Test deprecated path still works +func TestSecrets_getRoleSetKey(t *testing.T) { + rsName := "test-genkey" + testGetRoleSetKey(t, rsName, fmt.Sprintf("roleset/%s/key", rsName)) +} + +// Test deprecated path still works +func TestSecretsDeprecated_getRoleSetAccessToken(t *testing.T) { + rsName := "test-gentoken" + testGetRoleSetAccessToken(t, rsName, fmt.Sprintf("token/%s", rsName)) +} + +// Test deprecated path still works +func TestSecretsDeprecated_getRoleSetKey(t *testing.T) { + rsName := "test-genkey" + testGetRoleSetKey(t, rsName, fmt.Sprintf("key/%s", rsName)) +} + +func testGetRoleSetAccessToken(t *testing.T, rsName, path string) { + secretType := SecretTypeAccessToken td := setupTest(t, "0s", "2h") defer cleanup(t, td, rsName, testRoles) @@ -56,7 +79,7 @@ func TestSecrets_GenerateAccessToken(t *testing.T) { // expect error for trying to read key from token roleset testGetKeyFail(t, td, rsName) - token := testGetToken(t, td, rsName) + token := testGetToken(t, path, td) callC := oauth2.NewClient( context.Background(), @@ -69,9 +92,8 @@ func TestSecrets_GenerateAccessToken(t *testing.T) { verifyProjectBindingsRemoved(t, td, sa.Email, testRoles) } -func TestSecrets_GenerateKeyConfigTTL(t *testing.T) { +func testGetRoleSetKey(t *testing.T, rsName, path string) { secretType := SecretTypeKey - rsName := "test-genkey" td := setupTest(t, "1h", "2h") defer cleanup(t, td, rsName, testRoles) @@ -95,10 +117,7 @@ func TestSecrets_GenerateKeyConfigTTL(t *testing.T) { // expect error for trying to read token from key roleset testGetTokenFail(t, td, rsName) - creds, resp := testGetKey(t, td, rsName) - if int(resp.Secret.LeaseTotal().Hours()) != 1 { - t.Fatalf("expected lease duration %d, got %d", 1, int(resp.Secret.LeaseTotal().Hours())) - } + creds, secret := testGetKey(t, path, td) // Confirm calls with key work keyHttpC := oauth2.NewClient(context.Background(), creds.TokenSource) @@ -118,145 +137,12 @@ func TestSecrets_GenerateKeyConfigTTL(t *testing.T) { testRevokeSecretKey(t, td, resp.Secret) k, err := td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() - - if k != nil { - t.Fatalf("expected error as revoked key was deleted, instead got key: %v", k) - } if err == nil || !isGoogleAccountKeyNotFoundErr(err) { t.Fatalf("expected 404 error from getting deleted key, instead got error: %v", err) } - - // Cleanup: Delete role set - testRoleSetDelete(t, td, rsName, sa.Name) - verifyProjectBindingsRemoved(t, td, sa.Email, testRoles) -} - -func TestSecrets_GenerateKeyTTLOverride(t *testing.T) { - secretType := SecretTypeKey - rsName := "test-genkey" - - td := setupTest(t, "1h", "2h") - defer cleanup(t, td, rsName, testRoles) - - projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) - - // Create new role set - expectedBinds := ResourceBindings{projRes: testRoles} - bindsRaw, err := util.BindingsHCL(expectedBinds) - if err != nil { - t.Fatalf("unable to convert resource bindings to HCL string: %v", err) - } - testRoleSetCreate(t, td, rsName, - map[string]interface{}{ - "secret_type": secretType, - "project": td.Project, - "bindings": bindsRaw, - }) - sa := getRoleSetAccount(t, td, rsName) - - // expect error for trying to read token from key roleset - testGetTokenFail(t, td, rsName) - - // call the POST endpoint of /gcp/key/:roleset with updated TTL - creds, resp := testPostKey(t, td, rsName, "60s") - if int(resp.Secret.LeaseTotal().Seconds()) != 60 { - t.Fatalf("expected lease duration %d, got %d", 60, int(resp.Secret.LeaseTotal().Seconds())) - } - - // Confirm calls with key work - keyHttpC := oauth2.NewClient(context.Background(), creds.TokenSource) - checkSecretPermissions(t, td, keyHttpC) - - keyName := resp.Secret.InternalData["key_name"].(string) - if keyName == "" { - t.Fatalf("expected internal data to include key name") - } - - _, err = td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() - if err != nil { - t.Fatalf("could not get key from given internal 'key_name': %v", err) - } - - testRenewSecretKey(t, td, resp.Secret) - testRevokeSecretKey(t, td, resp.Secret) - - k, err := td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() - if k != nil { t.Fatalf("expected error as revoked key was deleted, instead got key: %v", k) } - if err == nil || !isGoogleAccountKeyNotFoundErr(err) { - t.Fatalf("expected 404 error from getting deleted key, instead got error: %v", err) - } - - // Cleanup: Delete role set - testRoleSetDelete(t, td, rsName, sa.Name) - verifyProjectBindingsRemoved(t, td, sa.Email, testRoles) -} - -// TestSecrets_GenerateKeyMaxTTLCheck verifies the MaxTTL is set for the -// configured backend -func TestSecrets_GenerateKeyMaxTTLCheck(t *testing.T) { - secretType := SecretTypeKey - rsName := "test-genkey" - - td := setupTest(t, "1h", "2h") - defer cleanup(t, td, rsName, testRoles) - - projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) - - // Create new role set - expectedBinds := ResourceBindings{projRes: testRoles} - bindsRaw, err := util.BindingsHCL(expectedBinds) - if err != nil { - t.Fatalf("unable to convert resource bindings to HCL string: %v", err) - } - testRoleSetCreate(t, td, rsName, - map[string]interface{}{ - "secret_type": secretType, - "project": td.Project, - "bindings": bindsRaw, - }) - sa := getRoleSetAccount(t, td, rsName) - - // expect error for trying to read token from key roleset - testGetTokenFail(t, td, rsName) - - // call the POST endpoint of /gcp/key/:roleset with updated TTL - creds, resp := testPostKey(t, td, rsName, "60s") - if int(resp.Secret.LeaseTotal().Seconds()) != 60 { - t.Fatalf("expected lease duration %d, got %d", 60, int(resp.Secret.LeaseTotal().Seconds())) - } - - if int(resp.Secret.LeaseOptions.MaxTTL.Hours()) != 2 { - t.Fatalf("expected max lease %d, got %d", 2, int(resp.Secret.LeaseOptions.MaxTTL.Hours())) - } - - // Confirm calls with key work - keyHttpC := oauth2.NewClient(context.Background(), creds.TokenSource) - checkSecretPermissions(t, td, keyHttpC) - - keyName := resp.Secret.InternalData["key_name"].(string) - if keyName == "" { - t.Fatalf("expected internal data to include key name") - } - - _, err = td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() - if err != nil { - t.Fatalf("could not get key from given internal 'key_name': %v", err) - } - - testRenewSecretKey(t, td, resp.Secret) - testRevokeSecretKey(t, td, resp.Secret) - - k, err := td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() - - if k != nil { - t.Fatalf("expected error as revoked key was deleted, instead got key: %v", k) - } - if err == nil || !isGoogleAccountKeyNotFoundErr(err) { - t.Fatalf("expected 404 error from getting deleted key, instead got error: %v", err) - } // Cleanup: Delete role set testRoleSetDelete(t, td, rsName, sa.Name) @@ -303,10 +189,10 @@ func testGetKeyFail(t *testing.T, td *testData, rsName string) { } } -func testGetToken(t *testing.T, td *testData, rsName string) (token string) { +func testGetToken(t *testing.T, path string, td *testData) (token string) { resp, err := td.B.HandleRequest(context.Background(), &logical.Request{ Operation: logical.ReadOperation, - Path: fmt.Sprintf("token/%s", rsName), + Path: path, Storage: td.S, }) @@ -346,16 +232,10 @@ func testGetToken(t *testing.T, td *testData, rsName string) (token string) { return tokenRaw.(string) } -// testPostKey enables the POST call to /gcp/key/:roleset -func testPostKey(t *testing.T, td *testData, rsName, ttl string) (*google.Credentials, *logical.Response) { - data := map[string]interface{}{} - if ttl != "" { - data["ttl"] = ttl - } - +func testGetKey(t *testing.T, path string, td *testData) (*google.Credentials, *logical.Secret) { resp, err := td.B.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.UpdateOperation, - Path: fmt.Sprintf("key/%s", rsName), + Operation: logical.ReadOperation, + Path: path, Storage: td.S, Data: data, }) diff --git a/plugin/static_account.go b/plugin/static_account.go new file mode 100644 index 00000000..bfaa039e --- /dev/null +++ b/plugin/static_account.go @@ -0,0 +1,343 @@ +package gcpsecrets + +import ( + "context" + "errors" + "fmt" + "github.com/hashicorp/errwrap" + "github.com/hashicorp/go-gcp-common/gcputil" + "github.com/hashicorp/go-multierror" + "github.com/hashicorp/vault/sdk/framework" + "github.com/hashicorp/vault/sdk/helper/strutil" + "github.com/hashicorp/vault/sdk/logical" +) + +func (b *backend) getStaticAccount(name string, ctx context.Context, s logical.Storage) (*StaticAccount, error) { + b.Logger().Debug("getting static account from storage", "static_account_name", name) + entry, err := s.Get(ctx, fmt.Sprintf("%s/%s", staticAccountStoragePrefix, name)) + if err != nil { + return nil, err + } + if entry == nil { + return nil, nil + } + + a := &StaticAccount{} + if err := entry.DecodeJSON(a); err != nil { + return nil, err + } + return a, nil +} + +type StaticAccount struct { + Name string + SecretType string + RawBindings string + Bindings ResourceBindings + gcputil.ServiceAccountId + + TokenGen *TokenGenerator +} + +func (a *StaticAccount) boundResources() *gcpAccountResources { + return &gcpAccountResources{ + accountId: a.ServiceAccountId, + bindings: a.Bindings, + tokenGen: a.TokenGen, + } +} + +func (a *StaticAccount) bindingHash() string { + return getStringHash(a.RawBindings) +} + +func (a *StaticAccount) validate() error { + err := &multierror.Error{} + if a.Name == "" { + err = multierror.Append(err, errors.New("static account name is empty")) + } + + if a.SecretType == "" { + err = multierror.Append(err, errors.New("static account secret type is empty")) + } + + if a.EmailOrId == "" { + err = multierror.Append(err, fmt.Errorf("static account must have service account email")) + } + + switch a.SecretType { + case SecretTypeAccessToken: + if a.TokenGen == nil { + err = multierror.Append(err, fmt.Errorf("access token static account should have initialized token generator")) + } else if len(a.TokenGen.Scopes) == 0 { + err = multierror.Append(err, fmt.Errorf("access token static account should have defined scopes")) + } + case SecretTypeKey: + break + default: + err = multierror.Append(err, fmt.Errorf("unknown secret type: %s", a.SecretType)) + } + return err.ErrorOrNil() +} + +func (a *StaticAccount) save(ctx context.Context, s logical.Storage) error { + if err := a.validate(); err != nil { + return err + } + + entry, err := logical.StorageEntryJSON(fmt.Sprintf("%s/%s", staticAccountStoragePrefix, a.Name), a) + if err != nil { + return err + } + + return s.Put(ctx, entry) +} + +func (b *backend) tryDeleteStaticAccountResources(ctx context.Context, req *logical.Request, boundResources *gcpAccountResources, walIds []string) []string { + return b.tryDeleteGcpAccountResources(ctx, req, boundResources, flagMustKeepServiceAccount, walIds) +} + +func (b *backend) createStaticAccount(ctx context.Context, req *logical.Request, input *inputParams) (err error) { + iamAdmin, err := b.IAMAdminClient(req.Storage) + if err != nil { + return err + } + + gcpAcct, err := b.getServiceAccount(iamAdmin, &gcputil.ServiceAccountId{ + Project: gcpServiceAccountInferredProject, + EmailOrId: input.serviceAccountEmail, + }) + + if err != nil { + if isGoogleAccountNotFoundErr(err) { + return fmt.Errorf("unable to create static account - service account %q should exist", input.serviceAccountEmail) + } + return errwrap.Wrapf(fmt.Sprintf("unable to create static account, could not confirm service account %q exists: {{err}}", input.serviceAccountEmail), err) + } + + acctId := gcputil.ServiceAccountId{ + Project: gcpAcct.ProjectId, + EmailOrId: gcpAcct.Email, + } + + // Construct gcpAccountResources references. Note bindings/key are yet to be created. + newResources := &gcpAccountResources{ + accountId: acctId, + bindings: input.bindings, + } + if input.secretType == SecretTypeAccessToken { + newResources.tokenGen = &TokenGenerator{ + Scopes: input.scopes, + } + } + + // add WALs for static account resources + newWalIds, err := b.addWalsForStaticAccountResources(ctx, req, input.name, newResources) + if err != nil { + b.tryDeleteWALs(ctx, req.Storage, newWalIds...) + return err + } + + // Create new IAM bindings. + if err := b.createIamBindings(ctx, req, gcpAcct.Email, newResources.bindings); err != nil { + return err + } + + // Create new token gen if a stubbed tokenGenerator (with scopes) is given. + if newResources.tokenGen != nil && len(newResources.tokenGen.Scopes) > 0 { + tokenGen, err := b.createNewTokenGen(ctx, req, gcpAcct.Name, newResources.tokenGen.Scopes) + if err != nil { + return err + } + newResources.tokenGen = tokenGen + } + + // Construct new static account + a := &StaticAccount{ + Name: input.name, + SecretType: input.secretType, + RawBindings: input.rawBindings, + Bindings: input.bindings, + ServiceAccountId: acctId, + TokenGen: newResources.tokenGen, + } + + // Save to storage. + if err := a.save(ctx, req.Storage); err != nil { + return err + } + + // We successfully saved the new static account so try cleaning up WALs + // that would rollback the GCP resources (will no-op if still in use) + b.tryDeleteWALs(ctx, req.Storage, newWalIds...) + return err +} + +func (b *backend) updateStaticAccount(ctx context.Context, req *logical.Request, a *StaticAccount, updateInput *inputParams) (warnings []string, err error) { + iamAdmin, err := b.IAMAdminClient(req.Storage) + if err != nil { + return nil, err + } + + _, err = b.getServiceAccount(iamAdmin, &a.ServiceAccountId) + if err != nil { + if isGoogleAccountNotFoundErr(err) { + return nil, fmt.Errorf("unable to update static account, could not find service account %q", a.ResourceName()) + } + return nil, errwrap.Wrapf(fmt.Sprintf("unable to create static account, could not confirm service account %q exists: {{err}}", a.ResourceName()), err) + } + + var walIds []string + madeChange := false + + if updateInput.hasBindings && a.bindingHash() != updateInput.rawBindings { + b.Logger().Debug("detected bindings change, updating bindings for static account") + newBindings := updateInput.bindings + + bindingWals, err := b.updateBindingsForStaticAccount(ctx, req, a, newBindings) + if err != nil { + return nil, err + } + walIds = append(walIds, bindingWals...) + + a.RawBindings = updateInput.rawBindings + a.Bindings = newBindings + madeChange = true + } + + if a.SecretType == "access_token" { + if a.TokenGen == nil { + return nil, fmt.Errorf("unexpected invalid access_token static account has no TokenGen") + } + if !strutil.EquivalentSlices(updateInput.scopes, a.TokenGen.Scopes) { + b.Logger().Debug("detected scopes change, updating scopes for static account") + a.TokenGen.Scopes = updateInput.scopes + madeChange = true + } + } + + if !madeChange { + return []string{"no changes to bindings or token_scopes detected, no update needed"}, nil + } + + if err := a.save(ctx, req.Storage); err != nil { + return nil, err + } + + b.tryDeleteWALs(ctx, req.Storage, walIds...) + return + +} + +func (b *backend) updateBindingsForStaticAccount(ctx context.Context, req *logical.Request, a *StaticAccount, newBindings ResourceBindings) ([]string, error) { + oldBindings := a.Bindings + bindsToAdd := newBindings.sub(oldBindings) + bindsToRemove := oldBindings.sub(newBindings) + + b.Logger().Debug("updating bindings for static account") + walIds, err := b.addWalsForStaticAccountBindings(ctx, req, a, bindsToAdd, bindsToRemove) + if err != nil { + return nil, err + } + + if err := b.createIamBindings(ctx, req, a.EmailOrId, bindsToAdd); err != nil { + return nil, err + } + + if err := b.removeBindings(ctx, req, a.EmailOrId, bindsToRemove); err != nil { + return nil, err + } + + return walIds, nil +} + +// addWalsForStaticAccountResources creates WALs to clean up a roleset's service account, bindings, and a key if needed. +func (b *backend) addWalsForStaticAccountResources(ctx context.Context, req *logical.Request, staticAcctName string, boundResources *gcpAccountResources) (walIds []string, err error) { + if boundResources == nil { + b.Logger().Debug("skip WALs for nil GCP account resources") + return nil, nil + } + + walIds = make([]string, 0, len(boundResources.bindings)+1) + for resName, roles := range boundResources.bindings { + walId, err := framework.PutWAL(ctx, req.Storage, walTypeIamPolicyDiff, &walIamPolicyStaticAccount{ + StaticAccount: staticAcctName, + AccountId: boundResources.accountId, + Resource: resName, + RolesAdded: roles.ToSlice(), + }) + if err != nil { + return walIds, errwrap.Wrapf("unable to create WAL entry to clean up service account bindings: {{err}}", err) + } + walIds = append(walIds, walId) + } + + if boundResources.tokenGen != nil { + walId, err := b.addWalStaticAccountServiceAccountKey(ctx, req, staticAcctName, &boundResources.accountId, boundResources.tokenGen.KeyName) + if err != nil { + return walIds, err + } + walIds = append(walIds, walId) + } + return walIds, nil +} + +func (b *backend) addWalsForStaticAccountBindings(ctx context.Context, req *logical.Request, a *StaticAccount, removed, added ResourceBindings) (walIds []string, err error) { + walIds = make([]string, 0, len(removed)+len(added)) + + // Add WALs for resources in added bindings + for resource, rolesAdded := range added { + walEntry := &walIamPolicyStaticAccount{ + AccountId: a.ServiceAccountId, + Resource: resource, + RolesAdded: rolesAdded.ToSlice(), + } + if rolesRemoved, ok := removed[resource]; ok { + walEntry.RolesRemoved = rolesRemoved.ToSlice() + } + walId, err := framework.PutWAL(ctx, req.Storage, walTypeIamPolicyDiff, walEntry) + if err != nil { + return walIds, errwrap.Wrapf("unable to create WAL entry to clean up service account bindings: {{err}}", err) + } + walIds = append(walIds, walId) + } + + // Add WALs for resources in removed bindings not in added bindings. + for resource, rolesRemoved := range removed { + if _, ok := added[resource]; ok { + continue + } + walEntry := &walIamPolicyStaticAccount{ + AccountId: a.ServiceAccountId, + Resource: resource, + RolesRemoved: rolesRemoved.ToSlice(), + } + + walId, err := framework.PutWAL(ctx, req.Storage, walTypeIamPolicyDiff, walEntry) + if err != nil { + return walIds, errwrap.Wrapf("unable to create WAL entry to clean up service account bindings: {{err}}", err) + } + walIds = append(walIds, walId) + } + + return walIds, nil +} + +// addWalStaticAccountServiceAccountKey creates WAL to clean up a static account's service account key (for access tokens) if needed. +func (b *backend) addWalStaticAccountServiceAccountKey(ctx context.Context, req *logical.Request, acct string, accountId *gcputil.ServiceAccountId, keyName string) (string, error) { + if accountId == nil { + return "", fmt.Errorf("given nil account ID for WAL for roleset service account key") + } + + b.Logger().Debug("add WAL for service account key", "account", accountId.ResourceName(), "keyName", keyName) + + walId, err := framework.PutWAL(ctx, req.Storage, walTypeAccount, &walAccountKey{ + StaticAccount: acct, + ServiceAccountName: accountId.ResourceName(), + KeyName: keyName, + }) + if err != nil { + return "", errwrap.Wrapf("unable to create WAL entry to clean up service account key: {{err}}", err) + } + return walId, nil +} From 0f6ebaa0a92f809444da3d509488a5af639dd42d Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Wed, 11 Nov 2020 14:46:43 +0800 Subject: [PATCH 02/35] Fix test build --- plugin/path_role_set.go | 9 -- plugin/secrets_test.go | 213 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 207 insertions(+), 15 deletions(-) diff --git a/plugin/path_role_set.go b/plugin/path_role_set.go index 75db344a..00053045 100644 --- a/plugin/path_role_set.go +++ b/plugin/path_role_set.go @@ -207,9 +207,6 @@ func (b *backend) pathRoleSetCreateUpdate(ctx context.Context, req *logical.Requ b.rolesetLock.Lock() defer b.rolesetLock.Unlock() - b.rolesetLock.Lock() - defer b.rolesetLock.Unlock() - rs, err := getRoleSet(name, ctx, req.Storage) if err != nil { return nil, err @@ -346,9 +343,6 @@ func (b *backend) pathRoleSetRotateAccount(ctx context.Context, req *logical.Req b.rolesetLock.Lock() defer b.rolesetLock.Unlock() - b.rolesetLock.Lock() - defer b.rolesetLock.Unlock() - rs, err := getRoleSet(name, ctx, req.Storage) if err != nil { return nil, err @@ -377,9 +371,6 @@ func (b *backend) pathRoleSetRotateKey(ctx context.Context, req *logical.Request b.rolesetLock.Lock() defer b.rolesetLock.Unlock() - b.rolesetLock.Lock() - defer b.rolesetLock.Unlock() - rs, err := getRoleSet(name, ctx, req.Storage) if err != nil { return nil, err diff --git a/plugin/secrets_test.go b/plugin/secrets_test.go index a9afdad3..bb2e4dce 100644 --- a/plugin/secrets_test.go +++ b/plugin/secrets_test.go @@ -95,6 +95,66 @@ func testGetRoleSetAccessToken(t *testing.T, rsName, path string) { func testGetRoleSetKey(t *testing.T, rsName, path string) { secretType := SecretTypeKey + td := setupTest(t, "0s", "2h") + defer cleanup(t, td, rsName, testRoles) + + projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) + + // Create new role set + expectedBinds := ResourceBindings{projRes: testRoles} + bindsRaw, err := util.BindingsHCL(expectedBinds) + if err != nil { + t.Fatalf("unable to convert resource bindings to HCL string: %v", err) + } + testRoleSetCreate(t, td, rsName, + map[string]interface{}{ + "secret_type": secretType, + "project": td.Project, + "bindings": bindsRaw, + }) + sa := getRoleSetAccount(t, td, rsName) + + // expect error for trying to read token from key roleset + testGetTokenFail(t, td, rsName) + + creds, resp := testGetKey(t, path, td) + secret := resp.Secret + + // Confirm calls with key work + keyHttpC := oauth2.NewClient(context.Background(), creds.TokenSource) + checkSecretPermissions(t, td, keyHttpC) + + keyName := secret.InternalData["key_name"].(string) + if keyName == "" { + t.Fatalf("expected internal data to include key name") + } + + _, err = td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() + if err != nil { + t.Fatalf("could not get key from given internal 'key_name': %v", err) + } + + testRenewSecretKey(t, td, secret) + testRevokeSecretKey(t, td, secret) + + k, err := td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() + if err == nil || !isGoogleAccountKeyNotFoundErr(err) { + t.Fatalf("expected 404 error from getting deleted key, instead got error: %v", err) + } + if k != nil { + t.Fatalf("expected error as revoked key was deleted, instead got key: %v", k) + } + + // Cleanup: Delete role set + testRoleSetDelete(t, td, rsName, sa.Name) + verifyProjectBindingsRemoved(t, td, sa.Email, testRoles) +} + +func TestSecrets_GenerateKeyConfigTTL(t *testing.T) { + secretType := SecretTypeKey + rsName := "test-genkey" + path := fmt.Sprintf("key/%s", rsName) + td := setupTest(t, "1h", "2h") defer cleanup(t, td, rsName, testRoles) @@ -117,7 +177,142 @@ func testGetRoleSetKey(t *testing.T, rsName, path string) { // expect error for trying to read token from key roleset testGetTokenFail(t, td, rsName) - creds, secret := testGetKey(t, path, td) + creds, resp := testGetKey(t, path, td) + if int(resp.Secret.LeaseTotal().Hours()) != 1 { + t.Fatalf("expected lease duration %d, got %d", 1, int(resp.Secret.LeaseTotal().Hours())) + } + + // Confirm calls with key work + keyHttpC := oauth2.NewClient(context.Background(), creds.TokenSource) + checkSecretPermissions(t, td, keyHttpC) + + keyName := resp.Secret.InternalData["key_name"].(string) + if keyName == "" { + t.Fatalf("expected internal data to include key name") + } + + _, err = td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() + if err != nil { + t.Fatalf("could not get key from given internal 'key_name': %v", err) + } + + testRenewSecretKey(t, td, resp.Secret) + testRevokeSecretKey(t, td, resp.Secret) + + k, err := td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() + + if err == nil || !isGoogleAccountKeyNotFoundErr(err) { + t.Fatalf("expected 404 error from getting deleted key, instead got error: %v", err) + } + if k != nil { + t.Fatalf("expected error as revoked key was deleted, instead got key: %v", k) + } + + // Cleanup: Delete role set + testRoleSetDelete(t, td, rsName, sa.Name) + verifyProjectBindingsRemoved(t, td, sa.Email, testRoles) +} + +func TestSecrets_GenerateKeyTTLOverride(t *testing.T) { + secretType := SecretTypeKey + rsName := "test-genkey" + + td := setupTest(t, "1h", "2h") + defer cleanup(t, td, rsName, testRoles) + + projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) + + // Create new role set + expectedBinds := ResourceBindings{projRes: testRoles} + bindsRaw, err := util.BindingsHCL(expectedBinds) + if err != nil { + t.Fatalf("unable to convert resource bindings to HCL string: %v", err) + } + testRoleSetCreate(t, td, rsName, + map[string]interface{}{ + "secret_type": secretType, + "project": td.Project, + "bindings": bindsRaw, + }) + sa := getRoleSetAccount(t, td, rsName) + + // expect error for trying to read token from key roleset + testGetTokenFail(t, td, rsName) + + // call the POST endpoint of /gcp/key/:roleset with updated TTL + creds, resp := testPostKey(t, td, rsName, "60s") + if int(resp.Secret.LeaseTotal().Seconds()) != 60 { + t.Fatalf("expected lease duration %d, got %d", 60, int(resp.Secret.LeaseTotal().Seconds())) + } + + // Confirm calls with key work + keyHttpC := oauth2.NewClient(context.Background(), creds.TokenSource) + checkSecretPermissions(t, td, keyHttpC) + + keyName := resp.Secret.InternalData["key_name"].(string) + if keyName == "" { + t.Fatalf("expected internal data to include key name") + } + + _, err = td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() + if err != nil { + t.Fatalf("could not get key from given internal 'key_name': %v", err) + } + + testRenewSecretKey(t, td, resp.Secret) + testRevokeSecretKey(t, td, resp.Secret) + + k, err := td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() + + if k != nil { + t.Fatalf("expected error as revoked key was deleted, instead got key: %v", k) + } + if err == nil || !isGoogleAccountKeyNotFoundErr(err) { + t.Fatalf("expected 404 error from getting deleted key, instead got error: %v", err) + } + + // Cleanup: Delete role set + testRoleSetDelete(t, td, rsName, sa.Name) + verifyProjectBindingsRemoved(t, td, sa.Email, testRoles) +} + +// TestSecrets_GenerateKeyMaxTTLCheck verifies the MaxTTL is set for the +// configured backend +func TestSecrets_GenerateKeyMaxTTLCheck(t *testing.T) { + secretType := SecretTypeKey + rsName := "test-genkey" + + td := setupTest(t, "1h", "2h") + defer cleanup(t, td, rsName, testRoles) + + projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) + + // Create new role set + expectedBinds := ResourceBindings{projRes: testRoles} + bindsRaw, err := util.BindingsHCL(expectedBinds) + if err != nil { + t.Fatalf("unable to convert resource bindings to HCL string: %v", err) + } + testRoleSetCreate(t, td, rsName, + map[string]interface{}{ + "secret_type": secretType, + "project": td.Project, + "bindings": bindsRaw, + }) + sa := getRoleSetAccount(t, td, rsName) + + // expect error for trying to read token from key roleset + testGetTokenFail(t, td, rsName) + + // call the POST endpoint of /gcp/key/:roleset with updated TTL + creds, resp := testPostKey(t, td, rsName, "60s") + if int(resp.Secret.LeaseTotal().Seconds()) != 60 { + t.Fatalf("expected lease duration %d, got %d", 60, int(resp.Secret.LeaseTotal().Seconds())) + } + + if int(resp.Secret.LeaseOptions.MaxTTL.Hours()) != 2 { + t.Fatalf("expected max lease %d, got %d", 2, int(resp.Secret.LeaseOptions.MaxTTL.Hours())) + } // Confirm calls with key work keyHttpC := oauth2.NewClient(context.Background(), creds.TokenSource) @@ -232,10 +427,16 @@ func testGetToken(t *testing.T, path string, td *testData) (token string) { return tokenRaw.(string) } -func testGetKey(t *testing.T, path string, td *testData) (*google.Credentials, *logical.Secret) { +// testPostKey enables the POST call to /gcp/key/:roleset +func testPostKey(t *testing.T, td *testData, rsName, ttl string) (*google.Credentials, *logical.Response) { + data := map[string]interface{}{} + if ttl != "" { + data["ttl"] = ttl + } + resp, err := td.B.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.ReadOperation, - Path: path, + Operation: logical.UpdateOperation, + Path: fmt.Sprintf("key/%s", rsName), Storage: td.S, Data: data, }) @@ -254,12 +455,12 @@ func testGetKey(t *testing.T, path string, td *testData) (*google.Credentials, * return creds, resp } -func testGetKey(t *testing.T, td *testData, rsName string) (*google.Credentials, *logical.Response) { +func testGetKey(t *testing.T, path string, td *testData) (*google.Credentials, *logical.Response) { data := map[string]interface{}{} resp, err := td.B.HandleRequest(context.Background(), &logical.Request{ Operation: logical.ReadOperation, - Path: fmt.Sprintf("key/%s", rsName), + Path: path, Storage: td.S, Data: data, }) From 4b3381c09188ac56755f763a16eb9e0e8b4d4a5a Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Thu, 12 Nov 2020 16:07:12 +0800 Subject: [PATCH 03/35] Rename file for consistency --- plugin/{path_roleset_secrets.go => path_role_set_secrets.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename plugin/{path_roleset_secrets.go => path_role_set_secrets.go} (100%) diff --git a/plugin/path_roleset_secrets.go b/plugin/path_role_set_secrets.go similarity index 100% rename from plugin/path_roleset_secrets.go rename to plugin/path_role_set_secrets.go From 8d022dc2008c84063f725ff7987ccba5d6fbe948 Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Thu, 12 Nov 2020 16:33:33 +0800 Subject: [PATCH 04/35] Move common test code --- plugin/backend_test.go | 109 +++++++++++++++++++++++++++++++++++ plugin/path_role_set_test.go | 102 -------------------------------- 2 files changed, 109 insertions(+), 102 deletions(-) diff --git a/plugin/backend_test.go b/plugin/backend_test.go index 6a3bf67d..d9b76eff 100644 --- a/plugin/backend_test.go +++ b/plugin/backend_test.go @@ -2,11 +2,19 @@ package gcpsecrets import ( "context" + "fmt" + "net/http" + "strings" "testing" "time" + "github.com/hashicorp/go-gcp-common/gcputil" "github.com/hashicorp/go-hclog" + "github.com/hashicorp/vault-plugin-secrets-gcp/plugin/util" "github.com/hashicorp/vault/sdk/logical" + "google.golang.org/api/cloudresourcemanager/v1" + "google.golang.org/api/iam/v1" + "google.golang.org/api/option" ) const ( @@ -31,3 +39,104 @@ func getTestBackend(tb testing.TB) (*backend, logical.Storage) { } return b.(*backend), config.StorageView } + +// Set up/Teardown +type testData struct { + B logical.Backend + S logical.Storage + Project string + HttpClient *http.Client + IamAdmin *iam.Service +} + +func setupTest(t *testing.T, ttl, maxTTL string) *testData { + proj := util.GetTestProject(t) + credsJson, creds := util.GetTestCredentials(t) + httpC, err := gcputil.GetHttpClient(creds, iam.CloudPlatformScope) + if err != nil { + t.Fatal(err) + } + + iamAdmin, err := iam.NewService(context.Background(), option.WithHTTPClient(httpC)) + if err != nil { + t.Fatal(err) + } + + b, reqStorage := getTestBackend(t) + + testConfigUpdate(t, b, reqStorage, map[string]interface{}{ + "credentials": credsJson, + "ttl": ttl, + "max_ttl": maxTTL, + }) + + return &testData{ + B: b, + S: reqStorage, + Project: proj, + HttpClient: httpC, + IamAdmin: iamAdmin, + } +} + +func cleanup(t *testing.T, td *testData, rsName string, roles util.StringSet) { + resp, err := td.IamAdmin.Projects.ServiceAccounts.List(fmt.Sprintf("projects/%s", td.Project)).Do() + if err != nil { + t.Logf("[WARNING] Could not clean up test service accounts for role set %s or projects/%s IAM policy bindings (did test fail?)", rsName, td.Project) + return + } + + memberStrs := make(util.StringSet) + for _, sa := range resp.Accounts { + if sa.DisplayName == fmt.Sprintf(serviceAccountDisplayNameTmpl, rsName) { + memberStrs.Add("serviceAccount:" + sa.Email) + t.Logf("[WARNING] found test service account %s that should have been deleted, did test fail? Manually deleting...", sa.Name) + if _, err := td.IamAdmin.Projects.ServiceAccounts.Delete(sa.Name).Do(); err != nil { + if isGoogleAccountNotFoundErr(err) { + t.Logf("[WARNING] Disregard previous warning - manual delete returned 404, probably IAM eventual consistency") + continue + } + t.Logf("[WARNING] Auto-delete failed - manually clean up service account %s: %v", sa.Name, err) + } + } + } + + crm, err := cloudresourcemanager.New(td.HttpClient) + if err != nil { + t.Logf("[WARNING] Unable to ensure test project bindings deleted: %v", err) + return + } + + p, err := crm.Projects.GetIamPolicy(td.Project, &cloudresourcemanager.GetIamPolicyRequest{}).Do() + if err != nil { + t.Logf("[WARNING] Unable to ensure test project bindings deleted, could not get policy: %v", err) + return + } + + var changesMade bool + found := make(util.StringSet) + for idx, b := range p.Bindings { + if roles.Includes(b.Role) { + members := make([]string, 0, len(b.Members)) + for _, m := range b.Members { + if memberStrs.Includes(m) { + changesMade = true + found.Add(b.Role) + } else { + members = append(members, m) + } + } + p.Bindings[idx].Members = members + } + } + + if !changesMade { + return + } + + t.Logf("[WARNING] had to clean up some roles (%s) for test role set %s - should have been deleted (did test fail?)", + strings.Join(found.ToSlice(), ","), rsName) + if _, err := crm.Projects.SetIamPolicy(td.Project, &cloudresourcemanager.SetIamPolicyRequest{Policy: p}).Do(); err != nil { + t.Logf("[WARNING] Auto-delete failed - manually remove bindings on project %s: %v", td.Project, err) + } +} diff --git a/plugin/path_role_set_test.go b/plugin/path_role_set_test.go index 4b06b95e..352de6c2 100644 --- a/plugin/path_role_set_test.go +++ b/plugin/path_role_set_test.go @@ -13,7 +13,6 @@ import ( "github.com/hashicorp/vault/sdk/logical" "google.golang.org/api/cloudresourcemanager/v1" "google.golang.org/api/iam/v1" - "google.golang.org/api/option" ) const testProjectResourceTemplate = "//cloudresourcemanager.googleapis.com/projects/%s" @@ -734,104 +733,3 @@ func roleSubsetBoundOnProject(t *testing.T, httpC *http.Client, project, email s } return found } - -// Set up/Teardown -type testData struct { - B logical.Backend - S logical.Storage - Project string - HttpClient *http.Client - IamAdmin *iam.Service -} - -func setupTest(t *testing.T, ttl, maxTTL string) *testData { - proj := util.GetTestProject(t) - credsJson, creds := util.GetTestCredentials(t) - httpC, err := gcputil.GetHttpClient(creds, iam.CloudPlatformScope) - if err != nil { - t.Fatal(err) - } - - iamAdmin, err := iam.NewService(context.Background(), option.WithHTTPClient(httpC)) - if err != nil { - t.Fatal(err) - } - - b, reqStorage := getTestBackend(t) - - testConfigUpdate(t, b, reqStorage, map[string]interface{}{ - "credentials": credsJson, - "ttl": ttl, - "max_ttl": maxTTL, - }) - - return &testData{ - B: b, - S: reqStorage, - Project: proj, - HttpClient: httpC, - IamAdmin: iamAdmin, - } -} - -func cleanup(t *testing.T, td *testData, rsName string, roles util.StringSet) { - resp, err := td.IamAdmin.Projects.ServiceAccounts.List(fmt.Sprintf("projects/%s", td.Project)).Do() - if err != nil { - t.Logf("[WARNING] Could not clean up test service accounts for role set %s or projects/%s IAM policy bindings (did test fail?)", rsName, td.Project) - return - } - - memberStrs := make(util.StringSet) - for _, sa := range resp.Accounts { - if sa.DisplayName == fmt.Sprintf(serviceAccountDisplayNameTmpl, rsName) { - memberStrs.Add("serviceAccount:" + sa.Email) - t.Logf("[WARNING] found test service account %s that should have been deleted, did test fail? Manually deleting...", sa.Name) - if _, err := td.IamAdmin.Projects.ServiceAccounts.Delete(sa.Name).Do(); err != nil { - if isGoogleAccountNotFoundErr(err) { - t.Logf("[WARNING] Disregard previous warning - manual delete returned 404, probably IAM eventual consistency") - continue - } - t.Logf("[WARNING] Auto-delete failed - manually clean up service account %s: %v", sa.Name, err) - } - } - } - - crm, err := cloudresourcemanager.New(td.HttpClient) - if err != nil { - t.Logf("[WARNING] Unable to ensure test project bindings deleted: %v", err) - return - } - - p, err := crm.Projects.GetIamPolicy(td.Project, &cloudresourcemanager.GetIamPolicyRequest{}).Do() - if err != nil { - t.Logf("[WARNING] Unable to ensure test project bindings deleted, could not get policy: %v", err) - return - } - - var changesMade bool - found := make(util.StringSet) - for idx, b := range p.Bindings { - if roles.Includes(b.Role) { - members := make([]string, 0, len(b.Members)) - for _, m := range b.Members { - if memberStrs.Includes(m) { - changesMade = true - found.Add(b.Role) - } else { - members = append(members, m) - } - } - p.Bindings[idx].Members = members - } - } - - if !changesMade { - return - } - - t.Logf("[WARNING] had to clean up some roles (%s) for test role set %s - should have been deleted (did test fail?)", - strings.Join(found.ToSlice(), ","), rsName) - if _, err := crm.Projects.SetIamPolicy(td.Project, &cloudresourcemanager.SetIamPolicyRequest{Policy: p}).Do(); err != nil { - t.Logf("[WARNING] Auto-delete failed - manually remove bindings on project %s: %v", td.Project, err) - } -} From 1b6ef728effce81a8ea294a9d17e2dad7f25809c Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Thu, 12 Nov 2020 16:41:30 +0800 Subject: [PATCH 05/35] Refactor cleanup --- plugin/backend_test.go | 14 +++++++++----- plugin/path_role_set_test.go | 12 ++++++------ plugin/secrets_test.go | 10 +++++----- 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/plugin/backend_test.go b/plugin/backend_test.go index d9b76eff..358ad7d8 100644 --- a/plugin/backend_test.go +++ b/plugin/backend_test.go @@ -79,16 +79,16 @@ func setupTest(t *testing.T, ttl, maxTTL string) *testData { } } -func cleanup(t *testing.T, td *testData, rsName string, roles util.StringSet) { +func cleanup(t *testing.T, td *testData, saDisplayName string, roles util.StringSet) { resp, err := td.IamAdmin.Projects.ServiceAccounts.List(fmt.Sprintf("projects/%s", td.Project)).Do() if err != nil { - t.Logf("[WARNING] Could not clean up test service accounts for role set %s or projects/%s IAM policy bindings (did test fail?)", rsName, td.Project) + t.Logf("[WARNING] Could not clean up test service accounts %s or projects/%s IAM policy bindings (did test fail?)", saDisplayName, td.Project) return } memberStrs := make(util.StringSet) for _, sa := range resp.Accounts { - if sa.DisplayName == fmt.Sprintf(serviceAccountDisplayNameTmpl, rsName) { + if sa.DisplayName == saDisplayName { memberStrs.Add("serviceAccount:" + sa.Email) t.Logf("[WARNING] found test service account %s that should have been deleted, did test fail? Manually deleting...", sa.Name) if _, err := td.IamAdmin.Projects.ServiceAccounts.Delete(sa.Name).Do(); err != nil { @@ -134,9 +134,13 @@ func cleanup(t *testing.T, td *testData, rsName string, roles util.StringSet) { return } - t.Logf("[WARNING] had to clean up some roles (%s) for test role set %s - should have been deleted (did test fail?)", - strings.Join(found.ToSlice(), ","), rsName) + t.Logf("[WARNING] had to clean up some roles (%s) for test service account %s - should have been deleted (did test fail?)", + strings.Join(found.ToSlice(), ","), saDisplayName) if _, err := crm.Projects.SetIamPolicy(td.Project, &cloudresourcemanager.SetIamPolicyRequest{Policy: p}).Do(); err != nil { t.Logf("[WARNING] Auto-delete failed - manually remove bindings on project %s: %v", td.Project, err) } } + +func cleanupRoleset(t *testing.T, td *testData, rsName string, roles util.StringSet) { + cleanup(t, td, fmt.Sprintf(serviceAccountDisplayNameTmpl, rsName), roles) +} diff --git a/plugin/path_role_set_test.go b/plugin/path_role_set_test.go index 352de6c2..456246da 100644 --- a/plugin/path_role_set_test.go +++ b/plugin/path_role_set_test.go @@ -24,7 +24,7 @@ func TestPathRoleSet_Basic(t *testing.T) { } td := setupTest(t, "0s", "2h") - defer cleanup(t, td, rsName, roles) + defer cleanupRoleset(t, td, rsName, roles) projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) @@ -80,7 +80,7 @@ func TestPathRoleSet_UpdateKeyRoleSet(t *testing.T) { // Initial test set up - backend, initial config, test resources in project td := setupTest(t, "0s", "2h") - defer cleanup(t, td, rsName, initRoles.Union(updatedRoles)) + defer cleanupRoleset(t, td, rsName, initRoles.Union(updatedRoles)) projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) @@ -180,7 +180,7 @@ func TestPathRoleSet_RotateKeyRoleSet(t *testing.T) { // Initial test set up - backend, initial config, test resources in project td := setupTest(t, "0s", "2h") - defer cleanup(t, td, rsName, roles) + defer cleanupRoleset(t, td, rsName, roles) projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) @@ -236,7 +236,7 @@ func TestPathRoleSet_UpdateTokenRoleSetScopes(t *testing.T) { // Initial test set up - backend, initial config, test resources in project td := setupTest(t, "0s", "2h") - defer cleanup(t, td, rsName, roles) + defer cleanupRoleset(t, td, rsName, roles) projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) @@ -326,7 +326,7 @@ func TestPathRoleSet_UpdateTokenRoleSet(t *testing.T) { // Initial test set up - backend, initial config, test resources in project td := setupTest(t, "0s", "2h") - defer cleanup(t, td, rsName, initRoles.Union(updatedRoles)) + defer cleanupRoleset(t, td, rsName, initRoles.Union(updatedRoles)) projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) @@ -420,7 +420,7 @@ func TestPathRoleSet_RotateTokenRoleSet(t *testing.T) { // Initial test set up - backend, initial config, test resources in project td := setupTest(t, "0s", "2h") - defer cleanup(t, td, rsName, roles) + defer cleanupRoleset(t, td, rsName, roles) projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) diff --git a/plugin/secrets_test.go b/plugin/secrets_test.go index bb2e4dce..d6be302a 100644 --- a/plugin/secrets_test.go +++ b/plugin/secrets_test.go @@ -57,7 +57,7 @@ func testGetRoleSetAccessToken(t *testing.T, rsName, path string) { secretType := SecretTypeAccessToken td := setupTest(t, "0s", "2h") - defer cleanup(t, td, rsName, testRoles) + defer cleanupRoleset(t, td, rsName, testRoles) projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) @@ -96,7 +96,7 @@ func testGetRoleSetKey(t *testing.T, rsName, path string) { secretType := SecretTypeKey td := setupTest(t, "0s", "2h") - defer cleanup(t, td, rsName, testRoles) + defer cleanupRoleset(t, td, rsName, testRoles) projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) @@ -156,7 +156,7 @@ func TestSecrets_GenerateKeyConfigTTL(t *testing.T) { path := fmt.Sprintf("key/%s", rsName) td := setupTest(t, "1h", "2h") - defer cleanup(t, td, rsName, testRoles) + defer cleanupRoleset(t, td, rsName, testRoles) projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) @@ -218,7 +218,7 @@ func TestSecrets_GenerateKeyTTLOverride(t *testing.T) { rsName := "test-genkey" td := setupTest(t, "1h", "2h") - defer cleanup(t, td, rsName, testRoles) + defer cleanupRoleset(t, td, rsName, testRoles) projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) @@ -283,7 +283,7 @@ func TestSecrets_GenerateKeyMaxTTLCheck(t *testing.T) { rsName := "test-genkey" td := setupTest(t, "1h", "2h") - defer cleanup(t, td, rsName, testRoles) + defer cleanupRoleset(t, td, rsName, testRoles) projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) From 38782ec51246f4c2a579b98c54b46838ec8e1f26 Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Thu, 12 Nov 2020 18:32:04 +0800 Subject: [PATCH 06/35] Add basic tests --- plugin/backend_test.go | 4 + plugin/path_static_account_test.go | 188 +++++++++++++++++++++++++++++ 2 files changed, 192 insertions(+) create mode 100644 plugin/path_static_account_test.go diff --git a/plugin/backend_test.go b/plugin/backend_test.go index 358ad7d8..713b6a0e 100644 --- a/plugin/backend_test.go +++ b/plugin/backend_test.go @@ -144,3 +144,7 @@ func cleanup(t *testing.T, td *testData, saDisplayName string, roles util.String func cleanupRoleset(t *testing.T, td *testData, rsName string, roles util.StringSet) { cleanup(t, td, fmt.Sprintf(serviceAccountDisplayNameTmpl, rsName), roles) } + +func cleanupStatic(t *testing.T, td *testData, saName string, roles util.StringSet) { + cleanup(t, td, fmt.Sprintf(staticAccountDisplayNameTmpl, saName), roles) +} diff --git a/plugin/path_static_account_test.go b/plugin/path_static_account_test.go new file mode 100644 index 00000000..5d796d0b --- /dev/null +++ b/plugin/path_static_account_test.go @@ -0,0 +1,188 @@ +package gcpsecrets + +import ( + "context" + "fmt" + "strings" + "testing" + "time" + + "github.com/hashicorp/vault-plugin-secrets-gcp/plugin/util" + "github.com/hashicorp/vault/sdk/logical" + "google.golang.org/api/iam/v1" +) + +const staticAccountDisplayNameTmpl = "Test Static Account for Vault secrets backend %s" + +func TestPathStatic_Basic(t *testing.T) { + staticName := "test-static-basic" + + td := setupTest(t, "0s", "2h") + defer cleanupStatic(t, td, staticName, util.StringSet{}) + + sa := createStaticAccount(t, td, staticName) + defer deleteStaticAccount(t, td, sa) + + // 1. Read should return nothing + respData := testStaticRead(t, td, staticName) + if respData != nil { + t.Fatalf("expected static account to not exist initially") + } + + // 2. Create new static account + testStaticCreate(t, td, staticName, + map[string]interface{}{ + "email": sa.Email, + "token_scopes": []string{iam.CloudPlatformScope}, + }) + + // 3. Read static account + respData = testStaticRead(t, td, staticName) + if respData == nil { + t.Fatalf("expected static account to have been created") + } + + verifyReadData(t, respData, map[string]interface{}{ + "secret_type": SecretTypeAccessToken, // default + "service_account_email": sa.Email, + "service_account_project": sa.ProjectId, + }) + + // 4. Delete static account + testStaticDelete(t, td, staticName) +} + +func TestPathStatic_WithBindings(t *testing.T) { + staticName := "test-static-binding" + roles := util.StringSet{ + "roles/viewer": struct{}{}, + } + + td := setupTest(t, "0s", "2h") + defer cleanupStatic(t, td, staticName, roles) + + sa := createStaticAccount(t, td, staticName) + defer deleteStaticAccount(t, td, sa) + + projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) + + // 1. Read should return nothing + respData := testStaticRead(t, td, staticName) + if respData != nil { + t.Fatalf("expected static account to not exist initially") + } + + // 2. Create new static account + expectedBinds := ResourceBindings{projRes: roles} + bindsRaw, err := util.BindingsHCL(expectedBinds) + if err != nil { + t.Fatalf("unable to convert resource bindings to HCL string: %v", err) + } + testStaticCreate(t, td, staticName, + map[string]interface{}{ + "email": sa.Email, + "bindings": bindsRaw, + "token_scopes": []string{iam.CloudPlatformScope}, + }) + + // 3. Read static account + respData = testStaticRead(t, td, staticName) + if respData == nil { + t.Fatalf("expected static account to have been created") + } + + verifyReadData(t, respData, map[string]interface{}{ + "secret_type": SecretTypeAccessToken, // default + "service_account_email": sa.Email, + "service_account_project": sa.ProjectId, + "bindings": expectedBinds, + }) + + // Verify service account has given role on project + verifyProjectBinding(t, td, sa.Email, roles) + + // 4. Delete static account + testStaticDelete(t, td, staticName) + verifyProjectBindingsRemoved(t, td, sa.Email, roles) +} + +func createStaticAccount(t *testing.T, td *testData, staticName string) *iam.ServiceAccount { + intSuffix := fmt.Sprintf("%d", time.Now().Unix()) + fullName := fmt.Sprintf("%s-%s", staticName, intSuffix) + createSaReq := &iam.CreateServiceAccountRequest{ + AccountId: fullName, + ServiceAccount: &iam.ServiceAccount{ + DisplayName: fmt.Sprintf(staticAccountDisplayNameTmpl, staticName), + }, + } + + sa, err := td.IamAdmin.Projects.ServiceAccounts.Create(fmt.Sprintf("projects/%s", td.Project), createSaReq).Do() + if err != nil { + t.Fatalf("could not create static service account %q", err) + } + + return sa +} + +func deleteStaticAccount(t *testing.T, td *testData, sa *iam.ServiceAccount) { + _, err := td.IamAdmin.Projects.ServiceAccounts.Delete(sa.Name).Do() + + if err != nil { + t.Logf("[WARNING] Could not clean up test static service account %s", sa.Name) + } +} + +func testStaticRead(t *testing.T, td *testData, staticName string) map[string]interface{} { + resp, err := td.B.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.ReadOperation, + Path: fmt.Sprintf("%s/%s", staticAccountStoragePrefix, staticName), + Storage: td.S, + }) + + if err != nil { + t.Fatal(err) + } + + if resp.IsError() { + t.Fatal(resp.Error()) + } + if resp == nil { + return nil + } + + return resp.Data +} + +func testStaticCreate(t *testing.T, td *testData, staticName string, d map[string]interface{}) { + resp, err := td.B.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.CreateOperation, + Path: fmt.Sprintf("%s/%s", staticAccountPathPrefix, staticName), + Data: d, + Storage: td.S, + }) + if err != nil { + t.Fatal(err) + } + if resp != nil && resp.IsError() { + t.Fatal(resp.Error()) + } +} + +func testStaticDelete(t *testing.T, td *testData, staticName string) { + resp, err := td.B.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.DeleteOperation, + Path: fmt.Sprintf("%s/%s", staticAccountPathPrefix, staticName), + Storage: td.S, + }) + + if err != nil { + t.Fatalf("unable to delete role set: %v", err) + } else if resp != nil { + if len(resp.Warnings) > 0 { + t.Logf("warnings returned from role set delete. Warnings:\n %s\n", strings.Join(resp.Warnings, ",\n")) + } + if resp.IsError() { + t.Fatalf("unable to delete role set: %v", resp.Error()) + } + } +} From 5505a80d30540045e4c53459fe71ace861ceb71d Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Wed, 18 Nov 2020 12:31:55 +0800 Subject: [PATCH 07/35] Add binding tests --- plugin/path_role_set_test.go | 5 +- plugin/path_static_account_test.go | 212 +++++++++++++++++++++++++++++ 2 files changed, 216 insertions(+), 1 deletion(-) diff --git a/plugin/path_role_set_test.go b/plugin/path_role_set_test.go index 456246da..bc151e67 100644 --- a/plugin/path_role_set_test.go +++ b/plugin/path_role_set_test.go @@ -602,7 +602,10 @@ func verifyReadData(t *testing.T, actual map[string]interface{}, expected map[st for k, v := range expected { actV, ok := actual[k] if !ok { - t.Errorf("key '%s' not found, expected: %v", k, v) + // Allow testing for absence of data if v is set to nil + if v != nil { + t.Errorf("key '%s' not found, expected: %v", k, v) + } } else if k == "bindings" { verifyReadBindings(t, v.(ResourceBindings), actV) } else if k == "token_scopes" { diff --git a/plugin/path_static_account_test.go b/plugin/path_static_account_test.go index 5d796d0b..a6378b3f 100644 --- a/plugin/path_static_account_test.go +++ b/plugin/path_static_account_test.go @@ -46,12 +46,80 @@ func TestPathStatic_Basic(t *testing.T) { "secret_type": SecretTypeAccessToken, // default "service_account_email": sa.Email, "service_account_project": sa.ProjectId, + "bindings": nil, }) // 4. Delete static account testStaticDelete(t, td, staticName) } +// Tests that some fields cannot be updated +func TestPathStatic_UpdateDisallowed(t *testing.T) { + staticName := "test-static-update-fail" + + td := setupTest(t, "0s", "2h") + defer cleanupStatic(t, td, staticName, util.StringSet{}) + + sa := createStaticAccount(t, td, staticName) + defer deleteStaticAccount(t, td, sa) + + saNew := createStaticAccount(t, td, staticName+"-new") + defer deleteStaticAccount(t, td, saNew) + defer cleanupStatic(t, td, staticName+"-new", util.StringSet{}) + + // 1. Read should return nothing + respData := testStaticRead(t, td, staticName) + if respData != nil { + t.Fatalf("expected static account to not exist initially") + } + + // 2. Create new static account + testStaticCreate(t, td, staticName, + map[string]interface{}{ + "email": sa.Email, + "secret_type": SecretTypeKey, + }) + + // 3. Read static account + respData = testStaticRead(t, td, staticName) + if respData == nil { + t.Fatalf("expected static account to have been created") + } + + verifyReadData(t, respData, map[string]interface{}{ + "secret_type": SecretTypeKey, + "service_account_email": sa.Email, + "service_account_project": sa.ProjectId, + "bindings": nil, + }) + + // 4. Verify theses updates don't work: + errCases := []map[string]interface{}{ + { + // Email cannot be changed + "email": saNew.Email, + }, + { + // Cannot change secret type + "secret_type": SecretTypeAccessToken, + }, + } + for _, d := range errCases { + resp, err := td.B.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.UpdateOperation, + Path: fmt.Sprintf("%s/%s", staticAccountPathPrefix, staticName), + Data: d, + Storage: td.S, + }) + if err == nil && (resp != nil && !resp.IsError()) { + t.Fatalf("expected update to fail; data: %v", d) + } + } + + // 5. Delete static account + testStaticDelete(t, td, staticName) +} + func TestPathStatic_WithBindings(t *testing.T) { staticName := "test-static-binding" roles := util.StringSet{ @@ -106,9 +174,138 @@ func TestPathStatic_WithBindings(t *testing.T) { verifyProjectBindingsRemoved(t, td, sa.Email, roles) } +func TestPathStatic_UpdateBinding(t *testing.T) { + staticName := "test-static-up-binding" + + initRoles := util.StringSet{ + "roles/viewer": struct{}{}, + } + updatedRoles := util.StringSet{ + "roles/browser": struct{}{}, + "roles/cloudsql.client": struct{}{}, + } + + td := setupTest(t, "0s", "2h") + defer cleanupStatic(t, td, staticName, initRoles.Union(updatedRoles)) + + sa := createStaticAccount(t, td, staticName) + defer deleteStaticAccount(t, td, sa) + + projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) + + // 1. Read should return nothing + respData := testStaticRead(t, td, staticName) + if respData != nil { + t.Fatalf("expected static account to not exist initially") + } + + // 2. Create new static account + testStaticCreate(t, td, staticName, + map[string]interface{}{ + "email": sa.Email, + "token_scopes": []string{iam.CloudPlatformScope}, + }) + + // 3. Read static account + respData = testStaticRead(t, td, staticName) + if respData == nil { + t.Fatalf("expected static account to have been created") + } + + verifyReadData(t, respData, map[string]interface{}{ + "secret_type": SecretTypeAccessToken, // default + "service_account_email": sa.Email, + "service_account_project": sa.ProjectId, + "bindings": nil, + }) + + // Verify service account has no role on project + verifyProjectBinding(t, td, sa.Email, util.StringSet{}) + + // 4. Add Binding + expectedBinds := ResourceBindings{projRes: initRoles} + bindsRaw, err := util.BindingsHCL(expectedBinds) + if err != nil { + t.Fatalf("unable to convert resource bindings to HCL string: %v", err) + } + testStaticUpdate(t, td, staticName, map[string]interface{}{ + "email": sa.Email, + "token_scopes": []string{iam.CloudPlatformScope}, + "bindings": bindsRaw, + }) + + // 5. Check Binding is added + respData = testStaticRead(t, td, staticName) + if respData == nil { + t.Fatalf("expected static account to still exist") + } + verifyReadData(t, respData, map[string]interface{}{ + "secret_type": SecretTypeAccessToken, // default + "service_account_email": sa.Email, + "service_account_project": sa.ProjectId, + "bindings": expectedBinds, + }) + // Verify service account has given role on project + verifyProjectBinding(t, td, sa.Email, initRoles) + + // 6. Modify Binding + expectedBinds = ResourceBindings{projRes: updatedRoles} + bindsRaw, err = util.BindingsHCL(expectedBinds) + if err != nil { + t.Fatalf("unable to convert resource bindings to HCL string: %v", err) + } + testStaticUpdate(t, td, staticName, map[string]interface{}{ + "email": sa.Email, + "token_scopes": []string{iam.CloudPlatformScope}, + "bindings": bindsRaw, + }) + + // 7. Check Binding is modified + respData = testStaticRead(t, td, staticName) + if respData == nil { + t.Fatalf("expected static account to still exist") + } + verifyReadData(t, respData, map[string]interface{}{ + "secret_type": SecretTypeAccessToken, // default + "service_account_email": sa.Email, + "service_account_project": sa.ProjectId, + "bindings": expectedBinds, + }) + // Verify service account has given role on project + verifyProjectBinding(t, td, sa.Email, updatedRoles) + verifyProjectBindingsRemoved(t, td, sa.Email, initRoles) + + // 8. Remove Binding + testStaticUpdate(t, td, staticName, map[string]interface{}{ + "email": sa.Email, + "token_scopes": []string{iam.CloudPlatformScope}, + "bindings": "", + }) + + // 9. Check Binding is removed + respData = testStaticRead(t, td, staticName) + if respData == nil { + t.Fatalf("expected static account to still exist") + } + verifyReadData(t, respData, map[string]interface{}{ + "secret_type": SecretTypeAccessToken, // default + "service_account_email": sa.Email, + "service_account_project": sa.ProjectId, + "bindings": nil, + }) + verifyProjectBindingsRemoved(t, td, sa.Email, updatedRoles) + + // 10. Delete static account + testStaticDelete(t, td, staticName) +} + func createStaticAccount(t *testing.T, td *testData, staticName string) *iam.ServiceAccount { intSuffix := fmt.Sprintf("%d", time.Now().Unix()) fullName := fmt.Sprintf("%s-%s", staticName, intSuffix) + if len(fullName) > 30 { + fullName = fullName[0:30] + } + createSaReq := &iam.CreateServiceAccountRequest{ AccountId: fullName, ServiceAccount: &iam.ServiceAccount{ @@ -168,6 +365,21 @@ func testStaticCreate(t *testing.T, td *testData, staticName string, d map[strin } } +func testStaticUpdate(t *testing.T, td *testData, staticName string, d map[string]interface{}) { + resp, err := td.B.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.UpdateOperation, + Path: fmt.Sprintf("%s/%s", staticAccountPathPrefix, staticName), + Data: d, + Storage: td.S, + }) + if err != nil { + t.Fatal(err) + } + if resp != nil && resp.IsError() { + t.Fatal(resp.Error()) + } +} + func testStaticDelete(t *testing.T, td *testData, staticName string) { resp, err := td.B.HandleRequest(context.Background(), &logical.Request{ Operation: logical.DeleteOperation, From 7a60d27f0bb434ff12a4d3b03b49e564f8aec45b Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Wed, 18 Nov 2020 13:48:07 +0800 Subject: [PATCH 08/35] Move roleset secrets tests to new file --- plugin/path_role_set_secrets_test.go | 326 +++++++++++++++++++++++++++ plugin/secrets_test.go | 316 -------------------------- 2 files changed, 326 insertions(+), 316 deletions(-) create mode 100644 plugin/path_role_set_secrets_test.go diff --git a/plugin/path_role_set_secrets_test.go b/plugin/path_role_set_secrets_test.go new file mode 100644 index 00000000..f48c85a3 --- /dev/null +++ b/plugin/path_role_set_secrets_test.go @@ -0,0 +1,326 @@ +package gcpsecrets + +import ( + "context" + "fmt" + "testing" + + "github.com/hashicorp/vault-plugin-secrets-gcp/plugin/util" + "golang.org/x/oauth2" + "google.golang.org/api/iam/v1" +) + +// Test deprecated path still works +func TestSecrets_getRoleSetAccessToken(t *testing.T) { + rsName := "test-gentoken" + testGetRoleSetAccessToken(t, rsName, fmt.Sprintf("roleset/%s/token", rsName)) +} + +// Test deprecated path still works +func TestSecrets_getRoleSetKey(t *testing.T) { + rsName := "test-genkey" + testGetRoleSetKey(t, rsName, fmt.Sprintf("roleset/%s/key", rsName)) +} + +// Test deprecated path still works +func TestSecretsDeprecated_getRoleSetAccessToken(t *testing.T) { + rsName := "test-gentoken" + testGetRoleSetAccessToken(t, rsName, fmt.Sprintf("token/%s", rsName)) +} + +// Test deprecated path still works +func TestSecretsDeprecated_getRoleSetKey(t *testing.T) { + rsName := "test-genkey" + testGetRoleSetKey(t, rsName, fmt.Sprintf("key/%s", rsName)) +} + +func testGetRoleSetAccessToken(t *testing.T, rsName, path string) { + secretType := SecretTypeAccessToken + + td := setupTest(t, "0s", "2h") + defer cleanupRoleset(t, td, rsName, testRoles) + + projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) + + // Create new role set + expectedBinds := ResourceBindings{projRes: testRoles} + bindsRaw, err := util.BindingsHCL(expectedBinds) + if err != nil { + t.Fatalf("unable to convert resource bindings to HCL string: %v", err) + } + testRoleSetCreate(t, td, rsName, + map[string]interface{}{ + "secret_type": secretType, + "project": td.Project, + "bindings": bindsRaw, + "token_scopes": []string{iam.CloudPlatformScope}, + }) + sa := getRoleSetAccount(t, td, rsName) + + // expect error for trying to read key from token roleset + testGetKeyFail(t, td, rsName) + + token := testGetToken(t, path, td) + + callC := oauth2.NewClient( + context.Background(), + oauth2.StaticTokenSource(&oauth2.Token{AccessToken: token}), + ) + checkSecretPermissions(t, td, callC) + + // Cleanup: Delete role set + testRoleSetDelete(t, td, rsName, sa.Name) + verifyProjectBindingsRemoved(t, td, sa.Email, testRoles) +} + +func testGetRoleSetKey(t *testing.T, rsName, path string) { + secretType := SecretTypeKey + + td := setupTest(t, "0s", "2h") + defer cleanupRoleset(t, td, rsName, testRoles) + + projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) + + // Create new role set + expectedBinds := ResourceBindings{projRes: testRoles} + bindsRaw, err := util.BindingsHCL(expectedBinds) + if err != nil { + t.Fatalf("unable to convert resource bindings to HCL string: %v", err) + } + testRoleSetCreate(t, td, rsName, + map[string]interface{}{ + "secret_type": secretType, + "project": td.Project, + "bindings": bindsRaw, + }) + sa := getRoleSetAccount(t, td, rsName) + + // expect error for trying to read token from key roleset + testGetTokenFail(t, td, rsName) + + creds, resp := testGetKey(t, path, td) + secret := resp.Secret + + // Confirm calls with key work + keyHttpC := oauth2.NewClient(context.Background(), creds.TokenSource) + checkSecretPermissions(t, td, keyHttpC) + + keyName := secret.InternalData["key_name"].(string) + if keyName == "" { + t.Fatalf("expected internal data to include key name") + } + + _, err = td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() + if err != nil { + t.Fatalf("could not get key from given internal 'key_name': %v", err) + } + + testRenewSecretKey(t, td, secret) + testRevokeSecretKey(t, td, secret) + + k, err := td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() + if err == nil || !isGoogleAccountKeyNotFoundErr(err) { + t.Fatalf("expected 404 error from getting deleted key, instead got error: %v", err) + } + if k != nil { + t.Fatalf("expected error as revoked key was deleted, instead got key: %v", k) + } + + // Cleanup: Delete role set + testRoleSetDelete(t, td, rsName, sa.Name) + verifyProjectBindingsRemoved(t, td, sa.Email, testRoles) +} + +func TestSecrets_GenerateKeyConfigTTL(t *testing.T) { + secretType := SecretTypeKey + rsName := "test-genkey" + path := fmt.Sprintf("key/%s", rsName) + + td := setupTest(t, "1h", "2h") + defer cleanupRoleset(t, td, rsName, testRoles) + + projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) + + // Create new role set + expectedBinds := ResourceBindings{projRes: testRoles} + bindsRaw, err := util.BindingsHCL(expectedBinds) + if err != nil { + t.Fatalf("unable to convert resource bindings to HCL string: %v", err) + } + testRoleSetCreate(t, td, rsName, + map[string]interface{}{ + "secret_type": secretType, + "project": td.Project, + "bindings": bindsRaw, + }) + sa := getRoleSetAccount(t, td, rsName) + + // expect error for trying to read token from key roleset + testGetTokenFail(t, td, rsName) + + creds, resp := testGetKey(t, path, td) + if int(resp.Secret.LeaseTotal().Hours()) != 1 { + t.Fatalf("expected lease duration %d, got %d", 1, int(resp.Secret.LeaseTotal().Hours())) + } + + // Confirm calls with key work + keyHttpC := oauth2.NewClient(context.Background(), creds.TokenSource) + checkSecretPermissions(t, td, keyHttpC) + + keyName := resp.Secret.InternalData["key_name"].(string) + if keyName == "" { + t.Fatalf("expected internal data to include key name") + } + + _, err = td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() + if err != nil { + t.Fatalf("could not get key from given internal 'key_name': %v", err) + } + + testRenewSecretKey(t, td, resp.Secret) + testRevokeSecretKey(t, td, resp.Secret) + + k, err := td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() + + if err == nil || !isGoogleAccountKeyNotFoundErr(err) { + t.Fatalf("expected 404 error from getting deleted key, instead got error: %v", err) + } + if k != nil { + t.Fatalf("expected error as revoked key was deleted, instead got key: %v", k) + } + + // Cleanup: Delete role set + testRoleSetDelete(t, td, rsName, sa.Name) + verifyProjectBindingsRemoved(t, td, sa.Email, testRoles) +} + +func TestSecrets_GenerateKeyTTLOverride(t *testing.T) { + secretType := SecretTypeKey + rsName := "test-genkey" + + td := setupTest(t, "1h", "2h") + defer cleanupRoleset(t, td, rsName, testRoles) + + projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) + + // Create new role set + expectedBinds := ResourceBindings{projRes: testRoles} + bindsRaw, err := util.BindingsHCL(expectedBinds) + if err != nil { + t.Fatalf("unable to convert resource bindings to HCL string: %v", err) + } + testRoleSetCreate(t, td, rsName, + map[string]interface{}{ + "secret_type": secretType, + "project": td.Project, + "bindings": bindsRaw, + }) + sa := getRoleSetAccount(t, td, rsName) + + // expect error for trying to read token from key roleset + testGetTokenFail(t, td, rsName) + + // call the POST endpoint of /gcp/key/:roleset with updated TTL + creds, resp := testPostKey(t, td, rsName, "60s") + if int(resp.Secret.LeaseTotal().Seconds()) != 60 { + t.Fatalf("expected lease duration %d, got %d", 60, int(resp.Secret.LeaseTotal().Seconds())) + } + + // Confirm calls with key work + keyHttpC := oauth2.NewClient(context.Background(), creds.TokenSource) + checkSecretPermissions(t, td, keyHttpC) + + keyName := resp.Secret.InternalData["key_name"].(string) + if keyName == "" { + t.Fatalf("expected internal data to include key name") + } + + _, err = td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() + if err != nil { + t.Fatalf("could not get key from given internal 'key_name': %v", err) + } + + testRenewSecretKey(t, td, resp.Secret) + testRevokeSecretKey(t, td, resp.Secret) + + k, err := td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() + + if k != nil { + t.Fatalf("expected error as revoked key was deleted, instead got key: %v", k) + } + if err == nil || !isGoogleAccountKeyNotFoundErr(err) { + t.Fatalf("expected 404 error from getting deleted key, instead got error: %v", err) + } + + // Cleanup: Delete role set + testRoleSetDelete(t, td, rsName, sa.Name) + verifyProjectBindingsRemoved(t, td, sa.Email, testRoles) +} + +// TestSecrets_GenerateKeyMaxTTLCheck verifies the MaxTTL is set for the +// configured backend +func TestSecrets_GenerateKeyMaxTTLCheck(t *testing.T) { + secretType := SecretTypeKey + rsName := "test-genkey" + + td := setupTest(t, "1h", "2h") + defer cleanupRoleset(t, td, rsName, testRoles) + + projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) + + // Create new role set + expectedBinds := ResourceBindings{projRes: testRoles} + bindsRaw, err := util.BindingsHCL(expectedBinds) + if err != nil { + t.Fatalf("unable to convert resource bindings to HCL string: %v", err) + } + testRoleSetCreate(t, td, rsName, + map[string]interface{}{ + "secret_type": secretType, + "project": td.Project, + "bindings": bindsRaw, + }) + sa := getRoleSetAccount(t, td, rsName) + + // expect error for trying to read token from key roleset + testGetTokenFail(t, td, rsName) + + // call the POST endpoint of /gcp/key/:roleset with updated TTL + creds, resp := testPostKey(t, td, rsName, "60s") + if int(resp.Secret.LeaseTotal().Seconds()) != 60 { + t.Fatalf("expected lease duration %d, got %d", 60, int(resp.Secret.LeaseTotal().Seconds())) + } + + if int(resp.Secret.LeaseOptions.MaxTTL.Hours()) != 2 { + t.Fatalf("expected max lease %d, got %d", 2, int(resp.Secret.LeaseOptions.MaxTTL.Hours())) + } + + // Confirm calls with key work + keyHttpC := oauth2.NewClient(context.Background(), creds.TokenSource) + checkSecretPermissions(t, td, keyHttpC) + + keyName := resp.Secret.InternalData["key_name"].(string) + if keyName == "" { + t.Fatalf("expected internal data to include key name") + } + + _, err = td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() + if err != nil { + t.Fatalf("could not get key from given internal 'key_name': %v", err) + } + + testRenewSecretKey(t, td, resp.Secret) + testRevokeSecretKey(t, td, resp.Secret) + + k, err := td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() + if err == nil || !isGoogleAccountKeyNotFoundErr(err) { + t.Fatalf("expected 404 error from getting deleted key, instead got error: %v", err) + } + if k != nil { + t.Fatalf("expected error as revoked key was deleted, instead got key: %v", k) + } + + // Cleanup: Delete role set + testRoleSetDelete(t, td, rsName, sa.Name) + verifyProjectBindingsRemoved(t, td, sa.Email, testRoles) +} diff --git a/plugin/secrets_test.go b/plugin/secrets_test.go index d6be302a..22c01df7 100644 --- a/plugin/secrets_test.go +++ b/plugin/secrets_test.go @@ -11,7 +11,6 @@ import ( "github.com/hashicorp/vault-plugin-secrets-gcp/plugin/util" "github.com/hashicorp/vault/sdk/logical" - "golang.org/x/oauth2" "golang.org/x/oauth2/google" "google.golang.org/api/googleapi" "google.golang.org/api/iam/v1" @@ -29,321 +28,6 @@ var testRoles = util.StringSet{ "roles/iam.roleViewer": struct{}{}, } -// Test deprecated path still works -func TestSecrets_getRoleSetAccessToken(t *testing.T) { - rsName := "test-gentoken" - testGetRoleSetAccessToken(t, rsName, fmt.Sprintf("roleset/%s/token", rsName)) -} - -// Test deprecated path still works -func TestSecrets_getRoleSetKey(t *testing.T) { - rsName := "test-genkey" - testGetRoleSetKey(t, rsName, fmt.Sprintf("roleset/%s/key", rsName)) -} - -// Test deprecated path still works -func TestSecretsDeprecated_getRoleSetAccessToken(t *testing.T) { - rsName := "test-gentoken" - testGetRoleSetAccessToken(t, rsName, fmt.Sprintf("token/%s", rsName)) -} - -// Test deprecated path still works -func TestSecretsDeprecated_getRoleSetKey(t *testing.T) { - rsName := "test-genkey" - testGetRoleSetKey(t, rsName, fmt.Sprintf("key/%s", rsName)) -} - -func testGetRoleSetAccessToken(t *testing.T, rsName, path string) { - secretType := SecretTypeAccessToken - - td := setupTest(t, "0s", "2h") - defer cleanupRoleset(t, td, rsName, testRoles) - - projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) - - // Create new role set - expectedBinds := ResourceBindings{projRes: testRoles} - bindsRaw, err := util.BindingsHCL(expectedBinds) - if err != nil { - t.Fatalf("unable to convert resource bindings to HCL string: %v", err) - } - testRoleSetCreate(t, td, rsName, - map[string]interface{}{ - "secret_type": secretType, - "project": td.Project, - "bindings": bindsRaw, - "token_scopes": []string{iam.CloudPlatformScope}, - }) - sa := getRoleSetAccount(t, td, rsName) - - // expect error for trying to read key from token roleset - testGetKeyFail(t, td, rsName) - - token := testGetToken(t, path, td) - - callC := oauth2.NewClient( - context.Background(), - oauth2.StaticTokenSource(&oauth2.Token{AccessToken: token}), - ) - checkSecretPermissions(t, td, callC) - - // Cleanup: Delete role set - testRoleSetDelete(t, td, rsName, sa.Name) - verifyProjectBindingsRemoved(t, td, sa.Email, testRoles) -} - -func testGetRoleSetKey(t *testing.T, rsName, path string) { - secretType := SecretTypeKey - - td := setupTest(t, "0s", "2h") - defer cleanupRoleset(t, td, rsName, testRoles) - - projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) - - // Create new role set - expectedBinds := ResourceBindings{projRes: testRoles} - bindsRaw, err := util.BindingsHCL(expectedBinds) - if err != nil { - t.Fatalf("unable to convert resource bindings to HCL string: %v", err) - } - testRoleSetCreate(t, td, rsName, - map[string]interface{}{ - "secret_type": secretType, - "project": td.Project, - "bindings": bindsRaw, - }) - sa := getRoleSetAccount(t, td, rsName) - - // expect error for trying to read token from key roleset - testGetTokenFail(t, td, rsName) - - creds, resp := testGetKey(t, path, td) - secret := resp.Secret - - // Confirm calls with key work - keyHttpC := oauth2.NewClient(context.Background(), creds.TokenSource) - checkSecretPermissions(t, td, keyHttpC) - - keyName := secret.InternalData["key_name"].(string) - if keyName == "" { - t.Fatalf("expected internal data to include key name") - } - - _, err = td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() - if err != nil { - t.Fatalf("could not get key from given internal 'key_name': %v", err) - } - - testRenewSecretKey(t, td, secret) - testRevokeSecretKey(t, td, secret) - - k, err := td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() - if err == nil || !isGoogleAccountKeyNotFoundErr(err) { - t.Fatalf("expected 404 error from getting deleted key, instead got error: %v", err) - } - if k != nil { - t.Fatalf("expected error as revoked key was deleted, instead got key: %v", k) - } - - // Cleanup: Delete role set - testRoleSetDelete(t, td, rsName, sa.Name) - verifyProjectBindingsRemoved(t, td, sa.Email, testRoles) -} - -func TestSecrets_GenerateKeyConfigTTL(t *testing.T) { - secretType := SecretTypeKey - rsName := "test-genkey" - path := fmt.Sprintf("key/%s", rsName) - - td := setupTest(t, "1h", "2h") - defer cleanupRoleset(t, td, rsName, testRoles) - - projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) - - // Create new role set - expectedBinds := ResourceBindings{projRes: testRoles} - bindsRaw, err := util.BindingsHCL(expectedBinds) - if err != nil { - t.Fatalf("unable to convert resource bindings to HCL string: %v", err) - } - testRoleSetCreate(t, td, rsName, - map[string]interface{}{ - "secret_type": secretType, - "project": td.Project, - "bindings": bindsRaw, - }) - sa := getRoleSetAccount(t, td, rsName) - - // expect error for trying to read token from key roleset - testGetTokenFail(t, td, rsName) - - creds, resp := testGetKey(t, path, td) - if int(resp.Secret.LeaseTotal().Hours()) != 1 { - t.Fatalf("expected lease duration %d, got %d", 1, int(resp.Secret.LeaseTotal().Hours())) - } - - // Confirm calls with key work - keyHttpC := oauth2.NewClient(context.Background(), creds.TokenSource) - checkSecretPermissions(t, td, keyHttpC) - - keyName := resp.Secret.InternalData["key_name"].(string) - if keyName == "" { - t.Fatalf("expected internal data to include key name") - } - - _, err = td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() - if err != nil { - t.Fatalf("could not get key from given internal 'key_name': %v", err) - } - - testRenewSecretKey(t, td, resp.Secret) - testRevokeSecretKey(t, td, resp.Secret) - - k, err := td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() - - if err == nil || !isGoogleAccountKeyNotFoundErr(err) { - t.Fatalf("expected 404 error from getting deleted key, instead got error: %v", err) - } - if k != nil { - t.Fatalf("expected error as revoked key was deleted, instead got key: %v", k) - } - - // Cleanup: Delete role set - testRoleSetDelete(t, td, rsName, sa.Name) - verifyProjectBindingsRemoved(t, td, sa.Email, testRoles) -} - -func TestSecrets_GenerateKeyTTLOverride(t *testing.T) { - secretType := SecretTypeKey - rsName := "test-genkey" - - td := setupTest(t, "1h", "2h") - defer cleanupRoleset(t, td, rsName, testRoles) - - projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) - - // Create new role set - expectedBinds := ResourceBindings{projRes: testRoles} - bindsRaw, err := util.BindingsHCL(expectedBinds) - if err != nil { - t.Fatalf("unable to convert resource bindings to HCL string: %v", err) - } - testRoleSetCreate(t, td, rsName, - map[string]interface{}{ - "secret_type": secretType, - "project": td.Project, - "bindings": bindsRaw, - }) - sa := getRoleSetAccount(t, td, rsName) - - // expect error for trying to read token from key roleset - testGetTokenFail(t, td, rsName) - - // call the POST endpoint of /gcp/key/:roleset with updated TTL - creds, resp := testPostKey(t, td, rsName, "60s") - if int(resp.Secret.LeaseTotal().Seconds()) != 60 { - t.Fatalf("expected lease duration %d, got %d", 60, int(resp.Secret.LeaseTotal().Seconds())) - } - - // Confirm calls with key work - keyHttpC := oauth2.NewClient(context.Background(), creds.TokenSource) - checkSecretPermissions(t, td, keyHttpC) - - keyName := resp.Secret.InternalData["key_name"].(string) - if keyName == "" { - t.Fatalf("expected internal data to include key name") - } - - _, err = td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() - if err != nil { - t.Fatalf("could not get key from given internal 'key_name': %v", err) - } - - testRenewSecretKey(t, td, resp.Secret) - testRevokeSecretKey(t, td, resp.Secret) - - k, err := td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() - - if k != nil { - t.Fatalf("expected error as revoked key was deleted, instead got key: %v", k) - } - if err == nil || !isGoogleAccountKeyNotFoundErr(err) { - t.Fatalf("expected 404 error from getting deleted key, instead got error: %v", err) - } - - // Cleanup: Delete role set - testRoleSetDelete(t, td, rsName, sa.Name) - verifyProjectBindingsRemoved(t, td, sa.Email, testRoles) -} - -// TestSecrets_GenerateKeyMaxTTLCheck verifies the MaxTTL is set for the -// configured backend -func TestSecrets_GenerateKeyMaxTTLCheck(t *testing.T) { - secretType := SecretTypeKey - rsName := "test-genkey" - - td := setupTest(t, "1h", "2h") - defer cleanupRoleset(t, td, rsName, testRoles) - - projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) - - // Create new role set - expectedBinds := ResourceBindings{projRes: testRoles} - bindsRaw, err := util.BindingsHCL(expectedBinds) - if err != nil { - t.Fatalf("unable to convert resource bindings to HCL string: %v", err) - } - testRoleSetCreate(t, td, rsName, - map[string]interface{}{ - "secret_type": secretType, - "project": td.Project, - "bindings": bindsRaw, - }) - sa := getRoleSetAccount(t, td, rsName) - - // expect error for trying to read token from key roleset - testGetTokenFail(t, td, rsName) - - // call the POST endpoint of /gcp/key/:roleset with updated TTL - creds, resp := testPostKey(t, td, rsName, "60s") - if int(resp.Secret.LeaseTotal().Seconds()) != 60 { - t.Fatalf("expected lease duration %d, got %d", 60, int(resp.Secret.LeaseTotal().Seconds())) - } - - if int(resp.Secret.LeaseOptions.MaxTTL.Hours()) != 2 { - t.Fatalf("expected max lease %d, got %d", 2, int(resp.Secret.LeaseOptions.MaxTTL.Hours())) - } - - // Confirm calls with key work - keyHttpC := oauth2.NewClient(context.Background(), creds.TokenSource) - checkSecretPermissions(t, td, keyHttpC) - - keyName := resp.Secret.InternalData["key_name"].(string) - if keyName == "" { - t.Fatalf("expected internal data to include key name") - } - - _, err = td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() - if err != nil { - t.Fatalf("could not get key from given internal 'key_name': %v", err) - } - - testRenewSecretKey(t, td, resp.Secret) - testRevokeSecretKey(t, td, resp.Secret) - - k, err := td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() - if err == nil || !isGoogleAccountKeyNotFoundErr(err) { - t.Fatalf("expected 404 error from getting deleted key, instead got error: %v", err) - } - if k != nil { - t.Fatalf("expected error as revoked key was deleted, instead got key: %v", k) - } - - // Cleanup: Delete role set - testRoleSetDelete(t, td, rsName, sa.Name) - verifyProjectBindingsRemoved(t, td, sa.Email, testRoles) -} - func getRoleSetAccount(t *testing.T, td *testData, rsName string) *iam.ServiceAccount { rs, err := getRoleSet(rsName, context.Background(), td.S) if err != nil { From 07daefabe0468542e4f00c5ae66b1d7ef5261a63 Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Wed, 18 Nov 2020 14:09:32 +0800 Subject: [PATCH 09/35] Refactor roleset secrets test functions --- plugin/path_role_set_secrets.go | 1 - plugin/path_role_set_secrets_test.go | 20 +++++++++----------- plugin/secrets_test.go | 14 +++++++------- 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/plugin/path_role_set_secrets.go b/plugin/path_role_set_secrets.go index d6283000..6b94f05f 100644 --- a/plugin/path_role_set_secrets.go +++ b/plugin/path_role_set_secrets.go @@ -39,7 +39,6 @@ var fieldSchemaRoleSetAccessToken = map[string]*framework.FieldSchema{ func pathRoleSetSecretServiceAccountKey(b *backend) *framework.Path { return &framework.Path{ Pattern: fmt.Sprintf("roleset/%s/key", framework.GenericNameRegex("roleset")), - Deprecated: true, Fields: fieldSchemaRoleSetServiceAccountKey, ExistenceCheck: b.pathRoleSetExistenceCheck("roleset"), Operations: map[logical.Operation]framework.OperationHandler{ diff --git a/plugin/path_role_set_secrets_test.go b/plugin/path_role_set_secrets_test.go index f48c85a3..48411ee7 100644 --- a/plugin/path_role_set_secrets_test.go +++ b/plugin/path_role_set_secrets_test.go @@ -10,13 +10,11 @@ import ( "google.golang.org/api/iam/v1" ) -// Test deprecated path still works func TestSecrets_getRoleSetAccessToken(t *testing.T) { rsName := "test-gentoken" testGetRoleSetAccessToken(t, rsName, fmt.Sprintf("roleset/%s/token", rsName)) } -// Test deprecated path still works func TestSecrets_getRoleSetKey(t *testing.T) { rsName := "test-genkey" testGetRoleSetKey(t, rsName, fmt.Sprintf("roleset/%s/key", rsName)) @@ -58,7 +56,7 @@ func testGetRoleSetAccessToken(t *testing.T, rsName, path string) { sa := getRoleSetAccount(t, td, rsName) // expect error for trying to read key from token roleset - testGetKeyFail(t, td, rsName) + testGetKeyFail(t, td, fmt.Sprintf("roleset/%s/key", rsName)) token := testGetToken(t, path, td) @@ -96,7 +94,7 @@ func testGetRoleSetKey(t *testing.T, rsName, path string) { sa := getRoleSetAccount(t, td, rsName) // expect error for trying to read token from key roleset - testGetTokenFail(t, td, rsName) + testGetTokenFail(t, td, fmt.Sprintf("roleset/%s/token", rsName)) creds, resp := testGetKey(t, path, td) secret := resp.Secret @@ -134,7 +132,7 @@ func testGetRoleSetKey(t *testing.T, rsName, path string) { func TestSecrets_GenerateKeyConfigTTL(t *testing.T) { secretType := SecretTypeKey rsName := "test-genkey" - path := fmt.Sprintf("key/%s", rsName) + path := fmt.Sprintf("roleset/%s/key", rsName) td := setupTest(t, "1h", "2h") defer cleanupRoleset(t, td, rsName, testRoles) @@ -156,7 +154,7 @@ func TestSecrets_GenerateKeyConfigTTL(t *testing.T) { sa := getRoleSetAccount(t, td, rsName) // expect error for trying to read token from key roleset - testGetTokenFail(t, td, rsName) + testGetTokenFail(t, td, fmt.Sprintf("roleset/%s/token", rsName)) creds, resp := testGetKey(t, path, td) if int(resp.Secret.LeaseTotal().Hours()) != 1 { @@ -218,10 +216,10 @@ func TestSecrets_GenerateKeyTTLOverride(t *testing.T) { sa := getRoleSetAccount(t, td, rsName) // expect error for trying to read token from key roleset - testGetTokenFail(t, td, rsName) + testGetTokenFail(t, td, fmt.Sprintf("roleset/%s/token", rsName)) // call the POST endpoint of /gcp/key/:roleset with updated TTL - creds, resp := testPostKey(t, td, rsName, "60s") + creds, resp := testPostKey(t, td, fmt.Sprintf("roleset/%s/key", rsName), "60s") if int(resp.Secret.LeaseTotal().Seconds()) != 60 { t.Fatalf("expected lease duration %d, got %d", 60, int(resp.Secret.LeaseTotal().Seconds())) } @@ -283,10 +281,10 @@ func TestSecrets_GenerateKeyMaxTTLCheck(t *testing.T) { sa := getRoleSetAccount(t, td, rsName) // expect error for trying to read token from key roleset - testGetTokenFail(t, td, rsName) + testGetTokenFail(t, td, fmt.Sprintf("roleset/%s/token", rsName)) - // call the POST endpoint of /gcp/key/:roleset with updated TTL - creds, resp := testPostKey(t, td, rsName, "60s") + // call the POST endpoint of /gcp/roleset/:roleset/key with updated TTL + creds, resp := testPostKey(t, td, fmt.Sprintf("roleset/%s/key", rsName), "60s") if int(resp.Secret.LeaseTotal().Seconds()) != 60 { t.Fatalf("expected lease duration %d, got %d", 60, int(resp.Secret.LeaseTotal().Seconds())) } diff --git a/plugin/secrets_test.go b/plugin/secrets_test.go index 22c01df7..24f02ecc 100644 --- a/plugin/secrets_test.go +++ b/plugin/secrets_test.go @@ -44,10 +44,10 @@ func getRoleSetAccount(t *testing.T, td *testData, rsName string) *iam.ServiceAc return sa } -func testGetTokenFail(t *testing.T, td *testData, rsName string) { +func testGetTokenFail(t *testing.T, td *testData, path string) { resp, err := td.B.HandleRequest(context.Background(), &logical.Request{ Operation: logical.UpdateOperation, - Path: fmt.Sprintf("token/%s", rsName), + Path: path, Data: make(map[string]interface{}), Storage: td.S, }) @@ -56,10 +56,10 @@ func testGetTokenFail(t *testing.T, td *testData, rsName string) { } } -func testGetKeyFail(t *testing.T, td *testData, rsName string) { +func testGetKeyFail(t *testing.T, td *testData, path string) { resp, err := td.B.HandleRequest(context.Background(), &logical.Request{ Operation: logical.UpdateOperation, - Path: fmt.Sprintf("key/%s", rsName), + Path: path, Data: make(map[string]interface{}), Storage: td.S, }) @@ -111,8 +111,8 @@ func testGetToken(t *testing.T, path string, td *testData) (token string) { return tokenRaw.(string) } -// testPostKey enables the POST call to /gcp/key/:roleset -func testPostKey(t *testing.T, td *testData, rsName, ttl string) (*google.Credentials, *logical.Response) { +// testPostKey enables the POST call to roleset|static/:name:/key +func testPostKey(t *testing.T, td *testData, path, ttl string) (*google.Credentials, *logical.Response) { data := map[string]interface{}{} if ttl != "" { data["ttl"] = ttl @@ -120,7 +120,7 @@ func testPostKey(t *testing.T, td *testData, rsName, ttl string) (*google.Creden resp, err := td.B.HandleRequest(context.Background(), &logical.Request{ Operation: logical.UpdateOperation, - Path: fmt.Sprintf("key/%s", rsName), + Path: path, Storage: td.S, Data: data, }) From e6fc849935f01df124a511499e760ae702fcb64c Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Wed, 18 Nov 2020 16:31:10 +0800 Subject: [PATCH 10/35] Improve robustness of some secrets tests --- plugin/backend_test.go | 6 +++--- plugin/secrets_test.go | 41 ++++++++++++++++++++++++++++++++++++----- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/plugin/backend_test.go b/plugin/backend_test.go index 713b6a0e..76e787a4 100644 --- a/plugin/backend_test.go +++ b/plugin/backend_test.go @@ -90,14 +90,14 @@ func cleanup(t *testing.T, td *testData, saDisplayName string, roles util.String for _, sa := range resp.Accounts { if sa.DisplayName == saDisplayName { memberStrs.Add("serviceAccount:" + sa.Email) - t.Logf("[WARNING] found test service account %s that should have been deleted, did test fail? Manually deleting...", sa.Name) if _, err := td.IamAdmin.Projects.ServiceAccounts.Delete(sa.Name).Do(); err != nil { if isGoogleAccountNotFoundErr(err) { - t.Logf("[WARNING] Disregard previous warning - manual delete returned 404, probably IAM eventual consistency") + // Eventual consistency. We can ignore. continue } - t.Logf("[WARNING] Auto-delete failed - manually clean up service account %s: %v", sa.Name, err) + t.Logf("[WARNING] found test service account %s that should have been deleted, did test fail? Auto-delete failed - manually clean up service account: %v", sa.Name, err) } + t.Logf("[WARNING] found test service account %s that should have been deleted, did test fail? Manually deleted", sa.Name) } } diff --git a/plugin/secrets_test.go b/plugin/secrets_test.go index 24f02ecc..40255476 100644 --- a/plugin/secrets_test.go +++ b/plugin/secrets_test.go @@ -6,6 +6,7 @@ import ( "fmt" "log" "net/http" + "strings" "testing" "time" @@ -54,6 +55,11 @@ func testGetTokenFail(t *testing.T, td *testData, path string) { if err == nil && !resp.IsError() { t.Fatalf("expected error, instead got valid response (data: %v)", resp.Data) } + + error := resp.Error().Error() + if !strings.Contains(error, "cannot generate access tokens (has secret type service_account_key)") { + t.Fatalf("unexpected error: %s", error) + } } func testGetKeyFail(t *testing.T, td *testData, path string) { @@ -66,14 +72,39 @@ func testGetKeyFail(t *testing.T, td *testData, path string) { if err == nil && !resp.IsError() { t.Fatalf("expected error, instead got valid response (data: %v)", resp.Data) } + + error := resp.Error().Error() + if !strings.Contains(error, "cannot generate service account keys (has secret type access_token)") { + t.Fatalf("unexpected error: %s", error) + } +} + +func retryGetToken(td *testData, path string) (*logical.Response, error) { + // Newly created key in backend is eventually consistent. + // Might take up to 60s according to Google's docs + rawResp, err := retryTestFunc(func() (interface{}, error) { + resp, err := td.B.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.ReadOperation, + Path: path, + Storage: td.S, + }) + + if err != nil { + return resp, err + } + + if resp != nil && resp.IsError() { + return resp, resp.Error() + } + return resp, err + }, maxTokenTestCalls) + + resp := rawResp.(*logical.Response) + return resp, err } func testGetToken(t *testing.T, path string, td *testData) (token string) { - resp, err := td.B.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.ReadOperation, - Path: path, - Storage: td.S, - }) + resp, err := retryGetToken(td, path) if err != nil { t.Fatal(err) From 39a9f94e9f977eab53c88e53f76adc2e7795fc34 Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Wed, 18 Nov 2020 16:34:39 +0800 Subject: [PATCH 11/35] Add static secrets tests --- plugin/path_role_set_secrets_test.go | 2 +- plugin/path_static_account.go | 15 ++- plugin/path_static_account_secrets.go | 5 +- plugin/path_static_account_secrets_test.go | 144 +++++++++++++++++++++ plugin/secrets_service_account_key.go | 16 +++ 5 files changed, 172 insertions(+), 10 deletions(-) create mode 100644 plugin/path_static_account_secrets_test.go diff --git a/plugin/path_role_set_secrets_test.go b/plugin/path_role_set_secrets_test.go index 48411ee7..60060423 100644 --- a/plugin/path_role_set_secrets_test.go +++ b/plugin/path_role_set_secrets_test.go @@ -218,7 +218,7 @@ func TestSecrets_GenerateKeyTTLOverride(t *testing.T) { // expect error for trying to read token from key roleset testGetTokenFail(t, td, fmt.Sprintf("roleset/%s/token", rsName)) - // call the POST endpoint of /gcp/key/:roleset with updated TTL + // call the POST endpoint of /gcp/roleset/:roleset:/key with TTL creds, resp := testPostKey(t, td, fmt.Sprintf("roleset/%s/key", rsName), "60s") if int(resp.Secret.LeaseTotal().Seconds()) != 60 { t.Fatalf("expected lease duration %d, got %d", 60, int(resp.Secret.LeaseTotal().Seconds())) diff --git a/plugin/path_static_account.go b/plugin/path_static_account.go index b894cdd5..070137af 100644 --- a/plugin/path_static_account.go +++ b/plugin/path_static_account.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "github.com/hashicorp/errwrap" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/logical" @@ -78,7 +79,7 @@ func pathStaticAccountList(b *backend) *framework.Path { func (b *backend) pathStaticAccountExistenceCheck(ctx context.Context, req *logical.Request, d *framework.FieldData) (bool, error) { nameRaw, ok := d.GetOk("name") if !ok { - return false, errors.New("roleset name is required") + return false, errors.New("static account name is required") } acct, err := b.getStaticAccount(nameRaw.(string), ctx, req.Storage) @@ -144,8 +145,8 @@ func (b *backend) pathStaticAccountDelete(ctx context.Context, req *logical.Requ return nil, errwrap.Wrapf(fmt.Sprintf("unable to create WALs for static account GCP resources %s: {{err}}", name), err) } - // Delete roleset - b.Logger().Debug("deleting roleset from storage", "name", name) + // Delete static account + b.Logger().Debug("deleting static account from storage", "name", name) if err := req.Storage.Delete(ctx, fmt.Sprintf("%s/%s", staticAccountStoragePrefix, name)); err != nil { return nil, err } @@ -153,8 +154,8 @@ func (b *backend) pathStaticAccountDelete(ctx context.Context, req *logical.Requ // Try to clean up resources. if warnings := b.tryDeleteStaticAccountResources(ctx, req, resources, walIds); len(warnings) > 0 { b.Logger().Debug( - "unable to delete GCP resources for deleted roleset but WALs exist to clean up, ignoring errors", - "roleset", name, "errors", warnings) + "unable to delete GCP resources for deleted static account but WALs exist to clean up, ignoring errors", + "static account", name, "errors", warnings) return &logical.Response{Warnings: warnings}, nil } @@ -174,7 +175,7 @@ func (b *backend) pathStaticAccountCreate(ctx context.Context, req *logical.Requ b.staticAccountLock.Lock() defer b.staticAccountLock.Unlock() - // Create and save roleset with new resources. + // Create and save static account with new resources. if err := b.createStaticAccount(ctx, req, input); err != nil { return logical.ErrorResponse(err.Error()), nil } @@ -292,7 +293,7 @@ This path allows you to register a static GCP service account that you want to g This creates sets of IAM roles to specific GCP resources. Secrets (either service account keys or access tokens) are generated under this account. The account must exist at creation of static account creation. -If bindings are specified, Vault will assign IAM permissions to the given service account. Bindings +If bindings are specified, Vault will assign IAM permissions to the given service account. Bindings can be given as a HCL (or JSON) string with the following format: resource "some/gcp/resource/uri" { diff --git a/plugin/path_static_account_secrets.go b/plugin/path_static_account_secrets.go index 38f858ce..0dc11df5 100644 --- a/plugin/path_static_account_secrets.go +++ b/plugin/path_static_account_secrets.go @@ -3,6 +3,7 @@ package gcpsecrets import ( "context" "fmt" + "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/logical" ) @@ -75,7 +76,7 @@ func (b *backend) pathStaticAccountSecretKey(ctx context.Context, req *logical.R return logical.ErrorResponse("static account %q does not exists", acctName), nil } if acct.SecretType != SecretTypeKey { - return logical.ErrorResponse("static account %q cannot generate service account keys (has secret type %q)", acctName, acct.SecretType), nil + return logical.ErrorResponse("static account %q cannot generate service account keys (has secret type %s)", acctName, acct.SecretType), nil } params := secretKeyParams{ @@ -102,7 +103,7 @@ func (b *backend) pathStaticAccountAccessToken(ctx context.Context, req *logical return logical.ErrorResponse("static account %q does not exists", acctName), nil } if acct.SecretType != SecretTypeAccessToken { - return logical.ErrorResponse("static account %q cannot generate access tokens (has secret type %q)", acctName, acct.SecretType), nil + return logical.ErrorResponse("static account %q cannot generate access tokens (has secret type %s)", acctName, acct.SecretType), nil } return b.secretAccessTokenResponse(ctx, req.Storage, acct.TokenGen) diff --git a/plugin/path_static_account_secrets_test.go b/plugin/path_static_account_secrets_test.go new file mode 100644 index 00000000..05e70fdd --- /dev/null +++ b/plugin/path_static_account_secrets_test.go @@ -0,0 +1,144 @@ +package gcpsecrets + +import ( + "context" + "fmt" + "testing" + + "github.com/hashicorp/vault-plugin-secrets-gcp/plugin/util" + "github.com/hashicorp/vault/sdk/logical" + "golang.org/x/oauth2" + "golang.org/x/oauth2/google" + "google.golang.org/api/iam/v1" +) + +func TestStaticSecrets_GetAccessToken(t *testing.T) { + staticName := "test-static-token" + testGetStaticAccessToken(t, staticName) +} + +func TestStaticSecrets_GetKey(t *testing.T) { + staticName := "test-static-key" + testGetStaticKey(t, staticName, 0) +} + +func TestStaticSecrets_GetKeyTTLOverride(t *testing.T) { + staticName := "test-static-key-ttl" + testGetStaticKey(t, staticName, 1200) +} + +func testGetStaticAccessToken(t *testing.T, staticName string) { + secretType := SecretTypeAccessToken + + td := setupTest(t, "0s", "2h") + defer cleanupStatic(t, td, staticName, testRoles) + + sa := createStaticAccount(t, td, staticName) + defer deleteStaticAccount(t, td, sa) + + projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) + + expectedBinds := ResourceBindings{projRes: testRoles} + bindsRaw, err := util.BindingsHCL(expectedBinds) + if err != nil { + t.Fatalf("unable to convert resource bindings to HCL string: %v", err) + } + testStaticCreate(t, td, staticName, + map[string]interface{}{ + "email": sa.Email, + "token_scopes": []string{iam.CloudPlatformScope}, + "secret_type": secretType, + "bindings": bindsRaw, + }) + + // expect error for trying to read key from token + testGetKeyFail(t, td, fmt.Sprintf("%s/%s/key", staticAccountPathPrefix, staticName)) + + token := testGetToken(t, fmt.Sprintf("%s/%s/token", staticAccountPathPrefix, staticName), td) + + callC := oauth2.NewClient( + context.Background(), + oauth2.StaticTokenSource(&oauth2.Token{AccessToken: token}), + ) + checkSecretPermissions(t, td, callC) + + // Cleanup + testStaticDelete(t, td, staticName) + verifyProjectBindingsRemoved(t, td, sa.Email, testRoles) +} + +func testGetStaticKey(t *testing.T, staticName string, ttl uint64) { + secretType := SecretTypeKey + + td := setupTest(t, "60s", "2h") + defer cleanupStatic(t, td, staticName, testRoles) + + sa := createStaticAccount(t, td, staticName) + defer deleteStaticAccount(t, td, sa) + + projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) + + expectedBinds := ResourceBindings{projRes: testRoles} + bindsRaw, err := util.BindingsHCL(expectedBinds) + if err != nil { + t.Fatalf("unable to convert resource bindings to HCL string: %v", err) + } + testStaticCreate(t, td, staticName, + map[string]interface{}{ + "email": sa.Email, + "secret_type": secretType, + "bindings": bindsRaw, + }) + + // expect error for trying to read token + testGetTokenFail(t, td, fmt.Sprintf("%s/%s/token", staticAccountPathPrefix, staticName)) + + var creds *google.Credentials + var resp *logical.Response + if ttl == 0 { + creds, resp = testGetKey(t, fmt.Sprintf("%s/%s/key", staticAccountPathPrefix, staticName), td) + if uint(resp.Secret.LeaseTotal().Seconds()) != 60 { + t.Fatalf("expected lease duration %d, got %d", 60, int(resp.Secret.LeaseTotal().Seconds())) + } + } else { + // call the POST endpoint of /gcp/key/:roleset:/key with TTL + creds, resp = testPostKey(t, td, fmt.Sprintf("%s/%s/key", staticAccountPathPrefix, staticName), fmt.Sprintf("%ds", ttl)) + if uint64(resp.Secret.LeaseTotal().Seconds()) != ttl { + t.Fatalf("expected lease duration %d, got %d", ttl, int(resp.Secret.LeaseTotal().Seconds())) + } + } + + if int(resp.Secret.LeaseOptions.MaxTTL.Hours()) != 2 { + t.Fatalf("expected max lease %d, got %d", 2, int(resp.Secret.LeaseOptions.MaxTTL.Hours())) + } + + secret := resp.Secret + // Confirm calls with key work + keyHttpC := oauth2.NewClient(context.Background(), creds.TokenSource) + checkSecretPermissions(t, td, keyHttpC) + + keyName := secret.InternalData["key_name"].(string) + if keyName == "" { + t.Fatalf("expected internal data to include key name") + } + + _, err = td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() + if err != nil { + t.Fatalf("could not get key from given internal 'key_name': %v", err) + } + + testRenewSecretKey(t, td, secret) + testRevokeSecretKey(t, td, secret) + + k, err := td.IamAdmin.Projects.ServiceAccounts.Keys.Get(keyName).Do() + if err == nil || !isGoogleAccountKeyNotFoundErr(err) { + t.Fatalf("expected 404 error from getting deleted key, instead got error: %v", err) + } + if k != nil { + t.Fatalf("expected error as revoked key was deleted, instead got key: %v", k) + } + + // Cleanup + testStaticDelete(t, td, staticName) + verifyProjectBindingsRemoved(t, td, sa.Email, testRoles) +} diff --git a/plugin/secrets_service_account_key.go b/plugin/secrets_service_account_key.go index 35648df7..40c051cb 100644 --- a/plugin/secrets_service_account_key.go +++ b/plugin/secrets_service_account_key.go @@ -86,6 +86,22 @@ func (b *backend) verifyBindingsNotUpdatedForSecret(ctx context.Context, req *lo if rs.bindingHash() != bindingSum.(string) { return fmt.Errorf("role set '%v' bindings were updated since secret was generated, cannot renew", v) } + } else if v, ok := req.Secret.InternalData["static_account"]; ok { + bindingSum, ok := req.Secret.InternalData["static_account_bindings"] + if !ok { + return fmt.Errorf("invalid secret, internal data is missing static account bindings checksum") + } + + // Verify static account was not deleted. + sa, err := b.getStaticAccount(v.(string), ctx, req.Storage) + if err != nil { + return fmt.Errorf("could not find static account %q to verify secret", v) + } + + // Verify static account bindings have not changed since secret was generated. + if sa.bindingHash() != bindingSum.(string) { + return fmt.Errorf("static account '%v' bindings were updated since secret was generated, cannot renew", v) + } } else { return fmt.Errorf("invalid secret, internal data is missing role set or static account name") } From 7f99cc1a03fc0a4146006aa539d3a6c6a01c6da8 Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Wed, 18 Nov 2020 17:33:58 +0800 Subject: [PATCH 12/35] Add rotate key tests --- plugin/path_static_account_rotate_key_test.go | 104 ++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 plugin/path_static_account_rotate_key_test.go diff --git a/plugin/path_static_account_rotate_key_test.go b/plugin/path_static_account_rotate_key_test.go new file mode 100644 index 00000000..95e5078b --- /dev/null +++ b/plugin/path_static_account_rotate_key_test.go @@ -0,0 +1,104 @@ +package gcpsecrets + +import ( + "context" + "fmt" + "reflect" + "sort" + "testing" + + "github.com/hashicorp/vault-plugin-secrets-gcp/plugin/util" + "github.com/hashicorp/vault/sdk/logical" + "golang.org/x/oauth2" + "google.golang.org/api/iam/v1" +) + +func TestStatic_Rotate(t *testing.T) { + staticName := "test-static-rotate" + secretType := SecretTypeAccessToken + + td := setupTest(t, "0s", "2h") + defer cleanupStatic(t, td, staticName, testRoles) + + sa := createStaticAccount(t, td, staticName) + defer deleteStaticAccount(t, td, sa) + + projRes := fmt.Sprintf(testProjectResourceTemplate, td.Project) + + expectedBinds := ResourceBindings{projRes: testRoles} + bindsRaw, err := util.BindingsHCL(expectedBinds) + if err != nil { + t.Fatalf("unable to convert resource bindings to HCL string: %v", err) + } + testStaticCreate(t, td, staticName, + map[string]interface{}{ + "email": sa.Email, + "token_scopes": []string{iam.CloudPlatformScope}, + "secret_type": secretType, + "bindings": bindsRaw, + }) + + // expect error for trying to read key from token + testGetKeyFail(t, td, fmt.Sprintf("%s/%s/key", staticAccountPathPrefix, staticName)) + + // Obtain current keys + keysRaw, err := td.IamAdmin.Projects.ServiceAccounts.Keys.List(sa.Name).Do() + if err != nil { + t.Fatalf("error listing service account keys: %v", err) + } + var oldKeys []string + for _, key := range keysRaw.Keys { + oldKeys = append(oldKeys, key.Name) + } + sort.Strings(oldKeys) + + // Get token and check + token := testGetToken(t, fmt.Sprintf("%s/%s/token", staticAccountPathPrefix, staticName), td) + callC := oauth2.NewClient( + context.Background(), + oauth2.StaticTokenSource(&oauth2.Token{AccessToken: token}), + ) + checkSecretPermissions(t, td, callC) + + // Rotate key + resp, err := td.B.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.UpdateOperation, + Path: fmt.Sprintf("%s/%s/rotate-key", staticAccountPathPrefix, staticName), + Data: map[string]interface{}{}, + Storage: td.S, + }) + if err != nil { + t.Fatal(err) + } + if resp != nil && resp.IsError() { + t.Fatal(resp.Error()) + } + + // Get new keys + keysRaw, err = td.IamAdmin.Projects.ServiceAccounts.Keys.List(sa.Name).Do() + if err != nil { + t.Fatalf("error listing service account keys: %v", err) + } + var newKeys []string + for _, key := range keysRaw.Keys { + newKeys = append(newKeys, key.Name) + } + sort.Strings(newKeys) + + // Check that keys are actually rotated + if reflect.DeepEqual(oldKeys, newKeys) { + t.Fatal("expected keys to have been rotated, but they were not") + } + + // Test token still works + token = testGetToken(t, fmt.Sprintf("%s/%s/token", staticAccountPathPrefix, staticName), td) + callC = oauth2.NewClient( + context.Background(), + oauth2.StaticTokenSource(&oauth2.Token{AccessToken: token}), + ) + checkSecretPermissions(t, td, callC) + + // Cleanup + testStaticDelete(t, td, staticName) + verifyProjectBindingsRemoved(t, td, sa.Email, testRoles) +} From b0ea17e5086e06fd387b35aa197670168112f331 Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Fri, 20 Nov 2020 14:03:07 +0800 Subject: [PATCH 13/35] Rename "email" to "service_account_email" --- plugin/field_data_utils.go | 5 ++- plugin/path_static_account.go | 6 +-- plugin/path_static_account_rotate_key_test.go | 8 ++-- plugin/path_static_account_secrets_test.go | 14 +++---- plugin/path_static_account_test.go | 38 +++++++++---------- 5 files changed, 36 insertions(+), 35 deletions(-) diff --git a/plugin/field_data_utils.go b/plugin/field_data_utils.go index 0c9255d6..77f1e7bd 100644 --- a/plugin/field_data_utils.go +++ b/plugin/field_data_utils.go @@ -2,6 +2,7 @@ package gcpsecrets import ( "fmt" + "github.com/hashicorp/errwrap" "github.com/hashicorp/vault-plugin-secrets-gcp/plugin/util" "github.com/hashicorp/vault/sdk/framework" @@ -39,8 +40,8 @@ func (input *inputParams) parseOkInputSecretType(d *framework.FieldData) (warnin } } -func (input *inputParams) parseOkInputEmail(d *framework.FieldData) (warnings []string, err error) { - email := d.Get("email").(string) +func (input *inputParams) parseOkInputServiceAccountEmail(d *framework.FieldData) (warnings []string, err error) { + email := d.Get("service_account_email").(string) if email == "" && input.serviceAccountEmail == "" { return nil, fmt.Errorf("email is required") } diff --git a/plugin/path_static_account.go b/plugin/path_static_account.go index 070137af..4411b149 100644 --- a/plugin/path_static_account.go +++ b/plugin/path_static_account.go @@ -29,9 +29,9 @@ func pathStaticAccount(b *backend) *framework.Path { Description: fmt.Sprintf("Type of secret generated for this account. Defaults to %q", SecretTypeAccessToken), Default: SecretTypeAccessToken, }, - "email": { + "service_account_email": { Type: framework.TypeString, - Description: "Name of the GCP service account to manage.", + Description: "Email of the GCP service account to manage.", }, "bindings": { Type: framework.TypeString, @@ -263,7 +263,7 @@ func (b *backend) parseStaticAccountInformation(prevValues *inputParams, d *fram warnings = append(warnings, ws...) } - ws, err = input.parseOkInputEmail(d) + ws, err = input.parseOkInputServiceAccountEmail(d) if err != nil { return nil, nil, err } else if len(ws) > 0 { diff --git a/plugin/path_static_account_rotate_key_test.go b/plugin/path_static_account_rotate_key_test.go index 95e5078b..74517cac 100644 --- a/plugin/path_static_account_rotate_key_test.go +++ b/plugin/path_static_account_rotate_key_test.go @@ -32,10 +32,10 @@ func TestStatic_Rotate(t *testing.T) { } testStaticCreate(t, td, staticName, map[string]interface{}{ - "email": sa.Email, - "token_scopes": []string{iam.CloudPlatformScope}, - "secret_type": secretType, - "bindings": bindsRaw, + "service_account_email": sa.Email, + "token_scopes": []string{iam.CloudPlatformScope}, + "secret_type": secretType, + "bindings": bindsRaw, }) // expect error for trying to read key from token diff --git a/plugin/path_static_account_secrets_test.go b/plugin/path_static_account_secrets_test.go index 05e70fdd..15ecc9b0 100644 --- a/plugin/path_static_account_secrets_test.go +++ b/plugin/path_static_account_secrets_test.go @@ -45,10 +45,10 @@ func testGetStaticAccessToken(t *testing.T, staticName string) { } testStaticCreate(t, td, staticName, map[string]interface{}{ - "email": sa.Email, - "token_scopes": []string{iam.CloudPlatformScope}, - "secret_type": secretType, - "bindings": bindsRaw, + "service_account_email": sa.Email, + "token_scopes": []string{iam.CloudPlatformScope}, + "secret_type": secretType, + "bindings": bindsRaw, }) // expect error for trying to read key from token @@ -85,9 +85,9 @@ func testGetStaticKey(t *testing.T, staticName string, ttl uint64) { } testStaticCreate(t, td, staticName, map[string]interface{}{ - "email": sa.Email, - "secret_type": secretType, - "bindings": bindsRaw, + "service_account_email": sa.Email, + "secret_type": secretType, + "bindings": bindsRaw, }) // expect error for trying to read token diff --git a/plugin/path_static_account_test.go b/plugin/path_static_account_test.go index a6378b3f..732af904 100644 --- a/plugin/path_static_account_test.go +++ b/plugin/path_static_account_test.go @@ -32,8 +32,8 @@ func TestPathStatic_Basic(t *testing.T) { // 2. Create new static account testStaticCreate(t, td, staticName, map[string]interface{}{ - "email": sa.Email, - "token_scopes": []string{iam.CloudPlatformScope}, + "service_account_email": sa.Email, + "token_scopes": []string{iam.CloudPlatformScope}, }) // 3. Read static account @@ -76,8 +76,8 @@ func TestPathStatic_UpdateDisallowed(t *testing.T) { // 2. Create new static account testStaticCreate(t, td, staticName, map[string]interface{}{ - "email": sa.Email, - "secret_type": SecretTypeKey, + "service_account_email": sa.Email, + "secret_type": SecretTypeKey, }) // 3. Read static account @@ -97,7 +97,7 @@ func TestPathStatic_UpdateDisallowed(t *testing.T) { errCases := []map[string]interface{}{ { // Email cannot be changed - "email": saNew.Email, + "service_account_email": saNew.Email, }, { // Cannot change secret type @@ -148,9 +148,9 @@ func TestPathStatic_WithBindings(t *testing.T) { } testStaticCreate(t, td, staticName, map[string]interface{}{ - "email": sa.Email, - "bindings": bindsRaw, - "token_scopes": []string{iam.CloudPlatformScope}, + "service_account_email": sa.Email, + "bindings": bindsRaw, + "token_scopes": []string{iam.CloudPlatformScope}, }) // 3. Read static account @@ -202,8 +202,8 @@ func TestPathStatic_UpdateBinding(t *testing.T) { // 2. Create new static account testStaticCreate(t, td, staticName, map[string]interface{}{ - "email": sa.Email, - "token_scopes": []string{iam.CloudPlatformScope}, + "service_account_email": sa.Email, + "token_scopes": []string{iam.CloudPlatformScope}, }) // 3. Read static account @@ -229,9 +229,9 @@ func TestPathStatic_UpdateBinding(t *testing.T) { t.Fatalf("unable to convert resource bindings to HCL string: %v", err) } testStaticUpdate(t, td, staticName, map[string]interface{}{ - "email": sa.Email, - "token_scopes": []string{iam.CloudPlatformScope}, - "bindings": bindsRaw, + "service_account_email": sa.Email, + "token_scopes": []string{iam.CloudPlatformScope}, + "bindings": bindsRaw, }) // 5. Check Binding is added @@ -255,9 +255,9 @@ func TestPathStatic_UpdateBinding(t *testing.T) { t.Fatalf("unable to convert resource bindings to HCL string: %v", err) } testStaticUpdate(t, td, staticName, map[string]interface{}{ - "email": sa.Email, - "token_scopes": []string{iam.CloudPlatformScope}, - "bindings": bindsRaw, + "service_account_email": sa.Email, + "token_scopes": []string{iam.CloudPlatformScope}, + "bindings": bindsRaw, }) // 7. Check Binding is modified @@ -277,9 +277,9 @@ func TestPathStatic_UpdateBinding(t *testing.T) { // 8. Remove Binding testStaticUpdate(t, td, staticName, map[string]interface{}{ - "email": sa.Email, - "token_scopes": []string{iam.CloudPlatformScope}, - "bindings": "", + "service_account_email": sa.Email, + "token_scopes": []string{iam.CloudPlatformScope}, + "bindings": "", }) // 9. Check Binding is removed From 6189821d93d877ab8d323a8fe158df41c38f98fb Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Fri, 20 Nov 2020 14:24:04 +0800 Subject: [PATCH 14/35] Add test for SA Keys --- plugin/path_static_account_rotate_key_test.go | 21 +----- plugin/path_static_account_test.go | 69 +++++++++++++++++++ 2 files changed, 71 insertions(+), 19 deletions(-) diff --git a/plugin/path_static_account_rotate_key_test.go b/plugin/path_static_account_rotate_key_test.go index 74517cac..fb9559f2 100644 --- a/plugin/path_static_account_rotate_key_test.go +++ b/plugin/path_static_account_rotate_key_test.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "reflect" - "sort" "testing" "github.com/hashicorp/vault-plugin-secrets-gcp/plugin/util" @@ -42,15 +41,7 @@ func TestStatic_Rotate(t *testing.T) { testGetKeyFail(t, td, fmt.Sprintf("%s/%s/key", staticAccountPathPrefix, staticName)) // Obtain current keys - keysRaw, err := td.IamAdmin.Projects.ServiceAccounts.Keys.List(sa.Name).Do() - if err != nil { - t.Fatalf("error listing service account keys: %v", err) - } - var oldKeys []string - for _, key := range keysRaw.Keys { - oldKeys = append(oldKeys, key.Name) - } - sort.Strings(oldKeys) + oldKeys := getServiceAccountKeys(t, td, sa.Name) // Get token and check token := testGetToken(t, fmt.Sprintf("%s/%s/token", staticAccountPathPrefix, staticName), td) @@ -75,15 +66,7 @@ func TestStatic_Rotate(t *testing.T) { } // Get new keys - keysRaw, err = td.IamAdmin.Projects.ServiceAccounts.Keys.List(sa.Name).Do() - if err != nil { - t.Fatalf("error listing service account keys: %v", err) - } - var newKeys []string - for _, key := range keysRaw.Keys { - newKeys = append(newKeys, key.Name) - } - sort.Strings(newKeys) + newKeys := getServiceAccountKeys(t, td, sa.Name) // Check that keys are actually rotated if reflect.DeepEqual(oldKeys, newKeys) { diff --git a/plugin/path_static_account_test.go b/plugin/path_static_account_test.go index 732af904..c730d7e7 100644 --- a/plugin/path_static_account_test.go +++ b/plugin/path_static_account_test.go @@ -3,6 +3,7 @@ package gcpsecrets import ( "context" "fmt" + "sort" "strings" "testing" "time" @@ -23,6 +24,9 @@ func TestPathStatic_Basic(t *testing.T) { sa := createStaticAccount(t, td, staticName) defer deleteStaticAccount(t, td, sa) + // Obtain current keys + oldKeys := getServiceAccountKeys(t, td, sa.Name) + // 1. Read should return nothing respData := testStaticRead(t, td, staticName) if respData != nil { @@ -36,6 +40,13 @@ func TestPathStatic_Basic(t *testing.T) { "token_scopes": []string{iam.CloudPlatformScope}, }) + // Get new keys + newKeys := getServiceAccountKeys(t, td, sa.Name) + + if len(newKeys)-len(oldKeys) != 1 { + t.Fatal("expected one new key to be created for the service account but that was not the case") + } + // 3. Read static account respData = testStaticRead(t, td, staticName) if respData == nil { @@ -49,6 +60,50 @@ func TestPathStatic_Basic(t *testing.T) { "bindings": nil, }) + // 4. Delete static account + testStaticDelete(t, td, staticName) + finalKeys := getServiceAccountKeys(t, td, sa.Name) + if len(finalKeys)-len(newKeys) != -1 { + t.Fatal("expected one fewer service account key upon deletion but that was not the case") + } +} + +func TestPathStatic_Key(t *testing.T) { + staticName := "test-static-key" + + td := setupTest(t, "0s", "2h") + defer cleanupStatic(t, td, staticName, util.StringSet{}) + + sa := createStaticAccount(t, td, staticName) + defer deleteStaticAccount(t, td, sa) + + // 1. Read should return nothing + respData := testStaticRead(t, td, staticName) + if respData != nil { + t.Fatalf("expected static account to not exist initially") + } + + // 2. Create new static account + testStaticCreate(t, td, staticName, + map[string]interface{}{ + "secret_type": SecretTypeKey, + "service_account_email": sa.Email, + "token_scopes": []string{iam.CloudPlatformScope}, + }) + + // 3. Read static account + respData = testStaticRead(t, td, staticName) + if respData == nil { + t.Fatalf("expected static account to have been created") + } + + verifyReadData(t, respData, map[string]interface{}{ + "secret_type": SecretTypeKey, + "service_account_email": sa.Email, + "service_account_project": sa.ProjectId, + "bindings": nil, + }) + // 4. Delete static account testStaticDelete(t, td, staticName) } @@ -398,3 +453,17 @@ func testStaticDelete(t *testing.T, td *testData, staticName string) { } } } + +// Return a list of key IDs for a service account +func getServiceAccountKeys(t *testing.T, td *testData, saName string) []string { + keysRaw, err := td.IamAdmin.Projects.ServiceAccounts.Keys.List(saName).Do() + if err != nil { + t.Fatalf("error listing service account keys: %v", err) + } + var keys []string + for _, key := range keysRaw.Keys { + keys = append(keys, key.Name) + } + sort.Strings(keys) + return keys +} From 8b3072f038828ea59fcd1028baf8897bd373c364 Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Wed, 17 Feb 2021 11:00:44 +0800 Subject: [PATCH 15/35] Reorder cleaning up of account to remove test warnings --- plugin/path_static_account_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugin/path_static_account_test.go b/plugin/path_static_account_test.go index c730d7e7..c911dc70 100644 --- a/plugin/path_static_account_test.go +++ b/plugin/path_static_account_test.go @@ -119,8 +119,8 @@ func TestPathStatic_UpdateDisallowed(t *testing.T) { defer deleteStaticAccount(t, td, sa) saNew := createStaticAccount(t, td, staticName+"-new") - defer deleteStaticAccount(t, td, saNew) defer cleanupStatic(t, td, staticName+"-new", util.StringSet{}) + defer deleteStaticAccount(t, td, saNew) // 1. Read should return nothing respData := testStaticRead(t, td, staticName) @@ -443,13 +443,13 @@ func testStaticDelete(t *testing.T, td *testData, staticName string) { }) if err != nil { - t.Fatalf("unable to delete role set: %v", err) + t.Fatalf("unable to delete static account: %v", err) } else if resp != nil { if len(resp.Warnings) > 0 { - t.Logf("warnings returned from role set delete. Warnings:\n %s\n", strings.Join(resp.Warnings, ",\n")) + t.Logf("warnings returned from static account delete. Warnings:\n %s\n", strings.Join(resp.Warnings, ",\n")) } if resp.IsError() { - t.Fatalf("unable to delete role set: %v", resp.Error()) + t.Fatalf("unable to delete static account: %v", resp.Error()) } } } From d8a8d86ac166f93ded4a553a24256492ca2a9ba6 Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Mon, 28 Jun 2021 10:46:43 +0800 Subject: [PATCH 16/35] Address feedback - Remove unnecessary check - Fix description - Fix some inconsistencies --- plugin/path_role_set.go | 6 +----- plugin/path_static_account.go | 2 +- plugin/path_static_account_secrets.go | 6 ++---- plugin/static_account.go | 3 ++- 4 files changed, 6 insertions(+), 11 deletions(-) diff --git a/plugin/path_role_set.go b/plugin/path_role_set.go index 00053045..c9d3e72f 100644 --- a/plugin/path_role_set.go +++ b/plugin/path_role_set.go @@ -157,11 +157,7 @@ func (b *backend) pathRoleSetRead(ctx context.Context, req *logical.Request, d * } func (b *backend) pathRoleSetDelete(ctx context.Context, req *logical.Request, d *framework.FieldData) (resp *logical.Response, err error) { - nameRaw, ok := d.GetOk("name") - if !ok { - return logical.ErrorResponse("name is required"), nil - } - rsName := nameRaw.(string) + rsName := d.Get("name").(string) b.rolesetLock.Lock() defer b.rolesetLock.Unlock() diff --git a/plugin/path_static_account.go b/plugin/path_static_account.go index 4411b149..561138db 100644 --- a/plugin/path_static_account.go +++ b/plugin/path_static_account.go @@ -39,7 +39,7 @@ func pathStaticAccount(b *backend) *framework.Path { }, "token_scopes": { Type: framework.TypeCommaStringSlice, - Description: fmt.Sprintf(`List of OAuth scopes to assign to access tokens generated under this account. Ignored if secret_type not %q`, SecretTypeKey), + Description: fmt.Sprintf(`List of OAuth scopes to assign to access tokens generated under this account. Ignored if "secret_type" is not "%q"`, SecretTypeAccessToken), }, }, ExistenceCheck: b.pathStaticAccountExistenceCheck, diff --git a/plugin/path_static_account_secrets.go b/plugin/path_static_account_secrets.go index 0dc11df5..cfd75985 100644 --- a/plugin/path_static_account_secrets.go +++ b/plugin/path_static_account_secrets.go @@ -10,8 +10,7 @@ import ( func pathStaticAccountSecretServiceAccountKey(b *backend) *framework.Path { return &framework.Path{ - Pattern: fmt.Sprintf("%s/%s/key", staticAccountPathPrefix, framework.GenericNameRegex("static_account")), - Deprecated: true, + Pattern: fmt.Sprintf("%s/%s/key", staticAccountPathPrefix, framework.GenericNameRegex("static_account")), Fields: map[string]*framework.FieldSchema{ "static_account": { Type: framework.TypeString, @@ -44,8 +43,7 @@ func pathStaticAccountSecretServiceAccountKey(b *backend) *framework.Path { func pathStaticAccountSecretAccessToken(b *backend) *framework.Path { return &framework.Path{ - Pattern: fmt.Sprintf("%s/%s/token", staticAccountPathPrefix, framework.GenericNameRegex("static_account")), - Deprecated: true, + Pattern: fmt.Sprintf("%s/%s/token", staticAccountPathPrefix, framework.GenericNameRegex("static_account")), Fields: map[string]*framework.FieldSchema{ "static_account": { Type: framework.TypeString, diff --git a/plugin/static_account.go b/plugin/static_account.go index bfaa039e..f9b14501 100644 --- a/plugin/static_account.go +++ b/plugin/static_account.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "github.com/hashicorp/errwrap" "github.com/hashicorp/go-gcp-common/gcputil" "github.com/hashicorp/go-multierror" @@ -110,7 +111,7 @@ func (b *backend) createStaticAccount(ctx context.Context, req *logical.Request, if err != nil { if isGoogleAccountNotFoundErr(err) { - return fmt.Errorf("unable to create static account - service account %q should exist", input.serviceAccountEmail) + return fmt.Errorf("unable to create static account, service account %q should exist", input.serviceAccountEmail) } return errwrap.Wrapf(fmt.Sprintf("unable to create static account, could not confirm service account %q exists: {{err}}", input.serviceAccountEmail), err) } From a60ef6974b2f28f3179d560591fe2671c19a40ef Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Mon, 28 Jun 2021 13:04:59 +0800 Subject: [PATCH 17/35] Rename `static_account` to `name` --- plugin/path_static_account_secrets.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/plugin/path_static_account_secrets.go b/plugin/path_static_account_secrets.go index cfd75985..d16a9a63 100644 --- a/plugin/path_static_account_secrets.go +++ b/plugin/path_static_account_secrets.go @@ -10,11 +10,11 @@ import ( func pathStaticAccountSecretServiceAccountKey(b *backend) *framework.Path { return &framework.Path{ - Pattern: fmt.Sprintf("%s/%s/key", staticAccountPathPrefix, framework.GenericNameRegex("static_account")), + Pattern: fmt.Sprintf("%s/%s/key", staticAccountPathPrefix, framework.GenericNameRegex("name")), Fields: map[string]*framework.FieldSchema{ - "static_account": { + "name": { Type: framework.TypeString, - Description: "Required. Name of the static_account.", + Description: "Required. Name of the static account.", }, "key_algorithm": { Type: framework.TypeString, @@ -43,11 +43,11 @@ func pathStaticAccountSecretServiceAccountKey(b *backend) *framework.Path { func pathStaticAccountSecretAccessToken(b *backend) *framework.Path { return &framework.Path{ - Pattern: fmt.Sprintf("%s/%s/token", staticAccountPathPrefix, framework.GenericNameRegex("static_account")), + Pattern: fmt.Sprintf("%s/%s/token", staticAccountPathPrefix, framework.GenericNameRegex("name")), Fields: map[string]*framework.FieldSchema{ - "static_account": { + "name": { Type: framework.TypeString, - Description: "Required. Name of the static_account.", + Description: "Required. Name of the static account.", }, }, ExistenceCheck: b.pathStaticAccountExistenceCheck, @@ -61,7 +61,7 @@ func pathStaticAccountSecretAccessToken(b *backend) *framework.Path { } func (b *backend) pathStaticAccountSecretKey(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - acctName := d.Get("static_account").(string) + acctName := d.Get("name").(string) keyType := d.Get("key_type").(string) keyAlg := d.Get("key_algorithm").(string) ttl := d.Get("ttl").(int) @@ -91,7 +91,7 @@ func (b *backend) pathStaticAccountSecretKey(ctx context.Context, req *logical.R } func (b *backend) pathStaticAccountAccessToken(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - acctName := d.Get("static_account").(string) + acctName := d.Get("name").(string) acct, err := b.getStaticAccount(acctName, ctx, req.Storage) if err != nil { From bd71751494b9115265249e8e7f4ed91a29b48d74 Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Mon, 28 Jun 2021 13:09:31 +0800 Subject: [PATCH 18/35] Remove existence check --- plugin/path_static_account_secrets.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/plugin/path_static_account_secrets.go b/plugin/path_static_account_secrets.go index d16a9a63..71154b5c 100644 --- a/plugin/path_static_account_secrets.go +++ b/plugin/path_static_account_secrets.go @@ -31,7 +31,6 @@ func pathStaticAccountSecretServiceAccountKey(b *backend) *framework.Path { Description: "Lifetime of the service account key", }, }, - ExistenceCheck: b.pathStaticAccountExistenceCheck, Operations: map[logical.Operation]framework.OperationHandler{ logical.ReadOperation: &framework.PathOperation{Callback: b.pathStaticAccountSecretKey}, logical.UpdateOperation: &framework.PathOperation{Callback: b.pathStaticAccountSecretKey}, @@ -50,7 +49,6 @@ func pathStaticAccountSecretAccessToken(b *backend) *framework.Path { Description: "Required. Name of the static account.", }, }, - ExistenceCheck: b.pathStaticAccountExistenceCheck, Operations: map[logical.Operation]framework.OperationHandler{ logical.ReadOperation: &framework.PathOperation{Callback: b.pathStaticAccountAccessToken}, logical.UpdateOperation: &framework.PathOperation{Callback: b.pathStaticAccountAccessToken}, From d95019432505fb7c69ff7d47bfc617dfef9801d1 Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Tue, 29 Jun 2021 14:39:42 +0800 Subject: [PATCH 19/35] Add nil check --- plugin/rollback.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/plugin/rollback.go b/plugin/rollback.go index c4947d31..7b021686 100644 --- a/plugin/rollback.go +++ b/plugin/rollback.go @@ -260,6 +260,9 @@ func (b *backend) serviceAccountPolicyDiffRollback(ctx context.Context, req *log if err != nil { return err } + if sa == nil { + return fmt.Errorf("static account %s not found", entry.StaticAccount) + } if sa.ResourceName() == entry.AccountId.ResourceName() { rolesInUse = sa.Bindings[entry.Resource] } From 02ea94493d14214a661fdda7c69193e685554b78 Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Tue, 29 Jun 2021 14:53:08 +0800 Subject: [PATCH 20/35] Add missing `StaticAccount` field to WAL entries --- plugin/static_account.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/plugin/static_account.go b/plugin/static_account.go index f9b14501..f730aa03 100644 --- a/plugin/static_account.go +++ b/plugin/static_account.go @@ -289,9 +289,10 @@ func (b *backend) addWalsForStaticAccountBindings(ctx context.Context, req *logi // Add WALs for resources in added bindings for resource, rolesAdded := range added { walEntry := &walIamPolicyStaticAccount{ - AccountId: a.ServiceAccountId, - Resource: resource, - RolesAdded: rolesAdded.ToSlice(), + StaticAccount: a.Name, + AccountId: a.ServiceAccountId, + Resource: resource, + RolesAdded: rolesAdded.ToSlice(), } if rolesRemoved, ok := removed[resource]; ok { walEntry.RolesRemoved = rolesRemoved.ToSlice() @@ -309,9 +310,10 @@ func (b *backend) addWalsForStaticAccountBindings(ctx context.Context, req *logi continue } walEntry := &walIamPolicyStaticAccount{ - AccountId: a.ServiceAccountId, - Resource: resource, - RolesRemoved: rolesRemoved.ToSlice(), + StaticAccount: a.Name, + AccountId: a.ServiceAccountId, + Resource: resource, + RolesRemoved: rolesRemoved.ToSlice(), } walId, err := framework.PutWAL(ctx, req.Storage, walTypeIamPolicyDiff, walEntry) From f50e480e2c8e750f2d1ef0a18bbc746a0fec2a60 Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Wed, 30 Jun 2021 13:58:12 +0800 Subject: [PATCH 21/35] Drop WAL Entry on missing Static Account --- plugin/rollback.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugin/rollback.go b/plugin/rollback.go index 7b021686..8ae8436d 100644 --- a/plugin/rollback.go +++ b/plugin/rollback.go @@ -261,7 +261,8 @@ func (b *backend) serviceAccountPolicyDiffRollback(ctx context.Context, req *log return err } if sa == nil { - return fmt.Errorf("static account %s not found", entry.StaticAccount) + b.Logger().Warn("static account %s not found, dropping WAL entry", entry.StaticAccount) + return nil } if sa.ResourceName() == entry.AccountId.ResourceName() { rolesInUse = sa.Bindings[entry.Resource] From 10c13c5ef124fa951917b73e7179ebce07e3bce4 Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Wed, 30 Jun 2021 14:00:54 +0800 Subject: [PATCH 22/35] Fix incorrect method call --- plugin/field_data_utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/field_data_utils.go b/plugin/field_data_utils.go index 77f1e7bd..dc079626 100644 --- a/plugin/field_data_utils.go +++ b/plugin/field_data_utils.go @@ -56,7 +56,7 @@ func (input *inputParams) parseOkInputServiceAccountEmail(d *framework.FieldData func (input *inputParams) parseOkInputTokenScopes(d *framework.FieldData) (warnings []string, err error) { // Parse secretType if not yet parsed if input.secretType == "" { - warnings, err = input.parseOkInputTokenScopes(d) + warnings, err = input.parseOkInputSecretType(d) if err != nil { return nil, err } From d22508b471ceedd303655acdb1b186d43e391807 Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Wed, 30 Jun 2021 14:05:13 +0800 Subject: [PATCH 23/35] Text consistency --- plugin/field_data_utils.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugin/field_data_utils.go b/plugin/field_data_utils.go index dc079626..e4460991 100644 --- a/plugin/field_data_utils.go +++ b/plugin/field_data_utils.go @@ -72,11 +72,11 @@ func (input *inputParams) parseOkInputTokenScopes(d *framework.FieldData) (warni } if input.secretType == SecretTypeAccessToken && len(input.scopes) == 0 { - return nil, fmt.Errorf("non-empty token_scopes must be provided for generating access token secrets") + return nil, fmt.Errorf("non-empty token_scopes must be provided for generating access_token secrets") } if input.secretType != SecretTypeAccessToken && ok && len(input.scopes) > 0 { - warnings = append(warnings, "ignoring non-empty token scopes, secret type not access_token") + warnings = append(warnings, "ignoring non-empty token_scopes, secret type not access_token") } return } From d10dd146c4ada1753ae1355596e6eca0da7812ac Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Wed, 30 Jun 2021 14:06:46 +0800 Subject: [PATCH 24/35] Use `staticAccountLock` --- plugin/rollback.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugin/rollback.go b/plugin/rollback.go index 8ae8436d..3da858a5 100644 --- a/plugin/rollback.go +++ b/plugin/rollback.go @@ -245,8 +245,8 @@ func (b *backend) serviceAccountPolicyRollback(ctx context.Context, req *logical } func (b *backend) serviceAccountPolicyDiffRollback(ctx context.Context, req *logical.Request, data interface{}) error { - b.rolesetLock.Lock() - defer b.rolesetLock.Unlock() + b.staticAccountLock.Lock() + defer b.staticAccountLock.Unlock() var entry walIamPolicyStaticAccount if err := mapstructure.Decode(data, &entry); err != nil { From c1aef82742272b0d10554db5249d999ea86355ef Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Thu, 1 Jul 2021 15:06:40 +0800 Subject: [PATCH 25/35] Pass context to GCP IAM Calls --- plugin/gcp_account_resources.go | 8 ++++---- plugin/secrets_service_account_key.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/plugin/gcp_account_resources.go b/plugin/gcp_account_resources.go index 6a8d3ff6..8bd0eca6 100644 --- a/plugin/gcp_account_resources.go +++ b/plugin/gcp_account_resources.go @@ -80,7 +80,7 @@ func (b *backend) createNewTokenGen(ctx context.Context, req *logical.Request, p parent, &iam.CreateServiceAccountKeyRequest{ PrivateKeyType: privateKeyTypeJson, - }).Do() + }).Context(ctx).Do() if err != nil { return nil, err } @@ -147,7 +147,7 @@ func (b *backend) createServiceAccount(ctx context.Context, req *logical.Request return nil, err } - return iamAdmin.Projects.ServiceAccounts.Create(fmt.Sprintf("projects/%s", project), createSaReq).Do() + return iamAdmin.Projects.ServiceAccounts.Create(fmt.Sprintf("projects/%s", project), createSaReq).Context(ctx).Do() } // tryDeleteGcpAccountResources creates WALs to clean up a service account's @@ -200,7 +200,7 @@ func (b *backend) deleteTokenGenKey(ctx context.Context, iamAdmin *iam.Service, return nil } - _, err := iamAdmin.Projects.ServiceAccounts.Keys.Delete(tgen.KeyName).Do() + _, err := iamAdmin.Projects.ServiceAccounts.Keys.Delete(tgen.KeyName).Context(ctx).Do() if err != nil && !isGoogleAccountKeyNotFoundErr(err) { return errwrap.Wrapf("unable to delete service account key: {{err}}", err) } @@ -248,7 +248,7 @@ func (b *backend) deleteServiceAccount(ctx context.Context, iamAdmin *iam.Servic return nil } - _, err := iamAdmin.Projects.ServiceAccounts.Delete(account.ResourceName()).Do() + _, err := iamAdmin.Projects.ServiceAccounts.Delete(account.ResourceName()).Context(ctx).Do() if err != nil && !isGoogleAccountNotFoundErr(err) { return errwrap.Wrapf("unable to delete service account: {{err}}", err) } diff --git a/plugin/secrets_service_account_key.go b/plugin/secrets_service_account_key.go index 40c051cb..7e721d9b 100644 --- a/plugin/secrets_service_account_key.go +++ b/plugin/secrets_service_account_key.go @@ -143,7 +143,7 @@ func (b *backend) secretKeyRevoke(ctx context.Context, req *logical.Request, d * return logical.ErrorResponse(err.Error()), nil } - _, err = iamAdmin.Projects.ServiceAccounts.Keys.Delete(keyNameRaw.(string)).Do() + _, err = iamAdmin.Projects.ServiceAccounts.Keys.Delete(keyNameRaw.(string)).Context(ctx).Do() if err != nil && !isGoogleAccountKeyNotFoundErr(err) { return logical.ErrorResponse("unable to delete service account key: %v", err), nil } From 44650b1555e4dadfbeb3d4ef4c10ea9b5719bf2e Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Fri, 2 Jul 2021 09:06:34 +0800 Subject: [PATCH 26/35] Apply suggestions from code review Co-authored-by: Calvin Leung Huang <1883212+calvn@users.noreply.github.com> Co-authored-by: Austin Gebauer <34121980+austingebauer@users.noreply.github.com> --- plugin/gcp_account_resources.go | 2 +- plugin/path_static_account_secrets.go | 8 ++++---- plugin/rollback.go | 2 +- plugin/secrets_service_account_key.go | 3 ++- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/plugin/gcp_account_resources.go b/plugin/gcp_account_resources.go index 8bd0eca6..84099044 100644 --- a/plugin/gcp_account_resources.go +++ b/plugin/gcp_account_resources.go @@ -158,7 +158,7 @@ func (b *backend) tryDeleteGcpAccountResources(ctx context.Context, req *logical return nil } - b.Logger().Debug("try to delete GCP account resources", "bound resources", boundResources, "remove service account", removeServiceAccount) + b.Logger().Debug("try to delete GCP account resources", "bound_resources", boundResources, "remove_service_account", removeServiceAccount) iamAdmin, err := b.IAMAdminClient(req.Storage) if err != nil { diff --git a/plugin/path_static_account_secrets.go b/plugin/path_static_account_secrets.go index 71154b5c..dc0cbec3 100644 --- a/plugin/path_static_account_secrets.go +++ b/plugin/path_static_account_secrets.go @@ -18,12 +18,12 @@ func pathStaticAccountSecretServiceAccountKey(b *backend) *framework.Path { }, "key_algorithm": { Type: framework.TypeString, - Description: fmt.Sprintf(`Private key algorithm for service account key - defaults to %s"`, keyAlgorithmRSA2k), + Description: fmt.Sprintf(`Private key algorithm for service account key. Defaults to %s."`, keyAlgorithmRSA2k), Default: keyAlgorithmRSA2k, }, "key_type": { Type: framework.TypeString, - Description: fmt.Sprintf(`Private key type for service account key - defaults to %s"`, privateKeyTypeJson), + Description: fmt.Sprintf(`Private key type for service account key. Defaults to %s."`, privateKeyTypeJson), Default: privateKeyTypeJson, }, "ttl": { @@ -53,8 +53,8 @@ func pathStaticAccountSecretAccessToken(b *backend) *framework.Path { logical.ReadOperation: &framework.PathOperation{Callback: b.pathStaticAccountAccessToken}, logical.UpdateOperation: &framework.PathOperation{Callback: b.pathStaticAccountAccessToken}, }, - HelpSynopsis: pathServiceAccountKeySyn, - HelpDescription: pathServiceAccountKeyDesc, + HelpSynopsis: pathTokenHelpSyn, + HelpDescription: pathTokenHelpDesc, } } diff --git a/plugin/rollback.go b/plugin/rollback.go index 3da858a5..357b74dc 100644 --- a/plugin/rollback.go +++ b/plugin/rollback.go @@ -139,7 +139,7 @@ func (b *backend) serviceAccountKeyRollback(ctx context.Context, req *logical.Re } } default: - b.Logger().Error("removing invalid walAccountKey with empty RoleSet and empty StaticAccount - may need manual cleanup: %v", entry) + b.Logger().Error("removing invalid walAccountKey with empty RoleSet and empty StaticAccount, may need manual cleanup: %v", entry) return nil } diff --git a/plugin/secrets_service_account_key.go b/plugin/secrets_service_account_key.go index 7e721d9b..326c65d3 100644 --- a/plugin/secrets_service_account_key.go +++ b/plugin/secrets_service_account_key.go @@ -20,7 +20,8 @@ const ( ) type secretKeyParams struct { - keyType, keyAlgorithm string + keyType string + keyAlgorithm string ttl int extraInternalData map[string]interface{} } From 17c1921211c6a43e17361e4ff1363d526c018654 Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Mon, 5 Jul 2021 11:09:37 +0800 Subject: [PATCH 27/35] Use function instead of global variable --- plugin/path_role_set_secrets.go | 61 +++++++++++++++------------ plugin/secrets_service_account_key.go | 8 ++-- 2 files changed, 37 insertions(+), 32 deletions(-) diff --git a/plugin/path_role_set_secrets.go b/plugin/path_role_set_secrets.go index 6b94f05f..69457ba0 100644 --- a/plugin/path_role_set_secrets.go +++ b/plugin/path_role_set_secrets.go @@ -8,38 +8,43 @@ import ( "github.com/hashicorp/vault/sdk/logical" ) -var fieldSchemaRoleSetServiceAccountKey = map[string]*framework.FieldSchema{ - "roleset": { - Type: framework.TypeString, - Description: "Required. Name of the role set.", - }, - "key_algorithm": { - Type: framework.TypeString, - Description: fmt.Sprintf(`Private key algorithm for service account key - defaults to %s"`, keyAlgorithmRSA2k), - Default: keyAlgorithmRSA2k, - }, - "key_type": { - Type: framework.TypeString, - Description: fmt.Sprintf(`Private key type for service account key - defaults to %s"`, privateKeyTypeJson), - Default: privateKeyTypeJson, - }, - "ttl": { - Type: framework.TypeDurationSecond, - Description: "Lifetime of the service account key", - }, +func fieldSchemaRoleSetServiceAccountKey() map[string]*framework.FieldSchema { + return map[string]*framework.FieldSchema{ + "roleset": { + Type: framework.TypeString, + Description: "Required. Name of the role set.", + }, + "key_algorithm": { + Type: framework.TypeString, + Description: fmt.Sprintf(`Private key algorithm for service account key - defaults to %s"`, keyAlgorithmRSA2k), + Default: keyAlgorithmRSA2k, + }, + "key_type": { + Type: framework.TypeString, + Description: fmt.Sprintf(`Private key type for service account key - defaults to %s"`, privateKeyTypeJson), + Default: privateKeyTypeJson, + }, + "ttl": { + Type: framework.TypeDurationSecond, + Description: "Lifetime of the service account key", + }, + } } -var fieldSchemaRoleSetAccessToken = map[string]*framework.FieldSchema{ - "roleset": { - Type: framework.TypeString, - Description: "Required. Name of the role set.", - }, +func fieldSchemaRoleSetAccessToken() map[string]*framework.FieldSchema { + return map[string]*framework.FieldSchema{ + "roleset": { + Type: framework.TypeString, + Description: "Required. Name of the role set.", + }, + } + } func pathRoleSetSecretServiceAccountKey(b *backend) *framework.Path { return &framework.Path{ Pattern: fmt.Sprintf("roleset/%s/key", framework.GenericNameRegex("roleset")), - Fields: fieldSchemaRoleSetServiceAccountKey, + Fields: fieldSchemaRoleSetServiceAccountKey(), ExistenceCheck: b.pathRoleSetExistenceCheck("roleset"), Operations: map[logical.Operation]framework.OperationHandler{ logical.ReadOperation: &framework.PathOperation{Callback: b.pathRoleSetSecretKey}, @@ -54,7 +59,7 @@ func deprecatedPathRoleSetSecretServiceAccountKey(b *backend) *framework.Path { return &framework.Path{ Pattern: fmt.Sprintf("key/%s", framework.GenericNameRegex("roleset")), Deprecated: true, - Fields: fieldSchemaRoleSetServiceAccountKey, + Fields: fieldSchemaRoleSetServiceAccountKey(), ExistenceCheck: b.pathRoleSetExistenceCheck("roleset"), Operations: map[logical.Operation]framework.OperationHandler{ logical.ReadOperation: &framework.PathOperation{Callback: b.pathRoleSetSecretKey}, @@ -68,7 +73,7 @@ func deprecatedPathRoleSetSecretServiceAccountKey(b *backend) *framework.Path { func pathRoleSetSecretAccessToken(b *backend) *framework.Path { return &framework.Path{ Pattern: fmt.Sprintf("roleset/%s/token", framework.GenericNameRegex("roleset")), - Fields: fieldSchemaRoleSetAccessToken, + Fields: fieldSchemaRoleSetAccessToken(), ExistenceCheck: b.pathRoleSetExistenceCheck("roleset"), Operations: map[logical.Operation]framework.OperationHandler{ logical.ReadOperation: &framework.PathOperation{Callback: b.pathRoleSetSecretAccessToken}, @@ -83,7 +88,7 @@ func deprecatedPathRoleSetSecretAccessToken(b *backend) *framework.Path { return &framework.Path{ Pattern: fmt.Sprintf("token/%s", framework.GenericNameRegex("roleset")), Deprecated: true, - Fields: fieldSchemaRoleSetAccessToken, + Fields: fieldSchemaRoleSetAccessToken(), ExistenceCheck: b.pathRoleSetExistenceCheck("roleset"), Operations: map[logical.Operation]framework.OperationHandler{ logical.ReadOperation: &framework.PathOperation{Callback: b.pathRoleSetSecretAccessToken}, diff --git a/plugin/secrets_service_account_key.go b/plugin/secrets_service_account_key.go index 326c65d3..01494415 100644 --- a/plugin/secrets_service_account_key.go +++ b/plugin/secrets_service_account_key.go @@ -20,10 +20,10 @@ const ( ) type secretKeyParams struct { - keyType string - keyAlgorithm string - ttl int - extraInternalData map[string]interface{} + keyType string + keyAlgorithm string + ttl int + extraInternalData map[string]interface{} } func secretServiceAccountKey(b *backend) *framework.Secret { From 51441cc254cf328b170b8a744f23d14864bab012 Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Mon, 5 Jul 2021 11:18:51 +0800 Subject: [PATCH 28/35] Add lock in `pathStaticAccountDelete` --- plugin/path_static_account.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/plugin/path_static_account.go b/plugin/path_static_account.go index 561138db..9b894803 100644 --- a/plugin/path_static_account.go +++ b/plugin/path_static_account.go @@ -129,6 +129,9 @@ func (b *backend) pathStaticAccountDelete(ctx context.Context, req *logical.Requ } name := nameRaw.(string) + b.staticAccountLock.Lock() + defer b.staticAccountLock.Unlock() + acct, err := b.getStaticAccount(name, ctx, req.Storage) if err != nil { return nil, errwrap.Wrapf(fmt.Sprintf("unable to get static account %q: {{err}}", name), err) From f75a274a74edc4843e4c604a5ebd430c4ddf48ec Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Mon, 5 Jul 2021 12:54:45 +0800 Subject: [PATCH 29/35] Reorder imports --- plugin/secrets_service_account_key.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugin/secrets_service_account_key.go b/plugin/secrets_service_account_key.go index 01494415..06ad6f57 100644 --- a/plugin/secrets_service_account_key.go +++ b/plugin/secrets_service_account_key.go @@ -5,9 +5,8 @@ import ( "fmt" "time" - "github.com/hashicorp/go-gcp-common/gcputil" - "github.com/hashicorp/errwrap" + "github.com/hashicorp/go-gcp-common/gcputil" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/logical" "google.golang.org/api/iam/v1" From e73c57412438a864dcb1cc6cca897df40f53db64 Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Mon, 5 Jul 2021 13:24:20 +0800 Subject: [PATCH 30/35] Remove "currently xxx" in errors --- plugin/field_data_utils.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugin/field_data_utils.go b/plugin/field_data_utils.go index e4460991..47dad2b8 100644 --- a/plugin/field_data_utils.go +++ b/plugin/field_data_utils.go @@ -28,7 +28,7 @@ func (input *inputParams) parseOkInputSecretType(d *framework.FieldData) (warnin return nil, fmt.Errorf("secret_type is required") } if input.secretType != "" && secretType != "" && input.secretType != secretType { - return nil, fmt.Errorf("cannot update secret_type - currently %q", input.secretType) + return nil, fmt.Errorf("cannot update secret_type") } switch secretType { @@ -46,7 +46,7 @@ func (input *inputParams) parseOkInputServiceAccountEmail(d *framework.FieldData return nil, fmt.Errorf("email is required") } if input.serviceAccountEmail != "" && email != "" && input.serviceAccountEmail != email { - return nil, fmt.Errorf("cannot update email - currently %q", input.serviceAccountEmail) + return nil, fmt.Errorf("cannot update email") } input.serviceAccountEmail = email From 1d69048ad9d494bf74c464ff69e06c16c74e51a2 Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Mon, 5 Jul 2021 13:36:35 +0800 Subject: [PATCH 31/35] Add godoc for `parseOkInputServiceAccountEmail` --- plugin/field_data_utils.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/plugin/field_data_utils.go b/plugin/field_data_utils.go index 47dad2b8..1b009017 100644 --- a/plugin/field_data_utils.go +++ b/plugin/field_data_utils.go @@ -40,6 +40,9 @@ func (input *inputParams) parseOkInputSecretType(d *framework.FieldData) (warnin } } +// parseOkInputServiceAccountEmail checks that when creating a static acocunt, a service account +// email is provided. A service account email can be provide while updating the static account +// but it must be the same as the one in the static account and cannot be updated. func (input *inputParams) parseOkInputServiceAccountEmail(d *framework.FieldData) (warnings []string, err error) { email := d.Get("service_account_email").(string) if email == "" && input.serviceAccountEmail == "" { From a7e9ea70ce064a17ad5d14db0e192beaab44f825 Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Mon, 5 Jul 2021 13:47:56 +0800 Subject: [PATCH 32/35] Add more details to field description --- plugin/path_static_account.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugin/path_static_account.go b/plugin/path_static_account.go index 9b894803..7e43d887 100644 --- a/plugin/path_static_account.go +++ b/plugin/path_static_account.go @@ -22,16 +22,16 @@ func pathStaticAccount(b *backend) *framework.Path { Fields: map[string]*framework.FieldSchema{ "name": { Type: framework.TypeString, - Description: "Required. Name to refer to this static account in Vault.", + Description: "Required. Name to refer to this static account in Vault. Cannot be updated.", }, "secret_type": { Type: framework.TypeString, - Description: fmt.Sprintf("Type of secret generated for this account. Defaults to %q", SecretTypeAccessToken), + Description: fmt.Sprintf("Type of secret generated for this account. Cannot be updated. Defaults to %q", SecretTypeAccessToken), Default: SecretTypeAccessToken, }, "service_account_email": { Type: framework.TypeString, - Description: "Email of the GCP service account to manage.", + Description: "Required. Email of the GCP service account to manage. Cannot be updated.", }, "bindings": { Type: framework.TypeString, From 533727eb13d6092015b229f1d87852c095ce0960 Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Mon, 5 Jul 2021 13:57:14 +0800 Subject: [PATCH 33/35] Update Secret Path help wording --- plugin/secrets_access_token.go | 10 +++------- plugin/secrets_service_account_key.go | 6 +++++- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/plugin/secrets_access_token.go b/plugin/secrets_access_token.go index 50727c86..9e1d7869 100644 --- a/plugin/secrets_access_token.go +++ b/plugin/secrets_access_token.go @@ -56,16 +56,12 @@ engine and will be cleaned up now. Note that there is the chance that this acces will still be valid up to one hour. ` -const pathTokenHelpSyn = `Generate an OAuth2 access token under a specific role set.` +const pathTokenHelpSyn = `Generate an OAuth2 access token secret.` const pathTokenHelpDesc = ` This path will generate a new OAuth2 access token for accessing GCP APIs. -A role set, binding IAM roles to specific GCP resources, will be specified -by name - for example, if this backend is mounted at "gcp", -then "gcp/token/deploy" would generate tokens for the "deploy" role set. -On the backend, each roleset is associated with a service account. -The token will be associated with this service account. Tokens have a -short-term lease (1-hour) associated with them but cannot be renewed. +Either specify "roleset/my-roleset" or "static/my-account" to generate a key corresponding +to a roleset or static account respectively. Please see backend documentation for more information: https://www.vaultproject.io/docs/secrets/gcp/index.html diff --git a/plugin/secrets_service_account_key.go b/plugin/secrets_service_account_key.go index 06ad6f57..da85b46c 100644 --- a/plugin/secrets_service_account_key.go +++ b/plugin/secrets_service_account_key.go @@ -201,9 +201,13 @@ func (b *backend) createServiceAccountKeySecret(ctx context.Context, s logical.S return resp, nil } -const pathServiceAccountKeySyn = `Generate an service account private key secret.` +const pathServiceAccountKeySyn = `Generate a service account private key secret.` const pathServiceAccountKeyDesc = ` This path will generate a new service account key for accessing GCP APIs. + Either specify "roleset/my-roleset" or "static/my-account" to generate a key corresponding to a roleset or static account respectively. + +Please see backend documentation for more information: +https://www.vaultproject.io/docs/secrets/gcp/index.html ` From 1bdd252470b3b65796ced60b10d530430d44a64d Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Mon, 5 Jul 2021 14:24:31 +0800 Subject: [PATCH 34/35] Add test step to list static accounts --- plugin/path_static_account.go | 4 +-- plugin/path_static_account_test.go | 39 ++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/plugin/path_static_account.go b/plugin/path_static_account.go index 7e43d887..416c831d 100644 --- a/plugin/path_static_account.go +++ b/plugin/path_static_account.go @@ -63,9 +63,9 @@ func pathStaticAccount(b *backend) *framework.Path { } func pathStaticAccountList(b *backend) *framework.Path { - // Paths for listing role sets + // Paths for listing static accounts return &framework.Path{ - Pattern: fmt.Sprintf("%ss?/?", staticAccountPathPrefix), + Pattern: fmt.Sprintf("%s?/?", staticAccountPathPrefix), Operations: map[logical.Operation]framework.OperationHandler{ logical.ListOperation: &framework.PathOperation{ Callback: b.pathStaticAccountList, diff --git a/plugin/path_static_account_test.go b/plugin/path_static_account_test.go index c911dc70..1ead8476 100644 --- a/plugin/path_static_account_test.go +++ b/plugin/path_static_account_test.go @@ -59,6 +59,8 @@ func TestPathStatic_Basic(t *testing.T) { "service_account_project": sa.ProjectId, "bindings": nil, }) + // Test static account is listed + testStaticList(t, td, staticName) // 4. Delete static account testStaticDelete(t, td, staticName) @@ -103,6 +105,8 @@ func TestPathStatic_Key(t *testing.T) { "service_account_project": sa.ProjectId, "bindings": nil, }) + // Test static account is listed + testStaticList(t, td, staticName) // 4. Delete static account testStaticDelete(t, td, staticName) @@ -220,6 +224,8 @@ func TestPathStatic_WithBindings(t *testing.T) { "service_account_project": sa.ProjectId, "bindings": expectedBinds, }) + // Test static account is listed + testStaticList(t, td, staticName) // Verify service account has given role on project verifyProjectBinding(t, td, sa.Email, roles) @@ -454,6 +460,39 @@ func testStaticDelete(t *testing.T, td *testData, staticName string) { } } +func testStaticList(t *testing.T, td *testData, staticName string) { + resp, err := td.B.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.ListOperation, + Path: staticAccountPathPrefix, + Storage: td.S, + }) + + if err != nil { + t.Fatalf("unable to list static accounts: %v", err) + } else if resp != nil { + if len(resp.Warnings) > 0 { + t.Logf("warnings returned from static account list. Warnings:\n %s\n", strings.Join(resp.Warnings, ",\n")) + } + if resp.IsError() { + t.Fatalf("unable to list static accounts: %v", resp.Error()) + } + keys, ok := resp.Data["keys"].([]string) + if !ok { + t.Fatalf("expected response to contain data map with key 'key' of type []string keys") + } + found := false + for _, s := range keys { + if s == staticName { + found = true + break + } + } + if !found { + t.Fatalf("expected static accounts listing to contain static account but did not") + } + } +} + // Return a list of key IDs for a service account func getServiceAccountKeys(t *testing.T, td *testData, saName string) []string { keysRaw, err := td.IamAdmin.Projects.ServiceAccounts.Keys.List(saName).Do() From 8fba665423e0f328670ccc579a5537998ffd8f94 Mon Sep 17 00:00:00 2001 From: Yong Wen Chua Date: Wed, 7 Jul 2021 14:17:58 +0800 Subject: [PATCH 35/35] Change path prefix --- plugin/path_static_account.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugin/path_static_account.go b/plugin/path_static_account.go index 416c831d..2311a754 100644 --- a/plugin/path_static_account.go +++ b/plugin/path_static_account.go @@ -11,8 +11,8 @@ import ( ) const ( - staticAccountStoragePrefix = "static" - staticAccountPathPrefix = "static" + staticAccountStoragePrefix = "static-account" + staticAccountPathPrefix = "static-account" gcpServiceAccountInferredProject = "-" ) @@ -65,7 +65,7 @@ func pathStaticAccount(b *backend) *framework.Path { func pathStaticAccountList(b *backend) *framework.Path { // Paths for listing static accounts return &framework.Path{ - Pattern: fmt.Sprintf("%s?/?", staticAccountPathPrefix), + Pattern: fmt.Sprintf("%ss?/?", staticAccountPathPrefix), Operations: map[logical.Operation]framework.OperationHandler{ logical.ListOperation: &framework.PathOperation{ Callback: b.pathStaticAccountList,