Skip to content

Conversation

cklein05
Copy link
Contributor

@michael-o Here's the patch to add support for additional user attributes to DataSourceRealm.

@rmaucher
Copy link
Contributor

Since I am looking at open PRs once in a while, I merged the pluming code for the user attributes with fd5b0fb
I added more of the code to RealmBase, since I couldn't figure out why realms would need to do something different (and if they somehow do overriding works ...).

Status update: Still not motivated by having to maintain the JDBC code though ...

@ChristopherSchultz
Copy link
Contributor

I kinda think this is out of scope for the container. There are loads of wonderful things we could add to GenericPrincipal and eventually Tomcat becomes an application framework.

When I started out with the servlet spec, it was clear to me that Principal wasn't going to be enough to handle "real" user information and so we load our own User object after authentication and stick it into the user's session. You can do whatever you want, there, of course, and it doesn't require container support, and remains portable between containers.

I might even recommend reverting fd5b0fb.

@michael-o
Copy link
Member

michael-o commented Aug 14, 2025

I kinda think this is out of scope for the container. There are loads of wonderful things we could add to GenericPrincipal and eventually Tomcat becomes an application framework.

When I started out with the servlet spec, it was clear to me that Principal wasn't going to be enough to handle "real" user information and so we load our own User object after authentication and stick it into the user's session. You can do whatever you want, there, of course, and it doesn't require container support, and remains portable between containers.

I might even recommend reverting fd5b0fb.

That would break some stuff. Is the gain worth the pain?

@rmaucher
Copy link
Contributor

That would break some stuff. I'd the gain worth the pain?

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants