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
1 change: 1 addition & 0 deletions apps/meteor/app/api/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import './v1/teams';
import './v1/moderation';
import './v1/uploads';
import './v1/twoFactorChallenges';
import './v1/loginCode';
// This has to come last so all endpoints are registered before generating the OpenAPI documentation
import './default/openApi';

Expand Down
71 changes: 71 additions & 0 deletions apps/meteor/app/api/server/v1/loginCode.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { LoginCodes } from '@rocket.chat/models';
import { ajv, validateBadRequestErrorResponse, validateUnauthorizedErrorResponse } from '@rocket.chat/rest-typings';
import type { JSONSchemaType } from 'ajv';
import { Accounts } from 'meteor/accounts-base';

import type { ExtractRoutesFromAPI } from '../ApiClass';
import { API } from '../api';

const loginCodeRedeemResponse = ajv.compile<{ loginToken: string; userId: string }>({
type: 'object',
properties: {
loginToken: { type: 'string' },
userId: { type: 'string' },
success: { type: 'boolean', enum: [true] },
},
required: ['loginToken', 'userId', 'success'],
additionalProperties: false,
});

type LoginCodeRedeemParams = { code: string };

const LoginCodeRedeemSchema: JSONSchemaType<LoginCodeRedeemParams> = {
type: 'object',
properties: {
code: { type: 'string', minLength: 64, maxLength: 64 },
Comment thread
yash-rajpal marked this conversation as resolved.
},
required: ['code'],
additionalProperties: false,
};

const isLoginCodeRedeemParamsPOST = ajv.compile<LoginCodeRedeemParams>(LoginCodeRedeemSchema);

const loginCodeEndpoints = API.v1.post(
'loginCode.redeem',
{
authRequired: false,
body: isLoginCodeRedeemParamsPOST,
rateLimiterOptions: { intervalTimeInMS: 60000, numRequestsAllowed: 10 },
response: {
200: loginCodeRedeemResponse,
400: validateBadRequestErrorResponse,
401: validateUnauthorizedErrorResponse,
},
},
async function action() {
const { code } = this.bodyParams;

const loginCode = await LoginCodes.findOneNotExpiredByCodeAndDelete(code);

if (!loginCode) {
return API.v1.failure('error-invalid-code');
}

const { userId } = loginCode;

const stampedToken = Accounts._generateStampedLoginToken();
await Accounts._insertLoginToken(userId, stampedToken);

return API.v1.success({
loginToken: stampedToken.token,
userId,
});
},
);

type LoginCodeEndpoints = ExtractRoutesFromAPI<typeof loginCodeEndpoints>;

