Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Backport of Support clearing an identity alias' custom_metadata into release/1.9.x #13704

Merged
merged 4 commits into from
Jan 19, 2022
Merged

Backport of Support clearing an identity alias' custom_metadata into release/1.9.x #13704

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f805cc3
backport of commit 43760fe9538f9b4dd6e72d024a06394aedc49925
benashz Dec 10, 2021
a3f0956
backport of commit 76bc89361f40561c4f05863150eb48014e9a67cf
benashz Dec 10, 2021
7e1c191
backport of commit 6d2e589f69ce7b500fde232b21d0f4c22d9597db
benashz Dec 10, 2021
cd8f013
Merge branch 'release/1.9.x' into backport/VAULT-4074/identity-alias-…
benashz Jan 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog/13395.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
core/identity: Support updating an alias' `custom_metadata` to be empty.
```
32 changes: 14 additions & 18 deletions vault/identity_store_aliases.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,20 @@ import (
"github.com/golang/protobuf/ptypes"
"github.com/hashicorp/go-multierror"
"github.com/hashicorp/go-secure-stdlib/strutil"

"github.com/hashicorp/vault/helper/identity"
"github.com/hashicorp/vault/helper/namespace"
"github.com/hashicorp/vault/helper/storagepacker"
"github.com/hashicorp/vault/sdk/framework"
"github.com/hashicorp/vault/sdk/logical"
"github.com/mitchellh/mapstructure"
)

const maxCustomMetadataKeys = 64
const maxCustomMetadataKeyLength = 128
const maxCustomMetadataValueLength = 512
const customMetadataValidationErrorPrefix = "custom_metadata validation failed"
const (
maxCustomMetadataKeys = 64
maxCustomMetadataKeyLength = 128
maxCustomMetadataValueLength = 512
customMetadataValidationErrorPrefix = "custom_metadata validation failed"
)

// aliasPaths returns the API endpoints to operate on aliases.
// Following are the paths supported:
Expand Down Expand Up @@ -138,10 +140,7 @@ func (i *IdentityStore) handleAliasCreateUpdate() framework.OperationFunc {
customMetadata := make(map[string]string)
data, customMetadataExists := d.GetOk("custom_metadata")
if customMetadataExists {
err = mapstructure.Decode(data, &customMetadata)
if err != nil {
return nil, err
}
customMetadata = data.(map[string]string)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could a client craft a request that causes a panic if they set custom_metadata to something that's not a map[string]string?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not my PR, but I reviewed the original prior to the backport. This type assertion should be safe due to the GetOk call above. The custom_metadata field is defined as framework.TypeKVPairs which is ultimately parsed as map[string]string.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explainer! TIL

}

// Get entity id
Expand All @@ -151,11 +150,9 @@ func (i *IdentityStore) handleAliasCreateUpdate() framework.OperationFunc {
canonicalID = d.Get("entity_id").(string)
}

//validate customMetadata if provided
// validate customMetadata if provided
if len(customMetadata) != 0 {

err := validateCustomMetadata(customMetadata)
if err != nil {
if err := validateCustomMetadata(customMetadata); err != nil {
return nil, err
}
}
Expand All @@ -178,25 +175,24 @@ func (i *IdentityStore) handleAliasCreateUpdate() framework.OperationFunc {
if alias.NamespaceID != ns.ID {
return logical.ErrorResponse("cannot modify aliases across namespaces"), logical.ErrPermissionDenied
}
if !customMetadataExists {
customMetadata = alias.CustomMetadata
}
switch {
case mountAccessor == "" && name == "" && len(customMetadata) == 0:
case mountAccessor == "" && name == "":
// Just a canonical ID update, maybe
if canonicalID == "" {
// Nothing to do, so be idempotent
return nil, nil
}
name = alias.Name
mountAccessor = alias.MountAccessor
customMetadata = alias.CustomMetadata
case mountAccessor == "":
// No change to mount accessor
mountAccessor = alias.MountAccessor
case name == "":
// No change to mount name
name = alias.Name
case len(customMetadata) == 0:
// No change to custom metadata
customMetadata = alias.CustomMetadata
default:
// mountAccessor, name and customMetadata provided
}
Expand Down
208 changes: 162 additions & 46 deletions vault/identity_store_aliases_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,55 +355,173 @@ func TestIdentityStore_AliasRegister(t *testing.T) {
}

func TestIdentityStore_AliasUpdate(t *testing.T) {
var err error
var resp *logical.Response
ctx := namespace.RootContext(nil)
is, githubAccessor, _ := testIdentityStoreWithGithubAuth(ctx, t)

aliasData := map[string]interface{}{
"name": "testaliasname",
"mount_accessor": githubAccessor,
}

aliasReq := &logical.Request{
Operation: logical.UpdateOperation,
Path: "entity-alias",
Data: aliasData,
}

// This will create an alias and a corresponding entity
resp, err = is.HandleRequest(ctx, aliasReq)
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("err:%v resp:%#v", err, resp)
}
aliasID := resp.Data["id"].(string)
customMetadata := make(map[string]string)
customMetadata["foo"] = "abc"

updateData := map[string]interface{}{
"name": "updatedaliasname",
"mount_accessor": githubAccessor,
"custom_metadata": customMetadata,
}

aliasReq.Data = updateData
aliasReq.Path = "entity-alias/id/" + aliasID
resp, err = is.HandleRequest(ctx, aliasReq)
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("err:%v resp:%#v", err, resp)
}

aliasReq.Operation = logical.ReadOperation
resp, err = is.HandleRequest(ctx, aliasReq)
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("err:%v resp:%#v", err, resp)
tests := []struct {
name string
createData map[string]interface{}
updateData map[string]interface{}
}{
{
name: "noop",
createData: map[string]interface{}{
"name": "noop",
"mount_accessor": githubAccessor,
"custom_metadata": map[string]string{
"foo": "baz",
"bar": "qux",
},
},
updateData: map[string]interface{}{
"name": "noop",
"mount_accessor": githubAccessor,
"custom_metadata": map[string]string{
"foo": "baz",
"bar": "qux",
},
},
},
{
name: "no-custom-metadata",
createData: map[string]interface{}{
"name": "no-custom-metadata",
"mount_accessor": githubAccessor,
},
updateData: map[string]interface{}{
"name": "no-custom-metadata",
"mount_accessor": githubAccessor,
},
},
{
name: "update-name-custom-metadata",
createData: map[string]interface{}{
"name": "update-name-custom-metadata",
"mount_accessor": githubAccessor,
"custom_metadata": make(map[string]string),
},
updateData: map[string]interface{}{
"name": "update-name-custom-metadata-2",
"mount_accessor": githubAccessor,
"custom_metadata": map[string]string{
"bar": "qux",
},
},
},
{
name: "update-name",
createData: map[string]interface{}{
"name": "update-name",
"mount_accessor": githubAccessor,
"custom_metadata": map[string]string{
"foo": "baz",
},
},
updateData: map[string]interface{}{
"name": "update-name-2",
"mount_accessor": githubAccessor,
"custom_metadata": map[string]string{
"foo": "baz",
},
},
},
{
name: "update-metadata",
createData: map[string]interface{}{
"name": "update-metadata",
"mount_accessor": githubAccessor,
"custom_metadata": map[string]string{
"foo": "bar",
},
},
updateData: map[string]interface{}{
"name": "update-metadata",
"mount_accessor": githubAccessor,
"custom_metadata": map[string]string{
"foo": "baz",
"bar": "qux",
},
},
},
{
name: "clear-metadata",
createData: map[string]interface{}{
"name": "clear",
"mount_accessor": githubAccessor,
"custom_metadata": map[string]string{
"foo": "bar",
},
},
updateData: map[string]interface{}{
"name": "clear",
"mount_accessor": githubAccessor,
"custom_metadata": map[string]string{},
},
},
}

if resp.Data["name"] != "updatedaliasname" {
t.Fatalf("failed to update alias information; \n response data: %#v\n", resp.Data)
}
if !reflect.DeepEqual(resp.Data["custom_metadata"], customMetadata) {
t.Fatalf("failed to update alias information; \n response data: %#v\n", resp.Data)
handleRequest := func(t *testing.T, req *logical.Request) *logical.Response {
t.Helper()
resp, err := is.HandleRequest(ctx, req)
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("err:%v resp:%#v", err, resp)
}
return resp
}

respDefaults := map[string]interface{}{
"custom_metadata": map[string]string{},
}

checkValues := func(t *testing.T, reqData, respData map[string]interface{}) {
t.Helper()
expected := make(map[string]interface{})
for k := range reqData {
expected[k] = reqData[k]
}

for k := range respDefaults {
if _, ok := expected[k]; !ok {
expected[k] = respDefaults[k]
}
}

actual := make(map[string]interface{}, len(expected))
for k := range expected {
actual[k] = respData[k]
}

if !reflect.DeepEqual(expected, actual) {
t.Fatalf("expected data %#v, actual %#v", expected, actual)
}
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
resp := handleRequest(t, &logical.Request{
Operation: logical.UpdateOperation,
Path: "entity-alias",
Data: tt.createData,
})

readReq := &logical.Request{
Operation: logical.ReadOperation,
Path: "entity-alias/id/" + resp.Data["id"].(string),
}

// check create
resp = handleRequest(t, readReq)
checkValues(t, tt.createData, resp.Data)

// check update
resp = handleRequest(t, &logical.Request{
Operation: logical.UpdateOperation,
Path: readReq.Path,
Data: tt.updateData,
})
resp = handleRequest(t, readReq)
checkValues(t, tt.updateData, resp.Data)
})
}
}

Expand Down Expand Up @@ -489,7 +607,6 @@ func TestIdentityStore_AliasMove_DuplicateAccessor(t *testing.T) {
if resp == nil || !resp.IsError() {
t.Fatalf("expected an error as alias on the github accessor exists for testentity1")
}

}

// Test that the alias cannot be changed to a mount for which
Expand Down Expand Up @@ -563,7 +680,6 @@ func TestIdentityStore_AliasUpdate_DuplicateAccessor(t *testing.T) {
if resp == nil || !resp.IsError() {
t.Fatalf("expected an error as an alias on the github accessor already exists for testentity")
}

}

// Test that alias creation fails if an alias for the specified mount
Expand Down