drone29a / clj-oauth

OAuth Consumer support for Clojure
BSD 2-Clause "Simplified" License
273 stars 52 forks source link

Etsy flow #30

Closed danielsz closed 10 years ago

danielsz commented 10 years ago

Hi,

I've used clj-oauth succesfully with Twitter. I'm now trying to use it against Etsy. https://www.etsy.com/developers/documentation/getting_started/oauth When I issue the request-token call, I get signature invalid. I'm not sure what's going on. Will continue to test, but can somebody confirm that the library will work with etsy's OAuth implementation (advertised as OAuth 1.0), or is this new territory?

drone29a commented 10 years ago

I seem to remember the signature can either be included as a header or a URL parameter. It may be that Etsy is only checking one of those. At first glance, their docs aren't specific about which they expect.

drone29a commented 10 years ago

Hmm, I may have misremembered.

http://oauth.net/core/1.0/#signing_process

According to the OAuth spec the signature should always be included as a URL parameter.

drone29a commented 10 years ago

Are you passing a callback-uri in to your call to request-token? If you don't have one try using "oob" per http://tools.ietf.org/html/rfc5849#section-2.1

danielsz commented 10 years ago

Yes, I am passing a callback URI to the call to request-token.

Does clj-oauth pass the signature as a URL parameter or in the header? Is that something that we as users can configure with the clj-oauth API?

danielsz commented 10 years ago

OK, I found the problem.

With the Etsy OAuth machinery, the request token url can have url params indicating the scope of permissions we are requiring. The signature invalid error occurs when such params are given as is. For example: "https://openapi.etsy.com/v2/oauth/request_token?scope=email_r%20listings_r"

When using "https://openapi.etsy.com/v2/oauth/request_token" without params, no such error occurs.

So my next question is how to properly pass a request token with url encoded params in clj-oauth?

Thank you in advance.

drone29a commented 10 years ago

That makes sense. You should be able to pass those parameters to oauth.client/credentials as the final (and optional) parameter request-params. Then those parameters will be included in the signing. Note that the return value from credentials will not include those request-params items in its return value. You'll need to merge the scope of permissions params with the OAuth credentials when making the API request.

danielsz commented 10 years ago

oauth.client/credentials is the last step which will be used for signing API calls, right?

This use case is before the user has authorized the app. I need to pass those params in the very first request-token call. That's what makes the Etsy OAuth flow a little bit different than the other services. Does that make sense?

drone29a commented 10 years ago

You're correct about credentials being irrelevant for this issue.

We can add an optional parameter, request-params, like in credentials to request-token. That optional parameter will be merged with the unsigned-params local and passed to get-oauth-token.

Want to try that out? I'll gladly accept a pull request.

danielsz commented 10 years ago

I'm done, and I'm trying to run the tests. Can this line be removed to avoid errors? This is a private repository, no?

:repositories {"snapshots" {:url "s3p://lein-snapshots/snapshots"}} 

Do the tests actually pass? I get an error with lein test on the master branch (without my modifications).

lein test :only oauth.client-term-test/request-token

ERROR in (request-token) (PlainSocketImpl.java:-2)
Uncaught exception, not in assertion.
expected: nil
  actual: Exception in thread "main" java.lang.NullPointerException
danielsz commented 10 years ago

Actually, I'm not done.

Here is the refactored request-token function.

(defn request-token
  "Fetch request token for the consumer."
  ([consumer]
     (request-token consumer nil))
  ([consumer callback-uri & [request-params]]
     (let [unsigned-params (sig/oauth-params consumer
                                             (sig/rand-str 30)
                                             (sig/msecs->secs (System/currentTimeMillis)))]
       (get-oauth-token consumer (:request-uri consumer) (-> unsigned-params
                                                             (conj (when callback-uri [:oauth_callback callback-uri]))
                                                             (merge request-params))))))

I'm calling it like so:

(request-token consumer "https://localhost:3000" [:scope "email_r"])

And yet I'm still getting signature invalid errors. Now I'm just confused. What am I missing?

drone29a commented 10 years ago

Yes, that private repo came from an accepted pull request and should be removed.

The oath.client-term-test test is failing because the OAuth test host http://term.ie is down.

It's not immediately clear from Etsy's documentation how the proprietary scope parameter should be treated. I checked the docs for the PHP OAuth lib they use in their example and it appears that the the scope URL parameter should be included as a part of the request-uri parameter passed to oauth.client/make-consumer.

