Add support for OAuth 2.0 Attestation-based client authentication by tdiesler · Pull Request #47962 · keycloak/keycloak

tdiesler · 2026-04-10T16:27:35Z

mposolda

@tdiesler Thanks! Added some inline comments. Can you check please?

TODOs are fine as we can address them hopefully in the follow-up PRs.

mposolda · 2026-04-16T14:29:29Z

+            execution.setParentFlow(clients.getId());
+            execution.setRequirement(AuthenticationExecutionModel.Requirement.ALTERNATIVE);
+            execution.setAuthenticator("attestation-based");
+            execution.setPriority(5);


Suggested change

execution.setPriority(5);

execution.setPriority(60);

Is it possible to use priority 60, so that this new authenticator is added after existing ones (also note other authenticators that they increase by 10)

We can unfortunately not do that at the moment. AFAIU, the first (alternative) client authenticator that succeeds determines the outcome. e.g. if client secret passes, abca is not checked

ABCA is the stronger authorization mechanism - this is checked by OpenID conformance.

I have more details on how this plays with public clients and direct access grants if you care to know.

Yes, ok. Maybe we can discuss that on one of next meetings? At least for this PR it should be OK since the feature is experimental.

mposolda · 2026-04-16T14:34:55Z

+                // when AttestationBasedClientAuthenticator is processed
+                //
+                String abcaAuthType = AttestationBasedClientAuthenticator.PROVIDER_ID;
+                if (client.isPublicClient() && factory.getId().equals(abcaAuthType) && Profile.isFeatureEnabled(Profile.Feature.CLIENT_AUTH_ABCA)) {


I wonder if this is really needed? I assume that if client is authenticating with ABCA, it should be always confidential client? Is it possible to make sure that only confidential clients authenticate with ABCA?

Public clients are not supposed to authenticate with any client authentication methods. They usually use just client_id parameter to "identify" themselves in the request andfor this purpose, we just use expectedClientAuthType set to KeycloakModelUtils.getDefaultClientAuthenticatorType() .

There was indeed a change in the spec from “mechanism for public clients to authenticate” to “general client authentication mechanism”. The draft decoupled itself from OAuth’s client-type distinction entirely - it effectively introduces a third model: attested clients (orthogonal to public/confidential).

This shift aligns with the underlying idea: attestation provides hardware/software-backed proof which can substitute:

secrets (confidential clients)

or lack thereof (public clients)

Without this change, the AttestationBasedClientAuthenticator is not called for confidential clients. I added a test thereof.

Note, an invalid abca request or missing attester pub key currently fails in the AttestationBasedClientAuthenticator but is silently ignored because some other ClientAuthenticator succeeds - this might be a TODO

Yes, more details would be good. Maybe we can discuss on one of our next meetings?

Just to doublecheck: Is it this specification draft, which you talk about https://datatracker.ietf.org/doc/html/draft-ietf-oauth-attestation-based-client-auth-08 ?

This change is OK in this PR, but we possibly need to figure this as a follow-up to avoid directly referencing the concrete authenticator from ClientAuthenticationFlow .

It is actually https://www.ietf.org/archive/id/draft-ietf-oauth-attestation-based-client-auth-07.html
which I have bookmarked. I could however not find a reference to it in https://openid.net/specs/openid4vc-high-assurance-interoperability-profile-1_0-final.html @thomasdarimont could you perhaps confirm the exact abca spec version that we ought to use?

mposolda · 2026-04-16T14:42:35Z

-                if (!normalize(new URI(t.getHttpUri())).equals(normalize(uri)))
-                    throw new DPoPVerificationException(t, "DPoP HTTP URL mismatch");
+                URI dpopUri = new URI(t.getHttpUri());
+                if (!normalize(dpopUri).equals(normalize(uri))) {


I think this change should not be needed and may have security implications.

For the setup of Keycloak behind reverse proxy, you may take a look on how to configure hostname. See the "Server" guides from https://www.keycloak.org/guides . Especially https://www.keycloak.org/server/hostname or https://www.keycloak.org/server/reverseproxy might be probably useful.

Also not sure if you are aware, but there is environment in the Keycloak org, which is maintained mostly by @tnorimat and which contains some support for running conformance tests: https://github.com/keycloak/keycloak-oauth-sig .

For example see here how Keycloak is configured (there are other config files as well): https://github.com/keycloak/keycloak-oauth-sig/blob/main/conformance-tests-env/docker-compose.yml#L159

I already set the hostname

start-dev --hostname=https://keycloak.nessustech.io:8443 --proxy-headers=xforwarded --features=oid4vc-vci,oid4vc-vci-preauth-code,client-auth-abca --bootstrap-admin-username=admin --bootstrap-admin-password=***** --db=postgres --db-url=jdbc:postgresql://localhost:32543/keycloak --db-username=postgres --db-password=***** --log-level=org.keycloak.protocol.oid4vc:debug,org.keycloak.services:debug,org.keycloak.events:debug,org.keycloak.authentication:debug,root:info

Can we perhaps investigate this separately?

Yes, it would be good to investigate this separately. But will be good if you can remove this change from this PR as it is not directly related to OAuth 2.0 attestation-based client auth, but seems rather as a setup issue.

DPoP is supported feature and should not be updated to possibly insecure way due your (maybe incorrect) setup of hostname.

Do you have a chance to try https://github.com/keycloak/keycloak-oauth-sig and compare with your setup?

mposolda · 2026-04-16T14:46:22Z

    public List<ProviderConfigProperty> getConfigProperties() {
-        return List.of();
+        ProviderConfigProperty jwks = new ProviderConfigProperty();
+        jwks.setName(OAUTH_CLIENT_ATTESTATION_CONFIG_ATTESTER_JWKS);


I assume this is temporary (as we briefly discussed on the meeting yesterday) and should probably identity provider mechanisms similarly like for example CLIENT_AUTH_FEDERATED is using (See for example FederatedJWTClientAuthenticator and related classes for the inspiration).

But it is probably fine this way for the initial PRand we can update later.

IngridPuppet

No new concerns in addition to Marek's. All my previous concerns were addressed.

mposolda

@tdiesler Thanks for the latest updates! LGTM if you remove the DPoP related change.

Other issues can be possibly addressed as a follow-ups.

mposolda · 2026-04-17T15:14:13Z

+            execution.setParentFlow(clients.getId());
+            execution.setRequirement(AuthenticationExecutionModel.Requirement.ALTERNATIVE);
+            execution.setAuthenticator("attestation-based");
+            execution.setPriority(5);


Yes, ok. Maybe we can discuss that on one of next meetings? At least for this PR it should be OK since the feature is experimental.

mposolda · 2026-04-17T15:16:18Z

+                // when AttestationBasedClientAuthenticator is processed
+                //
+                String abcaAuthType = AttestationBasedClientAuthenticator.PROVIDER_ID;
+                if (client.isPublicClient() && factory.getId().equals(abcaAuthType) && Profile.isFeatureEnabled(Profile.Feature.CLIENT_AUTH_ABCA)) {


Yes, more details would be good. Maybe we can discuss on one of our next meetings?

Just to doublecheck: Is it this specification draft, which you talk about https://datatracker.ietf.org/doc/html/draft-ietf-oauth-attestation-based-client-auth-08 ?

This change is OK in this PR, but we possibly need to figure this as a follow-up to avoid directly referencing the concrete authenticator from ClientAuthenticationFlow .

mposolda · 2026-04-17T15:21:34Z

-                if (!normalize(new URI(t.getHttpUri())).equals(normalize(uri)))
-                    throw new DPoPVerificationException(t, "DPoP HTTP URL mismatch");
+                URI dpopUri = new URI(t.getHttpUri());
+                if (!normalize(dpopUri).equals(normalize(uri))) {


Yes, it would be good to investigate this separately. But will be good if you can remove this change from this PR as it is not directly related to OAuth 2.0 attestation-based client auth, but seems rather as a setup issue.

DPoP is supported feature and should not be updated to possibly insecure way due your (maybe incorrect) setup of hostname.

Do you have a chance to try https://github.com/keycloak/keycloak-oauth-sig and compare with your setup?

Signed-off-by: Thomas Diesler <[email protected]>

tdiesler · 2026-04-17T17:51:19Z

Reverse unrelated DPoP changes

mposolda

@tdiesler Thanks!

@IngridPuppet Thank you as well for the review!

FYI. I've created dedicated issue #48265 for this PR and added it as subtask to #43136 (parent issue having other ABCA related issues).

Also will create some follow-up GH issues for additional things discussed in the PR review of this PR.

…keycloak#47962) closes keycloak#48265 Signed-off-by: Thomas Diesler <[email protected]>

tdiesler marked this pull request as draft April 10, 2026 16:27

tdiesler mentioned this pull request Apr 10, 2026

[OID4VCI-HAIP] Pass oid4vci-1_0-issuer-happy-flow #47405

Closed

tdiesler force-pushed the ghi43136 branch 2 times, most recently from a8154d7 to acbefee Compare April 11, 2026 05:58

tdiesler marked this pull request as ready for review April 11, 2026 06:05

tdiesler requested a review from a team as a code owner April 11, 2026 06:05

mposolda self-assigned this Apr 14, 2026

tdiesler force-pushed the ghi43136 branch 2 times, most recently from 16f3181 to 2b22fa0 Compare April 16, 2026 04:28

tdiesler mentioned this pull request Apr 16, 2026

[OID4VCI-HAIP] Pass oid4vci-1_0-issuer-fail-invalid-client-attestation-pop-signature (blocked) #47605

Closed

tdiesler force-pushed the ghi43136 branch from 2b22fa0 to 2bd918c Compare April 16, 2026 06:39

keycloak-github-bot Bot added the flaky-test label Apr 16, 2026

keycloak-github-bot Bot mentioned this pull request Apr 16, 2026

Flaky test: org.keycloak.testsuite.forms.AuthenticatorSubflowsTest2#testSubflow2 #47477

Closed

tdiesler force-pushed the ghi43136 branch 7 times, most recently from 2f7ef7d to 4d1a665 Compare April 16, 2026 11:15

github-advanced-security AI found potential problems Apr 16, 2026

View reviewed changes

Comment thread tests/base/src/test/java/org/keycloak/tests/oid4vc/OID4VCProofTestUtils.java Fixed

tdiesler force-pushed the ghi43136 branch 3 times, most recently from 6374b0c to 3ce4b68 Compare April 16, 2026 14:44

mposolda reviewed Apr 16, 2026

View reviewed changes

tdiesler force-pushed the ghi43136 branch 3 times, most recently from 89512ae to 2e4d430 Compare April 16, 2026 15:08

tdiesler mentioned this pull request Apr 16, 2026

[OID4VCI-HAIP] Pass oid4vci-1_0-issuer-fail-missing-proof (blocked) #47638

Closed

IngridPuppet approved these changes Apr 17, 2026

View reviewed changes

tdiesler force-pushed the ghi43136 branch 5 times, most recently from 872349d to 55d81b5 Compare April 17, 2026 09:59

mposolda reviewed Apr 17, 2026

View reviewed changes

tdiesler mentioned this pull request Apr 17, 2026

[OID4VCI-HAIP] Pass oid4vci-1_0-issuer-happy-flow (encrypted) #47674

Merged

tdiesler force-pushed the ghi43136 branch from 55d81b5 to badb77f Compare April 17, 2026 15:42

tdiesler added 7 commits April 17, 2026 19:48

Add support for OAuth 2.0 Attestation-based client authentication

34acfc0

Signed-off-by: Thomas Diesler <[email protected]>

-- address review comments from @IngridPuppet

c8a8a26

Signed-off-by: Thomas Diesler <[email protected]>

-- add OIDCMockClientAttester and ECDSAProvider

6c46eb6

Signed-off-by: Thomas Diesler <[email protected]>

-- test client attestation happy flow

04adf4c

Signed-off-by: Thomas Diesler <[email protected]>

-- address review comments from @mposolda (thanks)

f5be153

Signed-off-by: Thomas Diesler <[email protected]>

-- revisit key algorithm handling

0fb3861

Signed-off-by: Thomas Diesler <[email protected]>

-- reverse unrelated DPoP changes

0687153

Signed-off-by: Thomas Diesler <[email protected]>

tdiesler force-pushed the ghi43136 branch from e770703 to 0687153 Compare April 17, 2026 17:48

mposolda approved these changes Apr 20, 2026

View reviewed changes

mposolda merged commit e9a30f1 into keycloak:main Apr 20, 2026
85 checks passed

This was referenced Apr 20, 2026

[ABCA] Remove reference of ABCA from ClientAuthenticationFlow class #48267

Open

[ABCA] Doublecheck if client attestation authenticator needs to be first in the default client authentication flow #48268

Open

tdiesler deleted the ghi43136 branch April 20, 2026 10:58

This was referenced Apr 20, 2026

[ABCA] Doublecheck how to configure valid keys for client attestation authentication #48269

Open

[ABCA] Review TODOs in AttestationBasedClientAuthenticator #48270

Open

msdaly200 pushed a commit to msdaly200/keycloak that referenced this pull request Apr 22, 2026

Initial support for OAuth 2.0 Attestation-based client authentication (…

2b21ca6

…keycloak#47962) closes keycloak#48265 Signed-off-by: Thomas Diesler <[email protected]>

Conversation

tdiesler commented Apr 10, 2026 • edited by mposolda Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

mposolda left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tdiesler Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tdiesler Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tdiesler Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

IngridPuppet left a comment

Choose a reason for hiding this comment

Uh oh!

mposolda left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tdiesler commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mposolda left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tdiesler commented Apr 10, 2026 •

edited by mposolda

Loading

tdiesler Apr 17, 2026 •

edited

Loading

tdiesler Apr 17, 2026 •

edited

Loading

tdiesler Apr 17, 2026 •

edited

Loading

tdiesler commented Apr 17, 2026 •

edited

Loading