coinbase / coinbase-advanced-py

The Advanced API Python SDK is a Python package that makes it easy to interact with the Coinbase Advanced API. The SDK handles authentication, HTTP connections, and provides helpful methods for interacting with the API.
https://docs.cdp.coinbase.com/advanced-trade/docs/welcome/
Apache License 2.0
101 stars 27 forks source link

Modify RESTClient post() to add CB-2FA-Token for SIWC transaction endpoints #43

Closed brianddk closed 4 months ago

brianddk commented 4 months ago

With the April 2nd expansion of the CDP api keys to support SIWC urls, there is extra support needed for the /v2/accounts/:account_id/transactions endpoints. This appears to be a holdover from the OAuth2 support, and may actually be an error in the API. If not, support is needed for the transactions endpoints. Specifically, posts this endpoint requires the CB-2FA-Token header. This PR adds a cb_2fa_token argument to the RESTClient constructor to facilitate that.

Example

from requests.exceptions import HTTPError
from coinbase.rest import RESTClient

params = {
  "type": "send",
  "currency": "BTC",
  "to": "bc1qwc2203uym96u0nmq04pcgqfs9ldqz9l3mz8fpj",
  "amount": "0.00045678",
}

api_key = "organizations/{org_id}/apiKeys/{key_id}"
api_secret = "-----BEGIN EC PRIVATE KEY-----\nYOUR PRIVATE KEY\n-----END EC PRIVATE KEY-----\n"
client = RESTClient(
    api_key = api_key,
    api_secret = api_secret,
)
try:
    client.post("/v2/accounts/456789ab-cdef-0123-4567-89abcdef0123/transactions", params)
except HTTPError as e:
    if "two_factor_required" in e.args[0]:
        cb_2fa_token=input("CB-2FA-Token: ").strip()
        client = RESTClient(
            api_key = api_key,
            api_secret = api_secret,
            cb_2fa_token = cb_2fa_token,
        )
        client.post("/v2/accounts/456789ab-cdef-0123-4567-89abcdef0123/transactions", params)
    else:
        raise e

Changes

urischwartz-cb commented 4 months ago

Thanks for reporting @brianddk ! I checked with our team and they said a fix was pushed for this. Could you check if you are able to hit this endpoint without the 2FA header now?

brianddk commented 4 months ago

I checked with our team and they said a fix was pushed for this. Could you check if you are able to hit this endpoint without the 2FA header now?

still requesting the token:

{
  "errors": [
    {
      "id": "two_factor_required",
      "message": "Two-step verification code required to complete this request. Re-play the request with CB-2FA-Token header."
    }
  ]
}
urischwartz-cb commented 4 months ago

@brianddk Looking at the SIWC docs, it seems that it is expected for 2FA to be required for this endpoint. Closing out this PR since we do not plan on supporting SIWC in this SDK for now.

Have you been able to access this endpoint before without 2FA token, or has this always been the case?

urischwartz-cb commented 4 months ago

@brianddk thank you again for flagging. This was indeed a bug in our service. A fix has now been pushed and you should not need the 2FA token header for this endpoint anymore.

brianddk commented 4 months ago

A fix has now been pushed and you should not need the 2FA token header for this endpoint anymore.

Confirmed... Thanks.

You might want to let your contact in the API-dev team know that the update bugged the /v2/user/auth endpoint. It now shows "method": null in the response JSON when CDP keys are used on it.

urischwartz-cb commented 4 months ago

Thanks for that. That is now fixed as well.

brianddk commented 4 months ago

Thanks for that. That is now fixed as well.

Confirmed. Thx again.