Architecture of User, UserDetails, Principal

[RobinKamps]

The current implementation of principal/security.authentication is not well designed to handle different setups and different usecases. The insufficient internal design is leading to many related issues which may appear more on the "surface". These issues include the oauth/keycloak integration as well as the jwt part, where the sub is not delivered as a payload. It is related to the issues #7065, #7008 and to the jwt counterpart and the "remove the login" part e.g. #6987. Since this covers a lot of related issues this issue is a bit long - sorry for that.

Background/Motivation: I have refactored JWT to use the id as sub for the plain jwt setting and i have analyzed/refactored the current implementation of oauth as well.

Looking on the different generated implementations (jwt and oauth) in different setups (monoliths and microservices), it is quite obviuos that JHipsters current strength is more to configure and integrate different technologies than to provide a production ready "framework". The User / Principal part is where JHipster lacks the most from an architectural point of view. The user entity is immanent to all applications and it should be more easy to generate it for different usecases and to use it more convenient in different setups (usecases: username or no unique username, setups: jwt or oauth, in conjunction with monoliths or microservices and also different types of microservices (where relations maybe managed to the users directly or not e.g.).

As already mentioned, there are currently a couple of issues, wich are somehow related, because they affect the JHipster implementation of User. However they should not be seen as separated issues. Otherwise fixes will only be on the surface. It will be better to get the underlying causes fixed an come up with an architecture that really serves as a foundation, covers the most common usecases and is easy extendable.

Details: In the current JHipster implementation the login is used as if it was a primary key and the login is used as a foreign key, too. The login is used as principal.getName() - wich is the right place to put the current username for display purpose. org.springframework.security.core.userdetails.User is used as principal instead of a domain User (jwt/oauth implementation is a bit different in some places (no UserDetailsService Implementation e.g.) However security.core principal and getName/UserDetails were not designed to be used as an identifier/pk/fk or to be stored to and fetched from a db. The Principal.getName()/login is also used as a foreign key (instead of sub/id) at various places of the current implementation (i can not recall them all, but e.g. for oauth/keycloak integration #7065 the user audits will be broke, because login is used twice as a fk (hibernate default implementation of @ CreatedBy and @ LastModifiedBy calls principal.getName in AbstractAuditingEntity, thus login - perhaps this could/should be refactored to use javers to be db independent and to have an easy way to restore a whole entityGraph for any point in time).

OAuth/OpenId specific explanation:

To perhaps understand the recommended solution more easily here are some example logs from the current keycloak/JHipster implementation. At the moment when a new user "test" from keycloak logs in to JHipster, the token is carrying the payload: Details{sub=cc8234b4-6137-4fc2-8593-805481bf45fc, roles=[ROLE_USER, offline_access, uma_authorization], preferred_username=test, email=test@test.de}

Then a user is created/synchronized via preferred_username to login wich is wrong (this is what #6677 and #7008 is all about). The token details are mapped to a new User (a jhi_user) and the user is stored to db without referencing the sub: User{login='test', firstName='null', lastName='null', email='test@test.de', imageUrl='null', activated='false', langKey='null'

Note that you have to set up keycloak/JHipster to fetch the missing/null attributes, if you need them in JHipster (or just let the user add them just in JHipster/not manage them by keycloak for imageUrl e.g.), but you will most likely add custom attributes anyway (langKey, phonenumber, etc.).

When you call the principal via getCurrentUserLogin you get the spring default user implementation with no id/sub etc. and you cannot cast it to JHipster User: Principal: org.springframework.security.core.userdetails.User@364492: Username: test; Password: [PROTECTED]; Enabled: true; AccountNonExpired: true; credentialsNonExpired: true; AccountNonLocked: true; Granted Authorities: ROLE_USER; Credentials: [PROTECTED]; Authenticated: true; Details: null; Granted Authorities: ROLE_USER

Please note that besides the coincidental usage of org.springframework.security.core.userdetails.User and the JHipster User, the JHipster User is not always used in the same context: Of course the JHipster User is used to fetch and store users from/to a db in UserService for JWT (and synchronize for OAuth). org.springframework.security.core.userdetails.User is used when the principal comes in play (and this will be generated from a token (jwt) not fetched from db). Sometimes there happen things behind the scenes wich i suggest could be kind of missleading at calling points in different setups (e.g. private static User getUser(Map<String, Object> details and the creation of the UserDTO) in the UserService on OAuth) - because the returned objects are of type User or UserDTO and could be to easily assumed to be a "real", "persisted" jhi_user object (an "verified" entity read from db), but it will be constructed from token only.

Furthermore this issue is not only related to jwt/oauth integration, but has another dimension to it: Different setups (monolith/microservices - what issue #7065 is about, too (point 3.)). The current usage of principal is not good, because if you want get the id from a user you always have to query the db (or a feign client to request it from gateway/uaa etc.) to look the user up by login (should be sub/id). This db lookup might be ok in a monolith, because you have direct access to the db containing the jhi_user. On a microservice setting it depends on the usecase, where to manage the users/store relationships to them. You will most likely add and sync the user to a few microservices, just because inter db joins are not possible (especially if these microservices are using different db types (sql/nosql) or if this feature is not implemented in the db engine). Even if you can do them like on oracle, or with custom in-memory joins you will avoid them and just (e.g. for jwt) setup kafka or rabbitMQ to sync the users from the uaa with all microservices, just sync the user tables per db log if your db engine supports that or provide your custom sync mechanismen as it is now the case with oauth (Note: Perhaps it would be better to turn on and listen to all administration messages regarding the user synchronisation from keycloak (at the moment the synchronized users in jhipster do not contain all existing keycloak users for the realm: they can exist in keycloak, but are not synchronized, if they have not signed in to jhipster. Furthermore you might have old data in jhipster, if the user changes data in keycloak and does not login to jhipster app afterwards). However with jwt it is essential (security wise) to make a db/lookup for each call and determine if the user is still active and has the GrantedAuthorities he claims to have. On all other implementations of rbac or acf i know, there are always automatic db queries to match the user from session or jwt for route protection usage - but not on jhipster/JWT (since this is more a generator and "no batteries included" framework here). So you might have two different kinds of microservices:

1. microservices wich may rely only on the claims in the tokens and not store the users (they can use feign clients etc. to establish or fetch user relations and details).
2. microservices wich are providing more confidential services or wich will have a heavier usage of joins to users e.g. and therefore store/sync users.

Solution: User should implement the UserDetail interface. This will change/add some attributes causing breaking changes. Best way is to create an interface e.g. DomainUser wich inherits from UserDetails and expands the other now missing attributes (id, etc.). Then this interface should be implemented by classes wich suite the usecase (i created JWTUser and PersistenceUser - a class OAuthUser could easily be added), just to have a cleaner architecture and make it more obvious/separated wich kind of user is dealed with at the moment. For the jwt setting the Authorities must also be refactored to implement GrantedAthorities to fit the UserDetails interface. For jwt the UserDetailService implementation needs to be changed to return a DomainUser instead of org.springframework.security.core.userdetails.User. For OAuth the way the current principal is created in security/getCurrentUserLogin has to be changed - this should be refactored to getCurrentUserId anyway (of course getUserFromAuthentication and so on must be refactored as well). This way you will always be able to typecast the principal and fetch the id/uuid from it (- as long it is not the anonymousUser/of type String).

For my jwt adoption i have set up the userDetailsService implementation to look up the sub/user from db and on authentication the users claims were looked up in the db for each rest call - as i need it to be secured by default (this results in a db query for user with all authorities on each request (just like a normal session db handler). Amongst increased security the payload of the jwt can be decreased to just sub and expired e.g..

Regardless of jwt/oauth token only or db lookup: it always should be possible to cast the security authentication / principal to the appropriate user implementation of UserDetails if needed (jwt example from my implementation here): org.springframework.security.authentication.UsernamePasswordAuthenticationToken@5b1afff3: Principal: User{id='3', firstName='Administrator', lastName='Administrator', email='admin@localhost', imageUrl='', activated='true', langKey='en', activationKey='null'}; Credentials: [PROTECTED]; Authenticated: true; Details: null; Granted Authorities: ROLE_USER, ROLE_ADMIN

However it would be good to exploit oop for the authentication implementation as well: Extract interfaces for jwt-/oauth- authentication and provide implementations. One implementation for plain token usage (user gets created from token, no db lookup, higher token payload) and one implementation for db usage (db lookup each time, less payload, more secure, user have to be synchronized on microservice setup). This way also an implementation with a feign client lookup could be easily added.

If the architecture is setup like above, JHipster would be more "framework" as right now. It will be more easy to swap in the desired implementation or customize it from there on (e.g. JWTUser with Long id or uuid) As the login is not used as a pk/fk anymore and an unique username is still a common and valid usecase, it could be optional. Furthermore in the future the microservice generator could be extended to ask, if they should store a synchronized user, and perhaps even generate a uaa setup to push user changed events to a messaging system of your choice, or microservice can setup to autosync users with oauth etc. The suggested solution is also flexible enough for future additions, perhaps jhipster-needles/hooks to the persisteduser implementation to generate custom properties and relations.

[deepu105]

LT; DR; PR for improvements are always welcome. Since we are working on v5 the core team is tied up, and if you want to change this, it is the right time as we can afford to move things around and break things if required for v5 Since I'm not a security expert I'm pinging @mraible @jdubois @ruddell @xetys

[gmarziou]

This looks good to me, maybe be sharing some code would help to better understand. I see it also as an opportunity to enable easier extension of User or adding relationships to it.

JHipster v5 is the right target for these changes.

Regarding our JWT implementation, few things that could be good to implement:

• Token should contain issuer and we should provide an access to its value
• JWT secret could be replaced by a key pair with only the public key used by microservices while private key is only used by gateway.

[jdubois]

No time to read this at the moment but yes it's the right time to modify this. Lots of code I coded originally without knowing things like JWT or even entity relationships would exist. So yes there's room for improvement.

[mraible]

@RobinKamps Can you simplify this issue into a few bullet points? I believe I'll fix most of the OAuth issues with https://github.com/jhipster/generator-jhipster/issues/7065.

[RobinKamps]

@mraible Yes your issue #7065 is highly related to the OAuth side of this issue, regarding the sub/id and managing the user on microservices. The solution proposal from #7065 of "changing the id property of a user from a Long to a String" is the same as suggested here, but it maybe not as easy as it may look like, mainly because of the current usage of login as a pk/fk and the current principal (and related audits etc.).

If you want to get the user from security.authentication you have to call getCurrentUserLogin from SecurityUtils at the moment. If you refactor this to getCurrentUserFromId the main problem is, that in SecurityUtil org.springframework.security.core.userdetails.UserDetails instead of a DomainUser is used and therefore has no id attribute (and no langKey, firstName, lastName e.g.). This problem also occurs on pure jwt.

The here proposed solution (implement and extend UserDetail interface for a DomainUser) is a way to be able to cast the principal to a domain user (with id, firstName, lastName, langKey etc.) for both, jwt and oauth.

The proposal includes to perhaps provide different implementations for the DomainUser - persistedUser, and different TokenUsers (perhaps JWTUser, OAuthUser and perhaps BasicJWTUser with less payload etc.). This could be a good way in conjunction with providing different implementations of the authentication mechanismens and mechanismen how to get the userdetails (just from token, with a feign client or from db) to cover the most common usecases/setups for all combinations (jwt or oauth / monolith or microservices).

[RobinKamps]

@gmarziou sorry i can not provide a clean showcase code at the moment.

A few weeks ago i messed around with jwt and removed the login and replaced it with email and refactored a few things regarding the principal (implement UserDetails in User, change UserDetailServiceImplementation and jwt TokenProvider to always lookup the user from db etc.)

But this was just a quick and dirty refactoring mainly to see what is possible and where will be obstacles.

If you want to take a look at it, be warned:

• the UserDetail implementation of User is just a fake
• the implementation of GrantedAuthorities is nonsense, too.
• the User audits are still wrong (still using AbstractAuditingEntity @ CreatedBy wich calls getUsername - was setup to return email here, due to long id / String ugly conversion).
• tests will fail (just removed the login on them) and some routes will fail (due to login removal /elasticsearch nodes not found etc.).
• the repo does not include interface based design, so no interface for DomainUserDetails and no different authentication mechanisms, etc.

However

• the principal can be casted to User
• the login is removed, sign in via email works
• it features some debug logs with many hashtags to find them easily :-)
• the token contains the id as payload and
• some routes are changed from login to id (where login was part of the url like on account etc.)

So if you run it and sign in via email you will see the security.authentication / principal is a jhi_user instead of a spring user in the log.

https://github.com/RobinKamps/jhipster_test_port

PS: On windows i get an compile error regarding the health endpoint configuration, wich was not there three weeks ago. Just remove "endpoint: health: show-details: true" in application.yml will work as a quick fix.

[Blackdread]

Hi @RobinKamps,

I am not using OAuth neither OpenId but only JWT, jHipster used to have a custom implementation of UserDetails HERE but was removed.

If only using JWT on monoliths, creating a custom UserDetails is very easy, data inside the JWT token can be added as well (see Token Provider) like user id, etc. Both methods "createToken" and "getAuthentication" needs to reflect those changes. With those changes you can also use mvc-authentication-principal from Spring even though SecurityUtils can be modified to return user id and any other field from the custom UserDetails.

Instead of issuing db lookup for each request, you could create a service class to contain id/login (unique immutable attribute) that only keep users that should be checked again as their roles or any security related data has recently changed then do a db lookup. It would at least reduce the overhead of db lookup for all users for each request. You could also :

• reduce the time validity of JWT
• keep a unique id for each JWT, store it and invalidate it in a specific service (checked at each request) Those have their pros and cons but many solutions are available.

What you described is great and would allow to get a generated jHipster app with more use cases but some of those are very easy to implement once the app is generated. JHipster tends to not be too much opinionated on very specific implementation details but allowing more options would be great.

[Tcharl]

+1 for making an inheritance from User to UserDetails. In 99% of the use cases, the user that's loggued in is the same as the one that is authorized to consume endpoints :-).

[gzsombor]

No, please do not try to put JPA entities into http sessions, this is a recipe for disaster. (Entities can refer each other, and when the http server serialize the session - for scalabiltiy reason, to other servers, or to file system, etc)

Having a CustomUserDetails is a better solution, I think.

[Blackdread]

Yes CustomUserDetails is better. JPA in sessions might fetch other relations or have lazy exception (hibernate)

[deepu105]

@jdubois I dont think anyone is working on this. @jhipster/developers is anyone willing to work on this else lets close it as 'wont-fix'

[jdubois]

I'd love to, and this is very important, but I'm just lacking time - a big part of my free time is taken by JHipster Conf, and also by personal matters (yes, I also use my free time for personal things :-) )

[jdubois]

I'm sorry but as @deepu105 says nobody is working on this. While I totally agree our current architecture is not good (as I coded this quickly a long time ago, and nobody ever reviewed it before), it currently "does the job", and the proposal here is so complex that nobody dares to start doing it.

My recommandation would be to propose those changes step by step: for example, if you want to add inheritance from User to UserDetails, let's do this on a specific ticket, and do a simple PR for it.

This comes from the way the project works: nearly everybody works here on their free time, so it's nearly impossible to do something that complex, and it has to be broken into smaller parts.

So I'm closing this as "won't fix" because that's really what happens at the moment, but I'm totally for doing changes here. They just need to be smaller and easier to understand for everyone.