From commits-return-46407-apmail-cxf-commits-archive=cxf.apache.org@cxf.apache.org Fri May 19 09:51:06 2017 Return-Path: X-Original-To: apmail-cxf-commits-archive@www.apache.org Delivered-To: apmail-cxf-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 02DD719DD2 for ; Fri, 19 May 2017 09:51:06 +0000 (UTC) Received: (qmail 5117 invoked by uid 500); 19 May 2017 09:51:05 -0000 Delivered-To: apmail-cxf-commits-archive@cxf.apache.org Received: (qmail 5059 invoked by uid 500); 19 May 2017 09:51:05 -0000 Mailing-List: contact commits-help@cxf.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cxf.apache.org Delivered-To: mailing list commits@cxf.apache.org Received: (qmail 5044 invoked by uid 99); 19 May 2017 09:51:04 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 19 May 2017 09:51:04 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id A561ADF9FD; Fri, 19 May 2017 09:51:04 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: sergeyb@apache.org To: commits@cxf.apache.org Message-Id: X-Mailer: ASF-Git Admin Mailer 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 09:51:04 +0000 (UTC) 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 Authored: Fri May 19 10:50:48 2017 +0100 Committer: Sergey Beryozkin 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 permissionMap = new HashMap<>(); private MessageContext messageContext; private List defaultScopes; @@ -59,7 +60,6 @@ public abstract class AbstractOAuthDataProvider implements OAuthDataProvider, Cl private Map 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() { + + @Override + public Void execute(EntityManager em) { + Map 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 execute(EntityManagerOperation 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 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 rtl = getProvider().getRefreshTokens(c, null); + assertNotNull(rtl); + assertEquals(1, rtl.size()); + List 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);