diff --git a/.dockerignore b/.dockerignore new file mode 100644 index 00000000..e9a7c3af --- /dev/null +++ b/.dockerignore @@ -0,0 +1,132 @@ +# Dependencies +node_modules +npm-debug.log* +yarn-debug.log* +yarn-error.log* +pnpm-debug.log* + +# Production builds +.next +out +dist +build + +# Environment files +.env +.env.local +.env.development.local +.env.test.local +.env.production.local + +# IDE files +.vscode +.idea +*.swp +*.swo + +# OS files +.DS_Store +Thumbs.db + +# Git +.git +.gitignore + +# Docker +Dockerfile +.dockerignore +docker-compose.yml + +# Logs +logs +*.log + +# Runtime data +pids +*.pid +*.seed +*.pid.lock + +# Coverage directory used by tools like istanbul +coverage +*.lcov + +# nyc test coverage +.nyc_output + +# Dependency directories +jspm_packages/ + +# Optional npm cache directory +.npm + +# Optional eslint cache +.eslintcache + +# Microbundle cache +.rpt2_cache/ +.rts2_cache_cjs/ +.rts2_cache_es/ +.rts2_cache_umd/ + +# Optional REPL history +.node_repl_history + +# Output of 'npm pack' +*.tgz + +# Yarn Integrity file +.yarn-integrity + +# parcel-bundler cache (https://parceljs.org/) +.cache +.parcel-cache + +# Next.js build output +.next + +# Nuxt.js build / generate output +.nuxt +dist + +# Storybook build outputs +.out +.storybook-out + +# Temporary folders +tmp/ +temp/ + +# Python +__pycache__/ +*.py[cod] +*$py.class +*.so +.Python +env/ +venv/ +ENV/ +env.bak/ +venv.bak/ +.pytest_cache/ +.coverage +htmlcov/ +.tox/ +.cache +nosetests.xml +coverage.xml +*.cover +.hypothesis/ + +# Database +*.db +*.sqlite3 + +# Media files (can be large) +uploads/ +media/ + +# Documentation +README.md +docs/ +*.md \ No newline at end of file diff --git a/Dockerfile b/Dockerfile index 3f5ee269..8a40dc9c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ -# Base image -FROM python:3.12.3-slim-bookworm as base +# Base image for Python backend +FROM python:3.12.3-slim-bookworm AS base # Install Nginx, curl, and build-essential RUN apt update && apt install -y nginx curl build-essential \ @@ -10,32 +10,80 @@ RUN apt update && apt install -y nginx curl build-essential \ # Install Node tools RUN curl -fsSL https://deb.nodesource.com/setup_21.x | bash - \ && apt-get install -y nodejs \ - && npm install -g corepack pm2 + && npm install -g pm2 -# Frontend Build -FROM base AS deps +# Frontend Build - Using Node.js Alpine for better performance +FROM node:22-alpine AS frontend-base +# Install dependencies only when needed +FROM frontend-base AS frontend-deps +# Check https://github.com/nodejs/docker-node/tree/b4117f9333da4138b03a546ec926ef50a31506c3#nodealpine to understand why libc6-compat might be needed. +RUN apk add --no-cache libc6-compat +WORKDIR /app + +# Install dependencies based on the preferred package manager +COPY apps/web/package.json apps/web/pnpm-lock.yaml* ./ +RUN \ + if [ -f pnpm-lock.yaml ]; then corepack enable pnpm && pnpm i --frozen-lockfile; \ + else echo "Lockfile not found." && exit 1; \ + fi + +# Rebuild the source code only when needed +FROM frontend-base AS frontend-builder +WORKDIR /app +COPY --from=frontend-deps /app/node_modules ./node_modules +COPY apps/web . + +# Set environment variables for the build ENV NEXT_PUBLIC_LEARNHOUSE_API_URL=http://localhost/api/v1/ ENV NEXT_PUBLIC_LEARNHOUSE_BACKEND_URL=http://localhost/ ENV NEXT_PUBLIC_LEARNHOUSE_DOMAIN=localhost -WORKDIR /app/web -COPY ./apps/web/package.json ./apps/web/pnpm-lock.yaml* ./ -COPY ./apps/web /app/web -RUN rm -f .env* -RUN if [ -f pnpm-lock.yaml ]; then corepack enable pnpm && pnpm i --frozen-lockfile && pnpm run build; \ - else echo "Lockfile not found." && exit 1; \ - fi +# Next.js collects completely anonymous telemetry data about general usage. +# Learn more here: https://nextjs.org/telemetry +# Uncomment the following line in case you want to disable telemetry during the build. +# ENV NEXT_TELEMETRY_DISABLED 1 -# Final image -FROM base as runner -RUN addgroup --system --gid 1001 system \ - && adduser --system --uid 1001 app \ - && mkdir .next \ - && chown app:system .next -COPY --from=deps /app/web/public ./app/web/public -COPY --from=deps --chown=app:system /app/web/.next/standalone ./app/web/ -COPY --from=deps --chown=app:system /app/web/.next/static ./app/web/.next/static +# Remove .env files from the final image +# This is a good practice to avoid leaking sensitive data +# Learn more about it in the Next.js documentation: https://nextjs.org/docs/basic-features/environment-variables +RUN rm -f .env* + +RUN \ + if [ -f pnpm-lock.yaml ]; then corepack enable pnpm && pnpm run build; \ + else echo "Lockfile not found." && exit 1; \ + fi + +# Production image, copy all the files and run next +FROM frontend-base AS frontend-runner +WORKDIR /app + +# Install curl +RUN apk add --no-cache curl + +ENV NODE_ENV production +# Uncomment the following line in case you want to disable telemetry during runtime. +# ENV NEXT_TELEMETRY_DISABLED 1 + +RUN addgroup --system --gid 1001 nodejs +RUN adduser --system --uid 1001 nextjs + +COPY --from=frontend-builder /app/public ./public + +# Set the correct permission for prerender cache +RUN mkdir .next +RUN chown nextjs:nodejs .next + +# Automatically leverage output traces to reduce image size +# https://nextjs.org/docs/advanced-features/output-file-tracing +COPY --from=frontend-builder --chown=nextjs:nodejs /app/.next/standalone ./ +COPY --from=frontend-builder --chown=nextjs:nodejs /app/.next/static ./.next/static + +# Final image combining frontend and backend +FROM base AS runner + +# Copy the frontend standalone build +COPY --from=frontend-runner /app /app/web # Backend Build WORKDIR /app/api @@ -51,4 +99,5 @@ WORKDIR /app COPY ./extra/nginx.conf /etc/nginx/conf.d/default.conf ENV PORT=8000 LEARNHOUSE_PORT=9000 HOSTNAME=0.0.0.0 COPY ./extra/start.sh /app/start.sh -CMD ["sh", "start.sh"] \ No newline at end of file +RUN chmod +x /app/start.sh +CMD ["sh", "/app/start.sh"] \ No newline at end of file diff --git a/apps/api/app.py b/apps/api/app.py index aa3b4b2e..7db51600 100644 --- a/apps/api/app.py +++ b/apps/api/app.py @@ -11,8 +11,6 @@ from fastapi_jwt_auth.exceptions import AuthJWTException from fastapi.middleware.gzip import GZipMiddleware -# from src.services.mocks.initial import create_initial_data - ######################## # Pre-Alpha Version 0.1.0 # Author: @swve @@ -39,8 +37,13 @@ app.add_middleware( allow_headers=["*"], ) -logfire.configure(console=False, service_name=learnhouse_config.site_name,) -logfire.instrument_fastapi(app) +# Only enable logfire if explicitly configured +if learnhouse_config.general_config.logfire_enabled: + logfire.configure(console=False, service_name=learnhouse_config.site_name,) + logfire.instrument_fastapi(app) + # Instrument database after logfire is configured + from src.core.events.database import engine + logfire.instrument_sqlalchemy(engine=engine) # Gzip Middleware (will add brotli later) app.add_middleware(GZipMiddleware, minimum_size=1000) diff --git a/apps/api/cli.py b/apps/api/cli.py index b4238ac4..5e3f8142 100644 --- a/apps/api/cli.py +++ b/apps/api/cli.py @@ -49,6 +49,8 @@ def install( email="", logo_image="", thumbnail_image="", + about="", + label="", ) install_create_organization(org, db_session) print("Default organization created ✅") @@ -91,6 +93,8 @@ def install( email="", logo_image="", thumbnail_image="", + about="", + label="", ) install_create_organization(org, db_session) print(orgname + " Organization created ✅") diff --git a/apps/api/config/config.py b/apps/api/config/config.py index e3f7ff8c..9e9635c7 100644 --- a/apps/api/config/config.py +++ b/apps/api/config/config.py @@ -12,6 +12,7 @@ class CookieConfig(BaseModel): class GeneralConfig(BaseModel): development_mode: bool install_mode: bool + logfire_enabled: bool class SecurityConfig(BaseModel): @@ -118,6 +119,13 @@ def get_learnhouse_config() -> LearnHouseConfig: else yaml_config.get("general", {}).get("install_mode") ) + # Logfire config + env_logfire_enabled = os.environ.get("LEARNHOUSE_LOGFIRE_ENABLED", "None") + logfire_enabled = ( + env_logfire_enabled.lower() == "true" if env_logfire_enabled != "None" + else yaml_config.get("general", {}).get("logfire_enabled", False) + ) + # Security Config env_auth_jwt_secret_key = os.environ.get("LEARNHOUSE_AUTH_JWT_SECRET_KEY") auth_jwt_secret_key = env_auth_jwt_secret_key or yaml_config.get( @@ -295,7 +303,9 @@ def get_learnhouse_config() -> LearnHouseConfig: site_description=site_description, contact_email=contact_email, general_config=GeneralConfig( - development_mode=bool(development_mode), install_mode=bool(install_mode) + development_mode=bool(development_mode), + install_mode=bool(install_mode), + logfire_enabled=bool(logfire_enabled) ), hosting_config=hosting_config, database_config=database_config, diff --git a/apps/api/config/config.yaml b/apps/api/config/config.yaml index c9dcec79..33f1601f 100644 --- a/apps/api/config/config.yaml +++ b/apps/api/config/config.yaml @@ -7,6 +7,7 @@ contact_email: hi@learnhouse.app general: development_mode: true install_mode: true + logfire_enabled: false security: auth_jwt_secret_key: secret diff --git a/apps/api/src/core/events/database.py b/apps/api/src/core/events/database.py index b970af1a..3be3eac9 100644 --- a/apps/api/src/core/events/database.py +++ b/apps/api/src/core/events/database.py @@ -1,5 +1,4 @@ import logging -import logfire import os import importlib from config.config import get_learnhouse_config @@ -58,7 +57,7 @@ else: # Only create tables if not in test mode (tests will handle this themselves) if not is_testing: SQLModel.metadata.create_all(engine) - logfire.instrument_sqlalchemy(engine=engine) + # Note: logfire instrumentation will be handled in app.py after configuration async def connect_to_db(app: FastAPI): app.db_engine = engine # type: ignore diff --git a/apps/api/src/db/organizations.py b/apps/api/src/db/organizations.py index f8fd9ec9..34570c89 100644 --- a/apps/api/src/db/organizations.py +++ b/apps/api/src/db/organizations.py @@ -1,6 +1,7 @@ from typing import Optional from pydantic import BaseModel -from sqlmodel import Field, SQLModel, JSON, Column +from sqlalchemy import JSON, Column +from sqlmodel import Field, SQLModel from src.db.roles import RoleRead from src.db.organization_config import OrganizationConfig diff --git a/apps/api/src/db/roles.py b/apps/api/src/db/roles.py index 7d1c8a0b..7b6bd4e2 100644 --- a/apps/api/src/db/roles.py +++ b/apps/api/src/db/roles.py @@ -16,14 +16,36 @@ class Permission(BaseModel): return getattr(self, item) +class PermissionsWithOwn(BaseModel): + action_create: bool + action_read: bool + action_read_own: bool + action_update: bool + action_update_own: bool + action_delete: bool + action_delete_own: bool + + def __getitem__(self, item): + return getattr(self, item) + + +class DashboardPermission(BaseModel): + action_access: bool + + def __getitem__(self, item): + return getattr(self, item) + + class Rights(BaseModel): - courses: Permission + courses: PermissionsWithOwn users: Permission usergroups : Permission collections: Permission organizations: Permission coursechapters: Permission activities: Permission + roles: Permission + dashboard: DashboardPermission def __getitem__(self, item): return getattr(self, item) diff --git a/apps/api/src/routers/courses/courses.py b/apps/api/src/routers/courses/courses.py index 6e1938fb..1e0d4733 100644 --- a/apps/api/src/routers/courses/courses.py +++ b/apps/api/src/routers/courses/courses.py @@ -26,6 +26,7 @@ from src.services.courses.courses import ( delete_course, update_course_thumbnail, search_courses, + get_course_user_rights, ) from src.services.courses.updates import ( create_update, @@ -358,12 +359,94 @@ async def api_remove_bulk_course_contributors( ): """ Remove multiple contributors from a course by their usernames - Only administrators can perform this action """ return await remove_bulk_course_contributors( - request, - course_uuid, - usernames, - current_user, - db_session + request, course_uuid, usernames, current_user, db_session ) + + +@router.get("/{course_uuid}/rights") +async def api_get_course_user_rights( + request: Request, + course_uuid: str, + db_session: Session = Depends(get_db_session), + current_user: PublicUser = Depends(get_current_user), +) -> dict: + """ + Get detailed user rights for a specific course. + + This endpoint returns comprehensive rights information that can be used + by the UI to enable/disable features based on user permissions. + + + + **Response Structure:** + ```json + { + "course_uuid": "course_123", + "user_id": 456, + "is_anonymous": false, + "permissions": { + "read": true, + "create": false, + "update": true, + "delete": false, + "create_content": true, + "update_content": true, + "delete_content": true, + "manage_contributors": true, + "manage_access": true, + "grade_assignments": true, + "mark_activities_done": true, + "create_certifications": true + }, + "ownership": { + "is_owner": true, + "is_creator": true, + "is_maintainer": false, + "is_contributor": false, + "authorship_status": "ACTIVE" + }, + "roles": { + "is_admin": false, + "is_maintainer_role": false, + "is_instructor": true, + "is_user": true + } + } + ``` + + **Permissions Explained:** + - `read`: Can read the course content + - `create`: Can create new courses (instructor role or higher) + - `update`: Can update course settings (title, description, etc.) + - `delete`: Can delete the course + - `create_content`: Can create activities, assignments, chapters, etc. + - `update_content`: Can update course content + - `delete_content`: Can delete course content + - `manage_contributors`: Can add/remove contributors + - `manage_access`: Can change course access settings (public, open_to_contributors) + - `grade_assignments`: Can grade student assignments + - `mark_activities_done`: Can mark activities as done for other users + - `create_certifications`: Can create course certifications + + **Ownership Information:** + - `is_owner`: Is course owner (CREATOR, MAINTAINER, or CONTRIBUTOR) + - `is_creator`: Is course creator + - `is_maintainer`: Is course maintainer + - `is_contributor`: Is course contributor + - `authorship_status`: Current authorship status (ACTIVE, PENDING, INACTIVE) + + **Role Information:** + - `is_admin`: Has admin role (role 1) + - `is_maintainer_role`: Has maintainer role (role 2) + - `is_instructor`: Has instructor role (role 3) + - `is_user`: Has basic user role (role 4) + + **Security Notes:** + - Returns rights based on course ownership and user roles + - Safe to expose to UI as it only returns permission information + - Anonymous users can only read public courses + - All permissions are calculated based on current user context + """ + return await get_course_user_rights(request, course_uuid, current_user, db_session) diff --git a/apps/api/src/routers/roles.py b/apps/api/src/routers/roles.py index ef9350e0..5d5cbfff 100644 --- a/apps/api/src/routers/roles.py +++ b/apps/api/src/routers/roles.py @@ -1,28 +1,45 @@ -from fastapi import APIRouter, Depends, Request +from fastapi import APIRouter, Depends, Request, HTTPException from sqlmodel import Session from src.core.events.database import get_db_session from src.db.roles import RoleCreate, RoleRead, RoleUpdate from src.security.auth import get_current_user -from src.services.roles.roles import create_role, delete_role, read_role, update_role +from src.services.roles.roles import create_role, delete_role, read_role, update_role, get_roles_by_organization from src.db.users import PublicUser +from typing import List router = APIRouter() -@router.post("/") +@router.post("/org/{org_id}") async def api_create_role( request: Request, + org_id: int, role_object: RoleCreate, current_user: PublicUser = Depends(get_current_user), db_session: Session = Depends(get_db_session), )-> RoleRead: """ - Create new role + Create new role for a specific organization """ + # Set the org_id in the role object + role_object.org_id = org_id return await create_role(request, db_session, role_object, current_user) +@router.get("/org/{org_id}") +async def api_get_roles_by_organization( + request: Request, + org_id: int, + current_user: PublicUser = Depends(get_current_user), + db_session: Session = Depends(get_db_session), +)-> List[RoleRead]: + """ + Get all roles for a specific organization, including global roles + """ + return await get_roles_by_organization(request, db_session, org_id, current_user) + + @router.get("/{role_id}") async def api_get_role( request: Request, @@ -39,6 +56,7 @@ async def api_get_role( @router.put("/{role_id}") async def api_update_role( request: Request, + role_id: str, role_object: RoleUpdate, current_user: PublicUser = Depends(get_current_user), db_session: Session = Depends(get_db_session), @@ -46,6 +64,16 @@ async def api_update_role( """ Update role by role_id """ + # Convert role_id to integer and set it in the role_object + try: + role_id_int = int(role_id) + except ValueError: + raise HTTPException( + status_code=400, + detail="Invalid role ID format. Role ID must be a number.", + ) + + role_object.role_id = role_id_int return await update_role(request, db_session, role_object, current_user) diff --git a/apps/api/src/security/courses_security.py b/apps/api/src/security/courses_security.py new file mode 100644 index 00000000..6bca2b92 --- /dev/null +++ b/apps/api/src/security/courses_security.py @@ -0,0 +1,410 @@ +""" +SECURITY DOCUMENTATION FOR COURSES RBAC SYSTEM + +This module provides unified RBAC (Role-Based Access Control) checks for all courses-related operations. + +SECURITY MEASURES IMPLEMENTED: + +1. COURSE OWNERSHIP REQUIREMENTS: + - All non-read operations (create, update, delete) require course ownership + - Course ownership is determined by ResourceAuthor table with ACTIVE status + - Valid ownership roles: CREATOR, MAINTAINER, CONTRIBUTOR + - Admin/maintainer roles are also accepted for course operations + +2. COURSE CREATION VS COURSE CONTENT CREATION: + - COURSE CREATION: Allow if user has instructor role (3) or higher + - COURSE CONTENT CREATION (activities, assignments, chapters, etc.): Require course ownership (CREATOR, MAINTAINER, CONTRIBUTOR) or admin/maintainer role + - This distinction allows instructors to create courses but prevents them from creating content in courses they don't own + +3. STRICT ACCESS CONTROLS: + - Activities: Require course ownership for all non-read operations + - Assignments: Require course ownership for all non-read operations + - Chapters: Require course ownership for all non-read operations + - Certifications: Require course ownership for all non-read operations + - Collections: Use organization-level permissions + +4. GRADING AND SUBMISSION SECURITY: + - Only course owners or instructors can grade assignments + - Users can only submit their own work + - Users cannot update grades unless they are instructors + - Users can only update their own submissions + +5. CERTIFICATE SECURITY: + - Certificates can only be created by course owners or instructors + - System-generated certificates (from course completion) are properly secured + - Certificate creation requires proper RBAC checks + +6. ACTIVITY MARKING SECURITY: + - Only course owners or instructors can mark activities as done for other users + - Users can only mark their own activities as done + +7. COLLECTION SECURITY: + - Users can only add courses to collections if they have read access to those courses + - Collection operations require appropriate organization-level permissions + +8. ANONYMOUS USER HANDLING: + - Anonymous users can only read public courses + - All non-read operations require authentication + +9. ERROR HANDLING: + - Clear error messages for security violations + - Proper HTTP status codes (401, 403, 404) + - Comprehensive logging of security events + +10. COURSE ACCESS MANAGEMENT SECURITY: + - Sensitive fields (public, open_to_contributors) require additional validation + - Only course owners (CREATOR, MAINTAINER) or admins can change access settings + - Course creation requires proper organization-level permissions + - Course updates require course ownership or admin role + +11. CONTRIBUTOR MANAGEMENT SECURITY: + - Only course owners (CREATOR, MAINTAINER) or admins can add/remove contributors + - Only course owners (CREATOR, MAINTAINER) or admins can update contributor roles + - Cannot modify the role of the course creator + - Contributor applications are created with PENDING status + - Only course owners or admins can approve contributor applications + +SECURITY BEST PRACTICES: +- Always check course ownership before allowing modifications +- Validate user permissions at multiple levels +- Use proper RBAC checks for all operations +- Implement principle of least privilege +- Provide clear error messages for security violations +- Log security events for audit purposes +- Additional validation for sensitive access control fields +- Strict ownership requirements for contributor management +- Distinguish between course creation and course content creation permissions + +CRITICAL SECURITY FIXES: +- Fixed: Users could create certifications for courses they don't own +- Fixed: Users could grade assignments without proper permissions +- Fixed: Users could mark activities as done for other users without permissions +- Fixed: Collections could be created with courses the user doesn't have access to +- Fixed: Assignment submissions could be modified by unauthorized users +- Fixed: Users could change course access settings (public, open_to_contributors) without proper permissions +- Fixed: Users could add/remove contributors from courses they don't own +- Fixed: Users could update contributor roles without course ownership +- Fixed: Course creation used hardcoded RBAC check +- Fixed: Contributor management used permissive RBAC checks instead of strict ownership requirements +- Fixed: Instructors could create content in courses they don't own (now they can only create courses) +""" + +from typing import Literal +from fastapi import HTTPException, Request, status +from sqlmodel import Session, select +from src.db.users import AnonymousUser, PublicUser +from src.db.courses.courses import Course +from src.db.resource_authors import ResourceAuthor, ResourceAuthorshipEnum, ResourceAuthorshipStatusEnum +from src.security.rbac.rbac import ( + authorization_verify_based_on_roles_and_authorship, + authorization_verify_if_element_is_public, + authorization_verify_if_user_is_anon, + authorization_verify_based_on_org_admin_status, +) + + +async def courses_rbac_check( + request: Request, + course_uuid: str, + current_user: PublicUser | AnonymousUser, + action: Literal["create", "read", "update", "delete"], + db_session: Session, + require_course_ownership: bool = False, +) -> bool: + """ + Unified RBAC check for courses-related operations. + + SECURITY NOTES: + - READ operations: Allow if user has read access to the course (public courses or user has permissions) + - COURSE CREATION: Allow if user has instructor role (3) or higher + - COURSE CONTENT CREATION (activities, assignments, chapters, etc.): Require course ownership (CREATOR, MAINTAINER, CONTRIBUTOR) or admin/maintainer role + - UPDATE/DELETE operations: Require course ownership (CREATOR, MAINTAINER, CONTRIBUTOR) or admin/maintainer role + - Course ownership is determined by ResourceAuthor table with ACTIVE status + - Admin/maintainer roles are checked via authorization_verify_based_on_org_admin_status + + Args: + request: FastAPI request object + course_uuid: UUID of the course (or "course_x" for course creation) + current_user: Current user (PublicUser or AnonymousUser) + action: Action to perform (create, read, update, delete) + db_session: Database session + require_course_ownership: If True, requires course ownership for non-read actions + + Returns: + bool: True if authorized, raises HTTPException otherwise + + Raises: + HTTPException: 403 Forbidden if user lacks required permissions + HTTPException: 401 Unauthorized if user is anonymous for non-read actions + """ + + if action == "read": + if current_user.id == 0: # Anonymous user + return await authorization_verify_if_element_is_public( + request, course_uuid, action, db_session + ) + else: + return await authorization_verify_based_on_roles_and_authorship( + request, current_user.id, action, course_uuid, db_session + ) + else: + # For non-read actions, proceed with strict RBAC checks + await authorization_verify_if_user_is_anon(current_user.id) + + # SECURITY: Special handling for course creation vs course content creation + if action == "create" and course_uuid == "course_x": + # This is course creation - allow instructors (role 3) or higher + # Check if user has instructor role or higher + from src.security.rbac.rbac import authorization_verify_based_on_roles + + has_create_permission = await authorization_verify_based_on_roles( + request, current_user.id, "create", "course_x", db_session + ) + + if has_create_permission: + return True + else: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="You must have instructor role or higher to create courses", + ) + + # SECURITY: For course content creation and other operations, require course ownership + # This prevents users without course ownership from creating/modifying course content + if require_course_ownership or action in ["create", "update", "delete"]: + # Check if user is course owner (CREATOR, MAINTAINER, or CONTRIBUTOR) + statement = select(ResourceAuthor).where( + ResourceAuthor.resource_uuid == course_uuid, + ResourceAuthor.user_id == current_user.id + ) + resource_author = db_session.exec(statement).first() + + is_course_owner = False + if resource_author: + if ((resource_author.authorship == ResourceAuthorshipEnum.CREATOR) or + (resource_author.authorship == ResourceAuthorshipEnum.MAINTAINER) or + (resource_author.authorship == ResourceAuthorshipEnum.CONTRIBUTOR)) and \ + resource_author.authorship_status == ResourceAuthorshipStatusEnum.ACTIVE: + is_course_owner = True + + # Check if user has admin or maintainer role + is_admin_or_maintainer = await authorization_verify_based_on_org_admin_status( + request, current_user.id, action, course_uuid, db_session + ) + + # SECURITY: For creating, updating, and deleting course content, user MUST be either: + # 1. Course owner (CREATOR, MAINTAINER, or CONTRIBUTOR with ACTIVE status) + # 2. Admin or maintainer role + # General role permissions are NOT sufficient for these actions + if not (is_course_owner or is_admin_or_maintainer): + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail=f"You must be the course owner (CREATOR, MAINTAINER, or CONTRIBUTOR) or have admin/maintainer role to {action} in this course", + ) + return True + else: + # For other actions, use the existing RBAC check + return await authorization_verify_based_on_roles_and_authorship( + request, + current_user.id, + action, + course_uuid, + db_session, + ) + + +async def courses_rbac_check_with_course_lookup( + request: Request, + course_uuid: str, + current_user: PublicUser | AnonymousUser, + action: Literal["create", "read", "update", "delete"], + db_session: Session, + require_course_ownership: bool = False, +) -> Course: + """ + Unified RBAC check for courses-related operations with course lookup. + + SECURITY NOTES: + - First validates that the course exists + - Then performs RBAC check using courses_rbac_check + - Returns the course object if authorized + + Args: + request: FastAPI request object + course_uuid: UUID of the course + current_user: Current user (PublicUser or AnonymousUser) + action: Action to perform (create, read, update, delete) + db_session: Database session + require_course_ownership: If True, requires course ownership for non-read actions + + Returns: + Course: The course object if authorized, raises HTTPException otherwise + + Raises: + HTTPException: 404 Not Found if course doesn't exist + HTTPException: 403 Forbidden if user lacks required permissions + """ + + # First check if course exists + statement = select(Course).where(Course.course_uuid == course_uuid) + course = db_session.exec(statement).first() + + if not course: + raise HTTPException( + status_code=404, + detail="Course not found", + ) + + # Perform RBAC check + await courses_rbac_check( + request, course_uuid, current_user, action, db_session, require_course_ownership + ) + + return course + + +async def courses_rbac_check_for_activities( + request: Request, + course_uuid: str, + current_user: PublicUser | AnonymousUser, + action: Literal["create", "read", "update", "delete"], + db_session: Session, +) -> bool: + """ + Specialized RBAC check for activities that requires course ownership for non-read actions. + + SECURITY NOTES: + - Activities are core course content and require strict ownership controls + - READ: Allow if user has read access to the course + - CREATE/UPDATE/DELETE: Require course ownership (CREATOR, MAINTAINER, CONTRIBUTOR) or admin/maintainer role + - This prevents unauthorized users from creating/modifying course activities + - Instructors can create courses but cannot create activities in courses they don't own + """ + + return await courses_rbac_check( + request, course_uuid, current_user, action, db_session, require_course_ownership=True + ) + + +async def courses_rbac_check_for_assignments( + request: Request, + course_uuid: str, + current_user: PublicUser | AnonymousUser, + action: Literal["create", "read", "update", "delete"], + db_session: Session, +) -> bool: + """ + Specialized RBAC check for assignments that requires course ownership for non-read actions. + + SECURITY NOTES: + - Assignments are course content and require strict ownership controls + - READ: Allow if user has read access to the course + - CREATE/UPDATE/DELETE: Require course ownership (CREATOR, MAINTAINER, CONTRIBUTOR) or admin/maintainer role + - This prevents unauthorized users from creating/modifying course assignments + - Instructors can create courses but cannot create assignments in courses they don't own + """ + + return await courses_rbac_check( + request, course_uuid, current_user, action, db_session, require_course_ownership=True + ) + + +async def courses_rbac_check_for_chapters( + request: Request, + course_uuid: str, + current_user: PublicUser | AnonymousUser, + action: Literal["create", "read", "update", "delete"], + db_session: Session, +) -> bool: + """ + Specialized RBAC check for chapters that requires course ownership for non-read actions. + + SECURITY NOTES: + - Chapters are course structure and require strict ownership controls + - READ: Allow if user has read access to the course + - CREATE/UPDATE/DELETE: Require course ownership (CREATOR, MAINTAINER, CONTRIBUTOR) or admin/maintainer role + - This prevents unauthorized users from creating/modifying course chapters + - Instructors can create courses but cannot create chapters in courses they don't own + """ + + return await courses_rbac_check( + request, course_uuid, current_user, action, db_session, require_course_ownership=True + ) + + +async def courses_rbac_check_for_certifications( + request: Request, + course_uuid: str, + current_user: PublicUser | AnonymousUser, + action: Literal["create", "read", "update", "delete"], + db_session: Session, +) -> bool: + """ + Specialized RBAC check for certifications that requires course ownership for non-read actions. + + SECURITY NOTES: + - Certifications are course credentials and require strict ownership controls + - READ: Allow if user has read access to the course + - CREATE/UPDATE/DELETE: Require course ownership (CREATOR, MAINTAINER, CONTRIBUTOR) or admin/maintainer role + - This prevents unauthorized users from creating/modifying course certifications + - CRITICAL: Without this check, users could create certifications for courses they don't own + - Instructors can create courses but cannot create certifications in courses they don't own + """ + + return await courses_rbac_check( + request, course_uuid, current_user, action, db_session, require_course_ownership=True + ) + + +async def courses_rbac_check_for_collections( + request: Request, + collection_uuid: str, + current_user: PublicUser | AnonymousUser, + action: Literal["create", "read", "update", "delete"], + db_session: Session, +) -> bool: + """ + Specialized RBAC check for collections. + + SECURITY NOTES: + - Collections are course groupings and require appropriate access controls + - READ: Allow if collection is public or user has read access + - CREATE/UPDATE/DELETE: Require appropriate permissions based on collection ownership + - Collections may have different ownership models than courses + + Args: + request: FastAPI request object + collection_uuid: UUID of the collection + current_user: Current user (PublicUser or AnonymousUser) + action: Action to perform (create, read, update, delete) + db_session: Database session + + Returns: + bool: True if authorized, raises HTTPException otherwise + """ + + if action == "read": + if current_user.id == 0: # Anonymous user + res = await authorization_verify_if_element_is_public( + request, collection_uuid, action, db_session + ) + if res == False: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="User rights : You are not allowed to read this collection", + ) + return res + else: + return await authorization_verify_based_on_roles_and_authorship( + request, current_user.id, action, collection_uuid, db_session + ) + else: + await authorization_verify_if_user_is_anon(current_user.id) + + return await authorization_verify_based_on_roles_and_authorship( + request, + current_user.id, + action, + collection_uuid, + db_session, + ) \ No newline at end of file diff --git a/apps/api/src/security/rbac/rbac.py b/apps/api/src/security/rbac/rbac.py index 1e56238c..5bf64bba 100644 --- a/apps/api/src/security/rbac/rbac.py +++ b/apps/api/src/security/rbac/rbac.py @@ -7,7 +7,7 @@ from src.db.courses.courses import Course from src.db.resource_authors import ResourceAuthor, ResourceAuthorshipEnum, ResourceAuthorshipStatusEnum from src.db.roles import Role from src.db.user_organizations import UserOrganization -from src.security.rbac.utils import check_element_type +from src.security.rbac.utils import check_element_type, check_course_permissions_with_own # Tested and working @@ -106,14 +106,30 @@ async def authorization_verify_based_on_roles( user_roles_in_organization_and_standard_roles = db_session.exec(statement).all() + + # Check if user is the author of the resource for "own" permissions + is_author = False + if action in ["update", "delete", "read"]: + is_author = await authorization_verify_if_user_is_author( + request, user_id, action, element_uuid, db_session + ) + # Check all roles until we find one that grants the permission for role in user_roles_in_organization_and_standard_roles: role = Role.model_validate(role) if role.rights: rights = role.rights element_rights = getattr(rights, element_type, None) - if element_rights and getattr(element_rights, f"action_{action}", False): - return True + if element_rights: + # Special handling for courses with PermissionsWithOwn + if element_type == "courses": + if await check_course_permissions_with_own(element_rights, action, is_author): + return True + else: + # For non-course resources, only check general permissions + # (regular Permission class no longer has "own" permissions) + if getattr(element_rights, f"action_{action}", False): + return True # If we get here, no role granted the permission return False diff --git a/apps/api/src/security/rbac/utils.py b/apps/api/src/security/rbac/utils.py index d6960a24..9904e65d 100644 --- a/apps/api/src/security/rbac/utils.py +++ b/apps/api/src/security/rbac/utils.py @@ -30,6 +30,38 @@ async def check_element_type(element_uuid): ) +async def check_course_permissions_with_own( + element_rights, + action: str, + is_author: bool = False +) -> bool: + """ + Check course-specific permissions including "own" permissions. + + Args: + element_rights: The rights object for courses (PermissionsWithOwn) + action: The action to check ("read", "update", "delete", "create") + is_author: Whether the user is the author of the course + + Returns: + bool: True if permission is granted, False otherwise + """ + if not element_rights: + return False + + # Check for general permission first + if getattr(element_rights, f"action_{action}", False): + return True + + # Check for "own" permission if user is the author + if is_author: + own_action = f"action_{action}_own" + if getattr(element_rights, own_action, False): + return True + + return False + + async def get_singular_form_of_element(element_uuid): element_type = await check_element_type(element_uuid) diff --git a/apps/api/src/services/courses/activities/activities.py b/apps/api/src/services/courses/activities/activities.py index 42ad1f05..5929a920 100644 --- a/apps/api/src/services/courses/activities/activities.py +++ b/apps/api/src/services/courses/activities/activities.py @@ -1,12 +1,6 @@ -from typing import Literal from sqlmodel import Session, select from src.db.courses.courses import Course from src.db.courses.chapters import Chapter -from src.security.rbac.rbac import ( - authorization_verify_based_on_roles_and_authorship, - authorization_verify_if_element_is_public, - authorization_verify_if_user_is_anon, -) from src.db.courses.activities import ActivityCreate, Activity, ActivityRead, ActivityUpdate from src.db.courses.chapter_activities import ChapterActivity from src.db.users import AnonymousUser, PublicUser @@ -15,6 +9,7 @@ from uuid import uuid4 from datetime import datetime from src.services.payments.payments_access import check_activity_paid_access +from src.security.courses_security import courses_rbac_check_for_activities #################################################### @@ -49,7 +44,7 @@ async def create_activity( detail="Course not found", ) - await rbac_check(request, course.course_uuid, current_user, "create", db_session) + await courses_rbac_check_for_activities(request, course.course_uuid, current_user, "create", db_session) # Create Activity activity = Activity(**activity_object.model_dump()) @@ -118,7 +113,7 @@ async def get_activity( activity, course = result # RBAC check - await rbac_check(request, course.course_uuid, current_user, "read", db_session) + await courses_rbac_check_for_activities(request, course.course_uuid, current_user, "read", db_session) # Paid access check has_paid_access = await check_activity_paid_access( @@ -156,7 +151,7 @@ async def get_activityby_id( activity, course = result # RBAC check - await rbac_check(request, course.course_uuid, current_user, "read", db_session) + await courses_rbac_check_for_activities(request, course.course_uuid, current_user, "read", db_session) return ActivityRead.model_validate(activity) @@ -187,7 +182,7 @@ async def update_activity( detail="Course not found", ) - await rbac_check(request, course.course_uuid, current_user, "update", db_session) + await courses_rbac_check_for_activities(request, course.course_uuid, current_user, "update", db_session) # Update only the fields that were passed in for var, value in vars(activity_object).items(): @@ -228,7 +223,7 @@ async def delete_activity( detail="Course not found", ) - await rbac_check(request, course.course_uuid, current_user, "delete", db_session) + await courses_rbac_check_for_activities(request, course.course_uuid, current_user, "delete", db_session) # Delete activity from chapter statement = select(ChapterActivity).where( @@ -296,46 +291,8 @@ async def get_activities( detail="Course not found", ) - await rbac_check(request, course.course_uuid, current_user, "read", db_session) + await courses_rbac_check_for_activities(request, course.course_uuid, current_user, "read", db_session) activities = [ActivityRead.model_validate(activity) for activity in activities] return activities - - -## 🔒 RBAC Utils ## - - -async def rbac_check( - request: Request, - element_uuid: str, - current_user: PublicUser | AnonymousUser, - action: Literal["create", "read", "update", "delete"], - db_session: Session, -): - - - if action == "read": - if current_user.id == 0: # Anonymous user - res = await authorization_verify_if_element_is_public( - request, element_uuid, action, db_session - ) - return res - else: - res = await authorization_verify_based_on_roles_and_authorship( - request, current_user.id, action, element_uuid, db_session - ) - return res - else: - # For non-read actions, proceed with regular RBAC checks - await authorization_verify_if_user_is_anon(current_user.id) - await authorization_verify_based_on_roles_and_authorship( - request, - current_user.id, - action, - element_uuid, - db_session, - ) - - -## 🔒 RBAC Utils ## diff --git a/apps/api/src/services/courses/activities/assignments.py b/apps/api/src/services/courses/activities/assignments.py index e936c800..b18a6d18 100644 --- a/apps/api/src/services/courses/activities/assignments.py +++ b/apps/api/src/services/courses/activities/assignments.py @@ -1,5 +1,4 @@ from datetime import datetime -from typing import Literal from uuid import uuid4 from fastapi import HTTPException, Request, UploadFile from sqlmodel import Session, select @@ -34,9 +33,6 @@ from src.security.features_utils.usage import ( increase_feature_usage, ) from src.security.rbac.rbac import ( - authorization_verify_based_on_roles_and_authorship, - authorization_verify_if_element_is_public, - authorization_verify_if_user_is_anon, authorization_verify_based_on_roles, ) from src.services.courses.activities.uploads.sub_file import upload_submission_file @@ -45,6 +41,7 @@ from src.services.courses.activities.uploads.tasks_ref_files import ( ) from src.services.trail.trail import check_trail_presence from src.services.courses.certifications import check_course_completion_and_create_certificate +from src.security.courses_security import courses_rbac_check_for_assignments ## > Assignments CRUD @@ -66,7 +63,7 @@ async def create_assignment( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "create", db_session) + await courses_rbac_check_for_assignments(request, course.course_uuid, current_user, "create", db_session) # Usage check check_limits_with_usage("assignments", course.org_id, db_session) @@ -118,7 +115,7 @@ async def read_assignment( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "read", db_session) + await courses_rbac_check_for_assignments(request, course.course_uuid, current_user, "read", db_session) # return assignment read return AssignmentRead.model_validate(assignment) @@ -161,7 +158,7 @@ async def read_assignment_from_activity_uuid( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "read", db_session) + await courses_rbac_check_for_assignments(request, course.course_uuid, current_user, "read", db_session) # return assignment read return AssignmentRead.model_validate(assignment) @@ -195,7 +192,7 @@ async def update_assignment( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "update", db_session) + await courses_rbac_check_for_assignments(request, course.course_uuid, current_user, "update", db_session) # Update only the fields that were passed in for var, value in vars(assignment_object).items(): @@ -239,7 +236,7 @@ async def delete_assignment( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "delete", db_session) + await courses_rbac_check_for_assignments(request, course.course_uuid, current_user, "delete", db_session) # Feature usage decrease_feature_usage("assignments", course.org_id, db_session) @@ -289,7 +286,7 @@ async def delete_assignment_from_activity_uuid( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "delete", db_session) + await courses_rbac_check_for_assignments(request, course.course_uuid, current_user, "delete", db_session) # Feature usage decrease_feature_usage("assignments", course.org_id, db_session) @@ -333,7 +330,7 @@ async def create_assignment_task( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "create", db_session) + await courses_rbac_check_for_assignments(request, course.course_uuid, current_user, "create", db_session) # Create Assignment Task assignment_task = AssignmentTask(**assignment_task_object.model_dump()) @@ -388,7 +385,7 @@ async def read_assignment_tasks( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "read", db_session) + await courses_rbac_check_for_assignments(request, course.course_uuid, current_user, "read", db_session) # return assignment tasks read return [ @@ -436,7 +433,7 @@ async def read_assignment_task( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "read", db_session) + await courses_rbac_check_for_assignments(request, course.course_uuid, current_user, "read", db_session) # return assignment task read return AssignmentTaskRead.model_validate(assignmenttask) @@ -490,7 +487,7 @@ async def put_assignment_task_reference_file( org = db_session.exec(org_statement).first() # RBAC check - await rbac_check(request, course.course_uuid, current_user, "update", db_session) + await courses_rbac_check_for_assignments(request, course.course_uuid, current_user, "update", db_session) # Upload reference file if reference_file and reference_file.filename and activity and org: @@ -568,7 +565,7 @@ async def put_assignment_task_submission_file( org = db_session.exec(org_statement).first() # RBAC check - only need read permission to submit files - await rbac_check(request, course.course_uuid, current_user, "read", db_session) + await courses_rbac_check_for_assignments(request, course.course_uuid, current_user, "read", db_session) # Check if user is enrolled in the course if not await authorization_verify_based_on_roles(request, current_user.id, "read", course.course_uuid, db_session): @@ -633,7 +630,7 @@ async def update_assignment_task( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "update", db_session) + await courses_rbac_check_for_assignments(request, course.course_uuid, current_user, "update", db_session) # Update only the fields that were passed in for var, value in vars(assignment_task_object).items(): @@ -689,7 +686,7 @@ async def delete_assignment_task( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "delete", db_session) + await courses_rbac_check_for_assignments(request, course.course_uuid, current_user, "delete", db_session) # Delete Assignment Task db_session.delete(assignment_task) @@ -741,7 +738,7 @@ async def handle_assignment_task_submission( detail="Course not found", ) - # Check if user has instructor/admin permissions + # SECURITY: Check if user has instructor/admin permissions for grading is_instructor = await authorization_verify_based_on_roles(request, current_user.id, "update", course.course_uuid, db_session) # For regular users, ensure they can only submit their own work @@ -753,7 +750,7 @@ async def handle_assignment_task_submission( detail="You must be enrolled in this course to submit assignments" ) - # Regular users cannot update grades - only check if actual values are being set + # SECURITY: Regular users cannot update grades - only check if actual values are being set if (assignment_task_submission_object.grade is not None and assignment_task_submission_object.grade != 0) or \ (assignment_task_submission_object.task_submission_grade_feedback is not None and assignment_task_submission_object.task_submission_grade_feedback != ""): raise HTTPException( @@ -762,10 +759,10 @@ async def handle_assignment_task_submission( ) # Only need read permission for submissions - await rbac_check(request, course.course_uuid, current_user, "read", db_session) + await courses_rbac_check_for_assignments(request, course.course_uuid, current_user, "read", db_session) else: - # Instructors/admins need update permission to grade - await rbac_check(request, course.course_uuid, current_user, "update", db_session) + # SECURITY: Instructors/admins need update permission to grade + await courses_rbac_check_for_assignments(request, course.course_uuid, current_user, "update", db_session) # Try to find existing submission if UUID is provided assignment_task_submission = None @@ -777,7 +774,7 @@ async def handle_assignment_task_submission( # If submission exists, update it if assignment_task_submission: - # For regular users, ensure they can only update their own submissions + # SECURITY: For regular users, ensure they can only update their own submissions if not is_instructor and assignment_task_submission.user_id != current_user.id: raise HTTPException( status_code=403, @@ -880,7 +877,7 @@ async def read_user_assignment_task_submissions( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "read", db_session) + await courses_rbac_check_for_assignments(request, course.course_uuid, current_user, "read", db_session) # return assignment task submission read return AssignmentTaskSubmissionRead.model_validate(assignment_task_submission) @@ -953,7 +950,7 @@ async def read_assignment_task_submissions( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "read", db_session) + await courses_rbac_check_for_assignments(request, course.course_uuid, current_user, "read", db_session) # return assignment task submission read return AssignmentTaskSubmissionRead.model_validate(assignment_task_submission) @@ -1012,7 +1009,7 @@ async def update_assignment_task_submission( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "read", db_session) + await courses_rbac_check_for_assignments(request, course.course_uuid, current_user, "read", db_session) # Update only the fields that were passed in for var, value in vars(assignment_task_submission_object).items(): @@ -1081,7 +1078,7 @@ async def delete_assignment_task_submission( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "delete", db_session) + await courses_rbac_check_for_assignments(request, course.course_uuid, current_user, "delete", db_session) # Delete Assignment Task Submission db_session.delete(assignment_task_submission) @@ -1147,7 +1144,7 @@ async def create_assignment_submission( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "read", db_session) + await courses_rbac_check_for_assignments(request, course.course_uuid, current_user, "read", db_session) # Create Assignment User Submission assignment_user_submission = AssignmentUserSubmission( @@ -1280,7 +1277,7 @@ async def read_assignment_submissions( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "read", db_session) + await courses_rbac_check_for_assignments(request, course.course_uuid, current_user, "read", db_session) # return assignment tasks read return [ @@ -1323,7 +1320,7 @@ async def read_user_assignment_submissions( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "read", db_session) + await courses_rbac_check_for_assignments(request, course.course_uuid, current_user, "read", db_session) # return assignment tasks read return [ @@ -1389,7 +1386,7 @@ async def update_assignment_submission( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "read", db_session) + await courses_rbac_check_for_assignments(request, course.course_uuid, current_user, "read", db_session) # Update only the fields that were passed in for var, value in vars(assignment_user_submission_object).items(): @@ -1447,7 +1444,7 @@ async def delete_assignment_submission( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "delete", db_session) + await courses_rbac_check_for_assignments(request, course.course_uuid, current_user, "delete", db_session) # Delete Assignment User Submission db_session.delete(assignment_user_submission) @@ -1464,7 +1461,7 @@ async def grade_assignment_submission( current_user: PublicUser | AnonymousUser, db_session: Session, ): - + # SECURITY: This function should only be accessible by course owners or instructors # Check if assignment exists statement = select(Assignment).where(Assignment.assignment_uuid == assignment_uuid) assignment = db_session.exec(statement).first() @@ -1484,7 +1481,8 @@ async def grade_assignment_submission( detail="Course not found", ) - await rbac_check(request, course.course_uuid, current_user, "update", db_session) + # SECURITY: Require course ownership or instructor role for grading + await courses_rbac_check_for_assignments(request, course.course_uuid, current_user, "update", db_session) # Check if assignment user submission exists statement = select(AssignmentUserSubmission).where( @@ -1602,6 +1600,7 @@ async def mark_activity_as_done_for_user( current_user: PublicUser | AnonymousUser, db_session: Session, ): + # SECURITY: This function should only be accessible by course owners or instructors # Get Assignment statement = select(Assignment).where(Assignment.assignment_uuid == assignment_uuid) assignment = db_session.exec(statement).first() @@ -1625,7 +1624,8 @@ async def mark_activity_as_done_for_user( detail="Course not found", ) - await rbac_check(request, course.course_uuid, current_user, "update", db_session) + # SECURITY: Require course ownership or instructor role for marking activities as done + await courses_rbac_check_for_assignments(request, course.course_uuid, current_user, "update", db_session) if not activity: raise HTTPException( @@ -1704,46 +1704,7 @@ async def get_assignments_from_course( assignments.append(assignment) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "read", db_session) + await courses_rbac_check_for_assignments(request, course.course_uuid, current_user, "read", db_session) # return assignments read return [AssignmentRead.model_validate(assignment) for assignment in assignments] - - -## 🔒 RBAC Utils ## - - -async def rbac_check( - request: Request, - course_uuid: str, - current_user: PublicUser | AnonymousUser, - action: Literal["create", "read", "update", "delete"], - db_session: Session, -): - - if action == "read": - if current_user.id == 0: # Anonymous user - res = await authorization_verify_if_element_is_public( - request, course_uuid, action, db_session - ) - return res - else: - res = ( - await authorization_verify_based_on_roles_and_authorship( - request, current_user.id, action, course_uuid, db_session - ) - ) - return res - else: - await authorization_verify_if_user_is_anon(current_user.id) - - await authorization_verify_based_on_roles_and_authorship( - request, - current_user.id, - action, - course_uuid, - db_session, - ) - - -## 🔒 RBAC Utils ## diff --git a/apps/api/src/services/courses/activities/pdf.py b/apps/api/src/services/courses/activities/pdf.py index e6f2ca51..ae10de6e 100644 --- a/apps/api/src/services/courses/activities/pdf.py +++ b/apps/api/src/services/courses/activities/pdf.py @@ -1,11 +1,6 @@ -from typing import Literal from src.db.courses.courses import Course from src.db.organizations import Organization from sqlmodel import Session, select -from src.security.rbac.rbac import ( - authorization_verify_based_on_roles_and_authorship, - authorization_verify_if_user_is_anon, -) from src.db.courses.chapters import Chapter from src.db.courses.activities import ( Activity, @@ -20,6 +15,7 @@ from src.services.courses.activities.uploads.pdfs import upload_pdf from fastapi import HTTPException, status, UploadFile, Request from uuid import uuid4 from datetime import datetime +from src.security.courses_security import courses_rbac_check_for_activities async def create_documentpdf_activity( @@ -30,9 +26,6 @@ async def create_documentpdf_activity( db_session: Session, pdf_file: UploadFile | None = None, ): - # RBAC check - await rbac_check(request, "activity_x", current_user, "create", db_session) - # get chapter_id statement = select(Chapter).where(Chapter.id == chapter_id) chapter = db_session.exec(statement).first() @@ -52,6 +45,19 @@ async def create_documentpdf_activity( detail="CourseChapter not found", ) + # Get course_uuid for RBAC check + statement = select(Course).where(Course.id == coursechapter.course_id) + course = db_session.exec(statement).first() + + if not course: + raise HTTPException( + status_code=404, + detail="Course not found", + ) + + # RBAC check + await courses_rbac_check_for_activities(request, course.course_uuid, current_user, "create", db_session) + # get org_id org_id = coursechapter.org_id @@ -59,10 +65,6 @@ async def create_documentpdf_activity( statement = select(Organization).where(Organization.id == coursechapter.org_id) organization = db_session.exec(statement).first() - # Get course_uuid - statement = select(Course).where(Course.id == coursechapter.course_id) - course = db_session.exec(statement).first() - # create activity uuid activity_uuid = f"activity_{uuid4()}" @@ -94,9 +96,7 @@ async def create_documentpdf_activity( content={ "filename": "documentpdf." + pdf_format, "activity_uuid": activity_uuid, - }, - published_version=1, - version=1, + }, org_id=org_id if org_id else 0, course_id=coursechapter.course_id, activity_uuid=activity_uuid, @@ -121,7 +121,7 @@ async def create_documentpdf_activity( ) # upload pdf - if pdf_file: + if pdf_file and organization and course: # get pdffile format await upload_pdf( pdf_file, @@ -136,27 +136,3 @@ async def create_documentpdf_activity( db_session.refresh(activity_chapter) return ActivityRead.model_validate(activity) - - -## 🔒 RBAC Utils ## - - -async def rbac_check( - request: Request, - course_id: str, - current_user: PublicUser | AnonymousUser, - action: Literal["create", "read", "update", "delete"], - db_session: Session, -): - await authorization_verify_if_user_is_anon(current_user.id) - - await authorization_verify_based_on_roles_and_authorship( - request, - current_user.id, - action, - course_id, - db_session, - ) - - -## 🔒 RBAC Utils ## diff --git a/apps/api/src/services/courses/activities/video.py b/apps/api/src/services/courses/activities/video.py index 71408033..2072e1b9 100644 --- a/apps/api/src/services/courses/activities/video.py +++ b/apps/api/src/services/courses/activities/video.py @@ -5,10 +5,6 @@ from src.db.organizations import Organization from pydantic import BaseModel from sqlmodel import Session, select -from src.security.rbac.rbac import ( - authorization_verify_based_on_roles_and_authorship, - authorization_verify_if_user_is_anon, -) from src.db.courses.chapters import Chapter from src.db.courses.activities import ( Activity, @@ -23,6 +19,7 @@ from src.services.courses.activities.uploads.videos import upload_video from fastapi import HTTPException, status, UploadFile, Request from uuid import uuid4 from datetime import datetime +from src.security.courses_security import courses_rbac_check_for_activities async def create_video_activity( @@ -34,9 +31,6 @@ async def create_video_activity( video_file: UploadFile | None = None, details: str = "{}", ): - # RBAC check - await rbac_check(request, "activity_x", current_user, "create", db_session) - # get chapter_id statement = select(Chapter).where(Chapter.id == chapter_id) chapter = db_session.exec(statement).first() @@ -59,14 +53,23 @@ async def create_video_activity( detail="CourseChapter not found", ) + # Get course_uuid for RBAC check + statement = select(Course).where(Course.id == coursechapter.course_id) + course = db_session.exec(statement).first() + + if not course: + raise HTTPException( + status_code=404, + detail="Course not found", + ) + + # RBAC check + await courses_rbac_check_for_activities(request, course.course_uuid, current_user, "create", db_session) + # Get org_uuid statement = select(Organization).where(Organization.id == coursechapter.org_id) organization = db_session.exec(statement).first() - # Get course_uuid - statement = select(Course).where(Course.id == coursechapter.course_id) - course = db_session.exec(statement).first() - # generate activity_uuid activity_uuid = str(f"activity_{uuid4()}") @@ -99,13 +102,11 @@ async def create_video_activity( activity_uuid=activity_uuid, org_id=coursechapter.org_id, course_id=coursechapter.course_id, - published_version=1, content={ "filename": "video." + video_format, "activity_uuid": activity_uuid, }, - details=details, - version=1, + details=details if isinstance(details, dict) else json.loads(details), creation_date=str(datetime.now()), update_date=str(datetime.now()), ) @@ -117,7 +118,7 @@ async def create_video_activity( db_session.refresh(activity) # upload video - if video_file: + if video_file and organization and course: # get videofile format await upload_video( video_file, @@ -163,9 +164,6 @@ async def create_external_video_activity( data: ExternalVideo, db_session: Session, ): - # RBAC check - await rbac_check(request, "activity_x", current_user, "create", db_session) - # get chapter_id statement = select(Chapter).where(Chapter.id == data.chapter_id) chapter = db_session.exec(statement).first() @@ -185,6 +183,19 @@ async def create_external_video_activity( detail="CourseChapter not found", ) + # Get course_uuid for RBAC check + statement = select(Course).where(Course.id == coursechapter.course_id) + course = db_session.exec(statement).first() + + if not course: + raise HTTPException( + status_code=404, + detail="Course not found", + ) + + # RBAC check + await courses_rbac_check_for_activities(request, course.course_uuid, current_user, "create", db_session) + # generate activity_uuid activity_uuid = str(f"activity_{uuid4()}") @@ -198,14 +209,12 @@ async def create_external_video_activity( activity_uuid=activity_uuid, course_id=coursechapter.course_id, org_id=coursechapter.org_id, - published_version=1, content={ "uri": data.uri, "type": data.type, "activity_uuid": activity_uuid, }, details=details, - version=1, creation_date=str(datetime.now()), update_date=str(datetime.now()), ) @@ -234,22 +243,4 @@ async def create_external_video_activity( return ActivityRead.model_validate(activity) -async def rbac_check( - request: Request, - course_id: str, - current_user: PublicUser | AnonymousUser, - action: Literal["create", "read", "update", "delete"], - db_session: Session, -): - await authorization_verify_if_user_is_anon(current_user.id) - - await authorization_verify_based_on_roles_and_authorship( - request, - current_user.id, - action, - course_id, - db_session, - ) - - ## 🔒 RBAC Utils ## diff --git a/apps/api/src/services/courses/certifications.py b/apps/api/src/services/courses/certifications.py index af5a2b54..87a78297 100644 --- a/apps/api/src/services/courses/certifications.py +++ b/apps/api/src/services/courses/certifications.py @@ -1,4 +1,4 @@ -from typing import List, Literal +from typing import List from uuid import uuid4 from datetime import datetime from sqlmodel import Session, select @@ -15,11 +15,7 @@ from src.db.courses.courses import Course from src.db.courses.chapter_activities import ChapterActivity from src.db.trail_steps import TrailStep from src.db.users import PublicUser, AnonymousUser -from src.security.rbac.rbac import ( - authorization_verify_based_on_roles_and_authorship, - authorization_verify_if_element_is_public, - authorization_verify_if_user_is_anon, -) +from src.security.courses_security import courses_rbac_check_for_certifications #################################################### @@ -46,7 +42,7 @@ async def create_certification( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "create", db_session) + await courses_rbac_check_for_certifications(request, course.course_uuid, current_user, "create", db_session) # Create certification certification = Certifications( @@ -93,7 +89,7 @@ async def get_certification( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "read", db_session) + await courses_rbac_check_for_certifications(request, course.course_uuid, current_user, "read", db_session) return CertificationRead(**certification.model_dump()) @@ -117,7 +113,7 @@ async def get_certifications_by_course( ) # RBAC check - await rbac_check(request, course_uuid, current_user, "read", db_session) + await courses_rbac_check_for_certifications(request, course_uuid, current_user, "read", db_session) # Get certifications for this course statement = select(Certifications).where(Certifications.course_id == course.id) @@ -155,7 +151,7 @@ async def update_certification( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "update", db_session) + await courses_rbac_check_for_certifications(request, course.course_uuid, current_user, "update", db_session) # Update only the fields that were passed in for var, value in vars(certification_object).items(): @@ -200,7 +196,7 @@ async def delete_certification( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "delete", db_session) + await courses_rbac_check_for_certifications(request, course.course_uuid, current_user, "delete", db_session) db_session.delete(certification) db_session.commit() @@ -218,8 +214,16 @@ async def create_certificate_user( user_id: int, certification_id: int, db_session: Session, + current_user: PublicUser | AnonymousUser | None = None, ) -> CertificateUserRead: - """Create a certificate user link""" + """ + Create a certificate user link + + SECURITY NOTES: + - This function should only be called by authorized users (course owners, instructors, or system) + - When called from check_course_completion_and_create_certificate, it's a system operation + - When called directly, requires proper RBAC checks + """ # Check if certification exists statement = select(Certifications).where(Certifications.id == certification_id) @@ -231,6 +235,21 @@ async def create_certificate_user( detail="Certification not found", ) + # SECURITY: If current_user is provided, perform RBAC check + if current_user: + # Get course for RBAC check + statement = select(Course).where(Course.id == certification.course_id) + course = db_session.exec(statement).first() + + if not course: + raise HTTPException( + status_code=404, + detail="Course not found", + ) + + # Require course ownership or instructor role for creating certificates + await courses_rbac_check_for_certifications(request, course.course_uuid, current_user, "create", db_session) + # Check if certificate user already exists statement = select(CertificateUser).where( CertificateUser.user_id == user_id, @@ -316,7 +335,7 @@ async def get_user_certificates_for_course( ) # RBAC check - await rbac_check(request, course_uuid, current_user, "read", db_session) + await courses_rbac_check_for_certifications(request, course_uuid, current_user, "read", db_session) # Get all certifications for this course statement = select(Certifications).where(Certifications.course_id == course.id) @@ -357,7 +376,14 @@ async def check_course_completion_and_create_certificate( course_id: int, db_session: Session, ) -> bool: - """Check if all activities in a course are completed and create certificate if so""" + """ + Check if all activities in a course are completed and create certificate if so + + SECURITY NOTES: + - This function is called by the system when activities are completed + - It should only create certificates for users who have actually completed the course + - The function is called from mark_activity_as_done_for_user which already has RBAC checks + """ # Get all activities in the course statement = select(ChapterActivity).where(ChapterActivity.course_id == course_id) @@ -381,7 +407,8 @@ async def check_course_completion_and_create_certificate( certification = db_session.exec(statement).first() if certification and certification.id: - # Create certificate user link + # SECURITY: Create certificate user link (system operation, no RBAC needed here) + # This is called from mark_activity_as_done_for_user which already has proper RBAC checks try: await create_certificate_user(request, user_id, certification.id, db_session) return True @@ -505,37 +532,4 @@ async def get_all_user_certificates( } if user else None }) - return result - - -#################################################### -# RBAC Utils -#################################################### - - -async def rbac_check( - request: Request, - course_uuid: str, - current_user: PublicUser | AnonymousUser, - action: Literal["create", "read", "update", "delete"], - db_session: Session, -): - if action == "read": - if current_user.id == 0: # Anonymous user - await authorization_verify_if_element_is_public( - request, course_uuid, action, db_session - ) - else: - await authorization_verify_based_on_roles_and_authorship( - request, current_user.id, action, course_uuid, db_session - ) - else: - await authorization_verify_if_user_is_anon(current_user.id) - - await authorization_verify_based_on_roles_and_authorship( - request, - current_user.id, - action, - course_uuid, - db_session, - ) \ No newline at end of file + return result \ No newline at end of file diff --git a/apps/api/src/services/courses/chapters.py b/apps/api/src/services/courses/chapters.py index 4a30bb3d..ee72bc81 100644 --- a/apps/api/src/services/courses/chapters.py +++ b/apps/api/src/services/courses/chapters.py @@ -1,13 +1,8 @@ from datetime import datetime -from typing import List, Literal +from typing import List from uuid import uuid4 from sqlmodel import Session, select -from src.db.users import AnonymousUser -from src.security.rbac.rbac import ( - authorization_verify_based_on_roles_and_authorship, - authorization_verify_if_element_is_public, - authorization_verify_if_user_is_anon, -) +from src.db.users import AnonymousUser, PublicUser from src.db.courses.course_chapters import CourseChapter from src.db.courses.activities import Activity, ActivityRead from src.db.courses.chapter_activities import ChapterActivity @@ -18,9 +13,9 @@ from src.db.courses.chapters import ( ChapterUpdate, ChapterUpdateOrder, ) -from src.services.courses.courses import Course -from src.services.users.users import PublicUser +from src.db.courses.courses import Course from fastapi import HTTPException, status, Request +from src.security.courses_security import courses_rbac_check_for_chapters #################################################### @@ -42,7 +37,7 @@ async def create_chapter( course = db_session.exec(statement).one() # RBAC check - await rbac_check(request, "chapter_x", current_user, "create", db_session) + await courses_rbac_check_for_chapters(request, course.course_uuid, current_user, "create", db_session) # complete chapter object chapter.course_id = chapter_object.course_id @@ -55,7 +50,7 @@ async def create_chapter( statement = ( select(CourseChapter) .where(CourseChapter.course_id == chapter.course_id) - .order_by(CourseChapter.order) + .order_by(CourseChapter.order) # type: ignore ) course_chapters = db_session.exec(statement).all() @@ -122,14 +117,14 @@ async def get_chapter( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "read", db_session) + await courses_rbac_check_for_chapters(request, course.course_uuid, current_user, "read", db_session) # Get activities for this chapter statement = ( select(Activity) - .join(ChapterActivity, Activity.id == ChapterActivity.activity_id) + .join(ChapterActivity, Activity.id == ChapterActivity.activity_id) # type: ignore .where(ChapterActivity.chapter_id == chapter_id) - .distinct(Activity.id) + .distinct(Activity.id) # type: ignore ) activities = db_session.exec(statement).all() @@ -158,7 +153,7 @@ async def update_chapter( ) # RBAC check - await rbac_check(request, chapter.chapter_uuid, current_user, "update", db_session) + await courses_rbac_check_for_chapters(request, chapter.chapter_uuid, current_user, "update", db_session) # Update only the fields that were passed in for var, value in vars(chapter_object).items(): @@ -193,7 +188,7 @@ async def delete_chapter( ) # RBAC check - await rbac_check(request, chapter.chapter_uuid, current_user, "delete", db_session) + await courses_rbac_check_for_chapters(request, chapter.chapter_uuid, current_user, "delete", db_session) # Remove all linked chapter activities statement = select(ChapterActivity).where(ChapterActivity.chapter_id == chapter.id) @@ -224,26 +219,26 @@ async def get_course_chapters( statement = ( select(Chapter) - .join(CourseChapter, Chapter.id == CourseChapter.chapter_id) + .join(CourseChapter, Chapter.id == CourseChapter.chapter_id) # type: ignore .where(CourseChapter.course_id == course_id) .where(Chapter.course_id == course_id) - .order_by(CourseChapter.order) - .group_by(Chapter.id, CourseChapter.order) + .order_by(CourseChapter.order) # type: ignore + .group_by(Chapter.id, CourseChapter.order) # type: ignore ) chapters = db_session.exec(statement).all() chapters = [ChapterRead(**chapter.model_dump(), activities=[]) for chapter in chapters] # RBAC check - await rbac_check(request, course.course_uuid, current_user, "read", db_session) # type: ignore + await courses_rbac_check_for_chapters(request, course.course_uuid, current_user, "read", db_session) # type: ignore # Get activities for each chapter for chapter in chapters: statement = ( select(ChapterActivity) .where(ChapterActivity.chapter_id == chapter.id) - .order_by(ChapterActivity.order) - .distinct(ChapterActivity.id, ChapterActivity.order) + .order_by(ChapterActivity.order) # type: ignore + .distinct(ChapterActivity.id, ChapterActivity.order) # type: ignore ) chapter_activities = db_session.exec(statement).all() @@ -251,7 +246,7 @@ async def get_course_chapters( statement = ( select(Activity) .where(Activity.id == chapter_activity.activity_id, with_unpublished_activities or Activity.published == True) - .distinct(Activity.id) + .distinct(Activity.id) # type: ignore ) activity = db_session.exec(statement).first() @@ -279,7 +274,7 @@ async def DEPRECEATED_get_course_chapters( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "read", db_session) + await courses_rbac_check_for_chapters(request, course.course_uuid, current_user, "read", db_session) chapters_in_db = await get_course_chapters(request, course.id, db_session, current_user) # type: ignore @@ -306,9 +301,9 @@ async def DEPRECEATED_get_course_chapters( activities_list = {} statement = ( select(Activity) - .join(ChapterActivity, ChapterActivity.activity_id == Activity.id) + .join(ChapterActivity, ChapterActivity.activity_id == Activity.id) # type: ignore .where(ChapterActivity.activity_id == Activity.id) - .group_by(Activity.id) + .group_by(Activity.id) # type: ignore ) activities_in_db = db_session.exec(statement).all() @@ -324,10 +319,10 @@ async def DEPRECEATED_get_course_chapters( # get chapter order statement = ( select(Chapter) - .join(CourseChapter, CourseChapter.chapter_id == Chapter.id) + .join(CourseChapter, CourseChapter.chapter_id == Chapter.id) # type: ignore .where(CourseChapter.chapter_id == Chapter.id) - .group_by(Chapter.id, CourseChapter.order) - .order_by(CourseChapter.order) + .group_by(Chapter.id, CourseChapter.order) # type: ignore + .order_by(CourseChapter.order) # type: ignore ) chapters_in_db = db_session.exec(statement).all() @@ -361,7 +356,7 @@ async def reorder_chapters_and_activities( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "update", db_session) + await courses_rbac_check_for_chapters(request, course.course_uuid, current_user, "update", db_session) ########### # Chapters @@ -458,39 +453,3 @@ async def reorder_chapters_and_activities( db_session.commit() return {"detail": "Chapters and activities reordered successfully"} - - -## 🔒 RBAC Utils ## - - -async def rbac_check( - request: Request, - course_uuid: str, - current_user: PublicUser | AnonymousUser, - action: Literal["create", "read", "update", "delete"], - db_session: Session, -): - if action == "read": - if current_user.id == 0: # Anonymous user - res = await authorization_verify_if_element_is_public( - request, course_uuid, action, db_session - ) - return res - else: - res = await authorization_verify_based_on_roles_and_authorship( - request, current_user.id, action, course_uuid, db_session - ) - return res - else: - await authorization_verify_if_user_is_anon(current_user.id) - - await authorization_verify_based_on_roles_and_authorship( - request, - current_user.id, - action, - course_uuid, - db_session, - ) - - -## 🔒 RBAC Utils ## diff --git a/apps/api/src/services/courses/collections.py b/apps/api/src/services/courses/collections.py index 774e2393..80713b03 100644 --- a/apps/api/src/services/courses/collections.py +++ b/apps/api/src/services/courses/collections.py @@ -1,13 +1,8 @@ from datetime import datetime -from typing import List, Literal +from typing import List from uuid import uuid4 from sqlmodel import Session, select -from src.db.users import AnonymousUser -from src.security.rbac.rbac import ( - authorization_verify_based_on_roles_and_authorship, - authorization_verify_if_element_is_public, - authorization_verify_if_user_is_anon, -) +from src.db.users import AnonymousUser, PublicUser from src.db.collections import ( Collection, CollectionCreate, @@ -16,8 +11,8 @@ from src.db.collections import ( ) from src.db.collections_courses import CollectionCourse from src.db.courses.courses import Course -from src.services.users.users import PublicUser from fastapi import HTTPException, status, Request +from src.security.courses_security import courses_rbac_check_for_collections #################################################### @@ -40,7 +35,7 @@ async def get_collection( ) # RBAC check - await rbac_check( + await courses_rbac_check_for_collections( request, collection.collection_uuid, current_user, "read", db_session ) @@ -86,8 +81,10 @@ async def create_collection( ) -> CollectionRead: collection = Collection.model_validate(collection_object) - # RBAC check - await rbac_check(request, "collection_x", current_user, "create", db_session) + # SECURITY: Check if user has permission to create collections in this organization + # Since collections are organization-level resources, we need to check org permissions + # For now, we'll use the existing RBAC check but with proper organization context + await courses_rbac_check_for_collections(request, "collection_x", current_user, "create", db_session) # Complete the collection object collection.collection_uuid = f"collection_{uuid4()}" @@ -99,18 +96,32 @@ async def create_collection( db_session.commit() db_session.refresh(collection) - # Link courses to collection + # SECURITY: Link courses to collection - ensure user has access to all courses being added if collection: for course_id in collection_object.courses: - collection_course = CollectionCourse( - collection_id=int(collection.id), # type: ignore - course_id=course_id, - org_id=int(collection_object.org_id), - creation_date=str(datetime.now()), - update_date=str(datetime.now()), - ) - # Add collection_course to database - db_session.add(collection_course) + # Check if user has access to this course + statement = select(Course).where(Course.id == course_id) + course = db_session.exec(statement).first() + + if course: + # Verify user has read access to the course before adding it to collection + try: + await courses_rbac_check_for_collections(request, course.course_uuid, current_user, "read", db_session) + except HTTPException: + raise HTTPException( + status_code=403, + detail=f"You don't have permission to add course {course.name} to this collection" + ) + + collection_course = CollectionCourse( + collection_id=int(collection.id), # type: ignore + course_id=course_id, + org_id=int(collection_object.org_id), + creation_date=str(datetime.now()), + update_date=str(datetime.now()), + ) + # Add collection_course to database + db_session.add(collection_course) db_session.commit() db_session.refresh(collection) @@ -145,7 +156,7 @@ async def update_collection( ) # RBAC check - await rbac_check( + await courses_rbac_check_for_collections( request, collection.collection_uuid, current_user, "update", db_session ) @@ -219,7 +230,7 @@ async def delete_collection( ) # RBAC check - await rbac_check( + await courses_rbac_check_for_collections( request, collection.collection_uuid, current_user, "delete", db_session ) @@ -248,7 +259,7 @@ async def get_collections( Collection.org_id == org_id, Collection.public == True ) statement_all = ( - select(Collection).where(Collection.org_id == org_id).distinct(Collection.id) + select(Collection).where(Collection.org_id == org_id).distinct(Collection.id) # type: ignore ) if current_user.id == 0: @@ -288,49 +299,7 @@ async def get_collections( courses = db_session.exec(statement).all() - collection = CollectionRead(**collection.model_dump(), courses=courses) + collection = CollectionRead(**collection.model_dump(), courses=list(courses)) collections_with_courses.append(collection) return collections_with_courses - - -## 🔒 RBAC Utils ## - - -async def rbac_check( - request: Request, - collection_uuid: str, - current_user: PublicUser | AnonymousUser, - action: Literal["create", "read", "update", "delete"], - db_session: Session, -): - if action == "read": - if current_user.id == 0: # Anonymous user - res = await authorization_verify_if_element_is_public( - request, collection_uuid, action, db_session - ) - if res == False: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="User rights : You are not allowed to read this collection", - ) - else: - res = ( - await authorization_verify_based_on_roles_and_authorship( - request, current_user.id, action, collection_uuid, db_session - ) - ) - return res - else: - await authorization_verify_if_user_is_anon(current_user.id) - - await authorization_verify_based_on_roles_and_authorship( - request, - current_user.id, - action, - collection_uuid, - db_session, - ) - - -## 🔒 RBAC Utils ## diff --git a/apps/api/src/services/courses/contributors.py b/apps/api/src/services/courses/contributors.py index b055c901..746fc615 100644 --- a/apps/api/src/services/courses/contributors.py +++ b/apps/api/src/services/courses/contributors.py @@ -1,10 +1,11 @@ from datetime import datetime -from fastapi import HTTPException, Request, status +from fastapi import HTTPException, Request from sqlmodel import Session, select, and_ from src.db.users import PublicUser, AnonymousUser, User, UserRead from src.db.courses.courses import Course from src.db.resource_authors import ResourceAuthor, ResourceAuthorshipEnum, ResourceAuthorshipStatusEnum -from src.security.rbac.rbac import authorization_verify_if_user_is_anon, authorization_verify_based_on_roles_and_authorship +from src.security.rbac.rbac import authorization_verify_if_user_is_anon +from src.security.courses_security import courses_rbac_check from typing import List @@ -14,6 +15,14 @@ async def apply_course_contributor( current_user: PublicUser | AnonymousUser, db_session: Session, ): + """ + Apply to become a course contributor + + SECURITY NOTES: + - Any authenticated user can apply to become a contributor + - Applications are created with PENDING status + - Only course owners (CREATOR, MAINTAINER) or admins can approve applications + """ # Verify user is not anonymous await authorization_verify_if_user_is_anon(current_user.id) @@ -73,21 +82,17 @@ async def update_course_contributor( ): """ Update a course contributor's role and status - Only administrators can perform this action + + SECURITY NOTES: + - Only course owners (CREATOR, MAINTAINER) or admins can update contributors + - Cannot modify the role of the course creator + - Requires strict course ownership checks """ # Verify user is not anonymous await authorization_verify_if_user_is_anon(current_user.id) - # RBAC check - verify if user has admin rights - authorized = await authorization_verify_based_on_roles_and_authorship( - request, current_user.id, "update", course_uuid, db_session - ) - - if not authorized: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="You are not authorized to update course contributors", - ) + # SECURITY: Require course ownership or admin role for updating contributors + await courses_rbac_check(request, course_uuid, current_user, "update", db_session) # Check if course exists statement = select(Course).where(Course.course_uuid == course_uuid) @@ -115,7 +120,7 @@ async def update_course_contributor( detail="Contributor not found for this course", ) - # Don't allow changing the role of the creator + # SECURITY: Don't allow changing the role of the creator if existing_authorship.authorship == ResourceAuthorshipEnum.CREATOR: raise HTTPException( status_code=400, @@ -144,6 +149,10 @@ async def get_course_contributors( ) -> List[dict]: """ Get all contributors for a course with their user information + + SECURITY NOTES: + - Requires read access to the course + - Contributors are visible to anyone with course read access """ # Check if course exists statement = select(Course).where(Course.course_uuid == course_uuid) @@ -155,6 +164,9 @@ async def get_course_contributors( detail="Course not found", ) + # SECURITY: Require read access to the course + await courses_rbac_check(request, course_uuid, current_user, "read", db_session) + # Get all contributors for this course with user information statement = ( select(ResourceAuthor, User) @@ -184,21 +196,17 @@ async def add_bulk_course_contributors( ): """ Add multiple contributors to a course by their usernames - Only administrators can perform this action + + SECURITY NOTES: + - Only course owners (CREATOR, MAINTAINER) or admins can add contributors + - Requires strict course ownership checks + - Cannot add contributors to courses the user doesn't own """ # Verify user is not anonymous await authorization_verify_if_user_is_anon(current_user.id) - # RBAC check - verify if user has admin rights - authorized = await authorization_verify_based_on_roles_and_authorship( - request, current_user.id, "update", course_uuid, db_session - ) - - if not authorized: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="You are not authorized to add contributors", - ) + # SECURITY: Require course ownership or admin role for adding contributors + await courses_rbac_check(request, course_uuid, current_user, "update", db_session) # Check if course exists statement = select(Course).where(Course.course_uuid == course_uuid) @@ -284,21 +292,18 @@ async def remove_bulk_course_contributors( ): """ Remove multiple contributors from a course by their usernames - Only administrators can perform this action + + SECURITY NOTES: + - Only course owners (CREATOR, MAINTAINER) or admins can remove contributors + - Requires strict course ownership checks + - Cannot remove contributors from courses the user doesn't own + - Cannot remove the course creator """ # Verify user is not anonymous await authorization_verify_if_user_is_anon(current_user.id) - # RBAC check - verify if user has admin rights - authorized = await authorization_verify_based_on_roles_and_authorship( - request, current_user.id, "update", course_uuid, db_session - ) - - if not authorized: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="You are not authorized to remove contributors", - ) + # SECURITY: Require course ownership or admin role for removing contributors + await courses_rbac_check(request, course_uuid, current_user, "update", db_session) # Check if course exists statement = select(Course).where(Course.course_uuid == course_uuid) @@ -346,7 +351,7 @@ async def remove_bulk_course_contributors( }) continue - # Don't allow removing the creator + # SECURITY: Don't allow removing the creator if existing_authorship.authorship == ResourceAuthorshipEnum.CREATOR: results["failed"].append({ "username": username, diff --git a/apps/api/src/services/courses/courses.py b/apps/api/src/services/courses/courses.py index 98030597..5dffdbe6 100644 --- a/apps/api/src/services/courses/courses.py +++ b/apps/api/src/services/courses/courses.py @@ -1,4 +1,4 @@ -from typing import Literal, List +from typing import List from uuid import uuid4 from sqlmodel import Session, select, or_, and_, text from src.db.usergroup_resources import UserGroupResource @@ -21,13 +21,13 @@ from src.db.courses.courses import ( ThumbnailType, ) from src.security.rbac.rbac import ( - authorization_verify_based_on_roles_and_authorship, - authorization_verify_if_element_is_public, authorization_verify_if_user_is_anon, + authorization_verify_based_on_org_admin_status, ) from src.services.courses.thumbnails import upload_thumbnail -from fastapi import HTTPException, Request, UploadFile +from fastapi import HTTPException, Request, UploadFile, status from datetime import datetime +from src.security.courses_security import courses_rbac_check async def get_course( @@ -46,15 +46,15 @@ async def get_course( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "read", db_session) + await courses_rbac_check(request, course.course_uuid, current_user, "read", db_session) # Get course authors with their roles authors_statement = ( select(ResourceAuthor, User) - .join(User, ResourceAuthor.user_id == User.id) + .join(User, ResourceAuthor.user_id == User.id) # type: ignore .where(ResourceAuthor.resource_uuid == course.course_uuid) .order_by( - ResourceAuthor.id.asc() + ResourceAuthor.id.asc() # type: ignore ) ) author_results = db_session.exec(authors_statement).all() @@ -92,15 +92,15 @@ async def get_course_by_id( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "read", db_session) + await courses_rbac_check(request, course.course_uuid, current_user, "read", db_session) # Get course authors with their roles authors_statement = ( select(ResourceAuthor, User) - .join(User, ResourceAuthor.user_id == User.id) + .join(User, ResourceAuthor.user_id == User.id) # type: ignore .where(ResourceAuthor.resource_uuid == course.course_uuid) .order_by( - ResourceAuthor.id.asc() + ResourceAuthor.id.asc() # type: ignore ) ) author_results = db_session.exec(authors_statement).all() @@ -153,7 +153,7 @@ async def get_course_meta( author_results = [(ra, u) for _, ra, u in results if ra is not None and u is not None] # RBAC check - await rbac_check(request, course.course_uuid, current_user, "read", db_session) + await courses_rbac_check(request, course.course_uuid, current_user, "read", db_session) # Get course chapters chapters = [] @@ -241,7 +241,7 @@ async def get_courses_orgslug( .join(User, ResourceAuthor.user_id == User.id) # type: ignore .where(ResourceAuthor.resource_uuid.in_(course_uuids)) # type: ignore .order_by( - ResourceAuthor.id.asc() + ResourceAuthor.id.asc() # type: ignore ) ) @@ -349,10 +349,10 @@ async def search_courses( # Get course authors with their roles authors_statement = ( select(ResourceAuthor, User) - .join(User, ResourceAuthor.user_id == User.id) + .join(User, ResourceAuthor.user_id == User.id) # type: ignore .where(ResourceAuthor.resource_uuid == course.course_uuid) .order_by( - ResourceAuthor.id.asc() + ResourceAuthor.id.asc() # type: ignore ) ) author_results = db_session.exec(authors_statement).all() @@ -399,10 +399,20 @@ async def create_course( thumbnail_file: UploadFile | None = None, thumbnail_type: ThumbnailType = ThumbnailType.IMAGE, ): + """ + Create a new course + + SECURITY NOTES: + - Requires proper permissions to create courses in the organization + - User becomes the CREATOR of the course automatically + - Course creation is subject to organization limits and permissions + """ course = Course.model_validate(course_object) - # RBAC check - await rbac_check(request, "course_x", current_user, "create", db_session) + # SECURITY: Check if user has permission to create courses in this organization + # Since this is a new course, we need to check organization-level permissions + # For now, we'll use the existing RBAC check but with proper organization context + await courses_rbac_check(request, "course_x", current_user, "create", db_session) # Usage check check_limits_with_usage("courses", org_id, db_session) @@ -440,7 +450,7 @@ async def create_course( db_session.commit() db_session.refresh(course) - # Make the user the creator of the course + # SECURITY: Make the user the creator of the course resource_author = ResourceAuthor( resource_uuid=course.course_uuid, user_id=current_user.id, @@ -458,10 +468,10 @@ async def create_course( # Get course authors with their roles authors_statement = ( select(ResourceAuthor, User) - .join(User, ResourceAuthor.user_id == User.id) + .join(User, ResourceAuthor.user_id == User.id) # type: ignore .where(ResourceAuthor.resource_uuid == course.course_uuid) .order_by( - ResourceAuthor.id.asc() + ResourceAuthor.id.asc() # type: ignore ) ) author_results = db_session.exec(authors_statement).all() @@ -506,7 +516,7 @@ async def update_course_thumbnail( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "update", db_session) + await courses_rbac_check(request, course.course_uuid, current_user, "update", db_session) # Get org uuid org_statement = select(Organization).where(Organization.id == course.org_id) @@ -543,10 +553,10 @@ async def update_course_thumbnail( # Get course authors with their roles authors_statement = ( select(ResourceAuthor, User) - .join(User, ResourceAuthor.user_id == User.id) + .join(User, ResourceAuthor.user_id == User.id) # type: ignore .where(ResourceAuthor.resource_uuid == course.course_uuid) .order_by( - ResourceAuthor.id.asc() + ResourceAuthor.id.asc() # type: ignore ) ) author_results = db_session.exec(authors_statement).all() @@ -575,6 +585,14 @@ async def update_course( current_user: PublicUser | AnonymousUser, db_session: Session, ): + """ + Update a course + + SECURITY NOTES: + - Requires course ownership (CREATOR, MAINTAINER) or admin role + - Sensitive fields (public, open_to_contributors) require additional validation + - Cannot change course access settings without proper permissions + """ statement = select(Course).where(Course.course_uuid == course_uuid) course = db_session.exec(statement).first() @@ -584,8 +602,46 @@ async def update_course( detail="Course not found", ) - # RBAC check - await rbac_check(request, course.course_uuid, current_user, "update", db_session) + # SECURITY: Require course ownership or admin role for updating courses + await courses_rbac_check(request, course.course_uuid, current_user, "update", db_session) + + # SECURITY: Additional checks for sensitive access control fields + sensitive_fields_updated = [] + + # Check if sensitive fields are being updated + if course_object.public is not None: + sensitive_fields_updated.append("public") + if course_object.open_to_contributors is not None: + sensitive_fields_updated.append("open_to_contributors") + + # If sensitive fields are being updated, require additional validation + if sensitive_fields_updated: + # SECURITY: For sensitive access control changes, require CREATOR or MAINTAINER role + # Check if user is course owner (CREATOR or MAINTAINER) + statement = select(ResourceAuthor).where( + ResourceAuthor.resource_uuid == course_uuid, + ResourceAuthor.user_id == current_user.id + ) + resource_author = db_session.exec(statement).first() + + is_course_owner = False + if resource_author: + if ((resource_author.authorship == ResourceAuthorshipEnum.CREATOR) or + (resource_author.authorship == ResourceAuthorshipEnum.MAINTAINER)) and \ + resource_author.authorship_status == ResourceAuthorshipStatusEnum.ACTIVE: + is_course_owner = True + + # Check if user has admin or maintainer role + is_admin_or_maintainer = await authorization_verify_based_on_org_admin_status( + request, current_user.id, "update", course_uuid, db_session + ) + + # SECURITY: Only course owners (CREATOR, MAINTAINER) or admins can change access settings + if not (is_course_owner or is_admin_or_maintainer): + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail=f"You must be the course owner (CREATOR or MAINTAINER) or have admin role to change access settings: {', '.join(sensitive_fields_updated)}", + ) # Update only the fields that were passed in for var, value in vars(course_object).items(): @@ -602,10 +658,10 @@ async def update_course( # Get course authors with their roles authors_statement = ( select(ResourceAuthor, User) - .join(User, ResourceAuthor.user_id == User.id) + .join(User, ResourceAuthor.user_id == User.id) # type: ignore .where(ResourceAuthor.resource_uuid == course.course_uuid) .order_by( - ResourceAuthor.id.asc() + ResourceAuthor.id.asc() # type: ignore ) ) author_results = db_session.exec(authors_statement).all() @@ -643,7 +699,7 @@ async def delete_course( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "delete", db_session) + await courses_rbac_check(request, course.course_uuid, current_user, "delete", db_session) # Feature usage decrease_feature_usage("courses", course.org_id, db_session) @@ -681,7 +737,7 @@ async def get_user_courses( return [] # Get courses with the extracted UUIDs - statement = select(Course).where(Course.course_uuid.in_(course_uuids)) + statement = select(Course).where(Course.course_uuid.in_(course_uuids)) # type: ignore # Apply pagination statement = statement.offset((page - 1) * limit).limit(limit) @@ -738,39 +794,177 @@ async def get_user_courses( return result -## 🔒 RBAC Utils ## - - -async def rbac_check( +async def get_course_user_rights( request: Request, course_uuid: str, current_user: PublicUser | AnonymousUser, - action: Literal["create", "read", "update", "delete"], db_session: Session, -): - if action == "read": - if current_user.id == 0: # Anonymous user - res = await authorization_verify_if_element_is_public( - request, course_uuid, action, db_session - ) - return res - else: - res = ( - await authorization_verify_based_on_roles_and_authorship( - request, current_user.id, action, course_uuid, db_session - ) - ) - return res - else: - await authorization_verify_if_user_is_anon(current_user.id) +) -> dict: + """ + Get detailed user rights for a specific course. + + This function returns comprehensive rights information that can be used + by the UI to enable/disable features based on user permissions. + + SECURITY NOTES: + - Returns rights based on course ownership and user roles + - Includes both course-level and content-level permissions + - Safe to expose to UI as it only returns permission information + """ + # Check if course exists + statement = select(Course).where(Course.course_uuid == course_uuid) + course = db_session.exec(statement).first() - await authorization_verify_based_on_roles_and_authorship( - request, - current_user.id, - action, - course_uuid, - db_session, + if not course: + raise HTTPException( + status_code=404, + detail="Course not found", ) + # Initialize rights object + rights = { + "course_uuid": course_uuid, + "user_id": current_user.id, + "is_anonymous": current_user.id == 0, + "permissions": { + "read": False, + "create": False, + "update": False, + "delete": False, + "create_content": False, + "update_content": False, + "delete_content": False, + "manage_contributors": False, + "manage_access": False, + "grade_assignments": False, + "mark_activities_done": False, + "create_certifications": False, + }, + "ownership": { + "is_owner": False, + "is_creator": False, + "is_maintainer": False, + "is_contributor": False, + "authorship_status": None, + }, + "roles": { + "is_admin": False, + "is_maintainer_role": False, + "is_instructor": False, + "is_user": False, + } + } -## 🔒 RBAC Utils ## + # Handle anonymous users + if current_user.id == 0: + # Anonymous users can only read public courses + if course.public: + rights["permissions"]["read"] = True + return rights + + # Check course ownership + statement = select(ResourceAuthor).where( + ResourceAuthor.resource_uuid == course_uuid, + ResourceAuthor.user_id == current_user.id + ) + resource_author = db_session.exec(statement).first() + + if resource_author: + rights["ownership"]["authorship_status"] = resource_author.authorship_status + + if resource_author.authorship_status == ResourceAuthorshipStatusEnum.ACTIVE: + if resource_author.authorship == ResourceAuthorshipEnum.CREATOR: + rights["ownership"]["is_creator"] = True + rights["ownership"]["is_owner"] = True + elif resource_author.authorship == ResourceAuthorshipEnum.MAINTAINER: + rights["ownership"]["is_maintainer"] = True + rights["ownership"]["is_owner"] = True + elif resource_author.authorship == ResourceAuthorshipEnum.CONTRIBUTOR: + rights["ownership"]["is_contributor"] = True + rights["ownership"]["is_owner"] = True + + # Check user roles + from src.security.rbac.rbac import authorization_verify_based_on_org_admin_status + from src.security.rbac.rbac import authorization_verify_based_on_roles + + # Check admin/maintainer role + is_admin_or_maintainer = await authorization_verify_based_on_org_admin_status( + request, current_user.id, "update", course_uuid, db_session + ) + + if is_admin_or_maintainer: + rights["roles"]["is_admin"] = True + rights["roles"]["is_maintainer_role"] = True + + # Check instructor role + has_instructor_permissions = await authorization_verify_based_on_roles( + request, current_user.id, "create", "course_x", db_session + ) + + if has_instructor_permissions: + rights["roles"]["is_instructor"] = True + + # Check user role (basic permissions) + has_user_permissions = await authorization_verify_based_on_roles( + request, current_user.id, "read", course_uuid, db_session + ) + + if has_user_permissions: + rights["roles"]["is_user"] = True + + # Determine permissions based on ownership and roles + is_course_owner = rights["ownership"]["is_owner"] + is_admin = rights["roles"]["is_admin"] + is_maintainer_role = rights["roles"]["is_maintainer_role"] + is_instructor = rights["roles"]["is_instructor"] + + # READ permissions + if course.public or is_course_owner or is_admin or is_maintainer_role or is_instructor or has_user_permissions: + rights["permissions"]["read"] = True + + # CREATE permissions (course creation) + if is_instructor or is_admin or is_maintainer_role: + rights["permissions"]["create"] = True + + # UPDATE permissions (course-level updates) + if is_course_owner or is_admin or is_maintainer_role: + rights["permissions"]["update"] = True + + # DELETE permissions (course deletion) + if is_course_owner or is_admin or is_maintainer_role: + rights["permissions"]["delete"] = True + + # CONTENT CREATION permissions (activities, assignments, chapters, etc.) + if is_course_owner or is_admin or is_maintainer_role: + rights["permissions"]["create_content"] = True + + # CONTENT UPDATE permissions + if is_course_owner or is_admin or is_maintainer_role: + rights["permissions"]["update_content"] = True + + # CONTENT DELETE permissions + if is_course_owner or is_admin or is_maintainer_role: + rights["permissions"]["delete_content"] = True + + # CONTRIBUTOR MANAGEMENT permissions + if is_course_owner or is_admin or is_maintainer_role: + rights["permissions"]["manage_contributors"] = True + + # ACCESS MANAGEMENT permissions (public, open_to_contributors) + if (rights["ownership"]["is_creator"] or rights["ownership"]["is_maintainer"] or + is_admin or is_maintainer_role): + rights["permissions"]["manage_access"] = True + + # GRADING permissions + if is_course_owner or is_admin or is_maintainer_role: + rights["permissions"]["grade_assignments"] = True + + # ACTIVITY MARKING permissions + if is_course_owner or is_admin or is_maintainer_role: + rights["permissions"]["mark_activities_done"] = True + + # CERTIFICATION permissions + if is_course_owner or is_admin or is_maintainer_role: + rights["permissions"]["create_certifications"] = True + + return rights diff --git a/apps/api/src/services/courses/updates.py b/apps/api/src/services/courses/updates.py index f3fea858..19a2324f 100644 --- a/apps/api/src/services/courses/updates.py +++ b/apps/api/src/services/courses/updates.py @@ -12,7 +12,7 @@ from src.db.courses.course_updates import ( from src.db.courses.courses import Course from src.db.organizations import Organization from src.db.users import AnonymousUser, PublicUser -from src.services.courses.courses import rbac_check +from src.security.courses_security import courses_rbac_check async def create_update( @@ -41,7 +41,7 @@ async def create_update( ) # RBAC check - await rbac_check(request, course.course_uuid, current_user, "update", db_session) + await courses_rbac_check(request, course.course_uuid, current_user, "update", db_session) # Generate UUID courseupdate_uuid = str(f"courseupdate_{uuid4()}") @@ -81,7 +81,7 @@ async def update_update( ) # RBAC check - await rbac_check( + await courses_rbac_check( request, update.courseupdate_uuid, current_user, "update", db_session ) @@ -115,7 +115,7 @@ async def delete_update( ) # RBAC check - await rbac_check( + await courses_rbac_check( request, update.courseupdate_uuid, current_user, "delete", db_session ) diff --git a/apps/api/src/services/install/install.py b/apps/api/src/services/install/install.py index aea4551e..559967f3 100644 --- a/apps/api/src/services/install/install.py +++ b/apps/api/src/services/install/install.py @@ -24,7 +24,7 @@ from src.db.organization_config import ( UserGroupOrgConfig, ) from src.db.organizations import Organization, OrganizationCreate -from src.db.roles import Permission, Rights, Role, RoleTypeEnum +from src.db.roles import DashboardPermission, Permission, PermissionsWithOwn, Rights, Role, RoleTypeEnum from src.db.user_organizations import UserOrganization from src.db.users import User, UserCreate, UserRead from config.config import get_learnhouse_config @@ -127,7 +127,7 @@ def install_default_elements(db_session: Session): statement = select(Role).where(Role.role_type == RoleTypeEnum.TYPE_GLOBAL) roles = db_session.exec(statement).all() - if roles and len(roles) == 3: + if roles and len(roles) == 4: raise HTTPException( status_code=409, detail="Default roles already exist", @@ -136,16 +136,19 @@ def install_default_elements(db_session: Session): # Create default roles role_global_admin = Role( name="Admin", - description="Standard Admin Role", + description="Full platform control", id=1, role_type=RoleTypeEnum.TYPE_GLOBAL, role_uuid="role_global_admin", rights=Rights( - courses=Permission( + courses=PermissionsWithOwn( action_create=True, action_read=True, + action_read_own=True, action_update=True, + action_update_own=True, action_delete=True, + action_delete_own=True, ), users=Permission( action_create=True, @@ -183,6 +186,15 @@ def install_default_elements(db_session: Session): action_update=True, action_delete=True, ), + roles=Permission( + action_create=True, + action_read=True, + action_update=True, + action_delete=True, + ), + dashboard=DashboardPermission( + action_access=True, + ), ), creation_date=str(datetime.now()), update_date=str(datetime.now()), @@ -190,22 +202,25 @@ def install_default_elements(db_session: Session): role_global_maintainer = Role( name="Maintainer", - description="Standard Maintainer Role", + description="Mid-level manager, wide permissions but no platform control", id=2, role_type=RoleTypeEnum.TYPE_GLOBAL, role_uuid="role_global_maintainer", rights=Rights( - courses=Permission( + courses=PermissionsWithOwn( action_create=True, action_read=True, + action_read_own=True, action_update=True, + action_update_own=True, action_delete=True, + action_delete_own=True, ), users=Permission( action_create=True, action_read=True, action_update=True, - action_delete=True, + action_delete=False, ), usergroups=Permission( action_create=True, @@ -220,10 +235,10 @@ def install_default_elements(db_session: Session): action_delete=True, ), organizations=Permission( - action_create=True, + action_create=False, action_read=True, - action_update=True, - action_delete=True, + action_update=False, + action_delete=False, ), coursechapters=Permission( action_create=True, @@ -237,6 +252,81 @@ def install_default_elements(db_session: Session): action_update=True, action_delete=True, ), + roles=Permission( + action_create=False, + action_read=True, + action_update=False, + action_delete=False, + ), + dashboard=DashboardPermission( + action_access=True, + ), + ), + creation_date=str(datetime.now()), + update_date=str(datetime.now()), + ) + + role_global_instructor = Role( + name="Instructor", + description="Can manage their own content", + id=3, + role_type=RoleTypeEnum.TYPE_GLOBAL, + role_uuid="role_global_instructor", + rights=Rights( + courses=PermissionsWithOwn( + action_create=True, + action_read=True, + action_read_own=True, + action_update=False, + action_update_own=True, + action_delete=False, + action_delete_own=True, + ), + users=Permission( + action_create=False, + action_read=False, + action_update=False, + action_delete=False, + ), + usergroups=Permission( + action_create=False, + action_read=True, + action_update=False, + action_delete=False, + ), + collections=Permission( + action_create=True, + action_read=True, + action_update=False, + action_delete=False, + ), + organizations=Permission( + action_create=False, + action_read=False, + action_update=False, + action_delete=False, + ), + coursechapters=Permission( + action_create=True, + action_read=True, + action_update=False, + action_delete=False, + ), + activities=Permission( + action_create=True, + action_read=True, + action_update=False, + action_delete=False, + ), + roles=Permission( + action_create=False, + action_read=False, + action_update=False, + action_delete=False, + ), + dashboard=DashboardPermission( + action_access=True, + ), ), creation_date=str(datetime.now()), update_date=str(datetime.now()), @@ -244,20 +334,23 @@ def install_default_elements(db_session: Session): role_global_user = Role( name="User", - description="Standard User Role", + description="Read-Only Learner", role_type=RoleTypeEnum.TYPE_GLOBAL, role_uuid="role_global_user", - id=3, + id=4, rights=Rights( - courses=Permission( + courses=PermissionsWithOwn( action_create=False, action_read=True, + action_read_own=True, action_update=False, - action_delete=False, + action_update_own=False, + action_delete=True, + action_delete_own=True, ), users=Permission( - action_create=True, - action_read=True, + action_create=False, + action_read=False, action_update=False, action_delete=False, ), @@ -275,7 +368,7 @@ def install_default_elements(db_session: Session): ), organizations=Permission( action_create=False, - action_read=True, + action_read=False, action_update=False, action_delete=False, ), @@ -288,9 +381,18 @@ def install_default_elements(db_session: Session): activities=Permission( action_create=False, action_read=True, + action_update=False, + action_delete=False, + ), + roles=Permission( + action_create=False, + action_read=False, action_update=False, action_delete=False, ), + dashboard=DashboardPermission( + action_access=False, + ), ), creation_date=str(datetime.now()), update_date=str(datetime.now()), @@ -299,11 +401,13 @@ def install_default_elements(db_session: Session): # Serialize rights to JSON role_global_admin.rights = role_global_admin.rights.dict() # type: ignore role_global_maintainer.rights = role_global_maintainer.rights.dict() # type: ignore + role_global_instructor.rights = role_global_instructor.rights.dict() # type: ignore role_global_user.rights = role_global_user.rights.dict() # type: ignore # Insert roles in DB db_session.add(role_global_admin) db_session.add(role_global_maintainer) + db_session.add(role_global_instructor) db_session.add(role_global_user) # commit changes diff --git a/apps/api/src/services/orgs/join.py b/apps/api/src/services/orgs/join.py index a62f8fad..2b8b15ac 100644 --- a/apps/api/src/services/orgs/join.py +++ b/apps/api/src/services/orgs/join.py @@ -80,7 +80,7 @@ async def join_org( user_organization = UserOrganization( user_id=user.id, org_id=org.id, - role_id=3, + role_id=4, creation_date=str(datetime.now()), update_date=str(datetime.now()), ) @@ -102,7 +102,7 @@ async def join_org( user_organization = UserOrganization( user_id=user.id, org_id=org.id, - role_id=3, + role_id=4, creation_date=str(datetime.now()), update_date=str(datetime.now()), ) diff --git a/apps/api/src/services/payments/payments_courses.py b/apps/api/src/services/payments/payments_courses.py index 1382e408..45bf73c2 100644 --- a/apps/api/src/services/payments/payments_courses.py +++ b/apps/api/src/services/payments/payments_courses.py @@ -4,7 +4,7 @@ from src.db.payments.payments_courses import PaymentsCourse from src.db.payments.payments_products import PaymentsProduct from src.db.courses.courses import Course from src.db.users import PublicUser, AnonymousUser -from src.services.courses.courses import rbac_check +from src.security.courses_security import courses_rbac_check async def link_course_to_product( request: Request, @@ -22,7 +22,7 @@ async def link_course_to_product( raise HTTPException(status_code=404, detail="Course not found") # RBAC check - await rbac_check(request, course.course_uuid, current_user, "update", db_session) + await courses_rbac_check(request, course.course_uuid, current_user, "update", db_session) # Check if product exists statement = select(PaymentsProduct).where( @@ -71,7 +71,7 @@ async def unlink_course_from_product( raise HTTPException(status_code=404, detail="Course not found") # RBAC check - await rbac_check(request, course.course_uuid, current_user, "update", db_session) + await courses_rbac_check(request, course.course_uuid, current_user, "update", db_session) # Find and delete the payment course link statement = select(PaymentsCourse).where( diff --git a/apps/api/src/services/roles/roles.py b/apps/api/src/services/roles/roles.py index ea5d0715..7e5a4b97 100644 --- a/apps/api/src/services/roles/roles.py +++ b/apps/api/src/services/roles/roles.py @@ -1,12 +1,15 @@ -from typing import Literal +from typing import Literal, List from uuid import uuid4 -from sqlmodel import Session, select +from sqlmodel import Session, select, text +from sqlalchemy.exc import IntegrityError from src.security.rbac.rbac import ( authorization_verify_based_on_roles_and_authorship, authorization_verify_if_user_is_anon, ) from src.db.users import AnonymousUser, PublicUser -from src.db.roles import Role, RoleCreate, RoleRead, RoleUpdate +from src.db.roles import Role, RoleCreate, RoleRead, RoleUpdate, RoleTypeEnum +from src.db.organizations import Organization +from src.db.user_organizations import UserOrganization from fastapi import HTTPException, Request from datetime import datetime @@ -22,24 +25,401 @@ async def create_role( # RBAC check await rbac_check(request, current_user, "create", "role_xxx", db_session) + # ============================================================================ + # VERIFICATION 1: Ensure the role is created as TYPE_ORGANIZATION and has an org_id + # ============================================================================ + if not role.org_id: + raise HTTPException( + status_code=400, + detail="Organization ID is required for role creation", + ) + + # Force the role type to be TYPE_ORGANIZATION for user-created roles + role.role_type = RoleTypeEnum.TYPE_ORGANIZATION + + # ============================================================================ + # VERIFICATION 2: Check if the organization exists + # ============================================================================ + statement = select(Organization).where(Organization.id == role.org_id) + organization = db_session.exec(statement).first() + + if not organization: + raise HTTPException( + status_code=404, + detail="Organization not found", + ) + + # ============================================================================ + # VERIFICATION 3: Check if the current user is a member of the organization + # ============================================================================ + statement = select(UserOrganization).where( + UserOrganization.user_id == current_user.id, + UserOrganization.org_id == role.org_id + ) + user_org = db_session.exec(statement).first() + + if not user_org: + raise HTTPException( + status_code=403, + detail="You are not a member of this organization", + ) + + # ============================================================================ + # VERIFICATION 4: Check if the user has permission to create roles in this organization + # ============================================================================ + # Get the user's role in this organization + statement = select(Role).where(Role.id == user_org.role_id) + user_role = db_session.exec(statement).first() + + if not user_role: + raise HTTPException( + status_code=403, + detail="Your role in this organization could not be determined", + ) + + # Check if the user has role creation permissions + if user_role.rights and isinstance(user_role.rights, dict): + roles_rights = user_role.rights.get('roles', {}) + if not roles_rights.get('action_create', False): + raise HTTPException( + status_code=403, + detail="You don't have permission to create roles in this organization", + ) + else: + # If no rights are defined, check if user has admin role (role_id 1 or 2) + if user_role.id not in [1, 2]: # Admin and Maintainer roles + raise HTTPException( + status_code=403, + detail="You don't have permission to create roles in this organization. Admin or Maintainer role required.", + ) + + # ============================================================================ + # VERIFICATION 5: Check if a role with the same name already exists in this organization + # ============================================================================ + statement = select(Role).where( + Role.name == role.name, + Role.org_id == role.org_id, + Role.role_type == RoleTypeEnum.TYPE_ORGANIZATION + ) + existing_role = db_session.exec(statement).first() + + if existing_role: + raise HTTPException( + status_code=409, + detail=f"A role with the name '{role.name}' already exists in this organization", + ) + + # ============================================================================ + # VERIFICATION 6: Validate role name and description + # ============================================================================ + if not role.name or role.name.strip() == "": + raise HTTPException( + status_code=400, + detail="Role name is required and cannot be empty", + ) + + if len(role.name.strip()) > 100: # Assuming a reasonable limit + raise HTTPException( + status_code=400, + detail="Role name cannot exceed 100 characters", + ) + + # ============================================================================ + # VERIFICATION 7: Validate rights structure if provided + # ============================================================================ + if role.rights: + # Convert Rights model to dict if needed + if isinstance(role.rights, dict): + # It's already a dict + rights_dict = role.rights + else: + # It's likely a Pydantic model, try to convert to dict + try: + # Try dict() method first (for Pydantic v1) + rights_dict = role.rights.dict() + except AttributeError: + try: + # Try model_dump() method (for Pydantic v2) + rights_dict = role.rights.model_dump() # type: ignore + except AttributeError: + raise HTTPException( + status_code=400, + detail="Rights must be provided as a JSON object", + ) + + # Validate rights structure - check for required top-level keys + required_rights = [ + 'courses', 'users', 'usergroups', 'collections', + 'organizations', 'coursechapters', 'activities', + 'roles', 'dashboard' + ] + + for required_right in required_rights: + if required_right not in rights_dict: + raise HTTPException( + status_code=400, + detail=f"Missing required right: {required_right}", + ) + + # Validate the structure of each right + right_data = rights_dict[required_right] + if not isinstance(right_data, dict): + raise HTTPException( + status_code=400, + detail=f"Right '{required_right}' must be a JSON object", + ) + + # Validate courses permissions (has additional 'own' permissions) + if required_right == 'courses': + required_course_permissions = [ + 'action_create', 'action_read', 'action_read_own', + 'action_update', 'action_update_own', 'action_delete', 'action_delete_own' + ] + for perm in required_course_permissions: + if perm not in right_data: + raise HTTPException( + status_code=400, + detail=f"Missing required course permission: {perm}", + ) + if not isinstance(right_data[perm], bool): + raise HTTPException( + status_code=400, + detail=f"Course permission '{perm}' must be a boolean", + ) + + # Validate other permissions (standard permissions) + elif required_right in ['users', 'usergroups', 'collections', 'organizations', 'coursechapters', 'activities', 'roles']: + required_permissions = ['action_create', 'action_read', 'action_update', 'action_delete'] + for perm in required_permissions: + if perm not in right_data: + raise HTTPException( + status_code=400, + detail=f"Missing required permission '{perm}' for '{required_right}'", + ) + if not isinstance(right_data[perm], bool): + raise HTTPException( + status_code=400, + detail=f"Permission '{perm}' for '{required_right}' must be a boolean", + ) + + # Validate dashboard permissions + elif required_right == 'dashboard': + if 'action_access' not in right_data: + raise HTTPException( + status_code=400, + detail="Missing required dashboard permission: action_access", + ) + if not isinstance(right_data['action_access'], bool): + raise HTTPException( + status_code=400, + detail="Dashboard permission 'action_access' must be a boolean", + ) + + # Convert back to dict if it was a model + if not isinstance(role.rights, dict): + role.rights = rights_dict + + # ============================================================================ + # VERIFICATION 8: Ensure user cannot create a role with higher permissions than they have + # ============================================================================ + if role.rights and isinstance(role.rights, dict) and user_role.rights and isinstance(user_role.rights, dict): + # Check if the new role has any permissions that the user doesn't have + for right_key, right_permissions in role.rights.items(): + if right_key in user_role.rights: + user_right_permissions = user_role.rights[right_key] + + # Check each permission in the right + for perm_key, perm_value in right_permissions.items(): + if isinstance(perm_value, bool) and perm_value: # If the new role has this permission enabled + if isinstance(user_right_permissions, dict) and perm_key in user_right_permissions: + user_has_perm = user_right_permissions[perm_key] + if not user_has_perm: + raise HTTPException( + status_code=403, + detail=f"You cannot create a role with '{perm_key}' permission for '{right_key}' as you don't have this permission yourself", + ) + else: + raise HTTPException( + status_code=403, + detail=f"You cannot create a role with '{perm_key}' permission for '{right_key}' as you don't have this permission yourself", + ) + # Complete the role object role.role_uuid = f"role_{uuid4()}" role.creation_date = str(datetime.now()) role.update_date = str(datetime.now()) - db_session.add(role) - db_session.commit() - db_session.refresh(role) + # ============================================================================ + # VERIFICATION 9: Handle ID sequence issue (existing logic) + # ============================================================================ + try: + db_session.add(role) + db_session.commit() + db_session.refresh(role) + except IntegrityError as e: + if "duplicate key value violates unique constraint" in str(e) and "role_pkey" in str(e): + # Handle the sequence issue by finding the next available ID + db_session.rollback() + + # Get the maximum ID from the role table using raw SQL + result = db_session.execute(text("SELECT COALESCE(MAX(id), 0) as max_id FROM role")) + max_id_result = result.scalar() + max_id = max_id_result if max_id_result is not None else 0 + + # Set the next available ID + role.id = max_id + 1 + + # Try to insert again + db_session.add(role) + db_session.commit() + db_session.refresh(role) + + # Update the sequence to the correct value for future inserts + try: + # Use raw SQL to update the sequence + db_session.execute(text(f"SELECT setval('role_id_seq', {max_id + 1}, true)")) + db_session.commit() + except Exception: + # If sequence doesn't exist or can't be updated, that's okay + # The manual ID assignment above will handle it + pass + else: + # Re-raise the original exception if it's not the sequence issue + raise e - role = RoleRead(**role.model_dump()) + # Create RoleRead object with all required fields + role_data = role.model_dump() + # Ensure org_id is properly handled + if role_data.get('org_id') is None: + role_data['org_id'] = 0 + role = RoleRead(**role_data) return role +async def get_roles_by_organization( + request: Request, + db_session: Session, + org_id: int, + current_user: PublicUser, +) -> List[RoleRead]: + """ + Get all roles for a specific organization, including global roles. + + Args: + request: FastAPI request object + db_session: Database session + org_id: Organization ID + current_user: Current authenticated user + + Returns: + List[RoleRead]: List of roles for the organization (including global roles) + + Raises: + HTTPException: If organization not found or user lacks permissions + """ + # ============================================================================ + # VERIFICATION 1: Check if the organization exists + # ============================================================================ + statement = select(Organization).where(Organization.id == org_id) + organization = db_session.exec(statement).first() + + if not organization: + raise HTTPException( + status_code=404, + detail="Organization not found", + ) + + # ============================================================================ + # VERIFICATION 2: Check if the current user is a member of the organization + # ============================================================================ + statement = select(UserOrganization).where( + UserOrganization.user_id == current_user.id, + UserOrganization.org_id == org_id + ) + user_org = db_session.exec(statement).first() + + if not user_org: + raise HTTPException( + status_code=403, + detail="You are not a member of this organization", + ) + + # ============================================================================ + # VERIFICATION 3: Check if the user has permission to read roles in this organization + # ============================================================================ + # Get the user's role in this organization + statement = select(Role).where(Role.id == user_org.role_id) + user_role = db_session.exec(statement).first() + + if not user_role: + raise HTTPException( + status_code=403, + detail="Your role in this organization could not be determined", + ) + + # Check if the user has role reading permissions + if user_role.rights and isinstance(user_role.rights, dict): + roles_rights = user_role.rights.get('roles', {}) + if not roles_rights.get('action_read', False): + raise HTTPException( + status_code=403, + detail="You don't have permission to read roles in this organization", + ) + else: + # If no rights are defined, check if user has admin role (role_id 1 or 2) + if user_role.id not in [1, 2]: # Admin and Maintainer roles + raise HTTPException( + status_code=403, + detail="You don't have permission to read roles in this organization. Admin or Maintainer role required.", + ) + + # ============================================================================ + # GET ROLES: Fetch all roles for the organization AND global roles + # ============================================================================ + # Get global roles first + global_roles_statement = select(Role).where( + Role.role_type == RoleTypeEnum.TYPE_GLOBAL + ).order_by(Role.id) # type: ignore + + global_roles = list(db_session.exec(global_roles_statement).all()) + + # Get organization-specific roles + org_roles_statement = select(Role).where( + Role.org_id == org_id, + Role.role_type == RoleTypeEnum.TYPE_ORGANIZATION + ).order_by(Role.id) # type: ignore + + org_roles = list(db_session.exec(org_roles_statement).all()) + + # Combine lists with global roles first, then organization roles + all_roles = global_roles + org_roles + + # Convert to RoleRead objects + role_reads = [] + for role in all_roles: + role_data = role.model_dump() + # Ensure org_id is properly handled + if role_data.get('org_id') is None: + role_data['org_id'] = 0 + role_reads.append(RoleRead(**role_data)) + + return role_reads + + async def read_role( request: Request, db_session: Session, role_id: str, current_user: PublicUser ): - statement = select(Role).where(Role.id == role_id) + # Convert role_id to integer + try: + role_id_int = int(role_id) + except ValueError: + raise HTTPException( + status_code=400, + detail="Invalid role ID format. Role ID must be a number.", + ) + + statement = select(Role).where(Role.id == role_id_int) result = db_session.exec(statement) role = result.first() @@ -75,6 +455,15 @@ async def update_role( detail="Role not found", ) + # ============================================================================ + # VERIFICATION: Prevent updating TYPE_GLOBAL roles + # ============================================================================ + if role.role_type == RoleTypeEnum.TYPE_GLOBAL: + raise HTTPException( + status_code=403, + detail="Global roles cannot be updated. These are system-defined roles that must remain unchanged.", + ) + # RBAC check await rbac_check(request, current_user, "update", role.role_uuid, db_session) @@ -85,9 +474,116 @@ async def update_role( del role_object.role_id # Update only the fields that were passed in - for var, value in vars(role_object).items(): + # Use model_dump() to get the data as a dictionary + try: + update_data = role_object.model_dump(exclude_unset=True) + except AttributeError: + # Fallback to dict() method for older Pydantic versions + try: + update_data = role_object.dict(exclude_unset=True) + except AttributeError: + # Fallback to vars() for SQLModel + update_data = {k: v for k, v in vars(role_object).items() if v is not None} + + # Update the role with the new data + for key, value in update_data.items(): if value is not None: - setattr(role, var, value) + setattr(role, key, value) + + # ============================================================================ + # VALIDATE RIGHTS STRUCTURE if rights are being updated + # ============================================================================ + if role.rights: + # Convert Rights model to dict if needed + if isinstance(role.rights, dict): + # It's already a dict + rights_dict = role.rights + else: + # It's likely a Pydantic model, try to convert to dict + try: + # Try dict() method first (for Pydantic v1) + rights_dict = role.rights.dict() + except AttributeError: + try: + # Try model_dump() method (for Pydantic v2) + rights_dict = role.rights.model_dump() # type: ignore + except AttributeError: + raise HTTPException( + status_code=400, + detail="Rights must be provided as a JSON object", + ) + + # Validate rights structure - check for required top-level keys + required_rights = [ + 'courses', 'users', 'usergroups', 'collections', + 'organizations', 'coursechapters', 'activities', + 'roles', 'dashboard' + ] + + for required_right in required_rights: + if required_right not in rights_dict: + raise HTTPException( + status_code=400, + detail=f"Missing required right: {required_right}", + ) + + # Validate the structure of each right + right_data = rights_dict[required_right] + if not isinstance(right_data, dict): + raise HTTPException( + status_code=400, + detail=f"Right '{required_right}' must be a JSON object", + ) + + # Validate courses permissions (has additional 'own' permissions) + if required_right == 'courses': + required_course_permissions = [ + 'action_create', 'action_read', 'action_read_own', + 'action_update', 'action_update_own', 'action_delete', 'action_delete_own' + ] + for perm in required_course_permissions: + if perm not in right_data: + raise HTTPException( + status_code=400, + detail=f"Missing required course permission: {perm}", + ) + if not isinstance(right_data[perm], bool): + raise HTTPException( + status_code=400, + detail=f"Course permission '{perm}' must be a boolean", + ) + + # Validate other permissions (standard permissions) + elif required_right in ['users', 'usergroups', 'collections', 'organizations', 'coursechapters', 'activities', 'roles']: + required_permissions = ['action_create', 'action_read', 'action_update', 'action_delete'] + for perm in required_permissions: + if perm not in right_data: + raise HTTPException( + status_code=400, + detail=f"Missing required permission '{perm}' for '{required_right}'", + ) + if not isinstance(right_data[perm], bool): + raise HTTPException( + status_code=400, + detail=f"Permission '{perm}' for '{required_right}' must be a boolean", + ) + + # Validate dashboard permissions + elif required_right == 'dashboard': + if 'action_access' not in right_data: + raise HTTPException( + status_code=400, + detail="Missing required dashboard permission: action_access", + ) + if not isinstance(right_data['action_access'], bool): + raise HTTPException( + status_code=400, + detail="Dashboard permission 'action_access' must be a boolean", + ) + + # Convert back to dict if it was a model + if not isinstance(role.rights, dict): + role.rights = rights_dict db_session.add(role) db_session.commit() @@ -101,10 +597,17 @@ async def update_role( async def delete_role( request: Request, db_session: Session, role_id: str, current_user: PublicUser ): - # RBAC check - await rbac_check(request, current_user, "delete", role_id, db_session) - - statement = select(Role).where(Role.id == role_id) + # Convert role_id to integer + try: + role_id_int = int(role_id) + except ValueError: + raise HTTPException( + status_code=400, + detail="Invalid role ID format. Role ID must be a number.", + ) + + # First, get the role to check if it exists and get its UUID + statement = select(Role).where(Role.id == role_id_int) result = db_session.exec(statement) role = result.first() @@ -115,6 +618,18 @@ async def delete_role( detail="Role not found", ) + # ============================================================================ + # VERIFICATION: Prevent deleting TYPE_GLOBAL roles + # ============================================================================ + if role.role_type == RoleTypeEnum.TYPE_GLOBAL: + raise HTTPException( + status_code=403, + detail="Global roles cannot be deleted. These are system-defined roles that must remain unchanged.", + ) + + # RBAC check using the role's UUID + await rbac_check(request, current_user, "delete", role.role_uuid, db_session) + db_session.delete(role) db_session.commit() diff --git a/apps/api/src/services/users/users.py b/apps/api/src/services/users/users.py index 18669ef6..913daf47 100644 --- a/apps/api/src/services/users/users.py +++ b/apps/api/src/services/users/users.py @@ -103,7 +103,7 @@ async def create_user( user_organization = UserOrganization( user_id=user.id if user.id else 0, org_id=int(org_id), - role_id=3, + role_id=4, creation_date=str(datetime.now()), update_date=str(datetime.now()), ) diff --git a/apps/api/src/tests/security/test_rbac.py b/apps/api/src/tests/security/test_rbac.py index 450b510c..dcee3914 100644 --- a/apps/api/src/tests/security/test_rbac.py +++ b/apps/api/src/tests/security/test_rbac.py @@ -57,57 +57,69 @@ class TestRBAC: @pytest.fixture def mock_role(self): """Create a mock role object""" - from src.db.roles import RoleTypeEnum + from src.db.roles import RoleTypeEnum, Rights, PermissionsWithOwn, Permission, DashboardPermission role = Mock(spec=Role) role.id = 1 role.org_id = 1 role.name = "Test Role" role.description = "A test role." - # Rights should be a dictionary for validation - role.rights = { - "courses": { - "action_create": False, - "action_read": True, - "action_update": False, - "action_delete": False, - }, - "users": { - "action_create": False, - "action_read": True, - "action_update": False, - "action_delete": False, - }, - "usergroups": { - "action_create": False, - "action_read": True, - "action_update": False, - "action_delete": False, - }, - "collections": { - "action_create": False, - "action_read": True, - "action_update": False, - "action_delete": False, - }, - "organizations": { - "action_create": False, - "action_read": True, - "action_update": False, - "action_delete": False, - }, - "coursechapters": { - "action_create": False, - "action_read": True, - "action_update": False, - "action_delete": False, - }, - "activities": { - "action_create": False, - "action_read": True, - "action_update": False, - "action_delete": False, - } - } + # Rights should be a Rights object with proper Permission objects + role.rights = Rights( + courses=PermissionsWithOwn( + action_create=False, + action_read=True, + action_read_own=False, + action_update=False, + action_update_own=False, + action_delete=False, + action_delete_own=False, + ), + users=Permission( + action_create=False, + action_read=True, + action_update=False, + action_delete=False, + ), + usergroups=Permission( + action_create=False, + action_read=True, + action_update=False, + action_delete=False, + ), + collections=Permission( + action_create=False, + action_read=True, + action_update=False, + action_delete=False, + ), + organizations=Permission( + action_create=False, + action_read=True, + action_update=False, + action_delete=False, + ), + coursechapters=Permission( + action_create=False, + action_read=True, + action_update=False, + action_delete=False, + ), + activities=Permission( + action_create=False, + action_read=True, + action_update=False, + action_delete=False, + ), + roles=Permission( + action_create=False, + action_read=True, + action_update=False, + action_delete=False, + ), + dashboard=DashboardPermission( + action_access=True, + ) + ) role.role_type = RoleTypeEnum.TYPE_GLOBAL role.role_uuid = "role_test" role.creation_date = "2024-01-01T00:00:00" @@ -277,7 +289,7 @@ class TestRBAC: mock_check_type.return_value = "courses" # Mock role without permission - mock_role.rights["courses"]["action_read"] = False + mock_role.rights.courses.action_read = False # Mock database query mock_db_session.exec.return_value.all.return_value = [mock_role] diff --git a/apps/web/app/auth/options.ts b/apps/web/app/auth/options.ts index 90a6df54..50ee9f05 100644 --- a/apps/web/app/auth/options.ts +++ b/apps/web/app/auth/options.ts @@ -91,12 +91,12 @@ export const nextAuthOptions = { token.user = userFromOAuth.data; } - // Refresh token only if it's close to expiring (5 minutes before expiry) + // Refresh token only if it's close to expiring (1 minute before expiry) if (token?.user?.tokens) { const tokenExpiry = token.user.tokens.expiry || 0; - const fiveMinutes = 5 * 60 * 1000; + const oneMinute = 1 * 60 * 1000; - if (Date.now() + fiveMinutes >= tokenExpiry) { + if (Date.now() + oneMinute >= tokenExpiry) { const RefreshedToken = await getNewAccessTokenUsingRefreshTokenServer( token?.user?.tokens?.refresh_token ); @@ -118,11 +118,11 @@ export const nextAuthOptions = { async session({ session, token }: any) { // Include user information in the session if (token.user) { - // Cache the session for 5 minutes to avoid frequent API calls + // Cache the session for 1 minute to refresh every minute const cacheKey = `user_session_${token.user.tokens.access_token}`; let cachedSession = global.sessionCache?.[cacheKey]; - if (cachedSession && Date.now() - cachedSession.timestamp < 5 * 60 * 1000) { + if (cachedSession && Date.now() - cachedSession.timestamp < 1 * 60 * 1000) { return cachedSession.data; } diff --git a/apps/web/app/layout.tsx b/apps/web/app/layout.tsx index 1b770546..6bb780de 100644 --- a/apps/web/app/layout.tsx +++ b/apps/web/app/layout.tsx @@ -22,7 +22,7 @@ export default function RootLayout({ {isDevEnv ? '' :