[Bug] DefaultHttpClient ignores supplied SSLSocketFactory

[stefanlourens]

Library version used

1.15.0

Java version

21.0.2

Scenario

ConfidentialClient - web site (AcquireTokenByAuthCode)

Is this a new or an existing app?

This is a new app or experiment

Issue description and reproduction steps

Supplying a SSLSocketFactory to the ConfidentialClientApplication builder has no effect.

I traced this down to the DefaultHttpClient's handling of SSL connections:

It currently checks if the connection is an instance of HttpURLConnection, but since HttpsURLConnection extends HttpURLConnection it's always true and the else is never executed.

if (connection instanceof HttpURLConnection) {
    return (HttpURLConnection) connection;
} else {
    HttpsURLConnection httpsConnection = (HttpsURLConnection) connection;

    if (sslSocketFactory != null) {
        httpsConnection.setSSLSocketFactory(sslSocketFactory);
    }

    return httpsConnection;
}

https://github.com/AzureAD/microsoft-authentication-library-for-java/blob/dev/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/DefaultHttpClient.java#L93-L103

I suggest changing this to:

if (connection instanceof HttpsURLConnection) {
    HttpsURLConnection httpsConnection = (HttpsURLConnection) connection;

    if (sslSocketFactory != null) {
        httpsConnection.setSSLSocketFactory(sslSocketFactory);
    }

    return httpsConnection;
} else {
    return (HttpURLConnection) connection;
}

Relevant code snippets

No response

Expected behavior

The supplied SSLSocketFactory should be set on the HttpsUrlConnection httpsConnection.setSSLSocketFactory(sslSocketFactory); or setting the default ssl handing externally.

Identity provider

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

Regression

No response

Solution and workarounds

Currently the only workaround is to supply your own http client, with the logic fixed.

[Avery-Dunn]

Hello @stefanlourens : Thanks for bringing this to our attention! I was able to reproduce the issue, and this is definitely unintended behavior: if the URL is 'https' then the instance of Http... and instance of Https... checks are both true and the else block will never be reached.

I don't have an ETA but the fix will be in either our next release or the one after, and I'll update this thread once it's done.

[bgavrilMS]

@stefanlourens - please contribute your code fix by opening a PR if you need this to be resolved faster.

[stefanlourens]

@bgavrilMS Was originally hoping to add a test as well, but ran into some lombok compatibility issues, ended up just doing the change as suggested: #821

[Avery-Dunn]

Done as part of #821 and release in 1.15.1

Thanks again for the fix!