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.

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

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

Merged
merged 3 commits into from
Aug 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog/16599.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
identity/oidc: Change the `state` parameter of the Authorization Endpoint to optional.
```
5 changes: 0 additions & 5 deletions vault/identity_store_oidc_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
35 changes: 17 additions & 18 deletions vault/identity_store_oidc_provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
2 changes: 1 addition & 1 deletion website/content/docs/concepts/oidc-provider.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down