jammus / lastfm-node

Read and write to Last.fm with node.js
http://return12.tumblr.com/tagged/lastfm-node
MIT License
202 stars 33 forks source link

Invalid medthod signature when using authorise method #13

Closed maxkueng closed 11 years ago

maxkueng commented 11 years ago

Hello

Creating the method signature fails when scrobbling a track without an MBID or when using the authorise method.

I hope this helps. Thank you very much.

maxkueng commented 11 years ago

Just realized I should have created a branch for this issue instead of using master. I hope it doesn't cause any problems.

jammus commented 11 years ago

Hi Max,

Many thanks for the patch. I really appreciate it. Don't worry about the separate branch, I'll figure it out.

Great spot with the MBID parameter, thank you. I'll merge this change in and get it out with the 0.8.3 release.

Regarding the auth.getsession issue, I can't reproduce the problem. Could you create a test case for me (or privately link me to some code which suffers from the problem)? I'm using the session.authorise() on http://boxsocialfm.com (see https://github.com/jammus/boxsocial/blob/master/controllers/logincontroller.js for usage) and session validate correctly. I also tried the following steps.

  1. Log in to http://www.last.fm.
  2. Visit http://www.last.fm/api/auth?api_key=[your_api_key]&cb=http%3A%2F%2F127.0.0.1
  3. Click 'yes, allow access'.
  4. After being redirect note down token from http://127.0.0.1/?token=[token]
  5. At /path/to/lastfm-node type node lastfm-repl.js (if this doesn't work change line 22 of lastfm-repl.js to var context = repl.start({}).context;
  6. Type var session = lastfm.session();
  7. Type session.authorise('[token_from_step_4]', { handlers: _echoHandlers });
  8. session.user and session.key should now contain your session details.

If you're still seeing problems could you trying outputting and attaching the values of data in LastFmRequest#sendRequest and sig in LastFmRequest#createSignature (blanking out any api_key, session and secret details)? From the above my values were:

Note that the casing of the method value in both of these is identical. The Last.fm API constructs the expected signature from the key/value pairs supplied in the request. So if the method is set to AUTH.GETSESSION it will construct the signature from that exact value. The client is expected to do the same.

Again, thanks for the patch. I'll merge the first part in now and add you as a contributor in a minute. If you can provide a test case for part two then we'll get that added or addressed separately.

Cheers, James

jammus commented 11 years ago

1a252b0 merged and published in v0.8.3. Thanks.

maxkueng commented 11 years ago

Hi James

thanks for merging 1a252b0! And thanks for crediting me as a contributor for just one commit. I blushed a bit ;)

Regarding the "getsession" issue, it's false alarm. It must have been the just the "mbid" thing that was wrong and the "getSession" method was something that I fixed along while debugging the signature, thinking both helped to fix the bug - but no. So 0.8.3 works perfectly and I can no longer reproduce the issue :)

If you like consistency, you may sill but an upper-case "S" in "getSession" as you are also using mixed-case method names in lib/lastfm/lastfm-update.js on line 51. Or lowercase it all. Or leave it, it's not a bug.

, requestMethod = method == "scrobble" ? "track.scrobble" : "track.updateNowPlaying";

Thanks again! Your library saved me a lot of work.

Cheers Max

jammus commented 11 years ago

No problems. Glad everything is working. Good spot on the casing inconsistency too, I'll sort that next time I'm in there (and one I've figured out what I want to standardise on).

Thanks again, James