Skip to content

Keep APIKeyAuth fail-closed after revoking the last key #183

Description

@Justinabox

Summary

APIKeyAuth.revoke_key() disables the authentication middleware when the last configured key is removed. After that, the middleware returns await handler(request) for every request, so a runtime key-revocation flow can turn a previously protected HTTP API into an unauthenticated one.

This is a fail-open security boundary: revoking the last known/compromised key should leave SMS/USSD endpoints closed until a replacement key is added, not open them to requests with no bearer token.

Evidence

Baseline gates are healthy on main at d5b7c6f8248c078305122acc1a00529d76253781:

$ git diff --check
# exit 0

$ PYTHONPATH=. uv run --no-project --with pytest --with pytest-asyncio --with pytest-aiohttp --with pyserial-asyncio --with aiosqlite pytest tests/ -q
698 passed in 6.28s

Affected code:

  • server.py:53-59 stores configured keys and sets self.enabled = bool(self._keys).
  • server.py:61-75 bypasses the handler entirely when self.enabled is false.
  • server.py:110-112 makes revoke_key() set enabled = bool(self._keys), so revoking the last key disables the middleware.
  • tests/test_api_auth.py:150-154 currently codifies this fail-open state by asserting auth.enabled is False after revoking only-key.

No-hardware aiohttp repro:

PYTHONPATH=. uv run --no-project --with aiohttp --with pytest-aiohttp --with pyserial-asyncio python - <<'PY'
import asyncio
from aiohttp import web
from aiohttp.test_utils import TestClient, TestServer
from server import APIKeyAuth

async def main():
    auth = APIKeyAuth(api_keys=['valid-key'])
    app = web.Application(middlewares=[auth.middleware])
    app.router.add_get('/private', lambda request: web.json_response({'ok': True}))
    server = TestServer(app)
    client = TestClient(server)
    await client.start_server()
    try:
        before = await client.get('/private')
        print('before_revoke_no_auth=', before.status, await before.text())
        auth.revoke_key('valid-key')
        print('auth_enabled_after_last_revoke=', auth.enabled)
        after_no_auth = await client.get('/private')
        print('after_revoke_no_auth=', after_no_auth.status, await after_no_auth.text())
        after_wrong = await client.get('/private', headers={'Authorization':'Bearer wrong-key'})
        print('after_revoke_wrong_auth=', after_wrong.status, await after_wrong.text())
    finally:
        await client.close()
asyncio.run(main())
PY

Observed output:

before_revoke_no_auth= 401 {"error": "Missing or invalid Authorization header. Use 'Bearer <api_key>'."}
auth_enabled_after_last_revoke= False
after_revoke_no_auth= 200 {"ok": true}
after_revoke_wrong_auth= 200 {"ok": true}

Duplicate check

Related but distinct existing work:

Searches run before filing found no issue/PR for runtime last-key revocation fail-open behavior:

gh issue list --repo Justinabox/Callstack --state all --search 'revoke API key disables auth fail open in:title,body' --limit 20
gh issue list --repo Justinabox/Callstack --state all --search 'key revocation fail open auth in:title,body' --limit 20
gh issue list --repo Justinabox/Callstack --state all --search 'last API key revoked unauthenticated in:title,body' --limit 20
gh issue list --repo Justinabox/Callstack --state all --search 'auth enabled false revoke_key in:title,body' --limit 20
gh pr list --repo Justinabox/Callstack --state all --search 'revoke API key auth fail open' --limit 20
gh pr list --repo Justinabox/Callstack --state all --search 'last API key revoked' --limit 20

Expected behavior

Once an APIKeyAuth instance has been configured to protect an app, revoking keys should never make the middleware permissive. If the key set becomes empty through revocation, requests should fail closed with a 401/403/503-style auth/config error until a new key is added or an explicit development-only unauthenticated mode is selected at startup.

Suggested fix direction

Separate "middleware installed in protected mode" from "current valid key set". For example:

  • Track a private _protected / _ever_configured / policy flag that remains true after the first nonblank key is configured.
  • Keep constructor behavior for intentionally unauthenticated test/dev apps if needed, but do not let revoke_key() move a protected app back into bypass mode.
  • Add regression coverage for last-key revocation through the middleware, not only the enabled property.

Acceptance criteria

  • Revoking one key from a multi-key auth instance invalidates that key while preserving access for remaining keys.
  • Revoking the last key in a previously protected auth instance causes missing/invalid bearer requests to fail closed, not reach the handler.
  • Adding a replacement key after last-key revocation restores access for that key only.
  • Existing blank-key, non-ASCII bearer, invalid-key throttling, and startup-auth tests remain compatible with the chosen policy.

Verification gates

git diff --check
PYTHONPATH=. uv run --no-project --with pytest --with pytest-asyncio --with pytest-aiohttp --with pyserial-asyncio --with aiosqlite pytest tests/test_api_auth.py -q
PYTHONPATH=. uv run --no-project --with pytest --with pytest-asyncio --with pytest-aiohttp --with pyserial-asyncio --with aiosqlite pytest tests/ -q

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions