From 9054f0dba996d3c2a653756542f374153443a670 Mon Sep 17 00:00:00 2001 From: Shiqi Mei Date: Tue, 28 Jan 2020 05:29:10 +0800 Subject: [PATCH] [NEW] Create a user for the Apps during installation (#15896) * Create a new role `app` based on the role bot * Reimplement AppUserBridge.create method * checkUsernameAvailability before creating app user * Notify admin when meeting Apps_User_Already_Exists error * Implementing createAppUser in Rocket.Chat side * Prevent "App users" from logging-in on Rocket.Chat * Add new user type 'app' * Change active user count query * Add createdAt automatically to users created via app Co-authored-by: Upendra Reddy Co-authored-by: Douglas Gubert --- app/apps/client/admin/appInstall.js | 11 +++- app/apps/server/bridges/users.js | 64 +++++++++++++++++++++- app/apps/server/communication/rest.js | 25 ++++++--- app/apps/server/converters/users.js | 26 +++++++++ app/authorization/server/startup.js | 23 ++++---- app/lib/server/functions/deleteUser.js | 4 ++ app/models/server/models/Users.js | 6 +- app/models/server/raw/BaseRaw.js | 4 ++ app/ui-login/client/login/form.js | 2 + app/ui/client/views/app/directory.js | 2 +- package-lock.json | 6 +- package.json | 2 +- packages/rocketchat-i18n/i18n/en.i18n.json | 2 + packages/rocketchat-i18n/i18n/zh.i18n.json | 4 +- server/lib/accounts.js | 6 ++ 15 files changed, 158 insertions(+), 29 deletions(-) diff --git a/app/apps/client/admin/appInstall.js b/app/apps/client/admin/appInstall.js index 969486a38220..1db80c13b11e 100644 --- a/app/apps/client/admin/appInstall.js +++ b/app/apps/client/admin/appInstall.js @@ -11,6 +11,7 @@ import { ReactiveVar } from 'meteor/reactive-var'; import { FlowRouter } from 'meteor/kadira:flow-router'; import { Template } from 'meteor/templating'; import { Tracker } from 'meteor/tracker'; +import { TAPi18n } from 'meteor/rocketchat:tap-i18n'; import toastr from 'toastr'; import { APIClient } from '../../../utils'; @@ -19,7 +20,7 @@ import { SideNav } from '../../../ui-utils/client'; function handleInstallError(apiError) { if (!apiError.xhr || !apiError.xhr.responseJSON) { return; } - const { status, messages, error } = apiError.xhr.responseJSON; + const { status, messages, error, payload = null } = apiError.xhr.responseJSON; let message; @@ -27,11 +28,15 @@ function handleInstallError(apiError) { case 'storage_error': message = messages.join(''); break; - case 'compiler_error': message = 'There has been compiler errors. App cannot be installed'; break; - + case 'app_user_error': + message = messages.join(''); + if (payload && payload.username) { + message = TAPi18n.__('Apps_User_Already_Exists', { username: payload.username }); + } + break; default: if (error) { message = error; diff --git a/app/apps/server/bridges/users.js b/app/apps/server/bridges/users.js index 7329777f7026..a35a9502eb63 100644 --- a/app/apps/server/bridges/users.js +++ b/app/apps/server/bridges/users.js @@ -1,4 +1,8 @@ -import { Users } from '../../../models/server'; +import { Random } from 'meteor/random'; + +import { setUserAvatar, checkUsernameAvailability, deleteUser } from '../../../lib/server/functions'; +import { Users } from '../../../models'; + export class AppUserBridge { constructor(orch) { @@ -17,6 +21,64 @@ export class AppUserBridge { return this.orch.getConverters().get('users').convertByUsername(username); } + async getAppUser(appId) { + this.orch.debugLog(`The App ${ appId } is getting its assigned user`); + + const user = Users.findOne({ appId }); + + return this.orch.getConverters().get('users').convertToApp(user); + } + + async create(userDescriptor, appId, { avatarUrl }) { + this.orch.debugLog(`The App ${ appId } is requesting to create a new user.`); + const user = this.orch.getConverters().get('users').convertToRocketChat(userDescriptor); + + if (!user._id) { + user._id = Random.id(); + } + + if (!user.createdAt) { + user.createdAt = new Date(); + } + + switch (user.type) { + case 'app': + if (!checkUsernameAvailability(user.username)) { + throw new Error(`The username "${ user.username }" is already being used. Rename or remove the user using it to install this App`); + } + + Users.insert(user); + + if (avatarUrl) { + setUserAvatar(user, avatarUrl, '', 'local'); + } + + break; + + default: + throw new Error('Creating normal users is currently not supported'); + } + + return user._id; + } + + async remove(user, appId) { + this.orch.debugLog(`The App's user is being removed: ${ appId }`); + + // It's actually not a problem if there is no App user to delete - just means we don't need to do anything more. + if (!user) { + return true; + } + + try { + deleteUser(user.id); + } catch (err) { + throw new Error(`Errors occurred while deleting an app user: ${ err }`); + } + + return true; + } + async getActiveUserCount() { return Users.getActiveLocalUserCount(); } diff --git a/app/apps/server/communication/rest.js b/app/apps/server/communication/rest.js index 2787a965135a..4d3571aae453 100644 --- a/app/apps/server/communication/rest.js +++ b/app/apps/server/communication/rest.js @@ -239,6 +239,14 @@ export class AppsRestApi { return API.v1.failure({ status: 'compiler_error', messages: aff.getCompilerErrors() }); } + if (aff.hasAppUserError()) { + return API.v1.failure({ + status: 'app_user_error', + messages: [aff.getAppUserError().message], + payload: { username: aff.getAppUserError().username }, + }); + } + info.status = aff.getApp().getStatus(); return API.v1.success({ @@ -449,15 +457,18 @@ export class AppsRestApi { delete() { const prl = manager.getOneById(this.urlParams.id); - if (prl) { - Promise.await(manager.remove(prl.getID())); + if (!prl) { + return API.v1.notFound(`No App found by the id of: ${ this.urlParams.id }`); + } - const info = prl.getInfo(); - info.status = prl.getStatus(); + manager.remove(prl.getID()) + .then(() => { + const info = prl.getInfo(); + info.status = prl.getStatus(); - return API.v1.success({ app: info }); - } - return API.v1.notFound(`No App found by the id of: ${ this.urlParams.id }`); + API.v1.success({ app: info }); + }) + .catch((err) => API.v1.failure(err)); }, }); diff --git a/app/apps/server/converters/users.js b/app/apps/server/converters/users.js index bccbfa0fce53..79d4016e7c20 100644 --- a/app/apps/server/converters/users.js +++ b/app/apps/server/converters/users.js @@ -41,6 +41,30 @@ export class AppUsersConverter { createdAt: user.createdAt, updatedAt: user._updatedAt, lastLoginAt: user.lastLogin, + appId: user.appId, + }; + } + + convertToRocketChat(user) { + if (!user) { + return undefined; + } + + return { + _id: user.id, + username: user.username, + emails: user.emails, + type: user.type, + active: user.isEnabled, + name: user.name, + roles: user.roles, + status: user.status, + statusConnection: user.statusConnection, + utcOffset: user.utfOffset, + createdAt: user.createdAt, + _updatedAt: user.updatedAt, + lastLogin: user.lastLoginAt, + appId: user.appId, }; } @@ -50,6 +74,8 @@ export class AppUsersConverter { return UserType.USER; case 'bot': return UserType.BOT; + case 'app': + return UserType.APP; case '': case undefined: return UserType.UNKNOWN; diff --git a/app/authorization/server/startup.js b/app/authorization/server/startup.js index 3314bd586900..ad5e7b7b08e2 100644 --- a/app/authorization/server/startup.js +++ b/app/authorization/server/startup.js @@ -17,15 +17,15 @@ Meteor.startup(function() { { _id: 'add-user-to-joined-room', roles: ['admin', 'owner', 'moderator'] }, { _id: 'add-user-to-any-c-room', roles: ['admin'] }, { _id: 'add-user-to-any-p-room', roles: [] }, - { _id: 'api-bypass-rate-limit', roles: ['admin', 'bot'] }, + { _id: 'api-bypass-rate-limit', roles: ['admin', 'bot', 'app'] }, { _id: 'archive-room', roles: ['admin', 'owner'] }, { _id: 'assign-admin-role', roles: ['admin'] }, { _id: 'assign-roles', roles: ['admin'] }, { _id: 'ban-user', roles: ['admin', 'owner', 'moderator'] }, { _id: 'bulk-register-user', roles: ['admin'] }, - { _id: 'create-c', roles: ['admin', 'user', 'bot'] }, - { _id: 'create-d', roles: ['admin', 'user', 'bot'] }, - { _id: 'create-p', roles: ['admin', 'user', 'bot'] }, + { _id: 'create-c', roles: ['admin', 'user', 'bot', 'app'] }, + { _id: 'create-d', roles: ['admin', 'user', 'bot', 'app'] }, + { _id: 'create-p', roles: ['admin', 'user', 'bot', 'app'] }, { _id: 'create-personal-access-tokens', roles: ['admin', 'user'] }, { _id: 'create-user', roles: ['admin'] }, { _id: 'clean-channel-history', roles: ['admin'] }, @@ -44,9 +44,9 @@ Meteor.startup(function() { { _id: 'edit-room', roles: ['admin', 'owner', 'moderator'] }, { _id: 'edit-room-retention-policy', roles: ['admin'] }, { _id: 'force-delete-message', roles: ['admin', 'owner'] }, - { _id: 'join-without-join-code', roles: ['admin', 'bot'] }, - { _id: 'leave-c', roles: ['admin', 'user', 'bot', 'anonymous'] }, - { _id: 'leave-p', roles: ['admin', 'user', 'bot', 'anonymous'] }, + { _id: 'join-without-join-code', roles: ['admin', 'bot', 'app'] }, + { _id: 'leave-c', roles: ['admin', 'user', 'bot', 'anonymous', 'app'] }, + { _id: 'leave-p', roles: ['admin', 'user', 'bot', 'anonymous', 'app'] }, { _id: 'manage-assets', roles: ['admin'] }, { _id: 'manage-emoji', roles: ['admin'] }, { _id: 'manage-user-status', roles: ['admin'] }, @@ -64,15 +64,15 @@ Meteor.startup(function() { { _id: 'run-migration', roles: ['admin'] }, { _id: 'set-moderator', roles: ['admin', 'owner'] }, { _id: 'set-owner', roles: ['admin', 'owner'] }, - { _id: 'send-many-messages', roles: ['admin', 'bot'] }, + { _id: 'send-many-messages', roles: ['admin', 'bot', 'app'] }, { _id: 'set-leader', roles: ['admin', 'owner'] }, { _id: 'unarchive-room', roles: ['admin'] }, - { _id: 'view-c-room', roles: ['admin', 'user', 'bot', 'anonymous'] }, + { _id: 'view-c-room', roles: ['admin', 'user', 'bot', 'app', 'anonymous'] }, { _id: 'user-generate-access-token', roles: ['admin'] }, - { _id: 'view-d-room', roles: ['admin', 'user', 'bot'] }, + { _id: 'view-d-room', roles: ['admin', 'user', 'bot', 'app'] }, { _id: 'view-full-other-user-info', roles: ['admin'] }, { _id: 'view-history', roles: ['admin', 'user', 'anonymous'] }, - { _id: 'view-joined-room', roles: ['guest', 'bot', 'anonymous'] }, + { _id: 'view-joined-room', roles: ['guest', 'bot', 'app', 'anonymous'] }, { _id: 'view-join-code', roles: ['admin'] }, { _id: 'view-logs', roles: ['admin'] }, { _id: 'view-other-user-channels', roles: ['admin'] }, @@ -125,6 +125,7 @@ Meteor.startup(function() { { name: 'owner', scope: 'Subscriptions', description: 'Owner' }, { name: 'user', scope: 'Users', description: '' }, { name: 'bot', scope: 'Users', description: '' }, + { name: 'app', scope: 'Users', description: '' }, { name: 'guest', scope: 'Users', description: '' }, { name: 'anonymous', scope: 'Users', description: '' }, { name: 'livechat-agent', scope: 'Users', description: 'Livechat Agent' }, diff --git a/app/lib/server/functions/deleteUser.js b/app/lib/server/functions/deleteUser.js index 031ce3c612eb..4962c5522f06 100644 --- a/app/lib/server/functions/deleteUser.js +++ b/app/lib/server/functions/deleteUser.js @@ -12,6 +12,10 @@ export const deleteUser = function(userId) { fields: { username: 1, avatarOrigin: 1, federation: 1 }, }); + if (!user) { + return; + } + if (user.federation) { const existingSubscriptions = Subscriptions.find({ 'u._id': user._id }).count(); diff --git a/app/models/server/models/Users.js b/app/models/server/models/Users.js index 2cf52c4e9238..d4dd87f594f4 100644 --- a/app/models/server/models/Users.js +++ b/app/models/server/models/Users.js @@ -541,7 +541,11 @@ export class Users extends Base { } findActive(options = {}) { - return this.find({ active: true }, options); + return this.find({ + active: true, + type: { $nin: ['app'] }, + emails: { $exists: true }, + }, options); } findActiveByUsernameOrNameRegexWithExceptionsAndConditions(searchTerm, exceptions, conditions, options) { diff --git a/app/models/server/raw/BaseRaw.js b/app/models/server/raw/BaseRaw.js index 614c196ee7ba..1a65a2c9cebc 100644 --- a/app/models/server/raw/BaseRaw.js +++ b/app/models/server/raw/BaseRaw.js @@ -19,6 +19,10 @@ export class BaseRaw { return this.col.find(...args); } + insert(...args) { + return this.col.insert(...args); + } + update(...args) { return this.col.update(...args); } diff --git a/app/ui-login/client/login/form.js b/app/ui-login/client/login/form.js index cabba10d0a3b..a0c5bfa3ef41 100644 --- a/app/ui-login/client/login/form.js +++ b/app/ui-login/client/login/form.js @@ -141,6 +141,8 @@ Template.loginForm.events({ instance.state.set('email-verification'); } else if (error.error === 'error-user-is-not-activated') { toastr.error(t('Wait_activation_warning')); + } else if (error.error === 'error-app-user-is-not-allowed-to-login') { + toastr.error(t('App_user_not_allowed_to_login')); } else { toastr.error(t('User_not_found_or_incorrect_password')); } diff --git a/app/ui/client/views/app/directory.js b/app/ui/client/views/app/directory.js index 73299732aa5c..c573495fff5b 100644 --- a/app/ui/client/views/app/directory.js +++ b/app/ui/client/views/app/directory.js @@ -30,7 +30,7 @@ function directorySearch(config, cb) { return { name: result.name, username: result.username, - // If there is no email address (probably only rocket.cat) show the username) + // If there is no email address (rocket.cat and app users) show the username) email: (result.emails && result.emails[0] && result.emails[0].address) || result.username, createdAt: timeAgo(result.createdAt, t), origin: result.federation && result.federation.origin, diff --git a/package-lock.json b/package-lock.json index 0ef3211ee79b..95b332ed73a7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2623,9 +2623,9 @@ } }, "@rocket.chat/apps-engine": { - "version": "1.11.0", - "resolved": "https://registry.npmjs.org/@rocket.chat/apps-engine/-/apps-engine-1.11.0.tgz", - "integrity": "sha512-tptq+Cw2VZudUzerfe6iUqRRnu1b1IIQAtAM64kT9oDXY5xydhhPR00YzF1KgFAKhhT1zQhtocfLiajGFacVEQ==", + "version": "1.12.0-beta.2452", + "resolved": "https://registry.npmjs.org/@rocket.chat/apps-engine/-/apps-engine-1.12.0-beta.2452.tgz", + "integrity": "sha512-kuM7ovliMlwu3GiVHeVDmcI9jQ69kRV+/ldSZPbZCTV3vlp93dwWmznB/8Z68JnuhBPNWY2dbs6wxnsrvbPvTg==", "requires": { "adm-zip": "^0.4.9", "cryptiles": "^4.1.3", diff --git a/package.json b/package.json index ddd839ca5c5b..49c16b6b82bc 100644 --- a/package.json +++ b/package.json @@ -120,7 +120,7 @@ "@google-cloud/language": "^2.0.0", "@google-cloud/storage": "^2.3.1", "@google-cloud/vision": "^0.23.0", - "@rocket.chat/apps-engine": "^1.11.0", + "@rocket.chat/apps-engine": "^1.12.0-beta.2452", "@rocket.chat/fuselage": "^0.2.0-alpha.19", "@rocket.chat/fuselage-hooks": "^0.2.0-dev.50", "@rocket.chat/icons": "^0.2.0-dev.49", diff --git a/packages/rocketchat-i18n/i18n/en.i18n.json b/packages/rocketchat-i18n/i18n/en.i18n.json index 069b060106f3..bed62609d62d 100644 --- a/packages/rocketchat-i18n/i18n/en.i18n.json +++ b/packages/rocketchat-i18n/i18n/en.i18n.json @@ -362,6 +362,7 @@ "App_support_url": "support url", "App_Url_to_Install_From": "Install from URL", "App_Url_to_Install_From_File": "Install from file", + "App_user_not_allowed_to_login": "App users are not allowed to log in directly.", "Appearance": "Appearance", "Application_added": "Application added", "Application_Name": "Application Name", @@ -385,6 +386,7 @@ "Apps_Marketplace_pricingPlan_yearly": "__price__ / year", "Apps_Marketplace_pricingPlan_yearly_perUser": "__price__ / year per user", "Apps_Settings": "App's Settings", + "Apps_User_Already_Exists": "The username \"__username__\" is already being used. Rename or remove the user using it to install this App", "Apps_WhatIsIt": "Apps: What Are They?", "Apps_WhatIsIt_paragraph1": "A new icon in the administration area! What does this mean and what are Apps?", "Apps_WhatIsIt_paragraph2": "First off, Apps in this context do not refer to the mobile applications. In fact, it would be best to think of them in terms of plugins or advanced integrations.", diff --git a/packages/rocketchat-i18n/i18n/zh.i18n.json b/packages/rocketchat-i18n/i18n/zh.i18n.json index b66d8405a146..2b97fea4ffab 100644 --- a/packages/rocketchat-i18n/i18n/zh.i18n.json +++ b/packages/rocketchat-i18n/i18n/zh.i18n.json @@ -354,6 +354,7 @@ "App_support_url": "支持网址", "App_Url_to_Install_From": "从 URL 安装", "App_Url_to_Install_From_File": "从文件安装", + "App_user_not_allowed_to_login": "App类型用户不允许直接登录。", "Appearance": "外观", "Application_added": "应用已添加", "Application_Name": "应用名称", @@ -373,6 +374,7 @@ "Apps_Marketplace_Login_Required_Title": "需要登录市场", "Apps_Marketplace_Login_Required_Description": "从 Rocket.Chat 市场购买应用需要在注册您的工作区并登陆。", "Apps_Settings": "应用的设置", + "Apps_User_Already_Exists": "与待安装App同名的用户 __username__ 已存在，请在安装前重命名或删除该用户。", "Apps_WhatIsIt": "应用程序：它们是什么？", "Apps_WhatIsIt_paragraph1": "管理后台的新图标！这是什么意思，什么是应用程序？", "Apps_WhatIsIt_paragraph2": "首先，在这种情况下的应用程序不涉及移动应用程序。事实上，最好从插件或高级集成角度考虑它们。", @@ -3388,4 +3390,4 @@ "Your_server_link": "您的服务器链接", "Your_temporary_password_is_password": "您的暂时密码为 [password]。", "Your_workspace_is_ready": "您的工作区已准备好使用🎉" -} \ No newline at end of file +} diff --git a/server/lib/accounts.js b/server/lib/accounts.js index dd1dafdadf8f..fa771ce9cb64 100644 --- a/server/lib/accounts.js +++ b/server/lib/accounts.js @@ -247,6 +247,12 @@ Accounts.validateLoginAttempt(function(login) { return true; } + if (login.user.type === 'app') { + throw new Meteor.Error('error-app-user-is-not-allowed-to-login', 'App user is not allowed to login', { + function: 'Accounts.validateLoginAttempt', + }); + } + if (!!login.user.active !== true) { throw new Meteor.Error('error-user-is-not-activated', 'User is not activated', { function: 'Accounts.validateLoginAttempt',