Skip to content

Allow specifying clientId together with clientAssertion/clientAssertionType #713

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.

Sign up for GitHub

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

Jump to bottom
Open
geovie opened this issue Jan 22, 2025 · 2 comments · May be fixed by #714
Open

Allow specifying clientId together with clientAssertion/clientAssertionType #713

geovie opened this issue Jan 22, 2025 · 2 comments · May be fixed by #714
Labels
enhancement help wanted

Comments

@geovie
Copy link

geovie commented Jan 22, 2025

Currently it's only possible to either specify the clientId or use clientAssertion/clientAssertionType.

However some provider require the use of clientId together with clientAssertion/clientAssertionType
e.g. Microsoft requires a clientId if the certificate credential is used see: https://learn.microsoft.com/en-us/entra/identity-platform/v2-oauth2-auth-code-flow#request-an-access-token-with-a-certificate-credential
and I think also keycloak requires clientId always to be present:
https://github.com/keycloak/keycloak/blob/5387aef0fa727ea5cae4816f682ec72798fabaa4/services/src/main/java/org/keycloak/protocol/oidc/endpoints/request/AuthorizationEndpointRequestParserProcessor.java#L119-L123

The code responsible for it is in:

vertx-auth/vertx-auth-oauth2/src/main/java/io/vertx/ext/auth/oauth2/impl/OAuth2API.java

Lines 184 to 194 in be97446

String clientId = config.getClientId();
if (clientId == null) {
if (config.getClientAssertionType() != null) {
form
.put("client_assertion_type", config.getClientAssertionType());
}
if (config.getClientAssertion() != null) {
form
.put("client_assertion", config.getClientAssertion());
}
}

I guess this could be relaxed to only check if clientSecret is null and allow both clientId and clientAssertion/clientAssertionType.

Happy to provide a PR for this.
@geovie
Copy link
Author

geovie commented Jan 23, 2025

Also reading the spec I think client_id should always be present and it's also included in the private_key_jwt (client_assertion/client_assertion_type) example of the spec:

Image

@tsegismont
Copy link
Contributor

Hi @geovie , please go ahead and send a PR

@pendula95 pendula95 linked a pull request Mar 9, 2025 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement help wanted
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Included client_assertion_type and client_assertion when other auth m… pendula95/vertx-auth
2 participants
@tsegismont @geovie