Fix/sr ws auth tokenstore credentials expiry - #1413
Merged
GoodnessJohn merged 4 commits intoAug 28, 2026
Merged
Conversation
… auth The Socket.IO auth middleware called a bare jwt.verify(token, secret) with no algorithms/issuer/audience options and never checked isAccessTokenRevoked, unlike the HTTP middleware's verifyAccessToken(). This let a token revoked via POST /api/auth/logout stay fully valid over an already-open or newly opened WebSocket connection until natural expiry, and left the WS layer exposed to alg:none / HS-RS confusion that the HTTP layer already defends against. createAuthMiddleware now delegates to the shared verifyAccessToken() so algorithm pinning, issuer/audience checks, and revocation checks apply uniformly across HTTP and WebSocket. Added reverifyOrDisconnect() plus a periodic sweep in websocket/index.ts so already-open sockets are dropped once their token is revoked, not just new connections. Added tests proving a token revoked via logout is rejected on both new and existing connections.
…d across instances services/tokenStore.ts held refresh-token families, access-token revocation, and login-lockout state entirely in per-process Maps, a known limitation documented in AUTH_MATRIX.md with no tracked follow-up. In a horizontally scaled deployment, a logout or reuse-detected revocation on instance A left the user fully authenticated against instances B/C until natural token expiry, and login lockout was bypassable by hitting a different instance. tokenStore is refactored into a TokenStore class: the existing synchronous Maps remain as a fast local L1 cache (so every exported function keeps its original synchronous signature and callers are unaffected), backed by Redis as the shared source of truth when REDIS_URL is configured. Every mutation writes through to Redis with the same TTL semantics as before (REFRESH_TOKEN_TTL_MS, LOGIN_LOCKOUT_MS, LOGIN_ATTEMPT_WINDOW_MS) and publishes an event on a pub/sub channel that every instance (including local dev with REDIS_URL unset, which no-ops) subscribes to and applies to its own cache. Added ioredis as a dependency and a multi-instance integration test that shares one backend between two TokenStore instances to prove revocation, refresh-token reuse detection, and login lockout now propagate across instances instead of staying per-process.
…al store routes/auth.ts's verifyCredentials() compared every login attempt's password against a single process.env.STUB_PASSWORD for every userId, so "authentication" only proved the caller knew one shared secret, not who they actually were. resolveRole() was similarly env-driven via ADMIN_USER_IDS/AGENT_USER_IDS allowlists. Both were called out as known limitations with no tracked remediation in AUTH_MATRIX.md, even though downstream code (ensureOwnership, AML subject tracking, compliance attribution) treats the JWT `sub` claim as a trustworthy per-user identity. Adds db/userStore.ts: a users table (Postgres when DATABASE_URL is configured, in-memory otherwise) holding a bcrypt password hash and role per user. verifyCredentials()/resolveRole() in routes/auth.ts now delegate to verifyUserCredentials()/getUserRole() behind the same call sites, so a valid password for one identity can no longer authenticate as a different one. ADMIN_USER_IDS/AGENT_USER_IDS are kept only as a one-time bootstrap/seed mechanism for the first operator accounts — seeded once into the store and never consulted again or allowed to overwrite a rotated password. Added tests covering cross-user password isolation, role resolution, and bootstrap seeding not clobbering rotated credentials. Note: existing auth tests that log in as arbitrary userIds against a shared STUB_PASSWORD will need to seed those users via upsertUser() first — that is the intended behavioural change this fix makes.
VULNERABILITY_EXCEPTIONS.md promises "Exceptions reaching their expiration date automatically fail CI builds until renewed or patched," but no workflow or script implemented it. container-security.yml's trivy-scan job never referenced .trivyignore at all, and dependency-security.yml's cargo-audit-and-deny job never evaluated deny.toml's [advisories] list. Both exception files are empty today, so the gap was latent, but the first exception added under the documented policy would never have expired in CI. Adds scripts/check-exception-expiry.js, which parses the "(Expires: YYYY-MM-DD)" comment convention required by VULNERABILITY_EXCEPTIONS.md out of .trivyignore and deny.toml and exits non-zero once any entry is past its date. Wires it into container-security.yml (new check-exception-expiry job, required by trivy-scan) and dependency-security.yml (new step ahead of cargo audit/deny) so an expired exception blocks the build. Also fixes trivy-scan to actually pass trivyignores: ".trivyignore" to the scan step, and adds the missing `cargo deny check advisories` step so deny.toml's [advisories] section is evaluated in CI at all. Diff is under 150 lines; see docs/CI_EXCEPTION_EXPIRY_README.md for a written summary of what was implemented and what was left out of scope.
|
@janetpius-cmd is attempting to deploy a commit to the Harold's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@janetpius-cmd 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! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Linked Issue
Closes #1289
Closes #1290
Closes #1291
Closes #1293
Type of Change
Checklist
sdk/) if this touches a published packageBreaking Change Notes