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

fix: Only allow expo XOR email reports #178

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
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
40 changes: 34 additions & 6 deletions src/backend/models/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ const UserSchema = new Schema<MongoUser>(
type: Schema.Types.String,
},
/**
* Station of the user to get the notifications. The value is an universalId,
* Station of the user to get the notifications. The value is a stationId,
* e.g. `openaq|FR1012` or `aqicn|1047`. For privacy reasons, we do not store
* the user's exact lat/lng.
*/
Expand All @@ -100,32 +100,60 @@ const UserSchema = new Schema<MongoUser>(
type: Schema.Types.String,
validate: {
message: ({ value }: { value: string }): string =>
`${value} is not a valid universalId`,
validator: (universalId: string): boolean => {
const [provider, station] = universalId.split('|');
`${value} is not a valid stationId`,
validator: (stationId: string): boolean => {
const [provider, station] = stationId.split('|');

return !!station && AllProviders.includes(provider);
},
},
},
/**
* User's Expo repots preferences.
* User's Expo reports preferences.
*/
expoReport: {
type: ExpoReportSchema,
required: function () {
return !((this as unknown) as MongoUser).emailReport;
},
validate: {
message:
'either email or expo report must be set, but not both',
validator: function () {
if (((this as unknown) as MongoUser).emailReport) {
return false;
}

return true;
},
},
},
/**
* User's email repots preferences.
*/
emailReport: {
type: EmailReportSchema,
required: function () {
return !((this as unknown) as MongoUser).expoReport;
},
validate: {
message:
'either email or expo report must be set, but not both',
validator: function () {
if (((this as unknown) as MongoUser).expoReport) {
return false;
}

return true;
},
},
},
},
{ strict: 'throw', timestamps: true }
);

