identity/oidc: change the state parameter to optional (#16599)

* identity/oidc: change the state parameter to optional

* adds changelog

* update docs

changelog/16599.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+identity/oidc: Change the `state` parameter of the Authorization Endpoint to optional.
+```

vault/identity_store_oidc_provider.go

@@ -448,7 +448,6 @@ func oidcProviderPaths(i *IdentityStore) []*framework.Path {
 				"state": {
 					Type:        framework.TypeString,
 					Description: "The value used to maintain state between the authentication request and client.",
-					Required:    true,
 				},
 				"nonce": {
 					Type:        framework.TypeString,
@@ -1609,11 +1608,7 @@ func (i *IdentityStore) keyIDsReferencedByTargetClientIDs(ctx context.Context, s
 }
 
 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")
-	}
 
 	// Get the namespace
 	ns, err := namespace.FromContext(ctx)

vault/identity_store_oidc_provider_test.go

@@ -55,7 +55,7 @@ func TestOIDC_Path_OIDC_Cross_Provider_Exchange(t *testing.T) {
 	require.NoError(t, err)
 	require.NoError(t, json.Unmarshal(resp.Data["http_raw_body"].([]byte), &authRes))
 	require.Regexp(t, authCodeRegex, authRes.Code)
-	require.NotEmpty(t, authRes.State)
+	require.Equal(t, req.Data["state"], authRes.State)
 
 	// Assert that the authorization code cannot be exchanged using the second provider
 	var tokenRes struct {
@@ -477,7 +477,7 @@ func TestOIDC_Path_OIDC_Token(t *testing.T) {
 			}
 			require.NoError(t, json.Unmarshal(resp.Data["http_raw_body"].([]byte), &authRes))
 			require.Regexp(t, authCodeRegex, authRes.Code)
-			require.NotEmpty(t, authRes.State)
+			require.Equal(t, tt.args.authorizeReq.Data["state"], authRes.State)
 
 			// Update the assignment
 			tt.args.assignmentReq.Operation = logical.UpdateOperation
@@ -740,21 +740,6 @@ func TestOIDC_Path_OIDC_Authorize(t *testing.T) {
 			},
 			wantErr: ErrAuthInvalidRedirectURI,
 		},
-		{
-			name: "invalid authorize request with missing state",
-			args: args{
-				entityID:      entityID,
-				clientReq:     testClientReq(s),
-				providerReq:   testProviderReq(s, clientID),
-				assignmentReq: testAssignmentReq(s, entityID, groupID),
-				authorizeReq: func() *logical.Request {
-					req := testAuthorizeReq(s, clientID)
-					req.Data["state"] = ""
-					return req
-				}(),
-			},
-			wantErr: ErrAuthInvalidRequest,
-		},
 		{
 			name: "invalid authorize request with request parameter provided",
 			args: args{
@@ -910,6 +895,20 @@ func TestOIDC_Path_OIDC_Authorize(t *testing.T) {
 				}(),
 			},
 		},
+		{
+			name: "valid authorize request with empty state",
+			args: args{
+				entityID:      entityID,
+				clientReq:     testClientReq(s),
+				providerReq:   testProviderReq(s, clientID),
+				assignmentReq: testAssignmentReq(s, entityID, groupID),
+				authorizeReq: func() *logical.Request {
+					req := testAuthorizeReq(s, clientID)
+					req.Data["state"] = ""
+					return req
+				}(),
+			},
+		},
 		{
 			name: "active re-authentication required with token creation time exceeding max_age requirement",
 			args: args{
@@ -1143,7 +1142,7 @@ func TestOIDC_Path_OIDC_Authorize(t *testing.T) {
 			expectSuccess(t, resp, err)
 			require.Equal(t, http.StatusOK, resp.Data[logical.HTTPStatusCode].(int))
 			require.Regexp(t, authCodeRegex, authRes.Code)
-			require.NotEmpty(t, authRes.State)
+			require.Equal(t, tt.args.authorizeReq.Data["state"], authRes.State)
 			require.Empty(t, authRes.Error)
 			require.Empty(t, authRes.ErrorDescription)
 		})

website/content/api-docs/secret/identity/oidc-provider.mdx

@@ -672,7 +672,7 @@ to be used for the [Authorization Code Flow](https://openid.net/specs/openid-con
 
 - `redirect_uri` `(string: <required>)` - The redirection URI to which the response will be sent.
 
-- `state` `(string: <required>)` - A value used to maintain state between the authentication request and client.
+- `state` `(string: <optional>)` - A value used to maintain state between the authentication request and client.
 
 - `nonce` `(string: <optional>)` - A value that is returned in the ID token nonce claim. It is used to mitigate replay attacks, so we *strongly encourage* providing this optional parameter.
 

website/content/docs/concepts/oidc-provider.mdx

@@ -199,7 +199,7 @@ Each provider offers an unauthenticated endpoint that provides the public portio
 
 ### Authorization Endpoint
 
-Each provider offers an authenticated [authorization endpoint](https://openid.net/specs/openid-connect-core-1_0.html#AuthorizationEndpoint). The authorization endpoint for each provider is added to Vault's [default policy](/docs/concepts/policies#default-policy) using the `identity/oidc/provider/+/authorize` path. The endpoint incorporates all required [authentication request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest) parameters as input. Additionally, the `state` parameter is required.
+Each provider offers an authenticated [authorization endpoint](https://openid.net/specs/openid-connect-core-1_0.html#AuthorizationEndpoint). The authorization endpoint for each provider is added to Vault's [default policy](/docs/concepts/policies#default-policy) using the `identity/oidc/provider/+/authorize` path. The endpoint incorporates all required [authentication request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest) parameters as input.
 
 The endpoint [validates](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequestValidation) client requests and ensures that all required parameters are present and valid. The `redirect_uri` of the request is validated against the client's `redirect_uris`. The requesting Vault entity will be validated against the client's `assignments`. An appropriate [error code](https://openid.net/specs/openid-connect-core-1_0.html#AuthError) is returned for invalid requests.