Merge pull request #199 from Normal-OJ/fix/permission-check-for-cours…

…e-scoreboard

fix: permission check for course scoreboard

model/course.py

@@ -1,3 +1,4 @@
+from typing import Optional
 from flask import Blueprint, request
 
 from mongo import *
@@ -188,7 +189,13 @@ def delete_score(title):
 @login_required
 @Request.args('pids: str', 'start', 'end')
 @Request.doc('course_name', 'course', Course)
-def get_course_scoreboard(user, pids, start, end, course: Course):
+def get_course_scoreboard(
+    user,
+    pids: str,
+    start: Optional[str],
+    end: Optional[str],
+    course: Course,
+):
     try:
         pids = pids.split(',')
         pids = [int(pid.strip()) for pid in pids]
@@ -206,7 +213,7 @@ def get_course_scoreboard(user, pids, start, end, course: Course):
         except:
             return HTTPError('Type of `end` should be float.', 400)
 
-    if course.permission(user, Course.Permission.GRADE):
+    if not course.permission(user, Course.Permission.GRADE):
         return HTTPError('Permission denied', 403)
 
     ret = course.get_scoreboard(pids, start, end)

tests/test_course.py

@@ -1,5 +1,7 @@
-import pytest
+from mongo import engine
+from tests.conftest import ForgeClient
 from tests.base_tester import BaseTester
+from tests import utils
 
 
 class TestAdminCourse(BaseTester):
@@ -345,3 +347,43 @@ def test_get_score_when_not_in_course(self, client_teacher):
         assert rv.status_code == 403
         json = rv.get_json()
         assert json['message'] == 'You are not in this course.'
+
+
+class TestScoreBoard(BaseTester):
+
+    def test_admin_can_view_scoreboard(self, forge_client: ForgeClient):
+        course = utils.course.create_course()
+        client = forge_client('first_admin')
+        rv = client.get(f'/course/{course.course_name}/scoreboard?pids=1,2,3')
+        assert rv.status_code == 200, rv.json
+
+    def test_teacher_can_view_scoreboard(self, forge_client: ForgeClient):
+        course = utils.course.create_course()
+        client = forge_client(course.teacher.username)
+        rv = client.get(f'/course/{course.course_name}/scoreboard?pids=1,2,3')
+        assert rv.status_code == 200, rv.json
+
+    def test_student_cannot_view_scoreboard(
+        self,
+        forge_client: ForgeClient,
+    ):
+        user = utils.user.create_user(role=engine.User.Role.STUDENT)
+        course = utils.course.create_course(students=[user])
+        client = forge_client(user.username)
+        rv = client.get(f'/course/{course.course_name}/scoreboard?pids=1,2,3')
+        assert rv.status_code == 403, rv.json
+
+    def test_teacher_role_cannot_view_scoreboard(
+        self,
+        forge_client: ForgeClient,
+    ):
+        '''
+        Users that has role 'teacher' but is not the teacher of that
+        course should not have permission to view scoreboard
+        '''
+        course = utils.course.create_course()
+        user = utils.user.create_user(role=engine.User.Role.TEACHER)
+        assert user != course.teacher
+        client = forge_client(user.username)
+        rv = client.get(f'/course/{course.course_name}/scoreboard?pids=1,2,3')
+        assert rv.status_code == 403, rv.json

tests/test_homework.py

@@ -1,41 +1,41 @@
 from datetime import datetime
+from dataclasses import dataclass, field
+from typing import Dict, List
 import pytest
 from flask.testing import FlaskClient
 from tests.base_tester import BaseTester, random_string
 from tests.conftest import ForgeClient
 from mongo import *
 
 
+@dataclass
 class CourseData:
-
-    def __init__(self, name, teacher, students, tas):
-        self.name = name
-        self.teacher = teacher
-        self.students = students
-        self.tas = tas
-        self.homework_ids = []
+    name: str
+    teacher: str
+    students: Dict[str, str]
+    tas: List[str]
+    homework_ids: List[str] = field(default_factory=list, init=False)
 
     @property
     def homework_name(self):
         return f'Test HW 4 {self.name} {id(self)}'
 
 
-@pytest.fixture(params=[{
-    'name': 'Programming_I',
-    'teacher': 'Po-Wen-Chi',
-    'students': {
-        'Yin-Da-Chen': 'ala',
-        'Bo-Chieh-Chuang': 'bogay'
-    },
-    'tas': ['Tzu-Wei-Yu']
-}])
+@pytest.fixture
 def course_data(
-    request,
     client_admin: FlaskClient,
     problem_ids,
 ):
     BaseTester.setup_class()
-    cd = CourseData(**request.param)
+    cd = CourseData(
+        name='Programming_I',
+        teacher='Po-Wen-Chi',
+        students={
+            'Yin-Da-Chen': 'ala',
+            'Bo-Chieh-Chuang': 'bogay'
+        },
+        tas=['Tzu-Wei-Yu'],
+    )
     # add course
     Course.add_course(cd.name, cd.teacher)
     # add students and TA