From ab7f06c10854116f6875c3b9a99c0f9f63cc4fc6 Mon Sep 17 00:00:00 2001
From: Luke Taylor <luke.taylor@springsource.com>
Date: Sun, 14 Jun 2009 22:26:44 +0000
Subject: [PATCH] SEC-1156: Modified JdbcUserDetailsManager to only save/update
 authorities if enableAuthorities is set

---
 .../provisioning/JdbcUserDetailsManager.java  | 18 ++++++++++---
 .../JdbcUserDetailsManagerTests.java          | 25 +++++++++++++++----
 2 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java b/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java
index d954776961..0494c5444d 100644
--- a/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java
+++ b/core/src/main/java/org/springframework/security/provisioning/JdbcUserDetailsManager.java
@@ -28,7 +28,13 @@ import java.sql.SQLException;
 import java.util.List;
 
 /**
- * Jdbc user management service.
+ * Jdbc user management service, based on the same table structure as its parent class, <tt>JdbcDaoImpl</tt>.
+ * <p>
+ * Provides CRUD operations for both users and groups. Note that if the {@link #setEnableAuthorities(boolean)
+ * enableAuthorities} property is set to false, calls to createUser and updateUser will not store the
+ * authorities from the <tt>UserDetails</tt>. Since this class cannot differentiate between authorities which were
+ * loaded for an individual or for a group of which the individual is a member, it's important that you take this
+ * into account when using this implementation for managing your users.
  *
  * @author Luke Taylor
  * @version $Id$
@@ -141,7 +147,9 @@ public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsMa
 
         });
 
-        insertUserAuthorities(user);
+        if (getEnableAuthorities()) {
+            insertUserAuthorities(user);
+        }
     }
 
     public void updateUser(final UserDetails user) {
@@ -154,8 +162,10 @@ public class JdbcUserDetailsManager extends JdbcDaoImpl implements UserDetailsMa
             }
         });
 
-        deleteUserAuthorities(user.getUsername());
-        insertUserAuthorities(user);
+        if (getEnableAuthorities()) {
+            deleteUserAuthorities(user.getUsername());
+            insertUserAuthorities(user);
+        }
 
         userCache.removeUserFromCache(user.getUsername());
     }
diff --git a/core/src/test/java/org/springframework/security/provisioning/JdbcUserDetailsManagerTests.java b/core/src/test/java/org/springframework/security/provisioning/JdbcUserDetailsManagerTests.java
index 010ec95ede..7d1d3dd265 100644
--- a/core/src/test/java/org/springframework/security/provisioning/JdbcUserDetailsManagerTests.java
+++ b/core/src/test/java/org/springframework/security/provisioning/JdbcUserDetailsManagerTests.java
@@ -1,9 +1,6 @@
 package org.springframework.security.provisioning;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.junit.Assert.*;
 
 import java.util.Collections;
 import java.util.HashMap;
@@ -30,7 +27,6 @@ import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.security.core.userdetails.User;
 import org.springframework.security.core.userdetails.UserCache;
 import org.springframework.security.core.userdetails.UserDetails;
-import org.springframework.security.provisioning.JdbcUserDetailsManager;
 
 /**
  * Tests for {@link JdbcUserDetailsManager}
@@ -282,6 +278,24 @@ public class JdbcUserDetailsManagerTests {
         assertEquals(2, template.queryForList("select authority from group_authorities where group_id = 2").size());
     }
 
+    // SEC-1156
+    @Test
+    public void createUserDoesNotSaveAuthoritiesIfEnableAuthoritiesIsFalse() throws Exception {
+        manager.setEnableAuthorities(false);
+        manager.createUser(joe);
+        assertEquals(0, template.queryForList(SELECT_JOE_AUTHORITIES_SQL).size());
+    }
+
+    // SEC-1156
+    @Test
+    public void updateUserDoesNotSaveAuthoritiesIfEnableAuthoritiesIsFalse() throws Exception {
+        manager.setEnableAuthorities(false);
+        insertJoe();
+        template.execute("delete from authorities where username='joe'");
+        manager.updateUser(joe);
+        assertEquals(0, template.queryForList(SELECT_JOE_AUTHORITIES_SQL).size());
+    }
+
     private Authentication authenticateJoe() {
         UsernamePasswordAuthenticationToken auth =
                 new UsernamePasswordAuthenticationToken("joe","password", joe.getAuthorities());
@@ -290,6 +304,7 @@ public class JdbcUserDetailsManagerTests {
         return auth;
     }
 
+
     private void insertJoe() {
         template.execute("insert into users (username, password, enabled) values ('joe','password','true')");
         template.execute("insert into authorities (username, authority) values ('joe','A')");