store access_token in session

criso / fbgraph

NodeJs module to access the facebook graph api

http://criso.github.io/fbgraph/

1.09k stars 176 forks source link

store access_token in session #5

Closed SamDecrock closed 12 years ago

SamDecrock commented 12 years ago

Hi,

Love the work you've done here, but it's a bit tricky because we have to store the access_token in a session:

req.session.access_token = graph.getAccessToken();

And everytime the browser does some action, we have to pull the access_token from the session and set

graph.setAccessToken(req.session.access_token);

Could you add an option to the graph.get function so that we can pass the access_token directly. eg:

graph.get("/me", req.session.access_token, function(err, data) {
    console.log(data);
});

Thx

Edit: fyi, I'm using Express.js session support: http://expressjs.com/guide.html#session-support

criso commented 12 years ago

Once you set the access token on the graph object, any modules that requires graph should have that token set.

SamDecrock commented 12 years ago

the graph-object only holds the latest access token:

User A authenticates, graph.accessToken holds the access token for user A
User B authenticates, graph.accessToken now holds the access token for user B
User A requests his name through '/me', he gets the name of user B because graph.accesToken now holds the access token of user B. This is the problem.

Therefore you have to connect the access token to the current session.

criso commented 12 years ago

doh! good point. I'm thinking that this should probably be added as middleware so that the request is always tied in with the session. Not sure when I'll be able to implement this. If you have time, go nuts!

criso commented 12 years ago

Actually, instead of doing this:

graph.get("/me", req.session.access_token, function(err, data) {
    console.log(data);
});

Just do:

graph
  .setAccessToken(req.session.access_token)
  .get(graph.get("/me", function(err, data) {
      console.log(data);
  });

thisissami commented 12 years ago

just double checking here - is graph.setAccessToken() a synchronous call? aka will doing what you propose just above ALWAYS work without issue?

SamDecrock commented 12 years ago

Yes,

Line 376 states:

exports.setAccessToken = function(token) {
  accessToken = token;
  return this;
};

umosys commented 10 years ago

So does it mean that using:

graph
  .setAccessToken(req.session.access_token)
  .get(graph.get("/me", function(err, data) {
      console.log(data);
  });

will resolve the problem of:

User A authenticates, graph.accessToken holds the access token for user A
User B authenticates, graph.accessToken now holds the access token for user B
User A requests his name through '/me', he gets the name of user B because graph.accesToken now holds the access token of user B. This is the problem.

If one were to do this by accident:

graph.post(...)

we could be using some other user's access token and post to the wrong wall, right?

criso commented 10 years ago

https://github.com/criso/fbgraph#to-use-a-specific-access-token-for-a-particular-request

elado commented 10 years ago

It looks like a serious issue, why does the global setAccessToken even exist?

criso commented 10 years ago

Initial concept was for the accestoken to be static, as in you're going to do a bunch of operations for a particular user token, since I was using Facebook's JS api as a basis. Probably not the best plan in hindsight. Either way, right now you can pass it on each individual request or have it set for all requests.

Hasn't been removed to not break backwards compatibility