Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Option to disable 2FA for OAuth users #32945

Merged
merged 29 commits into from
Sep 10, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
214015c
implement new setting
yash-rajpal Jul 30, 2024
969caf2
Hide option in UI for oAuth users
yash-rajpal Jul 30, 2024
fdb0fac
add cs
yash-rajpal Jul 31, 2024
6a1a5a5
OAuth user definition
yash-rajpal Aug 2, 2024
5bf39c7
oops
yash-rajpal Aug 2, 2024
ae866f7
remove console.log
yash-rajpal Aug 2, 2024
53ff5ad
fix return condition
yash-rajpal Aug 2, 2024
1480e82
fix review
yash-rajpal Aug 2, 2024
87e974a
Merge branch 'develop' into feat/disable-email-2FA-oauth
yash-rajpal Aug 2, 2024
7c79251
revert oAuth types to any
yash-rajpal Aug 5, 2024
4d97527
fix review
yash-rajpal Aug 5, 2024
063ff50
replace setting behavior
yash-rajpal Aug 5, 2024
4149692
add unit tests
yash-rajpal Aug 5, 2024
c542bd0
Merge branch 'develop' into feat/disable-email-2FA-oauth
yash-rajpal Aug 12, 2024
351f0a0
fix review
yash-rajpal Aug 12, 2024
c9ad659
revert password fallack password check
yash-rajpal Aug 12, 2024
56bdbf9
translations
yash-rajpal Aug 12, 2024
94c57df
Merge branch 'develop' into feat/disable-email-2FA-oauth
yash-rajpal Aug 16, 2024
d7fc847
jest new test config
yash-rajpal Aug 16, 2024
c9bb363
don't mock isOAuthUser function
yash-rajpal Aug 20, 2024
0b3d6f3
Update apps/meteor/app/2fa/server/code/EmailCheck.spec.ts
yash-rajpal Aug 20, 2024
c9edf9e
control disabling and showing from higher component
yash-rajpal Aug 20, 2024
3c5ebca
unit testing the component no longer makes sense
yash-rajpal Aug 20, 2024
560df75
get entire services object
yash-rajpal Aug 20, 2024
2abce3c
setting name semantics
yash-rajpal Aug 22, 2024
d4cdd53
Merge branch 'develop' into feat/disable-email-2FA-oauth
kodiakhq[bot] Sep 9, 2024
974fe47
Merge branch 'develop' into feat/disable-email-2FA-oauth
scuciatto Sep 9, 2024
bb717b6
Merge branch 'develop' into feat/disable-email-2FA-oauth
kodiakhq[bot] Sep 9, 2024
b878dc7
Merge branch 'develop' into feat/disable-email-2FA-oauth
kodiakhq[bot] Sep 9, 2024
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
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.
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 Down Expand Up @@ -28,6 +28,10 @@ export class EmailCheck implements ICodeCheck {
return false;
}

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

return this.getUserVerifiedEmails(user).length > 0;
}

Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/app/2fa/server/code/PasswordCheckFallback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export class PasswordCheckFallback implements ICodeCheck {
// TODO: Remove this setting for version 4.0 forcing the
// password fallback for who has password set.
if (settings.get('Accounts_TwoFactorAuthentication_Enforce_Password_Fallback')) {
return user.services?.password?.bcrypt != null;
return user.services?.password?.bcrypt !== undefined;
yash-rajpal marked this conversation as resolved.
Show resolved Hide resolved
}
return false;
}
Expand Down
44 changes: 39 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,6 @@ 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 { isSMTPConfigured } from '../../../utils/server/functions/isSMTPConfigured';
import { getURL } from '../../../utils/server/getURL';
import { API } from '../api';
Expand All @@ -35,6 +34,38 @@ import { getPaginationItems } from '../helpers/getPaginationItems';
import { getUserFromParams } from '../helpers/getUserFromParams';
import { getUserInfo } from '../helpers/getUserInfo';

const 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,
'services': 1,
};
yash-rajpal marked this conversation as resolved.
Show resolved Hide resolved

