diff --git a/app/client/models/UserManagerModel.ts b/app/client/models/UserManagerModel.ts index 3a973507..0e3268e8 100644 --- a/app/client/models/UserManagerModel.ts +++ b/app/client/models/UserManagerModel.ts @@ -5,8 +5,8 @@ import {ShareAnnotations, ShareAnnotator} from 'app/common/ShareAnnotator'; import {normalizeEmail} from 'app/common/emails'; import {GristLoadConfig} from 'app/common/gristUrls'; import * as roles from 'app/common/roles'; -import {ANONYMOUS_USER_EMAIL, EVERYONE_EMAIL, Organization, PermissionData, PermissionDelta, - UserAPI, Workspace} from 'app/common/UserAPI'; +import {ANONYMOUS_USER_EMAIL, Document, EVERYONE_EMAIL, Organization, + PermissionData, PermissionDelta, UserAPI, Workspace} from 'app/common/UserAPI'; import {getRealAccess} from 'app/common/UserAPI'; import {computed, Computed, Disposable, obsArray, ObsArray, observable, Observable} from 'grainjs'; import some = require('lodash/some'); @@ -22,8 +22,10 @@ export interface UserManagerModel { publicMember: IEditableMember|null; // Member whose access (VIEWER or null) represents that of // anon@ or everyone@ (depending on the settings and resource). isAnythingChanged: Computed; // Indicates whether there are unsaved changes + isSelfRemoved: Computed; // Indicates whether current user is removed isOrg: boolean; // Indicates if the UserManager is for an org annotations: Observable; // More information about shares, keyed by email. + isPersonal: boolean; // If user access info/control is restricted to self. gristDoc: GristDoc|null; // Populated if there is an open document. @@ -119,6 +121,8 @@ export class UserManagerModelImpl extends Disposable implements UserManagerModel public annotations = this.autoDispose(observable({users: new Map()})); + public isPersonal = this.initData.personal || false; + public isOrg: boolean = this.resourceType === 'organization'; public gristDoc: GristDoc|null; @@ -133,6 +137,11 @@ export class UserManagerModelImpl extends Disposable implements UserManagerModel (this.publicMember ? isMemberChangedFn(this.publicMember) : false); })); + // Check if the current user is being removed. + public readonly isSelfRemoved: Computed = this.autoDispose(computed((use) => { + return some(use(this.membersEdited), m => m.isRemoved && m.email ===this._options.activeEmail); + })); + private _shareAnnotator?: ShareAnnotator; constructor( diff --git a/app/client/ui/AccountWidget.ts b/app/client/ui/AccountWidget.ts index 9ec42ab5..f6c180f4 100644 --- a/app/client/ui/AccountWidget.ts +++ b/app/client/ui/AccountWidget.ts @@ -105,8 +105,9 @@ export class AccountWidget extends Disposable { // Show 'Organization Settings' when on a home page of a valid org. (!this._docPageModel && currentOrg && !currentOrg.owner ? - menuItem(() => manageUsers(currentOrg), 'Manage Team', testId('dm-org-access'), - dom.cls('disabled', !roles.canEditAccess(currentOrg.access))) : + menuItem(() => manageUsers(currentOrg), + roles.canEditAccess(currentOrg.access) ? 'Manage Team' : 'Access Details', + testId('dm-org-access')) : // Don't show on doc pages, or for personal orgs. null), diff --git a/app/client/ui/AppHeader.ts b/app/client/ui/AppHeader.ts index 3f577c47..9bf02740 100644 --- a/app/client/ui/AppHeader.ts +++ b/app/client/ui/AppHeader.ts @@ -37,7 +37,7 @@ export class AppHeader extends Disposable { activeEmail: user ? user.email : null, resourceType: 'organization', resourceId: org.id, - resource: org, + resource: org }); }; diff --git a/app/client/ui/DocMenu.ts b/app/client/ui/DocMenu.ts index 58acb3d0..23f0e8bf 100644 --- a/app/client/ui/DocMenu.ts +++ b/app/client/ui/DocMenu.ts @@ -432,6 +432,7 @@ export function makeDocOptionsMenu(home: HomeModel, doc: Document, renaming: Obs activeEmail: user ? user.email : null, resourceType: 'document', resourceId: doc.id, + resource: doc, linkToCopy: urlState().makeUrl(docUrl(doc)), reload: () => api.getDocAccess(doc.id), appModel: home.app, @@ -463,10 +464,9 @@ export function makeDocOptionsMenu(home: HomeModel, doc: Document, renaming: Obs dom.cls('disabled', !roles.canEdit(orgAccess)), testId('pin-doc') ), - menuItem(manageUsers, "Manage Users", - dom.cls('disabled', !roles.canEditAccess(doc.access)), + menuItem(manageUsers, roles.canEditAccess(doc.access) ? "Manage Users" : "Access Details", testId('doc-access') - ), + ) ]; } diff --git a/app/client/ui/HomeLeftPane.ts b/app/client/ui/HomeLeftPane.ts index 073e71cf..dfd55ce2 100644 --- a/app/client/ui/HomeLeftPane.ts +++ b/app/client/ui/HomeLeftPane.ts @@ -204,7 +204,8 @@ function workspaceMenu(home: HomeModel, ws: Workspace, renaming: Observable !roles.canEdit(ws.access)), testId('dm-delete-workspace')), - upgradableMenuItem(needUpgrade, manageWorkspaceUsers, "Manage Users", - dom.cls('disabled', !roles.canEditAccess(ws.access)), + upgradableMenuItem(needUpgrade, manageWorkspaceUsers, + roles.canEditAccess(ws.access) ? "Manage Users" : "Access Details", testId('dm-workspace-access')), upgradeText(needUpgrade), ]; diff --git a/app/client/ui/ShareMenu.ts b/app/client/ui/ShareMenu.ts index f1c00242..f2f1bd8b 100644 --- a/app/client/ui/ShareMenu.ts +++ b/app/client/ui/ShareMenu.ts @@ -123,8 +123,9 @@ function shareButton(buttonText: string|null, menuCreateFunc: MenuCreateFunc, // Renders "Manage Users" menu item. function menuManageUsers(doc: DocInfo, pageModel: DocPageModel) { return [ - menuItem(() => manageUsers(doc, pageModel), 'Manage Users', - dom.cls('disabled', !roles.canEditAccess(doc.access) || doc.isFork), + menuItem(() => manageUsers(doc, pageModel), + roles.canEditAccess(doc.access) ? 'Manage Users' : 'Access Details', + dom.cls('disabled', doc.isFork), testId('tb-share-option') ), menuDivider(), @@ -244,11 +245,14 @@ async function manageUsers(doc: DocInfo, docPageModel: DocPageModel) { activeEmail: user ? user.email : null, resourceType: 'document', resourceId: doc.id, + resource: doc, docPageModel, appModel: docPageModel.appModel, linkToCopy: urlState().makeUrl(docUrl(doc)), // On save, re-fetch the document info, to toggle the "Public Access" icon if it changed. - onSave: () => docPageModel.refreshCurrentDoc(doc), + // Skip if personal, since personal cannot affect "Public Access", and the only + // change possible is to remove the user (which would make refreshCurrentDoc fail) + onSave: async (personal) => !personal && docPageModel.refreshCurrentDoc(doc), reload: () => api.getDocAccess(doc.id), }); } diff --git a/app/client/ui/UserManager.ts b/app/client/ui/UserManager.ts index e1a08997..b666eaf0 100644 --- a/app/client/ui/UserManager.ts +++ b/app/client/ui/UserManager.ts @@ -35,7 +35,8 @@ import {colors, testId, vars} from 'app/client/ui2018/cssVars'; import {icon} from 'app/client/ui2018/icons'; import {cssLink} from 'app/client/ui2018/links'; import {inputMenu, menu, menuItem, menuText} from 'app/client/ui2018/menus'; -import {cssModalBody, cssModalButtons, cssModalTitle, IModalControl, modal} from 'app/client/ui2018/modals'; +import {confirmModal, cssModalBody, cssModalButtons, cssModalTitle, IModalControl, + modal} from 'app/client/ui2018/modals'; export interface IUserManagerOptions { permissionData: Promise; @@ -47,7 +48,7 @@ export interface IUserManagerOptions { appModel?: AppModel; // If present, we offer access to a nested team-level dialog. linkToCopy?: string; reload?: () => Promise; - onSave?: () => Promise; + onSave?: (personal: boolean) => Promise; prompt?: { // If set, user manager should open with this email filled in and ready to go. email: string; }; @@ -71,22 +72,42 @@ export function showUserManagerModal(userApi: UserAPI, options: IUserManagerOpti async function onConfirm(ctl: IModalControl) { const model = modelObs.get(); - if (model) { + if (!model) { + ctl.close(); + return; + } + const tryToSaveChanges = async () => { // Save changes to the server, reporting any errors to the app. try { - if (model.isAnythingChanged.get()) { + const isAnythingChanged = model.isAnythingChanged.get(); + if (isAnythingChanged) { await model.save(userApi, options.resourceId); } - await options.onSave?.(); + await options.onSave?.(model.isPersonal); ctl.close(); + if (model.isPersonal && isAnythingChanged) { + // the only thing an individual without ACL_EDIT rights can do is + // remove themselves - so reload. + window.location.reload(); + } } catch (err) { reportError(err); } + }; + if (model.isSelfRemoved.get()) { + const name = resourceName(model.resourceType); + confirmModal( + `You are about to remove your own access to this ${name}`, + 'Remove my access', tryToSaveChanges, + 'Once you have removed your own access, ' + + 'you will not be able to get it back without assistance ' + + `from someone else with sufficient access to the ${name}.`); } else { - ctl.close(); + tryToSaveChanges().catch(reportError); } } + const personal = !roles.canEditAccess(options.resource?.access || null); // Get the model and assign it to the observable. Report errors to the app. getModel(options) .then(model => modelObs.set(model)) @@ -97,8 +118,9 @@ export function showUserManagerModal(userApi: UserAPI, options: IUserManagerOpti options.showAnimation ? dom.cls(cssAnimatedModal.className) : null, cssModalTitle( { style: 'margin: 40px 64px 0 64px;' }, - renderTitle(options.resourceType, options.resource), - (options.resourceType === 'document' ? makeCopyBtn(options.linkToCopy, cssCopyBtn.cls('-header')) : null), + renderTitle(options.resourceType, options.resource, personal), + ((options.resourceType === 'document' && !personal) ? + makeCopyBtn(options.linkToCopy, cssCopyBtn.cls('-header')) : null), testId('um-header') ), @@ -121,7 +143,7 @@ export function showUserManagerModal(userApi: UserAPI, options: IUserManagerOpti dom.on('click', () => ctl.close()), testId('um-cancel') ), - dom.maybe(use => use(modelObs)?.resourceType === 'document' && use(modelObs)?.gristDoc, () => + dom.maybe(use => use(modelObs)?.resourceType === 'document' && use(modelObs)?.gristDoc && !personal, () => cssAccessLink({href: urlState().makeUrl({docPage: 'acl'})}, dom.text(use => (use(modelObs) && use(use(modelObs)!.isAnythingChanged)) ? 'Save & ' : ''), 'Open Access Rules', @@ -159,8 +181,8 @@ export class UserManager extends Disposable { const memberEmail = this.autoDispose(new MemberEmail(this._onAdd.bind(this), this._options.prompt)); return [ - memberEmail.buildDom(), - this._buildOptionsDom(), + this._model.isPersonal ? null : memberEmail.buildDom(), + this._model.isPersonal ? null : this._buildOptionsDom(), this._dom = shadowScroll( testId('um-members'), this._buildPublicAccessMember(), @@ -212,6 +234,7 @@ export class UserManager extends Disposable { // Build a single member row. private _buildMemberDom(member: IEditableMember) { const disableRemove = Computed.create(null, (use) => + this._model.isPersonal ? !member.origAccess : Boolean(this._model.isActiveUser(member) || use(member.inheritedAccess))); return dom('div', dom.autoDispose(disableRemove), @@ -632,9 +655,10 @@ const cssAnimatedModal = styled('div', ` `); // Render the UserManager title for `resourceType` (e.g. org as "team site"). -function renderTitle(resourceType: ResourceType, resource?: Resource) { +function renderTitle(resourceType: ResourceType, resource?: Resource, personal?: boolean) { switch (resourceType) { case 'organization': { + if (personal) { return 'Your role for this team site'; } return [ 'Manage members of team site', !resource ? null : cssOrgName( @@ -645,7 +669,12 @@ function renderTitle(resourceType: ResourceType, resource?: Resource) { ]; } default: { - return `Invite people to ${resourceType}`; + return personal ? `Your role for this ${resourceType}` : `Invite people to ${resourceType}`; } } } + +// Rename organization to team site. +function resourceName(resourceType: ResourceType): string { + return resourceType === 'organization' ? 'team site' : resourceType; +} diff --git a/app/common/UserAPI.ts b/app/common/UserAPI.ts index 055de845..680b6d35 100644 --- a/app/common/UserAPI.ts +++ b/app/common/UserAPI.ts @@ -151,6 +151,7 @@ export interface PermissionDelta { } export interface PermissionData { + personal?: boolean; maxInheritedRole?: roles.BasicRole|null; users: UserAccessData[]; } diff --git a/app/gen-server/lib/HomeDBManager.ts b/app/gen-server/lib/HomeDBManager.ts index 0b6f0277..4b5f4bc7 100644 --- a/app/gen-server/lib/HomeDBManager.ts +++ b/app/gen-server/lib/HomeDBManager.ts @@ -112,6 +112,18 @@ export interface UserIdDelta { [userId: string]: roles.NonGuestRole|null; } +// A collection of fun facts derived from a PermissionDelta (used to describe +// a change of users) and a user. +export interface PermissionDeltaAnalysis { + userIdDelta: UserIdDelta | null; // New roles for users, indexed by user id. + permissionThreshold: Permissions; // The permissions needed to make the change. + // Usually Permissions.ACL_EDIT, but + // Permissions.ACL_VIEW is enough for a user + // to removed themselves. + affectsSelf: boolean; // Flags if the user making the change would + // be affected by the change. +} + // Options for certain create query helpers private to this file. interface QueryOptions { manager?: EntityManager; @@ -1818,7 +1830,9 @@ export class HomeDBManager extends EventEmitter { } // Get the ids of users to update. const billingAccountId = billingAccount.id; - const userIdDelta = await this._verifyAndLookupDeltaEmails(userId, permissionDelta, true, transaction); + const analysis = await this._verifyAndLookupDeltaEmails(userId, permissionDelta, true, transaction); + this._failIfPowerfulAndChangingSelf(analysis); + const {userIdDelta} = analysis; if (!userIdDelta) { throw new ApiError('No userIdDelta', 500); } // Any duplicated emails have been merged, and userIdDelta is now keyed by user ids. // Now we iterate over users and add/remove them as managers. @@ -1862,10 +1876,11 @@ export class HomeDBManager extends EventEmitter { const {userId} = scope; const notifications: Array<() => void> = []; const result = await this._connection.transaction(async manager => { - const userIdDelta = await this._verifyAndLookupDeltaEmails(userId, delta, true, manager); + const analysis = await this._verifyAndLookupDeltaEmails(userId, delta, true, manager); + const {userIdDelta} = analysis; let orgQuery = this.org(scope, orgKey, { manager, - markPermissions: Permissions.ACL_EDIT, + markPermissions: analysis.permissionThreshold, needRealOrg: true }) // Join the org's ACL rules (with 1st level groups/users listed) so we can edit them. @@ -1873,11 +1888,13 @@ export class HomeDBManager extends EventEmitter { .leftJoinAndSelect('acl_rules.group', 'org_groups') .leftJoinAndSelect('org_groups.memberUsers', 'org_member_users'); orgQuery = this._addFeatures(orgQuery); + orgQuery = this._withAccess(orgQuery, userId, 'orgs'); const queryResult = await verifyIsPermitted(orgQuery); if (queryResult.status !== 200) { // If the query for the organization failed, return the failure result. return queryResult; } + this._failIfPowerfulAndChangingSelf(analysis, queryResult); const org: Organization = queryResult.data; const groups = getNonGuestGroups(org); if (userIdDelta) { @@ -1916,10 +1933,11 @@ export class HomeDBManager extends EventEmitter { const {userId} = scope; const notifications: Array<() => void> = []; const result = await this._connection.transaction(async manager => { - let userIdDelta = await this._verifyAndLookupDeltaEmails(userId, delta, false, manager); + const analysis = await this._verifyAndLookupDeltaEmails(userId, delta, false, manager); + let {userIdDelta} = analysis; let wsQuery = this._workspace(scope, wsId, { manager, - markPermissions: Permissions.ACL_EDIT + markPermissions: analysis.permissionThreshold, }) // Join the workspace's ACL rules and groups/users so we can edit them. .leftJoinAndSelect('workspaces.aclRules', 'acl_rules') @@ -1931,11 +1949,13 @@ export class HomeDBManager extends EventEmitter { .leftJoinAndSelect('org_acl_rules.group', 'org_groups') .leftJoinAndSelect('org_groups.memberUsers', 'org_users'); wsQuery = this._addFeatures(wsQuery, 'org'); + wsQuery = this._withAccess(wsQuery, userId, 'workspaces'); const queryResult = await verifyIsPermitted(wsQuery); if (queryResult.status !== 200) { // If the query for the workspace failed, return the failure result. return queryResult; } + this._failIfPowerfulAndChangingSelf(analysis, queryResult); const ws: Workspace = queryResult.data; // Get all the non-guest groups on the org. const orgGroups = getNonGuestGroups(ws.org); @@ -1986,8 +2006,10 @@ export class HomeDBManager extends EventEmitter { const notifications: Array<() => void> = []; const result = await this._connection.transaction(async manager => { const {userId} = scope; - let userIdDelta = await this._verifyAndLookupDeltaEmails(userId, delta, false, manager); - const doc = await this._loadDocAccess(scope, Permissions.ACL_EDIT, manager); + const analysis = await this._verifyAndLookupDeltaEmails(userId, delta, false, manager); + let {userIdDelta} = analysis; + const doc = await this._loadDocAccess(scope, analysis.permissionThreshold, manager); + this._failIfPowerfulAndChangingSelf(analysis, {data: doc, status: 200}); // Get all the non-guest doc groups to be updated by the delta. const groups = getNonGuestGroups(doc); if ('maxInheritedRole' in delta) { @@ -2050,9 +2072,11 @@ export class HomeDBManager extends EventEmitter { ...this.makeFullUser(u), access: userRoleMap[u.id] })); + const personal = this._filterAccessData(scope, users, null); return { status: 200, data: { + ...personal, users } }; @@ -2094,10 +2118,13 @@ export class HomeDBManager extends EventEmitter { parentAccess: roles.getEffectiveRole(orgMap[u.id] || null) }; }); + const maxInheritedRole = this._getMaxInheritedRole(workspace); + const personal = this._filterAccessData(scope, users, maxInheritedRole); return { status: 200, data: { - maxInheritedRole: this._getMaxInheritedRole(workspace), + ...personal, + maxInheritedRole, users } }; @@ -2173,14 +2200,7 @@ export class HomeDBManager extends EventEmitter { maxInheritedRole = null; } - // Unless we have special access to the document, or are an owner, limit user information - // returned to being about the current user. - const thisUser = users.find(user => user.id === scope.userId); - if (scope.specialPermit?.docId !== doc.id && - (!thisUser || getRealAccess(thisUser, { maxInheritedRole, users }) !== 'owners')) { - // If not an owner, don't return information about other users. - users = thisUser ? [thisUser] : []; - } + const personal = this._filterAccessData(scope, users, maxInheritedRole, doc.id); // If we are on a fork, make any access changes needed. Assumes results // have been flattened. @@ -2193,6 +2213,7 @@ export class HomeDBManager extends EventEmitter { return { status: 200, data: { + ...personal, maxInheritedRole, users } @@ -2952,7 +2973,7 @@ export class HomeDBManager extends EventEmitter { delta: PermissionDelta, isOrg: boolean = false, transaction?: EntityManager - ): Promise { + ): Promise { if (!delta) { throw new ApiError('Bad request: missing permission delta', 400); } @@ -3001,12 +3022,35 @@ export class HomeDBManager extends EventEmitter { userIdMap[userIdAffected] = emailMap[email]; }); } - if (userId in userIdMap) { + const userIdDelta = delta.users ? userIdMap : null; + const userIds = Object.keys(userIdDelta || {}); + const removingSelf = userIds.length === 1 && userIds[0] === String(userId) && + delta.maxInheritedRole === undefined && userIdDelta?.[userId] === null; + const permissionThreshold = removingSelf ? Permissions.VIEW : Permissions.ACL_EDIT; + return { + userIdDelta, + permissionThreshold, + affectsSelf: userId in userIdMap, + }; + } + + /** + * A helper to throw an error if a user with ACL_EDIT permission attempts + * to change their own access rights. The user permissions are expected to + * be in the supplied QueryResult, or if none is supplied are assumed to be + * ACL_EDIT. + */ + private _failIfPowerfulAndChangingSelf(analysis: PermissionDeltaAnalysis, result?: QueryResult) { + const permissions: Permissions = result ? result.data.permissions : Permissions.ACL_EDIT; + if (permissions === undefined) { + throw new Error('Query malformed'); + } + if ((permissions & Permissions.ACL_EDIT) && analysis.affectsSelf) { + // editors don't get to remove themselves. // TODO: Consider when to allow updating own permissions - allowing updating own // permissions indiscriminately could lead to orphaned resources. throw new ApiError('Bad request: cannot update own permissions', 400); } - return delta.users ? userIdMap : null; } /** @@ -4113,6 +4157,21 @@ export class HomeDBManager extends EventEmitter { .execute(); }); } + + private _filterAccessData(scope: Scope, users: UserAccessData[], + maxInheritedRole: roles.BasicRole|null, docId?: string): { personal: true}|undefined { + if (scope.userId === this.getPreviewerUserId()) { return; } + // Unless we have special access to the resource, or are an owner, + // limit user information returned to being about the current user. + const thisUser = users.find(user => user.id === scope.userId); + if ((scope.specialPermit?.docId !== docId || !docId) && + (!thisUser || getRealAccess(thisUser, { maxInheritedRole, users }) !== 'owners')) { + // If not an owner, don't return information about other users. + users.length = 0; + if (thisUser) { users.push(thisUser); } + return { personal: true }; + } + } } // Return a QueryResult reflecting the output of a query builder. diff --git a/test/nbrowser/gristUtils.ts b/test/nbrowser/gristUtils.ts index fc8617ce..392f8732 100644 --- a/test/nbrowser/gristUtils.ts +++ b/test/nbrowser/gristUtils.ts @@ -1222,9 +1222,22 @@ export async function editOrgAcls(): Promise { await driver.findWait('.test-um-members', 3000); } -export async function saveAcls(): Promise { +/** + * Click confirm on a user manager dialog. If clickRemove is set, then + * any extra modal that pops up will be accepted. Returns true unless + * clickRemove was set and no modal popped up. + */ +export async function saveAcls(clickRemove: boolean = false): Promise { await driver.findWait('.test-um-confirm', 3000).click(); - await driver.wait(async () => !(await driver.find('.test-um-members').isPresent()), 3000); + let clickedRemove: boolean = false; + await driver.wait(async () => { + if (clickRemove && !clickedRemove && await driver.find('.test-modal-confirm').isPresent()) { + await driver.find('.test-modal-confirm').click(); + clickedRemove = true; + } + return !(await driver.find('.test-um-members').isPresent()); + }, 3000); + return clickedRemove || !clickRemove; } /**