Skip to content

feat(auth): Fine grained auth integration with external services - #1169

Open
nathanzilgo wants to merge 19 commits into
agentic-community:mainfrom
GuilhermeAlz:feat/fine-grained-auth-integration
Open

feat(auth): Fine grained auth integration with external services#1169
nathanzilgo wants to merge 19 commits into
agentic-community:mainfrom
GuilhermeAlz:feat/fine-grained-auth-integration

Conversation

@nathanzilgo

@nathanzilgo nathanzilgo commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Closes #358

Description of changes:
Integrates a pluggable custom authorizer webhook into the auth server's /validate endpoint, giving operators a drop-in replacement (or supplemental gate) for the built-in JWT/OAuth2 pipeline.

New AUTHORIZER_MODE env var (native / custom / both, default native) controls execution path — zero-change for existing deployments.

custom mode — skips all native validation and forwards the full request context to an external HTTP endpoint; access is granted or denied solely by the webhook response.

both mode — runs the full native JWT/OAuth2 pipeline first, then passes the validated identity (username, scopes, groups) to the custom authorizer as a final gate.

Startup validation — misconfigured CUSTOM_AUTHORIZER_URL or unsupported AUTHORIZER_MODE values raise a ValueError at startup rather than silently failing at request time.

Test plan

  • uv run pytest tests/auth_server/unit/test_custom_authorizer.py -v — all unit tests pass
  • uv run pytest tests/auth_server/integration/test_custom_authorizer_integration.py -v — all integration tests pass
  • AUTHORIZER_MODE=native — existing behavior unchanged (regression check)
  • AUTHORIZER_MODE=custom with a running webhook — request authorized/denied by external service
  • AUTHORIZER_MODE=both — native check passes, then custom authorizer consulted
  • Missing CUSTOM_AUTHORIZER_URL with AUTHORIZER_MODE=custom → server refuses to start with a clear error
  • Verify Authorization header is masked in the forwarded payload

@codecov-commenter

codecov-commenter commented Jun 2, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 69.45813% with 62 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
auth_server/server.py 4.76% 58 Missing and 2 partials ⚠️
auth_server/services/custom_authorizer.py 98.00% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@nathanzilgo nathanzilgo changed the title Fine grained auth integration feat: Fine grained auth integration Jun 2, 2026
@nathanzilgo nathanzilgo changed the title feat: Fine grained auth integration feat(auth): Fine grained auth integration with external services Jun 2, 2026
@nathanzilgo
nathanzilgo marked this pull request as ready for review June 8, 2026 18:04
@aarora79

Copy link
Copy Markdown
Contributor

@nathanzilgo

Thanks for this — the fail-closed client, the header masking, and the 78 unit tests are genuinely strong work, and the native-default keeps existing deployments safe. Before it can merge, though, there's a set of items to address. Grouped by severity, with a short note on how to fix each.

This is an important capability to add to the solution, and your implementation has helped us think more deeply about the design — so please read the comments below in that spirit, as us working through the design together rather than just as feedback on the PR. 🙏

Blockers (must fix before merge)

1. In custom mode, let the authorizer supply the principal rather than hardcoding one in the auth server.
Right now the custom-mode success path in auth_server/server.py returns a fixed identity (username="custom-authorized-user", groups=["mcp-registry-admin"], unrestricted scopes). The thing to reconsider here: in custom mode all native validation is skipped, so the auth server doesn't actually have any information about who the principal is or which groups they belong to. That means the only component that knows the principal's groups in this mode is the custom authorizer itself, so it would be safer (and more useful) to have the authorizer return that identity rather than assign a default in the gateway. As written, every approved request becomes a full-access admin and the audit log shows a single synthetic user, which probably isn't the intent.

