fix(server): Authorization: Bearer always accepted; legacy headers additive (#720)#721
Merged
Merged
Conversation
…ditive (#720) ``BearerTokenAuthMiddleware`` previously took an EXCLUSIVE ``header_name`` kwarg: setting it to a custom value (e.g. ``x-adcp-auth``) silently rejected every request carrying the spec-canonical ``Authorization: Bearer`` header. Every spec-compliant client — browsers, off-the-shelf HTTP libraries, the AdCP ``security_baseline/probe_api_key`` storyboard — failed against the middleware named for RFC 6750 because the middleware actively ignored the RFC 6750 carrier. This PR inverts the semantics: - ``Authorization: Bearer <token>`` is always accepted. Cannot be turned off — it's the spec and it's the class's own name. - Legacy custom headers become additive opt-in aliases. New middleware kwargs: ``legacy_header_aliases``, ``legacy_aliases_bearer_prefix_required``. Resolution order: Authorization first; if absent, each alias in order, first non-empty wins. - ``header_name`` / ``bearer_prefix_required`` are deprecated with a ``DeprecationWarning`` on construction. Folded into the alias list internally so existing configs keep working — but the behavior CHANGES from EXCLUSIVE to ADDITIVE. - ``BearerTokenAuth`` dataclass mirrors: new ``legacy_header_aliases`` / ``mcp_legacy_header_aliases`` / ``a2a_legacy_header_aliases`` fields. ``resolved_mcp_legacy_aliases()`` / ``resolved_a2a_legacy_aliases()`` accessors. Per-leg ``mcp_header_name`` / ``a2a_header_name`` emit ``DeprecationWarning``. - ``_wrap_mcp_with_auth`` threads the resolved alias list instead of the legacy single-header kwarg. Adopter migration: # Was: rejects spec-compliant clients silently BearerTokenAuth(validate_token=..., mcp_header_name="x-adcp-auth", mcp_bearer_prefix_required=False) # Is: both wire carriers work simultaneously BearerTokenAuth(validate_token=..., mcp_legacy_header_aliases=("x-adcp-auth",)) Tests: 7 new middleware-level + 6 new dataclass-level. 3 existing tests pinned the old EXCLUSIVE behavior and are renamed/re-targeted. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Reject bare-string alias values (``mcp_legacy_header_aliases="x-adcp-auth"`` without the trailing comma). Pre-fix, the resolver would walk it letter-by-letter. Loud ``ValueError`` at construction with a hint pointing at the fix — same loud-fail posture as the existing empty-string check on ``header_name``. - Reject ``"authorization"`` in any of the new-shape alias fields at construction. The spec-canonical header is always accepted by the middleware; listing it as an alias is a no-op and almost always a config error. - Type ``*_legacy_header_aliases`` fields as ``Sequence[str] | None`` so list literals work alongside tuples. Pairs with the trailing-comma foot-gun guard — agents generating list-shaped config no longer hit a frozen-dataclass type error. - Update the dataclass docstring's worked example to show the new ``mcp_legacy_header_aliases=[...]`` shape instead of the deprecated ``mcp_header_name=...``. - Comment in ``_wrap_mcp_with_auth`` explicitly says the dataclass- level deprecation warning is the only one that fires (the middleware-level warning is suppressed because the dataclass routes through the alias path). - Comment in ``_merge_alias_sources`` notes the preserved alias casing is display-only — the middleware lowercases at construction before wire matching. - 5 new tests pin the contracts: bare-string foot-gun rejection, ``authorization`` rejection, empty-alias rejection, list-form acceptance, no-spurious-warning on the new-shape happy path. - 1 existing test renamed and re-targeted: it pinned the silent-drop behavior of ``authorization`` listed as an alias; the new validation makes that a construction error, so the test now exercises the back-compat ``header_name="authorization"`` shim path's filter instead. 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.
Closes #720.
Summary
BearerTokenAuthMiddlewarepreviously took an EXCLUSIVEheader_namekwarg — setting it to a custom value (e.g.x-adcp-auth) silently rejected every request carrying the spec-canonicalAuthorization: Bearerheader. Every spec-compliant client (browsers, off-the-shelf HTTP libraries, the AdCPsecurity_baseline/probe_api_keystoryboard) failed against the middleware named for RFC 6750 because the middleware actively ignored the RFC 6750 carrier.This PR inverts the semantics:
Authorization: Beareris always accepted; legacy custom headers become additive opt-in aliases.Surface changes
BearerTokenAuthMiddleware— newlegacy_header_aliases: list[str] | None+legacy_aliases_bearer_prefix_required: boolkwargs. Oldheader_name/bearer_prefix_requireddeprecated, folded into the alias list internally; behavior shifts from EXCLUSIVE → ADDITIVE.BearerTokenAuthdataclass — newlegacy_header_aliases/mcp_legacy_header_aliases/a2a_legacy_header_aliasesfields. Newresolved_mcp_legacy_aliases()/resolved_a2a_legacy_aliases()accessors. Per-legmcp_header_name/a2a_header_nameemitDeprecationWarning._wrap_mcp_with_auththreads the resolved alias list to the middleware instead of the legacy single-header kwarg.Behavior change (changelog-worthy)
Pre-#720:
mcp_header_name=\"x-adcp-auth\"made the middleware ONLY checkx-adcp-auth. Spec-compliant clients silently 401'd.Post-#720: same config makes
x-adcp-authan additive alias;Authorization: Beareris also accepted. Existing legacy clients keep working; spec-compliant clients start working. The deprecation warning surfaces the migration.Adopter migration
Test plan
ruff+mypyclean on touched files.🤖 Generated with Claude Code