Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(backend): remove payment pointer from url, retrieve it from request query or body #2042

Merged
merged 6 commits into from
Oct 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -40,27 +40,21 @@ exports.up = function (knex) {
table.dropIndex(['paymentPointerId', 'createdAt', 'id'])
table.renameColumn('paymentPointerId', 'walletAddressId')
table.foreign('walletAddressId').references('walletAddresses.id')
table.index(['walletAddressId'])
table.index(['createdAt'])
table.index(['id'])
table.index(['walletAddressId', 'createdAt', 'id'])
}),
knex.schema.alterTable('incomingPayments', function (table) {
table.dropForeign(['paymentPointerId'])
table.dropIndex(['paymentPointerId', 'createdAt', 'id'])
table.renameColumn('paymentPointerId', 'walletAddressId')
table.foreign('walletAddressId').references('walletAddresses.id')
table.index(['walletAddressId'])
table.index(['createdAt'])
table.index(['id'])
table.index(['walletAddressId', 'createdAt', 'id'])
}),
knex.schema.alterTable('outgoingPayments', function (table) {
table.dropForeign(['paymentPointerId'])
table.dropIndex(['paymentPointerId', 'createdAt', 'id'])
table.renameColumn('paymentPointerId', 'walletAddressId')
table.foreign('walletAddressId').references('walletAddresses.id')
table.index(['walletAddressId'])
table.index(['createdAt'])
table.index(['id'])
table.index(['walletAddressId', 'createdAt', 'id'])
}),
knex('webhookEvents')
.update({
Expand Down Expand Up @@ -113,7 +107,7 @@ exports.down = function (knex) {
'ALTER INDEX "walletAddresses_pkey" RENAME TO "paymentPointers_pkey"'
),
knex.raw(
'ALTER INDEX "walletAddresses_url_index" RENAME TO "paymentPointers_url_index"'
'ALTER INDEX "walletaddresses_url_index" RENAME TO "paymentpointers_url_index"'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixing some typos in the migration

),
knex.raw(
'ALTER INDEX "walletaddresses_processat_index" RENAME TO "paymentpointers_processat_index"'
Expand All @@ -122,19 +116,18 @@ exports.down = function (knex) {
'ALTER TABLE "paymentPointers" DROP CONSTRAINT "walletaddresses_url_unique"'
),
knex.raw(
'ALTER TABLE "paymentPointers" DROP CONSTRAINT "walletaddreses_assetid_foreign"'
'ALTER TABLE "paymentPointers" DROP CONSTRAINT "walletaddresses_assetid_foreign"'
),
knex.schema.renameTable('walletAddressKeys', 'paymentPointerKeys'),
knex.schema.alterTable('paymentPointerKeys', function (table) {
table.dropForeign(['walletAddressId'])
table.renameColumn('walletAddressId', 'paymentPointerId')
table.foreign('paymentPointerId').references('paymentPointers.id')
}),
knex.raw(
'ALTER INDEX "walletAddressKeys_pkey" RENAME TO "paymentPointerKeys_pkey"'
),
knex.raw(
'ALTER TABLE "paymentpointerKeys" DROP CONSTRAINT "walletaddresskeys_paymentpointerid_foreign"'
'ALTER TABLE "paymentPointerKeys" DROP CONSTRAINT "walletaddresskeys_walletaddressid_foreign"'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is walletaddresskeys_walletaddressid_foreign?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a foreign key on the table walletAddressKeys that links it to walletAddress via its id property. The line referenced here is the rollback for this migration, so it's part of reverting the foreign key from referring to wallet addresses to payment pointers.

),
knex.schema.alterTable('quotes', function (table) {
table.dropForeign(['walletAddressId'])
Expand Down
2 changes: 1 addition & 1 deletion packages/backend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
"@graphql-tools/load": "^8.0.0",
"@graphql-tools/schema": "^10.0.0",
"@interledger/http-signature-utils": "1.1.0",
"@interledger/open-payments": "5.0.0",
"@interledger/open-payments": "5.2.0",
"@interledger/openapi": "1.0.3",
"@interledger/pay": "0.4.0-alpha.9",
"@interledger/stream-receiver": "^0.3.3-alpha.3",
Expand Down
63 changes: 34 additions & 29 deletions packages/backend/src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,14 @@ export type HttpSigContext = AppContext & {
client: string
}

// Wallet address subresources
export type HttpSigWithAuthenticatedStatusContext = HttpSigContext &
AuthenticatedStatusContext

// Wallet address subresources
interface GetCollectionQuery {
'wallet-address': string
}

type CollectionRequest<BodyT = never, QueryT = ParsedUrlQuery> = Omit<
WalletAddressContext['request'],
'body'
Expand Down Expand Up @@ -371,7 +375,7 @@ export class App {
// POST /incoming-payments
// Create incoming payment
router.post<DefaultState, SignedCollectionContext<IncomingCreateBody>>(
WALLET_ADDRESS_PATH + '/incoming-payments',
'/incoming-payments',
createWalletAddressMiddleware(),
createValidatorMiddleware<
ContextType<SignedCollectionContext<IncomingCreateBody>>
Expand All @@ -389,16 +393,18 @@ export class App {

// GET /incoming-payments
// List incoming payments
router.get<DefaultState, SignedCollectionContext>(
WALLET_ADDRESS_PATH + '/incoming-payments',
router.get<
DefaultState,
SignedCollectionContext<never, GetCollectionQuery>
>(
'/incoming-payments',
createWalletAddressMiddleware(),
createValidatorMiddleware<ContextType<SignedCollectionContext>>(
resourceServerSpec,
{
path: '/incoming-payments',
method: HttpMethod.GET
}
),
createValidatorMiddleware<
ContextType<SignedCollectionContext<never, GetCollectionQuery>>
>(resourceServerSpec, {
path: '/incoming-payments',
method: HttpMethod.GET
}),
createTokenIntrospectionMiddleware({
requestType: AccessType.IncomingPayment,
requestAction: RequestAction.List
Expand All @@ -410,7 +416,7 @@ export class App {
// POST /outgoing-payment
// Create outgoing payment
router.post<DefaultState, SignedCollectionContext<OutgoingCreateBody>>(
WALLET_ADDRESS_PATH + '/outgoing-payments',
'/outgoing-payments',
createWalletAddressMiddleware(),
createValidatorMiddleware<
ContextType<SignedCollectionContext<OutgoingCreateBody>>
Expand All @@ -428,16 +434,18 @@ export class App {

// GET /outgoing-payment
// List outgoing payments
router.get<DefaultState, SignedCollectionContext>(
WALLET_ADDRESS_PATH + '/outgoing-payments',
router.get<
DefaultState,
SignedCollectionContext<never, GetCollectionQuery>
>(
'/outgoing-payments',
createWalletAddressMiddleware(),
createValidatorMiddleware<ContextType<SignedCollectionContext>>(
resourceServerSpec,
{
path: '/outgoing-payments',
method: HttpMethod.GET
}
),
createValidatorMiddleware<
ContextType<SignedCollectionContext<never, GetCollectionQuery>>
>(resourceServerSpec, {
path: '/outgoing-payments',
method: HttpMethod.GET
}),
createTokenIntrospectionMiddleware({
requestType: AccessType.OutgoingPayment,
requestAction: RequestAction.List
Expand All @@ -449,7 +457,7 @@ export class App {
// POST /quotes
// Create quote
router.post<DefaultState, SignedCollectionContext<QuoteCreateBody>>(
WALLET_ADDRESS_PATH + '/quotes',
'/quotes',
createWalletAddressMiddleware(),
createValidatorMiddleware<
ContextType<SignedCollectionContext<QuoteCreateBody>>
Expand All @@ -468,8 +476,7 @@ export class App {
// GET /incoming-payments/{id}
// Read incoming payment
router.get<DefaultState, SubresourceContextWithAuthenticatedStatus>(
WALLET_ADDRESS_PATH + '/incoming-payments/:id',
createWalletAddressMiddleware(),
'/incoming-payments/:id',
createValidatorMiddleware<
ContextType<SubresourceContextWithAuthenticatedStatus>
>(resourceServerSpec, {
Expand All @@ -488,8 +495,7 @@ export class App {
// POST /incoming-payments/{id}/complete
// Complete incoming payment
router.post<DefaultState, SignedSubresourceContext>(
WALLET_ADDRESS_PATH + '/incoming-payments/:id/complete',
createWalletAddressMiddleware(),
'/incoming-payments/:id/complete',
createValidatorMiddleware<ContextType<SignedSubresourceContext>>(
resourceServerSpec,
{
Expand All @@ -508,8 +514,7 @@ export class App {
// GET /outgoing-payments/{id}
// Read outgoing payment
router.get<DefaultState, SignedSubresourceContext>(
WALLET_ADDRESS_PATH + '/outgoing-payments/:id',
createWalletAddressMiddleware(),
'/outgoing-payments/:id',
createValidatorMiddleware<ContextType<SignedSubresourceContext>>(
resourceServerSpec,
{
Expand All @@ -528,7 +533,7 @@ export class App {
// GET /quotes/{id}
// Read quote
router.get<DefaultState, SignedSubresourceContext>(
WALLET_ADDRESS_PATH + '/quotes/:id',
'/quotes/:id',
createWalletAddressMiddleware(),
createValidatorMiddleware<ContextType<SignedSubresourceContext>>(
resourceServerSpec,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ describe('Incoming Payment Routes', (): void => {
describe('get unauthenticated incoming payment', (): void => {
test('Can get incoming payment with public fields', async (): Promise<void> => {
const incomingPayment = await createIncomingPayment(deps, {
paymentPointerId: paymentPointer.id,
walletAddressId: walletAddress.id,
expiresAt,
incomingAmount,
metadata
Expand All @@ -312,7 +312,7 @@ describe('Incoming Payment Routes', (): void => {
params: {
id: incomingPayment.id
},
paymentPointer
walletAddress
})
ctx.authenticated = false

Expand Down
20 changes: 15 additions & 5 deletions packages/backend/src/open_payments/payment/incoming/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ async function getIncomingPaymentPublic(
const incomingPayment = await deps.incomingPaymentService.get({
id: ctx.params.id,
client: ctx.accessAction === AccessAction.Read ? ctx.client : undefined,
paymentPointerId: ctx.paymentPointer.id
walletAddressId: ctx.walletAddress.id
})
ctx.body = incomingPayment?.toPublicOpenPaymentsType()
} catch (err) {
Expand All @@ -105,16 +105,17 @@ async function getIncomingPaymentPrivate(
} catch (err) {
ctx.throw(500, 'Error trying to get incoming payment')
}
if (!incomingPayment) return ctx.throw(404)
if (!incomingPayment || !incomingPayment.walletAddress) return ctx.throw(404)
const connection = deps.connectionService.get(incomingPayment)
ctx.body = incomingPaymentToBody(
ctx.walletAddress,
incomingPayment.walletAddress,
incomingPayment,
connection
)
Comment on lines +108 to 114
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming the connection part is going to be removed in a subsequent PR from @BlairCurrey ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct.

Copy link
Contributor

@BlairCurrey BlairCurrey Oct 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part will actually still be here but with a paired down connection type. After talking w/ Max our plan was to leave the connection service because we still need to get credentials from the stream server. So we will just have ConnectionService.get that returns something like this (and getting rid of the current Connection model):

{
  ilpAddress: IlpAddress
  sharedSecret: Buffer
}

So internally we still have some concept of connections but they are basically just the stream credentials. I could probably see a case for putting this somewhere in IncomingPaymentsService or the model but for now this is the simpler refactor.

EDIT: we have since decided to change this to rename this to streamCredentialService

}

export type CreateBody = {
walletAddress: string
expiresAt?: string
incomingAmount?: AmountJSON
metadata?: Record<string, unknown>
Expand Down Expand Up @@ -146,10 +147,14 @@ async function createIncomingPayment(
)
}

if (!incomingPaymentOrError.walletAddress) {
ctx.throw(404)
}

ctx.status = 201
const connection = deps.connectionService.get(incomingPaymentOrError)
ctx.body = incomingPaymentToBody(
ctx.walletAddress,
incomingPaymentOrError.walletAddress,
incomingPaymentOrError,
connection
)
Expand All @@ -174,7 +179,12 @@ async function completeIncomingPayment(
errorToMessage[incomingPaymentOrError]
)
}
ctx.body = incomingPaymentToBody(ctx.walletAddress, incomingPaymentOrError)

if (!incomingPaymentOrError.walletAddress) {
ctx.throw(404)
}

ctx.body = incomingPaymentToBody(incomingPaymentOrError.walletAddress, incomingPaymentOrError)
}

async function listIncomingPayments(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,11 @@ async function create(
try {
return await deps.openPaymentsClient.incomingPayment.create(
{
walletAddress: walletAddressUrl,
url: walletAddressUrl,
accessToken: grantOrError.accessToken
},
{
walletAddress: walletAddressUrl,
incomingAmount: args.incomingAmount
? serializeAmount(args.incomingAmount)
: undefined,
Expand Down
11 changes: 7 additions & 4 deletions packages/backend/src/open_payments/payment/outgoing/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,17 +47,20 @@ async function getOutgoingPayment(
try {
outgoingPayment = await deps.outgoingPaymentService.get({
id: ctx.params.id,
client: ctx.accessAction === AccessAction.Read ? ctx.client : undefined,
walletAddressId: ctx.walletAddress.id
client: ctx.accessAction === AccessAction.Read ? ctx.client : undefined
})
} catch (_) {
ctx.throw(500, 'Error trying to get outgoing payment')
}
if (!outgoingPayment) return ctx.throw(404)
ctx.body = outgoingPaymentToBody(ctx.walletAddress, outgoingPayment)
if (!outgoingPayment || !outgoingPayment.walletAddress) return ctx.throw(404)
ctx.body = outgoingPaymentToBody(
outgoingPayment.walletAddress,
outgoingPayment
)
}

export type CreateBody = {
walletAddress: string
quoteId: string
metadata?: Record<string, unknown>
}
Expand Down
7 changes: 3 additions & 4 deletions packages/backend/src/open_payments/quote/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,10 @@ async function getQuote(
): Promise<void> {
const quote = await deps.quoteService.get({
id: ctx.params.id,
client: ctx.accessAction === AccessAction.Read ? ctx.client : undefined,
walletAddressId: ctx.walletAddress.id
client: ctx.accessAction === AccessAction.Read ? ctx.client : undefined
})
if (!quote) return ctx.throw(404)
ctx.body = quoteToBody(ctx.walletAddress, quote)
if (!quote || !quote.walletAddress) return ctx.throw(404)
ctx.body = quoteToBody(quote.walletAddress, quote)
}

interface CreateBodyBase {
Expand Down
19 changes: 18 additions & 1 deletion packages/backend/src/open_payments/wallet_address/middleware.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,28 @@
import { AppContext } from '../../app'
import { CreateBody as IncomingCreateBody } from '../../open_payments/payment/incoming/routes'
import { CreateBody as OutgoingCreateBody } from '../../open_payments/payment/outgoing/routes'

type CreateBody = IncomingCreateBody | OutgoingCreateBody

export function createWalletAddressMiddleware() {
return async (
ctx: AppContext,
next: () => Promise<unknown>
): Promise<void> => {
ctx.walletAddressUrl = `https://${ctx.request.host}/${ctx.params.walletAddressPath}`
if (
ctx.path === '/incoming-payments' ||
ctx.path === '/outgoing-payments'
) {
if (ctx.method === 'GET') {
ctx.walletAddressUrl = ctx.query['wallet-address'] as string
} else if (ctx.method === 'POST') {
ctx.walletAddressUrl = (ctx.request.body as CreateBody).walletAddress
} else {
ctx.throw(401)
}
} else {
ctx.walletAddressUrl = `https://${ctx.request.host}/${ctx.params.walletAddressPath}`
}
const config = await ctx.container.use('config')
if (ctx.walletAddressUrl !== config.walletAddressUrl) {
const walletAddressService = await ctx.container.use(
Expand Down
4 changes: 3 additions & 1 deletion packages/backend/src/shared/pagination.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { PaginationArgs } from '@interledger/open-payments'
import { PaginationArgs as OpenPaymentsPaginationArgs } from '@interledger/open-payments'

import { BaseModel, PageInfo, Pagination } from './baseModel'

type PaginationArgs = Omit<OpenPaymentsPaginationArgs, 'wallet-address'>

export function parsePaginationQueryParameters({
first,
last,
Expand Down
Loading