From 14d3b2692722ead55a4f4e03c24f472667f58481 Mon Sep 17 00:00:00 2001 From: gonzalad Date: Tue, 14 Mar 2017 21:17:19 +0100 Subject: [PATCH] CXF-7264 JPA change userSubject pk to id Change primary key of UserSubject from login attribute to id attribute. Needed because a userSubject instance is per-authentication and an end-user can be authenticated multiple times simultaneously (either using different flows or by multiple end-user using same credentials). This commit also fixed CXF-7286: PersistenceException on refreshAccessToken: PersistenceException: org.hibernate.HibernateException: Found shared references to a collection: org.apache.cxf.rs.security.oauth2.tokens.bearer.BearerAccessToken.audiences --- .../security/oauth2/common/UserSubject.java | 16 ++++++-- .../grants/code/JPACodeDataProvider.java | 2 +- .../provider/AbstractOAuthDataProvider.java | 6 ++- .../oauth2/provider/JPAOAuthDataProvider.java | 16 ++++---- .../oauth2/common/UserSubjectTest.java | 39 +++++++++++++++++++ .../provider/JPAOAuthDataProviderTest.java | 30 ++++++++++++++ .../oauth2/provider/TestingUserSubject.java | 35 +++++++++++++++++ .../test/resources/META-INF/persistence.xml | 2 + 8 files changed, 132 insertions(+), 14 deletions(-) create mode 100644 rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/common/UserSubjectTest.java create mode 100644 rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/provider/TestingUserSubject.java diff --git a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/common/UserSubject.java b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/common/UserSubject.java index 7621481f7fa..014c5f55c8f 100644 --- a/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/common/UserSubject.java +++ b/rt/rs/security/oauth-parent/oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/common/UserSubject.java @@ -32,6 +32,9 @@ import javax.persistence.OrderColumn; import javax.xml.bind.annotation.XmlRootElement; +import org.apache.cxf.common.util.Base64UrlUtility; +import org.apache.cxf.rt.security.crypto.CryptoUtils; + /** * Represents a login name which AuthorizationService * may capture after the end user approved a given third party request @@ -49,26 +52,28 @@ public class UserSubject implements Serializable { private AuthenticationMethod am; public UserSubject() { - + this.id = newId(); } public UserSubject(String login) { + this(); this.login = login; } public UserSubject(String login, List roles) { + this(); this.login = login; this.roles = roles; } public UserSubject(String login, String id) { this.login = login; - this.id = id; + this.id = id != null ? id : newId(); } public UserSubject(String login, String id, List roles) { this.login = login; - this.id = id; + this.id = id != null ? id : newId(); this.roles = roles; } @@ -76,7 +81,10 @@ public UserSubject(UserSubject sub) { this(sub.getLogin(), sub.getId(), sub.getRoles()); this.properties = sub.getProperties(); this.am = sub.getAuthenticationMethod(); + } + private String newId() { + return Base64UrlUtility.encode(CryptoUtils.generateSecureRandomBytes(16)); } /** @@ -84,7 +92,6 @@ public UserSubject(UserSubject sub) { * * @return the login name */ - @Id public String getLogin() { return login; } @@ -145,6 +152,7 @@ public void setProperties(Map properties) { * * @return the user's id */ + @Id public String getId() { return this.id; } 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 84bfb8e9e8e..a0b20013abc 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 @@ -50,7 +50,7 @@ protected void saveCodeGrant(final ServerAuthorizationCodeGrant grant) { @Override public Void execute(EntityManager em) { if (grant.getSubject() != null) { - UserSubject sub = em.find(UserSubject.class, grant.getSubject().getLogin()); + UserSubject sub = em.find(UserSubject.class, grant.getSubject().getId()); if (sub == null) { em.persist(grant.getSubject()); } else { 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 dec7775014c..4b0509fe4ba 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 @@ -365,14 +365,16 @@ protected ServerAccessToken doRefreshAccessToken(Client client, RefreshToken oldRefreshToken, List restrictedScopes) { ServerAccessToken at = createNewAccessToken(client, oldRefreshToken.getSubject()); - at.setAudiences(oldRefreshToken.getAudiences()); + at.setAudiences(oldRefreshToken.getAudiences() != null + ? new ArrayList(oldRefreshToken.getAudiences()) : null); at.setGrantType(oldRefreshToken.getGrantType()); at.setGrantCode(oldRefreshToken.getGrantCode()); at.setSubject(oldRefreshToken.getSubject()); at.setNonce(oldRefreshToken.getNonce()); at.setClientCodeVerifier(oldRefreshToken.getClientCodeVerifier()); if (restrictedScopes.isEmpty()) { - at.setScopes(oldRefreshToken.getScopes()); + at.setScopes(oldRefreshToken.getScopes() != null + ? new ArrayList(oldRefreshToken.getScopes()) : null); } else { List theNewScopes = convertScopeToPermissions(client, restrictedScopes); if (oldRefreshToken.getScopes().containsAll(theNewScopes)) { 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 ce496737bc7..f59b40e858f 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 @@ -105,7 +105,7 @@ public void setClient(final Client client) { public Void execute(EntityManager em) { if (client.getResourceOwnerSubject() != null) { UserSubject sub = - em.find(UserSubject.class, client.getResourceOwnerSubject().getLogin()); + em.find(UserSubject.class, client.getResourceOwnerSubject().getId()); if (sub == null) { em.persist(client.getResourceOwnerSubject()); } else { @@ -263,12 +263,14 @@ public Void execute(EntityManager em) { } serverToken.setScopes(perms); - UserSubject sub = em.find(UserSubject.class, serverToken.getSubject().getLogin()); - if (sub == null) { - em.persist(serverToken.getSubject()); - } else { - sub = em.merge(serverToken.getSubject()); - serverToken.setSubject(sub); + if (serverToken.getSubject() != null) { + UserSubject sub = em.find(UserSubject.class, serverToken.getSubject().getId()); + if (sub == null) { + em.persist(serverToken.getSubject()); + } else { + sub = em.merge(serverToken.getSubject()); + serverToken.setSubject(sub); + } } // ensure we have a managed association // (needed for OpenJPA : InvalidStateException: Encountered unmanaged object) diff --git a/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/common/UserSubjectTest.java b/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/common/UserSubjectTest.java new file mode 100644 index 00000000000..52d77c05739 --- /dev/null +++ b/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/common/UserSubjectTest.java @@ -0,0 +1,39 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.cxf.rs.security.oauth2.common; + +import java.util.Collections; + +import org.junit.Assert; +import org.junit.Test; + +public class UserSubjectTest extends Assert { + + @Test + public void testId() { + UserSubject userSubject = new UserSubject(); + assertNotNull(userSubject.getId()); + userSubject = new UserSubject("someLogin"); + assertNotNull(userSubject.getId()); + userSubject = new UserSubject("someLogin", Collections.singletonList("somerole")); + assertNotNull(userSubject.getId()); + UserSubject newSubject = new UserSubject(userSubject); + assertEquals(userSubject.getId(), newSubject.getId()); + } +} \ 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/JPAOAuthDataProviderTest.java b/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/provider/JPAOAuthDataProviderTest.java index b71a9206862..cc0cf2959b7 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 @@ -199,6 +199,36 @@ public void testAddGetDeleteAccessTokenWithNullSubject() { assertEquals(0, tokens.size()); } + /** + * Checks that having multiple token each with its own + * userSubject (but having same login) works. + */ + @Test + public void testAddGetDeleteMultipleAccessToken() { + Client c = addClient("101", "bob"); + + AccessTokenRegistration atr = new AccessTokenRegistration(); + atr.setClient(c); + atr.setApprovedScope(Collections.singletonList("a")); + atr.setSubject(c.getResourceOwnerSubject()); + ServerAccessToken at = getProvider().createAccessToken(atr); + at = getProvider().getAccessToken(at.getTokenKey()); + + AccessTokenRegistration atr2 = new AccessTokenRegistration(); + atr2.setClient(c); + atr2.setApprovedScope(Collections.singletonList("a")); + atr2.setSubject(new TestingUserSubject(c.getResourceOwnerSubject().getLogin())); + ServerAccessToken at2 = getProvider().createAccessToken(atr2); + at2 = getProvider().getAccessToken(at2.getTokenKey()); + + assertNotNull(at.getSubject().getId()); + assertTrue(at.getSubject() instanceof UserSubject); + assertNotNull(at2.getSubject().getId()); + assertTrue(at2.getSubject() instanceof TestingUserSubject); + assertEquals(at.getSubject().getLogin(), at2.getSubject().getLogin()); + assertNotEquals(at.getSubject().getId(), at2.getSubject().getId()); + } + @Test public void testAddGetDeleteRefreshToken() { Client c = addClient("101", "bob"); diff --git a/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/provider/TestingUserSubject.java b/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/provider/TestingUserSubject.java new file mode 100644 index 00000000000..e762f3f4b85 --- /dev/null +++ b/rt/rs/security/oauth-parent/oauth2/src/test/java/org/apache/cxf/rs/security/oauth2/provider/TestingUserSubject.java @@ -0,0 +1,35 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.cxf.rs.security.oauth2.provider; + +import javax.persistence.Entity; + +import org.apache.cxf.rs.security.oauth2.common.UserSubject; + +@Entity +public class TestingUserSubject extends UserSubject { + + public TestingUserSubject() { + super(); + } + + public TestingUserSubject(String login) { + super(login); + } +} \ No newline at end of file diff --git a/rt/rs/security/oauth-parent/oauth2/src/test/resources/META-INF/persistence.xml b/rt/rs/security/oauth-parent/oauth2/src/test/resources/META-INF/persistence.xml index 8d7ad1c3714..41e7e83ab28 100644 --- a/rt/rs/security/oauth-parent/oauth2/src/test/resources/META-INF/persistence.xml +++ b/rt/rs/security/oauth-parent/oauth2/src/test/resources/META-INF/persistence.xml @@ -14,6 +14,7 @@ org.apache.cxf.rs.security.oauth2.common.AccessToken org.apache.cxf.rs.security.oauth2.common.OAuthPermission org.apache.cxf.rs.security.oauth2.tokens.refresh.RefreshToken + org.apache.cxf.rs.security.oauth2.provider.TestingUserSubject true ENABLE_SELECTIVE @@ -41,6 +42,7 @@ org.apache.cxf.rs.security.oauth2.common.AccessToken org.apache.cxf.rs.security.oauth2.common.OAuthPermission org.apache.cxf.rs.security.oauth2.tokens.refresh.RefreshToken + org.apache.cxf.rs.security.oauth2.provider.TestingUserSubject true ENABLE_SELECTIVE