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

Trying to expand your API and getting Signature Problems #21

Closed mitchcapper closed 8 years ago

mitchcapper commented 8 years ago

Thanks for your work:) I figured I would work to add some more methods to it, starting with order listing: https://github.com/mitchcapper/Coinbase/tree/ApiOrderListing / https://github.com/mitchcapper/Coinbase/commit/14514ff831166a46f611a7bd6fab2015baf738d4

I am getting "authentication_error" with "invalid signature" on the request. Fetching time, or using your built in requests work but for some reason the order listing does not. The API key has permission (gives a different error otherwise) and the keys are correct for the other requests to work. Unfortunately I am not sure what I am missing. As it seems just doing the get request to orders using the system causes the error even looking at other apis I am not sure.

bchavez commented 8 years ago

Hi Mitch,

Thank you for reporting the issue. I'm not sure, but you might want to check:

If you still have trouble, feel free to post the Fiddler output of your ListOrder() HTTP Request/Response here.

I do want to urge some level of caution before creating new Object Models like Order, MoneyHash, TransactionHash. A major problem in the past has been keeping Object Model's current and up-to-date. The Coinbase APIs and Object Models change semi-frequently. The API is too large for me to keep Object Models up-to-date and maintain by myself without some kind of automation. Additionally, the API documentation doesn't fully describe all object model fields their status flags. Previously, I've had to deal with GitHub Issues describing missing fields in object models.

So, in the v2 re-write I did, I completely removed the type-safe object models and replaced them with anonymous/dynamic types. Developers using this .NET API library can choose to implement their own type-safe models for only what they need or use anonymous / dynamic types in body of their Coinbase requests.

In short, and it kills me inside to reject PRs, but I can't accept any pull-requests that contain type-safe object models (like Order, OrderType, TransactionHash) because it would mean I would need to maintain object models that can get easily out of date.

Instead, I would be more than happy to accept PRs for API method calls like ListOrders() that accept dynamic / anonymous type parameters and return basic generalized response types.

I hope this sounds reasonable.

Thanks, Brian

mitchcapper commented 8 years ago

No problem. I think there is limited value to adding functions like listorders that take dynamic types (as its very easy with your class to just make the call directly, as my listorders function already does. It is completely understandable of not wanting to maintain the additional classes however so I will not submit such PR.

Sadly I did try string.empty among other things here is the request and response:

GET https://api.coinbase.com/v2/orders HTTP/1.1
CB-ACCESS-KEY: anwr9nawgne
CB-ACCESS-SIGN: 1239awefnawef
CB-ACCESS-TIMESTAMP: 1452652680
CB-VERSION: 2015-11-17
Accept: application/json, application/xml, text/json, text/x-json, text/javascript, text/xml, application/json
User-Agent: RestSharp/105.2.3.0
Host: api.coinbase.com
Accept-Encoding: gzip, deflate

and response

HTTP/1.1 401 Unauthorized
Server: cloudflare-nginx
Date: Wed, 13 Jan 2016 02:38:00 GMT
Content-Type: application/json; charset=utf-8
Content-Length: 72
Connection: keep-alive
Set-Cookie: __cfduid=d2280be6amwef3ce390a56c4e571452652680; expires=Thu, 12-Jan-17 02:38:00 GMT; path=/; domain=.coinbase.com; HttpOnly
Cache-Control: private, no-cache, no-store, must-revalidate
content-disposition: attachment; filename=response.json
Expires: Sat, 01 Jan 2000 00:00:00 GMT
Pragma: no-cache
Public-Key-Pins: max-age=5184000; pin-sha256="r/m12n33q23fn3/ko/cwxzOMo1bk4TyHIlByibiA5E="; pin-sha256="r/m12n33q23fn3/ko/cwxzOMo1bk4TyHIlByibiA5E="; pin-sha256="r/m12n33q23fn3/ko/cwxzOMo1bk4TyHIlByibiA5E="
Set-Cookie: _coinbase=cookieinfo123; path=/; secure; HttpOnly
Strict-Transport-Security: max-age=15552000; includeSubDomains; preload
Vary: Origin,Accept-Encoding
X-Content-Type-Options: nosniff
X-Download-Options: noopen
X-Frame-Options: SAMEORIGIN
X-Permitted-Cross-Domain-Policies: none
X-Powered-By: Proof-of-Work
X-Request-Id: 8454ae15-5903-4cee-90ab-e942f88cb9e0
X-XSS-Protection: 1; mode=block
CF-RAY: 263db8f57d742a91-SEA

{"errors":[{"id":"authentication_error","message":"invalid signature"}]}
bchavez commented 8 years ago

Hi Mitch,

This looks like it was a bug in the signature generation code.

Basically "null" as a string was getting included in the signature. Added a check to avoid this. Additionally, the CoinbaseResponse.Data now exposes a JToken which is more appropriate when using it with an endpoint like orders.

You can pull the latest changes, and verify they're working for you. If not, please feel free to re-open the issue and I'll take a second look.

Thanks again for reporting these bugs.

bchavez commented 8 years ago

Also, pushed v2.0.4 into NuGet

mitchcapper commented 8 years ago

yep that fixed it:)