Repository: cxf
Updated Branches:
refs/heads/master c4e9c63ba -> 5724e6154
[CXF-7374] Lock RefreshToken entity with pessimistic locking to avoid concurrent updates,
patch from Viacheslav Gagara applied with minor updates, test being disabled for now
Project: http://git-wip-us.apache.org/repos/asf/cxf/repo
Commit: http://git-wip-us.apache.org/repos/asf/cxf/commit/5724e615
Tree: http://git-wip-us.apache.org/repos/asf/cxf/tree/5724e615
Diff: http://git-wip-us.apache.org/repos/asf/cxf/diff/5724e615
Branch: refs/heads/master
Commit: 5724e615409916ce3176d430cb91bae44f3447d9
Parents: c4e9c63
Author: Sergey Beryozkin <sberyozkin@gmail.com>
Authored: Fri May 19 10:50:48 2017 +0100
Committer: Sergey Beryozkin <sberyozkin@gmail.com>
Committed: Fri May 19 10:50:48 2017 +0100
----------------------------------------------------------------------
.../provider/AbstractOAuthDataProvider.java | 12 +++-
.../oauth2/provider/JPAOAuthDataProvider.java | 41 ++++++++++++++
.../code/JPACMTOAuthDataProviderTest.java | 59 +++++++++++++++++++-
.../provider/JPAOAuthDataProviderTest.java | 2 +-
4 files changed, 111 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cxf/blob/5724e615/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/provider/AbstractOAuthDataProvider.java
----------------------------------------------------------------------
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 cc1b623..9df94db 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
@@ -47,6 +47,7 @@ public abstract class AbstractOAuthDataProvider implements OAuthDataProvider,
Cl
private long accessTokenLifetime = 3600L;
private long refreshTokenLifetime; // refresh tokens are eternal by default
private boolean recycleRefreshTokens = true;
+ private Object refreshTokenLock;
private Map<String, OAuthPermission> permissionMap = new HashMap<>();
private MessageContext messageContext;
private List<String> defaultScopes;
@@ -59,7 +60,6 @@ public abstract class AbstractOAuthDataProvider implements OAuthDataProvider,
Cl
private Map<String, String> jwtAccessTokenClaimMap;
private ProviderAuthenticationStrategy authenticationStrategy;
-
protected AbstractOAuthDataProvider() {
}
@@ -344,6 +344,11 @@ public abstract class AbstractOAuthDataProvider implements OAuthDataProvider,
Cl
return null;
}
}
+ protected RefreshToken updateExistingRefreshToken(RefreshToken rt, ServerAccessToken
at) {
+ synchronized (refreshTokenLock) {
+ return updateRefreshToken(rt, at);
+ }
+ }
protected RefreshToken updateRefreshToken(RefreshToken rt, ServerAccessToken at) {
linkAccessTokenToRefreshToken(rt, at);
saveRefreshToken(rt);
@@ -416,8 +421,13 @@ public abstract class AbstractOAuthDataProvider implements OAuthDataProvider,
Cl
public void setRecycleRefreshTokens(boolean recycleRefreshTokens) {
this.recycleRefreshTokens = recycleRefreshTokens;
+ this.refreshTokenLock = recycleRefreshTokens ? null : new Object();
}
+ public boolean isRecycleRefreshTokens() {
+ return this.recycleRefreshTokens;
+ }
+
public void init() {
for (OAuthPermission perm : permissionMap.values()) {
if (defaultScopes != null && defaultScopes.contains(perm.getPermission()))
{
http://git-wip-us.apache.org/repos/asf/cxf/blob/5724e615/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/provider/JPAOAuthDataProvider.java
----------------------------------------------------------------------
diff --git a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/provider/JPAOAuthDataProvider.java
b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/provider/JPAOAuthDataProvider.java
index f59b40e..28c5d2a 100644
--- a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/provider/JPAOAuthDataProvider.java
+++ b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/provider/JPAOAuthDataProvider.java
@@ -19,12 +19,16 @@
package org.apache.cxf.rs.security.oauth2.provider;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
+import java.util.Map;
+
import javax.persistence.EntityManager;
import javax.persistence.EntityManagerFactory;
import javax.persistence.EntityTransaction;
+import javax.persistence.LockModeType;
import javax.persistence.TypedQuery;
import org.apache.cxf.helpers.CastUtils;
@@ -52,6 +56,10 @@ public class JPAOAuthDataProvider extends AbstractOAuthDataProvider {
private static final String CLIENT_QUERY = "SELECT client FROM Client client"
+ " INNER JOIN client.resourceOwnerSubject ros";
+ private static final int DEFAULT_PESSIMISTIC_LOCK_TIMEOUT = 10000;
+
+ private int pessimisticLockTimeout = DEFAULT_PESSIMISTIC_LOCK_TIMEOUT;
+
private EntityManagerFactory entityManagerFactory;
public void setEntityManagerFactory(EntityManagerFactory emf) {
@@ -68,6 +76,35 @@ public class JPAOAuthDataProvider extends AbstractOAuthDataProvider {
});
}
+ protected void lockRefreshTokenForUpdate(final RefreshToken refreshToken) {
+ try {
+ execute(new EntityManagerOperation<Void>() {
+
+ @Override
+ public Void execute(EntityManager em) {
+ Map<String, Object> options = null;
+ if (pessimisticLockTimeout > 0) {
+ options = Collections.singletonMap("javax.persistence.lock.timeout",
pessimisticLockTimeout);
+ } else {
+ options = Collections.emptyMap();
+ }
+ em.refresh(refreshToken, LockModeType.PESSIMISTIC_WRITE, options);
+ return null;
+ }
+ });
+ } catch (IllegalArgumentException e) {
+ // entity is not managed yet. ignore
+ }
+ }
+
+ @Override
+ protected RefreshToken updateExistingRefreshToken(RefreshToken rt, ServerAccessToken
at) {
+ // lock RT for update
+ lockRefreshTokenForUpdate(rt);
+ return super.updateRefreshToken(rt, at);
+ }
+
+
protected <T> T execute(EntityManagerOperation<T> operation) {
EntityManager em = getEntityManager();
T value;
@@ -416,6 +453,10 @@ public class JPAOAuthDataProvider extends AbstractOAuthDataProvider {
em.close();
}
+ public int getPessimisticLockTimeout() {
+ return pessimisticLockTimeout;
+ }
+
public interface EntityManagerOperation<T> {
T execute(EntityManager em);
}
http://git-wip-us.apache.org/repos/asf/cxf/blob/5724e615/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/grants/code/JPACMTOAuthDataProviderTest.java
----------------------------------------------------------------------
diff --git a/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/grants/code/JPACMTOAuthDataProviderTest.java
b/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/grants/code/JPACMTOAuthDataProviderTest.java
index a7245e2..cd93378 100644
--- a/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/grants/code/JPACMTOAuthDataProviderTest.java
+++ b/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/grants/code/JPACMTOAuthDataProviderTest.java
@@ -18,17 +18,29 @@
*/
package org.apache.cxf.rs.security.oauth2.grants.code;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import org.apache.cxf.rs.security.oauth2.common.AccessTokenRegistration;
+import org.apache.cxf.rs.security.oauth2.common.Client;
+import org.apache.cxf.rs.security.oauth2.common.ServerAccessToken;
import org.apache.cxf.rs.security.oauth2.provider.JPAOAuthDataProvider;
import org.apache.cxf.rs.security.oauth2.provider.JPAOAuthDataProviderTest;
+import org.apache.cxf.rs.security.oauth2.tokens.refresh.RefreshToken;
import org.junit.After;
import org.junit.Before;
+import org.junit.Ignore;
+import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.test.annotation.DirtiesContext;
+import org.springframework.test.annotation.DirtiesContext.ClassMode;
import org.springframework.test.context.ActiveProfiles;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
+
/**
* Runs the same tests as JPAOAuthDataProviderTest but within a Spring Managed Transaction.
*
@@ -42,7 +54,7 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
*/
@RunWith(SpringJUnit4ClassRunner.class)
@ContextConfiguration("JPACMTCodeDataProvider.xml")
-@DirtiesContext
+@DirtiesContext(classMode = ClassMode.AFTER_EACH_TEST_METHOD)
@ActiveProfiles("hibernate")
public class JPACMTOAuthDataProviderTest extends JPAOAuthDataProviderTest {
@@ -64,4 +76,49 @@ public class JPACMTOAuthDataProviderTest extends JPAOAuthDataProviderTest
{
@Override
public void tearDown() {
}
+
+ @Test
+ @Ignore
+ public void testRefreshAccessTokenConcurrently() throws Exception {
+ getProvider().setRecycleRefreshTokens(false);
+
+ Client c = addClient("101", "bob");
+
+ AccessTokenRegistration atr = new AccessTokenRegistration();
+ atr.setClient(c);
+ atr.setApprovedScope(Arrays.asList("a", "refreshToken"));
+ atr.setSubject(null);
+ final ServerAccessToken at = getProvider().createAccessToken(atr);
+
+ Runnable task = new Runnable() {
+
+ @Override
+ public void run() {
+ getProvider().refreshAccessToken(c, at.getRefreshToken(), Collections.emptyList());
+ }
+ };
+
+ Thread th1 = new Thread(task);
+ Thread th2 = new Thread(task);
+ Thread th3 = new Thread(task);
+
+ th1.start();
+ th2.start();
+ th3.start();
+
+ th1.join();
+ th2.join();
+ th3.join();
+
+ assertNotNull(getProvider().getAccessToken(at.getTokenKey()));
+ List<RefreshToken> rtl = getProvider().getRefreshTokens(c, null);
+ assertNotNull(rtl);
+ assertEquals(1, rtl.size());
+ List<String> atl = rtl.get(0).getAccessTokens();
+ assertNotNull(atl);
+
+ // after 3 parallel refreshes we should have 4 AccessTokens
+ assertEquals(4, atl.size());
+ }
}
+
http://git-wip-us.apache.org/repos/asf/cxf/blob/5724e615/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/provider/JPAOAuthDataProviderTest.java
----------------------------------------------------------------------
diff --git a/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/provider/JPAOAuthDataProviderTest.java
b/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/provider/JPAOAuthDataProviderTest.java
index cc0cf29..7094a28 100644
--- a/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/provider/JPAOAuthDataProviderTest.java
+++ b/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/provider/JPAOAuthDataProviderTest.java
@@ -263,7 +263,7 @@ public class JPAOAuthDataProviderTest extends Assert {
assertNull(getProvider().getRefreshToken(rt.getTokenKey()));
}
- private Client addClient(String clientId, String userLogin) {
+ protected Client addClient(String clientId, String userLogin) {
Client c = new Client();
c.setRedirectUris(Collections.singletonList("http://client/redirect"));
c.setClientId(clientId);
|