From d8da42e5be4217fd2926b0c60b0db4f1a138c8cd Mon Sep 17 00:00:00 2001 From: Justyn Oh Date: Tue, 4 Apr 2023 01:42:47 -0400 Subject: [PATCH] fix: add Unconnected payment channel type to fix mongoose 'feature' (#6036) * fix: add Unconnected payment channel type to fix mongoose 'feature' * chore: remove unneeded conditional * chore: change enum translation for unconnected: * chore: update default payment channel in form model tests --- .../PaymentSettingsSection.tsx | 4 +-- shared/types/form/form.ts | 4 +-- shared/types/payment.ts | 1 + .../__tests__/form.server.model.spec.ts | 2 +- src/app/models/form.server.model.ts | 20 +++++++------ .../admin-form.payments.controller.ts | 23 ++++++++++++--- src/app/modules/payments/stripe.controller.ts | 6 ++-- src/app/modules/payments/stripe.errors.ts | 6 ---- src/app/modules/payments/stripe.service.ts | 28 ++++++++----------- .../encrypt-submission.controller.ts | 2 +- src/types/form.ts | 6 ++-- 11 files changed, 54 insertions(+), 48 deletions(-) diff --git a/frontend/src/features/admin-form/settings/components/PaymentSettingsSection/PaymentSettingsSection.tsx b/frontend/src/features/admin-form/settings/components/PaymentSettingsSection/PaymentSettingsSection.tsx index c3ca5b08dc..ace68d26bb 100644 --- a/frontend/src/features/admin-form/settings/components/PaymentSettingsSection/PaymentSettingsSection.tsx +++ b/frontend/src/features/admin-form/settings/components/PaymentSettingsSection/PaymentSettingsSection.tsx @@ -1,6 +1,6 @@ import { Flex, FormControl, Icon, Skeleton, Text } from '@chakra-ui/react' -import { FormResponseMode } from '~shared/types' +import { FormResponseMode, PaymentChannel } from '~shared/types' import { BxsCheckCircle, BxsError, BxsInfoCircle } from '~assets/icons' import FormLabel from '~components/FormControl/FormLabel' @@ -101,7 +101,7 @@ const PaymentsSectionText = () => { if ( settings?.responseMode === FormResponseMode.Encrypt && - settings?.payments_channel + settings?.payments_channel.channel !== PaymentChannel.Unconnected ) { return ( <> diff --git a/shared/types/form/form.ts b/shared/types/form/form.ts index 1b3547cacd..6817e28c0e 100644 --- a/shared/types/form/form.ts +++ b/shared/types/form/form.ts @@ -118,8 +118,8 @@ export interface EmailFormBase extends FormBase { export interface StorageFormBase extends FormBase { responseMode: FormResponseMode.Encrypt publicKey: string - payments_channel?: FormPaymentsChannel - payments_field?: FormPaymentsField + payments_channel: FormPaymentsChannel + payments_field: FormPaymentsField } /** diff --git a/shared/types/payment.ts b/shared/types/payment.ts index 4a6189eae5..8c05bee6c2 100644 --- a/shared/types/payment.ts +++ b/shared/types/payment.ts @@ -12,6 +12,7 @@ export enum PaymentStatus { } export enum PaymentChannel { + Unconnected = 'Unconnected', Stripe = 'Stripe', // for extensibility to future payment options } diff --git a/src/app/models/__tests__/form.server.model.spec.ts b/src/app/models/__tests__/form.server.model.spec.ts index ab7cb0a858..c0b0fba6c9 100644 --- a/src/app/models/__tests__/form.server.model.spec.ts +++ b/src/app/models/__tests__/form.server.model.spec.ts @@ -91,7 +91,7 @@ const FORM_DEFAULTS = { const PAYMENTS_DEFAULTS = { payments_channel: { - channel: PaymentChannel.Stripe, + channel: PaymentChannel.Unconnected, target_account_id: '', publishable_key: '', }, diff --git a/src/app/models/form.server.model.ts b/src/app/models/form.server.model.ts index fcc7a21edb..0406588a11 100644 --- a/src/app/models/form.server.model.ts +++ b/src/app/models/form.server.model.ts @@ -133,11 +133,12 @@ const EncryptedFormSchema = new Schema({ type: String, required: true, }, + payments_channel: { channel: { type: String, enum: Object.values(PaymentChannel), - default: PaymentChannel.Stripe, + default: PaymentChannel.Unconnected, }, target_account_id: { type: String, @@ -149,7 +150,6 @@ const EncryptedFormSchema = new Schema({ default: '', validate: [/^\S*$/i, 'publishable_key must not contain whitespace.'], }, - required: false, }, payments_field: { @@ -159,6 +159,7 @@ const EncryptedFormSchema = new Schema({ }, description: { type: String, + trim: true, default: '', }, amount_cents: { @@ -170,7 +171,6 @@ const EncryptedFormSchema = new Schema({ message: 'Payment amount must be at least 50 cents and an integer.', }, }, - required: false, }, }) @@ -184,7 +184,7 @@ EncryptedFormDocumentSchema.methods.addPaymentAccountId = async function ({ accountId: FormPaymentsChannel['target_account_id'] publishableKey: FormPaymentsChannel['publishable_key'] }) { - if (!this.payments_channel) { + if (this.payments_channel?.channel === PaymentChannel.Unconnected) { this.payments_channel = { // Definitely Stripe for now, may be different later on. channel: PaymentChannel.Stripe, @@ -196,11 +196,13 @@ EncryptedFormDocumentSchema.methods.addPaymentAccountId = async function ({ } EncryptedFormDocumentSchema.methods.removePaymentAccount = async function () { - if (this.payments_channel) { - this.payments_channel = undefined - if (this.payments_field) { - this.payments_field.enabled = false - } + this.payments_channel = { + channel: PaymentChannel.Unconnected, + target_account_id: '', + publishable_key: '', + } + if (this.payments_field) { + this.payments_field.enabled = false } return this.save() } diff --git a/src/app/modules/form/admin-form/admin-form.payments.controller.ts b/src/app/modules/form/admin-form/admin-form.payments.controller.ts index 3ffbc959bf..1eba5d55ec 100644 --- a/src/app/modules/form/admin-form/admin-form.payments.controller.ts +++ b/src/app/modules/form/admin-form/admin-form.payments.controller.ts @@ -84,7 +84,7 @@ export const handleConnectAccount: ControllerHandler<{ } /** - * Handler for DELETE /:formId/stripe + * Handler for DELETE /:formId/stripe. * @security session * * @returns 200 when Stripe credentials successfully deleted @@ -134,6 +134,20 @@ export const handleUnlinkAccount: ControllerHandler<{ ) } +/** + * Handler for GET /:formId/stripe/validate. + * @security session + * + * @returns 200 when Stripe credentials have been validated + * @returns 401 when user is not logged in + * @returns 403 when user does not have permissions to update the form + * @returns 404 when form to update cannot be found + * @returns 410 when form to update has been deleted + * @returns 422 when id of user who is updating the form cannot be found + * @returns 422 when the form to be updated is not an encrypt mode form + * @returns 500 when database error occurs + * @returns 502 when the connected Stripe credentials are invalid + */ export const handleValidatePaymentAccount: ControllerHandler<{ formId: string }> = async (req, res) => { @@ -155,7 +169,7 @@ export const handleValidatePaymentAccount: ControllerHandler<{ .andThen(checkFormIsEncryptMode) // Step 4: Validate the associated Stripe account. .andThen((form) => - validateAccount(form.payments_channel?.target_account_id), + validateAccount(form.payments_channel.target_account_id), ) .map((account) => res.json({ account })) .mapErr((error) => { @@ -175,12 +189,13 @@ export const handleValidatePaymentAccount: ControllerHandler<{ } /** + * Private handler for PUT /:formId/payment * NOTE: Exported for testing. - * Private handler for PUT /forms/:formId/payment * @precondition Must be preceded by request validation * @security session * * @returns 200 with updated payments + * @returns 400 when updated payment amount is out of bounds * @returns 403 when current user does not have permissions to update the payments * @returns 404 when form cannot be found * @returns 410 when updating the payments for an archived form @@ -232,7 +247,7 @@ export const _handleUpdatePayments: ControllerHandler< } /** - * Handler for PUT /forms/:formId/payment + * Handler for PUT /:formId/payment */ export const handleUpdatePayments = [ celebrate({ diff --git a/src/app/modules/payments/stripe.controller.ts b/src/app/modules/payments/stripe.controller.ts index cd649c2736..6c2c7bec5f 100644 --- a/src/app/modules/payments/stripe.controller.ts +++ b/src/app/modules/payments/stripe.controller.ts @@ -333,12 +333,10 @@ const _handleConnectOauthCallback: ControllerHandler< > = async (req, res) => { const { code, state } = req.query - //Extracting state parameter previously signed and stored in cookies + // Step 0: Extract state parameter previously signed and stored in cookies. + // Compare state values to ensure that no tampering has occurred. const { stripeState } = req.signedCookies - - //Comparing state parameters if (state !== stripeState) { - //throwing unprocessable entity error return res.status(StatusCodes.UNPROCESSABLE_ENTITY).json({ message: 'Invalid state parameter', }) diff --git a/src/app/modules/payments/stripe.errors.ts b/src/app/modules/payments/stripe.errors.ts index 2e817dc7bd..a1ff6a35b9 100644 --- a/src/app/modules/payments/stripe.errors.ts +++ b/src/app/modules/payments/stripe.errors.ts @@ -50,12 +50,6 @@ export class StripeAccountError extends ApplicationError { } } -export class StripeAccountNotFoundError extends ApplicationError { - constructor(message = 'Stripe account not found') { - super(message) - } -} - export class ComputePaymentStateError extends ApplicationError { constructor(message = 'Error while computing payment state') { super(message) diff --git a/src/app/modules/payments/stripe.service.ts b/src/app/modules/payments/stripe.service.ts index 867ffd9bba..7793c4b421 100644 --- a/src/app/modules/payments/stripe.service.ts +++ b/src/app/modules/payments/stripe.service.ts @@ -8,7 +8,11 @@ import Stripe from 'stripe' import { MarkRequired } from 'ts-essentials' import isURL from 'validator/lib/isURL' -import { IPaymentSchema, IPopulatedEncryptedForm } from '../../../types' +import { + IEncryptedFormSchema, + IPaymentSchema, + IPopulatedEncryptedForm, +} from '../../../types' import config from '../../config/config' import { paymentConfig } from '../../config/features/payment.config' import { createLoggerWithLabel } from '../../config/logger' @@ -306,14 +310,8 @@ export const linkStripeAccountToForm = ( accountId: string publishableKey: string }, -): ResultAsync => { - // Check if form already has account id - if (form.payments_channel) { - return okAsync(form.payments_channel.target_account_id) - } - - // No account id, create and inject into form - return ResultAsync.fromPromise( +): ResultAsync => + ResultAsync.fromPromise( form.addPaymentAccountId({ accountId, publishableKey }), (error) => { const errMsg = 'Failed to update payment account id' @@ -330,14 +328,11 @@ export const linkStripeAccountToForm = ( return new DatabaseError(errMsg) }, ).map((updatedForm) => updatedForm.payments_channel.target_account_id) -} - -export const unlinkStripeAccountFromForm = (form: IPopulatedEncryptedForm) => { - if (!form.payments_channel) { - return okAsync(true) - } - return ResultAsync.fromPromise(form.removePaymentAccount(), (error) => { +export const unlinkStripeAccountFromForm = ( + form: IPopulatedEncryptedForm, +): ResultAsync => + ResultAsync.fromPromise(form.removePaymentAccount(), (error) => { const errMsg = 'Failed to remove payment account from form' logger.error({ message: errMsg, @@ -349,7 +344,6 @@ export const unlinkStripeAccountFromForm = (form: IPopulatedEncryptedForm) => { }) return new DatabaseError(errMsg) }) -} export const createAccountLink = ( accountId: string, diff --git a/src/app/modules/submission/encrypt-submission/encrypt-submission.controller.ts b/src/app/modules/submission/encrypt-submission/encrypt-submission.controller.ts index 490dae3111..91ff8bda7b 100644 --- a/src/app/modules/submission/encrypt-submission/encrypt-submission.controller.ts +++ b/src/app/modules/submission/encrypt-submission/encrypt-submission.controller.ts @@ -350,7 +350,7 @@ const submitEncryptModeForm: ControllerHandler< // Handle submissions for payments forms if ( form.payments_field?.enabled && - form.payments_channel?.channel === PaymentChannel.Stripe + form.payments_channel.channel === PaymentChannel.Stripe ) { // Step 0: Perform validation checks const amount = form.payments_field.amount_cents diff --git a/src/types/form.ts b/src/types/form.ts index b37c8ff51c..066bcef95d 100644 --- a/src/types/form.ts +++ b/src/types/form.ts @@ -272,8 +272,10 @@ export interface IPopulatedForm extends Omit { export interface IEncryptedForm extends IForm { publicKey: string - payments_channel?: FormPaymentsChannel - payments_field?: FormPaymentsField + // Nested objects will always be returned from mongoose finds, even if they + // are not defined in DB. See https://github.com/Automattic/mongoose/issues/5310 + payments_channel: FormPaymentsChannel + payments_field: FormPaymentsField emails?: never }