Skip to content
This repository was archived by the owner on Mar 2, 2022. It is now read-only.

migrated to msal.js v2 #119

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Large diffs are not rendered by default.

This file was deleted.

This file was deleted.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,21 @@ object-assign
* Wait for document loaded before starting the execution
*/

/*! *****************************************************************************
Copyright (c) Microsoft Corporation.

Permission to use, copy, modify, and/or distribute this software for any
purpose with or without fee is hereby granted.

THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH
REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT,
INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR
OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
PERFORMANCE OF THIS SOFTWARE.
***************************************************************************** */

/*! *****************************************************************************
Copyright (c) Microsoft Corporation. All rights reserved.
Licensed under the Apache License, Version 2.0 (the "License"); you may not use
Expand Down Expand Up @@ -268,6 +283,10 @@ and limitations under the License.

/*! ./utils */

/*! @azure/msal-browser v2.16.1 2021-08-02 */

/*! @azure/msal-common v4.5.1 2021-08-02 */

/*! @braintree/sanitize-url */

/*! Check if previously processed */
Expand Down

Large diffs are not rendered by default.

This file was deleted.

This file was deleted.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

24 changes: 16 additions & 8 deletions durablefunctionsmonitor.react/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion durablefunctionsmonitor.react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"homepage": "http://localhost:7072",
"dependencies": {
"@aspnet/signalr": "^1.1.4",
"@azure/msal-browser": "^2.16.1",
"@date-io/moment": "^1.3.13",
"@material-ui/core": "^4.9.10",
"@material-ui/icons": "^4.2.1",
Expand All @@ -15,7 +16,6 @@
"mobx": "^5.10.1",
"mobx-react": "^6.1.1",
"moment": "^2.27.0",
"msal": "^1.4.2",
"react": "^16.12.0",
"react-dom": "^16.12.0",
"react-scripts": "^4.0.3",
Expand Down
56 changes: 31 additions & 25 deletions durablefunctionsmonitor.react/src/states/LoginState.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { observable, computed } from 'mobx'
import axios from 'axios';
import * as Msal from 'msal';
import * as Msal from '@azure/msal-browser';

import { ErrorMessageState } from './ErrorMessageState';
import { BackendUri } from '../services/BackendClient';
Expand Down Expand Up @@ -104,33 +104,35 @@ export class LoginState extends ErrorMessageState {
return new Promise<undefined>((resolve, reject) => resolve(undefined));
}

return new Promise<{ Authorization: string }>((resolve, reject) => {
// Obtaining a token to access our own AAD app
const authParams: Msal.AuthenticationParameters = {
scopes: [this._aadApp.getCurrentConfiguration().auth.clientId]
};
const tokenRequest: Msal.RedirectRequest = {
account: this._aadApp.getAllAccounts()[0],

// This way of requesting a token for your own app registration is not documented, but seems to be correct. https://github.com/AzureAD/microsoft-authentication-library-for-js/issues/3972
scopes: [`${this._clientId}/.default`]
};

return new Promise<{ Authorization: string }>((resolve, reject) => {

this._aadApp.acquireTokenSilent(tokenRequest).then(tokenResponse => {

this._aadApp.acquireTokenSilent(authParams)
.then((authResponse) => {
resolve({ Authorization: `Bearer ${tokenResponse.accessToken}` });

var accessToken = authResponse.accessToken;
if (!accessToken) {
// https://github.com/AzureAD/microsoft-authentication-library-for-js/issues/736
// accessToken might randomly be returned as null, in which case we can probably use id_token
// (which is supposed to be the same)
console.log('DFM: accessToken is null, so using idToken.rawIdToken instead');
accessToken = authResponse.idToken.rawIdToken;
}
}, err => {

resolve({ Authorization: `Bearer ${accessToken}` });
if (err instanceof Msal.InteractionRequiredAuthError) {

}, err => {
// If silent token aquiring failed, then just redirecting the user back to AAD,
// so that the page is reloaded anyway.
// This is supposed to happen very rarely, as default refresh token lifetime is quite long.
console.log(`DFM: acquireTokenSilent() failed (${err}), so calling acquireTokenRedirect()...`);
this._aadApp.acquireTokenRedirect(authParams);
});

this._aadApp.acquireTokenRedirect(tokenRequest);

} else {
reject(err);
}
});
});
}

Expand All @@ -146,7 +148,10 @@ export class LoginState extends ErrorMessageState {
@observable
private _allowedTaskHubNames: string[];

private _aadApp: Msal.UserAgentApplication;
private _aadApp: Msal.PublicClientApplication;

// Need to remember this separately, because @azure/msal-browser lacks properties to obtain this value later on
private _clientId: string;

private loginWithEasyAuthConfig(config: {userName: string, clientId: string, authority: string}) {

Expand All @@ -172,7 +177,8 @@ export class LoginState extends ErrorMessageState {
}

// Configuring MSAL with values received from backend
this._aadApp = new Msal.UserAgentApplication({
this._clientId = config.clientId;
this._aadApp = new Msal.PublicClientApplication({
auth: {
clientId: config.clientId,
authority: config.authority,
Expand All @@ -181,20 +187,20 @@ export class LoginState extends ErrorMessageState {
})

// Checking if it was a redirect from AAD
this._aadApp.handleRedirectCallback(() => { }, (authErr: Msal.AuthError, accountState: string) => {

console.log(`Failed to handle login redirect. name: ${authErr.name}, message: ${authErr.message}, errorCode: ${authErr.errorCode}, errorMessage: ${authErr.errorMessage}, accountState: ${accountState}`);
this._aadApp.handleRedirectPromise().catch(err => {
Copy link
Owner Author

@scale-tone scale-tone Sep 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discovered, the promise returned by .handleRedirectPromise() actually needs to be handled. Immediately upon redirect from AAD this._aadApp.getAllAccounts() would still be empty and will only get populated in that promise handler

console.log(`Failed to handle login redirect. ${err}`);
});

const account = this._aadApp.getAccount();
const account = this._aadApp.getAllAccounts()[0];

if (!account) {
// Redirecting user to AAD. Redirect flow is more reliable (doesn't need popups enabled)
this._aadApp.loginRedirect();

} else {
// We've logged in successfully. Setting user name.
this._userName = account.userName;
this._userName = account.username;
this.initializeTaskHubNameAndConfirmLogin();
}
}
Expand Down