framework.FieldSchema's Required field should be enforced

[grahamc]

Describe the bug

Making a FieldSchema be Required does not appear to do anything. For example, creating a path like this (with quite a lot of other code removed):

func (b *backend) pathToken() *framework.Path {
	return &framework.Path{
		Pattern: "token",
		Fields: map[string]*framework.FieldSchema{
			"required_integer": {
				Type:        framework.TypeInt,
				Description: "my integer",
				Required:    true,
			},
		},
	}
}

If I make a request to this path and don't provide the required_integer field the request is accepted.

To Reproduce

(I think I wrote down the reproduction behavior above, but if it isn't clear I'd be happy to be more linear here.)

Expected behavior

I expected the framework to reject the request with an error saying the required_integer field is required. Instead I had to manually implement this:

	foo = d.Get("required_integer").(int)
	if foo == 0 {
		return logical.ErrorResponse("required_integer is a required parameter"), nil
	}

Environment:

• Vault Server Version (retrieve with vault status): 1.9.2
• Vault CLI Version (retrieve with vault version): 1.9.2
• Server Operating System/Architecture: NixOS 21.05, x86

Vault server configuration file(s):

log_level = "Debug"
plugin_directory = "./plugin-dir"

Additional context

It looks like the Required field is only used for OpenAPI compatibility, which is where it seems to originate from: #5546

I did notice there are very few uses of Required: true in Vault's codebase.

Here is one example of a field being marked Required:

vault/vault/identity_store_oidc_provider.go

Lines 398 to 402 in 2d14aa7

	"state": {
	Type: framework.TypeString,
	Description: "The value used to maintain state between the authentication request and client.",
	Required: true,
	},

but later on when that field is used, we can see they're specifically guarding against it:

vault/vault/identity_store_oidc_provider.go

Lines 1447 to 1452 in 2d14aa7

	func (i *IdentityStore) pathOIDCAuthorize(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) {
	// Validate the state
	state := d.Get("state").(string)
	if state == "" {
	return authResponse("", "", ErrAuthInvalidRequest, "state parameter is required")
	}

I must say that as a developer of Vault plugins it would be nice if Required was automatically enforced.

[hghaf099]

@grahamc Thank you for filing this issue. Actually, the Required field is used only by the openapi, and it is not used by the framework. I have updated the docs indicating the intention behind using that. I am going to close this issue, but please feel free to open a new one or reopen this one for further discussions.

[kalafut]

@grahamc to add: there is some interest if leveraging this sort of declaration in a functional way, as you mentioned, but we don't have concrete plans yet. The code comment is meant to clarify the current use/scope of those members.