[Bug] Invalid unlocking in the Persistence extension

[sbasegmez]

Library version used

1.18.0 and 1.3.0

Java version

Temurin JDK 17

Scenario

PublicClient (AcquireTokenInteractive, AcquireTokenByUsernamePassword)

Is this a new or an existing app?

This is a new app or experiment

Issue description and reproduction steps

When using the msal4j-persistence extension on MacOS Keychain, the library fails to perform an unlock operation during the afterCacheAccess step, even during a basic read operation. This results in an unnecessary error being logged without any explanation.

The code example is below. The issue arises when I create a public client application with a persistent token cache enabled for Keychain. I only read the account cache, but it logs a "null" error during the second unlock attempt. The log reads:

[ForkJoinPool.commonPool-worker-1] DEBUG com.microsoft.aad.msal4jextensions.CrossProcessCacheFileLock - pid:78864 thread:15 acquiring file lock
[ForkJoinPool.commonPool-worker-1] DEBUG com.microsoft.aad.msal4jextensions.CrossProcessCacheFileLock - pid:78864 thread:15 acquired OK file lock
[ForkJoinPool.commonPool-worker-1] DEBUG com.microsoft.aad.msal4jextensions.CrossProcessCacheFileLock - pid:78864 thread:15 releasing lock
[ForkJoinPool.commonPool-worker-1] DEBUG com.microsoft.aad.msal4jextensions.CrossProcessCacheFileLock - pid:78864 thread:15 releasing lock
[ForkJoinPool.commonPool-worker-1] ERROR com.microsoft.aad.msal4jextensions.CrossProcessCacheFileLock - null

When I debug the code, I observe that it acquires a lock as a normal behavior. But it releases the lock during the beforeCacheAccess step. Then, it reads the cache and executes afterCacheAccess, where it does not have a proper lock but still attempts to unlock.

I was able to reproduce this issue with MSAL4j 1.17.3 and 1.18.0, using the persistence extension 1.2.0 and 1.3.0. Unfortunately, I don’t have access to a Windows test environment, so I couldn’t test it there.

Relevant code snippets

public class MsalPersistenceTest {

    public static final String CLIENTID = "<irrelevant>";
    public static final String AUTHURL = "https://login.microsoftonline.com/irrelevant";
    private static final Set<String> SCOPES = Set.of("User.Read");

    public static void main(String[] args) {

        try {
            var ps = PersistenceSettings.builder("temp.cache", Path.of(System.getProperty("user.home"), ".deleteme"))
                                        .setMacKeychain("DeleteMe", "me")
                                        .setLockRetry(1000, 50)
                                        .build();

            var pca = PublicClientApplication.builder(CLIENTID)
                                             .authority(AUTHURL)
                                             .setTokenCacheAccessAspect(new PersistenceTokenCacheAccessAspect(ps))
                                             .build();

            // Force cache to be loaded
            pca.getAccounts().join();

        } catch (Exception e) {
            e.printStackTrace();
        }


    }

}

Expected behavior

I guess it doesn't need to unlock on the beforeCacheAccess, or it doesn't need to unlock later.

Identity provider

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

Regression

No response

Solution and workarounds

No response

[bowen-facere]

Hey, were you able to find a solution to this?

[sbasegmez]

Hey, were you able to find a solution to this?

I just added a band-aid if to the line in the error message just to prevent the null logs.

PersistenceTokenCacheAccessAspect > afterCacheAccess:

@Override
    public void afterCacheAccess(ITokenCacheAccessContext iTokenCacheAccessContext) {
        try {
            if (isWriteAccess(iTokenCacheAccessContext)) {
                cacheAccessor.write(iTokenCacheAccessContext.tokenCache().serialize().getBytes(StandardCharset.UTF_8));
                updateLastSeenCacheFileModifiedTimestamp();
            }
        } finally {
            // Added by SB to check the lock before unlocking.
            if(lock != null && lock.isLocked()) {
                try {
                    lock.unlock();
                } catch (IOException e) {
                    LOG.error("Unable to unlock the lock", e);
                }
            }
        }
    }