Skip to content
This repository was archived by the owner on Nov 23, 2024. It is now read-only.

Conversation

@disbelief
Copy link

Originally was PR #47 but I needed to get it off of my master. See that PR for comments.

@stevenbarragan
Copy link

👍

Can we get this merged?

@disbelief
Copy link
Author

Sorry for the delay. There are a couple bugs still need to be fixed, and the test suite needs to be updated. Will try to get to it asap!

@stevenbarragan
Copy link

Nice.

Please let me know if you need any help?

@joallard
Copy link

+1 Any news on this?

@disbelief
Copy link
Author

On "my todo very soon" list. Sorry again.

@joallard
Copy link

Maybe you can point to what's missing/tests and we can pitch in some help?

@disbelief
Copy link
Author

I'm going to try and get to it tomorrow. If not, I'll point you in the right direction. There were a couple bugs I've uncovered in this PR that need fixing.

Data should be passed as parameters in the body of the request instead
of converting it into query string params. Query string will work for
smaller amounts of data, but errors arise when the query string becomes
too long. For example this is often the case when requesting deltas
using a long cursor string.
Some notes:

- I had to modify the logic in one Dropbox::API::Client#delta test
  because in my tests the last item returned by the delta API was
  actually the `test_dir` itself, not the file within it. I made the
  test flexible enough to handle either case: "file last" or "dir
  last".

- Added a `type` key to connection.yml to indicate that the token
  provided is either "oauth2" or "oauth1". When ommitted, "oauth1"
  is assumed.

- Alternatively ENV['AUTH']=oauth2 can be set when running the tests.
  This will override whatever is specified in connection.yml

- I think ideally the spec would run all of the tests twice: once
  with an OAuth1 token and once with OAuth2, but that's a bit more than
  I have time for.
@disbelief
Copy link
Author

Okay very sorry for the crazy long delay. Fixed the remaining bugs and all tests are green for me. @marcinbunsch I think it's ready for a merge.

Something you might want to do is run the tests with an OAuth1 token in connection.yml. I was only able run them with an OAuth2 token.

@marcinbunsch
Copy link

Thanks! I'll set aside some time to go through this - I'll keep you posted!

@joallard
Copy link

One thing: consumer.auth_code.get_token needs to be called on a different endpoint (:main) than the authorize action. Ideally all that logic would be enclosed in the library API, but to get it at least working, the token exchange should read something like:

    consumer = Dropbox::API::OAuth2.consumer :main
    consumer.auth_code.get_token(code, redirect_uri: dropbox_auth_finish_url).token

@disbelief
Copy link
Author

@joallard strange, it works for me without changing the consumer's endpoint. Are you hitting an exception when you run rake dropbox:authorize_oauth2 ?

@joallard
Copy link

Don't know how to build task 'dropbox:authorize_oauth2', which has me scratching my head. Weird cause I'm using your fork on my development server, I reloaded spring and all.

What I saw is that my token exchange returned an OAuth2::Error à la 400 non-descript error, when I noticed it was calling the wrong endpoint.

@disbelief
Copy link
Author

I ran that task from the dropbox-api gem's root dir, not from an app that's using it. Guessing you tried running it from a Rails app's root dir? Not sure if that should be possible or not.

@joallard
Copy link

Correct. Ha. I hadn't cloned the gem.

@joallard
Copy link

joallard commented Jul 2, 2015

The task does work.

@joallard
Copy link

joallard commented Jul 6, 2015

@disbelief Does the token exchange by default happen on the main endpoint?

@disbelief
Copy link
Author

I'm relying on the oauth2 gem to figure that out. When initializing the OAuth2 client, the endpoints and their purposes are indicated:

::OAuth2::Client.new(Dropbox::API::Config.app_key, Dropbox::API::Config.app_secret,
  :site               => Dropbox::API::Config.endpoints[endpoint],
  :authorize_url      => "#{Dropbox::API::Config.prefix}/oauth2/authorize",
  :token_url          => "#{Dropbox::API::Config.prefix}/oauth2/token")

(https://github.com/unbox/dropbox-api/blob/oauth2-support/lib/dropbox-api/util/oauth2.rb#L12)

I've been using this in production for close to a year without needing to do anything special.

@joallard
Copy link

joallard commented Jul 7, 2015

Actually no, the endpoint is a parameter to the method

@disbelief
Copy link
Author

Ah yes you're right! Pushed an update.

@joallard
Copy link

joallard commented Jul 7, 2015

Excellent, and great work by the way!

@disbelief
Copy link
Author

Thanks! Let me know if there's anything else required to get this PR merged.

@joallard joallard mentioned this pull request Jul 8, 2015
@ivanovaleksey
Copy link

Hi guys!
What about this nice feature? Can it be merged?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants