From 99890a950bef7e5799fa1fe8aa44cd57cdb5b4f7 Mon Sep 17 00:00:00 2001 From: swve Date: Fri, 14 Jul 2023 11:29:15 +0100 Subject: [PATCH 1/5] fix: admin buttons issue --- front/app/install/steps/get_started.tsx | 16 +++++++++------- .../[orgslug]/(withmenu)/collections/page.tsx | 2 +- .../[orgslug]/(withmenu)/courses/courses.tsx | 5 +++-- .../orgs/[orgslug]/(withmenu)/courses/page.tsx | 3 ++- .../Security/AuthenticatedClientElement.tsx | 4 ++-- src/security/auth.py | 2 +- 6 files changed, 18 insertions(+), 14 deletions(-) diff --git a/front/app/install/steps/get_started.tsx b/front/app/install/steps/get_started.tsx index 980912da..f57a9931 100644 --- a/front/app/install/steps/get_started.tsx +++ b/front/app/install/steps/get_started.tsx @@ -9,20 +9,22 @@ function GetStarted() { const { data: install, error: error, isLoading } = useSWR(`${getAPIUrl()}install/latest`, swrFetcher); const router = useRouter() - function startInstallation() { - fetch(`${getAPIUrl()}install/start`, { + async function startInstallation() { + let res = await fetch(`${getAPIUrl()}install/start`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({}) - }).then(res => res.json()).then(res => { - if (res.success) { - mutate(`${getAPIUrl()}install/latest`) - router.push(`/install?step=1`) - } }) + if (res.status == 200) { + mutate(`${getAPIUrl()}install/latest`) + router.refresh(); + router.push(`/install?step=1`) + } + + } function redirectToStep() { diff --git a/front/app/orgs/[orgslug]/(withmenu)/collections/page.tsx b/front/app/orgs/[orgslug]/(withmenu)/collections/page.tsx index 5bb3ca38..0524577b 100644 --- a/front/app/orgs/[orgslug]/(withmenu)/collections/page.tsx +++ b/front/app/orgs/[orgslug]/(withmenu)/collections/page.tsx @@ -45,7 +45,7 @@ const CollectionsPage = async (params: any) => {
- + diff --git a/front/app/orgs/[orgslug]/(withmenu)/courses/courses.tsx b/front/app/orgs/[orgslug]/(withmenu)/courses/courses.tsx index 7b860abb..e1b57d4d 100644 --- a/front/app/orgs/[orgslug]/(withmenu)/courses/courses.tsx +++ b/front/app/orgs/[orgslug]/(withmenu)/courses/courses.tsx @@ -20,6 +20,7 @@ import { getCourseThumbnailMediaDirectory } from '@services/media/media'; interface CourseProps { orgslug: string; courses: any; + org_id: string; } // function to remove "course_" from the course_id @@ -49,7 +50,7 @@ function Courses(props: CourseProps) {
- + {courses.map((course: any) => (
- +
diff --git a/front/app/orgs/[orgslug]/(withmenu)/courses/page.tsx b/front/app/orgs/[orgslug]/(withmenu)/courses/page.tsx index 027e7719..8b4e5371 100644 --- a/front/app/orgs/[orgslug]/(withmenu)/courses/page.tsx +++ b/front/app/orgs/[orgslug]/(withmenu)/courses/page.tsx @@ -25,13 +25,14 @@ export async function generateMetadata( const CoursesPage = async (params: any) => { const orgslug = params.params.orgslug; + const org = await getOrganizationContextInfo(orgslug, { revalidate: 1800, tags: ['organizations'] }); const cookieStore = cookies(); const access_token_cookie: any = cookieStore.get('access_token_cookie'); const courses = await getOrgCoursesWithAuthHeader(orgslug, { revalidate: 0, tags: ['courses'] }, access_token_cookie ? access_token_cookie.value : null); return (
- +
); }; diff --git a/front/components/Security/AuthenticatedClientElement.tsx b/front/components/Security/AuthenticatedClientElement.tsx index e46618bd..e43e41e6 100644 --- a/front/components/Security/AuthenticatedClientElement.tsx +++ b/front/components/Security/AuthenticatedClientElement.tsx @@ -14,7 +14,7 @@ export const AuthenticatedClientElement = (props: AuthenticatedClientElementProp // Available roles const org_roles_values = ["admin", "owner"]; - const user_roles_values = ["role_admin", "role_super_admin"]; + const user_roles_values = ["role_admin"]; @@ -24,7 +24,7 @@ export const AuthenticatedClientElement = (props: AuthenticatedClientElementProp const user_roles = auth.userInfo.user_object.roles; const org_role = org_roles.find((org: any) => org.org_id == org_id); const user_role = user_roles.find((role: any) => role.org_id == org_id); - + if (org_role && user_role) { if (org_roles_values.includes(org_role.org_role) && user_roles_values.includes(user_role.role_id)) { return true; diff --git a/src/security/auth.py b/src/security/auth.py index 32309774..d125c15d 100644 --- a/src/security/auth.py +++ b/src/security/auth.py @@ -18,7 +18,7 @@ class Settings(BaseModel): authjwt_secret_key: str = "secret" if isDevModeEnabled() else SECRET_KEY authjwt_token_location = {"cookies", "headers"} authjwt_cookie_csrf_protect = False - authjwt_access_token_expires = False if isDevModeEnabled() else 3600 + authjwt_access_token_expires = False if isDevModeEnabled() else 28800 authjwt_cookie_samesite = "lax" authjwt_cookie_secure = True authjwt_cookie_domain = get_learnhouse_config().hosting_config.cookie_config.domain From 72c5d13028d7ac3260975d15a2401d05cd79213c Mon Sep 17 00:00:00 2001 From: swve Date: Fri, 14 Jul 2023 11:35:41 +0100 Subject: [PATCH 2/5] fix: install remaining issues --- front/services/install/install.ts | 2 +- src/services/install/install.py | 71 ++++--------------------------- 2 files changed, 9 insertions(+), 64 deletions(-) diff --git a/front/services/install/install.ts b/front/services/install/install.ts index ab69dc22..031d8bf2 100644 --- a/front/services/install/install.ts +++ b/front/services/install/install.ts @@ -33,7 +33,7 @@ export async function createDefaultElements() { export async function isInstallModeEnabled() { const result = await fetch(`${getAPIUrl()}install/latest`, RequestBody("GET", null, null)); - if (result.status === 200) { + if (result.status === 200 || result.status === 404) { return true; } else { diff --git a/src/services/install/install.py b/src/services/install/install.py index e32b7185..b7eda69d 100644 --- a/src/services/install/install.py +++ b/src/services/install/install.py @@ -125,8 +125,8 @@ async def install_default_elements(request: Request, data: dict): roles = request.app.db["roles"] # check if default roles ADMIN_ROLE and USER_ROLE already exist - admin_role = await roles.find_one({"role_id": "role_super_admin"}) - user_role = await roles.find_one({"role_id": "role_user"}) + admin_role = await roles.find_one({"role_id": "role_admin"}) + user_role = await roles.find_one({"role_id": "role_member"}) if admin_role is not None or user_role is not None: raise HTTPException( @@ -135,61 +135,8 @@ async def install_default_elements(request: Request, data: dict): ) # get default roles - SUPER_ADMIN_ROLE = RoleInDB( - name="SuperAdmin Role", - description="This role grants all permissions to the user", - elements=Elements( - courses=Permission( - action_create=True, - action_read=True, - action_update=True, - action_delete=True, - ), - users=Permission( - action_create=True, - action_read=True, - action_update=True, - action_delete=True, - ), - houses=Permission( - action_create=True, - action_read=True, - action_update=True, - action_delete=True, - ), - collections=Permission( - action_create=True, - action_read=True, - action_update=True, - action_delete=True, - ), - organizations=Permission( - action_create=True, - action_read=True, - action_update=True, - action_delete=True, - ), - coursechapters=Permission( - action_create=True, - action_read=True, - action_update=True, - action_delete=True, - ), - activities=Permission( - action_create=True, - action_read=True, - action_update=True, - action_delete=True, - ), - ), - org_id="*", - role_id="role_super_admin", - created_at=str(datetime.now()), - updated_at=str(datetime.now()), - ) - ADMIN_ROLE = RoleInDB( - name="SuperAdmin Role", + name="Admin Role", description="This role grants all permissions to the user", elements=Elements( courses=Permission( @@ -236,13 +183,13 @@ async def install_default_elements(request: Request, data: dict): ), ), org_id="*", - role_id="role_super_admin", + role_id="role_admin", created_at=str(datetime.now()), updated_at=str(datetime.now()), ) USER_ROLE = RoleInDB( - name="User role", + name="Member Role", description="This role grants read-only permissions to the user", elements=Elements( courses=Permission( @@ -289,16 +236,14 @@ async def install_default_elements(request: Request, data: dict): ), ), org_id="*", - role_id="role_user", + role_id="role_member", created_at=str(datetime.now()), updated_at=str(datetime.now()), ) try: # insert default roles - await roles.insert_many( - [ADMIN_ROLE.dict(), USER_ROLE.dict(), SUPER_ADMIN_ROLE.dict()] - ) + await roles.insert_many([USER_ROLE.dict(), ADMIN_ROLE.dict()]) return True except Exception: @@ -386,7 +331,7 @@ async def install_create_organization_user( orgs = [UserOrganization(org_id=org_id, org_role="owner")] # Give role - roles = [UserRolesInOrganization(role_id="role_super_admin", org_id=org_id)] + roles = [UserRolesInOrganization(role_id="role_admin", org_id=org_id)] # Create the user user = UserInDB( From 3c2f6b3a9864d6c4201471f71dc620373d253409 Mon Sep 17 00:00:00 2001 From: swve Date: Thu, 20 Jul 2023 01:10:54 +0200 Subject: [PATCH 3/5] feat: revamp authorization mechanism across app --- app.py | 40 ++++ .../(withmenu)/collections/new/page.tsx | 1 + src/security/rbac/rbac.py | 127 ++++++++++ src/security/rbac/utils.py | 45 ++++ src/security/security.py | 121 ---------- src/services/courses/activities/activities.py | 108 +++++---- src/services/courses/activities/pdf.py | 20 +- src/services/courses/activities/video.py | 40 ++-- src/services/courses/chapters.py | 218 +++++++++++++----- src/services/courses/collections.py | 50 ++-- src/services/courses/courses.py | 96 ++++---- src/services/orgs/orgs.py | 20 +- src/services/users/schemas/users.py | 3 + src/services/users/users.py | 130 +++++++---- 14 files changed, 648 insertions(+), 371 deletions(-) create mode 100644 src/security/rbac/rbac.py create mode 100644 src/security/rbac/utils.py diff --git a/app.py b/app.py index 0baf3d42..ef5b7580 100644 --- a/app.py +++ b/app.py @@ -8,6 +8,9 @@ from fastapi.staticfiles import StaticFiles from fastapi_jwt_auth.exceptions import AuthJWTException from fastapi.middleware.gzip import GZipMiddleware +from src.security.rbac.rbac import authorization_verify_based_on_roles, authorization_verify_if_element_is_public, authorization_verify_if_user_is_author +from src.services.users.schemas.users import UserRolesInOrganization + # from src.services.mocks.initial import create_initial_data @@ -66,3 +69,40 @@ app.include_router(v1_router) @app.get("/") async def root(): return {"Message": "Welcome to LearnHouse ✨"} + + +@app.get("/test") +async def rootd(request: Request): + res = await authorization_verify_based_on_roles( + request=request, + user_id="user_c441e47e-5c04-4b03-9886-b0f5cb333c06", + action="read", + roles_list=[ + UserRolesInOrganization( + org_id="org_e7085838-2efc-48f3-b414-77318572d9f5", role_id="role_admin" + ), + ], + element_id="collection_1c277b46-5a4b-440a-ac29-94b874ef7cf4", + ) + return res + + +@app.get("/test2") +async def rootds(request: Request): + res = await authorization_verify_if_user_is_author( + request=request, + user_id="user_c441e47e-5c04-4b03-9886-b0f5cb333c06", + action="read", + element_id="course_1c277b46-5a4b-440a-ac29-94b874ef7cf4", + ) + return res + +@app.get("/test3") +async def rootdsc(request: Request): + res = await authorization_verify_if_element_is_public( + request=request, + user_id="anonymous", + action="read", + element_id="course_1c277b46-5a4b-440a-ac29-94b874ef7cf4", + ) + return res \ No newline at end of file diff --git a/front/app/orgs/[orgslug]/(withmenu)/collections/new/page.tsx b/front/app/orgs/[orgslug]/(withmenu)/collections/new/page.tsx index 41f05ff0..560b6b37 100644 --- a/front/app/orgs/[orgslug]/(withmenu)/collections/new/page.tsx +++ b/front/app/orgs/[orgslug]/(withmenu)/collections/new/page.tsx @@ -40,6 +40,7 @@ function NewCollection(params: any) { name: name, description: description, courses: selectedCourses, + public: true, org_id: org.org_id, }; await createCollection(collection); diff --git a/src/security/rbac/rbac.py b/src/security/rbac/rbac.py new file mode 100644 index 00000000..8f131cca --- /dev/null +++ b/src/security/rbac/rbac.py @@ -0,0 +1,127 @@ +from typing import Literal +from fastapi import HTTPException, status, Request +from src.security.rbac.utils import check_element_type, get_id_identifier_of_element +from src.services.roles.schemas.roles import RoleInDB +from src.services.users.schemas.users import UserRolesInOrganization + + +async def authorization_verify_if_element_is_public( + request, + element_id: str, + user_id: str, + action: Literal["read"], +): + element_nature = await check_element_type(element_id) + + # Verifies if the element is public + if ( + element_nature == ("courses" or "collections") + and action == "read" + and user_id == "anonymous" + ): + if element_nature == "courses": + courses = request.app.db["courses"] + course = await courses.find_one({"course_id": element_id}) + + if course["public"]: + return True + else: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="User rights (public content) : You don't have the right to perform this action", + ) + + if element_nature == "collections": + collections = request.app.db["collections"] + collection = await collections.find_one({"collection_id": element_id}) + + if collection["public"]: + return True + else: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="User rights (public content) : You don't have the right to perform this action", + ) + else: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="User rights (public content) : You don't have the right to perform this action", + ) + + +async def authorization_verify_if_user_is_author( + request, + user_id: str, + action: Literal["read", "update", "delete", "create"], + element_id: str, +): + if action == "update" or "delete" or "read": + element_nature = await check_element_type(element_id) + elements = request.app.db[element_nature] + element_identifier = await get_id_identifier_of_element(element_id) + element = await elements.find_one({element_identifier: element_id}) + if user_id in element["authors"]: + return True + else: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="User rights (author) : You don't have the right to perform this action", + ) + else: + return False + + +async def authorization_verify_based_on_roles( + request: Request, + user_id: str, + action: Literal["read", "update", "delete", "create"], + roles_list: list[UserRolesInOrganization], + element_id: str, +): + element_type = await check_element_type(element_id) + print(element_type) + element = request.app.db[element_type] + roles = request.app.db["roles"] + + # Get the element + element_identifier = await get_id_identifier_of_element(element_id) + element = await element.find_one({element_identifier: element_id}) + + # Get the roles of the user + roles_id_list = [role["role_id"] for role in roles_list] + roles = await roles.find({"role_id": {"$in": roles_id_list}}).to_list(length=100) + + # Get the rights of the roles + for role in roles: + role = RoleInDB(**role) + if role.elements[element_type][f"action_{action}"] is True: + return True + else: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="User rights (roles) : You don't have the right to perform this action", + ) + + +async def authorization_verify_based_on_roles_and_authorship( + request: Request, + user_id: str, + action: Literal["read", "update", "delete", "create"], + roles_list: list[UserRolesInOrganization], + element_id: str, +): + isAuthor = await authorization_verify_if_user_is_author( + request, user_id, action, element_id + ) + + isRole = await authorization_verify_based_on_roles( + request, user_id, action, roles_list, element_id + ) + + if isAuthor or isRole: + return True + else: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="User rights (roles & authorship) : You don't have the right to perform this action", + ) diff --git a/src/security/rbac/utils.py b/src/security/rbac/utils.py new file mode 100644 index 00000000..3ef48a09 --- /dev/null +++ b/src/security/rbac/utils.py @@ -0,0 +1,45 @@ +from fastapi import HTTPException, status + + +async def check_element_type(element_id): + """ + Check if the element is a course, a user, a house or a collection, by checking its prefix + """ + if element_id.startswith("course_"): + return "courses" + elif element_id.startswith("user_"): + return "users" + elif element_id.startswith("house_"): + return "houses" + elif element_id.startswith("org_"): + return "organizations" + elif element_id.startswith("coursechapter_"): + return "coursechapters" + elif element_id.startswith("collection_"): + return "collections" + elif element_id.startswith("activity_"): + return "activities" + else: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail="User rights : Issue verifying element nature", + ) + + +async def get_singular_form_of_element(element_id): + element_type = await check_element_type(element_id) + + if element_type == "activities": + return "activity" + else: + singular_form_element = element_type[:-1] + return singular_form_element + + +async def get_id_identifier_of_element(element_id): + singular_form_element = await get_singular_form_of_element(element_id) + + if singular_form_element == "ogranizations": + return "org_id" + else: + return str(singular_form_element) + "_id" diff --git a/src/security/security.py b/src/security/security.py index fdd58323..0972df02 100644 --- a/src/security/security.py +++ b/src/security/security.py @@ -1,10 +1,7 @@ -from fastapi import HTTPException, status, Request from passlib.context import CryptContext from passlib.hash import pbkdf2_sha256 from config.config import get_learnhouse_config -from src.services.roles.schemas.roles import RoleInDB -from src.services.users.schemas.users import UserInDB, UserRolesInOrganization ### 🔒 JWT ############################################################## @@ -30,122 +27,4 @@ async def security_verify_password(plain_password: str, hashed_password: str): ### 🔒 Passwords Hashing ############################################################## -### 🔒 Roles checking ############################################################## - -async def verify_user_rights_with_roles( - request: Request, action: str, user_id: str, element_id: str, element_org_id: str -): - """ - Check if the user has the right to perform the action on the element - """ - request.app.db["roles"] - users = request.app.db["users"] - - user = await users.find_one({"user_id": user_id}) - - ######### - # Users existence verification - ######### - - if not user and user_id != "anonymous": - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, detail="User rights : User not found" - ) - - # Check if user is anonymous - if user_id == "anonymous": - return False - - # Get User - user: UserInDB = UserInDB(**await users.find_one({"user_id": user_id})) - - ######### - # Organization Roles verification - ######### - - for org in user.orgs: - if org.org_id == element_org_id: - # Check if user is owner or reader of the organization - if org.org_role == ("owner" or "editor"): - return True - - ######### - # Roles verification - ######### - user_roles = user.roles - - if action != "create": - return await check_user_role_org_with_element_org( - request, element_id, user_roles, action - ) - - # If no role is found, raise an error - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="User rights : You don't have the right to perform this action", - ) - - -async def check_element_type(element_id): - """ - Check if the element is a course, a user, a house or a collection, by checking its prefix - """ - if element_id.startswith("course_"): - return "courses" - elif element_id.startswith("user_"): - return "users" - elif element_id.startswith("house_"): - return "houses" - elif element_id.startswith("org_"): - return "organizations" - elif element_id.startswith("coursechapter_"): - return "coursechapters" - elif element_id.startswith("collection_"): - return "collections" - elif element_id.startswith("activity_"): - return "activities" - else: - raise HTTPException( - status_code=status.HTTP_409_CONFLICT, - detail="User rights : Issue verifying element nature", - ) - - -async def check_user_role_org_with_element_org( - request: Request, - element_id: str, - roles_list: list[UserRolesInOrganization], - action: str, -): - element_type = await check_element_type(element_id) - element = request.app.db[element_type] - roles = request.app.db["roles"] - - # get singular element type - singular_form_element = element_type[:-1] - - element_type_id = singular_form_element + "_id" - - element_org = await element.find_one({element_type_id: element_id}) - - for role in roles_list: - # Check if The role belongs to the same organization as the element - role_db = await roles.find_one({"role_id": role.role_id}) - role = RoleInDB(**role_db) - if (role.org_id == element_org["org_id"]) or role.org_id == "*": - # Check if user has the right role - for role in roles_list: - role_db = await roles.find_one({"role_id": role.role_id}) - role = RoleInDB(**role_db) - if role.elements[element_type][f"action_{action}"]: - return True - - else: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="User rights (roles) : You don't have the right to perform this action", - ) - - -### 🔒 Roles checking ############################################################## diff --git a/src/services/courses/activities/activities.py b/src/services/courses/activities/activities.py index 57d1d410..9551518d 100644 --- a/src/services/courses/activities/activities.py +++ b/src/services/courses/activities/activities.py @@ -1,6 +1,10 @@ +from typing import Literal from pydantic import BaseModel -from src.security.security import verify_user_rights_with_roles -from src.services.users.schemas.users import PublicUser +from src.security.rbac.rbac import ( + authorization_verify_based_on_roles, + authorization_verify_if_element_is_public, +) +from src.services.users.schemas.users import AnonymousUser, PublicUser from fastapi import HTTPException, status, Request from uuid import uuid4 from datetime import datetime @@ -40,23 +44,26 @@ async def create_activity( ): activities = request.app.db["activities"] courses = request.app.db["courses"] + users = request.app.db["users"] + + # get user + user = await users.find_one({"user_id": current_user.user_id}) # generate activity_id activity_id = str(f"activity_{uuid4()}") - hasRoleRights = await verify_user_rights_with_roles( - request, "create", current_user.user_id, activity_id, org_id + # verify activity rights + await authorization_verify_based_on_roles( + request, + current_user.user_id, + "create", + user["roles"], + activity_id, ) # get course_id from activity course = await courses.find_one({"chapters": coursechapter_id}) - if not hasRoleRights: - raise HTTPException( - status_code=status.HTTP_409_CONFLICT, - detail="Roles : Insufficient rights to perform this action", - ) - # create activity activity = ActivityInDB( **activity_object.dict(), @@ -86,29 +93,10 @@ async def get_activity(request: Request, activity_id: str, current_user: PublicU # get course_id from activity coursechapter_id = activity["coursechapter_id"] - course = await courses.find_one({"chapters": coursechapter_id}) - - isCoursePublic = course["public"] - isAuthor = current_user.user_id in course["authors"] - - if isAuthor: - activity = ActivityInDB(**activity) - return activity + await courses.find_one({"chapters": coursechapter_id}) # verify course rights - hasRoleRights = await verify_user_rights_with_roles( - request, - "read", - current_user.user_id, - activity_id, - element_org_id=activity["org_id"], - ) - - if not hasRoleRights and not isCoursePublic: - raise HTTPException( - status_code=status.HTTP_409_CONFLICT, - detail="Roles : Insufficient rights to perform this action", - ) + await verify_rights(request, activity["course_id"], current_user, "read") if not activity: raise HTTPException( @@ -128,14 +116,9 @@ async def update_activity( activities = request.app.db["activities"] activity = await activities.find_one({"activity_id": activity_id}) + # verify course rights - await verify_user_rights_with_roles( - request, - "update", - current_user.user_id, - activity_id, - element_org_id=activity["org_id"], - ) + await verify_rights(request, activity_id, current_user, "update") if activity: creationDate = activity["creationDate"] @@ -171,13 +154,7 @@ async def delete_activity(request: Request, activity_id: str, current_user: Publ activity = await activities.find_one({"activity_id": activity_id}) # verify course rights - await verify_user_rights_with_roles( - request, - "delete", - current_user.user_id, - activity_id, - element_org_id=activity["org_id"], - ) + await verify_rights(request, activity_id, current_user, "delete") if not activity: raise HTTPException( @@ -217,3 +194,44 @@ async def get_activities( ] return activities + + +#### Security #################################################### + + +async def verify_rights( + request: Request, + activity_id: str, # course_id in case of read + current_user: PublicUser | AnonymousUser, + action: Literal["create", "read", "update", "delete"], +): + if action == "read": + if current_user.user_id == "anonymous": + await authorization_verify_if_element_is_public( + request, activity_id, current_user.user_id, action + ) + else: + users = request.app.db["users"] + user = await users.find_one({"user_id": current_user.user_id}) + + await authorization_verify_based_on_roles( + request, + current_user.user_id, + action, + user["roles"], + activity_id, + ) + else: + users = request.app.db["users"] + user = await users.find_one({"user_id": current_user.user_id}) + + await authorization_verify_based_on_roles( + request, + current_user.user_id, + action, + user["roles"], + activity_id, + ) + + +#### Security #################################################### diff --git a/src/services/courses/activities/pdf.py b/src/services/courses/activities/pdf.py index 8b59821b..8919639b 100644 --- a/src/services/courses/activities/pdf.py +++ b/src/services/courses/activities/pdf.py @@ -1,4 +1,4 @@ -from src.security.security import verify_user_rights_with_roles +from src.security.rbac.rbac import authorization_verify_based_on_roles from src.services.courses.activities.uploads.pdfs import upload_pdf from src.services.users.users import PublicUser from src.services.courses.activities.activities import ActivityInDB @@ -16,6 +16,10 @@ async def create_documentpdf_activity( ): activities = request.app.db["activities"] courses = request.app.db["courses"] + users = request.app.db["users"] + + # get user + user = await users.find_one({"user_id": current_user.user_id}) # generate activity_id activity_id = str(f"activity_{uuid4()}") @@ -64,16 +68,14 @@ async def create_documentpdf_activity( updateDate=str(datetime.now()), ) - hasRoleRights = await verify_user_rights_with_roles( - request, "create", current_user.user_id, activity_id, element_org_id=org_id + await authorization_verify_based_on_roles( + request, + current_user.user_id, + "create", + user["roles"], + activity_id, ) - if not hasRoleRights: - raise HTTPException( - status_code=status.HTTP_409_CONFLICT, - detail="Roles : Insufficient rights to perform this action", - ) - # create activity activity = ActivityInDB(**activity_object.dict()) await activities.insert_one(activity.dict()) diff --git a/src/services/courses/activities/video.py b/src/services/courses/activities/video.py index 0667513c..470e4ddc 100644 --- a/src/services/courses/activities/video.py +++ b/src/services/courses/activities/video.py @@ -1,7 +1,9 @@ from typing import Literal from pydantic import BaseModel -from src.security.security import verify_user_rights_with_roles +from src.security.rbac.rbac import ( + authorization_verify_based_on_roles, +) from src.services.courses.activities.uploads.videos import upload_video from src.services.users.users import PublicUser from src.services.courses.activities.activities import ActivityInDB @@ -19,6 +21,10 @@ async def create_video_activity( ): activities = request.app.db["activities"] courses = request.app.db["courses"] + users = request.app.db["users"] + + # get user + user = await users.find_one({"user_id": current_user.user_id}) # generate activity_id activity_id = str(f"activity_{uuid4()}") @@ -75,16 +81,14 @@ async def create_video_activity( updateDate=str(datetime.now()), ) - hasRoleRights = await verify_user_rights_with_roles( - request, "create", current_user.user_id, activity_id, element_org_id=org_id + await authorization_verify_based_on_roles( + request, + current_user.user_id, + "create", + user["roles"], + activity_id, ) - if not hasRoleRights: - raise HTTPException( - status_code=status.HTTP_409_CONFLICT, - detail="Roles : Insufficient rights to perform this action", - ) - # create activity activity = ActivityInDB(**activity_object.dict()) await activities.insert_one(activity.dict()) @@ -122,6 +126,10 @@ async def create_external_video_activity( ): activities = request.app.db["activities"] courses = request.app.db["courses"] + users = request.app.db["users"] + + # get user + user = await users.find_one({"user_id": current_user.user_id}) # generate activity_id activity_id = str(f"activity_{uuid4()}") @@ -157,16 +165,14 @@ async def create_external_video_activity( updateDate=str(datetime.now()), ) - hasRoleRights = await verify_user_rights_with_roles( - request, "create", current_user.user_id, activity_id, element_org_id=org_id + await authorization_verify_based_on_roles( + request, + current_user.user_id, + "create", + user["roles"], + activity_id, ) - if not hasRoleRights: - raise HTTPException( - status_code=status.HTTP_409_CONFLICT, - detail="Roles : Insufficient rights to perform this action", - ) - # create activity activity = ActivityInDB(**activity_object.dict()) await activities.insert_one(activity.dict()) diff --git a/src/services/courses/chapters.py b/src/services/courses/chapters.py index 1548533e..ea35905d 100644 --- a/src/services/courses/chapters.py +++ b/src/services/courses/chapters.py @@ -1,11 +1,15 @@ from datetime import datetime -from typing import List +from typing import List, Literal from uuid import uuid4 from pydantic import BaseModel from src.security.auth import non_public_endpoint +from src.security.rbac.rbac import ( + authorization_verify_based_on_roles, + authorization_verify_based_on_roles_and_authorship, + authorization_verify_if_element_is_public, +) from src.services.courses.courses import Course from src.services.courses.activities.activities import ActivityInDB -from src.security.security import verify_user_rights_with_roles from src.services.users.users import PublicUser from fastapi import HTTPException, status, Request @@ -29,6 +33,7 @@ class CourseChapterMetaData(BaseModel): chapters: dict activities: object + #### Classes #################################################### #################################################### @@ -36,35 +41,60 @@ class CourseChapterMetaData(BaseModel): #################################################### -async def create_coursechapter(request: Request, coursechapter_object: CourseChapter, course_id: str, current_user: PublicUser): +async def create_coursechapter( + request: Request, + coursechapter_object: CourseChapter, + course_id: str, + current_user: PublicUser, +): courses = request.app.db["courses"] - print(course_id) + users = request.app.db["users"] # get course org_id and verify rights - course = await courses.find_one({"course_id": course_id}) + await courses.find_one({"course_id": course_id}) + user = await users.find_one({"user_id": current_user.user_id}) # generate coursechapter_id with uuid4 coursechapter_id = str(f"coursechapter_{uuid4()}") - hasRoleRights = await verify_user_rights_with_roles(request, "create", current_user.user_id, coursechapter_id, course["org_id"]) + hasRoleRights = await authorization_verify_based_on_roles( + request, current_user.user_id, "create", user["roles"], course_id + ) if not hasRoleRights: raise HTTPException( - status_code=status.HTTP_409_CONFLICT, detail="Roles : Insufficient rights to perform this action") + status_code=status.HTTP_409_CONFLICT, + detail="Roles : Insufficient rights to perform this action", + ) - coursechapter = CourseChapterInDB(coursechapter_id=coursechapter_id, creationDate=str( - datetime.now()), updateDate=str(datetime.now()), course_id=course_id, **coursechapter_object.dict()) + coursechapter = CourseChapterInDB( + coursechapter_id=coursechapter_id, + creationDate=str(datetime.now()), + updateDate=str(datetime.now()), + course_id=course_id, + **coursechapter_object.dict(), + ) - courses.update_one({"course_id": course_id}, { - "$addToSet": {"chapters": coursechapter_id, "chapters_content": coursechapter.dict()}}) + courses.update_one( + {"course_id": course_id}, + { + "$addToSet": { + "chapters": coursechapter_id, + "chapters_content": coursechapter.dict(), + } + }, + ) return coursechapter.dict() -async def get_coursechapter(request: Request, coursechapter_id: str, current_user: PublicUser): +async def get_coursechapter( + request: Request, coursechapter_id: str, current_user: PublicUser +): courses = request.app.db["courses"] coursechapter = await courses.find_one( - {"chapters_content.coursechapter_id": coursechapter_id}) + {"chapters_content.coursechapter_id": coursechapter_id} + ) if coursechapter: # verify course rights @@ -75,64 +105,87 @@ async def get_coursechapter(request: Request, coursechapter_id: str, current_use else: raise HTTPException( - status_code=status.HTTP_409_CONFLICT, detail="CourseChapter does not exist") + status_code=status.HTTP_409_CONFLICT, detail="CourseChapter does not exist" + ) -async def update_coursechapter(request: Request, coursechapter_object: CourseChapter, coursechapter_id: str, current_user: PublicUser): +async def update_coursechapter( + request: Request, + coursechapter_object: CourseChapter, + coursechapter_id: str, + current_user: PublicUser, +): courses = request.app.db["courses"] coursechapter = await courses.find_one( - {"chapters_content.coursechapter_id": coursechapter_id}) + {"chapters_content.coursechapter_id": coursechapter_id} + ) if coursechapter: - # verify course rights await verify_rights(request, coursechapter["course_id"], current_user, "update") - coursechapter = CourseChapterInDB(coursechapter_id=coursechapter_id, creationDate=str( - datetime.now()), updateDate=str(datetime.now()), course_id=coursechapter["course_id"], **coursechapter_object.dict()) + coursechapter = CourseChapterInDB( + coursechapter_id=coursechapter_id, + creationDate=str(datetime.now()), + updateDate=str(datetime.now()), + course_id=coursechapter["course_id"], + **coursechapter_object.dict(), + ) - courses.update_one({"chapters_content.coursechapter_id": coursechapter_id}, { - "$set": {"chapters_content.$": coursechapter.dict()}}) + courses.update_one( + {"chapters_content.coursechapter_id": coursechapter_id}, + {"$set": {"chapters_content.$": coursechapter.dict()}}, + ) return coursechapter else: raise HTTPException( - status_code=status.HTTP_409_CONFLICT, detail="Coursechapter does not exist") + status_code=status.HTTP_409_CONFLICT, detail="Coursechapter does not exist" + ) -async def delete_coursechapter(request: Request, coursechapter_id: str, current_user: PublicUser): +async def delete_coursechapter( + request: Request, coursechapter_id: str, current_user: PublicUser +): courses = request.app.db["courses"] course = await courses.find_one( - {"chapters_content.coursechapter_id": coursechapter_id}) + {"chapters_content.coursechapter_id": coursechapter_id} + ) if course: # verify course rights await verify_rights(request, course["course_id"], current_user, "delete") # Remove coursechapter from course - await courses.update_one({"course_id": course["course_id"]}, { - "$pull": {"chapters": coursechapter_id}}) + await courses.update_one( + {"course_id": course["course_id"]}, + {"$pull": {"chapters": coursechapter_id}}, + ) - await courses.update_one({"chapters_content.coursechapter_id": coursechapter_id}, { - "$pull": {"chapters_content": {"coursechapter_id": coursechapter_id}}}) - - + await courses.update_one( + {"chapters_content.coursechapter_id": coursechapter_id}, + {"$pull": {"chapters_content": {"coursechapter_id": coursechapter_id}}}, + ) return {"message": "Coursechapter deleted"} else: raise HTTPException( - status_code=status.HTTP_409_CONFLICT, detail="Course does not exist") + status_code=status.HTTP_409_CONFLICT, detail="Course does not exist" + ) + #################################################### # Misc #################################################### -async def get_coursechapters(request: Request, course_id: str, page: int = 1, limit: int = 10): +async def get_coursechapters( + request: Request, course_id: str, page: int = 1, limit: int = 10 +): courses = request.app.db["courses"] course = await courses.find_one({"course_id": course_id}) @@ -144,19 +197,26 @@ async def get_coursechapters(request: Request, course_id: str, page: int = 1, li return coursechapters -async def get_coursechapters_meta(request: Request, course_id: str, current_user: PublicUser): +async def get_coursechapters_meta( + request: Request, course_id: str, current_user: PublicUser +): courses = request.app.db["courses"] activities = request.app.db["activities"] await non_public_endpoint(current_user) - coursechapters = await courses.find_one({"course_id": course_id}, {"chapters": 1, "chapters_content": 1, "_id": 0}) + await verify_rights(request, course_id, current_user, "read") + + coursechapters = await courses.find_one( + {"course_id": course_id}, {"chapters": 1, "chapters_content": 1, "_id": 0} + ) coursechapters = coursechapters if not coursechapters: raise HTTPException( - status_code=status.HTTP_409_CONFLICT, detail="Course does not exist") + status_code=status.HTTP_409_CONFLICT, detail="Course does not exist" + ) # activities coursechapter_activityIds_global = [] @@ -165,7 +225,6 @@ async def get_coursechapters_meta(request: Request, course_id: str, current_user chapters = {} if coursechapters["chapters_content"]: for coursechapter in coursechapters["chapters_content"]: - coursechapter = CourseChapterInDB(**coursechapter) coursechapter_activityIds = [] @@ -174,37 +233,55 @@ async def get_coursechapters_meta(request: Request, course_id: str, current_user coursechapter_activityIds_global.append(activity) chapters[coursechapter.coursechapter_id] = { - "id": coursechapter.coursechapter_id, "name": coursechapter.name, "activityIds": coursechapter_activityIds + "id": coursechapter.coursechapter_id, + "name": coursechapter.name, + "activityIds": coursechapter_activityIds, } # activities activities_list = {} - for activity in await activities.find({"activity_id": {"$in": coursechapter_activityIds_global}}).to_list(length=100): + for activity in await activities.find( + {"activity_id": {"$in": coursechapter_activityIds_global}} + ).to_list(length=100): activity = ActivityInDB(**activity) activities_list[activity.activity_id] = { - "id": activity.activity_id, "name": activity.name, "type": activity.type, "content": activity.content + "id": activity.activity_id, + "name": activity.name, + "type": activity.type, + "content": activity.content, } final = { "chapters": chapters, "chapterOrder": coursechapters["chapters"], - "activities": activities_list + "activities": activities_list, } return final -async def update_coursechapters_meta(request: Request, course_id: str, coursechapters_metadata: CourseChapterMetaData, current_user: PublicUser): +async def update_coursechapters_meta( + request: Request, + course_id: str, + coursechapters_metadata: CourseChapterMetaData, + current_user: PublicUser, +): courses = request.app.db["courses"] + await verify_rights(request, course_id, current_user, "update") + # update chapters in course - await courses.update_one({"course_id": course_id}, { - "$set": {"chapters": coursechapters_metadata.chapterOrder}}) + await courses.update_one( + {"course_id": course_id}, + {"$set": {"chapters": coursechapters_metadata.chapterOrder}}, + ) if coursechapters_metadata.chapters is not None: - for coursechapter_id, chapter_metadata in coursechapters_metadata.chapters.items(): - filter_query = { - "chapters_content.coursechapter_id": coursechapter_id} + for ( + coursechapter_id, + chapter_metadata, + ) in coursechapters_metadata.chapters.items(): + filter_query = {"chapters_content.coursechapter_id": coursechapter_id} update_query = { "$set": { "chapters_content.$.activities": chapter_metadata["activityIds"] @@ -213,30 +290,57 @@ async def update_coursechapters_meta(request: Request, course_id: str, coursecha result = await courses.update_one(filter_query, update_query) if result.matched_count == 0: # handle error when no documents are matched by the filter query - print( - f"No documents found for course chapter ID {coursechapter_id}") + print(f"No documents found for course chapter ID {coursechapter_id}") return {"detail": "coursechapters metadata updated"} + #### Security #################################################### -async def verify_rights(request: Request, course_id: str, current_user: PublicUser, action: str): +async def verify_rights( + request: Request, + course_id: str, + current_user: PublicUser, + action: Literal["read", "update", "delete"], +): courses = request.app.db["courses"] - + users = request.app.db["users"] + user = await users.find_one({"user_id": current_user.user_id}) course = await courses.find_one({"course_id": course_id}) if not course: raise HTTPException( - status_code=status.HTTP_409_CONFLICT, detail="Course does not exist") + status_code=status.HTTP_409_CONFLICT, detail="Course does not exist" + ) - hasRoleRights = await verify_user_rights_with_roles(request, action, current_user.user_id, course_id, course["org_id"]) - isAuthor = current_user.user_id in course["authors"] + if action == "read": + if current_user.user_id == "anonymous": + await authorization_verify_if_element_is_public( + request, course_id, current_user.user_id, action + ) + else: + users = request.app.db["users"] + user = await users.find_one({"user_id": current_user.user_id}) - if not hasRoleRights and not isAuthor: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, detail="Roles/Ownership : Insufficient rights to perform this action") + await authorization_verify_based_on_roles_and_authorship( + request, + current_user.user_id, + action, + user["roles"], + course_id, + ) + else: + users = request.app.db["users"] + user = await users.find_one({"user_id": current_user.user_id}) + + await authorization_verify_based_on_roles_and_authorship( + request, + current_user.user_id, + action, + user["roles"], + course_id, + ) - return True #### Security #################################################### diff --git a/src/services/courses/collections.py b/src/services/courses/collections.py index f07c97dc..dbbf9be1 100644 --- a/src/services/courses/collections.py +++ b/src/services/courses/collections.py @@ -1,8 +1,8 @@ -from typing import List +from typing import List, Literal from uuid import uuid4 from pydantic import BaseModel +from src.security.rbac.rbac import authorization_verify_based_on_roles_and_authorship from src.services.users.users import PublicUser -from src.security.security import verify_user_rights_with_roles from fastapi import HTTPException, status, Request #### Classes #################################################### @@ -12,11 +12,13 @@ class Collection(BaseModel): name: str description: str courses: List[str] # course_id + public: bool org_id: str # org_id class CollectionInDB(Collection): collection_id: str + authors: List[str] # user_id #### Classes #################################################### @@ -81,7 +83,11 @@ async def create_collection( # generate collection_id with uuid4 collection_id = str(f"collection_{uuid4()}") - collection = CollectionInDB(collection_id=collection_id, **collection_object.dict()) + collection = CollectionInDB( + collection_id=collection_id, + authors=[current_user.user_id], + **collection_object.dict(), + ) collection_in_db = await collections.insert_one(collection.dict()) @@ -169,15 +175,18 @@ async def get_collections( print(org_id) - # get all collections from database without ObjectId - all_collections = ( - collections.find({"org_id": org_id}) - .sort("name", 1) - .skip(10 * (page - 1)) - .limit(limit) - ) - - await verify_collection_rights(request, "*", current_user, "read", org_id) + if current_user.user_id == "anonymous": + all_collections = collections.find( + {"org_id": org_id, "public": True}, {"_id": 0} + ) + else: + # get all collections from database without ObjectId + all_collections = ( + collections.find({"org_id": org_id}) + .sort("name", 1) + .skip(10 * (page - 1)) + .limit(limit) + ) # create list of collections and include courses in each collection collections_list = [] @@ -207,11 +216,12 @@ async def verify_collection_rights( request: Request, collection_id: str, current_user: PublicUser, - action: str, + action: Literal["create", "read", "update", "delete"], org_id: str, ): collections = request.app.db["collections"] - + users = request.app.db["users"] + user = await users.find_one({"user_id": current_user.user_id}) collection = await collections.find_one({"collection_id": collection_id}) if not collection and action != "create" and collection_id != "*": @@ -223,17 +233,9 @@ async def verify_collection_rights( if current_user.user_id == "anonymous" and action == "read": return True - hasRoleRights = await verify_user_rights_with_roles( - request, action, current_user.user_id, collection_id, org_id + await authorization_verify_based_on_roles_and_authorship( + request, current_user.user_id, action, user["roles"], collection_id ) - if not hasRoleRights: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="You do not have rights to this Collection", - ) - - return True - #### Security #################################################### diff --git a/src/services/courses/courses.py b/src/services/courses/courses.py index 8088c41b..eb72b881 100644 --- a/src/services/courses/courses.py +++ b/src/services/courses/courses.py @@ -1,12 +1,16 @@ import json -from typing import List, Optional +from typing import List, Literal, Optional from uuid import uuid4 from pydantic import BaseModel +from src.security.rbac.rbac import ( + authorization_verify_based_on_roles, + authorization_verify_based_on_roles_and_authorship, + authorization_verify_if_element_is_public, +) from src.services.courses.activities.activities import ActivityInDB from src.services.courses.thumbnails import upload_thumbnail from src.services.users.schemas.users import AnonymousUser from src.services.users.users import PublicUser -from src.security.security import verify_user_rights_with_roles from fastapi import HTTPException, Request, status, UploadFile from datetime import datetime @@ -144,7 +148,6 @@ async def get_course_meta(request: Request, course_id: str, current_user: Public trail = await trails.find_one( {"courses.course_id": course_id, "user_id": current_user.user_id} ) - print(trail) if trail: # get only the course where course_id == course_id trail_course = next( @@ -169,6 +172,8 @@ async def create_course( thumbnail_file: UploadFile | None = None, ): courses = request.app.db["courses"] + users = request.app.db["users"] + user = await users.find_one({"user_id": current_user.user_id}) # generate course_id with uuid4 course_id = str(f"course_{uuid4()}") @@ -176,10 +181,16 @@ async def create_course( # TODO(fix) : the implementation here is clearly not the best one (this entire function) course_object.org_id = org_id course_object.chapters_content = [] - await verify_user_rights_with_roles( - request, "create", current_user.user_id, course_id, org_id + + await authorization_verify_based_on_roles( + request, + current_user.user_id, + "create", + user["roles"], + course_id, ) + if thumbnail_file and thumbnail_file.filename: name_in_disk = ( f"{course_id}_thumbnail_{uuid4()}.{thumbnail_file.filename.split('.')[-1]}" @@ -214,12 +225,13 @@ async def update_course_thumbnail( current_user: PublicUser, thumbnail_file: UploadFile | None = None, ): - # verify course rights - await verify_rights(request, course_id, current_user, "update") - courses = request.app.db["courses"] course = await courses.find_one({"course_id": course_id}) + + # verify course rights + await verify_rights(request, course_id, current_user, "update") + # TODO(fix) : the implementation here is clearly not the best one if course: creationDate = course["creationDate"] @@ -254,13 +266,13 @@ async def update_course_thumbnail( async def update_course( request: Request, course_object: Course, course_id: str, current_user: PublicUser ): - # verify course rights - await verify_rights(request, course_id, current_user, "update") - courses = request.app.db["courses"] course = await courses.find_one({"course_id": course_id}) + # verify course rights + await verify_rights(request, course_id, current_user, "update") + if course: creationDate = course["creationDate"] authors = course["authors"] @@ -289,13 +301,13 @@ async def update_course( async def delete_course(request: Request, course_id: str, current_user: PublicUser): - # verify course rights - await verify_rights(request, course_id, current_user, "delete") - courses = request.app.db["courses"] course = await courses.find_one({"course_id": course_id}) + # verify course rights + await verify_rights(request, course_id, current_user, "delete") + if not course: raise HTTPException( status_code=status.HTTP_409_CONFLICT, detail="Course does not exist" @@ -364,41 +376,35 @@ async def verify_rights( request: Request, course_id: str, current_user: PublicUser | AnonymousUser, - action: str, + action: Literal["create", "read", "update", "delete"], ): - courses = request.app.db["courses"] + if action == "read": + if current_user.user_id == "anonymous": + await authorization_verify_if_element_is_public( + request, course_id, current_user.user_id, action + ) + else: + users = request.app.db["users"] + user = await users.find_one({"user_id": current_user.user_id}) - course = await courses.find_one({"course_id": course_id}) + await authorization_verify_based_on_roles_and_authorship( + request, + current_user.user_id, + action, + user["roles"], + course_id, + ) + else: + users = request.app.db["users"] + user = await users.find_one({"user_id": current_user.user_id}) - isAuthor = current_user.user_id in course["authors"] - - if isAuthor: - return True - - if ( - current_user.user_id == "anonymous" - and course["public"] is True - and action == "read" - ): - return True - - if not course: - raise HTTPException( - status_code=status.HTTP_409_CONFLICT, - detail="Course/CourseChapter does not exist", + await authorization_verify_based_on_roles_and_authorship( + request, + current_user.user_id, + action, + user["roles"], + course_id, ) - hasRoleRights = await verify_user_rights_with_roles( - request, action, current_user.user_id, course_id, course["org_id"] - ) - - if not hasRoleRights and not isAuthor: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="Roles/Ownership : Insufficient rights to perform this action", - ) - - return True - #### Security #################################################### diff --git a/src/services/orgs/orgs.py b/src/services/orgs/orgs.py index af8f1efe..17913554 100644 --- a/src/services/orgs/orgs.py +++ b/src/services/orgs/orgs.py @@ -1,5 +1,7 @@ import json +from typing import Literal from uuid import uuid4 +from src.security.rbac.rbac import authorization_verify_based_on_roles from src.services.orgs.logos import upload_org_logo from src.services.orgs.schemas.orgs import ( Organization, @@ -8,7 +10,6 @@ from src.services.orgs.schemas.orgs import ( ) from src.services.users.schemas.users import UserOrganization from src.services.users.users import PublicUser -from src.security.security import verify_user_rights_with_roles from fastapi import HTTPException, UploadFile, status, Request @@ -197,9 +198,12 @@ async def verify_org_rights( request: Request, org_id: str, current_user: PublicUser, - action: str, + action: Literal["create", "read", "update", "delete"], ): orgs = request.app.db["organizations"] + users = request.app.db["users"] + + user = await users.find_one({"user_id": current_user.user_id}) org = await orgs.find_one({"org_id": org_id}) @@ -208,17 +212,9 @@ async def verify_org_rights( status_code=status.HTTP_409_CONFLICT, detail="Organization does not exist" ) - hasRoleRights = await verify_user_rights_with_roles( - request, action, current_user.user_id, org_id, org_id + await authorization_verify_based_on_roles( + request, current_user.user_id, action, user["roles"], org_id ) - if not hasRoleRights: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="You do not have rights to this organization", - ) - - return True - #### Security #################################################### diff --git a/src/services/users/schemas/users.py b/src/services/users/schemas/users.py index 98075579..940ec703 100644 --- a/src/services/users/schemas/users.py +++ b/src/services/users/schemas/users.py @@ -41,6 +41,9 @@ class UserInDB(User): creation_date: str update_date: str + def __getitem__(self, item): + return getattr(self, item) + diff --git a/src/services/users/users.py b/src/services/users/users.py index ef57bc56..5e0e5745 100644 --- a/src/services/users/users.py +++ b/src/services/users/users.py @@ -2,24 +2,39 @@ from datetime import datetime from typing import Literal from uuid import uuid4 from fastapi import HTTPException, Request, status +from src.security.rbac.rbac import authorization_verify_based_on_roles from src.security.security import security_hash_password, security_verify_password -from src.services.users.schemas.users import PasswordChangeForm, PublicUser, User, UserOrganization, UserRolesInOrganization, UserWithPassword, UserInDB +from src.services.users.schemas.users import ( + PasswordChangeForm, + PublicUser, + User, + UserOrganization, + UserRolesInOrganization, + UserWithPassword, + UserInDB, +) -async def create_user(request: Request, current_user: PublicUser | None, user_object: UserWithPassword, org_slug: str): +async def create_user( + request: Request, + current_user: PublicUser | None, + user_object: UserWithPassword, + org_slug: str, +): users = request.app.db["users"] isUsernameAvailable = await users.find_one({"username": user_object.username}) isEmailAvailable = await users.find_one({"email": user_object.email}) - if isUsernameAvailable: raise HTTPException( - status_code=status.HTTP_409_CONFLICT, detail="Username already exists") - + status_code=status.HTTP_409_CONFLICT, detail="Username already exists" + ) + if isEmailAvailable: raise HTTPException( - status_code=status.HTTP_409_CONFLICT, detail="Email already exists") + status_code=status.HTTP_409_CONFLICT, detail="Email already exists" + ) # Generate user_id with uuid4 user_id = str(f"user_{uuid4()}") @@ -33,7 +48,7 @@ async def create_user(request: Request, current_user: PublicUser | None, user_o user_object.username = user_object.username.lower() user_object.password = await security_hash_password(user_object.password) - # Get org_id from org_slug + # Get org_id from org_slug orgs = request.app.db["organizations"] # Check if the org exists @@ -42,10 +57,11 @@ async def create_user(request: Request, current_user: PublicUser | None, user_o # If the org does not exist, raise an error if not isOrgExists: raise HTTPException( - status_code=status.HTTP_409_CONFLICT, detail="You are trying to create a user in an organization that does not exist") - + status_code=status.HTTP_409_CONFLICT, + detail="You are trying to create a user in an organization that does not exist", + ) + org_id = isOrgExists["org_id"] - # Create initial orgs list with the org_id passed in orgs = [UserOrganization(org_id=org_id, org_role="member")] @@ -54,8 +70,14 @@ async def create_user(request: Request, current_user: PublicUser | None, user_o roles = [UserRolesInOrganization(role_id="role_member", org_id=org_id)] # Create the user - user = UserInDB(user_id=user_id, creation_date=str(datetime.now()), - update_date=str(datetime.now()), orgs=orgs, roles=roles, **user_object.dict()) + user = UserInDB( + user_id=user_id, + creation_date=str(datetime.now()), + update_date=str(datetime.now()), + orgs=orgs, + roles=roles, + **user_object.dict(), + ) # Insert the user into the database await users.insert_one(user.dict()) @@ -75,12 +97,15 @@ async def read_user(request: Request, current_user: PublicUser, user_id: str): # If the user does not exist, raise an error if not isUserExists: raise HTTPException( - status_code=status.HTTP_409_CONFLICT, detail="User does not exist") + status_code=status.HTTP_409_CONFLICT, detail="User does not exist" + ) return User(**isUserExists) -async def update_user(request: Request, user_id: str, user_object: User,current_user: PublicUser): +async def update_user( + request: Request, user_id: str, user_object: User, current_user: PublicUser +): users = request.app.db["users"] # Verify rights @@ -92,7 +117,8 @@ async def update_user(request: Request, user_id: str, user_object: User,current if not isUserExists: raise HTTPException( - status_code=status.HTTP_409_CONFLICT, detail="User does not exist") + status_code=status.HTTP_409_CONFLICT, detail="User does not exist" + ) # okay if username is not changed if isUserExists["username"] == user_object.username: @@ -101,11 +127,13 @@ async def update_user(request: Request, user_id: str, user_object: User,current else: if isUsernameAvailable: raise HTTPException( - status_code=status.HTTP_409_CONFLICT, detail="Username already used") - + status_code=status.HTTP_409_CONFLICT, detail="Username already used" + ) + if isEmailAvailable: raise HTTPException( - status_code=status.HTTP_409_CONFLICT, detail="Email already used") + status_code=status.HTTP_409_CONFLICT, detail="Email already used" + ) updated_user = {"$set": user_object.dict()} users.update_one({"user_id": user_id}, updated_user) @@ -113,8 +141,12 @@ async def update_user(request: Request, user_id: str, user_object: User,current return User(**user_object.dict()) - -async def update_user_password(request: Request, current_user: PublicUser, user_id: str, password_change_form: PasswordChangeForm): +async def update_user_password( + request: Request, + current_user: PublicUser, + user_id: str, + password_change_form: PasswordChangeForm, +): users = request.app.db["users"] isUserExists = await users.find_one({"user_id": user_id}) @@ -124,11 +156,15 @@ async def update_user_password(request: Request, current_user: PublicUser, user_ if not isUserExists: raise HTTPException( - status_code=status.HTTP_409_CONFLICT, detail="User does not exist") + status_code=status.HTTP_409_CONFLICT, detail="User does not exist" + ) - if not await security_verify_password(password_change_form.old_password, isUserExists["password"]): + if not await security_verify_password( + password_change_form.old_password, isUserExists["password"] + ): raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, detail="Wrong password") + status_code=status.HTTP_401_UNAUTHORIZED, detail="Wrong password" + ) new_password = await security_hash_password(password_change_form.new_password) @@ -148,7 +184,8 @@ async def delete_user(request: Request, current_user: PublicUser, user_id: str): if not isUserExists: raise HTTPException( - status_code=status.HTTP_409_CONFLICT, detail="User does not exist") + status_code=status.HTTP_409_CONFLICT, detail="User does not exist" + ) await users.delete_one({"user_id": user_id}) @@ -157,18 +194,21 @@ async def delete_user(request: Request, current_user: PublicUser, user_id: str): # Utils & Security functions + async def security_get_user(request: Request, email: str): users = request.app.db["users"] - user = await users.find_one({"email": email}) if not user: raise HTTPException( - status_code=status.HTTP_409_CONFLICT, detail="User with Email does not exist") + status_code=status.HTTP_409_CONFLICT, + detail="User with Email does not exist", + ) return UserInDB(**user) + async def get_userid_by_username(request: Request, username: str): users = request.app.db["users"] @@ -176,10 +216,12 @@ async def get_userid_by_username(request: Request, username: str): if not user: raise HTTPException( - status_code=status.HTTP_409_CONFLICT, detail="User does not exist") + status_code=status.HTTP_409_CONFLICT, detail="User does not exist" + ) return user["user_id"] + async def get_user_by_userid(request: Request, user_id: str): users = request.app.db["users"] @@ -187,32 +229,36 @@ async def get_user_by_userid(request: Request, user_id: str): if not user: raise HTTPException( - status_code=status.HTTP_409_CONFLICT, detail="User does not exist") + status_code=status.HTTP_409_CONFLICT, detail="User does not exist" + ) user = User(**user) return user + async def get_profile_metadata(request: Request, user): users = request.app.db["users"] request.app.db["roles"] - user = await users.find_one({"user_id": user['user_id']}) + user = await users.find_one({"user_id": user["user_id"]}) if not user: raise HTTPException( - status_code=status.HTTP_409_CONFLICT, detail="User does not exist") + status_code=status.HTTP_409_CONFLICT, detail="User does not exist" + ) - - - return { - "user_object": PublicUser(**user), - "roles": "random" - } + return {"user_object": PublicUser(**user), "roles": "random"} # Verification of the user's permissions on the roles -async def verify_user_rights_on_user(request: Request, current_user: PublicUser, action: Literal["create", "read", "update", "delete"], user_id: str): + +async def verify_user_rights_on_user( + request: Request, + current_user: PublicUser, + action: Literal["create", "read", "update", "delete"], + user_id: str, +): users = request.app.db["users"] user = UserInDB(**await users.find_one({"user_id": user_id})) @@ -235,11 +281,12 @@ async def verify_user_rights_on_user(request: Request, current_user: PublicUser, for org in current_user.orgs: if org.org_id in [org.org_id for org in user.orgs]: - if org.org_role == "owner": return True - # TODO: Verify user roles on the org + await authorization_verify_based_on_roles( + request, current_user.user_id, "update", user["roles"], user_id + ) return False @@ -249,8 +296,9 @@ async def verify_user_rights_on_user(request: Request, current_user: PublicUser, for org in current_user.orgs: if org.org_id in [org.org_id for org in user.orgs]: - if org.org_role == "owner": return True - # TODO: Verify user roles on the org + await authorization_verify_based_on_roles( + request, current_user.user_id, "update", user["roles"], user_id + ) From 42c99f3939f93fe1ee6dcbd9a5a8c0da6cce33f5 Mon Sep 17 00:00:00 2001 From: swve Date: Thu, 20 Jul 2023 01:42:20 +0200 Subject: [PATCH 4/5] feat: additional verification for anon users --- src/security/rbac/rbac.py | 9 ++++++++- src/services/courses/activities/activities.py | 5 +++++ src/services/courses/chapters.py | 5 +++++ src/services/courses/collections.py | 4 +++- src/services/courses/courses.py | 3 +++ src/services/orgs/orgs.py | 7 ++++++- src/services/roles/roles.py | 3 +++ src/services/users/schemas/users.py | 3 +++ src/services/users/users.py | 11 ++++++++++- 9 files changed, 46 insertions(+), 4 deletions(-) diff --git a/src/security/rbac/rbac.py b/src/security/rbac/rbac.py index 8f131cca..fbd81bd1 100644 --- a/src/security/rbac/rbac.py +++ b/src/security/rbac/rbac.py @@ -79,7 +79,6 @@ async def authorization_verify_based_on_roles( element_id: str, ): element_type = await check_element_type(element_id) - print(element_type) element = request.app.db[element_type] roles = request.app.db["roles"] @@ -125,3 +124,11 @@ async def authorization_verify_based_on_roles_and_authorship( status_code=status.HTTP_403_FORBIDDEN, detail="User rights (roles & authorship) : You don't have the right to perform this action", ) + + +async def authorization_verify_if_user_is_anon(user_id: str): + if user_id == "anonymous": + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="You should be logged in to perform this action", + ) diff --git a/src/services/courses/activities/activities.py b/src/services/courses/activities/activities.py index 9551518d..8a8a0aa3 100644 --- a/src/services/courses/activities/activities.py +++ b/src/services/courses/activities/activities.py @@ -3,6 +3,7 @@ from pydantic import BaseModel from src.security.rbac.rbac import ( authorization_verify_based_on_roles, authorization_verify_if_element_is_public, + authorization_verify_if_user_is_anon, ) from src.services.users.schemas.users import AnonymousUser, PublicUser from fastapi import HTTPException, status, Request @@ -214,6 +215,8 @@ async def verify_rights( users = request.app.db["users"] user = await users.find_one({"user_id": current_user.user_id}) + await authorization_verify_if_user_is_anon(current_user.user_id) + await authorization_verify_based_on_roles( request, current_user.user_id, @@ -225,6 +228,8 @@ async def verify_rights( users = request.app.db["users"] user = await users.find_one({"user_id": current_user.user_id}) + await authorization_verify_if_user_is_anon(current_user.user_id) + await authorization_verify_based_on_roles( request, current_user.user_id, diff --git a/src/services/courses/chapters.py b/src/services/courses/chapters.py index ea35905d..470730f8 100644 --- a/src/services/courses/chapters.py +++ b/src/services/courses/chapters.py @@ -7,6 +7,7 @@ from src.security.rbac.rbac import ( authorization_verify_based_on_roles, authorization_verify_based_on_roles_and_authorship, authorization_verify_if_element_is_public, + authorization_verify_if_user_is_anon, ) from src.services.courses.courses import Course from src.services.courses.activities.activities import ActivityInDB @@ -323,6 +324,8 @@ async def verify_rights( users = request.app.db["users"] user = await users.find_one({"user_id": current_user.user_id}) + await authorization_verify_if_user_is_anon(current_user.user_id) + await authorization_verify_based_on_roles_and_authorship( request, current_user.user_id, @@ -333,6 +336,8 @@ async def verify_rights( else: users = request.app.db["users"] user = await users.find_one({"user_id": current_user.user_id}) + + await authorization_verify_if_user_is_anon(current_user.user_id) await authorization_verify_based_on_roles_and_authorship( request, diff --git a/src/services/courses/collections.py b/src/services/courses/collections.py index dbbf9be1..3829e079 100644 --- a/src/services/courses/collections.py +++ b/src/services/courses/collections.py @@ -1,7 +1,7 @@ from typing import List, Literal from uuid import uuid4 from pydantic import BaseModel -from src.security.rbac.rbac import authorization_verify_based_on_roles_and_authorship +from src.security.rbac.rbac import authorization_verify_based_on_roles_and_authorship, authorization_verify_if_user_is_anon from src.services.users.users import PublicUser from fastapi import HTTPException, status, Request @@ -233,6 +233,8 @@ async def verify_collection_rights( if current_user.user_id == "anonymous" and action == "read": return True + await authorization_verify_if_user_is_anon(current_user.user_id) + await authorization_verify_based_on_roles_and_authorship( request, current_user.user_id, action, user["roles"], collection_id ) diff --git a/src/services/courses/courses.py b/src/services/courses/courses.py index eb72b881..86841d26 100644 --- a/src/services/courses/courses.py +++ b/src/services/courses/courses.py @@ -6,6 +6,7 @@ from src.security.rbac.rbac import ( authorization_verify_based_on_roles, authorization_verify_based_on_roles_and_authorship, authorization_verify_if_element_is_public, + authorization_verify_if_user_is_anon, ) from src.services.courses.activities.activities import ActivityInDB from src.services.courses.thumbnails import upload_thumbnail @@ -398,6 +399,8 @@ async def verify_rights( users = request.app.db["users"] user = await users.find_one({"user_id": current_user.user_id}) + await authorization_verify_if_user_is_anon(current_user.user_id) + await authorization_verify_based_on_roles_and_authorship( request, current_user.user_id, diff --git a/src/services/orgs/orgs.py b/src/services/orgs/orgs.py index 17913554..57bbd437 100644 --- a/src/services/orgs/orgs.py +++ b/src/services/orgs/orgs.py @@ -1,7 +1,10 @@ import json from typing import Literal from uuid import uuid4 -from src.security.rbac.rbac import authorization_verify_based_on_roles +from src.security.rbac.rbac import ( + authorization_verify_based_on_roles, + authorization_verify_if_user_is_anon, +) from src.services.orgs.logos import upload_org_logo from src.services.orgs.schemas.orgs import ( Organization, @@ -212,6 +215,8 @@ async def verify_org_rights( status_code=status.HTTP_409_CONFLICT, detail="Organization does not exist" ) + await authorization_verify_if_user_is_anon(current_user.user_id) + await authorization_verify_based_on_roles( request, current_user.user_id, action, user["roles"], org_id ) diff --git a/src/services/roles/roles.py b/src/services/roles/roles.py index 2ef3ee34..cc61c452 100644 --- a/src/services/roles/roles.py +++ b/src/services/roles/roles.py @@ -1,5 +1,6 @@ from typing import Literal from uuid import uuid4 +from src.security.rbac.rbac import authorization_verify_if_user_is_anon from src.services.roles.schemas.roles import Role, RoleInDB from src.services.users.schemas.users import PublicUser from fastapi import HTTPException, status, Request @@ -85,6 +86,8 @@ async def verify_user_permissions_on_roles( status_code=status.HTTP_401_UNAUTHORIZED, detail="Roles : Not authenticated" ) + await authorization_verify_if_user_is_anon(current_user.user_id) + if action == "create": if "owner" in [org.org_role for org in current_user.orgs]: return True diff --git a/src/services/users/schemas/users.py b/src/services/users/schemas/users.py index 940ec703..2dea0e97 100644 --- a/src/services/users/schemas/users.py +++ b/src/services/users/schemas/users.py @@ -57,6 +57,9 @@ class PublicUser(User): class AnonymousUser(BaseModel): user_id: str = "anonymous" username: str = "anonymous" + roles: list[UserRolesInOrganization] = [ + UserRolesInOrganization(org_id="anonymous", role_id="role_anonymous") + ] diff --git a/src/services/users/users.py b/src/services/users/users.py index 5e0e5745..da65b9fd 100644 --- a/src/services/users/users.py +++ b/src/services/users/users.py @@ -2,7 +2,7 @@ from datetime import datetime from typing import Literal from uuid import uuid4 from fastapi import HTTPException, Request, status -from src.security.rbac.rbac import authorization_verify_based_on_roles +from src.security.rbac.rbac import authorization_verify_based_on_roles, authorization_verify_if_user_is_anon from src.security.security import security_hash_password, security_verify_password from src.services.users.schemas.users import ( PasswordChangeForm, @@ -266,6 +266,9 @@ async def verify_user_rights_on_user( return True if action == "read": + + await authorization_verify_if_user_is_anon(current_user.user_id) + if current_user.user_id == user_id: return True @@ -276,6 +279,9 @@ async def verify_user_rights_on_user( return False if action == "update": + + await authorization_verify_if_user_is_anon(current_user.user_id) + if current_user.user_id == user_id: return True @@ -291,6 +297,9 @@ async def verify_user_rights_on_user( return False if action == "delete": + + await authorization_verify_if_user_is_anon(current_user.user_id) + if current_user.user_id == user_id: return True From fac56f0c7efb2e818850f3dad1a348c0343ddbfb Mon Sep 17 00:00:00 2001 From: swve Date: Thu, 20 Jul 2023 02:02:08 +0200 Subject: [PATCH 5/5] feat: add org roles verification --- .../Security/AuthenticatedClientElement.tsx | 4 +- src/security/rbac/rbac.py | 43 +++++++++++++------ 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/front/components/Security/AuthenticatedClientElement.tsx b/front/components/Security/AuthenticatedClientElement.tsx index e43e41e6..bc793b0e 100644 --- a/front/components/Security/AuthenticatedClientElement.tsx +++ b/front/components/Security/AuthenticatedClientElement.tsx @@ -24,9 +24,9 @@ export const AuthenticatedClientElement = (props: AuthenticatedClientElementProp const user_roles = auth.userInfo.user_object.roles; const org_role = org_roles.find((org: any) => org.org_id == org_id); const user_role = user_roles.find((role: any) => role.org_id == org_id); - + if (org_role && user_role) { - if (org_roles_values.includes(org_role.org_role) && user_roles_values.includes(user_role.role_id)) { + if (org_roles_values.includes(org_role.org_role) || user_roles_values.includes(user_role.role_id)) { return true; } else { diff --git a/src/security/rbac/rbac.py b/src/security/rbac/rbac.py index fbd81bd1..ea2e1838 100644 --- a/src/security/rbac/rbac.py +++ b/src/security/rbac/rbac.py @@ -63,10 +63,7 @@ async def authorization_verify_if_user_is_author( if user_id in element["authors"]: return True else: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="User rights (author) : You don't have the right to perform this action", - ) + return False else: return False @@ -90,16 +87,34 @@ async def authorization_verify_based_on_roles( roles_id_list = [role["role_id"] for role in roles_list] roles = await roles.find({"role_id": {"$in": roles_id_list}}).to_list(length=100) - # Get the rights of the roles - for role in roles: - role = RoleInDB(**role) - if role.elements[element_type][f"action_{action}"] is True: - return True - else: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="User rights (roles) : You don't have the right to perform this action", - ) + async def checkRoles(): + # Check Roles + for role in roles: + role = RoleInDB(**role) + if role.elements[element_type][f"action_{action}"] is True: + return True + else: + return False + + async def checkOrgRoles(): + # Check Org Roles + users = request.app.db["users"] + user = await users.find_one({"user_id": user_id}) + if element is not None: + for org in user["orgs"]: + if org["org_id"] == element["org_id"]: + if org["org_role"] == "owner" or org["org_role"] == "editor": + return True + else: + return False + + if await checkRoles() or await checkOrgRoles(): + return True + else: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="User rights (roless) : You don't have the right to perform this action", + ) async def authorization_verify_based_on_roles_and_authorship(