Failed Authentication throwing `ArrayIndexOutOfBoundsException`

[vikasbo]

Steps to reproduce

What steps can reproduce the defect?

IPartnerCredentials appCredentials = 
      PartnerCredentials.getInstance()
      .generateByApplicationCredentials(clientId, applicationSecret, aadApplicationDomain)

throws:

2020-02-12 16:33:00,191 155022 ERROR [com.microsoft.aad.msal4j.ConfidentialClientApplication] (ForkJoinPool.commonPool-worker-1:) [Correlation ID: d1562f78-c8b2-4f14-9937-350080770ada] Execution of class com.microsoft.aad.msal4j.AcquireTokenByAuthorizationGrantSupplier failed.
    java.lang.ArrayIndexOutOfBoundsException: 0
        at com.microsoft.aad.msal4j.Authority.getTenant(Authority.java:88)
        at com.microsoft.aad.msal4j.AadInstanceDiscoveryProvider.sendInstanceDiscoveryRequest(AadInstanceDiscoveryProvider.java:110)
        at com.microsoft.aad.msal4j.AadInstanceDiscoveryProvider.doInstanceDiscoveryAndCache(AadInstanceDiscoveryProvider.java:131)
        at com.microsoft.aad.msal4j.AadInstanceDiscoveryProvider.getMetadataEntry(AadInstanceDiscoveryProvider.java:42)
        at com.microsoft.aad.msal4j.AuthenticationResultSupplier.getAuthorityWithPrefNetworkHost(AuthenticationResultSupplier.java:32)
        at com.microsoft.aad.msal4j.AcquireTokenByAuthorizationGrantSupplier.execute(AcquireTokenByAuthorizationGrantSupplier.java:49)
        at com.microsoft.aad.msal4j.AuthenticationResultSupplier.get(AuthenticationResultSupplier.java:59)
        at com.microsoft.aad.msal4j.AuthenticationResultSupplier.get(AuthenticationResultSupplier.java:17)
        at java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1590)
        at java.util.concurrent.CompletableFuture$AsyncSupply.exec(CompletableFuture.java:1582)
        at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:289)
        at java.util.concurrent.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1056)
        at java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1692)
        at java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:157)

Please share the setup, sample project, version of Java etc.

Expected behavior

The above error does not make sense. Same code, same credentials worked in the previous version 1.5.1 but suddenly not working on switching to 1.5.3 latest build.

What could be the change that caused this and what is the root cause that can be attributed to the ArrayIndexOutOfBoundsException.

Share the expected output

Actual behavior

What is the behavior observed?

Diagnostic logs

Please share test platform diagnostics logs.
The logs may contain test assembly paths, kindly review and mask those before sharing.

Environment

Please share additional details about your environment. Version

[msize]

@vikasbo You are wrong with the project. This issue has to report into https://github.com/AzureAD/microsoft-authentication-library-for-java

[sangonzal]

@vikasbo @msize I looked into this and this as bug in the partner center SDK (although MSAL should do a better job of validating authorities and throwing an useful error message in this case - https://github.com/AzureAD/microsoft-authentication-library-for-java/issues/180).

The reason this is happening is because when instantiating

IPartnerCredentials appCredentials = 
      PartnerCredentials.getInstance()
      .generateByApplicationCredentials(clientId, applicationSecret, aadApplicationDomain)

Partner center sets activeDirectoryAuthority to "https://login.microsoftonline.com/". Then when passing in the authority to MSAL it adds another trailing slash before adding the aadApplicationDomain. So the authority that gets passed in is "https://login.microsoftonline.com//aadApplicationDomain" which is invalid. It should actually be "https://login.microsoftonline.com/aadApplicationDomain"

If you confirm that this is the issue, I'll see if I can submit a PR at some point this week.

[msize]

This was not clear from the first comment. There was only a stack trace from the msal4j component.

As a workaround, the fix which removes the last slash may be fixed the problem. But, I'm don't sure that the msal4j don't remove the first slash from them code somewhen in the future too. And this workaround can produce a new problem.

Therefore, I'd don't make this workaround in the main branch of the PC SDK.

[sangonzal]

@msize I'm not sure what you mean by "But, I'm don't sure that the msal4j don't remove the first slash from them code somewhen in the future too. And this workaround can produce a new problem". Can you please explain?

Partner center SDK is passing the wrong value in to msal4j for authority. As long as this is not fixed in partner SDK, this API (PartnerCredentials.getInstance() .generateByApplicationCredentials(clientId, applicationSecret, aadApplicationDomain)) will always fail. Therefore this logic should probably to be fixed in the master branch and released.

[vikasbo]

@msize @sangonzal Based on your comments above, I believe the bug is in two folds:

1. MSAL must throw an understandable error message instead of throwing the run time error i.e. ArrayIndexOutOfBoundsException. Run time errors are a good sign that some validation is missing

2. The Partner center should not be initializing the authority with an extra slash '/' which is being added by MSAL anyways. Therefore, it should initialize with "https://login.microsoftonline.com" instead of "https://login.microsoftonline.com/".

I would consider it a bug in both the projects because even if MSAL was to send a readable/actionable error, the extra slash causing the run time error of ArrayIndexOutOfBoundsException will not be fixed/removed by any code changes of clients using Partner Center and MSAL as its dependency.

A quick fix for the above bug in Partner Center would be highly appreciated.

[vikasbo]

@msize Thank you for this. Please share an ETA for this since it is a blocking issue. Thanks.