Skip to content
Merged
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
15 changes: 15 additions & 0 deletions modules/auth/controllers/auth.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -666,6 +666,21 @@ const verifyEmail = async (req, res) => {
emailVerificationToken: null,
emailVerificationExpires: null,
}, 'recover');
// Mark verified on the local object so handleSignupOrganization sees emailVerified=true
user.emailVerified = true;

// Post-verification org setup — provision org/grant if not yet done (best-effort).
// handleSignupOrganization is idempotent: if the org already exists it converges
// without double-crediting. Failure must never block email verification success.
try {
await AuthOrganizationService.handleSignupOrganization(user);
} catch (orgErr) {
logger.warn('[auth.verifyEmail] org/grant provisioning failed (non-fatal)', {
userId: user.id,
error: orgErr?.message,
});
}
Comment on lines +672 to +682
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Best-effort swallow is correct, but consider a reconciliation path for permanent provisioning failures.

Wrapping handleSignupOrganization in try/catch and logging is the right call for the "must not block verification" requirement, and the service's idempotent convergence guard makes re-invocation safe. However, when provisioning fails here the error is only logged — a verified user can end up with no organization/grants and no automatic recovery, since signin/token only call autoSetCurrentOrganization (which sets an existing membership, never creates one). Consider a reconciliation trigger (e.g., re-attempt handleSignupOrganization on next signin when the user has verified email but no active membership, or a background sweep) so a transient failure here doesn't strand the account.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@modules/auth/controllers/auth.controller.js` around lines 672 - 682, When
handleSignupOrganization fails we should record a reconciliation trigger so
provisioning is retried later; catch the error in
AuthOrganizationService.handleSignupOrganization call (the existing try/catch)
and instead of only logging, set a durable flag on the User (e.g.,
user.needsOrgProvisioning = true) or enqueue a background job to retry
provisioning, then save the user; also update signin/token flow (where
autoSetCurrentOrganization is invoked) to detect verified users with no active
membership and either call AuthOrganizationService.handleSignupOrganization
again or enqueue the same retry job so provisioning occurs on next signin;
optionally implement a periodic background sweep that queries users with
needsOrgProvisioning to re-run handleSignupOrganization and clear the flag on
success.


return responses.success(res, 'Email verified successfully')({ emailVerified: true });
} catch (err) {
responses.error(res, 422, 'Unprocessable Entity', errors.getMessage(err))(err);
Expand Down
146 changes: 146 additions & 0 deletions modules/auth/tests/auth.verifyEmail.signup-org.unit.tests.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
/**
* Module dependencies.
*/
import { jest, describe, test, expect, beforeEach } from '@jest/globals';

/**
* Unit tests for auth.controller verifyEmail() — handleSignupOrganization wiring.
*
* Verifies that:
* 1. verifyEmail calls handleSignupOrganization after the user's emailVerified flag is set.
* 2. A failure in handleSignupOrganization does NOT cause verifyEmail to fail (best-effort).
*/
describe('auth.controller verifyEmail — handleSignupOrganization wiring:', () => {
let handleSignupOrganizationMock;
let mockUserService;
let mockResponses;

beforeEach(() => {
jest.resetModules();

handleSignupOrganizationMock = jest.fn().mockResolvedValue({ _id: 'org_001' });

mockUserService = {
create: jest.fn(),
getBrut: jest.fn().mockResolvedValue({
_id: 'user_001',
id: 'user_001',
email: 'user@example.com',
emailVerificationToken: 'tok',
emailVerificationExpires: Date.now() + 3600000,
}),
update: jest.fn().mockResolvedValue({}),
remove: jest.fn(),
search: jest.fn(),
count: jest.fn().mockResolvedValue(0),
};

mockResponses = {
successCb: jest.fn(),
errorCb: jest.fn(),
};

jest.unstable_mockModule('../../../lib/services/logger.js', () => ({
default: { warn: jest.fn(), error: jest.fn(), info: jest.fn() },
}));
jest.unstable_mockModule('../../../config/index.js', () => ({
default: {
sign: { up: true, in: true },
jwt: { secret: 's', expiresIn: 3600 },
cookie: { secure: false, sameSite: 'lax' },
organizations: { enabled: true },
app: { title: 'Test', contact: 'a@b.com' },
},
}));
jest.unstable_mockModule('../../../modules/users/services/users.service.js', () => ({
default: mockUserService,
}));
jest.unstable_mockModule('../../../modules/auth/services/auth.invitation.service.js', () => ({
default: { findValid: jest.fn().mockResolvedValue(null), consume: jest.fn().mockResolvedValue(null) },
}));
jest.unstable_mockModule('../../../modules/auth/services/auth.signupCapacity.js', () => ({
computeSignupCapacity: jest.fn().mockResolvedValue({ cap: null, remaining: null }),
}));
jest.unstable_mockModule('../../../modules/users/repositories/users.repository.js', () => ({
default: { update: jest.fn() },
}));
jest.unstable_mockModule('../../../modules/organizations/services/organizations.service.js', () => ({
default: { handleSignupOrganization: handleSignupOrganizationMock },
}));
jest.unstable_mockModule('../../../modules/organizations/services/organizations.crud.service.js', () => ({
default: { autoSetCurrentOrganization: jest.fn() },
}));
jest.unstable_mockModule('../../../modules/organizations/services/organizations.membership.service.js', () => ({
default: { findByUserAndOrganization: jest.fn(), listPendingByUser: jest.fn().mockResolvedValue([]) },
}));
jest.unstable_mockModule('../../../modules/users/models/users.schema.js', () => ({
default: { User: {} },
}));
jest.unstable_mockModule('../../../lib/middlewares/model.js', () => ({
default: { getResultFromZod: jest.fn(), checkError: jest.fn() },
}));
jest.unstable_mockModule('../../../lib/middlewares/policy.js', () => ({
default: { defineAbilityFor: jest.fn().mockResolvedValue({}) },
}));
jest.unstable_mockModule('../../../lib/helpers/mailer/index.js', () => ({
default: { isConfigured: jest.fn().mockReturnValue(false), sendMail: jest.fn() },
}));
jest.unstable_mockModule('../../../lib/helpers/responses.js', () => ({
default: {
success: jest.fn().mockReturnValue(mockResponses.successCb),
error: jest.fn().mockReturnValue(mockResponses.errorCb),
},
}));
jest.unstable_mockModule('../../../lib/helpers/errors.js', () => ({
default: { getMessage: jest.fn().mockReturnValue('error') },
}));
jest.unstable_mockModule('../../../lib/helpers/AppError.js', () => ({
default: class AppError extends Error {
constructor(msg, opts) {
super(msg);
this.code = opts?.code;
this.details = opts?.details;
}
},
}));
jest.unstable_mockModule('../../../lib/helpers/abilities.js', () => ({
default: jest.fn().mockReturnValue([]),
}));
jest.unstable_mockModule('../../../lib/helpers/getBaseUrl.js', () => ({
default: jest.fn().mockReturnValue('http://localhost:3000'),
}));
jest.unstable_mockModule('../../../lib/services/analytics.js', () => ({
default: { identify: jest.fn(), capture: jest.fn(), groupIdentify: jest.fn() },
}));
});

test('calls handleSignupOrganization when email verification succeeds', async () => {
const { default: AuthController } = await import('../../../modules/auth/controllers/auth.controller.js');

const req = { params: { token: 'tok' } };
const res = {};

await AuthController.verifyEmail(req, res);

expect(handleSignupOrganizationMock).toHaveBeenCalledTimes(1);
// Must be called with a user that has emailVerified=true (marked before the call)
expect(handleSignupOrganizationMock).toHaveBeenCalledWith(
expect.objectContaining({ emailVerified: true }),
);
});

test('does not crash if handleSignupOrganization throws (best-effort)', async () => {
handleSignupOrganizationMock.mockRejectedValue(new Error('org boom'));

const { default: AuthController } = await import('../../../modules/auth/controllers/auth.controller.js');

const req = { params: { token: 'tok' } };
const res = {};

// Email verification must NOT fail because of an org-setup error
await AuthController.verifyEmail(req, res);

// The success response must still be sent
expect(mockResponses.successCb).toHaveBeenCalledWith({ emailVerified: true });
});
});
Loading