From e0bfddf2eb94b169cc146473a63908f88eb53ff9 Mon Sep 17 00:00:00 2001 From: Lucian Date: Wed, 14 Feb 2024 17:25:40 -0700 Subject: [PATCH] fix(platforms): fixed issue with updated coinbase API, refactored coinbase error handling (#2174) * fix(platforms): fixed issue with updated coinbase API, refactored coinbase error handling * fix: encoding access token request body as params instead of JSON --- platforms/src/Coinbase/Providers/coinbase.ts | 133 ++++++++---------- .../src/Coinbase/__tests__/coinbase.test.ts | 121 ++++++++++------ 2 files changed, 132 insertions(+), 122 deletions(-) diff --git a/platforms/src/Coinbase/Providers/coinbase.ts b/platforms/src/Coinbase/Providers/coinbase.ts index 11667646ac..0604f96e98 100644 --- a/platforms/src/Coinbase/Providers/coinbase.ts +++ b/platforms/src/Coinbase/Providers/coinbase.ts @@ -1,118 +1,96 @@ // ----- Types import type { RequestPayload, VerifiedPayload } from "@gitcoin/passport-types"; -import { type Provider, type ProviderOptions } from "../../types"; +import type { Provider } from "../../types"; import axios from "axios"; import { handleProviderAxiosError } from "../../utils/handleProviderAxiosError"; export type CoinbaseTokenResponse = { - access_token: string; + access_token?: string; }; export type CoinbaseUserData = { - id: string; + id?: string; }; export type CoinbaseFindMyUserResponse = { data?: { - data: CoinbaseUserData; + data?: CoinbaseUserData; }; - status: number; + status?: number; }; export class CoinbaseProvider implements Provider { // Give the provider a type so that we can select it with a payload type = "CoinbaseDualVerification"; - // Options can be set here and/or via the constructor - _options = {}; - - // construct the provider instance with supplied options - constructor(options: ProviderOptions = {}) { - this._options = { ...this._options, ...options }; - } - - // verify that the proof object contains valid === "true" async verify(payload: RequestPayload): Promise { - try { - const coinbaseAccountId = await verifyCoinbaseLogin(payload.proofs.code); + let errors; + let valid = false; - const verifiedCoinbaseAttestation = await verifyCoinbaseAttestation(payload.address); - - if (verifiedCoinbaseAttestation) { - return { - valid: true, - errors: [], - record: { id: coinbaseAccountId }, - }; + const coinbaseAccountId = await verifyCoinbaseLogin(payload.proofs.code); + if (coinbaseAccountId) { + if (await verifyCoinbaseAttestation(payload.address)) { + valid = true; } else { - throw `We could not find a Coinbase-verified onchain attestation for your account: ${coinbaseAccountId}.`; + errors = [`We could not find a Coinbase-verified onchain attestation for your account: ${coinbaseAccountId}.`]; } - } catch (e: unknown) { - return { - valid: false, - record: undefined, - errors: [String(e)], - }; + } else { + errors = ["Coinbase user id was not found."]; } + + return { + valid, + errors, + record: { id: coinbaseAccountId }, + }; } } -export const requestAccessToken = async (code: string): Promise => { +export const requestAccessToken = async (code: string): Promise => { const clientId = process.env.COINBASE_CLIENT_ID; const clientSecret = process.env.COINBASE_CLIENT_SECRET; const callback = process.env.COINBASE_CALLBACK; - // Exchange the code for an access token - const tokenRequest = await axios.post( - `https://api.coinbase.com/oauth/token?grant_type=authorization_code&client_id=${clientId}&client_secret=${clientSecret}&code=${code}&redirect_uri=${callback}`, - {}, - { - headers: { Accept: "application/json" }, - } - ); - - if (tokenRequest.status != 200) { - throw `Post for request returned status code ${tokenRequest.status} instead of the expected 200`; + let tokenRequest: { data?: CoinbaseTokenResponse }; + try { + // Used to format POST body as expected + const params = new URLSearchParams(); + params.append("grant_type", "authorization_code"); + params.append("client_id", clientId); + params.append("client_secret", clientSecret); + params.append("code", code); + params.append("redirect_uri", callback); + + // Exchange the code for an access token + tokenRequest = await axios.post("https://api.coinbase.com/oauth/token", params.toString(), { + headers: { + "Content-Type": "application/x-www-form-urlencoded", + Accept: "application/json", + }, + }); + } catch (e) { + console.log("error", e); + handleProviderAxiosError(e, "Coinbase access token", [clientSecret, code]); } + console.log("tokenRequest", tokenRequest); - const tokenResponse = tokenRequest.data as CoinbaseTokenResponse; - - return tokenResponse.access_token; + return tokenRequest?.data?.access_token; }; -export const verifyCoinbaseLogin = async (code: string): Promise => { +export const verifyCoinbaseLogin = async (code: string): Promise => { let userResponse: CoinbaseFindMyUserResponse; - try { - // retrieve user's auth bearer token to authenticate client - const accessToken = await requestAccessToken(code); + const accessToken = await requestAccessToken(code); + try { // Now that we have an access token fetch the user details userResponse = await axios.get("https://api.coinbase.com/v2/user", { headers: { Authorization: `Bearer ${accessToken}` }, }); - - if (userResponse.status != 200) { - throw `Get user request returned status code ${userResponse.status} instead of the expected 200`; - } } catch (e) { - const error = e as { - response: { - data: { - error_description: string; - }; - }; - request: string; - message: string; - }; - handleProviderAxiosError(error, "Coinbase access token request error", [code]); + handleProviderAxiosError(e, "Coinbase user info", [accessToken, code]); } - const userData = userResponse.data; - - if (!userData.data || !userData.data.id) { - throw "Coinbase user id was not found."; - } - return userData.data.id; + return userResponse?.data?.data?.id; }; const COINBASE_ATTESTER = "0x357458739F90461b99789350868CD7CF330Dd7EE"; @@ -155,16 +133,17 @@ export const verifyCoinbaseAttestation = async (address: string): Promise attestation.revoked === false && attestation.revocationTime === 0 && diff --git a/platforms/src/Coinbase/__tests__/coinbase.test.ts b/platforms/src/Coinbase/__tests__/coinbase.test.ts index 627655d862..d22bb0afb3 100644 --- a/platforms/src/Coinbase/__tests__/coinbase.test.ts +++ b/platforms/src/Coinbase/__tests__/coinbase.test.ts @@ -5,7 +5,7 @@ import * as coinbaseProviderModule from "../Providers/coinbase"; import { RequestPayload } from "@gitcoin/passport-types"; // ----- Libs -import axios from "axios"; +import axios, { AxiosError } from "axios"; jest.mock("axios"); @@ -133,31 +133,33 @@ describe("verifyCoinbaseAttestation", () => { describe("Attempt verification", function () { it("should throw Provider External Verification error when unable to retrieve auth token", async () => { - const e = "Post for request returned status code 500 instead of the expected 200"; - mockedAxios.post.mockImplementation(async () => { - return { - status: 500, - }; - }); + const mockAxiosError = new Error("Network error") as AxiosError; + mockedAxios.isAxiosError.mockReturnValueOnce(true); + mockAxiosError.response = { + status: 500, + data: {}, + headers: {}, + statusText: "Internal Server Error", + config: {}, + }; + + mockedAxios.post.mockRejectedValueOnce(mockAxiosError); + const coinbase = new coinbaseProviderModule.CoinbaseProvider(); - expect( - await coinbase.verify({ + await expect( + coinbase.verify({ proofs: { code, }, } as unknown as RequestPayload) - ).toMatchObject({ - valid: false, - record: undefined, - errors: [e], - }); + ).rejects.toThrow( + "Error making Coinbase access token request, received error response with code 500: {}, headers: {}" + ); expect(mockedAxios.post).toBeCalledTimes(1); expect(mockedAxios.post).toBeCalledWith( - `https://api.coinbase.com/oauth/token?grant_type=authorization_code&client_id=${clientId}&client_secret=${clientSecret}&code=${code}&redirect_uri=${callback}`, - {}, - { - headers: { Accept: "application/json" }, - } + "https://api.coinbase.com/oauth/token", + `grant_type=authorization_code&client_id=${clientId}&client_secret=${clientSecret}&code=${code}&redirect_uri=${callback}`, + { headers: { "Content-Type": "application/x-www-form-urlencoded", Accept: "application/json" } } ); }); @@ -183,17 +185,14 @@ describe("Attempt verification", function () { ).toMatchObject({ valid: false, errors: ["Coinbase user id was not found."], - record: undefined, }); expect(mockedAxios.post).toBeCalledTimes(1); // Check the request to get the token expect(mockedAxios.post).toBeCalledWith( - `https://api.coinbase.com/oauth/token?grant_type=authorization_code&client_id=${clientId}&client_secret=${clientSecret}&code=${code}&redirect_uri=${callback}`, - {}, - { - headers: { Accept: "application/json" }, - } + "https://api.coinbase.com/oauth/token", + `grant_type=authorization_code&client_id=${clientId}&client_secret=${clientSecret}&code=${code}&redirect_uri=${callback}`, + { headers: { "Content-Type": "application/x-www-form-urlencoded", Accept: "application/json" } } ); expect(mockedAxios.get).toBeCalledTimes(1); // Check the request to get the user @@ -203,9 +202,50 @@ describe("Attempt verification", function () { }); it("should return invalid payload when a bad status code is returned by coinbase user api", async () => { + const mockAxiosError = new Error("Network error") as AxiosError; + mockedAxios.isAxiosError.mockReturnValueOnce(true); + mockAxiosError.response = { + status: 500, + data: {}, + headers: {}, + statusText: "Internal Server Error", + config: {}, + }; + + mockedAxios.get.mockRejectedValueOnce(mockAxiosError); + + const coinbase = new coinbaseProviderModule.CoinbaseProvider(); + await expect( + coinbase.verify({ + proofs: { + code, + }, + } as unknown as RequestPayload) + ).rejects.toThrow( + "Error making Coinbase user info request, received error response with code 500: {}, headers: {}" + ); + + expect(mockedAxios.post).toBeCalledTimes(1); + + // Check the request to get the token + expect(mockedAxios.post).toBeCalledWith( + "https://api.coinbase.com/oauth/token", + `grant_type=authorization_code&client_id=${clientId}&client_secret=${clientSecret}&code=${code}&redirect_uri=${callback}`, + { headers: { "Content-Type": "application/x-www-form-urlencoded", Accept: "application/json" } } + ); + expect(mockedAxios.get).toBeCalledTimes(1); + // Check the request to get the user + expect(mockedAxios.get).toBeCalledWith("https://api.coinbase.com/v2/user", { + headers: { Authorization: "Bearer cnbstkn294745627362562" }, + }); + }); + + it("should fail if unable to find ID", async () => { mockedAxios.get.mockImplementation(async (url, config) => { return { - status: 500, + data: { + id: undefined, + }, }; }); @@ -220,11 +260,9 @@ describe("Attempt verification", function () { // Check the request to get the token expect(mockedAxios.post).toBeCalledWith( - `https://api.coinbase.com/oauth/token?grant_type=authorization_code&client_id=${clientId}&client_secret=${clientSecret}&code=${code}&redirect_uri=${callback}`, - {}, - { - headers: { Accept: "application/json" }, - } + "https://api.coinbase.com/oauth/token", + `grant_type=authorization_code&client_id=${clientId}&client_secret=${clientSecret}&code=${code}&redirect_uri=${callback}`, + { headers: { "Content-Type": "application/x-www-form-urlencoded", Accept: "application/json" } } ); expect(mockedAxios.get).toBeCalledTimes(1); // Check the request to get the user @@ -234,8 +272,7 @@ describe("Attempt verification", function () { expect(coinbasePayload).toMatchObject({ valid: false, - errors: ["Get user request returned status code 500 instead of the expected 200"], - record: undefined, + errors: ["Coinbase user id was not found."], }); }); @@ -252,11 +289,9 @@ describe("Attempt verification", function () { expect(mockedAxios.post).toBeCalledTimes(1); // Check the request to get the token expect(mockedAxios.post).toBeCalledWith( - `https://api.coinbase.com/oauth/token?grant_type=authorization_code&client_id=${clientId}&client_secret=${clientSecret}&code=${code}&redirect_uri=${callback}`, - {}, - { - headers: { Accept: "application/json" }, - } + "https://api.coinbase.com/oauth/token", + `grant_type=authorization_code&client_id=${clientId}&client_secret=${clientSecret}&code=${code}&redirect_uri=${callback}`, + { headers: { "Content-Type": "application/x-www-form-urlencoded", Accept: "application/json" } } ); expect(mockedAxios.get).toBeCalledTimes(1); @@ -265,12 +300,8 @@ describe("Attempt verification", function () { headers: { Authorization: "Bearer cnbstkn294745627362562" }, }); - expect(coinbasePayload).toEqual({ - valid: true, - record: { - id: validCoinbaseUserResponse.data.data.id, - }, - errors: [], - }); + expect(coinbasePayload).toEqual( + expect.objectContaining({ valid: true, record: { id: validCoinbaseUserResponse.data.data.id } }) + ); }); });