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

refresh_token flow #10

Closed
davidgilbertson opened this issue Oct 6, 2014 · 7 comments
Closed

refresh_token flow #10

davidgilbertson opened this issue Oct 6, 2014 · 7 comments

Comments

@davidgilbertson
Copy link

Hi,

I can't get the refresh_token process to work. My initial authentication works:

req.session.state = utils.randomString(32);
passport.authenticate('reddit', {
  state: req.session.state,
  duration: 'permanent',
  scope: 'read,vote'
})(req, res, next);

Now, I can, say, vote (via an AJAX call from the client) on a reddit link and it works fine. After an hour, the token expires as expected and I get an {error: 401} response when I try to vote. So when this happens I want to go away in the background, update my token using the refresh token, then continue on with the voting call. So I try this (in the callback from the vote request):

if (data.error === 401) {
  req.session.state = utils.randomString(32);
  passport.authenticate('reddit', {
    state: req.session.state,
    refresh_token: refreshToken,
    grant_type: 'refresh_token'
  })(req, res, next);
}

But this tries to redirect the client so I get this error (in the browser console):
XMLHttpRequest cannot load https://ssl.reddit.com/api/v1/authorize?response_type=code&redirect_uri=[my callback url]&scope=identity&client_id=[my client id]. No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://localhost' is therefore not allowed access.

Please forgive me if this is the wrong place to be asking, I'm not sure if it's an actual issue with passport-reddit or not. I suspect it's me just not understanding something.

Thanks heaps in advance...

@Slotos
Copy link
Owner

Slotos commented Oct 6, 2014

Passport doesn't handle token expiration, it fetches them and uses them once to load user profile. Here's a relevant thread on passport issues.

As for how you should act in this situation - simply fetch a new token from Reddit, as shown here.

Be careful, though, you do want to run refresh only once to avoid race conditions. In a simple single-process case you can simply wrap your refresh in a code, that will collect all the callbacks and run them once refresh is done. If you use cluster or even multi-server architecture, you have a bit of a challenge in front of you, though moving API connector to a separate process might help.

@davidgilbertson
Copy link
Author

Thanks Slotos.

Can I use passport.authenticate() to fetch that new token (which is what I'm doing, but having that redirect issue) or should I be making my own post request and setting Basic Auth for this (and just follow the reddit docs that you linked to)?

I don't quite understand your last paragraph, what do you mean by "run refresh only once"? Do you mean if a user clicks 'vote' (or anything) again while I'm waiting for the first request to come back?

@Slotos
Copy link
Owner

Slotos commented Oct 6, 2014

It's all on your shoulders, I'm afraid. Passport simply doesn't get involved at all. You can reuse strategy.js code, that sets up Basic Auth headers, however.

Do you mean if a user clicks 'vote' (or anything) again while I'm waiting for the first request to come back?

Pretty much. If you don't account for it, you'll have different requests refreshing your tokens before other ones had a chance to use a refreshed one. It is also not unlikely to get your app blocked on reddit side.

@davidgilbertson
Copy link
Author

Awesome, thanks so much. My solution (I'm using the 'request' module) looks something like this:

function refreshRedditToken(req, res, cb) {
  req.session.tokenRefreshInProgress = true; //this is set tested before calling this function
  var refreshToken = req.user.reddit.refreshToken;

  var options = {
    url: 'https://ssl.reddit.com/api/v1/access_token',
    form: {
      grant_type: 'refresh_token',
      refresh_token: refreshToken
    },
    auth: {
      user: REDDIT_CLIENT_ID,
      pass: REDDIT_CLIENT_SECRET,
      sendImmediately: true
    },
    json: true,
    headers: {
      'User-Agent': 'my UA goes here'
    }
  };

  request.post(options, function(err, httpResponse, data) {
    //error handling omitted
    req.user.reddit.token = data.access_token;

    //write the token to the database
    userController.updateToken(req.user._id, data.access_token, function(err) {
      req.session.tokenRefreshInProgress = false;
      cb(err); //callback would try the original action (e.g. vote) again
    });
  });
}

@Slotos
Copy link
Owner

Slotos commented Oct 6, 2014

Note of warning, be careful with session data handling. Your code will not work as you might expect it to if you use cookies as session storage. Unlike server-side storages, it won't update till you respond to client's request.

@davidgilbertson
Copy link
Author

Oh, yeah that's a gotcha! So (newbie question) what is the best way to set a variable server side for a user?

@Slotos
Copy link
Owner

Slotos commented Oct 7, 2014

Generally server-side out-of-process session storage works quite well. While adding some overhead, it will behave persistently for single-process, multi-process and to some degree even for multi-server deployments.

However, it still won't save you from races (I must sound repetitive at this point), it will simply move the control over opportunity window to your side. I.e. instead of having reddit delay determine races, session storage update speed will. But at this point it is good enough to let it occasionally fail with intelligent error message. Fully strengthening it against such races might not be worth the development time past this point (though it's quite fun and frustrating activity).

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

2 participants