This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cxf.git
The following commit(s) were added to refs/heads/main by this push:
new 7019b0fc05f Make sure that users can't revoke other users tokens
(#3099)
7019b0fc05f is described below
commit 7019b0fc05f9c203ae2fd6c07bedb7e65a2f0374
Author: Colm O hEigeartaigh <[email protected]>
AuthorDate: Mon May 11 16:36:18 2026 +0100
Make sure that users can't revoke other users tokens (#3099)
---
.../oauth2/provider/AbstractOAuthDataProvider.java | 31 +++++++++++-
.../oauth2/provider/OAuthDataProvider.java | 21 ++++++++
.../oauth2/services/TokenRevocationService.java | 35 +++++++++++++-
.../provider/AbstractOAuthDataProviderTest.java | 56 ++++++++++++++++++++++
4 files changed, 140 insertions(+), 3 deletions(-)
diff --git
a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/provider/AbstractOAuthDataProvider.java
b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/provider/AbstractOAuthDataProvider.java
index a78f0d9df87..24b1a1e1477 100644
---
a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/provider/AbstractOAuthDataProvider.java
+++
b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/provider/AbstractOAuthDataProvider.java
@@ -252,14 +252,20 @@ public abstract class AbstractOAuthDataProvider
implements OAuthDataProvider, Cl
@Override
public void revokeToken(Client client, String tokenKey, String
tokenTypeHint) throws OAuthServiceException {
+ revokeToken(client, null, tokenKey, tokenTypeHint);
+ }
+
+ @Override
+ public void revokeToken(Client client, UserSubject callerSubject, String
tokenKey, String tokenTypeHint)
+ throws OAuthServiceException {
ServerAccessToken accessToken = null;
if (!OAuthConstants.REFRESH_TOKEN.equals(tokenTypeHint)) {
- accessToken = revokeAccessToken(client, tokenKey);
+ accessToken = revokeAccessToken(client, callerSubject, tokenKey);
}
if (accessToken != null) {
handleLinkedRefreshToken(client, accessToken);
} else if (!OAuthConstants.ACCESS_TOKEN.equals(tokenTypeHint)) {
- RefreshToken currentRefreshToken = revokeRefreshToken(client,
tokenKey);
+ RefreshToken currentRefreshToken = revokeRefreshToken(client,
callerSubject, tokenKey);
revokeAccessTokens(client, currentRefreshToken);
}
}
@@ -568,21 +574,42 @@ public abstract class AbstractOAuthDataProvider
implements OAuthDataProvider, Cl
}
protected ServerAccessToken revokeAccessToken(Client client, String
accessTokenKey) {
+ return revokeAccessToken(client, null, accessTokenKey);
+ }
+
+ protected ServerAccessToken revokeAccessToken(Client client, UserSubject
callerSubject, String accessTokenKey) {
ServerAccessToken at = getAccessToken(accessTokenKey);
if (at != null) {
if (!at.getClient().getClientId().equals(client.getClientId())) {
throw new OAuthServiceException(OAuthConstants.INVALID_GRANT);
}
+ // IDOR guard: if the revocation request carries a resource owner
identity,
+ // it must match the subject bound to the token. This prevents
User A from
+ // revoking User B's token when both share the same OAuth2 client.
+ if (callerSubject != null && at.getSubject() != null
+ &&
!callerSubject.getLogin().equals(at.getSubject().getLogin())) {
+ throw new OAuthServiceException(OAuthConstants.INVALID_GRANT);
+ }
doRevokeAccessToken(at);
}
return at;
}
+
protected RefreshToken revokeRefreshToken(Client client, String
refreshTokenKey) {
+ return revokeRefreshToken(client, null, refreshTokenKey);
+ }
+
+ protected RefreshToken revokeRefreshToken(Client client, UserSubject
callerSubject, String refreshTokenKey) {
RefreshToken refreshToken = getRefreshToken(refreshTokenKey);
if (refreshToken != null) {
if
(!refreshToken.getClient().getClientId().equals(client.getClientId())) {
throw new OAuthServiceException(OAuthConstants.INVALID_GRANT);
}
+ // IDOR guard: same subject check as for access tokens.
+ if (callerSubject != null && refreshToken.getSubject() != null
+ &&
!callerSubject.getLogin().equals(refreshToken.getSubject().getLogin())) {
+ throw new OAuthServiceException(OAuthConstants.INVALID_GRANT);
+ }
doRevokeRefreshToken(refreshToken);
}
return refreshToken;
diff --git
a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/provider/OAuthDataProvider.java
b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/provider/OAuthDataProvider.java
index 8178edd7f60..a01e28383c8 100644
---
a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/provider/OAuthDataProvider.java
+++
b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/provider/OAuthDataProvider.java
@@ -113,6 +113,27 @@ public interface OAuthDataProvider {
*/
void revokeToken(Client client, String tokenId, String tokenTypeHint)
throws OAuthServiceException;
+ /**
+ * Revokes a refresh or access token, verifying that the authenticated
resource owner's
+ * identity ({@code callerSubject}) matches the subject bound to the token.
+ * This prevents one user from revoking another user's token when multiple
resource
+ * owners share the same OAuth2 client (IDOR guard).
+ * <p>
+ * {@code callerSubject} may be {@code null} for machine-to-machine
(client credentials)
+ * flows where no end-user principal is present; in that case only the
client-level
+ * check is applied.
+ *
+ * @param client the authenticated client
+ * @param callerSubject the authenticated resource owner, or {@code null}
for M2M flows
+ * @param tokenId token identifier
+ * @param tokenTypeHint can be access_token or refresh_token or null
+ * @throws OAuthServiceException
+ */
+ default void revokeToken(Client client, UserSubject callerSubject, String
tokenId, String tokenTypeHint)
+ throws OAuthServiceException {
+ revokeToken(client, tokenId, tokenTypeHint);
+ }
+
/**
* Converts the requested scopes to the list of permissions.
* The scopes are extracted from OAuth2 'scope' property which
diff --git
a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/services/TokenRevocationService.java
b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/services/TokenRevocationService.java
index 98698f30762..967410e117e 100644
---
a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/services/TokenRevocationService.java
+++
b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/services/TokenRevocationService.java
@@ -25,7 +25,9 @@ import jakarta.ws.rs.Path;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.core.MultivaluedMap;
import jakarta.ws.rs.core.Response;
+import jakarta.ws.rs.core.SecurityContext;
import org.apache.cxf.rs.security.oauth2.common.Client;
+import org.apache.cxf.rs.security.oauth2.common.UserSubject;
import org.apache.cxf.rs.security.oauth2.provider.OAuthServiceException;
import org.apache.cxf.rs.security.oauth2.utils.OAuthConstants;
@@ -58,7 +60,7 @@ public class TokenRevocationService extends
AbstractTokenService {
return
createErrorResponseFromErrorCode(OAuthConstants.UNSUPPORTED_TOKEN_TYPE);
}
try {
- getDataProvider().revokeToken(client, token, tokenTypeHint);
+ getDataProvider().revokeToken(client,
getCallerUserSubject(client), token, tokenTypeHint);
} catch (OAuthServiceException ex) {
// Spec: The authorization server responds with HTTP status code
200 if the
// token has been revoked successfully or if the client submitted
an
@@ -66,4 +68,35 @@ public class TokenRevocationService extends
AbstractTokenService {
}
return Response.ok().build();
}
+
+ /**
+ * Resolves the resource owner identity from the JAX-RS security context.
+ * <p>
+ * In standard OAuth2 flows the revocation endpoint is authenticated by the
+ * <em>client</em> (HTTP Basic Auth, client_secret_post, mTLS, …), so
+ * {@code SecurityContext.getUserPrincipal()} normally carries the
client's own
+ * name, not an end-user identity. We must not mistake the client
principal for
+ * a resource-owner subject, or we would block every legitimate
client-initiated
+ * revocation (e.g. the canonical authorization-code flow where the client
+ * revokes its own token on logout).
+ * <p>
+ * A non-null {@link UserSubject} is returned only when a distinct end-user
+ * principal is present — i.e. the principal name differs from the
authenticated
+ * client's {@code clientId}. In that case the data provider will verify
that
+ * the token belongs to that end-user (IDOR guard). When the caller is the
+ * client itself, {@code null} is returned and only the client-level check
applies.
+ */
+ private UserSubject getCallerUserSubject(Client client) {
+ SecurityContext sc = getMessageContext().getSecurityContext();
+ java.security.Principal principal = sc.getUserPrincipal();
+ if (principal == null || principal.getName() == null) {
+ return null;
+ }
+ // Principal name matches the client id → this is the client
authenticating
+ // at the token endpoint, not an independent resource-owner identity.
+ if (principal.getName().equals(client.getClientId())) {
+ return null;
+ }
+ return new UserSubject(principal.getName());
+ }
}
\ No newline at end of file
diff --git
a/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/provider/AbstractOAuthDataProviderTest.java
b/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/provider/AbstractOAuthDataProviderTest.java
index 439b9d69a93..0d160d352cd 100644
---
a/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/provider/AbstractOAuthDataProviderTest.java
+++
b/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/provider/AbstractOAuthDataProviderTest.java
@@ -47,6 +47,7 @@ import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
abstract class AbstractOAuthDataProviderTest {
private static KeyPair keyPair;
@@ -256,6 +257,59 @@ abstract class AbstractOAuthDataProviderTest {
assertNotEquals(at.getSubject().getId(), at2.getSubject().getId());
}
+ @Test
+ public void testRevokeAccessTokenIdorAcrossUsersOfSharedClient() {
+ // A multi-user shared client (e.g. a public mobile app with a known
client_id).
+ // Its resourceOwnerSubject is intentionally null — many users share
this client.
+ Client sharedClient = new Client();
+
sharedClient.setRedirectUris(Collections.singletonList("http://client/redirect";));
+ sharedClient.setClientId("103");
+ sharedClient.setClientSecret("secret");
+ getProvider().setClient(sharedClient);
+
+ // Alice obtains an access token via the shared client.
+ AccessTokenRegistration aliceReg = new AccessTokenRegistration();
+ aliceReg.setClient(sharedClient);
+ aliceReg.setApprovedScope(Collections.singletonList("a"));
+ aliceReg.setSubject(new UserSubject("alice"));
+ ServerAccessToken aliceToken =
getProvider().createAccessToken(aliceReg);
+ assertNotNull(getProvider().getAccessToken(aliceToken.getTokenKey()));
+
+ // Bob obtains a separate access token via the same shared client.
+ AccessTokenRegistration bobReg = new AccessTokenRegistration();
+ bobReg.setClient(sharedClient);
+ bobReg.setApprovedScope(Collections.singletonList("a"));
+ bobReg.setSubject(new UserSubject("bob"));
+ ServerAccessToken bobToken = getProvider().createAccessToken(bobReg);
+ assertNotNull(getProvider().getAccessToken(bobToken.getTokenKey()));
+
+ // IDOR attempt: Bob (authenticated only as the shared client) submits
Alice's token
+ // key to the revocation endpoint. The subject-aware revokeToken
overload is called
+ // with Bob's identity, which must not match Alice's subject.
+ // The data provider must reject this with OAuthServiceException; the
HTTP layer
+ // (TokenRevocationService) swallows it per RFC 7009 and returns 200
anyway.
+ try {
+ getProvider().revokeToken(sharedClient, new UserSubject("bob"),
aliceToken.getTokenKey(),
+ OAuthConstants.ACCESS_TOKEN);
+ // Reaching here means the cross-user revocation was NOT blocked —
fail the test.
+ fail(
+ "IDOR: revokeToken should have thrown OAuthServiceException
when Bob tried to "
+ + "revoke Alice's token, but it succeeded silently"
+ );
+ } catch (OAuthServiceException expected) {
+ // Expected: the subject mismatch guard correctly rejected the
request.
+ }
+
+ // Alice's token must still be intact after the blocked revocation
attempt.
+ assertNotNull(
+ "Alice's token must survive a cross-user revocation attempt",
+ getProvider().getAccessToken(aliceToken.getTokenKey())
+ );
+
+ // Bob's own token must remain unaffected regardless.
+ assertNotNull(getProvider().getAccessToken(bobToken.getTokenKey()));
+ }
+
@Test
public void testAddGetDeleteRefreshToken() {
Client c = addClient("101", "bob");
@@ -330,6 +384,8 @@ abstract class AbstractOAuthDataProviderTest {
protected void tearDownClients() {
tearDownClient("101");
+ tearDownClient("102");
+ tearDownClient("103");
tearDownClient("12345");
tearDownClient("56789");
tearDownClient("09876");
