[#11099] Audit logs: restrict viewing and searching permissions to co…

…urse owners (#11112) * Add guard clause in backend * Add error in frontend * Fix lint * Filter courses by privilege * Add test for course without priveleges * Fix lint * Add modify session and modify instructor permissions * Fix test * Change testcase name Co-authored-by: Adithya Narayan Rangarajan Sreenivasan <e0426343@u.nus.edu>
TEAMMATES · Jun 5, 2021 · 99f62a5 · 99f62a5
1 parent 1123653
commit 99f62a5
Show file tree

Hide file tree

Showing 6 changed files with 87 additions and 12 deletions.
diff --git a/src/main/java/teammates/ui/webapi/GetFeedbackSessionLogsAction.java b/src/main/java/teammates/ui/webapi/GetFeedbackSessionLogsAction.java
@@ -44,7 +44,9 @@ void checkSpecificAccessControl() throws UnauthorizedAccessException {
         }
 
         InstructorAttributes instructor = logic.getInstructorForGoogleId(courseId, userInfo.getId());
-        gateKeeper.verifyAccessible(instructor, courseAttributes);
+        gateKeeper.verifyAccessible(instructor, courseAttributes, Const.InstructorPermissions.CAN_MODIFY_STUDENT);
+        gateKeeper.verifyAccessible(instructor, courseAttributes, Const.InstructorPermissions.CAN_MODIFY_SESSION);
+        gateKeeper.verifyAccessible(instructor, courseAttributes, Const.InstructorPermissions.CAN_MODIFY_INSTRUCTOR);
     }
 
     @Override

diff --git a/src/test/java/teammates/ui/webapi/GetFeedbackSessionLogsActionTest.java b/src/test/java/teammates/ui/webapi/GetFeedbackSessionLogsActionTest.java
@@ -170,14 +170,28 @@ protected void testExecute() throws Exception {
     @Test
     @Override
     protected void testAccessControl() throws Exception {
-        InstructorAttributes instructor = typicalBundle.instructors.get("instructor1OfCourse1");
+        InstructorAttributes instructor = typicalBundle.instructors.get("instructor2OfCourse1");
+        InstructorAttributes helper = typicalBundle.instructors.get("helperOfCourse1");
         String courseId = instructor.getCourseId();
 
         ______TS("Only instructors of the same course can access");
         String[] submissionParams = new String[] {
                 Const.ParamsNames.COURSE_ID, courseId,
         };
         verifyOnlyInstructorsOfTheSameCourseCanAccess(submissionParams);
+
+        ______TS("Only instructors with modify student, session and instructor privilege can access");
+        submissionParams = new String[] {
+                Const.ParamsNames.COURSE_ID, courseId,
+        };
+
+        verifyCannotAccess(submissionParams);
+
+        loginAsInstructor(helper.googleId);
+        verifyCannotAccess(submissionParams);
+
+        loginAsInstructor(instructor.googleId);
+        verifyCanAccess(submissionParams);
     }
 
 }
diff --git a/...nstructor-audit-logs-page/__snapshots__/instructor-audit-logs-page.component.spec.ts.snap b/...nstructor-audit-logs-page/__snapshots__/instructor-audit-logs-page.component.spec.ts.snap
@@ -21,7 +21,11 @@ exports[`InstructorAuditLogsPageComponent should snap when page is still loading
   <h1>
     Student Activity Logs
   </h1><p>
-    This page allows you to find when your students have accessed or submitted a particular feedback session for a given course. The earliest date you can search for is 
+    This page allows you to find when your students have accessed or submitted a particular feedback session for a given course. Note that you can only view student logs if you have 
+    <b>
+      owner or manager privileges
+    </b>
+     for the course. The earliest date you can search for is 
     <b>
       30 days
     </b>
@@ -72,7 +76,11 @@ exports[`InstructorAuditLogsPageComponent should snap when searching for details
   <h1>
     Student Activity Logs
   </h1><p>
-    This page allows you to find when your students have accessed or submitted a particular feedback session for a given course. The earliest date you can search for is 
+    This page allows you to find when your students have accessed or submitted a particular feedback session for a given course. Note that you can only view student logs if you have 
+    <b>
+      owner or manager privileges
+    </b>
+     for the course. The earliest date you can search for is 
     <b>
       30 days
     </b>
@@ -552,7 +560,11 @@ exports[`InstructorAuditLogsPageComponent should snap with default fields 1`] =
   <h1>
     Student Activity Logs
   </h1><p>
-    This page allows you to find when your students have accessed or submitted a particular feedback session for a given course. The earliest date you can search for is 
+    This page allows you to find when your students have accessed or submitted a particular feedback session for a given course. Note that you can only view student logs if you have 
+    <b>
+      owner or manager privileges
+    </b>
+     for the course. The earliest date you can search for is 
     <b>
       30 days
     </b>
@@ -603,7 +615,11 @@ exports[`InstructorAuditLogsPageComponent should snap with results of a search 1
   <h1>
     Student Activity Logs
   </h1><p>
-    This page allows you to find when your students have accessed or submitted a particular feedback session for a given course. The earliest date you can search for is 
+    This page allows you to find when your students have accessed or submitted a particular feedback session for a given course. Note that you can only view student logs if you have 
+    <b>
+      owner or manager privileges
+    </b>
+     for the course. The earliest date you can search for is 
     <b>
       30 days
     </b>

diff --git a/...app/pages-instructor/instructor-audit-logs-page/instructor-audit-logs-page.component.html b/...app/pages-instructor/instructor-audit-logs-page/instructor-audit-logs-page.component.html
@@ -1,5 +1,5 @@
 <h1>Student Activity Logs</h1>
-<p>This page allows you to find when your students have accessed or submitted a particular feedback session for a given course. The earliest date you can search for is <b>{{this.LOGS_RETENTION_PERIOD}} days</b> before today.</p>
+<p>This page allows you to find when your students have accessed or submitted a particular feedback session for a given course. Note that you can only view student logs if you have <b>owner or manager privileges</b> for the course. The earliest date you can search for is <b>{{this.LOGS_RETENTION_PERIOD}} days</b> before today.</p>
 <hr/>
 
 <div *tmIsLoading="isLoading" class="card bg-form no-border">

diff --git a/.../pages-instructor/instructor-audit-logs-page/instructor-audit-logs-page.component.spec.ts b/.../pages-instructor/instructor-audit-logs-page/instructor-audit-logs-page.component.spec.ts
@@ -45,13 +45,50 @@ describe('InstructorAuditLogsPageComponent', () => {
     timeZone: 'Asia/Singapore',
     creationTimestamp: 0,
     deletionTimestamp: 0,
+    privileges: {
+      canModifyCourse: true,
+      canModifySession: true,
+      canModifyStudent: true,
+      canModifyInstructor: true,
+      canViewStudentInSections: true,
+      canModifySessionCommentsInSections: true,
+      canViewSessionInSections: true,
+      canSubmitSessionInSections: true,
+    },
   };
   const testCourse2: Course = {
     courseId: 'MA1234',
     courseName: 'MA1234',
     timeZone: 'Asia/Singapore',
     creationTimestamp: 0,
     deletionTimestamp: 0,
+    privileges: {
+      canModifyCourse: true,
+      canModifySession: true,
+      canModifyStudent: true,
+      canModifyInstructor: true,
+      canViewStudentInSections: true,
+      canModifySessionCommentsInSections: true,
+      canViewSessionInSections: true,
+      canSubmitSessionInSections: true,
+    },
+  };
+  const testCourse3: Course = {
+    courseId: 'EE1111',
+    courseName: 'EE1111',
+    timeZone: 'Asia/Singapore',
+    creationTimestamp: 0,
+    deletionTimestamp: 0,
+    privileges: {
+      canModifyCourse: false,
+      canModifySession: false,
+      canModifyStudent: false,
+      canModifyInstructor: false,
+      canViewStudentInSections: true,
+      canModifySessionCommentsInSections: true,
+      canViewSessionInSections: true,
+      canSubmitSessionInSections: true,
+    },
   };
   const emptyStudent: Student = {
     courseId: '', email: '', name: '', sectionName: '', teamName: '',
@@ -184,7 +221,7 @@ describe('InstructorAuditLogsPageComponent', () => {
     const courseSpy: Spy = spyOn(courseService, 'getAllCoursesAsInstructor').and
         .returnValue(of({
           courses: [
-            testCourse1, testCourse2,
+            testCourse1, testCourse2, testCourse3,
           ],
         }));
     const studentSpy: Spy = spyOn(studentService, 'getStudentsFromCourse').and
@@ -199,6 +236,7 @@ describe('InstructorAuditLogsPageComponent', () => {
     expect(component.isLoading).toBeFalsy();
     expect(courseSpy).toBeCalledWith('active');
     expect(component.courses.length).toEqual(2);
+    expect(component.courses).not.toContainEqual(testCourse3);
     expect(component.courseToStudents[testCourse1.courseId][0]).toEqual(emptyStudent);
     expect(component.courseToStudents[testCourse1.courseId][1]).toEqual(testStudent);
     expect(studentSpy).toHaveBeenNthCalledWith(1, { courseId: testCourse1.courseId });

diff --git a/...b/app/pages-instructor/instructor-audit-logs-page/instructor-audit-logs-page.component.ts b/...b/app/pages-instructor/instructor-audit-logs-page/instructor-audit-logs-page.component.ts
@@ -139,10 +139,15 @@ export class InstructorAuditLogsPageComponent implements OnInit {
     this.courseService
         .getAllCoursesAsInstructor('active')
         .pipe(
-            concatMap((courses: Courses) => courses.courses.map((course: Course) => {
-              this.courses.push(course);
-              return this.studentService.getStudentsFromCourse({ courseId: course.courseId });
-            })),
+            concatMap((courses: Courses) => courses.courses
+                .filter((course: Course) =>
+                    course.privileges?.canModifyStudent
+                    && course.privileges?.canModifySession
+                    && course.privileges?.canModifySession)
+                .map((course: Course) => {
+                  this.courses.push(course);
+                  return this.studentService.getStudentsFromCourse({ courseId: course.courseId });
+                })),
             mergeAll(),
             finalize(() => this.isLoading = false))
         .subscribe(((student: Students) =>