Skip to content

Fix #3785#3792

Merged
bgavrilMS merged 1 commit into
masterfrom
jmprieur/fix3785
Apr 27, 2026
Merged

Fix #3785#3792
bgavrilMS merged 1 commit into
masterfrom
jmprieur/fix3785

Conversation

@jmprieur
Copy link
Copy Markdown
Collaborator

Fixes #3785

(Microsoft.Identity.Web.UI / AccountController.Challenge):

  • Reject redirectUri values that pass Url.IsLocalUrl() but decode to a protocol-relative form (leading %2F%2F...), which Cookie auth would emit verbatim in Location and some clients decode to //host before following. New IsPercentEncodedSlashBypass helper runs after both raw input and Uri-coerced candidate.
  • 44 tests on net8/net9 (16 unit + 28 integration).

(Microsoft.Identity.Web / MapLoginAndLogout):

  • Remove .DisableAntiforgery() from /logout.
  • Add .RequireAuthorization() as primary CSRF gate.
  • Validate IAntiforgery.IsRequestValidAsync(context) in the handler when IAntiforgery is registered, independent of whether UseAntiforgery() middleware is in the pipeline. Makes /logout pipeline-shape-agnostic (Blazor, MVC, minimal-API hosts all get equivalent protection).
  • When IAntiforgery is not registered, log a one-time warning at MapLoginAndLogout time (EventId=1, AntiforgeryNotRegistered) so operators can see the graceful-degradation state; RequireAuthorization
    • SameSite=Lax cookies remain as the gate.
  • 22 tests on net8/net9/net10 (16 unit + 6 integration including MVC-shape graceful-degradation coverage).

Fixes #3785

@jmprieur jmprieur requested a review from a team as a code owner April 24, 2026 14:37
F1 (Microsoft.Identity.Web.UI / AccountController.Challenge):
- Reject redirectUri values that pass Url.IsLocalUrl() but decode to
  a protocol-relative form (leading %2F%2F...), which Cookie auth would
  emit verbatim in Location and some clients decode to //host before
  following. New IsPercentEncodedSlashBypass helper runs after both raw
  input and Uri-coerced candidate.

F3 (Microsoft.Identity.Web / MapLoginAndLogout):
- Remove .DisableAntiforgery() from /logout.
- Add .RequireAuthorization() as primary CSRF gate.
- Validate IAntiforgery.IsRequestValidAsync(context) in the handler
  when IAntiforgery is registered, independent of whether UseAntiforgery()
  middleware is in the pipeline. Makes /logout pipeline-shape-agnostic
  (Blazor, MVC, minimal-API hosts all get equivalent protection).
- When IAntiforgery is not registered, log a one-time warning at
  MapLoginAndLogout time (EventId=1, AntiforgeryNotRegistered) so
  operators can see the graceful-degradation state; RequireAuthorization
  + SameSite=Lax cookies remain as the gate.

Tests:
- F1: 44/44 on net8/net9 (16 unit + 28 integration).
- F3: 22/22 on net8/net9/net10 (16 unit + 6 integration including
  MVC-shape graceful-degradation coverage).

Reviewed independently by two models (both passes sign-off).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@bgavrilMS bgavrilMS merged commit 9a628ab into master Apr 27, 2026
4 checks passed
@bgavrilMS bgavrilMS deleted the jmprieur/fix3785 branch April 27, 2026 12:31
@bgavrilMS
Copy link
Copy Markdown
Member

Tested manually, LGTM. Thanks @jmprieur

This was referenced May 1, 2026
This was referenced May 24, 2026
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.

Do not use DisableAntiforgery on logout controller

2 participants