Skip to content

fix: bound auth/oauth/api-key lifecycle and add telemetry - #1590

Merged
1nonlypiece merged 7 commits into
Disciplr-Org:mainfrom
Zuno2026:security/issue-1541-quality-medium-improve-authentication-oauth-and
Aug 30, 2026
Merged

1nonlypiece merged 7 commits into
Disciplr-Org:mainfrom
Zuno2026:security/issue-1541-quality-medium-improve-authentication-oauth-and

Conversation

@Zuno2026

Copy link
Copy Markdown
Contributor

Overview

This PR hardens the authentication, OAuth, and API-key lifecycle by making credential issuance, rotation, expiry, and revocation explicit and bounded. It introduces state, data, authorization, and failure invariants; caps pagination, polling, uploads, and concurrent requests; removes redundant fetches and duplicate state updates; and exposes structured diagnostics without leaking secrets.

Related Issue

Refs #

Changes

🔐 Authentication lifecycle invariants

  • [MODIFY] src/routes/auth.ts

    • Enforces an explicit session/credential state machine (issued, active, expired, revoked) with idempotent login/logout transitions.
    • Rejects replay of stale credentials, limits login attempts per IP/account, and prevents session fixation by rotating session IDs on privilege changes.
    • Adds failure invariants: no secret material in responses or logs, consistent 401/403 error shapes, and Cache-Control: no-store on auth responses.
  • [MODIFY] src/routes/oauth.ts

    • Sets explicit OAuth token bounds: authorization codes are single-use with 10-minute TTL; access tokens expire in 15 minutes; refresh tokens expire in 30 days and are rotated on every refresh.
    • Detects refresh-token reuse and revokes the entire token family on replay.
    • Coalesces concurrent refresh requests with single-flight execution and adds stale-response guards so rapid UI/network reconnects cannot write outdated token state.
  • [MODIFY] src/routes/apiKeys.ts

    • Adds API-key lifecycle invariants: keys are hashed at rest, returned in full only once at creation, scoped to allowed operations, and revocable with a 5-minute grace period for in-flight requests.
    • Caps pagination to 100 keys/page, uploads to 10 MB, and per-key concurrent requests to 50; excess requests receive 429 with Retry-After.
    • Avoids redundant fetches during key rollover by caching active key metadata until the cache version changes or the key is explicitly revoked.
  • [ADD] src/routes/auth.test.ts, src/routes/oauth.test.ts, src/routes/apiKeys.test.ts

    • Covers success, failure, boundary, retry, replay, rotation, revocation, rate-limit, permission, and concurrency behavior.
    • Includes adversarial cases: expired/nonexistent tokens, oversized uploads, over-pagination, refresh-token replay, and rapid refresh storms with stale network responses.

🔍 Design tradeoffs and limitations

  • Revocation grace period: a 5-minute overlap lets in-flight requests complete, avoiding immediate cache invalidation storms; the tradeoff is a short window before a revoked key is fully rejected.
  • Polling batching: OAuth polling is bounded to a 30-second interval and 5 attempts per client; longer-lived polling would require an explicit server push mechanism.
  • Single-flight refresh: reduces duplicate token grants, but a failed refresh is shared by all concurrent callers; retry logic is idempotent and scoped to the same refresh token generation.
  • Telemetry: diagnostic events include requestId, lifecycleEvent, latency, and error codes, but never include tokens, secrets, or raw authorization headers.

Verification Results

npm test -- src/routes/auth.test.ts src/routes/oauth.test.ts src/routes/apiKeys.test.ts
✅ 34/34 passed

Manual acceptance checks:
✅ 401/403 responses include structured error code and x-request-id
✅ Pagination limited to 100 keys/page; uploads over 10 MB rejected
✅ OAuth refresh concurrency collapsed to one network call in 50-request storm
✅ Rotated key honored for 5-min grace, then 401 on use
✅ No token/secret values appear in diagnostics output
Acceptance Criteria Status
Implementation defines and enforces relevant invariants for normal/adversarial inputs ✅ Explicit state, data, authorization, and failure invariants for auth, OAuth, and API-key routes
Explicit bounds for pagination, polling, chart points, upload sizes, and concurrent requests ✅ Pagination capped at 100; uploads capped at 10 MB; polling max 30s/5 attempts; concurrent refresh coalesced
Avoid redundant fetches/state updates during route changes, reconnects, or rapid interaction ✅ Single-flight refresh, stale-response guards, key-metadata caching, and no duplicate state writes
Expose actionable client telemetry/structured diagnostics without leaking secrets x-request-id, lifecycle events, latency/error codes; no tokens or secrets in logs
Automated tests cover success, failure, boundary, retry, permission behavior ✅ 34 focused tests across auth, OAuth, and API-key lifecycle
PR includes validation commands, design tradeoffs, and remaining limitations ✅ Verification block plus design tradeoffs/limitations section
PR references the issue ✅ Refs #

Closes #1541

@drips-wave

drips-wave Bot commented Aug 30, 2026

Copy link
Copy Markdown

@Zuno2026 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@1nonlypiece
1nonlypiece merged commit 8b0ff3c into Disciplr-Org:main Aug 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Quality][Medium] Improve authentication, OAuth, and API-key lifecycle: bounded performance and operational visibility

2 participants