diff --git a/src/Security/Authentication/OpenIdConnect/src/LoggingExtensions.cs b/src/Security/Authentication/OpenIdConnect/src/LoggingExtensions.cs index 791eb6064b..d2ae62a5bd 100644 --- a/src/Security/Authentication/OpenIdConnect/src/LoggingExtensions.cs +++ b/src/Security/Authentication/OpenIdConnect/src/LoggingExtensions.cs @@ -57,6 +57,8 @@ namespace Microsoft.Extensions.Logging private static Action _remoteSignOutSessionIdInvalid; private static Action _authenticationSchemeSignedOut; private static Action _handleChallenge; + private static Action _remoteSignOutIssuerMissing; + private static Action _remoteSignOutIssuerInvalid; static LoggingExtensions() { @@ -264,7 +266,17 @@ namespace Microsoft.Extensions.Logging _handleChallenge = LoggerMessage.Define( 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 @@ namespace Microsoft.Extensions.Logging 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); + } } } diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs index fe6a03a3ea..40e0eacbb8 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs @@ -123,10 +123,9 @@ namespace Microsoft.AspNetCore.Authentication.OpenIdConnect // 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 @@ namespace Microsoft.AspNetCore.Authentication.OpenIdConnect } } + 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 diff --git a/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectTests.cs b/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectTests.cs index da52e0e4cb..76de0d29ff 100644 --- a/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectTests.cs +++ b/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectTests.cs @@ -5,6 +5,7 @@ using System; 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 @@ namespace Microsoft.AspNetCore.Authentication.Test.OpenIdConnect 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() diff --git a/src/Security/Authentication/test/OpenIdConnect/TestSettings.cs b/src/Security/Authentication/test/OpenIdConnect/TestSettings.cs index eb045b4381..76d003cc31 100644 --- a/src/Security/Authentication/test/OpenIdConnect/TestSettings.cs +++ b/src/Security/Authentication/test/OpenIdConnect/TestSettings.cs @@ -14,6 +14,7 @@ using System.Threading; 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 @@ namespace Microsoft.AspNetCore.Authentication.Test.OpenIdConnect 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 handler = null) => TestServerBuilder.CreateServer(_configureOptions, handler: handler, properties: properties); public IDictionary ValidateChallengeFormPost(string responseBody, params string[] parametersToValidate) { @@ -347,4 +348,4 @@ namespace Microsoft.AspNetCore.Authentication.Test.OpenIdConnect } } } -} \ No newline at end of file +} diff --git a/src/Security/Authentication/test/OpenIdConnect/TestTransaction.cs b/src/Security/Authentication/test/OpenIdConnect/TestTransaction.cs index 4f924172c6..4583ebc0af 100644 --- a/src/Security/Authentication/test/OpenIdConnect/TestTransaction.cs +++ b/src/Security/Authentication/test/OpenIdConnect/TestTransaction.cs @@ -26,7 +26,7 @@ namespace Microsoft.AspNetCore.Authentication.Test.OpenIdConnect { 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 @@ namespace Microsoft.AspNetCore.Authentication.Test.OpenIdConnect } } } -} \ No newline at end of file +}