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 90206a28630 Fix some atomicity issues in removeCodeGrant (#3358)
90206a28630 is described below
commit 90206a2863073b39a1090183bb7ea8d52e581f94
Author: Colm O hEigeartaigh <[email protected]>
AuthorDate: Tue Aug 11 11:00:35 2026 +0100
Fix some atomicity issues in removeCodeGrant (#3358)
---
.../oauth2/grants/code/JPACMTCodeDataProvider.java | 30 +++++++++++++++++++++-
.../oauth2/grants/code/JPACodeDataProvider.java | 8 +++++-
.../grants/code/JPACodeDataProviderTest.java | 25 ++++++++++++++++++
3 files changed, 61 insertions(+), 2 deletions(-)
diff --git
a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/JPACMTCodeDataProvider.java
b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/JPACMTCodeDataProvider.java
index 094e64c39a6..18101769825 100644
---
a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/JPACMTCodeDataProvider.java
+++
b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/JPACMTCodeDataProvider.java
@@ -19,12 +19,14 @@
package org.apache.cxf.rs.security.oauth2.grants.code;
import java.util.Collections;
+import java.util.HashMap;
import java.util.Map;
import jakarta.persistence.EntityManager;
import jakarta.persistence.EntityTransaction;
import jakarta.persistence.LockModeType;
import org.apache.cxf.rs.security.oauth2.common.ServerAccessToken;
+import org.apache.cxf.rs.security.oauth2.provider.OAuthServiceException;
import org.apache.cxf.rs.security.oauth2.tokens.refresh.RefreshToken;
/**
@@ -79,6 +81,7 @@ import
org.apache.cxf.rs.security.oauth2.tokens.refresh.RefreshToken;
public class JPACMTCodeDataProvider extends JPACodeDataProvider {
private static final int DEFAULT_PESSIMISTIC_LOCK_TIMEOUT = 10000;
+ private static final String JPA_LOCK_TIMEOUT_HINT =
"jakarta.persistence.lock.timeout";
private int pessimisticLockTimeout = DEFAULT_PESSIMISTIC_LOCK_TIMEOUT;
private boolean useJpaLockForExistingRefreshToken = true;
@@ -129,12 +132,37 @@ public class JPACMTCodeDataProvider extends
JPACodeDataProvider {
return super.updateExistingRefreshToken(rt, at);
}
+ @Override
+ public ServerAuthorizationCodeGrant removeCodeGrant(final String code)
throws OAuthServiceException {
+ return executeInTransaction(em -> findAndRemoveWithLock(code, em));
+ }
+
+ @Override
+ protected ServerAuthorizationCodeGrant removeCodeGrant(String code,
EntityManager em,
+ LockModeType
lockModeType) throws OAuthServiceException {
+ return findAndRemoveWithLock(code, em);
+ }
+
+ private ServerAuthorizationCodeGrant findAndRemoveWithLock(String code,
EntityManager em) {
+ final Map<String, Object> options = new HashMap<>();
+ options.put(JPA_LOCK_TIMEOUT_HINT, pessimisticLockTimeout);
+ ServerAuthorizationCodeGrant grant =
+ em.find(ServerAuthorizationCodeGrant.class, code,
LockModeType.PESSIMISTIC_WRITE, options);
+ try {
+ if (grant != null) {
+ em.remove(grant);
+ }
+ } catch (jakarta.persistence.EntityNotFoundException e) {
+ }
+ return grant;
+ }
+
protected void lockRefreshTokenForUpdate(final RefreshToken refreshToken) {
try {
execute(em -> {
final Map<String, Object> options;
if (pessimisticLockTimeout > 0) {
- options =
Collections.singletonMap("jakarta.persistence.lock.timeout",
pessimisticLockTimeout);
+ options = Collections.singletonMap(JPA_LOCK_TIMEOUT_HINT,
pessimisticLockTimeout);
} else {
options = Collections.emptyMap();
}
diff --git
a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/JPACodeDataProvider.java
b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/JPACodeDataProvider.java
index 082cacf2bbc..54188e0b3be 100644
---
a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/JPACodeDataProvider.java
+++
b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/grants/code/JPACodeDataProvider.java
@@ -22,6 +22,7 @@ import java.util.List;
import jakarta.persistence.EntityManager;
import jakarta.persistence.EntityNotFoundException;
+import jakarta.persistence.LockModeType;
import jakarta.persistence.TypedQuery;
import org.apache.cxf.rs.security.oauth2.common.Client;
import org.apache.cxf.rs.security.oauth2.common.UserSubject;
@@ -96,7 +97,12 @@ public class JPACodeDataProvider extends
JPAOAuthDataProvider implements Authori
}
private ServerAuthorizationCodeGrant removeCodeGrant(String code,
EntityManager em) throws OAuthServiceException {
- ServerAuthorizationCodeGrant grant =
em.find(ServerAuthorizationCodeGrant.class, code);
+ return removeCodeGrant(code, em, LockModeType.PESSIMISTIC_WRITE);
+ }
+
+ protected ServerAuthorizationCodeGrant removeCodeGrant(String code,
EntityManager em,
+ LockModeType
lockModeType) throws OAuthServiceException {
+ ServerAuthorizationCodeGrant grant =
em.find(ServerAuthorizationCodeGrant.class, code, lockModeType);
try {
if (grant != null) {
em.remove(grant);
diff --git
a/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/grants/code/JPACodeDataProviderTest.java
b/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/grants/code/JPACodeDataProviderTest.java
index b75e78099df..011c4221373 100644
---
a/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/grants/code/JPACodeDataProviderTest.java
+++
b/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/grants/code/JPACodeDataProviderTest.java
@@ -34,6 +34,7 @@ import org.junit.Test;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
import static org.junit.Assert.fail;
public class JPACodeDataProviderTest {
@@ -102,6 +103,30 @@ public class JPACodeDataProviderTest {
assertEquals(0, grants.size());
}
+ @Test
+ public void testRemoveCodeGrantTwiceReturnsNullOnSecondCall() {
+ Client c = addClient("222", "alice");
+
+ AuthorizationCodeRegistration atr = new
AuthorizationCodeRegistration();
+ atr.setClient(c);
+ atr.setApprovedScope(Collections.singletonList("a"));
+ atr.setSubject(c.getResourceOwnerSubject());
+
+ try {
+ ServerAuthorizationCodeGrant grant =
getProvider().createCodeGrant(atr);
+
+ ServerAuthorizationCodeGrant first =
getProvider().removeCodeGrant(grant.getCode());
+ assertNotNull(first);
+ assertEquals(grant.getCode(), first.getCode());
+
+ // second remove must not return the grant (single-use enforcement)
+ ServerAuthorizationCodeGrant second =
getProvider().removeCodeGrant(grant.getCode());
+ assertNull(second);
+ } finally {
+ getProvider().removeClient(c.getClientId());
+ }
+ }
+
@Test
public void testResetClient() {
Client c = addClient("111", "bob");
