From eab7677909b96d401dcfc8e21d8d6601caee92c5 Mon Sep 17 00:00:00 2001 From: Boris Peterbarg Date: Mon, 1 May 2017 20:20:06 +0300 Subject: [PATCH] Make all channel/group calls answer to roomName, except rename In .rename, having both name and roomName parameters would be confusing. Also: Sync groups.close error message with channels.close Sync and fix error message in groups.open that was using a parameter that couldn't be there before this patch. Extract the bodyParams/queryParams logic. --- packages/rocketchat-api/package.js | 1 + packages/rocketchat-api/server/v1/channels.js | 64 +++++++++---------- packages/rocketchat-api/server/v1/groups.js | 62 +++++++++--------- .../server/v1/helpers/getUserFromParams.js | 12 +--- .../server/v1/helpers/isUserFromParams.js | 12 +--- .../server/v1/helpers/requestParams.js | 3 + tests/end-to-end/api/02-channels.js | 15 +++++ tests/end-to-end/api/03-groups.js | 15 +++++ 8 files changed, 99 insertions(+), 85 deletions(-) create mode 100644 packages/rocketchat-api/server/v1/helpers/requestParams.js diff --git a/packages/rocketchat-api/package.js b/packages/rocketchat-api/package.js index ef8aaf15932..16d34b94804 100644 --- a/packages/rocketchat-api/package.js +++ b/packages/rocketchat-api/package.js @@ -17,6 +17,7 @@ Package.onUse(function(api) { api.addFiles('server/settings.js', 'server'); //Register v1 helpers + api.addFiles('server/v1/helpers/requestParams.js', 'server'); api.addFiles('server/v1/helpers/getPaginationItems.js', 'server'); api.addFiles('server/v1/helpers/getUserFromParams.js', 'server'); api.addFiles('server/v1/helpers/isUserFromParams.js', 'server'); diff --git a/packages/rocketchat-api/server/v1/channels.js b/packages/rocketchat-api/server/v1/channels.js index 566c2b8d21a..3d2e33387f9 100644 --- a/packages/rocketchat-api/server/v1/channels.js +++ b/packages/rocketchat-api/server/v1/channels.js @@ -1,18 +1,18 @@ //Returns the channel IF found otherwise it will return the failure of why it didn't. Check the `statusCode` property -function findChannelByIdOrName({ roomId, roomName, checkedArchived = true }) { - if ((!roomId || !roomId.trim()) && (!roomName || !roomName.trim())) { +function findChannelByIdOrName({ params, checkedArchived = true }) { + if ((!params.roomId || !params.roomId.trim()) && (!params.roomName || !params.roomName.trim())) { throw new Meteor.Error('error-roomid-param-not-provided', 'The parameter "roomId" or "roomName" is required'); } let room; - if (roomId) { - room = RocketChat.models.Rooms.findOneById(roomId, { fields: RocketChat.API.v1.defaultFieldsToExclude }); - } else if (roomName) { - room = RocketChat.models.Rooms.findOneByName(roomName, { fields: RocketChat.API.v1.defaultFieldsToExclude }); + if (params.roomId) { + room = RocketChat.models.Rooms.findOneById(params.roomId, { fields: RocketChat.API.v1.defaultFieldsToExclude }); + } else if (params.roomName) { + room = RocketChat.models.Rooms.findOneByName(params.roomName, { fields: RocketChat.API.v1.defaultFieldsToExclude }); } if (!room || room.t !== 'c') { - throw new Meteor.Error('error-room-not-found', `No channel found by the id of: ${ roomId }`); + throw new Meteor.Error('error-room-not-found', 'The required "roomId" or "roomName" param provided does not match any channel'); } if (checkedArchived && room.archived) { @@ -24,7 +24,7 @@ function findChannelByIdOrName({ roomId, roomName, checkedArchived = true }) { RocketChat.API.v1.addRoute('channels.addAll', { authRequired: true }, { post() { - const findResult = findChannelByIdOrName({ roomId: this.bodyParams.roomId }); + const findResult = findChannelByIdOrName({ params: this.requestParams() }); Meteor.runAsUser(this.userId, () => { Meteor.call('addAllUserToRoom', findResult._id, this.bodyParams.activeUsersOnly); @@ -38,7 +38,7 @@ RocketChat.API.v1.addRoute('channels.addAll', { authRequired: true }, { RocketChat.API.v1.addRoute('channels.addModerator', { authRequired: true }, { post() { - const findResult = findChannelByIdOrName({ roomId: this.bodyParams.roomId }); + const findResult = findChannelByIdOrName({ params: this.requestParams() }); const user = this.getUserFromParams(); @@ -52,7 +52,7 @@ RocketChat.API.v1.addRoute('channels.addModerator', { authRequired: true }, { RocketChat.API.v1.addRoute('channels.addOwner', { authRequired: true }, { post() { - const findResult = findChannelByIdOrName({ roomId: this.bodyParams.roomId }); + const findResult = findChannelByIdOrName({ params: this.requestParams() }); const user = this.getUserFromParams(); @@ -66,7 +66,7 @@ RocketChat.API.v1.addRoute('channels.addOwner', { authRequired: true }, { RocketChat.API.v1.addRoute('channels.archive', { authRequired: true }, { post() { - const findResult = findChannelByIdOrName({ roomId: this.bodyParams.roomId }); + const findResult = findChannelByIdOrName({ params: this.requestParams() }); Meteor.runAsUser(this.userId, () => { Meteor.call('archiveRoom', findResult._id); @@ -78,7 +78,7 @@ RocketChat.API.v1.addRoute('channels.archive', { authRequired: true }, { RocketChat.API.v1.addRoute('channels.cleanHistory', { authRequired: true }, { post() { - const findResult = findChannelByIdOrName({ roomId: this.bodyParams.roomId }); + const findResult = findChannelByIdOrName({ params: this.requestParams() }); if (!this.bodyParams.latest) { return RocketChat.API.v1.failure('Body parameter "latest" is required.'); @@ -106,7 +106,7 @@ RocketChat.API.v1.addRoute('channels.cleanHistory', { authRequired: true }, { RocketChat.API.v1.addRoute('channels.close', { authRequired: true }, { post() { - const findResult = findChannelByIdOrName({ roomId: this.bodyParams.roomId, checkedArchived: false }); + const findResult = findChannelByIdOrName({ params: this.requestParams(), checkedArchived: false }); const sub = RocketChat.models.Subscriptions.findOneByRoomIdAndUserId(findResult._id, this.userId); @@ -162,7 +162,7 @@ RocketChat.API.v1.addRoute('channels.create', { authRequired: true }, { RocketChat.API.v1.addRoute('channels.delete', { authRequired: true }, { post() { - const findResult = findChannelByIdOrName({ roomId: this.bodyParams.roomId, roomName: this.bodyParams.roomName, checkedArchived: false }); + const findResult = findChannelByIdOrName({ params: this.requestParams(), checkedArchived: false }); //The find method returns either with the group or the failur @@ -182,7 +182,7 @@ RocketChat.API.v1.addRoute('channels.getIntegrations', { authRequired: true }, { return RocketChat.API.v1.unauthorized(); } - const findResult = findChannelByIdOrName({ roomId: this.queryParams.roomId, checkedArchived: false }); + const findResult = findChannelByIdOrName({ params: this.requestParams(), checkedArchived: false }); let includeAllPublicChannels = true; if (typeof this.queryParams.includeAllPublicChannels !== 'undefined') { @@ -222,7 +222,7 @@ RocketChat.API.v1.addRoute('channels.getIntegrations', { authRequired: true }, { RocketChat.API.v1.addRoute('channels.history', { authRequired: true }, { get() { - const findResult = findChannelByIdOrName({ roomId: this.queryParams.roomId, checkedArchived: false }); + const findResult = findChannelByIdOrName({ params: this.requestParams(), checkedArchived: false }); let latestDate = new Date(); if (this.queryParams.latest) { @@ -262,7 +262,7 @@ RocketChat.API.v1.addRoute('channels.history', { authRequired: true }, { RocketChat.API.v1.addRoute('channels.info', { authRequired: true }, { get() { - const findResult = findChannelByIdOrName({ roomId: this.queryParams.roomId, roomName: this.queryParams.roomName, checkedArchived: false }); + const findResult = findChannelByIdOrName({ params: this.requestParams(), checkedArchived: false }); return RocketChat.API.v1.success({ channel: RocketChat.models.Rooms.findOneById(findResult._id, { fields: RocketChat.API.v1.defaultFieldsToExclude }) @@ -272,7 +272,7 @@ RocketChat.API.v1.addRoute('channels.info', { authRequired: true }, { RocketChat.API.v1.addRoute('channels.invite', { authRequired: true }, { post() { - const findResult = findChannelByIdOrName({ roomId: this.bodyParams.roomId }); + const findResult = findChannelByIdOrName({ params: this.requestParams() }); const user = this.getUserFromParams(); @@ -288,7 +288,7 @@ RocketChat.API.v1.addRoute('channels.invite', { authRequired: true }, { RocketChat.API.v1.addRoute('channels.join', { authRequired: true }, { post() { - const findResult = findChannelByIdOrName({ roomId: this.bodyParams.roomId }); + const findResult = findChannelByIdOrName({ params: this.requestParams() }); Meteor.runAsUser(this.userId, () => { Meteor.call('joinRoom', findResult._id, this.bodyParams.joinCode); @@ -302,7 +302,7 @@ RocketChat.API.v1.addRoute('channels.join', { authRequired: true }, { RocketChat.API.v1.addRoute('channels.kick', { authRequired: true }, { post() { - const findResult = findChannelByIdOrName({ roomId: this.bodyParams.roomId }); + const findResult = findChannelByIdOrName({ params: this.requestParams() }); const user = this.getUserFromParams(); @@ -318,7 +318,7 @@ RocketChat.API.v1.addRoute('channels.kick', { authRequired: true }, { RocketChat.API.v1.addRoute('channels.leave', { authRequired: true }, { post() { - const findResult = findChannelByIdOrName({ roomId: this.bodyParams.roomId }); + const findResult = findChannelByIdOrName({ params: this.requestParams() }); Meteor.runAsUser(this.userId, () => { Meteor.call('leaveRoom', findResult._id); @@ -414,7 +414,7 @@ RocketChat.API.v1.addRoute('channels.online', { authRequired: true }, { RocketChat.API.v1.addRoute('channels.open', { authRequired: true }, { post() { - const findResult = findChannelByIdOrName({ roomId: this.bodyParams.roomId, checkedArchived: false }); + const findResult = findChannelByIdOrName({ params: this.requestParams(), checkedArchived: false }); const sub = RocketChat.models.Subscriptions.findOneByRoomIdAndUserId(findResult._id, this.userId); @@ -436,7 +436,7 @@ RocketChat.API.v1.addRoute('channels.open', { authRequired: true }, { RocketChat.API.v1.addRoute('channels.removeModerator', { authRequired: true }, { post() { - const findResult = findChannelByIdOrName({ roomId: this.bodyParams.roomId }); + const findResult = findChannelByIdOrName({ params: this.requestParams() }); const user = this.getUserFromParams(); @@ -450,7 +450,7 @@ RocketChat.API.v1.addRoute('channels.removeModerator', { authRequired: true }, { RocketChat.API.v1.addRoute('channels.removeOwner', { authRequired: true }, { post() { - const findResult = findChannelByIdOrName({ roomId: this.bodyParams.roomId }); + const findResult = findChannelByIdOrName({ params: this.requestParams() }); const user = this.getUserFromParams(); @@ -468,7 +468,7 @@ RocketChat.API.v1.addRoute('channels.rename', { authRequired: true }, { return RocketChat.API.v1.failure('The bodyParam "name" is required'); } - const findResult = findChannelByIdOrName({ roomId: this.bodyParams.roomId }); + const findResult = findChannelByIdOrName({ params: { roomId: this.bodyParams.roomId} }); if (findResult.name === this.bodyParams.name) { return RocketChat.API.v1.failure('The channel name is the same as what it would be renamed to.'); @@ -490,7 +490,7 @@ RocketChat.API.v1.addRoute('channels.setDescription', { authRequired: true }, { return RocketChat.API.v1.failure('The bodyParam "description" is required'); } - const findResult = findChannelByIdOrName({ roomId: this.bodyParams.roomId }); + const findResult = findChannelByIdOrName({ params: this.requestParams() }); if (findResult.description === this.bodyParams.description) { return RocketChat.API.v1.failure('The channel description is the same as what it would be changed to.'); @@ -512,7 +512,7 @@ RocketChat.API.v1.addRoute('channels.setJoinCode', { authRequired: true }, { return RocketChat.API.v1.failure('The bodyParam "joinCode" is required'); } - const findResult = findChannelByIdOrName({ roomId: this.bodyParams.roomId }); + const findResult = findChannelByIdOrName({ params: this.requestParams() }); Meteor.runAsUser(this.userId, () => { Meteor.call('saveRoomSettings', findResult._id, 'joinCode', this.bodyParams.joinCode); @@ -530,7 +530,7 @@ RocketChat.API.v1.addRoute('channels.setPurpose', { authRequired: true }, { return RocketChat.API.v1.failure('The bodyParam "purpose" is required'); } - const findResult = findChannelByIdOrName({ roomId: this.bodyParams.roomId }); + const findResult = findChannelByIdOrName({ params: this.requestParams() }); if (findResult.description === this.bodyParams.purpose) { return RocketChat.API.v1.failure('The channel purpose (description) is the same as what it would be changed to.'); @@ -552,7 +552,7 @@ RocketChat.API.v1.addRoute('channels.setReadOnly', { authRequired: true }, { return RocketChat.API.v1.failure('The bodyParam "readOnly" is required'); } - const findResult = findChannelByIdOrName({ roomId: this.bodyParams.roomId }); + const findResult = findChannelByIdOrName({ params: this.requestParams() }); if (findResult.ro === this.bodyParams.readOnly) { return RocketChat.API.v1.failure('The channel read only setting is the same as what it would be changed to.'); @@ -574,7 +574,7 @@ RocketChat.API.v1.addRoute('channels.setTopic', { authRequired: true }, { return RocketChat.API.v1.failure('The bodyParam "topic" is required'); } - const findResult = findChannelByIdOrName({ roomId: this.bodyParams.roomId }); + const findResult = findChannelByIdOrName({ params: this.requestParams() }); if (findResult.topic === this.bodyParams.topic) { return RocketChat.API.v1.failure('The channel topic is the same as what it would be changed to.'); @@ -596,7 +596,7 @@ RocketChat.API.v1.addRoute('channels.setType', { authRequired: true }, { return RocketChat.API.v1.failure('The bodyParam "type" is required'); } - const findResult = findChannelByIdOrName({ roomId: this.bodyParams.roomId }); + const findResult = findChannelByIdOrName({ params: this.requestParams() }); if (findResult.t === this.bodyParams.type) { return RocketChat.API.v1.failure('The channel type is the same as what it would be changed to.'); @@ -614,7 +614,7 @@ RocketChat.API.v1.addRoute('channels.setType', { authRequired: true }, { RocketChat.API.v1.addRoute('channels.unarchive', { authRequired: true }, { post() { - const findResult = findChannelByIdOrName({ roomId: this.bodyParams.roomId, checkedArchived: false }); + const findResult = findChannelByIdOrName({ params: this.requestParams(), checkedArchived: false }); if (!findResult.archived) { return RocketChat.API.v1.failure(`The channel, ${ findResult.name }, is not archived`); diff --git a/packages/rocketchat-api/server/v1/groups.js b/packages/rocketchat-api/server/v1/groups.js index 6559b886dfd..d5c11136d38 100644 --- a/packages/rocketchat-api/server/v1/groups.js +++ b/packages/rocketchat-api/server/v1/groups.js @@ -1,18 +1,18 @@ //Returns the private group subscription IF found otherwise it will return the failure of why it didn't. Check the `statusCode` property -function findPrivateGroupByIdOrName({ roomId, roomName, userId, checkedArchived = true }) { - if ((!roomId || !roomId.trim()) && (!roomName || !roomName.trim())) { +function findPrivateGroupByIdOrName({ params, userId, checkedArchived = true }) { + if ((!params.roomId || !params.roomId.trim()) && (!params.roomName || !params.roomName.trim())) { throw new Meteor.Error('error-roomid-param-not-provided', 'The parameter "roomId" or "roomName" is required'); } let roomSub; - if (roomId) { - roomSub = RocketChat.models.Subscriptions.findOneByRoomIdAndUserId(roomId, userId); - } else if (roomName) { - roomSub = RocketChat.models.Subscriptions.findOneByRoomNameAndUserId(roomName, userId); + if (params.roomId) { + roomSub = RocketChat.models.Subscriptions.findOneByRoomIdAndUserId(params.roomId, userId); + } else if (params.roomName) { + roomSub = RocketChat.models.Subscriptions.findOneByRoomNameAndUserId(params.roomName, userId); } if (!roomSub || roomSub.t !== 'p') { - throw new Meteor.Error('error-room-not-found', `No private group by the id of: ${ roomId }`); + throw new Meteor.Error('error-room-not-found', 'The required "roomId" or "roomName" param provided does not match any group'); } if (checkedArchived && roomSub.archived) { @@ -24,7 +24,7 @@ function findPrivateGroupByIdOrName({ roomId, roomName, userId, checkedArchived RocketChat.API.v1.addRoute('groups.addAll', { authRequired: true }, { post() { - const findResult = findPrivateGroupByIdOrName({ roomId: this.bodyParams.roomId, userId: this.userId }); + const findResult = findPrivateGroupByIdOrName({ params: this.requestParams(), userId: this.userId }); Meteor.runAsUser(this.userId, () => { Meteor.call('addAllUserToRoom', findResult.rid, this.bodyParams.activeUsersOnly); @@ -38,7 +38,7 @@ RocketChat.API.v1.addRoute('groups.addAll', { authRequired: true }, { RocketChat.API.v1.addRoute('groups.addModerator', { authRequired: true }, { post() { - const findResult = findPrivateGroupByIdOrName({ roomId: this.bodyParams.roomId, userId: this.userId }); + const findResult = findPrivateGroupByIdOrName({ params: this.requestParams(), userId: this.userId }); const user = this.getUserFromParams(); @@ -52,7 +52,7 @@ RocketChat.API.v1.addRoute('groups.addModerator', { authRequired: true }, { RocketChat.API.v1.addRoute('groups.addOwner', { authRequired: true }, { post() { - const findResult = findPrivateGroupByIdOrName({ roomId: this.bodyParams.roomId, userId: this.userId }); + const findResult = findPrivateGroupByIdOrName({ params: this.requestParams(), userId: this.userId }); const user = this.getUserFromParams(); @@ -67,7 +67,7 @@ RocketChat.API.v1.addRoute('groups.addOwner', { authRequired: true }, { //Archives a private group only if it wasn't RocketChat.API.v1.addRoute('groups.archive', { authRequired: true }, { post() { - const findResult = findPrivateGroupByIdOrName({ roomId: this.bodyParams.roomId, userId: this.userId }); + const findResult = findPrivateGroupByIdOrName({ params: this.requestParams(), userId: this.userId }); Meteor.runAsUser(this.userId, () => { Meteor.call('archiveRoom', findResult.rid); @@ -79,10 +79,10 @@ RocketChat.API.v1.addRoute('groups.archive', { authRequired: true }, { RocketChat.API.v1.addRoute('groups.close', { authRequired: true }, { post() { - const findResult = findPrivateGroupByIdOrName({ roomId: this.bodyParams.roomId, userId: this.userId, checkedArchived: false }); + const findResult = findPrivateGroupByIdOrName({ params: this.requestParams(), userId: this.userId, checkedArchived: false }); if (!findResult.open) { - return RocketChat.API.v1.failure(`The private group with an id "${ this.bodyParams.roomId }" is already closed to the sender`); + return RocketChat.API.v1.failure(`The private group, ${ findResult.name }, is already closed to the sender`); } Meteor.runAsUser(this.userId, () => { @@ -130,7 +130,7 @@ RocketChat.API.v1.addRoute('groups.create', { authRequired: true }, { RocketChat.API.v1.addRoute('groups.delete', { authRequired: true }, { post() { - const findResult = findPrivateGroupByIdOrName({ roomId: this.bodyParams.roomId, roomName: this.bodyParams.roomName, userId: this.userId, checkedArchived: false }); + const findResult = findPrivateGroupByIdOrName({ params: this.requestParams(), userId: this.userId, checkedArchived: false }); Meteor.runAsUser(this.userId, () => { Meteor.call('eraseRoom', findResult.rid); @@ -148,7 +148,7 @@ RocketChat.API.v1.addRoute('groups.getIntegrations', { authRequired: true }, { return RocketChat.API.v1.unauthorized(); } - const findResult = findPrivateGroupByIdOrName({ roomId: this.queryParams.roomId, userId: this.userId, checkedArchived: false }); + const findResult = findPrivateGroupByIdOrName({ params: this.requestParams(), userId: this.userId, checkedArchived: false }); let includeAllPrivateGroups = true; if (typeof this.queryParams.includeAllPrivateGroups !== 'undefined') { @@ -182,7 +182,7 @@ RocketChat.API.v1.addRoute('groups.getIntegrations', { authRequired: true }, { RocketChat.API.v1.addRoute('groups.history', { authRequired: true }, { get() { - const findResult = findPrivateGroupByIdOrName({ roomId: this.queryParams.roomId, userId: this.userId, checkedArchived: false }); + const findResult = findPrivateGroupByIdOrName({ params: this.requestParams(), userId: this.userId, checkedArchived: false }); let latestDate = new Date(); if (this.queryParams.latest) { @@ -222,7 +222,7 @@ RocketChat.API.v1.addRoute('groups.history', { authRequired: true }, { RocketChat.API.v1.addRoute('groups.info', { authRequired: true }, { get() { - const findResult = findPrivateGroupByIdOrName({ roomId: this.queryParams.roomId, roomName: this.queryParams.roomName, userId: this.userId, checkedArchived: false }); + const findResult = findPrivateGroupByIdOrName({ params: this.requestParams(), userId: this.userId, checkedArchived: false }); return RocketChat.API.v1.success({ group: RocketChat.models.Rooms.findOneById(findResult.rid, { fields: RocketChat.API.v1.defaultFieldsToExclude }) @@ -232,7 +232,7 @@ RocketChat.API.v1.addRoute('groups.info', { authRequired: true }, { RocketChat.API.v1.addRoute('groups.invite', { authRequired: true }, { post() { - const findResult = findPrivateGroupByIdOrName({ roomId: this.bodyParams.roomId, userId: this.userId }); + const findResult = findPrivateGroupByIdOrName({ params: this.requestParams(), userId: this.userId }); const user = this.getUserFromParams(); @@ -248,7 +248,7 @@ RocketChat.API.v1.addRoute('groups.invite', { authRequired: true }, { RocketChat.API.v1.addRoute('groups.kick', { authRequired: true }, { post() { - const findResult = findPrivateGroupByIdOrName({ roomId: this.bodyParams.roomId, userId: this.userId }); + const findResult = findPrivateGroupByIdOrName({ params: this.requestParams(), userId: this.userId }); const user = this.getUserFromParams(); @@ -262,7 +262,7 @@ RocketChat.API.v1.addRoute('groups.kick', { authRequired: true }, { RocketChat.API.v1.addRoute('groups.leave', { authRequired: true }, { post() { - const findResult = findPrivateGroupByIdOrName({ roomId: this.bodyParams.roomId, userId: this.userId }); + const findResult = findPrivateGroupByIdOrName({ params: this.requestParams(), userId: this.userId }); Meteor.runAsUser(this.userId, () => { Meteor.call('leaveRoom', findResult.rid); @@ -331,10 +331,10 @@ RocketChat.API.v1.addRoute('groups.online', { authRequired: true }, { RocketChat.API.v1.addRoute('groups.open', { authRequired: true }, { post() { - const findResult = findPrivateGroupByIdOrName({ roomId: this.bodyParams.roomId, userId: this.userId, checkedArchived: false }); + const findResult = findPrivateGroupByIdOrName({ params: this.requestParams(), userId: this.userId, checkedArchived: false }); if (findResult.open) { - return RocketChat.API.v1.failure(`The private group, ${ this.bodyParams.name }, is already open for the sender`); + return RocketChat.API.v1.failure(`The private group, ${ findResult.name }, is already open for the sender`); } Meteor.runAsUser(this.userId, () => { @@ -347,7 +347,7 @@ RocketChat.API.v1.addRoute('groups.open', { authRequired: true }, { RocketChat.API.v1.addRoute('groups.removeModerator', { authRequired: true }, { post() { - const findResult = findPrivateGroupByIdOrName({ roomId: this.bodyParams.roomId, userId: this.userId }); + const findResult = findPrivateGroupByIdOrName({ params: this.requestParams(), userId: this.userId }); const user = this.getUserFromParams(); @@ -361,7 +361,7 @@ RocketChat.API.v1.addRoute('groups.removeModerator', { authRequired: true }, { RocketChat.API.v1.addRoute('groups.removeOwner', { authRequired: true }, { post() { - const findResult = findPrivateGroupByIdOrName({ roomId: this.bodyParams.roomId, userId: this.userId }); + const findResult = findPrivateGroupByIdOrName({ params: this.requestParams(), userId: this.userId }); const user = this.getUserFromParams(); @@ -379,7 +379,7 @@ RocketChat.API.v1.addRoute('groups.rename', { authRequired: true }, { return RocketChat.API.v1.failure('The bodyParam "name" is required'); } - const findResult = findPrivateGroupByIdOrName({ roomId: this.bodyParams.roomId, userId: this.userId }); + const findResult = findPrivateGroupByIdOrName({ params: { roomId: this.bodyParams.roomId}, userId: this.userId }); Meteor.runAsUser(this.userId, () => { Meteor.call('saveRoomSettings', findResult.rid, 'roomName', this.bodyParams.name); @@ -397,7 +397,7 @@ RocketChat.API.v1.addRoute('groups.setDescription', { authRequired: true }, { return RocketChat.API.v1.failure('The bodyParam "description" is required'); } - const findResult = findPrivateGroupByIdOrName({ roomId: this.bodyParams.roomId, userId: this.userId }); + const findResult = findPrivateGroupByIdOrName({ params: this.requestParams(), userId: this.userId }); Meteor.runAsUser(this.userId, () => { Meteor.call('saveRoomSettings', findResult.rid, 'roomDescription', this.bodyParams.description); @@ -415,7 +415,7 @@ RocketChat.API.v1.addRoute('groups.setPurpose', { authRequired: true }, { return RocketChat.API.v1.failure('The bodyParam "purpose" is required'); } - const findResult = findPrivateGroupByIdOrName({ roomId: this.bodyParams.roomId, userId: this.userId }); + const findResult = findPrivateGroupByIdOrName({ params: this.requestParams(), userId: this.userId }); Meteor.runAsUser(this.userId, () => { Meteor.call('saveRoomSettings', findResult.rid, 'roomDescription', this.bodyParams.purpose); @@ -433,7 +433,7 @@ RocketChat.API.v1.addRoute('groups.setReadOnly', { authRequired: true }, { return RocketChat.API.v1.failure('The bodyParam "readOnly" is required'); } - const findResult = findPrivateGroupByIdOrName({ roomId: this.bodyParams.roomId, userId: this.userId }); + const findResult = findPrivateGroupByIdOrName({ params: this.requestParams(), userId: this.userId }); if (findResult.ro === this.bodyParams.readOnly) { return RocketChat.API.v1.failure('The private group read only setting is the same as what it would be changed to.'); @@ -455,7 +455,7 @@ RocketChat.API.v1.addRoute('groups.setTopic', { authRequired: true }, { return RocketChat.API.v1.failure('The bodyParam "topic" is required'); } - const findResult = findPrivateGroupByIdOrName({ roomId: this.bodyParams.roomId, userId: this.userId }); + const findResult = findPrivateGroupByIdOrName({ params: this.requestParams(), userId: this.userId }); Meteor.runAsUser(this.userId, () => { Meteor.call('saveRoomSettings', findResult.rid, 'roomTopic', this.bodyParams.topic); @@ -473,7 +473,7 @@ RocketChat.API.v1.addRoute('groups.setType', { authRequired: true }, { return RocketChat.API.v1.failure('The bodyParam "type" is required'); } - const findResult = findPrivateGroupByIdOrName({ roomId: this.bodyParams.roomId, userId: this.userId }); + const findResult = findPrivateGroupByIdOrName({ params: this.requestParams(), userId: this.userId }); if (findResult.t === this.bodyParams.type) { return RocketChat.API.v1.failure('The private group type is the same as what it would be changed to.'); @@ -491,7 +491,7 @@ RocketChat.API.v1.addRoute('groups.setType', { authRequired: true }, { RocketChat.API.v1.addRoute('groups.unarchive', { authRequired: true }, { post() { - const findResult = findPrivateGroupByIdOrName({ roomId: this.bodyParams.roomId, userId: this.userId, checkedArchived: false }); + const findResult = findPrivateGroupByIdOrName({ params: this.requestParams(), userId: this.userId, checkedArchived: false }); Meteor.runAsUser(this.userId, () => { Meteor.call('unarchiveRoom', findResult.rid); diff --git a/packages/rocketchat-api/server/v1/helpers/getUserFromParams.js b/packages/rocketchat-api/server/v1/helpers/getUserFromParams.js index f3e4c81950b..c52296f0fb7 100644 --- a/packages/rocketchat-api/server/v1/helpers/getUserFromParams.js +++ b/packages/rocketchat-api/server/v1/helpers/getUserFromParams.js @@ -2,17 +2,7 @@ RocketChat.API.v1.helperMethods.set('getUserFromParams', function _getUserFromParams() { const doesntExist = { _doesntExist: true }; let user; - let params; - - switch (this.request.method) { - case 'POST': - case 'PUT': - params = this.bodyParams; - break; - default: - params = this.queryParams; - break; - } + const params = this.requestParams(); if (params.userId && params.userId.trim()) { user = RocketChat.models.Users.findOneById(params.userId) || doesntExist; diff --git a/packages/rocketchat-api/server/v1/helpers/isUserFromParams.js b/packages/rocketchat-api/server/v1/helpers/isUserFromParams.js index 28c8bf4ee5e..fab907bc96d 100644 --- a/packages/rocketchat-api/server/v1/helpers/isUserFromParams.js +++ b/packages/rocketchat-api/server/v1/helpers/isUserFromParams.js @@ -1,15 +1,5 @@ RocketChat.API.v1.helperMethods.set('isUserFromParams', function _isUserFromParams() { - let params; - - switch (this.request.method) { - case 'POST': - case 'PUT': - params = this.bodyParams; - break; - default: - params = this.queryParams; - break; - } + const params = this.requestParams(); return (!params.userId && !params.username && !params.user) || (params.userId && this.userId === params.userId) || diff --git a/packages/rocketchat-api/server/v1/helpers/requestParams.js b/packages/rocketchat-api/server/v1/helpers/requestParams.js new file mode 100644 index 00000000000..bc571831391 --- /dev/null +++ b/packages/rocketchat-api/server/v1/helpers/requestParams.js @@ -0,0 +1,3 @@ +RocketChat.API.v1.helperMethods.set('requestParams', function _requestParams() { + return ['POST', 'PUT'].includes(this.request.method) ? this.bodyParams : this.queryParams; +}); diff --git a/tests/end-to-end/api/02-channels.js b/tests/end-to-end/api/02-channels.js index 7ba3f79d1f1..0d3d0643328 100644 --- a/tests/end-to-end/api/02-channels.js +++ b/tests/end-to-end/api/02-channels.js @@ -320,6 +320,21 @@ describe('channels', function() { .end(done); }); + it('/channels.close', (done) => { + request.post(api('channels.close')) + .set(credentials) + .send({ + roomName: apiPublicChannelName + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error', `The channel, ${ apiPublicChannelName }, is already closed to the sender`); + }) + .end(done); + }); + it('/channels.open', (done) => { request.post(api('channels.open')) .set(credentials) diff --git a/tests/end-to-end/api/03-groups.js b/tests/end-to-end/api/03-groups.js index 94aff71aa03..3323c992585 100644 --- a/tests/end-to-end/api/03-groups.js +++ b/tests/end-to-end/api/03-groups.js @@ -298,6 +298,21 @@ describe('groups', function() { .end(done); }); + it('/groups.close', (done) => { + request.post(api('groups.close')) + .set(credentials) + .send({ + roomName: apiPrivateChannelName + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error', `The private group, ${ apiPrivateChannelName }, is already closed to the sender`); + }) + .end(done); + }); + it('/groups.open', (done) => { request.post(api('groups.open')) .set(credentials)