self-hosted/joplin
1
0
Fork 0
mirror of https://github.com/laurent22/joplin.git synced 2025-01-11 18:24:43 +02:00
Code Issues Releases Activity

Server: Fail-safe when trying to delete a non-disabled account

Browse Source
This commit is contained in:
Laurent Cozic 2022-11-04 16:18:41 +00:00
parent bbc4228ed9
commit 21883b4e6b
3 changed files with 47 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -339,6 +339,7 @@ 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);
await models.user().checkIfAllowed(owner, AclAction.Delete, user);
await models.user().setEnabled(path.id, !!fields.restore_button);
} else if (fields.send_account_confirmation_email) {

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

@ -32,6 +32,8 @@ describe('UserDeletionService', function() {
const t0 = new Date('2021-12-14').getTime();
const t1 = t0 + 1000;
await models().user().setEnabled(user1.id, false);
const job = await models().userDeletion().add(user1.id, t1, {
processData: true,
processAccount: false,
@ -63,6 +65,8 @@ describe('UserDeletionService', function() {
const t0 = new Date('2021-12-14').getTime();
const t1 = t0 + 1000;
await models().user().setEnabled(user1.id, false);
const job = await models().userDeletion().add(user1.id, t1, {
processData: false,
processAccount: true,
@ -92,7 +96,7 @@ describe('UserDeletionService', function() {
const content = JSON.parse(backupItem.content.toString());
expect(content.user.id).toBe(user1.id);
expect(content.user.email).toBe(user1.email);
expect(content.flags.length).toBe(0);
expect(content.flags.length).toBe(1);
});
test('should not delete notebooks that are not owned', async function() {
@ -113,6 +117,8 @@ describe('UserDeletionService', function() {
expect(await models().share().count()).toBe(1);
expect(await models().shareUser().count()).toBe(1);
await models().user().setEnabled(user2.id, false);
const job = await models().userDeletion().add(user2.id, Date.now());
const service = newService();
await service.processDeletionJob(job, { sleepBetweenOperations: 0 });
@ -140,6 +146,8 @@ describe('UserDeletionService', function() {
expect(await models().share().count()).toBe(1);
expect(await models().shareUser().count()).toBe(1);
await models().user().setEnabled(user1.id, false);
const job = await models().userDeletion().add(user1.id, Date.now());
const service = newService();
await service.processDeletionJob(job, { sleepBetweenOperations: 0 });
@ -149,4 +157,27 @@ describe('UserDeletionService', function() {
expect(await models().item().count()).toBe(0);
});
test('should not do anything if the user is still enabled', async function() {
const { user: user1 } = await createUserAndSession(1);
const t0 = new Date('2021-12-14').getTime();
const t1 = t0 + 1000;
const job = await models().userDeletion().add(user1.id, t1, {
processData: false,
processAccount: true,
});
expect(await models().userDeletion().count()).toBe(1);
const service = newService();
await service.processDeletionJob(job, { sleepBetweenOperations: 0 });
// Nothing has been done because the user is still enabled
expect(await models().user().count()).toBe(1);
// And the job should have been removed from the queue
expect(await models().userDeletion().count()).toBe(0);
});
});

14
packages/server/src/services/UserDeletionService.ts
View File

@ -90,6 +90,20 @@ export default class UserDeletionService extends BaseService {
logger.info('Starting user deletion: ', deletion);
// Normally, a user that is still enabled should not be processed here,
// because it should not have been queued to begin with (or if it was
// queued, then enabled, it should have been removed from the queue).
// But as a fail safe we have this extra check.
//
// We also remove the job from the queue so that the service doesn't try
// to process it again.
const user = await this.models.user().load(deletion.user_id);
if (user.enabled) {
logger.error(`Trying to delete a user that is still enabled - aborting and removing the user from the queue. Deletion job: ${JSON.stringify(deletion)}`);
await this.models.userDeletion().removeFromQueueByUserId(user.id);
return;
}
let error: any = null;
let success: boolean = true;