refactor cnf verification of refresh tokens by graziang · Pull Request #50927 · keycloak/keycloak

graziang · 2026-07-15T14:18:14Z

Extracts the cnf binding logic from verifyRefreshToken into a dedicated verifyConfirmationBinding method.
Adds client-config guards so that tokens issued before enabling a binding type (MTLS, DPoP, ABCA)
are rejected, before the PR only MTLS was rejected. Also rejects refresh tokens with a cnf claim with no known binding type.

Copilot

Pull request overview

Consolidates refresh-token cnf verification in TokenManager.

Changes:

Validates mTLS certificate bindings from x5t#S256.
Retains DPoP/ABCA validation and rejects unsupported confirmations.

+            AccessToken.Confirmation cnf = refreshToken.getConfirmation();
+            if (cnf != null) {
+                // MTLS certificate binding (x5t#S256)
+                if (cnf.getCertThumbprint() != null) {


+                if (cnf.getCertThumbprint() == null && cnf.getKeyThumbprint() == null) {
+                    throw new OAuthErrorException(OAuthErrorException.INVALID_GRANT, "Invalid refresh token confirmation");


Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

+            AccessToken.Confirmation cnf = refreshToken.getConfirmation();
+            if (cnf != null) {


+                if (cnf.getCertThumbprint() != null) {
+                    if (!MtlsHoKTokenUtil.verifyTokenBindingWithClientCertificate(refreshToken, request, session)) {


Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

graziang · 2026-07-16T12:31:58Z

+        // DPoP required for public client but token has no DPoP binding (RFC 9449 §5)
+        if (clientConfig.isUseDPoP() && client.isPublicClient()) {
+            boolean dpopBound = cnf != null && cnf.getKeyThumbprint() != null && (cnf.getJktType() == null || DPOP_JKT_TYPE.equals(cnf.getJktType()));
+            if (!dpopBound) {
+                throw new OAuthErrorException(OAuthErrorException.INVALID_GRANT, "DPoP proof key binding is required");


DPoP and ABCA are alternatives to MTLS HoK, not complements.

graziang · 2026-07-16T12:32:04Z

+        // ABCA attestation present but token has no attestation binding
+        if (abcaResult != null) {
+            boolean abcaBound = cnf != null && cnf.getKeyThumbprint() != null && ABCA_JKT_TYPE.equals(cnf.getJktType());
+            if (!abcaBound) {
+                throw new OAuthErrorException(OAuthErrorException.INVALID_GRANT, "Attestation-based key binding is required for this refresh token");


DPoP and ABCA are alternatives to MTLS HoK, not complements.

Closes keycloak#50459 Signed-off-by: Giuseppe Graziano <g.graziano94@gmail.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

graziang · 2026-07-16T12:32:28Z

+        // ABCA attestation present but token has no attestation binding
+        if (abcaResult != null) {
+            boolean abcaBound = cnf != null && cnf.getKeyThumbprint() != null && ABCA_JKT_TYPE.equals(cnf.getJktType());
+            if (!abcaBound) {
+                throw new OAuthErrorException(OAuthErrorException.INVALID_GRANT, "Attestation-based key binding is required for this refresh token");


DPoP and ABCA are alternatives to MTLS HoK, not complements.

lhanusov

@graziang nice, thanks for finising it :-), please take a look on my comments and consider applying them.

lhanusov · 2026-07-16T13:53:25Z

+        EventAssertion.assertSuccess(events.poll()).type(EventType.LOGIN);
+
+        String code = oauth.parseLoginResponse().getCode();


I propose to switch the position of these two methods, it may be a potential flaky test candidate if done otherwise.

Suggested change

EventAssertion.assertSuccess(events.poll()).type(EventType.LOGIN);

String code = oauth.parseLoginResponse().getCode();

String code = oauth.parseLoginResponse().getCode();

EventAssertion.assertSuccess(events.poll()).type(EventType.LOGIN);

lhanusov · 2026-07-16T13:55:36Z

+        EventAssertion.assertSuccess(events.poll()).type(EventType.LOGIN);
+
+        String code = oauth.parseLoginResponse().getCode();


please, swap the position, discussed above

lhanusov · 2026-07-16T13:56:23Z

+        assertEquals(200, response.getStatusCode());
+        refreshTokenString = response.getRefreshToken();
+
+        events.poll();


if you need to skip 1 event or more you can do

Suggested change

events.poll();

events.skip(1);

lhanusov · 2026-07-16T14:03:18Z

+
+    @Test
+    public void refreshTokenRejectedAfterEnablingCNFCheck() throws Exception {
+        ClientRepresentation clientRep = testAppClient.toRepresentation();


please, consider removing testAppClient and replacing it with oauth.clientResource()
potentially doing it in the whole class, but it's out of scope of this PR, since it's refactoring

Closes keycloak#50459 Signed-off-by: Giuseppe Graziano <g.graziano94@gmail.com>

graziang · 2026-07-16T14:21:36Z

@graziang nice, thanks for finising it :-), please take a look on my comments and consider applying them.

@lhanusov thanks! applied all the suggestions!

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

+        ClientRepresentation clientRep = oauth.clientResource().toRepresentation();
+        clientRep.setPublicClient(true);
+        oauth.clientResource().update(clientRep);


+        // ABCA attestation present but token has no attestation binding
+        if (abcaResult != null) {
+            boolean abcaBound = cnf != null && cnf.getKeyThumbprint() != null && ABCA_JKT_TYPE.equals(cnf.getJktType());
+            if (!abcaBound) {
+                throw new OAuthErrorException(OAuthErrorException.INVALID_GRANT, "Attestation-based key binding is required for this refresh token");


Copilot AI review requested due to automatic review settings July 15, 2026 14:18

graziang requested a review from a team as a code owner July 15, 2026 14:18

Copilot started reviewing on behalf of graziang July 15, 2026 14:18 View session

Copilot AI reviewed Jul 15, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 15, 2026 14:24

graziang force-pushed the issue-50459 branch from 058cf04 to 28ae21e Compare July 15, 2026 14:24

Copilot started reviewing on behalf of graziang July 15, 2026 14:25 View session

Copilot AI reviewed Jul 15, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 15, 2026 14:38

graziang force-pushed the issue-50459 branch from 28ae21e to df44198 Compare July 15, 2026 14:38

Copilot started reviewing on behalf of graziang July 15, 2026 14:39 View session

Copilot AI reviewed Jul 15, 2026

View reviewed changes

Comment thread services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java Outdated

Comment thread services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java Outdated

graziang marked this pull request as draft July 15, 2026 16:02

graziang force-pushed the issue-50459 branch from df44198 to cf07ac8 Compare July 16, 2026 10:04

graziang marked this pull request as ready for review July 16, 2026 11:41

Copilot AI review requested due to automatic review settings July 16, 2026 11:41

Copilot started reviewing on behalf of graziang July 16, 2026 11:42 View session

Copilot AI reviewed Jul 16, 2026

View reviewed changes

refactor cnf verification of refresh tokens

e7ba68b

Closes keycloak#50459 Signed-off-by: Giuseppe Graziano <g.graziano94@gmail.com>

Copilot AI review requested due to automatic review settings July 16, 2026 12:06

graziang force-pushed the issue-50459 branch from cf07ac8 to e7ba68b Compare July 16, 2026 12:06

Copilot started reviewing on behalf of graziang July 16, 2026 12:06 View session

Copilot AI reviewed Jul 16, 2026

View reviewed changes

lhanusov suggested changes Jul 16, 2026

View reviewed changes

RefreshTokenTest fixes

dc7ddab

Closes keycloak#50459 Signed-off-by: Giuseppe Graziano <g.graziano94@gmail.com>

Copilot AI review requested due to automatic review settings July 16, 2026 14:21

Copilot started reviewing on behalf of graziang July 16, 2026 14:29 View session

Copilot AI reviewed Jul 16, 2026

View reviewed changes

lhanusov approved these changes Jul 17, 2026

View reviewed changes

pskopek merged commit bd3c4d0 into keycloak:main Jul 17, 2026
91 checks passed

		if (cnf.getCertThumbprint() == null && cnf.getKeyThumbprint() == null) {
		throw new OAuthErrorException(OAuthErrorException.INVALID_GRANT, "Invalid refresh token confirmation");

		AccessToken.Confirmation cnf = refreshToken.getConfirmation();
		if (cnf != null) {

		if (cnf.getCertThumbprint() != null) {
		if (!MtlsHoKTokenUtil.verifyTokenBindingWithClientCertificate(refreshToken, request, session)) {

		EventAssertion.assertSuccess(events.poll()).type(EventType.LOGIN);

		String code = oauth.parseLoginResponse().getCode();

Uh oh!

Conversation

graziang commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

graziang Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

graziang Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

graziang Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lhanusov left a comment • 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!

lhanusov Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

lhanusov Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

lhanusov Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

lhanusov Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

graziang commented Jul 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

graziang commented Jul 15, 2026 •

edited

Loading

lhanusov left a comment •

edited

Loading