From f157a671839bbbe426b64b43306b64008c37ac1b Mon Sep 17 00:00:00 2001 From: Lars Kappert <9989650+yigitkonur@users.noreply.github.com> Date: Sun, 22 Mar 2026 07:49:28 -0700 Subject: [PATCH] fix(api-keys): normalize timezone-aware expirations before persistence --- app/modules/api_keys/service.py | 14 ++++-- .../proposal.md | 11 +++++ .../specs/api-keys/spec.md | 15 +++++++ .../fix-api-key-expiration-timezone/tasks.md | 3 ++ openspec/specs/api-keys/spec.md | 17 +++++-- tests/unit/test_api_keys_service.py | 44 ++++++++++++++++++- 6 files changed, 97 insertions(+), 7 deletions(-) create mode 100644 openspec/changes/fix-api-key-expiration-timezone/proposal.md create mode 100644 openspec/changes/fix-api-key-expiration-timezone/specs/api-keys/spec.md create mode 100644 openspec/changes/fix-api-key-expiration-timezone/tasks.md diff --git a/app/modules/api_keys/service.py b/app/modules/api_keys/service.py index e7b43ddc..2fd3731f 100644 --- a/app/modules/api_keys/service.py +++ b/app/modules/api_keys/service.py @@ -13,7 +13,7 @@ calculate_cost_from_usage, get_pricing_for_model, ) -from app.core.utils.time import utcnow +from app.core.utils.time import to_utc_naive, utcnow from app.db.models import ApiKey, ApiKeyLimit, LimitType, LimitWindow from app.modules.api_keys.repository import ( _UNSET, @@ -234,6 +234,7 @@ def __init__(self, repository: ApiKeysRepositoryProtocol) -> None: async def create_key(self, payload: ApiKeyCreateData) -> ApiKeyCreatedData: now = utcnow() + expires_at = _normalize_expires_at(payload.expires_at) plain_key = _generate_plain_key() normalized_allowed_models = _normalize_allowed_models(payload.allowed_models) enforced_model = _normalize_model_slug(payload.enforced_model) @@ -247,7 +248,7 @@ async def create_key(self, payload: ApiKeyCreateData) -> ApiKeyCreatedData: allowed_models=_serialize_allowed_models(normalized_allowed_models), enforced_model=enforced_model, enforced_reasoning_effort=enforced_reasoning_effort, - expires_at=payload.expires_at, + expires_at=expires_at, is_active=True, created_at=now, last_used_at=None, @@ -273,6 +274,7 @@ async def list_keys(self) -> list[ApiKeyData]: ] async def update_key(self, key_id: str, payload: ApiKeyUpdateData) -> ApiKeyData: + expires_at = _normalize_expires_at(payload.expires_at) if payload.expires_at_set else None if payload.allowed_models_set: allowed_models = _normalize_allowed_models(payload.allowed_models) else: @@ -309,7 +311,7 @@ async def update_key(self, key_id: str, payload: ApiKeyUpdateData) -> ApiKeyData allowed_models=_serialize_allowed_models(allowed_models) if payload.allowed_models_set else _UNSET, enforced_model=enforced_model if payload.enforced_model_set else _UNSET, enforced_reasoning_effort=(enforced_reasoning_effort if payload.enforced_reasoning_effort_set else _UNSET), - expires_at=payload.expires_at if payload.expires_at_set else _UNSET, + expires_at=expires_at if payload.expires_at_set else _UNSET, is_active=(payload.is_active if payload.is_active_set and payload.is_active is not None else _UNSET), ) if row is None: @@ -670,6 +672,12 @@ def _normalize_model_slug(value: str | None) -> str | None: _SUPPORTED_REASONING_EFFORTS = frozenset({"none", "minimal", "low", "medium", "high", "xhigh"}) +def _normalize_expires_at(value: datetime | None) -> datetime | None: + if value is None: + return None + return to_utc_naive(value) + + def _normalize_reasoning_effort(value: str | None) -> str | None: if value is None: return None diff --git a/openspec/changes/fix-api-key-expiration-timezone/proposal.md b/openspec/changes/fix-api-key-expiration-timezone/proposal.md new file mode 100644 index 00000000..b9c4d7fd --- /dev/null +++ b/openspec/changes/fix-api-key-expiration-timezone/proposal.md @@ -0,0 +1,11 @@ +## Why +Creating or updating an API key with `expiresAt` can fail against PostgreSQL when clients send an ISO 8601 datetime with timezone information. The dashboard and API clients already send offset-aware values, but the backend persists `expires_at` into a `timestamp without time zone` column and should normalize those datetimes before writing. + +## What Changes +- Normalize API key expiration datetimes to UTC naive before persistence. +- Preserve the public contract that `expiresAt` may be submitted with a timezone offset. +- Add regression coverage for API key create and update flows with timezone-aware expiration values. + +## Impact +- Prevents PostgreSQL datetime binding failures on offset-aware `expiresAt` values. +- Does not change the database backend contract or the default SQLite runtime behavior. diff --git a/openspec/changes/fix-api-key-expiration-timezone/specs/api-keys/spec.md b/openspec/changes/fix-api-key-expiration-timezone/specs/api-keys/spec.md new file mode 100644 index 00000000..5914ab6e --- /dev/null +++ b/openspec/changes/fix-api-key-expiration-timezone/specs/api-keys/spec.md @@ -0,0 +1,15 @@ +### MODIFIED Requirement: API Key creation +The system SHALL allow the admin to create API keys via `POST /api/api-keys` with a `name` (required), `allowed_models` (optional list), `weekly_token_limit` (optional integer), and `expires_at` (optional ISO 8601 datetime). The system MUST generate a key in the format `sk-clb-{48 hex chars}`, store only the `sha256` hash in the database, and return the plain key exactly once in the creation response. The system MUST accept timezone-aware ISO 8601 datetimes for `expiresAt`, normalize them to UTC naive for persistence, and return the expiration as UTC in API responses. + +#### Scenario: Create key with timezone-aware expiration +- **WHEN** admin submits `POST /api/api-keys` with `{ "name": "dev-key", "expiresAt": "2025-12-31T00:00:00Z" }` +- **THEN** the system persists the expiration successfully without PostgreSQL datetime binding errors +- **AND** the response returns `expiresAt` representing the same UTC instant + +### MODIFIED Requirement: API Key update +The system SHALL allow updating key properties via `PATCH /api/api-keys/{id}`. Updatable fields: `name`, `allowedModels`, `weeklyTokenLimit`, `expiresAt`, `isActive`. The key hash and prefix MUST NOT be modifiable. The system MUST accept timezone-aware ISO 8601 datetimes for `expiresAt` and normalize them to UTC naive before persistence. + +#### Scenario: Update key with timezone-aware expiration +- **WHEN** admin submits `PATCH /api/api-keys/{id}` with `{ "expiresAt": "2025-12-31T00:00:00Z" }` +- **THEN** the system persists the expiration successfully without PostgreSQL datetime binding errors +- **AND** the response returns `expiresAt` representing the same UTC instant diff --git a/openspec/changes/fix-api-key-expiration-timezone/tasks.md b/openspec/changes/fix-api-key-expiration-timezone/tasks.md new file mode 100644 index 00000000..063d1bf8 --- /dev/null +++ b/openspec/changes/fix-api-key-expiration-timezone/tasks.md @@ -0,0 +1,3 @@ +- [x] Normalize `expires_at` to UTC naive before API key create and update writes. +- [x] Add unit regression coverage for timezone-aware expiration values. +- [x] Update the API key spec to document offset-aware `expiresAt` support. diff --git a/openspec/specs/api-keys/spec.md b/openspec/specs/api-keys/spec.md index 3bd9d911..097b415c 100644 --- a/openspec/specs/api-keys/spec.md +++ b/openspec/specs/api-keys/spec.md @@ -5,13 +5,19 @@ TBD - created by archiving change admin-auth-and-api-keys. Update Purpose after ## Requirements ### Requirement: API Key creation -The system SHALL allow the admin to create API keys via `POST /api/api-keys` with a `name` (required), `allowed_models` (optional list), `weekly_token_limit` (optional integer), and `expires_at` (optional ISO 8601 datetime). The system MUST generate a key in the format `sk-clb-{48 hex chars}`, store only the `sha256` hash in the database, and return the plain key exactly once in the creation response. +The system SHALL allow the admin to create API keys via `POST /api/api-keys` with a `name` (required), `allowed_models` (optional list), `weekly_token_limit` (optional integer), and `expires_at` (optional ISO 8601 datetime). The system MUST generate a key in the format `sk-clb-{48 hex chars}`, store only the `sha256` hash in the database, and return the plain key exactly once in the creation response. The system MUST accept timezone-aware ISO 8601 datetimes for `expiresAt`, normalize them to UTC naive for persistence, and return the expiration as UTC in API responses. #### Scenario: Create key with all options - **WHEN** admin submits `POST /api/api-keys` with `{ "name": "dev-key", "allowedModels": ["o3-pro"], "weeklyTokenLimit": 1000000, "expiresAt": "2025-12-31T00:00:00Z" }` - **THEN** the system returns `{ "id": "", "name": "dev-key", "key": "sk-clb-...", "keyPrefix": "sk-clb-a1b2c3d4", "allowedModels": ["o3-pro"], "weeklyTokenLimit": 1000000, "expiresAt": "2025-12-31T00:00:00Z", "createdAt": "..." }` with the plain key visible only in this response +#### Scenario: Create key with timezone-aware expiration + +- **WHEN** admin submits `POST /api/api-keys` with `{ "name": "dev-key", "expiresAt": "2025-12-31T00:00:00Z" }` +- **THEN** the system persists the expiration successfully without PostgreSQL datetime binding errors +- **AND** the response returns `expiresAt` representing the same UTC instant + #### Scenario: Create key with defaults - **WHEN** admin submits `POST /api/api-keys` with `{ "name": "open-key" }` and no optional fields @@ -38,7 +44,7 @@ The system SHALL expose `GET /api/api-keys` returning all API keys with their me ### Requirement: API Key update -The system SHALL allow updating key properties via `PATCH /api/api-keys/{id}`. Updatable fields: `name`, `allowedModels`, `weeklyTokenLimit`, `expiresAt`, `isActive`. The key hash and prefix MUST NOT be modifiable. +The system SHALL allow updating key properties via `PATCH /api/api-keys/{id}`. Updatable fields: `name`, `allowedModels`, `weeklyTokenLimit`, `expiresAt`, `isActive`. The key hash and prefix MUST NOT be modifiable. The system MUST accept timezone-aware ISO 8601 datetimes for `expiresAt` and normalize them to UTC naive before persistence. #### Scenario: Update allowed models @@ -50,6 +56,12 @@ The system SHALL allow updating key properties via `PATCH /api/api-keys/{id}`. U - **WHEN** admin submits `PATCH /api/api-keys/{id}` with `{ "isActive": false }` - **THEN** the key is deactivated; subsequent Bearer requests using this key SHALL be rejected with 401 +#### Scenario: Update key with timezone-aware expiration + +- **WHEN** admin submits `PATCH /api/api-keys/{id}` with `{ "expiresAt": "2025-12-31T00:00:00Z" }` +- **THEN** the system persists the expiration successfully without PostgreSQL datetime binding errors +- **AND** the response returns `expiresAt` representing the same UTC instant + #### Scenario: Update non-existent key - **WHEN** admin submits `PATCH /api/api-keys/{id}` with an unknown ID @@ -457,4 +469,3 @@ The system MUST recognize `gpt-5.4-mini` pricing when computing request costs. S - **WHEN** a request for `gpt-5.4-mini-2026-03-17` completes - **THEN** the system resolves the snapshot alias to `gpt-5.4-mini` - **AND** the system applies the same standard rates - diff --git a/tests/unit/test_api_keys_service.py b/tests/unit/test_api_keys_service.py index 5d084b06..cf219be4 100644 --- a/tests/unit/test_api_keys_service.py +++ b/tests/unit/test_api_keys_service.py @@ -1,6 +1,6 @@ from __future__ import annotations -from datetime import datetime, timedelta +from datetime import datetime, timedelta, timezone import pytest @@ -20,6 +20,7 @@ ApiKeyRateLimitExceededError, ApiKeysRepositoryProtocol, ApiKeysService, + ApiKeyUpdateData, LimitRuleInput, ) @@ -388,6 +389,26 @@ async def test_create_key_stores_hash_and_prefix() -> None: assert stored.key_prefix == created.key[:15] +@pytest.mark.asyncio +async def test_create_key_normalizes_timezone_aware_expiry_to_utc_naive() -> None: + repo = _FakeApiKeysRepository() + service = ApiKeysService(repo) + + created = await service.create_key( + ApiKeyCreateData( + name="expiring-key", + allowed_models=None, + expires_at=datetime(2026, 3, 20, 23, 59, 59, tzinfo=timezone(timedelta(hours=9))), + ) + ) + + assert created.expires_at == datetime(2026, 3, 20, 14, 59, 59) + + stored = await repo.get_by_id(created.id) + assert stored is not None + assert stored.expires_at == datetime(2026, 3, 20, 14, 59, 59) + + @pytest.mark.asyncio async def test_create_key_rejects_enforced_model_outside_allowed_models() -> None: repo = _FakeApiKeysRepository() @@ -618,6 +639,27 @@ async def test_enforce_limits_reserves_tier_aware_cost_budget() -> None: assert standard_cost_limit.current_value == 143_360 +@pytest.mark.asyncio +async def test_update_key_normalizes_timezone_aware_expiry_to_utc_naive() -> None: + repo = _FakeApiKeysRepository() + service = ApiKeysService(repo) + created = await service.create_key(ApiKeyCreateData(name="update-expiry", allowed_models=None, expires_at=None)) + + updated = await service.update_key( + created.id, + ApiKeyUpdateData( + expires_at=datetime(2026, 4, 1, 5, 30, 0, tzinfo=timezone(timedelta(hours=-7))), + expires_at_set=True, + ), + ) + + assert updated.expires_at == datetime(2026, 4, 1, 12, 30, 0) + + stored = await repo.get_by_id(created.id) + assert stored is not None + assert stored.expires_at == datetime(2026, 4, 1, 12, 30, 0) + + @pytest.mark.asyncio async def test_regenerate_key_rotates_hash_and_prefix() -> None: repo = _FakeApiKeysRepository()