syncweek-react-aad / react-aad

A React wrapper for Azure AD using the Microsoft Authentication Library (MSAL). The easiest way to integrate AzureAD with your React for authentication.
MIT License
344 stars 94 forks source link

Safari Unhandled Promise Rejection: TypeError: undefined is not an object (evaluating 'response.tokenType') #188

Closed ThomVanL closed 1 year ago

ThomVanL commented 4 years ago

Describe the bug Whenever I try to get an access token with LoginType Redirect in Safari 13, I end up receiving the following error: Unhandled Promise Rejection: TypeError: undefined is not an object (evaluating 'response.tokenType').

I'm getting the same behavior with the sample app, LoginType Popup however does seem to work.

Additionally I found this error in the session state:

To Reproduce Steps to reproduce the behavior:

  1. Follow sample app setup
  2. Open Safari
  3. Browse to localhost:3000
  4. Click "Redirect Sample"
  5. Click "enable redirect"
  6. Log in with your Azure AD credentials
  7. Once you're back in the sample app, click get access token and the error should occur

Expected behavior Receive an access token similarly to when using LoginType Popup.

Desktop (please complete the following information):

Additional context package json dependencies:

  "dependencies": {
    "msal": "1.2.0",
    "react-aad-msal": "2.2.7",
    "react-scripts": "^3.3.0",
    "react": "16.12.0",
    "react-redux": "7.1.3",
    "react-dom": "16.12.0"
  }

Logs:

[Log] [HMR] Waiting for update signal from WDS... (0.chunk.js, line 52259)
[Log] [MSAL] – "Sun, 22 Dec 2019 19:21:06 GMT:1.2.0-Info Returned from redirect url" (main.chunk.js, line 921)
[Log] [MSAL] – "Sun, 22 Dec 2019 19:21:06 GMT:1.2.0-Info Processing the callback from redirect response" (main.chunk.js, line 921)
[Log] [MSAL] – "Sun, 22 Dec 2019 19:21:06 GMT:1.2.0-Info State status:true; Request type:LOGIN" (main.chunk.js, line 921)
[Log] [MSAL] – "Sun, 22 Dec 2019 19:21:06 GMT:1.2.0-Info State is right" (main.chunk.js, line 921)
[Log] [MSAL] – "Sun, 22 Dec 2019 19:21:06 GMT:1.2.0-Info Fragment has id token" (main.chunk.js, line 921)
[Log] [MSAL] – "Sun, 22 Dec 2019 19:21:06 GMT:1.2.0-Info Token is already in cache for scope:omitted-application-id" (main.chunk.js, line 921)
[Log] [MSAL] – "Sun, 22 Dec 2019 19:21:12 GMT:1.2.0-Verbose Token is not in cache for scope:openid" (main.chunk.js, line 921)
[Log] [MSAL] – ":1.2.0-Verbose renewing accesstoken" (main.chunk.js, line 921)
[Log] [MSAL] – "Sun, 22 Dec 2019 19:21:12 GMT:1.2.0-Verbose renewToken is called for scope:openid" (main.chunk.js, line 921)
[Log] [MSAL] – "Sun, 22 Dec 2019 19:21:12 GMT:1.2.0-Info Add msal frame to document:msalRenewFrameopenid" (main.chunk.js, line 921)
[Log] [MSAL] – "Sun, 22 Dec 2019 19:21:12 GMT:1.2.0-Verbose Renew token Expected state: guid" (main.chunk.js, line 921)
[Log] [MSAL] – "Sun, 22 Dec 2019 19:21:12 GMT:1.2.0-Verbose Set loading state to pending for: openid:guid" (main.chunk.js, line 921)
[Log] [MSAL] – "Sun, 22 Dec 2019 19:21:12 GMT:1.2.0-Info LoadFrame: msalRenewFrameopenid" (main.chunk.js, line 921)
[Log] [MSAL] – "Sun, 22 Dec 2019 19:21:13 GMT:1.2.0-Info Add msal frame to document:msalRenewFrameopenid" (main.chunk.js, line 921)
[Log] [MSAL] – "Sun, 22 Dec 2019 19:21:13 GMT:1.2.0-Info Returned from redirect url" (main.chunk.js, line 921)
[Log] [MSAL] – "Sun, 22 Dec 2019 19:21:13 GMT:1.2.0-Info Processing the callback from redirect response" (main.chunk.js, line 921)
[Log] [MSAL] – "Sun, 22 Dec 2019 19:21:13 GMT:1.2.0-Info State status:true; Request type:RENEW_TOKEN" (main.chunk.js, line 921)
[Error] Unhandled Promise Rejection: TypeError: undefined is not an object (evaluating 'response.tokenType')
    (anonymous function) (0.chunk.js:12049)
    promiseReactionJob

