refactor(vault): add context and remove deprecated errors (#2396)

* refact(vault): add context and remove deprecated errors
This commit is contained in:
Irena Rindos authored and GitHub committed 2022-08-25 14:37:22 -04:00
1 parent aef9073fa6
commit b4b95e0f0e
16 files changed
+169 -141

No files matched your search

+1 -1
View File
@@ -212,7 +212,7 @@ func (c *Controller) registerGrpcServices(s *grpc.Server) error {
services.RegisterManagedGroupServiceServer(s, mgs)
}
if _, ok := currentServices[services.CredentialStoreService_ServiceDesc.ServiceName]; !ok {
cs, err := credentialstores.NewService(c.VaultCredentialRepoFn, c.StaticCredentialRepoFn, c.IamRepoFn)
cs, err := credentialstores.NewService(c.baseContext, c.VaultCredentialRepoFn, c.StaticCredentialRepoFn, c.IamRepoFn)
if err != nil {
return fmt.Errorf("failed to create credential store handler service: %w", err)
}
@@ -94,19 +94,20 @@ type Service struct {
// NewService returns a credential store service which handles credential store related requests to boundary.
func NewService(
ctx context.Context,
vaultRepo common.VaultCredentialRepoFactory,
staticRepo common.StaticCredentialRepoFactory,
iamRepo common.IamRepoFactory,
) (Service, error) {
const op = "credentialstores.NewService"
if iamRepo == nil {
return Service{}, errors.NewDeprecated(errors.InvalidParameter, op, "missing iam repository")
return Service{}, errors.New(ctx, errors.InvalidParameter, op, "missing iam repository")
}
if vaultRepo == nil {
return Service{}, errors.NewDeprecated(errors.InvalidParameter, op, "missing vault credential repository")
return Service{}, errors.New(ctx, errors.InvalidParameter, op, "missing vault credential repository")
}
if staticRepo == nil {
return Service{}, errors.NewDeprecated(errors.InvalidParameter, op, "missing static credential repository")
return Service{}, errors.New(ctx, errors.InvalidParameter, op, "missing static credential repository")
}
return Service{iamRepoFn: iamRepo, vaultRepoFn: vaultRepo, staticRepoFn: staticRepo}, nil
}
@@ -183,7 +184,7 @@ func (s Service) ListCredentialStores(ctx context.Context, req *pbs.ListCredenti
outputOpts = append(outputOpts, handlers.WithAuthorizedCollectionActions(collectionActions))
}
item, err := toProto(item, outputOpts...)
item, err := toProto(ctx, item, outputOpts...)
if err != nil {
return nil, err
}
@@ -236,7 +237,7 @@ func (s Service) GetCredentialStore(ctx context.Context, req *pbs.GetCredentialS
outputOpts = append(outputOpts, handlers.WithAuthorizedCollectionActions(collectionActions))
}
item, err := toProto(cs, outputOpts...)
item, err := toProto(ctx, cs, outputOpts...)
if err != nil {
return nil, err
}
@@ -248,7 +249,7 @@ func (s Service) GetCredentialStore(ctx context.Context, req *pbs.GetCredentialS
func (s Service) CreateCredentialStore(ctx context.Context, req *pbs.CreateCredentialStoreRequest) (*pbs.CreateCredentialStoreResponse, error) {
const op = "credentialstores.(Service).CreateCredentialStore"
if err := validateCreateRequest(req); err != nil {
if err := validateCreateRequest(ctx, req); err != nil {
return nil, err
}
authResults := s.authResult(ctx, req.GetItem().GetScopeId(), action.Create)
@@ -281,7 +282,7 @@ func (s Service) CreateCredentialStore(ctx context.Context, req *pbs.CreateCrede
outputOpts = append(outputOpts, handlers.WithAuthorizedCollectionActions(collectionActions))
}
item, err := toProto(cs, outputOpts...)
item, err := toProto(ctx, cs, outputOpts...)
if err != nil {
return nil, err
}
@@ -296,7 +297,7 @@ func (s Service) CreateCredentialStore(ctx context.Context, req *pbs.CreateCrede
func (s Service) UpdateCredentialStore(ctx context.Context, req *pbs.UpdateCredentialStoreRequest) (*pbs.UpdateCredentialStoreResponse, error) {
const op = "credentialstores.(Service).UpdateCredentialStore"
if err := validateUpdateRequest(req); err != nil {
if err := validateUpdateRequest(ctx, req); err != nil {
return nil, err
}
authResults := s.authResult(ctx, req.GetId(), action.Update)
@@ -329,7 +330,7 @@ func (s Service) UpdateCredentialStore(ctx context.Context, req *pbs.UpdateCrede
outputOpts = append(outputOpts, handlers.WithAuthorizedCollectionActions(collectionActions))
}
item, err := toProto(cs, outputOpts...)
item, err := toProto(ctx, cs, outputOpts...)
if err != nil {
return nil, err
}
@@ -424,7 +425,7 @@ func (s Service) createInRepo(ctx context.Context, projId string, item *pb.Crede
switch item.Type {
case vault.Subtype.String():
cs, err := toStorageVaultStore(projId, item)
cs, err := toStorageVaultStore(ctx, projId, item)
if err != nil {
return nil, errors.Wrap(ctx, err, op)
}
@@ -439,7 +440,7 @@ func (s Service) createInRepo(ctx context.Context, projId string, item *pb.Crede
return out, nil
case static.Subtype.String():
cs, err := toStorageStaticStore(projId, item)
cs, err := toStorageStaticStore(ctx, projId, item)
if err != nil {
return nil, errors.Wrap(ctx, err, op)
}
@@ -471,7 +472,7 @@ func (s Service) updateInRepo(ctx context.Context, projId, id string, mask []str
switch subtypes.SubtypeFromId(domain, id) {
case vault.Subtype:
cs, err := toStorageVaultStore(projId, item)
cs, err := toStorageVaultStore(ctx, projId, item)
if err != nil {
return nil, errors.Wrap(ctx, err, op)
}
@@ -487,7 +488,7 @@ func (s Service) updateInRepo(ctx context.Context, projId, id string, mask []str
}
case static.Subtype:
cs, err := toStorageStaticStore(projId, item)
cs, err := toStorageStaticStore(ctx, projId, item)
if err != nil {
return nil, errors.Wrap(ctx, err, op)
}
@@ -607,7 +608,7 @@ func (s Service) authResult(ctx context.Context, id string, a action.Type) auth.
return auth.Verify(ctx, opts...)
}
func toProto(in credential.Store, opt ...handlers.Option) (*pb.CredentialStore, error) {
func toProto(ctx context.Context, in credential.Store, opt ...handlers.Option) (*pb.CredentialStore, error) {
const op = "credentialstores.toProto"
opts := handlers.GetOpts(opt...)
@@ -655,7 +656,7 @@ func toProto(in credential.Store, opt ...handlers.Option) (*pb.CredentialStore,
case vault.Subtype:
vaultIn, ok := in.(*vault.CredentialStore)
if !ok {
return nil, errors.NewDeprecated(errors.Internal, op, "unable to cast to vault credential store")
return nil, errors.New(ctx, errors.Internal, op, "unable to cast to vault credential store")
}
attrs := &pb.VaultCredentialStoreAttributes{
Address: wrapperspb.String(vaultIn.GetVaultAddress()),
@@ -695,7 +696,7 @@ func toProto(in credential.Store, opt ...handlers.Option) (*pb.CredentialStore,
return &out, nil
}
func toStorageStaticStore(scopeId string, in *pb.CredentialStore) (out *static.CredentialStore, err error) {
func toStorageStaticStore(ctx context.Context, scopeId string, in *pb.CredentialStore) (out *static.CredentialStore, err error) {
const op = "credentialstores.toStorageStaticStore"
var opts []static.Option
if in.GetName() != nil {
@@ -707,12 +708,12 @@ func toStorageStaticStore(scopeId string, in *pb.CredentialStore) (out *static.C
cs, err := static.NewCredentialStore(scopeId, opts...)
if err != nil {
return nil, errors.WrapDeprecated(err, op, errors.WithMsg("unable to build credential store for creation"))
return nil, errors.Wrap(ctx, err, op, errors.WithMsg("unable to build credential store for creation"))
}
return cs, err
}
func toStorageVaultStore(scopeId string, in *pb.CredentialStore) (out *vault.CredentialStore, err error) {
func toStorageVaultStore(ctx context.Context, scopeId string, in *pb.CredentialStore) (out *vault.CredentialStore, err error) {
const op = "credentialstores.toStorageVaultStore"
var opts []vault.Option
if in.GetName() != nil {
@@ -741,9 +742,9 @@ func toStorageVaultStore(scopeId string, in *pb.CredentialStore) (out *vault.Cre
if attrs.GetCaCert() != nil {
opts = append(opts, vault.WithCACert([]byte(attrs.GetCaCert().GetValue())))
}
pemCerts, pemPk, err := extractClientCertAndPk(attrs.GetClientCertificate().GetValue(), attrs.GetClientCertificateKey().GetValue())
pemCerts, pemPk, err := extractClientCertAndPk(ctx, attrs.GetClientCertificate().GetValue(), attrs.GetClientCertificateKey().GetValue())
if err != nil {
return nil, errors.WrapDeprecated(err, op)
return nil, errors.Wrap(ctx, err, op)
}
if len(pemCerts) != 0 {
var cert []byte
@@ -756,14 +757,14 @@ func toStorageVaultStore(scopeId string, in *pb.CredentialStore) (out *vault.Cre
}
cc, err := vault.NewClientCertificate(cert, pk)
if err != nil {
return nil, errors.WrapDeprecated(err, op)
return nil, errors.Wrap(ctx, err, op)
}
opts = append(opts, vault.WithClientCert(cc))
}
cs, err := vault.NewCredentialStore(scopeId, attrs.GetAddress().GetValue(), []byte(attrs.GetToken().GetValue()), opts...)
if err != nil {
return nil, errors.WrapDeprecated(err, op, errors.WithMsg("unable to build credential store for creation"))
return nil, errors.Wrap(ctx, err, op, errors.WithMsg("unable to build credential store for creation"))
}
return cs, err
}
@@ -777,7 +778,7 @@ func validateGetRequest(req *pbs.GetCredentialStoreRequest) error {
return handlers.ValidateGetRequest(handlers.NoopValidatorFn, req, vault.CredentialStorePrefix, static.CredentialStorePrefix, static.PreviousCredentialStorePrefix)
}
func validateCreateRequest(req *pbs.CreateCredentialStoreRequest) error {
func validateCreateRequest(ctx context.Context, req *pbs.CreateCredentialStoreRequest) error {
return handlers.ValidateCreateRequest(req.GetItem(), func() map[string]string {
badFields := map[string]string{}
if !handlers.ValidId(handlers.Id(req.GetItem().GetScopeId()), scope.Project.Prefix()) {
@@ -806,12 +807,12 @@ func validateCreateRequest(req *pbs.CreateCredentialStoreRequest) error {
}
}
// TODO(ICU-1478 and ICU-1479): Validate client and CA certificate payloads
_, err := decodePemBlocks(attrs.GetCaCert().GetValue())
_, err := decodePemBlocks(ctx, attrs.GetCaCert().GetValue())
if attrs.GetCaCert() != nil && err != nil {
badFields[caCertsField] = "Incorrectly formatted value."
}
cs, pk, err := extractClientCertAndPk(attrs.GetClientCertificate().GetValue(), attrs.GetClientCertificateKey().GetValue())
cs, pk, err := extractClientCertAndPk(ctx, attrs.GetClientCertificate().GetValue(), attrs.GetClientCertificateKey().GetValue())
if err != nil {
badFields[clientCertField] = fmt.Sprintf("Invalid values: %q", err.Error())
}
@@ -830,7 +831,7 @@ func validateCreateRequest(req *pbs.CreateCredentialStoreRequest) error {
})
}
func validateUpdateRequest(req *pbs.UpdateCredentialStoreRequest) error {
func validateUpdateRequest(ctx context.Context, req *pbs.UpdateCredentialStoreRequest) error {
return handlers.ValidateUpdateRequest(req, req.GetItem(), func() map[string]string {
badFields := map[string]string{}
switch subtypes.SubtypeFromId(domain, req.GetId()) {
@@ -859,12 +860,12 @@ func validateUpdateRequest(req *pbs.UpdateCredentialStoreRequest) error {
}
// TODO(ICU-1478 and ICU-1479): Validate client and CA certificate payloads
_, err := decodePemBlocks(attrs.GetCaCert().GetValue())
_, err := decodePemBlocks(ctx, attrs.GetCaCert().GetValue())
if attrs.GetCaCert() != nil && err != nil {
badFields[caCertsField] = "Incorrectly formatted value."
}
_, _, err = extractClientCertAndPk(attrs.GetClientCertificate().GetValue(), attrs.GetClientCertificateKey().GetValue())
_, _, err = extractClientCertAndPk(ctx, attrs.GetClientCertificate().GetValue(), attrs.GetClientCertificateKey().GetValue())
if err != nil {
badFields[clientCertField] = fmt.Sprintf("Invalid values: %q", err.Error())
}
@@ -57,6 +57,7 @@ var (
)
func TestList(t *testing.T) {
ctx := context.Background()
conn, _ := db.TestSetup(t, "postgres")
wrapper := db.TestWrapper(t)
kms := kms.TestKms(t, conn, wrapper)
@@ -168,7 +169,7 @@ func TestList(t *testing.T) {
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
s, err := NewService(vaultRepoFn, staticRepoFn, iamRepoFn)
s, err := NewService(ctx, vaultRepoFn, staticRepoFn, iamRepoFn)
require.NoError(t, err, "Couldn't create new host set service.")
// Test non-anonymous listing
@@ -197,6 +198,7 @@ func TestList(t *testing.T) {
}
func TestCreateVault(t *testing.T) {
ctx := context.Background()
conn, _ := db.TestSetup(t, "postgres")
wrapper := db.TestWrapper(t)
kms := kms.TestKms(t, conn, wrapper)
@@ -524,7 +526,7 @@ func TestCreateVault(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
assert, require := assert.New(t), require.New(t)
s, err := NewService(vaultRepoFn, staticRepoFn, iamRepoFn)
s, err := NewService(ctx, vaultRepoFn, staticRepoFn, iamRepoFn)
require.NoError(err, "Error when getting new credential store service.")
defer cleanup(s)
@@ -577,6 +579,7 @@ func TestCreateVault(t *testing.T) {
}
func TestCreateStatic(t *testing.T) {
ctx := context.Background()
conn, _ := db.TestSetup(t, "postgres")
wrapper := db.TestWrapper(t)
kms := kms.TestKms(t, conn, wrapper)
@@ -700,7 +703,7 @@ func TestCreateStatic(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
assert, require := assert.New(t), require.New(t)
s, err := NewService(vaultRepoFn, staticRepoFn, iamRepoFn)
s, err := NewService(ctx, vaultRepoFn, staticRepoFn, iamRepoFn)
require.NoError(err, "Error when getting new credential store service.")
defer cleanup(s)
@@ -742,6 +745,7 @@ func TestCreateStatic(t *testing.T) {
}
func TestGet(t *testing.T) {
ctx := context.Background()
conn, _ := db.TestSetup(t, "postgres")
wrapper := db.TestWrapper(t)
kms := kms.TestKms(t, conn, wrapper)
@@ -764,7 +768,7 @@ func TestGet(t *testing.T) {
vaultStore := vault.TestCredentialStores(t, conn, wrapper, prj.GetPublicId(), 1)[0]
staticStore := credstatic.TestCredentialStore(t, conn, wrapper, prj.GetPublicId())
staticStorePrev := credstatic.TestCredentialStore(t, conn, wrapper, prj.GetPublicId(), credstatic.WithPublicId(fmt.Sprintf("%s_1234567890", credstatic.PreviousCredentialStorePrefix)))
s, err := NewService(vaultRepoFn, staticRepoFn, iamRepoFn)
s, err := NewService(ctx, vaultRepoFn, staticRepoFn, iamRepoFn)
require.NoError(t, err)
cases := []struct {
@@ -872,6 +876,7 @@ func TestGet(t *testing.T) {
}
func TestDelete(t *testing.T) {
ctx := context.Background()
conn, _ := db.TestSetup(t, "postgres")
wrapper := db.TestWrapper(t)
kms := kms.TestKms(t, conn, wrapper)
@@ -895,7 +900,7 @@ func TestDelete(t *testing.T) {
vaultStore := vault.TestCredentialStores(t, conn, wrapper, prj.GetPublicId(), 2)[0]
staticStore := credstatic.TestCredentialStore(t, conn, wrapper, prj.GetPublicId())
s, err := NewService(vaultRepoFn, staticRepoFn, iamRepoFn)
s, err := NewService(ctx, vaultRepoFn, staticRepoFn, iamRepoFn)
require.NoError(t, err)
cases := []struct {
@@ -968,7 +973,7 @@ func TestUpdateVault(t *testing.T) {
_, prj := iam.TestScopes(t, iamRepo)
ctx := auth.DisabledAuthTestContext(iamRepoFn, prj.GetPublicId())
s, err := NewService(vaultRepoFn, staticRepoFn, iamRepoFn)
s, err := NewService(ctx, vaultRepoFn, staticRepoFn, iamRepoFn)
require.NoError(t, err)
fieldmask := func(paths ...string) *fieldmaskpb.FieldMask {
@@ -1300,7 +1305,7 @@ func TestUpdateStatic(t *testing.T) {
_, prj := iam.TestScopes(t, iamRepo)
ctx := auth.DisabledAuthTestContext(iamRepoFn, prj.GetPublicId())
s, err := NewService(vaultRepoFn, staticRepoFn, iamRepoFn)
s, err := NewService(ctx, vaultRepoFn, staticRepoFn, iamRepoFn)
require.NoError(t, err)
fieldmask := func(paths ...string) *fieldmaskpb.FieldMask {
@@ -1,6 +1,7 @@
package credentialstores
import (
"context"
"encoding/pem"
"fmt"
"strings"
@@ -12,11 +13,11 @@ import (
// certificate and client certificate key and parses them into pem blocks.
// Any non certificate pem blocks are treated as a private key. An error is
// returned if there is more than 1 private key provided across both fields.
func extractClientCertAndPk(cert, pk string) ([]*pem.Block, *pem.Block, error) {
func extractClientCertAndPk(ctx context.Context, cert, pk string) ([]*pem.Block, *pem.Block, error) {
const op = "credentialstores.extractClientCertAndPk"
pks, err := decodePemBlocks(pk)
pks, err := decodePemBlocks(ctx, pk)
if err != nil {
return nil, nil, errors.WrapDeprecated(err, op, errors.WithMsg("failed to parse client certificate private key"))
return nil, nil, errors.Wrap(ctx, err, op, errors.WithMsg("failed to parse client certificate private key"))
}
var pkPem *pem.Block
switch len(pks) {
@@ -24,12 +25,12 @@ func extractClientCertAndPk(cert, pk string) ([]*pem.Block, *pem.Block, error) {
case 1:
pkPem = pks[0]
default:
return nil, nil, errors.NewDeprecated(errors.InvalidParameter, op, "private key payload contained multiple pem blocks")
return nil, nil, errors.New(ctx, errors.InvalidParameter, op, "private key payload contained multiple pem blocks")
}
bs, err := decodePemBlocks(cert)
bs, err := decodePemBlocks(ctx, cert)
if err != nil {
return nil, nil, errors.WrapDeprecated(err, op, errors.WithMsg("failed to parse client certificate into pem blocks"))
return nil, nil, errors.Wrap(ctx, err, op, errors.WithMsg("failed to parse client certificate into pem blocks"))
}
pkIdx := -1
for i, b := range bs {
@@ -38,9 +39,9 @@ func extractClientCertAndPk(cert, pk string) ([]*pem.Block, *pem.Block, error) {
case pkPem == nil:
pkIdx, pkPem = i, b
case pkIdx < 0:
return nil, nil, errors.NewDeprecated(errors.InvalidParameter, op, "client certificate contains a private key when one was also provided separately")
return nil, nil, errors.New(ctx, errors.InvalidParameter, op, "client certificate contains a private key when one was also provided separately")
default:
return nil, nil, errors.NewDeprecated(errors.InvalidParameter, op, fmt.Sprintf("second primary key found at %d after previous one found at %d", i, pkIdx))
return nil, nil, errors.New(ctx, errors.InvalidParameter, op, fmt.Sprintf("second primary key found at %d after previous one found at %d", i, pkIdx))
}
}
}
@@ -50,7 +51,7 @@ func extractClientCertAndPk(cert, pk string) ([]*pem.Block, *pem.Block, error) {
return bs, pkPem, nil
}
func decodePemBlocks(input string) ([]*pem.Block, error) {
func decodePemBlocks(ctx context.Context, input string) ([]*pem.Block, error) {
const op = "credentialstores.decodePemBlocks"
cpIn := make([]byte, len(input))
copy(cpIn, input)
@@ -64,7 +65,7 @@ func decodePemBlocks(input string) ([]*pem.Block, error) {
ret = append(ret, p)
}
if len(cpIn) > 0 {
return nil, errors.NewDeprecated(errors.InvalidParameter, op, "not all data parseable by pem block")
return nil, errors.New(ctx, errors.InvalidParameter, op, "not all data parseable by pem block")
}
return ret, nil
}
@@ -1,6 +1,7 @@
package credentialstores
import (
"context"
"encoding/pem"
"testing"
@@ -10,10 +11,11 @@ import (
)
func TestPkAndClientCerts(t *testing.T) {
ctx := context.Background()
s := vault.NewTestVaultServer(t, vault.WithTestVaultTLS(vault.TestClientTLS))
t.Run("seperate client cert and key", func(t *testing.T) {
c, k, err := extractClientCertAndPk(string(s.ClientCert), string(s.ClientKey))
c, k, err := extractClientCertAndPk(ctx, string(s.ClientCert), string(s.ClientKey))
assert.NoError(t, err)
require.NotNil(t, k)
assert.Equal(t, s.ClientKey, pem.EncodeToMemory(k))
@@ -22,7 +24,7 @@ func TestPkAndClientCerts(t *testing.T) {
})
t.Run("client cert and key bundled in cert", func(t *testing.T) {
c, k, err := extractClientCertAndPk(string(s.ClientCert)+string(s.ClientKey), "")
c, k, err := extractClientCertAndPk(ctx, string(s.ClientCert)+string(s.ClientKey), "")
assert.NoError(t, err)
require.NotNil(t, k)
assert.Equal(t, s.ClientKey, pem.EncodeToMemory(k))
@@ -31,7 +33,7 @@ func TestPkAndClientCerts(t *testing.T) {
})
t.Run("cert with no private key", func(t *testing.T) {
c, k, err := extractClientCertAndPk(string(s.ClientCert), "")
c, k, err := extractClientCertAndPk(ctx, string(s.ClientCert), "")
assert.NoError(t, err)
assert.Nil(t, k)
require.Len(t, c, 1)
@@ -39,7 +41,7 @@ func TestPkAndClientCerts(t *testing.T) {
})
t.Run("private key with no cert", func(t *testing.T) {
c, k, err := extractClientCertAndPk("", string(s.ClientKey))
c, k, err := extractClientCertAndPk(ctx, "", string(s.ClientKey))
assert.NoError(t, err)
assert.NotNil(t, k)
assert.Equal(t, s.ClientKey, pem.EncodeToMemory(k))
@@ -47,13 +49,13 @@ func TestPkAndClientCerts(t *testing.T) {
})
t.Run("error cases", func(t *testing.T) {
_, _, err := extractClientCertAndPk(string(s.ClientCert), "invalid key")
_, _, err := extractClientCertAndPk(ctx, string(s.ClientCert), "invalid key")
assert.Error(t, err)
_, _, err = extractClientCertAndPk("invalid cert", string(s.ClientKey))
_, _, err = extractClientCertAndPk(ctx, "invalid cert", string(s.ClientKey))
assert.Error(t, err)
// private key with the cert and the private key
c, k, err := extractClientCertAndPk(string(s.ClientCert)+string(s.ClientKey), string(s.ClientKey))
c, k, err := extractClientCertAndPk(ctx, string(s.ClientCert)+string(s.ClientKey), string(s.ClientKey))
assert.Error(t, err)
assert.Nil(t, k)
assert.Empty(t, c)