(core) Adding creator as an owner for a new doc and ws

Summary:
By default editor inherits permission for a new document or workspace.
Now editor is added explicitly as an owner of a new doc or workspace.

Test Plan: Updated

Reviewers: georgegevoian, paulfitz

Reviewed By: georgegevoian, paulfitz

Subscribers: dsagal, paulfitz

Differential Revision: https://phab.getgrist.com/D3734

app/gen-server/lib/HomeDBManager.ts

@@ -251,6 +251,12 @@ export interface DocumentMetadata {
   usage?: DocumentUsage|null;
 }
 
+interface CreateWorkspaceOptions {
+  org: Organization,
+  props: Partial<WorkspaceProperties>,
+  ownerId?: number
+}
+
 /**
  * HomeDBManager handles interaction between the ApiServer and the Home database,
  * encapsulating the typeorm logic.
@@ -1418,7 +1424,7 @@ export class HomeDBManager extends EventEmitter {
       }
       // Add a starter workspace to the org.  Any limits on org workspace
       // count are not checked, this will succeed unconditionally.
-      await this._doAddWorkspace(savedOrg, {name: 'Home'}, manager);
+      await this._doAddWorkspace({org: savedOrg, props: {name: 'Home'}}, manager);
 
       if (!options.setUserAsOwner) {
         // This user just made a team site (once this transaction is applied).
@@ -1619,7 +1625,7 @@ export class HomeDBManager extends EventEmitter {
           });
         }
       }
-      const workspace = await this._doAddWorkspace(org, props, manager);
+      const workspace = await this._doAddWorkspace({org, props, ownerId: scope.userId}, manager);
       return {
         status: 200,
         data: workspace.id
@@ -1766,7 +1772,7 @@ export class HomeDBManager extends EventEmitter {
       }
       doc.workspace = workspace;
       // Create the special initial permission groups for the new workspace.
-      const groupMap = this._createGroups(workspace);
+      const groupMap = this._createGroups(workspace, scope.userId);
       doc.aclRules = this.defaultCommonGroups.map(_grpDesc => {
         // Get the special group with the name needed for this ACL Rule
         const group = groupMap[_grpDesc.name];
@@ -1780,6 +1786,15 @@ export class HomeDBManager extends EventEmitter {
       // Saves the document as well as its new ACL Rules and Group.
       const groups = doc.aclRules.map(rule => rule.group);
       const result = await manager.save([doc, ...doc.aclRules, ...doc.aliases, ...groups]);
+      // Ensure that the creator is in the ws and org's guests group. Creator already has
+      // access to the workspace (he is at least an editor), but we need to be sure that
+      // even if he is removed from the workspace, he will still have access to this doc.
+      // Guest groups are updated after any access is changed, so even if we won't add creator
+      // now, he will be added later. NOTE: those functions would normally fail in transaction
+      // as those groups might by already fixed (when there is another doc created in the same
+      // time), but they are ignoring any unique constraints errors.
+      await this._repairWorkspaceGuests(scope, workspace.id, manager);
+      await this._repairOrgGuests(scope, workspace.org.id, manager);
       return {
         status: 200,
         data: (result[0] as Document).id
@@ -2939,6 +2954,9 @@ export class HomeDBManager extends EventEmitter {
     if (toAdd.length > 0) {
       await manager.createQueryBuilder()
         .insert()
+        // Since we are adding new records in group_users, we may get a duplicate key error if two documents
+        // are added at the same time (even in transaction, since we are not blocking the whole table).
+        .orIgnore()
         .into('group_users')
         .values(toAdd.map(id => ({user_id: id, group_id: groupId})))
         .execute();
@@ -2968,8 +2986,10 @@ export class HomeDBManager extends EventEmitter {
    * Creates, initializes and saves a workspace in the given org with the given properties.
    * Product limits on number of workspaces allowed in org are not checked.
    */
