Skip to content

Commit a89bb4b

Browse files
committed
fix: review fixes
1 parent eec0a26 commit a89bb4b

File tree

2 files changed

+45
-31
lines changed

2 files changed

+45
-31
lines changed

src/Adapters/Auth/mfa.js

Lines changed: 41 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
* @param {Array<String>} options.options - Supported MFA methods. Must include `"SMS"` or `"TOTP"`.
77
* @param {Number} [options.digits=6] - The number of digits for the one-time password (OTP). Must be between 4 and 10.
88
* @param {Number} [options.period=30] - The validity period of the OTP in seconds. Must be greater than 10.
9-
* @param {Number} [options.emailExpiry=5*60] - The validity period of the email OTP in seconds. Must be greater than 10.
109
* @param {String} [options.algorithm="SHA1"] - The algorithm used for TOTP generation. Defaults to `"SHA1"`.
1110
* @param {Function} [options.sendSMS] - A callback function for sending SMS OTPs. Required if `"SMS"` is included in `options`.
1211
*
@@ -78,8 +77,7 @@
7877
*/
7978

8079
import { TOTP, Secret } from 'otpauth';
81-
import crypto from 'crypto';
82-
import { randomString } from '../../cryptoUtils';
80+
import { randomString, sha256Hash } from '../../cryptoUtils';
8381
import AuthAdapter from './AuthAdapter';
8482
class MFAAdapter extends AuthAdapter {
8583
validateOptions(opts) {
@@ -95,7 +93,31 @@ class MFAAdapter extends AuthAdapter {
9593
}
9694
const digits = opts.digits || 6;
9795
const period = opts.period || 30;
98-
const emailOTPExpiry = opts.emailOTPExpiry || 5*60; // Default to 5 minutes
96+
97+
// Define default periods for each method
98+
const defaultPeriods = {
99+
SMS: 30, // 5 minutes for SMS
100+
EMAIL: 30, // 5 minutes for Email
101+
TOTP: 30, // 30 seconds for TOTP
102+
};
103+
104+
if (typeof opts.period === 'number') {
105+
validOptions.forEach(method => {
106+
this.period[method] = this.period;
107+
});
108+
} else if (opts.period && typeof opts.period === 'object') {
109+
Object.keys(this.period).forEach(method => {
110+
if (this.periods.hasOwnProperty(method) && typeof this.period[method] === 'number') {
111+
this.period[method] = this.period[method] ?? defaultPeriods[method] ?? 30;
112+
}
113+
});
114+
this.period = { ...opts.period };
115+
} else {
116+
validOptions.forEach(method => {
117+
this.period[method] = defaultPeriods[method] ?? 30;
118+
});
119+
}
120+
99121
if (typeof digits !== 'number') {
100122
throw 'mfa.digits must be a number';
101123
}
@@ -108,11 +130,7 @@ class MFAAdapter extends AuthAdapter {
108130
if (period < 10) {
109131
throw 'mfa.period must be greater than 10';
110132
}
111-
if(this.email){
112-
if(this.emailOTPExpiry < 5*60){
113-
throw 'mfa.emailExpiry must be greater than 5 minutes';
114-
}
115-
}
133+
116134
const sendSMS = opts.sendSMS;
117135
const sendEmail = opts.sendEmail;
118136
if (this.email && typeof sendEmail !== 'function') {
@@ -125,14 +143,13 @@ class MFAAdapter extends AuthAdapter {
125143
this.emailCallback = sendEmail;
126144
this.digits = digits;
127145
this.period = period;
128-
this.emailOTPExpiry = emailOTPExpiry;
129146
this.algorithm = opts.algorithm || 'SHA1';
130147
}
131148
validateSetUp(mfaData) {
132149
if (mfaData.mobile && this.sms) {
133150
return this.setupMobileOTP(mfaData.mobile);
134151
}
135-
if(mfaData.email && this.email){
152+
if (mfaData.email && this.email) {
136153
return this.setupEmailOTP(mfaData.email);
137154
}
138155
if (this.totp) {
@@ -150,13 +167,11 @@ class MFAAdapter extends AuthAdapter {
150167
if (this.email && email) {
151168
if (token === 'request') {
152169
const { token: sendToken, expiry } = await this.sendEmail(email);
153-
const auth = req.original.get('authData') || {};
154170
auth.mfa = {
155171
token: sendToken,
156172
email: email,
157-
expiry: expiry
173+
expiry: expiry,
158174
};
159-
160175
// Use direct database access to avoid validation
161176
const query = new Parse.Query(Parse.User);
162177
const user = await query.get(req.object.id, { useMasterKey: true });
@@ -166,16 +181,16 @@ class MFAAdapter extends AuthAdapter {
166181
await user.save(null, {
167182
useMasterKey: true,
168183
context: { skipValidation: true },
169-
validateSave: false
184+
validateSave: false,
170185
});
171186

172-
throw new Parse.Error(209,'Please enter the token');
187+
throw new Parse.Error(209, 'Please enter the token');
173188
}
174189
if (!saved || token !== saved) {
175190
throw 'Invalid MFA token 1';
176191
}
177192
if (new Date() > expiry) {
178-
throw 'Expired MFA token';
193+
throw 'Invalid MFA token 2';
179194
}
180195
delete auth.mfa.token;
181196
delete auth.mfa.expiry;
@@ -235,14 +250,14 @@ class MFAAdapter extends AuthAdapter {
235250
}
236251
if (authData.mobile && this.sms) {
237252
if (!authData.token) {
238-
throw 'MFA is already set up on this account';
253+
throw 'Token required to confirm MFA changes.';
239254
}
240255
return this.confirmSMSOTP(authData, req.original.get('authData')?.mfa || {});
241256
}
242257

243258
if (authData.email && this.email) {
244259
if (!authData.token) {
245-
throw 'MFA is already set up on this account';
260+
throw 'Token required to confirm MFA changes.';
246261
}
247262
return this.confirmEmailOTP(authData, req.original.get('authData')?.mfa || {});
248263
}
@@ -299,11 +314,10 @@ class MFAAdapter extends AuthAdapter {
299314

300315
async setupEmailOTP(email) {
301316
const { token, expiry } = await this.sendEmail(email);
302-
const emailHash = this.md5Hash(email);
317+
const emailHash = sha256Hash(email);
303318
return {
304319
save: {
305320
pending: {
306-
// encode the email md5 has
307321
[emailHash]: {
308322
token,
309323
expiry,
@@ -328,20 +342,19 @@ class MFAAdapter extends AuthAdapter {
328342
}
329343

330344
async sendEmail(email) {
331-
const decodedEmail = email.replace(/___DOT___/g, '.')
332-
if (!/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(decodedEmail)) {
345+
if (!/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email)) {
333346
throw 'Invalid email address.';
334347
}
335348
let token = '';
336349
while (token.length < this.digits) {
337-
token += (0, _cryptoUtils.randomString)(10).replace(/\D/g, '');
350+
token += randomString(10).replace(/\D/g, '');
338351
}
339352
token = token.substring(0, this.digits);
340-
await Promise.resolve(this.emailCallback(token, decodedEmail));
341-
const expiry = new Date(new Date().getTime() + this.emailOTPExpiry * 1000);
353+
await Promise.resolve(this.emailCallback(token, email));
354+
const expiry = new Date(new Date().getTime() + this.period * 1000);
342355
return { token, expiry };
343356
}
344-
async confirmSMSOTP(inputData, authData) {
357+
async confirmSMSOTP(inputData, authData) {
345358
const { mobile, token } = inputData;
346359
if (!authData.pending?.[mobile]) {
347360
throw 'This number is not pending';
@@ -362,7 +375,7 @@ class MFAAdapter extends AuthAdapter {
362375

363376
async confirmEmailOTP(inputData, authData) {
364377
const { email, token } = inputData;
365-
const emailHash = this.md5Hash(email);
378+
const emailHash = sha256Hash(email);
366379
if (!authData.pending?.[emailHash]) {
367380
throw 'This email is not pending';
368381
}
@@ -403,8 +416,5 @@ class MFAAdapter extends AuthAdapter {
403416
save: { secret, recovery },
404417
};
405418
}
406-
md5Hash(str) {
407-
return crypto.createHash('md5').update(str).digest('hex');
408-
}
409419
}
410420
export default new MFAAdapter();

src/cryptoUtils.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,3 +45,7 @@ export function newToken(): string {
4545
export function md5Hash(string: string): string {
4646
return createHash('md5').update(string).digest('hex');
4747
}
48+
49+
export function sha256Hash(string: string): string {
50+
return crypto.createHash('sha256').update(string).digest('hex');
51+
}

0 commit comments

Comments
 (0)