Check iss in odic sign-out (#6378)

Author: Kahbazi

src/Security/Authentication/OpenIdConnect/src/LoggingExtensions.cs

@@ -57,6 +57,8 @@ internal static class LoggingExtensions
         private static Action<ILogger, Exception> _remoteSignOutSessionIdInvalid;
         private static Action<ILogger, string, Exception> _authenticationSchemeSignedOut;
         private static Action<ILogger, string, string, Exception> _handleChallenge;
+        private static Action<ILogger, Exception> _remoteSignOutIssuerMissing;
+        private static Action<ILogger, Exception> _remoteSignOutIssuerInvalid;
 
         static LoggingExtensions()
         {
@@ -264,7 +266,17 @@ static LoggingExtensions()
             _handleChallenge = LoggerMessage.Define<string, string>(
                 eventId: new EventId(53, "HandleChallenge"),
                 logLevel: LogLevel.Debug,
-                formatString: "HandleChallenge with Location: {Location}; and Set-Cookie: {Cookie}.");                
+                formatString: "HandleChallenge with Location: {Location}; and Set-Cookie: {Cookie}.");
+            _remoteSignOutIssuerMissing = LoggerMessage.Define(
+              eventId: new EventId(54, "RemoteSignOutIssuerMissing"),
+              logLevel: LogLevel.Error,
+              formatString: "The remote signout request was ignored because the 'iss' parameter " +
+                            "was missing, which may indicate an unsolicited logout.");
+            _remoteSignOutIssuerInvalid = LoggerMessage.Define(
+              eventId: new EventId(55, "RemoteSignOutIssuerInvalid"),
+               logLevel: LogLevel.Error,
+               formatString: "The remote signout request was ignored because the 'iss' parameter didn't match " +
+                             "the expected value, which may indicate an unsolicited logout.");
         }
 
         public static void UpdatingConfiguration(this ILogger logger)
@@ -514,5 +526,15 @@ public static void AuthenticationSchemeSignedOut(this ILogger logger, string aut
 
         public static void HandleChallenge(this ILogger logger, string location, string cookie)
             => _handleChallenge(logger, location, cookie, null);
+
+        public static void RemoteSignOutIssuerMissing(this ILogger logger)
+        {
+            _remoteSignOutIssuerMissing(logger, null);
+        }
+
+        public static void RemoteSignOutIssuerInvalid(this ILogger logger)
+        {
+            _remoteSignOutIssuerInvalid(logger, null);
+        }
     }
 }

src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs

@@ -123,10 +123,9 @@ protected virtual async Task<bool> HandleRemoteSignOutAsync()
             // If the identifier cannot be found, bypass the session identifier checks: this may indicate that the
             // authentication cookie was already cleared, that the session identifier was lost because of a lossy
             // external/application cookie conversion or that the identity provider doesn't support sessions.
-            var sid = (await Context.AuthenticateAsync(Options.SignOutScheme))
-                          ?.Principal
-                          ?.FindFirst(JwtRegisteredClaimNames.Sid)
-                          ?.Value;
+            var principal = (await Context.AuthenticateAsync(Options.SignOutScheme))?.Principal;
+
+            var sid = principal?.FindFirst(JwtRegisteredClaimNames.Sid)?.Value;
             if (!string.IsNullOrEmpty(sid))
             {
                 // Ensure a 'sid' parameter was sent by the identity provider.
@@ -143,6 +142,23 @@ protected virtual async Task<bool> HandleRemoteSignOutAsync()
                 }
             }
 
+            var iss = principal?.FindFirst(JwtRegisteredClaimNames.Iss)?.Value;
+            if (!string.IsNullOrEmpty(iss))
+            {
+                // Ensure a 'iss' parameter was sent by the identity provider.
+                if (string.IsNullOrEmpty(message.Iss))
+                {
+                    Logger.RemoteSignOutIssuerMissing();
+                    return true;
+                }
+                // Ensure the 'iss' parameter corresponds to the 'iss' stored in the authentication ticket.
+                if (!string.Equals(iss, message.Iss, StringComparison.Ordinal))
+                {
+                    Logger.RemoteSignOutIssuerInvalid();
+                    return true;
+                }
+            }
+
             Logger.RemoteSignOut();
 
             // We've received a remote sign-out request

src/Security/Authentication/test/OpenIdConnect/OpenIdConnectTests.cs

@@ -5,6 +5,7 @@
 using System.Globalization;
 using System.Linq;
 using System.Net;
+using System.Security.Claims;
 using System.Text.Encodings.Web;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Authentication.Cookies;
@@ -294,6 +295,77 @@ public async Task SignOut_WithMissingConfig_Throws()
             Assert.Equal("Cannot redirect to the end session endpoint, the configuration may be missing or invalid.", exception.Message);
         }
 
