From 3d6b9466fbb7442846ed93e7d012c32ea951c966 Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Sun, 4 Mar 2018 15:38:51 -0800 Subject: [PATCH 01/12] Backend support --- server/api/__snapshots__/user.test.js.snap | 118 ++++++++++++++ server/api/team.js | 42 +---- server/api/team.test.js | 69 --------- server/api/user.js | 92 ++++++++++- server/api/user.test.js | 145 ++++++++++++++++++ .../20180303193036-suspended-users.js | 18 +++ server/models/Team.js | 16 ++ server/models/User.js | 10 ++ server/pages/Api.js | 16 +- server/policies/user.js | 15 +- server/presenters/user.js | 1 + 11 files changed, 414 insertions(+), 128 deletions(-) create mode 100644 server/migrations/20180303193036-suspended-users.js diff --git a/server/api/__snapshots__/user.test.js.snap b/server/api/__snapshots__/user.test.js.snap index cf1545be2..81d7533b5 100644 --- a/server/api/__snapshots__/user.test.js.snap +++ b/server/api/__snapshots__/user.test.js.snap @@ -1,5 +1,64 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`#user.activate should activate a suspended user 1`] = ` +Object { + "data": Object { + "avatarUrl": "http://example.com/avatar.png", + "email": "user1@example.com", + "id": "46fde1d4-0050-428f-9f0b-0bf77f4bdf61", + "isAdmin": false, + "isSuspended": false, + "name": "User 1", + "username": "user1", + }, + "ok": true, + "status": 200, +} +`; + +exports[`#user.activate should require admin 1`] = ` +Object { + "error": "admin_required", + "message": "An admin role is required to access this resource", + "ok": false, + "status": 403, +} +`; + +exports[`#user.demote should demote an admin 1`] = ` +Object { + "data": Object { + "avatarUrl": "http://example.com/avatar.png", + "email": "user1@example.com", + "id": "46fde1d4-0050-428f-9f0b-0bf77f4bdf61", + "isAdmin": false, + "isSuspended": false, + "name": "User 1", + "username": "user1", + }, + "ok": true, + "status": 200, +} +`; + +exports[`#user.demote should require admin 1`] = ` +Object { + "error": "admin_required", + "message": "An admin role is required to access this resource", + "ok": false, + "status": 403, +} +`; + +exports[`#user.demote shouldn't demote admins if only one available 1`] = ` +Object { + "error": "validation_error", + "message": "At least one admin is required", + "ok": false, + "status": 400, +} +`; + exports[`#user.info should require authentication 1`] = ` Object { "error": "authentication_required", @@ -22,6 +81,65 @@ Object { } `; +exports[`#user.promote should promote a new admin 1`] = ` +Object { + "data": Object { + "avatarUrl": "http://example.com/avatar.png", + "email": "user1@example.com", + "id": "46fde1d4-0050-428f-9f0b-0bf77f4bdf61", + "isAdmin": true, + "isSuspended": false, + "name": "User 1", + "username": "user1", + }, + "ok": true, + "status": 200, +} +`; + +exports[`#user.promote should require admin 1`] = ` +Object { + "error": "admin_required", + "message": "An admin role is required to access this resource", + "ok": false, + "status": 403, +} +`; + +exports[`#user.suspend should require admin 1`] = ` +Object { + "error": "admin_required", + "message": "An admin role is required to access this resource", + "ok": false, + "status": 403, +} +`; + +exports[`#user.suspend should suspend an user 1`] = ` +Object { + "data": Object { + "avatarUrl": "http://example.com/avatar.png", + "email": "user1@example.com", + "id": "46fde1d4-0050-428f-9f0b-0bf77f4bdf61", + "isAdmin": false, + "isSuspended": true, + "name": "User 1", + "username": "user1", + }, + "ok": true, + "status": 200, +} +`; + +exports[`#user.suspend shouldn't allow suspending the user themselves 1`] = ` +Object { + "error": "validation_error", + "message": "Unable to suspend the current user", + "ok": false, + "status": 400, +} +`; + exports[`#user.update should require authentication 1`] = ` Object { "error": "authentication_required", diff --git a/server/api/team.js b/server/api/team.js index a812de8dc..d51cba7b2 100644 --- a/server/api/team.js +++ b/server/api/team.js @@ -1,14 +1,11 @@ // @flow import Router from 'koa-router'; -import { ValidationError } from '../errors'; -import { Team, User } from '../models'; +import { User } from '../models'; import auth from './middlewares/authentication'; import pagination from './middlewares/pagination'; import { presentUser } from '../presenters'; -import policy from '../policies'; -const { authorize } = policy; const router = new Router(); router.post('team.users', auth(), pagination(), async ctx => { @@ -31,41 +28,4 @@ router.post('team.users', auth(), pagination(), async ctx => { }; }); -router.post('team.addAdmin', auth(), async ctx => { - const userId = ctx.body.user; - const teamId = ctx.state.user.teamId; - ctx.assertPresent(userId, 'id is required'); - - const user = await User.findById(userId); - authorize(ctx.state.user, 'promote', user); - - const team = await Team.findById(teamId); - await team.addAdmin(user); - - ctx.body = { - data: presentUser(ctx, user, { includeDetails: true }), - }; -}); - -router.post('team.removeAdmin', auth(), async ctx => { - const userId = ctx.body.user; - const teamId = ctx.state.user.teamId; - ctx.assertPresent(userId, 'id is required'); - - const user = await User.findById(userId); - authorize(ctx.state.user, 'demote', user); - - const team = await Team.findById(teamId); - - try { - await team.removeAdmin(user); - } catch (err) { - throw new ValidationError(err.message); - } - - ctx.body = { - data: presentUser(ctx, user, { includeDetails: true }), - }; -}); - export default router; diff --git a/server/api/team.test.js b/server/api/team.test.js index 36aa3765d..19fb0b0ba 100644 --- a/server/api/team.test.js +++ b/server/api/team.test.js @@ -34,72 +34,3 @@ describe('#team.users', async () => { expect(body).toMatchSnapshot(); }); }); - -describe('#team.addAdmin', async () => { - it('should promote a new admin', async () => { - const { admin, user } = await seed(); - - const res = await server.post('/api/team.addAdmin', { - body: { token: admin.getJwtToken(), user: user.id }, - }); - const body = await res.json(); - - expect(res.status).toEqual(200); - expect(body).toMatchSnapshot(); - }); - - it('should require admin', async () => { - const { user } = await seed(); - const res = await server.post('/api/team.addAdmin', { - body: { token: user.getJwtToken(), user: user.id }, - }); - const body = await res.json(); - - expect(res.status).toEqual(403); - expect(body).toMatchSnapshot(); - }); -}); - -describe('#team.removeAdmin', async () => { - it('should demote an admin', async () => { - const { admin, user } = await seed(); - await user.update({ isAdmin: true }); // Make another admin - - const res = await server.post('/api/team.removeAdmin', { - body: { - token: admin.getJwtToken(), - user: user.id, - }, - }); - const body = await res.json(); - - expect(res.status).toEqual(200); - expect(body).toMatchSnapshot(); - }); - - it("shouldn't demote admins if only one available ", async () => { - const { admin } = await seed(); - - const res = await server.post('/api/team.removeAdmin', { - body: { - token: admin.getJwtToken(), - user: admin.id, - }, - }); - const body = await res.json(); - - expect(res.status).toEqual(400); - expect(body).toMatchSnapshot(); - }); - - it('should require admin', async () => { - const { user } = await seed(); - const res = await server.post('/api/team.addAdmin', { - body: { token: user.getJwtToken(), user: user.id }, - }); - const body = await res.json(); - - expect(res.status).toEqual(403); - expect(body).toMatchSnapshot(); - }); -}); diff --git a/server/api/user.js b/server/api/user.js index 252bc9e94..97c09a44e 100644 --- a/server/api/user.js +++ b/server/api/user.js @@ -2,10 +2,13 @@ import uuid from 'uuid'; import Router from 'koa-router'; import { makePolicy, signPolicy, publicS3Endpoint } from '../utils/s3'; -import { Event } from '../models'; +import { ValidationError } from '../errors'; +import { Event, User, Team } from '../models'; import auth from './middlewares/authentication'; import { presentUser } from '../presenters'; +import policy from '../policies'; +const { authorize } = policy; const router = new Router(); router.post('user.info', auth(), async ctx => { @@ -75,4 +78,91 @@ router.post('user.s3Upload', auth(), async ctx => { }; }); +// Admin specific + +router.post('user.promote', auth(), async ctx => { + const userId = ctx.body.id; + const teamId = ctx.state.user.teamId; + ctx.assertPresent(userId, 'id is required'); + + const user = await User.findById(userId); + authorize(ctx.state.user, 'promote', user); + + const team = await Team.findById(teamId); + await team.addAdmin(user); + + ctx.body = { + data: presentUser(ctx, user, { includeDetails: true }), + }; +}); + +router.post('user.demote', auth(), async ctx => { + const userId = ctx.body.id; + const teamId = ctx.state.user.teamId; + ctx.assertPresent(userId, 'id is required'); + + const user = await User.findById(userId); + authorize(ctx.state.user, 'demote', user); + + const team = await Team.findById(teamId); + try { + await team.removeAdmin(user); + } catch (err) { + throw new ValidationError(err.message); + } + + ctx.body = { + data: presentUser(ctx, user, { includeDetails: true }), + }; +}); + +/** + * Suspend user + * + * Admin can suspend users to reduce the number of accounts on their billing plan + */ +router.post('user.suspend', auth(), async ctx => { + const admin = ctx.state.user; + const userId = ctx.body.id; + const teamId = ctx.state.user.teamId; + ctx.assertPresent(userId, 'user is required'); + + const user = await User.findById(userId); + authorize(ctx.state.user, 'suspend', user); + + const team = await Team.findById(teamId); + try { + await team.suspendUser(user, admin); + } catch (err) { + throw new ValidationError(err.message); + } + + ctx.body = { + data: presentUser(ctx, user, { includeDetails: true }), + }; +}); + +/** + * Activate user + * + * Admin can activate users to let them access resources. These users will also + * account towards the billing plan limits. + */ +router.post('user.activate', auth(), async ctx => { + const admin = ctx.state.user; + const userId = ctx.body.id; + const teamId = ctx.state.user.teamId; + ctx.assertPresent(userId, 'user is required'); + + const user = await User.findById(userId); + authorize(ctx.state.user, 'activate', user); + + const team = await Team.findById(teamId); + await team.activateUser(user, admin); + + ctx.body = { + data: presentUser(ctx, user, { includeDetails: true }), + }; +}); + export default router; diff --git a/server/api/user.test.js b/server/api/user.test.js index e3bc2644f..941b3e0e0 100644 --- a/server/api/user.test.js +++ b/server/api/user.test.js @@ -66,3 +66,148 @@ describe('#user.update', async () => { expect(body).toMatchSnapshot(); }); }); + +describe('#user.promote', async () => { + it('should promote a new admin', async () => { + const { admin, user } = await seed(); + + const res = await server.post('/api/user.promote', { + body: { token: admin.getJwtToken(), user: user.id }, + }); + const body = await res.json(); + + expect(res.status).toEqual(200); + expect(body).toMatchSnapshot(); + }); + + it('should require admin', async () => { + const { user } = await seed(); + const res = await server.post('/api/user.promote', { + body: { token: user.getJwtToken(), user: user.id }, + }); + const body = await res.json(); + + expect(res.status).toEqual(403); + expect(body).toMatchSnapshot(); + }); +}); + +describe('#user.demote', async () => { + it('should demote an admin', async () => { + const { admin, user } = await seed(); + await user.update({ isAdmin: true }); // Make another admin + + const res = await server.post('/api/user.demote', { + body: { + token: admin.getJwtToken(), + user: user.id, + }, + }); + const body = await res.json(); + + expect(res.status).toEqual(200); + expect(body).toMatchSnapshot(); + }); + + it("shouldn't demote admins if only one available ", async () => { + const { admin } = await seed(); + + const res = await server.post('/api/user.demote', { + body: { + token: admin.getJwtToken(), + user: admin.id, + }, + }); + const body = await res.json(); + + expect(res.status).toEqual(400); + expect(body).toMatchSnapshot(); + }); + + it('should require admin', async () => { + const { user } = await seed(); + const res = await server.post('/api/user.promote', { + body: { token: user.getJwtToken(), user: user.id }, + }); + const body = await res.json(); + + expect(res.status).toEqual(403); + expect(body).toMatchSnapshot(); + }); +}); + +describe('#user.suspend', async () => { + it('should suspend an user', async () => { + const { admin, user } = await seed(); + + const res = await server.post('/api/user.suspend', { + body: { + token: admin.getJwtToken(), + user: user.id, + }, + }); + const body = await res.json(); + + expect(res.status).toEqual(200); + expect(body).toMatchSnapshot(); + }); + + it("shouldn't allow suspending the user themselves", async () => { + const { admin } = await seed(); + + const res = await server.post('/api/user.suspend', { + body: { + token: admin.getJwtToken(), + user: admin.id, + }, + }); + const body = await res.json(); + + expect(res.status).toEqual(400); + expect(body).toMatchSnapshot(); + }); + + it('should require admin', async () => { + const { user } = await seed(); + const res = await server.post('/api/user.suspend', { + body: { token: user.getJwtToken(), user: user.id }, + }); + const body = await res.json(); + + expect(res.status).toEqual(403); + expect(body).toMatchSnapshot(); + }); +}); + +describe('#user.activate', async () => { + it('should activate a suspended user', async () => { + const { admin, user } = await seed(); + await user.update({ + suspendedById: admin.id, + suspendedAt: new Date(), + }); + + expect(user.isSuspended).toBe(true); + const res = await server.post('/api/user.activate', { + body: { + token: admin.getJwtToken(), + user: user.id, + }, + }); + const body = await res.json(); + + expect(res.status).toEqual(200); + expect(body).toMatchSnapshot(); + }); + + it('should require admin', async () => { + const { user } = await seed(); + const res = await server.post('/api/user.activate', { + body: { token: user.getJwtToken(), user: user.id }, + }); + const body = await res.json(); + + expect(res.status).toEqual(403); + expect(body).toMatchSnapshot(); + }); +}); diff --git a/server/migrations/20180303193036-suspended-users.js b/server/migrations/20180303193036-suspended-users.js new file mode 100644 index 000000000..e49ce6fac --- /dev/null +++ b/server/migrations/20180303193036-suspended-users.js @@ -0,0 +1,18 @@ +module.exports = { + up: async (queryInterface, Sequelize) => { + await queryInterface.addColumn('users', 'suspendedById', { + type: Sequelize.UUID, + }); + await queryInterface.addColumn('users', 'suspendedAt', { + type: Sequelize.DATE, + allowNull: true, + }); + }, + + down: async (queryInterface, Sequelize) => { + await queryInterface.removeColumn('users', 'suspendedById'); + await queryInterface.removeColumn('users', 'suspendedAt'); + } +}; + + diff --git a/server/models/Team.js b/server/models/Team.js index 80dd76f12..87b1ae30c 100644 --- a/server/models/Team.js +++ b/server/models/Team.js @@ -63,4 +63,20 @@ Team.prototype.removeAdmin = async function(user: User) { } }; +Team.prototype.suspendUser = async function(user: User, admin: User) { + if (user.id === admin.id) + throw new Error('Unable to suspend the current user'); + return user.update({ + suspendedById: admin.id, + suspendedAt: new Date(), + }); +}; + +Team.prototype.activateUser = async function(user: User, admin: User) { + return user.update({ + suspendedById: null, + suspendedAt: null, + }); +}; + export default Team; diff --git a/server/models/User.js b/server/models/User.js index 8a2f69be5..93e44906a 100644 --- a/server/models/User.js +++ b/server/models/User.js @@ -28,8 +28,14 @@ const User = sequelize.define( slackId: { type: DataTypes.STRING, allowNull: true, unique: true }, slackData: DataTypes.JSONB, jwtSecret: encryptedFields.vault('jwtSecret'), + suspendedAt: DataTypes.DATE, }, { + getterMethods: { + isSuspended() { + return !!this.suspendedAt; + }, + }, indexes: [ { fields: ['email'], @@ -43,6 +49,10 @@ User.associate = models => { User.hasMany(models.ApiKey, { as: 'apiKeys' }); User.hasMany(models.Document, { as: 'documents' }); User.hasMany(models.View, { as: 'views' }); + User.belongsTo(models.User, { + as: 'suspendedBy', + foreignKey: 'suspendedById', + }); }; // Instance methods diff --git a/server/pages/Api.js b/server/pages/Api.js index 43c09878f..88f29c2ec 100644 --- a/server/pages/Api.js +++ b/server/pages/Api.js @@ -528,32 +528,24 @@ export default function Pricing() { - + Promote a user to be a team admin. This endpoint is only available for admin users. - + - + Demote existing team admin if there are more than one as one admin is always required. This endpoint is only available for admin users. - + diff --git a/server/policies/user.js b/server/policies/user.js index 4e8336b28..22c4858fe 100644 --- a/server/policies/user.js +++ b/server/policies/user.js @@ -19,8 +19,13 @@ allow(User, ['update', 'delete'], User, (actor, user) => { throw new AdminRequiredError(); }); -allow(User, ['promote', 'demote'], User, (actor, user) => { - if (!user || user.teamId !== actor.teamId) return false; - if (actor.isAdmin) return true; - throw new AdminRequiredError(); -}); +allow( + User, + ['promote', 'demote', 'activate', 'suspend'], + User, + (actor, user) => { + if (!user || user.teamId !== actor.teamId) return false; + if (actor.isAdmin) return true; + throw new AdminRequiredError(); + } +); diff --git a/server/presenters/user.js b/server/presenters/user.js index 99c6b17d8..6dbdeaeac 100644 --- a/server/presenters/user.js +++ b/server/presenters/user.js @@ -30,6 +30,7 @@ export default ( if (options.includeDetails) { userData.isAdmin = user.isAdmin; + userData.isSuspended = user.isSuspended; userData.email = user.email; } From 06a6573feb10a1569a2797422ee76b3f22e7915b Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Sun, 4 Mar 2018 15:39:17 -0800 Subject: [PATCH 02/12] Initial work on the frontend --- app/components/Auth.js | 6 +- app/scenes/Settings/Members.js | 38 ++++++++--- app/scenes/Settings/Tokens.js | 20 +++--- app/scenes/Settings/components/MemberMenu.js | 67 +++++++++++++++++++ .../ApiKeysStore.js} | 25 +------ app/stores/settings/MembersStore.js | 31 +++++++++ 6 files changed, 142 insertions(+), 45 deletions(-) create mode 100644 app/scenes/Settings/components/MemberMenu.js rename app/stores/{SettingsStore.js => settings/ApiKeysStore.js} (71%) create mode 100644 app/stores/settings/MembersStore.js diff --git a/app/components/Auth.js b/app/components/Auth.js index 90c9fd202..fa8ce301f 100644 --- a/app/components/Auth.js +++ b/app/components/Auth.js @@ -2,7 +2,8 @@ import React from 'react'; import { Provider } from 'mobx-react'; import stores from 'stores'; -import SettingsStore from 'stores/SettingsStore'; +import ApiKeysStore from 'stores/settings/ApiKeysStore'; +import MembersStore from 'stores/settings/MembersStore'; import DocumentsStore from 'stores/DocumentsStore'; import CollectionsStore from 'stores/CollectionsStore'; import CacheStore from 'stores/CacheStore'; @@ -22,7 +23,8 @@ const Auth = ({ children }: Props) => { const { user, team } = stores.auth; const cache = new CacheStore(user.id); authenticatedStores = { - settings: new SettingsStore(), + apiKeys: new ApiKeysStore(), + members: new MembersStore(), documents: new DocumentsStore({ ui: stores.ui, cache, diff --git a/app/scenes/Settings/Members.js b/app/scenes/Settings/Members.js index 748b90f83..c0c1b1dd5 100644 --- a/app/scenes/Settings/Members.js +++ b/app/scenes/Settings/Members.js @@ -1,5 +1,6 @@ // @flow import React, { Component } from 'react'; +import invariant from 'invariant'; import { observable } from 'mobx'; import { observer, inject } from 'mobx-react'; import styled from 'styled-components'; @@ -7,17 +8,20 @@ import Flex from 'shared/components/Flex'; import Avatar from 'components/Avatar'; import { color } from 'shared/styles/constants'; +import AuthStore from 'stores/AuthStore'; import ErrorsStore from 'stores/ErrorsStore'; -import SettingsStore from 'stores/SettingsStore'; +import MembersStore from 'stores/settings/MembersStore'; import CenteredContent from 'components/CenteredContent'; import LoadingPlaceholder from 'components/LoadingPlaceholder'; import PageTitle from 'components/PageTitle'; +import MemberMenu from './components/MemberMenu'; @observer class Members extends Component { props: { + auth: AuthStore, errors: ErrorsStore, - settings: SettingsStore, + members: MembersStore, }; @observable members; @@ -27,28 +31,37 @@ class Members extends Component { @observable isInviting: boolean = false; componentDidMount() { - this.props.settings.fetchMembers(); + this.props.members.fetchMembers(); } render() { + const user = this.props.auth.user; + invariant(user, 'User should exist'); + return (

Members

- {!this.props.settings.isFetching ? ( + {!this.props.members.isFetching ? ( - {this.props.settings.members && ( + {this.props.members.members && ( - {this.props.settings.members.map(member => ( + {this.props.members.members.map(member => ( {member.name} {member.email && `(${member.email})`} - {member.isAdmin && Admin} + {member.isAdmin && ( + Admin + )} + {member.isSuspended && Suspended} + + {user.id !== member.id && } + ))} @@ -84,12 +97,15 @@ const UserName = styled.span` padding-left: 8px; `; -const AdminBadge = styled.span` +const Badge = styled.span` margin-left: 10px; - color: #777; - font-size: 13px; + padding: 2px 6px; + background-color: ${({ admin }) => (admin ? color.primary : color.smokeDark)}; + color: ${({ admin }) => (admin ? color.white : color.text)}; + border-radius: 4px; + font-size: 11px; text-transform: uppercase; font-weight: normal; `; -export default inject('errors', 'settings')(Members); +export default inject('auth', 'errors', 'members')(Members); diff --git a/app/scenes/Settings/Tokens.js b/app/scenes/Settings/Tokens.js index c7c9ff516..3d8bdffc5 100644 --- a/app/scenes/Settings/Tokens.js +++ b/app/scenes/Settings/Tokens.js @@ -5,7 +5,7 @@ import { observer, inject } from 'mobx-react'; import { Link } from 'react-router-dom'; import styled from 'styled-components'; import ApiToken from './components/ApiToken'; -import SettingsStore from 'stores/SettingsStore'; +import ApiKeysStore from 'stores/settings/ApiKeysStore'; import { color } from 'shared/styles/constants'; import Button from 'components/Button'; @@ -19,11 +19,11 @@ import Subheading from 'components/Subheading'; class Tokens extends Component { @observable name: string = ''; props: { - settings: SettingsStore, + apiKeys: ApiKeysStore, }; componentDidMount() { - this.props.settings.fetchApiKeys(); + this.props.apiKeys.fetchApiKeys(); } handleUpdate = (ev: SyntheticInputEvent) => { @@ -32,13 +32,13 @@ class Tokens extends Component { handleSubmit = async (ev: SyntheticEvent) => { ev.preventDefault(); - await this.props.settings.createApiKey(this.name); + await this.props.apiKeys.createApiKey(this.name); this.name = ''; }; render() { - const { settings } = this.props; - const hasApiKeys = settings.apiKeys.length > 0; + const { apiKeys } = this.props; + const hasApiKeys = apiKeys.apiKeys.length > 0; return ( @@ -49,13 +49,13 @@ class Tokens extends Component { Your tokens, - {settings.apiKeys.map(key => ( + {apiKeys.apiKeys.map(key => ( ))} @@ -78,7 +78,7 @@ class Tokens extends Component {
- {apiKeys.apiKeys.map(key => ( + {apiKeys.data.map(key => ( { + fetchPage = async (options: ?PaginationParams): Promise<*> => { this.isFetching = true; try { - const res = await client.post('/apiKeys.list'); + const res = await client.post('/apiKeys.list', options); invariant(res && res.data, 'Data should be available'); const { data } = res; runInAction('fetchApiKeys', () => { - this.apiKeys = data; + this.data = data; }); } catch (e) { console.error('Something went wrong'); @@ -36,7 +36,7 @@ class SettingsApiKeySettingsStore { invariant(res && res.data, 'Data should be available'); const { data } = res; runInAction('createApiKey', () => { - this.apiKeys.push(data); + this.data.push(data); }); } catch (e) { console.error('Something went wrong'); @@ -49,7 +49,7 @@ class SettingsApiKeySettingsStore { try { await client.post('/apiKeys.delete', { id }); runInAction('deleteApiKey', () => { - this.fetchApiKeys(); + this.fetchPage(); }); } catch (e) { console.error('Something went wrong'); @@ -57,4 +57,4 @@ class SettingsApiKeySettingsStore { }; } -export default SettingsApiKeySettingsStore; +export default ApiKeysStore; From 71270bcb930024241b0e1d5575babeed204fabc2 Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Sun, 4 Mar 2018 22:34:06 -0800 Subject: [PATCH 07/12] Members -> Users --- app/components/Auth.js | 4 +- app/components/Sidebar/Settings.js | 4 +- app/index.js | 4 +- app/scenes/Settings/{Members.js => Users.js} | 63 +++++++++---------- .../components/{MemberMenu.js => UserMenu.js} | 24 +++---- .../{MemberSettingsStore.js => UsersStore.js} | 20 +++--- 6 files changed, 55 insertions(+), 64 deletions(-) rename app/scenes/Settings/{Members.js => Users.js} (52%) rename app/scenes/Settings/components/{MemberMenu.js => UserMenu.js} (78%) rename app/stores/{MemberSettingsStore.js => UsersStore.js} (74%) diff --git a/app/components/Auth.js b/app/components/Auth.js index d17cb9b8e..93a5dcd60 100644 --- a/app/components/Auth.js +++ b/app/components/Auth.js @@ -3,7 +3,7 @@ import React from 'react'; import { Provider } from 'mobx-react'; import stores from 'stores'; import ApiKeysStore from 'stores/ApiKeysStore'; -import MemberSettingsStore from 'stores/MemberSettingsStore'; +import UsersStore from 'stores/UsersStore'; import DocumentsStore from 'stores/DocumentsStore'; import CollectionsStore from 'stores/CollectionsStore'; import CacheStore from 'stores/CacheStore'; @@ -24,7 +24,7 @@ const Auth = ({ children }: Props) => { const cache = new CacheStore(user.id); authenticatedStores = { apiKeys: new ApiKeysStore(), - memberSettings: new MemberSettingsStore(), + users: new UsersStore(), documents: new DocumentsStore({ ui: stores.ui, cache, diff --git a/app/components/Sidebar/Settings.js b/app/components/Sidebar/Settings.js index 90995ffc0..8f9534eb2 100644 --- a/app/components/Sidebar/Settings.js +++ b/app/components/Sidebar/Settings.js @@ -53,8 +53,8 @@ class SettingsSidebar extends Component {
Team
- }> - Members + }> + Users - + - -

Members

+ +

Users

- {!this.props.memberSettings.isLoaded ? ( + {!this.props.users.isLoaded ? ( - {this.props.memberSettings.users && ( - - {this.props.memberSettings.users.map(member => ( - - - + {this.props.users.data && ( + + {this.props.users.data.map(user => ( + + + - {member.name} {member.email && `(${member.email})`} - {member.isAdmin && ( - Admin + {user.name} {user.email && `(${user.email})`} + {user.isAdmin && ( + Admin )} - {member.isSuspended && Suspended} + {user.isSuspended && Suspended} - + - {user.id !== member.id && } + {currentUser.id !== user.id && } - + ))} - + )} ) : ( @@ -75,7 +68,7 @@ class Members extends Component { } } -const MemberList = styled(Flex)` +const UserList = styled(Flex)` border: 1px solid ${color.smoke}; border-radius: 4px; @@ -83,7 +76,7 @@ const MemberList = styled(Flex)` margin-bottom: 40px; `; -const Member = styled(Flex)` +const User = styled(Flex)` padding: 10px; border-bottom: 1px solid ${color.smoke}; font-size: 15px; @@ -93,7 +86,7 @@ const Member = styled(Flex)` } `; -const MemberDetails = styled(Flex)` +const UserDetails = styled(Flex)` opacity: ${({ suspended }) => (suspended ? 0.5 : 1)}; `; @@ -112,4 +105,4 @@ const Badge = styled.span` font-weight: normal; `; -export default inject('auth', 'errors', 'memberSettings')(Members); +export default inject('auth', 'errors', 'users')(Users); diff --git a/app/scenes/Settings/components/MemberMenu.js b/app/scenes/Settings/components/UserMenu.js similarity index 78% rename from app/scenes/Settings/components/MemberMenu.js rename to app/scenes/Settings/components/UserMenu.js index d5664e6e8..70402b2c8 100644 --- a/app/scenes/Settings/components/MemberMenu.js +++ b/app/scenes/Settings/components/UserMenu.js @@ -2,23 +2,23 @@ import React, { Component } from 'react'; import { inject, observer } from 'mobx-react'; -import MemberSettingsStore from 'stores/MemberSettingsStore'; +import UsersStore from 'stores/UsersStore'; import MoreIcon from 'components/Icon/MoreIcon'; import { DropdownMenu, DropdownMenuItem } from 'components/DropdownMenu'; import type { User } from 'types'; type Props = { user: User, - memberSettings: MemberSettingsStore, + users: UsersStore, }; @observer -class MemberMenu extends Component { +class UserMenu extends Component { props: Props; handlePromote = (ev: SyntheticEvent) => { ev.preventDefault(); - const { user, memberSettings } = this.props; + const { user, users } = this.props; if ( !window.confirm( `Are you want to make ${ @@ -28,21 +28,21 @@ class MemberMenu extends Component { ) { return; } - memberSettings.promote(user); + users.promote(user); }; handleDemote = (ev: SyntheticEvent) => { ev.preventDefault(); - const { user, memberSettings } = this.props; + const { user, users } = this.props; if (!window.confirm(`Are you want to make ${user.name} a member?`)) { return; } - memberSettings.demote(user); + users.demote(user); }; handleSuspend = (ev: SyntheticEvent) => { ev.preventDefault(); - const { user, memberSettings } = this.props; + const { user, users } = this.props; if ( !window.confirm( "Are you want to suspend this account? Suspended users won't be able to access Outline." @@ -50,13 +50,13 @@ class MemberMenu extends Component { ) { return; } - memberSettings.suspend(user); + users.suspend(user); }; handleActivate = (ev: SyntheticEvent) => { ev.preventDefault(); - const { user, memberSettings } = this.props; - memberSettings.activate(user); + const { user, users } = this.props; + users.activate(user); }; render() { @@ -90,4 +90,4 @@ class MemberMenu extends Component { } } -export default inject('memberSettings')(MemberMenu); +export default inject('users')(UserMenu); diff --git a/app/stores/MemberSettingsStore.js b/app/stores/UsersStore.js similarity index 74% rename from app/stores/MemberSettingsStore.js rename to app/stores/UsersStore.js index 9a22fe9c2..d9df0b832 100644 --- a/app/stores/MemberSettingsStore.js +++ b/app/stores/UsersStore.js @@ -2,22 +2,22 @@ import { observable, action, runInAction } from 'mobx'; import invariant from 'invariant'; import { client } from 'utils/ApiClient'; -import type { User } from 'types'; +import type { User, PaginationParams } from 'types'; -class MemberSettingsStore { - @observable users: User[] = []; +class UsersStore { + @observable data: User[] = []; @observable isLoaded: boolean = false; @observable isSaving: boolean = false; @action - fetchUsers = async () => { + fetchPage = async (options: ?PaginationParams): Promise<*> => { try { - const res = await client.post('/team.users'); + const res = await client.post('/team.users', options); invariant(res && res.data, 'Data should be available'); const { data } = res; runInAction('fetchUsers', () => { - this.users = data.reverse(); + this.data = data.reverse(); }); } catch (e) { console.error('Something went wrong'); @@ -53,10 +53,8 @@ class MemberSettingsStore { invariant(res && res.data, 'Data should be available'); const { data } = res; - runInAction(`MemberSettingsStore#${action}`, () => { - this.users = this.users.map( - user => (user.id === data.id ? data : user) - ); + runInAction(`UsersStore#${action}`, () => { + this.data = this.data.map(user => (user.id === data.id ? data : user)); }); } catch (e) { console.error('Something went wrong'); @@ -64,4 +62,4 @@ class MemberSettingsStore { }; } -export default MemberSettingsStore; +export default UsersStore; From 8e4978c638da5f000d43c34842c2c14a9efcef66 Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Sun, 4 Mar 2018 22:38:14 -0800 Subject: [PATCH 08/12] Cleanup --- app/components/Layout/Layout.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/app/components/Layout/Layout.js b/app/components/Layout/Layout.js index 268e2b712..146e6f175 100644 --- a/app/components/Layout/Layout.js +++ b/app/components/Layout/Layout.js @@ -1,6 +1,6 @@ // @flow import React from 'react'; -import { Switch, Route, Redirect, withRouter } from 'react-router-dom'; +import { Switch, Route, withRouter } from 'react-router-dom'; import type { Location } from 'react-router-dom'; import { Helmet } from 'react-helmet'; import styled from 'styled-components'; @@ -67,16 +67,12 @@ class Layout extends React.Component { this.props.ui.setActiveModal('keyboard-shortcuts'); } - renderSuspended() { - return ; - } - render() { const { auth, ui } = this.props; const { user, team } = auth; const showSidebar = auth.authenticated && user && team; - if (auth.isSuspended) return this.renderSuspended(); + if (auth.isSuspended) return ; return ( From 2f972668f463e14a22a6a224bf000a0f3ffa8249 Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Sun, 4 Mar 2018 22:45:55 -0800 Subject: [PATCH 09/12] Tweaked label style --- app/scenes/Settings/Users.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/scenes/Settings/Users.js b/app/scenes/Settings/Users.js index 4e6613baf..0e726dc5a 100644 --- a/app/scenes/Settings/Users.js +++ b/app/scenes/Settings/Users.js @@ -96,10 +96,10 @@ const UserName = styled.span` const Badge = styled.span` margin-left: 10px; - padding: 2px 6px; + padding: 2px 6px 3px; background-color: ${({ admin }) => (admin ? color.primary : color.smokeDark)}; color: ${({ admin }) => (admin ? color.white : color.text)}; - border-radius: 4px; + border-radius: 2px; font-size: 11px; text-transform: uppercase; font-weight: normal; From e9e4538436436aaed05da62d10e22d7011204232 Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Tue, 6 Mar 2018 23:09:21 -0800 Subject: [PATCH 10/12] Addressed feedback --- app/scenes/ErrorSuspended/ErrorSuspended.js | 4 +- server/pages/Api.js | 73 +++++++++++++++------ 2 files changed, 54 insertions(+), 23 deletions(-) diff --git a/app/scenes/ErrorSuspended/ErrorSuspended.js b/app/scenes/ErrorSuspended/ErrorSuspended.js index 40ae40400..009aa6b98 100644 --- a/app/scenes/ErrorSuspended/ErrorSuspended.js +++ b/app/scenes/ErrorSuspended/ErrorSuspended.js @@ -15,8 +15,8 @@ const ErrorSuspended = () => (

- Team admin has suspended your account. To re-activate your account, please - reach out to them directly. + A team admin has suspended your account. To re-activate your account, + please reach out to them directly.

); diff --git a/server/pages/Api.js b/server/pages/Api.js index 88f29c2ec..d4ae28a9d 100644 --- a/server/pages/Api.js +++ b/server/pages/Api.js @@ -202,6 +202,58 @@ export default function Pricing() { + + + Promote a user to be a team admin. This endpoint is only available + for admin users. + + + + + + + + + Demote existing team admin if there are more than one as one admin + is always required. This endpoint is only available for admin + users. + + + + + + + + + Admin can suspend users to reduce the number of accounts on their + billing plan or prevent them from accessing documention. + + + + + + + + + Admin can re-active a suspended user. This will update the billing + plan and re-enable their access to the documention. + + + + + + - - - - Promote a user to be a team admin. This endpoint is only available - for admin users. - - - - - - - - - Demote existing team admin if there are more than one as one admin - is always required. This endpoint is only available for admin - users. - - - - - From f5c1ddf8b96e65dac3974b2ff3c3ee45319426df Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Tue, 6 Mar 2018 23:38:52 -0800 Subject: [PATCH 11/12] added suspending admin email to error screen --- app/scenes/ErrorSuspended/ErrorSuspended.js | 37 +++++++++++-------- app/stores/AuthStore.js | 2 + server/api/index.js | 6 ++- server/api/middlewares/authentication.js | 3 +- server/api/middlewares/authentication.test.js | 3 ++ server/errors.js | 7 ++-- server/models/User.js | 1 + 7 files changed, 37 insertions(+), 22 deletions(-) diff --git a/app/scenes/ErrorSuspended/ErrorSuspended.js b/app/scenes/ErrorSuspended/ErrorSuspended.js index 009aa6b98..dd4ab86ca 100644 --- a/app/scenes/ErrorSuspended/ErrorSuspended.js +++ b/app/scenes/ErrorSuspended/ErrorSuspended.js @@ -1,24 +1,29 @@ // @flow import React from 'react'; +import { inject, observer } from 'mobx-react'; import CenteredContent from 'components/CenteredContent'; import PageTitle from 'components/PageTitle'; +import AuthStore from 'stores/AuthStore'; -const ErrorSuspended = () => ( - - -

- - ⚠️ - {' '} - Your account has been suspended -

+const ErrorSuspended = observer(({ auth }: { auth: AuthStore }) => { + return ( + + +

+ + ⚠️ + {' '} + Your account has been suspended +

-

- A team admin has suspended your account. To re-activate your account, - please reach out to them directly. -

- -); +

+ A team admin ({auth.suspendedContactEmail}) has + suspended your account. To re-activate your account, please reach out to + them directly. +

+ + ); +}); -export default ErrorSuspended; +export default inject('auth')(ErrorSuspended); diff --git a/app/stores/AuthStore.js b/app/stores/AuthStore.js index 22a1f3a2b..ef1c9936a 100644 --- a/app/stores/AuthStore.js +++ b/app/stores/AuthStore.js @@ -15,6 +15,7 @@ class AuthStore { @observable oauthState: string; @observable isLoading: boolean = false; @observable isSuspended: boolean = false; + @observable suspendedContactEmail: ?string; /* Computed */ @@ -46,6 +47,7 @@ class AuthStore { } catch (err) { if (err.data.error === 'user_suspended') { this.isSuspended = true; + this.suspendedContactEmail = err.data.adminEmail; } } }; diff --git a/server/api/index.js b/server/api/index.js index 096a8c3e2..fb41620bb 100644 --- a/server/api/index.js +++ b/server/api/index.js @@ -29,6 +29,7 @@ api.use(async (ctx, next) => { } catch (err) { ctx.status = err.status || 500; let message = err.message || err.name; + let error; if (err instanceof Sequelize.ValidationError) { // super basic form error handling @@ -40,18 +41,21 @@ api.use(async (ctx, next) => { if (message.match('Authorization error')) { ctx.status = 403; + error = 'authorization_error'; } if (ctx.status === 500) { message = 'Internal Server Error'; + error = 'internal_server_error'; ctx.app.emit('error', err, ctx); } ctx.body = { ok: false, - error: _.snakeCase(err.id || err.message), + error: _.snakeCase(err.id || error), status: err.status, message, + adminEmail: err.adminEmail ? err.adminEmail : undefined, }; } }); diff --git a/server/api/middlewares/authentication.js b/server/api/middlewares/authentication.js index 85b865f2c..26e06a281 100644 --- a/server/api/middlewares/authentication.js +++ b/server/api/middlewares/authentication.js @@ -76,7 +76,8 @@ export default function auth() { } if (user.isSuspended) { - throw new UserSuspendedError(); + const suspendingAdmin = await User.findById(user.suspendedById); + throw new UserSuspendedError({ adminEmail: suspendingAdmin.email }); } ctx.state.token = token; diff --git a/server/api/middlewares/authentication.test.js b/server/api/middlewares/authentication.test.js index da6606ebc..a0a1208c0 100644 --- a/server/api/middlewares/authentication.test.js +++ b/server/api/middlewares/authentication.test.js @@ -159,8 +159,10 @@ describe('Authentication middleware', async () => { it('should return an error for suspended users', async () => { const state = {}; + const admin = await buildUser({}); const user = await buildUser({ suspendedAt: new Date(), + suspendedById: admin.id, }); const authMiddleware = auth(); @@ -179,6 +181,7 @@ describe('Authentication middleware', async () => { expect(e.message).toEqual( 'Your access has been suspended by the team admin' ); + expect(e.adminEmail).toEqual(admin.email); } }); }); diff --git a/server/errors.js b/server/errors.js index bcad3828e..0c509e99c 100644 --- a/server/errors.js +++ b/server/errors.js @@ -19,11 +19,10 @@ export function AdminRequiredError( return httpErrors(403, message, { id: 'admin_required' }); } -export function UserSuspendedError( - message: string = 'Your access has been suspended by the team admin' -) { - return httpErrors(403, message, { +export function UserSuspendedError({ adminEmail }: { adminEmail: string }) { + return httpErrors(403, 'Your access has been suspended by the team admin', { id: 'user_suspended', + adminEmail, }); } diff --git a/server/models/User.js b/server/models/User.js index 44ed4a09d..878757e4d 100644 --- a/server/models/User.js +++ b/server/models/User.js @@ -29,6 +29,7 @@ const User = sequelize.define( slackData: DataTypes.JSONB, jwtSecret: encryptedFields.vault('jwtSecret'), suspendedAt: DataTypes.DATE, + suspendedById: DataTypes.UUID, }, { getterMethods: { From 983b8fe9a00dd8224ec1c2b1a56e7d4b0c8b359c Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Wed, 7 Mar 2018 00:00:58 -0800 Subject: [PATCH 12/12] Wrapped additional error data under `data` --- app/stores/AuthStore.js | 4 ++-- app/utils/ApiClient.js | 2 +- server/api/index.js | 2 +- server/api/middlewares/authentication.test.js | 2 +- server/errors.js | 4 +++- 5 files changed, 8 insertions(+), 6 deletions(-) diff --git a/app/stores/AuthStore.js b/app/stores/AuthStore.js index ef1c9936a..318e939c5 100644 --- a/app/stores/AuthStore.js +++ b/app/stores/AuthStore.js @@ -45,9 +45,9 @@ class AuthStore { this.team = res.data.team; }); } catch (err) { - if (err.data.error === 'user_suspended') { + if (err.error.error === 'user_suspended') { this.isSuspended = true; - this.suspendedContactEmail = err.data.adminEmail; + this.suspendedContactEmail = err.error.data.adminEmail; } } }; diff --git a/app/utils/ApiClient.js b/app/utils/ApiClient.js index 88689709c..be832bfb7 100644 --- a/app/utils/ApiClient.js +++ b/app/utils/ApiClient.js @@ -84,7 +84,7 @@ class ApiClient { }) .catch(error => { error.response.json().then(json => { - error.data = json; + error.error = json; reject(error); }); }); diff --git a/server/api/index.js b/server/api/index.js index fb41620bb..da90ff112 100644 --- a/server/api/index.js +++ b/server/api/index.js @@ -55,7 +55,7 @@ api.use(async (ctx, next) => { error: _.snakeCase(err.id || error), status: err.status, message, - adminEmail: err.adminEmail ? err.adminEmail : undefined, + data: err.errorData ? err.errorData : undefined, }; } }); diff --git a/server/api/middlewares/authentication.test.js b/server/api/middlewares/authentication.test.js index a0a1208c0..2adb30661 100644 --- a/server/api/middlewares/authentication.test.js +++ b/server/api/middlewares/authentication.test.js @@ -181,7 +181,7 @@ describe('Authentication middleware', async () => { expect(e.message).toEqual( 'Your access has been suspended by the team admin' ); - expect(e.adminEmail).toEqual(admin.email); + expect(e.errorData.adminEmail).toEqual(admin.email); } }); }); diff --git a/server/errors.js b/server/errors.js index 0c509e99c..b35589ab0 100644 --- a/server/errors.js +++ b/server/errors.js @@ -22,7 +22,9 @@ export function AdminRequiredError( export function UserSuspendedError({ adminEmail }: { adminEmail: string }) { return httpErrors(403, 'Your access has been suspended by the team admin', { id: 'user_suspended', - adminEmail, + errorData: { + adminEmail, + }, }); }