Skip to content

Commit

Permalink
[IMPROVE] Inconsistent and misleading 2FA settings (#22042)
Browse files Browse the repository at this point in the history
Co-authored-by: Diego Sampaio <chinello@gmail.com>
  • Loading branch information
lucassartor and sampaiodiego authored May 21, 2021
1 parent 11a40e5 commit d3acf8b
Show file tree
Hide file tree
Showing 10 changed files with 48 additions and 25 deletions.
2 changes: 1 addition & 1 deletion app/2fa/server/code/TOTPCheck.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export class TOTPCheck implements ICodeCheck {
public readonly name = 'totp';

public isEnabled(user: IUser): boolean {
if (!settings.get('Accounts_TwoFactorAuthentication_Enabled')) {
if (!settings.get('Accounts_TwoFactorAuthentication_By_TOTP_Enabled')) {
return false;
}

Expand Down
41 changes: 29 additions & 12 deletions app/2fa/server/startup/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,48 +2,65 @@ import { settings } from '../../../settings';

settings.addGroup('Accounts', function() {
this.section('Two Factor Authentication', function() {
const enable2FA = {
_id: 'Accounts_TwoFactorAuthentication_Enabled',
value: true,
};

this.add('Accounts_TwoFactorAuthentication_Enabled', true, {
type: 'boolean',
public: true,
});
this.add('Accounts_TwoFactorAuthentication_MaxDelta', 1, {
type: 'int',
enableQuery: {
_id: 'Accounts_TwoFactorAuthentication_Enabled',
value: true,
},
enableQuery: enable2FA,
});

this.add('Accounts_TwoFactorAuthentication_By_TOTP_Enabled', true, {
type: 'boolean',
enableQuery: enable2FA,
public: true,
});

this.add('Accounts_TwoFactorAuthentication_By_Email_Enabled', true, {
type: 'boolean',
enableQuery: enable2FA,
public: true,
});
this.add('Accounts_TwoFactorAuthentication_By_Email_Auto_Opt_In', true, {
type: 'boolean',
enableQuery: {
_id: 'Accounts_TwoFactorAuthentication_By_Email_Enabled',
value: true,
},
enableQuery: [
enable2FA,
{
_id: 'Accounts_TwoFactorAuthentication_By_Email_Enabled',
value: true,
},
],
wizard: {
step: 3,
order: 3,
},
});
this.add('Accounts_TwoFactorAuthentication_By_Email_Code_Expiration', 3600, {
type: 'int',
enableQuery: {
_id: 'Accounts_TwoFactorAuthentication_By_Email_Enabled',
value: true,
},
enableQuery: [
enable2FA,
{
_id: 'Accounts_TwoFactorAuthentication_By_Email_Enabled',
value: true,
},
],
});

this.add('Accounts_TwoFactorAuthentication_RememberFor', 1800, {
type: 'int',
enableQuery: enable2FA,
});

// TODO: Remove this setting for version 4.0
this.add('Accounts_TwoFactorAuthentication_Enforce_Password_Fallback', true, {
type: 'boolean',
enableQuery: enable2FA,
public: true,
});
});
Expand Down
5 changes: 3 additions & 2 deletions client/views/account/security/AccountSecurityPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const AccountSecurityPage = () => {
const t = useTranslation();

const twoFactorEnabled = useSetting('Accounts_TwoFactorAuthentication_Enabled');
const twoFactorTOTP = useSetting('Accounts_TwoFactorAuthentication_By_TOTP_Enabled');
const twoFactorByEmailEnabled = useSetting('Accounts_TwoFactorAuthentication_By_Email_Enabled');
const e2eEnabled = useSetting('E2E_Enable');

Expand All @@ -26,9 +27,9 @@ const AccountSecurityPage = () => {
<Page.ScrollableContentWithShadow>
<Box maxWidth='x600' w='full' alignSelf='center'>
<Accordion>
{(twoFactorEnabled || twoFactorByEmailEnabled) && (
{(twoFactorTOTP || twoFactorByEmailEnabled) && twoFactorEnabled && (
<Accordion.Item title={t('Two Factor Authentication')} defaultExpanded>
{twoFactorEnabled && <TwoFactorTOTP />}
{twoFactorTOTP && <TwoFactorTOTP />}
{twoFactorByEmailEnabled && <TwoFactorEmail />}
</Accordion.Item>
)}
Expand Down
6 changes: 4 additions & 2 deletions packages/rocketchat-i18n/i18n/en.i18n.json
Original file line number Diff line number Diff line change
Expand Up @@ -225,13 +225,15 @@
"Accounts_SetDefaultAvatar": "Set Default Avatar",
"Accounts_SetDefaultAvatar_Description": "Tries to determine default avatar based on OAuth Account or Gravatar",
"Accounts_ShowFormLogin": "Show Default Login Form",
"Accounts_TwoFactorAuthentication_By_TOTP_Enabled": "Enable Two Factor Authentication via TOTP",
"Accounts_TwoFactorAuthentication_By_TOTP_Enabled_Description": "Users can setup their Two Factor Authentication using any TOTP App, like Google Authenticator or Authy.",
"Accounts_TwoFactorAuthentication_By_Email_Auto_Opt_In": "Auto opt in new users for Two Factor via Email",
"Accounts_TwoFactorAuthentication_By_Email_Auto_Opt_In_Description": "New users will have the Two Factor Authentication via Email enabled by default. They will be able to disable it in their profile page.",
"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_Enabled": "Enable Two Factor Authentication via TOTP",
"Accounts_TwoFactorAuthentication_Enabled_Description": "Users can setup their Two Factor Authentication using any TOTP app, like Google Authenticator or Authy",
"Accounts_TwoFactorAuthentication_Enabled": "Enable Two Factor Authentication",
"Accounts_TwoFactorAuthentication_Enabled_Description": "If deactivated, this setting will deactivate all Two Factor Authentication.<br>To 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",
"Accounts_TwoFactorAuthentication_Enforce_Password_Fallback_Description": "Users will be forced to enter their password, for important actions, if no other Two Factor Authentication method is enabled for that user and a password is set for him.",
"Accounts_TwoFactorAuthentication_MaxDelta": "Maximum Delta",
Expand Down
6 changes: 4 additions & 2 deletions packages/rocketchat-i18n/i18n/pt-BR.i18n.json
Original file line number Diff line number Diff line change
Expand Up @@ -208,13 +208,15 @@
"Accounts_SetDefaultAvatar": "Definir Avatar Padrão",
"Accounts_SetDefaultAvatar_Description": "Tenta determinar o avatar padrão com base em OAuth Account ou Gravatar",
"Accounts_ShowFormLogin": "Mostrar formulário de login padrão",
"Accounts_TwoFactorAuthentication_By_TOTP_Enabled": "Ativar autenticação de dois fatores por TOTP",
"Accounts_TwoFactorAuthentication_By_TOTP_Enabled_Description": "Os usuários podem configurar sua autenticação de dois fatores usando qualquer aplicativo de TOTP, como o Google Authenticator ou o Authy.",
"Accounts_TwoFactorAuthentication_By_Email_Auto_Opt_In": "Auto ativar a autenticação de duas etapas via email para novos usuários",
"Accounts_TwoFactorAuthentication_By_Email_Auto_Opt_In_Description": "Novos usuários terão a autenticação por duas etapas via email ativada por padrão. Eles poderão desabilitá-la em sua página de perfil.",
"Accounts_TwoFactorAuthentication_By_Email_Code_Expiration": "Tempo para expirar o código enviado por email (em segundos)",
"Accounts_TwoFactorAuthentication_By_Email_Enabled": "Ativar autenticação de dois fatores por Email",
"Accounts_TwoFactorAuthentication_By_Email_Enabled_Description": "Usuários com email verificado e com a opção habilitada em seu perfil receberão um email com um código temporário para autorizar certas ações como o login, salvar o perfil, etc.",
"Accounts_TwoFactorAuthentication_Enabled": "Ativar autenticação de dois fatores por TOTP",
"Accounts_TwoFactorAuthentication_Enabled_Description": "Os usuários podem configurar sua autenticação de dois fatores usando qualquer aplicativo de TOTP, como o Google Authenticator ou o Authy",
"Accounts_TwoFactorAuthentication_Enabled": "Ativar autenticação de dois fatores",
"Accounts_TwoFactorAuthentication_Enabled_Description": "Se essa opção estiver desativada, toda a autenticação de dois fatores também será desativada.<br>Para forçar os usúarios a usarem a autenticação de dois fatores, o administrador deve configurar a role 'user' para isso.",
"Accounts_TwoFactorAuthentication_MaxDelta": "Delta máximo",
"Accounts_TwoFactorAuthentication_MaxDelta_Description": "O Delta máximo determina quantos tokens são válidos em qualquer momento. Os tokens são gerados a cada 30 segundos e são válidos por (30 * Delta máximo) segundos. <br/>Exemplo: com um Delta máximo configurado para 10, cada token pode ser usado até 300 segundos antes ou depois do timestamp. Isso é útil quando o relógio do cliente não está corretamente sincronizado com o servidor.",
"Accounts_TwoFactorAuthentication_RememberFor": "Lembrar a autenticação por (segundos)",
Expand Down
1 change: 1 addition & 0 deletions tests/end-to-end/api/00-miscellaneous.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ describe('miscellaneous', function() {
'desktopNotifications',
'mobileNotifications',
'enableAutoAway',
'enableMessageParserEarlyAdoption',
// 'highlights',
'showMessageInMainThread',
'desktopNotificationRequireInteraction',
Expand Down
2 changes: 1 addition & 1 deletion tests/end-to-end/api/01-users.js
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ describe('[Users]', function() {
.expect(400)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('error').and.to.be.equal('User not found.');
expect(res.body).to.have.property('error');
})
.end(done);
});
Expand Down
6 changes: 3 additions & 3 deletions tests/end-to-end/api/04-direct-message.js
Original file line number Diff line number Diff line change
Expand Up @@ -451,10 +451,10 @@ describe('[Direct Messages]', function() {
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('count').and.to.be.equal(1);
expect(res.body).to.have.property('count').and.to.be.equal(2);
expect(res.body).to.have.property('offset').and.to.be.equal(0);
expect(res.body).to.have.property('total').and.to.be.equal(2);
expect(res.body).to.have.property('members').and.to.have.lengthOf(1);
expect(res.body).to.have.property('members').and.to.have.lengthOf(2);
})
.end(done);
});
Expand All @@ -471,7 +471,7 @@ describe('[Direct Messages]', function() {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('count').and.to.be.equal(1);
expect(res.body).to.have.property('offset').and.to.be.equal(0);
expect(res.body).to.have.property('total').and.to.be.equal(2);
expect(res.body).to.have.property('total').and.to.be.equal(1);
expect(res.body).to.have.property('members').and.to.have.lengthOf(1);
})
.end(done);
Expand Down
2 changes: 1 addition & 1 deletion tests/end-to-end/api/05-chat.js
Original file line number Diff line number Diff line change
Expand Up @@ -747,7 +747,7 @@ describe('[Chat]', function() {
expect(res.body.message.urls[0])
.to.have.property('meta')
.to.have.property('oembedHtml')
.to.have.string('<iframe style="max-width: 100%"');
.to.have.string('<iframe style="max-width: 100%;width:400px;height:225px"');
})
.end(done);
}, 500);
Expand Down
2 changes: 1 addition & 1 deletion tests/end-to-end/api/25-teams.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { updatePermission } from '../../data/permissions.helper.js';
import { createUser, login } from '../../data/users.helper';
import { password } from '../../data/user';

describe.only('[Teams]', () => {
describe('[Teams]', () => {
before((done) => getCredentials(done));

const community = `community${ Date.now() }`;
Expand Down

0 comments on commit d3acf8b

Please sign in to comment.