From ba8188e6d7a4ab1fd309c5599c827710019d4ba0 Mon Sep 17 00:00:00 2001 From: Michael Gaffney Date: Fri, 3 Dec 2021 14:50:40 -0500 Subject: [PATCH] feat(credential): Add mapping override to credential library on lookup This change returns any mapping override set on a credential library when the library is retrieved from the repository using the lookup method. --- .../vault/repository_credential_library.go | 59 +++- .../repository_credential_library_test.go | 255 ++++++++++++++---- .../postgres/10/04_vault_credential.up.sql | 1 + .../22/05_vault_private_library.up.sql | 20 ++ 4 files changed, 277 insertions(+), 58 deletions(-) diff --git a/internal/credential/vault/repository_credential_library.go b/internal/credential/vault/repository_credential_library.go index 6d2c763cd..2c1d8e0a7 100644 --- a/internal/credential/vault/repository_credential_library.go +++ b/internal/credential/vault/repository_credential_library.go @@ -7,6 +7,7 @@ import ( "github.com/hashicorp/boundary/internal/db" dbcommon "github.com/hashicorp/boundary/internal/db/common" + "github.com/hashicorp/boundary/internal/db/timestamp" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/oplog" @@ -214,7 +215,7 @@ func (r *Repository) LookupCredentialLibrary(ctx context.Context, publicId strin if publicId == "" { return nil, errors.New(ctx, errors.InvalidParameter, op, "no public id") } - l := allocCredentialLibrary() + l := allocPublicLibrary() l.PublicId = publicId if err := r.reader.LookupByPublicId(ctx, l); err != nil { if errors.IsNotFoundError(err) { @@ -222,9 +223,63 @@ func (r *Repository) LookupCredentialLibrary(ctx context.Context, publicId strin } return nil, errors.Wrap(ctx, err, op, errors.WithMsg(fmt.Sprintf("failed for: %s", publicId))) } - return l, nil + return l.toCredentialLibrary(), nil } +// publicLibrary is a credential library and any of library's credential +// mapping overrides. It does not include encrypted data and is safe to +// return external to boundary. +type publicLibrary struct { + PublicId string `gorm:"primary_key"` + StoreId string + Name string + Description string + CreateTime *timestamp.Timestamp + UpdateTime *timestamp.Timestamp + Version uint32 + VaultPath string + HttpMethod string + HttpRequestBody []byte + CredentialType string + UsernameAttribute string + PasswordAttribute string +} + +func allocPublicLibrary() *publicLibrary { + return &publicLibrary{} +} + +func (pl *publicLibrary) toCredentialLibrary() *CredentialLibrary { + cl := allocCredentialLibrary() + cl.PublicId = pl.PublicId + cl.StoreId = pl.StoreId + cl.Name = pl.Name + cl.Description = pl.Description + cl.CreateTime = pl.CreateTime + cl.UpdateTime = pl.UpdateTime + cl.Version = pl.Version + cl.VaultPath = pl.VaultPath + cl.HttpMethod = pl.HttpMethod + cl.HttpRequestBody = pl.HttpRequestBody + cl.CredentialLibrary.CredentialType = pl.CredentialType + + if pl.UsernameAttribute != "" || pl.PasswordAttribute != "" { + up := allocUserPasswordOverride() + up.LibraryId = pl.PublicId + up.UsernameAttribute = pl.UsernameAttribute + up.PasswordAttribute = pl.PasswordAttribute + up.sanitize() + cl.MappingOverride = up + } + return cl +} + +// TableName returns the table name for gorm. +func (_ *publicLibrary) TableName() string { return "credential_vault_library_public" } + +// GetPublicId returns the public id. +func (pl *publicLibrary) GetPublicId() string { return pl.PublicId } + // DeleteCredentialLibrary deletes publicId from the repository and returns // the number of records deleted. func (r *Repository) DeleteCredentialLibrary(ctx context.Context, scopeId string, publicId string, _ ...Option) (int, error) { diff --git a/internal/credential/vault/repository_credential_library_test.go b/internal/credential/vault/repository_credential_library_test.go index a3caaf27d..2b3b876f9 100644 --- a/internal/credential/vault/repository_credential_library_test.go +++ b/internal/credential/vault/repository_credential_library_test.go @@ -212,9 +212,11 @@ func TestRepository_CreateCredentialLibrary(t *testing.T) { wantErr: errors.VaultInvalidMappingOverride, }, { - name: "valid-user-password-credential-type-with-override", + name: "valid-user-password-credential-type-with-username-override", in: &CredentialLibrary{ - MappingOverride: NewUserPasswordOverride(WithOverrideUsernameAttribute("test")), + MappingOverride: NewUserPasswordOverride( + WithOverrideUsernameAttribute("utest"), + ), CredentialLibrary: &store.CredentialLibrary{ StoreId: cs.GetPublicId(), HttpMethod: "GET", @@ -223,7 +225,61 @@ func TestRepository_CreateCredentialLibrary(t *testing.T) { }, }, want: &CredentialLibrary{ - MappingOverride: NewUserPasswordOverride(WithOverrideUsernameAttribute("test")), + MappingOverride: NewUserPasswordOverride( + WithOverrideUsernameAttribute("utest"), + ), + CredentialLibrary: &store.CredentialLibrary{ + StoreId: cs.GetPublicId(), + HttpMethod: "GET", + VaultPath: "/some/path", + CredentialType: string(credential.UserPasswordType), + }, + }, + }, + { + name: "valid-user-password-credential-type-with-password-override", + in: &CredentialLibrary{ + MappingOverride: NewUserPasswordOverride( + WithOverridePasswordAttribute("ptest"), + ), + CredentialLibrary: &store.CredentialLibrary{ + StoreId: cs.GetPublicId(), + HttpMethod: "GET", + VaultPath: "/some/path", + CredentialType: string(credential.UserPasswordType), + }, + }, + want: &CredentialLibrary{ + MappingOverride: NewUserPasswordOverride( + WithOverridePasswordAttribute("ptest"), + ), + CredentialLibrary: &store.CredentialLibrary{ + StoreId: cs.GetPublicId(), + HttpMethod: "GET", + VaultPath: "/some/path", + CredentialType: string(credential.UserPasswordType), + }, + }, + }, + { + name: "valid-user-password-credential-type-with-username-and-password-override", + in: &CredentialLibrary{ + MappingOverride: NewUserPasswordOverride( + WithOverrideUsernameAttribute("utest"), + WithOverridePasswordAttribute("ptest"), + ), + CredentialLibrary: &store.CredentialLibrary{ + StoreId: cs.GetPublicId(), + HttpMethod: "GET", + VaultPath: "/some/path", + CredentialType: string(credential.UserPasswordType), + }, + }, + want: &CredentialLibrary{ + MappingOverride: NewUserPasswordOverride( + WithOverrideUsernameAttribute("utest"), + WithOverridePasswordAttribute("ptest"), + ), CredentialLibrary: &store.CredentialLibrary{ StoreId: cs.GetPublicId(), HttpMethod: "GET", @@ -1021,64 +1077,151 @@ func TestRepository_LookupCredentialLibrary(t *testing.T) { rw := db.New(conn) wrapper := db.TestWrapper(t) - _, prj := iam.TestScopes(t, iam.TestRepo(t, conn, wrapper)) - cs := TestCredentialStores(t, conn, wrapper, prj.GetPublicId(), 1)[0] - l := TestCredentialLibraries(t, conn, wrapper, cs.GetPublicId(), 1)[0] + { + _, prj := iam.TestScopes(t, iam.TestRepo(t, conn, wrapper)) + cs := TestCredentialStores(t, conn, wrapper, prj.GetPublicId(), 1)[0] - badId, err := newCredentialLibraryId() - require.NoError(t, err) - require.NotNil(t, badId) + tests := []struct { + name string + in *CredentialLibrary + }{ + { + name: "valid-no-options", + in: &CredentialLibrary{ + CredentialLibrary: &store.CredentialLibrary{ + StoreId: cs.GetPublicId(), + HttpMethod: "GET", + VaultPath: "/some/path", + }, + }, + }, + { + name: "valid-user-password-credential-type", + in: &CredentialLibrary{ + CredentialLibrary: &store.CredentialLibrary{ + StoreId: cs.GetPublicId(), + HttpMethod: "GET", + VaultPath: "/some/path", + CredentialType: string(credential.UserPasswordType), + }, + }, + }, + { + name: "valid-user-password-credential-type-with-username-override", + in: &CredentialLibrary{ + MappingOverride: NewUserPasswordOverride( + WithOverrideUsernameAttribute("utest"), + ), + CredentialLibrary: &store.CredentialLibrary{ + StoreId: cs.GetPublicId(), + HttpMethod: "GET", + VaultPath: "/some/path", + CredentialType: string(credential.UserPasswordType), + }, + }, + }, + { + name: "valid-user-password-credential-type-with-password-override", + in: &CredentialLibrary{ + MappingOverride: NewUserPasswordOverride( + WithOverridePasswordAttribute("ptest"), + ), + CredentialLibrary: &store.CredentialLibrary{ + StoreId: cs.GetPublicId(), + HttpMethod: "GET", + VaultPath: "/some/path", + CredentialType: string(credential.UserPasswordType), + }, + }, + }, + { + name: "valid-user-password-credential-type-with-username-and-password-override", + in: &CredentialLibrary{ + MappingOverride: NewUserPasswordOverride( + WithOverrideUsernameAttribute("utest"), + WithOverridePasswordAttribute("ptest"), + ), + CredentialLibrary: &store.CredentialLibrary{ + StoreId: cs.GetPublicId(), + HttpMethod: "GET", + VaultPath: "/some/path", + CredentialType: string(credential.UserPasswordType), + }, + }, + }, + } - tests := []struct { - name string - in string - want *CredentialLibrary - wantErr errors.Code - }{ - { - name: "valid", - in: l.GetPublicId(), - want: l, - }, - { - name: "empty-public-id", - in: "", - wantErr: errors.InvalidParameter, - }, - { - name: "not-found", - in: badId, - }, + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + // setup + assert, require := assert.New(t), require.New(t) + ctx := context.Background() + kms := kms.TestKms(t, conn, wrapper) + sche := scheduler.TestScheduler(t, conn, wrapper) + repo, err := NewRepository(rw, rw, kms, sche) + assert.NoError(err) + require.NotNil(repo) + orig, err := repo.CreateCredentialLibrary(ctx, prj.GetPublicId(), tt.in) + assert.NoError(err) + require.NotEmpty(orig) + // test + got, err := repo.LookupCredentialLibrary(ctx, orig.GetPublicId()) + assert.NoError(err) + require.NotEmpty(got) + assert.Equal(orig.Name, got.Name) + assert.Equal(orig.Description, got.Description) + assert.Equal(orig.CredentialType(), got.CredentialType()) + if tt.in.MappingOverride != nil { + require.NotNil(got.MappingOverride) + assert.IsType(orig.MappingOverride, got.MappingOverride) + switch w := orig.MappingOverride.(type) { + case *UserPasswordOverride: + g, ok := got.MappingOverride.(*UserPasswordOverride) + require.True(ok) + assert.Equal(w.UsernameAttribute, g.UsernameAttribute) + assert.Equal(w.PasswordAttribute, g.PasswordAttribute) + default: + assert.Fail("Unknown mapping override") + } + } + }) + } } - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - assert, require := assert.New(t), require.New(t) - ctx := context.Background() - kms := kms.TestKms(t, conn, wrapper) - sche := scheduler.TestScheduler(t, conn, wrapper) - repo, err := NewRepository(rw, rw, kms, sche) - assert.NoError(err) - require.NotNil(repo) + t.Run("empty-public-id", func(t *testing.T) { + // setup + assert, require := assert.New(t), require.New(t) + ctx := context.Background() + kms := kms.TestKms(t, conn, wrapper) + sche := scheduler.TestScheduler(t, conn, wrapper) + repo, err := NewRepository(rw, rw, kms, sche) + assert.NoError(err) + require.NotNil(repo) + // test + got, err := repo.LookupCredentialLibrary(ctx, "") + wantErr := errors.InvalidParameter + assert.Truef(errors.Match(errors.T(wantErr), err), "want err: %q got: %q", wantErr, err) + assert.Nil(got) + }) - got, err := repo.LookupCredentialLibrary(ctx, tt.in) - if tt.wantErr != 0 { - assert.Truef(errors.Match(errors.T(tt.wantErr), err), "want err: %q got: %q", tt.wantErr, err) - assert.Nil(got) - return - } - require.NoError(err) - - switch { - case tt.want == nil: - assert.Nil(got) - case tt.want != nil: - assert.NotNil(got) - assert.Equal(got, tt.want) - } - }) - } + t.Run("not-found", func(t *testing.T) { + // setup + assert, require := assert.New(t), require.New(t) + ctx := context.Background() + kms := kms.TestKms(t, conn, wrapper) + sche := scheduler.TestScheduler(t, conn, wrapper) + repo, err := NewRepository(rw, rw, kms, sche) + assert.NoError(err) + require.NotNil(repo) + badId, err := newCredentialLibraryId() + assert.NoError(err) + require.NotNil(badId) + // test + got, err := repo.LookupCredentialLibrary(ctx, badId) + assert.NoError(err) + assert.Empty(got) + }) } func TestRepository_DeleteCredentialLibrary(t *testing.T) { diff --git a/internal/db/schema/migrations/oss/postgres/10/04_vault_credential.up.sql b/internal/db/schema/migrations/oss/postgres/10/04_vault_credential.up.sql index f1ecbb2a6..e47ece684 100644 --- a/internal/db/schema/migrations/oss/postgres/10/04_vault_credential.up.sql +++ b/internal/db/schema/migrations/oss/postgres/10/04_vault_credential.up.sql @@ -577,6 +577,7 @@ begin; 'credential_vault_store_public is a view where each row contains a credential store. ' 'No encrypted data is returned. This view can be used to retrieve data which will be returned external to boundary.'; +-- Replaced in 21/05_vault_private_library.up.sql create view credential_vault_library_private as select library.public_id as public_id, library.store_id as store_id, diff --git a/internal/db/schema/migrations/oss/postgres/22/05_vault_private_library.up.sql b/internal/db/schema/migrations/oss/postgres/22/05_vault_private_library.up.sql index ac9fa30c2..b0964bbf8 100644 --- a/internal/db/schema/migrations/oss/postgres/22/05_vault_private_library.up.sql +++ b/internal/db/schema/migrations/oss/postgres/22/05_vault_private_library.up.sql @@ -1,5 +1,6 @@ begin; +-- Replaces view from 10/04_vault_credential.up.sql drop view credential_vault_library_private; create view credential_vault_library_private as @@ -45,4 +46,23 @@ begin; 'credential_vault_library_private is a view where each row contains a credential library and the credential library''s data needed to connect to Vault. ' 'Each row may contain encrypted data. This view should not be used to retrieve data which will be returned external to boundary.'; + create view credential_vault_library_public as + select public_id, + store_id, + name, + description, + create_time, + update_time, + version, + vault_path, + http_method, + http_request_body, + credential_type, + username_attribute, + password_attribute + from credential_vault_library_private; + comment on view credential_vault_library_public is + 'credential_vault_library_public is a view where each row contains a credential library and any of library''s credential mapping overrides. ' + 'No encrypted data is returned. This view can be used to retrieve data which will be returned external to boundary.'; + commit;