Apply consistent redirect-URI validation on AccountController.SignIn (#3940)

SignIn was missing the IsPercentEncodedSlashBypass guard that Challenge
already had. A redirectUri like /%2fevil.example passes Url.IsLocalUrl
(the framework only checks for literal // and /\), but a normalising
reverse proxy decodes it to //evil.example; browsers then follow it as
a protocol-relative URL after sign-in.

Add !IsPercentEncodedSlashBypass(redirectUri) to the SignIn guard,
matching the guard on Challenge.

Also update the IsPercentEncodedSlashBypass XML doc to note it is a
local copy of RedirectUriHelper.HasPercentEncodedSlashPrefix (internal
in a separate assembly) so future changes keep both in sync.

Tests: two new facts in AccountControllerTests covering the %2f and %5c
variants on the SignIn path, using the realistic IsLocalUrl mock so
removing either guard breaks the tests.


Copilot-Session: 00c4f24d-188f-4bd9-926c-a6d0620eb053

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: iNinja

src/Microsoft.Identity.Web.UI/Areas/MicrosoftIdentity/Controllers/AccountController.cs

@@ -53,7 +53,7 @@ public IActionResult SignIn(
         {
             scheme ??= OpenIdConnectDefaults.AuthenticationScheme;
             string redirect;
-            if (!string.IsNullOrEmpty(redirectUri) && Url.IsLocalUrl(redirectUri))
+            if (!string.IsNullOrEmpty(redirectUri) && Url.IsLocalUrl(redirectUri) && !IsPercentEncodedSlashBypass(redirectUri))
             {
                 redirect = redirectUri;
             }
@@ -254,6 +254,12 @@ private static bool IsSameOrigin(Uri absolute, HttpRequest request)
         /// closes. Comparison is case-insensitive because the RFC 3986 encoding is
         /// hex-case-insensitive.
         /// </summary>
+        /// <remarks>
+        /// Canonical implementation: <c>RedirectUriHelper.HasPercentEncodedSlashPrefix</c> in
+        /// <c>Microsoft.Identity.Web</c>. This private copy exists because <c>RedirectUriHelper</c>
+        /// is internal and <c>Microsoft.Identity.Web.UI</c> is a separate assembly. Keep both
+        /// in sync.
+        /// </remarks>
         private static bool IsPercentEncodedSlashBypass(string path) =>
             path.StartsWith("/%2f", StringComparison.OrdinalIgnoreCase)
             || path.StartsWith("/%5c", StringComparison.OrdinalIgnoreCase);

tests/Microsoft.Identity.Web.UI.Test/Areas/MicrosoftIdentity/Controllers/AccountControllerTests.cs

@@ -439,5 +439,51 @@ public void Challenge_WithSameOriginAbsoluteButSlashBackslashPathAndQuery_UsesDe
             var challengeResult = Assert.IsType<ChallengeResult>(result);
             Assert.Equal("/", challengeResult.Properties!.RedirectUri);
         }
+
+        // -----------------------------------------------------------------------------
+        // SignIn action — percent-encoded slash bypass.
+        //
+        // A redirectUri of "/%2fevil.example" (the value the controller sees after ASP.NET
+        // Core has query-decoded the request once — the raw query string would be "%252f")
+        // passes Url.IsLocalUrl because the framework checks for literal "//" and "/\" only.
+        // A normalising reverse proxy (NGINX, IIS ARR, F5) that decodes percent-encoded
+        // path segments on the way out turns that into "//evil.example", which browsers
+        // treat as a protocol-relative URL. The IsPercentEncodedSlashBypass guard must
+        // block this on the SignIn path, matching the guard already on the Challenge action.
+        // -----------------------------------------------------------------------------
+
+        [Fact]
+        public void SignIn_WithPercentEncodedSlashRedirectUri_UsesDefaultRedirectUri()
+        {
+            // Arrange: use a realistic IsLocalUrl mock so that removing either the
+            // IsLocalUrl check or the IsPercentEncodedSlashBypass check would cause this
+            // test to fail.
+            UseRealisticIsLocalUrl();
+            string scheme = OpenIdConnectDefaults.AuthenticationScheme;
+
+            // "/%2fevil.example" — passes IsLocalUrl (starts with "/" and second char is "%"),
+            // but must be rejected by IsPercentEncodedSlashBypass.
+            // Act
+            var result = _accountController.SignIn(scheme, "/%2fevil.example");
+
+            // Assert: redirect must fall back to default, not to the attacker-controlled URI.
+            var challengeResult = Assert.IsType<ChallengeResult>(result);
+            Assert.Equal("/", challengeResult.Properties!.RedirectUri);
+        }
+
+        [Fact]
+        public void SignIn_WithPercentEncodedBackslashRedirectUri_UsesDefaultRedirectUri()
+        {
+            // Arrange: "/%5cevil.example" — backslash variant of the same bypass.
+            UseRealisticIsLocalUrl();
+            string scheme = OpenIdConnectDefaults.AuthenticationScheme;
+
+            // Act
+            var result = _accountController.SignIn(scheme, "/%5cevil.example");
+
+            // Assert
+            var challengeResult = Assert.IsType<ChallengeResult>(result);
+            Assert.Equal("/", challengeResult.Properties!.RedirectUri);
+        }
     }
 }