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: update receiver regex validation #430

Merged
merged 6 commits into from
Mar 1, 2024
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
5 changes: 5 additions & 0 deletions .changeset/fuzzy-rivers-jog.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@interledger/open-payments': minor
---

Widened receiver regex to accept http and any id format. Removed previously deprecated 'connections' path.
2 changes: 1 addition & 1 deletion openapi/auth-server.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ paths:
- read
identifier: 'https://ilp.rafiki.money/alice'
limits:
receiver: 'https://ilp.rafiki.money/connections/45a0d0ee-26dc-4c66-89e0-01fbf93156f7'
receiver: 'https://ilp.rafiki.money/incoming-payments/45a0d0ee-26dc-4c66-89e0-01fbf93156f7'
interval: 'R12/2019-08-24T14:15:22Z/P1M'
debitAmount:
value: '500'
Expand Down
7 changes: 4 additions & 3 deletions openapi/schemas.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,13 @@ components:
receiver:
title: Receiver
type: string
description: The URL of the incoming payment or ILP STREAM connection that is being paid.
description: The URL of the incoming payment that is being paid.
format: uri
pattern: '^https://(.+)/(incoming-payments|connections)/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$'
pattern: '^(https|http)://(.+)/incoming-payments/(.+)$'
Copy link
Contributor Author

@BlairCurrey BlairCurrey Feb 22, 2024

Choose a reason for hiding this comment

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

As for the id change:

  1. figured it was correct to loosen it to any sort of id (not necessarily uuid) after discussing with Max. Thinking was that while we use uuid's in rafiki this is really a detail which might vary from implementation to implementation and it shouldn't (I think) matter what sort of id scheme the api implementation uses. More opinions welcome.

  2. I changed it to match anything but perhaps chars/numbers only would be better? I just wasn't sure about forbidding the use of query variables and such. While we don't support anything like this I don't know that we should have an opinion against it: incoming-payments/123?something=1

Copy link
Contributor

Choose a reason for hiding this comment

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

I think matching anything is fine in this case

examples:
- 'https://ilp.rafiki.money/incoming-payments/08394f02-7b7b-45e2-b645-51d04e7c330c'
- 'https://ilp.rafiki.money/connections/016da9d5-c9a4-4c80-a354-86b915a04ff8'
- 'http://ilp.rafiki.money/incoming-payments/08394f02-7b7b-45e2-b645-51d04e7c330c'
- 'https://ilp.rafiki.money/incoming-payments/1'
walletAddress:
title: Wallet Address
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ export interface external {
/**
* Receiver
* Format: uri
* @description The URL of the incoming payment or ILP STREAM connection that is being paid.
* @description The URL of the incoming payment that is being paid.
*/
receiver: string;
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,7 @@ export interface external {
/**
* Receiver
* Format: uri
* @description The URL of the incoming payment or ILP STREAM connection that is being paid.
* @description The URL of the incoming payment that is being paid.
*/
receiver: string;
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ export interface external {
/**
* Receiver
* Format: uri
* @description The URL of the incoming payment or ILP STREAM connection that is being paid.
* @description The URL of the incoming payment that is being paid.
*/
receiver: string;
/**
Expand Down
98 changes: 98 additions & 0 deletions packages/openapi/src/middleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,104 @@ describe('OpenAPI Validator', (): void => {
expect(next).toHaveBeenCalled()
}
)

describe('Quote', (): void => {
let validateQuotePostMiddleware: AppMiddleware

beforeAll((): void => {
validateQuotePostMiddleware = createValidatorMiddleware(openApi, {
path: '/quotes',
method: HttpMethod.POST
})
})

test.each`
body | description
${{ receiver: 'ht999tp://something.com/incoming-payments' }} | ${'invalid receiver, unknown protocol'}
${{ receiver: 'http://something.com/incoming-payments' }} | ${'invalid receiver, missing incoming payment id'}
${{ receiver: 'http://something.com/connections/c3a0d182-b221-4612-a500-07ad106b5f5d' }} | ${'invalid receiver, wrong path'}
`(
'returns 400 on invalid quote body ($description)',
async ({ body }): Promise<void> => {
const ctx = createContext(
{
headers: {
Accept: 'application/json',
'Content-Type': 'application/json'
}
},
{}
)
addTestSignatureHeaders(ctx)
ctx.request.body = {
...body,
walletAddress: WALLET_ADDRESS,
method: 'ilp'
}
await expect(
validateQuotePostMiddleware(ctx, next)
).rejects.toMatchObject({
status: 400,
message:
'body.receiver must match pattern "^(https|http):..(.+).incoming-payments.(.+)$"'
})
expect(next).not.toHaveBeenCalled()
}
)
test.each`
receiver | description
${'http://something.com/incoming-payments/c3a0d182-b221-4612-a500-07ad106b5f5d'} | ${'accepts http and uuid'}
${'https://something.com/incoming-payments/123'} | ${'accepts http and non uuid id format'}
`(
'calls next on valid request: ($description)',
async ({ receiver }): Promise<void> => {
const ctx = createContext(
{
headers: {
Accept: 'application/json',
'Content-Type': 'application/json'
},
url: '/quotes'
},
{}
)
addTestSignatureHeaders(ctx)
ctx.request.body = {
receiver,
walletAddress: WALLET_ADDRESS,
method: 'ilp'
}
const next = jest.fn().mockImplementation(() => {
expect(ctx.request.body.receiver).toEqual(receiver)
ctx.status = 201
ctx.response.body = {
id: 'https://something-else/quotes/3b461206-daae-4d97-88b0-abffbcaa6f96',
walletAddress: 'https://something-else/accounts/someone',
receiveAmount: {
value: '100',
assetCode: 'USD',
assetScale: 2
},
debitAmount: {
value: '205',
assetCode: 'USD',
assetScale: 2
},
receiver,
expiresAt: '2024-02-28T16:26:32.444Z',
createdAt: '2024-02-28T16:21:32.444Z',
method: 'ilp'
}
})

await expect(
validateQuotePostMiddleware(ctx, next)
).resolves.toBeUndefined()

expect(next).toHaveBeenCalled()
}
)
})
})
})

Expand Down
Loading