diff --git a/CHANGELOG.md b/CHANGELOG.md index 2986d30..5f332f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ and this project adheres to ## Unreleased +### Changed + +- Secure API endpoints with LTI token + ## [0.2.0] - 2024-05-23 ### Changed diff --git a/src/api/plugins/tdbp/tests/conftest.py b/src/api/plugins/tdbp/tests/conftest.py index ac9da74..36160e5 100644 --- a/src/api/plugins/tdbp/tests/conftest.py +++ b/src/api/plugins/tdbp/tests/conftest.py @@ -6,6 +6,7 @@ import pytest from warren.tests.fixtures.app import http_client from warren.tests.fixtures.asynchronous import anyio_backend +from warren.tests.fixtures.auth import auth_headers from warren.tests.fixtures.db import ( db_engine, db_session, diff --git a/src/api/plugins/tdbp/tests/test_api.py b/src/api/plugins/tdbp/tests/test_api.py index b5a6686..2559a1b 100644 --- a/src/api/plugins/tdbp/tests/test_api.py +++ b/src/api/plugins/tdbp/tests/test_api.py @@ -1,46 +1,44 @@ """Tests for the TdBP Warren plugin.""" -from datetime import date, datetime + +from datetime import datetime import httpx import pytest from pydantic import ValidationError +from warren.utils import LTIUser, forge_lti_token from warren_tdbp.models import Grades, Scores, SlidingWindow @pytest.mark.anyio -@pytest.mark.parametrize( - "query_params", - [ - {}, # Missing parameters - {"course_id": 1234}, # Wrong "course_id" parameter data type - { - "course_id": "course_id_101", - "until": "today", - }, # Wrong "until" parameter data type - {"until": date.today()}, # Missing "course_id" parameter - {"foo": "fake"}, # Unsupported parameter - ], -) async def test_api_sliding_window_with_invalid_params( - query_params, http_client: httpx.AsyncClient + http_client: httpx.AsyncClient, auth_headers: dict ): - """Test '/window' indicator with invalid query parameters returns 422 HTTP code.""" - response = await http_client.get("/api/v1/tdbp/window", params=query_params) + """Test `/window` endpoint with invalid query parameters returns 422 HTTP code.""" + query_params = {"until": "today"} + response = await http_client.get( + "/api/v1/tdbp/window", + params=query_params, + headers=auth_headers, + ) assert response.status_code == 422 @pytest.mark.anyio async def test_api_sliding_window_with_valid_params( - http_client: httpx.AsyncClient, db_session, sliding_window_fake_dataset + http_client: httpx.AsyncClient, + db_session, + sliding_window_fake_dataset, ): - """Test '/window' indicator with query parameters returns correct output.""" - course_id = "https://fake-lms.com/course/tdbp_101" + """Test `/window` endpoint with query parameters returns correct output.""" + token = forge_lti_token(course_id="https://fake-lms.com/course/tdbp_101") date_until = datetime.now().date() response = await http_client.get( - "/api/v1/tdbp/window", params={"course_id": course_id, "until": date_until} + "/api/v1/tdbp/window", + params={"until": date_until}, + headers={"Authorization": f"Bearer {token}"}, ) assert response.status_code == 200 @@ -52,38 +50,50 @@ async def test_api_sliding_window_with_valid_params( @pytest.mark.anyio -@pytest.mark.parametrize( - "query_params", - [ - {}, # Missing parameters - {"course_id": 1234}, # Wrong "course_id" parameter data type - { - "course_id": "course_id_101", - "until": "today", - }, # Wrong "until" parameter data type - {"until": date.today()}, # Missing "course_id" parameter - {"foo": "fake"}, # Unsupported parameter - ], -) +async def test_api_sliding_window_with_invalid_auth_headers( + http_client: httpx.AsyncClient, +): + """Test `/window` endpoint with an invalid `auth_headers`.""" + date_until = datetime.now().date() + response = await http_client.get( + "/api/v1/tdbp/window", + params={"until": date_until}, + headers={"Authorization": "Bearer Wrong_Token"}, + ) + + assert response.status_code == 401 + assert response.json().get("detail") == "Could not validate credentials" + + +@pytest.mark.anyio async def test_api_cohort_with_invalid_params( - query_params, http_client: httpx.AsyncClient + http_client: httpx.AsyncClient, auth_headers: dict ): - """Test '/cohort' indicator with invalid query parameters returns 422 HTTP code.""" - response = await http_client.get("/api/v1/tdbp/cohort", params=query_params) + """Test `/cohort` endpoint with invalid query parameters returns 422 HTTP code.""" + query_params = {"until": "today"} + response = await http_client.get( + "/api/v1/tdbp/cohort", + params=query_params, + headers=auth_headers, + ) assert response.status_code == 422 @pytest.mark.anyio async def test_api_cohort_with_valid_params( - http_client: httpx.AsyncClient, db_session, sliding_window_fake_dataset + http_client: httpx.AsyncClient, + db_session, + sliding_window_fake_dataset, ): - """Test '/window' indicator with query parameters returns correct output.""" - course_id = "https://fake-lms.com/course/tdbp_101" + """Test `/window` endpoint with query parameters returns correct output.""" + token = forge_lti_token(course_id="https://fake-lms.com/course/tdbp_101") date_until = datetime.now().date() response = await http_client.get( - "/api/v1/tdbp/cohort", params={"course_id": course_id, "until": date_until} + "/api/v1/tdbp/cohort", + params={"until": date_until}, + headers={"Authorization": f"Bearer {token}"}, ) assert response.status_code == 200 @@ -91,107 +101,232 @@ async def test_api_cohort_with_valid_params( assert len(response.json()) > 1 +@pytest.mark.anyio +async def test_api_cohort_with_invalid_auth_headers( + http_client: httpx.AsyncClient, +): + """Test `/cohort` endpoint with an invalid `auth_headers`.""" + date_until = datetime.now().date() + response = await http_client.get( + "/api/v1/tdbp/cohort", + params={"until": date_until}, + headers={"Authorization": "Bearer Wrong_Token"}, + ) + + assert response.status_code == 401 + assert response.json().get("detail") == "Could not validate credentials" + + @pytest.mark.anyio @pytest.mark.parametrize( "query_params", [ - {}, # Missing parameters - {"course_id": 1234}, # Wrong "course_id" parameter data type { - "course_id": "course_id_101", "until": "today", }, # Wrong "until" parameter data type { - "course_id": "course_id_101", - "student_id": 12345, - }, # Wrong "student_id" parameter data type - { - "course_id": "course_id_101", - "totals": 1, + "totals": 2, }, # Wrong "totals" parameter data type { - "course_id": "course_id_101", - "average": 1, + "average": 2, }, # Wrong "average" parameter data type - {"until": date.today()}, # Missing required "course_id" parameter - {"foo": "fake"}, # Unsupported parameter ], ) async def test_api_scores_with_invalid_params( - query_params, http_client: httpx.AsyncClient + query_params, http_client: httpx.AsyncClient, auth_headers ): - """Test '/scores' indicator with invalid query parameters returns 422 HTTP code.""" - response = await http_client.get("/api/v1/tdbp/scores", params=query_params) + """Test `/scores` endpoint with invalid query parameters returns 422 HTTP code.""" + response = await http_client.get( + "/api/v1/tdbp/scores", + params=query_params, + headers=auth_headers, + ) assert response.status_code == 422 @pytest.mark.anyio -async def test_api_scores_with_valid_params( - http_client: httpx.AsyncClient, db_session, sliding_window_fake_dataset +async def test_api_scores_with_valid_params_instructor( + http_client: httpx.AsyncClient, + db_session, + sliding_window_fake_dataset, ): - """Test '/scores' indicator with query parameters returns correct output.""" - course_id = "https://fake-lms.com/course/tdbp_101" + """Test `/scores` endpoint for an instructor returns correct output.""" + token = forge_lti_token( + roles=("instructor",), + course_id="https://fake-lms.com/course/tdbp_101", + ) date_until = datetime.now().date() response = await http_client.get( - "/api/v1/tdbp/scores", params={"course_id": course_id, "until": date_until} + "/api/v1/tdbp/scores", + params={"until": date_until, "average": True, "totals": True}, + headers={"Authorization": f"Bearer {token}"}, ) assert response.status_code == 200 try: - Scores.parse_obj(response.json()) + scores = Scores.parse_obj(response.json()) except ValidationError as err: pytest.fail(f"Scores indicator is invalid: {err}") + # Instructors can see average and totals scores + assert len(scores.scores) > 1 + assert scores.average + assert scores.total + + +@pytest.mark.anyio +async def test_api_scores_with_valid_params_student( + http_client: httpx.AsyncClient, + db_session, + sliding_window_fake_dataset, +): + """Test `/scores` endpoint for a student returns correct output.""" + token = forge_lti_token( + user=LTIUser( + id="johndoe", + email="johndoe@example.com", + ), + roles=("student",), + course_id="https://fake-lms.com/course/tdbp_101", + ) + date_until = datetime.now().date() + + response = await http_client.get( + "/api/v1/tdbp/scores", + params={"until": date_until, "average": 1, "total": 1}, + headers={"Authorization": f"Bearer {token}"}, + ) + + assert response.status_code == 200 + + try: + scores = Scores.parse_obj(response.json()) + except ValidationError as err: + pytest.fail(f"Scores indicator is invalid: {err}") + + # Student can only see its scores, and cannot see total and average scores + scores.scores.pop("johndoe") + assert not scores.scores + assert not scores.average + assert not scores.total + + +@pytest.mark.anyio +async def test_api_scores_with_invalid_auth_headers( + http_client: httpx.AsyncClient, +): + """Test `/scores` endpoint with an invalid `auth_headers`.""" + date_until = datetime.now().date() + response = await http_client.get( + "/api/v1/tdbp/scores", + params={"until": date_until}, + headers={"Authorization": "Bearer Wrong_Token"}, + ) + + assert response.status_code == 401 + assert response.json().get("detail") == "Could not validate credentials" + @pytest.mark.anyio @pytest.mark.parametrize( "query_params", [ - {}, # Missing parameters - {"course_id": 1234}, # Wrong "course_id" parameter data type { - "course_id": "course_id_101", "until": "today", }, # Wrong "until" parameter data type { - "course_id": "course_id_101", - "student_id": 12345, - }, # Wrong "student_id" parameter data type - { - "course_id": "course_id_101", - "average": 1, + "average": 2, }, # Wrong "average" parameter data type - {"until": date.today()}, # Missing required "course_id" parameter - {"foo": "fake"}, # Unsupported parameter ], ) async def test_api_grades_with_invalid_params( - query_params, http_client: httpx.AsyncClient + query_params, http_client: httpx.AsyncClient, auth_headers: dict ): - """Test '/grades' indicator with invalid query parameters returns 422 HTTP code.""" - response = await http_client.get("/api/v1/tdbp/grades", params=query_params) + """Test `/grades` endpoint with invalid query parameters returns 422 HTTP code.""" + response = await http_client.get( + "/api/v1/tdbp/grades", + params=query_params, + headers=auth_headers, + ) assert response.status_code == 422 @pytest.mark.anyio -async def test_api_grades_with_valid_params( - http_client: httpx.AsyncClient, db_session, sliding_window_fake_dataset +async def test_api_grades_with_valid_params_instructor( + http_client: httpx.AsyncClient, + db_session, + sliding_window_fake_dataset, ): - """Test '/grades' indicator with query parameters returns correct output.""" - course_id = "https://fake-lms.com/course/tdbp_101" + """Test `/grades` endpoint for an instructor returns correct output.""" + token = forge_lti_token( + roles=("instructor",), + course_id="https://fake-lms.com/course/tdbp_101", + ) date_until = datetime.now().date() response = await http_client.get( - "/api/v1/tdbp/grades", params={"course_id": course_id, "until": date_until} + "/api/v1/tdbp/grades", + params={"until": date_until, "average": True}, + headers={"Authorization": f"Bearer {token}"}, ) assert response.status_code == 200 try: - Grades.parse_obj(response.json()) + grades = Grades.parse_obj(response.json()) except ValidationError as err: pytest.fail(f"Grades indicator is invalid: {err}") + + assert len(grades.grades) > 1 + assert grades.average + + +@pytest.mark.anyio +async def test_api_grades_with_valid_params_student( + http_client: httpx.AsyncClient, + db_session, + sliding_window_fake_dataset, +): + """Test `/grades` endpoint for a student returns correct output.""" + token = forge_lti_token( + user=LTIUser(id="student_1", email="student_1@example.com"), + roles=("student",), + course_id="https://fake-lms.com/course/tdbp_101", + ) + date_until = datetime.now().date() + + response = await http_client.get( + "/api/v1/tdbp/grades", + params={"until": date_until}, + headers={"Authorization": f"Bearer {token}"}, + ) + + assert response.status_code == 200 + + try: + grades = Grades.parse_obj(response.json()) + except ValidationError as err: + pytest.fail(f"Grades indicator is invalid: {err}") + + # Student can only see its grades + assert len(grades.grades) == 1 + + +@pytest.mark.anyio +async def test_api_grades_with_invalid_auth_headers( + http_client: httpx.AsyncClient, +): + """Test `/grades` endpoint with an invalid `auth_headers`.""" + date_until = datetime.now().date() + response = await http_client.get( + "/api/v1/tdbp/grades", + params={"until": date_until}, + headers={"Authorization": "Bearer Wrong_Token"}, + ) + + assert response.status_code == 401 + assert response.json().get("detail") == "Could not validate credentials" diff --git a/src/api/plugins/tdbp/warren_tdbp/api.py b/src/api/plugins/tdbp/warren_tdbp/api.py index f9a32ee..a2b8cbd 100644 --- a/src/api/plugins/tdbp/warren_tdbp/api.py +++ b/src/api/plugins/tdbp/warren_tdbp/api.py @@ -2,10 +2,12 @@ import logging from datetime import date -from typing import Optional +from typing import Annotated, List, Optional -from fastapi import APIRouter, HTTPException, Query, status +from fastapi import APIRouter, Depends, HTTPException, Query, status +from lti_toolbox.launch_params import LTIRole from warren.exceptions import LrsClientException +from warren.utils import get_lti_course_id, get_lti_roles, get_lti_user_id from .indicators import ( CohortIndicator, @@ -13,6 +15,7 @@ ScoresIndicator, SlidingWindowIndicator, ) +from .models import Grades, Scores, SlidingWindow router = APIRouter( prefix="/tdbp", @@ -23,13 +26,14 @@ @router.get("/window") async def get_sliding_window( - course_id: str = Query( - description="The course identifier on Moodle", - ), - until: Optional[date] = Query( - description="End date until when to compute the sliding window", - ), -): + course_id: Annotated[str, Depends(get_lti_course_id)], + until: Annotated[ + Optional[date], + Query( + description="End date until when to compute the sliding window", + ), + ] = None, +) -> SlidingWindow: """Return course sliding window indicator. Args: @@ -61,12 +65,11 @@ async def get_sliding_window( @router.get("/cohort") async def get_cohort( - course_id: str = Query( - description="The course identifier on Moodle", - ), - until: Optional[date] = Query( - description="End date until when to compute the sliding window", - ), + course_id: Annotated[str, Depends(get_lti_course_id)], + until: Annotated[ + Optional[date], + Query(description="End date until when to compute the sliding window"), + ] = None, ): """Return course (static) cohort information. @@ -95,27 +98,30 @@ async def get_cohort( @router.get("/scores") -async def get_scores( - course_id: str = Query( - description="The course identifier on Moodle", - ), - until: Optional[date] = Query( - description="End date until when to compute the sliding window", - ), - student_id: Optional[str] = Query( - None, description="The student identifier on Moodle" - ), - totals: bool = Query(False, description="Flag to activate to compute total scores"), - average: bool = Query( - False, description="Flag to activate to compute average scores" - ), -): +async def get_scores( # noqa: PLR0913 + course_id: Annotated[str, Depends(get_lti_course_id)], + roles: Annotated[List[LTIRole], Depends(get_lti_roles)], + user_id: Annotated[str, Depends(get_lti_user_id)], + until: Annotated[ + Optional[date], + Query( + description="End date until when to compute the sliding window", + ), + ] = None, + totals: Annotated[ + bool, Query(description="Flag to activate to compute total scores") + ] = False, + average: Annotated[ + bool, Query(description="Flag to activate to compute average scores") + ] = False, +) -> Scores: """Return student or cohort scores on active actions. Args: course_id (str): The course identifier on Moodle. + roles (LTIRole): The roles of the user. + user_id (str): The user identifier on Moodle. until (datetime): End date until when to compute the sliding window. - student_id (str): The student identifier on Moodle. totals (bool): Flag to activate cohort totals scores computing on active actions. average (bool): Flag to activate cohort average scores for computing on active @@ -129,6 +135,12 @@ async def get_scores( - average (list): Average students' score per active action. """ logger.debug("Start computing 'scores' indicator") + # Instructors can see scores of all students, the totals and average + if any(role in ["instructor", "teacher", "staff"] for role in roles): + student_id = None + # Students can only see their scores + else: + student_id = user_id indicator = ScoresIndicator( course_id=course_id, until=until, @@ -152,25 +164,26 @@ async def get_scores( @router.get("/grades") async def get_grades( - course_id: str = Query( - description="The course identifier on Moodle", - ), - until: Optional[date] = Query( - description="End date until when to compute the sliding window", - ), - student_id: Optional[str] = Query( - None, description="The student identifier on Moodle" - ), - average: bool = Query( - False, description="Flag to activate to compute average grades" - ), -): + course_id: Annotated[str, Depends(get_lti_course_id)], + roles: Annotated[List[LTIRole], Depends(get_lti_roles)], + user_id: Annotated[str, Depends(get_lti_user_id)], + until: Annotated[ + Optional[date], + Query( + description="End date until when to compute the sliding window", + ), + ] = None, + average: Annotated[ + bool, Query(description="Flag to activate to compute average grades") + ] = False, +) -> Grades: """Return average mark for graded active activities. Args: course_id (str): The course identifier on Moodle. + roles (list): The roles of the user. + user_id (str): The user identifier on Moodle. until (datetime): End date until when to compute the sliding window. - student_id (str): The student identifier on Moodle. average (bool): Flag to activate average grade computing on each graded active activity. @@ -181,6 +194,12 @@ async def get_grades( - average (list): Average students' grades per active activity. """ logger.debug("Start computing 'grades' indicator") + # Instructors can see scores of all students, the totals and average + if any(role in ["instructor", "teacher", "staff"] for role in roles): + student_id = None + # Students can only see their scores + else: + student_id = user_id indicator = GradesIndicator( course_id=course_id, until=until, student_id=student_id, average=average )