Skip to content

Commit

Permalink
Generate a unique verification token for every unverified email address
Browse files Browse the repository at this point in the history
This change is a prerequisite to making the path unguessable.

Refs #1307, 95c9016
  • Loading branch information
thewilkybarkid committed Nov 2, 2023
1 parent 9f54978 commit 5459bc1
Show file tree
Hide file tree
Showing 6 changed files with 89 additions and 29 deletions.
32 changes: 28 additions & 4 deletions src/contact-email-address.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import * as RTE from 'fp-ts/ReaderTaskEither'
import type * as TE from 'fp-ts/TaskEither'
import { flow } from 'fp-ts/function'
import { flow, pipe } from 'fp-ts/function'
import * as C from 'io-ts/Codec'
import * as D from 'io-ts/Decoder'
import type { Orcid } from 'orcid-id-ts'
import { match } from 'ts-pattern'
import { type Uuid, isUuid } from 'uuid-ts'
import { type EmailAddress, EmailAddressC } from './types/email-address'

export type ContactEmailAddress = VerifiedContactEmailAddress | UnverifiedContactEmailAddress
Expand All @@ -16,6 +18,7 @@ export interface VerifiedContactEmailAddress {
export interface UnverifiedContactEmailAddress {
readonly type: 'unverified'
readonly value: EmailAddress
readonly verificationToken: Uuid
}

export interface GetContactEmailAddressEnv {
Expand All @@ -30,9 +33,30 @@ export interface EditContactEmailAddressEnv extends GetContactEmailAddressEnv {
) => TE.TaskEither<'unavailable', void>
}

export const ContactEmailAddressC = C.struct({
type: C.literal('verified', 'unverified'),
value: EmailAddressC,
const UuidC = C.make(
pipe(
D.string,
D.parse(s => {
if (s.toLowerCase() === s) {
return D.fromRefinement(isUuid, 'UUID').decode(s)
}

return D.failure(s, 'UUID')
}),
),
{ encode: uuid => uuid.toLowerCase() },
)

export const ContactEmailAddressC = C.sum('type')({
verified: C.struct({
type: C.literal('verified'),
value: EmailAddressC,
}),
unverified: C.struct({
type: C.literal('unverified'),
value: EmailAddressC,
verificationToken: UuidC,
}),
}) satisfies C.Codec<unknown, unknown, ContactEmailAddress>

export const getContactEmailAddress = (orcid: Orcid) =>
Expand Down
17 changes: 16 additions & 1 deletion src/my-details-page/change-contact-email-address.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { format } from 'fp-ts-routing'
import * as E from 'fp-ts/Either'
import type { IO } from 'fp-ts/IO'
import * as I from 'fp-ts/Identity'
import * as O from 'fp-ts/Option'
import type { Reader } from 'fp-ts/Reader'
import * as RIO from 'fp-ts/ReaderIO'
import { flow, pipe } from 'fp-ts/function'
import * as s from 'fp-ts/string'
import { type ResponseEnded, Status, type StatusOpen } from 'hyper-ts'
Expand All @@ -11,6 +13,7 @@ import * as RM from 'hyper-ts/ReaderMiddleware'
import * as D from 'io-ts/Decoder'
import { get } from 'spectacles-ts'
import { P, match } from 'ts-pattern'
import type { Uuid } from 'uuid-ts'
import { deleteContactEmailAddress, getContactEmailAddress, saveContactEmailAddress } from '../contact-email-address'
import { canChangeContactEmailAddress } from '../feature-flags'
import { type InvalidE, getInput, hasAnError, invalidE } from '../form'
Expand All @@ -25,6 +28,15 @@ import { type GetUserEnv, type User, getUser } from '../user'

export type Env = EnvFor<typeof changeContactEmailAddress>

interface GenerateUuidEnv {
generateUuid: IO<Uuid>
}

const generateUuid = pipe(
RIO.ask<GenerateUuidEnv>(),
RIO.chainIOK(({ generateUuid }) => generateUuid),
)

export const changeContactEmailAddress = pipe(
getUser,
RM.bindTo('user'),
Expand Down Expand Up @@ -122,7 +134,10 @@ const handleChangeContactEmailAddressForm = (user: User) =>
)
.with(P.string, emailAddress =>
pipe(
RM.fromReaderTaskEither(saveContactEmailAddress(user.orcid, { type: 'unverified', value: emailAddress })),
RM.fromReaderIO(generateUuid),
RM.chainReaderTaskEitherKW(verificationToken =>
saveContactEmailAddress(user.orcid, { type: 'unverified', value: emailAddress, verificationToken }),
),
RM.ichainMiddlewareK(() => seeOther(format(myDetailsMatch.formatter, {}))),
RM.orElseW(() => serviceUnavailable),
),
Expand Down
1 change: 1 addition & 0 deletions src/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,7 @@ const router: P.Parser<RM.ReaderMiddleware<RouterEnv, StatusOpen, ResponseEnded,
...env,
deleteContactEmailAddress: withEnv(deleteContactEmailAddress, env),
getContactEmailAddress: withEnv(getContactEmailAddress, env),
generateUuid: uuid.v4(),
saveContactEmailAddress: withEnv(saveContactEmailAddress, env),
})),
),
Expand Down
1 change: 1 addition & 0 deletions test/fc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ export const unverifiedContactEmailAddress = (): fc.Arbitrary<UnverifiedContactE
fc.record({
type: fc.constant('unverified'),
value: emailAddress(),
verificationToken: uuid(),
})

