Skip to content

Commit

Permalink
feat: Option to disable 2FA for OAuth users (#32945)
Browse files Browse the repository at this point in the history
  • Loading branch information
yash-rajpal committed Sep 10, 2024
1 parent 4146c39 commit 0f21fa0
Show file tree
Hide file tree
Showing 12 changed files with 217 additions and 75 deletions.
6 changes: 6 additions & 0 deletions .changeset/tiny-geckos-kiss.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@rocket.chat/i18n': minor
'@rocket.chat/meteor': minor
---

Added a new setting which allows workspace admins to disable email two factor authentication for SSO (OAuth) users. If enabled, SSO users won't be asked for email two factor authentication.
70 changes: 70 additions & 0 deletions apps/meteor/app/2fa/server/code/EmailCheck.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { expect } from 'chai';
import { describe, it } from 'mocha';
import proxyquire from 'proxyquire';
import sinon from 'sinon';

const settingsMock = sinon.stub();

const { EmailCheck } = proxyquire.noCallThru().load('./EmailCheck', {
'@rocket.chat/models': {
Users: {},
},
'meteor/accounts-base': {
Accounts: {
_bcryptRounds: () => '123',
},
},
'../../../../server/lib/i18n': {
i18n: {
t: (key: string) => key,
},
},
'../../../mailer/server/api': {
send: () => undefined,
},
'../../../settings/server': {
settings: {
get: settingsMock,
},
},
});

const normalUserMock = { services: { email2fa: { enabled: true } }, emails: [{ email: '[email protected]', verified: true }] };
const normalUserWithUnverifiedEmailMock = {
services: { email2fa: { enabled: true } },
emails: [{ email: '[email protected]', verified: false }],
};
const OAuthUserMock = { services: { google: {} }, emails: [{ email: '[email protected]', verified: true }] };

describe('EmailCheck', () => {
let emailCheck: typeof EmailCheck;
beforeEach(() => {
settingsMock.reset();

emailCheck = new EmailCheck();
});

it('should return EmailCheck is enabled for a normal user', () => {
settingsMock.returns(true);

const isEmail2FAEnabled = emailCheck.isEnabled(normalUserMock);

expect(isEmail2FAEnabled).to.be.equal(true);
});

it('should return EmailCheck is not enabled for a normal user with unverified email', () => {
settingsMock.returns(true);

const isEmail2FAEnabled = emailCheck.isEnabled(normalUserWithUnverifiedEmailMock);

expect(isEmail2FAEnabled).to.be.equal(false);
});

it('should return EmailCheck is not enabled for a OAuth user with setting being false', () => {
settingsMock.returns(true);

const isEmail2FAEnabled = emailCheck.isEnabled(OAuthUserMock);

expect(isEmail2FAEnabled).to.be.equal(false);
});
});
6 changes: 5 additions & 1 deletion apps/meteor/app/2fa/server/code/EmailCheck.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { IUser } from '@rocket.chat/core-typings';
import { isOAuthUser, type IUser } from '@rocket.chat/core-typings';
import { Users } from '@rocket.chat/models';
import { Random } from '@rocket.chat/random';
import bcrypt from 'bcrypt';
Expand All @@ -24,6 +24,10 @@ export class EmailCheck implements ICodeCheck {
return false;
}

if (!settings.get('Accounts_twoFactorAuthentication_email_available_for_OAuth_users') && isOAuthUser(user)) {
return false;
}

if (!user.services?.email2fa?.enabled) {
return false;
}
Expand Down
12 changes: 4 additions & 8 deletions apps/meteor/app/2fa/server/code/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,10 @@ function getAvailableMethodNames(user: IUser): string[] {
export async function getUserForCheck(userId: string): Promise<IUser | null> {
return Users.findOneById(userId, {
projection: {
'emails': 1,
'language': 1,
'createdAt': 1,
'services.totp': 1,
'services.email2fa': 1,
'services.emailCode': 1,
'services.password': 1,
'services.resume.loginTokens': 1,
emails: 1,
language: 1,
createdAt: 1,
services: 1,
},
});
}
Expand Down
14 changes: 9 additions & 5 deletions apps/meteor/app/api/server/v1/misc.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import crypto from 'crypto';

import type { IUser } from '@rocket.chat/core-typings';
import { isOAuthUser, type IUser } from '@rocket.chat/core-typings';
import { Settings, Users } from '@rocket.chat/models';
import {
isShieldSvgProps,
Expand All @@ -26,7 +26,7 @@ import { hasPermissionAsync } from '../../../authorization/server/functions/hasP
import { passwordPolicy } from '../../../lib/server';
import { notifyOnSettingChangedById } from '../../../lib/server/lib/notifyListener';
import { settings } from '../../../settings/server';
import { getDefaultUserFields } from '../../../utils/server/functions/getDefaultUserFields';
import { getBaseUserFields } from '../../../utils/server/functions/getBaseUserFields';
import { isSMTPConfigured } from '../../../utils/server/functions/isSMTPConfigured';
import { getURL } from '../../../utils/server/getURL';
import { API } from '../api';
Expand Down Expand Up @@ -176,15 +176,19 @@ API.v1.addRoute(
{ authRequired: true },
{
async get() {
const fields = getDefaultUserFields();
const { services, ...user } = (await Users.findOneById(this.userId, { projection: fields })) as IUser;
const userFields = { ...getBaseUserFields(), services: 1 };
const { services, ...user } = (await Users.findOneById(this.userId, { projection: userFields })) as IUser;

return API.v1.success(
await getUserInfo({
...user,
isOAuthUser: isOAuthUser({ ...user, services }),
...(services && {
services: {
...services,
...(services.github && { github: services.github }),
...(services.gitlab && { gitlab: services.gitlab }),
...(services.email2fa?.enabled && { email2fa: { enabled: services.email2fa.enabled } }),
...(services.totp?.enabled && { totp: { enabled: services.totp.enabled } }),
password: {
// The password hash shouldn't be leaked but the client may need to know if it exists.
exists: Boolean(services?.password?.bcrypt),
Expand Down
34 changes: 34 additions & 0 deletions apps/meteor/app/utils/server/functions/getBaseUserFields.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
type UserFields = {
[k: string]: number;
};

export const getBaseUserFields = (): UserFields => ({
'name': 1,
'username': 1,
'nickname': 1,
'emails': 1,
'status': 1,
'statusDefault': 1,
'statusText': 1,
'statusConnection': 1,
'bio': 1,
'avatarOrigin': 1,
'utcOffset': 1,
'language': 1,
'settings': 1,
'enableAutoAway': 1,
'idleTimeLimit': 1,
'roles': 1,
'active': 1,
'defaultRoom': 1,
'customFields': 1,
'requirePasswordChange': 1,
'requirePasswordChangeReason': 1,
'statusLivechat': 1,
'banners': 1,
'oauth.authorizedClients': 1,
'_updatedAt': 1,
'avatarETag': 1,
'extension': 1,
'openBusinessHours': 1,
});
35 changes: 5 additions & 30 deletions apps/meteor/app/utils/server/functions/getDefaultUserFields.ts
Original file line number Diff line number Diff line change
@@ -1,39 +1,14 @@
type DefaultUserFields = {
import { getBaseUserFields } from './getBaseUserFields';

type UserFields = {
[k: string]: number;
};

export const getDefaultUserFields = (): DefaultUserFields => ({
'name': 1,
'username': 1,
'nickname': 1,
'emails': 1,
'status': 1,
'statusDefault': 1,
'statusText': 1,
'statusConnection': 1,
'bio': 1,
'avatarOrigin': 1,
'utcOffset': 1,
'language': 1,
'settings': 1,
'enableAutoAway': 1,
'idleTimeLimit': 1,
'roles': 1,
'active': 1,
'defaultRoom': 1,
'customFields': 1,
'requirePasswordChange': 1,
'requirePasswordChangeReason': 1,
export const getDefaultUserFields = (): UserFields => ({
...getBaseUserFields(),
'services.github': 1,
'services.gitlab': 1,
'services.password.bcrypt': 1,
'services.totp.enabled': 1,
'services.email2fa.enabled': 1,
'statusLivechat': 1,
'banners': 1,
'oauth.authorizedClients': 1,
'_updatedAt': 1,
'avatarETag': 1,
'extension': 1,
'openBusinessHours': 1,
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Box, Accordion, ButtonGroup, Button } from '@rocket.chat/fuselage';
import { useUniqueId } from '@rocket.chat/fuselage-hooks';
import { useSetting, useTranslation } from '@rocket.chat/ui-contexts';
import { useSetting, useTranslation, useUser } from '@rocket.chat/ui-contexts';
import type { ReactElement } from 'react';
import React from 'react';
import { FormProvider, useForm } from 'react-hook-form';
Expand All @@ -15,6 +15,11 @@ const passwordDefaultValues = { password: '', confirmationPassword: '' };

const AccountSecurityPage = (): ReactElement => {
const t = useTranslation();
const user = useUser();

const isEmail2FAAvailableForOAuth = useSetting('Accounts_twoFactorAuthentication_email_available_for_OAuth_users');
const isOAuthUser = user?.isOAuthUser;
const isEmail2FAAllowed = !isOAuthUser || isEmail2FAAvailableForOAuth;

const methods = useForm({
defaultValues: passwordDefaultValues,
Expand All @@ -30,6 +35,7 @@ const AccountSecurityPage = (): ReactElement => {
const twoFactorByEmailEnabled = useSetting('Accounts_TwoFactorAuthentication_By_Email_Enabled');
const e2eEnabled = useSetting('E2E_Enable');
const allowPasswordChange = useSetting('Accounts_AllowPasswordChange');
const showEmailTwoFactor = twoFactorByEmailEnabled && isEmail2FAAllowed;

const passwordFormId = useUniqueId();

Expand All @@ -48,10 +54,10 @@ const AccountSecurityPage = (): ReactElement => {
</FormProvider>
)}
<Accordion>
{(twoFactorTOTP || twoFactorByEmailEnabled) && twoFactorEnabled && (
{(twoFactorTOTP || showEmailTwoFactor) && twoFactorEnabled && (
<Accordion.Item title={t('Two Factor Authentication')}>
{twoFactorTOTP && <TwoFactorTOTP />}
{twoFactorByEmailEnabled && <TwoFactorEmail />}
{showEmailTwoFactor && <TwoFactorEmail />}
</Accordion.Item>
)}
{e2eEnabled && (
Expand Down
4 changes: 2 additions & 2 deletions apps/meteor/client/views/account/security/TwoFactorEmail.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { Box, Button, Margins } from '@rocket.chat/fuselage';
import { useUser, useTranslation } from '@rocket.chat/ui-contexts';
import type { ComponentProps, ReactElement } from 'react';
import type { ComponentProps } from 'react';
import React, { useCallback } from 'react';

import { useEndpointAction } from '../../../hooks/useEndpointAction';

const TwoFactorEmail = (props: ComponentProps<typeof Box>): ReactElement => {
const TwoFactorEmail = (props: ComponentProps<typeof Box>) => {
const t = useTranslation();
const user = useUser();

Expand Down
12 changes: 12 additions & 0 deletions apps/meteor/server/settings/accounts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,18 @@ export const createAccountSettings = () =>
public: true,
});

await this.add('Accounts_twoFactorAuthentication_email_available_for_OAuth_users', true, {
type: 'boolean',
enableQuery: [
enable2FA,
{
_id: 'Accounts_TwoFactorAuthentication_By_Email_Enabled',
value: true,
},
],
public: true,
});

await this.add('Accounts_TwoFactorAuthentication_By_Email_Auto_Opt_In', true, {
type: 'boolean',
enableQuery: [
Expand Down
Loading

0 comments on commit 0f21fa0

Please sign in to comment.