-  private async _doAddWorkspace(org: Organization, props: Partial<WorkspaceProperties>,
+  private async _doAddWorkspace(
-                                transaction?: EntityManager): Promise<Workspace> {
+    {org, props, ownerId}: CreateWorkspaceOptions,
+    transaction?: EntityManager
+  ): Promise<Workspace> {
     if (!props.name) { throw new ApiError('Bad request: name required', 400); }
     return await this._runInTransaction(transaction, async manager => {
       // Create a new workspace.
@@ -2978,7 +2998,8 @@ export class HomeDBManager extends EventEmitter {
       workspace.updateFromProperties(props);
       workspace.org = org;
       // Create the special initial permission groups for the new workspace.
-      const groupMap = this._createGroups(org);
+      // Optionally add the owner to the workspace.
+      const groupMap = this._createGroups(org, ownerId);
       workspace.aclRules = this.defaultCommonGroups.map(_grpDesc => {
         // Get the special group with the name needed for this ACL Rule
         const group = groupMap[_grpDesc.name];
@@ -2992,6 +3013,11 @@ export class HomeDBManager extends EventEmitter {
       // Saves the workspace as well as its new ACL Rules and Group.
       const groups = workspace.aclRules.map(rule => rule.group);
       const result = await manager.save([workspace, ...workspace.aclRules, ...groups]);
+      if (ownerId) {
+        // If we modified direct access to the workspace, we need to update the
+        // guest group to include the owner.
+        await this._repairOrgGuests({userId: ownerId}, org.id, manager);
+      }
       return result[0];
     });
   }
@@ -3586,7 +3612,7 @@ export class HomeDBManager extends EventEmitter {
    * Returns a name to group mapping for the standard groups. Useful when adding a new child
    * entity. Finds and includes the correct parent groups as member groups.
    */
-  private _createGroups(inherit?: Organization|Workspace): {[name: string]: Group} {
+  private _createGroups(inherit?: Organization|Workspace, ownerId?: number): {[name: string]: Group} {
     const groupMap: {[name: string]: Group} = {};
     this.defaultGroups.forEach(groupProps => {
       if (!groupProps.orgOnly || !inherit) {
@@ -3599,6 +3625,13 @@ export class HomeDBManager extends EventEmitter {
         groupMap[groupProps.name] = group;
       }
     });
+    // Add the owner explicitly to the owner group.
+    if (ownerId) {
+      const ownerGroup = groupMap[roles.OWNER];
+      const user = new User();
+      user.id = ownerId;
+      ownerGroup.memberUsers = [user];
+    }
     return groupMap;
   }
 

test/server/lib/DocApi.ts

@@ -198,6 +198,47 @@ describe('DocApi', function() {
 
 // Contains the tests. This is where you want to add more test.
 function testDocApi() {
+  it("creator should be owner of a created ws", async () => {
+    const kiwiEmail = 'kiwi@getgrist.com';
+    const ws1 = (await userApi.getOrgWorkspaces('current'))[0].id;
+    // Make sure kiwi isn't allowed here.
+    await userApi.updateOrgPermissions(ORG_NAME, {users: {[kiwiEmail]: null}});
+    const kiwiApi = makeUserApi(ORG_NAME, 'kiwi');
+    await assert.isRejected(kiwiApi.getWorkspaceAccess(ws1), /Forbidden/);
+    // Add kiwi as an editor for the org.
+    await assert.isRejected(kiwiApi.getOrgAccess(ORG_NAME), /Forbidden/);
+    await userApi.updateOrgPermissions(ORG_NAME, {users: {[kiwiEmail]: 'editors'}});
+    // Make a workspace as Kiwi, he should be owner of it.
+    const kiwiWs = await kiwiApi.newWorkspace({name: 'kiwiWs'}, ORG_NAME);
+    const kiwiWsAccess = await kiwiApi.getWorkspaceAccess(kiwiWs);
+    assert.equal(kiwiWsAccess.users.find(u => u.email === kiwiEmail)?.access, 'owners');
+    // Delete workspace.
+    await kiwiApi.deleteWorkspace(kiwiWs);
+    // Remove kiwi from the org.
+    await userApi.updateOrgPermissions(ORG_NAME, {users: {[kiwiEmail]: null}});
+  });
+
+  it("creator should be owner of a created doc", async () => {
+    const kiwiEmail = 'kiwi@getgrist.com';
+    const ws1 = (await userApi.getOrgWorkspaces('current'))[0].id;
+    await userApi.updateOrgPermissions(ORG_NAME, {users: {[kiwiEmail]: null}});
+    // Make sure kiwi isn't allowed here.
+    const kiwiApi = makeUserApi(ORG_NAME, 'kiwi');
+    await assert.isRejected(kiwiApi.getWorkspaceAccess(ws1), /Forbidden/);
+    // Add kiwi as an editor of this workspace.
+    await userApi.updateWorkspacePermissions(ws1, {users: {[kiwiEmail]: 'editors'}});
+    await assert.isFulfilled(kiwiApi.getWorkspaceAccess(ws1));
+    // Create a document as kiwi.
+    const kiwiDoc = await kiwiApi.newDoc({name: 'kiwiDoc'}, ws1);
+    // Make sure kiwi is an owner of the document.
+    const kiwiDocAccess = await kiwiApi.getDocAccess(kiwiDoc);
+    assert.equal(kiwiDocAccess.users.find(u => u.email === kiwiEmail)?.access, 'owners');
+    await kiwiApi.deleteDoc(kiwiDoc);
+    // Remove kiwi from the workspace.
+    await userApi.updateWorkspacePermissions(ws1, {users: {[kiwiEmail]: null}});
+    await assert.isRejected(kiwiApi.getWorkspaceAccess(ws1), /Forbidden/);
+  });
+
   it("should allow only owners to remove a document", async () => {
     const ws1 = (await userApi.getOrgWorkspaces('current'))[0].id;
     const doc1 = await userApi.newDoc({name: 'testdeleteme1'}, ws1);
@@ -226,6 +267,8 @@ function testDocApi() {
     // Kiwi is owner of the document - now he can rename it.
     await userApi.updateDocPermissions(doc1, {users: {'kiwi@getgrist.com': 'owners'}});
     await assert.isFulfilled(kiwiApi.renameDoc(doc1, "testrenameme2"));
+
+    await userApi.deleteDoc(doc1);
   });
 
   it("guesses types of new columns", async () => {
@@ -3551,8 +3594,8 @@ function testDocApi() {
         assert.isAtLeast(stats[0].usage?.updatedTime ?? 0, now);
         assert.isNull(stats[0].usage?.lastErrorMessage);
         assert.isNull(stats[0].usage?.lastFailureTime);
-        assert.isAtLeast(stats[0].usage?.lastSuccessTime ?? 0, now);
         assert.equal(stats[0].usage?.lastHttpStatus, 200);
+        assert.isAtLeast(stats[0].usage?.lastSuccessTime ?? 0, now);
         assert.deepEqual(stats[0].usage?.lastEventBatch, {
           status: 'success',
           attempts: 1,