microsoft / azure-spring-boot

Spring Boot Starters for Azure services
MIT License
374 stars 460 forks source link

principal.getName() returns the default toString instead of the principals name #736

Closed mcarleio closed 3 years ago

mcarleio commented 5 years ago

Environment

Summary

When using the AADAppRoleStatelessAuthenticationFilter or AADAuthenticationFilter and you call principal.getName() you will not get the name, as the default Object.toString() is called, resulting in something like 'com.microsoft.azure.spring.autoconfigure.aad.UserPrincipal@4b6c8ec6'

Reproduce steps

  1. Use AADAppRoleStatelessAuthenticationFilter or AADAuthenticationFilter
  2. Inject the Principal during a request: e.g. @GetMapping("name") public String name(Principal principal)
  3. Call principal.getName()

Expected Results

Expected would be the name of the UserPrincipal. So something like 'John Doe'.

Actual Results

Something like 'com.microsoft.azure.spring.autoconfigure.aad.UserPrincipal@4b6c8ec6'

saragluna commented 5 years ago

Thanks for reaching out. The name of the UserPrincipal could be retrieved by calling this method. And you may get the UserPrincipal object like this.

mcarleio commented 5 years ago

Yes, that's right, but when you are working with various principals (e.g. users may also sign it with username & password), then you do not want to have to check every time if the principal is a PreAuthenticatedAuthenticationToken or something else. But yeah, there are ways to retrieve the name, so it is not a super high prio bug ;-)

saragluna commented 5 years ago

Yes, you are right about it. This solution is not a proper way to retrieve the name . We will evaluate this and come back to you later.

cmplank commented 5 years ago

It has been my experience, from several years of working in Spring apps, that principal.getName() returns a unique identifier like username or email address. See: https://docs.spring.io/spring-security/site/docs/current/reference/htmlsingle/#obtaining-information-about-the-current-user

Returning the "name" JWT claim, which is just a user's print name, would be pretty useless in my opinion. I would recommend returning the "upn" claim (if it exists on the JWT) or "oid" claim. The important point being that it is unique and can be used to lookup other user info as needed.

I would be interested to see what the folks at Spring would recommend.

dvut commented 4 years ago

Any chance that this issue might be fixed eventually?

Which attribute to return should follow the userNameAttribute / user-name-attribute setting I guess. That setting is currently also broken however, see: #901

yiliuTo commented 4 years ago

Hi, @dvut . We can discuss your question in #901 , which is not the same as the issue here.

chenrujun commented 3 years ago

Closing this issue. Because it's not active for a long time. If anyone have similar issue, please create issue in new repo: https://github.com/Azure/azure-sdk-for-java/issues