Skip to content

Commit 0c0212a

Browse files
baderdeanbaderdean
authored
fix first-login rendering glitches and harden quality checks (#2614)
1 parent 512eb26 commit 0c0212a

36 files changed

+2847
-1383
lines changed

‎.husky/pre-commit‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
#!/usr/bin/env sh
2+
3+
npm run precommit:check

‎backend/src/controllers/enrichment.controller.ts‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,27 @@ async function preEnrichmentMiddleware(
2020
const { contact, enrichAllContacts, updateEmptyFieldsOnly } = req.body;
2121
const contacts = [...(req.body.contacts ?? []), contact].filter(Boolean);
2222

23-
if (!enrichAllContacts && !Array.isArray(contacts)) {
23+
if (!enrichAllContacts && contacts.length === 0) {
2424
const msg = 'Missing parameter contact or contacts';
2525
return res.status(400).json({ message: msg });
2626
}
2727

2828
try {
29-
res.locals.enrichment = await prepareForEnrichment(
29+
const enrichment = await prepareForEnrichment(
3030
user.id,
3131
enrichAllContacts ?? false,
3232
updateEmptyFieldsOnly,
33-
contacts,
34-
res
33+
contacts
3534
);
35+
36+
if (enrichment.available === 0) {
37+
return res.status(402).json({
38+
total: enrichment.total,
39+
available: enrichment.available
40+
});
41+
}
42+
43+
res.locals.enrichment = enrichment;
3644
return next();
3745
} catch (error) {
3846
return next(error);

‎backend/src/controllers/enrichment.helpers.ts‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { Response } from 'express';
21
import { EngineResponse, EngineResult } from '../services/enrichment/Engine';
32

43
import Billing from '../utils/billing-plugin';
@@ -81,20 +80,18 @@ export async function prepareForEnrichment(
8180
userId: string,
8281
enrichAll: boolean,
8382
updateEmptyFieldsOnly: boolean,
84-
contacts: Contact[],
85-
res: Response
83+
contacts: Contact[]
8684
) {
8785
const toEnrich = await getContactsToEnrich(userId, enrichAll, contacts);
8886
const { total, available } = await restrictOrDecline(userId, toEnrich.length);
89-
90-
if (available === 0) {
91-
return res.status(402).json({ total, available });
92-
}
87+
const selectedContacts = toEnrich.slice(0, available);
9388

9489
return {
90+
total,
91+
available,
9592
updateEmptyFieldsOnly,
96-
contacts: toEnrich.slice(0, available),
97-
contact: toEnrich.slice(0, available)[0]
93+
contacts: selectedContacts,
94+
contact: selectedContacts[0]
9895
};
9996
}
10097

@@ -178,7 +175,7 @@ export async function enrichPersonSync(
178175
enriched.add(result.data[0].email);
179176
}
180177

181-
if (enriched.size > 1) {
178+
if (enriched.size > 0) {
182179
await Billing?.deductCustomerCredits(task.userId, enriched.size);
183180
}
184181

‎backend/src/controllers/imap.controller.ts‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,10 @@ export default function initializeImapController(miningSources: MiningSources) {
6767
.json({ message: `Invalid input: ${errors.join(', ')}` });
6868
}
6969

70+
let imapConnection: Awaited<
71+
ReturnType<typeof ImapConnectionProvider.getSingleConnection>
72+
> | null = null;
73+
7074
try {
7175
const userId = (res.locals.user as User).id;
7276
const data = await miningSources.getCredentialsBySourceEmail(
@@ -106,7 +110,7 @@ export default function initializeImapController(miningSources: MiningSources) {
106110
}
107111
}
108112

109-
const imapConnection = await ImapConnectionProvider.getSingleConnection(
113+
imapConnection = await ImapConnectionProvider.getSingleConnection(
110114
email,
111115
'accessToken' in data
112116
? {
@@ -123,8 +127,6 @@ export default function initializeImapController(miningSources: MiningSources) {
123127
const imapBoxesFetcher = new ImapBoxesFetcher(imapConnection, logger);
124128
const tree: any = await imapBoxesFetcher.getTree(data.email);
125129

126-
await imapConnection.logout();
127-
128130
logger.info('Mining IMAP tree succeeded.', {
129131
metadata: {
130132
user: hashEmail(data.email, userId)
@@ -152,6 +154,17 @@ export default function initializeImapController(miningSources: MiningSources) {
152154
return res.status(generatedError.status).send(generatedError);
153155
}
154156
return next(generatedError);
157+
} finally {
158+
if (imapConnection) {
159+
try {
160+
await imapConnection.logout();
161+
} catch (logoutError) {
162+
logger.warn(
163+
'Unable to close IMAP connection cleanly.',
164+
logoutError
165+
);
166+
}
167+
}
155168
}
156169
}
157170
};

‎backend/src/controllers/mining.controller.ts‎

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,39 @@ async function exchangeForToken(
5858
};
5959
}
6060

61+
function getSafeRedirectPath(path: unknown) {
62+
if (
63+
typeof path !== 'string' ||
64+
!path.startsWith('/') ||
65+
path.startsWith('//')
66+
) {
67+
return '/';
68+
}
69+
70+
return path;
71+
}
72+
73+
function parseOAuthState(state: string | undefined) {
74+
if (!state) {
75+
throw new Error('Missing OAuth state.');
76+
}
77+
78+
const decoded = Buffer.from(state, 'base64').toString('utf-8');
79+
const parsed = JSON.parse(decoded) as {
80+
userId?: string;
81+
afterCallbackRedirect?: string;
82+
};
83+
84+
if (!parsed.userId) {
85+
throw new Error('Invalid OAuth state payload.');
86+
}
87+
88+
return {
89+
userId: parsed.userId,
90+
afterCallbackRedirect: getSafeRedirectPath(parsed.afterCallbackRedirect)
91+
};
92+
}
93+
6194
async function publishPreviouslyUnverifiedEmailsToCleaning(
6295
contacts: Contacts,
6396
userId: string,
@@ -137,10 +170,11 @@ export default function initializeMiningController(
137170
const user = res.locals.user as User;
138171
const provider = req.params.provider as OAuthMiningSourceProvider;
139172
const { redirect } = req.body;
173+
const afterCallbackRedirect = getSafeRedirectPath(redirect);
140174

141175
const stateObj = JSON.stringify({
142176
userId: user.id,
143-
afterCallbackRedirect: redirect ?? '/'
177+
afterCallbackRedirect
144178
});
145179

146180
const authorizationUri = getAuthClient(provider).authorizeURL({
@@ -156,15 +190,7 @@ export default function initializeMiningController(
156190
const provider = req.params.provider as OAuthMiningSourceProvider;
157191
let redirect = '/';
158192
try {
159-
const {
160-
userId,
161-
afterCallbackRedirect
162-
}: {
163-
userId: string;
164-
afterCallbackRedirect: string;
165-
} = JSON.parse(
166-
Buffer.from(state as string, 'base64').toString('utf-8')
167-
);
193+
const { userId, afterCallbackRedirect } = parseOAuthState(state);
168194

169195
redirect = afterCallbackRedirect;
170196
const exchangedTokens = await exchangeForToken(code, provider);

‎backend/src/controllers/mining.helpers.ts‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -138,21 +138,12 @@ export function validateFileContactsData(
138138
throw new Error('Invalid email found in the contacts data');
139139
}
140140

141-
const URL_LIST = [
142-
contact.image?.split('@'),
143-
contact.same_as?.split('@')
144-
].filter((list) => list?.filter(Boolean)?.length);
141+
const URL_LIST = [contact.image?.split(','), contact.same_as?.split(',')]
142+
.map((list) => list?.map((url) => url.trim()).filter(Boolean))
143+
.filter((list) => list?.length);
145144
URL_LIST.forEach((list) => {
146-
if (list?.length) {
147-
if (typeof list === 'string') {
148-
if (!isValidURL(list)) {
149-
throw new Error('Invalid URL found in the contacts data');
150-
}
151-
} else if (Array.isArray(list)) {
152-
if (!list.every((url) => isValidURL(url))) {
153-
throw new Error('Invalid URL found in the contacts data');
154-
}
155-
}
145+
if (list?.length && !list.every((url) => isValidURL(url))) {
146+
throw new Error('Invalid URL found in the contacts data');
156147
}
157148
});
158149
});

‎backend/src/services/tasks-manager/TaskManagerFile.ts‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,28 @@ export default class TasksManagerFile {
117117

118118
// Set up the Redis subscriber to listen for updates
119119
this.redisSubscriber.on('message', async (_channel, data) => {
120-
const { miningId, progressType, count } = JSON.parse(data);
120+
let message: {
121+
miningId?: string;
122+
progressType?: keyof TaskProcessProgress;
123+
count?: number;
124+
};
125+
126+
try {
127+
message = JSON.parse(data);
128+
} catch (error) {
129+
logger.warn('Ignoring malformed Redis progress payload.', {
130+
data,
131+
error
132+
});
133+
return;
134+
}
135+
136+
const { miningId, progressType, count } = message;
137+
138+
if (!miningId || !progressType || typeof count !== 'number') {
139+
logger.warn('Ignoring incomplete Redis progress payload.', { data });
140+
return;
141+
}
121142

122143
if (count > 0) {
123144
this.updateProgress(miningId, progressType, count);

‎backend/src/services/tasks-manager/TasksManager.ts‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,31 @@ export default class TasksManager {
5454

5555
// Set up the Redis subscriber to listen for updates
5656
this.redisSubscriber.on('message', async (_channel, data) => {
57+
let message: {
58+
miningId?: string;
59+
progressType?: TaskProgressType;
60+
count?: number;
61+
isCompleted?: boolean;
62+
isCanceled?: boolean;
63+
};
64+
65+
try {
66+
message = JSON.parse(data);
67+
} catch (error) {
68+
logger.warn('Ignoring malformed Redis progress payload.', {
69+
data,
70+
error
71+
});
72+
return;
73+
}
74+
5775
const { miningId, progressType, count, isCompleted, isCanceled } =
58-
JSON.parse(data);
76+
message;
77+
78+
if (!miningId || !progressType || typeof count !== 'number') {
79+
logger.warn('Ignoring incomplete Redis progress payload.', { data });
80+
return;
81+
}
5982

6083
if (progressType === 'signatures' && isCompleted) {
6184
const task = this.ACTIVE_MINING_TASKS.get(miningId);

‎backend/src/services/tasks-manager/TasksManagerPST.ts‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,31 @@ export default class TasksManagerPST {
5353

5454
// Set up the Redis subscriber to listen for updates
5555
this.redisSubscriber.on('message', async (_channel, data) => {
56+
let message: {
57+
miningId?: string;
58+
progressType?: TaskProgressType;
59+
count?: number;
60+
isCompleted?: boolean;
61+
isCanceled?: boolean;
62+
};
63+
64+
try {
65+
message = JSON.parse(data);
66+
} catch (error) {
67+
logger.warn('Ignoring malformed Redis progress payload.', {
68+
data,
69+
error
70+
});
71+
return;
72+
}
73+
5674
const { miningId, progressType, count, isCompleted, isCanceled } =
57-
JSON.parse(data);
75+
message;
76+
77+
if (!miningId || !progressType || typeof count !== 'number') {
78+
logger.warn('Ignoring incomplete Redis progress payload.', { data });
79+
return;
80+
}
5881

5982
if (progressType === 'signatures' && isCompleted) {
6083
const task = this.ACTIVE_MINING_TASKS.get(miningId);

‎backend/src/utils/pubsub/redis/RedisSubscriber.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,14 @@ export default class RedisSubscriber<T> implements Subscriber<T> {
1919
this.logger.info(`Subscribed to redis channel ${this.channel}`);
2020
});
2121
this.redisClient.on('message', (_, data: string) => {
22-
onMessage(JSON.parse(data) as T);
22+
try {
23+
onMessage(JSON.parse(data) as T);
24+
} catch (error) {
25+
this.logger.warn(
26+
`Ignoring malformed Redis message on channel ${this.channel}`,
27+
error
28+
);
29+
}
2330
});
2431
} catch (error) {
2532
this.logger.error(

0 commit comments

Comments
 (0)