From 2ab31c15748a8540dbe1d6a9ed4f22824de7a070 Mon Sep 17 00:00:00 2001 From: cmekeirl Date: Wed, 20 Mar 2024 09:53:23 +0100 Subject: [PATCH 1/9] Removed is_teacher and is_admin, changed to role enum --- backend/db_construct.sql | 3 +- .../endpoints/index/OpenAPI_Object.json | 36 +++++++------------ backend/project/endpoints/users.py | 31 ++++++---------- backend/project/models/user.py | 15 +++++--- backend/project/utils/authentication.py | 8 ++--- backend/tests/conftest.py | 8 ++--- backend/tests/endpoints/conftest.py | 16 ++++----- backend/tests/endpoints/user_test.py | 31 +++++++++------- backend/tests/models/user_test.py | 4 +-- 9 files changed, 70 insertions(+), 82 deletions(-) diff --git a/backend/db_construct.sql b/backend/db_construct.sql index a1ad51fe..ae6be229 100644 --- a/backend/db_construct.sql +++ b/backend/db_construct.sql @@ -1,7 +1,6 @@ CREATE TABLE users ( uid VARCHAR(255), - is_teacher BOOLEAN, - is_admin BOOLEAN, + role ENUM('student', 'teacher', 'admin') 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 1e46994e..467534ba 100644 --- a/backend/project/endpoints/users.py +++ b/backend/project/endpoints/users.py @@ -28,14 +28,10 @@ def get(self): """ try: query = userModel.query - is_teacher = request.args.get('is_teacher') - is_admin = request.args.get('is_admin') + role = request.args.get("role") - 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')) + if role is not None: + query = query.filter(userModel.role == role.lower()) users = query.all() @@ -54,16 +50,14 @@ 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.args.get("role") - 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)" + "role": "User role (string)" },"url": f"{API_URL}/users" }, 400 try: @@ -71,8 +65,8 @@ def post(self): 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!", @@ -114,17 +108,14 @@ 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.args.get("role") 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() diff --git a/backend/project/models/user.py b/backend/project/models/user.py index bb130349..cd892a91 100644 --- a/backend/project/models/user.py +++ b/backend/project/models/user.py @@ -1,17 +1,22 @@ """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)) diff --git a/backend/project/utils/authentication.py b/backend/project/utils/authentication.py index 6501da9e..241aee2f 100644 --- a/backend/project/utils/authentication.py +++ b/backend/project/utils/authentication.py @@ -62,13 +62,13 @@ def return_authenticated_user_id(): if user: return auth_user_id - is_teacher = False + role = 'student' if user_info["jobTitle"] != None: - is_teacher = True + 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: @@ -89,7 +89,7 @@ def is_teacher(auth_user_id): "url": f"{API_URL}/users"}, 500 if not user: # should realistically never happen abort(500, "A database error occured") - if user.is_teacher: + if user.role == 'teacher': return True return False diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 7be87a8c..67313ba0 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -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='admin'), + User(uid="laermans", role='admin'), + User(uid="student01", role='student'), + User(uid="student02", role='student') ] def courses(): diff --git a/backend/tests/endpoints/conftest.py b/backend/tests/endpoints/conftest.py index 4466ee92..54d6b833 100644 --- a/backend/tests/endpoints/conftest.py +++ b/backend/tests/endpoints/conftest.py @@ -46,7 +46,7 @@ def valid_user(): """ return { "uid": "w_student", - "is_teacher": False + "role": 'student' } @pytest.fixture @@ -73,10 +73,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='teacher'), + User(uid="pat", role='teacher'), + User(uid="u_get", role='teacher'), + User(uid="query_user", role='admin')] session.add_all(users) session.commit() @@ -127,7 +127,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='teacher') return ad_teacher @pytest.fixture @@ -177,7 +177,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) + teacher = User(uid="Bart", role='teacher') session.add(teacher) session.commit() return teacher @@ -204,7 +204,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='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 25b28d62..a249c8e2 100644 --- a/backend/tests/endpoints/user_test.py +++ b/backend/tests/endpoints/user_test.py @@ -24,10 +24,10 @@ 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='teacher'), + User(uid="pat", role='teacher'), + User(uid="u_get", role='teacher'), + User(uid="query_user", role='admin') ] ) session.commit() @@ -117,37 +117,42 @@ def test_get_one_user_wrong_authentication(self, client, valid_user_entry): def test_patch_user(self, client, valid_user_entry): """Test updating a user.""" - new_is_teacher = not valid_user_entry.is_teacher + if valid_user_entry.role == 'teacher': + new_role = 'admin' + if valid_user_entry.role == 'admin': + new_role = 'student' + else: + new_role = 'teacher' 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 }) assert response.status_code == 403 # Patching a user is never necessary and thus not allowed def test_patch_non_existent(self, client): """Test updating a non-existent user.""" response = client.patch("/users/-20", json={ - 'is_teacher': False, - 'is_admin': True + 'role': 'teacher' }) assert response.status_code == 403 # Patching is not allowed def test_patch_non_json(self, client, 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"] == 'teacher': + valid_user_form["role"] = 'student' + else: + valid_user_form["role"] = 'teacher' response = client.patch(f"/users/{valid_user_form['uid']}", data=valid_user_form) assert response.status_code == 403 # Patching is not allowed 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", headers={"Authorization":"teacher1"}) + 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 user["role"] == 'admin' diff --git a/backend/tests/models/user_test.py b/backend/tests/models/user_test.py index 8a026711..652c79ff 100644 --- a/backend/tests/models/user_test.py +++ b/backend/tests/models/user_test.py @@ -10,7 +10,7 @@ class TestUserModel: 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='student') session.add(user) session.commit() assert session.get(User, "user01") is not None @@ -21,7 +21,7 @@ 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 == 'teacher' def test_update_user(self, session: Session): """Test if a user can be updated""" From 6682bfcd4843c9ac22b9eb3048747a5f930cf5ed Mon Sep 17 00:00:00 2001 From: cmekeirl Date: Wed, 20 Mar 2024 15:32:57 +0100 Subject: [PATCH 2/9] sql enum type --- backend/db_construct.sql | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/backend/db_construct.sql b/backend/db_construct.sql index ae6be229..6f3ccfab 100644 --- a/backend/db_construct.sql +++ b/backend/db_construct.sql @@ -1,6 +1,8 @@ +CREATE TYPE role_options AS ENUM ('student', 'teacher', 'admin'); + CREATE TABLE users ( uid VARCHAR(255), - role ENUM('student', 'teacher', 'admin') NOT NULL, + role role_options NOT NULL, PRIMARY KEY(uid) ); From 9751348bc573a0ef4fb9aa47cc1b29448773ed8d Mon Sep 17 00:00:00 2001 From: cmekeirl Date: Thu, 21 Mar 2024 09:44:09 +0100 Subject: [PATCH 3/9] usage of enum and fixed sql --- backend/db_construct.sql | 4 +-- backend/project/models/user.py | 2 +- backend/project/utils/authentication.py | 10 +++++--- backend/tests/conftest.py | 10 ++++---- backend/tests/endpoints/conftest.py | 18 ++++++------- backend/tests/endpoints/user_test.py | 34 ++++++++++++------------- backend/tests/models/user_test.py | 10 ++++---- 7 files changed, 45 insertions(+), 43 deletions(-) diff --git a/backend/db_construct.sql b/backend/db_construct.sql index 6f3ccfab..30744372 100644 --- a/backend/db_construct.sql +++ b/backend/db_construct.sql @@ -1,8 +1,8 @@ -CREATE TYPE role_options AS ENUM ('student', 'teacher', 'admin'); +CREATE TYPE role AS ENUM ('STUDENT', 'TEACHER', 'ADMIN'); CREATE TABLE users ( uid VARCHAR(255), - role role_options NOT NULL, + role role NOT NULL, PRIMARY KEY(uid) ); diff --git a/backend/project/models/user.py b/backend/project/models/user.py index cd892a91..8fc167c5 100644 --- a/backend/project/models/user.py +++ b/backend/project/models/user.py @@ -19,4 +19,4 @@ class User(db.Model): __tablename__ = "users" uid: str = Column(String(255), primary_key=True) - role: Role = Column(EnumField(Role)) + role: Role = Column(EnumField(Role), nullable=False) diff --git a/backend/project/utils/authentication.py b/backend/project/utils/authentication.py index 241aee2f..4d1ff2a1 100644 --- a/backend/project/utils/authentication.py +++ b/backend/project/utils/authentication.py @@ -11,7 +11,7 @@ from project import db -from project.models.user import User +from project.models.user import User,Role from project.models.course import Course from project.models.project import Project from project.models.submission import Submission @@ -62,9 +62,11 @@ def return_authenticated_user_id(): if user: return auth_user_id - role = 'student' + + # Use the Enum here + role = Role.STUDENT if user_info["jobTitle"] != None: - role = 'teacher' + role = Role.TEACHER # add user if not yet in database try: @@ -89,7 +91,7 @@ def is_teacher(auth_user_id): "url": f"{API_URL}/users"}, 500 if not user: # should realistically never happen abort(500, "A database error occured") - if user.role == 'teacher': + if user.role == Role.TEACHER: return True return False diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 67313ba0..55c564c2 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 @@ -34,10 +34,10 @@ def db_session(): def users(): """Return a list of users to populate the database""" return [ - User(uid="brinkmann", role='admin'), - User(uid="laermans", role='admin'), - User(uid="student01", role='student'), - User(uid="student02", role='student') + 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 54d6b833..a094e6cc 100644 --- a/backend/tests/endpoints/conftest.py +++ b/backend/tests/endpoints/conftest.py @@ -6,7 +6,7 @@ from zoneinfo import ZoneInfo import pytest from sqlalchemy import create_engine -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 @@ -46,7 +46,7 @@ def valid_user(): """ return { "uid": "w_student", - "role": 'student' + "role": Role.STUDENT } @pytest.fixture @@ -73,10 +73,10 @@ def valid_user_entries(session): Returns a list of users that are in the database """ users = [ - User(uid="del", role='teacher'), - User(uid="pat", role='teacher'), - User(uid="u_get", role='teacher'), - User(uid="query_user", role='admin')] + 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() @@ -127,7 +127,7 @@ def app(): @pytest.fixture def course_teacher_ad(): """A user that's a teacher for testing""" - ad_teacher = User(uid="Gunnar", role='teacher') + ad_teacher = User(uid="Gunnar", role=Role.TEACHER) return ad_teacher @pytest.fixture @@ -177,7 +177,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", role='teacher') + teacher = User(uid="Bart", role=Role.TEACHER) session.add(teacher) session.commit() return teacher @@ -204,7 +204,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}", role='student') + 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 a249c8e2..e93f8357 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", role='teacher'), - User(uid="pat", role='teacher'), - User(uid="u_get", role='teacher'), - User(uid="query_user", role='admin') - ] - ) + [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() @@ -117,12 +117,12 @@ def test_get_one_user_wrong_authentication(self, client, valid_user_entry): def test_patch_user(self, client, valid_user_entry): """Test updating a user.""" - if valid_user_entry.role == 'teacher': - new_role = 'admin' - if valid_user_entry.role == 'admin': - new_role = 'student' + 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 = 'teacher' + new_role = Role.TEACHER response = client.patch(f"/users/{valid_user_entry.uid}", json={ 'role': new_role @@ -132,17 +132,17 @@ def test_patch_user(self, client, valid_user_entry): def test_patch_non_existent(self, client): """Test updating a non-existent user.""" response = client.patch("/users/-20", json={ - 'role': 'teacher' + 'role': Role.TEACHER }) assert response.status_code == 403 # Patching is not allowed def test_patch_non_json(self, client, valid_user_entry): """Test sending a non-JSON patch request.""" valid_user_form = asdict(valid_user_entry) - if valid_user_form["role"] == 'teacher': - valid_user_form["role"] = 'student' + if valid_user_form["role"] == Role.TEACHER: + valid_user_form["role"] = Role.STUDENT else: - valid_user_form["role"] = 'teacher' + valid_user_form["role"] = Role.TEACHER response = client.patch(f"/users/{valid_user_form['uid']}", data=valid_user_form) assert response.status_code == 403 # Patching is not allowed @@ -155,4 +155,4 @@ def test_get_users_with_query(self, client, valid_user_entries): # Check that the response contains only the user that matches the query users = response.json["data"] for user in users: - assert user["role"] == 'admin' + assert user["role"] == Role.ADMIN diff --git a/backend/tests/models/user_test.py b/backend/tests/models/user_test.py index 652c79ff..3433c7c9 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", role='student') + 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.role == 'teacher' + assert teacher.role == Role.TEACHER 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""" From a59eae19a964af4ffa52e884aa3128c2be4dd595 Mon Sep 17 00:00:00 2001 From: cmekeirl Date: Fri, 22 Mar 2024 13:29:29 +0100 Subject: [PATCH 4/9] made role serializable --- backend/project/endpoints/users.py | 13 ++++++++----- backend/project/models/user.py | 5 +++++ backend/project/utils/models/user_utils.py | 6 +++--- backend/test_auth_server/__main__.py | 2 +- backend/tests/endpoints/conftest.py | 5 ++--- backend/tests/endpoints/user_test.py | 18 +++++++++--------- backend/tests/models/user_test.py | 2 +- 7 files changed, 29 insertions(+), 22 deletions(-) diff --git a/backend/project/endpoints/users.py b/backend/project/endpoints/users.py index ed250ad1..e44af5ee 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 @@ -30,11 +30,12 @@ def get(self): try: query = userModel.query role = request.args.get("role") - if role is not None: - query = query.filter(userModel.role == role.lower()) + 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}) @@ -51,6 +52,7 @@ def post(self): """ uid = request.json.get('uid') role = request.args.get("role") + role = Role[role.upper()] if role is not None else None url = f"{API_URL}/users" if role is None or uid is None: @@ -93,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", @@ -109,6 +111,7 @@ def patch(self, user_id): or failure of the update. """ role = request.args.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: @@ -120,7 +123,7 @@ def patch(self, user_id): # 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 8fc167c5..5cb173b7 100644 --- a/backend/project/models/user.py +++ b/backend/project/models/user.py @@ -20,3 +20,8 @@ class User(db.Model): __tablename__ = "users" uid: str = Column(String(255), primary_key=True) role: Role = Column(EnumField(Role), nullable=False) + def to_dict(self): + return { + 'uid': self.uid, + 'role': self.role.name, # Convert the enum to a string + } 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/test_auth_server/__main__.py b/backend/test_auth_server/__main__.py index 5d13b637..b79377f7 100644 --- a/backend/test_auth_server/__main__.py +++ b/backend/test_auth_server/__main__.py @@ -67,4 +67,4 @@ def get(self): app = Flask(__name__) app.register_blueprint(index_bp) -app.run(debug=True, host='0.0.0.0') +app.run(debug=True, host='0.0.0.0',port=5001) diff --git a/backend/tests/endpoints/conftest.py b/backend/tests/endpoints/conftest.py index 41f99e12..de74e6ab 100644 --- a/backend/tests/endpoints/conftest.py +++ b/backend/tests/endpoints/conftest.py @@ -47,7 +47,7 @@ def valid_user(): """ return { "uid": "w_student", - "role": Role.STUDENT + "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 diff --git a/backend/tests/endpoints/user_test.py b/backend/tests/endpoints/user_test.py index d0ad3fc0..7d3a0c39 100644 --- a/backend/tests/endpoints/user_test.py +++ b/backend/tests/endpoints/user_test.py @@ -128,10 +128,9 @@ def test_patch_user_not_authorized(self, client, valid_admin_entry, valid_user_e 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_role, - '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 @@ -144,7 +143,7 @@ def test_patch_user(self, client, valid_admin_entry, valid_user_entry): new_role = Role.STUDENT else: new_role = Role.TEACHER - + new_role = new_role.name response = client.patch(f"/users/{valid_user_entry.uid}", json={ 'role': new_role }, headers={"Authorization":"admin1"}) @@ -153,17 +152,18 @@ def test_patch_user(self, client, valid_admin_entry, valid_user_entry): def test_patch_non_existent(self, client, valid_admin_entry): """Test updating a non-existent user.""" response = client.patch("/users/-20", json={ - 'role': Role.TEACHER + '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) - if valid_user_form["role"] == Role.TEACHER: - valid_user_form["role"] = Role.STUDENT + if valid_user_form["role"] == Role.TEACHER.name: + valid_user_form["role"] = Role.STUDENT.name else: - valid_user_form["role"] = Role.TEACHER + 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 @@ -178,4 +178,4 @@ def test_get_users_with_query(self, client, valid_user_entries): # Check that the response contains only the user that matches the query users = response.json["data"] for user in users: - assert user["role"] == Role.ADMIN + assert Role[user["role"]] == Role.ADMIN diff --git a/backend/tests/models/user_test.py b/backend/tests/models/user_test.py index 3433c7c9..05520b8c 100644 --- a/backend/tests/models/user_test.py +++ b/backend/tests/models/user_test.py @@ -21,7 +21,7 @@ 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.role == Role.TEACHER + assert teacher.role == Role.ADMIN def test_update_user(self, session: Session): """Test if a user can be updated""" From fa590d944ca42f959f6fc12919ac2a58c4479557 Mon Sep 17 00:00:00 2001 From: cmekeirl Date: Sat, 23 Mar 2024 08:27:17 +0100 Subject: [PATCH 5/9] clean --- backend/project/endpoints/users.py | 3 ++- backend/project/utils/authentication.py | 15 ++++++++++----- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/backend/project/endpoints/users.py b/backend/project/endpoints/users.py index e44af5ee..024bcd9d 100644 --- a/backend/project/endpoints/users.py +++ b/backend/project/endpoints/users.py @@ -123,7 +123,8 @@ def patch(self, user_id): # Save the changes to the database db.session.commit() return jsonify({"message": "User updated successfully!", - "data": user.to_dict(), "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/utils/authentication.py b/backend/project/utils/authentication.py index 6a940af8..c1a96248 100644 --- a/backend/project/utils/authentication.py +++ b/backend/project/utils/authentication.py @@ -39,8 +39,11 @@ 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: @@ -51,7 +54,8 @@ def return_authenticated_user_id(): ({"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"] @@ -60,14 +64,15 @@ 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 # Use the Enum here role = Role.STUDENT - if user_info["jobTitle"] != None: + if user_info["jobTitle"] is not None: role = Role.TEACHER # add user if not yet in database From 98883905c8c7167a3da274a4de976d0a1dc9870e Mon Sep 17 00:00:00 2001 From: cmekeirl Date: Sat, 23 Mar 2024 08:32:04 +0100 Subject: [PATCH 6/9] docs --- backend/project/models/user.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/backend/project/models/user.py b/backend/project/models/user.py index 5cb173b7..7cd59fd1 100644 --- a/backend/project/models/user.py +++ b/backend/project/models/user.py @@ -21,6 +21,9 @@ class User(db.Model): uid: str = Column(String(255), primary_key=True) 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 From a585d451fb7f46e075dd9e90a53792d1e4a81845 Mon Sep 17 00:00:00 2001 From: cmekeirl Date: Sat, 23 Mar 2024 17:44:22 +0100 Subject: [PATCH 7/9] fixed user patch --- backend/project/endpoints/users.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/project/endpoints/users.py b/backend/project/endpoints/users.py index 024bcd9d..c2dbdabb 100644 --- a/backend/project/endpoints/users.py +++ b/backend/project/endpoints/users.py @@ -110,7 +110,7 @@ def patch(self, user_id): dict: A dictionary containing the message indicating the success or failure of the update. """ - role = request.args.get("role") + role = request.json.get("role") role = Role[role.upper()] if role is not None else None try: user = db.session.get(userModel, user_id) From 5ae35dcb177edcf54d267ba17e4ee90a7c8b14da Mon Sep 17 00:00:00 2001 From: cmekeirl Date: Sat, 23 Mar 2024 17:51:55 +0100 Subject: [PATCH 8/9] args vs json --- backend/project/endpoints/users.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/project/endpoints/users.py b/backend/project/endpoints/users.py index c2dbdabb..34e65817 100644 --- a/backend/project/endpoints/users.py +++ b/backend/project/endpoints/users.py @@ -51,7 +51,7 @@ def post(self): It should create a new user and return a success message. """ uid = request.json.get('uid') - role = request.args.get("role") + role = request.json.get("role") role = Role[role.upper()] if role is not None else None url = f"{API_URL}/users" From 49ff16075863355dd36f0de743de6ba70b0f82d5 Mon Sep 17 00:00:00 2001 From: cmekeirl Date: Mon, 25 Mar 2024 13:50:44 +0100 Subject: [PATCH 9/9] test --- backend/test_auth_server/__main__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/test_auth_server/__main__.py b/backend/test_auth_server/__main__.py index b79377f7..5d13b637 100644 --- a/backend/test_auth_server/__main__.py +++ b/backend/test_auth_server/__main__.py @@ -67,4 +67,4 @@ def get(self): app = Flask(__name__) app.register_blueprint(index_bp) -app.run(debug=True, host='0.0.0.0',port=5001) +app.run(debug=True, host='0.0.0.0')