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

Erratic behavior: Popup window do not close #629

Open
paulocheque opened this issue Nov 3, 2015 · 19 comments
Open

Erratic behavior: Popup window do not close #629

paulocheque opened this issue Nov 3, 2015 · 19 comments

Comments

@paulocheque
Copy link

This is an erratic error. I dont know if it is a browser cache, satellizer issue or Provider's issue. I thought it would be a Google issue because of this: google/google-api-javascript-client#74

But I faced this issue with LinkedIn too.

  1. Click in the button "sign in with Google or Linkedin"
  2. The user authorize the application
    Then the popup does not close and the website refresh itself in the popup.

Any ideas?

Thanks in advance

@sahat
Copy link
Owner

sahat commented Nov 4, 2015

Have you tried it on https://satellizer.herokuapp.com? I will need more information to know why the popup did not close.

What you saw inside the popup is a normal behavior. It redirects to itself, grabs the code parameter, then closes really fast.

@paulocheque
Copy link
Author

I have tried the app. It is hard to me to get more information. If I get anything I will post here.

Maybe it is something related with AngularJS and $locationProvider.html5Mode(true).

@paulocheque
Copy link
Author

Satellizer: bower: "satellizer": "0.13.*",
Chrome Version 46.0.2490.80 (64-bit)
OS: OSX 10.9.5

AngularJS 1.4.x integration with $locationProvider.html5Mode(true)

When the authentication succeed, my server receive two requests:
GET /auth/google
POST /auth/google

When it fails, it receive only the GET /auth/google.

The POST request is sent by the main window after the popup is closed, right?

Reading the Satellizer source code, I saw a piece of code like this during the popup pooling:

       } catch (error) {
          // Ignore DOMException: Blocked a frame with origin from accessing a cross-origin frame.
        }

I will try to investigate if I am getting some error there.

@nass600
Copy link

nass600 commented Jan 11, 2016

Hi @paulocheque. Did you find why is this happening?

@paulocheque
Copy link
Author

Not really, but I have two hypothesis:

  1. it was related to AngularJS. Probably some async issue, since I was using some Angular variables in Satellizer.

  2. Some browser cache issue. So I added a ?random=new Date().getTime() into my GET parameters to avoid that.

I made many changes and it appears to have fixed.

Thanks!

@sahat
Copy link
Owner

sahat commented Jan 12, 2016

@paulocheque @nass600 Can you try something for me?

Edit satellizer.js code starting on line 753. Add console.log here and comment out the line which closes the popup.

if (popupWindowOrigin === documentOrigin && (Popup.popupWindow.location.search || Popup.popupWindow.location.hash)) {
  var queryParams = Popup.popupWindow.location.search.substring(1).replace(/\/$/, '');
  var hashParams = Popup.popupWindow.location.hash.substring(1).replace(/[\/$]/, '');
  var hash = utils.parseQueryString(hashParams);
  var qs = utils.parseQueryString(queryParams);

  // Print current URL location to console
  console.log(Popup.popupWindow.location);

  angular.extend(qs, hash);

  if (qs.error) {
    deferred.reject(qs);
  } else {
    deferred.resolve(qs);
  }

  $interval.cancel(polling);

  // Do not close the popup
  //Popup.popupWindow.close();
}

Then click on this popup and open Chrome Developer Tools to check console output, as well as potential errors that could cause this issue.

@sahat
Copy link
Owner

sahat commented Jan 12, 2016

One more thing, @paulocheque are you doing any page redirects on root path? For example, when you go to http://yoursite.com, does it try to automatically take you to http://yoursite.com/login?

@nass600
Copy link

nass600 commented Jan 12, 2016

My problem is I am not getting anything in (Popup.popupWindow.location.search || Popup.popupWindow.location.hash) so that way the flow does not reach the if clause and therefore the popup is not being closed.

Maybe it has something to do with cache as @paulocheque suggests so I am going to try the random query param trick to see if this solves the issue.

Thanks guys

@nass600
Copy link

nass600 commented Jan 12, 2016

Well, after some hours of research I have narrowed the error to a cache issue in the angular application web server (Nginx). The random query param didn't work for me all the time.

I have page redirects on root path and they don't seem to affect the behaviour.
It seems that the redirect_uri was being cached somehow when my API redirects the successful response back to my angular app.

The lines I added to the virtual hosts were:

  location / {
    if_modified_since off;
    add_header Last-Modified "";
  }

Not sure if this is correct patch (because now the app loads slower) but it works for me right now.

@Ngunyi-Pertech
Copy link

