Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions app/modules/api_keys/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand All @@ -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,
Expand All @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions openspec/changes/fix-api-key-expiration-timezone/proposal.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions openspec/changes/fix-api-key-expiration-timezone/tasks.md
Original file line number Diff line number Diff line change
@@ -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.
17 changes: 14 additions & 3 deletions openspec/specs/api-keys/spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -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": "<uuid>", "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
Expand All @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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

44 changes: 43 additions & 1 deletion tests/unit/test_api_keys_service.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from __future__ import annotations

from datetime import datetime, timedelta
from datetime import datetime, timedelta, timezone

import pytest

Expand All @@ -20,6 +20,7 @@
ApiKeyRateLimitExceededError,
ApiKeysRepositoryProtocol,
ApiKeysService,
ApiKeyUpdateData,
LimitRuleInput,
)

Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down
Loading