// eslint-disable-next-line @typescript-eslint/no-misused-promises
UserSchema.pre('remove', async () => {
UserSchema.pre('remove', async function () {
const userId = ((this as unknown) as MongoUser)._id;

// Cascade delete all related PushTickets.
Expand Down
12 changes: 11 additions & 1 deletion src/frontend/util/sentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,14 @@ if (typeof window !== 'undefined') {
});
}

// We don't send the following errors to Sentry to not pollute it.
const UNTRACKED_ERRORS = [
// Weird amplitude error.
'0 No request sent',
// First time fetching a user
'No user with',
];

/**
* Capture an error, and send it to Sentry.
*
Expand All @@ -35,5 +43,7 @@ export function sentryException(err: Error): void {
return;
}
console.error(err);
captureException(err);
if (!UNTRACKED_ERRORS.some((msg) => err.message.includes(msg))) {
captureException(err);
}
}
54 changes: 39 additions & 15 deletions test/e2e/backend/createUser.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { connection } from 'mongoose';

import { User } from '../../../src/backend/models';
import { connectToDatabase } from '../../../src/backend/util';
import { alice, axiosConfig, BACKEND_URL } from './util/testdata';
import { alice, axiosConfig, BACKEND_URL, bob } from './util/testdata';

function testBadInput<T>(name: string, input: T, expErr: string) {
it(`should require correct input: ${name}`, async (done) => {
Expand Down Expand Up @@ -56,7 +56,7 @@ describe('users::createUser', () => {
testBadInput(
'invalid lastStationId',
{ ...alice, lastStationId: 'foo' },
'lastStationId: foo is not a valid universalId'
'lastStationId: foo is not a valid stationId'
);
testBadInput(
'no timezone',
Expand Down Expand Up @@ -86,36 +86,60 @@ describe('users::createUser', () => {
testBadInput(
'no expoPushToken',
{
...alice,
expoReport: { ...alice.expoReport, expoPushToken: undefined },
...bob,
expoReport: { ...bob.expoReport, expoPushToken: undefined },
},
'expoReport.expoPushToken: Path `expoPushToken` is required'
);
testBadInput(
'wrong expo frequency',
{ ...alice, expoReport: { ...alice.expoReport, frequency: 'foo' } },
{ ...bob, expoReport: { ...bob.expoReport, frequency: 'foo' } },
'expoReport.frequency: `foo` is not a valid enum value for path `frequency`'
);

it('should successfully create a user', async () => {
const { data } = await axios.post<MongoUser>(
`${BACKEND_URL}/api/users`,
alice,
axiosConfig
);
expect(data._id).toBeTruthy();
expect(data).toMatchObject(alice);
testBadInput(
'must set either email and expo notifications',
{ ...alice, emailReport: undefined },
'emailReport: Path `emailReport` is required., expoReport: Path `expoReport` is required.'
);

testBadInput(
'cannot set both email and expo notifications',
{ ...alice, expoReport: { expoPushToken: 'foo', frequency: 'daily' } },
'either email or expo report must be set, but not both'
);

it('should successfully create two users', async () => {
const aliceData = (
await axios.post<MongoUser>(
`${BACKEND_URL}/api/users`,
alice,
axiosConfig
)
).data;
expect(aliceData._id).toBeTruthy();
expect(aliceData).toMatchObject(alice);

const bobData = (
await axios.post<MongoUser>(
`${BACKEND_URL}/api/users`,
bob,
axiosConfig
)
).data;
expect(bobData._id).toBeTruthy();
expect(bobData).toMatchObject(bob);
});

testBadInput(
'duplicate expoPushToken',
alice,
bob,
'E11000 duplicate key error collection: shootismoke.users index: expoReport.expoPushToken_1 dup key'
);

testBadInput(
'duplicate email',
{ ...alice, expoReport: undefined },
alice,
'E11000 duplicate key error collection: shootismoke.users index: emailReport.email_1 dup key'
);

Expand Down
15 changes: 12 additions & 3 deletions test/e2e/backend/findUsersForReport.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ describe('findUsersForReport', () => {
const now = new Date('2021-01-15T08:27:21.771Z');
const users = await findUsersForReport('email', now);

expect(users.length).toBe(2);
expect(users.length).toBe(1); // Bob
});

it('should work for at UTC 21:00 on a Sunday', async () => {
Expand All @@ -55,7 +55,7 @@ describe('findUsersForReport', () => {
const now = new Date('2021-01-17T20:27:21.771Z');
const users = await findUsersForReport('expo', now);

expect(users.length).toBe(2);
expect(users.length).toBe(1); // Alice
});

it('should work for at UTC 21:00 on the 1st of the month', async () => {
Expand All @@ -65,7 +65,16 @@ describe('findUsersForReport', () => {
const now = new Date('2021-01-01T20:27:21.771Z');
const users = await findUsersForReport('expo', now);

expect(users.length).toBe(2);
expect(users.length).toBe(2); // Bob and Charlie
});

it('should work at a random time', async () => {
const now = new Date('2021-01-01T10:27:21.771Z');

let users = await findUsersForReport('expo', now);
expect(users.length).toBe(0);
users = await findUsersForReport('email', now);
expect(users.length).toBe(0);
});

afterAll(() => connection.close());
Expand Down
16 changes: 7 additions & 9 deletions test/e2e/backend/getUserByExpoPushToken.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ import { connection } from 'mongoose';

import { User } from '../../../src/backend/models';
import { connectToDatabase } from '../../../src/backend/util';
import { alice, axiosConfig, BACKEND_URL } from './util/testdata';
import { axiosConfig, BACKEND_URL, bob } from './util/testdata';

let dbAlice: MongoUser;
let dbBob: MongoUser;

describe('users::getUserByExpoPushToken', () => {
beforeAll(async (done) => {
Expand All @@ -34,11 +34,11 @@ describe('users::getUserByExpoPushToken', () => {

const { data } = await axios.post<MongoUser>(
`${BACKEND_URL}/api/users`,
alice,
bob,
axiosConfig
);

dbAlice = data;
dbBob = data;

done();
});
Expand Down Expand Up @@ -77,14 +77,12 @@ describe('users::getUserByExpoPushToken', () => {

it('should fetch correct user', async (done) => {
const { data } = await axios.get<MongoUser>(
`${BACKEND_URL}/api/users/expoPushToken/${
dbAlice?.expoReport?.expoPushToken as string
}`,
`${BACKEND_URL}/api/users/expoPushToken/${bob.expoReport.expoPushToken}`,
axiosConfig
);

expect(data._id).toBe(dbAlice._id);
expect(data).toMatchObject(dbAlice);
expect(data._id).toBe(dbBob._id);
expect(data).toMatchObject(dbBob);

done();
});
Expand Down
Loading