cklein05 opened a new pull request #428: URL: https://github.com/apache/tomcat/pull/428
This enhancement adds support for additional user attributes to be queried by MemoryRealm, UserDatabaseRealm, DataSourceRealm and JNDIRealm. That has already been discussed here http://tomcat.10.x6.nabble.com/Getting-additional-attributes-for-logged-on-users-tt5110316.html and here http://tomcat.10.x6.nabble.com/Enhancement-Additional-user-attributes-queried-by-some-realms-td5111884.html Bold sentences require your special attention (like questions, decisions, changes not related to the enhancement, etc). **That's still work in progress. There's no documentation and change log entries yet. Let's agree on the code first.** **Have a look at example at `/examples/jsp/security/protected/`. It has been modified to list all the Principal's attributes.** ### General implementation notes: - Interface `TomcatPrincipal` now supports an attributes map, accessible by methods `Object getAttribute(String name);` `Enumeration<String> getAttributeNames();` `boolean isAttributesIgnoreCase();` Have a look at these method's Javadoc comments. Note, that it's not named _userAttributes_, but simply _attributes_. It uses that more generic name, since the Principal _IS_ (or at least represents) the user. With JNDIRealm, and, depending on the directory server used, attribute names may be _case-insensitive_ (if ` javax.naming.directory.Attributes.isCaseIgnored()` returns `true`). If so, the Principal's attributes map uses case-insensitive names as well. - Class `GenericPrincipal`, which implements that interface, also adds serialization support for the attributes map. - Realm classes `MemoryRealm`, `UserDatabaseRealm`, `DataSourceRealm` and `JNDIRealm` now have a new configuration option `userAttributes`. It takes a delimiter separated list of attribute/field names to query from the _user table/entry_. These additional attributes are then provided through the Principal's attributes map. That option supports a wildcard character (*), which indicates to retrieve all available attributes/fields (except the _password_ field). Both the delimiter and the wildcard character are defined by a constant in class `RealmBase`. - Using the `userAttributes` option requires some preprocessing, depending on the Realm implementation (parsing, validating etc.). Its result is then cached in a Realm's private field. The preprocessing may be quite expensive for some Realms. For example, DataSourceRealm must retrieve all the user table's field names to determine, which requested attributes are valid (if no wildcard character was specified). I decided not to use any JDBC- or JNDI connections prior to the first authentication attempt so, preprocessing is done lazily for some Realms. As Realms are used by concurrent threads, it uses _double-checked locking, DCL_ in order to be thread-safe (like in `UserDatabaseRealm.getDatabase()`). That, in particular, applies to `DataSourceRealm`. Since JNDI simply ignores requested non-existing attributes, it's much easier to use than JDBC and SQL. Together with the fact, that JNDIRealm anyway logs missing or invalid fields for every login attempt (in contrast to while preprocessing only), lazy initialization and the DCL is not needed with that Realm. - Preprocessing logs a warning to the container log for every unknown or invalid attribute requested. Except for `MemoryRealm`, warnings are logged on the first authentication attempt. `JNDIRealm` even logs warnings on every authentication attempt, since in a directory users may have different sets of attributes. - All Realms emit warning messages - `realmBase.userAttributeAccessDenied`, if an attribute containing the user's password is requested. - `realmBase.userAttributeNotFound`, if the attribute could not be found. - JNDIRealm emits warning messages - `jndiRealm.userAttributeAccessDenied`, if an attribute containing the user's password is requested. - `jndiRealm.userAttributeNotFound`, if the attribute could not be found. - JNDIRealm emits debug message - `jndiRealm.userAttributeNotRequested`, if a _related_ attribute was sent by the directory server, but was not explicitly requested **We may still agree on the log levels used.** - Realms (are encouraged to) use a LinkedHashMap to store user attributes, so insertion order is preserved. That is, `getAttributeNames()` returns names in the order specified in the `userAttributes` option (with invalid attributes removed). ### Realm-specific notes - `MemoryRealm` - Uses a fixed set of available user attributes, determined by its implementation: `username` `fullname` `roles` (comma separated list of roles like in the XML attribute) - Emits an _AccessDenied_ warning when requesting the `password` attribute. - Does not perform _lazy_ preprocessing and logging, as all Principals are created in `startInternal()`. - **Refactored role accumulation using a `LinkedHashSet` to remove duplicate roles.** Uses a **Linked**HashSet to preserve order for user attribute `roles`. - `UserDatabaseRealm` - Uses a fixed set of available user attributes, determined by its implementation: `username` `fullname` `groups` (comma sep. list of groups like in the XML attribute) `roles` (comma sep. list of roles like in the XML attribute) `effectiveRoles` (comma sep. list of effective roles w/ group-based roles resolved) - Emits an _AccessDenied_ warning when requesting the `password` attribute. - Performs _lazy_ preprocessing and logging in order to be consistent with the other Realms intended for production use (`MemoryRealm` is not). **However, that could easily be changed if you like.** - User attribute values are not cached in a map. They are retrieved from the user's `User` object for every invocation of method `getAttribute` so, changes applied to the `UserDatabase` are reflected immediately. - `DataSourceRealm` - Performs an extra SQL query to retrieve user attributes. - Emits an _AccessDenied_ warning when requesting the attribute specified in `userCredCol`. - Performs _lazy_ preprocessing and logging. - **Refactored role accumulation using a `HashSet` to remove duplicate roles.** - `JDNIRealm` - There is no extra LDAP query required to retrieve additional user attributes. Requested user attributes are merged with the attributes list created in method `getUser(JNDIConnection, String, String, int)` (now uses a `Set` to remove duplicates). - Emits an _AccessDenied_ warning when requesting the `userPassword` attribute (if specified). - Needs no _lazy_ preprocessing and logging. - Logs messages for non-existing and denied attributes (WARNING level) as well as for attributes, not explicitly requested (DEBUG Level), on every authentication attempt (users may have different sets of attributes). - With `userAttributes` containing the wildcard character, it is now legal to query _ALL_ attributes from the user's directory entry. The current implementation of JNDIRealm denied that explicitly. **That has been changed and marked with a TODO comment.** - A directory's attribute names may be case-insensitive, that is `Attributes.isIgnoreCase()` returns `true`. In that case, the keys of the _attributes map_ provided through the user's Principal are case-insensitive as well. - Provides all available values for multiple value (ordered) attributes as an Object[] array. - Supports the Sun LDAP provider's ";binary" option to be appended to an attribute in order to get the value as an array of bytes. However, the ";binary" suffix is then also part of the attribute's name in the Principal's attributes map. See _Attribute Values_ at https://docs.oracle.com/javase/jndi/tutorial/ldap/misc/attrs.html - I only have access to a single Active Directory Server and did not manage to make it work with the `userPattern` option. **So, the _user attributes_ implementation in method `getUserByPattern(DirContext, String, String[], String)` is not yet completely tested!** Although I believe it should work as expected, **could someone with another directory server around test this case?** ### Defensive copies of attribute values As already discussed, the implementers of method `TomcatPrincipal.getAttribute(String)` always return a _defensive copy_ of the attribute's value. That is done by a couple of `instanceof`checks, implementing a _fastpath_ copy for well-known and commonly used types, like String, Integer, Date etc. For all other types, creating a copy through serialization/deserialization to/from a memory byte buffer is tried. That only works if all classes involved are serializable. Otherwise, the attribute value's string representation returned from its `toString()` method is returned. The _fastpath_ copy process used copy constructors where possible. However, as recently pointed out by Adam Rauch, many of those are deprecated as of Java 9 and will be removed in the future: https://www.oracle.com/java/technologies/javase/9-deprecated-features.html#JDK-8065614 So, I now use `valueOf()` for those as recommended by Oracle. Since for numbers there is a _number cache_, providing singletons for values smaller than 256, the whole defensive copying is quite pointless. Also, the _number cache_ is pointless or even dangerous, if someone could ever change the _native_ value of such an instance. **How shall we deal with the _immutable attributes problem_? Is the proposed solution over-secured or even paranoid?** ### Final notes - **Class `GenericPrincipal`: removed trailing comma from roles list in `toString()`method.** - Added new class `CaseInsensitiveKeyLinkedMap`: Extends class `CaseInsensitiveKeyMap` and uses a `LinkedHashMap` instance as its backing store. - Slightly modified class `CaseInsensitiveKeyMap` in order to be extendable easily. Also enhanced Javadoc comments to better distinguish between `...Map` and `...LinkedMap` versions. - **What about proposing to add these new _attributes methods_ in interface `TomcatPrincipal` to a new _official_ interface 'jakarta.servlet.Principal' (or 'jakarta.servlet.ServletPrincipal')?** -- 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