/**
* @openapi
* /api/v1/me:
Expand Down Expand Up @@ -176,15 +207,18 @@ API.v1.addRoute(
{ authRequired: true },
{
async get() {
const fields = getDefaultUserFields();
const { services, ...user } = (await Users.findOneById(this.userId, { projection: fields })) as IUser;
const { services, ...user } = (await Users.findOneById(this.userId, { projection: userFields })) as IUser;

return API.v1.success(
await getUserInfo({
...user,
isOAuthUser: isOAuthUser({ ...user, services }),
yash-rajpal marked this conversation as resolved.
Show resolved Hide resolved
...(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
13 changes: 10 additions & 3 deletions apps/meteor/client/views/account/security/TwoFactorEmail.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
import { Box, Button, Margins } from '@rocket.chat/fuselage';
import { useUser, useTranslation } from '@rocket.chat/ui-contexts';
import type { ComponentProps, ReactElement } from 'react';
import { useUser, useTranslation, useSetting } from '@rocket.chat/ui-contexts';
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>): JSX.Element | null => {
yash-rajpal marked this conversation as resolved.
Show resolved Hide resolved
const t = useTranslation();
const user = useUser();

const disableEmail2FAForOAuth = useSetting('Accounts_TwoFactorAuthentication_Disable_Email_For_OAuth_Users');
const isOAuthUser = user?.isOAuthUser;
const isEnabled = user?.services?.email2fa?.enabled;
const isAllowed = !isOAuthUser || !disableEmail2FAForOAuth;

const enable2faAction = useEndpointAction('POST', '/v1/users.2fa.enableEmail', {
successMessage: t('Two-factor_authentication_enabled'),
Expand All @@ -25,6 +28,10 @@ const TwoFactorEmail = (props: ComponentProps<typeof Box>): ReactElement => {
await disable2faAction();
}, [disable2faAction]);

if (!isAllowed) {
return null;
}

return (
<Box display='flex' flexDirection='column' alignItems='flex-start' mbs={16} {...props}>
<Margins blockEnd={8}>
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_Disable_Email_For_OAuth_Users', false, {
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
85 changes: 59 additions & 26 deletions packages/core-typings/src/IUser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,35 @@ export type ILoginUsername =
};
export type LoginUsername = string | ILoginUsername;

export interface IUserServices {
export interface IOAuthUserServices {
google?: any;
yash-rajpal marked this conversation as resolved.
Show resolved Hide resolved
facebook?: any;
github?: any;
linkedin?: any;
twitter?: any;
gitlab?: any;
saml?: {
inResponseTo?: string;
provider?: string;
idp?: string;
idpSession?: string;
nameID?: string;
};
ldap?: {
id: string;
idAttribute?: string;
};
nextcloud?: {
accessToken: string;
refreshToken: string;
serverURL: string;
};
dolphin?: {
NickName?: string;
};
}

export interface IUserServices extends IOAuthUserServices {
password?: {
exists?: boolean;
bcrypt?: string;
Expand All @@ -62,12 +90,6 @@ export interface IUserServices {
refreshToken: string;
expiresAt: Date;
};
google?: any;
facebook?: any;
github?: any;
linkedin?: any;
twitter?: any;
gitlab?: any;
totp?: {
enabled: boolean;
hashedBackup: string[];
Expand All @@ -79,27 +101,37 @@ export interface IUserServices {
changedAt: Date;
};
emailCode?: IUserEmailCode;
saml?: {
inResponseTo?: string;
provider?: string;
idp?: string;
idpSession?: string;
nameID?: string;
};
ldap?: {
id: string;
idAttribute?: string;
};
nextcloud?: {
accessToken: string;
refreshToken: string;
serverURL: string;
};
dolphin?: {
NickName?: string;
};
}

type IUserService = keyof IUserServices;
type IOAuthServices = keyof IOAuthUserServices;
yash-rajpal marked this conversation as resolved.
Show resolved Hide resolved

const defaultOAuthKeys = [
'google',
'dolphin',
'facebook',
'github',
'gitlab',
'google',
'ldap',
'linkedin',
'nextcloud',
'saml',
'twitter',
] as IOAuthServices[];
yash-rajpal marked this conversation as resolved.
Show resolved Hide resolved
const userServiceKeys = ['emailCode', 'email2fa', 'totp', 'resume', 'password', 'passwordHistory', 'cloud', 'email'] as IUserService[];

export const isUserServiceKey = (key: string): key is IUserService => key in userServiceKeys || key in defaultOAuthKeys;
yash-rajpal marked this conversation as resolved.
Show resolved Hide resolved

export const isDefaultOAuthUser = (user: IUser): boolean =>
!!(user.services && Object.keys(user.services).some((key) => key in defaultOAuthKeys));
yash-rajpal marked this conversation as resolved.
Show resolved Hide resolved
yash-rajpal marked this conversation as resolved.
Show resolved Hide resolved

export const isCustomOAuthUser = (user: IUser): boolean =>
!!(user.services && Object.keys(user.services).some((key) => !isUserServiceKey(key)));

export const isOAuthUser = (user: IUser): boolean =>
!!(user.services && Object.keys(user.services).some((key: any) => !userServiceKeys.includes(key)));
yash-rajpal marked this conversation as resolved.
Show resolved Hide resolved
yash-rajpal marked this conversation as resolved.
Show resolved Hide resolved

export interface IUserEmail {
address: string;
verified?: boolean;
Expand Down Expand Up @@ -185,6 +217,7 @@ export interface IUser extends IRocketChatRecord {
_pendingAvatarUrl?: string;
requirePasswordChange?: boolean;
requirePasswordChangeReason?: string;
isOAuthUser?: boolean;
yash-rajpal marked this conversation as resolved.
Show resolved Hide resolved
}

export interface IRegisterUser extends IUser {
Expand Down
1 change: 1 addition & 0 deletions packages/i18n/src/locales/en.i18n.json
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,7 @@
"Accounts_TwoFactorAuthentication_By_Email_Code_Expiration": "Time to expire the code sent via email in seconds",
"Accounts_TwoFactorAuthentication_By_Email_Enabled": "Enable Two Factor Authentication via Email",
"Accounts_TwoFactorAuthentication_By_Email_Enabled_Description": "Users with email verified and the option enabled in their profile page will receive an email with a temporary code to authorize certain actions like login, save the profile, etc.",
"Accounts_TwoFactorAuthentication_Disable_Email_For_OAuth_Users": "Disable two factor authentication via email for OAuth users",
"Accounts_TwoFactorAuthentication_Enabled": "Enable Two Factor Authentication",
"Accounts_TwoFactorAuthentication_Enabled_Description": "If deactivated, this setting will deactivate all Two Factor Authentication. \nTo force users to use Two Factor Authentication, the admin has to configure the 'user' role to enforce it.",
"Accounts_TwoFactorAuthentication_Enforce_Password_Fallback": "Enforce password fallback",
Expand Down
Loading