cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r655222693



##########
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##########
@@ -539,6 +612,162 @@ private boolean isRoleStoreDefined() {
     }
 
 
+    /**
+     * Return the specified user's requested user attributes as a map.
+     * 
+     * @param dbConnection The database connection to be used
+     * @param username User name for which to return user attributes
+     * 
+     * @return a map containing the specified user's requested user attributes
+     */
+    protected Map<String, Object> getUserAttributesMap(Connection 
dbConnection, String username) {
+
+        String preparedAttributes = getUserAttributesStatement(dbConnection);
+        if (preparedAttributes == null || preparedAttributes == 
USER_ATTRIBUTES_NONE_REQUESTED) {
+            // The above reference comparison is intentional. 
USER_ATTRIBUTES_NONE_REQUESTED
+            // is a tag object (empty String) to distinguish between null (not 
yet
+            // initialized) and empty (no attributes requested).
+            // TODO Could as well be changed to `preparedAttributes.lenghth() 
= 0`
+
+            // Return null if no user attributes are requested (or if the 
statement was not
+            // yet built successfully)
+            return null;
+        }
+
+        try (PreparedStatement stmt = 
dbConnection.prepareStatement(preparedAttributes)) {
+            stmt.setString(1, username);
+
+            try (ResultSet rs = stmt.executeQuery()) {
+
+                if (rs.next()) {
+                    Map<String, Object> attrs = new LinkedHashMap<>();
+                    ResultSetMetaData md = rs.getMetaData();
+                    int ncols = md.getColumnCount();
+                    for (int columnIndex = 1; columnIndex <= ncols; 
columnIndex++) {
+                        String columnName = md.getColumnName(columnIndex);
+                        // Ignore case, database may have case-insensitive 
field names
+                        if (columnName.equalsIgnoreCase(userCredCol)) {
+                            // Always skip userCredCol (must be there if all 
columns
+                            // have been requested)
+                            continue;
+                        }
+                        attrs.put(columnName, rs.getObject(columnIndex));
+                    }
+                    return attrs.size() > 0 ? attrs : null;

Review comment:
       Ah, OK, multiple records for a user... Since by definition, the 
attributes are extra fields of the _user table_ (that has logon name and 
password), there is generally no support for user tables that contain the user 
more than once. That's just not allowed. In method `getPassword`  credentials 
are read from the first record returned (there's no `while (rs.hasNext())` 
loop).
   
   So generally, having a user more than once in the user table may cause 
problems with attributes: in `getUserAttributesMap` the multiple rows for that 
username could come in a different order. Than, the Principal will get the 
wrong attributes.
   
   We could generally deny authenticating in a user, for which more than one 
row is present in the user table (as done in JNDIRealm, AFAIK). However, that 
has nothing to do with the enhancement and should probably got to a new PR.
   
   As an alternative, we could add an ORDER BY <credential_field> to both the 
_credential_ and the _attributes_ statements in order to force the same order 
of rows.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



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

Reply via email to