+        [Fact]
+        public async Task RemoteSignOut_WithMissingIssuer()
+        {
+            var settings = new TestSettings(o =>
+            {
+                o.SignInScheme = CookieAuthenticationDefaults.AuthenticationScheme;
+                o.Authority = TestServerBuilder.DefaultAuthority;
+                o.ClientId = "Test Id";
+            });
+            var server = settings.CreateTestServer(handler: async context =>
+            {
+                var claimsIdentity = new ClaimsIdentity("Cookies");
+                claimsIdentity.AddClaim(new Claim("iss", "test"));
+                await context.SignInAsync(new ClaimsPrincipal(claimsIdentity));
+            });
+
+            var signInTransaction = await server.SendAsync(DefaultHost);
+
+            var remoteSignOutTransaction = await server.SendAsync(DefaultHost + "/signout-oidc", signInTransaction.AuthenticationCookieValue);
+            Assert.Equal(HttpStatusCode.OK, remoteSignOutTransaction.Response.StatusCode);
+            Assert.DoesNotContain(remoteSignOutTransaction.Response.Headers, h => h.Key == "Set-Cookie");
+
+        }
+
+        [Fact]
+        public async Task RemoteSignOut_WithInvalidIssuer()
+        {
+            var settings = new TestSettings(o =>
+            {
+                o.SignInScheme = CookieAuthenticationDefaults.AuthenticationScheme;
+                o.Authority = TestServerBuilder.DefaultAuthority;
+                o.ClientId = "Test Id";
+            });
+            var server = settings.CreateTestServer(handler: async context =>
+            {
+                var claimsIdentity = new ClaimsIdentity("Cookies");
+                claimsIdentity.AddClaim(new Claim("iss", "test"));
+                await context.SignInAsync(new ClaimsPrincipal(claimsIdentity));
+            });
+
+            var signInTransaction = await server.SendAsync(DefaultHost);
+
+            var remoteSignOutTransaction = await server.SendAsync(DefaultHost + "/signout-oidc?iss=invalid", signInTransaction.AuthenticationCookieValue);
+            Assert.Equal(HttpStatusCode.OK, remoteSignOutTransaction.Response.StatusCode);
+            Assert.DoesNotContain(remoteSignOutTransaction.Response.Headers, h => h.Key == "Set-Cookie");
+        }
+
+        [Fact]
+        public async Task RemoteSignOut_Get_Successful()
+        {
+            var settings = new TestSettings(o =>
+            {
+                o.SignInScheme = CookieAuthenticationDefaults.AuthenticationScheme;
+                o.Authority = TestServerBuilder.DefaultAuthority;
+                o.ClientId = "Test Id";
+            });
+            var server = settings.CreateTestServer(handler: async context =>
+            {
+                var claimsIdentity = new ClaimsIdentity("Cookies");
+                claimsIdentity.AddClaim(new Claim("iss", "test"));
+                claimsIdentity.AddClaim(new Claim("sid", "something"));
+                await context.SignInAsync(new ClaimsPrincipal(claimsIdentity));
+            });
+
+            var signInTransaction = await server.SendAsync(DefaultHost);
+
+            var remoteSignOutTransaction = await server.SendAsync(DefaultHost + "/signout-oidc?iss=test&sid=something", signInTransaction.AuthenticationCookieValue);
+            Assert.Equal(HttpStatusCode.OK, remoteSignOutTransaction.Response.StatusCode);
+            Assert.Contains(remoteSignOutTransaction.Response.Headers, h => h.Key == "Set-Cookie");
+        }
+
         // Test Cases for calculating the expiration time of cookie from cookie name
         [Fact]
         public void NonceCookieExpirationTime()

src/Security/Authentication/test/OpenIdConnect/TestSettings.cs

@@ -14,6 +14,7 @@
 using System.Threading.Tasks;
 using System.Xml.Linq;
 using Microsoft.AspNetCore.Authentication.OpenIdConnect;
+using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.TestHost;
 using Microsoft.IdentityModel.Protocols.OpenIdConnect;
 using Xunit;
@@ -46,7 +47,7 @@ public TestSettings(Action<OpenIdConnectOptions> configure)
 
         public string ExpectedState { get; set; }
 
-        public TestServer CreateTestServer(AuthenticationProperties properties = null) => TestServerBuilder.CreateServer(_configureOptions, handler: null, properties: properties);
+        public TestServer CreateTestServer(AuthenticationProperties properties = null, Func<HttpContext, Task> handler = null) => TestServerBuilder.CreateServer(_configureOptions, handler: handler, properties: properties);
 
         public IDictionary<string, string> ValidateChallengeFormPost(string responseBody, params string[] parametersToValidate)
         {
@@ -347,4 +348,4 @@ private async Task<HttpResponseMessage> ReturnResource(string resource)
             }
         }
     }
-}
+}

src/Security/Authentication/test/OpenIdConnect/TestTransaction.cs

@@ -26,7 +26,7 @@ public string AuthenticationCookieValue
             {
                 if (SetCookie != null && SetCookie.Count > 0)
                 {
-                    var authCookie = SetCookie.SingleOrDefault(c => c.Contains(".AspNetCore.Cookie="));
+                    var authCookie = SetCookie.SingleOrDefault(c => c.Contains(".AspNetCore.Cookies="));
                     if (authCookie != null)
                     {
                         return authCookie.Substring(0, authCookie.IndexOf(';'));
@@ -37,4 +37,4 @@ public string AuthenticationCookieValue
             }
         }
     }
-}
+}