Skip to content

SEC-3198: SecurityContextHolderAwareRequestWrapper#getRemoteUser ignores Authenticaion#getName #3357

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
spring-projects-issues opened this issue Jan 21, 2016 · 1 comment
Labels
in: web An issue in web modules (web, webmvc) type: bug A general bug type: jira An issue that was migrated from JIRA

Comments

@spring-projects-issues
Copy link

Michael Osipov (Migrated from SEC-3198) said:

The public method getRemoteUser tries to return auth.getPrincipal().toString() but there is no guarantee that the principal which is an arbitrary implementation will acttually return the remote user. The assumption that it does is wrong. Since Authentication implements Principal, it would be the safest bet to return auth.getName() which is by interface guaranteed to be a proper name.

@spring-projects-issues spring-projects-issues added in: web An issue in web modules (web, webmvc) Open type: bug A general bug type: jira An issue that was migrated from JIRA labels Feb 5, 2016
@tomikmar
Copy link

I have the similar issue. My application uses two types of login - OAuth2 and LDAP. This is the reason why I need to handle AuthenticatedPrincipal (DefaultOAuth2User) and UserDetails.

Current implementation of SecurityContextHolderAwareRequestWrapper.getRemoteUser() only supports UserDetails. It would be great if this method could also work with AuthenticatedPrincipal returning username instead of the whole object (toString), e.g. (alternative solution to the one above):

@Override
public String getRemoteUser() {
        Authentication auth = getAuthentication();

        if ((auth == null) || (auth.getPrincipal() == null)) {
                return null;
        }

        if (auth.getPrincipal() instanceof UserDetails) {
                return ((UserDetails) auth.getPrincipal()).getUsername();
        }
+       if (auth.getPrincipal() instanceof AuthenticatedPrincipal) {
+               return ((AuthenticatedPrincipal) auth.getPrincipal()).getName();
+       }

        return auth.getPrincipal().toString();
}

Similar approach is used in AbstractAuthenticationToken.getName().

@rwinch rwinch removed the Open label May 3, 2019
sjrd218 added a commit to sjrd218/spring-security that referenced this issue Nov 20, 2020
@jzheaux jzheaux closed this as completed in 9c373ef Dec 3, 2020
jzheaux added a commit that referenced this issue Dec 3, 2020
- Corrected instanceof check

Issue gh-3357
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc) type: bug A general bug type: jira An issue that was migrated from JIRA
Projects
None yet
3 participants