bchavez / Coinbase

:moneybag: A .NET/C# implementation of the Coinbase API.
https://developers.coinbase.com/api/v2
MIT License
170 stars 92 forks source link

Adding Support For OAuth #30

Closed granthoff1107 closed 5 years ago

granthoff1107 commented 5 years ago

I've added support for OAuth. Adding Simple Error Handling. Simplified Constructors Everything should be backwards comaptible

This method seemed wierd to me seemed like this should be post only why would someone add a json body to a GET Request?:

public virtual CoinbaseResponse SendRequest(string endpoint, object body, Method httpMethod = Method.POST)
{
    var client = CreateClient();

    var req = CreateRequest(endpoint, httpMethod)
            .AddJsonBody(body);
    //...
bchavez commented 5 years ago

Hi @granthoff1107,

Could you please try out v5.0.0-beta-1. I've added OAuth support.

https://www.nuget.org/packages/Coinbase/5.0.0-beta-1

Check the README page for details.. Please let me know if it works.

Thanks, Brian

:black_circle: :dizzy: "Black magic... bla-a-a-ack... black magic..."

granthoff1107 commented 5 years ago

I’m reviewing this from my phone so it’s hard to see everything. But it looks like you don’t provide raw access to the request from the api endpoints.

That’s fine in most cases but when using the Endpoints from OAuth occasionally you’ll need to supply a CB-2FA-TOKEN header to add 2 factor and replay a request such as in the Transfer api

On Tue, Nov 6, 2018 at 1:40 AM Brian Chavez notifications@github.com wrote:

Hi @granthoff1107 https://github.com/granthoff1107,

Could you please try out v5.0.0-beta-1. I've added OAuth support.

https://www.nuget.org/packages/Coinbase/5.0.0-beta-1

Check the README https://github.com/bchavez/Coinbase page for details.. Please let me know if it works.

Thanks, Brian

⚫️ 💫 "Black magic... bla-a-a-ack... black magic..." https://www.youtube.com/watch?v=3SnlsTtUZK0

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bchavez/Coinbase/pull/30#issuecomment-436146997, or mute the thread https://github.com/notifications/unsubscribe-auth/AMsMnX-_rWyxkVArMfcWvyqVBon-fp27ks5usS7OgaJpZM4YNml2 .

granthoff1107 commented 5 years ago

Thanks, this saves me a lot of work. I'm in the process of refactoring my local examples. I only found two Issues with the current beta.

  1. OAuth Requires 2FA, I think its only for the Transactions api.
  2. I don't see a way to get the next page in the List Api's

When I'm done refactoring my local examples I can make a pull request.
If you get a chance let me know your preference on how I should write it. Should the Transactions Api Take in 2Fa Token. Or would it be better to allow an options param which has an action to to modify the FlurlRequest,

the later would be more versatile in case in the the end users need to add other headers etc.

Once, I get my examples updated I'll start testing more but thanks for putting out the beta when you did!

bchavez commented 5 years ago

I just now pushed some code that promotes the CoinbaseApi as a FlurlClient.

So, usage should remain the same, but you can mutate the headers before sending your request. IE:

//using OAuth Token
var client = new CoinbaseApi(new OAuthConfig{ OAuthToken = "..." });
var create = new CreateTransaction
   {
      Amount = 1.0m,
      Currency = "BTC"
   };
var response = await client
    .WithHeader(TwoFactorToken, "ffff")
    .Transactions.SendMoneyAsync("accountId", create);

Hold tight for a few more minutes and I'll send out another beta release with the updated changes. :sunglasses:

:car: :blue_car: "Let the good times roll..."

granthoff1107 commented 5 years ago

Thanks, this should work perfect. I’ll test this out in a bit and let you know

On Tue, Nov 6, 2018 at 9:04 PM Brian Chavez notifications@github.com wrote:

I just now pushed some code that promotes the CoinbaseApi as a FlurlClient .

So, usage should remain the same, but you can mutate the headers before sending your request. IE:

//using OAuth Token var client = new CoinbaseApi(new OAuthConfig{ OAuthToken = "..." }); var create = new CreateTransaction { Amount = 1.0m, Currency = "BTC" }; var response = await client .WithHeader(TwoFactorToken, "ffff") .Transactions.SendMoneyAsync("accountId", create);

Hold tight for a few more minutes and I'll send out another beta release with the updated changes. 😎

🚗 🚙 "Let the good times roll..." https://www.youtube.com/watch?v=7BDBzgHXf64

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bchavez/Coinbase/pull/30#issuecomment-436477428, or mute the thread https://github.com/notifications/unsubscribe-auth/AMsMnT0WM1KRFBl3EE_SK43UXnIZ-Gbdks5usj-5gaJpZM4YNml2 .

bchavez commented 5 years ago

Hi Grant,

Just published v5.0.0-beta-2 here: https://www.nuget.org/packages/Coinbase/5.0.0-beta-2

Let me know how it goes. If all goes well, I'll promote beta-2 to as the latest release.

Docs have been updated on the README page

Thanks, Brian

:chocolate_bar: :cookie: :lollipop: Ronald Jenkees - Stay Crunchy