You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2021/09/02 09:36:21 UTC

[tomcat] branch 9.0.x updated: Tighten up for partial roles and groups configuration

This is an automated email from the ASF dual-hosted git repository.

remm pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/9.0.x by this push:
     new 98075af  Tighten up for partial roles and groups configuration
98075af is described below

commit 98075af3745589d73e8278ccf7f8e3030a09b197
Author: remm <re...@apache.org>
AuthorDate: Thu Sep 2 11:33:14 2021 +0200

    Tighten up for partial roles and groups configuration
    
    Add debug log in open() that show the current feature in use, as there
    are three levels and lots of configuration attributes.
    Also harmonize some names.
---
 .../catalina/users/DataSourceUserDatabase.java     | 243 +++++++++++++--------
 webapps/docs/jndi-resources-howto.xml              |  12 +-
 2 files changed, 155 insertions(+), 100 deletions(-)

diff --git a/java/org/apache/catalina/users/DataSourceUserDatabase.java b/java/org/apache/catalina/users/DataSourceUserDatabase.java
index 2c6eb5e..ec44e8b 100644
--- a/java/org/apache/catalina/users/DataSourceUserDatabase.java
+++ b/java/org/apache/catalina/users/DataSourceUserDatabase.java
@@ -110,7 +110,7 @@ public class DataSourceUserDatabase extends SparseUserDatabase {
     /**
      * The generated string for the roles PreparedStatement
      */
-    private String preparedRoles = null;
+    private String preparedUserRoles = null;
 
 
     /**
@@ -122,7 +122,7 @@ public class DataSourceUserDatabase extends SparseUserDatabase {
     /**
      * The generated string for the groups PreparedStatement
      */
-    private String preparedGroups = null;
+    private String preparedUserGroups = null;
 
 
     /**
@@ -615,15 +615,19 @@ public class DataSourceUserDatabase extends SparseUserDatabase {
                 return null;
             }
 
-            Connection dbConnection = openConnection();
-            if (dbConnection == null) {
+            if (isGroupStoreDefined()) {
+                Connection dbConnection = openConnection();
+                if (dbConnection == null) {
+                    return null;
+                }
+                try {
+                    return findGroupInternal(dbConnection, groupname);
+                } finally {
+                    close(dbConnection);
+                }
+            } else {
                 return null;
             }
-            try {
-                return findGroupInternal(dbConnection, groupname);
-            } finally {
-                close(dbConnection);
-            }
         } finally {
             readLock.unlock();
         }
@@ -685,15 +689,19 @@ public class DataSourceUserDatabase extends SparseUserDatabase {
                 return null;
             }
 
-            Connection dbConnection = openConnection();
-            if (dbConnection == null) {
+            if (userRoleTable != null && roleNameCol != null) {
+                Connection dbConnection = openConnection();
+                if (dbConnection == null) {
+                    return null;
+                }
+                try {
+                    return findRoleInternal(dbConnection, rolename);
+                } finally {
+                    close(dbConnection);
+                }
+            } else {
                 return null;
             }
-            try {
-                return findRoleInternal(dbConnection, rolename);
-            } finally {
-                close(dbConnection);
-            }
         } finally {
             readLock.unlock();
         }
@@ -773,7 +781,7 @@ public class DataSourceUserDatabase extends SparseUserDatabase {
         // Lookup groups
         ArrayList<Group> groups = new ArrayList<>();
         if (isGroupStoreDefined()) {
-            try (PreparedStatement stmt = dbConnection.prepareStatement(preparedGroups)) {
+            try (PreparedStatement stmt = dbConnection.prepareStatement(preparedUserGroups)) {
                 stmt.setString(1, userName);
                 try (ResultSet rs = stmt.executeQuery()) {
                     while (rs.next()) {
@@ -792,8 +800,8 @@ public class DataSourceUserDatabase extends SparseUserDatabase {
         }
 
         ArrayList<Role> roles = new ArrayList<>();
-        if (isRoleStoreDefined()) {
-            try (PreparedStatement stmt = dbConnection.prepareStatement(preparedRoles)) {
+        if (userRoleTable != null && roleNameCol != null) {
+            try (PreparedStatement stmt = dbConnection.prepareStatement(preparedUserRoles)) {
                 stmt.setString(1, userName);
                 try (ResultSet rs = stmt.executeQuery()) {
                     while (rs.next()) {
@@ -857,19 +865,45 @@ public class DataSourceUserDatabase extends SparseUserDatabase {
     @Override
     public void open() throws Exception {
 
+        if (log.isDebugEnabled()) {
+            // As there are lots of parameters to configure, log some debug to help out
+            log.debug("DataSource UserDatabase features: User<->Role ["
+                    + Boolean.toString(userRoleTable != null && roleNameCol != null)
+                    + "], Roles [" + Boolean.toString(isRoleStoreDefined())
+                    + "], Groups [" + Boolean.toString(isRoleStoreDefined()) + "]");
+        }
+
         writeLock.lock();
         try {
 
             StringBuilder temp = new StringBuilder("SELECT ");
+            temp.append(userCredCol);
+            if (userFullNameCol != null) {
+                temp.append(",").append(userFullNameCol);
+            }
+            temp.append(" FROM ");
+            temp.append(userTable);
+            temp.append(" WHERE ");
+            temp.append(userNameCol);
+            temp.append(" = ?");
+            preparedUser = temp.toString();
+
+            temp = new StringBuilder("SELECT ");
+            temp.append(userNameCol);
+            temp.append(" FROM ");
+            temp.append(userTable);
+            preparedAllUsers = temp.toString();
+
+            temp = new StringBuilder("SELECT ");
             temp.append(roleNameCol);
             temp.append(" FROM ");
             temp.append(userRoleTable);
             temp.append(" WHERE ");
             temp.append(userNameCol);
             temp.append(" = ?");
-            preparedRoles = temp.toString();
+            preparedUserRoles = temp.toString();
 
-            if (userGroupTable != null) {
+            if (isGroupStoreDefined()) {
                 temp = new StringBuilder("SELECT ");
                 temp.append(groupNameCol);
                 temp.append(" FROM ");
@@ -877,10 +911,8 @@ public class DataSourceUserDatabase extends SparseUserDatabase {
                 temp.append(" WHERE ");
                 temp.append(userNameCol);
                 temp.append(" = ?");
-                preparedGroups = temp.toString();
-            }
+                preparedUserGroups = temp.toString();
 
-            if (groupRoleTable != null) {
                 temp = new StringBuilder("SELECT ");
                 temp.append(roleNameCol);
                 temp.append(" FROM ");
@@ -889,27 +921,7 @@ public class DataSourceUserDatabase extends SparseUserDatabase {
                 temp.append(groupNameCol);
                 temp.append(" = ?");
                 preparedGroupRoles = temp.toString();
-            }
-
-            temp = new StringBuilder("SELECT ");
-            temp.append(userCredCol);
-            if (userFullNameCol != null) {
-                temp.append(",").append(userFullNameCol);
-            }
-            temp.append(" FROM ");
-            temp.append(userTable);
-            temp.append(" WHERE ");
-            temp.append(userNameCol);
-            temp.append(" = ?");
-            preparedUser = temp.toString();
 
-            temp = new StringBuilder("SELECT ");
-            temp.append(userNameCol);
-            temp.append(" FROM ");
-            temp.append(userTable);
-            preparedAllUsers = temp.toString();
-
-            if (groupTable != null) {
                 temp = new StringBuilder("SELECT ");
                 temp.append(groupNameCol);
                 if (roleAndGroupDescriptionCol != null) {
@@ -929,7 +941,7 @@ public class DataSourceUserDatabase extends SparseUserDatabase {
                 preparedAllGroups = temp.toString();
             }
 
-            if (roleTable != null) {
+            if (isRoleStoreDefined()) {
                 // Create the role PreparedStatement string
                 temp = new StringBuilder("SELECT ");
                 temp.append(roleNameCol);
@@ -948,7 +960,7 @@ public class DataSourceUserDatabase extends SparseUserDatabase {
                 temp.append(" FROM ");
                 temp.append(roleTable);
                 preparedAllRoles = temp.toString();
-            } else {
+            } else if (userRoleTable != null && roleNameCol != null) {
                 // Validate roles existence from the user <-> roles table
                 temp = new StringBuilder("SELECT ");
                 temp.append(roleNameCol);
@@ -1034,7 +1046,7 @@ public class DataSourceUserDatabase extends SparseUserDatabase {
         StringBuilder tempRelation = null;
         StringBuilder tempRelationDelete = null;
 
-        if (roleTable != null) {
+        if (isRoleStoreDefined()) {
 
             // Created roles
             if (!createdRoles.isEmpty()) {
@@ -1125,9 +1137,25 @@ public class DataSourceUserDatabase extends SparseUserDatabase {
                 removedRoles.clear();
             }
 
+        } else if (userRoleTable != null && roleNameCol != null) {
+            // Only remove role from users
+            tempRelationDelete = new StringBuilder("DELETE FROM ");
+            tempRelationDelete.append(userRoleTable);
+            tempRelationDelete.append(" WHERE ");
+            tempRelationDelete.append(roleNameCol);
+            tempRelationDelete.append(" = ?");
+            for (Role role : removedRoles.values()) {
+                try (PreparedStatement stmt = dbConnection.prepareStatement(tempRelationDelete.toString())) {
+                    stmt.setString(1, role.getRolename());
+                    stmt.executeUpdate();
+                } catch (SQLException e) {
+                    log.error(sm.getString("dataSourceUserDatabase.exception"), e);
+                }
+            }
+            removedRoles.clear();
         }
 
-        if (groupTable != null && groupRoleTable != null) {
+        if (isGroupStoreDefined()) {
 
             tempRelation = new StringBuilder("INSERT INTO ");
             tempRelation.append(groupRoleTable);
@@ -1257,22 +1285,26 @@ public class DataSourceUserDatabase extends SparseUserDatabase {
 
         }
 
-        tempRelation = new StringBuilder("INSERT INTO ");
-        tempRelation.append(userRoleTable);
-        tempRelation.append('(').append(userNameCol).append(", ");
-        tempRelation.append(roleNameCol);
-        tempRelation.append(") VALUES (?, ?)");
-        String userRoleRelation = tempRelation.toString();
-        // Always drop and recreate all user <-> role relations
-        tempRelationDelete = new StringBuilder("DELETE FROM ");
-        tempRelationDelete.append(userRoleTable);
-        tempRelationDelete.append(" WHERE ");
-        tempRelationDelete.append(userNameCol);
-        tempRelationDelete.append(" = ?");
-        String userRoleRelationDelete = tempRelationDelete.toString();
+        String userRoleRelation = null;
+        String userRoleRelationDelete = null;
+        if (userRoleTable != null && roleNameCol != null) {
+            tempRelation = new StringBuilder("INSERT INTO ");
+            tempRelation.append(userRoleTable);
+            tempRelation.append('(').append(userNameCol).append(", ");
+            tempRelation.append(roleNameCol);
+            tempRelation.append(") VALUES (?, ?)");
+            userRoleRelation = tempRelation.toString();
+            // Always drop and recreate all user <-> role relations
+            tempRelationDelete = new StringBuilder("DELETE FROM ");
+            tempRelationDelete.append(userRoleTable);
+            tempRelationDelete.append(" WHERE ");
+            tempRelationDelete.append(userNameCol);
+            tempRelationDelete.append(" = ?");
+            userRoleRelationDelete = tempRelationDelete.toString();
+        }
         String userGroupRelation = null;
         String userGroupRelationDelete = null;
-        if (userGroupTable != null) {
+        if (isGroupStoreDefined()) {
             tempRelation = new StringBuilder("INSERT INTO ");
             tempRelation.append(userGroupTable);
             tempRelation.append('(').append(userNameCol).append(", ");
@@ -1313,15 +1345,17 @@ public class DataSourceUserDatabase extends SparseUserDatabase {
                 } catch (SQLException e) {
                     log.error(sm.getString("dataSourceUserDatabase.exception"), e);
                 }
-                Iterator<Role> roles = user.getRoles();
-                while (roles.hasNext()) {
-                    Role role = roles.next();
-                    try (PreparedStatement stmt = dbConnection.prepareStatement(userRoleRelation)) {
-                        stmt.setString(1, user.getUsername());
-                        stmt.setString(2, role.getRolename());
-                        stmt.executeUpdate();
-                    } catch (SQLException e) {
-                        log.error(sm.getString("dataSourceUserDatabase.exception"), e);
+                if (userRoleRelation != null) {
+                    Iterator<Role> roles = user.getRoles();
+                    while (roles.hasNext()) {
+                        Role role = roles.next();
+                        try (PreparedStatement stmt = dbConnection.prepareStatement(userRoleRelation)) {
+                            stmt.setString(1, user.getUsername());
+                            stmt.setString(2, role.getRolename());
+                            stmt.executeUpdate();
+                        } catch (SQLException e) {
+                            log.error(sm.getString("dataSourceUserDatabase.exception"), e);
+                        }
                     }
                 }
                 if (userGroupRelation != null) {
@@ -1365,40 +1399,46 @@ public class DataSourceUserDatabase extends SparseUserDatabase {
                 } catch (SQLException e) {
                     log.error(sm.getString("dataSourceUserDatabase.exception"), e);
                 }
-                try (PreparedStatement stmt = dbConnection.prepareStatement(userRoleRelationDelete)) {
-                    stmt.setString(1, user.getUsername());
-                    stmt.executeUpdate();
-                } catch (SQLException e) {
-                    log.error(sm.getString("dataSourceUserDatabase.exception"), e);
-                }
-                if (userGroupRelationDelete != null) {
-                    try (PreparedStatement stmt = dbConnection.prepareStatement(userGroupRelationDelete)) {
+                if (userRoleRelationDelete != null) {
+                    try (PreparedStatement stmt = dbConnection.prepareStatement(userRoleRelationDelete)) {
                         stmt.setString(1, user.getUsername());
                         stmt.executeUpdate();
                     } catch (SQLException e) {
                         log.error(sm.getString("dataSourceUserDatabase.exception"), e);
                     }
                 }
-                Iterator<Role> roles = user.getRoles();
-                while (roles.hasNext()) {
-                    Role role = roles.next();
-                    try (PreparedStatement stmt = dbConnection.prepareStatement(userRoleRelation)) {
+                if (userGroupRelationDelete != null) {
+                    try (PreparedStatement stmt = dbConnection.prepareStatement(userGroupRelationDelete)) {
                         stmt.setString(1, user.getUsername());
-                        stmt.setString(2, role.getRolename());
                         stmt.executeUpdate();
                     } catch (SQLException e) {
                         log.error(sm.getString("dataSourceUserDatabase.exception"), e);
                     }
                 }
-                Iterator<Group> groups = user.getGroups();
-                while (groups.hasNext()) {
-                    Group group = groups.next();
-                    try (PreparedStatement stmt = dbConnection.prepareStatement(userGroupRelation)) {
-                        stmt.setString(1, user.getUsername());
-                        stmt.setString(2, group.getGroupname());
-                        stmt.executeUpdate();
-                    } catch (SQLException e) {
-                        log.error(sm.getString("dataSourceUserDatabase.exception"), e);
+                if (userRoleRelation != null) {
+                    Iterator<Role> roles = user.getRoles();
+                    while (roles.hasNext()) {
+                        Role role = roles.next();
+                        try (PreparedStatement stmt = dbConnection.prepareStatement(userRoleRelation)) {
+                            stmt.setString(1, user.getUsername());
+                            stmt.setString(2, role.getRolename());
+                            stmt.executeUpdate();
+                        } catch (SQLException e) {
+                            log.error(sm.getString("dataSourceUserDatabase.exception"), e);
+                        }
+                    }
+                }
+                if (userGroupRelation != null) {
+                    Iterator<Group> groups = user.getGroups();
+                    while (groups.hasNext()) {
+                        Group group = groups.next();
+                        try (PreparedStatement stmt = dbConnection.prepareStatement(userGroupRelation)) {
+                            stmt.setString(1, user.getUsername());
+                            stmt.setString(2, group.getGroupname());
+                            stmt.executeUpdate();
+                        } catch (SQLException e) {
+                            log.error(sm.getString("dataSourceUserDatabase.exception"), e);
+                        }
                     }
                 }
             }
@@ -1443,13 +1483,22 @@ public class DataSourceUserDatabase extends SparseUserDatabase {
         return connectionSuccess;
     }
 
-    private boolean isGroupStoreDefined() {
-        return userGroupTable != null || groupNameCol != null;
+    /**
+     * Only use groups if the tables are fully defined.
+     * @return true when groups are used
+     */
+    protected boolean isGroupStoreDefined() {
+        return groupTable != null && userGroupTable != null && groupNameCol != null
+                && groupRoleTable != null && isRoleStoreDefined();
     }
 
 
-    private boolean isRoleStoreDefined() {
-        return userRoleTable != null || roleNameCol != null;
+    /**
+     * Only use roles if the tables are fully defined.
+     * @return true when roles are used
+     */
+    protected boolean isRoleStoreDefined() {
+        return roleTable != null && userRoleTable != null && roleNameCol != null;
     }
 
 
diff --git a/webapps/docs/jndi-resources-howto.xml b/webapps/docs/jndi-resources-howto.xml
index 0c682f8..abb3cba 100644
--- a/webapps/docs/jndi-resources-howto.xml
+++ b/webapps/docs/jndi-resources-howto.xml
@@ -631,7 +631,7 @@ create table user_roles (
         <p>If this is set to <code>true</code>, then changes to the
         <code>UserDatabase</code> can be persisted to the
         <code>DataSource</code> by using the <code>save</code> method.
-        The default value is <code>false</code>.</p>
+        The default value is <code>true</code>.</p>
       </attribute>
 
       <attribute name="roleAndGroupDescriptionCol" required="false">
@@ -639,10 +639,13 @@ create table user_roles (
         the description for the roles and groups.</p>
       </attribute>
 
-      <attribute name="roleNameCol" required="true">
+      <attribute name="roleNameCol" required="false">
         <p>Name of the column, in the "roles", "user roles" and "group roles"
         tables, which contains a role name assigned to the corresponding
         user.</p>
+        <p>This attribute is <strong>required</strong> in majority of
+        configurations. See <strong>allRolesMode</strong> attribute of the
+        associated realm for a rare case when it can be omitted.</p>
       </attribute>
 
       <attribute name="roleTable" required="false">
@@ -675,10 +678,13 @@ create table user_roles (
         full name.</p>
       </attribute>
 
-      <attribute name="userRoleTable" required="true">
+      <attribute name="userRoleTable" required="false">
         <p>Name of the "user roles" table, which must contain columns
         named by the <code>userNameCol</code> and <code>roleNameCol</code>
         attributes.</p>
+        <p>This attribute is <strong>required</strong> in majority of
+        configurations. See <strong>allRolesMode</strong> attribute of the
+        associated realm for a rare case when it can be omitted.</p>
       </attribute>
 
       <attribute name="userTable" required="true">

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org