Reauthorize tools changed by call-tool filters

[PranavSenthilnathan]

Summary

• keep the existing alternate-result authorization pass outside Tasks so unauthorized calls are rejected before task creation
• add one ordinary authorization checkpoint for each AddAuthorizationFilters() registration so an intervening call-tool filter can replace and reauthorize the matched tool
• remove the Tasks registration-order restriction because ordinary filters execute inside task-backed calls regardless of registration order
• preserve authorization reevaluation at every explicit checkpoint, including changes to the matched tool or user
• add regression coverage for direct and task-backed tool replacement, pre-task denial, and ordinary filters registered before Tasks

Closes #1733

Testing

• dotnet build
• dotnet test tests\ModelContextProtocol.AspNetCore.Tests\ModelContextProtocol.AspNetCore.Tests.csproj --no-build --filter "FullyQualifiedName~AuthorizeAttributeTests|FullyQualifiedName~HttpTaskIntegrationTests"
• dotnet test --no-build (all ASP.NET Core tests pass; existing Core integration tests fail because TestServer.exe is not available on PATH)

[jeffhandley]

The lack of API surface area changes, the new tests, and changes to the comments look good to me. I'll defer to @halter73 and/or @tarekgh for the implementation review.

[tarekgh]

Reviewed the change in depth. It is correct and the Tasks order-guard removal is justified (ordinary filters always run inside the task background via the alternate pipeline, so the old restriction was over-strict). A few risks/gaps and an alternative worth considering:

Risks / gaps

1. Reauthorization is opt-in and order-dependent. Correctness relies on the developer calling AddAuthorizationFilters() again, after the tool-changing filter. A single call before the filter silently under-authorizes the replaced tool, guarded only by a one-line remark.
2. Double policy evaluation in the common case. Any tool with auth metadata is now authorized twice per call (the pre-task alternate filter plus the new ordinary filter), even when nothing changed the primitive. Custom IAuthorizationHandlers (DB/remote lookups, audit logging) fire twice. Reauthorize tools changed by intervening call-tool filters #1733 suggested caching the last-authorized primitive to avoid this; that is not implemented (the code sets AuthorizationFilterInvokedKey but never reads it to short-circuit).
3. Calling AddAuthorizationFilters() twice re-adds all list/read/get auth filters too, not just the call-tool one, so those operations authorize twice as well. Redundant, not incorrect.
4. The "unauthorized call does not create a task" guarantee only covers the original matched tool. A filter that swaps to an unauthorized tool is caught only in the background, after the task is created.

Alternative
A single terminal reauthorization (one always-last ordinary filter that authorizes the final MatchedPrimitive) would cover any number of swaps with no developer ordering discipline, removing risk 1 entirely, since only the executed tool matters for authorization. Combined with the #1733 caching hint (skip when unchanged from the pre-task authorized primitive) it removes the double-eval in risks 2 and 3, and is strictly safer and cheaper than manual re-registration.

Tests
Happy paths are well covered. Missing: a test documenting that a single pre-filter registration does not reauthorize (the footgun), and a multi-swap-across-multiple-filters case.

[PranavSenthilnathan]

@tarekgh Thanks for the detailed review. I kept the explicit ordered-checkpoint design because that is both the pre-#1722 behavior and the behavior requested by #1733: each AddAuthorizationFilters() call authorizes the tool/user at that point in the ordinary filter pipeline. A terminal always-last filter would be a different contract: a registration before arbitrary filters would implicitly authorize after all of them, and repeated registrations would no longer represent checkpoints. That may be worth considering separately as a safer-default API, but it is not required to restore this regression.

I pushed 2c38637 to make the checkpoint semantics explicit in the API remarks (including changes to either MatchedPrimitive or User) and added the missing multi-swap regression test.

On the other points:

1. I did not add a test asserting that an omitted post-transformation checkpoint under-authorizes, because that would lock the absence of a requested checkpoint in as expected behavior. The API remarks now state the ordering requirement directly.
2. The duplicate evaluation is intentional here. The outer alternate check prevents task creation for an unauthorized original tool; the ordinary checkpoint preserves the explicit pipeline semantics. Caching only the primitive is not correct because context.User is mutable, and even the same ClaimsPrincipal can be mutated. A later checkpoint can therefore need to re-evaluate the same primitive against changed authorization state.
3. Repeated list/read/get filters are redundant but preserve the pre-Fix HTTP task filter composition #1722 behavior and are not a correctness issue. Deduplicating those independently can be a separate optimization.
4. The replacement is only known after the Tasks alternate filter dispatches ordinary execution. Preventing task creation for that replacement would require running the transforming ordinary filter outside Tasks, which would change filter/task semantics. The replacement is still denied before tool execution inside the task.

[halter73]

I agree it'd be nice to avoid running auth in both the legacy and new experimental alternate-tool-result pipeline, but I think it's the best approach currently.

[tarekgh]

@PranavSenthilnathan Thanks, your responses address the points well. Agreed the checkpoint contract matches #1733, and points 2 and 4 are valid (User is mutable so a primitive-only cache is unsafe, and the replacement is only known after Tasks dispatches). The PR as-is is good enough for the current release.

One follow-up worth tracking: the checkpoint model is fail-open by default. If an author rewrites MatchedPrimitive and forgets the post-swap AddAuthorizationFilters(), the replaced tool runs with no policy evaluation and nothing detects it (list/read/get have Check* guards, but call-tool has no equivalent that catches a swap, since the invoked-key is already set on the original). A terminal guard that verifies the final MatchedPrimitive was authorized against the final user would make this fail-closed.

This can be done later without a breaking change: it is internal wiring in PostConfigure plus tracking the last-authorized (primitive, user) in context.Items, no public API or wire change, and it only flips an already-insecure path from allow to deny. Could you open a tracking issue for it? Not required for this PR.