Skip to content

Commit

Permalink
[NEW] Create a user for the Apps during installation (#15896)
Browse files Browse the repository at this point in the history
* 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 <upendrareddy2511@gmail.com>
Co-authored-by: Douglas Gubert <d-gubert@users.noreply.github.com>
  • Loading branch information
3 people committed Jan 27, 2020
1 parent bf17f4c commit 9054f0d
Show file tree
Hide file tree
Showing 15 changed files with 158 additions and 29 deletions.
11 changes: 8 additions & 3 deletions app/apps/client/admin/appInstall.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -19,19 +20,23 @@ 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;

switch (status) {
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;
Expand Down
64 changes: 63 additions & 1 deletion app/apps/server/bridges/users.js
Original file line number Diff line number Diff line change
@@ -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) {
Expand All @@ -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();
}
Expand Down
25 changes: 18 additions & 7 deletions app/apps/server/communication/rest.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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));
},
});

Expand Down
26 changes: 26 additions & 0 deletions app/apps/server/converters/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
}

Expand All @@ -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;
Expand Down
23 changes: 12 additions & 11 deletions app/authorization/server/startup.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'] },
Expand All @@ -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'] },
Expand All @@ -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'] },
Expand Down Expand Up @@ -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' },
Expand Down
4 changes: 4 additions & 0 deletions app/lib/server/functions/deleteUser.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
6 changes: 5 additions & 1 deletion app/models/server/models/Users.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 4 additions & 0 deletions app/models/server/raw/BaseRaw.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
2 changes: 2 additions & 0 deletions app/ui-login/client/login/form.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
}
Expand Down
2 changes: 1 addition & 1 deletion app/ui/client/views/app/directory.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 2 additions & 0 deletions packages/rocketchat-i18n/i18n/en.i18n.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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.",
Expand Down
4 changes: 3 additions & 1 deletion packages/rocketchat-i18n/i18n/zh.i18n.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": "应用名称",
Expand All @@ -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": "首先,在这种情况下的应用程序不涉及移动应用程序。事实上,最好从插件或高级集成角度考虑它们。",
Expand Down Expand Up @@ -3388,4 +3390,4 @@
"Your_server_link": "您的服务器链接",
"Your_temporary_password_is_password": "您的暂时密码为 <strong>[password]</strong>。",
"Your_workspace_is_ready": "您的工作区已准备好使用🎉"
}
}
6 changes: 6 additions & 0 deletions server/lib/accounts.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down

0 comments on commit 9054f0d

Please sign in to comment.