AzureAD / microsoft-authentication-library-for-js

Microsoft Authentication Library (MSAL) for JS
http://aka.ms/aadv2
MIT License
3.67k stars 2.65k forks source link

Distributed cache won't remove the last item #6954

Open piquet-h opened 8 months ago

piquet-h commented 8 months ago

Core Library

MSAL Node (@azure/msal-node)

Core Library Version

2.5

Wrapper Library

Not Applicable

Wrapper Library Version

n/a

Public or Confidential Client?

Confidential

Description

When you use a distributed cache, it won't remove the last item or the partition it belongs to when using

cache.getAccountByHomeId(homeId).then(account => {
  if(account) {
    tokenCache.removeAccount(accounts[0])
  }
})

Error Message

No error message - it just won't delete.

The key code is here. In lib/msal-node/src/cache/distributed/DistributedCachePlugin.ts - in the afterCacheAccess method. What's happening is that it only serializes the cache if there are still accountEntities in there... but if you remove the last one, it won't update. Therefore the last entity won't ever be removed.

            if (accountEntities.length > 0) {
                const accountEntity = accountEntities[0] as AccountEntity;
                const partitionKey = await this.partitionManager.extractKey(
                    accountEntity
                );

                await this.client.set(
                    partitionKey,
                    cacheContext.tokenCache.serialize()
                );
            }

MSAL Logs

[2024-03-15T06:48:03.217Z] Executing HTTP request: { [2024-03-15T06:48:03.217Z] "requestId": "14ff06be-b635-4a78-bb9d-a3383b462c29", [2024-03-15T06:48:03.217Z] "method": "DELETE", [2024-03-15T06:48:03.217Z] "userAgent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/122.0.0.0 Safari/537.36", [2024-03-15T06:48:03.217Z] "uri": "/api/deleteAccount" [2024-03-15T06:48:03.217Z] } [2024-03-15T06:48:03.218Z] Executing 'Functions.account-delete' (Reason='This function was programmatically called via the host APIs.', Id=355e0997-0453-44d4-a4c9-534ea8f762e9) [2024-03-15T06:48:03.221Z] Executing 1 "preInvocation" hooks[2024-03-15T06:48:03.221Z] Executed "preInvocation" hooks [2024-03-15T06:48:03.221Z] Worker 456f09db-337a-4202-86c3-0189f752a46f received FunctionInvocationRequest with invocationId 355e0997-0453-44d4-a4c9-534ea8f762e9

[2024-03-15T06:48:03.865Z] [Fri, 15 Mar 2024 06:48:03 GMT] : [ff1e6beb-2d6d-4895-b103-6c9f2c7390c4] : @azure/msal-common@14.7.1 : Warning - No client info in response [2024-03-15T06:48:05.095Z] [Fri, 15 Mar 2024 06:48:05 GMT] : [] : @azure/msal-node@2.6.4 : Info - getTokenCache called [2024-03-15T06:48:05.509Z] [Fri, 15 Mar 2024 06:48:05 GMT] : [] : @azure/msal-common@14.7.1 : Info - CacheManager:getIdToken - Returning ID token [2024-03-15T06:48:05.818Z] [Fri, 15 Mar 2024 06:48:05 GMT] : [ef96915a-84bb-4e93-a753-52b228683662] : @azure/msal-common@14.7.1 : Warning - No client info in response [2024-03-15T06:48:07.286Z] Executed 'Functions.account-delete' (Succeeded, Id=355e0997-0453-44d4-a4c9-534ea8f762e9, Duration=4068ms) [2024-03-15T06:48:07.286Z] Executed HTTP request: { [2024-03-15T06:48:07.286Z] "requestId": "14ff06be-b635-4a78-bb9d-a3383b462c29", [2024-03-15T06:48:07.286Z] "identities": "", [2024-03-15T06:48:07.286Z] "status": "204", [2024-03-15T06:48:07.286Z] "duration": "4069" [2024-03-15T06:48:07.286Z] }

Network Trace (Preferrably Fiddler)

MSAL Configuration

{
  auth: {
    clientId: process.env.CLIENT_ID,
    authority: 'https://login.microsoftonline.com/common,
    clientSecret: process.env.CLIENT_SECRET
  },
  system: {
    loggerOptions: {
      logLevel: msal.LogLevel.Trace,
      loggerCallback(loglevel, message, containsPii) {
        console.debug(message)
      }
    }
  },
  cache: {
    cachePlugin: new msal.DistributedCachePlugin(cache, new PartitionManager(cache, id))
  }
}

Relevant Code Snippets

if (accountEntities.length > 0) {
                const accountEntity = accountEntities[0] as AccountEntity;
                const partitionKey = await this.partitionManager.extractKey(
                    accountEntity
                );

                await this.client.set(
                    partitionKey,
                    cacheContext.tokenCache.serialize()
                );
            }

### Reproduction Steps

1. Create a distributed cache
2. Login with an account where the token is added to a distributed cache
3. Try and delete the last/only account in a partition

cache.getAccountByHomeId(homeId).then(account => { if(account) { tokenCache.removeAccount(accounts[0]) } })



### Expected Behavior

I expect to be able to delete the last entry and the partition.

### Identity Provider

Azure B2C Basic Policy

### Browsers Affected (Select all that apply)

None (Server)

### Regression

n/a

### Source

Internal (Microsoft)
piquet-h commented 8 months ago

Just thinking more on this, I'd consider this a potential security / privacy issue. If as a user I want to delete my account, I should expect that all tokens associated in a persistent cache would also be deleted.