diff --git a/backend/db_construct.sql b/backend/db_construct.sql index bb7c7eb7..e3f6af41 100644 --- a/backend/db_construct.sql +++ b/backend/db_construct.sql @@ -1,9 +1,10 @@ +CREATE TYPE role AS ENUM ('STUDENT', 'TEACHER', 'ADMIN'); + CREATE TYPE submission_status AS ENUM ('SUCCESS', 'LATE', 'FAIL', 'RUNNING'); CREATE TABLE users ( uid VARCHAR(255), - is_teacher BOOLEAN, - is_admin BOOLEAN, + role role NOT NULL, PRIMARY KEY(uid) ); diff --git a/backend/project/endpoints/index/OpenAPI_Object.json b/backend/project/endpoints/index/OpenAPI_Object.json index 829f7c38..5ac3ef53 100644 --- a/backend/project/endpoints/index/OpenAPI_Object.json +++ b/backend/project/endpoints/index/OpenAPI_Object.json @@ -1374,14 +1374,11 @@ "uid": { "type": "string" }, - "is_teacher": { - "type": "boolean" - }, - "is_admin": { - "type": "boolean" + "role": { + "type": "enum" } }, - "required": ["uid", "is_teacher", "is_admin"] + "required": ["uid", "role"] } } } @@ -1399,14 +1396,11 @@ "uid": { "type": "string" }, - "is_teacher": { - "type": "boolean" - }, - "is_admin": { - "type": "boolean" + "role": { + "type": "enum" } }, - "required": ["uid", "is_teacher", "is_admin"] + "required": ["uid", "role"] } } } @@ -1451,14 +1445,11 @@ "uid": { "type": "string" }, - "is_teacher": { - "type": "boolean" - }, - "is_admin": { - "type": "boolean" + "role": { + "type": "enum" } }, - "required": ["uid", "is_teacher", "is_admin"] + "required": ["uid", "role"] } } } @@ -1487,14 +1478,11 @@ "schema": { "type": "object", "properties": { - "is_teacher": { - "type": "boolean" - }, - "is_admin": { - "type": "boolean" + "role": { + "type": "role" } }, - "required": ["is_teacher", "is_admin"] + "required": ["role"] } } } diff --git a/backend/project/endpoints/users.py b/backend/project/endpoints/users.py index 7d073c6c..34e65817 100644 --- a/backend/project/endpoints/users.py +++ b/backend/project/endpoints/users.py @@ -7,7 +7,7 @@ from sqlalchemy.exc import SQLAlchemyError from project import db -from project.models.user import User as userModel +from project.models.user import User as userModel, Role from project.utils.authentication import login_required, authorize_user, \ authorize_admin, not_allowed @@ -29,16 +29,13 @@ def get(self): """ try: query = userModel.query - is_teacher = request.args.get('is_teacher') - is_admin = request.args.get('is_admin') - - if is_teacher is not None: - query = query.filter(userModel.is_teacher == (is_teacher.lower() == 'true')) - - if is_admin is not None: - query = query.filter(userModel.is_admin == (is_admin.lower() == 'true')) + role = request.args.get("role") + if role is not None: + role = Role[role.upper()] + query = query.filter(userModel.role == role) users = query.all() + users = [user.to_dict() for user in users] result = jsonify({"message": "Queried all users", "data": users, "url":f"{API_URL}/users", "status_code": 200}) @@ -54,26 +51,25 @@ def post(self): It should create a new user and return a success message. """ uid = request.json.get('uid') - is_teacher = request.json.get('is_teacher') - is_admin = request.json.get('is_admin') + role = request.json.get("role") + role = Role[role.upper()] if role is not None else None url = f"{API_URL}/users" - if is_teacher is None or is_admin is None or uid is None: + if role is None or uid is None: return { "message": "Invalid request data!", "correct_format": { "uid": "User ID (string)", - "is_teacher": "Teacher status (boolean)", - "is_admin": "Admin status (boolean)" - },"url": url + "role": "User role (string)" + },"url": f"{API_URL}/users" }, 400 try: user = db.session.get(userModel, uid) if user is not None: # Bad request, error code could be 409 but is rarely used return {"message": f"User {uid} already exists"}, 400 - # Code to create a new user in the database using the uid, is_teacher, and is_admin - new_user = userModel(uid=uid, is_teacher=is_teacher, is_admin=is_admin) + # Code to create a new user in the database using the uid and role + new_user = userModel(uid=uid, role=role) db.session.add(new_user) db.session.commit() return jsonify({"message": "User created successfully!", @@ -99,7 +95,7 @@ def get(self, user_id): if user is None: return {"message": "User not found!","url": f"{API_URL}/users"}, 404 - return jsonify({"message": "User queried","data":user, + return jsonify({"message": "User queried","data":user.to_dict(), "url": f"{API_URL}/users/{user.uid}", "status_code": 200}) except SQLAlchemyError: return {"message": "An error occurred while fetching the user", @@ -114,22 +110,21 @@ def patch(self, user_id): dict: A dictionary containing the message indicating the success or failure of the update. """ - is_teacher = request.json.get('is_teacher') - is_admin = request.json.get('is_admin') + role = request.json.get("role") + role = Role[role.upper()] if role is not None else None try: user = db.session.get(userModel, user_id) if user is None: return {"message": "User not found!","url": f"{API_URL}/users"}, 404 - if is_teacher is not None: - user.is_teacher = is_teacher - if is_admin is not None: - user.is_admin = is_admin + if role is not None: + user.role = role # Save the changes to the database db.session.commit() return jsonify({"message": "User updated successfully!", - "data": user, "url": f"{API_URL}/users/{user.uid}", "status_code": 200}) + "data": user.to_dict(), + "url": f"{API_URL}/users/{user.uid}", "status_code": 200}) except SQLAlchemyError: # every exception should result in a rollback db.session.rollback() diff --git a/backend/project/models/user.py b/backend/project/models/user.py index bb130349..7cd59fd1 100644 --- a/backend/project/models/user.py +++ b/backend/project/models/user.py @@ -1,17 +1,30 @@ """User model""" +from enum import Enum from dataclasses import dataclass -from sqlalchemy import Boolean, Column, String +from sqlalchemy import Column, String, Enum as EnumField from project.db_in import db +class Role(Enum): + """This class defines the roles of a user""" + STUDENT = 0 + TEACHER = 1 + ADMIN = 2 + @dataclass class User(db.Model): """This class defines the users table, - a user has a uid, - is_teacher and is_admin booleans because a user + a user has a uid and a role, a user can be either a student,admin or teacher""" __tablename__ = "users" uid: str = Column(String(255), primary_key=True) - is_teacher: bool = Column(Boolean) - is_admin: bool = Column(Boolean) + role: Role = Column(EnumField(Role), nullable=False) + def to_dict(self): + """ + Converts a User to a serializable dict + """ + return { + 'uid': self.uid, + 'role': self.role.name, # Convert the enum to a string + } diff --git a/backend/project/utils/authentication.py b/backend/project/utils/authentication.py index 61b64e61..c1a96248 100644 --- a/backend/project/utils/authentication.py +++ b/backend/project/utils/authentication.py @@ -13,7 +13,7 @@ from project import db -from project.models.user import User +from project.models.user import User, Role from project.utils.models.course_utils import is_admin_of_course, \ is_student_of_course, is_teacher_of_course from project.utils.models.project_utils import get_course_of_project, project_visible @@ -29,7 +29,7 @@ def not_allowed(f): """Decorator function to immediately abort the current request and return 403: Forbidden""" @wraps(f) def wrap(*args, **kwargs): - return {"message":"Forbidden action"}, 403 + return {"message": "Forbidden action"}, 403 return wrap @@ -39,20 +39,23 @@ def return_authenticated_user_id(): """ authentication = request.headers.get("Authorization") if not authentication: - abort(make_response(({"message": - "No authorization given, you need an access token to use this API"} - , 401))) + abort( + make_response(( + {"message": + "No authorization given, you need an access token to use this API"}, + 401))) auth_header = {"Authorization": authentication} try: - response = requests.get(AUTHENTICATION_URL, headers=auth_header, timeout=5) + response = requests.get( + AUTHENTICATION_URL, headers=auth_header, timeout=5) except TimeoutError: - abort(make_response(({"message":"Request to Microsoft timed out"} - , 500))) + abort(make_response( + ({"message": "Request to Microsoft timed out"}, 500))) if not response or response.status_code != 200: abort(make_response(({"message": - "An error occured while trying to authenticate your access token"} - , 401))) + "An error occured while trying to authenticate your access token"}, + 401))) user_info = response.json() auth_user_id = user_info["id"] @@ -61,26 +64,30 @@ def return_authenticated_user_id(): except SQLAlchemyError: db.session.rollback() abort(make_response(({"message": - "An unexpected database error occured while fetching the user"}, 500))) + "An unexpected database error occured while fetching the user"}, + 500))) if user: return auth_user_id - is_teacher = False + + # Use the Enum here + role = Role.STUDENT if user_info["jobTitle"] is not None: - is_teacher = True + role = Role.TEACHER # add user if not yet in database try: - new_user = User(uid=auth_user_id, is_teacher=is_teacher, is_admin=False) + new_user = User(uid=auth_user_id, role=role) db.session.add(new_user) db.session.commit() except SQLAlchemyError: db.session.rollback() abort(make_response(({"message": - """An unexpected database error occured + """An unexpected database error occured while creating the user during authentication"""}, 500))) return auth_user_id + def login_required(f): """ This function will check if the person sending a request to the API is logged in @@ -104,7 +111,7 @@ def wrap(*args, **kwargs): if is_admin(auth_user_id): return f(*args, **kwargs) abort(make_response(({"message": - """You are not authorized to perfom this action, + """You are not authorized to perfom this action, only admins are authorized"""}, 403))) return wrap @@ -121,7 +128,7 @@ def wrap(*args, **kwargs): kwargs["teacher_id"] = auth_user_id return f(*args, **kwargs) abort(make_response(({"message": - """You are not authorized to perfom this action, + """You are not authorized to perfom this action, only teachers are authorized"""}, 403))) return wrap @@ -138,7 +145,8 @@ def wrap(*args, **kwargs): if is_teacher_of_course(auth_user_id, kwargs["course_id"]): return f(*args, **kwargs) - abort(make_response(({"message":"You're not authorized to perform this action"}, 403))) + abort(make_response( + ({"message": "You're not authorized to perform this action"}, 403))) return wrap @@ -153,10 +161,10 @@ def wrap(*args, **kwargs): auth_user_id = return_authenticated_user_id() course_id = kwargs["course_id"] if (is_teacher_of_course(auth_user_id, course_id) - or is_admin_of_course(auth_user_id, course_id)): + or is_admin_of_course(auth_user_id, course_id)): return f(*args, **kwargs) - abort(make_response(({"message":"""You are not authorized to perfom this action, + abort(make_response(({"message": """You are not authorized to perfom this action, only teachers and course admins are authorized"""}, 403))) return wrap @@ -174,7 +182,7 @@ def wrap(*args, **kwargs): if auth_user_id == user_id: return f(*args, **kwargs) - abort(make_response(({"message":"""You are not authorized to perfom this action, + abort(make_response(({"message": """You are not authorized to perfom this action, you are not this user"""}, 403))) return wrap @@ -194,7 +202,7 @@ def wrap(*args, **kwargs): if is_teacher_of_course(auth_user_id, course_id): return f(*args, **kwargs) - abort(make_response(({"message":"""You are not authorized to perfom this action, + abort(make_response(({"message": """You are not authorized to perfom this action, you are not the teacher of this project"""}, 403))) return wrap @@ -211,9 +219,9 @@ def wrap(*args, **kwargs): project_id = kwargs["project_id"] course_id = get_course_of_project(project_id) if (is_teacher_of_course(auth_user_id, course_id) - or is_admin_of_course(auth_user_id, course_id)): + or is_admin_of_course(auth_user_id, course_id)): return f(*args, **kwargs) - abort(make_response(({"message":"""You are not authorized to perfom this action, + abort(make_response(({"message": """You are not authorized to perfom this action, you are not the teacher or an admin of this project"""}, 403))) return wrap @@ -232,13 +240,15 @@ def wrap(*args, **kwargs): project_id = kwargs["project_id"] course_id = get_course_of_project(project_id) if (is_teacher_of_course(auth_user_id, course_id) - or is_admin_of_course(auth_user_id, course_id)): + or is_admin_of_course(auth_user_id, course_id)): return f(*args, **kwargs) if is_student_of_course(auth_user_id, course_id) and project_visible(project_id): return f(*args, **kwargs) - abort(make_response(({"message":"You're not authorized to perform this action"}, 403))) + abort(make_response( + ({"message": "You're not authorized to perform this action"}, 403))) return wrap + def authorize_submissions_request(f): """This function will check if the person sending a request to the API is logged in, and either the teacher/admin of the course or the student @@ -251,14 +261,15 @@ def wrap(*args, **kwargs): course_id = get_course_of_project(project_id) if (is_teacher_of_course(auth_user_id, course_id) - or is_admin_of_course(auth_user_id, course_id)): + or is_admin_of_course(auth_user_id, course_id)): return f(*args, **kwargs) if (is_student_of_course(auth_user_id, course_id) and project_visible(project_id) - and auth_user_id == request.args.get("uid")): + and auth_user_id == request.args.get("uid")): return f(*args, **kwargs) - abort(make_response(({"message":"You're not authorized to perform this action"}, 403))) + abort(make_response( + ({"message": "You're not authorized to perform this action"}, 403))) return wrap @@ -273,9 +284,10 @@ def wrap(*args, **kwargs): course_id = get_course_of_project(project_id) if (is_student_of_course(auth_user_id, course_id) and project_visible(project_id) - and auth_user_id == request.form.get("uid")): + and auth_user_id == request.form.get("uid")): return f(*args, **kwargs) - abort(make_response(({"message":"You're not authorized to perform this action"}, 403))) + abort(make_response( + ({"message": "You're not authorized to perform this action"}, 403))) return wrap @@ -290,7 +302,8 @@ def wrap(*args, **kwargs): submission = get_submission(submission_id) if submission.uid == auth_user_id: return f(*args, **kwargs) - abort(make_response(({"message":"You're not authorized to perform this action"}, 403))) + abort(make_response( + ({"message": "You're not authorized to perform this action"}, 403))) return wrap @@ -303,9 +316,10 @@ def wrap(*args, **kwargs): auth_user_id = return_authenticated_user_id() course_id = get_course_of_submission(kwargs["submission_id"]) if (is_teacher_of_course(auth_user_id, course_id) - or is_admin_of_course(auth_user_id, course_id)): + or is_admin_of_course(auth_user_id, course_id)): return f(*args, **kwargs) - abort(make_response(({"message":"You're not authorized to perform this action"}, 403))) + abort(make_response( + ({"message": "You're not authorized to perform this action"}, 403))) return wrap @@ -323,9 +337,8 @@ def wrap(*args, **kwargs): return f(*args, **kwargs) course_id = get_course_of_project(submission.project_id) if (is_teacher_of_course(auth_user_id, course_id) - or is_admin_of_course(auth_user_id, course_id)): + or is_admin_of_course(auth_user_id, course_id)): return f(*args, **kwargs) abort(make_response(({"message": - "You're not authorized to perform this action"} - , 403))) + "You're not authorized to perform this action"}, 403))) return wrap diff --git a/backend/project/utils/models/user_utils.py b/backend/project/utils/models/user_utils.py index f601c8b3..37cd263c 100644 --- a/backend/project/utils/models/user_utils.py +++ b/backend/project/utils/models/user_utils.py @@ -8,7 +8,7 @@ from sqlalchemy.exc import SQLAlchemyError from project import db -from project.models.user import User +from project.models.user import User, Role load_dotenv() API_URL = getenv("API_HOST") @@ -28,9 +28,9 @@ def get_user(user_id): def is_teacher(auth_user_id): """This function checks whether the user with auth_user_id is a teacher""" user = get_user(auth_user_id) - return user.is_teacher + return user.role == Role.TEACHER def is_admin(auth_user_id): """This function checks whether the user with auth_user_id is a teacher""" user = get_user(auth_user_id) - return user.is_admin + return user.role == Role.ADMIN diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index cc605602..fe9d3961 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -6,7 +6,7 @@ from project.sessionmaker import engine, Session from project.db_in import db from project.models.course import Course -from project.models.user import User +from project.models.user import User,Role from project.models.project import Project from project.models.course_relation import CourseStudent,CourseAdmin from project.models.submission import Submission, SubmissionStatus @@ -34,10 +34,10 @@ def db_session(): def users(): """Return a list of users to populate the database""" return [ - User(uid="brinkmann", is_admin=True, is_teacher=True), - User(uid="laermans", is_admin=True, is_teacher=True), - User(uid="student01", is_admin=False, is_teacher=False), - User(uid="student02", is_admin=False, is_teacher=False) + User(uid="brinkmann", role=Role.ADMIN), + User(uid="laermans", role=Role.ADMIN), + User(uid="student01", role=Role.STUDENT), + User(uid="student02", role=Role.STUDENT) ] def courses(): diff --git a/backend/tests/endpoints/conftest.py b/backend/tests/endpoints/conftest.py index 3f234b62..de74e6ab 100644 --- a/backend/tests/endpoints/conftest.py +++ b/backend/tests/endpoints/conftest.py @@ -7,7 +7,7 @@ import pytest from sqlalchemy import create_engine from sqlalchemy.exc import SQLAlchemyError -from project.models.user import User +from project.models.user import User,Role from project.models.course import Course from project.models.course_share_code import CourseShareCode from project import create_app_with_db @@ -47,7 +47,7 @@ def valid_user(): """ return { "uid": "w_student", - "is_teacher": False + "role": Role.STUDENT.name } @pytest.fixture @@ -67,8 +67,7 @@ def valid_admin(): """ return { "uid": "admin_person", - "is_teacher": False, - "is_admin":True + "role": Role.ADMIN, } @pytest.fixture @@ -95,10 +94,10 @@ def valid_user_entries(session): Returns a list of users that are in the database """ users = [ - User(uid="del", is_admin=False, is_teacher=True), - User(uid="pat", is_admin=False, is_teacher=True), - User(uid="u_get", is_admin=False, is_teacher=True), - User(uid="query_user", is_admin=True, is_teacher=False)] + User(uid="del", role=Role.TEACHER), + User(uid="pat", role=Role.TEACHER), + User(uid="u_get", role=Role.TEACHER), + User(uid="query_user", role=Role.ADMIN)] session.add_all(users) session.commit() @@ -149,7 +148,7 @@ def app(): @pytest.fixture def course_teacher_ad(): """A user that's a teacher for testing""" - ad_teacher = User(uid="Gunnar", is_teacher=True, is_admin=True) + ad_teacher = User(uid="Gunnar", role=Role.TEACHER) return ad_teacher @pytest.fixture @@ -199,7 +198,7 @@ def client(app): @pytest.fixture def valid_teacher_entry(session): """A valid teacher for testing that's already in the db""" - teacher = User(uid="Bart", is_teacher=True, is_admin=False) + teacher = User(uid="Bart", role=Role.TEACHER) try: session.add(teacher) session.commit() @@ -229,7 +228,7 @@ def valid_course_entry(session, valid_course): def valid_students_entries(session): """Valid students for testing that are already in the db""" students = [ - User(uid=f"student_sel2_{i}", is_teacher=False) + User(uid=f"student_sel2_{i}", role=Role.STUDENT) for i in range(3) ] session.add_all(students) diff --git a/backend/tests/endpoints/user_test.py b/backend/tests/endpoints/user_test.py index c6044db2..7d3a0c39 100644 --- a/backend/tests/endpoints/user_test.py +++ b/backend/tests/endpoints/user_test.py @@ -11,7 +11,7 @@ import pytest from sqlalchemy.orm import sessionmaker from sqlalchemy import create_engine -from project.models.user import User +from project.models.user import User,Role from project.db_in import db from tests import db_url @@ -24,12 +24,12 @@ def user_db_session(): db.metadata.create_all(engine) session = Session() session.add_all( - [User(uid="del", is_admin=False, is_teacher=True), - User(uid="pat", is_admin=False, is_teacher=True), - User(uid="u_get", is_admin=False, is_teacher=True), - User(uid="query_user", is_admin=True, is_teacher=False) - ] - ) + [User(uid="del", role=Role.TEACHER), + User(uid="pat", role=Role.TEACHER), + User(uid="u_get", role=Role.TEACHER), + User(uid="query_user", role=Role.ADMIN) + ] + ) session.commit() yield session session.rollback() @@ -120,38 +120,50 @@ def test_get_one_user_wrong_authentication(self, client, valid_user_entry): assert response.status_code == 401 def test_patch_user_not_authorized(self, client, valid_admin_entry, valid_user_entry): - """Test trying to patch a user without authorization""" - new_is_teacher = not valid_user_entry.is_teacher + """Test updating a user.""" + if valid_user_entry.role == Role.TEACHER: + new_role = Role.ADMIN + if valid_user_entry.role == Role.ADMIN: + new_role = Role.STUDENT + else: + new_role = Role.TEACHER + new_role = new_role.name response = client.patch(f"/users/{valid_user_entry.uid}", json={ - 'is_teacher': new_is_teacher, - 'is_admin': not valid_user_entry.is_admin + 'role': new_role }, headers={"Authorization":"student01"}) assert response.status_code == 403 # Patching a user is not allowed as a not-admin def test_patch_user(self, client, valid_admin_entry, valid_user_entry): """Test updating a user.""" - new_is_teacher = not valid_user_entry.is_teacher - + if valid_user_entry.role == Role.TEACHER: + new_role = Role.ADMIN + if valid_user_entry.role == Role.ADMIN: + new_role = Role.STUDENT + else: + new_role = Role.TEACHER + new_role = new_role.name response = client.patch(f"/users/{valid_user_entry.uid}", json={ - 'is_teacher': new_is_teacher, - 'is_admin': not valid_user_entry.is_admin + 'role': new_role }, headers={"Authorization":"admin1"}) assert response.status_code == 200 def test_patch_non_existent(self, client, valid_admin_entry): """Test updating a non-existent user.""" response = client.patch("/users/-20", json={ - 'is_teacher': False, - 'is_admin': True + 'role': Role.TEACHER.name }, headers={"Authorization":"admin1"}) assert response.status_code == 404 def test_patch_non_json(self, client, valid_admin_entry, valid_user_entry): """Test sending a non-JSON patch request.""" valid_user_form = asdict(valid_user_entry) - valid_user_form["is_teacher"] = not valid_user_form["is_teacher"] + if valid_user_form["role"] == Role.TEACHER.name: + valid_user_form["role"] = Role.STUDENT.name + else: + valid_user_form["role"] = Role.TEACHER.name + response = client.patch(f"/users/{valid_user_form['uid']}", data=valid_user_form, headers={"Authorization":"admin1"}) assert response.status_code == 415 @@ -159,12 +171,11 @@ def test_patch_non_json(self, client, valid_admin_entry, valid_user_entry): def test_get_users_with_query(self, client, valid_user_entries): """Test getting users with a query.""" # Send a GET request with query parameters, this is a nonsense entry but good for testing - response = client.get("/users?is_admin=true&is_teacher=false", + response = client.get("/users?role=ADMIN", headers={"Authorization":"teacher1"}) assert response.status_code == 200 # Check that the response contains only the user that matches the query users = response.json["data"] for user in users: - assert user["is_admin"] is True - assert user["is_teacher"] is False + assert Role[user["role"]] == Role.ADMIN diff --git a/backend/tests/models/user_test.py b/backend/tests/models/user_test.py index 8a026711..05520b8c 100644 --- a/backend/tests/models/user_test.py +++ b/backend/tests/models/user_test.py @@ -3,14 +3,14 @@ from pytest import raises, mark from sqlalchemy.orm import Session from sqlalchemy.exc import IntegrityError -from project.models.user import User +from project.models.user import User,Role class TestUserModel: """Class to test the User model""" def test_create_user(self, session: Session): """Test if a user can be created""" - user = User(uid="user01", is_teacher=False, is_admin=False) + user = User(uid="user01", role=Role.STUDENT) session.add(user) session.commit() assert session.get(User, "user01") is not None @@ -21,14 +21,14 @@ def test_query_user(self, session: Session): assert session.query(User).count() == 4 teacher = session.query(User).filter_by(uid="brinkmann").first() assert teacher is not None - assert teacher.is_teacher + assert teacher.role == Role.ADMIN def test_update_user(self, session: Session): """Test if a user can be updated""" student = session.query(User).filter_by(uid="student01").first() - student.is_admin = True + student.role = Role.ADMIN session.commit() - assert session.get(User, "student01").is_admin + assert session.get(User, "student01").role == Role.ADMIN def test_delete_user(self, session: Session): """Test if a user can be deleted"""