diff --git a/packages/backend/src/graphql/resolvers/tenant_settings.test.ts b/packages/backend/src/graphql/resolvers/tenant_settings.test.ts index 4fc96c90c8..a27219fc44 100644 --- a/packages/backend/src/graphql/resolvers/tenant_settings.test.ts +++ b/packages/backend/src/graphql/resolvers/tenant_settings.test.ts @@ -15,10 +15,17 @@ import { createHttpLink, ApolloLink, InMemoryCache, - gql + gql, + ApolloError } from '@apollo/client' import { setContext } from '@apollo/client/link/context' import { TenantSettingKeys } from '../../tenants/settings/model' +import { faker } from '@faker-js/faker' +import { + errorToCode, + errorToMessage, + TenantSettingError +} from '../../tenants/settings/errors' function createTenantedApolloClient( appContainer: TestContainer, @@ -93,7 +100,10 @@ describe('Tenant Settings Resolvers', (): void => { test('can create tenant setting', async (): Promise => { const input: CreateTenantSettingsInput = { settings: [ - { key: TenantSettingKeys.EXCHANGE_RATES_URL.name, value: 'MY_VALUE' } + { + key: TenantSettingKeys.EXCHANGE_RATES_URL.name, + value: faker.internet.url() + } ] } @@ -122,6 +132,56 @@ describe('Tenant Settings Resolvers', (): void => { expect(response.settings.length).toBeGreaterThan(0) }) + + test('errors when invalid input is provided', async (): Promise => { + const input: CreateTenantSettingsInput = { + settings: [ + { + key: TenantSettingKeys.WEBHOOK_MAX_RETRY.name, + value: '-1' + } + ] + } + + const tenant = await createTenant(deps) + const client = createTenantedApolloClient(appContainer, tenant.id) + expect.assertions(2) + + try { + await client + .mutate({ + mutation: gql` + mutation CreateTenantSettings( + $input: CreateTenantSettingsInput! + ) { + createTenantSettings(input: $input) { + settings { + key + value + } + } + } + `, + variables: { input } + }) + .then((query): CreateTenantSettingsMutationResponse => { + if (query.data) { + return query.data.createTenantSettings + } + throw new Error('Data was empty') + }) + } catch (error) { + expect(error).toBeInstanceOf(ApolloError) + expect((error as ApolloError).graphQLErrors).toContainEqual( + expect.objectContaining({ + message: errorToMessage[TenantSettingError.InvalidSetting], + extensions: expect.objectContaining({ + code: errorToCode[TenantSettingError.InvalidSetting] + }) + }) + ) + } + }) }) describe('Get Tenant Settings', (): void => { diff --git a/packages/backend/src/graphql/resolvers/tenant_settings.ts b/packages/backend/src/graphql/resolvers/tenant_settings.ts index ceb474c524..3e98fc149a 100644 --- a/packages/backend/src/graphql/resolvers/tenant_settings.ts +++ b/packages/backend/src/graphql/resolvers/tenant_settings.ts @@ -1,4 +1,10 @@ +import { GraphQLError } from 'graphql' import { TenantedApolloContext } from '../../app' +import { + isTenantSettingError, + errorToCode, + errorToMessage +} from '../../tenants/settings/errors' import { TenantSetting } from '../../tenants/settings/model' import { ResolversTypes, @@ -32,13 +38,21 @@ export const createTenantSettings: MutationResolvers['cre ): Promise => { const tenantSettingService = await ctx.container.use('tenantSettingService') - const tenantSettings = await tenantSettingService.create({ + const tenantSettingsOrError = await tenantSettingService.create({ tenantId: ctx.tenant.id, setting: args.input.settings }) + if (isTenantSettingError(tenantSettingsOrError)) { + throw new GraphQLError(errorToMessage[tenantSettingsOrError], { + extensions: { + code: errorToCode[tenantSettingsOrError] + } + }) + } + return { - settings: tenantSettingsToGraphql(tenantSettings) + settings: tenantSettingsToGraphql(tenantSettingsOrError) } } diff --git a/packages/backend/src/tenants/settings/errors.ts b/packages/backend/src/tenants/settings/errors.ts index 131786e1ef..6d0680a2ad 100644 --- a/packages/backend/src/tenants/settings/errors.ts +++ b/packages/backend/src/tenants/settings/errors.ts @@ -2,7 +2,8 @@ import { GraphQLErrorCode } from '../../graphql/errors' export enum TenantSettingError { TenantNotFound = 'TenantNotFound', - UnknownError = 'UnknownError' + UnknownError = 'UnknownError', + InvalidSetting = 'InvalidSettingError' } // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -12,6 +13,7 @@ export const isTenantSettingError = (t: any): t is TenantSettingError => export const errorToCode: { [key in TenantSettingError]: GraphQLErrorCode } = { + [TenantSettingError.InvalidSetting]: GraphQLErrorCode.BadUserInput, [TenantSettingError.TenantNotFound]: GraphQLErrorCode.NotFound, [TenantSettingError.UnknownError]: GraphQLErrorCode.InternalServerError } @@ -20,5 +22,7 @@ export const errorToMessage: { [key in TenantSettingError]: string } = { [TenantSettingError.TenantNotFound]: 'Tenant not found', - [TenantSettingError.UnknownError]: 'Unknown error' + [TenantSettingError.UnknownError]: 'Unknown error', + [TenantSettingError.InvalidSetting]: + 'Invalid value for one or more tenant settings' } diff --git a/packages/backend/src/tenants/settings/model.ts b/packages/backend/src/tenants/settings/model.ts index b5faa01c5d..7b1b257d2e 100644 --- a/packages/backend/src/tenants/settings/model.ts +++ b/packages/backend/src/tenants/settings/model.ts @@ -56,7 +56,8 @@ const TENANT_KEY_MAPPING = { [TenantSettingKeys.EXCHANGE_RATES_URL.name]: 'exchangeRatesUrl', [TenantSettingKeys.WEBHOOK_MAX_RETRY.name]: 'webhookMaxRetry', [TenantSettingKeys.WEBHOOK_TIMEOUT.name]: 'webhookTimeout', - [TenantSettingKeys.WEBHOOK_URL.name]: 'webhookUrl' + [TenantSettingKeys.WEBHOOK_URL.name]: 'webhookUrl', + [TenantSettingKeys.WALLET_ADDRESS_URL.name]: 'walletAddressUrl' } as const export type FormattedTenantSettings = Record< @@ -74,3 +75,27 @@ export const formatSettings = ( } return settingsObj } + +const validateUrlTenantSetting = (url: string): boolean => { + try { + return !!new URL(url) + } catch (err) { + return false + } +} + +const validateNonNegativeTenantSetting = (numberString: string): boolean => { + return !!(Number.isFinite(Number(numberString)) && Number(numberString) > -1) +} + +const validatePositiveTenantSetting = (numberString: string): boolean => { + return !!(Number.isFinite(Number(numberString)) && Number(numberString) > 0) +} + +export const TENANT_SETTING_VALIDATORS = { + [TenantSettingKeys.EXCHANGE_RATES_URL.name]: validateUrlTenantSetting, + [TenantSettingKeys.WEBHOOK_MAX_RETRY.name]: validateNonNegativeTenantSetting, + [TenantSettingKeys.WEBHOOK_TIMEOUT.name]: validatePositiveTenantSetting, + [TenantSettingKeys.WEBHOOK_URL.name]: validateUrlTenantSetting, + [TenantSettingKeys.WALLET_ADDRESS_URL.name]: validateUrlTenantSetting +} diff --git a/packages/backend/src/tenants/settings/service.test.ts b/packages/backend/src/tenants/settings/service.test.ts index 5ceeb2556b..2af0fa28a6 100644 --- a/packages/backend/src/tenants/settings/service.test.ts +++ b/packages/backend/src/tenants/settings/service.test.ts @@ -1,4 +1,5 @@ import { IocContract } from '@adonisjs/fold' +import assert from 'assert' import { AppServices } from '../../app' import { initIocContainer } from '../..' import { Config } from '../../config/app' @@ -19,6 +20,7 @@ import { import { AuthServiceClient } from '../../auth-service-client/client' import { v4 as uuid } from 'uuid' import { createTenant } from '../../tests/tenant' +import { isTenantSettingError, TenantSettingError } from './errors' describe('TenantSetting Service', (): void => { let deps: IocContract @@ -123,18 +125,136 @@ describe('TenantSetting Service', (): void => { expect(result[0].key).toEqual(initialOptions.setting[0].key) expect(result[0].value).toEqual(newValue) }) + + test.each` + key + ${TenantSettingKeys.EXCHANGE_RATES_URL.name} + ${TenantSettingKeys.WEBHOOK_MAX_RETRY.name} + ${TenantSettingKeys.WEBHOOK_TIMEOUT.name} + ${TenantSettingKeys.WEBHOOK_URL.name} + ${TenantSettingKeys.WALLET_ADDRESS_URL.name} + `( + 'cannot use invalid setting value for $key', + async ({ key }): Promise => { + const invalidSettingOption: CreateOptions = { + tenantId: tenant.id, + setting: [ + { + key, + value: 'invalid_value' + } + ] + } + + await expect( + tenantSettingService.create(invalidSettingOption) + ).resolves.toEqual(TenantSettingError.InvalidSetting) + } + ) + + test.each` + key + ${TenantSettingKeys.EXCHANGE_RATES_URL.name} + ${TenantSettingKeys.WEBHOOK_URL.name} + ${TenantSettingKeys.WALLET_ADDRESS_URL.name} + `( + 'accepts URL string for $key tenant setting', + async ({ key }): Promise => { + const url = faker.internet.url() + const createOption: CreateOptions = { + tenantId: tenant.id, + setting: [ + { + key, + value: url + } + ] + } + + const tenantSetting = await tenantSettingService.create(createOption) + expect(tenantSetting).toEqual([ + expect.objectContaining({ + tenantId: tenant.id, + key, + value: url + }) + ]) + } + ) + + test('cannot use invalid numeric values for positive tenant settings', async (): Promise => { + const infiniteOption: CreateOptions = { + tenantId: tenant.id, + setting: [ + { + key: TenantSettingKeys.WEBHOOK_TIMEOUT.name, + value: 'Infinity' + } + ] + } + + await expect( + tenantSettingService.create(infiniteOption) + ).resolves.toEqual(TenantSettingError.InvalidSetting) + + const zeroOption: CreateOptions = { + tenantId: tenant.id, + setting: [ + { + key: TenantSettingKeys.WEBHOOK_TIMEOUT.name, + value: '0' + } + ] + } + + await expect(tenantSettingService.create(zeroOption)).resolves.toEqual( + TenantSettingError.InvalidSetting + ) + }) + + test('cannot use invalid numeric values for non-negative numeric tenant settings', async (): Promise => { + const infiniteOption: CreateOptions = { + tenantId: tenant.id, + setting: [ + { + key: TenantSettingKeys.WEBHOOK_MAX_RETRY.name, + value: 'Infinity' + } + ] + } + + await expect( + tenantSettingService.create(infiniteOption) + ).resolves.toEqual(TenantSettingError.InvalidSetting) + + const negativeOption: CreateOptions = { + tenantId: tenant.id, + setting: [ + { + key: TenantSettingKeys.WEBHOOK_MAX_RETRY.name, + value: '-1' + } + ] + } + + await expect( + tenantSettingService.create(negativeOption) + ).resolves.toEqual(TenantSettingError.InvalidSetting) + }) }) describe('get', () => { let tenantSetting: TenantSetting[] - function createTenantSetting() { + async function createTenantSetting(): Promise { const options: CreateOptions = { tenantId: tenant.id, setting: [randomSetting()] } - return tenantSettingService.create(options) + const createdTenantSetting = await tenantSettingService.create(options) + assert(!isTenantSettingError(createdTenantSetting)) + return createdTenantSetting } beforeEach(async (): Promise => { @@ -201,7 +321,7 @@ describe('TenantSetting Service', (): void => { test('can update own setting', async () => { const newValues = { ...updateOptions, - value: 'test' + value: faker.internet.url() } await tenantSettingService.update(newValues) @@ -214,6 +334,132 @@ describe('TenantSetting Service', (): void => { expect.arrayContaining([expect.objectContaining(newValues)]) ) }) + + test.each` + key + ${TenantSettingKeys.EXCHANGE_RATES_URL.name} + ${TenantSettingKeys.WEBHOOK_MAX_RETRY.name} + ${TenantSettingKeys.WEBHOOK_TIMEOUT.name} + ${TenantSettingKeys.WEBHOOK_URL.name} + ${TenantSettingKeys.WALLET_ADDRESS_URL.name} + `( + 'cannot use invalid setting value for $key', + async ({ key }): Promise => { + const invalidSettingOption: UpdateOptions = { + tenantId: tenant.id, + key, + value: 'invalid_value' + } + + await expect( + tenantSettingService.update(invalidSettingOption) + ).resolves.toEqual(TenantSettingError.InvalidSetting) + } + ) + + test.each` + key + ${TenantSettingKeys.EXCHANGE_RATES_URL.name} + ${TenantSettingKeys.WEBHOOK_URL.name} + ${TenantSettingKeys.WALLET_ADDRESS_URL.name} + `( + 'accepts URL string for $key tenant setting', + async ({ key }): Promise => { + const createOption: CreateOptions = { + tenantId: tenant.id, + setting: [ + { + key, + value: faker.internet.url() + } + ] + } + await tenantSettingService.create(createOption) + + const url = faker.internet.url() + const updateOption: UpdateOptions = { + tenantId: tenant.id, + key, + value: url + } + + const updatedTenantSetting = + await tenantSettingService.update(updateOption) + expect(updatedTenantSetting).toEqual([ + expect.objectContaining({ + tenantId: tenant.id, + key, + value: url + }) + ]) + } + ) + + test('cannot use invalid numeric values for positive numeric tenant settings', async (): Promise => { + const createOption: CreateOptions = { + tenantId: tenant.id, + setting: [ + { + key: TenantSettingKeys.WEBHOOK_TIMEOUT.name, + value: '5000' + } + ] + } + await tenantSettingService.create(createOption) + const infiniteOption: UpdateOptions = { + tenantId: tenant.id, + key: TenantSettingKeys.WEBHOOK_TIMEOUT.name, + value: 'Infinity' + } + + await expect( + tenantSettingService.update(infiniteOption) + ).resolves.toEqual(TenantSettingError.InvalidSetting) + + const zeroOption: UpdateOptions = { + tenantId: tenant.id, + key: TenantSettingKeys.WEBHOOK_TIMEOUT.name, + value: '0' + } + + await expect(tenantSettingService.update(zeroOption)).resolves.toEqual( + TenantSettingError.InvalidSetting + ) + }) + + test('cannot use invalid numeric values for non-negative numeric tenant settings', async (): Promise => { + const createOption: CreateOptions = { + tenantId: tenant.id, + setting: [ + { + key: TenantSettingKeys.WEBHOOK_MAX_RETRY.name, + value: '10' + } + ] + } + + await tenantSettingService.create(createOption) + + const infiniteOption: UpdateOptions = { + tenantId: tenant.id, + key: TenantSettingKeys.WEBHOOK_MAX_RETRY.name, + value: 'Infinity' + } + + await expect( + tenantSettingService.update(infiniteOption) + ).resolves.toEqual(TenantSettingError.InvalidSetting) + + const negativeOption: UpdateOptions = { + tenantId: tenant.id, + key: TenantSettingKeys.WEBHOOK_MAX_RETRY.name, + value: '-1' + } + + await expect( + tenantSettingService.update(negativeOption) + ).resolves.toEqual(TenantSettingError.InvalidSetting) + }) }) describe('delete', (): void => { @@ -235,14 +481,16 @@ describe('TenantSetting Service', (): void => { setting: [exchangeRatesSetting()] } - const tenantSetting = await tenantSettingService.create(createOptions) + const tenantSettingsOrError = + await tenantSettingService.create(createOptions) + assert(!isTenantSettingError(tenantSettingsOrError)) await tenantSettingService.delete({ - tenantId: tenantSetting[0].tenantId, + tenantId: tenantSettingsOrError[0].tenantId, key: createOptions.setting[0].key }) const dbTenantSetting = await TenantSetting.query().findById( - tenantSetting[0].id + tenantSettingsOrError[0].id ) expect(dbTenantSetting?.deletedAt).toBeDefined() expect(dbTenantSetting?.deletedAt?.getTime()).toBeLessThanOrEqual( @@ -257,6 +505,7 @@ describe('TenantSetting Service', (): void => { } const tenantSetting = await tenantSettingService.create(createOptions) + assert(!isTenantSettingError(tenantSetting)) await tenantSettingService.delete({ tenantId: tenantSetting[0].tenantId, key: createOptions.setting[0].key @@ -312,7 +561,10 @@ describe('TenantSetting Service', (): void => { setting: [exchangeRatesSetting()] } - tenantSetting = await tenantSettingService.create(createOptions) + const createdTenantSetting = + await tenantSettingService.create(createOptions) + assert(!isTenantSettingError(createdTenantSetting)) + tenantSetting = createdTenantSetting }) afterEach(async (): Promise => { diff --git a/packages/backend/src/tenants/settings/service.ts b/packages/backend/src/tenants/settings/service.ts index 9989f814a1..ae6fc437e1 100644 --- a/packages/backend/src/tenants/settings/service.ts +++ b/packages/backend/src/tenants/settings/service.ts @@ -1,8 +1,13 @@ import { TransactionOrKnex } from 'objection' import { Pagination, SortOrder } from '../../shared/baseModel' import { BaseService } from '../../shared/baseService' -import { TenantSetting, TenantSettingKeys } from './model' +import { + TENANT_SETTING_VALIDATORS, + TenantSetting, + TenantSettingKeys +} from './model' import { Knex } from 'knex' +import { TenantSettingError } from './errors' export interface KeyValuePair { key: string @@ -35,8 +40,10 @@ export interface TenantSettingService { create: ( options: CreateOptions, extra?: ExtraOptions - ) => Promise - update: (options: UpdateOptions) => Promise + ) => Promise + update: ( + options: UpdateOptions + ) => Promise delete: (options: GetOptions, extra?: ExtraOptions) => Promise getPage: ( tenantId: string, @@ -106,8 +113,14 @@ async function deleteTenantSetting( async function updateTenantSetting( deps: ServiceDependencies, options: UpdateOptions -): Promise { - await TenantSetting.query(deps.knex) +): Promise { + if ( + Object.keys(TENANT_SETTING_VALIDATORS).includes(options.key) && + !TENANT_SETTING_VALIDATORS[options.key](options.value) + ) { + return TenantSettingError.InvalidSetting + } + return TenantSetting.query(deps.knex) .patch({ value: options.value }) .whereNull('deletedAt') .andWhere('tenantId', options.tenantId) @@ -120,7 +133,16 @@ async function createTenantSetting( deps: ServiceDependencies, options: CreateOptions, extra?: ExtraOptions -) { +): Promise { + for (const setting of options.setting) { + if ( + Object.keys(TENANT_SETTING_VALIDATORS).includes(setting.key) && + !TENANT_SETTING_VALIDATORS[setting.key](setting.value) + ) { + return TenantSettingError.InvalidSetting + } + } + const dataToUpsert = options.setting .filter((setting) => Object.keys(TenantSettingKeys).includes(setting.key)) .map((s) => ({