Display AD Login Info Not Static String #46
Conversation
Rather than displaying the string “Azure AD” for all users with connections to AAD, set the “Provider Login” value to the Displayable ID from the AD info. Fixes the new requirement for a unique value in SonarQube 7.2 and makes it easier to see what AD account is associated with a specific SQ user.
| @@ -127,7 +127,7 @@ public void callback(CallbackContext context) { | |||
|
|
|||
| UserInfo aadUser = result.getUserInfo(); | |||
| UserIdentity.Builder userIdentityBuilder = UserIdentity.builder() | |||
| .setProviderLogin(getName()) | |||
| .setProviderLogin(aadUser.getDisplayableId()) | |||
There was a problem hiding this comment.
This PR could also be a good opportunity to use the new API to define the provider ID (https://github.com/SonarSource/sonar-enterprise/blob/master/sonar-plugin-api/src/main/java/org/sonar/api/server/authentication/UserIdentity.java#L68), but using Sonarqube 7.2 dependency and adding :
.setProviderId(aadUser.getUniqueId())
(even if by doing that I understand that it will break compatibility of this plugin with previous version of SonarQube)
There was a problem hiding this comment.
I don't personally think breaking compatibility with older versions is a bad thing, the older version of the plugin will continue to work with older versions of SQ.
I'd be very happy to see this plugin get enhancements like getting the group sync working, moving off the deprecated features in the 6.7 API (let alone to base on a newer API), etc. My fix was more just a quick patch to get a version that will work on 7.2 so I can have a successful upgrade and maybe other users can as well.
Rather than displaying the string “Azure AD” for all users with connections to AAD, set the “Provider Login” value to the Displayable ID from the AD info. Fixes the new requirement for a unique value in SonarQube 7.2 and makes it easier to see what AD account is associated with a specific SQ user.
Fixes #45