SF-3637 Deduplicate permissions

src/SIL.XForge.Scripture/ClientApp/src/app/core/permissions.service.spec.ts

@@ -105,16 +105,16 @@ describe('PermissionsService', () => {
 
     const textDoc: Partial<TextDocId> = { projectId: 'project01', bookNum: 41, chapterNum: 1 };
 
-    expect(await env.service.canAccessText(cloneDeep(textDoc) as TextDocId)).toBe(true);
+    expect(await env.service.canAccessTextAsync(cloneDeep(textDoc) as TextDocId)).toBe(true);
   }));
 
   it('allows access to text if user is on project and does not have chapter permission', fakeAsync(async () => {
     const env = new TestEnvironment();
-    env.setupProjectData(undefined);
+    env.setupProjectData(undefined, false);
 
     const textDoc: Partial<TextDocId> = { projectId: 'project01', bookNum: 41, chapterNum: 1 };
 
-    expect(await env.service.canAccessText(cloneDeep(textDoc) as TextDocId)).toBe(true);
+    expect(await env.service.canAccessTextAsync(cloneDeep(textDoc) as TextDocId)).toBe(true);
   }));
 
   it('does not allow access to text if user is not on project', fakeAsync(async () => {
@@ -123,7 +123,7 @@ describe('PermissionsService', () => {
 
     const textDoc: Partial<TextDocId> = { projectId: 'project01', bookNum: 41, chapterNum: 1 };
 
-    expect(await env.service.canAccessText(cloneDeep(textDoc) as TextDocId)).toBe(false);
+    expect(await env.service.canAccessTextAsync(cloneDeep(textDoc) as TextDocId)).toBe(false);
   }));
 
   it('does not allow access to text if user has no access', fakeAsync(async () => {
@@ -132,7 +132,7 @@ describe('PermissionsService', () => {
 
     const textDoc: Partial<TextDocId> = { projectId: 'project01', bookNum: 41, chapterNum: 1 };
 
-    expect(await env.service.canAccessText(cloneDeep(textDoc) as TextDocId)).toBe(false);
+    expect(await env.service.canAccessTextAsync(cloneDeep(textDoc) as TextDocId)).toBe(false);
   }));
 
   it('does not allow access to text if the chapter does not exist', fakeAsync(async () => {
@@ -141,7 +141,7 @@ describe('PermissionsService', () => {
 
     const textDoc: Partial<TextDocId> = { projectId: 'project01', bookNum: 41, chapterNum: 2 };
 
-    expect(await env.service.canAccessText(cloneDeep(textDoc) as TextDocId)).toBe(false);
+    expect(await env.service.canAccessTextAsync(cloneDeep(textDoc) as TextDocId)).toBe(false);
   }));
 
   it('checks current user doc to determine if user is on project', fakeAsync(async () => {
@@ -264,7 +264,7 @@ class TestEnvironment {
     this.setupUserData();
   }
 
-  setupProjectData(textPermission?: TextInfoPermission | undefined): void {
+  setupProjectData(textPermission?: TextInfoPermission | undefined, chapterPermissions: boolean = true): void {
     const projectId: string = 'project01';
     const permission: TextInfoPermission = textPermission ?? TextInfoPermission.Write;
 
@@ -284,10 +284,12 @@ class TestEnvironment {
                 number: 1,
                 lastVerse: 3,
                 isValid: true,
-                permissions: {
-                  user01: permission,
-                  user02: permission
-                }
+                permissions: chapterPermissions
+                  ? {
+                      user01: permission,
+                      user02: permission
+                    }
+                  : {}
               }
             ],
             hasSource: true,

src/SIL.XForge.Scripture/ClientApp/src/app/core/permissions.service.ts

@@ -1,8 +1,9 @@
 import { Injectable } from '@angular/core';
 import { Operation } from 'realtime-server/lib/esm/common/models/project-rights';
+import { SFProjectProfile } from 'realtime-server/lib/esm/scriptureforge/models/sf-project';
 import { SF_PROJECT_RIGHTS, SFProjectDomain } from 'realtime-server/lib/esm/scriptureforge/models/sf-project-rights';
 import { isParatextRole, SFProjectRole } from 'realtime-server/lib/esm/scriptureforge/models/sf-project-role';
-import { Chapter } from 'realtime-server/lib/esm/scriptureforge/models/text-info';
+import { Chapter, TextInfo } from 'realtime-server/lib/esm/scriptureforge/models/text-info';
 import { TextInfoPermission } from 'realtime-server/lib/esm/scriptureforge/models/text-info-permission';
 import { UserDoc } from 'xforge-common/models/user-doc';
 import { UserService } from 'xforge-common/user.service';
@@ -58,25 +59,26 @@ export class PermissionsService {
     return isParatextRole(projectDoc.data?.userRoles[currentUserDoc.id] ?? SFProjectRole.None);
   }
 
-  async canAccessText(textDocId: TextDocId): Promise<boolean> {
-    // Get the project doc, if the user is on that project
-    let projectDoc: SFProjectProfileDoc | undefined;
-    if (textDocId.projectId != null) {
-      const isUserOnProject = await this.isUserOnProject(textDocId.projectId);
-      projectDoc = isUserOnProject ? await this.projectService.getProfile(textDocId.projectId) : undefined;
-    }
-
+  /**
+   * Determines if a user can access the text in the specified project.
+   * @param textDocId The text document id.
+   * @param project The project.
+   * @returns A boolean value.
+   */
+  canAccessText(textDocId?: TextDocId, project?: SFProjectProfile): boolean {
     // Ensure the user has project level permission to view the text
     if (
-      projectDoc?.data != null &&
-      SF_PROJECT_RIGHTS.hasRight(projectDoc.data, this.userService.currentUserId, SFProjectDomain.Texts, Operation.View)
+      textDocId != null &&
+      project != null &&
+      SF_PROJECT_RIGHTS.hasRight(project, this.userService.currentUserId, SFProjectDomain.Texts, Operation.View)
     ) {
       // Check chapter permissions
-      const chapter: Chapter | undefined = projectDoc.data.texts
-        .find(t => t.bookNum === textDocId.bookNum)
-        ?.chapters.find(c => c.number === textDocId.chapterNum);
-      if (chapter != null) {
-        const chapterPermission: string | undefined = chapter.permissions[this.userService.currentUserId];
+      const text: TextInfo | undefined = project.texts.find(t => t.bookNum === textDocId.bookNum);
+      const chapter: Chapter | undefined = text?.chapters.find(c => c.number === textDocId.chapterNum);
+      if (text != null && chapter != null) {
+        // If the chapter permission is not present, use the book permission instead
+        const chapterPermission: string | undefined =
+          chapter.permissions[this.userService.currentUserId] ?? text.permissions[this.userService.currentUserId];
         // If there is no chapter permission, they will have access to the chapter as they have access to the project.
         // We should only deny access if there is an explicit "None" permission.
         return chapterPermission !== TextInfoPermission.None;
@@ -86,6 +88,23 @@ export class PermissionsService {
     return false;
   }
 
+  /**
+   * Determines if a user can access a text.
+   * @param textDocId The text document id.
+   * @returns A promise for a boolean value.
+   */
+  async canAccessTextAsync(textDocId: TextDocId): Promise<boolean> {
+    // Get the project doc, if the user is on that project
+    let projectDoc: SFProjectProfileDoc | undefined;
+    if (textDocId.projectId != null) {
+      const isUserOnProject = await this.isUserOnProject(textDocId.projectId);
+      projectDoc = isUserOnProject ? await this.projectService.getProfile(textDocId.projectId) : undefined;
+      return this.canAccessText(textDocId, projectDoc?.data);
+    }
+
+    return false;
+  }
+
   canSync(projectDoc: SFProjectProfileDoc, userId?: string): boolean {
     if (projectDoc.data == null) {
       return false;

src/SIL.XForge.Scripture/ClientApp/src/app/core/text-doc.service.spec.ts

@@ -419,9 +419,10 @@ describe('TextDocService', () => {
       expect(actual).toBeUndefined();
     });
 
-    it('should return false if the user does not have the edit permission', () => {
+    it('should return false if the user does not have the write permission for the chapter', () => {
       const env = new TestEnvironment();
       const text: Partial<TextInfo> = {
+        permissions: { user01: TextInfoPermission.Write },
         chapters: [
           { number: 1, permissions: { user01: TextInfoPermission.Read }, lastVerse: 0, isValid: true } as Chapter
         ]
@@ -432,7 +433,7 @@ describe('TextDocService', () => {
       expect(actual).toBe(false);
     });
 
-    it('should return true if the user has the write permission', () => {
+    it('should return true if the user has the write permission for the chapter', () => {
       const env = new TestEnvironment();
       const text: Partial<TextInfo> = {
         chapters: [
@@ -444,6 +445,18 @@ describe('TextDocService', () => {
       const actual: boolean | undefined = env.textDocService.hasChapterEditPermissionForText(text as TextInfo, 1);
       expect(actual).toBe(true);
     });
+
+    it('should return true if the user has the write permission for the book and no chapter permission set', () => {
+      const env = new TestEnvironment();
+      const text: Partial<TextInfo> = {
+        permissions: { user01: TextInfoPermission.Write },
+        chapters: [{ number: 1, lastVerse: 0, isValid: true } as Chapter]
+      };
+
+      // SUT
+      const actual: boolean | undefined = env.textDocService.hasChapterEditPermissionForText(text as TextInfo, 1);
+      expect(actual).toBe(true);
+    });
   });
 });
 

src/SIL.XForge.Scripture/ClientApp/src/app/core/text-doc.service.ts

@@ -189,7 +189,8 @@ export class TextDocService {
     const chapter: Chapter | undefined = text?.chapters.find(c => c.number === chapterNum);
     // Even though permissions is guaranteed to be there in the model, its not in IndexedDB the first time the project
     // is accessed after migration
-    const permission: string | undefined = chapter?.permissions?.[this.userService.currentUserId];
+    const permission: string | undefined =
+      chapter?.permissions?.[this.userService.currentUserId] ?? text?.permissions?.[this.userService.currentUserId];
     return permission == null ? undefined : permission === TextInfoPermission.Write;
   }
 

src/SIL.XForge.Scripture/ClientApp/src/app/shared/cache-service/cache.service.spec.ts

@@ -24,7 +24,7 @@ describe('cache service', () => {
   describe('load all texts', () => {
     it('does not get texts from project service if no permission', fakeAsync(async () => {
       const env = new TestEnvironment();
-      when(mockedPermissionService.canAccessText(anything())).thenResolve(false);
+      when(mockedPermissionService.canAccessTextAsync(anything())).thenResolve(false);
       await env.service.cache(env.projectDoc);
       env.wait();
 
@@ -72,9 +72,9 @@ describe('cache service', () => {
 
     it('gets the source texts if they are present and the user can access', fakeAsync(async () => {
       const env = new TestEnvironment();
-      when(mockedPermissionService.canAccessText(deepEqual(new TextDocId('sourceId', 0, 0, 'target')))).thenResolve(
-        false
-      ); //remove access for one source doc
+      when(
+        mockedPermissionService.canAccessTextAsync(deepEqual(new TextDocId('sourceId', 0, 0, 'target')))
+      ).thenResolve(false); //remove access for one source doc
 
       await env.service.cache(env.projectDoc);
       env.wait();
@@ -106,7 +106,7 @@ class TestEnvironment {
     });
 
     when(mockedProjectDoc.data).thenReturn(data);
-    when(mockedPermissionService.canAccessText(anything())).thenResolve(true);
+    when(mockedPermissionService.canAccessTextAsync(anything())).thenResolve(true);
   }
 
   createTexts(): TextInfo[] {

src/SIL.XForge.Scripture/ClientApp/src/app/shared/cache-service/cache.service.ts

@@ -32,13 +32,13 @@ export class CacheService {
           }
 
           const textDocId = new TextDocId(project.id, text.bookNum, chapter.number, 'target');
-          if (await this.permissionsService.canAccessText(textDocId)) {
+          if (await this.permissionsService.canAccessTextAsync(textDocId)) {
             await this.projectService.getText(textDocId);
           }
 
           if (text.hasSource && sourceId != null) {
             const sourceTextDocId = new TextDocId(sourceId, text.bookNum, chapter.number, 'target');
-            if (await this.permissionsService.canAccessText(sourceTextDocId)) {
+            if (await this.permissionsService.canAccessTextAsync(sourceTextDocId)) {
               await this.projectService.getText(sourceTextDocId);
             }
           }

src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.spec.ts

@@ -139,7 +139,7 @@ describe('progress service', () => {
 
   it('cannot train suggestions if no source permission', fakeAsync(async () => {
     const env = new TestEnvironment();
-    when(mockPermissionService.canAccessText(anything())).thenCall((textDocId: TextDocId) => {
+    when(mockPermissionService.canAccessTextAsync(anything())).thenCall((textDocId: TextDocId) => {
       return Promise.resolve(textDocId.projectId !== 'sourceId');
     });
     tick();
@@ -192,7 +192,7 @@ class TestEnvironment {
     when(this.mockProject.id).thenReturn('project01');
     when(mockProjectService.changes$).thenReturn(this.project$);
 
-    when(mockPermissionService.canAccessText(anything())).thenResolve(true);
+    when(mockPermissionService.canAccessTextAsync(anything())).thenResolve(true);
     when(mockSFProjectService.getProfile('project01')).thenResolve({
       data,
       id: 'project01',

src/SIL.XForge.Scripture/ClientApp/src/app/shared/progress-service/progress.service.ts

@@ -304,7 +304,7 @@ export class ProgressService extends DataLoadingComponent implements OnDestroy {
 
         // Only retrieve the source text if the user has permission
         let sourceNonEmptyVerses: string[] = [];
-        if (await this.permissionsService.canAccessText(sourceTextDocId)) {
+        if (await this.permissionsService.canAccessTextAsync(sourceTextDocId)) {
           const sourceChapterText: TextDoc = await this.projectService.getText(sourceTextDocId);
           sourceNonEmptyVerses = sourceChapterText.getNonEmptyVerses();
         }

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.spec.ts

@@ -4920,6 +4920,7 @@ class TestEnvironment {
     when(mockedDraftGenerationService.getLastCompletedBuild(anything())).thenReturn(of({} as BuildDto));
     when(mockedDraftOptionsService.areFormattingOptionsAvailableButUnselected(anything())).thenReturn(false);
     when(mockedPermissionsService.isUserOnProject(anything())).thenResolve(true);
+    when(mockedPermissionsService.canAccessText(anything(), anything())).thenReturn(true);
     when(mockedFeatureFlagService.newDraftHistory).thenReturn(createTestFeatureFlag(false));
     when(mockedLynxWorkspaceService.rawInsightSource$).thenReturn(of([]));
 

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/editor.component.ts

@@ -58,7 +58,6 @@ import { isParatextRole, SFProjectRole } from 'realtime-server/lib/esm/scripture
 import { TextAnchor } from 'realtime-server/lib/esm/scriptureforge/models/text-anchor';
 import { TextType } from 'realtime-server/lib/esm/scriptureforge/models/text-data';
 import { Chapter, TextInfo } from 'realtime-server/lib/esm/scriptureforge/models/text-info';
-import { TextInfoPermission } from 'realtime-server/lib/esm/scriptureforge/models/text-info-permission';
 import { TranslateSource } from 'realtime-server/lib/esm/scriptureforge/models/translate-config';
 import { fromVerseRef } from 'realtime-server/lib/esm/scriptureforge/models/verse-ref-data';
 import { DeltaOperation } from 'rich-text';
@@ -324,7 +323,6 @@ export class EditorComponent extends DataLoadingComponent implements OnDestroy,
   private isParatextUserRole: boolean = false;
   private projectUserConfigChangesSub?: Subscription;
   private text?: TextInfo;
-  private sourceText?: TextInfo;
   sourceProjectDoc?: SFProjectProfileDoc;
   private _unsupportedTags = new Set<string>();
   private sourceLoaded: boolean = false;
@@ -535,20 +533,7 @@ export class EditorComponent extends DataLoadingComponent implements OnDestroy,
       return this.isParatextUserRole;
     }
 
-    if (
-      SF_PROJECT_RIGHTS.hasRight(sourceProject, this.userService.currentUserId, SFProjectDomain.Texts, Operation.View)
-    ) {
-      // Check for chapter rights
-      const chapter = this.sourceText?.chapters.find(c => c.number === this.chapter);
-      // Even though permissions is guaranteed to be there in the model, its not in IndexedDB the first time the project
-      // is accessed after migration
-      if (chapter != null && chapter.permissions != null && !this.isParatextUserRole) {
-        const chapterPermission: string = chapter.permissions[this.userService.currentUserId];
-        return chapterPermission === TextInfoPermission.Write || chapterPermission === TextInfoPermission.Read;
-      }
-    }
-
-    return this.isParatextUserRole;
+    return this.isParatextUserRole && this.permissionsService.canAccessText(this.source?.id, sourceProject);
   }
 
   get canInsertNote(): boolean {
@@ -846,10 +831,6 @@ export class EditorComponent extends DataLoadingComponent implements OnDestroy,
           return;
         }
 
-        if (this.sourceProjectDoc?.data != null) {
-          this.sourceText = this.sourceProjectDoc.data.texts.find(t => t.bookNum === bookNum);
-        }
-
         this.chapters = this.text.chapters.map(c => c.number);
 
         this.updateVerseNumber();

src/SIL.XForge.Scripture/ClientApp/src/app/translate/translate-overview/translate-overview.component.spec.ts

@@ -364,7 +364,9 @@ class TestEnvironment {
     const projectId: string = projectConfig.projectId ?? 'project01';
     const translationSuggestionsEnabled = projectConfig.translationSuggestionsEnabled ?? true;
     const textPermission: TextInfoPermission = projectConfig?.textPermission ?? TextInfoPermission.Write;
-    when(mockedPermissionService.canAccessText(anything())).thenResolve(textPermission !== TextInfoPermission.None);
+    when(mockedPermissionService.canAccessTextAsync(anything())).thenResolve(
+      textPermission !== TextInfoPermission.None
+    );
 
     // Setup the project data
     this.realtimeService.addSnapshot<SFProjectProfile>(SFProjectProfileDoc.COLLECTION, {

src/SIL.XForge.Scripture/Services/MachineApiService.cs

@@ -488,12 +488,13 @@ await hubContext.NotifyDraftApplyProgress(
                     continue;
                 }
 
-                bool canWriteChapter =
-                    targetProjectDoc
-                        .Data.Texts[textIndex]
-                        .Chapters[chapterIndex]
-                        .Permissions.TryGetValue(curUserId, out string chapterPermission)
-                    && chapterPermission == TextInfoPermission.Write;
+                // If a user does not have permission set at chapter level, use the book level permission
+                bool canWriteChapter = targetProjectDoc
+                    .Data.Texts[textIndex]
+                    .Chapters[chapterIndex]
+                    .Permissions.TryGetValue(curUserId, out string chapterPermission)
+                    ? chapterPermission == TextInfoPermission.Write
+                    : canWriteBook;
                 if (!canWriteChapter)
                 {
                     // Remove the chapter from the project if we created it, and proceed to add the next chapter

src/SIL.XForge.Scripture/Services/ParatextService.cs

@@ -876,7 +876,8 @@ public async Task<Dictionary<string, string>> GetPermissionsAsync(
                     || scrText!.Permissions.GetRole(userName) == UserRoles.None
                 )
                 {
-                    permissions.Add(uid, TextInfoPermission.None);
+                    // The user will either have read only access or no access
+                    // This will be handled by their role on the project (or lack of role) on the project
                 }
                 else
                 {
@@ -1775,11 +1776,13 @@ await projectDoc.SubmitJson0OpAsync(op =>
                             {
                                 int chapterIndex = j;
                                 int chapterNumber = text.Chapters[chapterIndex].Number;
-                                if (editableChapters.Contains(chapterNumber) || currentUserIsAdministrator)
+
+                                // Only record chapter level permissions that contradict the book level permissions
+                                if (!(editableChapters.Contains(chapterNumber) || currentUserIsAdministrator))
                                 {
                                     op.Set(
                                         p => p.Texts[textIndex].Chapters[chapterIndex].Permissions[user.SFUserId],
-                                        TextInfoPermission.Write
+                                        TextInfoPermission.Read
                                     );
                                 }
                             }