Compare commits
11
Commits
659f08e576
...
main
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
3536c40608 | ||
|
|
0c7e1cb04e | ||
|
|
6695215aeb | ||
|
|
d2e0a49e7c | ||
|
|
755aa61107 | ||
|
|
01349a83f2 | ||
|
|
7815e1c231 | ||
|
|
e9fc09af26 | ||
|
|
56b991c965 | ||
|
|
387a3dbb02 | ||
|
|
7f90369621 |
+37
-30
@@ -1,4 +1,7 @@
|
|||||||
{
|
{
|
||||||
|
"env": {
|
||||||
|
"PQL_VAULT": "/mnt/media/Projects/core-api"
|
||||||
|
},
|
||||||
"permissions": {
|
"permissions": {
|
||||||
"allow": [
|
"allow": [
|
||||||
"Bash(pql)",
|
"Bash(pql)",
|
||||||
@@ -16,47 +19,51 @@
|
|||||||
"Bash(curl -s http://localhost:8083/*)"
|
"Bash(curl -s http://localhost:8083/*)"
|
||||||
],
|
],
|
||||||
"deny": [
|
"deny": [
|
||||||
"Bash(sudo *)",
|
"Bash(/mnt/media/Projects/cladmin/ops/bin/toj)",
|
||||||
"Bash(su *)",
|
"Bash(/mnt/media/Projects/cladmin/ops/bin/toj:*)",
|
||||||
"Bash(rm -rf /*)",
|
"Bash(alembic downgrade base*)",
|
||||||
"Bash(rm -rf ~*)",
|
|
||||||
"Bash(rm -rf $HOME*)",
|
|
||||||
"Bash(mkfs*)",
|
|
||||||
"Bash(dd if=*)",
|
|
||||||
"Bash(chmod 777 *)",
|
|
||||||
"Bash(chmod -R 777 *)",
|
"Bash(chmod -R 777 *)",
|
||||||
"Bash(find * -exec*)",
|
"Bash(chmod 777 *)",
|
||||||
|
"Bash(dd if=*)",
|
||||||
"Bash(find * -delete*)",
|
"Bash(find * -delete*)",
|
||||||
"Bash(git add -A*)",
|
"Bash(find * -exec*)",
|
||||||
"Bash(git * add -A*)",
|
|
||||||
"Bash(git add --all*)",
|
|
||||||
"Bash(git * add --all*)",
|
"Bash(git * add --all*)",
|
||||||
"Bash(git add .)",
|
"Bash(git * add -A*)",
|
||||||
"Bash(git * add .)",
|
"Bash(git * add .)",
|
||||||
"Bash(git push --force*)",
|
|
||||||
"Bash(git * push --force*)",
|
|
||||||
"Bash(git push -f*)",
|
|
||||||
"Bash(git * push -f*)",
|
|
||||||
"Bash(git reset --hard*)",
|
|
||||||
"Bash(git * reset --hard*)",
|
|
||||||
"Bash(git clean -fd*)",
|
|
||||||
"Bash(git * clean -fd*)",
|
|
||||||
"Bash(git clean -fdx*)",
|
|
||||||
"Bash(git * clean -fdx*)",
|
|
||||||
"Bash(git branch -D *)",
|
|
||||||
"Bash(git * branch -D *)",
|
"Bash(git * branch -D *)",
|
||||||
"Bash(git checkout -- *)",
|
|
||||||
"Bash(git * checkout -- *)",
|
"Bash(git * checkout -- *)",
|
||||||
"Bash(git restore .*)",
|
"Bash(git * clean -fd*)",
|
||||||
"Bash(git * restore .*)",
|
"Bash(git * clean -fdx*)",
|
||||||
"Bash(git commit --no-verify*)",
|
|
||||||
"Bash(git * commit --no-verify*)",
|
"Bash(git * commit --no-verify*)",
|
||||||
"Bash(git merge --no-ff*)",
|
|
||||||
"Bash(git * merge --no-ff*)",
|
"Bash(git * merge --no-ff*)",
|
||||||
|
"Bash(git * push --force*)",
|
||||||
|
"Bash(git * push -f*)",
|
||||||
|
"Bash(git * reset --hard*)",
|
||||||
|
"Bash(git * restore .*)",
|
||||||
|
"Bash(git add --all*)",
|
||||||
|
"Bash(git add -A*)",
|
||||||
|
"Bash(git add .)",
|
||||||
|
"Bash(git branch -D *)",
|
||||||
|
"Bash(git checkout -- *)",
|
||||||
|
"Bash(git clean -fd*)",
|
||||||
|
"Bash(git clean -fdx*)",
|
||||||
|
"Bash(git commit --no-verify*)",
|
||||||
|
"Bash(git merge --no-ff*)",
|
||||||
|
"Bash(git push --force*)",
|
||||||
|
"Bash(git push -f*)",
|
||||||
|
"Bash(git reset --hard*)",
|
||||||
|
"Bash(git restore .*)",
|
||||||
|
"Bash(mkfs*)",
|
||||||
"Bash(psql * -c DROP*)",
|
"Bash(psql * -c DROP*)",
|
||||||
"Bash(psql * DROP DATABASE*)",
|
"Bash(psql * DROP DATABASE*)",
|
||||||
"Bash(psql * TRUNCATE*)",
|
"Bash(psql * TRUNCATE*)",
|
||||||
"Bash(alembic downgrade base*)"
|
"Bash(rm -rf $HOME)",
|
||||||
|
"Bash(rm -rf /)",
|
||||||
|
"Bash(rm -rf ~)",
|
||||||
|
"Bash(su *)",
|
||||||
|
"Bash(sudo *)",
|
||||||
|
"Bash(toj)",
|
||||||
|
"Bash(toj:*)"
|
||||||
]
|
]
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Executable
+13
@@ -0,0 +1,13 @@
|
|||||||
|
#!/usr/bin/env bash
|
||||||
|
# Trigger only. The checks live in the Makefile, where they can be read, run by
|
||||||
|
# hand (`make pre-push`), and changed under review.
|
||||||
|
#
|
||||||
|
# This file is identical in every repo in this workspace, deliberately: the call
|
||||||
|
# surface is the same everywhere even though what each gate runs is not, so
|
||||||
|
# nobody has to read a repo to find out how to check it (D-27).
|
||||||
|
#
|
||||||
|
# Enable per clone with: git config core.hooksPath .githooks
|
||||||
|
# Never bypass with --no-verify. Suppress a specific finding deliberately
|
||||||
|
# instead, with a reason — see `make pre-push`.
|
||||||
|
set -euo pipefail
|
||||||
|
exec make -C "$(git rev-parse --show-toplevel)" pre-push
|
||||||
@@ -145,3 +145,11 @@ Thumbs.db
|
|||||||
.pql/*
|
.pql/*
|
||||||
!.pql/changelog/
|
!.pql/changelog/
|
||||||
|
|
||||||
|
|
||||||
|
# pql shims planted by `pql init` into the dir core.hooksPath points at.
|
||||||
|
# Per-clone: each embeds the absolute path of the pql binary that planted it.
|
||||||
|
# Only .githooks/pre-push is shared.
|
||||||
|
.githooks/pre-commit
|
||||||
|
.githooks/post-merge
|
||||||
|
.githooks/post-checkout
|
||||||
|
.githooks/post-rewrite
|
||||||
|
|||||||
@@ -119,7 +119,7 @@ before tagging a release.
|
|||||||
|
|
||||||
Work lives in **pql**, not a markdown TODO. **This repo's vault is standalone** — its tickets
|
Work lives in **pql**, not a markdown TODO. **This repo's vault is standalone** — its tickets
|
||||||
and its internal decisions live here in `.pql/` and `governance/`, and travel with a clone,
|
and its internal decisions live here in `.pql/` and `governance/`, and travel with a clone,
|
||||||
because `.pql/changelog/` is committed and replayed by the git hooks (D-15). The databases are
|
because `.pql/changelog/` is committed and replayed by the git hooks (workspace D-15). The databases are
|
||||||
gitignored and rebuildable with `pql plan rebuild`.
|
gitignored and rebuildable with `pql plan rebuild`.
|
||||||
|
|
||||||
`pql` is **not** on the non-interactive `PATH` — invoke it as
|
`pql` is **not** on the non-interactive `PATH` — invoke it as
|
||||||
|
|||||||
@@ -0,0 +1,67 @@
|
|||||||
|
# core-api — the repo's command surface (D-27).
|
||||||
|
#
|
||||||
|
# Paths resolve here rather than in callers (D-10). `python3` on this host is
|
||||||
|
# 3.8 and cannot parse these sources, and a bare `pytest` resolves only in a
|
||||||
|
# login shell — so both are named explicitly through the venv.
|
||||||
|
|
||||||
|
VENV := $(CURDIR)/.venv
|
||||||
|
PYTHON ?= python3.12
|
||||||
|
|
||||||
|
.DEFAULT_GOAL := help
|
||||||
|
|
||||||
|
.PHONY: help
|
||||||
|
help: ## Show this help
|
||||||
|
@grep -hE '^[a-z][a-z0-9_-]*:.*?## ' $(MAKEFILE_LIST) \
|
||||||
|
| awk 'BEGIN{FS=":.*?## "}{printf " \033[36m%-14s\033[0m %s\n", $$1, $$2}'
|
||||||
|
|
||||||
|
.PHONY: setup
|
||||||
|
setup: ## Create the venv, install dependencies, and prove the result is usable
|
||||||
|
$(PYTHON) -m venv .venv
|
||||||
|
$(VENV)/bin/pip install -r requirements.txt -r dev-requirements.txt
|
||||||
|
# Exit 0 from pip install is not evidence (D-24) — it is the step's job not
|
||||||
|
# to fail, so a broken result and a working one look identical from here.
|
||||||
|
# On 2026-08-09 the venv existed and pip had exited 0, but sqlalchemy was
|
||||||
|
# declared in requirements.txt and not installed; that surfaced as 11
|
||||||
|
# pytest collection errors that read as broken imports, not as a setup
|
||||||
|
# problem. `--collect-only` exercises every import the suite touches
|
||||||
|
# without running a single test, so it catches exactly that class of
|
||||||
|
# drift and stays cheap. `pip check` was considered too, but it only
|
||||||
|
# verifies the *installed* set's internal consistency against itself —
|
||||||
|
# it would not have caught this case, because sqlalchemy was still
|
||||||
|
# present as another package's transitive dependency even when dropped
|
||||||
|
# from requirements.txt. collect-only checks declared-vs-actually-usable
|
||||||
|
# directly, which is the axis that broke.
|
||||||
|
$(VENV)/bin/python -m pytest --collect-only tests/
|
||||||
|
|
||||||
|
.PHONY: test
|
||||||
|
test: ## Run the test suite
|
||||||
|
@test -x $(VENV)/bin/python || { echo "FAIL — no venv; run: make setup"; exit 69; }
|
||||||
|
$(VENV)/bin/python -m pytest tests/
|
||||||
|
|
||||||
|
# No `lint` target, deliberately. This repo configures no linter — no ruff or
|
||||||
|
# flake8 config, and neither in requirements. Per D-27 the name is reserved for
|
||||||
|
# repos that lint; an empty target here would report clean for something never
|
||||||
|
# run. Add the target when a linter is added, not before.
|
||||||
|
|
||||||
|
# git hands a hook a non-login shell, which never sees ~/.local/bin — where
|
||||||
|
# gitleaks lands. Without this the scan reports "not installed" on every push,
|
||||||
|
# which is a check that fails open (D-24).
|
||||||
|
export PATH := $(HOME)/.local/bin:/usr/local/bin:$(PATH)
|
||||||
|
|
||||||
|
.PHONY: secrets
|
||||||
|
secrets: ## Scan the commits about to be pushed for credentials
|
||||||
|
@ci/secrets.sh
|
||||||
|
|
||||||
|
# The call surface is identical in every repo; what it runs is not.
|
||||||
|
#
|
||||||
|
# `secrets` runs first, deliberately: it is the only failure here that cannot be
|
||||||
|
# undone by fixing it afterwards. A failed lint costs another commit; a pushed
|
||||||
|
# credential is cached and indexed whether or not it is later deleted.
|
||||||
|
#
|
||||||
|
# Some of these fail today, and are left wired anyway. The state was measured
|
||||||
|
# once and written down in T-56 rather than being worked around here — a gate
|
||||||
|
# quietly narrowed to what already passes is a gate that reports success for
|
||||||
|
# doing nothing, which is the failure this workspace keeps rediscovering.
|
||||||
|
.PHONY: pre-push
|
||||||
|
pre-push: secrets ## Everything the pre-push hook runs
|
||||||
|
@echo " -- not gated here yet: lint (no linter configured) and test (T-56)"
|
||||||
Executable
+50
@@ -0,0 +1,50 @@
|
|||||||
|
#!/usr/bin/env bash
|
||||||
|
# Secret scan over the commits about to be pushed.
|
||||||
|
#
|
||||||
|
# Lives here rather than inside .githooks/pre-push so it can be read, run by
|
||||||
|
# hand (`make secrets`), and changed under review. A hook is a trigger; it is
|
||||||
|
# not a home for logic. Identical in every repo in this workspace (D-27).
|
||||||
|
set -euo pipefail
|
||||||
|
|
||||||
|
cd "$(git rev-parse --show-toplevel)"
|
||||||
|
|
||||||
|
# A non-login shell — which is what git gives a hook — skips /etc/profile.d
|
||||||
|
# and never sees ~/.local/bin, where the gitleaks release tarball lands.
|
||||||
|
# Without this the scan reports "not installed" on every push.
|
||||||
|
[ -d "$HOME/.local/bin" ] && PATH="$HOME/.local/bin:$PATH"
|
||||||
|
|
||||||
|
if ! command -v gitleaks >/dev/null 2>&1; then
|
||||||
|
echo "FAIL secrets — gitleaks not installed, so this check would be a no-op pretending to pass." >&2
|
||||||
|
echo " https://github.com/gitleaks/gitleaks/releases → ~/.local/bin/gitleaks" >&2
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
|
# Scan the outgoing range, not full history. History here carries findings
|
||||||
|
# that are settled — test fixtures and vendored third-party code — and a gate
|
||||||
|
# that fails on something unfixable gets bypassed within a week. What matters
|
||||||
|
# is what is about to leave this machine.
|
||||||
|
if upstream=$(git rev-parse --abbrev-ref --symbolic-full-name '@{u}' 2>/dev/null); then
|
||||||
|
range="$upstream..HEAD"
|
||||||
|
elif git rev-parse --verify --quiet origin/main >/dev/null; then
|
||||||
|
range="origin/main..HEAD"
|
||||||
|
else
|
||||||
|
range=""
|
||||||
|
fi
|
||||||
|
|
||||||
|
if [ -z "$range" ]; then
|
||||||
|
gitleaks dir . --redact --no-banner --exit-code 1 || {
|
||||||
|
echo "FAIL secrets — gitleaks found a credential in the working tree." >&2; exit 1; }
|
||||||
|
exit 0
|
||||||
|
fi
|
||||||
|
|
||||||
|
[ -n "$(git log --oneline "$range" 2>/dev/null)" ] || exit 0
|
||||||
|
|
||||||
|
gitleaks git . --log-opts="$range" --redact --no-banner --exit-code 1 >/dev/null 2>&1 || {
|
||||||
|
echo "FAIL secrets — gitleaks found a credential in the commits being pushed." >&2
|
||||||
|
echo " inspect (values redacted): gitleaks git . --log-opts=\"$range\" --redact" >&2
|
||||||
|
echo " then remove and rotate it, or suppress deliberately:" >&2
|
||||||
|
echo " inline '# gitleaks:allow <reason>'" >&2
|
||||||
|
echo " or add the fingerprint to .gitleaksignore WITH a reason" >&2
|
||||||
|
exit 1
|
||||||
|
}
|
||||||
|
echo " ok secrets"
|
||||||
@@ -815,22 +815,32 @@ class TestPhase4Schemas:
|
|||||||
|
|
||||||
|
|
||||||
# =============================================================================
|
# =============================================================================
|
||||||
# GET /auth/me Endpoint Tests (NPM Forward Auth)
|
# GET /auth/users/me Endpoint Tests (NPM Forward Auth)
|
||||||
# =============================================================================
|
# =============================================================================
|
||||||
|
|
||||||
class TestAuthMeEndpoint:
|
class TestAuthMeEndpoint:
|
||||||
"""Test GET /auth/me endpoint with NPM forward auth."""
|
"""
|
||||||
|
Test GET /auth/users/me with NPM forward auth.
|
||||||
|
|
||||||
|
The path is /auth/users/me, not /auth/me — the route is declared as "/me"
|
||||||
|
inside AuthController.create_router(), which mounts under a users prefix.
|
||||||
|
These tests asserted /auth/me and had never passed; the generated spec is
|
||||||
|
authoritative and both the local app and the deployed service agree on 62
|
||||||
|
paths including this one.
|
||||||
|
"""
|
||||||
|
|
||||||
|
PATH = "/auth/users/me"
|
||||||
|
|
||||||
def test_auth_me_in_openapi(self, client):
|
def test_auth_me_in_openapi(self, client):
|
||||||
"""Auth me endpoint should be in OpenAPI spec."""
|
"""Auth me endpoint should be in OpenAPI spec."""
|
||||||
response = client.get("/openapi.json")
|
response = client.get("/openapi.json")
|
||||||
spec = response.json()
|
spec = response.json()
|
||||||
assert "/auth/me" in spec["paths"]
|
assert self.PATH in spec["paths"]
|
||||||
assert "get" in spec["paths"]["/auth/me"]
|
assert "get" in spec["paths"][self.PATH]
|
||||||
|
|
||||||
def test_auth_me_returns_401_without_forward_auth(self, client):
|
def test_auth_me_returns_401_without_forward_auth(self, client):
|
||||||
"""Should return 401 when accessed without forward auth headers."""
|
"""Should return 401 when accessed without forward auth headers."""
|
||||||
response = client.get("/auth/me")
|
response = client.get(self.PATH)
|
||||||
# Without NPM forward auth headers, should return 401
|
# Without NPM forward auth headers, should return 401
|
||||||
assert response.status_code == 401
|
assert response.status_code == 401
|
||||||
|
|
||||||
@@ -840,7 +850,7 @@ class TestAuthMeEndpoint:
|
|||||||
spec = response.json()
|
spec = response.json()
|
||||||
|
|
||||||
# Check response schema references AuthSyncResponse
|
# Check response schema references AuthSyncResponse
|
||||||
me_endpoint = spec["paths"]["/auth/me"]["get"]
|
me_endpoint = spec["paths"][self.PATH]["get"]
|
||||||
assert "responses" in me_endpoint
|
assert "responses" in me_endpoint
|
||||||
assert "200" in me_endpoint["responses"]
|
assert "200" in me_endpoint["responses"]
|
||||||
|
|
||||||
@@ -920,7 +930,11 @@ class TestForwardAuthParsing:
|
|||||||
|
|
||||||
result = await get_forward_auth_user(mock_request)
|
result = await get_forward_auth_user(mock_request)
|
||||||
|
|
||||||
assert result["groups"] == [""]
|
# An empty groups header means no groups, not one group named "".
|
||||||
|
# oidc.py has guarded this since the initial commit — this assertion
|
||||||
|
# expected [""] and had never passed. [""] would also be unsafe: any
|
||||||
|
# authorization check doing `"" in groups` would match.
|
||||||
|
assert result["groups"] == []
|
||||||
assert result["name"] == "jdoe" # Falls back to username
|
assert result["name"] == "jdoe" # Falls back to username
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
|
|||||||
@@ -89,10 +89,6 @@ class TestGetSettings:
|
|||||||
settings2 = get_settings()
|
settings2 = get_settings()
|
||||||
assert settings1 is settings2
|
assert settings1 is settings2
|
||||||
|
|
||||||
def test_model_aliases_property(self):
|
# Removed: test_model_aliases_property. Settings.model_aliases mapped
|
||||||
"""Model aliases property should return dict."""
|
# gpt-3.5-turbo and gpt-4 onto local models, and was deleted along with the
|
||||||
settings = get_settings()
|
# Ollama integration in c1f16d4. The test outlived the feature it covered.
|
||||||
aliases = settings.model_aliases
|
|
||||||
assert isinstance(aliases, dict)
|
|
||||||
assert "gpt-3.5-turbo" in aliases
|
|
||||||
assert "gpt-4" in aliases
|
|
||||||
|
|||||||
+114
-62
@@ -1,118 +1,173 @@
|
|||||||
"""Tests for OIDC authentication module."""
|
"""Tests for OIDC authentication module."""
|
||||||
import pytest
|
import pytest
|
||||||
from unittest.mock import patch, MagicMock, AsyncMock
|
from unittest.mock import patch, MagicMock
|
||||||
from fastapi import HTTPException
|
from fastapi import HTTPException
|
||||||
|
|
||||||
from src.auth.oidc import OIDCConfig, oidc_config, get_jwks, get_current_user
|
from src.auth.oidc import (
|
||||||
|
OIDCConfig,
|
||||||
|
oidc_config,
|
||||||
|
get_jwks_for_issuer,
|
||||||
|
get_current_user,
|
||||||
|
_jwks_cache,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture(autouse=True)
|
||||||
|
def clear_jwks_cache():
|
||||||
|
"""
|
||||||
|
The JWKS cache is module-level state, so a fetch in one test would satisfy
|
||||||
|
the next one and hide a regression. Clearing on both sides keeps the tests
|
||||||
|
order-independent.
|
||||||
|
"""
|
||||||
|
_jwks_cache.clear()
|
||||||
|
yield
|
||||||
|
_jwks_cache.clear()
|
||||||
|
|
||||||
|
|
||||||
class TestOIDCConfig:
|
class TestOIDCConfig:
|
||||||
"""Test OIDCConfig class."""
|
"""Test OIDCConfig class."""
|
||||||
|
|
||||||
def test_init_defaults(self):
|
def test_init_defaults(self):
|
||||||
"""Config should initialize with disabled state."""
|
"""Config should initialize disabled with no issuers or audiences."""
|
||||||
config = OIDCConfig()
|
config = OIDCConfig()
|
||||||
|
|
||||||
assert config.enabled is False
|
assert config.enabled is False
|
||||||
assert config.issuer == ""
|
assert config.issuers == []
|
||||||
assert config.audience == ""
|
assert config.audiences == []
|
||||||
assert config.jwks_uri == ""
|
|
||||||
|
|
||||||
def test_configure_sets_values(self):
|
def test_configure_sets_values(self):
|
||||||
"""configure should set all values."""
|
"""configure should set all values."""
|
||||||
config = OIDCConfig()
|
config = OIDCConfig()
|
||||||
config.configure(
|
config.configure(
|
||||||
enabled=True,
|
enabled=True,
|
||||||
issuer="https://auth.example.com",
|
issuers=["https://auth.example.com"],
|
||||||
audience="core-api"
|
audiences=["core-api"],
|
||||||
)
|
)
|
||||||
|
|
||||||
assert config.enabled is True
|
assert config.enabled is True
|
||||||
assert config.issuer == "https://auth.example.com"
|
assert config.issuers == ["https://auth.example.com"]
|
||||||
assert config.audience == "core-api"
|
assert config.audiences == ["core-api"]
|
||||||
assert config.jwks_uri == "https://auth.example.com/jwks/"
|
|
||||||
|
|
||||||
def test_configure_strips_trailing_slash(self):
|
def test_configure_strips_trailing_slash(self):
|
||||||
"""configure should handle trailing slash in issuer."""
|
"""configure should normalise issuers by dropping the trailing slash."""
|
||||||
config = OIDCConfig()
|
config = OIDCConfig()
|
||||||
config.configure(
|
config.configure(
|
||||||
enabled=True,
|
enabled=True,
|
||||||
issuer="https://auth.example.com/",
|
issuers=["https://auth.example.com/"],
|
||||||
audience="core-api"
|
audiences=["core-api"],
|
||||||
)
|
)
|
||||||
|
|
||||||
assert config.jwks_uri == "https://auth.example.com/jwks/"
|
assert config.issuers == ["https://auth.example.com"]
|
||||||
|
|
||||||
|
def test_configure_accepts_multiple_issuers(self):
|
||||||
|
"""The point of the multi-issuer change: more than one is allowed."""
|
||||||
|
config = OIDCConfig()
|
||||||
|
config.configure(
|
||||||
|
enabled=True,
|
||||||
|
issuers=["https://a.example.com/", "https://b.example.com"],
|
||||||
|
audiences=["core-api", "other"],
|
||||||
|
)
|
||||||
|
|
||||||
|
assert config.issuers == ["https://a.example.com", "https://b.example.com"]
|
||||||
|
assert config.audiences == ["core-api", "other"]
|
||||||
|
|
||||||
|
def test_get_jwks_uri_derives_from_issuer(self):
|
||||||
|
"""The JWKS URI is derived per issuer rather than configured."""
|
||||||
|
config = OIDCConfig()
|
||||||
|
|
||||||
|
assert config.get_jwks_uri("https://auth.example.com") == "https://auth.example.com/jwks/"
|
||||||
|
assert config.get_jwks_uri("https://auth.example.com/") == "https://auth.example.com/jwks/"
|
||||||
|
|
||||||
|
def test_is_valid_issuer_only_accepts_configured(self):
|
||||||
|
"""
|
||||||
|
An unconfigured issuer must be rejected. This is the security-relevant
|
||||||
|
half of multi-issuer support: accepting any issuer would let a token
|
||||||
|
from an unrelated identity provider through.
|
||||||
|
"""
|
||||||
|
config = OIDCConfig()
|
||||||
|
config.configure(
|
||||||
|
enabled=True,
|
||||||
|
issuers=["https://auth.example.com"],
|
||||||
|
audiences=["core-api"],
|
||||||
|
)
|
||||||
|
|
||||||
|
assert config.is_valid_issuer("https://auth.example.com") is True
|
||||||
|
assert config.is_valid_issuer("https://auth.example.com/") is True
|
||||||
|
assert config.is_valid_issuer("https://evil.example.com") is False
|
||||||
|
|
||||||
|
|
||||||
class TestGetJWKS:
|
class TestGetJWKSForIssuer:
|
||||||
"""Test get_jwks function."""
|
"""Test get_jwks_for_issuer function."""
|
||||||
|
|
||||||
|
ISSUER = "https://auth.example.com"
|
||||||
|
|
||||||
def test_returns_empty_when_disabled(self):
|
def test_returns_empty_when_disabled(self):
|
||||||
"""get_jwks should return empty dict when OIDC disabled."""
|
"""Should return an empty dict when OIDC is disabled."""
|
||||||
# Save original state
|
|
||||||
original_enabled = oidc_config.enabled
|
original_enabled = oidc_config.enabled
|
||||||
|
|
||||||
try:
|
try:
|
||||||
oidc_config.enabled = False
|
oidc_config.enabled = False
|
||||||
# Clear the cache
|
|
||||||
get_jwks.cache_clear()
|
|
||||||
|
|
||||||
result = get_jwks()
|
assert get_jwks_for_issuer(self.ISSUER) == {}
|
||||||
|
|
||||||
assert result == {}
|
|
||||||
finally:
|
finally:
|
||||||
# Restore original state
|
|
||||||
oidc_config.enabled = original_enabled
|
oidc_config.enabled = original_enabled
|
||||||
get_jwks.cache_clear()
|
|
||||||
|
|
||||||
@patch("src.auth.oidc.httpx.get")
|
@patch("src.auth.oidc.httpx.get")
|
||||||
def test_fetches_jwks_when_enabled(self, mock_get):
|
def test_fetches_jwks_when_enabled(self, mock_get):
|
||||||
"""get_jwks should fetch JWKS when enabled."""
|
"""Should fetch from the issuer's derived JWKS URI."""
|
||||||
# Save original state
|
|
||||||
original_enabled = oidc_config.enabled
|
original_enabled = oidc_config.enabled
|
||||||
original_jwks_uri = oidc_config.jwks_uri
|
|
||||||
|
|
||||||
try:
|
try:
|
||||||
oidc_config.enabled = True
|
oidc_config.enabled = True
|
||||||
oidc_config.jwks_uri = "https://auth.example.com/jwks/"
|
|
||||||
get_jwks.cache_clear()
|
|
||||||
|
|
||||||
mock_response = MagicMock()
|
mock_response = MagicMock()
|
||||||
mock_response.json.return_value = {"keys": [{"kid": "test"}]}
|
mock_response.json.return_value = {"keys": [{"kid": "abc"}]}
|
||||||
mock_response.raise_for_status = MagicMock()
|
|
||||||
mock_get.return_value = mock_response
|
mock_get.return_value = mock_response
|
||||||
|
|
||||||
result = get_jwks()
|
result = get_jwks_for_issuer(self.ISSUER)
|
||||||
|
|
||||||
assert "keys" in result
|
assert result == {"keys": [{"kid": "abc"}]}
|
||||||
mock_get.assert_called_once()
|
mock_get.assert_called_once()
|
||||||
|
assert mock_get.call_args[0][0] == f"{self.ISSUER}/jwks/"
|
||||||
finally:
|
finally:
|
||||||
oidc_config.enabled = original_enabled
|
oidc_config.enabled = original_enabled
|
||||||
oidc_config.jwks_uri = original_jwks_uri
|
|
||||||
get_jwks.cache_clear()
|
|
||||||
|
|
||||||
@patch("src.auth.oidc.httpx.get")
|
@patch("src.auth.oidc.httpx.get")
|
||||||
def test_raises_exception_on_error(self, mock_get):
|
def test_caches_per_issuer(self, mock_get):
|
||||||
"""get_jwks should raise HTTPException on fetch error."""
|
"""
|
||||||
# Save original state
|
A second call for the same issuer must not refetch, and a different
|
||||||
|
issuer must. Caching by issuer is the behaviour the multi-issuer change
|
||||||
|
introduced, and a shared cache would have served one issuer's keys for
|
||||||
|
another — which would be a verification bypass, not just a slow path.
|
||||||
|
"""
|
||||||
original_enabled = oidc_config.enabled
|
original_enabled = oidc_config.enabled
|
||||||
original_jwks_uri = oidc_config.jwks_uri
|
|
||||||
|
|
||||||
try:
|
try:
|
||||||
oidc_config.enabled = True
|
oidc_config.enabled = True
|
||||||
oidc_config.jwks_uri = "https://auth.example.com/jwks/"
|
mock_response = MagicMock()
|
||||||
get_jwks.cache_clear()
|
mock_response.json.return_value = {"keys": []}
|
||||||
|
mock_get.return_value = mock_response
|
||||||
|
|
||||||
mock_get.side_effect = Exception("Connection error")
|
get_jwks_for_issuer(self.ISSUER)
|
||||||
|
get_jwks_for_issuer(self.ISSUER + "/") # same issuer, normalised
|
||||||
|
assert mock_get.call_count == 1
|
||||||
|
|
||||||
|
get_jwks_for_issuer("https://other.example.com")
|
||||||
|
assert mock_get.call_count == 2
|
||||||
|
finally:
|
||||||
|
oidc_config.enabled = original_enabled
|
||||||
|
|
||||||
|
@patch("src.auth.oidc.httpx.get")
|
||||||
|
def test_raises_503_on_fetch_error(self, mock_get):
|
||||||
|
"""A JWKS fetch failure should surface as 503, not leak the cause."""
|
||||||
|
original_enabled = oidc_config.enabled
|
||||||
|
try:
|
||||||
|
oidc_config.enabled = True
|
||||||
|
mock_get.side_effect = Exception("Connection failed")
|
||||||
|
|
||||||
with pytest.raises(HTTPException) as exc_info:
|
with pytest.raises(HTTPException) as exc_info:
|
||||||
get_jwks()
|
get_jwks_for_issuer(self.ISSUER)
|
||||||
|
|
||||||
assert exc_info.value.status_code == 503
|
assert exc_info.value.status_code == 503
|
||||||
finally:
|
finally:
|
||||||
oidc_config.enabled = original_enabled
|
oidc_config.enabled = original_enabled
|
||||||
oidc_config.jwks_uri = original_jwks_uri
|
|
||||||
get_jwks.cache_clear()
|
|
||||||
|
|
||||||
|
|
||||||
class TestGetCurrentUser:
|
class TestGetCurrentUser:
|
||||||
@@ -121,9 +176,7 @@ class TestGetCurrentUser:
|
|||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_returns_none_when_disabled(self):
|
async def test_returns_none_when_disabled(self):
|
||||||
"""get_current_user should return None when OIDC disabled."""
|
"""get_current_user should return None when OIDC disabled."""
|
||||||
# Save original state
|
|
||||||
original_enabled = oidc_config.enabled
|
original_enabled = oidc_config.enabled
|
||||||
|
|
||||||
try:
|
try:
|
||||||
oidc_config.enabled = False
|
oidc_config.enabled = False
|
||||||
|
|
||||||
@@ -136,9 +189,7 @@ class TestGetCurrentUser:
|
|||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_raises_401_when_enabled_without_token(self):
|
async def test_raises_401_when_enabled_without_token(self):
|
||||||
"""get_current_user should raise 401 when enabled but no token."""
|
"""get_current_user should raise 401 when enabled but no token."""
|
||||||
# Save original state
|
|
||||||
original_enabled = oidc_config.enabled
|
original_enabled = oidc_config.enabled
|
||||||
|
|
||||||
try:
|
try:
|
||||||
oidc_config.enabled = True
|
oidc_config.enabled = True
|
||||||
|
|
||||||
@@ -157,11 +208,12 @@ class TestOIDCGlobalConfig:
|
|||||||
"""oidc_config should be an OIDCConfig instance."""
|
"""oidc_config should be an OIDCConfig instance."""
|
||||||
assert isinstance(oidc_config, OIDCConfig)
|
assert isinstance(oidc_config, OIDCConfig)
|
||||||
|
|
||||||
def test_global_config_starts_disabled(self):
|
def test_global_config_exposes_the_multi_issuer_surface(self):
|
||||||
"""oidc_config should start disabled by default."""
|
"""
|
||||||
# This tests the initial state before any configure() is called
|
Asserts the shape rather than the values, since the live state depends
|
||||||
# The actual state depends on app configuration
|
on app configuration. These four are what callers depend on.
|
||||||
assert hasattr(oidc_config, 'enabled')
|
"""
|
||||||
assert hasattr(oidc_config, 'issuer')
|
assert hasattr(oidc_config, "enabled")
|
||||||
assert hasattr(oidc_config, 'audience')
|
assert hasattr(oidc_config, "issuers")
|
||||||
assert hasattr(oidc_config, 'jwks_uri')
|
assert hasattr(oidc_config, "audiences")
|
||||||
|
assert callable(oidc_config.get_jwks_uri)
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ from fastapi.testclient import TestClient
|
|||||||
from unittest.mock import patch, AsyncMock, MagicMock
|
from unittest.mock import patch, AsyncMock, MagicMock
|
||||||
|
|
||||||
from src.main import app
|
from src.main import app
|
||||||
|
from src.domains.tools.controller import tools_controller
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
@@ -36,9 +37,21 @@ class TestDNSLookup:
|
|||||||
)
|
)
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
|
|
||||||
@patch("src.controllers.tools_controller.DNSService")
|
def test_dns_lookup_returns_result(self, client):
|
||||||
def test_dns_lookup_returns_result(self, mock_dns_class, client):
|
"""DNS lookup should return lookup results.
|
||||||
"""DNS lookup should return lookup results."""
|
|
||||||
|
Patches the live singleton's `dns_service` attribute, not the
|
||||||
|
`src.controllers.tools_controller.DNSService` class: that module is
|
||||||
|
the legacy top-level package (not wired into `src.main`, see
|
||||||
|
CLAUDE.md "Legacy top-level packages"). `client` exercises
|
||||||
|
`src.main.app`, which routes through
|
||||||
|
`src.domains.tools.controller.tools_controller`, a singleton built
|
||||||
|
at import time — so patching the class there would also miss,
|
||||||
|
since `tools_controller.dns_service` is already a constructed
|
||||||
|
instance by the time a test patches the class. Patching the
|
||||||
|
instance attribute directly is the only patch that actually
|
||||||
|
intercepts this request path.
|
||||||
|
"""
|
||||||
mock_response = MagicMock()
|
mock_response = MagicMock()
|
||||||
mock_response.success = True
|
mock_response.success = True
|
||||||
mock_response.domain = "example.com"
|
mock_response.domain = "example.com"
|
||||||
@@ -59,8 +72,8 @@ class TestDNSLookup:
|
|||||||
|
|
||||||
mock_service = MagicMock()
|
mock_service = MagicMock()
|
||||||
mock_service.lookup = AsyncMock(return_value=mock_response)
|
mock_service.lookup = AsyncMock(return_value=mock_response)
|
||||||
mock_dns_class.return_value = mock_service
|
|
||||||
|
|
||||||
|
with patch.object(tools_controller, "dns_service", mock_service):
|
||||||
response = client.post(
|
response = client.post(
|
||||||
"/tools/dns/lookup",
|
"/tools/dns/lookup",
|
||||||
json={"domain": "example.com", "record_type": "A"}
|
json={"domain": "example.com", "record_type": "A"}
|
||||||
|
|||||||
Reference in New Issue
Block a user