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

TokenStrategy doesn't play nicely with Express Routes #13

Open
dpjanes opened this issue Mar 12, 2016 · 4 comments
Open

TokenStrategy doesn't play nicely with Express Routes #13

dpjanes opened this issue Mar 12, 2016 · 4 comments

Comments

@dpjanes
Copy link

dpjanes commented Mar 12, 2016

Let's say you create an app with a router (free typing follows)

route = express.Router();
route.get('/profile',   
    passport.authenticate('token', {
            session: false
     }), _get_profile);

app = express();
app.use("/api/1.0", route);

when helpers.originalURL is called, it will come up with /profile/ which then will be fed into the HMAC-SHA1 algorithm for the base (at around line 206 of token.js)

Unfortunately, the client has computed the base using /api/1.0/profile and so the signatures don't match.

@jdormit
Copy link

jdormit commented Sep 11, 2016

I have run into this as well. Did you come up with a solution?

@dpjanes
Copy link
Author

dpjanes commented Sep 16, 2016

OMG I did but I have no idea what it was. I think (a very vague memory) that it was horrible, whatever I did to fix it. Sorry.

@jdormit
Copy link

jdormit commented Sep 16, 2016

I ended up figuring out a good solution:

In /lib/passport-http-oauth/strategies/util.js:

exports.originalURL = function(req, defaultHost) {
  var parsedUrl = req.url;
  // Handle case where req.url is '/' or '/?foo=bar'
  if (req.url.match(/^\/($|\?)/)) {
      parsedUrl = req.url.slice(1);
  }
  var headers = req.headers
    , protocol = (req.connection.encrypted || req.headers['x-forwarded-proto'] == 'https')
               ? 'https'
               : 'http'
    , host = defaultHost || headers.host
    , path = (req.baseUrl || '') + parsedUrl || ''; // this computes the full original url, including nested routes
  return protocol + '://' + host + path;
};

I didn't submit a PR, since this project doesn't seem to be actively maintained anymore, but my fork has a complete implementation.

@jarrodek
Copy link

I just run into the same problem today. I've found the same problem in the same line as @jdormit
I'll use his fork with the fix.

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

3 participants