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

GetAllAccounts fails started today #3233

Closed mbondoso closed 3 years ago

mbondoso commented 3 years ago

Library

Framework

Description

Since this afternoon we started to get this error in production and development. Can anyone help?

Error Message

ERROR TypeError: Cannot assign to read only property 'ToArray' of object '[object Object]' at Function.e.toObject (main-es2015.40768dffe1826b0305dc.js:1) at t.getAccount (main-es2015.40768dffe1826b0305dc.js:1) at main-es2015.40768dffe1826b0305dc.js:1 at Array.forEach () at t.e.getAccountsFilteredByInternal (main-es2015.40768dffe1826b0305dc.js:1) at t.e.getAccountsFilteredBy (main-es2015.40768dffe1826b0305dc.js:1) at t.e.getAllAccounts (main-es2015.40768dffe1826b0305dc.js:1) at t.e.getAllAccounts (main-es2015.40768dffe1826b0305dc.js:1)

MSAL Configuration

// Provide configuration values here.
// For Azure B2C issues, please include your policies.

Reproduction steps

const myAccounts: AccountInfo[] = this.msalService.instance.getAllAccounts();

// Provide relevant code snippets here.
// For Azure B2C issues, please include your policies.

Expected behavior

Identity Provider

Browsers/Environment

Regression

Security

Source

tnorling commented 3 years ago

@mbondoso Can you try upgrading to the latest versions and let us know if this is still an issue?

Latest versions are msal-browser@2.12.1 and msal-angular@2.0.0-beta.0

mbondoso commented 3 years ago

Still happens. This started today on prod. so I don't think it's version related

tnorling commented 3 years ago

@mbondoso Can you try to determine what changed? It's very hard to tell what may be causing this without insight into your code and environment and without being able to reproduce this.

mbondoso commented 3 years ago

We changed nothing from yesterday to today.

on localhost the error is this: core.js:5973 ERROR TypeError: Cannot assign to read only property 'ToArray' of object '[object Object]' at Function.push.u8tD.CacheManager.toObject (index.es.js:2692) at BrowserCacheManager.push.u8tD.BrowserCacheManager.getAccount (index.es.js:8276) at index.es.js:2202 at Array.forEach () at BrowserCacheManager.push.u8tD.CacheManager.getAccountsFilteredByInternal (index.es.js:2201) at BrowserCacheManager.push.u8tD.CacheManager.getAccountsFilteredBy (index.es.js:2188) at BrowserCacheManager.push.u8tD.CacheManager.getAllAccounts (index.es.js:2112) at PublicClientApplication.push.u8tD.ClientApplication.getAllAccounts (index.es.js:10560)

tnorling commented 3 years ago

@mbondoso A few questions to get more context: How long was it working correctly before it broke? Is it consistently erroring out or is it intermittent? Is this affecting all users or only some? If you clear browser storage and login again, does it still break? Can you reproduce this with one of our samples?

mbondoso commented 3 years ago

@tnorling We last updated the app 1 month ago. I tried to clear cache / browser storage and login again still the same issue. tried with a couple of accounts all with the same behavior.

Will try to replicate it with sample.

mbondoso commented 3 years ago

@tnorling with the sample angular11-sample-app works fine because it uses MsalGuard to validate routes, I need to use custom logic from my application so I use a custom guard that also depends on

const myAccounts: AccountInfo[] = this.msalService.instance.getAllAccounts();

I debugged and it works the first requests and then the problem starts to appear. has something changed on your side returning from Azure AD B2C ?

mbondoso commented 3 years ago

@tnorling I changed my guard to surpass this problem but on the rolling of the app it's breaking upfront on azure-msal-angular.js

MsalInterceptor:

        // Sets account as active account or first account
        let account;
        if (!!this.authService.instance.getActiveAccount()) {
            this.authService.getLogger().verbose("Interceptor - active account selected");
            account = this.authService.instance.getActiveAccount();
        }
        else {
            this.authService.getLogger().verbose("Interceptor - no active account, fallback to first account");
            account = this.authService.instance.getAllAccounts()[0];
        }
tnorling commented 3 years ago

@mbondoso

I need to use custom logic from my application so I use a custom guard that also depends on

Can you share this custom logic? A link to a public github repo that reproduces this issue would be helpful

has something changed on your side returning from Azure AD B2C ?

Not that I'm aware of, however, we don't own the B2C service so you would have to open a ticket with them if you believe it's a service issue.

I changed my guard to surpass this problem but on the rolling of the app it's breaking upfront on azure-msal-angular.js

Can you describe what is breaking? getActiveAccount will only return an account if you previously called setActiveAccount

mbondoso commented 3 years ago

@tnorling I changed cacheLocation from LocalStorage to SessionStorage and it started working.

Then I investigated and seems that CacheManager from msal-browser is handling my own localstorage items despite none of the keys match.

tnorling commented 3 years ago

@mbondoso Ahh interesting. Can you share an example cache key that msal-browser is attempting to read so that I can try to reproduce this? There should be code that prevents msal-browser from reading cache keys that don't belong to msal so I would be interested to see why yours seem to be getting through. We definitely want to make sure this doesn't happen.

mbondoso commented 3 years ago

@tnorling I believe the one that is breaking is google_auto_fc_cmp_setting

tnorling commented 3 years ago

@mbondoso I've identified the issue and will make sure this is addressed in the next release. Thanks for raising this and helping me get to the bottom of it

mbondoso commented 3 years ago

Thanks @tnorling for your support and effort!