Hey, some of you might be having the same problem as mine of the popupWindowOrigin not being equal as the redirectUri. It makes some authentication methods work and other not to work. In my case Facebook worked but Google didn't. I fixed it by adding the following code at line 752. It removes any trailing slashes:

//Treat popup window origin and redirectUri popupWindowOrigin = popupWindowOrigin.trim(); if(popupWindowOrigin.endsWith('/')){ popupWindowOrigin = popupWindowOrigin.substr(0, popupWindowOrigin.length-1); } redirectUri = redirectUri.trim(); if(redirectUri.endsWith('/')){ redirectUri = redirectUri.substr(0, redirectUri.length-1); }

@Martinsos
Copy link
Contributor

@nass600 check out this issue #727 - I had problem very similar to yours, and although I was also suspecting nginx at the beginning, I figured out later that the problem was in query params being hijacked by uiRouter's redirect. I still think that caching is somehow involved with all this but more as a mean that manifested the real problem, not the cause.

@nass600
Copy link

nass600 commented Feb 8, 2016

I'll check the issue. Thanks @Martinsos

@aligajani
Copy link

This bug still persists on iOS Firefox with 0.14.* @sahat

@hermanschutte
Copy link

hermanschutte commented Jun 6, 2016

I had a similar issue where the popup was closing for some users and not for others. In the end, it turns out it happened because some users had been visiting www.example.com and others example.com. This causes the popupWindowPath === redirectUriPath check in Satellizer's pollPopup method to remain unsatisfied due to the difference in domains.

The solution was to force all users to the non-www url.

@chungconscious
Copy link

@nass600 @aligajani
check out #885 and #829 - no resolutions yet but I think they're related to your issues

@aligajani
Copy link

@chungconscious True, it works for me most of the time though.
Nonetheless, ping me when there's a solution please :)

@igal1c0de4n
Copy link

Adding a note that it's still not solved for us. Problem happens on Chrome at about 70% of the login attempts (with FB or Linkedin) and a 100% of the attempts on Safari. Help!!

@Martinsos
Copy link
Contributor

Martinsos commented Apr 21, 2017

@knigal This sounds a lot like the problem that I had, I described it, including the solution, here: #727 - I recommend you give it a look.
Can you check at which moment the error happens? Does request get sent to your server? It probably crashes before that, after it receives response from LinkedIn and tries to redirect the popup window, but redirect probably fails at that moment for some reason.
Is maybe promise rejected, have you checked that out? If yes, what is the error message?
For debugging, I suggest checking out this function: https://github.com/sahat/satellizer/blob/master/src/popup.ts#L72, it is where satellizer waits for redirect to happen in order to catch the query parameters from it and continue working with them. You could try putting some logs here, to see if actually reads that params or not, and if not, figure out why. In my case, routing mechanism in angular was removing them before satellizer could read them, in most cases - there was a race condition and that is why it was behaving erratic, exhibiting exactly the same behavior as you described.

@igal1c0de4n
Copy link

igal1c0de4n commented Apr 21, 2017

Thanks @Martinsos your tip is right on target in my case.

A couple of days ago we solved the issue similarly. That's quite good since login stopped working completely in Safari, and panic was creeping in :-0

In case it'd be of use to others, I'll add a short summary:

I tracked down the problem to Satellizer's Popup.prototype.polling function. After authentication with the provider, it is testing:

if (popupWindowPath === redirectUriPath) {

This lines confirms that the url of the popup is same as value of the provider's redirectUri.
The issue was indeed a race condition between this polling function and a ui-router redirect which was set up to fwd the root of the app to a substate with a different url:

    $urlRouterProvider.when('/', () => {
        $state.transitionTo('some.other.state');
    });

I've added some logging which exposed that the value of popupWindowPath never equaled redirectUri. Essentially, the following condition was not met:

if ("https://app.mapme.com" === "https://app.mapme.com/other/path") {

So the authentication was completed successfully, but the root fwd rule was messing things up.

To fix it, I ended up doing as following:

  1. Configured non-empty redirectUri:
    .config(function ($authProvider) {
        $authProvider.baseUrl = ... ;
        const redirectUri = window.location.origin + '/auth-endpoint';
        $authProvider.facebook({
            clientId: appId,
            redirectUri,
        });
  1. Added a ui-router state, such as:
    $stateProvider.state('authEndpoint', {
            url: '/auth-endpoint',
        })
  1. Added to the providers (fb/linkedin) acceptable redirectUri page the new redirectUri path: https://app.mapme.com/auth-endpoint so that they don't redirect to my application's root.

Once all that was set up, Satellizer's polling function always unblocked and the authentication popup closed

So, the issue is 100% solved for me. Thanks everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants