From 32e36f7b2e8f9c0de4887a3bb437cadeeec69102 Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Tue, 22 Dec 2020 14:26:31 -0500 Subject: [PATCH] feat(api): group ownership API route, switch client-side to use API route --- public/openapi/write.yaml | 2 ++ public/src/admin/manage/group.js | 14 ++++---------- public/src/client/groups/details.js | 28 ++++++---------------------- src/api/groups.js | 22 ++++++++++++++++++++++ src/controllers/write/groups.js | 10 ++++++++++ src/groups/ownership.js | 7 +++---- src/routes/write/groups.js | 2 ++ src/socket.io/groups.js | 4 ++++ 8 files changed, 53 insertions(+), 36 deletions(-) diff --git a/public/openapi/write.yaml b/public/openapi/write.yaml index ab6f8753d9..2b6266d64c 100644 --- a/public/openapi/write.yaml +++ b/public/openapi/write.yaml @@ -74,6 +74,8 @@ paths: $ref: 'write/groups/slug.yaml' /groups/{slug}/membership/{uid}: $ref: 'write/groups/slug/membership/uid.yaml' + /groups/{slug}/ownership/{uid}: + $ref: 'write/groups/slug/ownership/uid.yaml' /categories/: $ref: 'write/categories.yaml' /categories/{cid}: diff --git a/public/src/admin/manage/group.js b/public/src/admin/manage/group.js index e041164bbf..5eb41bab9f 100644 --- a/public/src/admin/manage/group.js +++ b/public/src/admin/manage/group.js @@ -39,7 +39,7 @@ define('admin/manage/group', [ groupLabelPreview.css('color', changeGroupTextColor.val() || '#ffffff'); }); - setupGroupMembersMenu(groupName); + setupGroupMembersMenu(); $('#group-icon, #group-icon-label').on('click', function () { var currentIcon = groupIcon.attr('value'); @@ -96,7 +96,7 @@ define('admin/manage/group', [ }); }; - function setupGroupMembersMenu(groupName) { + function setupGroupMembersMenu() { $('[component="groups/members"]').on('click', '[data-action]', function () { var btnEl = $(this); var userRow = btnEl.parents('[data-uid]'); @@ -107,15 +107,9 @@ define('admin/manage/group', [ switch (action) { case 'toggleOwnership': - socket.emit('groups.' + (isOwner ? 'rescind' : 'grant'), { - toUid: uid, - groupName: groupName, - }, function (err) { - if (err) { - return app.alertError(err.message); - } + api[isOwner ? 'del' : 'put'](`/groups/${ajaxify.data.group.slug}/ownership/${uid}`, {}).then(() => { ownerFlagEl.toggleClass('invisible'); - }); + }).catch(app.alertError); break; case 'kick': diff --git a/public/src/client/groups/details.js b/public/src/client/groups/details.js index 502af28949..9b240fc5d3 100644 --- a/public/src/client/groups/details.js +++ b/public/src/client/groups/details.js @@ -64,16 +64,9 @@ define('forum/groups/details', [ switch (action) { case 'toggleOwnership': - socket.emit('groups.' + (isOwner ? 'rescind' : 'grant'), { - toUid: uid, - groupName: groupName, - }, function (err) { - if (!err) { - ownerFlagEl.toggleClass('invisible'); - } else { - app.alertError(err.message); - } - }); + api[isOwner ? 'del' : 'put'](`/groups/${ajaxify.data.group.slug}/ownership/${uid}`, {}).then(() => { + ownerFlagEl.toggleClass('invisible'); + }).catch(app.alertError); break; case 'kick': @@ -83,16 +76,7 @@ define('forum/groups/details', [ return; } - socket.emit('groups.kick', { - uid: uid, - groupName: groupName, - }, function (err) { - if (!err) { - userRow.slideUp().remove(); - } else { - app.alertError(err.message); - } - }); + api.del(`/groups/${ajaxify.data.group.slug}/membership/${uid}`, undefined).then(() => userRow.slideUp().remove()).catch(app.alertError); }); }); break; @@ -203,7 +187,7 @@ define('forum/groups/details', [ } }); - api.put(`/groups/${slugify(groupName)}`, settings).then(() => { + api.put(`/groups/${ajaxify.data.group.slug}`, settings).then(() => { if (settings.name) { var pathname = window.location.pathname; pathname = pathname.substr(1, pathname.lastIndexOf('/')); @@ -222,7 +206,7 @@ define('forum/groups/details', [ if (confirm) { bootbox.prompt('Please enter the name of this group in order to delete it:', function (response) { if (response === groupName) { - api.del(`/groups/${slugify(groupName)}`, {}).then(() => { + api.del(`/groups/${ajaxify.data.group.slug}`, {}).then(() => { app.alertSuccess('[[groups:event.deleted, ' + utils.escapeHTML(groupName) + ']]'); ajaxify.go('groups'); }).catch(app.alertError); diff --git a/src/api/groups.js b/src/api/groups.js index 1a0157e8e9..9f68d31afe 100644 --- a/src/api/groups.js +++ b/src/api/groups.js @@ -177,6 +177,28 @@ groupsAPI.leave = async function (caller, data) { }); }; +groupsAPI.grant = async (caller, data) => { + const groupName = await groups.getGroupNameByGroupSlug(data.slug); + await isOwner(caller, groupName); + + await groups.ownership.grant(data.uid, groupName); + logGroupEvent(caller, 'group-owner-grant', { + groupName: groupName, + targetUid: data.uid, + }); +}; + +groupsAPI.rescind = async (caller, data) => { + const groupName = await groups.getGroupNameByGroupSlug(data.slug); + await isOwner(caller, groupName); + + await groups.ownership.rescind(data.uid, groupName); + logGroupEvent(caller, 'group-owner-rescind', { + groupName: groupName, + targetUid: data.uid, + }); +}; + async function isOwner(caller, groupName) { if (typeof groupName !== 'string') { throw new Error('[[error:invalid-group-name]]'); diff --git a/src/controllers/write/groups.js b/src/controllers/write/groups.js index 0831e2ed43..f4019cb075 100644 --- a/src/controllers/write/groups.js +++ b/src/controllers/write/groups.js @@ -37,3 +37,13 @@ Groups.leave = async (req, res) => { await api.groups.leave(req, req.params); helpers.formatApiResponse(200, res); }; + +Groups.grant = async (req, res) => { + await api.groups.grant(req, req.params); + helpers.formatApiResponse(200, res); +}; + +Groups.rescind = async (req, res) => { + await api.groups.rescind(req, req.params); + helpers.formatApiResponse(200, res); +}; diff --git a/src/groups/ownership.js b/src/groups/ownership.js index 4296c307c0..9f0ce9a08f 100644 --- a/src/groups/ownership.js +++ b/src/groups/ownership.js @@ -22,16 +22,15 @@ module.exports = function (Groups) { }; Groups.ownership.grant = async function (toUid, groupName) { - // Note: No ownership checking is done here on purpose! await db.setAdd('group:' + groupName + ':owners', toUid); plugins.hooks.fire('action:group.grantOwnership', { uid: toUid, groupName: groupName }); }; Groups.ownership.rescind = async function (toUid, groupName) { - // Note: No ownership checking is done here on purpose! - // If the owners set only contains one member, error out! + // If the owners set only contains one member (and toUid is that member), error out! const numOwners = await db.setCount('group:' + groupName + ':owners'); - if (numOwners <= 1) { + const isOwner = await db.isSortedSetMember(`group:${groupName}:owners`); + if (numOwners <= 1 && isOwner) { throw new Error('[[error:group-needs-owner]]'); } await db.setRemove('group:' + groupName + ':owners', toUid); diff --git a/src/routes/write/groups.js b/src/routes/write/groups.js index 8a5014c6f0..e5adf28a40 100644 --- a/src/routes/write/groups.js +++ b/src/routes/write/groups.js @@ -16,6 +16,8 @@ module.exports = function () { setupApiRoute(router, 'delete', '/:slug', [...middlewares, middleware.assert.group], controllers.write.groups.delete); setupApiRoute(router, 'put', '/:slug/membership/:uid', [...middlewares, middleware.assert.group], controllers.write.groups.join); setupApiRoute(router, 'delete', '/:slug/membership/:uid', [...middlewares, middleware.assert.group], controllers.write.groups.leave); + setupApiRoute(router, 'put', '/:slug/ownership/:uid', [...middlewares, middleware.assert.group], controllers.write.groups.grant); + setupApiRoute(router, 'delete', '/:slug/ownership/:uid', [...middlewares, middleware.assert.group], controllers.write.groups.rescind); return router; }; diff --git a/src/socket.io/groups.js b/src/socket.io/groups.js index 73cae30dc6..8ed59e2bf6 100644 --- a/src/socket.io/groups.js +++ b/src/socket.io/groups.js @@ -79,6 +79,8 @@ async function isInvited(socket, data) { } SocketGroups.grant = async (socket, data) => { + sockets.warnDeprecated(socket, 'PUT /api/v3/groups/:slug/ownership/:uid'); + await isOwner(socket, data); await groups.ownership.grant(data.toUid, data.groupName); logGroupEvent(socket, 'group-owner-grant', { @@ -88,6 +90,8 @@ SocketGroups.grant = async (socket, data) => { }; SocketGroups.rescind = async (socket, data) => { + sockets.warnDeprecated(socket, 'DELETE /api/v3/groups/:slug/ownership/:uid'); + await isOwner(socket, data); await groups.ownership.rescind(data.toUid, data.groupName); logGroupEvent(socket, 'group-owner-rescind', {