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): unique keys per wallet address #2863

Open
wants to merge 32 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
c0e4574
feat(backend): make keys unique
sabineschaller Aug 1, 2024
8378b0b
fix: only make keys unique per wallet address
sabineschaller Aug 15, 2024
705bf44
fix(frontend): It is ambiguous on what scale is the withdrawal and de…
Emanuel-Palestino Jul 31, 2024
ba203bc
chore: sync docs and readmes (#2830)
JoblersTune Jul 31, 2024
a5f953a
chore(deps): update dependency @apollo/client to ^3.11.2 (#2822)
renovate[bot] Aug 1, 2024
612f671
feat(frontend): ux improvements to liquidity dialog component (#2839)
Emanuel-Palestino Aug 5, 2024
7ef4d44
feat(docker): switch to alpine3.19 (#2842)
golobitch Aug 6, 2024
60a9148
fix(auth): interact redirect (#2832)
sabineschaller Aug 7, 2024
c3671dd
feat(interaction): return grantId (#2843)
golobitch Aug 7, 2024
d2ef6b8
feat(outgoing-payment): add grantId to admin api (#2841)
golobitch Aug 7, 2024
8f3c147
feat(auth): soft delete access tokens and grant accesses (#2837)
njlie Aug 8, 2024
3dbd870
feat(auth): set session expiry based on interaction expiry env (#2851)
njlie Aug 9, 2024
aea3361
feat(localenv): span metrics generation (#2849)
BlairCurrey Aug 9, 2024
ef8f2da
chore(deps): update dependency @types/node to ^20.14.15 (#2838)
renovate[bot] Aug 13, 2024
2bf62f3
chore(deps): update dependency @apollo/client to ^3.11.4 (#2845)
renovate[bot] Aug 13, 2024
af91a0d
feat(2737): add fees as metric for outgoing payment. (#2831)
koekiebox Aug 14, 2024
31bf57c
refactor(dependencies): axios to 1.7.4 (#2861)
golobitch Aug 15, 2024
f01fd23
chore: add tests and better error handling
sabineschaller Aug 15, 2024
6b43cef
chore: formatting
sabineschaller Aug 15, 2024
fdaf29b
Merge branch 'main' into s2-unique-key-upload
sabineschaller Aug 15, 2024
5e81569
fix: build
sabineschaller Aug 15, 2024
0653ff9
fix: add camelcase quotes and make `up` async
sabineschaller Aug 16, 2024
56aba1e
chore: keep latest version of key
sabineschaller Aug 16, 2024
83664de
fix: formatting
sabineschaller Aug 16, 2024
7aa3c12
Updated branch
oana-lolea Nov 19, 2024
6a75ee9
Added unrevoke wallet address key query resolver
oana-lolea Nov 20, 2024
78be128
Updated migration and removed unrevoked resolver
oana-lolea Nov 27, 2024
d166800
Checkstyle fix
oana-lolea Nov 27, 2024
2b2b34e
Updated walletAddressKey deletion migration, removed unnecessary test…
oana-lolea Dec 5, 2024
dd6353c
Use ctid instead of createdAt to determine which rows are deleted
oana-lolea Dec 10, 2024
6161e06
Fixed typo
oana-lolea Dec 10, 2024
4d4d980
Delete the least recent rows that have the same kid and are unrevoked
oana-lolea Dec 11, 2024
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
@@ -0,0 +1,36 @@
/**
* @param { import("knex").Knex } knex
* @returns { Promise<void> }
*/
exports.up = async function (knex) {
return knex
.raw(
// Keep only one active walletAddressKey (the most recent row)
`DELETE FROM "walletAddressKeys" w
WHERE revoked = false
AND ctid NOT IN (
SELECT MAX(ctid)
FROM "walletAddressKeys"
WHERE revoked = false
AND kid = w.kid
GROUP BY kid, "walletAddressId"
)`
Comment on lines +9 to +17
Copy link
Contributor

@mkurapov mkurapov Dec 12, 2024

Choose a reason for hiding this comment

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

Suggested change
`DELETE FROM "walletAddressKeys" w
WHERE revoked = false
AND ctid NOT IN (
SELECT MAX(ctid)
FROM "walletAddressKeys"
WHERE revoked = false
AND kid = w.kid
GROUP BY kid, "walletAddressId"
)`
`DELETE FROM "walletAddressKeys" w
AND ctid NOT IN (
SELECT MAX(ctid)
FROM "walletAddressKeys"
WHERE revoked = false
GROUP BY kid, "walletAddressId"
)`

can you double check if this suggestion has the same behaviour? I think it should. Otherwise, looks good 👍

)
.then(() => {
return knex.raw(`
CREATE UNIQUE INDEX "wallet_address_keys_revoked_false_idx"
ON "walletAddressKeys" ("walletAddressId", kid)
WHERE revoked = false;
`)
})
}

/**
* @param { import("knex").Knex } knex
* @returns { Promise<void> }
*/
exports.down = function (knex) {
return knex.raw(
`DROP INDEX IF EXISTS "wallet_address_keys_revoked_false_idx"`
)
}
67 changes: 67 additions & 0 deletions packages/backend/src/graphql/resolvers/walletAddressKey.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ import { createWalletAddress } from '../../tests/walletAddress'
import { getPageTests } from './page.test'
import { createWalletAddressKey } from '../../tests/walletAddressKey'
import { GraphQLErrorCode } from '../errors'
import {
errorToCode,
errorToMessage,
isWalletAddressKeyError,
WalletAddressKeyError
} from '../../open_payments/wallet_address/key/errors'

const TEST_KEY = generateJwk({ keyId: uuid() })

Expand Down Expand Up @@ -99,6 +105,66 @@ describe('Wallet Address Key Resolvers', (): void => {
})
})

test('Cannot add duplicate key', async (): Promise<void> => {
const walletAddress = await createWalletAddress(deps)

const input: CreateWalletAddressKeyInput = {
walletAddressId: walletAddress.id,
jwk: TEST_KEY as JwkInput
}

await walletAddressKeyService.create(input)

expect.assertions(2)

try {
await appContainer.apolloClient
.mutate({
mutation: gql`
mutation CreateWalletAddressKey(
$input: CreateWalletAddressKeyInput!
) {
createWalletAddressKey(input: $input) {
walletAddressKey {
id
walletAddressId
jwk {
kid
x
alg
kty
crv
}
revoked
createdAt
}
}
}
`,
variables: {
input
}
})
.then((query): CreateWalletAddressKeyMutationResponse => {
if (query.data) {
return query.data.createWalletAddressKey
} else {
throw new Error('Data was empty')
}
})
} catch (error) {
expect(error).toBeInstanceOf(ApolloError)
expect((error as ApolloError).graphQLErrors).toContainEqual(
expect.objectContaining({
message: errorToMessage[WalletAddressKeyError.DuplicateKey],
extensions: expect.objectContaining({
code: errorToCode[WalletAddressKeyError.DuplicateKey]
})
})
)
}
})

test('internal server error', async (): Promise<void> => {
jest
.spyOn(walletAddressKeyService, 'create')
Expand Down Expand Up @@ -170,6 +236,7 @@ describe('Wallet Address Key Resolvers', (): void => {
walletAddressId: walletAddress.id,
jwk: TEST_KEY
})
assert.ok(!isWalletAddressKeyError(key))

const response = await appContainer.apolloClient
.mutate({
Expand Down
16 changes: 14 additions & 2 deletions packages/backend/src/graphql/resolvers/walletAddressKey.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ import { GraphQLError } from 'graphql'
import { GraphQLErrorCode } from '../errors'
import { Pagination } from '../../shared/baseModel'
import { getPageInfo } from '../../shared/pagination'
import {
errorToCode,
errorToMessage,
isWalletAddressKeyError
} from '../../open_payments/wallet_address/key/errors'

export const getWalletAddressKeys: WalletAddressResolvers<ApolloContext>['walletAddressKeys'] =
async (
Expand Down Expand Up @@ -85,10 +90,17 @@ export const createWalletAddressKey: MutationResolvers<ApolloContext>['createWal
'walletAddressKeyService'
)

const key = await walletAddressKeyService.create(args.input)
const keyOrError = await walletAddressKeyService.create(args.input)
if (isWalletAddressKeyError(keyOrError)) {
throw new GraphQLError(errorToMessage[keyOrError], {
extensions: {
code: errorToCode[keyOrError]
}
})
}

return {
walletAddressKey: walletAddressKeyToGraphql(key)
walletAddressKey: walletAddressKeyToGraphql(keyOrError)
}
}

Expand Down
21 changes: 21 additions & 0 deletions packages/backend/src/open_payments/wallet_address/key/errors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { GraphQLErrorCode } from '../../../graphql/errors'

export enum WalletAddressKeyError {
DuplicateKey = 'DuplicateKey'
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types
export const isWalletAddressKeyError = (o: any): o is WalletAddressKeyError =>
Object.values(WalletAddressKeyError).includes(o)

export const errorToCode: {
[key in WalletAddressKeyError]: GraphQLErrorCode
} = {
[WalletAddressKeyError.DuplicateKey]: GraphQLErrorCode.Duplicate
}

export const errorToMessage: {
[key in WalletAddressKeyError]: string
} = {
[WalletAddressKeyError.DuplicateKey]: 'Key already exists'
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { createWalletAddress } from '../../../tests/walletAddress'
import { WalletAddressService } from '../service'
import { OpenPaymentsServerRouteError } from '../../route-errors'
import assert from 'assert'
import { isWalletAddressKeyError } from './errors'

const TEST_KEY = generateJwk({ keyId: uuid() })

Expand Down Expand Up @@ -52,6 +53,7 @@ describe('Wallet Address Keys Routes', (): void => {
jwk: TEST_KEY
}
const key = await walletAddressKeyService.create(keyOption)
assert.ok(!isWalletAddressKeyError(key))

const ctx = createContext<WalletAddressUrlContext>({
headers: { Accept: 'application/json' },
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import assert from 'assert'
import { generateJwk } from '@interledger/http-signature-utils'
import { v4 as uuid } from 'uuid'

Expand All @@ -13,6 +14,7 @@ import { getPageTests } from '../../../shared/baseModel.test'
import { createWalletAddressKey } from '../../../tests/walletAddressKey'
import { WalletAddress } from '../model'
import { Pagination, SortOrder } from '../../../shared/baseModel'
import { isWalletAddressKeyError, WalletAddressKeyError } from './errors'

const TEST_KEY = generateJwk({ keyId: uuid() })

Expand Down Expand Up @@ -51,6 +53,39 @@ describe('Wallet Address Key Service', (): void => {
walletAddressKeyService.create(options)
).resolves.toMatchObject(options)
})

test('cannot add duplicate key to a wallet address', async (): Promise<void> => {
const options = {
walletAddressId: walletAddress.id,
jwk: TEST_KEY
}

await walletAddressKeyService.create(options)
await expect(walletAddressKeyService.create(options)).resolves.toEqual(
WalletAddressKeyError.DuplicateKey
)
})

test('Creates a new key unrevoked', async (): Promise<void> => {
const keyOption = {
walletAddressId: walletAddress.id,
jwk: TEST_KEY
}

const key = await walletAddressKeyService.create(keyOption)
assert.ok(!isWalletAddressKeyError(key))
await walletAddressKeyService.revoke(key.id)

const unrevokedKey = await walletAddressKeyService.create(keyOption)

assert.ok(!isWalletAddressKeyError(key))

expect(unrevokedKey).toMatchObject({
jwk: key.jwk,
walletAddressId: key.walletAddressId,
revoked: false
})
})
})

describe('Fetch Wallet Address Keys', (): void => {
Expand Down Expand Up @@ -79,6 +114,8 @@ describe('Wallet Address Key Service', (): void => {

const key1 = await walletAddressKeyService.create(keyOption1)
const key2 = await walletAddressKeyService.create(keyOption2)
assert.ok(!isWalletAddressKeyError(key1))
assert.ok(!isWalletAddressKeyError(key2))
await walletAddressKeyService.revoke(key1.id)

await expect(
Expand Down Expand Up @@ -107,6 +144,7 @@ describe('Wallet Address Key Service', (): void => {
}

const key = await walletAddressKeyService.create(keyOption)
assert.ok(!isWalletAddressKeyError(key))
const revokedKey = await walletAddressKeyService.revoke(key.id)
expect(revokedKey).toEqual({
...key,
Expand All @@ -129,6 +167,7 @@ describe('Wallet Address Key Service', (): void => {
}

const key = await walletAddressKeyService.create(keyOption)
assert.ok(!isWalletAddressKeyError(key))

const revokedKey = await walletAddressKeyService.revoke(key.id)
await expect(walletAddressKeyService.revoke(key.id)).resolves.toEqual(
Expand Down
32 changes: 24 additions & 8 deletions packages/backend/src/open_payments/wallet_address/key/service.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import { TransactionOrKnex } from 'objection'
import { TransactionOrKnex, UniqueViolationError } from 'objection'

import { WalletAddressKey } from './model'
import { BaseService } from '../../../shared/baseService'
import { JWK } from '@interledger/http-signature-utils'
import { Pagination, SortOrder } from '../../../shared/baseModel'
import { WalletAddressKeyError } from './errors'

export interface WalletAddressKeyService {
create(options: CreateOptions): Promise<WalletAddressKey>
create(
options: CreateOptions
): Promise<WalletAddressKey | WalletAddressKeyError>
revoke(id: string): Promise<WalletAddressKey | undefined>
getPage(
walletAddressId: string,
Expand Down Expand Up @@ -52,12 +55,25 @@ export interface CreateOptions {
async function create(
deps: ServiceDependencies,
options: CreateOptions
): Promise<WalletAddressKey> {
const key = await WalletAddressKey.query(deps.knex).insertAndFetch({
walletAddressId: options.walletAddressId,
jwk: options.jwk
})
return key
): Promise<WalletAddressKey | WalletAddressKeyError> {
try {
const key = await WalletAddressKey.query(deps.knex).insertAndFetch({
walletAddressId: options.walletAddressId,
jwk: options.jwk
})
return key
} catch (err) {
if (err instanceof UniqueViolationError) {
return WalletAddressKeyError.DuplicateKey
}
deps.logger.error(
{
err
},
'error adding key'
)
throw err
}
}

async function revoke(
Expand Down
7 changes: 6 additions & 1 deletion packages/backend/src/tests/walletAddressKey.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { WalletAddressKey } from '../open_payments/wallet_address/key/model'
import { CreateOptions } from '../open_payments/wallet_address/key/service'
import { v4 as uuidv4 } from 'uuid'
import { generateJwk, generateKey } from '@interledger/http-signature-utils'
import { isWalletAddressKeyError } from '../open_payments/wallet_address/key/errors'
export async function createWalletAddressKey(
deps: IocContract<AppServices>,
walletAddressId: string
Expand All @@ -18,5 +19,9 @@ export async function createWalletAddressKey(
})
}

return walletAddressKeyService.create(options)
const keyOrError = await walletAddressKeyService.create(options)
if (isWalletAddressKeyError(keyOrError)) {
throw keyOrError
}
return keyOrError
}
Loading