CXF-7264: NPE on OAuth RO/CC flows using JPA

* UserSubject can already be an OidcUserSubject in database while in current request (when using RO flow) it is a UserSubject. Merging UserSubject produces an error. We fix this by avoiding merge when userSubject already exists in db. * client.subject can be null when using CC flow.
apache · Mar 9, 2017 · 2180ca9 · 2180ca9
1 parent afdf936
commit 2180ca9
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 6 deletions.
diff --git a/...oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/provider/JPAOAuthDataProvider.java b/...oauth2/src/main/java/org/apache/cxf/rs/security/oauth2/provider/JPAOAuthDataProvider.java
@@ -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().getLogin());
+                    if (sub == null) {
+                        em.persist(serverToken.getSubject());
+                    } else {
+                        sub = serverToken.getSubject();
+                        serverToken.setSubject(sub);
+                    }
                 }
                 // ensure we have a managed association
                 // (needed for OpenJPA : InvalidStateException: Encountered unmanaged object)

diff --git a/...h2/src/test/java/org/apache/cxf/rs/security/oauth2/provider/JPAOAuthDataProviderTest.java b/...h2/src/test/java/org/apache/cxf/rs/security/oauth2/provider/JPAOAuthDataProviderTest.java
@@ -176,6 +176,27 @@ public void testAddGetDeleteAccessToken2() {
         assertEquals(0, tokens.size());
     }
 
+    @Test
+    public void testAddGetDeleteAccessTokenWithNullSubject() {
+        Client c = addClient("102", "bob");
+
+        AccessTokenRegistration atr = new AccessTokenRegistration();
+        atr.setClient(c);
+        atr.setApprovedScope(Collections.singletonList("a"));
+        atr.setSubject(null);
+
+        getProvider().createAccessToken(atr);
+        List<ServerAccessToken> tokens = getProvider().getAccessTokens(c, null);
+        assertNotNull(tokens);
+        assertEquals(1, tokens.size());
+
+        getProvider().removeClient(c.getClientId());
+
+        tokens = getProvider().getAccessTokens(c, null);
+        assertNotNull(tokens);
+        assertEquals(0, tokens.size());
+    }
+
     @Test
     public void testAddGetDeleteRefreshToken() {
         Client c = addClient("101", "bob");