search

AzureAD / microsoft-authentication-library-for-js

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

msal-node and msal-common incompatible in latest versions #2891

Closed naugtur closed 3 years ago

naugtur commented 3 years ago

Library

Framework

Description

When using persistencePlugin there's an internal incompatibility between msal-node and msal-common that didn't exist in the following pair of versions:

"@azure/msal-common": "1.1.1",
"@azure/msal-node": "1.0.0-alpha.4",

Having inspected the code of msal I don't see a way to extend our cachePlugin code to cover the method - it's missing in msal code.

Error Message

TypeError: this.persistencePlugin.afterCacheAccess is not a function at ResponseHandler. (/code/common/node_modules/@azure/msal-node/node_modules/@azure/msal-common/dist/index.js:4471:69) at step (/code/common/node_modules/@azure/msal-node/node_modules/@azure/msal-common/dist/index.js:79:23) at Object.next (/code/common/node_modules/@azure/msal-node/node_modules/@azure/msal-common/dist/index.js:60:53) at /code/common/node_modules/@azure/msal-node/node_modules/@azure/msal-common/dist/index.js:53:71 at new Promise () at __awaiter (/code/common/node_modules/@azure/msal-node/node_modules/@azure/msal-common/dist/index.js:49:12) at ResponseHandler.handleServerTokenResponse (/code/common/node_modules/@azure/msal-node/node_modules/@azure/msal-common/dist/index.js:4422:16) at RefreshTokenClient. (/code/common/node_modules/@azure/msal-node/node_modules/@azure/msal-common/dist/index.js:5091:63) at step (/code/common/node_modules/@azure/msal-node/node_modules/@azure/msal-common/dist/index.js:79:23) at Object.next (/code/common/node_modules/@azure/msal-node/node_modules/@azure/msal-common/dist/index.js:60:53) at fulfilled (/code/common/node_modules/@azure/msal-node/node_modules/@azure/msal-common/dist/index.js:50:58) at processTicksAndRejections (internal/process/task_queues.js:93:5)

MSAL Configuration

const cachePlugin = {
            readFromStorage: async () => cacheStorage,
            writeToStorage: async (getMergedState) => {
                cacheStorage = getMergedState(cacheStorage);
            },
        };

        const config = {
            auth: {
                clientId: clientId,
                clientSecret: clientSecret,
                authority: authority,
            },
            cache: {
                cachePlugin,
            },
            system: {
                loggerOptions: msloggerCallbackConfiguration.setLoggerCallback(logger),
            },
        };

        const confidentialClient = new msal.ConfidentialClientApplication(config);

Reproduction steps

calling a refresh or any action that wants to read the cache

confidentialClient.acquireTokenByRefreshToken({ refreshToken, scopes })

Expected behavior

cachePlugin works as documented

Identity Provider

Browsers/Environment

Node.js

Regression

Security

1.0.0-alpha.4 depends on an axios version that has a High vulnerability reported against it. beta.3 doesn't seem to support cachePlugin with msal-common 1x or 2x

Source

github-actions[bot] commented 3 years ago

This issue has not seen activity in 14 days. It will be closed in 7 days if it remains stale.

naugtur commented 3 years ago

We have removed msal from our app in favor of making requests directly, so I won't be able to provide further updates on new released versions.

Not being able to control credentials storage and/or limiting it to RAM is a blocker for scaling our services horizontally. Also, msal was adding scopes to authorization requests that caused issues depending on tenants' configurations. Feel free to reach out for more detailed feedback

github-actions[bot] commented 3 years ago

This issue has not seen activity in 14 days. If your issue has not been resolved please leave a comment to keep this open. It will be closed in 7 days if it remains stale.

github-actions[bot] commented 3 years ago

This issue has been closed due to inactivity. If this has not been resolved please open a new issue. Thanks!

sameerag commented 3 years ago

@naugtur We do support persistence (both encrypted and unencrypted) in native apps. Are you looking for persistence in a web app?

naugtur commented 3 years ago

Yes, a web app in a sense of how it's accessed. The main part is a process that runs in the background and does things for the users. We scale horizontally, which means we run many copies of the app and they all need access to fresh tokens

samuelkubai commented 3 years ago

Hi @naugtur

Also, msal was adding scopes to authorization requests that caused issues depending on tenants' configurations.

Could you shed more light on which scopes were being added and how they affected your tenant configurations?

naugtur commented 3 years ago

It added some of the lowercase scopes by default. Some of these: email, openid, profile, offline_access, don't remember which exactly. openid and offline I think.

github-actions[bot] commented 3 years ago

This issue has not seen activity in 14 days. If your issue has not been resolved please leave a comment to keep this open. It will be closed in 7 days if it remains stale.

github-actions[bot] commented 3 years ago

This issue has been closed due to inactivity. If this has not been resolved please open a new issue. Thanks!