diff --git a/apps/backend/src/rhesis/backend/app/crud/__init__.py b/apps/backend/src/rhesis/backend/app/crud/__init__.py index 8289e487f3..094240031d 100644 --- a/apps/backend/src/rhesis/backend/app/crud/__init__.py +++ b/apps/backend/src/rhesis/backend/app/crud/__init__.py @@ -5,13 +5,11 @@ import logging import uuid from typing import Any, Dict, List, Optional, Union -from uuid import UUID -from sqlalchemy import and_, select, text +from sqlalchemy import and_, select from sqlalchemy.orm import Session from rhesis.backend.app import models, schemas -from rhesis.backend.app.database import reset_session_context from rhesis.backend.app.models.test import test_test_set_association from rhesis.backend.app.utils.crud_utils import ( bulk_delete_by_ids, @@ -28,26 +26,6 @@ logger = logging.getLogger(__name__) -# Helper function to print session variables -def get_session_variables(db: Session): - """Get and return the current PostgreSQL session variables for debugging""" - results = {} - try: - # Check if variables exist before trying to show them - check_org = db.execute( - text("SELECT current_setting('app.current_organization', true)") - ).scalar() - check_user = db.execute(text("SELECT current_setting('app.current_user', true)")).scalar() - - results["app.current_organization"] = check_org if check_org else "Not set" - results["app.current_user"] = check_user if check_user else "Not set" - - return results - except Exception as e: - logger.debug(f"Error getting session variables: {e}") - return {"error": str(e)} - - # Endpoint CRUD _ENDPOINT_RELATED_FIELDS = ( include(models.Endpoint.status), @@ -789,95 +767,6 @@ def delete_topic( return delete_item(db, models.Topic, topic_id, organization_id, user_id) -# Organization CRUD -def get_organization( - db: Session, organization_id: uuid.UUID, tenant_organization_id: str = None, user_id: str = None -) -> Optional[models.Organization]: - """Get organization.""" - return get_item(db, models.Organization, organization_id, tenant_organization_id, user_id) - - -def get_organizations( - db: Session, - skip: int = 0, - limit: int = 10, - sort_by: str = "created_at", - sort_order: str = "desc", - filter: str | None = None, - organization_id: str = None, - user_id: str = None, -) -> List[models.Organization]: - return get_items( - db, - models.Organization, - skip, - limit, - sort_by, - sort_order, - filter, - organization_id=organization_id, - user_id=user_id, - ) - - -def create_organization( - db: Session, - organization: schemas.OrganizationCreate, - owner_user_id: Optional[UUID] = None, -) -> models.Organization: - """Create a new organization without RLS checks, because we're creating a new organization. - - When *owner_user_id* is supplied (always the case on the HTTP path) it overrides any - client-supplied ``owner_id``/``user_id`` values in the schema, making the backend - authoritative for org ownership (SP3 decision — server-set, cannot be forged). - Internal callers such as ``local_init.py`` that already supply the correct IDs in the - schema may pass ``owner_user_id=None`` to preserve the existing behaviour. - """ - # Print session variables before reset - before_vars = get_session_variables(db) - logger.info(f"Session variables BEFORE reset: {before_vars}") - - # Reset session context to ensure the new organization is created correctly - reset_session_context(db) - - # Verify variables are cleared - after_vars = get_session_variables(db) - logger.info(f"Session variables AFTER reset: {after_vars}") - - # Make sure session is clean to avoid RLS issues - db.expire_all() - - # Convert Pydantic model to dict; project_id is not a column on Organization - org_data = organization.model_dump(exclude={"project_id"}) - - # Backend is authoritative for ownership when owner_user_id is provided. - if owner_user_id is not None: - org_data["owner_id"] = str(owner_user_id) - org_data["user_id"] = str(owner_user_id) - - db_org = models.Organization(**org_data) - - # Add to session - transaction management is handled by context manager - db.add(db_org) - db.flush() # Flush to get the ID - - # Simply return the object without refreshing - # The refresh operation is what often triggers RLS issues - logger.info(f"Organization created successfully: {db_org.id}") - return db_org - - -def update_organization( - db: Session, organization_id: uuid.UUID, organization: schemas.OrganizationUpdate -) -> Optional[models.Organization]: - return update_item(db, models.Organization, organization_id, organization) - - -def delete_organization(db: Session, organization_id: uuid.UUID) -> Optional[models.Organization]: - """Delete organization - requires superuser permissions (handled in router)""" - return delete_item(db, models.Organization, organization_id) - - def get_test( db: Session, test_id: uuid.UUID, organization_id: str = None, user_id: str = None ) -> Optional[models.Test]: diff --git a/apps/backend/src/rhesis/backend/app/crud/organization.py b/apps/backend/src/rhesis/backend/app/crud/organization.py new file mode 100644 index 0000000000..627e2eae7a --- /dev/null +++ b/apps/backend/src/rhesis/backend/app/crud/organization.py @@ -0,0 +1,144 @@ +"""CRUD operations for organizations. + +Part of the incremental split of the ``crud`` monolith: ``crud/__init__.py`` still holds +the bulk of the functions, and per-entity modules like this one take over as the code +around them is touched. + +``create_organization`` is the one function here that steps outside the normal tenant +machinery. Every other entity is created *inside* a tenant, so the org/user GUCs are +already set and RLS applies; an organization *is* the tenant, so there is nothing to scope +it to yet. It therefore calls ``reset_session_context`` to blank those GUCs, expires the +session so no already-loaded row drags a stale tenant context along, and returns the +flushed object without a refresh -- the refresh is what typically trips RLS here. + +``get_session_variables`` lives in this module because ``create_organization`` is its only +caller: it reads back ``app.current_organization``/``app.current_user`` before and after +that reset so the debug log shows the GUCs really were cleared. +""" + +import logging +import uuid +from typing import List, Optional +from uuid import UUID + +from sqlalchemy import text +from sqlalchemy.orm import Session + +from rhesis.backend.app import models, schemas +from rhesis.backend.app.database import reset_session_context +from rhesis.backend.app.utils.crud_utils import ( + delete_item, + get_item, + get_items, + update_item, +) + +logger = logging.getLogger(__name__) + + +# Helper function to print session variables +def get_session_variables(db: Session): + """Get and return the current PostgreSQL session variables for debugging""" + results = {} + try: + # Check if variables exist before trying to show them + check_org = db.execute( + text("SELECT current_setting('app.current_organization', true)") + ).scalar() + check_user = db.execute(text("SELECT current_setting('app.current_user', true)")).scalar() + + results["app.current_organization"] = check_org if check_org else "Not set" + results["app.current_user"] = check_user if check_user else "Not set" + + return results + except Exception as e: + logger.debug(f"Error getting session variables: {e}") + return {"error": str(e)} + + +def get_organization( + db: Session, organization_id: uuid.UUID, tenant_organization_id: str = None, user_id: str = None +) -> Optional[models.Organization]: + """Get organization.""" + return get_item(db, models.Organization, organization_id, tenant_organization_id, user_id) + + +def get_organizations( + db: Session, + skip: int = 0, + limit: int = 10, + sort_by: str = "created_at", + sort_order: str = "desc", + filter: str | None = None, + organization_id: str = None, + user_id: str = None, +) -> List[models.Organization]: + return get_items( + db, + models.Organization, + skip, + limit, + sort_by, + sort_order, + filter, + organization_id=organization_id, + user_id=user_id, + ) + + +def create_organization( + db: Session, + organization: schemas.OrganizationCreate, + owner_user_id: Optional[UUID] = None, +) -> models.Organization: + """Create a new organization without RLS checks, because we're creating a new organization. + + When *owner_user_id* is supplied (always the case on the HTTP path) it overrides any + client-supplied ``owner_id``/``user_id`` values in the schema, making the backend + authoritative for org ownership (SP3 decision — server-set, cannot be forged). + Internal callers such as ``local_init.py`` that already supply the correct IDs in the + schema may pass ``owner_user_id=None`` to preserve the existing behaviour. + """ + # Print session variables before reset + before_vars = get_session_variables(db) + logger.info(f"Session variables BEFORE reset: {before_vars}") + + # Reset session context to ensure the new organization is created correctly + reset_session_context(db) + + # Verify variables are cleared + after_vars = get_session_variables(db) + logger.info(f"Session variables AFTER reset: {after_vars}") + + # Make sure session is clean to avoid RLS issues + db.expire_all() + + # Convert Pydantic model to dict; project_id is not a column on Organization + org_data = organization.model_dump(exclude={"project_id"}) + + # Backend is authoritative for ownership when owner_user_id is provided. + if owner_user_id is not None: + org_data["owner_id"] = str(owner_user_id) + org_data["user_id"] = str(owner_user_id) + + db_org = models.Organization(**org_data) + + # Add to session - transaction management is handled by context manager + db.add(db_org) + db.flush() # Flush to get the ID + + # Simply return the object without refreshing + # The refresh operation is what often triggers RLS issues + logger.info(f"Organization created successfully: {db_org.id}") + return db_org + + +def update_organization( + db: Session, organization_id: uuid.UUID, organization: schemas.OrganizationUpdate +) -> Optional[models.Organization]: + return update_item(db, models.Organization, organization_id, organization) + + +def delete_organization(db: Session, organization_id: uuid.UUID) -> Optional[models.Organization]: + """Delete organization - requires superuser permissions (handled in router)""" + return delete_item(db, models.Organization, organization_id) diff --git a/apps/backend/src/rhesis/backend/app/routers/organization.py b/apps/backend/src/rhesis/backend/app/routers/organization.py index 79608818d9..f9a0902920 100644 --- a/apps/backend/src/rhesis/backend/app/routers/organization.py +++ b/apps/backend/src/rhesis/backend/app/routers/organization.py @@ -6,11 +6,12 @@ from rhesis.backend.app.auth.capabilities import Permission, capability from sqlalchemy.orm import Session -from rhesis.backend.app import crud, models, schemas +from rhesis.backend.app import models, schemas from rhesis.backend.app.auth.user_utils import ( require_current_user_or_token, require_current_user_or_token_without_context, ) +from rhesis.backend.app.crud import organization as organization_crud from rhesis.backend.app.database import set_session_variables from rhesis.backend.app.dependencies import ( get_db_session, @@ -50,7 +51,9 @@ def create_organization( ): # owner_id and user_id are always set server-side from the authenticated caller — # the client-supplied values are ignored so the onboarding flow cannot forge ownership. - return crud.create_organization(db=db, organization=organization, owner_user_id=current_user.id) + return organization_crud.create_organization( + db=db, organization=organization, owner_user_id=current_user.id + ) @router.get("/", response_model=list[schemas.Organization]) @@ -69,7 +72,7 @@ def read_organizations( """Get all organizations with their related objects""" try: organization_id, user_id = tenant_context - return crud.get_organizations( + return organization_crud.get_organizations( db=db, skip=skip, limit=limit, @@ -92,7 +95,7 @@ def read_organization( ): try: tenant_organization_id, user_id = tenant_context - db_organization = crud.get_organization( + db_organization = organization_crud.get_organization( db, organization_id=organization_id, tenant_organization_id=tenant_organization_id, @@ -115,7 +118,7 @@ def update_organization( db: Session = Depends(get_tenant_db_session), current_user: User = Depends(require_current_user_or_token), ): - db_organization = crud.update_organization( + db_organization = organization_crud.update_organization( db, organization_id=organization_id, organization=organization ) if db_organization is None: @@ -135,7 +138,7 @@ def initialize_organization_data( ): """Load initial data for an organization if onboarding is not complete.""" try: - org = crud.get_organization(db, organization_id=organization_id) + org = organization_crud.get_organization(db, organization_id=organization_id) if not org: raise HTTPException(status_code=404, detail="Organization not found") @@ -271,7 +274,7 @@ def rollback_organization_data( """Rollback initial data for an organization.""" try: print(f"Rolling back initial data for organization {organization_id}") - org = crud.get_organization(db, organization_id=organization_id) + org = organization_crud.get_organization(db, organization_id=organization_id) if not org: raise HTTPException(status_code=404, detail="Organization not found") diff --git a/apps/backend/src/rhesis/backend/app/services/platform_key.py b/apps/backend/src/rhesis/backend/app/services/platform_key.py index 5ab800eb8f..26a8c41d2e 100644 --- a/apps/backend/src/rhesis/backend/app/services/platform_key.py +++ b/apps/backend/src/rhesis/backend/app/services/platform_key.py @@ -49,7 +49,7 @@ def _load_org(db: Session, organization_id) -> Organization | None: ``Organization`` is exempt from the ambient tenant auto-filter (it is queried before any tenant context exists), so a direct ``id`` lookup is the correct pattern here -- mirroring ``feature_gates._load_org`` and - ``crud.get_organization``. Callers pass the authenticated user's own + ``crud.organization.get_organization``. Callers pass the authenticated user's own ``organization_id``, which keeps access org-scoped. Soft-deleted orgs are still excluded despite the plain ``db.query(...)``: diff --git a/tests/backend/crud/test_transaction_management.py b/tests/backend/crud/test_transaction_management.py index 323e518d8f..09b43eec9f 100644 --- a/tests/backend/crud/test_transaction_management.py +++ b/tests/backend/crud/test_transaction_management.py @@ -30,8 +30,9 @@ from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session -from rhesis.backend.app import crud, models, schemas +from rhesis.backend.app import models, schemas from rhesis.backend.app.constants import EntityType +from rhesis.backend.app.crud import organization as organization_crud from rhesis.backend.app.crud import tag as tag_crud from rhesis.backend.app.crud import user as user_crud from rhesis.backend.app.crud.comment import add_emoji_reaction, remove_emoji_reaction @@ -57,7 +58,7 @@ def test_create_organization_commits_on_success(self, test_db: Session, authenti org_create = schemas.OrganizationCreate(**org_data) # Create organization - result = crud.create_organization(test_db, org_create) + result = organization_crud.create_organization(test_db, org_create) # Verify organization was created and persisted assert result is not None @@ -84,7 +85,7 @@ def test_create_organization_rollback_on_exception(self, test_db: Session, authe # Mock db.add to raise an exception to test rollback with patch.object(test_db, "add", side_effect=IntegrityError("", "", "")): with pytest.raises(IntegrityError): - crud.create_organization(test_db, org_create) + organization_crud.create_organization(test_db, org_create) # Verify no organization was created (transaction rolled back) final_count = test_db.query(models.Organization).count() @@ -328,7 +329,7 @@ def test_transaction_isolation_between_operations(self, test_db: Session, authen org_data1["user_id"] = str(authenticated_user.id) org_create1 = schemas.OrganizationCreate(**org_data1) - result1 = crud.create_organization(test_db, org_create1) + result1 = organization_crud.create_organization(test_db, org_create1) assert result1 is not None # Create second organization @@ -338,7 +339,7 @@ def test_transaction_isolation_between_operations(self, test_db: Session, authen org_data2["user_id"] = str(authenticated_user.id) org_create2 = schemas.OrganizationCreate(**org_data2) - result2 = crud.create_organization(test_db, org_create2) + result2 = organization_crud.create_organization(test_db, org_create2) assert result2 is not None # Verify both organizations exist independently @@ -366,7 +367,7 @@ def test_exception_in_one_operation_does_not_affect_others( org_data1["user_id"] = str(authenticated_user.id) org_create1 = schemas.OrganizationCreate(**org_data1) - result1 = crud.create_organization(test_db, org_create1) + result1 = organization_crud.create_organization(test_db, org_create1) assert result1 is not None # Try to create second organization with exception @@ -378,7 +379,7 @@ def test_exception_in_one_operation_does_not_affect_others( with patch.object(test_db, "add", side_effect=IntegrityError("", "", "")): with pytest.raises(IntegrityError): - crud.create_organization(test_db, org_create2) + organization_crud.create_organization(test_db, org_create2) # Verify first organization still exists (not affected by second failure) db_org1 = ( diff --git a/tests/backend/fixtures/test_setup.py b/tests/backend/fixtures/test_setup.py index 4d25998a7b..1a24aa7f6a 100644 --- a/tests/backend/fixtures/test_setup.py +++ b/tests/backend/fixtures/test_setup.py @@ -18,8 +18,9 @@ from sqlalchemy.orm import Session, sessionmaker # Import backend modules -from rhesis.backend.app import crud, models +from rhesis.backend.app import models from rhesis.backend.app.auth.token_utils import generate_api_token +from rhesis.backend.app.crud import organization as organization_crud from rhesis.backend.app.crud import user as user_crud from rhesis.backend.app.crud.token import create_token from rhesis.backend.app.database import get_database_url @@ -64,7 +65,7 @@ def create_test_organization(db: Session, name: str = "Test Organization") -> mo is_onboarding_complete=False, # Will be set to True after initial data load ) - organization = crud.create_organization(db, org_data) + organization = organization_crud.create_organization(db, org_data) print(f"✅ Created test organization: {organization.name} (ID: {organization.id})") return organization