cxf-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From serg...@apache.org
Subject cxf git commit: [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
Date Fri, 19 May 2017 10:08:39 GMT
Repository: cxf
Updated Branches:
  refs/heads/3.1.x-fixes 9eaf532d7 -> 8d5d7e76e


[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/8d5d7e76
Tree: http://git-wip-us.apache.org/repos/asf/cxf/tree/8d5d7e76
Diff: http://git-wip-us.apache.org/repos/asf/cxf/diff/8d5d7e76

Branch: refs/heads/3.1.x-fixes
Commit: 8d5d7e76e12faffd4451cc60b089cbdf2c609f2e
Parents: 9eaf532
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 11:08:04 2017 +0100

----------------------------------------------------------------------
 .../provider/AbstractOAuthDataProvider.java     | 11 ++++
 .../oauth2/provider/JPAOAuthDataProvider.java   | 42 ++++++++++++++
 .../code/JPACMTOAuthDataProviderTest.java       | 59 +++++++++++++++++++-
 .../provider/JPAOAuthDataProviderTest.java      |  2 +-
 4 files changed, 112 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cxf/blob/8d5d7e76/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 82d182f..4f407b0 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<String, OAuthPermission>();
     private MessageContext messageContext;
     private List<String> defaultScopes;
@@ -348,6 +349,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);
@@ -420,6 +426,11 @@ 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() {

http://git-wip-us.apache.org/repos/asf/cxf/blob/8d5d7e76/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 60a7615..b93205b 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,36 @@ 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.<String, Object>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 +454,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/8d5d7e76/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..d0f5b56 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);
+
+        final 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.<String>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/8d5d7e76/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 70a882c..0b243f5 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);


Mime
View raw message