Try not passing the scope to request-token but do use a consumer with a request-uri that includes the desired scope.

danielsz commented 10 years ago

I think you are right about that. However, it's what got me to open a ticket in the first place. Putting the scope in the request-uri yields an invalid signature error. I'm at a loss. Maybe you can have a look here and see if you get a clue. http://stackoverflow.com/questions/8321034/getting-signature-invalid-calling-oauth-request-token-for-etsys-api-using-rests

drone29a commented 10 years ago

Can you use the signature validation tool mentioned in the SO link and share both the request it builds and the request clj-oauth builds? Sounds like it may be an encoding issue when the request-uri includes URL parameters.

danielsz commented 10 years ago

Does the request-urigets signed as is, or is it being deconstructed before it gets signed? If the URI is signed without the params, that could be the source of the problem.

danielsz commented 10 years ago

The SO link refers to a tool to be used in Visual Studio. I don't have that.

drone29a commented 10 years ago

Maybe try this for testing? https://developer.linkedin.com/oauth-test-console

Take a look at oauth.signature/base-string. If we understand how Etsy constructs the base string, then we can verify base-string produces the same output.

danielsz commented 10 years ago

Sure, I'll investigate. The scope should most likely be sent as url encoded params with the request url. Same pattern with the Ruby gem. https://github.com/kytrinyx/etsy/blob/master/lib/etsy/secure_client.rb I'll focus my effort on the signature generation.

danielsz commented 10 years ago

Hi,

There is another OAuth library for Clojure where the same problem arose. I explained the problem in an issue, and the author committed a patch that fixed it. It really was a problem of passing the scope as query params. Maybe you can have a look at his solution, and see how it translates in your codebase. Feel free to patch yourself, or guide me further. 3b9313f056132d2f5484aa03d75a03c7dc7979c9 d1c8b2b585b2812d73b19607093043bdef5b2749

drone29a commented 10 years ago

Thanks for the update. In that case, I'm a little confused about why adding param support to request-token didn't work for you.

danielsz commented 10 years ago

I think something is missing. The params need to be signed and passed along with a field called "query-params". Does this make sense?

drone29a commented 10 years ago

That is correct and that is what happens in the updated function you shared above. The request-token calls get-oauth-token and get-oauth-token calls the signing function, oauth.signature/sign which uses all the parameters, including the scope param, to create the signature.

That's why I'm confused that this library with the change we discussed doesn't work.

danielsz commented 10 years ago

I'm passing a Clojure map for the params. Will they properly be transformed and sorted when they are passed along? (Thinking aloud here.)

danielsz commented 10 years ago

What happens here with form-paramsneeds to be done for query-params too, ie. a separate field. Makes sense?

(defn build-request [oauth-params & [form-params]]
  (let [req (merge
             {:headers {"Authorization" (authorization-header
                                         oauth-params)}}
             (if form-params {:form-params form-params}))]
    req))

See here: https://github.com/r0man/oauth-clj/blob/master/src/oauth/v1.clj#L16

drone29a commented 10 years ago

Ah, that could be. In other words, it's not enough to sign them with the scope URL parameter and include the URL parameter on the URL string. It needs to be included in the Authorization header value string too. Etsy really needs to provide better docs for their "proprietary extension". I have a feeling oauth-clj just had a potential bug introduced into it too, since not all URL parameters should be included in the Authorization string.

danielsz commented 10 years ago

Sounds right to me.

I agree, with no documentation for "proprietary extensions", it is hair-rising.

The Devil really is in the detail. God have mercy with authors of OAuth libraries :-)

drone29a commented 10 years ago

Thanks for reporting this. It's now fixed and there's a new release on clojars with the update: [clj-oauth "1.5.0"]

The issue was that the scope parameter was being included in the Authorization string when it should've been sent as a regular form parameter.

danielsz commented 10 years ago

Works here. Well done. And thank you for sharing the explanation of the issue.

danielsz commented 8 years ago

Hi Matt,

Funny that I should write two years later with a similar question, but regarding Twitter this time. I'm interested to use the force_login option when doing the call for a request token for users who are signing in (and are already authorized), ie "https://api.twitter.com/oauth/authenticate" (and not ""https://api.twitter.com/oauth/authorize")

The problem is that user-approval-uri doesn't accept extra parameters. I'll submit a PR.