feat(auth): in-memory and Redis OAuth token storage#233
Merged
Conversation
… writes Replaces the default FileTreeStore (disk-based) with a MemoryStore (default) or RedisStore (when REDIS_HOST_PORT is set) so no OAuth token state is ever written to the filesystem — a security requirement for K8s deployments.
Up to standards ✅🟢 Issues
|
| Category | Results |
|---|---|
| Documentation | 3 minor |
| CodeStyle | 1 minor |
🟢 Metrics 35 complexity
Metric Results Complexity 35
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
- Safe REDIS_HOST_DB parse: ValueError at module load with a clear message instead of a bare int() crash - Add REDIS_PASSWORD env var for Redis AUTH, wired into RedisStore constructor - Move MemoryStore import to top level; parse host/port explicitly so build_token_store() doesn't rely on URL string manipulation - Document REDIS_PASSWORD in README, CLAUDE.md, and K8s manifest (Secret-backed, optional: true)
REDIS_HOST_PORT now accepts a full URI (redis:// or rediss://) in addition to bare host:port. When rediss:// is provided, the Redis client is constructed with ssl=True — bypassing key_value's URL parser which ignores the scheme. Password and db can be embedded in the URI or fall back to REDIS_HOST_DB / REDIS_PASSWORD env vars.
Add the auth layer (APIKeyVerifier / GitHub OAuth2), MemoryStore, and RedisStore (with TLS) to the README ASCII diagram.
saidsef
changed the title
Move token store (MemoryStore / RedisStore) to a right-hand branch off the Auth Layer box so the main vertical flow to PRIssueAnalyser is unbroken — the previous layout left a floating | and v after the Redis box with no visible connection back to the flow.
redis>=5.0.0 was added to pyproject.toml and uv.lock but requirements.txt was never regenerated. The Dockerfile installs deps from requirements.txt, so redis was absent from the Docker image, causing a ModuleNotFoundError at runtime when REDIS_HOST_PORT is set.
The database index can be embedded directly in REDIS_HOST_PORT as a URI path component (redis://host:6379/1), making a separate REDIS_HOST_DB fallback redundant. Bare host:port or URIs without a /db path default to database 0.
saidsef
force-pushed
the
feat/oauth-memory-redis-token-store
branch
from
c302cbd to
a01e1dd
Compare
Also adds em-dash style guideline to the MCP server prompt.
- Add blank line after h1 heading in README.md (MD022) - Convert _PermissiveGitHubProvider docstring to NumPy style (D203, D212, D205) - Convert resolve_token Raises section to NumPy style (D406, D407, D413)
…Redis deployments Wrap RedisStore with PrefixCollectionsWrapper using a 12-char SHA-256 hash of GITHUB_OAUTH_BASE_URL as the collection prefix. Two server instances that share the same Redis instance (e.g. personal and company deployments) now write to fully isolated keyspaces, preventing one server's OAuth session from overwriting or colliding with the other's JTI mappings, upstream tokens, and client registrations.
Extract _build_redis_client() helper to reduce build_token_store() cyclomatic complexity below the limit of 8. Fix D212 and D205 docstring violations in build_token_store() and get_oauth_verifier().
Add _derive_jwt_signing_key() that automatically derives a deterministic JWT signing key from GITHUB_OAUTH_CLIENT_SECRET when JWT_SIGNING_KEY is not explicitly set. This ensures all pods in a multi-replica deployment share the same signing key, preventing token validation failures caused by divergent derived keys. An explicit JWT_SIGNING_KEY env var can still be provided as an override.
…B validation - Add tests/test_auth.py with 15 tests covering _build_redis_client URI parsing (host:port, redis://, rediss://, db path, password precedence, invalid db raises) and build_token_store backend selection (MemoryStore default, RedisStore construction, PrefixCollectionsWrapper applied/skipped, prefix stability and isolation) - _build_redis_client now raises ValueError for non-integer db path instead of silently defaulting to 0, matching the documented test plan behaviour - Fix resolve_token docstring: move summary onto opening line (D212) and add blank line before body (D205)
…clusion - Extract _parse_redis_db() from _build_redis_client() to bring cyclomatic complexity from 9 to 6 (limit is 8) - Switch multi-line docstrings in auth.py to D213 style (summary on second line) for build_token_store, _derive_jwt_signing_key, and resolve_token - Add blank line before class docstrings in tests (D203) - Add [tool.bandit] exclude_dirs=["tests"] to pyproject.toml so assert statements and test-fixture strings are not flagged as security issues - Suppress ANN annotations for test helpers in per-file-ignores - Update CLAUDE.md: document D213+D203 as the enforced convention (Codacy), note ruff ignores both conflicting pairs, update Testing section
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
1 participant
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
FileTreeStore) to in-processMemoryStoreby default - required for K8s pods withreadOnlyRootFilesystem: trueREDIS_HOST_PORTis set,RedisStoreis used instead; accepts barehost:portor a full URI (redis://plaintext,rediss://TLS)REDIS_PASSWORD; database defaults to0or is read from the/dbpath component in the URIREDIS_HOST_PORTvia ConfigMap andREDIS_PASSWORDvia Secret (both optional)redis>=5.0.0added to dependencies;requirements.txtregenerated so Docker builds include itTest plan
REDIS_HOST_PORT- confirmMemoryStoreis used (no disk writes)REDIS_HOST_PORT=redis://localhost:6379- confirm OAuth state lands in RedisREDIS_HOST_PORT=rediss://localhost:6380- confirm TLS connection (ssl=True)REDIS_HOST_DB=abc- confirm server refuses to start with a clear errorclient_ids require re-auth (expected, documented)--read-only --tmpfs /tmp- confirm no filesystem writes