sonar-auth-aad
sonar-auth-aad copied to clipboard
hkamel
Support for Microsoft Graph for login instead of AAD Graph
According to Microsoft, there is no addition support being added for AAD Graph and has not been since July 2020:
Starting June 30th, 2020 we will no longer add any new features to Azure AD Graph API. We strongly recommend that you use Microsoft Graph API instead of Azure AD Graph API to access Azure Active Directory resources.
which then links to https://techcommunity.microsoft.com/t5/azure-active-directory-identity/update-your-applications-to-use-microsoft-authentication-library/ba-p/1257363
Are there any plans to update the plugin to use Microsoft Graph API?
I'm currently working on some updates to the plugin that will drop the deprecated ADAL4J library and move to a more generic OAuth library, as MSAL4J is overkill for this project. With that move, there will also be a move to the MS Graph API. Unfortunately, this also necessitates a larger refactor, and I'm basing the new structure on the GH and similar OAuth-style plugins to allow better testing instead of having all the logic in the callback.
That said, Microsoft is not removing the older AAD Graph API, they just aren't adding anything new to it. As we're only using existing functionality, I don't expect any problems continuing with the old system until I can land my private work once it's in a semi-usable state.
Any update on this?
I'm getting complaints from tenant owner about ADAL usage. https://docs.microsoft.com/en-us/azure/active-directory/develop/msal-migration
If you choose not to migrate to MSAL before ADAL support ends in December, 2022, you put your app's security at risk. Existing apps that use ADAL will continue to work after the end-of-support date, but Microsoft will no longer release security fixes on ADAL.
It's in the pipeline.
Since that post, the place I work has gone through a lot of changes and left me with little time to handle this plugin. Since the plugin continues to work, it's been a bit lower priority. I still have my earlier work and hopefully will have some time coming up where I can concentrate on finishing that work and getting it landed.
I did publish a new version yesterday with some changes that had been contributed earlier and other cleanup I had been doing in preparation for swapping out ADAL4J.
@srvrguy Any update on when these changes are going to be available considering ADAL support is going to end in December 2022?
I don't have a timeline currently. Work is progressing, and mainly I have to finish group sync support and compatibility (with the user data ADAL4J provides). That's the short of it. A longer explanation follows:
First, on the code. I know that MSAL4J is marked as the successor to the ADAL4J project. The thing is that MSAL4J is extreme overkill for what this plugin does, which is just allowing SSO. MSAL4J is very much positioned to be part of code where you're doing a lot more than just that, and the complexity shows. Since there is a migration path for our use, I did attempt to use this. Unfortunately, it kept resulting in strange errors. I couldn't determine if it was something I was doing or just how SonarQube works. Eventually, I settled on just implementing the SSO manually using the Nimbus OAuth SDK. This has gone a lot smoother and allowed me to also add in some extra security like checking the tokens to match the audience ID and that the returned data is what we should expect. Unfortunately, I also have to implement functionality that ADAL4J was providing, like parsing out the user info from the tokens we get. To keep compatible with existing installs, I also have to ensure that it's basically doing it the exact same way. Likewise, once I get the user portion settled, I have to review the group sync functionality and ensure it's still working. That part is handled outside ADAL4J, so I hope that it will be a matter of simply uncommenting the code.
Second, on my time for this project. I started contributing to this project when the company where I was working started to implement Sonarqube scanning on the main product it offered to clients. Group support was one of the major draws along with the fact that I didn't have to manage a whole separate account structure. Since the time I began contributing, the company where I work has changed twice, both as a result of an acquisition. The first time wasn't very disruptive as the acquiring company mainly wanted the product to launch their own services. With the second acquisition, a much larger company acquired the combined company and I've been slowly integrated into the much larger DevOps group at this place. It's resulted in a large loss of time for projects like this as I learn and train on the way this company handles things. I'm glad that I can still work on this project with their knowledge, but work on the stuff that keeps them in business takes priority (and rightly so) over volunteer work.
I really hope to be able to get the ADAL4J removal complete before the discontinuation of that project, but this plugin will continue to work even after that date. It is just that, as time passes, it will require more understanding of what possible security issues continuing to use that component will bring up.
Update on progress:
Group sync using the default group sync method (using the access token from the user) is working from my limited tests. I haven't implemented the newer client credential method yet. The retrieval is using the MS Graph Java SDK, so the plugin doesn't need all the custom JSON parsing and pagination code any longer.
Once group sync is complete, I'll post the work publicly on the develop branch for anyone that wants to review and suggest changes. All that will be left are compatibility tests and updating/creating unit tests for the new code.
Hello, any news on this issue? Has it been fixed in the latest release (1.3.2)? Thanks in advance!
1.3.2 is only a patch release that updated commons-text to resolve a CVE. The changes for dropping ADAL4J and moving to MS Graph is large enough that it will be designated as version 2 when it's released. I don't have the work in this repo right now, but probably will soon as an update on the develop branch. You can take a look at the "feature/oauth-rewrite" branch over at the srvrguy/sonar-auth-aad fork to see my in-progress work. Once it's merged here I'd be happy to accept any improvements, especially in increasing unit testing.
Thank you so much for the information 👍 I look forward to version 2
Updated code is now in the develop branch. I'd appreciate users testing this code in their installs, if they can. I've done some preliminary tests against two different setups, but I know there's a lot of variation in setups out there. If you can't compile, you can download a binary from the action artifact. Just go to the Test and Package action and select the newest run in the "develop" branch.
I have downloaded the plugin binary from your Actions pipeline and I have installed it in my SonarQube instance, but I get this error when I try to authenticate:
You're not authorized to access this page. Please contact the administrator.
Reason: class com.microsoft.graph.models.DirectoryRole cannot be cast to class com.microsoft.graph.models.Group (com.microsoft.graph.models.DirectoryRole and com.microsoft.graph.models.Group are in unnamed module of loader org.sonar.classloader.ClassRealm @26865482)
Do I have to add any other API permission to my Azure app registrations? Before using this beta version of the plugin, the plugin worked only with this API permission:
- https://graph.microsoft.com/Directory.AccessAsUser.All
After the first login using the beta version, the login screen has let me add these API permissions:
- https://graph.microsoft.com/email
- https://graph.microsoft.com/openid
- https://graph.microsoft.com/profile
- https://graph.microsoft.com/User.Read
That is odd. In my tests, I've only needed to allow two permissions for MS Graph: User.Read (needed for basic login) and Directory.Read.All (for group sync). Both permissions are delegated type for the traditional flow. This is on an application I created (via the portal) only three weeks ago for testing to make sure that the setup process wasn't vastly different for new registrations. I do have the ID tokens enabled under the authentication settings, but I also have a test app with no permissions that works okay for basic sign-in and doesn't have that option enabled.
My test AAD is on the Free tier and doesn't have any customizations made to it.
Does the web.log show anything interesting in it? I've added a bunch more logging so when things fail it might provide more help in troubleshooting. If you post lines, please be sure to redact any sensitive info including any GUIDs (to be safe).
Hey @srvrguy , We've tested the "develop" plug-in and it's working well for us. That said, 1 of our users also experienced the same error as @vittoriocanilli, so it might be worth investigating.
In the logs, I can see something about an invalid cookie just before the java error, but nothing else:
2022.11.23 00:44:55 INFO web[AYSh7FcWMreCVXMYAAAM][o.a.t.u.h.p.Cookie] A cookie header was received [PS_DEVICEFEATURES=maf:0 width:1920 height:1080 clientWidth:1919 clientHeight:937 pixelratio:1 touch:0 geolocation:1 websockets:1 webworkers:1 datepicker:1 dtpicker:1 timepicker:1 dnd:1 sessionstorage:1 localstorage:1 history:1 canvas:1 svg:1 postmessage:1 hc:0;] that contained an invalid cookie. That cookie will be ignored.\n Note: further occurrences of this error will be logged at DEBUG level.
2022.11.23 00:46:37 ERROR web[AYSh7FcWMreCVXMYAABF][o.a.a.a.AadIdentityProvider] Exception:java.lang.ClassCastException: class com.microsoft.graph.models.DirectoryRole cannot be cast to class com.microsoft.graph.models.Group (com.microsoft.graph.models.DirectoryRole and com.microsoft.graph.models.Group are in unnamed module of loader org.sonar.classloader.ClassRealm @740dcae3)
2022.11.23 00:46:37 ERROR web[AYSh7FcWMreCVXMYAABF][o.a.a.a.AadIdentityProvider] Exception:org.sonar.api.server.authentication.UnauthorizedException: class com.microsoft.graph.models.DirectoryRole cannot be cast to class com.microsoft.graph.models.Group (com.microsoft.graph.models.DirectoryRole and com.microsoft.graph.models.Group are in unnamed module of loader org.sonar.classloader.ClassRealm @740dcae3)
Thanks!
Okay. That's interesting. Can you give me some information on your setup and also that user? Perhaps there's something specific on that user's account that's causing this. For example, guest/invite accounts that are homed in a different directory do behave differently.
I could not find any explanation why this error occurred for this user. He can now log in successfully. So I guess version 2 can be released :)
That is odd. In my tests, I've only needed to allow two permissions for MS Graph: User.Read (needed for basic login) and Directory.Read.All (for group sync). Both permissions are delegated type for the traditional flow. This is on an application I created (via the portal) only three weeks ago for testing to make sure that the setup process wasn't vastly different for new registrations. I do have the ID tokens enabled under the authentication settings, but I also have a test app with no permissions that works okay for basic sign-in and doesn't have that option enabled.
My test AAD is on the Free tier and doesn't have any customizations made to it.
Does the web.log show anything interesting in it? I've added a bunch more logging so when things fail it might provide more help in troubleshooting. If you post lines, please be sure to redact any sensitive info including any GUIDs (to be safe).
Hello @srvrguy, here is what I can find in my logs when I try to log in:
2022.11.29 08:59:20 ERROR web[AYSkxRipJtoNEpdUAILX][o.a.a.a.AadIdentityProvider] Exception:java.lang.ClassCastException: class com.microsoft.graph.models.DirectoryRole cannot be cast to class com.microsoft.graph.models.Group (com.microsoft.graph.models.DirectoryRole and com.microsoft.graph.models.Group are in unnamed module of loader org.sonar.classloader.ClassRealm @26865482)
2022.11.29 08:59:20 ERROR web[AYSkxRipJtoNEpdUAILX][o.a.a.a.AadIdentityProvider] Exception:org.sonar.api.server.authentication.UnauthorizedException: class com.microsoft.graph.models.DirectoryRole cannot be cast to class com.microsoft.graph.models.Group (com.microsoft.graph.models.DirectoryRole and com.microsoft.graph.models.Group are in unnamed module of loader org.sonar.classloader.ClassRealm @26865482)
I can try to adjust the permissions of my Azure app registration to have it matching yours.
@srvrguy I have just added the same permission in my Azure app registration as yours and I have removed the permissions that I had before, but I still get the same errors when I try to log in.
That's odd. Something must be different, and I'd like to at least handle the issue a bit more sanely. It's obviously happening when communicating with MS Graph, but the problem is finding out the cause.
@vittoriocanilli, if you look at my code commits, you'll see an e-mail address. Can you contact me on that? I can provide a build with a bit more debugging/logging so I can at least try to track down the exact code area where it's failing.
I found the issue and there's a new automated build over at the GH action. If you were having that issue, please try the new snapshot.
Hi @srvrguy, thanks for your messages and sorry for the late reply.
I have managed today to setup a new environment with the latest build that I got in the GH action pipeline of your last message and I can confirm that the authentication works now 😃
The authentication is now working even with the only API permission that I had in my Azure app registration originally:
- https://graph.microsoft.com/Directory.AccessAsUser.All
after trying the this snapshot I get an error that the email-address is already used by another user. so it looks like the user mapping fails and it want to create a new one. has there something changed here?
Yes, the way the user information is created has changed due to differences between the v1 and v2 MS endpoints. I created a new issue, #144 for this issue. Please look there for updates on that problem.
Given that the MS Graph and MS Auth v2 support is now part of the development code and we have working 2.0 snapshots, I'm going to close this issue as fixed. For problems with the 2.0 snapshots, please open a new issue for that specific problem and noting that it's a 2.0 issue.