On 27/05/2021 10:59, Mark Thomas wrote:
<snip/>
As far as I can tell, removing UserDatabasePrincipal, relying on
GenericPrincipal and User remaining an internal object not exposed via
the Servlet API would achieve the same result with less code.
At this point I am looking for a reason not to remove
UserDatabasePrincipal and I'm not seeing one.
Since User and Role instances support dynamic updates (have a look at
methods addGroup/Role and removeGroup/Role), the special handling in
UserDatabaseRealm.hasRole() is required. Although that feature is
currently not used by Tomcat (is it?), those possible live updates must
be taken into account in the code.
If the UserDatabase was immutable, we could simply go ahead with the
pre-resolved effective roles list and GenericPrincipal. But actually,
those effective roles may be wrong after an update of the UserDatabase.
So, I suggest not resolving effective roles up front in
UserDatabaseRealm.getPrincipal() and use null as the roles list when
constructing the new GenericPrincipal instance.
The special handling in UserDatabaseRealm.hasRole() is triggered, if the
Principal is a GenericPrincipal with a userPrincipal of type
UserDatabasePrincipal:
if (principal instanceof GenericPrincipal) {
GenericPrincipal gp = (GenericPrincipal) principal;
if (gp.getUserPrincipal() instanceof UserDatabasePrincipal) {
principal = database.findUser(gp.getName());
}
}
The current test requires an extra UserDatabasePrincipal class. Making
that class support attributes and serializable is possible.
However, a simple default/package visible boolean flag
'userDatabasePrincipal' in GenericPrincipal would work as well and lets
us finally drop class UserDatabasePrincipal.
I think it would be worth handling this is a separate commit to give
folks the chance to review it before proceeding to add attribute support.
+1
Are you talking about a separate commit in my additional-user-attributes
branch? Or is that worth an extra BUG/case/issue?
I suspect the users that worry about that sort of thing aren't using the
UserDatabaseRealm but it would be nice to fix that anyway.
That will be fixed after having added attribute support either way.
Carsten
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org
