N/A
N/A
I find the design of msal very confusing. Fundamentally, all most developers want to do is authenticate a user given a config. msal, in contrast, has a lot of boilerplate and confusing branching/callback/synchronization control flow.
I would like a single configurable wrapper that handles the boilerplate calls to msal.
import {authenticate} from "msal";
const minimalConfigurationForMsal = { ... };
const handleUserIdentity: (token: string) => void = ...;
const handleAuthenticationFailure: (error: any) => void = ...;
authenticate(minimalConfigurationForMsal )
.then(handleUserIdentity)
.catch(handleAuthenticationFailure);
How would I implement this authenticate function?
If all you need to do is authenticate a user (and you don't need an access token for a resource such as Graph), your code could be as simple as:
import { UserAgentApplication } from 'msal';
// Should be created as a singleton
const msalApp = new UserAgentApplication({
clientId: "client-id-guid",
authority: "https://login.microsoftonline.com/common"
});
let account = null;
msalApp
.loginPopup({
scopes: [ "openid", "profile", "User.Read" ]
})
.then(loginResponse => {
account = loginResponse.account;
})
.catch(error => {
console.error(error.message);
});
Which could be implemented to match your code as such:
import { UserAgentApplication } from 'msal';
const minimalConfigurationForMsal = {
clientId: "client-id-guid",
authority: "https://login.microsoftonline.com/common"
}
let account = null;
const handleUserIdentity = loginResponse => {
account = loginResponse.account;
};
let errorMessage = null;
const handleAuthenticationFailure = error => {
errorMessage = error.message;
};
let msalApp = null;
const authenticate = config => {
if (!msalApp) {
msalApp = new UserAgentApplication(config);
}
return msalApp.loginPopUp({
scopes: [ "openid", "profile", "User.Read" ]
});
}
authenticate(minimalConfigurationForMsal)
.then(handleUserIdentity)
.catch(handleAuthenticationFailure);
This could in theory work for simple use cases that just need to authenticate a user, but may not fit well if you need access tokens or have a more complicated use case. Ultimately, our library needs to be flexible to enough to handle more than just this use case, and so I would disagree that "all most" developers just use it to authenticate a user. That said, we know our API could be simpler (especially when requesting access tokens, for example), and are open to feedback about how we could make it better.
Thanks. Perhaps a greater concern (worth opening a separate issue for?) is that the redirect flow is described as if its a legacy shim and everyone should be using the popup flow--only using the popup flow would mean less boilerplate; however, the popup flow seems to be blocked in all browsers I've tried (Chrome, Legacy Edge, Chromium Edge), so the boilerplate seems still necessary.
Is this expected? Is there a way to make the auth popup not blocked by default for a site?
@chriskuech that is good feedback, redirect in my opinion should not be treated as a legacy flow. Are there issues you are experiencing trying to implement redirect?
I will try and implement redirect now. This sample is why I did not try to implement redirect prior, as all code paths relevant to my scenario would trigger popup flow.
Some context: I'm trying to use MSAL to obtain an access token from a React app for use with my team's service (no graph scopes necessary). However, I believe I've hit a mix of design, bugs, and doc issues:
Why do we need to handle redirect callbacks in two different ways? We need to both register callbacks with the UserAgentApplication, or else MSAL throws an error that I need to call setRedirectCallbacks. The only similar method I can find is handleRedirectCallback and neither setRedirectCallbacks nor handleRedirectCallback are documented in the wiki. But besides the method, we also have to account for redirect callbacks with branching logic around the condition myMSALObj.getAccount() && !myMSALObj.isCallback(window.location.hash). I find this very confusing.
I'm trying to integrate MSAL with React, but I don't think an idiomatic solution is possible. An idiomatic solution would abstract logic into a Context component (vaguely analogous to a dependency injection container), but doing so would require moving MSAL logic from the module scope (run when the app loads) to the Component scope. This is not possible due to the branching logic required in the module scope.
Reading the docs, it seems that access token retrieval is supposed to be implemented lazily using acquireTokenSilent, but if the token expires, then it must be acquired with a redirect flow (popup does not work as described in previous comments). This means that if a user fills out a form, they will lose all their progress with an unexpected redirect.
I realize now that my proposed wrapper is a little too simple, but I still think it could be implemented if MSAL internally handled refreshing the access token, then the wrapper returned Promise<Account>, but where Account is extended to include a getAccessToken() method that returns the latest refreshed access token. I believe this would sufficiently abstract away the callbacks and cascading try/catches for the login and token acquisition methods, as well as resynchronize control flow from all the callback branching that occurs in the samples. It generally took a long time to understand the samples, as they work with browser-safe JS--it would be nice if there was an ES2017 or TypeScript example.
Redirect callbacks: You should only need to handle the redirect callback with handleRedirectCallback, (which is documented in the README, but could be made clearer in the wiki). I believe that error message referencing setRedirectCallbacks is out of date, so we can fix that. cc: @negoe
The checks for isCallback and getAccount are there due to the way MSAL handles the redirect back from the login page (the app will get reloaded as the url hash containing the response data is removed and stored), and is there so that your app doesn't unnecessarily execute code during that step. In the React sample below, I was able to get away with just checking for the truthiness of getAccount, so you may be able to also.
React: I created a sample React application here, which uses a higher-order component as opposed to context, and works with both popup and redirect logic. I'm hoping to add more React samples, and we could definitely add one that uses context as opposed to HoCs.
Redirect: You are correct that if your app uses the redirect flow, users will lose state when they navigate away. Given that, your app should save any state (e.g. in local storage) you need to persist between redirects.
Note that popup flow does work in those browsers, users just need to allow popups, which isn't optimal, but still possible (e.g. you could provide a hint to your users in the UI that they will need to allow popups for login to work).
Simplifying acquireToken calls: We know we could make the acquireToken flows easier by abstracting away the error handling logic needed after calling acquireTokenSilent, and is something we hope to improve upon.
Samples / Browser JS: We are working on add to and improving our samples, agree they could be easier to understand.
Currently, MSAL.js only supports browsers, but we have plans to build version of the library that will work in Node.
Thanks again for the feedback, let us know if you have any further questions or concerns!
One thing to add regarding token refresh:
MSAL JS not refreshing the token for you is by design AFAIK and is line with MSFT Auth libraries altogether, cc @negoe. This is mainly because tokens are supposed to be on-demand for obvious security reasons. There are however silent ways to refresh your token, more on this here.
@jasonnutter , this react sample is amazing! you should definitely put it in the sample pages. I'm sure it would save people a lot of time.
Thanks! Glad you found it useful. And yeah we'll be sure to add it there soon.
Closing, as there are no further actionable items.
Most helpful comment
@jasonnutter , this react sample is amazing! you should definitely put it in the sample pages. I'm sure it would save people a lot of time.