From b475d27bb7df5a3af4cd33450fbec2a7563d70ca Mon Sep 17 00:00:00 2001 From: nishika26 Date: Thu, 26 Jun 2025 22:12:29 +0530 Subject: [PATCH 01/13] access control --- backend/app/api/deps.py | 18 +++++ backend/app/api/routes/organization.py | 105 +++++++++++-------------- 2 files changed, 65 insertions(+), 58 deletions(-) diff --git a/backend/app/api/deps.py b/backend/app/api/deps.py index 43810c38..e8cd327b 100644 --- a/backend/app/api/deps.py +++ b/backend/app/api/deps.py @@ -147,6 +147,24 @@ def get_current_active_superuser_org(current_user: CurrentUserOrg) -> User: return current_user +def has_org_access(_current_user: CurrentUserOrg, org_id: int): + if _current_user.is_superuser: + return + if _current_user.organization_id != org_id: + raise HTTPException( + status_code=403, detail="Access to this organization is forbidden" + ) + + +def check_org_creation_allowed(_current_user: CurrentUserOrg): + if _current_user.is_superuser: + return + if _current_user.organization_id is not None: + raise HTTPException( + status_code=403, detail="Not allowed to create another organization" + ) + + def verify_user_project_organization( db: SessionDep, current_user: CurrentUserOrg, diff --git a/backend/app/api/routes/organization.py b/backend/app/api/routes/organization.py index 099494ee..0b687a39 100644 --- a/backend/app/api/routes/organization.py +++ b/backend/app/api/routes/organization.py @@ -1,8 +1,7 @@ -from typing import Any, List +from typing import List -from fastapi import APIRouter, Depends, HTTPException -from sqlalchemy import func -from sqlmodel import Session, select +from fastapi import APIRouter +from sqlmodel import select from app.models import ( Organization, @@ -11,70 +10,65 @@ OrganizationPublic, ) from app.api.deps import ( - CurrentUser, + CurrentUserOrg, SessionDep, - get_current_active_superuser, + has_org_access, + check_org_creation_allowed, ) -from app.crud.organization import create_organization, get_organization_by_id +from app.crud.organization import create_organization, validate_organization from app.utils import APIResponse + router = APIRouter(prefix="/organizations", tags=["organizations"]) # Retrieve organizations -@router.get( - "/", - dependencies=[Depends(get_current_active_superuser)], - response_model=APIResponse[List[OrganizationPublic]], -) -def read_organizations(session: SessionDep, skip: int = 0, limit: int = 100): - count_statement = select(func.count()).select_from(Organization) - count = session.exec(count_statement).one() - - statement = select(Organization).offset(skip).limit(limit) +@router.get("/", response_model=APIResponse[List[OrganizationPublic]]) +def read_organizations( + session: SessionDep, _current_user: CurrentUserOrg, skip: int = 0, limit: int = 100 +): + if _current_user.is_superuser: + statement = select(Organization).offset(skip).limit(limit) + else: + statement = ( + select(Organization) + .where(Organization.id == _current_user.organization_id) + .offset(skip) + .limit(limit) + ) organizations = session.exec(statement).all() - return APIResponse.success_response(organizations) -# Create a new organization -@router.post( - "/", - dependencies=[Depends(get_current_active_superuser)], - response_model=APIResponse[OrganizationPublic], -) -def create_new_organization(*, session: SessionDep, org_in: OrganizationCreate): +@router.post("/", response_model=APIResponse[OrganizationPublic]) +def create_new_organization( + *, session: SessionDep, _current_user: CurrentUserOrg, org_in: OrganizationCreate +): + check_org_creation_allowed(_current_user) new_org = create_organization(session=session, org_create=org_in) return APIResponse.success_response(new_org) -@router.get( - "/{org_id}", - dependencies=[Depends(get_current_active_superuser)], - response_model=APIResponse[OrganizationPublic], -) -def read_organization(*, session: SessionDep, org_id: int): - """ - Retrieve an organization by ID. - """ - org = get_organization_by_id(session=session, org_id=org_id) - if org is None: - raise HTTPException(status_code=404, detail="Organization not found") +@router.get("/{org_id}", response_model=APIResponse[OrganizationPublic]) +def read_organization( + *, session: SessionDep, _current_user: CurrentUserOrg, org_id: int +): + org = validate_organization(session=session, org_id=org_id) + has_org_access(_current_user, org_id) + return APIResponse.success_response(org) -# Update an organization -@router.patch( - "/{org_id}", - dependencies=[Depends(get_current_active_superuser)], - response_model=APIResponse[OrganizationPublic], -) +@router.patch("/{org_id}", response_model=APIResponse[OrganizationPublic]) def update_organization( - *, session: SessionDep, org_id: int, org_in: OrganizationUpdate + *, + session: SessionDep, + _current_user: CurrentUserOrg, + org_id: int, + org_in: OrganizationUpdate, ): - org = get_organization_by_id(session=session, org_id=org_id) - if org is None: - raise HTTPException(status_code=404, detail="Organization not found") + org = validate_organization(session=session, org_id=org_id) + has_org_access(_current_user, org_id) org_data = org_in.model_dump(exclude_unset=True) org = org.model_copy(update=org_data) @@ -86,17 +80,12 @@ def update_organization( return APIResponse.success_response(org) -# Delete an organization -@router.delete( - "/{org_id}", - dependencies=[Depends(get_current_active_superuser)], - response_model=APIResponse[None], - include_in_schema=False, -) -def delete_organization(session: SessionDep, org_id: int): - org = get_organization_by_id(session=session, org_id=org_id) - if org is None: - raise HTTPException(status_code=404, detail="Organization not found") +@router.delete("/{org_id}", response_model=APIResponse[None], include_in_schema=False) +def delete_organization( + session: SessionDep, _current_user: CurrentUserOrg, org_id: int +): + org = validate_organization(session=session, org_id=org_id) + has_org_access(_current_user, org_id) session.delete(org) session.commit() From aefe65fa76ec1ed46fd0ec05d9d468ce1cc8f73b Mon Sep 17 00:00:00 2001 From: nishika26 Date: Thu, 26 Jun 2025 22:46:42 +0530 Subject: [PATCH 02/13] test cases --- backend/app/tests/api/routes/test_org.py | 89 +++++++++++++++++++++--- 1 file changed, 78 insertions(+), 11 deletions(-) diff --git a/backend/app/tests/api/routes/test_org.py b/backend/app/tests/api/routes/test_org.py index 709bb7f5..b7c82537 100644 --- a/backend/app/tests/api/routes/test_org.py +++ b/backend/app/tests/api/routes/test_org.py @@ -24,8 +24,21 @@ def test_organization(db: Session, superuser_token_headers: dict[str, str]): return organization -# Test retrieving organizations -def test_read_organizations(db: Session, superuser_token_headers: dict[str, str]): +@pytest.fixture +def other_organization(db: Session, superuser_token_headers: dict[str, str]): + unique_name = f"OtherOrg-{random_lower_string()}" + org_data = OrganizationCreate(name=unique_name, is_active=True) + organization = create_organization(session=db, org_create=org_data) + db.commit() + return organization + + +# Test retrieving organizations (Superuser - sees all organizations) +def test_read_organizations_as_superuser( + db: Session, + superuser_token_headers: dict[str, str], + test_organization: Organization, +): response = client.get( f"{settings.API_V1_STR}/organizations/", headers=superuser_token_headers ) @@ -33,10 +46,15 @@ def test_read_organizations(db: Session, superuser_token_headers: dict[str, str] response_data = response.json() assert "data" in response_data assert isinstance(response_data["data"], list) + assert ( + len(response_data["data"]) > 0 + ) # Ensure that multiple organizations are returned -# Test creating an organization -def test_create_organization(db: Session, superuser_token_headers: dict[str, str]): +# Test creating an organization (Superuser) +def test_create_organization_as_superuser( + db: Session, superuser_token_headers: dict[str, str] +): unique_name = f"Org-{random_lower_string()}" org_data = {"name": unique_name, "is_active": True} response = client.post( @@ -44,23 +62,22 @@ def test_create_organization(db: Session, superuser_token_headers: dict[str, str json=org_data, headers=superuser_token_headers, ) - assert 200 <= response.status_code < 300 created_org = response.json() - assert "data" in created_org # Make sure there's a 'data' field + assert "data" in created_org created_org_data = created_org["data"] org = get_organization_by_id(session=db, org_id=created_org_data["id"]) - assert org is not None # The organization should be found in the DB + assert org is not None assert org.name == created_org_data["name"] assert org.is_active == created_org_data["is_active"] -def test_update_organization( +def test_update_organization_as_superuser( db: Session, test_organization: Organization, superuser_token_headers: dict[str, str], ): - unique_name = f"UpdatedOrg-{random_lower_string()}" # Ensure a unique name + unique_name = f"UpdatedOrg-{random_lower_string()}" update_data = {"name": unique_name, "is_active": False} response = client.patch( @@ -77,8 +94,28 @@ def test_update_organization( assert updated_org["is_active"] == update_data["is_active"] -# Test deleting an organization -def test_delete_organization( +# Test updating an organization (Regular user - should only update their own) +def test_update_organization_as_regular_user( + db: Session, + test_organization: Organization, + normal_user_token_headers: dict[str, str], +): + unique_name = f"UpdatedOrg-{random_lower_string()}" + update_data = {"name": unique_name, "is_active": False} + + # Regular user should only be allowed to update their own organization + response = client.patch( + f"{settings.API_V1_STR}/organizations/{test_organization.id}", + json=update_data, + headers=normal_user_token_headers, + ) + assert ( + response.status_code == 403 + ) # Forbidden if trying to update someone else's organization + + +# Test deleting an organization (Superuser) +def test_delete_organization_as_superuser( db: Session, test_organization: Organization, superuser_token_headers: dict[str, str], @@ -88,8 +125,38 @@ def test_delete_organization( headers=superuser_token_headers, ) assert response.status_code == 200 + response = client.get( f"{settings.API_V1_STR}/organizations/{test_organization.id}", headers=superuser_token_headers, ) assert response.status_code == 404 + + +# Test deleting an organization (Regular user - should only delete their own) +def test_delete_organization_as_regular_user( + db: Session, + test_organization: Organization, + normal_user_token_headers: dict[str, str], +): + response = client.delete( + f"{settings.API_V1_STR}/organizations/{test_organization.id}", + headers=normal_user_token_headers, + ) + assert ( + response.status_code == 403 + ) # Forbidden for regular user to delete someone else's org + + +# Test regular user accessing another organization (should be forbidden) +def test_read_organization_as_regular_user_without_access( + db: Session, + normal_user_token_headers: dict[str, str], + other_organization: Organization, +): + # Simulate a regular user with no access to another organization + response = client.get( + f"{settings.API_V1_STR}/organizations/{other_organization.id}", + headers=normal_user_token_headers, + ) + assert response.status_code == 403 # Forbidden, as the user doesn't have access From 6fa32915c19aeaf9959b08aa91fe6e384710f617 Mon Sep 17 00:00:00 2001 From: nishika26 Date: Thu, 26 Jun 2025 23:05:45 +0530 Subject: [PATCH 03/13] for username password login --- backend/app/api/deps.py | 8 ++++++++ backend/app/tests/api/routes/test_org.py | 20 +++----------------- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/backend/app/api/deps.py b/backend/app/api/deps.py index e8cd327b..b19d4b94 100644 --- a/backend/app/api/deps.py +++ b/backend/app/api/deps.py @@ -23,6 +23,7 @@ ProjectUser, Project, Organization, + APIKey, ) reusable_oauth2 = OAuth2PasswordBearer( @@ -99,6 +100,13 @@ def get_current_user_org( validate_organization(session, api_key_record.organization_id) organization_id = api_key_record.organization_id + if not api_key: + api_key_record = ( + session.query(APIKey).filter(APIKey.user_id == current_user.id).first() + ) + if api_key_record: + organization_id = api_key_record.organization_id + return UserOrganization( **current_user.model_dump(), organization_id=organization_id ) diff --git a/backend/app/tests/api/routes/test_org.py b/backend/app/tests/api/routes/test_org.py index b7c82537..c43b6588 100644 --- a/backend/app/tests/api/routes/test_org.py +++ b/backend/app/tests/api/routes/test_org.py @@ -33,7 +33,6 @@ def other_organization(db: Session, superuser_token_headers: dict[str, str]): return organization -# Test retrieving organizations (Superuser - sees all organizations) def test_read_organizations_as_superuser( db: Session, superuser_token_headers: dict[str, str], @@ -46,12 +45,9 @@ def test_read_organizations_as_superuser( response_data = response.json() assert "data" in response_data assert isinstance(response_data["data"], list) - assert ( - len(response_data["data"]) > 0 - ) # Ensure that multiple organizations are returned + assert len(response_data["data"]) > 0 -# Test creating an organization (Superuser) def test_create_organization_as_superuser( db: Session, superuser_token_headers: dict[str, str] ): @@ -94,7 +90,6 @@ def test_update_organization_as_superuser( assert updated_org["is_active"] == update_data["is_active"] -# Test updating an organization (Regular user - should only update their own) def test_update_organization_as_regular_user( db: Session, test_organization: Organization, @@ -103,18 +98,14 @@ def test_update_organization_as_regular_user( unique_name = f"UpdatedOrg-{random_lower_string()}" update_data = {"name": unique_name, "is_active": False} - # Regular user should only be allowed to update their own organization response = client.patch( f"{settings.API_V1_STR}/organizations/{test_organization.id}", json=update_data, headers=normal_user_token_headers, ) - assert ( - response.status_code == 403 - ) # Forbidden if trying to update someone else's organization + assert response.status_code == 403 -# Test deleting an organization (Superuser) def test_delete_organization_as_superuser( db: Session, test_organization: Organization, @@ -133,7 +124,6 @@ def test_delete_organization_as_superuser( assert response.status_code == 404 -# Test deleting an organization (Regular user - should only delete their own) def test_delete_organization_as_regular_user( db: Session, test_organization: Organization, @@ -143,18 +133,14 @@ def test_delete_organization_as_regular_user( f"{settings.API_V1_STR}/organizations/{test_organization.id}", headers=normal_user_token_headers, ) - assert ( - response.status_code == 403 - ) # Forbidden for regular user to delete someone else's org + assert response.status_code == 403 -# Test regular user accessing another organization (should be forbidden) def test_read_organization_as_regular_user_without_access( db: Session, normal_user_token_headers: dict[str, str], other_organization: Organization, ): - # Simulate a regular user with no access to another organization response = client.get( f"{settings.API_V1_STR}/organizations/{other_organization.id}", headers=normal_user_token_headers, From 2af331f017bb801fe2e08c04b8ff18e2a1e6a6f9 Mon Sep 17 00:00:00 2001 From: nishika26 Date: Fri, 27 Jun 2025 11:12:21 +0530 Subject: [PATCH 04/13] clubbing acesss function --- backend/app/api/deps.py | 9 +++------ backend/app/api/routes/organization.py | 21 ++++++++++----------- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/backend/app/api/deps.py b/backend/app/api/deps.py index b19d4b94..6f4e1379 100644 --- a/backend/app/api/deps.py +++ b/backend/app/api/deps.py @@ -155,18 +155,15 @@ def get_current_active_superuser_org(current_user: CurrentUserOrg) -> User: return current_user -def has_org_access(_current_user: CurrentUserOrg, org_id: int): +def check_org_access(_current_user: CurrentUserOrg, org_id: int = None): if _current_user.is_superuser: return - if _current_user.organization_id != org_id: + + if org_id is not None and _current_user.organization_id != org_id: raise HTTPException( status_code=403, detail="Access to this organization is forbidden" ) - -def check_org_creation_allowed(_current_user: CurrentUserOrg): - if _current_user.is_superuser: - return if _current_user.organization_id is not None: raise HTTPException( status_code=403, detail="Not allowed to create another organization" diff --git a/backend/app/api/routes/organization.py b/backend/app/api/routes/organization.py index 0b687a39..47620105 100644 --- a/backend/app/api/routes/organization.py +++ b/backend/app/api/routes/organization.py @@ -9,12 +9,7 @@ OrganizationUpdate, OrganizationPublic, ) -from app.api.deps import ( - CurrentUserOrg, - SessionDep, - has_org_access, - check_org_creation_allowed, -) +from app.api.deps import CurrentUserOrg, SessionDep, check_org_access from app.crud.organization import create_organization, validate_organization from app.utils import APIResponse @@ -27,6 +22,7 @@ def read_organizations( session: SessionDep, _current_user: CurrentUserOrg, skip: int = 0, limit: int = 100 ): + """Get a list of organizations for the current user.""" if _current_user.is_superuser: statement = select(Organization).offset(skip).limit(limit) else: @@ -44,7 +40,8 @@ def read_organizations( def create_new_organization( *, session: SessionDep, _current_user: CurrentUserOrg, org_in: OrganizationCreate ): - check_org_creation_allowed(_current_user) + """Create a new organization if allowed.""" + check_org_access(_current_user) new_org = create_organization(session=session, org_create=org_in) return APIResponse.success_response(new_org) @@ -53,9 +50,9 @@ def create_new_organization( def read_organization( *, session: SessionDep, _current_user: CurrentUserOrg, org_id: int ): + """Read a specific organization by ID.""" org = validate_organization(session=session, org_id=org_id) - has_org_access(_current_user, org_id) - + check_org_access(_current_user, org_id) # Check access to the organization return APIResponse.success_response(org) @@ -67,8 +64,9 @@ def update_organization( org_id: int, org_in: OrganizationUpdate, ): + """Update an existing organization.""" org = validate_organization(session=session, org_id=org_id) - has_org_access(_current_user, org_id) + check_org_access(_current_user, org_id) # Check access to the organization org_data = org_in.model_dump(exclude_unset=True) org = org.model_copy(update=org_data) @@ -84,8 +82,9 @@ def update_organization( def delete_organization( session: SessionDep, _current_user: CurrentUserOrg, org_id: int ): + """Delete an organization.""" org = validate_organization(session=session, org_id=org_id) - has_org_access(_current_user, org_id) + check_org_access(_current_user, org_id) # Check access to the organization session.delete(org) session.commit() From 0ed0111539bd30de5c1be20b4aae3968ba9b069b Mon Sep 17 00:00:00 2001 From: nishika26 Date: Fri, 27 Jun 2025 13:16:47 +0530 Subject: [PATCH 05/13] added logging --- backend/app/api/deps.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/backend/app/api/deps.py b/backend/app/api/deps.py index 6f4e1379..395b4faa 100644 --- a/backend/app/api/deps.py +++ b/backend/app/api/deps.py @@ -25,6 +25,9 @@ Organization, APIKey, ) +import logging + +logger = logging.getLogger(__name__) reusable_oauth2 = OAuth2PasswordBearer( tokenUrl=f"{settings.API_V1_STR}/login/access-token", auto_error=False @@ -156,19 +159,36 @@ def get_current_active_superuser_org(current_user: CurrentUserOrg) -> User: def check_org_access(_current_user: CurrentUserOrg, org_id: int = None): + """Helper function to check organization access and creation permissions.""" + + # If the user is a superuser, allow all access if _current_user.is_superuser: return + # Check if the user is trying to access a different organization if org_id is not None and _current_user.organization_id != org_id: + logger.warning( + f"[check_org_access] Access violation | user_id={_current_user.id}, attempted_org_id={org_id}, " + f"current_org_id={_current_user.organization_id}" + ) raise HTTPException( status_code=403, detail="Access to this organization is forbidden" ) if _current_user.organization_id is not None: + logger.warning( + f"[check_org_access] Organization creation violation | user_id={_current_user.id}, " + f"attempted_create_org={org_id}, current_org_id={_current_user.organization_id}" + ) raise HTTPException( status_code=403, detail="Not allowed to create another organization" ) + logger.error( + f"[check_org_access] Missing organization assignment | user_id={_current_user.id}, " + "attempted_operation_requires_organization" + ) + def verify_user_project_organization( db: SessionDep, From ce11ff6a65fa4cb60f4109d615589c82b00b0abf Mon Sep 17 00:00:00 2001 From: nishika26 Date: Mon, 30 Jun 2025 12:32:57 +0530 Subject: [PATCH 06/13] cleaner deps injection --- backend/app/api/deps.py | 34 ++----- backend/app/api/routes/organization.py | 119 +++++++++++++++++-------- 2 files changed, 89 insertions(+), 64 deletions(-) diff --git a/backend/app/api/deps.py b/backend/app/api/deps.py index 395b4faa..dacb8dd1 100644 --- a/backend/app/api/deps.py +++ b/backend/app/api/deps.py @@ -2,7 +2,7 @@ from typing import Annotated, Optional import jwt -from fastapi import Depends, HTTPException, status, Request, Header, Security +from fastapi import Depends, HTTPException, status, Request, Path, Query from fastapi.responses import JSONResponse from fastapi.security import OAuth2PasswordBearer, APIKeyHeader from jwt.exceptions import InvalidTokenError @@ -14,12 +14,13 @@ from app.core.db import engine from app.utils import APIResponse from app.crud.organization import validate_organization -from app.crud.api_key import get_api_key_by_value +from app.crud.api_key import get_api_key_by_value, get_api_key_by_user from app.models import ( TokenPayload, User, UserProjectOrg, UserOrganization, + UserOrganizations, ProjectUser, Project, Organization, @@ -95,6 +96,9 @@ def get_current_user_org( ) -> UserOrganization: """Extend `User` with organization_id if available, otherwise return UserOrganization without it.""" + if current_user.is_superuser: + return UserOrganization(**current_user.model_dump(), organization_id=None) + organization_id = None api_key = request.headers.get("X-API-KEY") if api_key: @@ -158,15 +162,9 @@ def get_current_active_superuser_org(current_user: CurrentUserOrg) -> User: return current_user -def check_org_access(_current_user: CurrentUserOrg, org_id: int = None): - """Helper function to check organization access and creation permissions.""" - - # If the user is a superuser, allow all access - if _current_user.is_superuser: - return - - # Check if the user is trying to access a different organization - if org_id is not None and _current_user.organization_id != org_id: +def check_org_access(_current_user: CurrentUserOrg, org_id: int = Path): + """Helper function to check organization access.""" + if not _current_user.is_superuser and org_id != _current_user.organization_id: logger.warning( f"[check_org_access] Access violation | user_id={_current_user.id}, attempted_org_id={org_id}, " f"current_org_id={_current_user.organization_id}" @@ -175,20 +173,6 @@ def check_org_access(_current_user: CurrentUserOrg, org_id: int = None): status_code=403, detail="Access to this organization is forbidden" ) - if _current_user.organization_id is not None: - logger.warning( - f"[check_org_access] Organization creation violation | user_id={_current_user.id}, " - f"attempted_create_org={org_id}, current_org_id={_current_user.organization_id}" - ) - raise HTTPException( - status_code=403, detail="Not allowed to create another organization" - ) - - logger.error( - f"[check_org_access] Missing organization assignment | user_id={_current_user.id}, " - "attempted_operation_requires_organization" - ) - def verify_user_project_organization( db: SessionDep, diff --git a/backend/app/api/routes/organization.py b/backend/app/api/routes/organization.py index 47620105..22a018d0 100644 --- a/backend/app/api/routes/organization.py +++ b/backend/app/api/routes/organization.py @@ -1,7 +1,8 @@ -from typing import List +from typing import Any, List -from fastapi import APIRouter -from sqlmodel import select +from fastapi import APIRouter, Depends, HTTPException +from sqlalchemy import func +from sqlmodel import Session, select from app.models import ( Organization, @@ -9,64 +10,100 @@ OrganizationUpdate, OrganizationPublic, ) -from app.api.deps import CurrentUserOrg, SessionDep, check_org_access -from app.crud.organization import create_organization, validate_organization +from app.api.deps import ( + CurrentUserOrg, + SessionDep, + get_current_active_superuser, + check_org_access, +) +from app.crud.organization import ( + create_organization, + get_organization_by_id, + validate_organization, +) from app.utils import APIResponse - router = APIRouter(prefix="/organizations", tags=["organizations"]) # Retrieve organizations -@router.get("/", response_model=APIResponse[List[OrganizationPublic]]) +@router.get( + "/", + response_model=APIResponse[List[OrganizationPublic]], +) def read_organizations( - session: SessionDep, _current_user: CurrentUserOrg, skip: int = 0, limit: int = 100 + session: SessionDep, + skip: int = 0, + limit: int = 100, + current_user=CurrentUserOrg, ): - """Get a list of organizations for the current user.""" - if _current_user.is_superuser: - statement = select(Organization).offset(skip).limit(limit) + """ + Return all organizations for superuser, + or only the one associated with the current user. + """ + + if current_user.is_superuser: + statement = select(Organization).where(Organization.is_deleted == False) else: - statement = ( - select(Organization) - .where(Organization.id == _current_user.organization_id) - .offset(skip) - .limit(limit) + if not current_user.organization_id: + return APIResponse.success_response([]) + + statement = select(Organization).where( + Organization.id == current_user.organization_id, + Organization.is_deleted == False, ) + + statement = statement.offset(skip).limit(limit) organizations = session.exec(statement).all() + return APIResponse.success_response(organizations) -@router.post("/", response_model=APIResponse[OrganizationPublic]) -def create_new_organization( - *, session: SessionDep, _current_user: CurrentUserOrg, org_in: OrganizationCreate -): - """Create a new organization if allowed.""" - check_org_access(_current_user) +# Create a new organization +@router.post( + "/", + dependencies=[Depends(get_current_active_superuser)], + response_model=APIResponse[OrganizationPublic], +) +def create_new_organization(*, session: SessionDep, org_in: OrganizationCreate): + """ + Creates a new organization, note that only a superuser can create an organization. + """ new_org = create_organization(session=session, org_create=org_in) return APIResponse.success_response(new_org) -@router.get("/{org_id}", response_model=APIResponse[OrganizationPublic]) -def read_organization( - *, session: SessionDep, _current_user: CurrentUserOrg, org_id: int -): - """Read a specific organization by ID.""" +# Retrieve an organization by ID +@router.get( + "/{org_id}", + response_model=APIResponse[OrganizationPublic], +) +def read_organization(*, session: SessionDep, org_id: int, _=Depends(check_org_access)): + """ + Retrieves an organization by ID, + a normal user can only retrieve the organization(s) associated with their user ID. + """ org = validate_organization(session=session, org_id=org_id) - check_org_access(_current_user, org_id) # Check access to the organization return APIResponse.success_response(org) -@router.patch("/{org_id}", response_model=APIResponse[OrganizationPublic]) +# Update an organization +@router.patch( + "/{org_id}", + response_model=APIResponse[OrganizationPublic], +) def update_organization( *, session: SessionDep, - _current_user: CurrentUserOrg, org_id: int, org_in: OrganizationUpdate, + _=Depends(check_org_access), ): - """Update an existing organization.""" + """ + Updates an organization by ID, + a normal user can only update the organization(s) associated with their user ID. + """ org = validate_organization(session=session, org_id=org_id) - check_org_access(_current_user, org_id) # Check access to the organization org_data = org_in.model_dump(exclude_unset=True) org = org.model_copy(update=org_data) @@ -78,14 +115,18 @@ def update_organization( return APIResponse.success_response(org) -@router.delete("/{org_id}", response_model=APIResponse[None], include_in_schema=False) -def delete_organization( - session: SessionDep, _current_user: CurrentUserOrg, org_id: int -): - """Delete an organization.""" +# Delete an organization +@router.delete( + "/{org_id}", + dependencies=[Depends(get_current_active_superuser)], + response_model=APIResponse[None], + include_in_schema=False, +) +def delete_organization(session: SessionDep, org_id: int): + """ + Deletes an existing organization, note that only a superuser can delete an organization. + """ org = validate_organization(session=session, org_id=org_id) - check_org_access(_current_user, org_id) # Check access to the organization - session.delete(org) session.commit() From 98e5f796fca66324ff86640553c103f7f952a881 Mon Sep 17 00:00:00 2001 From: nishika26 Date: Mon, 30 Jun 2025 12:34:42 +0530 Subject: [PATCH 07/13] small change --- backend/app/api/deps.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/app/api/deps.py b/backend/app/api/deps.py index dacb8dd1..e4c3e1ce 100644 --- a/backend/app/api/deps.py +++ b/backend/app/api/deps.py @@ -14,7 +14,7 @@ from app.core.db import engine from app.utils import APIResponse from app.crud.organization import validate_organization -from app.crud.api_key import get_api_key_by_value, get_api_key_by_user +from app.crud.api_key import get_api_key_by_value from app.models import ( TokenPayload, User, From f7d17edb344bd00cd7b1b853228c633e03d7fe17 Mon Sep 17 00:00:00 2001 From: nishika26 Date: Mon, 30 Jun 2025 12:36:38 +0530 Subject: [PATCH 08/13] small change --- backend/app/api/deps.py | 1 - 1 file changed, 1 deletion(-) diff --git a/backend/app/api/deps.py b/backend/app/api/deps.py index e4c3e1ce..7f4a94d6 100644 --- a/backend/app/api/deps.py +++ b/backend/app/api/deps.py @@ -20,7 +20,6 @@ User, UserProjectOrg, UserOrganization, - UserOrganizations, ProjectUser, Project, Organization, From 0338575396ec96de9bd93f8208a1813f11483482 Mon Sep 17 00:00:00 2001 From: nishika26 Date: Mon, 30 Jun 2025 13:24:44 +0530 Subject: [PATCH 09/13] colon and equal to confusion --- backend/app/api/deps.py | 1 + backend/app/api/routes/organization.py | 7 ++----- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/backend/app/api/deps.py b/backend/app/api/deps.py index 7f4a94d6..ebe9196b 100644 --- a/backend/app/api/deps.py +++ b/backend/app/api/deps.py @@ -93,6 +93,7 @@ def get_current_user( def get_current_user_org( current_user: CurrentUser, session: SessionDep, request: Request ) -> UserOrganization: + print("🔍 Current user inside dependency:", current_user) """Extend `User` with organization_id if available, otherwise return UserOrganization without it.""" if current_user.is_superuser: diff --git a/backend/app/api/routes/organization.py b/backend/app/api/routes/organization.py index 22a018d0..b71ad7a4 100644 --- a/backend/app/api/routes/organization.py +++ b/backend/app/api/routes/organization.py @@ -32,10 +32,7 @@ response_model=APIResponse[List[OrganizationPublic]], ) def read_organizations( - session: SessionDep, - skip: int = 0, - limit: int = 100, - current_user=CurrentUserOrg, + session: SessionDep, current_user: CurrentUserOrg, skip: int = 0, limit: int = 100 ): """ Return all organizations for superuser, @@ -43,7 +40,7 @@ def read_organizations( """ if current_user.is_superuser: - statement = select(Organization).where(Organization.is_deleted == False) + statement = select(Organization).where(Organization.is_active == True) else: if not current_user.organization_id: return APIResponse.success_response([]) From 6cd19c48807a563847de4e3351950d84a9de5f5f Mon Sep 17 00:00:00 2001 From: nishika26 Date: Mon, 30 Jun 2025 13:30:03 +0530 Subject: [PATCH 10/13] removing superuser check from deps for responses --- backend/app/api/deps.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/backend/app/api/deps.py b/backend/app/api/deps.py index ebe9196b..e01bcf06 100644 --- a/backend/app/api/deps.py +++ b/backend/app/api/deps.py @@ -93,12 +93,8 @@ def get_current_user( def get_current_user_org( current_user: CurrentUser, session: SessionDep, request: Request ) -> UserOrganization: - print("🔍 Current user inside dependency:", current_user) """Extend `User` with organization_id if available, otherwise return UserOrganization without it.""" - if current_user.is_superuser: - return UserOrganization(**current_user.model_dump(), organization_id=None) - organization_id = None api_key = request.headers.get("X-API-KEY") if api_key: From c0e1bc07094e70bffeb8f86b640195536927b1e9 Mon Sep 17 00:00:00 2001 From: nishika26 Date: Mon, 30 Jun 2025 14:36:21 +0530 Subject: [PATCH 11/13] pr review fixes --- backend/app/api/deps.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/backend/app/api/deps.py b/backend/app/api/deps.py index e01bcf06..b4d3a08b 100644 --- a/backend/app/api/deps.py +++ b/backend/app/api/deps.py @@ -2,6 +2,7 @@ from typing import Annotated, Optional import jwt +import logging from fastapi import Depends, HTTPException, status, Request, Path, Query from fastapi.responses import JSONResponse from fastapi.security import OAuth2PasswordBearer, APIKeyHeader @@ -25,7 +26,7 @@ Organization, APIKey, ) -import logging + logger = logging.getLogger(__name__) @@ -105,7 +106,9 @@ def get_current_user_org( if not api_key: api_key_record = ( - session.query(APIKey).filter(APIKey.user_id == current_user.id).first() + session.query(APIKey) + .filter(APIKey.user_id == current_user.id, APIKey.is_deleted.is_(True)) + .first() ) if api_key_record: organization_id = api_key_record.organization_id From ce06f93cdfc7024401372ae2ebe725cae47ac537 Mon Sep 17 00:00:00 2001 From: nishika26 Date: Mon, 30 Jun 2025 14:46:11 +0530 Subject: [PATCH 12/13] pr review fixes --- backend/app/api/deps.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/app/api/deps.py b/backend/app/api/deps.py index b4d3a08b..7abed794 100644 --- a/backend/app/api/deps.py +++ b/backend/app/api/deps.py @@ -107,7 +107,7 @@ def get_current_user_org( if not api_key: api_key_record = ( session.query(APIKey) - .filter(APIKey.user_id == current_user.id, APIKey.is_deleted.is_(True)) + .filter(APIKey.user_id == current_user.id, APIKey.is_deleted.is_(False)) .first() ) if api_key_record: From 4f6be4e9674ce88c22db2cefbe261781fd7c6699 Mon Sep 17 00:00:00 2001 From: nishika26 Date: Mon, 30 Jun 2025 17:55:58 +0530 Subject: [PATCH 13/13] removing normal user via username password deps --- backend/app/api/deps.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/backend/app/api/deps.py b/backend/app/api/deps.py index 7abed794..d19f0653 100644 --- a/backend/app/api/deps.py +++ b/backend/app/api/deps.py @@ -104,15 +104,6 @@ def get_current_user_org( validate_organization(session, api_key_record.organization_id) organization_id = api_key_record.organization_id - if not api_key: - api_key_record = ( - session.query(APIKey) - .filter(APIKey.user_id == current_user.id, APIKey.is_deleted.is_(False)) - .first() - ) - if api_key_record: - organization_id = api_key_record.organization_id - return UserOrganization( **current_user.model_dump(), organization_id=organization_id )