Access token is set to the whole app !

[WalidOfNow]

i'm working on multi sessions app (user login/logout) so when when set_accessToken is triggered the access_token of current user is set to all online users so any user of those can post under the name of last user !!!

[criso]

fbgraph is static. It's not set to the whole app, it's set to fbgraph.

Just do:

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

[kennu]

I also have trouble understanding this. Are you supposed to call .setAccessToken() every time immediately before calling .get(), to make sure it's the right token? Why not make it an argument to get() so that there will be no race conditions?

[criso]

The idea for accesstoken wasn't to change it on every request. I didn't really picture people hitting get on a for loop switching the access token. It was more of a "do a bunch of operations for X user... now do a bunch for Y user", which is why the token was static.

Anyhow, there's very little possibility for a race condition here. The token would be set synchronously. But there's still a possibility, I guess. Can't really work on this right now. I'll put in a patch when I get a chance, or you can submit something :tada:

[kennu]

My current use case is a multi-player server, where I post Facebook notifications inside the server logic when a user gets a new highscore. So it seems "wrong" to keep the token in a global variable, always changing the state of the whole fbgraph module briefly. But I assume it works as long as the API call is always made immediately after setAccessToken(). I can make a patch if I get any unforeseen problems.

[sagish]

@kennu exactly my thoughts when seeing this :\

[Prinzhorn]

@kennu exactly my thoughts when seeing this :\

dito.

Example which in theory causes race conditions:

graph.setAccessToken(access_token).get('/me', function(err, res) {

    //At this point the token could have been overwritten.

    graph.get('/me/accounts', function(err, res) {
        //This could return the accounts of someone else!
    });
});

+1 for creating a class FBGraph which needs to be instantiated and keeps it's token.

var FBGraph = require('fbgraph').FBGraph;
var graph = new FBGraph(access_token);

graph.get('/me', function(err, res) {
    //...
});

[criso]

Switching it to detect the access_token on the get call will be easier and won't break backwards compatibility. So the cal will become

graph.get('/me', {access_token: 'foo'}, function()})

[sagish]

@criso +1 on graph.get('/me', {access_token: 'foo'}, function()})

[Prinzhorn]

I can live with that (having setAccessToken to be optional), because basically it's just another get/post param, nothing more. On the other hand we still have to repeat ourselves and having setAccessToken still around leaves the door open for race conditions, especially for people who are new to the async world.

[Prinzhorn]

I have to raise another point: how we gonna pass access_token to DELETE requests, without breaking backwards compat? The current del function only accepts a single id, no object.

[criso]

@Prinzhorn del passes through to post so it's just a matter of adding the second param and detecting if it's an object or a callback function.

setAccessToken would start outputting a deprecated log to the console for now

[Prinzhorn]

I see. Sounds good.

[Ferdy89]

@criso I'm going to love this new feature. Can't wait for the new version with it

[criso]

This has been fixed as part of issue #34

Basically, passing access_token in the url trumps the static setting.