Michael Osipov (Migrated from SEC-3199) said:
If you have configured to use security:jee and likely expect request#getUserPrincipal to return the original principal created by your container, you are out of luck.
Preauth implementation completely removes the ability to access the original principal. Disabling servlet-api-provision gives you access but the employed Authentication implementation is not aware of and we have a disparity. It should be either resolved or at least documented to be problematic.
Comment From: michael-o
Is there any remedy to the issue after five years?
Comment From: jgrandja
@michael-o The original issue comment is actually not valid:
If you have configured to use
security:jeeand likely expectrequest#getUserPrincipalto return the original principal created by your container, you are out of luck.
The javadoc for J2eePreAuthenticatedProcessingFilter is:
This AbstractPreAuthenticatedProcessingFilter implementation is based on the J2EE container-based authentication mechanism. It will use the J2EE user principal name as the pre-authenticated principal.
Furthermore, the javadoc for J2eePreAuthenticatedProcessingFilter#getPreAuthenticatedPrincipal() is:
Return the J2EE user name.
J2eePreAuthenticatedProcessingFilter was never designed to return the original Principal but instead the name of the Principal via httpRequest.getUserPrincipal().getName().
I'm going to close this issue as invalid.
Comment From: michael-o
The sources you cited are a contradiction. Principal != name. This code will handle the proper case well:
https://github.com/spring-projects/spring-security/blob/006b9b960797d279b31cf8c8d16f1549c5632b2c/web/src/main/java/org/springframework/security/web/authentication/preauth/AbstractPreAuthenticatedProcessingFilter.java#L162-L173
I don't understand why one should lose information due to a doc/source code mismatch.
Moreover, AbstractPreAuthenticatedProcessingFilter almost always operates on the principal object rather the name representation.
Quite disappointing.
Comment From: jgrandja
@michael-o
The sources you cited are a contradiction.
Principal != name.
I don't see how? The first conditional check if ((principal instanceof String) && currentAuthentication.getName().equals(principal)) evaluates to true based on the implementation in J2eePreAuthenticatedProcessingFilter.getPreAuthenticatedPrincipal().
Moreover, AbstractPreAuthenticatedProcessingFilter almost always operates on the principal object rather the name representation.
Authentication.getName() OR httpRequest.getUserPrincipal().getName() should represent a unique identifier of the Principal and when it's passed into AuthenticationManager via AbstractPreAuthenticatedProcessingFilter, it should always operate on the same Principal, whether it's a full Principal (or Authentication) Object or a String representation of Principal name. Again, the current implementation is by design.
If you feel an improvement can be made, we are open to suggestions. And given that there hasn't been much movement on this particular issue, a PR submission would help move this forward.
Comment From: michael-o
@michael-o
The sources you cited are a contradiction.
Principal != name.I don't see how? The first conditional check
if ((principal instanceof String) && currentAuthentication.getName().equals(principal))evaluates totruebased on the implementation inJ2eePreAuthenticatedProcessingFilter.getPreAuthenticatedPrincipal().
My comment solely referred to J2eePreAuthenticatedProcessingFilter#getPreAuthenticatedPrincipal(). The comment says name, but the the method says ...Principal and not name. This isn't equal. A Principal is a non-string object.
Moreover, AbstractPreAuthenticatedProcessingFilter almost always operates on the principal object rather the name representation.
Authentication.getName()ORhttpRequest.getUserPrincipal().getName()should represent a unique identifier of thePrincipaland when it's passed intoAuthenticationManagerviaAbstractPreAuthenticatedProcessingFilter, it should always operate on the samePrincipal, whether it's a fullPrincipal(orAuthentication)Objector aStringrepresentation ofPrincipalname. Again, the current implementation is by design.
While I don't disagree with this, I would expect that the (original) principal object shall be available at all times, not just a string name. In my usecase Tomcat performs CMS and provides an ActiveDirectoryPrincipal. This principal contains futher attributes from Active Directory. If the principal object is lost, I don't have access to the data anymore. I don't want Spring Sec to perform any more lookups because this is done by Tomcat already.
If you feel an improvement can be made, we are open to suggestions. And given that there hasn't been much movement on this particular issue, a PR submission would help move this forward.
I need to look more deeply into this. Haven't touched Spring Sec for years because the current code is running with those limitations and workarounds.