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 go into 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.
So, since the _multiple rows for one user_ case should actually not occur,
it is no proper way to add multi-valued attributes by having multiple records.
--
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