export const verifiedContactEmailAddress = (): fc.Arbitrary<VerifiedContactEmailAddress> =>
Expand Down
62 changes: 39 additions & 23 deletions test/my-details-page/change-contact-email-address.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ describe('changeContactEmailAddress', () => {
getUser: () => M.fromEither(E.right(user)),
publicUrl,
oauth,
generateUuid: shouldNotBeCalled,
deleteContactEmailAddress: shouldNotBeCalled,
getContactEmailAddress: () => TE.fromEither(emailAddress),
saveContactEmailAddress: shouldNotBeCalled,
Expand Down Expand Up @@ -63,9 +64,10 @@ describe('changeContactEmailAddress', () => {
),
fc.user(),
fc.either(fc.constant('not-found' as const), fc.contactEmailAddress()),
fc.uuid(),
])(
'when it is different to the previous value',
async (oauth, publicUrl, [emailAddress, connection], user, existingEmailAddress) => {
async (oauth, publicUrl, [emailAddress, connection], user, existingEmailAddress, verificationToken) => {
const saveContactEmailAddress = jest.fn<_.Env['saveContactEmailAddress']>(_ => TE.right(undefined))

const actual = await runMiddleware(
Expand All @@ -74,6 +76,7 @@ describe('changeContactEmailAddress', () => {
getUser: () => M.right(user),
publicUrl,
oauth,
generateUuid: () => verificationToken,
deleteContactEmailAddress: shouldNotBeCalled,
getContactEmailAddress: () => TE.fromEither(existingEmailAddress),
saveContactEmailAddress,
Expand All @@ -91,6 +94,7 @@ describe('changeContactEmailAddress', () => {
expect(saveContactEmailAddress).toHaveBeenCalledWith(user.orcid, {
type: 'unverified',
value: emailAddress,
verificationToken,
})
},
)
Expand All @@ -117,6 +121,7 @@ describe('changeContactEmailAddress', () => {
getUser: () => M.right(user),
publicUrl,
oauth,
generateUuid: shouldNotBeCalled,
deleteContactEmailAddress: shouldNotBeCalled,
getContactEmailAddress: () => TE.right(existingEmailAddress),
saveContactEmailAddress: shouldNotBeCalled,
Expand Down Expand Up @@ -155,6 +160,7 @@ describe('changeContactEmailAddress', () => {
getUser: () => M.right(user),
publicUrl,
oauth,
generateUuid: shouldNotBeCalled,
deleteContactEmailAddress: shouldNotBeCalled,
getContactEmailAddress: () => TE.fromEither(emailAddress),
saveContactEmailAddress: shouldNotBeCalled,
Expand All @@ -180,29 +186,34 @@ describe('changeContactEmailAddress', () => {
}),
fc.user(),
fc.either(fc.constant('not-found' as const), fc.contactEmailAddress()),
])('the email address cannot be saved', async (oauth, publicUrl, connection, user, emailAddress) => {
const actual = await runMiddleware(
_.changeContactEmailAddress({
canChangeContactEmailAddress: () => true,
getUser: () => M.right(user),
publicUrl,
oauth,
deleteContactEmailAddress: () => TE.left('unavailable'),
getContactEmailAddress: () => TE.fromEither(emailAddress),
saveContactEmailAddress: () => TE.left('unavailable'),
}),
connection,
)()
fc.uuid(),
])(
'the email address cannot be saved',
async (oauth, publicUrl, connection, user, emailAddress, verificationToken) => {
const actual = await runMiddleware(
_.changeContactEmailAddress({
canChangeContactEmailAddress: () => true,
getUser: () => M.right(user),
publicUrl,
oauth,
generateUuid: () => verificationToken,
deleteContactEmailAddress: () => TE.left('unavailable'),
getContactEmailAddress: () => TE.fromEither(emailAddress),
saveContactEmailAddress: () => TE.left('unavailable'),
}),
connection,
)()

expect(actual).toStrictEqual(
E.right([
{ type: 'setStatus', status: Status.ServiceUnavailable },
{ type: 'setHeader', name: 'Cache-Control', value: 'no-store, must-revalidate' },
{ type: 'setHeader', name: 'Content-Type', value: MediaType.textHTML },
{ type: 'setBody', body: expect.anything() },
]),
)
})
expect(actual).toStrictEqual(
E.right([
{ type: 'setStatus', status: Status.ServiceUnavailable },
{ type: 'setHeader', name: 'Cache-Control', value: 'no-store, must-revalidate' },
{ type: 'setHeader', name: 'Content-Type', value: MediaType.textHTML },
{ type: 'setBody', body: expect.anything() },
]),
)
},
)

describe('when no email address is set', () => {
test.prop([
Expand All @@ -225,6 +236,7 @@ describe('changeContactEmailAddress', () => {
getUser: () => M.right(user),
publicUrl,
oauth,
generateUuid: shouldNotBeCalled,
deleteContactEmailAddress,
getContactEmailAddress: () => TE.right(existingEmailAddress),
saveContactEmailAddress: shouldNotBeCalled,
Expand Down Expand Up @@ -258,6 +270,7 @@ describe('changeContactEmailAddress', () => {
getUser: () => M.right(user),
publicUrl,
oauth,
generateUuid: shouldNotBeCalled,
deleteContactEmailAddress: shouldNotBeCalled,
getContactEmailAddress: () => TE.left('not-found'),
saveContactEmailAddress: shouldNotBeCalled,
Expand Down Expand Up @@ -286,6 +299,7 @@ describe('changeContactEmailAddress', () => {
getUser: () => M.left('no-session'),
publicUrl,
oauth,
generateUuid: shouldNotBeCalled,
deleteContactEmailAddress: shouldNotBeCalled,
getContactEmailAddress: shouldNotBeCalled,
saveContactEmailAddress: shouldNotBeCalled,
Expand Down Expand Up @@ -325,6 +339,7 @@ describe('changeContactEmailAddress', () => {
getUser: () => M.left(error),
oauth,
publicUrl,
generateUuid: shouldNotBeCalled,
deleteContactEmailAddress: shouldNotBeCalled,
getContactEmailAddress: shouldNotBeCalled,
saveContactEmailAddress: shouldNotBeCalled,
Expand Down Expand Up @@ -352,6 +367,7 @@ describe('changeContactEmailAddress', () => {
getUser: () => M.right(user),
publicUrl,
oauth,
generateUuid: shouldNotBeCalled,
deleteContactEmailAddress: shouldNotBeCalled,
getContactEmailAddress: shouldNotBeCalled,
saveContactEmailAddress: shouldNotBeCalled,
Expand Down
5 changes: 4 additions & 1 deletion test/my-details-page/verify-contact-email-address.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ describe('verifyContactEmailAddress', () => {
)
expect(canChangeContactEmailAddress).toHaveBeenCalledWith(user)
expect(getContactEmailAddress).toHaveBeenCalledWith(user.orcid)
expect(saveContactEmailAddress).toHaveBeenCalledWith(user.orcid, { ...contactEmailAddress, type: 'verified' })
expect(saveContactEmailAddress).toHaveBeenCalledWith(user.orcid, {
type: 'verified',
value: contactEmailAddress.value,
})
})

test.prop([
Expand Down

0 comments on commit 5459bc1

Please sign in to comment.