From 328f7315414b3f19d737c4b1d8d40e1b71d18276 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Sun, 19 Aug 2018 16:06:39 -0700 Subject: [PATCH] Share Permissions (#761) * Share restrictions * Tweak language, add spec --- app/components/Checkbox.js | 45 +++++ app/components/HelpText/HelpText.js | 1 + app/components/Sidebar/Settings.js | 6 + app/components/Toasts/components/Toast.js | 2 +- app/menus/DocumentMenu.js | 21 ++- app/routes.js | 2 + app/scenes/Document/components/Header.js | 11 +- app/scenes/Settings/Details.js | 2 +- app/scenes/Settings/Security.js | 80 +++++++++ app/scenes/Settings/Shares.js | 20 ++- app/stores/AuthStore.js | 6 +- app/types/index.js | 1 + flow-typed/npm/react-router-dom_v4.x.x.js | 160 ------------------ server/api/shares.js | 4 +- server/api/shares.test.js | 9 + server/api/team.js | 3 +- .../20180819054252-disable-sharing.js | 12 ++ server/models/Team.js | 16 +- server/policies/team.js | 5 + server/presenters/team.js | 1 + shared/styles/theme.js | 2 +- 21 files changed, 224 insertions(+), 185 deletions(-) create mode 100644 app/components/Checkbox.js create mode 100644 app/scenes/Settings/Security.js delete mode 100644 flow-typed/npm/react-router-dom_v4.x.x.js create mode 100644 server/migrations/20180819054252-disable-sharing.js diff --git a/app/components/Checkbox.js b/app/components/Checkbox.js new file mode 100644 index 00000000..6f17e645 --- /dev/null +++ b/app/components/Checkbox.js @@ -0,0 +1,45 @@ +// @flow +import * as React from 'react'; +import styled from 'styled-components'; +import HelpText from 'components/HelpText'; + +export type Props = { + checked?: boolean, + label?: string, + className?: string, + note?: string, +}; + +const LabelText = styled.span` + font-weight: 500; + margin-left: 10px; +`; + +const Wrapper = styled.div` + padding-bottom: 8px; +`; + +const Label = styled.label` + display: flex; + align-items: center; +`; + +export default function Checkbox({ + label, + note, + className, + short, + ...rest +}: Props) { + return ( + + + + {note && {note}} + + + ); +} diff --git a/app/components/HelpText/HelpText.js b/app/components/HelpText/HelpText.js index f096c9f2..9a0df7f6 100644 --- a/app/components/HelpText/HelpText.js +++ b/app/components/HelpText/HelpText.js @@ -4,6 +4,7 @@ import styled from 'styled-components'; const HelpText = styled.p` margin-top: 0; color: ${props => props.theme.slateDark}; + font-size: ${props => (props.small ? '13px' : 'auto')}; `; export default HelpText; diff --git a/app/components/Sidebar/Settings.js b/app/components/Sidebar/Settings.js index 8a38ba73..a5b9d6fe 100644 --- a/app/components/Sidebar/Settings.js +++ b/app/components/Sidebar/Settings.js @@ -5,6 +5,7 @@ import { DocumentIcon, ProfileIcon, SettingsIcon, + PadlockIcon, CodeIcon, UserIcon, LinkIcon, @@ -61,6 +62,11 @@ class SettingsSidebar extends React.Component { Details )} + {user.isAdmin && ( + }> + Security + + )} }> People diff --git a/app/components/Toasts/components/Toast.js b/app/components/Toasts/components/Toast.js index e2b60355..9bc3ab7d 100644 --- a/app/components/Toasts/components/Toast.js +++ b/app/components/Toasts/components/Toast.js @@ -49,7 +49,7 @@ const Container = styled.li` align-items: center; animation: ${fadeAndScaleIn} 100ms ease; margin: 8px 0; - padding: 8px; + padding: 10px 12px; color: ${props => props.theme.white}; background: ${props => props.theme[props.type]}; font-size: 15px; diff --git a/app/menus/DocumentMenu.js b/app/menus/DocumentMenu.js index 026891b4..cc2889a8 100644 --- a/app/menus/DocumentMenu.js +++ b/app/menus/DocumentMenu.js @@ -6,11 +6,13 @@ import { MoreIcon } from 'outline-icons'; import Document from 'models/Document'; import UiStore from 'stores/UiStore'; +import AuthStore from 'stores/AuthStore'; import { documentMoveUrl } from 'utils/routeHelpers'; import { DropdownMenu, DropdownMenuItem } from 'components/DropdownMenu'; type Props = { ui: UiStore, + auth: AuthStore, label?: React.Node, history: Object, document: Document, @@ -69,7 +71,8 @@ class DocumentMenu extends React.Component { }; render() { - const { document, label, className, showPrint } = this.props; + const { document, label, className, showPrint, auth } = this.props; + const canShareDocuments = auth.team && auth.team.sharing; return ( } className={className}> @@ -91,12 +94,14 @@ class DocumentMenu extends React.Component { Star )} - - Share link… - + {canShareDocuments && ( + + Share link… + + )}
{ } } -export default withRouter(inject('ui')(DocumentMenu)); +export default withRouter(inject('ui', 'auth')(DocumentMenu)); diff --git a/app/routes.js b/app/routes.js index b2be2b83..5450806d 100644 --- a/app/routes.js +++ b/app/routes.js @@ -11,6 +11,7 @@ import Document from 'scenes/Document'; import Search from 'scenes/Search'; import Settings from 'scenes/Settings'; import Details from 'scenes/Settings/Details'; +import Security from 'scenes/Settings/Security'; import People from 'scenes/Settings/People'; import Slack from 'scenes/Settings/Slack'; import Shares from 'scenes/Settings/Shares'; @@ -43,6 +44,7 @@ export default function Routes() { + diff --git a/app/scenes/Document/components/Header.js b/app/scenes/Document/components/Header.js index 4ae8202d..2b9c4668 100644 --- a/app/scenes/Document/components/Header.js +++ b/app/scenes/Document/components/Header.js @@ -2,11 +2,12 @@ import * as React from 'react'; import { throttle } from 'lodash'; import { observable } from 'mobx'; -import { observer } from 'mobx-react'; +import { observer, inject } from 'mobx-react'; import styled from 'styled-components'; import breakpoint from 'styled-components-breakpoint'; import { NewDocumentIcon } from 'outline-icons'; import Document from 'models/Document'; +import AuthStore from 'stores/AuthStore'; import { documentEditUrl } from 'utils/routeHelpers'; import Flex from 'shared/components/Flex'; @@ -32,6 +33,7 @@ type Props = { autosave?: boolean, }) => *, history: Object, + auth: AuthStore, }; @observer @@ -90,7 +92,9 @@ class Header extends React.Component { isPublishing, isSaving, savingIsDisabled, + auth, } = this.props; + const canShareDocuments = auth.team && auth.team.sharing; return ( { )} {!isDraft && - !isEditing && ( + !isEditing && + canShareDocuments && ( Share @@ -251,4 +256,4 @@ const Link = styled.a` cursor: ${props => (props.disabled ? 'default' : 'pointer')}; `; -export default Header; +export default inject('auth')(Header); diff --git a/app/scenes/Settings/Details.js b/app/scenes/Settings/Details.js index 4854e480..686d6d74 100644 --- a/app/scenes/Settings/Details.js +++ b/app/scenes/Settings/Details.js @@ -44,7 +44,7 @@ class Details extends React.Component { name: this.name, avatarUrl: this.avatarUrl, }); - this.props.ui.showToast('Details saved', 'success'); + this.props.ui.showToast('Settings saved', 'success'); }; handleNameChange = (ev: SyntheticInputEvent<*>) => { diff --git a/app/scenes/Settings/Security.js b/app/scenes/Settings/Security.js new file mode 100644 index 00000000..35f31dcc --- /dev/null +++ b/app/scenes/Settings/Security.js @@ -0,0 +1,80 @@ +// @flow +import * as React from 'react'; +import { observable } from 'mobx'; +import { observer, inject } from 'mobx-react'; + +import AuthStore from 'stores/AuthStore'; +import UiStore from 'stores/UiStore'; +import Checkbox from 'components/Checkbox'; +import Button from 'components/Button'; +import CenteredContent from 'components/CenteredContent'; +import PageTitle from 'components/PageTitle'; +import HelpText from 'components/HelpText'; + +type Props = { + auth: AuthStore, + ui: UiStore, +}; + +@observer +class Security extends React.Component { + form: ?HTMLFormElement; + + @observable sharing: boolean; + + componentDidMount() { + const { auth } = this.props; + if (auth.team) { + this.sharing = auth.team.sharing; + } + } + + handleSubmit = async (ev: SyntheticEvent<*>) => { + ev.preventDefault(); + + await this.props.auth.updateTeam({ + sharing: this.sharing, + }); + this.props.ui.showToast('Settings saved', 'success'); + }; + + handleChange = (ev: SyntheticInputEvent<*>) => { + if (ev.target.name === 'sharing') { + this.sharing = ev.target.checked; + } + }; + + get isValid() { + return this.form && this.form.checkValidity(); + } + + render() { + const { isSaving } = this.props.auth; + + return ( + + +

Security

+ + Settings that impact the access, security and privacy of your + knowledgebase. + + +
(this.form = ref)}> + + + +
+ ); + } +} + +export default inject('auth', 'ui')(Security); diff --git a/app/scenes/Settings/Shares.js b/app/scenes/Settings/Shares.js index cefaaac2..73382373 100644 --- a/app/scenes/Settings/Shares.js +++ b/app/scenes/Settings/Shares.js @@ -1,7 +1,9 @@ // @flow import * as React from 'react'; import { observer, inject } from 'mobx-react'; +import { Link } from 'react-router-dom'; import SharesStore from 'stores/SharesStore'; +import AuthStore from 'stores/AuthStore'; import ShareListItem from './components/ShareListItem'; import List from 'components/List'; @@ -11,6 +13,7 @@ import HelpText from 'components/HelpText'; type Props = { shares: SharesStore, + auth: AuthStore, }; @observer @@ -20,7 +23,9 @@ class Shares extends React.Component { } render() { - const { shares } = this.props; + const { shares, auth } = this.props; + const { user } = auth; + const canShareDocuments = auth.team && auth.team.sharing; return ( @@ -31,7 +36,16 @@ class Shares extends React.Component { can access a read-only version of the document until the link has been revoked. - + {user && + user.isAdmin && ( + + {!canShareDocuments && ( + Sharing is currently disabled. + )}{' '} + You can turn {canShareDocuments ? 'off' : 'on'} public document + sharing in security settings. + + )} {shares.orderedData.map(share => ( @@ -42,4 +56,4 @@ class Shares extends React.Component { } } -export default inject('shares')(Shares); +export default inject('shares', 'auth')(Shares); diff --git a/app/stores/AuthStore.js b/app/stores/AuthStore.js index ba9cd30d..02672538 100644 --- a/app/stores/AuthStore.js +++ b/app/stores/AuthStore.js @@ -78,7 +78,11 @@ class AuthStore { }; @action - updateTeam = async (params: { name: string, avatarUrl: ?string }) => { + updateTeam = async (params: { + name?: string, + avatarUrl?: ?string, + sharing?: boolean, + }) => { this.isSaving = true; try { diff --git a/app/types/index.js b/app/types/index.js index f1d0ce77..a461d793 100644 --- a/app/types/index.js +++ b/app/types/index.js @@ -31,6 +31,7 @@ export type Team = { avatarUrl: string, slackConnected: boolean, googleConnected: boolean, + sharing: boolean, }; export type NavigationNode = { diff --git a/flow-typed/npm/react-router-dom_v4.x.x.js b/flow-typed/npm/react-router-dom_v4.x.x.js deleted file mode 100644 index c573be56..00000000 --- a/flow-typed/npm/react-router-dom_v4.x.x.js +++ /dev/null @@ -1,160 +0,0 @@ -// flow-typed signature: cf916fca23433d4bbcb7a75f2604407d -// flow-typed version: f821d89401/react-router-dom_v4.x.x/flow_>=v0.53.x - -declare module "react-router-dom" { - declare export class BrowserRouter extends React$Component<{ - basename?: string, - forceRefresh?: boolean, - getUserConfirmation?: GetUserConfirmation, - keyLength?: number, - children?: React$Node - }> {} - - declare export class HashRouter extends React$Component<{ - basename?: string, - getUserConfirmation?: GetUserConfirmation, - hashType?: "slash" | "noslash" | "hashbang", - children?: React$Node - }> {} - - declare export class Link extends React$Component<{ - className?: string, - to: string | LocationShape, - replace?: boolean, - children?: React$Node - }> {} - - declare export class NavLink extends React$Component<{ - to: string | LocationShape, - activeClassName?: string, - className?: string, - activeStyle?: Object, - style?: Object, - isActive?: (match: Match, location: Location) => boolean, - children?: React$Node, - exact?: boolean, - strict?: boolean - }> {} - - // NOTE: Below are duplicated from react-router. If updating these, please - // update the react-router and react-router-native types as well. - declare export type Location = { - pathname: string, - search: string, - hash: string, - state?: any, - key?: string - }; - - declare export type LocationShape = { - pathname?: string, - search?: string, - hash?: string, - state?: any - }; - - declare export type HistoryAction = "PUSH" | "REPLACE" | "POP"; - - declare export type RouterHistory = { - length: number, - location: Location, - action: HistoryAction, - listen( - callback: (location: Location, action: HistoryAction) => void - ): () => void, - push(path: string | LocationShape, state?: any): void, - replace(path: string | LocationShape, state?: any): void, - go(n: number): void, - goBack(): void, - goForward(): void, - canGo?: (n: number) => boolean, - block( - callback: (location: Location, action: HistoryAction) => boolean - ): void, - // createMemoryHistory - index?: number, - entries?: Array - }; - - declare export type Match = { - params: { [key: string]: ?string }, - isExact: boolean, - path: string, - url: string - }; - - declare export type ContextRouter = {| - history: RouterHistory, - location: Location, - match: Match, - staticContext?: StaticRouterContext, -|}; - - declare export type GetUserConfirmation = ( - message: string, - callback: (confirmed: boolean) => void - ) => void; - - declare type StaticRouterContext = { - url?: string - }; - - declare export class StaticRouter extends React$Component<{ - basename?: string, - location?: string | Location, - context: StaticRouterContext, - children?: React$Node - }> {} - - declare export class MemoryRouter extends React$Component<{ - initialEntries?: Array, - initialIndex?: number, - getUserConfirmation?: GetUserConfirmation, - keyLength?: number, - children?: React$Node - }> {} - - declare export class Router extends React$Component<{ - history: RouterHistory, - children?: React$Node - }> {} - - declare export class Prompt extends React$Component<{ - message: string | ((location: Location) => string | boolean), - when?: boolean - }> {} - - declare export class Redirect extends React$Component<{ - to: string | LocationShape, - push?: boolean - }> {} - - declare export class Route extends React$Component<{ - component?: React$ComponentType<*>, - render?: (router: ContextRouter) => React$Node, - children?: React$ComponentType | React$Node, - path?: string, - exact?: boolean, - strict?: boolean - }> {} - - declare export class Switch extends React$Component<{ - children?: React$Node - }> {} - - declare export function withRouter

