Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
38 changes: 38 additions & 0 deletions apps/meteor/app/api/server/v1/loginCode.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { LoginCodes } from '@rocket.chat/models';
import { isLoginCodeRedeemParamsPOST } from '@rocket.chat/rest-typings';
import { Accounts } from 'meteor/accounts-base';

import { API } from '../api';

API.v1.addRoute(
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
Resolved
Comment thread
yash-rajpal marked this conversation as resolved.
Resolved
'loginCode.redeem',
{
authRequired: false,
validateParams: isLoginCodeRedeemParamsPOST,
rateLimiterOptions: { intervalTimeInMS: 60000, numRequestsAllowed: 10 },
},
{
async post() {
const { code } = this.bodyParams;

const loginCode = await LoginCodes.findOneNotExpiredByCode(code);
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
Resolved

if (!loginCode) {
throw new Meteor.Error('error-invalid-code', 'Invalid or expired login code');
}

//single-use: remove code before issuing the token
await LoginCodes.removeByCode(code);

const { userId } = loginCode;

const stampedToken = Accounts._generateStampedLoginToken();
await Accounts._insertLoginToken(userId, stampedToken);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

return API.v1.success({
loginToken: stampedToken.token,
userId,
});
},
Comment thread
yash-rajpal marked this conversation as resolved.
Resolved
},
);
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.
Resolved
}

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

try {
await loginWithToken(resumeToken);

Expand Down
49 changes: 49 additions & 0 deletions apps/meteor/client/views/root/hooks/useOAuthLogin.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { useEndpoint, useRouter, useSearchParameter, useLoginWithToken } from '@rocket.chat/ui-contexts';
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();

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

let timeout: ReturnType<typeof setTimeout>;
Comment thread
yash-rajpal marked this conversation as resolved.
Resolved

const handleLogin = async () => {
try {
const { loginToken, userId } = await redeemLoginCode({ code: loginCode });

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

if (loginClient === 'desktop' || loginClient === 'mobile') {
const loginURL = buildDeepLinkURL(loginToken, userId);
window.location.href = loginURL;
return;
}

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

handleLogin();

return () => clearTimeout(timeout);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
}, [loginCode, router, redeemLoginCode, loginWithToken, loginClient]);
};
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
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
10 changes: 10 additions & 0 deletions packages/model-typings/src/models/ILoginCodesModel.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import type { ILoginCode } from '@rocket.chat/core-typings';
import type { DeleteResult } from 'mongodb';

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

export interface ILoginCodesModel extends IBaseModel<ILoginCode> {
createCode(userId: string): Promise<string>;
findOneNotExpiredByCode(code: string): Promise<ILoginCode | null>;
removeByCode(code: string): Promise<DeleteResult>;
}
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
42 changes: 42 additions & 0 deletions packages/models/src/models/LoginCodes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import { randomBytes } from 'crypto';

import type { ILoginCode } from '@rocket.chat/core-typings';
import type { ILoginCodesModel } from '@rocket.chat/model-typings';
import type { Db, DeleteResult, IndexDescription } from 'mongodb';

import { BaseRaw } from './BaseRaw';

const CODE_TTL_SECONDS = 60;

export class LoginCodesRaw extends BaseRaw<ILoginCode> implements ILoginCodesModel {
constructor(db: Db) {
super(db, 'login_codes');
}

override modelIndexes(): IndexDescription[] {
return [{ key: { expireAt: 1 }, expireAfterSeconds: 0 }];
}

async createCode(userId: string): Promise<string> {
const now = new Date();
const code = randomBytes(32).toString('hex');
await this.insertOne({
_id: code,
userId,
createdAt: now,
expireAt: new Date(now.getTime() + CODE_TTL_SECONDS * 1000),
Comment thread
yash-rajpal marked this conversation as resolved.
Resolved
});
return code;
}

findOneNotExpiredByCode(code: string): Promise<ILoginCode | null> {
return this.findOne({
_id: code,
expireAt: { $gt: new Date() },
});
}

removeByCode(code: string): Promise<DeleteResult> {
Comment thread
yash-rajpal marked this conversation as resolved.
Resolved
return this.deleteOne({ _id: code });
}
}
3 changes: 3 additions & 0 deletions packages/rest-typings/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import type { IntegrationHooksEndpoints } from './v1/integrations/hooks';
import type { InvitesEndpoints } from './v1/invites';
import type { LDAPEndpoints } from './v1/ldap';
import type { LicensesEndpoints } from './v1/licenses';
import type { LoginCodeEndpoints } from './v1/loginCode';
import type { MailerEndpoints } from './v1/mailer';
import type { MeEndpoints } from './v1/me';
import type { MiscEndpoints } from './v1/misc';
Expand Down Expand Up @@ -92,6 +93,7 @@ export interface Endpoints
ImportEndpoints,
ServerEventsEndpoints,
TwoFactorChallengesEndpoints,
LoginCodeEndpoints,
DefaultEndpoints {}

type OperationsByPathPatternAndMethod<
Expand Down Expand Up @@ -267,6 +269,7 @@ export * from './v1/cloud';
export * from './v1/banners';
export * from './default';
export * from './v1/twoFactorChallenges';
export * from './v1/loginCode';

// Export the ajv instance for use in other packages
export * from './v1/Ajv';
20 changes: 20 additions & 0 deletions packages/rest-typings/src/v1/loginCode.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { ajv } from './Ajv';

type LoginCodeRedeemParams = { code: string };

const LoginCodeRedeemSchema = {
type: 'object',
properties: {
code: { type: 'string' },
Comment thread
yash-rajpal marked this conversation as resolved.
Resolved
},
required: ['code'],
additionalProperties: false,
};

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

export type LoginCodeEndpoints = {
Comment thread
yash-rajpal marked this conversation as resolved.
Resolved
'/v1/loginCode.redeem': {
POST: (params: LoginCodeRedeemParams) => { loginToken: string; userId: string };
};
};
Loading