Should AuthenticationContext.aquireToken() check for "loginError" ?

[CadiChris]

Hi there,

While working with react-adal I faced an issue where I got an infinite login loop for users that are not authorized to access my application.

In other words :

1. User visits http://myapp/
2. Azure authentication window is displayed
3. User logs in, with a account that can't access http://myapp/
4. User is redirected to http://myapp/#error=access_denied
5. Due to adal code (that is the topic of this issue) user is redirected to http://myapp/
6. Azure authentication is displayed (2) 💥 there goes the infinite loop

I went for a bit of digging into the code, to see what was causing the redirection to http://myapp/ at (5).

The result of this investigation is that I don't understand why adal, in acquireToken(), does not check getLoginError() before indicating callback('User login is required', null, 'login required');

In other words, I don't understand why this piece of code doest not check getLoginError():

if (!this._user && !(this.config.extraQueryParameter && this.config.extraQueryParameter.indexOf('login_hint') !== -1)) {
  this.warn('User login is required');
  callback('User login is required', null, 'login required');
  return;
}

this._user is not set, but it could be due to a login error that occurred after the user actually tried to login...no ?

Would it make sense, in acquireToken() to have something like:

if (!this._user && this.getLoginError()) {
  this.warn('User login failed');
  callback('User login failed', null, 'login failed');
  return;
}

// then, the current code
if (!this._user && !(this.config.extraQueryParameter && this.config.extraQueryParameter.indexOf('login_hint') !== -1)) {
  this.warn('User login is required');
  callback('User login is required', null, 'login required');
  return;
}

I can fix my problem by changing code in react-adal, but I'd really like to have your opinion on this.

[miguelemosreverte]

It truly is a shame that such a detailed post is received like this by the community and collaborators of the project.

[miguelemosreverte]

Hey we managed to fix this problem, in a (sadly necessary) work-around way.

Check out *our repo!

[chan4lk]

How did you fix this ? I am still getting this error.

[miguelemosreverte]

@chan4lk I am so sorry I cannot provide you of the repo because the company I worked for decided it would not go the open-source way, and asked for removal.

Now let me tell you, what I did was at most 4 days of work, nothing more. Information is out there, just iterate over the proposed solutions until you land a stable solution and then please come back and propose a link to a repo like I did on November 2018.

Again, sorry for not reproducing the solution I made for the company I worked for, it was up to them to censor it to the community. But still, good news are that I did not provide of a new solution: Just a formalization of the StackOverflow answers on the topic.

[zsid]

Hi,

I am having the same problem with the infinite loop when working with react-adal. Are there any plans to address it soon? Is this repo not supported any more?

[CadiChris]

Hi @zsid. I don't know if changes have happen in the adal library, but a new version of react-adal is now available. Maybe you can try it https://github.com/salvoravida/react-adal/issues/33#issuecomment-536196697

[salvoravida]

@zsid it would be great a feedback on your use case!

Salvo

[zsid]

I will update to the packages later this week and test it out

[jmckennon]

Unfortunately, adal js is generally not being updated except for high priority security fixes. You may have to defer to the react-adal maintainers for a workaround. Infinite redirect loops are a known bug here.

We also recommend that people move to the msal js library here, as this is the library that is getting all of Microsoft's current support. We have plans for an official react implementation on our roadmap here. There is also an active community project called react-aad-msal that helps integrate msal js into a react app.