rootvector2 commented on code in PR #3652:
URL: https://github.com/apache/cxf/pull/3652#discussion_r4420563172
##########
rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/provider/AbstractOAuthDataProviderTest.java:
##########
@@ -403,6 +407,26 @@ public void
testCrossClientRefreshTokenRejectedWhenRecycleDisabled() {
assertNotNull(getProvider().getAccessToken(atA.getTokenKey()));
}
+ @Test
+ public void testRefreshedAccessTokenKeepsCertificateBinding() {
Review Comment:
Each subclass builds a new provider in its `@Before`, so the mock
`MessageContext` doesn't outlive the test. Client `101` is already removed by
`tearDownClients()` in the base `@After`, same as the other tests here that use
it.
##########
rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/provider/AbstractOAuthDataProviderTest.java:
##########
@@ -403,6 +407,26 @@ public void
testCrossClientRefreshTokenRejectedWhenRecycleDisabled() {
assertNotNull(getProvider().getAccessToken(atA.getTokenKey()));
}
+ @Test
+ public void testRefreshedAccessTokenKeepsCertificateBinding() {
+ MessageContext mc = mock(MessageContext.class);
+
when(mc.get(JoseConstants.HEADER_X509_THUMBPRINT_SHA256)).thenReturn("thumbprint");
+ getProvider().setMessageContext(mc);
+
+ Client c = addClient("101", "bob");
+
+ AccessTokenRegistration atr = new AccessTokenRegistration();
+ atr.setClient(c);
+ atr.setApprovedScope(Arrays.asList("a", "refreshToken"));
+ atr.setSubject(c.getResourceOwnerSubject());
+
+ ServerAccessToken at = getProvider().createAccessToken(atr);
+ assertEquals("thumbprint",
at.getExtraProperties().get(JoseConstants.HEADER_X509_THUMBPRINT_SHA256));
+
+ ServerAccessToken at2 = getProvider().refreshAccessToken(c,
at.getRefreshToken(), Collections.emptyList());
+ assertEquals("thumbprint",
at2.getExtraProperties().get(JoseConstants.HEADER_X509_THUMBPRINT_SHA256));
+ }
+
protected Client addClient(String clientId, String userLogin) {
Review Comment:
A refresh without a cert gives an unbound token, which is exactly what
`doCreateAccessToken(AccessTokenRegistration)` does for the initial grant.
`AbstractTokenService` only puts `x5t#S256` in the context when the client
authenticates with its certificate, so the binding follows the cert on the
current request. A `tls_client_auth` client can't refresh without it, and a
confidential client's stolen refresh token is still useless without the client
credentials. Rejecting an uncertified refresh of a bound token (the RFC 8705
section 4 public client case) needs the thumbprint persisted on `RefreshToken`,
which I'd keep as a separate change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
