diff --git a/vault/logical_system_custom_messages.go b/vault/logical_system_custom_messages.go index ec504b339d..cfa800d3ff 100644 --- a/vault/logical_system_custom_messages.go +++ b/vault/logical_system_custom_messages.go @@ -428,25 +428,9 @@ func (b *SystemBackend) handleCreateCustomMessages(ctx context.Context, req *log return logical.ErrorResponse(err.Error()), nil } - if len(linkMap) > 1 { - return logical.ErrorResponse("invalid number of elements in link parameter value; only a single element can be provided"), nil - } - - var link *uicustommessages.MessageLink - if linkMap != nil { - link = &uicustommessages.MessageLink{} - - for k, v := range linkMap { - href, ok := v.(string) - if !ok { - return logical.ErrorResponse(fmt.Sprintf("invalid url for %q key in link parameter value", k)), nil - } - - link.Title = k - link.Href = href - - break - } + link, resp := validateLinkMap(linkMap) + if resp != nil { + return resp, nil } options, err := parameterValidateMap("options", d) @@ -490,6 +474,50 @@ func (b *SystemBackend) handleCreateCustomMessages(ctx context.Context, req *log }, nil } +// validateLinkMap takes care of detecting either incomplete or invalid link +// parameter value. +// An invalid link parameter value is one where there's either +// an empty string for the title key or the href value or the href value not +// being a string. A linkMap that is invalid, results in only a logical.Response +// containing an error response being returned. +// An incomplete link parameter value is one where the linkMap is either nil or +// empty, where the title key is an empty string and the href value is an empty +// string. A linkMap that is incomplete, results in neither a MessageLink nor a +// Response being returned. +// If the linkMap is neither invalid nor incomplete, a MessageLink is returned. +func validateLinkMap(linkMap map[string]any) (*uicustommessages.MessageLink, *logical.Response) { + if len(linkMap) > 1 { + return nil, logical.ErrorResponse("invalid number of elements in link parameter value; only a single element can be provided") + } + + for k, v := range linkMap { + href, ok := v.(string) + + switch { + case !ok: + // href value is not a string, so return an error + return nil, logical.ErrorResponse(fmt.Sprintf("invalid url for %q key in link parameter value", k)) + case len(k) == 0 && len(href) > 0: + // no title key, but there's an href value, so return an error + return nil, logical.ErrorResponse("missing title key in link parameter value") + case len(k) > 0 && len(href) == 0: + // no href value, but there's a title key, so return an error + return nil, logical.ErrorResponse(fmt.Sprintf("missing url for %q key in link parameter value", k)) + case len(k) != 0 && len(href) != 0: + // when title key and href value are not empty, return a MessageLink + // pointer + return &uicustommessages.MessageLink{ + Title: k, + Href: href, + }, nil + } + + } + + // no title key and no href value, treat it as if no link was specified + return nil, nil +} + // handleReadCustomMessage is the operation callback for the READ operation of // the custom messages endpoint. func (b *SystemBackend) handleReadCustomMessage(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { @@ -572,25 +600,9 @@ func (b *SystemBackend) handleUpdateCustomMessage(ctx context.Context, req *logi return logical.ErrorResponse(err.Error()), nil } - if len(linkMap) > 1 { - return logical.ErrorResponse("invalid number of elements in link parameter value; only a single element can be provided"), nil - } - - var link *uicustommessages.MessageLink - if linkMap != nil { - link = &uicustommessages.MessageLink{} - - for k, v := range linkMap { - href, ok := v.(string) - if !ok { - return logical.ErrorResponse("invalid url for %q key link parameter value", k), nil - } - - link.Title = k - link.Href = href - - break - } + link, resp := validateLinkMap(linkMap) + if resp != nil { + return resp, nil } options, err := parameterValidateMap("options", d) diff --git a/vault/logical_system_custom_messages_test.go b/vault/logical_system_custom_messages_test.go index f7ce85643e..b003955cf1 100644 --- a/vault/logical_system_custom_messages_test.go +++ b/vault/logical_system_custom_messages_test.go @@ -437,6 +437,160 @@ func TestHandleCreateCustomMessage(t *testing.T) { assert.Contains(t, resp.Data, "error") } +// TestHandleCreateAndUpdateCustomMessageLinkProperty verifies some particular +// cases in the handleCreateCustomMessage method surrounding the link property +// of the custom message. Because the link property is modeled as a JSON object, +// the possibility of an empty or partially constructed object is provided, so +// this test function verifies that each of these cases is appropriately +// handled. +func TestHandleCreateAndUpdateCustomMessageLinkProperty(t *testing.T) { + testBackend := &SystemBackend{ + Core: &Core{ + customMessageManager: uicustommessages.NewManager(&logical.InmemStorage{}), + }, + } + + fieldData := &framework.FieldData{ + Schema: map[string]*framework.FieldSchema{ + "title": { + Type: framework.TypeString, + Required: true, + }, + "message": { + Type: framework.TypeString, + Required: true, + }, + "authenticated": { + Type: framework.TypeBool, + Default: true, + }, + "type": { + Type: framework.TypeString, + Default: "banner", + }, + "start_time": { + Type: framework.TypeTime, + Required: true, + }, + "end_time": { + Type: framework.TypeTime, + }, + "link": { + Type: framework.TypeMap, + }, + "options": { + Type: framework.TypeMap, + }, + }, + Raw: map[string]any{ + "title": "The title", + "message": "VGhlIG1lc3NhZ2UK", + "start_time": "2024-01-01T00:00:00.000Z", + }, + } + + for _, testcase := range []struct { + name string + linkField any + responseErrorExpected bool + }{ + // First group of testcases, setup an incomplete link parameter, + // which result in a CustomMessage with a Link field set to nil + { + name: "nil-link", + linkField: nil, + }, + { + name: "empty-link-map", + linkField: map[string]any{}, + }, + { + name: "empty-key-and-value-link-map", + linkField: map[string]any{ + "": "", + }, + }, + { + name: "empty-value-link-map", + linkField: map[string]any{ + "Click me": "", + }, + }, + { + name: "empty-key-nil-value-link-map", + linkField: map[string]any{ + "": nil, + }, + }, + // Second group of testcases, setup an invalid link parameter, + // which result in an error being returned and the message not being + // created or updated. + { + name: "nil-value-link-map", + linkField: map[string]any{ + "Click me": nil, + }, + responseErrorExpected: true, + }, + { + name: "invalid-value-link-map", + linkField: map[string]any{ + "Click me": []int{}, + }, + responseErrorExpected: true, + }, + { + name: "empty-key-link-map", + linkField: map[string]any{ + "": "https://www.example.org", + }, + responseErrorExpected: true, + }, + } { + t.Run(testcase.name, func(t *testing.T) { + delete(fieldData.Raw, "link") + + // Create a custom message with no link to be used to test the + // handleUpdateCustomMessage case. + resp, err := testBackend.handleCreateCustomMessages(namespace.ContextWithNamespace(context.Background(), namespace.RootNamespace), &logical.Request{}, fieldData) + require.NoError(t, err) + require.Contains(t, resp.Data, "id") + updatableMessageId := resp.Data["id"] + + // Update the fieldData to include the testcase + fieldData.Raw["link"] = testcase.linkField + fieldData.Schema["id"] = &framework.FieldSchema{ + Type: framework.TypeString, + Required: true, + } + fieldData.Raw["id"] = updatableMessageId + + // Test the Update operation + resp, err = testBackend.handleUpdateCustomMessage(namespace.ContextWithNamespace(context.Background(), namespace.RootNamespace), &logical.Request{}, fieldData) + assert.NoError(t, err, "UpdateCustomMessage") + + if testcase.responseErrorExpected { + assert.Contains(t, resp.Data, "error", "UpdateCustomMessage") + } else { + assert.Nil(t, resp.Data["link"], "UpdateCustomMessage") + } + + // Reset for testing the Create operation + delete(fieldData.Schema, "id") + delete(fieldData.Raw, "id") + + resp, err = testBackend.handleCreateCustomMessages(namespace.ContextWithNamespace(context.Background(), namespace.RootNamespace), &logical.Request{}, fieldData) + assert.NoError(t, err, "CreateCustomMessage") + + if testcase.responseErrorExpected { + assert.Contains(t, resp.Data, "error", "CreateCustomMessage") + } else { + assert.Nil(t, resp.Data["link"], "CreateCustomMessage") + } + }) + } +} + // TestHandleReadCustomMessage verifies the proper functioning of the // (*SystemBackend).handleReadCustomMessage method. The tests focus on missing // or invalid request parameters as well as reading existing and non-