Suggested approach — make the authorizer the identity source in custom mode:

  • Extend CustomAuthorizerResponse to return the principal. Add a principal block carrying username, groups (list), scopes (list), and optional client_id. The authorizer returns this on a authorized=true decision.

  • In custom-mode success, populate X-User / X-Scopes / X-Groups / X-Auth-Method from result.principal — replacing the hardcoded custom-authorized-user / mcp-registry-admin / unrestricted-scopes block. The idea is that group (and scope) assignment stays configurable in the authorizer rather than fixed in the gateway, which is exactly what the Cedar reference authorizer is positioned to own.

  • Missing principal/groups → deny (fail-closed). If the authorizer returns authorized=true but omits the principal or its groups, the gateway has no authorization basis, so it must deny rather than guess. Do not fall back to any default identity, and never to admin.

  • Leave both mode as-is on identity. In both mode native auth has already resolved the real identity/groups from the JWT before the webhook runs, so the webhook stays a pure yes/no gate and the real token identity flows through. Only custom mode needs the principal in the response.

Net effect: custom mode becomes "the authorizer IS the identity provider" (it returns who the user is and what groups/scopes they have, and policy lives in the authorizer), and per-user auditability is restored — logs show the real principal, not one synthetic admin account.

2. New config not propagated to the parameter reference or deployment surfaces.
AUTHORIZER_MODE, CUSTOM_AUTHORIZER_URL, CUSTOM_AUTHORIZER_TIMEOUT, CUSTOM_AUTHORIZER_API_KEY only exist in .env.example + docker-compose.yml.
How to fix:

  • Add a row for each to docs/unified-parameter-reference.md (fill Docker/Terraform/Helm columns; mark CUSTOM_AUTHORIZER_API_KEY as (secret)).
  • Add them to CONFIG_GROUPS in registry/api/config_routes.py so they surface in the System Config UI.
  • Wire them into the auth-server task definition under terraform/aws-ecs and into charts/auth-server (with CUSTOM_AUTHORIZER_API_KEY via secretKeyRef / Secrets Manager, not a plain value).

3. The integration tests are currently placeholders.
tests/integration/test_custom_authorizer_integration.py has 31 methods, but most are pass with commented-out pseudocode (only one real assert across the file), so they don't exercise anything yet. The PR test plan also references a slightly different path (tests/auth_server/integration/...).
How to fix: Flesh these out to drive /validate via FastAPI TestClient across all three modes — native (webhook not called), custom (native skipped, webhook decides, fail-closed 503 on timeout/unreachable), and both (native-then-webhook ordering, 401 if native fails before the webhook is called, 403 if the webhook denies). If they can't be completed in this PR, it's fine to remove the file and the test-plan line for now, and follow up separately — just so the PR doesn't imply coverage that isn't there yet.

4. Lint is failing (30 ruff errors).
Mostly Optional[X] / from typing import Optional and datetime.timezone.utc; CLAUDE.md mandates PEP 604/585.
How to fix: uv run ruff check --fix . then re-verify with uv run ruff check .. Convert Optional[X]X | None, timezone.utcdatetime.UTC.

Should fix (important)

5. Full request body + query params are egressed to the external authorizer — needs guardrails.
First, to be clear: this is not a regression. X-Body already exists on main (set by docker/lua/capture_body.lua, consumed by metrics_middleware.py), and the egress only runs in custom/both mode, so the default native path is unchanged. The concern is that once an operator enables custom/both, build_custom_auth_payload forwards the entire request body (the JSON-RPC tool-call payload — which can contain API keys, PII, prompts) and all query params to an external endpoint, masking only covers headers, and CUSTOM_AUTHORIZER_URL may be plain HTTP (warn-only). The authorizer is now part of the trust boundary, so this needs to be a conscious, bounded operator decision.

