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(editor): User account deletion endpoint #843

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@
"rewire": "^5.0.0",
"sass": "^1.49.0",
"sass-loader": "^12.4.0",
"superagent-mock": "^4.0.0",
"typescript": "^4.0.5",
"uuid": "^8.3.2",
"webpack": "^5.69.1",
Expand Down
41 changes: 41 additions & 0 deletions src/server/routes/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import express from 'express';
import passport from 'passport';
import request from 'superagent';
import status from 'http-status';


Expand Down Expand Up @@ -73,4 +74,44 @@ router.get('/logout', (req, res) => {
res.redirect(status.SEE_OTHER, '/');
});

router.post('/delete-user/:mbRowId', async (req, res) => {
// Special Account on MB
const USER_DELETER = 'UserDeleter';
const USER_DELETER_MBID = 2007538;
const {access_token: accessToken} = req.query;
const {mbRowId} = req.params;
try {
const rBody = await request.get('https://musicbrainz.org/oauth2/userinfo').set('Authorization', `Bearer ${accessToken}`).then((rs) => rs.body);
if (rBody.metabrainz_user_id !== USER_DELETER_MBID || rBody.sub !== USER_DELETER) {
return res.status(401).send();
}
const {orm} = req.app.locals;
const {Editor} = orm;
const editor = await Editor.forge({metabrainzUserId: mbRowId}).fetch({require: false});
if (!editor) {
return res.status(404).send();
}
// deleting all user info
const deletedUser = `Deleted User #${editor.get('id')}`;
editor.set('bio', '');
editor.set('genderId', null);
editor.set('areaId', null);
editor.set('revisionsApplied', 0);
editor.set('revisionsReverted', 0);
editor.set('totalRevisions', 0);
editor.set('metabrainzUserId', null);
editor.set('cachedMetabrainzName', null);
editor.set('titleUnlockId', null);


editor.set('name', deletedUser);
await editor.save();
return res.status(200).send();
}
catch (err) {
return res.status(err.status ?? 500).send();
}
});


export default router;
69 changes: 69 additions & 0 deletions test/src/server/routes/auth.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import {createEditor, truncateEntities} from '../../../test-helpers/create-entities';
import app from '../../../../src/server/app';
import chai from 'chai';
import chaiHttp from 'chai-http';
import config from '../../../test-helpers/superagent-mock-config';
import mock from 'superagent-mock';
import {omit} from 'lodash';
import orm from '../../../bookbrainz-data';
import request from 'superagent';


mock(request, config);

chai.use(chaiHttp);
const {expect} = chai;
const {Editor} = orm;

const deletedUserAttrib = {
areaId: null,
bio: '',
cachedMetabrainzName: null,
genderId: null,
id: 2,
metabrainzUserId: null,
name: 'Deleted User #2',
reputation: 0,
revisionsApplied: 0,
revisionsReverted: 0,
titleUnlockId: null,
totalRevisions: 0
};

describe('delete user', () => {
const adminEditorId = 1;
const userEditorId = 2;
const userMBId = 2;
const otherEditorId = 3;
let agent;
before(async () => {
await createEditor(userEditorId, {metabrainzUserId: userMBId});
agent = chai.request.agent(app);
await agent.get('/cb');
});

after(truncateEntities);

it('Normal User should not be able to delete other users', async () => {
const res = await agent.post(`/delete-user/${userMBId}?access_token=${otherEditorId}`);
expect(res.ok).to.be.false;
// unauthorized
expect(res.status).to.be.equal(401);
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd also love to see, for good measure, another test to ensure we get a 401 for an unauthenticated user.
Basically the same test as the above, but using await chai.request(app).post(… <- this will not use a user session because we won't call /cb as we do in the before function


it('User Deleter should be able to delete a user', async () => {
const res = await agent.post(`/delete-user/${userMBId}?access_token=${adminEditorId}`);
expect(res.ok).to.be.true;
const editor = await new Editor({id: userEditorId}).fetch();
expect(editor).to.exist;
const editorJson = omit(editor.toJSON(), ['activeAt', 'createdAt', 'typeId']);
expect(editorJson).to.deep.equal(deletedUserAttrib);
});

it('Unauthenticated user should not be able to delete other users', async () => {
const res = await chai.request(app).post(`/delete-user/${userMBId}`);
expect(res.ok).to.be.false;
// unauthorized
expect(res.status).to.be.equal(401);
});
});
4 changes: 2 additions & 2 deletions test/test-helpers/create-entities.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ const entityAttribs = {
revisionId: 1
};

export function createEditor(editorId) {
export function createEditor(editorId, otherEditorAttribs) {
return orm.bookshelf.knex.transaction(async (transacting) => {
const editorType = await new EditorType(editorTypeAttribs)
.save(null, {method: 'insert', transacting});
Expand All @@ -125,7 +125,7 @@ export function createEditor(editorId) {
editorAttribs.metabrainzUserId = random.number();
editorAttribs.cachedMetabrainzName = editorAttribs.name;

const editor = await new Editor(editorAttribs)
const editor = await new Editor({...editorAttribs, ...otherEditorAttribs})
.save(null, {method: 'insert', transacting});
return editor;
});
Expand Down
41 changes: 41 additions & 0 deletions test/test-helpers/superagent-mock-config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/* eslint-disable camelcase */
const mockConfig = [
{
fixtures: (match, params, headers) => {
// extract auth token from headers
const token = headers.Authorization.split(' ')[1];
// using user id as tokens for testing
let sub;
let metabrainz_user_id;
switch (token) {
case '1':
sub = 'UserDeleter';
metabrainz_user_id = 2007538;
break;
case '2':
sub = 'NormalUser1';
metabrainz_user_id = 2;
break;
case '3':
sub = 'NormalUser2';
metabrainz_user_id = 3;
break;
default:
sub = 'UnknownUser';
metabrainz_user_id = 0;
break;
}
const body = {
metabrainz_user_id,
sub
};
return {body};
},
get(match, data) {
return data;
},
pattern: 'https://musicbrainz.org/oauth2/userinfo'
}
];

export default mockConfig;
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -7817,6 +7817,11 @@ stylis@4.0.13:
resolved "https://registry.yarnpkg.com/stylis/-/stylis-4.0.13.tgz#f5db332e376d13cc84ecfe5dace9a2a51d954c91"
integrity sha512-xGPXiFVl4YED9Jh7Euv2V220mriG9u4B2TA6Ybjc1catrstKD2PpIdU3U0RKpkVBC2EhmL/F0sPCr9vrFTNRag==

superagent-mock@^4.0.0:
version "4.0.0"
resolved "https://registry.yarnpkg.com/superagent-mock/-/superagent-mock-4.0.0.tgz#6c33457c312c4dc4082220c8b9c4eb0067dc67a9"
integrity sha512-+xj+q+sL5sJIcFxwmj5Wuq59Kns0ocOd8OrMkbEXEwseWImAVvM7cP8G7raQRZ+vloUN32t/yKosD+YAXa9rcg==

superagent@^3.7.0:
version "3.8.3"
resolved "https://registry.yarnpkg.com/superagent/-/superagent-3.8.3.tgz#460ea0dbdb7d5b11bc4f78deba565f86a178e128"
Expand Down