( - Component: React$ComponentType<{| ...ContextRouter, ...P |}> - ): React$ComponentType

; - - declare type MatchPathOptions = { - path?: string, - exact?: boolean, - sensitive?: boolean, - strict?: boolean - }; - - declare export function matchPath( - pathname: string, - options?: MatchPathOptions | string - ): null | Match; -} diff --git a/server/api/shares.js b/server/api/shares.js index 35b9f345..be3c8c26 100644 --- a/server/api/shares.js +++ b/server/api/shares.js @@ -4,7 +4,7 @@ import Sequelize from 'sequelize'; import auth from '../middlewares/authentication'; import pagination from './middlewares/pagination'; import { presentShare } from '../presenters'; -import { Document, User, Share } from '../models'; +import { Document, User, Share, Team } from '../models'; import policy from '../policies'; const Op = Sequelize.Op; @@ -57,7 +57,9 @@ router.post('shares.create', auth(), async ctx => { const user = ctx.state.user; const document = await Document.findById(documentId); + const team = await Team.findById(user.teamId); authorize(user, 'share', document); + authorize(user, 'share', team); const [share] = await Share.findOrCreate({ where: { diff --git a/server/api/shares.test.js b/server/api/shares.test.js index fd65a08e..b5c01e33 100644 --- a/server/api/shares.test.js +++ b/server/api/shares.test.js @@ -122,6 +122,15 @@ describe('#shares.create', async () => { expect(body.data.id).toBe(share.id); }); + it('should not allow creating a share record if disabled', async () => { + const { user, document, team } = await seed(); + await team.update({ sharing: false }); + const res = await server.post('/api/shares.create', { + body: { token: user.getJwtToken(), documentId: document.id }, + }); + expect(res.status).toEqual(403); + }); + it('should require authentication', async () => { const { document } = await seed(); const res = await server.post('/api/shares.create', { diff --git a/server/api/team.js b/server/api/team.js index d04c729c..3bc4a1b8 100644 --- a/server/api/team.js +++ b/server/api/team.js @@ -12,7 +12,7 @@ const { authorize } = policy; const router = new Router(); router.post('team.update', auth(), async ctx => { - const { name, avatarUrl } = ctx.body; + const { name, avatarUrl, sharing } = ctx.body; const endpoint = publicS3Endpoint(); const user = ctx.state.user; @@ -20,6 +20,7 @@ router.post('team.update', auth(), async ctx => { authorize(user, 'update', team); if (name) team.name = name; + if (sharing !== undefined) team.sharing = sharing; if (avatarUrl && avatarUrl.startsWith(`${endpoint}/uploads/${user.id}`)) { team.avatarUrl = avatarUrl; } diff --git a/server/migrations/20180819054252-disable-sharing.js b/server/migrations/20180819054252-disable-sharing.js new file mode 100644 index 00000000..38246eee --- /dev/null +++ b/server/migrations/20180819054252-disable-sharing.js @@ -0,0 +1,12 @@ +module.exports = { + up: async (queryInterface, Sequelize) => { + await queryInterface.addColumn('teams', 'sharing', { + type: Sequelize.BOOLEAN, + allowNull: false, + defaultValue: true + }); + }, + down: async (queryInterface, Sequelize) => { + await queryInterface.removeColumn('teams', 'sharing'); + } +} \ No newline at end of file diff --git a/server/models/Team.js b/server/models/Team.js index 808e85b5..93330385 100644 --- a/server/models/Team.js +++ b/server/models/Team.js @@ -17,6 +17,7 @@ const Team = sequelize.define( slackId: { type: DataTypes.STRING, allowNull: true }, googleId: { type: DataTypes.STRING, allowNull: true }, avatarUrl: { type: DataTypes.STRING, allowNull: true }, + sharing: { type: DataTypes.BOOLEAN, allowNull: false, defaultValue: true }, slackData: DataTypes.JSONB, }, { @@ -39,11 +40,16 @@ const uploadAvatar = async model => { const endpoint = publicS3Endpoint(); if (model.avatarUrl && !model.avatarUrl.startsWith(endpoint)) { - const newUrl = await uploadToS3FromUrl( - model.avatarUrl, - `avatars/${model.id}/${uuid.v4()}` - ); - if (newUrl) model.avatarUrl = newUrl; + try { + const newUrl = await uploadToS3FromUrl( + model.avatarUrl, + `avatars/${model.id}/${uuid.v4()}` + ); + if (newUrl) model.avatarUrl = newUrl; + } catch (err) { + // we can try again next time + console.error(err); + } } }; diff --git a/server/policies/team.js b/server/policies/team.js index acd07777..414a9cc3 100644 --- a/server/policies/team.js +++ b/server/policies/team.js @@ -7,6 +7,11 @@ const { allow } = policy; allow(User, 'read', Team, (user, team) => team && user.teamId === team.id); +allow(User, 'share', Team, (user, team) => { + if (!team || user.teamId !== team.id) return false; + return team.sharing; +}); + allow(User, ['update', 'export'], Team, (user, team) => { if (!team || user.teamId !== team.id) return false; if (user.isAdmin) return true; diff --git a/server/presenters/team.js b/server/presenters/team.js index 863de4a3..a600ac2c 100644 --- a/server/presenters/team.js +++ b/server/presenters/team.js @@ -11,6 +11,7 @@ function present(ctx: Object, team: Team) { team.avatarUrl || (team.slackData ? team.slackData.image_88 : null), slackConnected: !!team.slackId, googleConnected: !!team.googleId, + sharing: team.sharing, }; } diff --git a/shared/styles/theme.js b/shared/styles/theme.js index cfa9b4b4..b8dc020c 100644 --- a/shared/styles/theme.js +++ b/shared/styles/theme.js @@ -11,7 +11,7 @@ const theme = { placeholder: '#b1becc', danger: '#D0021B', warning: '#f08a24', - success: '#1AB6FF', + success: '#2f3336', info: '#a0d3e8', slate: '#9BA6B2',