Skip to content

Commit

Permalink
Deny access to UserInfo endpoint if client no longer allowed by provi…
Browse files Browse the repository at this point in the history
…der (#12949)
  • Loading branch information
austingebauer committed Oct 28, 2021
1 parent fd04ff3 commit 19b53e5
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 1 deletion.
27 changes: 26 additions & 1 deletion vault/external_tests/identity/oidc_provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,22 @@ func TestOIDC_Auth_Code_Flow_CAP_Client(t *testing.T) {
require.NoError(t, err)
groupID := resp.Data["id"].(string)

// Create a policy that allows updating the provider
err = active.Sys().PutPolicy("test-policy", `
path "identity/oidc/provider/test-provider" {
capabilities = ["update"]
}
`)
require.NoError(t, err)

// Enable userpass auth and create a user
err = active.Sys().EnableAuthWithOptions("userpass", &api.EnableAuthOptions{
Type: "userpass",
})
require.NoError(t, err)
_, err = active.Logical().Write("auth/userpass/users/end-user", map[string]interface{}{
"password": testPassword,
"password": testPassword,
"token_policies": "test-policy",
})
require.NoError(t, err)

Expand Down Expand Up @@ -277,6 +286,11 @@ func TestOIDC_Auth_Code_Flow_CAP_Client(t *testing.T) {
}
client.SetToken(clientToken)

// Update allowed client IDs before the authentication flow
_, err = client.Logical().Write("identity/oidc/provider/test-provider", map[string]interface{}{
"allowed_client_ids": []string{clientID},
})

// Create the client-side OIDC request state
oidcRequest, err := oidc.NewRequest(10*time.Minute, testRedirectURI, tt.args.options...)
require.NoError(t, err)
Expand Down Expand Up @@ -339,6 +353,17 @@ func TestOIDC_Auth_Code_Flow_CAP_Client(t *testing.T) {
require.True(t, ok)
require.EqualValues(t, expectedVal, actualVal)
}

// Assert that the access token is no longer able to obtain user info
// after removing the client from the provider's allowed client ids
_, err = client.Logical().Write("identity/oidc/provider/test-provider", map[string]interface{}{
"allowed_client_ids": []string{},
})
require.NoError(t, err)
err = p.UserInfo(context.Background(), accessToken, subject, &allClaims)
require.Error(t, err)
require.Equal(t, `Provider.UserInfo: provider UserInfo request failed: 403 Forbidden: {"error":"access_denied","error_description":"client is not authorized to use the provider"}`,
err.Error())
})
}
}
Expand Down
6 changes: 6 additions & 0 deletions vault/identity_store_oidc_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -1988,6 +1988,12 @@ func (i *IdentityStore) pathOIDCUserInfo(ctx context.Context, req *logical.Reque
return userInfoResponse(nil, ErrUserInfoAccessDenied, "identity entity not authorized by client assignment")
}

// Validate that the client is authorized to use the provider
if !strutil.StrListContains(provider.AllowedClientIDs, "*") &&
!strutil.StrListContains(provider.AllowedClientIDs, clientID) {
return userInfoResponse(nil, ErrUserInfoAccessDenied, "client is not authorized to use the provider")
}

claims := map[string]interface{}{
// The subject claim must always be in the response
"sub": entity.ID,
Expand Down

0 comments on commit 19b53e5

Please sign in to comment.