Skip to content

Commit e03232a

Browse files
fix(auth,users): address second round of CodeRabbit review
- Use centralized mails.isConfigured() instead of duplicated logic - Add getBaseUrl() helper for defensive cors.origin access - Add @returns {void} to auth route registrar JSDoc - Add full JSDoc for optionalJwt middleware - Add .exec() to findByEmail for consistency with other repo methods
1 parent c658417 commit e03232a

3 files changed

Lines changed: 25 additions & 4 deletions

File tree

modules/auth/controllers/auth.controller.js

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,23 @@ const tokenCookieOptions = {
2626
};
2727

2828
/**
29-
* @desc Check whether the mailer is configured with a real sender address
29+
* @desc Check whether the mailer is configured with a real sender address.
30+
* Delegates to the centralized helper in lib/helpers/mailer.
3031
* @returns {boolean} true when SMTP mail sending is available
3132
*/
32-
const isMailerConfigured = () => !!(config.mailer && config.mailer.from && !config.mailer.from.startsWith('DEVKIT_NODE_'));
33+
const isMailerConfigured = () => mails.isConfigured();
34+
35+
/**
36+
* @desc Resolve the first CORS origin as a base URL string.
37+
* Handles both array and string forms of config.cors.origin.
38+
* @returns {string} The base URL for building client-facing links.
39+
*/
40+
const getBaseUrl = () => {
41+
const origin = config.cors?.origin;
42+
if (Array.isArray(origin) && origin.length > 0) return origin[0];
43+
if (typeof origin === 'string') return origin;
44+
return '';
45+
};
3346

3447
/**
3548
* @desc Send a verification email to the user with a signed token link
@@ -44,7 +57,7 @@ const sendVerificationEmail = async (user, verificationToken) => {
4457
subject: 'Verify your email address',
4558
params: {
4659
displayName: `${user.firstName} ${user.lastName}`,
47-
url: `${config.cors.origin[0]}/verify-email?token=${verificationToken}`,
60+
url: `${getBaseUrl()}/verify-email?token=${verificationToken}`,
4861
appName: config.app.title,
4962
appContact: config.app.contact,
5063
},

modules/auth/routes/auth.routes.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,19 @@ import authPassword from '../controllers/auth.password.controller.js';
1212
/**
1313
* Register authentication routes on the Express application.
1414
* @param {Object} app - Express application instance
15+
* @returns {void}
1516
*/
1617
export default (app) => {
1718
const authLimiter = limiters.auth;
1819

1920
// Auth config — optional JWT: public fields for everyone, org details for authenticated users
21+
/**
22+
* @desc Middleware that optionally authenticates via JWT, attaching user if valid but allowing unauthenticated requests.
23+
* @param {Object} req - Express request object
24+
* @param {Object} res - Express response object
25+
* @param {Function} next - Express next middleware function
26+
* @returns {void}
27+
*/
2028
const optionalJwt = (req, res, next) => {
2129
passport.authenticate('jwt', { session: false }, (err, user) => {
2230
if (user) req.user = user;

modules/users/repositories/users.repository.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ const searchByNameOrEmail = (search) => {
154154
* @param {String} email - The email to search for
155155
* @returns {Promise<Object|null>} The matching user or null
156156
*/
157-
const findByEmail = (email) => User.findOne({ email });
157+
const findByEmail = (email) => User.findOne({ email }).exec();
158158

159159
/**
160160
* @desc Function to update a user by ID with a partial update object

0 commit comments

Comments
 (0)