declare module '@rocket.chat/rest-typings' {
// eslint-disable-next-line @typescript-eslint/naming-convention, @typescript-eslint/no-empty-interface
interface Endpoints extends LoginCodeEndpoints {}
}
2 changes: 2 additions & 0 deletions apps/meteor/client/views/root/AppLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { useLoginOtherClients } from './hooks/useLoginOtherClients';
import { useLoginViaQuery } from './hooks/useLoginViaQuery';
import { useMessageLinkClicks } from './hooks/useMessageLinkClicks';
import { useNotificationPermission } from './hooks/useNotificationPermission';
import { useOAuthLogin } from './hooks/useOAuthLogin';
import { useRedirectToSetupWizard } from './hooks/useRedirectToSetupWizard';
import { useSettingsOnLoadSiteUrl } from './hooks/useSettingsOnLoadSiteUrl';
import { useShareSessionWithOtherClients } from './hooks/useShareSessionWithOtherClients';
Expand Down Expand Up @@ -73,6 +74,7 @@ const AppLayout = () => {
useCodeHighlight();
useLoginViaQuery();
useLoginOtherClients();
useOAuthLogin();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1: OAuth login error handling is overridden by an unconditional /home redirect in finally, causing incorrect post-failure navigation.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/client/views/root/AppLayout.tsx, line 77:

<comment>OAuth login error handling is overridden by an unconditional `/home` redirect in `finally`, causing incorrect post-failure navigation.</comment>

<file context>
@@ -73,6 +74,7 @@ const AppLayout = () => {
 	useCodeHighlight();
 	useLoginViaQuery();
 	useLoginOtherClients();
+	useOAuthLogin();
 	useShareSessionWithOtherClients();
 	useLoadMissedMessages();
</file context>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1: Adding useOAuthLogin alongside useLoginViaQuery introduces a concurrent dual-login race when both query credentials are present.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/client/views/root/AppLayout.tsx, line 77:

<comment>Adding `useOAuthLogin` alongside `useLoginViaQuery` introduces a concurrent dual-login race when both query credentials are present.</comment>

<file context>
@@ -73,6 +74,7 @@ const AppLayout = () => {
 	useCodeHighlight();
 	useLoginViaQuery();
 	useLoginOtherClients();
+	useOAuthLogin();
 	useShareSessionWithOtherClients();
 	useLoadMissedMessages();
</file context>

useShareSessionWithOtherClients();
useLoadMissedMessages();
useDesktopFavicon();
Expand Down
7 changes: 1 addition & 6 deletions apps/meteor/client/views/root/hooks/useLoginViaQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,12 @@ export const useLoginViaQuery = () => {

useEffect(() => {
const handleLogin = async () => {
const { resumeToken, loginClient } = router.getSearchParameters();
const { resumeToken } = router.getSearchParameters();

if (!resumeToken) {
return;
Comment thread
yash-rajpal marked this conversation as resolved.
}

//Case handled by useLoginOtherClients, we don't want to login here.
if (loginClient) {
return;
}

try {
await loginWithToken(resumeToken);

Expand Down
46 changes: 46 additions & 0 deletions apps/meteor/client/views/root/hooks/useOAuthLogin.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { useEndpoint, useRouter, useSearchParameter, useLoginWithToken } from '@rocket.chat/ui-contexts';
import { useMutation } from '@tanstack/react-query';
import { useEffect } from 'react';

import { buildDeepLinkURL } from '../../../lib/buildAuthDeeplinkURL';

export const useOAuthLogin = () => {
const router = useRouter();
const loginCode = useSearchParameter('loginCode');
const loginClient = useSearchParameter('loginClient');
const redeemLoginCode = useEndpoint('POST', '/v1/loginCode.redeem');
const loginWithToken = useLoginWithToken();

const { mutate: redeemLoginCodeMutation } = useMutation({
mutationFn: async (loginCode: string) => {
const { loginToken, userId } = await redeemLoginCode({ code: loginCode });

if (!loginToken || !userId) {
throw new Error('Invalid response from login code redemption');
}

return { loginToken, userId };
},
onSuccess: async ({ loginToken, userId }) => {
if (loginClient === 'desktop' || loginClient === 'mobile') {
window.location.href = buildDeepLinkURL(loginToken, userId);
return;
}

await loginWithToken(loginToken);
router.navigate('/home', { replace: true });
},
onError: (error) => {
console.error('Failed to redeem login code for client redirect', error);
router.navigate('/login', { replace: true });
},
});

useEffect(() => {
if (!loginCode) {
return;
}

redeemLoginCodeMutation(loginCode);
}, [loginCode, redeemLoginCodeMutation]);
};
8 changes: 3 additions & 5 deletions apps/meteor/server/lib/oauth/passportOAuthCallback.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { IUser } from '@rocket.chat/core-typings';
import { Logger } from '@rocket.chat/logger';
import { LoginCodes } from '@rocket.chat/models';
import type { Request, Response } from 'express';
import { Accounts } from 'meteor/accounts-base';

import { doesUserRequire2FA } from './twoFactorAuth';

Expand Down Expand Up @@ -29,13 +29,11 @@ export const passportOAuthCallback = (siteUrl: string) => async (req: Request, r
return res.redirect(twoFARedirectUrl.toString());
}

const stampedToken = Accounts._generateStampedLoginToken();
await Accounts._insertLoginToken(oAuthUser._id, stampedToken);
const loginCode = await LoginCodes.createCode(oAuthUser._id);

const redirectUrl = new URL(`/home`, siteUrl);

redirectUrl.searchParams.set('resumeToken', stampedToken.token);
redirectUrl.searchParams.set('userId', oAuthUser._id);
redirectUrl.searchParams.set('loginCode', loginCode);

if (loginClient) {
redirectUrl.searchParams.set('loginClient', loginClient);
Expand Down
2 changes: 2 additions & 0 deletions apps/meteor/server/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import {
OAuthAccessTokensRaw,
OAuthAppsRaw,
OAuthAuthCodesRaw,
LoginCodesRaw,
OAuthRefreshTokensRaw,
OEmbedCacheRaw,
PermissionsRaw,
Expand Down Expand Up @@ -138,6 +139,7 @@ registerModel('INpsVoteModel', new NpsVoteRaw(db));
registerModel('IOAuthAccessTokensModel', new OAuthAccessTokensRaw(db));
registerModel('IOAuthAppsModel', new OAuthAppsRaw(db));
registerModel('IOAuthAuthCodesModel', new OAuthAuthCodesRaw(db));
registerModel('ILoginCodesModel', new LoginCodesRaw(db));
registerModel('IOAuthRefreshTokensModel', new OAuthRefreshTokensRaw(db));
registerModel('IOEmbedCacheModel', new OEmbedCacheRaw(db));
registerModel('IPermissionsModel', new PermissionsRaw(db, trashCollection));
Expand Down
140 changes: 140 additions & 0 deletions apps/meteor/tests/end-to-end/api/login-code.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
import { randomBytes } from 'crypto';

import type { ILoginCode, IUser } from '@rocket.chat/core-typings';
import { expect } from 'chai';
import { after, before, describe, it } from 'mocha';
import { MongoClient } from 'mongodb';

import { api, request, getCredentials } from '../../data/api-data';
import type { TestUser } from '../../data/users.helper';
import { createUser, deleteUser } from '../../data/users.helper';
import { URL_MONGODB } from '../../e2e/config/constants';

const CODE_TTL_MS = 60 * 1000;
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.

async function insertLoginCode(connection: MongoClient, userId: string, insertExpiredToken: boolean = false): Promise<string> {
const now = new Date();
const code = randomBytes(32).toString('hex');

let expireAt = new Date(now.getTime() + CODE_TTL_MS);

if (insertExpiredToken) {
expireAt = new Date(now.getTime() - 1);
}

await connection.db().collection<ILoginCode>('rocketchat_login_codes').insertOne({
_id: code,
userId,
createdAt: now,
expireAt,
});

return code;
}

describe('[Login Codes]', () => {
let connection: MongoClient;

before((done) => getCredentials(done));

before(async () => {
connection = await MongoClient.connect(URL_MONGODB);
});

after(async () => {
await connection.close();
});

describe('POST [/loginCode.redeem]', () => {
let testUser: TestUser<IUser>;

before(async () => {
testUser = await createUser();
});

after(async () => {
await deleteUser(testUser);
});

it('should fail when code does not exist in the database', async () => {
await request
.post(api('loginCode.redeem'))
.send({ code: 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2' })
.expect(400)
.expect((res) => {
expect(res.body).to.have.property('success', false);
});
});

it('should fail when code is an empty string', async () => {
await request
.post(api('loginCode.redeem'))
.send({ code: '' })
.expect(400)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('errorType', 'invalid-params');
});
});

it('should return a loginToken and userId for a valid code', async () => {
const code = await insertLoginCode(connection, testUser._id);

await request
.post(api('loginCode.redeem'))
.send({ code })
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('loginToken').that.is.a('string').and.is.not.empty;
expect(res.body).to.have.property('userId', testUser._id);
});
});

it('should invalidate a code after it has been redeemed', async () => {
const code = await insertLoginCode(connection, testUser._id);

await request.post(api('loginCode.redeem')).send({ code }).expect(200);

await request
.post(api('loginCode.redeem'))
.send({ code })
.expect(400)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('error', 'error-invalid-code');
});
});

it('should return a loginToken that can be used for subsequent authenticated requests', async () => {
const code = await insertLoginCode(connection, testUser._id);

const redeemRes = await request.post(api('loginCode.redeem')).send({ code }).expect(200);

const { loginToken, userId } = redeemRes.body;

await request
.get(api('me'))
.set('X-Auth-Token', loginToken)
.set('X-User-Id', userId)
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('_id', testUser._id);
});
});

it('should fail when code is expired', async () => {
const code = await insertLoginCode(connection, testUser._id, true);

await request
.post(api('loginCode.redeem'))
.send({ code })
.expect(400)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('error', 'error-invalid-code');
});
});
});
});
6 changes: 6 additions & 0 deletions packages/core-typings/src/ILoginCode.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export interface ILoginCode {
_id: string;
userId: string;
createdAt: Date;
expireAt: Date;
}
1 change: 1 addition & 0 deletions packages/core-typings/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export type * from './IEmojiCustom';
export type * from './ICustomEmojiDescriptor';
export type * from './IAnalytics';
export type * from './ICredentialToken';
export type * from './ILoginCode';
export type * from './IAvatar';
export type * from './ICustomUserStatus';
export type * from './IEmailMessageHistory';
Expand Down
1 change: 1 addition & 0 deletions packages/model-typings/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export type * from './models/IBaseModel';
export type * from './models/IBaseUploadsModel';
export type * from './models/ICannedResponseModel';
export type * from './models/ICredentialTokensModel';
export type * from './models/ILoginCodesModel';
export type * from './models/ICustomSoundsModel';
export type * from './models/ICustomUserStatusModel';
export type * from './models/IEmailInboxModel';
Expand Down
8 changes: 8 additions & 0 deletions packages/model-typings/src/models/ILoginCodesModel.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import type { ILoginCode } from '@rocket.chat/core-typings';

import type { IBaseModel } from './IBaseModel';

export interface ILoginCodesModel extends IBaseModel<ILoginCode> {
createCode(userId: string): Promise<string>;
findOneNotExpiredByCodeAndDelete(code: string): Promise<ILoginCode | null>;
}
2 changes: 2 additions & 0 deletions packages/models/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import type {
IOAuthAppsModel,
IOAuthAuthCodesModel,
IOAuthAccessTokensModel,
ILoginCodesModel,
IOAuthRefreshTokensModel,
IOEmbedCacheModel,
IPushTokenModel,
Expand Down Expand Up @@ -174,6 +175,7 @@ export const NpsVote = proxify<INpsVoteModel>('INpsVoteModel');
export const OAuthApps = proxify<IOAuthAppsModel>('IOAuthAppsModel');
export const OAuthAuthCodes = proxify<IOAuthAuthCodesModel>('IOAuthAuthCodesModel');
export const OAuthAccessTokens = proxify<IOAuthAccessTokensModel>('IOAuthAccessTokensModel');
export const LoginCodes = proxify<ILoginCodesModel>('ILoginCodesModel');
export const OAuthRefreshTokens = proxify<IOAuthRefreshTokensModel>('IOAuthRefreshTokensModel');
export const OEmbedCache = proxify<IOEmbedCacheModel>('IOEmbedCacheModel');
export const PushToken = proxify<IPushTokenModel>('IPushTokenModel');
Expand Down
1 change: 1 addition & 0 deletions packages/models/src/modelClasses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export * from './models/NpsVote';
export * from './models/OAuthAccessTokens';
export * from './models/OAuthApps';
export * from './models/OAuthAuthCodes';
export * from './models/LoginCodes';
export * from './models/OAuthRefreshTokens';
export * from './models/OEmbedCache';
export * from './models/Permissions';
Expand Down
Loading
Loading