From 759a144fe9b8e136073dd948b671f0414f1a6ca5 Mon Sep 17 00:00:00 2001 From: swve Date: Mon, 19 Jun 2023 00:40:12 +0200 Subject: [PATCH 1/5] chore: upgrade typescript & react types --- front/package-lock.json | 30 +++++++++++++++--------------- front/package.json | 4 ++-- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/front/package-lock.json b/front/package-lock.json index 6e8c8ac1..d2b6e54e 100644 --- a/front/package-lock.json +++ b/front/package-lock.json @@ -44,7 +44,7 @@ }, "devDependencies": { "@types/node": "18.7.18", - "@types/react": "18.0.20", + "@types/react": "18.2.8", "@types/react-beautiful-dnd": "^13.1.2", "@types/react-dom": "18.0.6", "@types/react-katex": "^3.0.0", @@ -55,7 +55,7 @@ "eslint-config-next": "^13.0.6", "postcss": "^8.4.23", "tailwindcss": "^3.3.2", - "typescript": "4.8.3" + "typescript": "5.1.3" } }, "node_modules/@alloc/quick-lru": { @@ -3599,9 +3599,9 @@ "integrity": "sha512-JCB8C6SnDoQf0cNycqd/35A7MjcnK+ZTqE7judS6o7utxUCg6imJg3QK2qzHKszlTjcj2cn+NwMB2i96ubpj7w==" }, "node_modules/@types/react": { - "version": "18.0.20", - "resolved": "https://registry.npmjs.org/@types/react/-/react-18.0.20.tgz", - "integrity": "sha512-MWul1teSPxujEHVwZl4a5HxQ9vVNsjTchVA+xRqv/VYGCuKGAU6UhfrTdF5aBefwD1BHUD8i/zq+O/vyCm/FrA==", + "version": "18.2.8", + "resolved": "https://registry.npmjs.org/@types/react/-/react-18.2.8.tgz", + "integrity": "sha512-lTyWUNrd8ntVkqycEEplasWy2OxNlShj3zqS0LuB1ENUGis5HodmhM7DtCoUGbxj3VW/WsGA0DUhpG6XrM7gPA==", "dependencies": { "@types/prop-types": "*", "@types/scheduler": "*", @@ -8181,16 +8181,16 @@ } }, "node_modules/typescript": { - "version": "4.8.3", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-4.8.3.tgz", - "integrity": "sha512-goMHfm00nWPa8UvR/CPSvykqf6dVV8x/dp0c5mFTMTIu0u0FlGWRioyy7Nn0PGAdHxpJZnuO/ut+PpQ8UiHAig==", + "version": "5.1.3", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.1.3.tgz", + "integrity": "sha512-XH627E9vkeqhlZFQuL+UsyAXEnibT0kWR2FWONlr4sTjvxyJYnyefgrkyECLzM5NenmKzRAy2rR/OlYLA1HkZw==", "dev": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" }, "engines": { - "node": ">=4.2.0" + "node": ">=14.17" } }, "node_modules/unbox-primitive": { @@ -11084,9 +11084,9 @@ "integrity": "sha512-JCB8C6SnDoQf0cNycqd/35A7MjcnK+ZTqE7judS6o7utxUCg6imJg3QK2qzHKszlTjcj2cn+NwMB2i96ubpj7w==" }, "@types/react": { - "version": "18.0.20", - "resolved": "https://registry.npmjs.org/@types/react/-/react-18.0.20.tgz", - "integrity": "sha512-MWul1teSPxujEHVwZl4a5HxQ9vVNsjTchVA+xRqv/VYGCuKGAU6UhfrTdF5aBefwD1BHUD8i/zq+O/vyCm/FrA==", + "version": "18.2.8", + "resolved": "https://registry.npmjs.org/@types/react/-/react-18.2.8.tgz", + "integrity": "sha512-lTyWUNrd8ntVkqycEEplasWy2OxNlShj3zqS0LuB1ENUGis5HodmhM7DtCoUGbxj3VW/WsGA0DUhpG6XrM7gPA==", "requires": { "@types/prop-types": "*", "@types/scheduler": "*", @@ -14341,9 +14341,9 @@ "dev": true }, "typescript": { - "version": "4.8.3", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-4.8.3.tgz", - "integrity": "sha512-goMHfm00nWPa8UvR/CPSvykqf6dVV8x/dp0c5mFTMTIu0u0FlGWRioyy7Nn0PGAdHxpJZnuO/ut+PpQ8UiHAig==", + "version": "5.1.3", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.1.3.tgz", + "integrity": "sha512-XH627E9vkeqhlZFQuL+UsyAXEnibT0kWR2FWONlr4sTjvxyJYnyefgrkyECLzM5NenmKzRAy2rR/OlYLA1HkZw==", "dev": true }, "unbox-primitive": { diff --git a/front/package.json b/front/package.json index 68ad246c..7c1abd9e 100644 --- a/front/package.json +++ b/front/package.json @@ -45,7 +45,7 @@ }, "devDependencies": { "@types/node": "18.7.18", - "@types/react": "18.0.20", + "@types/react": "18.2.8", "@types/react-beautiful-dnd": "^13.1.2", "@types/react-dom": "18.0.6", "@types/react-katex": "^3.0.0", @@ -56,6 +56,6 @@ "eslint-config-next": "^13.0.6", "postcss": "^8.4.23", "tailwindcss": "^3.3.2", - "typescript": "4.8.3" + "typescript": "5.1.3" } } From d4497e03fbcbb5e5da5be9dc109d87430d390a8e Mon Sep 17 00:00:00 2001 From: swve Date: Mon, 19 Jun 2023 17:51:21 +0200 Subject: [PATCH 2/5] feat: org scope collections --- .../(withmenu)/collections/admin.tsx | 3 - .../[orgslug]/(withmenu)/collections/page.tsx | 8 +- front/app/orgs/[orgslug]/(withmenu)/page.tsx | 3 +- front/services/courses/collections.ts | 5 +- src/routers/courses/collections.py | 50 ++++++-- src/services/courses/collections.py | 111 ++++++++++++------ 6 files changed, 126 insertions(+), 54 deletions(-) diff --git a/front/app/orgs/[orgslug]/(withmenu)/collections/admin.tsx b/front/app/orgs/[orgslug]/(withmenu)/collections/admin.tsx index bb5b9c8d..0acef9ba 100644 --- a/front/app/orgs/[orgslug]/(withmenu)/collections/admin.tsx +++ b/front/app/orgs/[orgslug]/(withmenu)/collections/admin.tsx @@ -9,10 +9,7 @@ import React from 'react' const CollectionAdminEditsArea = (props: any) => { const org_roles_values = ["admin", "owner"]; const user_roles_values = ["role_admin"]; - console.log("props: ", props); - const auth: any = React.useContext(AuthContext); - console.log("auth: ", auth); // this is amazingly terrible code, but gotta release that MVP diff --git a/front/app/orgs/[orgslug]/(withmenu)/collections/page.tsx b/front/app/orgs/[orgslug]/(withmenu)/collections/page.tsx index 930b9b1c..03035fec 100644 --- a/front/app/orgs/[orgslug]/(withmenu)/collections/page.tsx +++ b/front/app/orgs/[orgslug]/(withmenu)/collections/page.tsx @@ -22,7 +22,6 @@ export async function generateMetadata( const access_token_cookie: any = cookieStore.get('access_token_cookie'); // Get Org context information const org = await getOrganizationContextInfo(params.orgslug, { revalidate: 1800, tags: ['organizations'] }); - return { title: `Collections — ${org.name}`, description: `Collections of courses from ${org.name}`, @@ -38,10 +37,9 @@ const CollectionsPage = async (params: any) => { const cookieStore = cookies(); const access_token_cookie: any = cookieStore.get('access_token_cookie'); const orgslug = params.params.orgslug; - const collections = await getOrgCollectionsWithAuthHeader(access_token_cookie ? access_token_cookie.value : null); - - - + const org = await getOrganizationContextInfo(orgslug, { revalidate: 1800, tags: ['organizations'] }); + const org_id = org.org_id; + const collections = await getOrgCollectionsWithAuthHeader(org_id, access_token_cookie ? access_token_cookie.value : null); return (
diff --git a/front/app/orgs/[orgslug]/(withmenu)/page.tsx b/front/app/orgs/[orgslug]/(withmenu)/page.tsx index f4d48efc..b51d1a7d 100644 --- a/front/app/orgs/[orgslug]/(withmenu)/page.tsx +++ b/front/app/orgs/[orgslug]/(withmenu)/page.tsx @@ -35,7 +35,8 @@ const OrgHomePage = async (params: any) => { 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); - const collections = await getOrgCollectionsWithAuthHeader(access_token_cookie ? access_token_cookie.value : null); + const org = await getOrganizationContextInfo(orgslug, { revalidate: 1800, tags: ['organizations'] }); + const collections = await getOrgCollectionsWithAuthHeader(org.org_id, access_token_cookie ? access_token_cookie.value : null); // function to remove "course_" from the course_id diff --git a/front/services/courses/collections.ts b/front/services/courses/collections.ts index ad0bf20c..26ba6f92 100644 --- a/front/services/courses/collections.ts +++ b/front/services/courses/collections.ts @@ -19,7 +19,6 @@ export async function createCollection(collection: any) { return res; } - // Get a colletion by id export async function getCollectionById(collection_id: any) { const result: any = await fetch(`${getAPIUrl()}collections/${collection_id}`, { next: { revalidate: 10 } }); @@ -41,8 +40,8 @@ export async function getOrgCollections() { return res; } -export async function getOrgCollectionsWithAuthHeader(access_token: string) { - const result: any = await fetch(`${getAPIUrl()}collections/page/1/limit/10`, RequestBodyWithAuthHeader("GET", null, { revalidate: 3 }, access_token)); +export async function getOrgCollectionsWithAuthHeader(org_id: string, access_token: string) { + const result: any = await fetch(`${getAPIUrl()}collections/org_id/${org_id}/page/1/limit/10`, RequestBodyWithAuthHeader("GET", null, { revalidate: 3 }, access_token)); const res = await errorHandling(result); return res; } diff --git a/src/routers/courses/collections.py b/src/routers/courses/collections.py index 3dc39569..87dfe81c 100644 --- a/src/routers/courses/collections.py +++ b/src/routers/courses/collections.py @@ -1,14 +1,25 @@ from fastapi import APIRouter, Depends, Request from src.security.auth import get_current_user from src.services.users.users import PublicUser -from src.services.courses.collections import Collection, create_collection, get_collection, get_collections, update_collection, delete_collection +from src.services.courses.collections import ( + Collection, + create_collection, + get_collection, + get_collections, + update_collection, + delete_collection, +) router = APIRouter() @router.post("/") -async def api_create_collection(request: Request,collection_object: Collection, current_user: PublicUser = Depends(get_current_user)): +async def api_create_collection( + request: Request, + collection_object: Collection, + current_user: PublicUser = Depends(get_current_user), +): """ Create new Collection """ @@ -16,31 +27,52 @@ async def api_create_collection(request: Request,collection_object: Collection, @router.get("/{collection_id}") -async def api_get_collection(request: Request,collection_id: str, current_user: PublicUser = Depends(get_current_user)): +async def api_get_collection( + request: Request, + collection_id: str, + current_user: PublicUser = Depends(get_current_user), +): """ Get single collection by ID """ return await get_collection(request, collection_id, current_user) -@router.get("/page/{page}/limit/{limit}") -async def api_get_collections_by(request: Request,page: int, limit: int, current_user: PublicUser = Depends(get_current_user)): +@router.get("/org_id/{org_id}/page/{page}/limit/{limit}") +async def api_get_collections_by( + request: Request, + page: int, + limit: int, + org_id: str, + current_user: PublicUser = Depends(get_current_user), +): """ Get collections by page and limit """ - return await get_collections(request, page, limit) + return await get_collections(request, org_id, current_user, page, limit) @router.put("/{collection_id}") -async def api_update_collection(request: Request,collection_object: Collection, collection_id: str, current_user: PublicUser = Depends(get_current_user)): +async def api_update_collection( + request: Request, + collection_object: Collection, + collection_id: str, + current_user: PublicUser = Depends(get_current_user), +): """ Update collection by ID """ - return await update_collection(request, collection_object, collection_id, current_user) + return await update_collection( + request, collection_object, collection_id, current_user + ) @router.delete("/{collection_id}") -async def api_delete_collection(request: Request,collection_id: str, current_user: PublicUser = Depends(get_current_user)): +async def api_delete_collection( + request: Request, + collection_id: str, + current_user: PublicUser = Depends(get_current_user), +): """ Delete collection by ID """ diff --git a/src/services/courses/collections.py b/src/services/courses/collections.py index 8f482f1c..aabd502c 100644 --- a/src/services/courses/collections.py +++ b/src/services/courses/collections.py @@ -2,7 +2,7 @@ from typing import List from uuid import uuid4 from pydantic import BaseModel from src.services.users.users import PublicUser -from src.security.security import * +from src.security.security import verify_user_rights_with_roles from fastapi import HTTPException, status, Request #### Classes #################################################### @@ -25,7 +25,10 @@ class CollectionInDB(Collection): # CRUD #################################################### -async def get_collection(request: Request,collection_id: str, current_user: PublicUser): + +async def get_collection( + request: Request, collection_id: str, current_user: PublicUser +): collections = request.app.db["collections"] collection = await collections.find_one({"collection_id": collection_id}) @@ -35,7 +38,8 @@ async def get_collection(request: Request,collection_id: str, current_user: Publ if not collection: raise HTTPException( - status_code=status.HTTP_409_CONFLICT, detail="Collection does not exist") + status_code=status.HTTP_409_CONFLICT, detail="Collection does not exist" + ) collection = Collection(**collection) @@ -44,46 +48,56 @@ async def get_collection(request: Request,collection_id: str, current_user: Publ courseids = [course for course in collection.courses] collection.courses = [] - collection.courses = courses.find( - {"course_id": {"$in": courseids}}, {'_id': 0}) - - collection.courses = [course for course in await collection.courses.to_list(length=100)] + collection.courses = courses.find({"course_id": {"$in": courseids}}, {"_id": 0}) + collection.courses = [ + course for course in await collection.courses.to_list(length=100) + ] return collection -async def create_collection(request: Request,collection_object: Collection, current_user: PublicUser): +async def create_collection( + request: Request, collection_object: Collection, current_user: PublicUser +): collections = request.app.db["collections"] # find if collection already exists using name isCollectionNameAvailable = await collections.find_one( - {"name": collection_object.name}) + {"name": collection_object.name} + ) # TODO # await verify_collection_rights("*", current_user, "create") if isCollectionNameAvailable: raise HTTPException( - status_code=status.HTTP_409_CONFLICT, detail="Collection name already exists") + status_code=status.HTTP_409_CONFLICT, + detail="Collection name already exists", + ) # 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, **collection_object.dict()) collection_in_db = await collections.insert_one(collection.dict()) if not collection_in_db: raise HTTPException( - status_code=status.HTTP_503_SERVICE_UNAVAILABLE, detail="Unavailable database") + status_code=status.HTTP_503_SERVICE_UNAVAILABLE, + detail="Unavailable database", + ) return collection.dict() -async def update_collection(request: Request,collection_object: Collection, collection_id: str, current_user: PublicUser): - +async def update_collection( + request: Request, + collection_object: Collection, + collection_id: str, + current_user: PublicUser, +): # verify collection rights await verify_collection_rights(request, collection_id, current_user, "update") @@ -93,19 +107,23 @@ async def update_collection(request: Request,collection_object: Collection, coll if not collection: raise HTTPException( - status_code=status.HTTP_409_CONFLICT, detail="Collection does not exist") + status_code=status.HTTP_409_CONFLICT, detail="Collection does not exist" + ) updated_collection = CollectionInDB( - collection_id=collection_id, **collection_object.dict()) + collection_id=collection_id, **collection_object.dict() + ) - await collections.update_one({"collection_id": collection_id}, { - "$set": updated_collection.dict()}) + await collections.update_one( + {"collection_id": collection_id}, {"$set": updated_collection.dict()} + ) return Collection(**updated_collection.dict()) -async def delete_collection(request: Request,collection_id: str, current_user: PublicUser): - +async def delete_collection( + request: Request, collection_id: str, current_user: PublicUser +): await verify_collection_rights(request, collection_id, current_user, "delete") collections = request.app.db["collections"] @@ -114,7 +132,8 @@ async def delete_collection(request: Request,collection_id: str, current_user: P if not collection: raise HTTPException( - status_code=status.HTTP_409_CONFLICT, detail="Collection does not exist") + status_code=status.HTTP_409_CONFLICT, detail="Collection does not exist" + ) isDeleted = await collections.delete_one({"collection_id": collection_id}) @@ -122,20 +141,34 @@ async def delete_collection(request: Request,collection_id: str, current_user: P return {"detail": "collection deleted"} else: raise HTTPException( - status_code=status.HTTP_503_SERVICE_UNAVAILABLE, detail="Unavailable database") + status_code=status.HTTP_503_SERVICE_UNAVAILABLE, + detail="Unavailable database", + ) + #################################################### # Misc #################################################### -async def get_collections(request: Request,page: int = 1, limit: int = 10): - ## TODO : auth +async def get_collections( + request: Request, + org_id: str, + current_user: PublicUser, + page: int = 1, + limit: int = 10, +): collections = request.app.db["collections"] # get all collections from database without ObjectId - all_collections = collections.find({}).sort( - "name", 1).skip(10 * (page - 1)).limit(limit) + 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") # create list of collections and include courses in each collection collections_list = [] @@ -148,30 +181,42 @@ async def get_collections(request: Request,page: int = 1, limit: int = 10): courses = request.app.db["courses"] collection.courses = [] collection.courses = courses.find( - {"course_id": {"$in": collection_courses}}, {'_id': 0}) + {"course_id": {"$in": collection_courses}}, {"_id": 0} + ) - collection.courses = [course for course in await collection.courses.to_list(length=100)] + collection.courses = [ + course for course in await collection.courses.to_list(length=100) + ] return collections_list + #### Security #################################################### -async def verify_collection_rights(request: Request,collection_id: str, current_user: PublicUser, action: str): +async def verify_collection_rights( + request: Request, collection_id: str, current_user: PublicUser, action: str +): collections = request.app.db["collections"] collection = await collections.find_one({"collection_id": collection_id}) if not collection and action != "create": raise HTTPException( - status_code=status.HTTP_409_CONFLICT, detail="Collection does not exist") + status_code=status.HTTP_409_CONFLICT, detail="Collection does not exist" + ) - hasRoleRights = await verify_user_rights_with_roles(request, action, current_user.user_id, collection_id, collection["org_id"]) + hasRoleRights = await verify_user_rights_with_roles( + request, action, current_user.user_id, collection_id, collection["org_id"] + ) if not hasRoleRights: raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, detail="You do not have rights to this Collection") + status_code=status.HTTP_403_FORBIDDEN, + detail="You do not have rights to this Collection", + ) return True + #### Security #################################################### From 9c2332961b5ba2234dbb2c3645f6d14a8b3fa17c Mon Sep 17 00:00:00 2001 From: swve Date: Mon, 19 Jun 2023 17:51:39 +0200 Subject: [PATCH 3/5] feat: remove star imports --- src/routers/auth.py | 4 ++-- src/routers/courses/activities.py | 14 ++++++++---- src/routers/users.py | 4 ++-- src/security/auth.py | 8 +++---- src/services/courses/collections.py | 33 ++++++++++++++++++++++------- src/services/courses/courses.py | 4 ++-- src/services/orgs/orgs.py | 11 +++------- src/services/orgs/schemas/orgs.py | 1 - src/services/roles/roles.py | 27 +++++++++++++++++------ 9 files changed, 69 insertions(+), 37 deletions(-) diff --git a/src/routers/auth.py b/src/routers/auth.py index b53ffb1f..4eaa4ba8 100644 --- a/src/routers/auth.py +++ b/src/routers/auth.py @@ -1,8 +1,8 @@ from urllib.request import Request from fastapi import Depends, APIRouter, HTTPException, Response, status, Request from fastapi.security import OAuth2PasswordRequestForm -from src.security.auth import * -from src.services.users.users import * +from src.security.auth import AuthJWT, authenticate_user +from src.services.users.users import PublicUser router = APIRouter() diff --git a/src/routers/courses/activities.py b/src/routers/courses/activities.py index 2694c650..f04af1d7 100644 --- a/src/routers/courses/activities.py +++ b/src/routers/courses/activities.py @@ -1,5 +1,12 @@ from fastapi import APIRouter, Depends, UploadFile, Form, Request -from src.services.courses.activities.activities import * +from src.services.courses.activities.activities import ( + Activity, + create_activity, + get_activity, + get_activities, + update_activity, + delete_activity, +) from src.security.auth import get_current_user from src.services.courses.activities.pdf import create_documentpdf_activity from src.services.courses.activities.video import ( @@ -7,6 +14,7 @@ from src.services.courses.activities.video import ( create_external_video_activity, create_video_activity, ) +from src.services.users.schemas.users import PublicUser router = APIRouter() @@ -104,9 +112,7 @@ async def api_create_external_video_activity( """ Create new activity """ - return await create_external_video_activity( - request, current_user, external_video - ) + return await create_external_video_activity(request, current_user, external_video) @router.post("/documentpdf") diff --git a/src/routers/users.py b/src/routers/users.py index 80b90753..3b4edd1a 100644 --- a/src/routers/users.py +++ b/src/routers/users.py @@ -1,5 +1,5 @@ -from fastapi import Depends, APIRouter -from src.security.auth import * +from fastapi import Depends, APIRouter, Request +from src.security.auth import get_current_user from src.services.users.schemas.users import PasswordChangeForm, PublicUser, User, UserWithPassword from src.services.users.users import create_user, delete_user, get_profile_metadata, get_user_by_userid, update_user, update_user_password diff --git a/src/security/auth.py b/src/security/auth.py index a4edaf49..aa811d3f 100644 --- a/src/security/auth.py +++ b/src/security/auth.py @@ -1,13 +1,13 @@ from webbrowser import get from config.config import get_learnhouse_config from pydantic import BaseModel -from fastapi import Depends, HTTPException, status +from fastapi import Depends, HTTPException, Request, status from fastapi.security import OAuth2PasswordBearer from jose import JWTError, jwt from datetime import datetime, timedelta -from src.services.users.schemas.users import AnonymousUser -from src.services.users.users import * -from src.security.security import * +from src.services.users.schemas.users import AnonymousUser, PublicUser +from src.services.users.users import security_get_user, security_verify_password +from src.security.security import ALGORITHM, SECRET_KEY, verify_user_rights_with_roles from fastapi_jwt_auth import AuthJWT oauth2_scheme = OAuth2PasswordBearer(tokenUrl="/api/auth/login") diff --git a/src/services/courses/collections.py b/src/services/courses/collections.py index aabd502c..f07c97dc 100644 --- a/src/services/courses/collections.py +++ b/src/services/courses/collections.py @@ -34,7 +34,9 @@ async def get_collection( collection = await collections.find_one({"collection_id": collection_id}) # verify collection rights - await verify_collection_rights(request, collection_id, current_user, "read") + await verify_collection_rights( + request, collection_id, current_user, "read", collection["org_id"] + ) if not collection: raise HTTPException( @@ -99,12 +101,15 @@ async def update_collection( current_user: PublicUser, ): # verify collection rights - await verify_collection_rights(request, collection_id, current_user, "update") collections = request.app.db["collections"] collection = await collections.find_one({"collection_id": collection_id}) + await verify_collection_rights( + request, collection_id, current_user, "update", collection["org_id"] + ) + if not collection: raise HTTPException( status_code=status.HTTP_409_CONFLICT, detail="Collection does not exist" @@ -124,12 +129,14 @@ async def update_collection( async def delete_collection( request: Request, collection_id: str, current_user: PublicUser ): - await verify_collection_rights(request, collection_id, current_user, "delete") - collections = request.app.db["collections"] collection = await collections.find_one({"collection_id": collection_id}) + await verify_collection_rights( + request, collection_id, current_user, "delete", collection["org_id"] + ) + if not collection: raise HTTPException( status_code=status.HTTP_409_CONFLICT, detail="Collection does not exist" @@ -160,6 +167,8 @@ async def get_collections( ): collections = request.app.db["collections"] + print(org_id) + # get all collections from database without ObjectId all_collections = ( collections.find({"org_id": org_id}) @@ -168,7 +177,7 @@ async def get_collections( .limit(limit) ) - await verify_collection_rights(request, "*", current_user, "read") + await verify_collection_rights(request, "*", current_user, "read", org_id) # create list of collections and include courses in each collection collections_list = [] @@ -195,19 +204,27 @@ async def get_collections( async def verify_collection_rights( - request: Request, collection_id: str, current_user: PublicUser, action: str + request: Request, + collection_id: str, + current_user: PublicUser, + action: str, + org_id: str, ): collections = request.app.db["collections"] collection = await collections.find_one({"collection_id": collection_id}) - if not collection and action != "create": + if not collection and action != "create" and collection_id != "*": raise HTTPException( status_code=status.HTTP_409_CONFLICT, detail="Collection does not exist" ) + # Collections are public by default for now + 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, collection["org_id"] + request, action, current_user.user_id, collection_id, org_id ) if not hasRoleRights: diff --git a/src/services/courses/courses.py b/src/services/courses/courses.py index 1be805f4..bd6f2786 100644 --- a/src/services/courses/courses.py +++ b/src/services/courses/courses.py @@ -6,8 +6,8 @@ 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 * -from fastapi import HTTPException, status, UploadFile +from src.security.security import verify_user_rights_with_roles +from fastapi import HTTPException, Request, status, UploadFile from datetime import datetime #### Classes #################################################### diff --git a/src/services/orgs/orgs.py b/src/services/orgs/orgs.py index 8fc78b83..bb54077f 100644 --- a/src/services/orgs/orgs.py +++ b/src/services/orgs/orgs.py @@ -9,7 +9,7 @@ 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 * +from src.security.security import verify_user_rights_with_roles from fastapi import HTTPException, UploadFile, status, Request @@ -103,7 +103,6 @@ async def update_org( # update org await orgs.update_one({"org_id": org_id}, {"$set": updated_org.dict()}) - return updated_org.dict() @@ -117,17 +116,13 @@ async def update_org_logo( org = await orgs.find_one({"org_id": org_id}) - name_in_disk = await upload_org_logo(logo_file) - # update org + # update org org = await orgs.update_one({"org_id": org_id}, {"$set": {"logo": name_in_disk}}) - + return {"detail": "Logo updated"} - - - async def delete_org(request: Request, org_id: str, current_user: PublicUser): await verify_org_rights(request, org_id, current_user, "delete") diff --git a/src/services/orgs/schemas/orgs.py b/src/services/orgs/schemas/orgs.py index 82da07e2..56531357 100644 --- a/src/services/orgs/schemas/orgs.py +++ b/src/services/orgs/schemas/orgs.py @@ -1,6 +1,5 @@ from typing import Optional from pydantic import BaseModel -from src.security.security import * #### Classes #################################################### diff --git a/src/services/roles/roles.py b/src/services/roles/roles.py index 51b1e01b..2ef3ee34 100644 --- a/src/services/roles/roles.py +++ b/src/services/roles/roles.py @@ -2,7 +2,6 @@ from typing import Literal from uuid import uuid4 from src.services.roles.schemas.roles import Role, RoleInDB from src.services.users.schemas.users import PublicUser -from src.security.security import * from fastapi import HTTPException, status, Request from datetime import datetime @@ -12,7 +11,6 @@ async def create_role(request: Request, role_object: Role, current_user: PublicU await verify_user_permissions_on_roles(request, current_user, "create", None) - # create the role object in the database and return the object role_id = "role_" + str(uuid4()) @@ -27,6 +25,7 @@ async def create_role(request: Request, role_object: Role, current_user: PublicU return role + async def read_role(request: Request, role_id: str, current_user: PublicUser): roles = request.app.db["roles"] @@ -36,7 +35,10 @@ async def read_role(request: Request, role_id: str, current_user: PublicUser): return role -async def update_role(request: Request, role_id: str, role_object: Role, current_user: PublicUser): + +async def update_role( + request: Request, role_id: str, role_object: Role, current_user: PublicUser +): roles = request.app.db["roles"] await verify_user_permissions_on_roles(request, current_user, "update", role_id) @@ -44,10 +46,15 @@ async def update_role(request: Request, role_id: str, role_object: Role, current role_object.updated_at = datetime.now() # Update the role object in the database and return the object - updated_role = RoleInDB(**await roles.find_one_and_update({"role_id": role_id}, {"$set": role_object.dict()}, return_document=True)) + updated_role = RoleInDB( + **await roles.find_one_and_update( + {"role_id": role_id}, {"$set": role_object.dict()}, return_document=True + ) + ) return updated_role + async def delete_role(request: Request, role_id: str, current_user: PublicUser): roles = request.app.db["roles"] @@ -58,9 +65,16 @@ async def delete_role(request: Request, role_id: str, current_user: PublicUser): return deleted_role + #### Security #################################################### -async def verify_user_permissions_on_roles(request: Request, current_user: PublicUser, action: Literal["create", "read", "update", "delete"], role_id: str | None): + +async def verify_user_permissions_on_roles( + request: Request, + current_user: PublicUser, + action: Literal["create", "read", "update", "delete"], + role_id: str | None, +): request.app.db["users"] roles = request.app.db["roles"] @@ -68,7 +82,8 @@ async def verify_user_permissions_on_roles(request: Request, current_user: Publi if not current_user: raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, detail="Roles : Not authenticated") + status_code=status.HTTP_401_UNAUTHORIZED, detail="Roles : Not authenticated" + ) if action == "create": if "owner" in [org.org_role for org in current_user.orgs]: From 94f50a89a5105a22aed7f9f288008ee549836d86 Mon Sep 17 00:00:00 2001 From: swve Date: Mon, 19 Jun 2023 17:57:11 +0200 Subject: [PATCH 4/5] fix: generic errors --- config/config.py | 3 +-- src/security/auth.py | 3 +-- src/services/orgs/orgs.py | 7 +++---- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/config/config.py b/config/config.py index f187fcc7..cad437a3 100644 --- a/config/config.py +++ b/config/config.py @@ -1,4 +1,3 @@ -from calendar import c from typing import Optional from pydantic import BaseModel import os @@ -51,7 +50,7 @@ def get_learnhouse_config() -> LearnHouseConfig: env_site_description = os.environ.get("LEARNHOUSE_SITE_DESCRIPTION") env_contact_email = os.environ.get("LEARNHOUSE_CONTACT_EMAIL") env_domain = os.environ.get("LEARNHOUSE_DOMAIN") - env_port = os.environ.get("LEARNHOUSE_PORT") + os.environ.get("LEARNHOUSE_PORT") env_ssl = os.environ.get("LEARNHOUSE_SSL") env_use_default_org = os.environ.get("LEARNHOUSE_USE_DEFAULT_ORG") env_allowed_origins = os.environ.get("LEARNHOUSE_ALLOWED_ORIGINS") diff --git a/src/security/auth.py b/src/security/auth.py index aa811d3f..a24ee2c8 100644 --- a/src/security/auth.py +++ b/src/security/auth.py @@ -1,4 +1,3 @@ -from webbrowser import get from config.config import get_learnhouse_config from pydantic import BaseModel from fastapi import Depends, HTTPException, Request, status @@ -7,7 +6,7 @@ from jose import JWTError, jwt from datetime import datetime, timedelta from src.services.users.schemas.users import AnonymousUser, PublicUser from src.services.users.users import security_get_user, security_verify_password -from src.security.security import ALGORITHM, SECRET_KEY, verify_user_rights_with_roles +from src.security.security import ALGORITHM, SECRET_KEY from fastapi_jwt_auth import AuthJWT oauth2_scheme = OAuth2PasswordBearer(tokenUrl="/api/auth/login") diff --git a/src/services/orgs/orgs.py b/src/services/orgs/orgs.py index bb54077f..e4c00433 100644 --- a/src/services/orgs/orgs.py +++ b/src/services/orgs/orgs.py @@ -1,5 +1,4 @@ import json -from typing import Optional from uuid import uuid4 from src.services.orgs.logos import upload_org_logo from src.services.orgs.schemas.orgs import ( @@ -96,7 +95,7 @@ async def update_org( orgs = request.app.db["organizations"] - org = await orgs.find_one({"org_id": org_id}) + await orgs.find_one({"org_id": org_id}) updated_org = OrganizationInDB(org_id=org_id, **org_object.dict()) @@ -114,12 +113,12 @@ async def update_org_logo( orgs = request.app.db["organizations"] - org = await orgs.find_one({"org_id": org_id}) + await orgs.find_one({"org_id": org_id}) name_in_disk = await upload_org_logo(logo_file) # update org - org = await orgs.update_one({"org_id": org_id}, {"$set": {"logo": name_in_disk}}) + await orgs.update_one({"org_id": org_id}, {"$set": {"logo": name_in_disk}}) return {"detail": "Logo updated"} From e786658d554c0509e688eb9ea8b3eb85417afe53 Mon Sep 17 00:00:00 2001 From: swve Date: Mon, 19 Jun 2023 17:59:36 +0200 Subject: [PATCH 5/5] fix: import issues --- src/routers/auth.py | 5 ++--- src/routers/blocks.py | 1 - 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/routers/auth.py b/src/routers/auth.py index 4eaa4ba8..0af18380 100644 --- a/src/routers/auth.py +++ b/src/routers/auth.py @@ -1,4 +1,3 @@ -from urllib.request import Request from fastapi import Depends, APIRouter, HTTPException, Response, status, Request from fastapi.security import OAuth2PasswordRequestForm from src.security.auth import AuthJWT, authenticate_user @@ -41,8 +40,8 @@ async def login( access_token = Authorize.create_access_token(subject=form_data.username) refresh_token = Authorize.create_refresh_token(subject=form_data.username) Authorize.set_refresh_cookies(refresh_token) - # set cookies using fastapi - response.set_cookie(key="access_token_cookie", value=access_token , httponly=False) + # set cookies using fastapi + response.set_cookie(key="access_token_cookie", value=access_token, httponly=False) user = PublicUser(**user.dict()) result = { diff --git a/src/routers/blocks.py b/src/routers/blocks.py index c456d585..3d2c4929 100644 --- a/src/routers/blocks.py +++ b/src/routers/blocks.py @@ -1,6 +1,5 @@ from fastapi import APIRouter, Depends, UploadFile, Form, Request from src.security.auth import get_current_user -from fastapi import UploadFile from src.services.blocks.block_types.imageBlock.images import create_image_block, get_image_block from src.services.blocks.block_types.videoBlock.videoBlock import create_video_block, get_video_block from src.services.blocks.block_types.pdfBlock.pdfBlock import create_pdf_block, get_pdf_block