Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Invalid unlocking in the Persistence extension #893

Open
sbasegmez opened this issue Dec 30, 2024 · 0 comments
Open

[Bug] Invalid unlocking in the Persistence extension #893

sbasegmez opened this issue Dec 30, 2024 · 0 comments
Labels
Bug Something isn't working, needs an investigation and a fix P2 Normal priority items, should be done after P1 persistence-package For issues related to the msal4j-persistence-extension package

Comments

@sbasegmez
Copy link

sbasegmez commented Dec 30, 2024

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

@sbasegmez sbasegmez added needs attention Automatically used when an issue is created through an issue template untriaged Automatically used when an issue is created through an issue template labels Dec 30, 2024
@Avery-Dunn Avery-Dunn added Bug Something isn't working, needs an investigation and a fix P2 Normal priority items, should be done after P1 persistence-package For issues related to the msal4j-persistence-extension package and removed needs attention Automatically used when an issue is created through an issue template untriaged Automatically used when an issue is created through an issue template labels Dec 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working, needs an investigation and a fix P2 Normal priority items, should be done after P1 persistence-package For issues related to the msal4j-persistence-extension package
Projects
None yet
Development

No branches or pull requests

2 participants