Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Msal Intermittently returning null in redirect uri on IE/Edge/Safari #347

Closed
adamtay opened this issue Jul 17, 2018 · 30 comments
Closed

Msal Intermittently returning null in redirect uri on IE/Edge/Safari #347

adamtay opened this issue Jul 17, 2018 · 30 comments
Assignees
Labels
b2c Related to Azure B2C library-specific issues bug A problem that needs to be fixed for the feature to function as intended.
Milestone

Comments

@adamtay
Copy link

adamtay commented Jul 17, 2018

Hi,

I understand that this is a known issue as per the library wiki, however, Safari (MacOS X/iOS) appears to also be impacted by this issue.

Tested browsers:

  • IE 11.1155.15063.0
  • Edge 40.15063.674.0
  • Safari 11.1.2 (MacOS X High Sierra 10.13.6)
  • Safari (iOS 11.4.1)

Our solution:

  • React v16.4.1 (via create-react-app)
  • Msal v0.1.7
  • Utilizing sessionStorage (we are unable to utilize localStorage due to our applications use case. Note that we also experience the same issue on localStorage)
  • Using the loginRedirect method

As per #330 (comment), adding navigateToLoginRedirectUri: false to the UserAgentApplication options appears to somewhat alleviate this issue on IE/Safari but not on Edge.

However, we are seeing mixed and intermittent results when testing between a normal browser session and private/incognito session for our tests browsers. i.e. Working on normal session Safari but not on private session.

As we are releasing a public facing website, we cannot be asking our end-users to be adding the websites domain to their 'trusted website' lists in order for msal authentication to function as intended.

If a work-around for this issue will not be ready within the upcoming months, please advise a suitable alternative for AADB2C integration. (perhaps hello.js?)

Thanks

@nehaagrawal nehaagrawal self-assigned this Jul 18, 2018
@nehaagrawal
Copy link
Contributor

@adamtay We are currently working on a fix for this issue and we are planing to release it in our future release.

@sayanghosh123
Copy link

@nehaagrawal Thanks for the quick response. We would need to understand a timeline for this fix please, as we are smack dab in the middle of a SDLC programme which is dependent on this. We are happy to get into Private previews if those allow us to test out the fix. An update will be appreciated.

@nehaagrawal nehaagrawal added the bug A problem that needs to be fixed for the feature to function as intended. label Jul 18, 2018
@jburman
Copy link

jburman commented Jul 24, 2018

Also ran into this issue tonight in Firefox 61 (Windows 10).

@vladkosarev
Copy link

@nehaagrawal what is the timeline for the fix? Are there any other alternatives that do work with b2c and modern browsers?

Thanks.

@vladkosarev
Copy link

We converted our app to hellojs and that actually mostly works (other than popup+IE). So if you need to make sure that your app actually works with multiple browsers give hellojs a try. It's a but more verbose than MSAL but it's not too far off.

@nehaagrawal
Copy link
Contributor

nehaagrawal commented Aug 18, 2018

@vladkosarev @jburman @sayanghosh123 @adamtay This is fixed and released in MSAL 0.2.3 . Please check release notes for more details.

@bh3605
Copy link

bh3605 commented Oct 1, 2018

This isn't really fixed. You are storing the state in a cookie which preserves it, but when you go to retrieve it you aren't passing the storeAuthStateInCookie flag into the getItem method. This causes the code to attempt to retrieve from the cleared out sessionStorage, instead of the cookie. I'm referring specifically to line 1747 when you are testing its existence and then on line 1747 when you go to retrieve it. It's in the saveTokenFromHash in the UserAgentApplication class.

@nehaagrawal
Copy link
Contributor

nehaagrawal commented Oct 11, 2018

@bh3605 Could you please confirm what version of msal you are using? Also can you please send the link for the lines you are talking about. It seems the issue that you are talking about doesn't match the line number.

@bh3605
Copy link

bh3605 commented Oct 11, 2018

0.2.3

This is the section of code I'm referring to between lines 1744 and 1749 in the saveTokenFromHash method.

authorityKey = Constants.authority + Constants.resourceDelimeter + tokenResponse.stateResponse;
let authority: string;

if (!Utils.isEmpty(this._cacheStorage.getItem(authorityKey))) {
     authority = this._cacheStorage.getItem(authorityKey);
     authority = Utils.replaceFirstPath(authority, idToken.tenantId);
 }

@nehaagrawal
Copy link
Contributor

nehaagrawal commented Oct 11, 2018

