Add missing Query: true metadata to API definitions (#21949)

* Add missing `Query: true` metadata to API definitions

Also improve the documentation comment for `Query` to guide people better how they should be setting `Query` in the future.

Endpoints affected:
- auth/approle/role/{role_name}/secret-id/destroy
- auth/approle/role/{role_name}/secret-id-accessor/destroy
- auth/token/lookup
- auth/token/lookup-self
- sys/internal/specs/openapi
- sys/wrapping/lookup
- identity/oidc/provider/{name}/authorize

There are also endpoints in the `aws` and `gcp` secrets engines which need the same treatment in their own PRs.

When working on the `auth/token/lookup-self` path, I discovered that it
had a parameter which was completely pointless - it was even documented
as unused. It only existed because the `auth/token/lookup-self` code
path was implemented by bodging the current token into the request data
and passing control to the `auth/token/lookup` handler directly -
instead of just factoring out the common code to a reusable function -
so I fixed that whilst I was there.

Note that two of the affected endpoints currently have one form of their
OpenAPI operation ID set to something mentioning "with-parameters":
- identity/oidc/provider/{name}/authorize
- sys/internal/specs/openapi

These operation IDs should be changed, as they perpetuate
a misunderstanding - both read (GET) and update (POST/PUT) forms of
these APIs are **equally** capable of being used with parameters.

* I failed to spot that the aws plugin is in-repo! Update that too.

* Remove code cleanup changes from this PR

* Wording and wrapping adjustment as requested.

builtin/credential/approle/path_role.go

@@ -1087,6 +1087,7 @@ Overrides secret_id_ttl role option when supplied. May not be longer than role's
 				"secret_id": {
 					Type:        framework.TypeString,
 					Description: "SecretID attached to the role.",
+					Query:       true,
 				},
 			},
 			Operations: map[logical.Operation]framework.OperationHandler{
@@ -1195,6 +1196,7 @@ Overrides secret_id_ttl role option when supplied. May not be longer than role's
 				"secret_id_accessor": {
 					Type:        framework.TypeString,
 					Description: "Accessor of the SecretID",
+					Query:       true,
 				},
 			},
 			Operations: map[logical.Operation]framework.OperationHandler{

builtin/logical/aws/path_user.go

@@ -35,15 +35,18 @@ func pathUser(b *backend) *framework.Path {
 			"role_arn": {
 				Type:        framework.TypeString,
 				Description: "ARN of role to assume when credential_type is " + assumedRoleCred,
+				Query:       true,
 			},
 			"ttl": {
 				Type:        framework.TypeDurationSecond,
 				Description: "Lifetime of the returned credentials in seconds",
 				Default:     3600,
+				Query:       true,
 			},
 			"role_session_name": {
 				Type:        framework.TypeString,
 				Description: "Session name to use when assuming role. Max chars: 64",
+				Query:       true,
 			},
 		},
 

sdk/framework/backend.go

@@ -748,16 +748,35 @@ type FieldSchema struct {
 	Required   bool
 	Deprecated bool
 
-	// Query indicates this field will be sent as a query parameter:
+	// Query indicates this field will be expected as a query parameter as part
+	// of ReadOperation, ListOperation or DeleteOperation requests:
 	//
 	//   /v1/foo/bar?some_param=some_value
 	//
-	// It doesn't affect handling of the value, but may be used for documentation.
+	// The field will still be expected as a request body parameter for
+	// CreateOperation or UpdateOperation requests!
+	//
+	// To put that another way, you should set Query for any non-path parameter
+	// you want to use in a read/list/delete operation.  While setting the Query
+	// field to `true` is not required in such cases (Vault will expose the
+	// query parameters to you via req.Data regardless), it is highly
+	// recommended to do so in order to improve the quality of the generated
+	// OpenAPI documentation (as well as any code generation based on it), which
+	// will otherwise incorrectly omit the parameter.
+	//
+	// The reason for this design is historical: back at the start of 2018,
+	// query parameters were not mapped to fields at all, and it was implicit
+	// that all non-path fields were exclusively for the use of create/update
+	// operations.  Since then, support for query parameters has gradually been
+	// extended to read, delete and list operations - and now this declarative
+	// metadata is needed, so that the OpenAPI generator can know which
+	// parameters are actually referred to, from within the code of
+	// read/delete/list operation handler functions.
 	Query bool
 
 	// AllowedValues is an optional list of permitted values for this field.
 	// This constraint is not (yet) enforced by the framework, but the list is
-	// output as part of OpenAPI generation and may effect documentation and
+	// output as part of OpenAPI generation and may affect documentation and
 	// dynamic UI generation.
 	AllowedValues []interface{}
 

vault/identity_store_oidc_provider.go

@@ -477,42 +477,51 @@ func oidcProviderPaths(i *IdentityStore) []*framework.Path {
 					Type:        framework.TypeString,
 					Description: "The ID of the requesting client.",
 					Required:    true,
+					Query:       true,
 				},
 				"scope": {
 					Type:        framework.TypeString,
 					Description: "A space-delimited, case-sensitive list of scopes to be requested. The 'openid' scope is required.",
 					Required:    true,
+					Query:       true,
 				},
 				"redirect_uri": {
 					Type:        framework.TypeString,
 					Description: "The redirection URI to which the response will be sent.",
 					Required:    true,
+					Query:       true,
 				},
 				"response_type": {
 					Type:        framework.TypeString,
 					Description: "The OIDC authentication flow to be used. The following response types are supported: 'code'",
 					Required:    true,
+					Query:       true,
 				},
 				"state": {
 					Type:        framework.TypeString,
 					Description: "The value used to maintain state between the authentication request and client.",
+					Query:       true,
 				},
 				"nonce": {
 					Type:        framework.TypeString,
 					Description: "The value that will be returned in the ID token nonce claim after a token exchange.",
+					Query:       true,
 				},
 				"max_age": {
 					Type:        framework.TypeInt,
 					Description: "The allowable elapsed time in seconds since the last time the end-user was actively authenticated.",
+					Query:       true,
 				},
 				"code_challenge": {
 					Type:        framework.TypeString,
 					Description: "The code challenge derived from the code verifier.",
+					Query:       true,
 				},
 				"code_challenge_method": {
 					Type:        framework.TypeString,
 					Description: "The method that was used to derive the code challenge. The following methods are supported: 'S256', 'plain'. Defaults to 'plain'.",
 					Default:     codeChallengeMethodPlain,
+					Query:       true,
 				},
 			},
 			Operations: map[logical.Operation]framework.OperationHandler{

vault/logical_system_paths.go

@@ -2225,6 +2225,7 @@ func (b *SystemBackend) internalPaths() []*framework.Path {
 				"context": {
 					Type:        framework.TypeString,
 					Description: "Context string appended to every operationId",
+					Query:       true,
 				},
 				"generic_mount_paths": {
 					Type:        framework.TypeBool,
@@ -4081,7 +4082,8 @@ func (b *SystemBackend) wrappingPaths() []*framework.Path {
 
 			Fields: map[string]*framework.FieldSchema{
 				"token": {
-					Type: framework.TypeString,
+					Type:  framework.TypeString,
+					Query: true,
 				},
 			},
 

vault/token_store.go

@@ -310,7 +310,8 @@ func (ts *TokenStore) paths() []*framework.Path {
 			Fields: map[string]*framework.FieldSchema{
 				"token": {
 					Type:        framework.TypeString,
-					Description: "Token to lookup (POST request body)",
+					Description: "Token to lookup",
+					Query:       true,
 				},
 			},