AzureAD / microsoft-authentication-library-for-js

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

BrowserCacheManager tries to access cookies which fails in iframe #4725

Closed restfulhead closed 2 years ago

restfulhead commented 2 years ago

Core Library

MSAL.js v2 (@azure/msal-browser)

Core Library Version

2.23.0

Wrapper Library

MSAL Angular (@azure/msal-angular)

Wrapper Library Version

2.2.0

Description

The BrowserCacheManager still tries to access cookies, even when specifying BrowserCacheLocation.LocalStorage as the cache location: https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/msal-browser-v2.23.0/lib/msal-browser/src/cache/BrowserCacheManager.ts#L826

This fails in an iFrame scenario (see error message below).

Error Message

ERROR DOMException: Failed to read the 'cookie' property from 'Document': The document is sandboxed and lacks the 'allow-same-origin' flag.
    at BrowserCacheManager.clearMsalCookies
    at BrowserCacheManager.cleanRequestByState
    at PopupClient.<anonymous>

Msal Logs

No response

MSAL Configuration

{
    cache: {
      cacheLocation: BrowserCacheLocation.LocalStorage,
      storeAuthStateInCookie: false,
    },
}

Relevant Code Snippets

https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/msal-browser-v2.23.0/lib/msal-browser/src/cache/BrowserCacheManager.ts#L826

Reproduction Steps

  1. Run https://github.com/AzureAD/microsoft-authentication-library-for-js/tree/dev/samples/msal-angular-v2-samples/angular12-sample-app in an iframe
  2. Click the login with popup button
  3. Observe browser error

Expected Behavior

I would expect cleanRequestByState not to call clearMsalCookies() if the cache method isn't cookies. In general I would expect the popup authentication to work in an iframe without same origin.

Identity Provider

Azure AD / MSA

Browsers Affected (Select all that apply)

Chrome

Regression

No response

Source

External (Customer)

bmahall commented 2 years ago

@restfulhead Curious to know whether this is breaking any of the intended behavior flows (e.g- logout) in your case?

It is expected that clearMsalCookies() is called , however we will be looking into improving the error handling in this scenario, and add a descriptive error message for when cookies are not found.

ghost commented 2 years ago

@restfulhead This issue has been automatically marked as stale because it is marked as requiring author feedback but has not had any activity for 5 days. If your issue has been resolved please let us know by closing the issue. If your issue has not been resolved please leave a comment to keep this open. It will be closed automatically in 7 days if it remains stale.