jaredhanson / passport

Simple, unobtrusive authentication for Node.js.
https://www.passportjs.org?utm_source=github&utm_medium=referral&utm_campaign=passport&utm_content=about
MIT License
22.65k stars 1.23k forks source link

angularjs automatic redirect #318

Open ArdentZeal opened 9 years ago

ArdentZeal commented 9 years ago

Hello there,

I am using passport with the local, facebook, github module and angularjs in my application.

With the standard passport behaviour, I get a redirect header which the browser tries to follow. Passport code below

strategy.redirect = function(url, status) {
        // NOTE: Do not use `res.redirect` from Express, because it can't decide
        //       what it wants.
        //
        //       Express 2.x: res.redirect(url, status)
        //       Express 3.x: res.redirect(status, url) -OR- res.redirect(url, status)
        //         - as of 3.14.0, deprecated warnings are issued if res.redirect(url, status)
        //           is used
        //       Express 4.x: res.redirect(status, url)
        //         - all versions (as of 4.8.7) continue to accept res.redirect(url, status)
        //           but issue deprecated versions

        res.statusCode = status || 302;
        res.setHeader('Location', url);
        res.setHeader('Content-Length', '0');
        res.end();
      };

The problem is, that this automatic redirect call violates CORS policy and so will not be successful.

XMLHttpRequest cannot load ##skipped url##. No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin ##skipped url## is therefore not allowed access.

There is no way to prevent the redirect in the application, because it is done by the browser before the response is hitting the angularjs client.

WORKAROUND:

I have built a functional workaround (read: hack) to implement that feature nonetheless, but I would like to discuss a way to implement it into the framework without the need to hack the response object

var fakeEndHack = function (req, res, next) {
    res._originalEnd = res.end;
    res.end = function() {
        var url = res.get('Location');
        res.end = res._originalEnd;
        res.status(200).send(url);
    };
    next(null);
};
AuthenticationRouter.post('/loginThirdParty', fakeEndHack, AuthenticationController.loginThirdParty);

my proposal would be a setting on passport, to send redirect urls to the client via data, maybe

options.clientHandlesRedirect = true;

strategy.redirect = function(url, status) {
        // NOTE: Do not use `res.redirect` from Express, because it can't decide
        //       what it wants.
        //
        //       Express 2.x: res.redirect(url, status)
        //       Express 3.x: res.redirect(status, url) -OR- res.redirect(url, status)
        //         - as of 3.14.0, deprecated warnings are issued if res.redirect(url, status)
        //           is used
        //       Express 4.x: res.redirect(status, url)
        //         - all versions (as of 4.8.7) continue to accept res.redirect(url, status)
        //           but issue deprecated versions

       if(options.clientHandlesRedirect === true) {
             res.status(status || 302).send(url);
       } else {
             res.statusCode = status || 302;
             res.setHeader('Location', url);
             res.setHeader('Content-Length', '0');
             res.end();
       }
      };

Kind regards

sharduul commented 8 years ago

how does the AuthenticationController.loginThirdParty look like?

ghost commented 8 years ago

I am also interested in this, ran in to this same issue today. @ArdentZeal

ghost commented 8 years ago

It's actually the first time I've ran into any issue at all with passport, and have used passport for a while now. So, just wanted to add this comment to say thanks for a great package @jaredhanson.