@bh3605 the line number you mentioned doesn't handle state but authorityKey. We are storing and fetching the state correctly from cookie by passing the flag 'storeAuthStateInCookie'.
this._cacheStorage.getItem(Constants.stateLogin, this.storeAuthStateInCookie)

why do you think that authorityKey needs to be stored in the cookie? Also what is the error you are getting?

@bh3605
Copy link

bh3605 commented Oct 11, 2018

I redirect to b2c to handle my sign in. In IE when I come back from b2c the sessionStorage will be cleared. You then handle the authentication response because of the presence of the hash. At this point in time nothing has been placed into sessionStorage because that code hasn't ran yet. When it gets the authority key from cache it will be null because it hasn't been written into the storageCache.

I've switched frameworks by now because my app is about to be deployed soon so I forget the exact error I was getting. I think I was getting constantly redirected to b2c because it wasn't able to create a user object and when I check to get a user if it's null I redirect the user to b2c to sign in.

@adamtay
Copy link
Author

adamtay commented Oct 11, 2018

@bh3605 Which library have you converted to (presumably hello.js?) and how have your experiences been with it so far?

@rohitnarula7176
Copy link
Contributor

@bh3605 We already have a fix for this authority issue in the following PR. It will soon be merged and released. Can you please try the branch and see if it resolves your issue. #452

@bh3605
Copy link

bh3605 commented Oct 11, 2018

It's neat you presume hello.js because that's the library our Microsoft Rep recommended and I was all for msal.js until this problem. After realizing I had to switch, I evaluated hello.js, angular-oauth2-oidc, oidc-client.js, and angular-auth-oidc-client. I ended up using oidc-client.js. I made a service to interact with it and it's really come along really well. Everything you call from oidc-client returns a promise so I make assumptions to determine if you're signed in and store the user in the service so not everything has to deal with a promise. Session storage provides better security over cookies so after someone completes sign in I take the user object, clear cookies, and tell the framework to use session storage. Odic-client.js is just enough, provides the ability to use a custom storage implementation (like an obj that stores cookies) and the ability to switch out configurations like when I switch from cookie storage to session storage. There are events you can tie in to. It doesn't go overboard with trying to do everything for you or completely fail at certain things like angular-oauth or angular-auth. Hello.js's problem is it doesn't do enough out of the box and I would have to do A LOT of stuff that all the other frameworks automatically do that I just didn't have time to implement but it is a great starting point if you have to deal with multiple IDPs

@darrelmiller
Copy link

@rohitnarula7176 Can you explain how the PR fixes this issue? As @bh3605 points out in his code snippet, when you retrieve the authorityKey it does not appear to be getting the authority from the cookie.

In the PR comments you state:

The entire storage can get wiped out during the transition between zones. Since the tokens are tied to authority, we need a way to restore the authority from cookie as well otherwise all the tokens will be saved with authority set to undefined.

When you store the authorityKey you do this:

 this._cacheStorage.setItem(authorityKey, this.authority, this.storeAuthStateInCookie);

but when you retrieve it, you do this

  authority = this._cacheStorage.getItem(authorityKey);

i.e. you don't pass in this.storeAuthStateInCookie.

I'm really just reiterating what @bh3605 previously said, but I don't see where we have addressed his concern.

@darrelmiller
Copy link

@rohitnarula7176 Thank you for the details. I was sure I had looked at the PR and I was convinced the this.storeAuthStateInCookie parameter wasn't provided in line 1676. But I was wrong. It does look to me like the PR will address @bh3605 's concerns.

@bh3605
Copy link

bh3605 commented Oct 12, 2018

@darrelmiller You're confusion is not unfounded. I was looking at the PR on mobile yesterday and was confused as to why I saw he was passing in the flag, then he wasn't, and then he was again. As far as I could tell, this was the only thing blocking IE and Edge from working. You can get IE working if you have the dev console opened. Maybe a tester had their console open?

Thank you @rohitnarula7176 for the fix!

@baltuonis
Copy link

So this was not actually fixed was it?
We use 0.2.3 and we still sometimes get "invalid_state" with state=null in latest Firefox

@adamtay
Copy link
Author

adamtay commented Jan 16, 2019

@baltuonis Have you initialized UserAgentApplication with the storeAuthStateInCookie flag set to true? The fix as part of 0.2.3 was to store state as part of the cookies.

@baltuonis
Copy link

Yeah but we can still reproduce the issue

@MarcusPenn
Copy link

MarcusPenn commented Jan 31, 2019

This issue is not fixed.
I am using version 0.2.4 and need to support Chrome, IE11, Firefox and Edge.
I am testing all changes I make with a new InPrivate browsing session after every change.

