node-flickrapi icon indicating copy to clipboard operation
node-flickrapi copied to clipboard

Most likely a problem with Flickr, but looking to see if anyone else had the same symptoms

Open learntoswim opened this issue 10 years ago • 21 comments

Using a custom callback uri in the options, i.e.:

    var options = {
        api_key: api_key,
        secret: secret,
        callback: 'http://mycallbackuri.com/flickr/'
    };

Flickr munges the URL into:

https://www.flickr.com/services/oauth/http%3A%2F%2Fmycallbackuri.com%2Fflickr%2F?oauth_token=xxx-xxx&oauth_verifier=xxxx

And subsequently we are presented with a 404...

Thoughts?

learntoswim avatar Jan 14 '15 21:01 learntoswim

hm, I'll have to look into that, thanks for reporting

Pomax avatar Jan 15 '15 05:01 Pomax

I submitted a pull request. But I would love to understand this some more from your documentation.

The callback URL handler will at its minimum need to implement the following middleware function:

function(req, res) {
  res.write("");
  options.exchange(req.query);
}

Can you provide an example?

learntoswim avatar Jan 15 '15 17:01 learntoswim

Certainly, see this stub server that I use in testing: https://github.com/Pomax/node-flickrapi/blob/master/test.js#L23

Pomax avatar Jan 15 '15 17:01 Pomax

I understand the stub server, though this is where the documentation gets fuzzy, as the options object never gets to have the .exchange method applied in your example and test sequence:

https://github.com/Pomax/node-flickrapi/blob/master/test.js#L34

TypeError: Object #<Object> has no method 'exchange'
    at /xxx/node-flickrapi/test.js:40:21

learntoswim avatar Jan 16 '15 15:01 learntoswim

Hm. https://github.com/Pomax/node-flickrapi/blob/master/src/auth/auth.js#L30 should be adding that function if you run in "not-oob" mode...

Pomax avatar Jan 16 '15 18:01 Pomax

Yeah, I don't think that ever gets called. Could this be related? https://github.com/Pomax/node-flickrapi/issues/50

learntoswim avatar Jan 16 '15 19:01 learntoswim

might be. I'll have time to look into this tomorrow. Let's see what's going on...

Pomax avatar Jan 17 '15 02:01 Pomax

If I use the current master, with an .env without any token information, an export FLICKR_CALLBACK="http://localhost:4321" as oauth callback, and then run node test testAuthenticated, I don't seem to be getting any errors. Authentication succeeds and the console shows the credential object with correct auth tokens and secrets.

Pomax avatar Jan 18 '15 22:01 Pomax

Confirmed. Thanks for your time on this.

learntoswim avatar Jan 19 '15 18:01 learntoswim

So I'm thinking this might be a documentation issue, with inadequate explanation of how to make an oath server work. Would you be willing to comment on where that could be improved if you agree with that assessment? (I'm happy to do the rephrasing and updates to the README)

Pomax avatar Jan 19 '15 21:01 Pomax

Hey, I admit the documentation was a little light. I came into using this package as a quick way to auth with Flickr and use an API wrapper. I didn't really take note of how habitat, an .env and a callback server was crucial to the execution of the auth process. In fact, I was thinking of writing to you to suggest a configuration option to not use a .env if you just want the user credentials returned as JSON. I simply wanted to use a custom callback url to get the user credentials to store in a database.

To give you some more context; I have a system of users who I want to give the ability to register their Flickr accounts with my app. Your auth process & API wrappers are what made me choose, and continue to pursue your package. Though I feel it's written more for a single-user scenario, where only one user will ever be authenticated, not more.

I'd be happy to clarify in the documentation where I felt confused, though I would urge you to consider an option where habitat and a .env is not required. I'd be happy to write that, also. Let me know.

learntoswim avatar Jan 20 '15 03:01 learntoswim

I use it as the basis for a multi-user setup on http://flickr.nihongoresources.com, actually, so if you have good ideas for improving multi-user, I'm all ears.

