From ad8811b5585d382bd37839c738d029f3b4f23fad Mon Sep 17 00:00:00 2001 From: tri10 Date: Sat, 21 May 2022 18:04:05 +0530 Subject: [PATCH 1/5] feat(editor): delete user from BB project --- src/server/routes/auth.js | 42 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/server/routes/auth.js b/src/server/routes/auth.js index d70203825d..9b77812250 100644 --- a/src/server/routes/auth.js +++ b/src/server/routes/auth.js @@ -18,6 +18,7 @@ import express from 'express'; import passport from 'passport'; +import request from 'superagent'; import status from 'http-status'; @@ -73,4 +74,45 @@ router.get('/logout', (req, res) => { res.redirect(status.SEE_OTHER, '/'); }); +router.post('/delete-user/:uname', async (req, res) => { + // Special Account on MB + const USER_DELETER = 'UserDeleter'; + const USER_DELETER_MBID = 2007538; + const {access_token: accessToken} = req.query; + const {uname} = 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({name: uname}).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('gender_id', null); + editor.set('area_id', null); + editor.set('revisions_applied', 0); + editor.set('revisions_reverted', 0); + editor.set('total_revisions', 0); + editor.set('metabrainz_user_id', null); + editor.set('cached_metabrainz_name', null); + editor.set('title_unlock_id', null); + + + editor.set('name', deletedUser); + await editor.save(); + // eslint-disable-next-line camelcase + return res.send({metabrainz_user_id: USER_DELETER_MBID, sub: USER_DELETER}); + } + catch (err) { + return res.status(err.status ?? 500).send(); + } +}); + + export default router; From b16f010c8643baccf3af9b7f89b84f128ea51b4e Mon Sep 17 00:00:00 2001 From: tr1ten Date: Wed, 21 Sep 2022 18:43:35 +0530 Subject: [PATCH 2/5] test: add tests for '/delete-user' endpoint --- package.json | 1 + src/server/routes/auth.js | 9 ++++ superagent-mock-config.js | 41 ++++++++++++++++++ test/src/server/routes/auth.js | 65 ++++++++++++++++++++++++++++ test/test-helpers/create-entities.js | 4 +- yarn.lock | 5 +++ 6 files changed, 123 insertions(+), 2 deletions(-) create mode 100644 superagent-mock-config.js create mode 100644 test/src/server/routes/auth.js diff --git a/package.json b/package.json index 908f3ce1ba..f6345966db 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/src/server/routes/auth.js b/src/server/routes/auth.js index 9b77812250..cfbe8e1d02 100644 --- a/src/server/routes/auth.js +++ b/src/server/routes/auth.js @@ -16,12 +16,21 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ +import config from '../../../superagent-mock-config'; import express from 'express'; +import mock from 'superagent-mock'; import passport from 'passport'; import request from 'superagent'; import status from 'http-status'; +// Setting up mocking agent for test +// eslint-disable-next-line node/no-process-env +if (process.env.NODE_ENV === 'test') { + mock(request, config); +} + + const router = express.Router(); // eslint-disable-next-line node/no-process-env diff --git a/superagent-mock-config.js b/superagent-mock-config.js new file mode 100644 index 0000000000..e83c03164b --- /dev/null +++ b/superagent-mock-config.js @@ -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; diff --git a/test/src/server/routes/auth.js b/test/src/server/routes/auth.js new file mode 100644 index 0000000000..4cd7e2b879 --- /dev/null +++ b/test/src/server/routes/auth.js @@ -0,0 +1,65 @@ +import {createEditor, truncateEntities} from '../../../test-helpers/create-entities'; +import app from '../../../../src/server/app'; +import chai from 'chai'; +import chaiHttp from 'chai-http'; +import {get} from 'lodash'; +import orm from '../../../bookbrainz-data'; + + +chai.use(chaiHttp); +const {expect} = chai; +const {Editor} = orm; + +const deletedUserAttrib = { + areaId: null, + bio: '', + cachedMetabrainzName: null, + genderId: null, + metabrainzUserId: null, + revisionsApplied: 0, + revisionsReverted: 0, + titleUnlockId: null, + totalRevisions: 0 +}; + +function verifyDeletedUser(user) { + if (!get(user, 'name', '').includes('Deleted User')) { return false; } + for (const key in deletedUserAttrib) { + if (Object.hasOwnProperty.call(deletedUserAttrib, key)) { + if (get(user, key) !== get(user, key, null)) { + return false; + } + } + } + return true; +} +describe('delete user', () => { + const adminEditorId = 1; + const adminEditorName = 'UserDeleter'; + const userEditorId = 2; + const userEditorName = 'NormalUser1'; + const otherEditorId = 3; + const otherEditorName = 'NormalUser2'; + let agent; + before(async () => { + await createEditor(adminEditorId, {name: adminEditorName}); + await createEditor(userEditorId, {name: userEditorName}); + await createEditor(otherEditorId, {name: otherEditorName}); + 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/${otherEditorName}?access_token=${userEditorId}`); + expect(res.ok).to.be.false; + // unauthorized + expect(res.status).to.be.equal(401); + }); + it('User Deleter should be able to delete a user', async () => { + const res = await agent.post(`/delete-user/${userEditorName}?access_token=${adminEditorId}`); + expect(res.ok).to.be.true; + const editor = await new Editor({id: userEditorId}).fetch(); + expect(editor).to.exist; + expect(verifyDeletedUser(editor.toJSON())).to.be.true; + }); +}); diff --git a/test/test-helpers/create-entities.js b/test/test-helpers/create-entities.js index 3785d9d8e6..db6b33f716 100644 --- a/test/test-helpers/create-entities.js +++ b/test/test-helpers/create-entities.js @@ -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}); @@ -121,7 +121,7 @@ export function createEditor(editorId) { editorAttribs.id = editorId || random.number(); editorAttribs.genderId = gender.id; editorAttribs.typeId = editorType.id; - editorAttribs.name = internet.userName(); + editorAttribs.name = otherEditorAttribs?.name || internet.userName(); editorAttribs.metabrainzUserId = random.number(); editorAttribs.cachedMetabrainzName = editorAttribs.name; diff --git a/yarn.lock b/yarn.lock index cd42eb5294..11857dd70e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -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" From 6bdaaebcbf19c8f46c322595d8dbaae506842db6 Mon Sep 17 00:00:00 2001 From: tr1ten Date: Fri, 23 Sep 2022 16:43:41 +0530 Subject: [PATCH 3/5] use user's metabrainz id to fetch their info --- src/server/routes/auth.js | 36 ++++++++++------------------ test/src/server/routes/auth.js | 36 ++++++++++++---------------- test/test-helpers/create-entities.js | 4 ++-- 3 files changed, 30 insertions(+), 46 deletions(-) diff --git a/src/server/routes/auth.js b/src/server/routes/auth.js index cfbe8e1d02..3cfbb4d16b 100644 --- a/src/server/routes/auth.js +++ b/src/server/routes/auth.js @@ -16,21 +16,12 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ -import config from '../../../superagent-mock-config'; import express from 'express'; -import mock from 'superagent-mock'; import passport from 'passport'; import request from 'superagent'; import status from 'http-status'; -// Setting up mocking agent for test -// eslint-disable-next-line node/no-process-env -if (process.env.NODE_ENV === 'test') { - mock(request, config); -} - - const router = express.Router(); // eslint-disable-next-line node/no-process-env @@ -83,12 +74,12 @@ router.get('/logout', (req, res) => { res.redirect(status.SEE_OTHER, '/'); }); -router.post('/delete-user/:uname', async (req, res) => { +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 {uname} = req.params; + 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) { @@ -96,27 +87,26 @@ router.post('/delete-user/:uname', async (req, res) => { } const {orm} = req.app.locals; const {Editor} = orm; - const editor = await Editor.forge({name: uname}).fetch({require: false}); + 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')}`; + const deletedUser = `Deleted User #${editor.get('id')}`; editor.set('bio', ''); - editor.set('gender_id', null); - editor.set('area_id', null); - editor.set('revisions_applied', 0); - editor.set('revisions_reverted', 0); - editor.set('total_revisions', 0); - editor.set('metabrainz_user_id', null); - editor.set('cached_metabrainz_name', null); - editor.set('title_unlock_id', null); + 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(); - // eslint-disable-next-line camelcase - return res.send({metabrainz_user_id: USER_DELETER_MBID, sub: USER_DELETER}); + return res.status(200).send(); } catch (err) { return res.status(err.status ?? 500).send(); diff --git a/test/src/server/routes/auth.js b/test/src/server/routes/auth.js index 4cd7e2b879..3dfaeab322 100644 --- a/test/src/server/routes/auth.js +++ b/test/src/server/routes/auth.js @@ -2,10 +2,15 @@ import {createEditor, truncateEntities} from '../../../test-helpers/create-entit import app from '../../../../src/server/app'; import chai from 'chai'; import chaiHttp from 'chai-http'; -import {get} from 'lodash'; +import config from '../../../../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; @@ -15,51 +20,40 @@ const deletedUserAttrib = { bio: '', cachedMetabrainzName: null, genderId: null, + id: 2, metabrainzUserId: null, + name: 'Deleted User #2', + reputation: 0, revisionsApplied: 0, revisionsReverted: 0, titleUnlockId: null, totalRevisions: 0 }; -function verifyDeletedUser(user) { - if (!get(user, 'name', '').includes('Deleted User')) { return false; } - for (const key in deletedUserAttrib) { - if (Object.hasOwnProperty.call(deletedUserAttrib, key)) { - if (get(user, key) !== get(user, key, null)) { - return false; - } - } - } - return true; -} describe('delete user', () => { const adminEditorId = 1; - const adminEditorName = 'UserDeleter'; const userEditorId = 2; - const userEditorName = 'NormalUser1'; + const userMBId = 2; const otherEditorId = 3; - const otherEditorName = 'NormalUser2'; let agent; before(async () => { - await createEditor(adminEditorId, {name: adminEditorName}); - await createEditor(userEditorId, {name: userEditorName}); - await createEditor(otherEditorId, {name: otherEditorName}); + 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/${otherEditorName}?access_token=${userEditorId}`); + 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); }); it('User Deleter should be able to delete a user', async () => { - const res = await agent.post(`/delete-user/${userEditorName}?access_token=${adminEditorId}`); + 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; - expect(verifyDeletedUser(editor.toJSON())).to.be.true; + const editorJson = omit(editor.toJSON(), ['activeAt', 'createdAt', 'typeId']); + expect(editorJson).to.deep.equal(deletedUserAttrib); }); }); diff --git a/test/test-helpers/create-entities.js b/test/test-helpers/create-entities.js index db6b33f716..69210098e7 100644 --- a/test/test-helpers/create-entities.js +++ b/test/test-helpers/create-entities.js @@ -121,11 +121,11 @@ export function createEditor(editorId, otherEditorAttribs) { editorAttribs.id = editorId || random.number(); editorAttribs.genderId = gender.id; editorAttribs.typeId = editorType.id; - editorAttribs.name = otherEditorAttribs?.name || internet.userName(); + editorAttribs.name = internet.userName(); 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; }); From 299baaccdb02dd36b8d181a039e9ef8560ae4c19 Mon Sep 17 00:00:00 2001 From: tr1ten Date: Fri, 23 Sep 2022 16:52:37 +0530 Subject: [PATCH 4/5] test: add test for unauthenticated user --- test/src/server/routes/auth.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/src/server/routes/auth.js b/test/src/server/routes/auth.js index 3dfaeab322..9d86d6699a 100644 --- a/test/src/server/routes/auth.js +++ b/test/src/server/routes/auth.js @@ -41,13 +41,16 @@ describe('delete user', () => { 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); }); + 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; @@ -56,4 +59,11 @@ describe('delete user', () => { 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); + }); }); From 14b9e601787b9d264211907fd33dd9d8c0ac06ee Mon Sep 17 00:00:00 2001 From: tr1ten Date: Fri, 23 Sep 2022 16:56:11 +0530 Subject: [PATCH 5/5] move superagent-config file into test directory --- test/src/server/routes/auth.js | 2 +- .../test-helpers/superagent-mock-config.js | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename superagent-mock-config.js => test/test-helpers/superagent-mock-config.js (100%) diff --git a/test/src/server/routes/auth.js b/test/src/server/routes/auth.js index 9d86d6699a..1aafba1081 100644 --- a/test/src/server/routes/auth.js +++ b/test/src/server/routes/auth.js @@ -2,7 +2,7 @@ import {createEditor, truncateEntities} from '../../../test-helpers/create-entit import app from '../../../../src/server/app'; import chai from 'chai'; import chaiHttp from 'chai-http'; -import config from '../../../../superagent-mock-config'; +import config from '../../../test-helpers/superagent-mock-config'; import mock from 'superagent-mock'; import {omit} from 'lodash'; import orm from '../../../bookbrainz-data'; diff --git a/superagent-mock-config.js b/test/test-helpers/superagent-mock-config.js similarity index 100% rename from superagent-mock-config.js rename to test/test-helpers/superagent-mock-config.js