Using loginPopup, Chrome is the only browser that works 100% of the time.
Firefox and IE11 require a refresh after logging in to acquire the access token.
Edge always redirects to the /null in the popup.

Switching over to use loginRedirect, Edge still always redirects to /null

Adding the storeAuthStateInCookie property and setting to true no longer redirects to /null. However no id token is set in local storage. This is the same for both loginRedirect and loginPopup.

@nehaagrawal can you reopen this issue to reflect this. I will continue to update this comment as I investigate in detail.

Looking into the the Microsoft docs to see how they suggest handling IE and Edge doesn't provide any answers.

Spent some more time looking at this. I can get the id token from the redirect. Following this I then try to get the access token on redirect. Coming back from the redirect for the access token the UserAgentApplication then crashes on setup as there is no id token.

With the logger attached:

logged: Thu, 31 Jan 2019 15:22:05 GMT:0.2.4-Info Processing the callback from redirect response
eval code (2327) (28,5)

logged: Thu, 31 Jan 2019 15:22:05 GMT:0.2.4-Info State status:true; Request type:RENEW_TOKEN
eval code (2327) (28,5)

logged: Thu, 31 Jan 2019 15:22:05 GMT:0.2.4-Info State is right
eval code (2327) (28,5)

logged: Thu, 31 Jan 2019 15:22:05 GMT:0.2.4-Info Fragment has access token
eval code (2327) (28,5)

SCRIPT5022: null or empty raw idtoken
IdToken.js (32,1)

@navyasric navyasric reopened this Apr 9, 2019
@navyasric navyasric added the core label Apr 10, 2019
@sameerag sameerag assigned pkanher617 and unassigned nehaagrawal May 1, 2019
@sameerag sameerag added this to the 1.0.0 milestone May 1, 2019
@sameerag
Copy link
Member

sameerag commented May 2, 2019

@MarcusPenn we cannot store idTokens in cookies. The length is a problem and so is the security. This specific case for IE/Edge is a browser limitation that is beyond MSALs reach. We have our guidance documented here: Known Issues for IE/Edge

One option is to save the idToken once it is emitted at the application end and append the response with it. This is more easier probably with our new API surface which has a fleshed out 'response' object.

Please download our latest preview package or pull the dev branch and try updating your code and see if the issue still persists.

We are also now throwing error stack traces which may help us for any other issues you are facing.

If you would like guidance on how to use the new version of the library, please review our wiki page here.

@sameerag sameerag closed this as completed May 2, 2019
@RichardPergament
Copy link

Feels a bit funky though, that even though one selects a full Microsoft stack, to cater enterprise users who default to Edge/IE, the pieces still don't interoperate.

@sameerag sameerag added the b2c Related to Azure B2C library-specific issues label May 30, 2019
@PraveenVerma17
Copy link

Today I saw few users are getting null in redirect uri that too in chrome. This mostly happens with guest users not with the same domain user. Guest means gmail, hotmail etc where MFA or conditional access is setup.

Microsoft should address this as these issues are really annoying.

@MubashirEbad
Copy link

@nehaagrawal Hi, so I am facing an issue, after I successfully login using the msal loginpopup, I should be redirected to a the URL I have specified, and in most of the cases and OS, it redirects and works perfectly fine. But I have some rare cases such as an MSI laptop with windows OS, which does not redirect and returns www.my_url/null.
I have used Chrome, Firefox, Edge, but only on this laptop I have this certain problem.

I am not able to debug it somehow.

@AndrewCraswell
Copy link
Contributor

AndrewCraswell commented Oct 9, 2019

My app which is about to launch to production is currently in pilot, but a few customers are reporting on Edge this is still occurring, where the site is attempting to redirect to [sitename].com/null. This is a blocker for our site to release at the moment... Doesn't seem like this issue should be closed unless it's being tracked somewhere else.

@MubashirEbad
Copy link

Yeah this issue still exists, it returns .com/null and throw the content not found Error.
This is really a blocker and occurs more frequently in Incognito mode too.
@nehaagrawal Please have a look.

@PraveenVerma17
Copy link

This issue came back.... we started seeing null in url. Please have a look.

@jasonnutter
Copy link
Contributor

We'll be tracking this issue in #1039 (please leave any future comments on that issue), and we'll investigate to get a fix in for this soon.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
b2c Related to Azure B2C library-specific issues bug A problem that needs to be fixed for the feature to function as intended.
Projects
None yet
Development

No branches or pull requests