Please implement all of the following:

  • Body-forwarding opt-in, default off. Add an env var (e.g. CUSTOM_AUTHORIZER_FORWARD_BODY, default false). When false, set body=None in build_custom_auth_payload and forward only metadata (method, path, query params, masked headers, client_ip, native auth result). Most authorizers (including the Cedar reference in this PR) decide on identity + path + method and never need the body. Document the variable in .env.example with an explicit warning about what enabling it sends.

  • Body size cap when forwarding is on. Add CUSTOM_AUTHORIZER_MAX_BODY_BYTES (suggest default 65536). If the body exceeds the cap, truncate or drop it (set body=None) and add a flag in the payload context (e.g. body_truncated: true) so the authorizer knows. Prevents forwarding multi-MB payloads on every request.

  • Hard-fail plain HTTP in production. Today validate_custom_authorizer_config() only warns on non-localhost http://. Change it to raise ValueError for a non-localhost, non-HTTPS URL unless an explicit escape hatch is set (e.g. CUSTOM_AUTHORIZER_ALLOW_INSECURE=true). Localhost/127.0.0.1 stay allowed for dev. Since credentials and bodies cross this hop, plaintext should be a deliberate opt-in, not a log line.

  • Document the egress prominently in .env.example next to CUSTOM_AUTHORIZER_URL: state that in custom/both mode the request metadata (and, if FORWARD_BODY=true, the body) is sent to that endpoint, and that it should be HTTPS and trusted.

  • Consider query-param redaction. Query params are forwarded unmasked. If feasible, apply the same masking allowlist idea to known-sensitive param names (e.g. token, api_key, code), or note explicitly that query strings are sent verbatim so operators avoid putting secrets there.

6. docker-compose.override.yml is committed but auto-applies to everyone.
docker compose up auto-merges docker-compose.override.yml, so committing it adds the cedar service and a runtime pip install to every local stack by default — and the file's own header notes it shouldn't be committed.
How to fix: Either add it to .gitignore and document it, or rename to an opt-in file (e.g. docker-compose.cedar.yml) used with -f. Replace the pip install ... command with a small pinned Dockerfile, and drop the baked API_KEY=...:-default_api_key default.

7. Config read directly from os.environ instead of central settings.
The client reads CUSTOM_AUTHORIZER_* straight from os.environ; a bad CUSTOM_AUTHORIZER_TIMEOUT raises ValueError at request time.
How to fix: Route through registry.core.config.settings like the rest of the codebase, and validate the timeout is numeric in validate_custom_authorizer_config() so it fails at startup.

8. CUSTOM_AUTHORIZER_API_KEY secret hygiene.
How to fix: Covered partly by #2 — flag it as a secret in the reference and wire it via secretKeyRef/Secrets Manager rather than a plain env var.

Nice to fix

9. Move sample-custom-authorizer.py out of the repo root into an examples/ or docs/ location.

10. The webhook Pydantic contract is copy-pasted across auth_server/, cedar-authorizer/app.py, and sample-custom-authorizer.py — note the divergence risk or factor out a shared/published schema.

11. Narrow the broad except Exception around the custom/both blocks in server.py so genuine bugs aren't all masked as 503 "authorizer unavailable" (keep the fail-closed behavior; just log/classify more precisely).


Happy to help on the identity-contract design (#1) if useful — that's the most important one. The rest is mostly mechanical. Thanks again!

Cc: @omrishiv

@aarora79

Copy link
Copy Markdown
Contributor

Hi @nathanzilgo , just checking in, appreciate if you could please let us know your plans for this. Thanks.

@ceng-p

ceng-p commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Great work @nathanzilgo @GuilhermeAlz are you 2 working on addressing @aarora79's comments otherwise I can also take a look if you are able to add me as a contributor? Alternatively I can open a new PR building on this, whatever is easiest for you :)

@nathanzilgo

Copy link
Copy Markdown
Contributor Author

Hey @aarora79 @ceng-p, sorry for the late response on this.

I have been off the project since June, as I moved to another company, and there were a lot of changes in planning and priorities. I'm not sure if this implementation is still within the lab project's scope, so it might be best to align the next steps with @GuilhermeAlz.

That said, I will try to address the review comments regarding my part of the PR over the next few days (it was completely impossible for me before this).

Apologies again for the delay, and thank you for your patience and attention! 🙏

@GuilhermeAlz

Copy link
Copy Markdown

Hey guys, i'm also very sorry for the late response. As i am in the final months of my graduation, i lacked the time to contribute on my own. The project i am (and @nathanzilgo was) part of changed it's scope and our coordinator blocked us from contributing any further with this PR within the project's activities. I just submitted my final project and now i have more free time to contribute by myself. That said, i'll also try to adress my part of the PR over the next week. And no problem in adding @ceng-p as a contributor for me.

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.

Add custom authorizer endpoint support for external policy engine integration

5 participants