That said, you don't need habitat and an .env file, they're just the obvious choice for quickly loading in single-user credentials (which, to be fair, is what I initially wrote it for. The only tools for full Flickr syncing were $30+, and I was offended by being asked to pay for what should be near trivially simple...). In my multi-user setup, for instance, I store user credentials in their own objects, and initialise a Flickr API object with those credentials when a user request a route that needs to call an API function.

Pomax avatar Jan 20 '15 03:01 Pomax

Regarding the comment about not needing a .env file, this is where I get caught up currently.

[ 'Error: ENOENT, no such file or directory \'.env\'',
     '    at Object.fs.openSync (fs.js:438:18)',
     '    at Object.fs.readFileSync (fs.js:289:15)',
     '    at Object.options.processCredentials (/xxx/node_modules/flickrapi/src/FlickrAPI.js:113:35)',

Code in question:

if(!options.silent) {
    console.log("Credentials object:");
    console.log(JSON.stringify(data,null,2));
}
var envContent = fs.readFileSync(".env") + "\n";
Object.keys(data).forEach(function(key) {
    envContent += "export " + key + "=" + data[key] + "\n";
});
fs.writeFileSync(".env", envContent);

I don't think I can avoid the .env part of this through a config alone.

I'd be happy to work that into an option if you want?

learntoswim avatar Jan 20 '15 17:01 learntoswim

ah, good point. forgot it did .env read/write during credential negotiation. Yes, let's feature flag that so that it either read/writes an .env, or calls a callback with the user credentials for arbitrary further processing

Pomax avatar Jan 20 '15 17:01 Pomax

Ok, I'll submit a pull request when I have something.

learntoswim avatar Jan 20 '15 17:01 learntoswim

I'm in two minds about using habitat as a requirement for this. Your thoughts?

Playing with this, where the .env is the default options, and you can override that with passed in options:

// pull options from .env file, passed in options.[param] should overwrite default .env settings
if (options.env) {
    var habitat = require("habitat"),
        env = habitat.load(options.env),
        FlickrOptions = env.get("FLICKR");
    if (FlickrOptions) {
        Object.keys(options).forEach(function(key) {
            FlickrOptions[key] = options[key];
        });
    }
}

I'm just not sure where you stand on relying on habitat for the read/write on .env files.

learntoswim avatar Jan 22 '15 18:01 learntoswim

I'd keep this "without any additional options, the new code should behaviour the same way", because the basic use case is a single user syncing their photos, rather than a multi-user setup. I'd probably instead switch on an options.noenv, and if that is specified, also require that an auth callback is passed along so that the user/creds combination can be sent on to whatever will be handling the data storage.

So we get something like

// log credentials to stdout
if(!options.silent) {
  console.log("Credentials object:");
  console.log(JSON.stringify(data,null,2));
}

// write env, unless told not to
if(!options.noenv) {
  var envContent = fs.readFileSync(".env") + "\n";
  Object.keys(data).forEach(function(key) {
      envContent += "export " + key + "=" + data[key] + "\n";
  });
  fs.writeFileSync(".env", envContent);
}

// send credentials on for (additional) processing
if(options.credentialHandler)
  // note that much earlier in the code, such as before
  // any real code in authenticate() is run, we may want to
  // verify that IF .noenv is set, then .credentialHandler
  // is also set. It would be silly to wait until this point in
  // the negotiation to throw an error if we have .noenv
  // but no .credentialHandler
  options.credentialHandler(data);
}

Pomax avatar Jan 22 '15 19:01 Pomax

Makes sense! I'll allow you to do the honors. :)

learntoswim avatar Jan 23 '15 01:01 learntoswim

I'll try to work that in, and update the README.md, on saturday.

Pomax avatar Jan 23 '15 06:01 Pomax

Hey there, did you still need help with this?

learntoswim avatar Feb 02 '15 13:02 learntoswim

been super busy, so I've not had time to work this in (I know, it shouldn't take long, but that's still longer than I've had so far...). If you want to work this in still, that'd be awesome

Pomax avatar Feb 03 '15 19:02 Pomax