AzureAD / microsoft-authentication-library-for-java

Microsoft Authentication Library (MSAL) for Java http://aka.ms/aadv2
MIT License
280 stars 137 forks source link

[Bug] OPTIONS preflight check is beaking the connection #803

Open sgadot opened 3 months ago

sgadot commented 3 months ago

Library version used

1.14.3

Java version

--

Scenario

Other - please specify

Is this a new or an existing app?

The app is in production, I haven't upgraded MSAL, but started seeing this issue

Issue description and reproduction steps

The context :

Third party app using MSAL4J for database Auth is trying to connect to a database using Microsoft JDBC driver. This has been reproduced using :

Login and navigation has been tested with

The flow is as follow

  1. App tries to connect
  2. Opens browser and connects to https://login.microsoftonline.com/common/oauth2/v2.0/authorize?scope=openid+profile+offline_access+https%3A%2F%2Fdatabase.windows.net%2Fuser_impersonation&response_type=code&redirect_uri=http%3A%2F%2Flocalhost%3A51355%2F
  3. User logs in
  4. The browser then does a preflight check to localhost: image image
  5. the library fails with error: ERROR Microsoft Authentication 4:2 com.microsoft.aad.msal4j.MsalClientException: No Authorization code was returned from the server
  6. the POST to localhost fails because socket has been closed

Relevant code snippets

No response

Expected behavior

OPTION Preflight check from the browser should be ignored and it should wait for the real POST payload

Identity provider

Microsoft Entra ID (Work and School accounts and Personal Microsoft accounts)

Regression

No response

Solution and workarounds

Things are OK if the browser does not perform preflight check

Avery-Dunn commented 3 months ago

Hello @sgadot : By 'the app is in production', does that mean it had been working for some time and the issue started happening recently? Did it start happening after updating Chrome/Brave/Edge?

We've had some other customers report issues in the latest Chrome due to some overly cautious checks that Chrome considers a bug (https://github.com/AzureAD/microsoft-authentication-library-for-java/issues/801). However, unlike that issue this preflight check might be an intended change that we'll have to handle in MSAL.

localden commented 3 months ago

Tested this on the latest Chrome and cannot repro either:

image

That being said, I did reproduce the issue by halting the app execution before the request is done, and issuing an OPTIONS call against the localhost endpoint (thanks netstat -ab for helping identify the port and process it's running on), which results in a failure. Likely something we need to address (only accept GET with the code)

sgadot commented 3 months ago

Hello @sgadot : By 'the app is in production', does that mean it had been working for some time and the issue started happening recently? Did it start happening after updating Chrome/Brave/Edge?

Hi @Avery-Dunn ,

Yes that's it. It was working for some time and suddently stopped working. I havent seen any particular update on my browser, but since it's updating automatically in the background, that does not mean much...

We've had some other customers report issues in the latest Chrome due to some overly cautious checks that Chrome considers a bug (#801). However, unlike that issue this preflight check might be an intended change that we'll have to handle in MSAL.

This looks indeed very similar to #801 and would explain why the issue does not always happen on every computer in my office.

=> I would recommend doing what @localden suggested, even if the problem is not purely a MSAL4J issue, but might rather be an issue with Chrome.

Thanks a lot for your time guys, much appreciated !

daniel768 commented 3 months ago

Hello, we had the same Problem with Microsoft Edge 123. The Version 122 works perfectly. We've done a QuickFix in AuthorizationResponseHandler.handle(HttpExchange httpExchange) and added a check to skip the handling if the request is not "get" or "post" to ignore the new "options" request on / by Edge 123. String requestMethod = httpExchange.getRequestMethod(); LOG.info("Received request on / path " + requestMethod + " method."); if (!(requestMethod.equalsIgnoreCase("GET") || requestMethod.equalsIgnoreCase("POST"))) { httpExchange.sendResponseHeaders(200, 0); return; }

localden commented 3 months ago

Thanks folks - we're looking into the issue! We might need to be a bit more explicit about handling HTTP verbs.

@daniel768 @sgadot