Skip to content

Commit

Permalink
Audit adjustments (#7116)
Browse files Browse the repository at this point in the history
* do not reject

* update message

* fix login time diffs

* lint fixes

* avoid features in settings

* test refactor
  • Loading branch information
Zasa-san authored Aug 14, 2024
1 parent f6de042 commit 2be0fbf
Show file tree
Hide file tree
Showing 15 changed files with 104 additions and 47 deletions.
9 changes: 8 additions & 1 deletion app/api/settings/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,14 @@ export default (app: Application) => {
const select = req.user && req.user.role === 'admin' ? '+publicFormDestination' : {};
settings
.get({}, select)
.then(response => res.json(response))
.then(response => {
const { features, ...partialSettings } = response;
if (req.user?.role === 'admin' || req.user?.role === 'editor') {
res.json(response);
} else {
res.json(partialSettings);
}
})
.catch(next);
});

Expand Down
9 changes: 9 additions & 0 deletions app/api/settings/specs/fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,15 @@ const fixtures: DBFixture = {
{ key: 'es', label: 'Spanish', default: true },
{ key: 'en', label: 'English' },
],
features: {
'metadata-extraction': true,
metadataExtraction: {
url: 'http:someurl',
},
segmentation: {
url: 'http://otherurl',
},
},
},
],
templates: [
Expand Down
34 changes: 31 additions & 3 deletions app/api/settings/specs/routes.spec.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { NextFunction, Request, Response } from 'express';
import entities from 'api/entities';
import { permissionsContext } from 'api/permissions/permissionsContext';
import { search } from 'api/search';
import settings from 'api/settings';
import templates from 'api/templates';
import { setUpApp } from 'api/utils/testingRoutes';
import { Application, NextFunction } from 'express';
import request from 'supertest';

import translations from 'api/i18n';
Expand All @@ -21,7 +21,11 @@ jest.mock(
);

describe('Settings routes', () => {
const app: Application = setUpApp(settingsRoutes);
const getApp = (userRole?: string) =>
setUpApp(settingsRoutes, (req: Request, _res: Response, next: NextFunction) => {
(req as any).user = { role: userRole };
next();
});

beforeAll(async () => {
jest.spyOn(search, 'indexEntities').mockResolvedValue();
Expand All @@ -34,12 +38,36 @@ describe('Settings routes', () => {

describe('GET', () => {
it('should respond with settings', async () => {
const response = await request(app).get('/api/settings').expect(200);
const response = await request(getApp()).get('/api/settings').expect(200);
expect(response.body).toEqual(expect.objectContaining({ site_name: 'Uwazi' }));
expect(response.body.features).toBe(undefined);
});

it('should return the collection features for admins and editors', async () => {
const [adminResponse, editorResponse] = await Promise.all([
await request(getApp('admin')).get('/api/settings').expect(200),
await request(getApp('editor')).get('/api/settings').expect(200),
]);

const expectedResponse = {
'metadata-extraction': true,
metadataExtraction: {
url: 'http:someurl',
},
segmentation: {
url: 'http://otherurl',
},
};

expect(adminResponse.body.features).toEqual(expect.objectContaining(expectedResponse));

expect(editorResponse.body.features).toEqual(expect.objectContaining(expectedResponse));
});
});

describe('POST', () => {
const app = getApp();

it('should save settings', async () => {
const response = await request(app)
.post('/api/settings')
Expand Down
4 changes: 2 additions & 2 deletions app/api/users/specs/__snapshots__/users.spec.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ Array [
"html": "<p>Hello,
Your account has been locked because of too many failed login attempts. To unlock your account open the following link:
<a href=\\"http://host.domain/unlockaccount/someuser1/636f6465\\">http://host.domain/unlockaccount/someuser1/636f6465</a></p>",
<a href=\\"http://host.domain/unlockaccount/someuser1/hash\\">http://host.domain/unlockaccount/someuser1/hash</a></p>",
"subject": "Account locked",
"text": "Hello,
Your account has been locked because of too many failed login attempts. To unlock your account open the following link:
http://host.domain/unlockaccount/someuser1/636f6465",
http://host.domain/unlockaccount/someuser1/hash",
"to": "someuser1@mailer.com",
},
]
Expand Down
18 changes: 8 additions & 10 deletions app/api/users/specs/users.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
/* eslint-disable max-statements */

import { createError } from 'api/utils';
import crypto from 'crypto';
import mailer from 'api/utils/mailer';
import db from 'api/utils/testing_db';
import * as random from 'shared/uniqueID';
Expand All @@ -26,6 +25,10 @@ import passwordRecoveriesModel from '../passwordRecoveriesModel';
import usersModel from '../usersModel';
import * as unlockCode from '../generateUnlockCode';

jest.mock('api/users/generateUnlockCode.ts', () => ({
generateUnlockCode: () => 'hash',
}));

describe('Users', () => {
beforeEach(async () => {
await db.setupFixturesAndContext(fixtures);
Expand Down Expand Up @@ -266,7 +269,6 @@ describe('Users', () => {

describe('login', () => {
let testUser;
const codeBuffer = Buffer.from('code');

beforeEach(async () => {
testUser = {
Expand All @@ -275,12 +277,10 @@ describe('Users', () => {
email: 'someuser1@mailer.com',
role: 'admin',
};
jest.spyOn(crypto, 'randomBytes').mockReturnValue(codeBuffer);
jest.spyOn(mailer, 'send').mockResolvedValue();
});

afterEach(() => {
crypto.randomBytes.mockRestore();
mailer.send.mockRestore();
});

Expand Down Expand Up @@ -342,14 +342,13 @@ describe('Users', () => {
await createUserAndTestLogin('someuser1', 'incorrect');
fail('should throw error');
} catch (e) {
expect(e).toEqual(createError('Account locked. Check your email to unlock.', 403));
expect(e).toEqual(createError('Invalid username or password', 401));
const [user] = await users.get(
{ username: 'someuser1' },
'+accountLocked +accountUnlockCode'
);
expect(user.accountLocked).toBe(true);
expect(user.accountUnlockCode).toBe(codeBuffer.toString('hex'));
expect(crypto.randomBytes).toHaveBeenCalledWith(32);
expect(user.accountUnlockCode).toEqual(expect.any(String));
}
});

Expand Down Expand Up @@ -380,8 +379,8 @@ describe('Users', () => {
await createUserAndTestLogin('someuser1', 'incorrect');
fail('should throw error');
} catch (e) {
expect(e.message).toMatch(/account locked/i);
expect(e.code).toBe(403);
expect(e.message).toBe('Invalid username or password');
expect(e.code).toBe(401);
}
});

Expand Down Expand Up @@ -510,7 +509,6 @@ describe('Users', () => {
jest.restoreAllMocks();
jest.spyOn(mailer, 'send').mockImplementation(async () => Promise.resolve('OK'));
jest.spyOn(Date, 'now').mockReturnValue(1000);
jest.spyOn(unlockCode, 'generateUnlockCode').mockReturnValue('ABCDEF1234');
});

it('should find the matching email create a recover password doc in the database and send an email', async () => {
Expand Down
31 changes: 23 additions & 8 deletions app/api/users/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,13 @@ const sendAccountLockedEmail = async (user, domain) => {

const validateUserStatus = user => {
if (!user) {
throw createError('Invalid username or password', 401);
return createError('Invalid username or password', 401);
}
if (user.accountLocked) {
throw createError('Account locked. Check your email to unlock.', 403);
return createError('Account locked. Check your email to unlock.', 403);
}

return undefined;
};

const updateOldPassword = async (user, password) => {
Expand All @@ -102,9 +104,8 @@ const newFailedLogin = async (user, domain) => {
{ $inc: { failedLogins: 1 } },
{ new: true, fields: '+failedLogins' }
);
if (updatedUser.failedLogins >= MAX_FAILED_LOGIN_ATTEMPTS) {
if (updatedUser?.failedLogins >= MAX_FAILED_LOGIN_ATTEMPTS) {
await blockAccount(user, domain);
throw createError('Account locked. Check your email to unlock.', 403);
}
};

Expand All @@ -118,8 +119,10 @@ const validateUserPassword = async (user, password, domain) => {

if (!oldPasswordValidated && !passwordValidated) {
await newFailedLogin(user, domain);
throw createError('Invalid username or password', 401);
return createError('Invalid username or password', 401);
}

return undefined;
};

const validate2fa = async (user, token, domain) => {
Expand Down Expand Up @@ -251,14 +254,26 @@ export default {
},

async login({ username, password, token }, domain) {
const [user] = await this.get(
const [dbuser] = await this.get(
{ username },
'+password +accountLocked +failedLogins +accountUnlockCode'
);

validateUserStatus(user);
await validateUserPassword(user, password, domain);
const dummy = { password: await encryptPassword('Avoid user enum on login req ms diff') };
const user = dbuser || dummy;

const passwordError = await validateUserPassword(user, password, domain);
const userStatusError = validateUserStatus(user);
await validate2fa(user, token, domain);

if (passwordError) {
throw passwordError;
}

if (userStatusError) {
throw userStatusError;
}

await model.db.updateOne({ _id: user._id }, { $unset: { failedLogins: 1 } });

return sanitizeUser(user);
Expand Down
30 changes: 15 additions & 15 deletions app/react/App/styles/globals.css
Original file line number Diff line number Diff line change
Expand Up @@ -1733,6 +1733,11 @@ input[type="range"]::-ms-fill-lower {
margin-right: 0.25rem;
}

.mx-2 {
margin-left: 0.5rem;
margin-right: 0.5rem;
}

.mx-4 {
margin-left: 1rem;
margin-right: 1rem;
Expand All @@ -1753,11 +1758,6 @@ input[type="range"]::-ms-fill-lower {
margin-bottom: 1rem;
}

.mx-2 {
margin-left: 0.5rem;
margin-right: 0.5rem;
}

.-ml-0 {
margin-left: -0px;
}
Expand Down Expand Up @@ -2960,6 +2960,11 @@ input[type="range"]::-ms-fill-lower {
background-color: rgb(134 239 172 / var(--tw-bg-opacity));
}

.bg-success-50 {
--tw-bg-opacity: 1;
background-color: rgb(240 253 244 / var(--tw-bg-opacity));
}

.bg-success-700 {
--tw-bg-opacity: 1;
background-color: rgb(21 128 61 / var(--tw-bg-opacity));
Expand All @@ -2984,11 +2989,6 @@ input[type="range"]::-ms-fill-lower {
background-color: rgb(253 246 178 / var(--tw-bg-opacity));
}

.bg-success-50 {
--tw-bg-opacity: 1;
background-color: rgb(240 253 244 / var(--tw-bg-opacity));
}

.bg-opacity-50 {
--tw-bg-opacity: 0.5;
}
Expand Down Expand Up @@ -3490,6 +3490,11 @@ input[type="range"]::-ms-fill-lower {
color: rgb(208 56 1 / var(--tw-text-opacity));
}

.text-orange-800 {
--tw-text-opacity: 1;
color: rgb(138 44 13 / var(--tw-text-opacity));
}

.text-pink-600 {
--tw-text-opacity: 1;
color: rgb(214 31 105 / var(--tw-text-opacity));
Expand Down Expand Up @@ -3560,11 +3565,6 @@ input[type="range"]::-ms-fill-lower {
color: rgb(114 59 19 / var(--tw-text-opacity));
}

.text-orange-800 {
--tw-text-opacity: 1;
color: rgb(138 44 13 / var(--tw-text-opacity));
}

.underline {
text-decoration-line: underline;
}
Expand Down
2 changes: 1 addition & 1 deletion contents/ui-translations/ar.csv
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ Install browser extension (dynamic link),Install browser extension (dynamic link
Install Language(s),Install Language(s)
INSTALL the browser extension,INSTALL the browser extension
"Instructions on how to achieve this will vary according to the app used, please refer to the app's documentation.","Instructions on how to achieve this will vary according to the app used, please refer to the app's documentation."
Instructions to reset password,Instructions to reset user password have been sent. Please check your email.
Instructions to reset password,If the email address supplied is correct, we will send instructions to reset the password.
Instructions to reset the password were sent to the user,Instructions to reset the password were sent to the user
Invalid captcha,حروف التحقق غير صحيحة
Invalid csv: all the values for a row must be either nested or non-nested,Invalid csv: all the values for a row must be either nested or non-nested
Expand Down
2 changes: 1 addition & 1 deletion contents/ui-translations/en.csv
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ Install browser extension (dynamic link),Install browser extension (dynamic link
Install Language(s),Install Language(s)
INSTALL the browser extension,INSTALL the browser extension
"Instructions on how to achieve this will vary according to the app used, please refer to the app's documentation.","Instructions on how to achieve this will vary according to the app used, please refer to the app's documentation."
Instructions to reset password,Instructions to reset user password have been sent. Please check your email.
Instructions to reset password,If the email address supplied is correct, we will send instructions to reset the password.
Instructions to reset the password were sent to the user,Instructions to reset the password were sent to the user
Invalid captcha,Invalid captcha
Invalid csv: all the values for a row must be either nested or non-nested,Invalid csv: all the values for a row must be either nested or non-nested
Expand Down
2 changes: 1 addition & 1 deletion contents/ui-translations/fr.csv
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ Install browser extension (dynamic link),Install browser extension (dynamic link
Install Language(s),Install Language(s)
INSTALL the browser extension,INSTALL the browser extension
"Instructions on how to achieve this will vary according to the app used, please refer to the app's documentation.","Instructions on how to achieve this will vary according to the app used, please refer to the app's documentation."
Instructions to reset password,Instructions to reset user password have been sent. Please check your email.
Instructions to reset password,If the email address supplied is correct, we will send instructions to reset the password.
Instructions to reset the password were sent to the user,Instructions to reset the password were sent to the user
Invalid captcha,Captcha invalide
Invalid csv: all the values for a row must be either nested or non-nested,Invalid csv: all the values for a row must be either nested or non-nested
Expand Down
2 changes: 1 addition & 1 deletion contents/ui-translations/ko.csv
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ Install browser extension (dynamic link),Install browser extension (dynamic link
Install Language(s),Install Language(s)
INSTALL the browser extension,INSTALL the browser extension
"Instructions on how to achieve this will vary according to the app used, please refer to the app's documentation.","Instructions on how to achieve this will vary according to the app used, please refer to the app's documentation."
Instructions to reset password,Instructions to reset user password have been sent. Please check your email.
Instructions to reset password,If the email address supplied is correct, we will send instructions to reset the password.
Instructions to reset the password were sent to the user,Instructions to reset the password were sent to the user
Invalid captcha,잘못된 보안문자
Invalid csv: all the values for a row must be either nested or non-nested,Invalid csv: all the values for a row must be either nested or non-nested
Expand Down
2 changes: 1 addition & 1 deletion contents/ui-translations/my.csv
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ Install browser extension (dynamic link),Install browser extension (dynamic link
Install Language(s),Install Language(s)
INSTALL the browser extension,INSTALL the browser extension
"Instructions on how to achieve this will vary according to the app used, please refer to the app's documentation.","Instructions on how to achieve this will vary according to the app used, please refer to the app's documentation."
Instructions to reset password,Instructions to reset user password have been sent. Please check your email.
Instructions to reset password,If the email address supplied is correct, we will send instructions to reset the password.
Instructions to reset the password were sent to the user,Instructions to reset the password were sent to the user
Invalid captcha,ကက်ပ်ချာ မှားနေသည်
Invalid csv: all the values for a row must be either nested or non-nested,Invalid csv: all the values for a row must be either nested or non-nested
Expand Down
2 changes: 1 addition & 1 deletion contents/ui-translations/ru.csv
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ Install browser extension (dynamic link),Install browser extension (dynamic link
Install Language(s),Install Language(s)
INSTALL the browser extension,INSTALL the browser extension
"Instructions on how to achieve this will vary according to the app used, please refer to the app's documentation.","Instructions on how to achieve this will vary according to the app used, please refer to the app's documentation."
Instructions to reset password,Instructions to reset user password have been sent. Please check your email.
Instructions to reset password,If the email address supplied is correct, we will send instructions to reset the password.
Instructions to reset the password were sent to the user,Instructions to reset the password were sent to the user
Invalid captcha,Неверная Captcha
Invalid csv: all the values for a row must be either nested or non-nested,Invalid csv: all the values for a row must be either nested or non-nested
Expand Down
2 changes: 1 addition & 1 deletion contents/ui-translations/th.csv
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ Install browser extension (dynamic link),Install browser extension (dynamic link
Install Language(s),Install Language(s)
INSTALL the browser extension,INSTALL the browser extension
"Instructions on how to achieve this will vary according to the app used, please refer to the app's documentation.","Instructions on how to achieve this will vary according to the app used, please refer to the app's documentation."
Instructions to reset password,Instructions to reset user password have been sent. Please check your email.
Instructions to reset password,If the email address supplied is correct, we will send instructions to reset the password.
Instructions to reset the password were sent to the user,Instructions to reset the password were sent to the user
Invalid captcha,captcha ไม่ถูกต้อง
Invalid csv: all the values for a row must be either nested or non-nested,Invalid csv: all the values for a row must be either nested or non-nested
Expand Down
2 changes: 1 addition & 1 deletion contents/ui-translations/tr.csv
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ Install browser extension (dynamic link),Install browser extension (dynamic link
Install Language(s),Install Language(s)
INSTALL the browser extension,INSTALL the browser extension
"Instructions on how to achieve this will vary according to the app used, please refer to the app's documentation.","Instructions on how to achieve this will vary according to the app used, please refer to the app's documentation."
Instructions to reset password,Instructions to reset user password have been sent. Please check your email.
Instructions to reset password,If the email address supplied is correct, we will send instructions to reset the password.
Instructions to reset the password were sent to the user,Instructions to reset the password were sent to the user
Invalid captcha,Geçersiz güvenlik kodu
Invalid csv: all the values for a row must be either nested or non-nested,Invalid csv: all the values for a row must be either nested or non-nested
Expand Down

0 comments on commit 2be0fbf

Please sign in to comment.