From 4936272a6e9bf8f9e5e90e1d8ecab57073273756 Mon Sep 17 00:00:00 2001 From: Max Kurapov Date: Mon, 6 May 2024 13:42:01 +0200 Subject: [PATCH 01/10] feat(backend): SPSPRouteError --- .../src/open_payments/route-errors.test.ts | 20 ++++++ .../backend/src/open_payments/route-errors.ts | 17 ++++- .../ilp/spsp/middleware.test.ts | 10 +-- .../src/payment-method/ilp/spsp/middleware.ts | 36 ++++++++-- .../payment-method/ilp/spsp/routes.test.ts | 68 +++++++++++++++++-- .../src/payment-method/ilp/spsp/routes.ts | 24 +++++-- 6 files changed, 152 insertions(+), 23 deletions(-) diff --git a/packages/backend/src/open_payments/route-errors.test.ts b/packages/backend/src/open_payments/route-errors.test.ts index c837c2a0e5..e9d9c9c28a 100644 --- a/packages/backend/src/open_payments/route-errors.test.ts +++ b/packages/backend/src/open_payments/route-errors.test.ts @@ -8,6 +8,7 @@ import { IocContract } from '@adonisjs/fold' import { initIocContainer } from '..' import { Config } from '../config/app' import { OpenAPIValidatorMiddlewareError } from '@interledger/openapi' +import { SPSPRouteError } from '../payment-method/ilp/spsp/middleware' describe('openPaymentServerErrorMiddleware', (): void => { let deps: IocContract @@ -68,6 +69,25 @@ describe('openPaymentServerErrorMiddleware', (): void => { expect(next).toHaveBeenCalledTimes(1) }) + test('handles SPSPRouteError error', async (): Promise => { + const error = new SPSPRouteError(400, 'SPSP error') + const next = jest.fn().mockImplementationOnce(() => { + throw error + }) + + const ctxThrowSpy = jest.spyOn(ctx, 'throw') + + await expect( + openPaymentsServerErrorMiddleware(ctx, next) + ).rejects.toMatchObject({ + status: error.status, + message: error.message + }) + + expect(ctxThrowSpy).toHaveBeenCalledWith(error.status, error.message) + expect(next).toHaveBeenCalledTimes(1) + }) + test('handles unspecified error', async (): Promise => { const error = new Error('Some unspecified error') const next = jest.fn().mockImplementationOnce(() => { diff --git a/packages/backend/src/open_payments/route-errors.ts b/packages/backend/src/open_payments/route-errors.ts index 702c9cf184..afd1d90a31 100644 --- a/packages/backend/src/open_payments/route-errors.ts +++ b/packages/backend/src/open_payments/route-errors.ts @@ -1,5 +1,6 @@ import { AppContext } from '../app' import { OpenAPIValidatorMiddlewareError } from '@interledger/openapi' +import { SPSPRouteError } from '../payment-method/ilp/spsp/middleware' export class OpenPaymentsServerRouteError extends Error { public status: number @@ -43,7 +44,9 @@ export async function openPaymentsServerErrorMiddleware( 'Received error when handling Open Payments request' ) ctx.throw(err.status, err.message) - } else if (err instanceof OpenAPIValidatorMiddlewareError) { + } + + if (err instanceof OpenAPIValidatorMiddlewareError) { const finalStatus = err.status || 400 logger.info( @@ -57,6 +60,18 @@ export async function openPaymentsServerErrorMiddleware( ctx.throw(finalStatus, err.message) } + if (err instanceof SPSPRouteError) { + logger.info( + { + ...baseLog, + message: err.message, + details: err.details + }, + 'Received error when handling SPSP request' + ) + ctx.throw(err.status, err.message) + } + logger.error( { ...baseLog, err }, 'Received unhandled error in Open Payments request' diff --git a/packages/backend/src/payment-method/ilp/spsp/middleware.test.ts b/packages/backend/src/payment-method/ilp/spsp/middleware.test.ts index 0bf9fd7ed1..1a9467f578 100644 --- a/packages/backend/src/payment-method/ilp/spsp/middleware.test.ts +++ b/packages/backend/src/payment-method/ilp/spsp/middleware.test.ts @@ -8,6 +8,7 @@ import { createTestApp, TestContainer } from '../../../tests/app' import { createAsset } from '../../../tests/asset' import { createWalletAddress } from '../../../tests/walletAddress' import { truncateTables } from '../../../tests/tableManager' +import { WalletAddress } from '../../../open_payments/wallet_address/model' describe('SPSP Middleware', (): void => { let deps: IocContract @@ -33,10 +34,11 @@ describe('SPSP Middleware', (): void => { describe('Wallet Address', (): void => { let ctx: SPSPWalletAddressContext + let walletAddress: WalletAddress beforeEach(async (): Promise => { const asset = await createAsset(deps) - const walletAddress = await createWalletAddress(deps, { + walletAddress = await createWalletAddress(deps, { assetId: asset.id }) ctx = setup({ @@ -68,10 +70,10 @@ describe('SPSP Middleware', (): void => { } else { expect(spspSpy).toHaveBeenCalledTimes(1) expect(next).not.toHaveBeenCalled() - expect(ctx.paymentTag).toEqual(ctx.walletAddress.id) + expect(ctx.paymentTag).toEqual(walletAddress.id) expect(ctx.asset).toEqual({ - code: ctx.walletAddress.asset.code, - scale: ctx.walletAddress.asset.scale + code: walletAddress.asset.code, + scale: walletAddress.asset.scale }) } } diff --git a/packages/backend/src/payment-method/ilp/spsp/middleware.ts b/packages/backend/src/payment-method/ilp/spsp/middleware.ts index 362f078e62..747ae7216a 100644 --- a/packages/backend/src/payment-method/ilp/spsp/middleware.ts +++ b/packages/backend/src/payment-method/ilp/spsp/middleware.ts @@ -1,9 +1,22 @@ import { WalletAddressContext, SPSPContext } from '../../../app' -export type SPSPWalletAddressContext = WalletAddressContext & - SPSPContext & { - incomingPayment?: never +export type SPSPWalletAddressContext = WalletAddressContext & SPSPContext + +export class SPSPRouteError extends Error { + public status: number + public details?: Record + + constructor( + status: number, + message: string, + details?: Record + ) { + super(message) + this.name = 'SPSPRouteError' + this.status = status + this.details = details } +} export function createSpspMiddleware(spspEnabled: boolean) { if (spspEnabled) { @@ -24,11 +37,20 @@ const spspMiddleware = async ( ): Promise => { // Fall back to legacy protocols if client doesn't support Open Payments. if (ctx.accepts('application/spsp4+json')) { - const receiver = ctx.walletAddress ?? ctx.incomingPayment - ctx.paymentTag = receiver.id + const walletAddressService = await ctx.container.use('walletAddressService') + + const walletAddress = await walletAddressService.getByUrl( + ctx.walletAddressUrl + ) + + if (!walletAddress) { + throw new SPSPRouteError(404, 'Could not get wallet address') + } + + ctx.paymentTag = walletAddress.id ctx.asset = { - code: receiver.asset.code, - scale: receiver.asset.scale + code: walletAddress.asset.code, + scale: walletAddress.asset.scale } const spspRoutes = await ctx.container.use('spspRoutes') await spspRoutes.get(ctx) diff --git a/packages/backend/src/payment-method/ilp/spsp/routes.test.ts b/packages/backend/src/payment-method/ilp/spsp/routes.test.ts index 95385c7078..7c6f1699ff 100644 --- a/packages/backend/src/payment-method/ilp/spsp/routes.test.ts +++ b/packages/backend/src/payment-method/ilp/spsp/routes.test.ts @@ -1,5 +1,6 @@ import * as crypto from 'crypto' import { v4 as uuid } from 'uuid' +import assert from 'assert' import { AppServices, SPSPContext } from '../../../app' import { SPSPRoutes } from './routes' @@ -13,6 +14,7 @@ import { StreamServer } from '@interledger/stream-receiver' import { randomAsset } from '../../../tests/asset' import { createContext } from '../../../tests/context' import { truncateTables } from '../../../tests/tableManager' +import { SPSPRouteError } from './middleware' type SPSPHeader = { Accept: string @@ -76,17 +78,46 @@ describe('SPSP Routes', (): void => { const ctx = createContext({ headers: { Accept: 'application/json' } }) - await expect(spspRoutes.get(ctx)).rejects.toHaveProperty('status', 406) + + expect.assertions(2) + try { + await spspRoutes.get(ctx) + } catch (err) { + assert.ok(err instanceof SPSPRouteError) + expect(err.status).toBe(406) + expect(err.message).toBe( + 'Request does not support application/spsp4+json' + ) + } }) test('nonce, no secret; returns 400', async () => { const ctx = setup({ nonce }) - await expect(spspRoutes.get(ctx)).rejects.toHaveProperty('status', 400) + + expect.assertions(2) + try { + await spspRoutes.get(ctx) + } catch (err) { + assert.ok(err instanceof SPSPRouteError) + expect(err.status).toBe(400) + expect(err.message).toBe( + 'Failed to generate credentials: receipt nonce and secret must accompany each other' + ) + } }) test('secret; no nonce; returns 400', async () => { const ctx = setup({ secret }) - await expect(spspRoutes.get(ctx)).rejects.toHaveProperty('status', 400) + expect.assertions(2) + try { + await spspRoutes.get(ctx) + } catch (err) { + assert.ok(err instanceof SPSPRouteError) + expect(err.status).toBe(400) + expect(err.message).toBe( + 'Failed to generate credentials: receipt nonce and secret must accompany each other' + ) + } }) test('malformed nonce; returns 400', async () => { @@ -94,7 +125,17 @@ describe('SPSP Routes', (): void => { nonce: Buffer.alloc(15).toString('base64'), secret }) - await expect(spspRoutes.get(ctx)).rejects.toHaveProperty('status', 400) + + expect.assertions(2) + try { + await spspRoutes.get(ctx) + } catch (err) { + assert.ok(err instanceof SPSPRouteError) + expect(err.status).toBe(400) + expect(err.message).toBe( + 'Failed to generate credentials: receipt nonce must be 16 bytes' + ) + } }) const receiptSetup = { @@ -139,6 +180,25 @@ describe('SPSP Routes', (): void => { } ) + test('handle error when generating credentials', async () => { + const ctx = setup() + + jest + .spyOn(streamServer, 'generateCredentials') + .mockImplementationOnce(() => { + throw new Error('Could not generate credentials') + }) + + expect.assertions(2) + try { + await spspRoutes.get(ctx) + } catch (err) { + assert.ok(err instanceof SPSPRouteError) + expect(err.status).toBe(400) + expect(err.message).toBe('Could not generate credentials') + } + }) + /** * Utility functions */ diff --git a/packages/backend/src/payment-method/ilp/spsp/routes.ts b/packages/backend/src/payment-method/ilp/spsp/routes.ts index 5f77ff17d7..0f443703aa 100644 --- a/packages/backend/src/payment-method/ilp/spsp/routes.ts +++ b/packages/backend/src/payment-method/ilp/spsp/routes.ts @@ -2,6 +2,7 @@ import { BaseService } from '../../../shared/baseService' import { SPSPContext } from '../../../app' import base64url from 'base64url' import { StreamServer } from '@interledger/stream-receiver' +import { SPSPRouteError } from './middleware' const CONTENT_TYPE_V4 = 'application/spsp4+json' @@ -34,14 +35,19 @@ async function getSPSP( deps: ServiceDependencies, ctx: SPSPContext ): Promise { - ctx.assert(ctx.accepts(CONTENT_TYPE_V4), 406) + if (!ctx.accepts(CONTENT_TYPE_V4)) { + throw new SPSPRouteError(406, `Request does not support ${CONTENT_TYPE_V4}`) + } + const nonce = ctx.request.headers['receipt-nonce'] const secret = ctx.request.headers['receipt-secret'] - ctx.assert( - !nonce === !secret, - 400, - 'Failed to generate credentials: receipt nonce and secret must accompany each other' - ) + + if (!!nonce !== !!secret) { + throw new SPSPRouteError( + 400, + 'Failed to generate credentials: receipt nonce and secret must accompany each other' + ) + } try { const { ilpAddress, sharedSecret } = deps.streamServer.generateCredentials({ @@ -63,6 +69,10 @@ async function getSPSP( receipts_enabled: !!(nonce && secret) }) } catch (err) { - ctx.throw(400, err instanceof Error && err.message) + const errorMessage = + err instanceof Error ? err.message : 'Could not generate SPSP credentials' + throw new SPSPRouteError(400, errorMessage, { + paymentTag: ctx.paymentTag + }) } } From c5bfc9f888c0afe1da124098cfdabe3f721d8aa7 Mon Sep 17 00:00:00 2001 From: Max Kurapov Date: Mon, 6 May 2024 17:50:59 +0200 Subject: [PATCH 02/10] chore(backend): remove wallet address middleware, replace with inline middleware --- packages/backend/src/app.ts | 134 ++++++++++++++---- packages/backend/src/index.ts | 12 +- .../src/open_payments/auth/middleware.test.ts | 2 +- .../src/open_payments/auth/middleware.ts | 2 +- .../payment/incoming/routes.test.ts | 58 +++++++- .../open_payments/payment/incoming/routes.ts | 27 +++- .../payment/outgoing/routes.test.ts | 53 ++++++- .../open_payments/payment/outgoing/routes.ts | 24 +++- .../src/open_payments/quote/routes.test.ts | 24 ++++ .../backend/src/open_payments/quote/routes.ts | 17 ++- .../wallet_address/key/routes.test.ts | 33 +++-- .../wallet_address/key/routes.ts | 44 +++--- .../wallet_address/middleware.test.ts | 112 --------------- .../wallet_address/middleware.ts | 87 ------------ .../wallet_address/model.test.ts | 2 +- .../wallet_address/routes.test.ts | 2 +- .../open_payments/wallet_address/routes.ts | 25 ++-- .../wallet_address/service.test.ts | 26 +++- .../open_payments/wallet_address/service.ts | 33 ++++- 19 files changed, 421 insertions(+), 296 deletions(-) delete mode 100644 packages/backend/src/open_payments/wallet_address/middleware.test.ts delete mode 100644 packages/backend/src/open_payments/wallet_address/middleware.ts diff --git a/packages/backend/src/app.ts b/packages/backend/src/app.ts index 164952bb88..002702f057 100644 --- a/packages/backend/src/app.ts +++ b/packages/backend/src/app.ts @@ -24,8 +24,6 @@ import { HttpTokenService } from './payment-method/ilp/peer-http-token/service' import { AssetService, AssetOptions } from './asset/service' import { AccountingService } from './accounting/service' import { PeerService } from './payment-method/ilp/peer/service' -import { createWalletAddressMiddleware } from './open_payments/wallet_address/middleware' -import { WalletAddress } from './open_payments/wallet_address/model' import { WalletAddressService } from './open_payments/wallet_address/service' import { createTokenIntrospectionMiddleware, @@ -86,7 +84,10 @@ import { PaymentMethodHandlerService } from './payment-method/handler/service' import { IlpPaymentService } from './payment-method/ilp/service' import { TelemetryService } from './telemetry/service' import { ApolloArmor } from '@escape.tech/graphql-armor' -import { openPaymentsServerErrorMiddleware } from './open_payments/route-errors' +import { + OpenPaymentsServerRouteError, + openPaymentsServerErrorMiddleware +} from './open_payments/route-errors' import { verifyApiSignature } from './shared/utils' export interface AppContextData { @@ -94,8 +95,6 @@ export interface AppContextData { container: AppContainer // Set by @koa/router. params: { [key: string]: string } - walletAddress?: WalletAddress - walletAddressUrl?: string } export interface ApolloContext { @@ -112,19 +111,12 @@ export type AppRequest = Omit< } export interface WalletAddressContext extends AppContext { - walletAddress: WalletAddress + walletAddressUrl: string grant?: Grant client?: string accessAction?: AccessAction } -export type WalletAddressKeysContext = Omit< - WalletAddressContext, - 'walletAddress' -> & { - walletAddress?: WalletAddress -} - type HttpSigHeaders = Record<'signature' | 'signature-input', string> type HttpSigRequest = Omit & { @@ -141,7 +133,7 @@ export type HttpSigWithAuthenticatedStatusContext = HttpSigContext & AuthenticatedStatusContext // Wallet address subresources -interface GetCollectionQuery { +type GetCollectionQuery = { 'wallet-address': string } @@ -426,7 +418,10 @@ export class App { // Create incoming payment router.post>( '/incoming-payments', - createWalletAddressMiddleware(), + async (ctx, next) => { + ctx.walletAddressUrl = ctx.request.body.walletAddress + await next() + }, createValidatorMiddleware< ContextType> >( @@ -452,7 +447,10 @@ export class App { SignedCollectionContext >( '/incoming-payments', - createWalletAddressMiddleware(), + async (ctx, next) => { + ctx.walletAddressUrl = ctx.query['wallet-address'] as string + await next() + }, createValidatorMiddleware< ContextType> >( @@ -475,7 +473,10 @@ export class App { // Create outgoing payment router.post>( '/outgoing-payments', - createWalletAddressMiddleware(), + async (ctx, next) => { + ctx.walletAddressUrl = ctx.request.body.walletAddress + await next() + }, createValidatorMiddleware< ContextType> >( @@ -501,7 +502,10 @@ export class App { SignedCollectionContext >( '/outgoing-payments', - createWalletAddressMiddleware(), + async (ctx, next) => { + ctx.walletAddressUrl = ctx.query['wallet-address'] as string + await next() + }, createValidatorMiddleware< ContextType> >( @@ -524,7 +528,10 @@ export class App { // Create quote router.post>( '/quotes', - createWalletAddressMiddleware(), + async (ctx, next) => { + ctx.walletAddressUrl = ctx.request.body.walletAddress + await next() + }, createValidatorMiddleware< ContextType> >( @@ -547,7 +554,24 @@ export class App { // Read incoming payment router.get( '/incoming-payments/:id', - createWalletAddressMiddleware(), + async (ctx, next) => { + const incomingPaymentService = await ctx.container.use( + 'incomingPaymentService' + ) + const incomingPayment = await incomingPaymentService.get({ + id: ctx.params.id + }) + + if (!incomingPayment?.walletAddress) { + throw new OpenPaymentsServerRouteError(401, 'Unauthorized', { + description: 'Failed to get wallet address from incomig payment', + id: ctx.params.id + }) + } + + ctx.walletAddressUrl = incomingPayment.walletAddress.url + await next() + }, createValidatorMiddleware< ContextType >( @@ -571,7 +595,24 @@ export class App { // Complete incoming payment router.post( '/incoming-payments/:id/complete', - createWalletAddressMiddleware(), + async (ctx, next) => { + const incomingPaymentService = await ctx.container.use( + 'incomingPaymentService' + ) + const incomingPayment = await incomingPaymentService.get({ + id: ctx.params.id + }) + + if (!incomingPayment?.walletAddress) { + throw new OpenPaymentsServerRouteError(401, 'Unauthorized', { + description: 'Failed to get wallet address from incoming payment', + id: ctx.params.id + }) + } + + ctx.walletAddressUrl = incomingPayment.walletAddress.url + await next() + }, createValidatorMiddleware>( resourceServerSpec, { @@ -592,7 +633,24 @@ export class App { // Read outgoing payment router.get( '/outgoing-payments/:id', - createWalletAddressMiddleware(), + async (ctx, next) => { + const outgoingPaymentService = await ctx.container.use( + 'outgoingPaymentService' + ) + const outgoingPayment = await outgoingPaymentService.get({ + id: ctx.params.id + }) + + if (!outgoingPayment?.walletAddress) { + throw new OpenPaymentsServerRouteError(401, 'Unauthorized', { + description: 'Failed to get wallet address from outgoing payment', + id: ctx.params.id + }) + } + + ctx.walletAddressUrl = outgoingPayment.walletAddress.url + await next() + }, createValidatorMiddleware>( resourceServerSpec, { @@ -613,7 +671,22 @@ export class App { // Read quote router.get( '/quotes/:id', - createWalletAddressMiddleware(), + async (ctx, next) => { + const quoteService = await ctx.container.use('quoteService') + const quote = await quoteService.get({ + id: ctx.params.id + }) + + if (!quote?.walletAddress) { + throw new OpenPaymentsServerRouteError(401, 'Unauthorized', { + description: 'Failed to get wallet address from quote', + id: ctx.params.id + }) + } + + ctx.walletAddressUrl = quote.walletAddress.url + await next() + }, createValidatorMiddleware>( resourceServerSpec, { @@ -632,8 +705,11 @@ export class App { router.get( WALLET_ADDRESS_PATH + '/jwks.json', - createWalletAddressMiddleware(), - createValidatorMiddleware( + async (ctx, next) => { + ctx.walletAddressUrl = `https://${ctx.request.host}/${ctx.params.walletAddressPath}` + await next() + }, + createValidatorMiddleware( walletAddressServerSpec, { path: '/jwks.json', @@ -641,15 +717,17 @@ export class App { }, validatorMiddlewareOptions ), - async (ctx: WalletAddressKeysContext): Promise => - await walletAddressKeyRoutes.getKeysByWalletAddressId(ctx) + walletAddressKeyRoutes.getKeysByWalletAddressId ) // Add the wallet address query route last. // Otherwise it will be matched instead of other Open Payments endpoints. router.get( WALLET_ADDRESS_PATH, - createWalletAddressMiddleware(), + async (ctx, next) => { + ctx.walletAddressUrl = `https://${ctx.request.host}/${ctx.params.walletAddressPath}` + await next() + }, createSpspMiddleware(this.config.spspEnabled), createValidatorMiddleware( walletAddressServerSpec, diff --git a/packages/backend/src/index.ts b/packages/backend/src/index.ts index 930f0f3535..388b3f156b 100644 --- a/packages/backend/src/index.ts +++ b/packages/backend/src/index.ts @@ -304,14 +304,16 @@ export function initIocContainer( config: await deps.use('config'), logger: await deps.use('logger'), incomingPaymentService: await deps.use('incomingPaymentService'), - streamCredentialsService: await deps.use('streamCredentialsService') + streamCredentialsService: await deps.use('streamCredentialsService'), + walletAddressService: await deps.use('walletAddressService') }) }) container.singleton('walletAddressRoutes', async (deps) => { const config = await deps.use('config') return createWalletAddressRoutes({ authServer: config.authServerGrantUrl, - resourceServer: config.openPaymentsUrl + resourceServer: config.openPaymentsUrl, + walletAddressService: await deps.use('walletAddressService') }) }) container.singleton('walletAddressKeyRoutes', async (deps) => { @@ -458,7 +460,8 @@ export function initIocContainer( return createQuoteRoutes({ config: await deps.use('config'), logger: await deps.use('logger'), - quoteService: await deps.use('quoteService') + quoteService: await deps.use('quoteService'), + walletAddressService: await deps.use('walletAddressService') }) }) @@ -485,7 +488,8 @@ export function initIocContainer( return createOutgoingPaymentRoutes({ config: await deps.use('config'), logger: await deps.use('logger'), - outgoingPaymentService: await deps.use('outgoingPaymentService') + outgoingPaymentService: await deps.use('outgoingPaymentService'), + walletAddressService: await deps.use('walletAddressService') }) }) diff --git a/packages/backend/src/open_payments/auth/middleware.test.ts b/packages/backend/src/open_payments/auth/middleware.test.ts index b7d6256fa3..a899db0a49 100644 --- a/packages/backend/src/open_payments/auth/middleware.test.ts +++ b/packages/backend/src/open_payments/auth/middleware.test.ts @@ -210,7 +210,7 @@ describe('Auth Middleware', (): void => { beforeEach(async (): Promise => { if (identifierOption === IdentifierOption.Matching) { - identifier = ctx.walletAddress.url + identifier = ctx.walletAddressUrl } else if (identifierOption === IdentifierOption.Conflicting) { identifier = faker.internet.url({ appendSlash: false }) } diff --git a/packages/backend/src/open_payments/auth/middleware.ts b/packages/backend/src/open_payments/auth/middleware.ts index bfb3f12243..235364c2fc 100644 --- a/packages/backend/src/open_payments/auth/middleware.ts +++ b/packages/backend/src/open_payments/auth/middleware.ts @@ -95,7 +95,7 @@ export function createTokenIntrospectionMiddleware({ const access = tokenInfo.access.find((access: Access) => { if ( access.type !== requestType || - (access.identifier && access.identifier !== ctx.walletAddress.url) + (access.identifier && access.identifier !== ctx.walletAddressUrl) ) { return false } diff --git a/packages/backend/src/open_payments/payment/incoming/routes.test.ts b/packages/backend/src/open_payments/payment/incoming/routes.test.ts index d6357e0b62..2929828ca5 100644 --- a/packages/backend/src/open_payments/payment/incoming/routes.test.ts +++ b/packages/backend/src/open_payments/payment/incoming/routes.test.ts @@ -1,5 +1,6 @@ import { faker } from '@faker-js/faker' import jestOpenAPI from 'jest-openapi' +import assert from 'assert' import { Amount, AmountJSON, parseAmount, serializeAmount } from '../../amount' import { WalletAddress } from '../../wallet_address/model' @@ -8,7 +9,12 @@ import { createTestApp, TestContainer } from '../../../tests/app' import { Config, IAppConfig } from '../../../config/app' import { IocContract } from '@adonisjs/fold' import { initIocContainer } from '../../..' -import { AppServices, CreateContext, CompleteContext } from '../../../app' +import { + AppServices, + CreateContext, + CompleteContext, + ListContext +} from '../../../app' import { truncateTables } from '../../../tests/tableManager' import { IncomingPayment, IncomingPaymentState } from './model' import { @@ -23,6 +29,8 @@ import { Asset } from '../../../asset/model' import { IncomingPaymentError, errorToCode, errorToMessage } from './errors' import { IncomingPaymentService } from './service' import { IncomingPaymentWithPaymentMethods as OpenPaymentsIncomingPaymentWithPaymentMethods } from '@interledger/open-payments' +import { WalletAddressService } from '../../wallet_address/service' +import { OpenPaymentsServerRouteError } from '../../route-errors' describe('Incoming Payment Routes', (): void => { let deps: IocContract @@ -30,6 +38,7 @@ describe('Incoming Payment Routes', (): void => { let config: IAppConfig let incomingPaymentRoutes: IncomingPaymentRoutes let incomingPaymentService: IncomingPaymentService + let walletAddressService: WalletAddressService beforeAll(async (): Promise => { config = Config @@ -50,6 +59,7 @@ describe('Incoming Payment Routes', (): void => { config = await deps.use('config') incomingPaymentRoutes = await deps.use('incomingPaymentRoutes') incomingPaymentService = await deps.use('incomingPaymentService') + walletAddressService = await deps.use('walletAddressService') expiresAt = new Date(Date.now() + 30_000) asset = await createAsset(deps) @@ -153,6 +163,28 @@ describe('Incoming Payment Routes', (): void => { ) }) + describe('list', (): void => { + test('throws if cannot get wallet address', async (): Promise => { + const ctx = setup({ + reqOpts: { body: {} }, + walletAddress + }) + + jest + .spyOn(walletAddressService, 'getOrPollByUrl') + .mockResolvedValueOnce(undefined) + + expect.assertions(2) + try { + await incomingPaymentRoutes.list(ctx) + } catch (err) { + assert(err instanceof OpenPaymentsServerRouteError) + expect(err.message).toBe('Could not get wallet address') + expect(err.status).toBe(400) + } + }) + }) + describe('create', (): void => { let amount: AmountJSON @@ -252,6 +284,29 @@ describe('Incoming Payment Routes', (): void => { }) } ) + + test('throws if cannot get wallet address', async () => { + const ctx = setup>({ + reqOpts: { body: {} }, + walletAddress + }) + + jest + .spyOn(walletAddressService, 'getOrPollByUrl') + .mockResolvedValueOnce(undefined) + + expect.assertions(3) + try { + await incomingPaymentRoutes.create(ctx) + } catch (err) { + assert(err instanceof OpenPaymentsServerRouteError) + expect(err.message).toBe('Could not get wallet address') + expect(err.status).toBe(400) + } + + const createSpy = jest.spyOn(incomingPaymentService, 'create') + expect(createSpy).not.toHaveBeenCalled() + }) }) describe('complete', (): void => { @@ -276,7 +331,6 @@ describe('Incoming Payment Routes', (): void => { }, walletAddress }) - ctx.walletAddress = walletAddress await expect(incomingPaymentRoutes.complete(ctx)).resolves.toBeUndefined() expect(ctx.response).toSatisfyApiSpec() expect(ctx.body).toEqual({ diff --git a/packages/backend/src/open_payments/payment/incoming/routes.ts b/packages/backend/src/open_payments/payment/incoming/routes.ts index 6c0993d4d7..b294d5c186 100644 --- a/packages/backend/src/open_payments/payment/incoming/routes.ts +++ b/packages/backend/src/open_payments/payment/incoming/routes.ts @@ -15,12 +15,14 @@ import { StreamCredentialsService } from '../../../payment-method/ilp/stream-cre import { AccessAction } from '@interledger/open-payments' import { OpenPaymentsServerRouteError } from '../../route-errors' import { throwIfMissingWalletAddress } from '../../wallet_address/model' +import { WalletAddressService } from '../../wallet_address/service' interface ServiceDependencies { config: IAppConfig logger: Logger incomingPaymentService: IncomingPaymentService streamCredentialsService: StreamCredentialsService + walletAddressService: WalletAddressService } export type ReadContextWithAuthenticatedStatus = ReadContext & @@ -127,13 +129,21 @@ async function createIncomingPayment( ): Promise { const { body } = ctx.request + const walletAddress = await deps.walletAddressService.getOrPollByUrl( + ctx.walletAddressUrl + ) + + if (!walletAddress) { + throw new OpenPaymentsServerRouteError(400, 'Could not get wallet address') + } + let expiresAt: Date | undefined if (body.expiresAt !== undefined) { expiresAt = new Date(body.expiresAt) } const incomingPaymentOrError = await deps.incomingPaymentService.create({ - walletAddressId: ctx.walletAddress.id, + walletAddressId: walletAddress.id, client: ctx.client, metadata: body.metadata, expiresAt, @@ -147,14 +157,12 @@ async function createIncomingPayment( ) } - throwIfMissingWalletAddress(deps, incomingPaymentOrError) - ctx.status = 201 const streamCredentials = deps.streamCredentialsService.get( incomingPaymentOrError ) ctx.body = incomingPaymentOrError.toOpenPaymentsTypeWithMethods( - incomingPaymentOrError.walletAddress, + walletAddress, streamCredentials ) } @@ -185,9 +193,18 @@ async function listIncomingPayments( deps: ServiceDependencies, ctx: ListContext ): Promise { + const walletAddress = await deps.walletAddressService.getOrPollByUrl( + ctx.walletAddressUrl + ) + + if (!walletAddress) { + throw new OpenPaymentsServerRouteError(400, 'Could not get wallet address') + } + await listSubresource({ ctx, + walletAddress, getWalletAddressPage: deps.incomingPaymentService.getWalletAddressPage, - toBody: (payment) => payment.toOpenPaymentsType(ctx.walletAddress) + toBody: (payment) => payment.toOpenPaymentsType(walletAddress) }) } diff --git a/packages/backend/src/open_payments/payment/outgoing/routes.test.ts b/packages/backend/src/open_payments/payment/outgoing/routes.test.ts index 85a327e69f..f3d50aff61 100644 --- a/packages/backend/src/open_payments/payment/outgoing/routes.test.ts +++ b/packages/backend/src/open_payments/payment/outgoing/routes.test.ts @@ -7,7 +7,7 @@ import { createTestApp, TestContainer } from '../../../tests/app' import { Config, IAppConfig } from '../../../config/app' import { IocContract } from '@adonisjs/fold' import { initIocContainer } from '../../..' -import { AppServices, CreateContext } from '../../../app' +import { AppServices, CreateContext, ListContext } from '../../../app' import { truncateTables } from '../../../tests/tableManager' import { createAsset } from '../../../tests/asset' import { errorToCode, errorToMessage, OutgoingPaymentError } from './errors' @@ -31,6 +31,7 @@ import { createOutgoingPayment } from '../../../tests/outgoingPayment' import { createWalletAddress } from '../../../tests/walletAddress' import { UnionOmit } from '../../../shared/utils' import { OpenPaymentsServerRouteError } from '../../route-errors' +import { WalletAddressService } from '../../wallet_address/service' describe('Outgoing Payment Routes', (): void => { let deps: IocContract @@ -39,6 +40,7 @@ describe('Outgoing Payment Routes', (): void => { let config: IAppConfig let outgoingPaymentRoutes: OutgoingPaymentRoutes let outgoingPaymentService: OutgoingPaymentService + let walletAddressService: WalletAddressService let walletAddress: WalletAddress let baseUrl: string @@ -71,6 +73,7 @@ describe('Outgoing Payment Routes', (): void => { config = await deps.use('config') outgoingPaymentRoutes = await deps.use('outgoingPaymentRoutes') outgoingPaymentService = await deps.use('outgoingPaymentService') + walletAddressService = await deps.use('walletAddressService') const { resourceServerSpec } = await deps.use('openApi') jestOpenAPI(resourceServerSpec) }) @@ -132,6 +135,31 @@ describe('Outgoing Payment Routes', (): void => { }) }) + describe('list', (): void => { + test('throws if cannot get wallet address', async () => { + const ctx = setupContext({ + reqOpts: { + headers: { Accept: 'application/json' }, + method: 'GET' + }, + walletAddress + }) + + jest + .spyOn(walletAddressService, 'getOrPollByUrl') + .mockResolvedValueOnce(undefined) + + expect.assertions(2) + try { + await outgoingPaymentRoutes.list(ctx) + } catch (err) { + assert(err instanceof OpenPaymentsServerRouteError) + expect(err.message).toBe('Could not get wallet address') + expect(err.status).toBe(400) + } + }) + }) + type SetupContextOptions = UnionOmit< CreateOutgoingPaymentOptions, 'walletAddressId' @@ -294,5 +322,28 @@ describe('Outgoing Payment Routes', (): void => { }) } ) + + test('throws if cannot get wallet address', async () => { + const quoteId = uuid() + const ctx = setup({ + quoteId: `${baseUrl}/quotes/${quoteId}` + }) + + jest + .spyOn(walletAddressService, 'getOrPollByUrl') + .mockResolvedValueOnce(undefined) + + expect.assertions(3) + try { + await outgoingPaymentRoutes.create(ctx) + } catch (err) { + assert(err instanceof OpenPaymentsServerRouteError) + expect(err.message).toBe('Could not get wallet address') + expect(err.status).toBe(400) + } + + const createSpy = jest.spyOn(outgoingPaymentService, 'create') + expect(createSpy).not.toHaveBeenCalled() + }) }) }) diff --git a/packages/backend/src/open_payments/payment/outgoing/routes.ts b/packages/backend/src/open_payments/payment/outgoing/routes.ts index fbedb18119..4e697a4d54 100644 --- a/packages/backend/src/open_payments/payment/outgoing/routes.ts +++ b/packages/backend/src/open_payments/payment/outgoing/routes.ts @@ -19,11 +19,13 @@ import { } from '../../wallet_address/model' import { OpenPaymentsServerRouteError } from '../../route-errors' import { Amount } from '../../amount' +import { WalletAddressService } from '../../wallet_address/service' interface ServiceDependencies { config: IAppConfig logger: Logger outgoingPaymentService: OutgoingPaymentService + walletAddressService: WalletAddressService } export interface OutgoingPaymentRoutes { @@ -100,9 +102,18 @@ async function createOutgoingPayment( deps: ServiceDependencies, ctx: CreateContext ): Promise { + const walletAddress = await deps.walletAddressService.getOrPollByUrl( + ctx.walletAddressUrl + ) + + if (!walletAddress) { + throw new OpenPaymentsServerRouteError(400, 'Could not get wallet address') + } + const { body } = ctx.request + const baseOptions: OutgoingPaymentCreateBaseOptions = { - walletAddressId: ctx.walletAddress.id, + walletAddressId: walletAddress.id, metadata: body.metadata, client: ctx.client, grant: ctx.grant @@ -153,10 +164,19 @@ async function listOutgoingPayments( deps: ServiceDependencies, ctx: ListContext ): Promise { + const walletAddress = await deps.walletAddressService.getOrPollByUrl( + ctx.walletAddressUrl + ) + + if (!walletAddress) { + throw new OpenPaymentsServerRouteError(400, 'Could not get wallet address') + } + await listSubresource({ ctx, + walletAddress, getWalletAddressPage: deps.outgoingPaymentService.getWalletAddressPage, - toBody: (payment) => outgoingPaymentToBody(ctx.walletAddress, payment) + toBody: (payment) => outgoingPaymentToBody(walletAddress, payment) }) } diff --git a/packages/backend/src/open_payments/quote/routes.test.ts b/packages/backend/src/open_payments/quote/routes.test.ts index 422759bd73..1fd5884510 100644 --- a/packages/backend/src/open_payments/quote/routes.test.ts +++ b/packages/backend/src/open_payments/quote/routes.test.ts @@ -21,11 +21,14 @@ import { import { createAsset, randomAsset } from '../../tests/asset' import { createWalletAddress } from '../../tests/walletAddress' import { createQuote } from '../../tests/quote' +import { WalletAddressService } from '../wallet_address/service' +import { OpenPaymentsServerRouteError } from '../route-errors' describe('Quote Routes', (): void => { let deps: IocContract let appContainer: TestContainer let quoteService: QuoteService + let walletAddressService: WalletAddressService let config: IAppConfig let quoteRoutes: QuoteRoutes let walletAddress: WalletAddress @@ -67,6 +70,7 @@ describe('Quote Routes', (): void => { config = await deps.use('config') quoteRoutes = await deps.use('quoteRoutes') quoteService = await deps.use('quoteService') + walletAddressService = await deps.use('walletAddressService') const { resourceServerSpec } = await deps.use('openApi') jestOpenAPI(resourceServerSpec) }) @@ -283,5 +287,25 @@ describe('Quote Routes', (): void => { }) }) }) + + test('throws if cannot get wallet address', async () => { + const ctx = setup({}) + + jest + .spyOn(walletAddressService, 'getOrPollByUrl') + .mockResolvedValueOnce(undefined) + + expect.assertions(3) + try { + await quoteRoutes.create(ctx) + } catch (err) { + assert(err instanceof OpenPaymentsServerRouteError) + expect(err.message).toBe('Could not get wallet address') + expect(err.status).toBe(400) + } + + const createSpy = jest.spyOn(quoteService, 'create') + expect(createSpy).not.toHaveBeenCalled() + }) }) }) diff --git a/packages/backend/src/open_payments/quote/routes.ts b/packages/backend/src/open_payments/quote/routes.ts index 35119f9653..430637ba1e 100644 --- a/packages/backend/src/open_payments/quote/routes.ts +++ b/packages/backend/src/open_payments/quote/routes.ts @@ -12,11 +12,13 @@ import { throwIfMissingWalletAddress } from '../wallet_address/model' import { OpenPaymentsServerRouteError } from '../route-errors' +import { WalletAddressService } from '../wallet_address/service' interface ServiceDependencies { config: IAppConfig logger: Logger quoteService: QuoteService + walletAddressService: WalletAddressService } export interface QuoteRoutes { @@ -78,8 +80,17 @@ async function createQuote( ctx: CreateContext ): Promise { const { body } = ctx.request + + const walletAddress = await deps.walletAddressService.getOrPollByUrl( + ctx.walletAddressUrl + ) + + if (!walletAddress) { + throw new OpenPaymentsServerRouteError(400, 'Could not get wallet address') + } + const options: CreateQuoteOptions = { - walletAddressId: ctx.walletAddress.id, + walletAddressId: walletAddress.id, receiver: body.receiver, client: ctx.client, method: body.method @@ -106,10 +117,8 @@ async function createQuote( ) } - throwIfMissingWalletAddress(deps, quoteOrErr) - ctx.status = 201 - ctx.body = quoteToBody(quoteOrErr.walletAddress, quoteOrErr) + ctx.body = quoteToBody(walletAddress, quoteOrErr) } function quoteToBody( diff --git a/packages/backend/src/open_payments/wallet_address/key/routes.test.ts b/packages/backend/src/open_payments/wallet_address/key/routes.test.ts index 69ac364e2c..3a6474f865 100644 --- a/packages/backend/src/open_payments/wallet_address/key/routes.test.ts +++ b/packages/backend/src/open_payments/wallet_address/key/routes.test.ts @@ -1,17 +1,20 @@ import jestOpenAPI from 'jest-openapi' import { generateJwk } from '@interledger/http-signature-utils' import { v4 as uuid } from 'uuid' +import assert from 'assert' import { createContext } from '../../../tests/context' import { createTestApp, TestContainer } from '../../../tests/app' import { Config } from '../../../config/app' import { IocContract } from '@adonisjs/fold' import { initIocContainer } from '../../..' -import { AppServices, WalletAddressKeysContext } from '../../../app' +import { AppServices, WalletAddressContext } from '../../../app' import { truncateTables } from '../../../tests/tableManager' import { WalletAddressKeyRoutes } from './routes' import { WalletAddressKeyService } from './service' import { createWalletAddress } from '../../../tests/walletAddress' +import { WalletAddressService } from '../service' +import { OpenPaymentsServerRouteError } from '../../route-errors' const TEST_KEY = generateJwk({ keyId: uuid() }) @@ -20,6 +23,7 @@ describe('Wallet Address Keys Routes', (): void => { let appContainer: TestContainer let walletAddressKeyService: WalletAddressKeyService let walletAddressKeyRoutes: WalletAddressKeyRoutes + let walletAddressService: WalletAddressService const mockMessageProducer = { send: jest.fn() } @@ -31,6 +35,7 @@ describe('Wallet Address Keys Routes', (): void => { const { walletAddressServerSpec } = await deps.use('openApi') jestOpenAPI(walletAddressServerSpec) walletAddressKeyService = await deps.use('walletAddressKeyService') + walletAddressService = await deps.use('walletAddressService') }) beforeEach(async (): Promise => { @@ -55,11 +60,10 @@ describe('Wallet Address Keys Routes', (): void => { } const key = await walletAddressKeyService.create(keyOption) - const ctx = createContext({ + const ctx = createContext({ headers: { Accept: 'application/json' }, url: `/jwks.json` }) - ctx.walletAddress = walletAddress ctx.walletAddressUrl = walletAddress.url await expect( @@ -74,11 +78,10 @@ describe('Wallet Address Keys Routes', (): void => { test('returns 200 with empty array if no keys for a wallet address', async (): Promise => { const walletAddress = await createWalletAddress(deps) - const ctx = createContext({ + const ctx = createContext({ headers: { Accept: 'application/json' }, url: `/jwks.json` }) - ctx.walletAddress = walletAddress ctx.walletAddressUrl = walletAddress.url await expect( @@ -96,7 +99,7 @@ describe('Wallet Address Keys Routes', (): void => { keyId: config.keyId }) - const ctx = createContext({ + const ctx = createContext({ headers: { Accept: 'application/json' }, url: '/jwks.json' }) @@ -111,15 +114,23 @@ describe('Wallet Address Keys Routes', (): void => { }) test('returns 404 if wallet address does not exist', async (): Promise => { - const ctx = createContext({ + const ctx = createContext({ headers: { Accept: 'application/json' }, url: `/jwks.json` }) - ctx.walletAddress = undefined - await expect( - walletAddressKeyRoutes.getKeysByWalletAddressId(ctx) - ).rejects.toThrow(/Could not get wallet address keys./) + jest + .spyOn(walletAddressService, 'getOrPollByUrl') + .mockResolvedValueOnce(undefined) + + expect.assertions(2) + try { + await walletAddressKeyRoutes.getKeysByWalletAddressId(ctx) + } catch (err) { + assert.ok(err instanceof OpenPaymentsServerRouteError) + expect(err.message).toBe('Could not get wallet address') + expect(err.status).toBe(404) + } }) }) }) diff --git a/packages/backend/src/open_payments/wallet_address/key/routes.ts b/packages/backend/src/open_payments/wallet_address/key/routes.ts index 65b3915a1c..266f9e426f 100644 --- a/packages/backend/src/open_payments/wallet_address/key/routes.ts +++ b/packages/backend/src/open_payments/wallet_address/key/routes.ts @@ -1,10 +1,11 @@ import { generateJwk, JWK } from '@interledger/http-signature-utils' -import { WalletAddressKeysContext } from '../../../app' +import { WalletAddressContext } from '../../../app' import { IAppConfig } from '../../../config/app' import { WalletAddressService } from '../service' import { WalletAddressKeyService } from './service' import { Logger } from 'pino' +import { OpenPaymentsServerRouteError } from '../../route-errors' interface ServiceDependencies { walletAddressKeyService: WalletAddressKeyService @@ -15,7 +16,7 @@ interface ServiceDependencies { } export interface WalletAddressKeyRoutes { - getKeysByWalletAddressId(ctx: WalletAddressKeysContext): Promise + getKeysByWalletAddressId(ctx: WalletAddressContext): Promise } export function createWalletAddressKeyRoutes( @@ -30,38 +31,37 @@ export function createWalletAddressKeyRoutes( } return { - getKeysByWalletAddressId: (ctx: WalletAddressKeysContext) => + getKeysByWalletAddressId: (ctx: WalletAddressContext) => getKeysByWalletAddressId(deps, ctx) } } export async function getKeysByWalletAddressId( deps: ServiceDependencies, - ctx: WalletAddressKeysContext + ctx: WalletAddressContext ): Promise { - if (ctx.walletAddress) { - const keys = await deps.walletAddressKeyService.getKeysByWalletAddressId( - ctx.walletAddress.id - ) - - ctx.body = { - keys: keys.map((key) => key.jwk) - } - } else if (deps.config.walletAddressUrl === ctx.walletAddressUrl) { + if (deps.config.walletAddressUrl === ctx.walletAddressUrl) { ctx.body = { keys: [deps.jwk] } } else { - const errorMessage = - 'Could not get wallet address keys. It is possible there is a wallet address configuration error for this Rafiki instance.' - deps.logger.error( - { - requestedWalletAddress: ctx.walletAddressUrl, - configuredWalletAddressUrl: deps.config.walletAddressUrl - }, - errorMessage + const walletAddress = await deps.walletAddressService.getOrPollByUrl( + ctx.walletAddressUrl + ) + + if (!walletAddress) { + throw new OpenPaymentsServerRouteError( + 404, + 'Could not get wallet address' + ) + } + + const keys = await deps.walletAddressKeyService.getKeysByWalletAddressId( + walletAddress.id ) - throw new Error(errorMessage) + ctx.body = { + keys: keys.map((key) => key.jwk) + } } } diff --git a/packages/backend/src/open_payments/wallet_address/middleware.test.ts b/packages/backend/src/open_payments/wallet_address/middleware.test.ts deleted file mode 100644 index 699557fea1..0000000000 --- a/packages/backend/src/open_payments/wallet_address/middleware.test.ts +++ /dev/null @@ -1,112 +0,0 @@ -import { URL } from 'url' -import { createWalletAddressMiddleware } from './middleware' -import { Config } from '../../config/app' -import { IocContract } from '@adonisjs/fold' -import { initIocContainer } from '../../' -import { AppContext, AppServices } from '../../app' -import { createTestApp, TestContainer } from '../../tests/app' -import { createContext } from '../../tests/context' -import { createWalletAddress } from '../../tests/walletAddress' -import { truncateTables } from '../../tests/tableManager' - -type AppMiddleware = ( - ctx: AppContext, - next: () => Promise -) => Promise - -describe('Wallet Address Middleware', (): void => { - let deps: IocContract - let appContainer: TestContainer - let middleware: AppMiddleware - let ctx: AppContext - let next: jest.MockedFunction<() => Promise> - - beforeAll(async (): Promise => { - deps = await initIocContainer(Config) - appContainer = await createTestApp(deps) - middleware = createWalletAddressMiddleware() - }) - - beforeEach((): void => { - ctx = createContext( - { - headers: { - Accept: 'application/json' - } - }, - {} - ) - ctx.container = deps - next = jest.fn() - }) - - afterEach(async (): Promise => { - await truncateTables(appContainer.knex) - }) - - afterAll(async (): Promise => { - await appContainer.shutdown() - }) - - test('returns 404 for unknown wallet address url', async (): Promise => { - await expect(middleware(ctx, next)).rejects.toMatchObject({ - status: 404, - message: 'Not Found' - }) - expect(next).not.toHaveBeenCalled() - }) - - test('returns 404 for wallet address url not matching', async (): Promise => { - await expect(middleware(ctx, next)).rejects.toMatchObject({ - status: 404, - message: 'Not Found' - }) - expect(next).not.toHaveBeenCalled() - }) - - test('sets the context walletAddress and calls next', async (): Promise => { - const walletAddress = await createWalletAddress(deps) - const walletAddressUrl = new URL(walletAddress.url) - ctx.request.headers.host = walletAddressUrl.host - ctx.request.url = `${walletAddressUrl.pathname}/endpoint` - // Strip preceding forward slash - ctx.params.walletAddressPath = walletAddressUrl.pathname.substring(1) - await expect(middleware(ctx, next)).resolves.toBeUndefined() - expect(next).toHaveBeenCalled() - expect(ctx.walletAddress).toEqual(walletAddress) - }) - - test('calls next for walletAddressUrl', async (): Promise => { - const config = await deps.use('config') - const walletAddressUrl = new URL(config.walletAddressUrl) - ctx.request.headers.host = walletAddressUrl.host - ctx.request.url = walletAddressUrl.pathname - // Strip preceding forward slash - ctx.params.walletAddressPath = walletAddressUrl.pathname.substring(1) - await expect(middleware(ctx, next)).resolves.toBeUndefined() - expect(next).toHaveBeenCalled() - expect(ctx.walletAddress).toBeUndefined() - }) - - test('returns 404 for deactivated wallet address', async (): Promise => { - const walletAddress = await createWalletAddress(deps) - - const deactivatedAt = new Date() - deactivatedAt.setDate(deactivatedAt.getDate() - 1) - await walletAddress - .$query(appContainer.knex) - .patch({ deactivatedAt: new Date() }) - - const walletAddressUrl = new URL(walletAddress.url) - ctx.request.headers.host = walletAddressUrl.host - ctx.request.url = `${walletAddressUrl.pathname}/endpoint` - // Strip preceding forward slash - ctx.params.walletAddressPath = walletAddressUrl.pathname.substring(1) - - await expect(middleware(ctx, next)).rejects.toMatchObject({ - status: 404, - message: 'Not Found' - }) - expect(next).not.toHaveBeenCalled() - }) -}) diff --git a/packages/backend/src/open_payments/wallet_address/middleware.ts b/packages/backend/src/open_payments/wallet_address/middleware.ts deleted file mode 100644 index 9df6dab57b..0000000000 --- a/packages/backend/src/open_payments/wallet_address/middleware.ts +++ /dev/null @@ -1,87 +0,0 @@ -import { AppContext } from '../../app' -import { CreateBody as IncomingCreateBody } from '../../open_payments/payment/incoming/routes' -import { CreateBody as OutgoingCreateBody } from '../../open_payments/payment/outgoing/routes' -import { CreateBody as QuoteCreateBody } from '../../open_payments/quote/routes' - -type CreateBody = IncomingCreateBody | OutgoingCreateBody | QuoteCreateBody - -export function createWalletAddressMiddleware() { - return async ( - ctx: AppContext, - next: () => Promise - ): Promise => { - if (ctx.method === 'GET') { - if ( - ctx.path === '/incoming-payments' || - ctx.path === '/outgoing-payments' || - ctx.path === '/quotes' - ) { - ctx.walletAddressUrl = ctx.query['wallet-address'] as string - } else if (ctx.path && ctx.path.startsWith('/incoming-payments')) { - const incomingPaymentService = await ctx.container.use( - 'incomingPaymentService' - ) - const incomingPayment = await incomingPaymentService.get({ - id: ctx.params.id - }) - if (!incomingPayment || !incomingPayment.walletAddress) { - ctx.throw(401) - } - ctx.walletAddressUrl = incomingPayment.walletAddress.url - } else if (ctx.path && ctx.path.startsWith('/outgoing-payments')) { - const outgoingPaymentService = await ctx.container.use( - 'outgoingPaymentService' - ) - const outgoingPayment = await outgoingPaymentService.get({ - id: ctx.params.id - }) - if (!outgoingPayment || !outgoingPayment.walletAddress) { - ctx.throw(401) - } - ctx.walletAddressUrl = outgoingPayment.walletAddress.url - } else if (ctx.path && ctx.path.startsWith('/quotes')) { - const quoteService = await ctx.container.use('quoteService') - const quote = await quoteService.get({ id: ctx.params.id }) - if (!quote || !quote.walletAddress) { - ctx.throw(401) - } - ctx.walletAddressUrl = quote.walletAddress.url - } else { - ctx.walletAddressUrl = `https://${ctx.request.host}/${ctx.params.walletAddressPath}` - } - } else if (ctx.method === 'POST') { - if (ctx.path === `/incoming-payments/${ctx.params.id}/complete`) { - const incomingPaymentService = await ctx.container.use( - 'incomingPaymentService' - ) - const incomingPayment = await incomingPaymentService.get({ - id: ctx.params.id - }) - if (!incomingPayment || !incomingPayment.walletAddress) { - ctx.throw(401) - } - ctx.walletAddressUrl = incomingPayment.walletAddress.url - } else { - ctx.walletAddressUrl = (ctx.request.body as CreateBody).walletAddress - } - } else { - ctx.throw(401) - } - - const config = await ctx.container.use('config') - if (ctx.walletAddressUrl !== config.walletAddressUrl) { - const walletAddressService = await ctx.container.use( - 'walletAddressService' - ) - const walletAddress = await walletAddressService.getOrPollByUrl( - ctx.walletAddressUrl - ) - - if (!walletAddress?.isActive) { - ctx.throw(404) - } - ctx.walletAddress = walletAddress - } - await next() - } -} diff --git a/packages/backend/src/open_payments/wallet_address/model.test.ts b/packages/backend/src/open_payments/wallet_address/model.test.ts index 7026f4dfa8..5b378f4c8c 100644 --- a/packages/backend/src/open_payments/wallet_address/model.test.ts +++ b/packages/backend/src/open_payments/wallet_address/model.test.ts @@ -59,7 +59,7 @@ export const setup = < }, options.params ) - ctx.walletAddress = options.walletAddress + ctx.walletAddressUrl = options.walletAddress.url ctx.grant = options.grant ctx.client = options.client ctx.accessAction = options.accessAction diff --git a/packages/backend/src/open_payments/wallet_address/routes.test.ts b/packages/backend/src/open_payments/wallet_address/routes.test.ts index d4ca9cfa3f..29a6008f78 100644 --- a/packages/backend/src/open_payments/wallet_address/routes.test.ts +++ b/packages/backend/src/open_payments/wallet_address/routes.test.ts @@ -58,7 +58,7 @@ describe('Wallet Address Routes', (): void => { headers: { Accept: 'application/json' }, url: '/' }) - ctx.walletAddress = walletAddress + ctx.walletAddressUrl = walletAddress.url await expect(walletAddressRoutes.get(ctx)).resolves.toBeUndefined() expect(ctx.response).toSatisfyApiSpec() expect(ctx.body).toEqual({ diff --git a/packages/backend/src/open_payments/wallet_address/routes.ts b/packages/backend/src/open_payments/wallet_address/routes.ts index 734f854fad..4bf146f0d2 100644 --- a/packages/backend/src/open_payments/wallet_address/routes.ts +++ b/packages/backend/src/open_payments/wallet_address/routes.ts @@ -1,6 +1,9 @@ import { AccessAction } from '@interledger/open-payments' -import { WalletAddressSubresource } from './model' -import { WalletAddressSubresourceService } from './service' +import { WalletAddress, WalletAddressSubresource } from './model' +import { + WalletAddressService, + WalletAddressSubresourceService +} from './service' import { WalletAddressContext, ListContext } from '../../app' import { getPageInfo, @@ -11,6 +14,7 @@ import { OpenPaymentsServerRouteError } from '../route-errors' interface ServiceDependencies { authServer: string resourceServer: string + walletAddressService: WalletAddressService } export interface WalletAddressRoutes { @@ -25,12 +29,15 @@ export function createWalletAddressRoutes( } } -// Spec: https://docs.openpayments.guide/reference/get-public-account export async function getWalletAddress( deps: ServiceDependencies, ctx: WalletAddressContext ): Promise { - if (!ctx.walletAddress) { + const walletAddress = await deps.walletAddressService.getOrPollByUrl( + ctx.walletAddressUrl + ) + + if (!walletAddress) { throw new OpenPaymentsServerRouteError( 404, 'Wallet address does not exist', @@ -40,7 +47,7 @@ export async function getWalletAddress( ) } - ctx.body = ctx.walletAddress.toOpenPaymentsType({ + ctx.body = walletAddress.toOpenPaymentsType({ authServer: deps.authServer, resourceServer: deps.resourceServer }) @@ -48,12 +55,14 @@ export async function getWalletAddress( interface ListSubresourceOptions { ctx: ListContext + walletAddress: WalletAddress getWalletAddressPage: WalletAddressSubresourceService['getWalletAddressPage'] toBody: (model: M) => Record } export const listSubresource = async ({ ctx, + walletAddress, getWalletAddressPage, toBody }: ListSubresourceOptions) => { @@ -73,19 +82,19 @@ export const listSubresource = async ({ const pagination = parsePaginationQueryParameters(ctx.request.query) const client = ctx.accessAction === AccessAction.List ? ctx.client : undefined const page = await getWalletAddressPage({ - walletAddressId: ctx.walletAddress.id, + walletAddressId: walletAddress.id, pagination, client }) const pageInfo = await getPageInfo({ getPage: (pagination) => getWalletAddressPage({ - walletAddressId: ctx.walletAddress.id, + walletAddressId: walletAddress.id, pagination, client }), page, - walletAddress: ctx.request.query['wallet-address'] + walletAddress: walletAddress.url }) const result = { pagination: pageInfo, diff --git a/packages/backend/src/open_payments/wallet_address/service.test.ts b/packages/backend/src/open_payments/wallet_address/service.test.ts index 2307a628e5..45df0e6d9b 100644 --- a/packages/backend/src/open_payments/wallet_address/service.test.ts +++ b/packages/backend/src/open_payments/wallet_address/service.test.ts @@ -310,7 +310,7 @@ describe('Open Payments Wallet Address Service', (): void => { ) }) - describe('Get Or Poll Wallet Addres By Url', (): void => { + describe('Get Or Poll Wallet Address By Url', (): void => { describe('existing wallet address', (): void => { test('can retrieve wallet address by url', async (): Promise => { const walletAddress = await createWalletAddress(deps) @@ -318,6 +318,30 @@ describe('Open Payments Wallet Address Service', (): void => { walletAddressService.getOrPollByUrl(walletAddress.url) ).resolves.toEqual(walletAddress) }) + + test('returns undefined if inactive wallet address', async (): Promise => { + const walletAddress = await createWalletAddress(deps) + await walletAddress.$query(knex).patch({ + deactivatedAt: new Date() + }) + + await expect( + walletAddressService.getOrPollByUrl(walletAddress.url) + ).resolves.toEqual(undefined) + }) + + test('returns inactive wallet address if mustBeActive=false', async (): Promise => { + const walletAddress = await createWalletAddress(deps) + await walletAddress.$query(knex).patch({ + deactivatedAt: new Date() + }) + + await expect( + walletAddressService.getOrPollByUrl(walletAddress.url, { + mustBeActive: false + }) + ).resolves.toEqual(walletAddress) + }) }) describe('non-existing wallet address', (): void => { diff --git a/packages/backend/src/open_payments/wallet_address/service.ts b/packages/backend/src/open_payments/wallet_address/service.ts index e1907c3b97..b9c06e677a 100644 --- a/packages/backend/src/open_payments/wallet_address/service.ts +++ b/packages/backend/src/open_payments/wallet_address/service.ts @@ -29,6 +29,10 @@ interface Options { publicName?: string } +interface GetOrPollByUrlOptions { + mustBeActive?: boolean +} + export interface CreateOptions extends Options { url: string assetId: string @@ -46,7 +50,10 @@ export interface WalletAddressService { update(options: UpdateOptions): Promise get(id: string): Promise getByUrl(url: string): Promise - getOrPollByUrl(url: string): Promise + getOrPollByUrl( + url: string, + options?: GetOrPollByUrlOptions + ): Promise getPage( pagination?: Pagination, sortOrder?: SortOrder @@ -84,7 +91,7 @@ export async function createWalletAddressService({ update: (options) => updateWalletAddress(deps, options), get: (id) => getWalletAddress(deps, id), getByUrl: (url) => getWalletAddressByUrl(deps, url), - getOrPollByUrl: (url) => getOrPollByUrl(deps, url), + getOrPollByUrl: (url, options) => getOrPollByUrl(deps, url, options), getPage: (pagination?, sortOrder?) => getWalletAddressPage(deps, pagination, sortOrder), processNext: () => processNextWalletAddress(deps), @@ -181,12 +188,13 @@ async function getWalletAddress( async function getOrPollByUrl( deps: ServiceDependencies, - url: string + url: string, + options: GetOrPollByUrlOptions = { mustBeActive: true } ): Promise { const existingWalletAddress = await getWalletAddressByUrl(deps, url) if (existingWalletAddress) { - return existingWalletAddress + return checkActiveWalletAddress(existingWalletAddress, options) } await WalletAddressEvent.query(deps.knex).insert({ @@ -203,7 +211,7 @@ async function getOrPollByUrl( timeoutMs: deps.config.walletAddressLookupTimeoutMs }) - return walletAddress + return checkActiveWalletAddress(walletAddress, options) } catch (error) { const errorMessage = 'Could not get wallet address' deps.logger.error( @@ -215,6 +223,21 @@ async function getOrPollByUrl( } } +function checkActiveWalletAddress( + walletAddress?: WalletAddress, + options?: GetOrPollByUrlOptions +) { + if (!walletAddress) { + return undefined + } + + if (options?.mustBeActive && !walletAddress.isActive) { + return undefined + } + + return walletAddress +} + async function getWalletAddressByUrl( deps: ServiceDependencies, url: string From 6cf1e7fe70de28fcb593b679777e6573b9d0c68c Mon Sep 17 00:00:00 2001 From: Max Kurapov Date: Mon, 6 May 2024 13:42:01 +0200 Subject: [PATCH 03/10] feat(backend): SPSPRouteError --- .../src/open_payments/route-errors.test.ts | 20 +++ .../backend/src/open_payments/route-errors.ts | 17 +- .../wallet_address/model.test.ts | 1 + .../ilp/spsp/middleware.test.ts | 149 ++++++++++++------ .../src/payment-method/ilp/spsp/middleware.ts | 36 ++++- .../payment-method/ilp/spsp/routes.test.ts | 68 +++++++- .../src/payment-method/ilp/spsp/routes.ts | 24 ++- 7 files changed, 250 insertions(+), 65 deletions(-) diff --git a/packages/backend/src/open_payments/route-errors.test.ts b/packages/backend/src/open_payments/route-errors.test.ts index c837c2a0e5..e9d9c9c28a 100644 --- a/packages/backend/src/open_payments/route-errors.test.ts +++ b/packages/backend/src/open_payments/route-errors.test.ts @@ -8,6 +8,7 @@ import { IocContract } from '@adonisjs/fold' import { initIocContainer } from '..' import { Config } from '../config/app' import { OpenAPIValidatorMiddlewareError } from '@interledger/openapi' +import { SPSPRouteError } from '../payment-method/ilp/spsp/middleware' describe('openPaymentServerErrorMiddleware', (): void => { let deps: IocContract @@ -68,6 +69,25 @@ describe('openPaymentServerErrorMiddleware', (): void => { expect(next).toHaveBeenCalledTimes(1) }) + test('handles SPSPRouteError error', async (): Promise => { + const error = new SPSPRouteError(400, 'SPSP error') + const next = jest.fn().mockImplementationOnce(() => { + throw error + }) + + const ctxThrowSpy = jest.spyOn(ctx, 'throw') + + await expect( + openPaymentsServerErrorMiddleware(ctx, next) + ).rejects.toMatchObject({ + status: error.status, + message: error.message + }) + + expect(ctxThrowSpy).toHaveBeenCalledWith(error.status, error.message) + expect(next).toHaveBeenCalledTimes(1) + }) + test('handles unspecified error', async (): Promise => { const error = new Error('Some unspecified error') const next = jest.fn().mockImplementationOnce(() => { diff --git a/packages/backend/src/open_payments/route-errors.ts b/packages/backend/src/open_payments/route-errors.ts index 702c9cf184..afd1d90a31 100644 --- a/packages/backend/src/open_payments/route-errors.ts +++ b/packages/backend/src/open_payments/route-errors.ts @@ -1,5 +1,6 @@ import { AppContext } from '../app' import { OpenAPIValidatorMiddlewareError } from '@interledger/openapi' +import { SPSPRouteError } from '../payment-method/ilp/spsp/middleware' export class OpenPaymentsServerRouteError extends Error { public status: number @@ -43,7 +44,9 @@ export async function openPaymentsServerErrorMiddleware( 'Received error when handling Open Payments request' ) ctx.throw(err.status, err.message) - } else if (err instanceof OpenAPIValidatorMiddlewareError) { + } + + if (err instanceof OpenAPIValidatorMiddlewareError) { const finalStatus = err.status || 400 logger.info( @@ -57,6 +60,18 @@ export async function openPaymentsServerErrorMiddleware( ctx.throw(finalStatus, err.message) } + if (err instanceof SPSPRouteError) { + logger.info( + { + ...baseLog, + message: err.message, + details: err.details + }, + 'Received error when handling SPSP request' + ) + ctx.throw(err.status, err.message) + } + logger.error( { ...baseLog, err }, 'Received unhandled error in Open Payments request' diff --git a/packages/backend/src/open_payments/wallet_address/model.test.ts b/packages/backend/src/open_payments/wallet_address/model.test.ts index 7026f4dfa8..3b6d878cb4 100644 --- a/packages/backend/src/open_payments/wallet_address/model.test.ts +++ b/packages/backend/src/open_payments/wallet_address/model.test.ts @@ -60,6 +60,7 @@ export const setup = < options.params ) ctx.walletAddress = options.walletAddress + ctx.walletAddressUrl = options.walletAddress.url ctx.grant = options.grant ctx.client = options.client ctx.accessAction = options.accessAction diff --git a/packages/backend/src/payment-method/ilp/spsp/middleware.test.ts b/packages/backend/src/payment-method/ilp/spsp/middleware.test.ts index 0bf9fd7ed1..2fba195bf4 100644 --- a/packages/backend/src/payment-method/ilp/spsp/middleware.test.ts +++ b/packages/backend/src/payment-method/ilp/spsp/middleware.test.ts @@ -1,4 +1,8 @@ -import { createSpspMiddleware, SPSPWalletAddressContext } from './middleware' +import { + createSpspMiddleware, + SPSPRouteError, + SPSPWalletAddressContext +} from './middleware' import { setup } from '../../../open_payments/wallet_address/model.test' import { Config } from '../../../config/app' import { IocContract } from '@adonisjs/fold' @@ -8,18 +12,38 @@ import { createTestApp, TestContainer } from '../../../tests/app' import { createAsset } from '../../../tests/asset' import { createWalletAddress } from '../../../tests/walletAddress' import { truncateTables } from '../../../tests/tableManager' +import { WalletAddress } from '../../../open_payments/wallet_address/model' +import assert from 'assert' +import { SPSPRoutes } from './routes' +import { WalletAddressService } from '../../../open_payments/wallet_address/service' describe('SPSP Middleware', (): void => { let deps: IocContract let appContainer: TestContainer + let spspRoutes: SPSPRoutes + let walletAddressService: WalletAddressService let next: jest.MockedFunction<() => Promise> + let ctx: SPSPWalletAddressContext + let walletAddress: WalletAddress + beforeAll(async (): Promise => { - deps = await initIocContainer(Config) + deps = initIocContainer(Config) appContainer = await createTestApp(deps) + spspRoutes = await deps.use('spspRoutes') + walletAddressService = await deps.use('walletAddressService') }) - beforeEach((): void => { + beforeEach(async (): Promise => { + const asset = await createAsset(deps) + walletAddress = await createWalletAddress(deps, { + assetId: asset.id + }) + ctx = setup({ + reqOpts: {}, + walletAddress + }) + ctx.container = deps next = jest.fn() }) @@ -31,50 +55,81 @@ describe('SPSP Middleware', (): void => { await appContainer.shutdown() }) - describe('Wallet Address', (): void => { - let ctx: SPSPWalletAddressContext - - beforeEach(async (): Promise => { - const asset = await createAsset(deps) - const walletAddress = await createWalletAddress(deps, { - assetId: asset.id - }) - ctx = setup({ - reqOpts: {}, - walletAddress - }) - ctx.container = deps - }) - - test.each` - header | spspEnabled | description - ${'application/json'} | ${true} | ${'calls next'} - ${'application/json'} | ${false} | ${'calls next'} - ${'application/spsp4+json'} | ${true} | ${'calls SPSP route'} - ${'application/spsp4+json'} | ${false} | ${'calls next'} - `( - '$description for accept header: $header and spspEnabled: $spspEnabled', - async ({ header, spspEnabled }): Promise => { - const spspRoutes = await ctx.container.use('spspRoutes') - const spspSpy = jest - .spyOn(spspRoutes, 'get') - .mockResolvedValueOnce(undefined) - ctx.headers['accept'] = header - const spspMiddleware = createSpspMiddleware(spspEnabled) - await expect(spspMiddleware(ctx, next)).resolves.toBeUndefined() - if (!spspEnabled || header == 'application/json') { - expect(spspSpy).not.toHaveBeenCalled() - expect(next).toHaveBeenCalled() - } else { - expect(spspSpy).toHaveBeenCalledTimes(1) - expect(next).not.toHaveBeenCalled() - expect(ctx.paymentTag).toEqual(ctx.walletAddress.id) - expect(ctx.asset).toEqual({ - code: ctx.walletAddress.asset.code, - scale: ctx.walletAddress.asset.scale - }) - } + test.each` + header | spspEnabled | description + ${'application/json'} | ${true} | ${'calls next'} + ${'application/json'} | ${false} | ${'calls next'} + ${'application/spsp4+json'} | ${true} | ${'calls SPSP route'} + ${'application/spsp4+json'} | ${false} | ${'calls next'} + `( + '$description for accept header: $header and spspEnabled: $spspEnabled', + async ({ header, spspEnabled }): Promise => { + const spspSpy = jest + .spyOn(spspRoutes, 'get') + .mockResolvedValueOnce(undefined) + ctx.headers['accept'] = header + const spspMiddleware = createSpspMiddleware(spspEnabled) + await expect(spspMiddleware(ctx, next)).resolves.toBeUndefined() + if (!spspEnabled || header == 'application/json') { + expect(spspSpy).not.toHaveBeenCalled() + expect(next).toHaveBeenCalled() + } else { + expect(spspSpy).toHaveBeenCalledTimes(1) + expect(next).not.toHaveBeenCalled() + expect(ctx.paymentTag).toEqual(walletAddress.id) + expect(ctx.asset).toEqual({ + code: walletAddress.asset.code, + scale: walletAddress.asset.scale + }) } - ) + } + ) + + test('throws error if could not find wallet address', async () => { + const spspGetRouteSpy = jest + .spyOn(spspRoutes, 'get') + .mockResolvedValueOnce('dogs' as unknown as void) + + jest + .spyOn(walletAddressService, 'getByUrl') + .mockResolvedValueOnce(undefined) + + ctx.header['accept'] = 'application/spsp4+json' + + const spspMiddleware = createSpspMiddleware(true) + + expect.assertions(4) + try { + await spspMiddleware(ctx, next) + } catch (err) { + assert.ok(err instanceof SPSPRouteError) + expect(err.status).toBe(404) + expect(err.message).toBe('Could not get wallet address') + expect(next).not.toHaveBeenCalled() + expect(spspGetRouteSpy).not.toHaveBeenCalled() + } + }) + + test('throws error if inactive wallet address', async () => { + const spspGetRouteSpy = jest.spyOn(spspRoutes, 'get') + + ctx.header['accept'] = 'application/spsp4+json' + + await walletAddress + .$query(appContainer.knex) + .patch({ deactivatedAt: new Date() }) + + const spspMiddleware = createSpspMiddleware(true) + + expect.assertions(4) + try { + await spspMiddleware(ctx, next) + } catch (err) { + assert.ok(err instanceof SPSPRouteError) + expect(err.status).toBe(404) + expect(err.message).toBe('Could not get wallet address') + expect(next).not.toHaveBeenCalled() + expect(spspGetRouteSpy).not.toHaveBeenCalled() + } }) }) diff --git a/packages/backend/src/payment-method/ilp/spsp/middleware.ts b/packages/backend/src/payment-method/ilp/spsp/middleware.ts index 362f078e62..69a15c473b 100644 --- a/packages/backend/src/payment-method/ilp/spsp/middleware.ts +++ b/packages/backend/src/payment-method/ilp/spsp/middleware.ts @@ -2,9 +2,25 @@ import { WalletAddressContext, SPSPContext } from '../../../app' export type SPSPWalletAddressContext = WalletAddressContext & SPSPContext & { - incomingPayment?: never + walletAddressUrl: string } +export class SPSPRouteError extends Error { + public status: number + public details?: Record + + constructor( + status: number, + message: string, + details?: Record + ) { + super(message) + this.name = 'SPSPRouteError' + this.status = status + this.details = details + } +} + export function createSpspMiddleware(spspEnabled: boolean) { if (spspEnabled) { return spspMiddleware @@ -22,13 +38,21 @@ const spspMiddleware = async ( ctx: SPSPWalletAddressContext, next: () => Promise ): Promise => { - // Fall back to legacy protocols if client doesn't support Open Payments. if (ctx.accepts('application/spsp4+json')) { - const receiver = ctx.walletAddress ?? ctx.incomingPayment - ctx.paymentTag = receiver.id + const walletAddressService = await ctx.container.use('walletAddressService') + + const walletAddress = await walletAddressService.getByUrl( + ctx.walletAddressUrl + ) + + if (!walletAddress?.isActive) { + throw new SPSPRouteError(404, 'Could not get wallet address') + } + + ctx.paymentTag = walletAddress.id ctx.asset = { - code: receiver.asset.code, - scale: receiver.asset.scale + code: walletAddress.asset.code, + scale: walletAddress.asset.scale } const spspRoutes = await ctx.container.use('spspRoutes') await spspRoutes.get(ctx) diff --git a/packages/backend/src/payment-method/ilp/spsp/routes.test.ts b/packages/backend/src/payment-method/ilp/spsp/routes.test.ts index 95385c7078..7c6f1699ff 100644 --- a/packages/backend/src/payment-method/ilp/spsp/routes.test.ts +++ b/packages/backend/src/payment-method/ilp/spsp/routes.test.ts @@ -1,5 +1,6 @@ import * as crypto from 'crypto' import { v4 as uuid } from 'uuid' +import assert from 'assert' import { AppServices, SPSPContext } from '../../../app' import { SPSPRoutes } from './routes' @@ -13,6 +14,7 @@ import { StreamServer } from '@interledger/stream-receiver' import { randomAsset } from '../../../tests/asset' import { createContext } from '../../../tests/context' import { truncateTables } from '../../../tests/tableManager' +import { SPSPRouteError } from './middleware' type SPSPHeader = { Accept: string @@ -76,17 +78,46 @@ describe('SPSP Routes', (): void => { const ctx = createContext({ headers: { Accept: 'application/json' } }) - await expect(spspRoutes.get(ctx)).rejects.toHaveProperty('status', 406) + + expect.assertions(2) + try { + await spspRoutes.get(ctx) + } catch (err) { + assert.ok(err instanceof SPSPRouteError) + expect(err.status).toBe(406) + expect(err.message).toBe( + 'Request does not support application/spsp4+json' + ) + } }) test('nonce, no secret; returns 400', async () => { const ctx = setup({ nonce }) - await expect(spspRoutes.get(ctx)).rejects.toHaveProperty('status', 400) + + expect.assertions(2) + try { + await spspRoutes.get(ctx) + } catch (err) { + assert.ok(err instanceof SPSPRouteError) + expect(err.status).toBe(400) + expect(err.message).toBe( + 'Failed to generate credentials: receipt nonce and secret must accompany each other' + ) + } }) test('secret; no nonce; returns 400', async () => { const ctx = setup({ secret }) - await expect(spspRoutes.get(ctx)).rejects.toHaveProperty('status', 400) + expect.assertions(2) + try { + await spspRoutes.get(ctx) + } catch (err) { + assert.ok(err instanceof SPSPRouteError) + expect(err.status).toBe(400) + expect(err.message).toBe( + 'Failed to generate credentials: receipt nonce and secret must accompany each other' + ) + } }) test('malformed nonce; returns 400', async () => { @@ -94,7 +125,17 @@ describe('SPSP Routes', (): void => { nonce: Buffer.alloc(15).toString('base64'), secret }) - await expect(spspRoutes.get(ctx)).rejects.toHaveProperty('status', 400) + + expect.assertions(2) + try { + await spspRoutes.get(ctx) + } catch (err) { + assert.ok(err instanceof SPSPRouteError) + expect(err.status).toBe(400) + expect(err.message).toBe( + 'Failed to generate credentials: receipt nonce must be 16 bytes' + ) + } }) const receiptSetup = { @@ -139,6 +180,25 @@ describe('SPSP Routes', (): void => { } ) + test('handle error when generating credentials', async () => { + const ctx = setup() + + jest + .spyOn(streamServer, 'generateCredentials') + .mockImplementationOnce(() => { + throw new Error('Could not generate credentials') + }) + + expect.assertions(2) + try { + await spspRoutes.get(ctx) + } catch (err) { + assert.ok(err instanceof SPSPRouteError) + expect(err.status).toBe(400) + expect(err.message).toBe('Could not generate credentials') + } + }) + /** * Utility functions */ diff --git a/packages/backend/src/payment-method/ilp/spsp/routes.ts b/packages/backend/src/payment-method/ilp/spsp/routes.ts index 5f77ff17d7..0f443703aa 100644 --- a/packages/backend/src/payment-method/ilp/spsp/routes.ts +++ b/packages/backend/src/payment-method/ilp/spsp/routes.ts @@ -2,6 +2,7 @@ import { BaseService } from '../../../shared/baseService' import { SPSPContext } from '../../../app' import base64url from 'base64url' import { StreamServer } from '@interledger/stream-receiver' +import { SPSPRouteError } from './middleware' const CONTENT_TYPE_V4 = 'application/spsp4+json' @@ -34,14 +35,19 @@ async function getSPSP( deps: ServiceDependencies, ctx: SPSPContext ): Promise { - ctx.assert(ctx.accepts(CONTENT_TYPE_V4), 406) + if (!ctx.accepts(CONTENT_TYPE_V4)) { + throw new SPSPRouteError(406, `Request does not support ${CONTENT_TYPE_V4}`) + } + const nonce = ctx.request.headers['receipt-nonce'] const secret = ctx.request.headers['receipt-secret'] - ctx.assert( - !nonce === !secret, - 400, - 'Failed to generate credentials: receipt nonce and secret must accompany each other' - ) + + if (!!nonce !== !!secret) { + throw new SPSPRouteError( + 400, + 'Failed to generate credentials: receipt nonce and secret must accompany each other' + ) + } try { const { ilpAddress, sharedSecret } = deps.streamServer.generateCredentials({ @@ -63,6 +69,10 @@ async function getSPSP( receipts_enabled: !!(nonce && secret) }) } catch (err) { - ctx.throw(400, err instanceof Error && err.message) + const errorMessage = + err instanceof Error ? err.message : 'Could not generate SPSP credentials' + throw new SPSPRouteError(400, errorMessage, { + paymentTag: ctx.paymentTag + }) } } From b0b6e130f4fbc2e8cd7cbe7c2d7de1311ec27a26 Mon Sep 17 00:00:00 2001 From: Max Kurapov Date: Mon, 6 May 2024 13:42:01 +0200 Subject: [PATCH 04/10] feat(backend): SPSPRouteError --- .../src/open_payments/route-errors.test.ts | 20 +++ .../backend/src/open_payments/route-errors.ts | 17 +- .../wallet_address/model.test.ts | 1 + .../ilp/spsp/middleware.test.ts | 147 ++++++++++++------ .../src/payment-method/ilp/spsp/middleware.ts | 36 ++++- .../payment-method/ilp/spsp/routes.test.ts | 68 +++++++- .../src/payment-method/ilp/spsp/routes.ts | 24 ++- 7 files changed, 248 insertions(+), 65 deletions(-) diff --git a/packages/backend/src/open_payments/route-errors.test.ts b/packages/backend/src/open_payments/route-errors.test.ts index c837c2a0e5..e9d9c9c28a 100644 --- a/packages/backend/src/open_payments/route-errors.test.ts +++ b/packages/backend/src/open_payments/route-errors.test.ts @@ -8,6 +8,7 @@ import { IocContract } from '@adonisjs/fold' import { initIocContainer } from '..' import { Config } from '../config/app' import { OpenAPIValidatorMiddlewareError } from '@interledger/openapi' +import { SPSPRouteError } from '../payment-method/ilp/spsp/middleware' describe('openPaymentServerErrorMiddleware', (): void => { let deps: IocContract @@ -68,6 +69,25 @@ describe('openPaymentServerErrorMiddleware', (): void => { expect(next).toHaveBeenCalledTimes(1) }) + test('handles SPSPRouteError error', async (): Promise => { + const error = new SPSPRouteError(400, 'SPSP error') + const next = jest.fn().mockImplementationOnce(() => { + throw error + }) + + const ctxThrowSpy = jest.spyOn(ctx, 'throw') + + await expect( + openPaymentsServerErrorMiddleware(ctx, next) + ).rejects.toMatchObject({ + status: error.status, + message: error.message + }) + + expect(ctxThrowSpy).toHaveBeenCalledWith(error.status, error.message) + expect(next).toHaveBeenCalledTimes(1) + }) + test('handles unspecified error', async (): Promise => { const error = new Error('Some unspecified error') const next = jest.fn().mockImplementationOnce(() => { diff --git a/packages/backend/src/open_payments/route-errors.ts b/packages/backend/src/open_payments/route-errors.ts index 702c9cf184..afd1d90a31 100644 --- a/packages/backend/src/open_payments/route-errors.ts +++ b/packages/backend/src/open_payments/route-errors.ts @@ -1,5 +1,6 @@ import { AppContext } from '../app' import { OpenAPIValidatorMiddlewareError } from '@interledger/openapi' +import { SPSPRouteError } from '../payment-method/ilp/spsp/middleware' export class OpenPaymentsServerRouteError extends Error { public status: number @@ -43,7 +44,9 @@ export async function openPaymentsServerErrorMiddleware( 'Received error when handling Open Payments request' ) ctx.throw(err.status, err.message) - } else if (err instanceof OpenAPIValidatorMiddlewareError) { + } + + if (err instanceof OpenAPIValidatorMiddlewareError) { const finalStatus = err.status || 400 logger.info( @@ -57,6 +60,18 @@ export async function openPaymentsServerErrorMiddleware( ctx.throw(finalStatus, err.message) } + if (err instanceof SPSPRouteError) { + logger.info( + { + ...baseLog, + message: err.message, + details: err.details + }, + 'Received error when handling SPSP request' + ) + ctx.throw(err.status, err.message) + } + logger.error( { ...baseLog, err }, 'Received unhandled error in Open Payments request' diff --git a/packages/backend/src/open_payments/wallet_address/model.test.ts b/packages/backend/src/open_payments/wallet_address/model.test.ts index 7026f4dfa8..3b6d878cb4 100644 --- a/packages/backend/src/open_payments/wallet_address/model.test.ts +++ b/packages/backend/src/open_payments/wallet_address/model.test.ts @@ -60,6 +60,7 @@ export const setup = < options.params ) ctx.walletAddress = options.walletAddress + ctx.walletAddressUrl = options.walletAddress.url ctx.grant = options.grant ctx.client = options.client ctx.accessAction = options.accessAction diff --git a/packages/backend/src/payment-method/ilp/spsp/middleware.test.ts b/packages/backend/src/payment-method/ilp/spsp/middleware.test.ts index 0bf9fd7ed1..a2a655e041 100644 --- a/packages/backend/src/payment-method/ilp/spsp/middleware.test.ts +++ b/packages/backend/src/payment-method/ilp/spsp/middleware.test.ts @@ -1,4 +1,8 @@ -import { createSpspMiddleware, SPSPWalletAddressContext } from './middleware' +import { + createSpspMiddleware, + SPSPRouteError, + SPSPWalletAddressContext +} from './middleware' import { setup } from '../../../open_payments/wallet_address/model.test' import { Config } from '../../../config/app' import { IocContract } from '@adonisjs/fold' @@ -8,18 +12,38 @@ import { createTestApp, TestContainer } from '../../../tests/app' import { createAsset } from '../../../tests/asset' import { createWalletAddress } from '../../../tests/walletAddress' import { truncateTables } from '../../../tests/tableManager' +import { WalletAddress } from '../../../open_payments/wallet_address/model' +import assert from 'assert' +import { SPSPRoutes } from './routes' +import { WalletAddressService } from '../../../open_payments/wallet_address/service' describe('SPSP Middleware', (): void => { let deps: IocContract let appContainer: TestContainer + let spspRoutes: SPSPRoutes + let walletAddressService: WalletAddressService let next: jest.MockedFunction<() => Promise> + let ctx: SPSPWalletAddressContext + let walletAddress: WalletAddress + beforeAll(async (): Promise => { - deps = await initIocContainer(Config) + deps = initIocContainer(Config) appContainer = await createTestApp(deps) + spspRoutes = await deps.use('spspRoutes') + walletAddressService = await deps.use('walletAddressService') }) - beforeEach((): void => { + beforeEach(async (): Promise => { + const asset = await createAsset(deps) + walletAddress = await createWalletAddress(deps, { + assetId: asset.id + }) + ctx = setup({ + reqOpts: {}, + walletAddress + }) + ctx.container = deps next = jest.fn() }) @@ -31,50 +55,79 @@ describe('SPSP Middleware', (): void => { await appContainer.shutdown() }) - describe('Wallet Address', (): void => { - let ctx: SPSPWalletAddressContext - - beforeEach(async (): Promise => { - const asset = await createAsset(deps) - const walletAddress = await createWalletAddress(deps, { - assetId: asset.id - }) - ctx = setup({ - reqOpts: {}, - walletAddress - }) - ctx.container = deps - }) - - test.each` - header | spspEnabled | description - ${'application/json'} | ${true} | ${'calls next'} - ${'application/json'} | ${false} | ${'calls next'} - ${'application/spsp4+json'} | ${true} | ${'calls SPSP route'} - ${'application/spsp4+json'} | ${false} | ${'calls next'} - `( - '$description for accept header: $header and spspEnabled: $spspEnabled', - async ({ header, spspEnabled }): Promise => { - const spspRoutes = await ctx.container.use('spspRoutes') - const spspSpy = jest - .spyOn(spspRoutes, 'get') - .mockResolvedValueOnce(undefined) - ctx.headers['accept'] = header - const spspMiddleware = createSpspMiddleware(spspEnabled) - await expect(spspMiddleware(ctx, next)).resolves.toBeUndefined() - if (!spspEnabled || header == 'application/json') { - expect(spspSpy).not.toHaveBeenCalled() - expect(next).toHaveBeenCalled() - } else { - expect(spspSpy).toHaveBeenCalledTimes(1) - expect(next).not.toHaveBeenCalled() - expect(ctx.paymentTag).toEqual(ctx.walletAddress.id) - expect(ctx.asset).toEqual({ - code: ctx.walletAddress.asset.code, - scale: ctx.walletAddress.asset.scale - }) - } + test.each` + header | spspEnabled | description + ${'application/json'} | ${true} | ${'calls next'} + ${'application/json'} | ${false} | ${'calls next'} + ${'application/spsp4+json'} | ${true} | ${'calls SPSP route'} + ${'application/spsp4+json'} | ${false} | ${'calls next'} + `( + '$description for accept header: $header and spspEnabled: $spspEnabled', + async ({ header, spspEnabled }): Promise => { + const spspSpy = jest + .spyOn(spspRoutes, 'get') + .mockResolvedValueOnce(undefined) + ctx.headers['accept'] = header + const spspMiddleware = createSpspMiddleware(spspEnabled) + await expect(spspMiddleware(ctx, next)).resolves.toBeUndefined() + if (!spspEnabled || header == 'application/json') { + expect(spspSpy).not.toHaveBeenCalled() + expect(next).toHaveBeenCalled() + } else { + expect(spspSpy).toHaveBeenCalledTimes(1) + expect(next).not.toHaveBeenCalled() + expect(ctx.paymentTag).toEqual(walletAddress.id) + expect(ctx.asset).toEqual({ + code: walletAddress.asset.code, + scale: walletAddress.asset.scale + }) } - ) + } + ) + + test('throws error if could not find wallet address', async () => { + const spspGetRouteSpy = jest.spyOn(spspRoutes, 'get') + + jest + .spyOn(walletAddressService, 'getByUrl') + .mockResolvedValueOnce(undefined) + + ctx.header['accept'] = 'application/spsp4+json' + + const spspMiddleware = createSpspMiddleware(true) + + expect.assertions(4) + try { + await spspMiddleware(ctx, next) + } catch (err) { + assert.ok(err instanceof SPSPRouteError) + expect(err.status).toBe(404) + expect(err.message).toBe('Could not get wallet address') + expect(next).not.toHaveBeenCalled() + expect(spspGetRouteSpy).not.toHaveBeenCalled() + } + }) + + test('throws error if inactive wallet address', async () => { + const spspGetRouteSpy = jest.spyOn(spspRoutes, 'get') + + ctx.header['accept'] = 'application/spsp4+json' + + await walletAddress + .$query(appContainer.knex) + .patch({ deactivatedAt: new Date() }) + + const spspMiddleware = createSpspMiddleware(true) + + expect.assertions(4) + try { + await spspMiddleware(ctx, next) + } catch (err) { + assert.ok(err instanceof SPSPRouteError) + expect(err.status).toBe(404) + expect(err.message).toBe('Could not get wallet address') + expect(next).not.toHaveBeenCalled() + expect(spspGetRouteSpy).not.toHaveBeenCalled() + } }) }) diff --git a/packages/backend/src/payment-method/ilp/spsp/middleware.ts b/packages/backend/src/payment-method/ilp/spsp/middleware.ts index 362f078e62..69a15c473b 100644 --- a/packages/backend/src/payment-method/ilp/spsp/middleware.ts +++ b/packages/backend/src/payment-method/ilp/spsp/middleware.ts @@ -2,9 +2,25 @@ import { WalletAddressContext, SPSPContext } from '../../../app' export type SPSPWalletAddressContext = WalletAddressContext & SPSPContext & { - incomingPayment?: never + walletAddressUrl: string } +export class SPSPRouteError extends Error { + public status: number + public details?: Record + + constructor( + status: number, + message: string, + details?: Record + ) { + super(message) + this.name = 'SPSPRouteError' + this.status = status + this.details = details + } +} + export function createSpspMiddleware(spspEnabled: boolean) { if (spspEnabled) { return spspMiddleware @@ -22,13 +38,21 @@ const spspMiddleware = async ( ctx: SPSPWalletAddressContext, next: () => Promise ): Promise => { - // Fall back to legacy protocols if client doesn't support Open Payments. if (ctx.accepts('application/spsp4+json')) { - const receiver = ctx.walletAddress ?? ctx.incomingPayment - ctx.paymentTag = receiver.id + const walletAddressService = await ctx.container.use('walletAddressService') + + const walletAddress = await walletAddressService.getByUrl( + ctx.walletAddressUrl + ) + + if (!walletAddress?.isActive) { + throw new SPSPRouteError(404, 'Could not get wallet address') + } + + ctx.paymentTag = walletAddress.id ctx.asset = { - code: receiver.asset.code, - scale: receiver.asset.scale + code: walletAddress.asset.code, + scale: walletAddress.asset.scale } const spspRoutes = await ctx.container.use('spspRoutes') await spspRoutes.get(ctx) diff --git a/packages/backend/src/payment-method/ilp/spsp/routes.test.ts b/packages/backend/src/payment-method/ilp/spsp/routes.test.ts index 95385c7078..7c6f1699ff 100644 --- a/packages/backend/src/payment-method/ilp/spsp/routes.test.ts +++ b/packages/backend/src/payment-method/ilp/spsp/routes.test.ts @@ -1,5 +1,6 @@ import * as crypto from 'crypto' import { v4 as uuid } from 'uuid' +import assert from 'assert' import { AppServices, SPSPContext } from '../../../app' import { SPSPRoutes } from './routes' @@ -13,6 +14,7 @@ import { StreamServer } from '@interledger/stream-receiver' import { randomAsset } from '../../../tests/asset' import { createContext } from '../../../tests/context' import { truncateTables } from '../../../tests/tableManager' +import { SPSPRouteError } from './middleware' type SPSPHeader = { Accept: string @@ -76,17 +78,46 @@ describe('SPSP Routes', (): void => { const ctx = createContext({ headers: { Accept: 'application/json' } }) - await expect(spspRoutes.get(ctx)).rejects.toHaveProperty('status', 406) + + expect.assertions(2) + try { + await spspRoutes.get(ctx) + } catch (err) { + assert.ok(err instanceof SPSPRouteError) + expect(err.status).toBe(406) + expect(err.message).toBe( + 'Request does not support application/spsp4+json' + ) + } }) test('nonce, no secret; returns 400', async () => { const ctx = setup({ nonce }) - await expect(spspRoutes.get(ctx)).rejects.toHaveProperty('status', 400) + + expect.assertions(2) + try { + await spspRoutes.get(ctx) + } catch (err) { + assert.ok(err instanceof SPSPRouteError) + expect(err.status).toBe(400) + expect(err.message).toBe( + 'Failed to generate credentials: receipt nonce and secret must accompany each other' + ) + } }) test('secret; no nonce; returns 400', async () => { const ctx = setup({ secret }) - await expect(spspRoutes.get(ctx)).rejects.toHaveProperty('status', 400) + expect.assertions(2) + try { + await spspRoutes.get(ctx) + } catch (err) { + assert.ok(err instanceof SPSPRouteError) + expect(err.status).toBe(400) + expect(err.message).toBe( + 'Failed to generate credentials: receipt nonce and secret must accompany each other' + ) + } }) test('malformed nonce; returns 400', async () => { @@ -94,7 +125,17 @@ describe('SPSP Routes', (): void => { nonce: Buffer.alloc(15).toString('base64'), secret }) - await expect(spspRoutes.get(ctx)).rejects.toHaveProperty('status', 400) + + expect.assertions(2) + try { + await spspRoutes.get(ctx) + } catch (err) { + assert.ok(err instanceof SPSPRouteError) + expect(err.status).toBe(400) + expect(err.message).toBe( + 'Failed to generate credentials: receipt nonce must be 16 bytes' + ) + } }) const receiptSetup = { @@ -139,6 +180,25 @@ describe('SPSP Routes', (): void => { } ) + test('handle error when generating credentials', async () => { + const ctx = setup() + + jest + .spyOn(streamServer, 'generateCredentials') + .mockImplementationOnce(() => { + throw new Error('Could not generate credentials') + }) + + expect.assertions(2) + try { + await spspRoutes.get(ctx) + } catch (err) { + assert.ok(err instanceof SPSPRouteError) + expect(err.status).toBe(400) + expect(err.message).toBe('Could not generate credentials') + } + }) + /** * Utility functions */ diff --git a/packages/backend/src/payment-method/ilp/spsp/routes.ts b/packages/backend/src/payment-method/ilp/spsp/routes.ts index 5f77ff17d7..0f443703aa 100644 --- a/packages/backend/src/payment-method/ilp/spsp/routes.ts +++ b/packages/backend/src/payment-method/ilp/spsp/routes.ts @@ -2,6 +2,7 @@ import { BaseService } from '../../../shared/baseService' import { SPSPContext } from '../../../app' import base64url from 'base64url' import { StreamServer } from '@interledger/stream-receiver' +import { SPSPRouteError } from './middleware' const CONTENT_TYPE_V4 = 'application/spsp4+json' @@ -34,14 +35,19 @@ async function getSPSP( deps: ServiceDependencies, ctx: SPSPContext ): Promise { - ctx.assert(ctx.accepts(CONTENT_TYPE_V4), 406) + if (!ctx.accepts(CONTENT_TYPE_V4)) { + throw new SPSPRouteError(406, `Request does not support ${CONTENT_TYPE_V4}`) + } + const nonce = ctx.request.headers['receipt-nonce'] const secret = ctx.request.headers['receipt-secret'] - ctx.assert( - !nonce === !secret, - 400, - 'Failed to generate credentials: receipt nonce and secret must accompany each other' - ) + + if (!!nonce !== !!secret) { + throw new SPSPRouteError( + 400, + 'Failed to generate credentials: receipt nonce and secret must accompany each other' + ) + } try { const { ilpAddress, sharedSecret } = deps.streamServer.generateCredentials({ @@ -63,6 +69,10 @@ async function getSPSP( receipts_enabled: !!(nonce && secret) }) } catch (err) { - ctx.throw(400, err instanceof Error && err.message) + const errorMessage = + err instanceof Error ? err.message : 'Could not generate SPSP credentials' + throw new SPSPRouteError(400, errorMessage, { + paymentTag: ctx.paymentTag + }) } } From 18a4ba5032598795a3dffbc36e0e6760696ba610 Mon Sep 17 00:00:00 2001 From: Max Kurapov Date: Tue, 7 May 2024 18:10:14 +0200 Subject: [PATCH 05/10] chore(backend): use getOrPollByUrl in SPSP middleware --- .../ilp/spsp/middleware.test.ts | 25 +------------------ .../src/payment-method/ilp/spsp/middleware.ts | 4 +-- 2 files changed, 3 insertions(+), 26 deletions(-) diff --git a/packages/backend/src/payment-method/ilp/spsp/middleware.test.ts b/packages/backend/src/payment-method/ilp/spsp/middleware.test.ts index a2a655e041..c249651d7e 100644 --- a/packages/backend/src/payment-method/ilp/spsp/middleware.test.ts +++ b/packages/backend/src/payment-method/ilp/spsp/middleware.test.ts @@ -89,7 +89,7 @@ describe('SPSP Middleware', (): void => { const spspGetRouteSpy = jest.spyOn(spspRoutes, 'get') jest - .spyOn(walletAddressService, 'getByUrl') + .spyOn(walletAddressService, 'getOrPollByUrl') .mockResolvedValueOnce(undefined) ctx.header['accept'] = 'application/spsp4+json' @@ -107,27 +107,4 @@ describe('SPSP Middleware', (): void => { expect(spspGetRouteSpy).not.toHaveBeenCalled() } }) - - test('throws error if inactive wallet address', async () => { - const spspGetRouteSpy = jest.spyOn(spspRoutes, 'get') - - ctx.header['accept'] = 'application/spsp4+json' - - await walletAddress - .$query(appContainer.knex) - .patch({ deactivatedAt: new Date() }) - - const spspMiddleware = createSpspMiddleware(true) - - expect.assertions(4) - try { - await spspMiddleware(ctx, next) - } catch (err) { - assert.ok(err instanceof SPSPRouteError) - expect(err.status).toBe(404) - expect(err.message).toBe('Could not get wallet address') - expect(next).not.toHaveBeenCalled() - expect(spspGetRouteSpy).not.toHaveBeenCalled() - } - }) }) diff --git a/packages/backend/src/payment-method/ilp/spsp/middleware.ts b/packages/backend/src/payment-method/ilp/spsp/middleware.ts index 0a87da70cc..17bd816bef 100644 --- a/packages/backend/src/payment-method/ilp/spsp/middleware.ts +++ b/packages/backend/src/payment-method/ilp/spsp/middleware.ts @@ -38,11 +38,11 @@ const spspMiddleware = async ( if (ctx.accepts('application/spsp4+json')) { const walletAddressService = await ctx.container.use('walletAddressService') - const walletAddress = await walletAddressService.getByUrl( + const walletAddress = await walletAddressService.getOrPollByUrl( ctx.walletAddressUrl ) - if (!walletAddress?.isActive) { + if (!walletAddress) { throw new SPSPRouteError(404, 'Could not get wallet address') } From 930743732667f8bc6b372d366d39f2ed7f45278c Mon Sep 17 00:00:00 2001 From: Max Kurapov Date: Tue, 7 May 2024 18:21:20 +0200 Subject: [PATCH 06/10] chore(backend): update log during wallet address polling --- packages/backend/src/open_payments/wallet_address/service.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/open_payments/wallet_address/service.ts b/packages/backend/src/open_payments/wallet_address/service.ts index b9c06e677a..5be44b7f24 100644 --- a/packages/backend/src/open_payments/wallet_address/service.ts +++ b/packages/backend/src/open_payments/wallet_address/service.ts @@ -213,8 +213,8 @@ async function getOrPollByUrl( return checkActiveWalletAddress(walletAddress, options) } catch (error) { - const errorMessage = 'Could not get wallet address' - deps.logger.error( + const errorMessage = 'Could not get wallet address after polling' + deps.logger.info( { errorMessage: error instanceof Error && error.message }, errorMessage ) From 62b3e9ad489027c0c2074a928d449daa60711f97 Mon Sep 17 00:00:00 2001 From: Max Kurapov Date: Tue, 7 May 2024 18:54:59 +0200 Subject: [PATCH 07/10] chore(backend): minor cleanup in app.ts --- packages/backend/src/app.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/backend/src/app.ts b/packages/backend/src/app.ts index 002702f057..1ad7ec94ef 100644 --- a/packages/backend/src/app.ts +++ b/packages/backend/src/app.ts @@ -133,7 +133,7 @@ export type HttpSigWithAuthenticatedStatusContext = HttpSigContext & AuthenticatedStatusContext // Wallet address subresources -type GetCollectionQuery = { +interface GetCollectionQuery { 'wallet-address': string } @@ -448,7 +448,7 @@ export class App { >( '/incoming-payments', async (ctx, next) => { - ctx.walletAddressUrl = ctx.query['wallet-address'] as string + ctx.walletAddressUrl = ctx.request.query['wallet-address'] await next() }, createValidatorMiddleware< @@ -503,7 +503,7 @@ export class App { >( '/outgoing-payments', async (ctx, next) => { - ctx.walletAddressUrl = ctx.query['wallet-address'] as string + ctx.walletAddressUrl = ctx.request.query['wallet-address'] await next() }, createValidatorMiddleware< @@ -564,7 +564,7 @@ export class App { if (!incomingPayment?.walletAddress) { throw new OpenPaymentsServerRouteError(401, 'Unauthorized', { - description: 'Failed to get wallet address from incomig payment', + description: 'Failed to get wallet address from incoming payment', id: ctx.params.id }) } From 67259372fe1118a0981d2ceea069524d2b508f39 Mon Sep 17 00:00:00 2001 From: Max Kurapov Date: Tue, 7 May 2024 19:16:42 +0200 Subject: [PATCH 08/10] chore(backend): convert error log to debug statement in getOrPollByUrl --- .../src/open_payments/wallet_address/service.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/backend/src/open_payments/wallet_address/service.ts b/packages/backend/src/open_payments/wallet_address/service.ts index 5be44b7f24..96a39f8630 100644 --- a/packages/backend/src/open_payments/wallet_address/service.ts +++ b/packages/backend/src/open_payments/wallet_address/service.ts @@ -197,6 +197,11 @@ async function getOrPollByUrl( return checkActiveWalletAddress(existingWalletAddress, options) } + deps.logger.debug( + { walletAddressUrl: url }, + 'Wallet address not found, polling to see if ASE creates wallet address' + ) + await WalletAddressEvent.query(deps.knex).insert({ type: WalletAddressEventType.WalletAddressNotFound, data: { @@ -213,12 +218,6 @@ async function getOrPollByUrl( return checkActiveWalletAddress(walletAddress, options) } catch (error) { - const errorMessage = 'Could not get wallet address after polling' - deps.logger.info( - { errorMessage: error instanceof Error && error.message }, - errorMessage - ) - return undefined } } From d657a8de029feb5ca96964234f3a1b7ae43c6eb7 Mon Sep 17 00:00:00 2001 From: Max Kurapov Date: Tue, 7 May 2024 19:16:58 +0200 Subject: [PATCH 09/10] chore(backend): updating wallet address get route --- .../wallet_address/routes.test.ts | 22 +++++++++++++++---- .../open_payments/wallet_address/routes.ts | 2 +- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/packages/backend/src/open_payments/wallet_address/routes.test.ts b/packages/backend/src/open_payments/wallet_address/routes.test.ts index 29a6008f78..e5286a81c2 100644 --- a/packages/backend/src/open_payments/wallet_address/routes.test.ts +++ b/packages/backend/src/open_payments/wallet_address/routes.test.ts @@ -9,12 +9,16 @@ import { createContext } from '../../tests/context' import { createWalletAddress } from '../../tests/walletAddress' import { truncateTables } from '../../tests/tableManager' import { WalletAddressRoutes } from './routes' +import { WalletAddressService } from './service' +import assert from 'assert' +import { OpenPaymentsServerRouteError } from '../route-errors' describe('Wallet Address Routes', (): void => { let deps: IocContract let appContainer: TestContainer let config: IAppConfig let walletAddressRoutes: WalletAddressRoutes + let walletAddressService: WalletAddressService beforeAll(async (): Promise => { config = Config @@ -22,6 +26,7 @@ describe('Wallet Address Routes', (): void => { deps = await initIocContainer(config) appContainer = await createTestApp(deps) const { walletAddressServerSpec } = await deps.use('openApi') + walletAddressService = await deps.use('walletAddressService') jestOpenAPI(walletAddressServerSpec) }) @@ -43,10 +48,19 @@ describe('Wallet Address Routes', (): void => { const ctx = createContext({ headers: { Accept: 'application/json' } }) - await expect(walletAddressRoutes.get(ctx)).rejects.toHaveProperty( - 'status', - 404 - ) + jest + .spyOn(walletAddressService, 'getOrPollByUrl') + .mockResolvedValueOnce(undefined) + + expect.assertions(2) + + try { + await walletAddressRoutes.get(ctx) + } catch (err) { + assert.ok(err instanceof OpenPaymentsServerRouteError) + expect(err.status).toBe(404) + expect(err.message).toBe('Could not get wallet address') + } }) test('returns 200 with an open payments wallet address', async (): Promise => { diff --git a/packages/backend/src/open_payments/wallet_address/routes.ts b/packages/backend/src/open_payments/wallet_address/routes.ts index 4bf146f0d2..2fba82f15d 100644 --- a/packages/backend/src/open_payments/wallet_address/routes.ts +++ b/packages/backend/src/open_payments/wallet_address/routes.ts @@ -40,7 +40,7 @@ export async function getWalletAddress( if (!walletAddress) { throw new OpenPaymentsServerRouteError( 404, - 'Wallet address does not exist', + 'Could not get wallet address', { walletAddressUrl: ctx.walletAddressUrl } From 20820157a05f52462d81d4ee0f1376afb7776449 Mon Sep 17 00:00:00 2001 From: Max Kurapov Date: Fri, 10 May 2024 20:33:57 +0200 Subject: [PATCH 10/10] chore(backend): remove unused walletAddress arg --- .../backend/src/open_payments/wallet_address/routes.ts | 3 +-- packages/backend/src/shared/pagination.test.ts | 9 +++------ packages/backend/src/shared/pagination.ts | 4 ---- 3 files changed, 4 insertions(+), 12 deletions(-) diff --git a/packages/backend/src/open_payments/wallet_address/routes.ts b/packages/backend/src/open_payments/wallet_address/routes.ts index 2fba82f15d..4b3831ebb2 100644 --- a/packages/backend/src/open_payments/wallet_address/routes.ts +++ b/packages/backend/src/open_payments/wallet_address/routes.ts @@ -93,8 +93,7 @@ export const listSubresource = async ({ pagination, client }), - page, - walletAddress: walletAddress.url + page }) const result = { pagination: pageInfo, diff --git a/packages/backend/src/shared/pagination.test.ts b/packages/backend/src/shared/pagination.test.ts index 18be4c5541..c0d65a0499 100644 --- a/packages/backend/src/shared/pagination.test.ts +++ b/packages/backend/src/shared/pagination.test.ts @@ -137,8 +137,7 @@ describe('Pagination', (): void => { walletAddressId: defaultWalletAddress.id, pagination }), - page, - walletAddress: defaultWalletAddress.url + page }) expect(pageInfo).toEqual({ startCursor: paymentIds[start], @@ -194,8 +193,7 @@ describe('Pagination', (): void => { walletAddressId: defaultWalletAddress.id, pagination }), - page, - walletAddress: defaultWalletAddress.url + page }) expect(pageInfo).toEqual({ startCursor: paymentIds[start], @@ -251,8 +249,7 @@ describe('Pagination', (): void => { walletAddressId: defaultWalletAddress.id, pagination }), - page, - walletAddress: defaultWalletAddress.url + page }) expect(pageInfo).toEqual({ startCursor: quoteIds[start], diff --git a/packages/backend/src/shared/pagination.ts b/packages/backend/src/shared/pagination.ts index 74dcb59f87..e9a1061890 100644 --- a/packages/backend/src/shared/pagination.ts +++ b/packages/backend/src/shared/pagination.ts @@ -20,14 +20,12 @@ export function parsePaginationQueryParameters({ type GetPageInfoArgs = { getPage: (pagination: Pagination, sortOrder?: SortOrder) => Promise page: T[] - walletAddress?: string sortOrder?: SortOrder } export async function getPageInfo({ getPage, page, - walletAddress, sortOrder }: GetPageInfoArgs): Promise { if (page.length == 0) @@ -43,7 +41,6 @@ export async function getPageInfo({ try { hasNextPage = await getPage( { - walletAddress, after: lastId, first: 1 }, @@ -55,7 +52,6 @@ export async function getPageInfo({ try { hasPreviousPage = await getPage( { - walletAddress, before: firstId, last: 1 },