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

Sometimes popup does not work but opens my page in it - query params are hijacked #727

Closed
Martinsos opened this issue Feb 4, 2016 · 9 comments

Comments

@Martinsos
Copy link
Contributor

Hi @sahat, thank you for this amazing project, it saved me a lot of time :)!

I have been using satellizer for few months with no issues. I am using linkedin sign in and custom login. Backend is nodejs + express, while both frontend and backend are served on Dokku, Amazon.

Last few weeks I started having a problem with linkedin sign in: on Chrome/Linux and Safari/Mac combinations, it happens often that sign-in popup, after redirect from linkedin, ends up on that same redirect page but does not continue with authorization.
There are no errors/logs written, and to make it even harder to debug, it does not happen all the time, but maybe in 80% of cases.
On Firefox/Linux and Chrome/Mac combinations, all works fine! Also in localhost.
I can confirm that something goes wrong before sending request to my backend - authorization token is fetched from linkedin, redirect is sent, popup is actually redirected and then nothing happens, it stays there.
I am totally stuck, not sure anymore where to look - if you have any idea how to debug this or proceed, it would mean a lot. Thanks!

Video of error (errors at the bottom are produced by some extension): https://youtu.be/DUi9w7qZvqk.

@Martinsos
Copy link
Contributor Author

I did some more testing (added console.logs to satellizer) and I can confirm that the problem happens in pollPopup function -> polling goes forever because although origin gets set, query and hash are never set. I have to figure out why is this happening sometimes, why is query/hash not read/set? Redirect from linkedin has those query string in it, so it should be read by pollPopup, but it is not.

@Martinsos
Copy link
Contributor Author

Martinsos commented Feb 6, 2016

Ok, I managed to fix this one! I will describe the problem and solution so it may help someone else in the same situation.

As I said above, redirect from linkedin was correct (has query params), but satellizer would not see any of them when trying to read them by polling popup's location.
So where did they go? It turned out that I had following setup with my uiRouter: / was redirected to '/home'. Since I did not set any redirectUri with satellizer, it was set to default which was /. So popup's location would be set to /?code=...&state=... and now it was a race -> who is going to get to it first, uiRouter or satellizer?
In case when uiRouter got first, he would redirect it to /home and query parameters would not be kept but lost (that is how uiRouter works). satellizer would continue polling popup's location forever, but query params would never come.
In case when satellizer got first, all was fine -> that is why it was sometimes working, and sometimes not.

I fixed this by removing the race condition - I set redirectUri in satellizer to /home so no redirecting was done by uiRouter and satellizer now has all the time it needs to read those query params.

There is still one mystery unsolved - why did it happen only on some combinations of browser/OS? I am guessing that it has to do something with their speed/caching, but it is only a hunch.

@Martinsos Martinsos changed the title Linkedin sign in does not always work Sometimes popup does not work but opens my page in it - query params are hijacked Feb 6, 2016
@sahat
Copy link
Owner

sahat commented Feb 19, 2016

@Martinsos To further add my own thoughts to this, I found setting redirectUri to something like window.location.origin + /auth/facebook/callbackworks better than just window.location.origin. Then, to avoid 404 errors simply create a server route for /auth/facebook/callback that returns 200 OK.

  1. No more race conditions with ui-router/ngRoute.
  2. If for whatever reason some error occurs and popup isn't closed, users won't be looking at your home page (/ route) inside popup. It will be just a blank page. Additionally, instead of returning 200 OK with empty page, you can also return a custom template that says "Loading". In any case, you have more flexibility than redirecting to base url - window.location.origin.

@Martinsos
Copy link
Contributor Author

Right @sahat, I agree that is the way to go! Maybe mentioning that in the README would be helpful? Right now it is not super clear that url can have path - at least it is not stated explicitly. Having an example of this could be useful.

@sahat
Copy link
Owner

sahat commented Feb 19, 2016

@Martinsos I wasn't sure where else to put it, so I added it to FAQ: https://github.com/sahat/satellizer#faq

@Martinsos
Copy link
Contributor Author

Great @sahat! May be worth noting that it can not contain any hash/query.

@Hesesses
Copy link

Hesesses commented Mar 8, 2016

I might have a problem related to this.

this is my satellizer config:

$authProvider.facebook({
    clientId: $window.appConfig.fbClientId,
    url: $window.appConfig.apiUrl + 'auth/facebook',
    redirectUri: $window.location.origin + '/login',
    scope: ['public_profile','email']
});

$authProvider.logoutRedirect = '/login';
$authProvider.loginUrl = $window.appConfig.apiUrl + 'auth/login';

apiUrl is 'http://api.domain.com/v1/'

The problem is that sometimes everything works as expected, but sometimes the client sends http post request to api.domain.com/v1/auth/facebook without the code parameter. So it only has clientId and redirectUri.

And this gives an error on my backend:
400 Bad Request response: {"error":{"message":"Missing authorization code","type":"OAuthException","code":1"}}

@sahat Do you have any ideas how to solve this?

@chungconscious
Copy link

@Martinsos - awesome find and thanks to @sahat also for the redirect suggestion!

@bostondevin
Copy link

bostondevin commented Apr 24, 2017

I'm using HTML5 mode with dynamic subfolders, so I had to do a little tweaking but this seems to work for me so far:

angular.module('app', []).config(function ($locationProvider, $authProvider){

    $locationProvider.html5Mode({enabled: true, requireBase: false});

    $stateProvider.state('app', {
        url: '/s',
        template: '<div ui-view></div>',
        controller: 'mainController'
    }).state('app.build', {
        url: '/:template/:page',
        templateUrl: 'app/modules/appbuilder/builder.tpl.html',
        controller: 'AppCtrl'
    });

    $authProvider.facebook({clientId: 'xxxxxxxx', redirectUri: window.location.origin + '/s/?linked=true'});
    $authProvider.google({clientId: 'xxxxxxx.apps.googleusercontent.com', redirectUri: window.location.origin + '/s/?linked=true'});
    $authProvider.linkedin({clientId: 'xxxxxxxx', redirectUri: window.location.origin + '/s/?linked=true'});

}).controller('mainController', function($window, $state, $timeout){

    if ($window.location.toString().indexOf('/s/?linked=true') > -1){
        $timeout(function(){ // Necessary to prevent race condition with UI router (site loads in popup)
            $state.go('app.build', {template: _defaultTemplate, page: _indexPage});
        }, 500);
    } else {
        $state.go('app.build', {template: _defaultTemplate, page: _indexPage});
    }
  
});

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

No branches or pull requests

5 participants