diff --git a/.github/workflows/claude-code-automation-pr-review.yml b/.github/workflows/claude-code-automation-pr-review.yml index 3e9d2f1..35643b5 100644 --- a/.github/workflows/claude-code-automation-pr-review.yml +++ b/.github/workflows/claude-code-automation-pr-review.yml @@ -30,7 +30,7 @@ jobs: track_progress: ${{ github.event_name == 'pull_request' && contains(fromJSON('["opened", "synchronize", "ready_for_review", "reopened"]'), github.event.action) }} use_sticky_comment: true claude_args: | - --max-turns 15 + --max-turns 100 --model claude-sonnet-4-5-20250929 --allowedTools "mcp__github_inline_comment__create_inline_comment,Read,Write,Edit,MultiEdit,Glob,Grep,LS,WebFetch,WebSearch,Bash(git:*),Bash(bun:*),Bash(npm:*),Bash(npx:*),Bash(gh:*),Bash(uv:*),Bash(make:*),Bash(export:*)" allowed_bots: "dependabot[bot],renovate[bot]" diff --git a/src/aignostics_foundry_core/AGENTS.md b/src/aignostics_foundry_core/AGENTS.md index 6a0cc01..1f6b2e8 100644 --- a/src/aignostics_foundry_core/AGENTS.md +++ b/src/aignostics_foundry_core/AGENTS.md @@ -102,7 +102,7 @@ This file provides an overview of all modules in `aignostics_foundry_core`, thei - **Purpose**: Provides Auth0 cookie-based session authentication dependencies for FastAPI routes. All project-specific settings (org ID, role claim) are loaded from `AuthSettings` whose env prefix is configurable at instantiation. - **Key Features**: - - `AuthSettings(OpaqueSettings)` — uses the active FoundryContext.env_prefix to derive the env prefix (`{ctx.env_prefix}AUTH_`). Fields: `internal_org_id` (for internal org check), `auth0_role_claim` (JWT claim name for role) + - `AuthSettings(OpaqueSettings)` — uses the active FoundryContext.env_prefix to derive the env prefix (`{ctx.env_prefix}AUTH_`). Fields: `internal_org_id` (required `str`; identifies the internal organization), `auth0_role_claim` (required `str`; JWT claim name for role). Both fields are mandatory — no defaults are provided. - `UnauthenticatedError(Exception)` — raised when a user session is missing or invalid - `ForbiddenError(ApiException)` — `status_code = 403`; raised when user lacks required role or org membership - `get_auth_client(request)` — retrieves `AuthClient` from `request.app.state.auth_client`; raises `RuntimeError` if not configured @@ -111,7 +111,7 @@ This file provides an overview of all modules in `aignostics_foundry_core`, thei - `require_admin` — dependency: requires admin role - `require_internal` — dependency: requires internal organization membership - `require_internal_admin` — dependency: requires internal org membership AND admin role - - Auth0 cookie security scheme constants: `AUTH0_SESSION_COOKIE_NAME`, `AUTH0_TRANSACTION_COOKIE_NAME`, `AUTH0_ROLE_ADMIN`, `DEFAULT_AUTH0_ROLE_CLAIM` + - Auth0 cookie security scheme constants: `AUTH0_SESSION_COOKIE_NAME`, `AUTH0_TRANSACTION_COOKIE_NAME`, `AUTH0_ROLE_ADMIN` - **Location**: `aignostics_foundry_core/api/auth.py` - **Dependencies**: `auth0-fastapi>=1.0.0b5,<2`, `fastapi>=0.110,<1`, `loguru>=0.7,<1` (all mandatory) - **Import**: `from aignostics_foundry_core.api.auth import AuthSettings, ForbiddenError, UnauthenticatedError, get_auth_client, get_user, require_authenticated, require_admin, require_internal, require_internal_admin` diff --git a/src/aignostics_foundry_core/api/__init__.py b/src/aignostics_foundry_core/api/__init__.py index bc57952..12923d3 100644 --- a/src/aignostics_foundry_core/api/__init__.py +++ b/src/aignostics_foundry_core/api/__init__.py @@ -12,7 +12,6 @@ AUTH0_ROLE_ADMIN, AUTH0_SESSION_COOKIE_NAME, AUTH0_TRANSACTION_COOKIE_NAME, - DEFAULT_AUTH0_ROLE_CLAIM, AuthSettings, ForbiddenError, UnauthenticatedError, @@ -67,7 +66,6 @@ "AUTH0_ROLE_ADMIN", "AUTH0_SESSION_COOKIE_NAME", "AUTH0_TRANSACTION_COOKIE_NAME", - "DEFAULT_AUTH0_ROLE_CLAIM", # exceptions "AccessDeniedException", "ApiException", diff --git a/src/aignostics_foundry_core/api/auth.py b/src/aignostics_foundry_core/api/auth.py index 8446148..55f4fd2 100644 --- a/src/aignostics_foundry_core/api/auth.py +++ b/src/aignostics_foundry_core/api/auth.py @@ -28,8 +28,6 @@ AUTH0_COOKIE_SCHEME_DESCRIPTION = "Auth0 session cookie authentication scheme." AUTH0_ROLE_ADMIN = "admin" USER_NOT_AUTHENTICATED = "User is not authenticated" -# TODO(oliverm): remove the default; it should not reference Bridge -DEFAULT_AUTH0_ROLE_CLAIM = "https://aignostics-platform-bridge/role" class AuthSettings(OpaqueSettings): @@ -37,12 +35,15 @@ class AuthSettings(OpaqueSettings): The effective prefix is ``{FoundryContext.env_prefix}AUTH_``, resolved at instantiation time via :func:`aignostics_foundry_core.foundry.get_context`. + + Both ``internal_org_id`` and ``auth0_role_claim`` are required — they must be + provided via the corresponding environment variables (no defaults). """ model_config = SettingsConfigDict(extra="ignore") - internal_org_id: str | None = None # TODO(oliverm): make mandatory - auth0_role_claim: str = DEFAULT_AUTH0_ROLE_CLAIM # TODO(oliverm): make mandatory and remove default + internal_org_id: str + auth0_role_claim: str def __init__(self, **kwargs: Any) -> None: # noqa: ANN401 """Initialise settings, deriving env_prefix from the active FoundryContext.""" diff --git a/tests/aignostics_foundry_core/api/__init__.py b/tests/aignostics_foundry_core/api/__init__.py index 6b58dce..9e46522 100644 --- a/tests/aignostics_foundry_core/api/__init__.py +++ b/tests/aignostics_foundry_core/api/__init__.py @@ -1 +1,6 @@ """Tests for aignostics_foundry_core.api sub-package.""" + +from tests.conftest import TEST_PROJECT_PREFIX + +INTERNAL_ORG_ID_VAR_NAME = f"{TEST_PROJECT_PREFIX}AUTH_INTERNAL_ORG_ID" +AUTH0_ROLE_CLAIM_VAR_NAME = f"{TEST_PROJECT_PREFIX}AUTH_AUTH0_ROLE_CLAIM" diff --git a/tests/aignostics_foundry_core/api/auth_test.py b/tests/aignostics_foundry_core/api/auth_test.py index 411beb7..a24c466 100644 --- a/tests/aignostics_foundry_core/api/auth_test.py +++ b/tests/aignostics_foundry_core/api/auth_test.py @@ -1,5 +1,6 @@ """Tests for aignostics_foundry_core.api.auth.""" +import os import time from collections.abc import Generator from unittest.mock import AsyncMock, MagicMock @@ -8,7 +9,6 @@ from aignostics_foundry_core.api.auth import ( AUTH0_ROLE_ADMIN, - DEFAULT_AUTH0_ROLE_CLAIM, AuthSettings, ForbiddenError, UnauthenticatedError, @@ -20,10 +20,12 @@ require_internal_admin, ) from aignostics_foundry_core.foundry import reset_context, set_context -from tests.conftest import TEST_PROJECT_PREFIX, make_context +from tests.aignostics_foundry_core.api import AUTH0_ROLE_CLAIM_VAR_NAME, INTERNAL_ORG_ID_VAR_NAME +from tests.conftest import make_context _INTERNAL_ORG_ID = "org_internal_123" _OTHER_ORG_ID = "org_other_456" +_TEST_ROLE_CLAIM = "https://aignostics-platform-bridge/role" _USER_NOT_AUTHENTICATED = "User is not authenticated" _USER_SUB = "auth0|x" _USER_EMAIL = "x@x.com" @@ -31,13 +33,17 @@ @pytest.fixture(autouse=True) def _auth_context() -> Generator[None, None, None]: # pyright: ignore[reportUnusedFunction] - """Set a real FoundryContext for all auth tests to preserve FOUNDRY_AUTH_* env var names. + """Set a real FoundryContext and required AuthSettings env vars for all auth tests. Yields: None """ set_context(make_context()) + os.environ[INTERNAL_ORG_ID_VAR_NAME] = _INTERNAL_ORG_ID + os.environ[AUTH0_ROLE_CLAIM_VAR_NAME] = _TEST_ROLE_CLAIM yield + os.environ.pop(INTERNAL_ORG_ID_VAR_NAME, None) + os.environ.pop(AUTH0_ROLE_CLAIM_VAR_NAME, None) reset_context() @@ -91,24 +97,23 @@ def test_get_auth_client_returns_client_when_present(self) -> None: @pytest.mark.unit class TestAuthSettings: - """Tests for AuthSettings defaults.""" - - def test_auth_settings_defaults(self) -> None: - """AuthSettings.auth0_role_claim has the expected default role claim URL.""" - settings = AuthSettings() - assert settings.auth0_role_claim == DEFAULT_AUTH0_ROLE_CLAIM - assert settings.internal_org_id is None - - def test_auth_settings_role_claim_value(self) -> None: - """The default role claim is the Aignostics platform bridge claim URL.""" - assert DEFAULT_AUTH0_ROLE_CLAIM == "https://aignostics-platform-bridge/role" + """Tests for AuthSettings.""" def test_auth_settings_uses_context_env_prefix(self, monkeypatch: pytest.MonkeyPatch) -> None: - """AuthSettings reads env vars from the prefix supplied by FoundryContext.""" - set_context(make_context()) - monkeypatch.setenv(f"{TEST_PROJECT_PREFIX}AUTH_AUTH0_ROLE_CLAIM", "https://custom/role") + """AuthSettings reads both required fields from env vars using the context's prefix.""" + monkeypatch.setenv(AUTH0_ROLE_CLAIM_VAR_NAME, "https://custom/role") settings = AuthSettings() assert settings.auth0_role_claim == "https://custom/role" + assert settings.internal_org_id == _INTERNAL_ORG_ID + + def test_auth_settings_raises_when_required_fields_absent(self, monkeypatch: pytest.MonkeyPatch) -> None: + """AuthSettings raises ValidationError when required env vars are absent.""" + import pydantic + + monkeypatch.delenv(INTERNAL_ORG_ID_VAR_NAME, raising=False) + monkeypatch.delenv(AUTH0_ROLE_CLAIM_VAR_NAME, raising=False) + with pytest.raises(pydantic.ValidationError): + AuthSettings() @pytest.mark.integration @@ -207,7 +212,7 @@ class TestRequireAdmin: async def test_no_user_raises_forbidden_error(self, monkeypatch: pytest.MonkeyPatch) -> None: """require_admin raises ForbiddenError when no session is available.""" - monkeypatch.delenv("FOUNDRY_AUTH_AUTH0_ROLE_CLAIM", raising=False) + monkeypatch.setenv(AUTH0_ROLE_CLAIM_VAR_NAME, _TEST_ROLE_CLAIM) request = MagicMock() request.app.state = MagicMock(spec=[]) # no auth_client → get_user returns None @@ -216,9 +221,9 @@ async def test_no_user_raises_forbidden_error(self, monkeypatch: pytest.MonkeyPa async def test_wrong_role_raises_forbidden_error(self, monkeypatch: pytest.MonkeyPatch) -> None: """require_admin raises ForbiddenError when user has a non-admin role.""" - monkeypatch.delenv("FOUNDRY_AUTH_AUTH0_ROLE_CLAIM", raising=False) + monkeypatch.setenv(AUTH0_ROLE_CLAIM_VAR_NAME, _TEST_ROLE_CLAIM) request = MagicMock() - user = {"sub": _USER_SUB, DEFAULT_AUTH0_ROLE_CLAIM: "viewer", "exp": int(time.time()) + 3600} + user = {"sub": _USER_SUB, _TEST_ROLE_CLAIM: "viewer", "exp": int(time.time()) + 3600} fake_client = MagicMock() fake_client.require_session = AsyncMock(return_value={"user": user}) request.app.state.auth_client = fake_client @@ -228,9 +233,9 @@ async def test_wrong_role_raises_forbidden_error(self, monkeypatch: pytest.Monke async def test_admin_role_passes(self, monkeypatch: pytest.MonkeyPatch) -> None: """require_admin returns None without raising when user has the admin role.""" - monkeypatch.delenv("FOUNDRY_AUTH_AUTH0_ROLE_CLAIM", raising=False) + monkeypatch.setenv(AUTH0_ROLE_CLAIM_VAR_NAME, _TEST_ROLE_CLAIM) request = MagicMock() - user = {"sub": _USER_SUB, DEFAULT_AUTH0_ROLE_CLAIM: AUTH0_ROLE_ADMIN, "exp": int(time.time()) + 3600} + user = {"sub": _USER_SUB, _TEST_ROLE_CLAIM: AUTH0_ROLE_ADMIN, "exp": int(time.time()) + 3600} fake_client = MagicMock() fake_client.require_session = AsyncMock(return_value={"user": user}) request.app.state.auth_client = fake_client @@ -245,7 +250,6 @@ class TestRequireInternal: async def test_unauthenticated_user_raises_forbidden_error(self, monkeypatch: pytest.MonkeyPatch) -> None: """require_internal raises ForbiddenError when no session is available.""" - monkeypatch.setenv("FOUNDRY_AUTH_INTERNAL_ORG_ID", _INTERNAL_ORG_ID) request = MagicMock() request.app.state = MagicMock(spec=[]) # no auth_client → get_user returns None @@ -254,7 +258,6 @@ async def test_unauthenticated_user_raises_forbidden_error(self, monkeypatch: py async def test_wrong_org_raises_forbidden_error(self, monkeypatch: pytest.MonkeyPatch) -> None: """require_internal raises ForbiddenError when user belongs to a different org.""" - monkeypatch.setenv("FOUNDRY_AUTH_INTERNAL_ORG_ID", _INTERNAL_ORG_ID) request = MagicMock() user = {"sub": _USER_SUB, "org_id": _OTHER_ORG_ID, "exp": int(time.time()) + 3600} fake_client = MagicMock() @@ -266,7 +269,7 @@ async def test_wrong_org_raises_forbidden_error(self, monkeypatch: pytest.Monkey async def test_internal_org_member_passes(self, monkeypatch: pytest.MonkeyPatch) -> None: """require_internal returns None without raising when user is in the internal org.""" - monkeypatch.setenv(f"{TEST_PROJECT_PREFIX}AUTH_INTERNAL_ORG_ID", _INTERNAL_ORG_ID) + monkeypatch.setenv(INTERNAL_ORG_ID_VAR_NAME, _INTERNAL_ORG_ID) request = MagicMock() user = {"sub": _USER_SUB, "org_id": _INTERNAL_ORG_ID, "exp": int(time.time()) + 3600} fake_client = MagicMock() @@ -283,7 +286,6 @@ class TestRequireInternalAdmin: async def test_unauthenticated_user_raises_forbidden_error(self, monkeypatch: pytest.MonkeyPatch) -> None: """require_internal_admin raises ForbiddenError when no session is available.""" - monkeypatch.setenv("FOUNDRY_AUTH_INTERNAL_ORG_ID", _INTERNAL_ORG_ID) request = MagicMock() request.app.state = MagicMock(spec=[]) # no auth_client → get_user returns None @@ -292,7 +294,6 @@ async def test_unauthenticated_user_raises_forbidden_error(self, monkeypatch: py async def test_wrong_org_raises_forbidden_error(self, monkeypatch: pytest.MonkeyPatch) -> None: """require_internal_admin raises ForbiddenError when user belongs to a different org.""" - monkeypatch.setenv("FOUNDRY_AUTH_INTERNAL_ORG_ID", _INTERNAL_ORG_ID) request = MagicMock() user = {"sub": _USER_SUB, "org_id": _OTHER_ORG_ID, "exp": int(time.time()) + 3600} fake_client = MagicMock() @@ -304,13 +305,11 @@ async def test_wrong_org_raises_forbidden_error(self, monkeypatch: pytest.Monkey async def test_correct_org_wrong_role_raises_forbidden_error(self, monkeypatch: pytest.MonkeyPatch) -> None: """require_internal_admin raises ForbiddenError when user is in internal org but lacks admin role.""" - monkeypatch.setenv(f"{TEST_PROJECT_PREFIX}AUTH_INTERNAL_ORG_ID", _INTERNAL_ORG_ID) - monkeypatch.delenv(f"{TEST_PROJECT_PREFIX}AUTH_AUTH0_ROLE_CLAIM", raising=False) request = MagicMock() user = { "sub": _USER_SUB, "org_id": _INTERNAL_ORG_ID, - DEFAULT_AUTH0_ROLE_CLAIM: "viewer", + _TEST_ROLE_CLAIM: "viewer", "exp": int(time.time()) + 3600, } fake_client = MagicMock() @@ -322,13 +321,11 @@ async def test_correct_org_wrong_role_raises_forbidden_error(self, monkeypatch: async def test_internal_admin_passes(self, monkeypatch: pytest.MonkeyPatch) -> None: """require_internal_admin returns None without raising when user is internal org admin.""" - monkeypatch.setenv(f"{TEST_PROJECT_PREFIX}AUTH_INTERNAL_ORG_ID", _INTERNAL_ORG_ID) - monkeypatch.delenv(f"{TEST_PROJECT_PREFIX}AUTH_AUTH0_ROLE_CLAIM", raising=False) request = MagicMock() user = { "sub": _USER_SUB, "org_id": _INTERNAL_ORG_ID, - DEFAULT_AUTH0_ROLE_CLAIM: AUTH0_ROLE_ADMIN, + _TEST_ROLE_CLAIM: AUTH0_ROLE_ADMIN, "exp": int(time.time()) + 3600, } fake_client = MagicMock() diff --git a/tests/aignostics_foundry_core/gui/gui_test.py b/tests/aignostics_foundry_core/gui/gui_test.py index 47cd9ab..432fc4a 100644 --- a/tests/aignostics_foundry_core/gui/gui_test.py +++ b/tests/aignostics_foundry_core/gui/gui_test.py @@ -1,5 +1,6 @@ """Tests for aignostics_foundry_core.gui.*.""" +import os import sys import time from collections.abc import Generator @@ -23,6 +24,7 @@ NavItem, gui_get_nav_groups, ) +from tests.aignostics_foundry_core.api import AUTH0_ROLE_CLAIM_VAR_NAME, INTERNAL_ORG_ID_VAR_NAME from tests.conftest import TEST_PROJECT_NAME, make_context _PATCH_GET_GUI_USER = "aignostics_foundry_core.gui.auth.get_gui_user" @@ -451,9 +453,13 @@ class TestGetGuiUser: @pytest.fixture(autouse=True) def _gui_context(self) -> Generator[None, None, None]: # pyright: ignore[reportUnusedFunction] - """Install a minimal context so AuthSettings can be loaded.""" + """Install a minimal context and required AuthSettings env vars.""" set_context(make_context()) + os.environ[INTERNAL_ORG_ID_VAR_NAME] = _INTERNAL_ORG + os.environ[AUTH0_ROLE_CLAIM_VAR_NAME] = _ROLE_CLAIM yield + os.environ.pop(INTERNAL_ORG_ID_VAR_NAME, None) + os.environ.pop(AUTH0_ROLE_CLAIM_VAR_NAME, None) reset_context() async def test_returns_none_when_auth_client_raises(self) -> None: @@ -533,9 +539,13 @@ class TestRequireGuiUser: @pytest.fixture(autouse=True) def _gui_context(self) -> Generator[None, None, None]: # pyright: ignore[reportUnusedFunction] - """Install a minimal context so AuthSettings can be loaded.""" + """Install a minimal context and required AuthSettings env vars.""" set_context(make_context()) + os.environ[INTERNAL_ORG_ID_VAR_NAME] = _INTERNAL_ORG + os.environ[AUTH0_ROLE_CLAIM_VAR_NAME] = _ROLE_CLAIM yield + os.environ.pop(INTERNAL_ORG_ID_VAR_NAME, None) + os.environ.pop(AUTH0_ROLE_CLAIM_VAR_NAME, None) reset_context() async def test_redirects_to_login_when_no_user(self) -> None: