Skip to content

fix(security): harden auth — UUID-first user resolution, weak-password guard, streamable-HTTP team checks - #5872

Open
jonpspri wants to merge 7 commits into
IBM:mainfrom
jonpspri:fix/auth-hardening
Open

fix(security): harden auth — UUID-first user resolution, weak-password guard, streamable-HTTP team checks#5872
jonpspri wants to merge 7 commits into
IBM:mainfrom
jonpspri:fix/auth-hardening

Conversation

@jonpspri

Copy link
Copy Markdown
Collaborator

Closes #5864

Summary

Ports the auth-hardening fixes out of the catch-all PR #5417 into a standalone, reviewable change:

  • UUID-first user resolution (mcpgateway/auth.py): _get_user_by_email_sync() and _get_auth_context_batched_sync() now try EmailUser.id first when the sub looks like a UUID and fall back to email lookup. Team queries (personal team, active memberships) bind the resolved user.email instead of the raw sub parameter, so UUID-sub tokens resolve teams identically to email-form tokens. normalize_token_teams()/resolve_session_teams() remain the single token-scoping policy points.
  • Weak basic-auth password guard (mcpgateway/config.py): validate_security_combinations() now rejects a weak/default BASIC_AUTH_PASSWORD when any basic-auth method (Admin UI, API, docs) is enabled in a non-development environment. Also adds the PASSWORD_CHECK_SEQUENTIAL_CHARS setting (honored by the password policy service; exposed in .env.example and the Helm chart values) and converts apply_environment_aware_defaults to model_validator(mode="after") so require_strong_secrets is derived after all field values are set.
  • Streamable-HTTP auth fixes (mcpgateway/transports/streamablehttp_transport.py): _check_streamable_permission() derives team_id from single-team tokens and sets check_any_team for multi-team tokens so team-scoped RBAC roles are actually evaluated; require_user_in_db no longer rejects tokens whose is_admin claim is valid; the UUID→email resolution from the batched auth context is propagated into the request user context.
  • Sanitized RBAC logging (mcpgateway/services/permission_service.py): debug/info traces around check_permission and _get_user_roles, all identity fields passed through SecurityValidator.sanitize_log_message.

Security notes

  • Deny-path regression coverage: unauthenticated context denied up-front, non-admin token with no DB row still rejected by require_user_in_db, public-only tokens get no team scope, weak basic-auth password rejected in staging/production but still allowed in development and when no basic-auth method is enabled.
  • No behavior change to token-scoping policy functions; no new transports or flags gated behind this change beyond the documented PASSWORD_CHECK_SEQUENTIAL_CHARS default-true setting.

jonpspri added 4 commits July 25, 2026 10:16
…ecks

- _get_user_by_email_sync/_get_auth_context_batched_sync: try EmailUser.id
  first when the sub looks like a UUID, fall back to email; team queries now
  bind the resolved user.email instead of the raw sub parameter
- streamablehttp _auth_jwt: propagate the resolved email into the request
  user context; require_user_in_db no longer rejects tokens whose is_admin
  claim is valid
- _check_streamable_permission: derive team_id from single-team tokens and
  set check_any_team for multi-team tokens so team-scoped RBAC roles are
  evaluated on the streamable-HTTP transport

Signed-off-by: Jonathan Springer <jps@s390x.com>
…icy setting

- Reject a weak/default BASIC_AUTH_PASSWORD at settings validation when any
  basic-auth method (Admin UI, API, docs) is enabled outside development
- Add PASSWORD_CHECK_SEQUENTIAL_CHARS setting and honor it in the password
  policy service so operators can disable the sequential-character check
- apply_environment_aware_defaults: run as a model_validator(mode='after')
  so require_strong_secrets is derived after all field values are set

Signed-off-by: Jonathan Springer <jps@s390x.com>
… and helm values

Signed-off-by: Jonathan Springer <jps@s390x.com>
Add SecurityValidator-sanitized debug/info traces around check_permission
and _get_user_roles so RBAC deny/grant paths are diagnosable without
leaking unsanitized identity data into logs

Signed-off-by: Jonathan Springer <jps@s390x.com>
jonpspri added 3 commits July 25, 2026 10:36
The baseline regen was a detect-secrets hook side effect of the
.env.example change and is out of scope for this PR.

Signed-off-by: Jonathan Springer <jps@s390x.com>
…t neutral

Convert both config-surface additions to 1-for-1 line replacements so
baseline-tracked line numbers in .secrets.baseline do not shift and the
detect-secrets hook stays green without regenerating the baseline.

Signed-off-by: Jonathan Springer <jps@s390x.com>
- Rewrite the obsolete before-validator passthrough test to cover the new
  model_validator(mode='after') contract (require_strong_secrets derived
  from environment after fields are set)
- Make _make_settings hermetic against runner environments that export
  basic-auth toggles (CI exports MCPGATEWAY_UI_ENABLED=true, which combined
  with the new weak basic_auth_password guard broke
  test_strong_key_passes_in_production)

Signed-off-by: Jonathan Springer <jps@s390x.com>
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.

[BUG]: Auth hardening — UUID resolution, weak basic-auth password validation, streamable-HTTP auth fixes

1 participant