self-hosted/joplin
1
0
Fork 0
mirror of https://github.com/laurent22/joplin.git synced 2024-11-27 08:21:03 +02:00
Code Issues Releases Activity

Server: Fixed account activation logic

Browse Source
This commit is contained in:
Laurent Cozic 2022-11-04 16:36:54 +00:00
parent 21883b4e6b
commit ccf5d204f0
5 changed files with 38 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
packages/server/src/models/UserFlagModel.ts
View File

@ -1,8 +1,11 @@
import Logger from '@joplin/lib/Logger';
import { isUniqueConstraintError } from '../db';
import { User, UserFlag, UserFlagType, userFlagTypeToLabel, Uuid } from '../services/database/types';
import { formatDateTime } from '../utils/time';
import BaseModel from './BaseModel';
const logger = Logger.create('UserFlagModel');
interface AddRemoveOptions {
updateUser?: boolean;
}
@ -138,15 +141,35 @@ export default class UserFlagModels extends BaseModel<UserFlag> {
newProps.enabled = 0;
}
let removeFromDeletionQueue = false;
if (!user.enabled && newProps.enabled) {
if (await this.models().userDeletion().isDeletedOrBeingDeleted(userId)) {
// User account is being deleted or already deleted and cannot
// be enabled again.
logger.error('Trying to enable an account that is queued for deletion - leaving account disabled');
newProps.enabled = 0;
} else {
// If the user has been re-enabled, we want to remove it from
// the deletion queue (if it has been queued there) immediately,
// so that it doesn't incorrectly get deleted.
removeFromDeletionQueue = true;
}
}
if (user.enabled !== newProps.enabled) {
newProps.disabled_time = !newProps.enabled ? Date.now() : 0;
}
if (user.can_upload !== newProps.can_upload || user.enabled !== newProps.enabled) {
await this.models().user().save({
id: userId,
...newProps,
});
await this.withTransaction(async () => {
if (removeFromDeletionQueue) await this.models().userDeletion().removeFromQueueByUserId(userId);
await this.models().user().save({
id: userId,
...newProps,
});
}, 'UserFlagModel::updateUserFromFlags');
}
}

8
packages/server/src/models/UserModel.test.ts
View File

@ -413,7 +413,7 @@ describe('UserModel', function() {
const t0 = new Date('2022-01-01').getTime();
jest.setSystemTime(t0);
await models().user().setEnabled(user1.id, false);
await models().userFlag().add(user1.id, UserFlagType.ManuallyDisabled);
expect((await models().user().load(user1.id)).enabled).toBe(0);
expect((await models().user().load(user2.id)).enabled).toBe(1);
@ -428,10 +428,10 @@ describe('UserModel', function() {
// If we make the account enabled again, the user should be immediately
// removed from the queue
await models().user().setEnabled(user1.id, true);
await models().userFlag().remove(user1.id, UserFlagType.ManuallyDisabled);
expect(await models().userDeletion().count()).toBe(0);
await models().user().setEnabled(user1.id, false);
await models().userFlag().add(user1.id, UserFlagType.ManuallyDisabled);
const t2 = new Date('2022-03-01').getTime();
jest.setSystemTime(t2);
@ -450,7 +450,7 @@ describe('UserModel', function() {
expect(job.user_id).toBe(user1.id);
await models().userDeletion().start(job.id);
await expectThrow(async () => models().user().setEnabled(user1.id, true));
await models().userFlag().add(user1.id, UserFlagType.ManuallyDisabled);
expect((await models().user().load(user1.id)).enabled).toBe(0);
});

14
packages/server/src/models/UserModel.ts
View File

@ -618,20 +618,6 @@ export default class UserModel extends BaseModel<User> {
return syncInfo.ppk?.value || null;
}
public async setEnabled(userId: string, enabled: boolean) {
const user = await this.load(userId);
if (!user) throw new ErrorNotFound(`Not found: ${userId}`);
if (await this.models().userDeletion().isDeletedOrBeingDeleted(userId)) throw new ErrorBadRequest('User account is being deleted or already deleted and cannot be enabled again');
await this.models().userFlag().toggle(user.id, UserFlagType.ManuallyDisabled, !enabled);
// If the user has been re-enabled, we want to remove it from the
// deletion queue (if it has been queued there) immediately, so that it
// doesn't incorrectly get deleted.
if (enabled) await this.models().userDeletion().removeFromQueueByUserId(userId);
}
// Note that when the "password" property is provided, it is going to be
// hashed automatically. It means that it is not safe to do:
//

4
packages/server/src/routes/admin/users.ts
View File

@ -339,9 +339,9 @@ router.post('admin/users', async (path: SubPath, ctx: AppContext) => {
await stopImpersonating(ctx);
return redirect(ctx, config().baseUrl);
} else if (fields.disable_button || fields.restore_button) {
const user = await models.user().load(userId);
const user = await models.user().load(path.id);
await models.user().checkIfAllowed(owner, AclAction.Delete, user);
await models.user().setEnabled(path.id, !!fields.restore_button);
await models.userFlag().toggle(user.id, UserFlagType.ManuallyDisabled, !fields.restore_button);
} else if (fields.send_account_confirmation_email) {
const user = await models.user().load(path.id);
await models.user().save({ id: user.id, must_set_password: 1 });

10
packages/server/src/services/UserDeletionService.test.ts
View File

@ -2,7 +2,7 @@ import config from '../config';
import { shareFolderWithUser } from '../utils/testing/shareApiUtils';
import { afterAllTests, beforeAllDb, beforeEachDb, createNote, createUserAndSession, models } from '../utils/testing/testUtils';
import { Env } from '../utils/types';
import { BackupItemType } from './database/types';
import { BackupItemType, UserFlagType } from './database/types';
import UserDeletionService from './UserDeletionService';
const newService = () => {
@ -32,7 +32,7 @@ describe('UserDeletionService', function() {
const t0 = new Date('2021-12-14').getTime();
const t1 = t0 + 1000;
await models().user().setEnabled(user1.id, false);
await models().userFlag().toggle(user1.id, UserFlagType.ManuallyDisabled, true);
const job = await models().userDeletion().add(user1.id, t1, {
processData: true,
@ -65,7 +65,7 @@ describe('UserDeletionService', function() {
const t0 = new Date('2021-12-14').getTime();
const t1 = t0 + 1000;
await models().user().setEnabled(user1.id, false);
await models().userFlag().toggle(user1.id, UserFlagType.ManuallyDisabled, true);
const job = await models().userDeletion().add(user1.id, t1, {
processData: false,
@ -117,7 +117,7 @@ describe('UserDeletionService', function() {
expect(await models().share().count()).toBe(1);
expect(await models().shareUser().count()).toBe(1);
await models().user().setEnabled(user2.id, false);
await models().userFlag().toggle(user2.id, UserFlagType.ManuallyDisabled, true);
const job = await models().userDeletion().add(user2.id, Date.now());
const service = newService();
@ -146,7 +146,7 @@ describe('UserDeletionService', function() {
expect(await models().share().count()).toBe(1);
expect(await models().shareUser().count()).toBe(1);
await models().user().setEnabled(user1.id, false);
await models().userFlag().toggle(user1.id, UserFlagType.ManuallyDisabled, true);
const job = await models().userDeletion().add(user1.id, Date.now());
const service = newService();