feat(auth): Auth Manager foundation (OpenSpec auth-manager-foundation)#310
Draft
RyanTheRobothead wants to merge 13 commits into
Draft
feat(auth): Auth Manager foundation (OpenSpec auth-manager-foundation)#310RyanTheRobothead wants to merge 13 commits into
RyanTheRobothead wants to merge 13 commits into
Conversation
Adds an OpenSpec change describing the foundational Auth Manager service, AuthClient, and middleware integration that gives MADSci a single source of truth for identity, ownership, and capabilities across managers, nodes, and experiments. Scope of this proposal: - madsci_auth_manager service (port 8007) with Authlib + RS256 JWTs, Argon2id password hashing, PostgreSQL persistence - AuthClient with JWKS caching and auto-refresh - Opt-in AuthMiddleware on AbstractManagerBase that binds validated JWT claims into OwnershipInfo - RBAC with project-scoped grants and a @requires() decorator - OAuth 2.0 client-credentials grant for managers and nodes - Default-off rollout for backwards compatibility Out of scope (deferred to follow-on changes): Globus/ORCID OIDC federation, mTLS for nodes, UI login flows, per-manager rollout of @requires across every existing endpoint. Coordinates with the SiLA2 migration (#293/#294) on node identity and unblocks layered location ownership (#210). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Lock in single `aud = lab_id` for the foundation: - Add Decision 8 to design.md with rationale and trade-offs - Promote the requirement to normative in the token-lifecycle spec (aud is a single string equal to lab_id; verifier rejects mismatches) - Drop the resolved question from the Open Questions list - Note per-principal aud narrowing as a deferred follow-on in the task 15.4 follow-up issue list Hybrid (per-principal narrowing for service accounts/nodes) is captured as a follow-on, not built into v1. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Lock in pre-provisioned NodeIdentity/ServiceAccount creation: - Add Decision 9 to design.md with rationale, trade-offs, and the enrollment-token follow-on path - Promote pre-provisioning to a normative requirement in the identity-model spec (no unauthenticated self-registration; schema forward-compatible with a future enrollment_tokens table) - Drop the resolved question from Open Questions - Note enrollment-token flow as a deferred follow-on in task 15.4 Enrollment tokens (Kubernetes / Tailscale / Nomad style) are captured for a follow-on change once dynamic-node use cases warrant. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Lock in two more open questions: 1. OwnershipInfo back-compat (Decision 10): when auth is disabled, continue accepting caller-asserted OwnershipInfo with a sampled deprecation warning. Removal coupled to the same release that removes auth_required=False, so operators make a single jump. Added a normative scenario in the auth-client-integration spec and a new task (9.6) to implement the warning. 2. Registry orthogonality (Decision 11): the existing identity registry remains unchanged in v1. Auth credentials live in the Auth Manager DB; the registry continues to resolve URLs from ULIDs without any auth awareness. Operators are responsible for keeping the two consistent; an atomic-add CLI convenience is noted as an optional follow-on. Open Questions list now down to 1 (lab_id as the security boundary for iss/aud). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Final open question resolved: - Decision 12 in design.md: Auth Manager is 1:1 with Lab Manager; lab_id is the security/tenancy boundary; schema is single-tenant. Cross-lab user identity is intentionally deferred to the Globus/ ORCID upstream-IdP follow-on, where it belongs architecturally. - New normative requirement in the auth-manager-service spec: Auth Manager binds to a single lab_id at bootstrap; tokens issued for one lab are not accepted by another lab in v1. - New scenario in the auth-token-lifecycle spec: iss is the lab's Auth Manager URL; verifiers fetch JWKS from that URL. - Task 15.4 expanded to call out cross-lab identity → upstream-IdP follow-on and the registry/auth atomic-add CLI convenience. - Open Questions section now empty; replaced with a placeholder noting all v1 questions are resolved. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses points raised in the review at .scratch/auth_manager_foundation_review.md. Significant fixes: - Pin jti deny-list distribution mechanism: managers poll a GET /deny-list endpoint with ETag conditional fetch at a configurable interval (default 30s); revocation SLA bounded by poll interval + clock skew. Adds spec scenarios, AuthClient task (7.5) and Auth Manager service tasks (4.5, 5.10). - Spec local audit-log fallback at consuming managers: persist auth events to disk when the Auth Manager is unreachable, drain on recovery, bounded with rotation and a warning. Adds requirement, three scenarios, and task 9.7. - Apply rate limiting to the /token endpoint and reject unsupported grant_type with HTTP 400 / unsupported_grant_type per RFC 6749. Adds requirement and task 5.11. - Tighten body-vs-claims OwnershipInfo precedence: principal-bound fields with a corresponding claim NEVER fall back to the body (absent claim => absent field). Operational identifiers (experiment_id, workflow_id, etc.) without claim slots are still taken from the body. Adds two scenarios. - Spec the OwnershipInfo.from_jwt_claims() field mapping explicitly so implementations don't drift. Minor fixes: - Operator doc (12.2) now covers 0600 file mode on .madsci/secrets, .gitignore treatment, X-Forwarded-For handling for accurate audit source IPs, and audit retention/PII guidance. - Task 1.3 calls out verifying madsci start port-collision logic. - Task 15.3 explicitly defers to project-wide coverage threshold. - Two new integration tests (11.8 deny-list, 11.9 audit fallback). Nits: - Proposal links Issue #86 explicitly. - Proposal clarifies that the sampled deprecation warning is a small behavior change even with auth_enabled=False. Skipped: review nit #13 (AuthClient auth_server_url default) - explicit setting is the right call for v1; registry-vended bootstrap is already on the follow-on list. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three non-blocking review observations addressed: 1. DenyListService restart durability: deny-list is now persisted in a revoked_access_tokens table (jti, exp, revoked_at), and the in-memory cache is hydrated from it on Auth Manager startup so revoked-but-unexpired jtis cannot silently re-validate after a restart. Adds normative requirement, scenario, schema entry, and updates task 4.5; adds restart-durability integration test 11.10. 2. Local audit-log bound: operator guide (12.2) now calls out the default 100 MB bound, the rotation+warning behavior, and a strong recommendation to alert on the rotation warning event so operators upsize before the bound bites at high request rates. 3. manager_id claim cleanup: introduces a dedicated manager_id claim for service-account tokens. sub remains the canonical principal id (client_id for service accounts and nodes, user_id for users). OwnershipInfo.from_jwt_claims now sources manager_id from the dedicated claim, not from sub - preserving canonical OAuth semantics while still letting consumers populate operational identifiers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…foundation) Adds the new madsci_auth_manager package (port 8007) plus all integration points needed for opt-in JWT-based authentication and authorization across MADSci services. Auth is disabled by default — existing deployments are unaffected. See docs/guides/auth.md and docs/guides/auth_operator.md for the architecture and operator runbook. Highlights: - madsci_auth_manager service: RS256 JWT issuance, opaque server-stored refresh tokens with reuse detection, JWKS, persistent jti deny-list (with ETag conditional fetch), users/projects/roles/service-accounts/node- identities, signing-key rotation, append-only audit log. Single-tenant per Decision 12 (aud = lab_id). PostgreSQL via SQLAlchemyHandler; in-memory SQLiteHandler for tests. - madsci.client.auth_client.AuthClient: login / refresh / client_credentials_login / verify_jwt (JWKS-cached + force-refresh on signature failure) / introspect / revoke / deny-list polling, plus the full admin surface. - AuthMiddleware on AbstractManagerBase: opt-in via auth_enabled=True; validates bearer tokens, populates request.state.principal, enters ownership_context() from claims; auth_required=False migration mode passes unauth'd requests through with structured warnings. - auth_client_context() ambient propagation: when set, create_httpx_client() injects Authorization: Bearer and force-refreshes- and-retries on 401. - @requires(permission=..., project_from=...) decorator. - madsci auth CLI: bootstrap | user | project | manager | node | credentials | keys subcommands. - Templates, example-lab compose service + Postgres DB, docs (auth.md, auth_operator.md), CHANGELOG. - 65 dedicated tests across server, client, middleware, decorators, CLI, integration; full suite (4171 tests) passes. - ruff lint clean across the project. Six tasks deferred with reasons recorded in tasks.md (docker-compose e2e, live audit-log drain, SiLA2/layered-location cross-team review, security-review and madsci-release-audit skill runs — to be invoked before merge). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ager-security-hardening) Implements the auth-manager-security-hardening OpenSpec change in response to an independent senior code review of PR #310. Five merge-blocking issues + six significant follow-ups, plus migration off the deprecated authlib.jose module. Security hardening: - Auth Manager admin endpoints now require authentication (@requires(...) on every admin route + AuthMiddleware on the Auth Manager itself with an explicit unauthenticated allowlist for /token, /jwks, /health*, /settings, /deny-list, /introspect). New auth.* permission strings. - POST /introspect follows RFC 7662 — unauthenticated callers get {"active": false}, never the full claims. - POST /revoke requires authentication; self-revocation always allowed, cross-principal requires auth.token.revoke. - JWT verification pins RS256 (defense-in-depth header check + JOSE library's algorithms= allowlist) — closes alg-confusion forgery class. - Configurable clock-skew leeway (token_clock_skew_seconds, default 30s) applied uniformly on server and client. - Refresh-token consumption is atomic via a claim-marker pattern; the loser fires the family-revoke reuse-detection path. New Alembic migration 0002 adds partial unique index refresh_tokens(token_hash) WHERE revoked_at IS NULL. rotated_to column populated for forensic walks. - Auth Manager refuses to start without a bound lab_id; "lab-unbound" placeholder removed. - Bootstrap CLI no longer accepts --password on argv (would leak via ps); sources from MADSCI_AUTH_BOOTSTRAP_PASSWORD env var or interactive prompt. - X-Forwarded-For no longer trusted by default (auth_trust_forwarded_for opt-in for trusted-proxy deployments). - Audit-write failures are no longer silently swallowed for token issuance. JOSE library swap (authlib.jose → joserfc): - Authlib 1.7+ deprecates authlib.jose in favor of joserfc (same author's successor library). Migrated TokenService, AuthClient, and test helpers. - Dependency declarations swap Authlib>=1.3.0 → joserfc>=1.0.0 in both madsci.auth_manager and madsci.client. joserfc was already pulled in transitively, so no install footprint change. - JWT format and verification semantics are identical; eliminates the runtime AuthlibDeprecationWarning. Tests: 92/92 auth tests pass (65 baseline + 24 new hardening + 3 CLI). 4197 full-suite green; project-wide ruff check clean. OpenSpec artifacts: openspec/changes/auth-manager-security-hardening/ {proposal,design,tasks}.md + 3 spec deltas (auth-manager-service, auth-token-lifecycle, auth-client-integration). Deferred (annotated in tasks.md): - 8.1, 8.2, 8.4 — full session-sharing audit-log refactor (8.3 + 8.5 cover the security-relevant property). - 11.1–11.3 — router refactor (explicitly bounded by design D9's 200-line cap). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses the post-implementation security review of the auth-manager-security-hardening change (.scratch/security_review_auth_manager.md) plus the release-audit findings (.scratch/auth_manager_security_hardening_audit.md). Security-review HIGH (Vuln 1: Auth Manager admin endpoints unauthenticated by default): - AuthManagerSettings overrides auth_enabled and auth_required defaults to True. Mitigates the foundational issue that a fresh deployment with no overrides exposed every admin route unauthenticated (auth_enabled inherited False from ManagerSettings; @requires no-ops when middleware isn't installed). - AuthManager._setup_auth_middleware override installs a self-verifying AuthMiddleware that uses the local TokenService instead of constructing a remote AuthClient — sidesteps the prefixed-alias collision that would otherwise prevent the base-class path from finding auth_server_url. - AuthManager.run_server() refuses to bind unless both flags are True. Defense-in-depth: catches misconfiguration at the startup boundary rather than at first request. Security-review filtered findings (defense-in-depth, not concretely exploitable today but worth fixing): - AuthClient gains expected_issuer + expected_audience constructor args. When set, verify_jwt validates iss and aud claims via JWTClaimsRegistry. manager_base._setup_auth_middleware plumbs the manager's auth_server_url through as expected_issuer and lab_id as expected_audience. - /revoke now applies the same self-vs-other rule on the refresh-token branch. Previously bob with knowledge of admin's refresh-token bearer string could force-sign-out admin without holding auth.token.revoke. Release-audit fixes (already applied locally before this commit, included here for completeness): - examples/example_lab/README.md: bootstrap snippet sources password from MADSCI_AUTH_BOOTSTRAP_PASSWORD env var, not --password on argv. - src/madsci_common/pyproject.toml: removed stale Authlib>=1.3.0 dep with no remaining importer. - src/madsci_auth_manager/.../token_service.py:272: comment updated from "authlib's decoder" to "joserfc's decoder". Test fixtures explicitly opt out of auth enforcement where needed (test_auth_server, test_auth_client, test_cli_auth, test_security_hardening ::server, testing.make_auth_manager via the new auth_enforced kwarg) — this preserves their unit-test semantics under the new safe defaults. New tests pin the invariants: test_auth_manager_settings_default_to_auth_enabled test_auth_manager_run_server_refuses_unsafe_config test_cross_principal_refresh_token_revoke_requires_permission test_auth_client_rejects_token_with_wrong_audience Tests: 96/96 auth tests pass, 4202/4202 full suite pass. ruff clean. docs/CHANGELOG.md and openspec/changes/auth-manager-security-hardening/ tasks.md updated; new Section 15 in tasks.md tracks the mitigations. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Manual smoke test against the example lab (auth-manager-security-hardening
task 13.6) — PASSED. End-to-end exercised against a freshly-built docker
stack:
- Bootstrap via MADSCI_AUTH_BOOTSTRAP_PASSWORD env var → success.
- Bootstrap with --password on argv → "No such option: --password" (CLI
rejects it as designed; closes the ps-leak vector).
- Bootstrap without env var or TTY → clear error citing the env var.
- Bootstrap re-run on populated DB → idempotent.
- POST /token with admin creds → access + refresh tokens issued.
- GET /.well-known/jwks.json unauthenticated → RS256 key returned.
- 10 admin endpoints (GET and POST across /users, /projects, /roles,
/service-accounts, /node-identities, /keys, /keys/rotate) all 401
unauthenticated, all 200 with the admin token.
- /introspect unauthenticated → {"active": false} (RFC 7662 — no claim
leak); /introspect authorized → full claims dict.
- Auth Manager startup log confirms "AuthMiddleware installed on Auth
Manager (self-verifying)".
Pre-existing bug surfaced and fixed during smoke:
examples/example_lab/compose.yaml set AUTH_DATABASE_URL to
postgresql://madsci:madsci@madsci_postgres_auth:5432/auth — the container
DNS name. But the auth_manager service uses network_mode: host (like every
other manager in the file), so container DNS doesn't resolve and the Auth
Manager crashed with OperationalError on startup. Changed to
postgresql://madsci:madsci@localhost:${AUTH_POSTGRES_PORT:-5435}/auth
matching the pattern other managers (resource, document_db) use to reach
their postgres.
Foundation tasks.md 15.1/15.2 also marked complete — both review skills
have now been run end-to-end (findings tracked + addressed in the
hardening change's Section 15).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
auth-manager-foundation— the foundational identity, token, and authorization layer for MADSci. Closes Feature: Auth Manager #86 (foundation only; follow-on changes for Globus/ORCID, mTLS, per-manager@requiresrollout are tracked indocs/guides/auth.md).madsci_auth_managerpackage on port 8007: RS256 JWT issuance, opaque server-stored refresh tokens with reuse detection, JWKS, persistent jti deny-list (with ETag conditional fetch), users/projects/roles/service-accounts/node-identities, signing-key rotation, append-only audit log. Single-tenant per Decision 12 (aud = lab_id). PostgreSQL viaSQLAlchemyHandler; in-memorySQLiteHandlerfor tests.AuthClient, ambientauth_client_context()propagation throughcreate_httpx_client(), opt-inAuthMiddlewareonAbstractManagerBase,@requires(permission=..., project_from=...)decorator, andmadsci authCLI (bootstrap | user | project | manager | node | credentials | keys).auth_enabled=Falseon every manager). Existing deployments are unaffected. Seedocs/guides/auth.mdfor the architecture anddocs/guides/auth_operator.mdfor the bootstrap and rollout runbook.auth-manager-security-hardening— review-driven follow-ups bundled into this same PR before any production rollout. See Security hardening section below.Status: draft. Pre-merge gates not yet run (see Test plan).
Security hardening (bundled change)
Triggered by an independent senior code review of the foundation. Five merge-blocking issues + six significant follow-ups, all addressed in this PR. Full proposal/design/tasks in
openspec/changes/auth-manager-security-hardening/./users,/projects,/roles,/roles/grant,/service-accounts,/node-identities,/credentials/*/rotate,/keys/*, all listing endpoints) carries@requires(permission=...)and the Auth Manager mountsAuthMiddlewareon itself with an explicit unauthenticated allowlist (/token,/.well-known/jwks.json,/health*,/settings,/deny-list,/introspect). 14 newauth.*permission strings.POST /introspectfollows RFC 7662 — unauthenticated callers (or callers withoutauth.token.introspect) get{"active": false}, never the full claims.POST /revokerequires authentication. Self-revocation (matchingsub) allowed; cross-principal revoke requiresauth.token.revoke.RS256. Both server and client reject any other JWS algorithm before the JOSE library sees the token (defense-in-depth) AND pass the same allowlist to the library'salgorithms=argument. Closes the alg-confusion attack class (HS256-with-public-key forgery).AuthManagerSettings.token_clock_skew_seconds, default 30s) — applied uniformly on server and client.rotated_tocolumn links parent → child for forensic walks. Alembic migration0002_refresh_token_partial_unique_indexadds a partial unique index onrefresh_tokens(token_hash) WHERE revoked_at IS NULL.lab_id. The"lab-unbound"placeholder audience is gone — two unbound deployments can no longer mutually trust each other's tokens.madsci auth bootstrapno longer accepts--passwordon argv (leaks viaps). Source fromMADSCI_AUTH_BOOTSTRAP_PASSWORDenv var or interactive prompt only.X-Forwarded-Forno longer trusted by default. NewAuthManagerSettings.trust_forwarded_for(defaultFalse) gates whether_client_ipreads the header.joserfc. Authlib 1.7+ deprecatesauthlib.jose;joserfcis the same author's successor library. TouchesTokenService,AuthClient, and the test helpers;joserfcwas already pulled in transitively, so no install footprint change. Eliminates the runtimeAuthlibDeprecationWarning.24 new security-focused tests + 2 new CLI tests cover: alg-confusion rejection (HS256 + alg=none), refresh concurrency, every admin endpoint × (no token / wrong permission / right permission),
/introspectand/revokeenforcement paths, clock-skew leeway, lab_id-required startup,X-Forwarded-Forgating, audit failure-closed.Deferred from the hardening change (annotated in
tasks.md):Scope
AuthManagerFastAPI serverAuthClientAuthMiddlewareonAbstractManagerBase@requiresdecoratorsecurity-reviewpass complete + addressed (see hardening section); 15.1 re-review and 15.2 (madsci-release-auditskill) MUST be invoked before mergeTest plan
pytest src/madsci_auth_manager src/madsci_common/tests/test_auth_*.py src/madsci_client/tests/test_auth_*.py src/madsci_client/tests/test_cli_auth.py— 92/92 passing (65 baseline + 24 new hardening + 3 new CLI; covers server, integration, types, context, middleware, decorators, client, CLI smoke, alg-confusion, refresh concurrency, admin authorization,X-Forwarded-Forgating, audit failure-closed).ruff check .— clean (project-wide).AuthlibDeprecationWarningno longer present in test output.security-reviewskill on this branch (gates merge — implementation complete, not yet re-reviewed).madsci-release-auditskill (gates merge, per task 15.2).NodeIdentitymodel + reservedmtls_cert_fingerprint.OwnershipInfo.from_jwt_claims.just up,MADSCI_AUTH_BOOTSTRAP_PASSWORD=... madsci auth bootstrap --username admin --lab-id <ulid>, exercise/token+/.well-known/jwks.json, and confirm admin endpoints reject unauthenticated calls.Notable design decisions
Foundation decisions in
openspec/changes/auth-manager-foundation/design.md; hardening decisions inopenspec/changes/auth-manager-security-hardening/design.md. The most consequential:joserfc(Authlib's successor library) — see hardening D10.aud = lab_idfor v1 (per-principal aud narrowing deferred).OwnershipInfoaccepted whileauth_enabled=False; sampled deprecation warning emitted; coupled-removal withauth_required=False.AuthMiddlewareon the Auth Manager itself with an explicit allowlist.UPDATE ... WHERE revoked_at IS NULL ... RETURNING ...for refresh-token consumption.authlib.jose→joserfc.Follow-on changes
Captured in
docs/guides/auth.md:auth-globus-orcid-federation— upstream IdP federation (cross-lab user identity).auth-node-mtls— mTLS trust for nodes (slots into the reservedmtls_cert_fingerprintfield).auth-per-manager-rbac-rollout— apply@requiresacross every manager endpoint.auth-per-principal-aud-narrowing— narrow the single-audmodel.auth-node-enrollment-tokens— replace pre-provisioned NodeIdentity registration with kubelet-bootstrap-style enrollment.auth-registry-and-auth-cli-merge— optional CLI convenience that doesmadsci registry add+madsci auth node registeratomically.auth_server.py(deferred from hardening 11.1–11.3).🤖 Generated with Claude Code