Apparently this points to the following piece of code:

import { AuthResponse } from 'msal';
import { TokenType } from './Interfaces';

export class AccessTokenResponse {
  public accessToken = '';
  public scopes: string[] = [];
  public expiresOn: Date;
  public state = '';

  constructor(response: AuthResponse) {
    if (response.tokenType !== TokenType.AccessToken) { // error occurs here
      throw new Error(
        `Can't construct an AccessTokenResponse from a AuthResponse that has a token type of "${response.tokenType}".`,
      );
    }

    this.accessToken = response.accessToken;
    this.expiresOn = response.expiresOn;
    this.scopes = response.scopes;
    this.state = response.accountState;
  }
}
AndrewCraswell commented 4 years ago

Thanks for the detailed issue! Can you share you configuration being used with the library? Interested in what sort of caching methods you're using. At a minimum, there is at least a null reference exception occurring in the code you highlighted which can be resolved. The AADSTS50058 code being received will take some more investigation.

ThomVanL commented 4 years ago

Sure thing, I was using a production app when I reported the issue but I'm seeing the same thing happen in the sample app. Again though, this is only occurring with the redirect flow.

So here is the config from the sample app.

import { MsalAuthProvider, LoginType } from "react-aad-msal";
import { Logger, LogLevel } from "msal";

export const authProvider = new MsalAuthProvider(
  {
    auth: {
      authority: "https://login.microsoftonline.com/common",
      clientId: "0f2c6253-3928-4fea-b131-bf6ef8f69e9c",
      postLogoutRedirectUri: window.location.origin,
      redirectUri: window.location.origin,
      validateAuthority: true,
      navigateToLoginRequestUrl: false
    },
    system: {
      logger: new Logger(
        (logLevel, message, containsPii) => {
          console.log("[MSAL]", message);
        },
        {
          level: LogLevel.Verbose,
          piiLoggingEnabled: false
        }
      )
    },
    cache: {
      cacheLocation: "sessionStorage",
      storeAuthStateInCookie: true
    }
  },
  {
    scopes: ["openid"]
  },
  {
    loginType: LoginType.Popup,
    tokenRefreshUri: window.location.origin + "/auth.html"
  }
);
AndrewCraswell commented 4 years ago

I just realized this bug filed on MSAL repo: https://github.com/AzureAD/microsoft-authentication-library-for-js/issues/1205

Seems this may be a known issue for Safari browsers until the next release includes the new Auth Code Flow w/ PKCE.

But the null reference exception should still be fixable at the least.

pexxi commented 4 years ago

I just realized this bug filed on MSAL repo: AzureAD/microsoft-authentication-library-for-js#1205

Hi @AndrewCraswell , Just checked that the bug you mentioned has been closed in March. The authors are now advising to use @azure/msal-browser for PKCE flow. Any plans on integrating this to react-aad ?

AndrewCraswell commented 4 years ago

Yes, there are discussions but it will require more or less a rewrite. At the moment I'm pretty swamped with a product release upcoming, but my goal would be to have something ready in a month or two.

gabriel-kohen-by commented 4 years ago

Hi @AndrewCraswell any update on working with @sameerag from the MSAL.js team to update this library to use PKCE? It seems that with the latest versions of Chrome(84) blocking 3rd party cookies, PKCE is fast becoming the only user friendly doing the MSAL auth. FYI: @alex-mason-by and @dj-jovic-by

GraemeF commented 4 years ago

Should be resolved by #238