diff --git a/samples/OpenIdConnectSample/Startup.cs b/samples/OpenIdConnectSample/Startup.cs index 26c9129e07..4c8e2f684b 100644 --- a/samples/OpenIdConnectSample/Startup.cs +++ b/samples/OpenIdConnectSample/Startup.cs @@ -32,8 +32,8 @@ namespace OpenIdConnectSample app.UseOpenIdConnectAuthentication(options => { - options.ClientId = "fe78e0b4-6fe7-47e6-812c-fb75cee266a4"; - options.Authority = "https://login.windows.net/cyrano.onmicrosoft.com"; + options.ClientId = "63a87a83-64b9-4ac1-b2c5-092126f8474f"; + options.Authority = "https://login.windows.net/tratcheroutlook.onmicrosoft.com"; options.RedirectUri = "http://localhost:42023"; }); diff --git a/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectAuthenticationDefaults.cs b/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectAuthenticationDefaults.cs index 2e1a808897..35a85e8437 100644 --- a/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectAuthenticationDefaults.cs +++ b/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectAuthenticationDefaults.cs @@ -24,15 +24,15 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect public const string Caption = "OpenIdConnect"; /// - /// The prefix used to provide a default OpenIdConnectAuthenticationOptions.CookieName. - /// - public const string CookiePrefix = ".AspNet.OpenIdConnect."; - - /// - /// The prefix used to for the a nonce in the cookie. + /// The prefix used to for the nonce in the cookie. /// public const string CookieNoncePrefix = ".AspNet.OpenIdConnect.Nonce."; + /// + /// The prefix used for the state in the cookie. + /// + public const string CookieStatePrefix = ".AspNet.OpenIdConnect.State."; + /// /// The property for the RedirectUri that was used when asking for a 'authorizationCode'. /// diff --git a/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectAuthenticationHandler.cs b/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectAuthenticationHandler.cs index 954f6e74ed..a7257db09a 100644 --- a/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectAuthenticationHandler.cs +++ b/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectAuthenticationHandler.cs @@ -11,6 +11,7 @@ using System.Linq; using System.Net.Http; using System.Net.Http.Headers; using System.Security.Claims; +using System.Security.Cryptography; using System.Text; using System.Threading.Tasks; using Microsoft.AspNet.Http; @@ -48,6 +49,8 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect "; + private static readonly RandomNumberGenerator CryptoRandom = RandomNumberGenerator.Create(); + private OpenIdConnectConfiguration _configuration; protected HttpClient Backchannel { get; private set; } @@ -220,6 +223,8 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect } } + GenerateCorrelationId(properties); + var redirectToIdentityProviderContext = new RedirectToIdentityProviderContext(Context, Options) { ProtocolMessage = message @@ -393,6 +398,11 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect throw new SecurityTokenException(string.Format(CultureInfo.InvariantCulture, Resources.OIDCH_0006_MessageContainsError, message.Error, message.ErrorDescription ?? "ErrorDecription null", message.ErrorUri ?? "ErrorUri null")); } + if (!ValidateCorrelationId(properties)) + { + return null; + } + if (_configuration == null && Options.ConfigurationManager != null) { Logger.LogVerbose(Resources.OIDCH_0007_UpdatingConfiguration); @@ -714,6 +724,66 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect return null; } + private void GenerateCorrelationId([NotNull] AuthenticationProperties properties) + { + var correlationKey = OpenIdConnectAuthenticationDefaults.CookieStatePrefix; + + var nonceBytes = new byte[32]; + CryptoRandom.GetBytes(nonceBytes); + var correlationId = TextEncodings.Base64Url.Encode(nonceBytes); + + var cookieOptions = new CookieOptions + { + HttpOnly = true, + Secure = Request.IsHttps, + Expires = DateTime.UtcNow + Options.ProtocolValidator.NonceLifetime + }; + + properties.Items[correlationKey] = correlationId; + + Response.Cookies.Append(correlationKey + correlationId, NonceProperty, cookieOptions); + } + + private bool ValidateCorrelationId([NotNull] AuthenticationProperties properties) + { + var correlationKey = OpenIdConnectAuthenticationDefaults.CookieStatePrefix; + + string correlationId; + if (!properties.Items.TryGetValue( + correlationKey, + out correlationId)) + { + Logger.LogWarning("{0} state property not found.", correlationKey); + return false; + } + + properties.Items.Remove(correlationKey); + + var cookieName = correlationKey + correlationId; + + var correlationCookie = Request.Cookies[cookieName]; + if (string.IsNullOrEmpty(correlationCookie)) + { + Logger.LogWarning("{0} cookie not found.", cookieName); + return false; + } + + var cookieOptions = new CookieOptions + { + HttpOnly = true, + Secure = Request.IsHttps + }; + Response.Cookies.Delete(cookieName, cookieOptions); + + if (!string.Equals(correlationCookie, NonceProperty, StringComparison.Ordinal)) + { + Logger.LogWarning("{0} correlation cookie and state property mismatch.", correlationKey); + return false; + } + + return true; + } + private AuthenticationProperties GetPropertiesFromState(string state) { // assume a well formed query string: OpenIdConnectAuthenticationDefaults.AuthenticationPropertiesKey=kasjd;fljasldkjflksdj<&c=d> diff --git a/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/ExpectedQueryValues.cs b/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/ExpectedQueryValues.cs index f80a6c7f62..ff48272b7d 100644 --- a/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/ExpectedQueryValues.cs +++ b/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/ExpectedQueryValues.cs @@ -169,7 +169,7 @@ namespace Microsoft.AspNet.Authentication.Tests.OpenIdConnect public string ExpectedState { - get { return OpenIdConnectParameterNames.State + "=" + Encoder.UrlEncode(State); } + get { return Encoder.UrlEncode(State); } } } } diff --git a/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectHandlerTests.cs b/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectHandlerTests.cs index 2e1ab50b5a..d0914fc8ad 100644 --- a/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectHandlerTests.cs +++ b/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectHandlerTests.cs @@ -119,145 +119,6 @@ namespace Microsoft.AspNet.Authentication.Tests.OpenIdConnect }; } - [Theory, MemberData("AuthenticateCoreDataSet")] - public async Task AuthenticateCore(LogLevel logLevel, int[] expectedLogIndexes, Action action, OpenIdConnectMessage message) - { - var errors = new List>(); - var expectedLogs = LoggingUtilities.PopulateLogEntries(expectedLogIndexes); - var handler = new OpenIdConnectAuthenticationHandlerForTestingAuthenticate(); - var loggerFactory = new InMemoryLoggerFactory(logLevel); - var server = CreateServer(new ConfigureOptions(action), UrlEncoder.Default, loggerFactory, handler); - - await server.CreateClient().PostAsync("http://localhost", new FormUrlEncodedContent(message.Parameters)); - LoggingUtilities.CheckLogs(loggerFactory.Logger.Logs, expectedLogs, errors); - Debug.WriteLine(LoggingUtilities.LoggingErrors(errors)); - Assert.True(errors.Count == 0, LoggingUtilities.LoggingErrors(errors)); - } - - public static TheoryData, OpenIdConnectMessage> AuthenticateCoreDataSet - { - get - { - var formater = new AuthenticationPropertiesFormaterKeyValue(); - var dataset = new TheoryData, OpenIdConnectMessage>(); - var properties = new AuthenticationProperties(); - var message = new OpenIdConnectMessage(); - var validState = UrlEncoder.Default.UrlEncode(formater.Protect(properties)); - message.State = validState; - - // MessageReceived - Handled / Skipped - dataset.Add(LogLevel.Debug, new int[] { 0, 1, 2 }, MessageReceivedHandledOptions, message); - dataset.Add(LogLevel.Verbose, new int[] { 2 }, MessageReceivedHandledOptions, message); - dataset.Add(LogLevel.Error, new int[] { }, MessageReceivedHandledOptions, message); - - dataset.Add(LogLevel.Debug, new int[] { 0, 1, 3 }, MessageReceivedSkippedOptions, message); - dataset.Add(LogLevel.Verbose, new int[] { 3 }, MessageReceivedSkippedOptions, message); - dataset.Add(LogLevel.Error, new int[] { }, MessageReceivedSkippedOptions, message); - - // State - null, empty string, invalid - message = new OpenIdConnectMessage(); - dataset.Add(LogLevel.Debug, new int[] { 0, 1, 4, 7, 28 }, StateNullOptions, message); - dataset.Add(LogLevel.Verbose, new int[] { 4, 7 }, StateNullOptions, message); - dataset.Add(LogLevel.Error, new int[] { }, StateNullOptions, message); - - message = new OpenIdConnectMessage(); - message.State = string.Empty; - dataset.Add(LogLevel.Debug, new int[] { 0, 1, 4, 7, 28 }, StateEmptyOptions, message); - dataset.Add(LogLevel.Verbose, new int[] { 4, 7 }, StateEmptyOptions, message); - dataset.Add(LogLevel.Error, new int[] { }, StateEmptyOptions, message); - - message = new OpenIdConnectMessage(); - message.State = Guid.NewGuid().ToString(); - dataset.Add(LogLevel.Debug, new int[] { 0, 1, 5 }, StateInvalidOptions, message); - dataset.Add(LogLevel.Verbose, new int[] { 5 }, StateInvalidOptions, message); - dataset.Add(LogLevel.Error, new int[] { 5 }, StateInvalidOptions, message); - - // OpenIdConnectMessage.Error != null - message = new OpenIdConnectMessage(); - message.Error = "Error"; - dataset.Add(LogLevel.Debug, new int[] { 0, 1, 4, 6, 17, 18 }, MessageWithErrorOptions, message); - dataset.Add(LogLevel.Verbose, new int[] { 4, 6, 17, 18 }, MessageWithErrorOptions, message); - dataset.Add(LogLevel.Error, new int[] { 6, 17 }, MessageWithErrorOptions, message); - - // SecurityTokenReceived - Handled / Skipped - message = new OpenIdConnectMessage(); - message.IdToken = "invalid"; - message.State = validState; - dataset.Add(LogLevel.Debug, new int[] { 0, 1, 7, 20, 8 }, SecurityTokenReceivedHandledOptions, message); - dataset.Add(LogLevel.Verbose, new int[] { 7, 8 }, SecurityTokenReceivedHandledOptions, message); - dataset.Add(LogLevel.Error, new int[] { }, SecurityTokenReceivedHandledOptions, message); - - dataset.Add(LogLevel.Debug, new int[] { 0, 1, 7, 20, 9 }, SecurityTokenReceivedSkippedOptions, message); - dataset.Add(LogLevel.Verbose, new int[] { 7, 9 }, SecurityTokenReceivedSkippedOptions, message); - dataset.Add(LogLevel.Error, new int[] { }, SecurityTokenReceivedSkippedOptions, message); - - // SecurityTokenValidation - ReturnsNull, Throws, Validates - dataset.Add(LogLevel.Debug, new int[] { 0, 1, 7, 20, 11, 17, 18 }, SecurityTokenValidatorCannotReadToken, message); - dataset.Add(LogLevel.Verbose, new int[] { 7, 11, 17, 18 }, SecurityTokenValidatorCannotReadToken, message); - dataset.Add(LogLevel.Error, new int[] { 11, 17 }, SecurityTokenValidatorCannotReadToken, message); - - dataset.Add(LogLevel.Debug, new int[] { 0, 1, 7, 20, 17, 21, 18 }, SecurityTokenValidatorThrows, message); - dataset.Add(LogLevel.Verbose, new int[] { 7, 17, 21, 18 }, SecurityTokenValidatorThrows, message); - dataset.Add(LogLevel.Error, new int[] { 17 }, SecurityTokenValidatorThrows, message); - - message.Nonce = nonceForJwt; - dataset.Add(LogLevel.Debug, new int[] { 0, 1, 7, 20 }, SecurityTokenValidatorValidatesAllTokens, message); - dataset.Add(LogLevel.Verbose, new int[] { 7 }, SecurityTokenValidatorValidatesAllTokens, message); - dataset.Add(LogLevel.Error, new int[] { }, SecurityTokenValidatorValidatesAllTokens, message); - - // SecurityTokenValidation - Handled / Skipped - dataset.Add(LogLevel.Debug, new int[] { 0, 1, 7, 20, 12 }, SecurityTokenValidatedHandledOptions, message); - dataset.Add(LogLevel.Verbose, new int[] { 7, 12 }, SecurityTokenValidatedHandledOptions, message); - dataset.Add(LogLevel.Error, new int[] { }, SecurityTokenValidatedHandledOptions, message); - - dataset.Add(LogLevel.Debug, new int[] { 0, 1, 7, 20, 13 }, SecurityTokenValidatedSkippedOptions, message); - dataset.Add(LogLevel.Verbose, new int[] { 7, 13 }, SecurityTokenValidatedSkippedOptions, message); - dataset.Add(LogLevel.Error, new int[] { }, SecurityTokenValidatedSkippedOptions, message); - - // AuthenticationCodeReceived - Handled / Skipped - message = new OpenIdConnectMessage(); - message.Code = Guid.NewGuid().ToString(); - message.State = validState; - dataset.Add(LogLevel.Debug, new int[] { 0, 1, 7, 14, 15 }, AuthorizationCodeReceivedHandledOptions, message); - dataset.Add(LogLevel.Verbose, new int[] { 7, 15 }, AuthorizationCodeReceivedHandledOptions, message); - dataset.Add(LogLevel.Error, new int[] { }, AuthorizationCodeReceivedHandledOptions, message); - - dataset.Add(LogLevel.Debug, new int[] { 0, 1, 7, 14, 16 }, AuthorizationCodeReceivedSkippedOptions, message); - dataset.Add(LogLevel.Verbose, new int[] { 7, 16 }, AuthorizationCodeReceivedSkippedOptions, message); - dataset.Add(LogLevel.Error, new int[] { }, AuthorizationCodeReceivedSkippedOptions, message); - - message = new OpenIdConnectMessage(); - message.Code = Guid.NewGuid().ToString(); - message.State = validState; - message.IdToken = "test token"; - dataset.Add(LogLevel.Debug, new int[] { 0, 1, 7, 20, 14, 15 }, AuthorizationCodeReceivedHandledOptions, message); - dataset.Add(LogLevel.Verbose, new int[] { 7, 15 }, AuthorizationCodeReceivedHandledOptions, message); - dataset.Add(LogLevel.Error, new int[] { }, AuthorizationCodeReceivedHandledOptions, message); - - dataset.Add(LogLevel.Debug, new int[] { 0, 1, 7, 20, 14, 16 }, AuthorizationCodeReceivedSkippedOptions, message); - dataset.Add(LogLevel.Verbose, new int[] { 7, 16 }, AuthorizationCodeReceivedSkippedOptions, message); - dataset.Add(LogLevel.Error, new int[] { }, AuthorizationCodeReceivedSkippedOptions, message); - - // CodeReceivedAndRedeemed and GetUserInformationFromUIEndpoint - message = new OpenIdConnectMessage(); - message.IdToken = null; - message.Code = Guid.NewGuid().ToString(); - dataset.Add(LogLevel.Debug, new int[] { 0, 1, 4, 7, 14, 23, 25, 26 }, CodeReceivedAndRedeemedHandledOptions, message); - dataset.Add(LogLevel.Verbose, new int[] { 4, 7, 26 }, CodeReceivedAndRedeemedHandledOptions, message); - dataset.Add(LogLevel.Error, new int[] { }, CodeReceivedAndRedeemedHandledOptions, message); - - dataset.Add(LogLevel.Debug, new int[] { 0, 1, 4, 7, 14, 23, 25, 27 }, CodeReceivedAndRedeemedSkippedOptions, message); - dataset.Add(LogLevel.Verbose, new int[] { 4, 7, 27 }, CodeReceivedAndRedeemedSkippedOptions, message); - dataset.Add(LogLevel.Error, new int[] { }, CodeReceivedAndRedeemedSkippedOptions, message); - - dataset.Add(LogLevel.Debug, new int[] { 0, 1, 4, 7, 14, 23, 25, 24, 12 }, GetUserInfoFromUIEndpoint, message); - dataset.Add(LogLevel.Verbose, new int[] { 4, 7, 12 }, GetUserInfoFromUIEndpoint, message); - dataset.Add(LogLevel.Error, new int[] { }, GetUserInfoFromUIEndpoint, message); - - return dataset; - } - } - #region Configure Options for AuthenticateCore variations private static void DefaultOptions(OpenIdConnectAuthenticationOptions options) diff --git a/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectMiddlewareTests.cs b/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectMiddlewareTests.cs index 693ff185d6..71fc4c855f 100644 --- a/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectMiddlewareTests.cs +++ b/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectMiddlewareTests.cs @@ -73,7 +73,7 @@ namespace Microsoft.AspNet.Authentication.Tests.OpenIdConnect } [Fact] - public async Task ChallengeWillSetNonceCookie() + public async Task ChallengeWillSetNonceAndStateCookies() { var server = CreateServer(options => { @@ -82,8 +82,14 @@ namespace Microsoft.AspNet.Authentication.Tests.OpenIdConnect options.Configuration = TestUtilities.DefaultOpenIdConnectConfiguration; }); var transaction = await SendAsync(server, DefaultHost + Challenge); - transaction.SetCookie.Single().ShouldContain(OpenIdConnectAuthenticationDefaults.CookieNoncePrefix); - transaction.SetCookie.Single().ShouldContain("Expires"); + + var firstCookie = transaction.SetCookie.First(); + firstCookie.ShouldContain(OpenIdConnectAuthenticationDefaults.CookieNoncePrefix); + firstCookie.ShouldContain("Expires"); + + var secondCookie = transaction.SetCookie.Skip(1).First(); + secondCookie.ShouldContain(OpenIdConnectAuthenticationDefaults.CookieStatePrefix); + secondCookie.ShouldContain("Expires"); } [Fact]