Skip to content

fix(auth): stop leaking oauth client secrets - #327

Open
failsafesecurity wants to merge 1 commit into
tadata-org:mainfrom
failsafesecurity:fix/oauth-register-no-secret
Open

fix(auth): stop leaking oauth client secrets#327
failsafesecurity wants to merge 1 commit into
tadata-org:mainfrom
failsafesecurity:fix/oauth-register-no-secret

Conversation

@failsafesecurity

Copy link
Copy Markdown

Summary

This stops the fake OAuth dynamic registration endpoint from returning the configured upstream client_secret to arbitrary callers.

Security impact

When AuthConfig.setup_proxies=True and the default setup_fake_dynamic_registration=True are used together, POST /oauth/register is mounted and currently echoes the configured client_secret back to any caller. That lets an anonymous network client recover the server's OAuth client secret and impersonate the MCP server's upstream OAuth client.

Changes

  • stop populating client_secret in the fake registration response
  • force the fake registration response to behave like a public client (token_endpoint_auth_method="none")
  • omit None fields from the response body
  • add a regression test covering POST /oauth/register

Validation

  • python3 -m compileall fastapi_mcp tests/test_auth_proxy.py
  • .venv/bin/python -m pytest -q tests/test_auth_proxy.py tests/test_types_validation.py -o addopts=''

Disclosure notes

  • repo: tadata-org/fastapi_mcp
  • finding: unauthenticated /oauth/register client secret disclosure
  • no public issue was opened

Edgarruiz8585

This comment was marked as resolved.

@failsafesecurity

Copy link
Copy Markdown
Author

Hi maintainers 👋 — friendly ping on this security fix PR. It's been open for a couple of weeks without review. Happy to adjust the approach, add tests, or split the change if there are scope or compatibility concerns. If you'd prefer to continue privately via security@ or GitHub private reporting, I can resubmit there.

@K4bain

K4bain commented Aug 27, 2026

Copy link
Copy Markdown

@failsafesecurity — I adopted your patch from this PR and opened #333 with it (rebased onto current main, plus a regression test that fails without the fix: the registration response really does contain the client secret today). Full suite 70/70 green. Full credit to you in the commit message and PR description — thank you for flagging this, and hopefully this helps it land.

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.

3 participants