diff --git a/api/server/controllers/AuthController.js b/api/server/controllers/AuthController.js index 47f5faf5b7..320f72f860 100644 --- a/api/server/controllers/AuthController.js +++ b/api/server/controllers/AuthController.js @@ -19,7 +19,13 @@ const { setAuthTokens, registerUser, } = require('~/server/services/AuthService'); -const { deleteAllUserSessions, getUserById, findSession, updateUser } = require('~/models'); +const { + deleteAllUserSessions, + getUserById, + findSession, + updateUser, + deleteTokens, +} = require('~/models'); const { getGraphApiToken } = require('~/server/services/GraphTokenService'); const { getRefreshTokenBridge } = require('~/server/services/RefreshTokenBridge'); const { @@ -52,6 +58,18 @@ const registrationController = async (req, res) => { try { const response = await registerUser(req.body); const { status, message } = response; + /** Consume the invite only once the account exists. `registerUser` returns the same + * 200 whether it created a user or found the email already in use, so the decision + * rests on `userCreated` rather than the status. A failure to delete leaves a + * usable invite, which is recoverable; failing the response here would tell a user + * whose account was just created that registration failed, which is not. */ + if (response.userCreated === true && req.invite?.token != null) { + try { + await deleteTokens({ token: req.invite.token }); + } catch (error) { + logger.error('[registrationController] Failed to consume invite after registration', error); + } + } res.status(status).send({ message }); } catch (err) { logger.error('[registrationController]', err); diff --git a/api/server/controllers/AuthController.spec.js b/api/server/controllers/AuthController.spec.js index d597e82edc..08d86646b1 100644 --- a/api/server/controllers/AuthController.spec.js +++ b/api/server/controllers/AuthController.spec.js @@ -45,6 +45,7 @@ jest.mock('~/models', () => ({ findSession: jest.fn(), updateUser: jest.fn(), findUser: jest.fn(), + deleteTokens: jest.fn(), })); jest.mock('~/server/services/RefreshTokenBridge', () => ({ OPENID_REFRESH_BRIDGE_GRACE_MS: 60 * 1000, @@ -119,7 +120,11 @@ const openIdClient = require('openid-client'); const jwt = require('jsonwebtoken'); const { logger } = require('@librechat/data-schemas'); const { isEnabled, findOpenIDUser, buildOpenIDRefreshParams } = require('@librechat/api'); -const { graphTokenController, refreshController } = require('./AuthController'); +const { + graphTokenController, + refreshController, + registrationController, +} = require('./AuthController'); const { getGraphApiToken } = require('~/server/services/GraphTokenService'); const { clearOpenIDAuthTokens, @@ -128,9 +133,10 @@ const { storeOpenIDSession, setCloudFrontAuthCookies, setAuthTokens, + registerUser, } = require('~/server/services/AuthService'); const { getOpenIdConfig, getOpenIdEmail } = require('~/strategies'); -const { deleteSession, getUserById, findSession, updateUser } = require('~/models'); +const { deleteSession, getUserById, findSession, updateUser, deleteTokens } = require('~/models'); const { createRefreshTokenBridgeFlightKey, deleteRefreshTokenBridges, @@ -2361,3 +2367,77 @@ describe('refreshController – LibreChat path', () => { }); }); }); + +describe('registrationController - invite consumption', () => { + const invite = { token: 'hashed-invite', email: 'invitee@example.com' }; + + const buildRes = () => { + const res = {}; + res.status = jest.fn(() => res); + res.send = jest.fn(() => res); + res.json = jest.fn(() => res); + return res; + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('consumes the invite once the account exists', async () => { + registerUser.mockResolvedValue({ status: 200, message: 'ok', userCreated: true }); + + await registrationController({ body: {}, invite }, buildRes()); + + expect(deleteTokens).toHaveBeenCalledWith({ token: 'hashed-invite' }); + }); + + it('leaves the invite when registration is rejected', () => { + /** A mistyped password confirmation is the common case; it has to stay retryable. */ + registerUser.mockResolvedValue({ status: 404, message: 'The passwords did not match' }); + + return registrationController({ body: {}, invite }, buildRes()).then(() => { + expect(deleteTokens).not.toHaveBeenCalled(); + }); + }); + + it('leaves the invite when the email is already in use, despite the 200', async () => { + /** `registerUser` returns the same status and message whether it created an account + * or found the email taken, so the status alone cannot drive this decision. */ + registerUser.mockResolvedValue({ status: 200, message: 'ok' }); + + await registrationController({ body: {}, invite }, buildRes()); + + expect(deleteTokens).not.toHaveBeenCalled(); + }); + + it('does not attempt a deletion for an uninvited registration', async () => { + registerUser.mockResolvedValue({ status: 200, message: 'ok', userCreated: true }); + + await registrationController({ body: {} }, buildRes()); + + expect(deleteTokens).not.toHaveBeenCalled(); + }); + + it('still reports success when consuming the invite fails', async () => { + /** The account exists by this point; reporting failure would be worse than + * leaving a usable invite behind. */ + registerUser.mockResolvedValue({ status: 200, message: 'ok', userCreated: true }); + deleteTokens.mockRejectedValue(new Error('mongo unavailable')); + const res = buildRes(); + + await registrationController({ body: {}, invite }, res); + + expect(res.status).toHaveBeenCalledWith(200); + expect(res.send).toHaveBeenCalledWith({ message: 'ok' }); + expect(logger.error).toHaveBeenCalled(); + }); + + it('never forwards the creation signal to the client', async () => { + registerUser.mockResolvedValue({ status: 200, message: 'ok', userCreated: true }); + const res = buildRes(); + + await registrationController({ body: {}, invite }, res); + + expect(res.send).toHaveBeenCalledWith({ message: 'ok' }); + }); +}); diff --git a/api/server/middleware/checkInviteUser.js b/api/server/middleware/checkInviteUser.js index 22f2824ffc..bb3fd778a8 100644 --- a/api/server/middleware/checkInviteUser.js +++ b/api/server/middleware/checkInviteUser.js @@ -1,5 +1,5 @@ const { getInvite: getInviteFn } = require('@librechat/api'); -const { createToken, findToken, deleteTokens } = require('~/models'); +const { createToken, findToken } = require('~/models'); const getInvite = (encodedToken, email) => getInviteFn(encodedToken, email, { createToken, findToken }); @@ -19,7 +19,11 @@ async function checkInviteUser(req, res, next) { return res.status(400).json({ message: 'Invalid invite token' }); } - await deleteTokens({ token: invite.token }); + /** The invite is deliberately left in place here. Everything that can still + * reject this registration — the schema, the allowed-domain check, an email + * already in use — runs after this middleware, and consuming the invite first + * meant a mistyped password confirmation destroyed it. `registrationController` + * deletes it once an account actually exists. */ req.invite = invite; next(); } catch (error) { diff --git a/api/server/middleware/checkInviteUser.spec.js b/api/server/middleware/checkInviteUser.spec.js new file mode 100644 index 0000000000..cc8b31f41b --- /dev/null +++ b/api/server/middleware/checkInviteUser.spec.js @@ -0,0 +1,70 @@ +const mockGetInvite = jest.fn(); +const mockDeleteTokens = jest.fn(); + +jest.mock('@librechat/api', () => ({ + getInvite: (...args) => mockGetInvite(...args), +})); + +jest.mock('~/models', () => ({ + createToken: jest.fn(), + findToken: jest.fn(), + deleteTokens: (...args) => mockDeleteTokens(...args), +})); + +const checkInviteUser = require('./checkInviteUser'); + +const buildRes = () => { + const res = {}; + res.status = jest.fn(() => res); + res.json = jest.fn(() => res); + return res; +}; + +describe('checkInviteUser', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('does not consume the invite, so a later failure cannot destroy it', async () => { + /** The reported bug: the invite was deleted here, before the schema, the + * allowed-domain check or the email-in-use check had run. A mistyped password + * confirmation left the invitee with no invite and no account. */ + mockGetInvite.mockResolvedValue({ token: 'hashed-invite', email: 'invitee@example.com' }); + const req = { body: { token: 'raw-token', email: 'invitee@example.com' } }; + const next = jest.fn(); + + await checkInviteUser(req, buildRes(), next); + + expect(mockDeleteTokens).not.toHaveBeenCalled(); + expect(next).toHaveBeenCalledTimes(1); + }); + + it('hands the validated invite to the rest of the chain', async () => { + const invite = { token: 'hashed-invite', email: 'invitee@example.com' }; + mockGetInvite.mockResolvedValue(invite); + const req = { body: { token: 'raw-token', email: 'invitee@example.com' } }; + + await checkInviteUser(req, buildRes(), jest.fn()); + + expect(req.invite).toBe(invite); + }); + + it('rejects an invalid invite without touching the token store', async () => { + mockGetInvite.mockResolvedValue(null); + const res = buildRes(); + + await checkInviteUser({ body: { token: 'raw-token', email: 'a@b.c' } }, res, jest.fn()); + + expect(res.status).toHaveBeenCalledWith(400); + expect(mockDeleteTokens).not.toHaveBeenCalled(); + }); + + it('passes straight through when no invite token is supplied', async () => { + const next = jest.fn(); + + await checkInviteUser({ body: {} }, buildRes(), next); + + expect(next).toHaveBeenCalledTimes(1); + expect(mockGetInvite).not.toHaveBeenCalled(); + }); +}); diff --git a/api/server/services/AuthService.js b/api/server/services/AuthService.js index fb736ded81..911b7ebdc4 100644 --- a/api/server/services/AuthService.js +++ b/api/server/services/AuthService.js @@ -443,7 +443,10 @@ const registerUser = async (user, additionalData = {}) => { await updateUser(newUserId, { emailVerified: true }); } - return { status: 200, message: genericVerificationMessage }; + /** `userCreated` separates this from the identical 200 returned when the email is + * already in use, so a caller can act on an account having actually been created + * without the response body revealing which of the two happened. */ + return { status: 200, message: genericVerificationMessage, userCreated: true }; } catch (err) { logger.error('[registerUser] Error in registering user:', err); if (newUserId) {