Skip to content
This repository has been archived by the owner on Aug 17, 2019. It is now read-only.

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

Open
learntoswim opened this issue Jan 14, 2015 · 21 comments
Labels

Comments

@learntoswim
Copy link

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?

@Pomax
Copy link
Owner

Pomax commented Jan 15, 2015

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

@learntoswim
Copy link
Author

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?

@Pomax
Copy link
Owner

Pomax commented Jan 15, 2015

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

@learntoswim
Copy link
Author

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

@Pomax
Copy link
Owner

Pomax commented Jan 16, 2015

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...

@learntoswim
Copy link
Author

Yeah, I don't think that ever gets called. Could this be related? #50

@Pomax
Copy link
Owner

Pomax commented Jan 17, 2015

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

@Pomax
Copy link
Owner

Pomax commented Jan 18, 2015

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.

@learntoswim
Copy link
Author

Confirmed. Thanks for your time on this.

@Pomax
Copy link
Owner

Pomax commented Jan 19, 2015

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)

@learntoswim
Copy link
Author

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.

@Pomax
Copy link
Owner

Pomax commented Jan 20, 2015

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.

@learntoswim
Copy link
Author

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?

@Pomax
Copy link
Owner

Pomax commented Jan 20, 2015

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

@learntoswim
Copy link
Author

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

@learntoswim
Copy link
Author

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.

@Pomax
Copy link
Owner

Pomax commented Jan 22, 2015

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);
}

@learntoswim
Copy link
Author

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

@Pomax
Copy link
Owner

Pomax commented Jan 23, 2015

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

@learntoswim
Copy link
Author

Hey there, did you still need help with this?

@Pomax
Copy link
Owner

Pomax commented Feb 3, 2015

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants