UserDatabaseRealm does not rely on cached roles only

[cklein05]

The UserDatabaseRealm queries its UserDatabase in override hasRole() in order to return a correct result, if the passed Principal is a GenericPrincipal with an associated userPrincipal of type UserDatabasePrincipal. That userPrincipal more or less acts like a tag interface to determine whether that special handling is required. If not, the override calls its super method.

The UserDatabase can be updated through JMX at any time. Currently, such changes are taken into account instantly (at every invocation of hasRole()), which is different from other Realms and the UserDatabaseRealm's documentation.

Since the logged on user's effective roles are calculated and stored in the GenericPrincipal returned from method getPrincipal, these could be used instead. This eliminates both the hasRole() override as well as the private class UserDatabaseRealm.UserDatabasePrincipal and makes the Realm behave according to the documentation (and like e. g. DataSourcRealm).

Also, duplicates will be ignored when effective roles are calculated from the User's Roles and Groups, simply by using a HashSet instead of an ArrayList in method getPrincipal(String),

[rmaucher]

I'm quite sure this was done on purpose [want to be able to lock away someone instantly] but this probably does not make much sense as the behavior cannot become consistent. Lookup every time is ok for the memory db, would cause problems everywhere else most likely. It is not surprising this was not really acked in the documentation.

[rmaucher]

I pushed this in the main branch, but there the backport question since it clearly changes the behavior and it could hurt someone actually using the live updating capability.

[cklein05]

Rémy,

thanks for pushing this. Primarily, I like to get rid of class UserDatabaseRealm.UserDatabasePrincipal, which was introduced as a replacement for org.apache.catalina.User. Since User has a getPassword() method, it's no good candidate for being used as a GenericPrincipal.userPrincipal. So it was replaced by UserDatabasePrincipal, which is both private and not serializable.

I'm working on a enhancement that also extends GenericPrincipal . For that enhancement, things would be easier without having UserDatabasePrincipal around.

However, I understand that backporting may be a problem due to users using JMX to modify groups and roles in a live manner.

Couldn't we instead only drop UserDatabasePrincipal (which acts like a tag interface) and leave the live role lookup in place with older versions/branches?

[rmaucher]

Ah ok, I did that, since getPassword was a problem that needed fixing. However, I think this does not work and I'll have to revert this: I don't see how groups are still handled, ultimately this really needs access to the User object and its own override implementation.

[rmaucher]

I reverted to examine things a bit more.

I think groups should still work since the complete role list is constructed with their content in getPrincipal, however it also means things are very static: even if you take out a user from the admin group (for example), it will stay there until its session ends, or if you add a new role to a "manager" group, then it will take effect "eventually" and break things in the meantime. So it doesn't seem to me the "liveness" of the feature is so "undocumented": it should really be supported otherwise its design doesn't make much sense.

[cklein05]

Yes, it's static, but most other realms behave the same. You'll need to re-login in order to have role and group changes in effect. So, me and Mark were thinking removal of the live feature is appropriate.

Maybe there was an idea of making the XML-based UserDatabase a live updatable full blown database in the early days of Tomcat.

[rmaucher]

Well, actually the MemoryUserDatabase will auto reload.

I caught up to the thread on the user mailing list, which I didn't really pay attention to last week (I was away for a part of it). The problem is the whole point of the user database is to have live updating. If it's now ok to remove that feature, then the user db as a whole can (should) be removed since the classic memory realm would do just as well.

I'm really sorry for that fake principal trick that was created out of the need to hide away the objects and the password. I'll try to come up with something. Would using a UserDatabasePrincipal that would extend GenericPrincipal instead work out for you ?

[markt-asf]

I'm fairly sure extending Generic Principal would work. Tx.

[cklein05]

For me, it's not about removing UserDatabaseRealm's live role querying feature. It's just that I need a way to deal with the UserDatabasePrincipal in my enhancement. Removing this class is the simplest solution among others. A simple (default visible) boolean flag userDatabasePrincipal in GenericPrincipal will do the job as well (so that the UserDatabaseRealm can simply check that boolean value instead of the instanceof UserDatabasePrincipal test in line 144).

However, extending GenericPrincipal is another option that's as good as the other (maybe it's the more clean solution from an OOP point of view). I agree with any of these options, since in then end (in both ends) I don't have to touch that class anymore when implementing my enhancement.

[rmaucher]

I commited a compromise as d1ffc30

[cklein05]

Thanks for that. I'm sure I can work well with that compromise.