diff --git a/samples/OpenIdConnectSample/Startup.cs b/samples/OpenIdConnectSample/Startup.cs index 1d40a0a92f..7c469ef299 100644 --- a/samples/OpenIdConnectSample/Startup.cs +++ b/samples/OpenIdConnectSample/Startup.cs @@ -26,7 +26,8 @@ namespace OpenIdConnectSample public void ConfigureServices(IServiceCollection services) { - services.AddAuthentication(sharedOptions => sharedOptions.SignInScheme = CookieAuthenticationDefaults.AuthenticationScheme); + services.AddAuthentication(sharedOptions => + sharedOptions.SignInScheme = CookieAuthenticationDefaults.AuthenticationScheme); } public void Configure(IApplicationBuilder app, ILoggerFactory loggerfactory) diff --git a/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectHandler.cs b/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectHandler.cs index 12d61e334b..e9f4d3ae00 100644 --- a/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectHandler.cs +++ b/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectHandler.cs @@ -84,21 +84,29 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect // 1. properties.Redirect // 2. Options.PostLogoutRedirectUri var properties = new AuthenticationProperties(signout.Properties); - if (!string.IsNullOrEmpty(properties.RedirectUri)) + var logoutRedirectUri = properties.RedirectUri; + if (!string.IsNullOrEmpty(logoutRedirectUri)) { - message.PostLogoutRedirectUri = properties.RedirectUri; + // Relative to PathBase + if (logoutRedirectUri.StartsWith("/", StringComparison.Ordinal)) + { + logoutRedirectUri = BuildRedirectUri(logoutRedirectUri); + } + message.PostLogoutRedirectUri = logoutRedirectUri; } else if (!string.IsNullOrEmpty(Options.PostLogoutRedirectUri)) { - message.PostLogoutRedirectUri = Options.PostLogoutRedirectUri; + logoutRedirectUri = Options.PostLogoutRedirectUri; + // Relative to PathBase + if (logoutRedirectUri.StartsWith("/", StringComparison.Ordinal)) + { + logoutRedirectUri = BuildRedirectUri(logoutRedirectUri); + } + message.PostLogoutRedirectUri = logoutRedirectUri; } - if (!string.IsNullOrEmpty(Options.SignInScheme)) - { - var principal = await Context.Authentication.AuthenticateAsync(Options.SignInScheme); - - message.IdTokenHint = principal?.FindFirst(OpenIdConnectParameterNames.IdToken)?.Value; - } + var principal = await Context.Authentication.AuthenticateAsync(Options.SignInScheme); + message.IdTokenHint = principal?.FindFirst(OpenIdConnectParameterNames.IdToken)?.Value; var redirectContext = new RedirectContext(Context, Options, properties) { @@ -308,6 +316,11 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect // See http://openid.net/specs/oauth-v2-multiple-response-types-1_0.html#Security if (!string.IsNullOrEmpty(message.IdToken) || !string.IsNullOrEmpty(message.AccessToken)) { + if (Options.SkipUnrecognizedRequests) + { + // Not for us? + return AuthenticateResult.Skip(); + } return AuthenticateResult.Fail("An OpenID Connect response cannot contain an " + "identity token or an access token when using response_mode=query"); } @@ -325,6 +338,11 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect if (message == null) { + if (Options.SkipUnrecognizedRequests) + { + // Not for us? + return AuthenticateResult.Skip(); + } return AuthenticateResult.Fail("No message."); } @@ -344,7 +362,11 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect // Fail if state is missing, it's required for the correlation id. if (string.IsNullOrEmpty(message.State)) { - // This wasn't a valid ODIC message, it may not have been intended for us. + // This wasn't a valid OIDC message, it may not have been intended for us. + if (Options.SkipUnrecognizedRequests) + { + return AuthenticateResult.Skip(); + } Logger.LogDebug(11, "message.State is null or empty."); return AuthenticateResult.Fail(Resources.MessageStateIsNullOrEmpty); } @@ -353,7 +375,12 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect var properties = Options.StateDataFormat.Unprotect(Uri.UnescapeDataString(message.State)); if (properties == null) { - Logger.LogError(12, "Unable to unprotect the message.State."); + if (Options.SkipUnrecognizedRequests) + { + // Not for us? + return AuthenticateResult.Skip(); + } + Logger.LogError(12, "Unable to read the message.State."); return AuthenticateResult.Fail(Resources.MessageStateIsInvalid); } @@ -516,7 +543,7 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect if (Options.SaveTokensAsClaims) { // Persist the tokens extracted from the token response. - SaveTokens(ticket.Principal, tokenEndpointResponse, saveRefreshToken: true); + SaveTokens(ticket.Principal, tokenEndpointResponse, jwt.Issuer, saveRefreshToken: true); } if (Options.GetClaimsFromUserInfoEndpoint) @@ -582,7 +609,7 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect { // TODO: call SaveTokens with the token response and set // saveRefreshToken to true when the hybrid flow is fully implemented. - SaveTokens(ticket.Principal, message, saveRefreshToken: false); + SaveTokens(ticket.Principal, message, jwt.Issuer, saveRefreshToken: false); } } // Implicit Flow @@ -594,7 +621,7 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect // response, since it's not a valid parameter when using the implicit flow. // See http://openid.net/specs/openid-connect-core-1_0.html#Authentication // and https://tools.ietf.org/html/rfc6749#section-4.2.2. - SaveTokens(ticket.Principal, message, saveRefreshToken: false); + SaveTokens(ticket.Principal, message, jwt.Issuer, saveRefreshToken: false); } } @@ -709,7 +736,7 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect { JToken value; var claimValue = user.TryGetValue(pair.Key, out value) ? value.ToString() : null; - identity.AddClaim(new Claim(pair.Key, claimValue, ClaimValueTypes.String, Options.ClaimsIssuer)); + identity.AddClaim(new Claim(pair.Key, claimValue, ClaimValueTypes.String, jwt.Issuer)); } return ticket; @@ -721,38 +748,38 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect /// The principal in which tokens are saved. /// The OpenID Connect response. /// A indicating whether the refresh token should be stored. - private void SaveTokens(ClaimsPrincipal principal, OpenIdConnectMessage message, bool saveRefreshToken) + private void SaveTokens(ClaimsPrincipal principal, OpenIdConnectMessage message, string issuer, bool saveRefreshToken) { var identity = (ClaimsIdentity)principal.Identity; if (!string.IsNullOrEmpty(message.AccessToken)) { identity.AddClaim(new Claim(OpenIdConnectParameterNames.AccessToken, message.AccessToken, - ClaimValueTypes.String, Options.ClaimsIssuer)); + ClaimValueTypes.String, issuer)); } if (!string.IsNullOrEmpty(message.IdToken)) { identity.AddClaim(new Claim(OpenIdConnectParameterNames.IdToken, message.IdToken, - ClaimValueTypes.String, Options.ClaimsIssuer)); + ClaimValueTypes.String, issuer)); } if (saveRefreshToken && !string.IsNullOrEmpty(message.RefreshToken)) { identity.AddClaim(new Claim(OpenIdConnectParameterNames.RefreshToken, message.RefreshToken, - ClaimValueTypes.String, Options.ClaimsIssuer)); + ClaimValueTypes.String, issuer)); } if (!string.IsNullOrEmpty(message.TokenType)) { identity.AddClaim(new Claim(OpenIdConnectParameterNames.TokenType, message.TokenType, - ClaimValueTypes.String, Options.ClaimsIssuer)); + ClaimValueTypes.String, issuer)); } if (!string.IsNullOrEmpty(message.ExpiresIn)) { identity.AddClaim(new Claim(OpenIdConnectParameterNames.ExpiresIn, message.ExpiresIn, - ClaimValueTypes.String, Options.ClaimsIssuer)); + ClaimValueTypes.String, issuer)); } } diff --git a/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectOptions.cs b/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectOptions.cs index af7a621de0..cd31e27d01 100644 --- a/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectOptions.cs +++ b/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectOptions.cs @@ -48,19 +48,12 @@ namespace Microsoft.AspNet.Builder public OpenIdConnectOptions(string authenticationScheme) { AuthenticationScheme = authenticationScheme; + AutomaticChallenge = true; DisplayName = OpenIdConnectDefaults.Caption; CallbackPath = new PathString("/signin-oidc"); Events = new OpenIdConnectEvents(); } - /// - /// Gets or sets the expected audience for any received JWT token. - /// - /// - /// The expected audience for any received JWT token. - /// - public string Audience { get; set; } - /// /// Gets or sets the Authority to use when making OpenIdConnect calls. /// @@ -141,7 +134,7 @@ namespace Microsoft.AspNet.Builder /// /// Gets or sets the method used to redirect the user agent to the identity provider. /// - public OpenIdConnectRedirectBehavior AuthenticationMethod { get; set; } + public OpenIdConnectRedirectBehavior AuthenticationMethod { get; set; } = OpenIdConnectRedirectBehavior.RedirectGet; /// /// Gets or sets the 'resource'. @@ -190,5 +183,13 @@ namespace Microsoft.AspNet.Builder /// This is disabled by default. /// public bool UseTokenLifetime { get; set; } + + /// + /// Indicates if requests to the CallbackPath may also be for other components. If enabled the middleware will pass + /// requests through that do not contain OpenIdConnect authentication responses. Disabling this and setting the + /// CallbackPath to a dedicated endpoint may provide better error handling. + /// This is disabled by default. + /// + public bool SkipUnrecognizedRequests { get; set; } = false; } } diff --git a/src/Microsoft.AspNet.Authentication/RemoteAuthenticationOptions.cs b/src/Microsoft.AspNet.Authentication/RemoteAuthenticationOptions.cs index afaee6c7b1..9392379398 100644 --- a/src/Microsoft.AspNet.Authentication/RemoteAuthenticationOptions.cs +++ b/src/Microsoft.AspNet.Authentication/RemoteAuthenticationOptions.cs @@ -11,7 +11,7 @@ namespace Microsoft.AspNet.Builder public class RemoteAuthenticationOptions : AuthenticationOptions { /// - /// Gets or sets timeout value in milliseconds for back channel communications with Twitter. + /// Gets or sets timeout value in milliseconds for back channel communications with the remote provider. /// /// /// The back channel timeout. @@ -50,7 +50,7 @@ namespace Microsoft.AspNet.Builder /// /// Defines whether access and refresh tokens should be stored in the - /// after a successful authentication. + /// after a successful authorization with the remote provider. /// This property is set to false by default to reduce /// the size of the final authentication cookie. /// diff --git a/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectHandlerForTestingAuthenticate.cs b/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectHandlerForTestingAuthenticate.cs deleted file mode 100644 index c0333cb2f4..0000000000 --- a/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectHandlerForTestingAuthenticate.cs +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System.IdentityModel.Tokens.Jwt; -using System.Security.Claims; -using System.Threading.Tasks; -using Microsoft.AspNet.Authentication.OpenIdConnect; -using Microsoft.IdentityModel.Protocols.OpenIdConnect; -using Newtonsoft.Json.Linq; - -namespace Microsoft.AspNet.Authentication.Tests.OpenIdConnect -{ - /// - /// Allows for custom processing of ApplyResponseChallenge, ApplyResponseGrant and AuthenticateCore - /// - public class OpenIdConnectHandlerForTestingAuthenticate : OpenIdConnectHandler - { - public OpenIdConnectHandlerForTestingAuthenticate() : base(null, null) - { - } - - protected override Task RedeemAuthorizationCodeAsync(string authorizationCode, string redirectUri) - { - var jsonResponse = new JObject(); - jsonResponse.Add(OpenIdConnectParameterNames.IdToken, "test token"); - return Task.FromResult(new OpenIdConnectMessage(jsonResponse)); - } - - protected override Task GetUserInformationAsync(OpenIdConnectMessage message, JwtSecurityToken jwt, AuthenticationTicket ticket) - { - var claimsIdentity = (ClaimsIdentity)ticket.Principal.Identity; - if (claimsIdentity == null) - { - claimsIdentity = new ClaimsIdentity(); - } - claimsIdentity.AddClaim(new Claim("test claim", "test value")); - return Task.FromResult(new AuthenticationTicket(new ClaimsPrincipal(claimsIdentity), ticket.Properties, ticket.AuthenticationScheme)); - } - } -} diff --git a/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectHandlerTests.cs b/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectHandlerTests.cs deleted file mode 100644 index fa49055391..0000000000 --- a/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectHandlerTests.cs +++ /dev/null @@ -1,118 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; -using System.Collections.Generic; -using System.IdentityModel.Tokens.Jwt; -using System.Linq; -using System.Net.Http; -using System.Security.Claims; -using System.Text.Encodings.Web; -using System.Threading.Tasks; -using Microsoft.AspNet.Authentication.OpenIdConnect; -using Microsoft.AspNet.Builder; -using Microsoft.AspNet.Hosting; -using Microsoft.AspNet.Http.Authentication; -using Microsoft.AspNet.TestHost; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Options; -using Microsoft.IdentityModel.Protocols.OpenIdConnect; -using Microsoft.IdentityModel.Tokens; -using Xunit; - -namespace Microsoft.AspNet.Authentication.Tests.OpenIdConnect -{ - /// - /// These tests are designed to test OpenIdConnectAuthenticationHandler. - /// - public class OpenIdConnectHandlerTests - { - private const string nonceForOpenIdConnect = "abc"; - private static SecurityToken specCompliantOpenIdConnect = new JwtSecurityToken("issuer", "audience", new List { new Claim("iat", EpochTime.GetIntDate(DateTime.UtcNow).ToString()), new Claim("nonce", nonceForOpenIdConnect) }, DateTime.UtcNow, DateTime.UtcNow + TimeSpan.FromDays(1)); - private const string ExpectedStateParameter = "expectedState"; - - [Theory, MemberData(nameof(AuthenticateCoreStateDataSet))] - public async Task AuthenticateCoreState(OpenIdConnectOptions option, OpenIdConnectMessage message) - { - var handler = new OpenIdConnectHandlerForTestingAuthenticate(); - var server = CreateServer(option, UrlEncoder.Default, handler); - await server.CreateClient().PostAsync("http://localhost", new FormUrlEncodedContent(message.Parameters.Where(pair => pair.Value != null))); - } - - public static TheoryData AuthenticateCoreStateDataSet - { - get - { - var formater = new AuthenticationPropertiesFormaterKeyValue(); - var properties = new AuthenticationProperties(); - var dataset = new TheoryData(); - - // expected user state is added to the message.Parameters.Items[ExpectedStateParameter] - // Userstate == null - var message = new OpenIdConnectMessage(); - message.State = UrlEncoder.Default.Encode(formater.Protect(properties)); - message.Code = Guid.NewGuid().ToString(); - message.Parameters.Add(ExpectedStateParameter, null); - dataset.Add(GetStateOptions(), message); - - // Userstate != null - message = new OpenIdConnectMessage(); - properties.Items.Clear(); - var userstate = Guid.NewGuid().ToString(); - message.Code = Guid.NewGuid().ToString(); - properties.Items.Add(OpenIdConnectDefaults.UserstatePropertiesKey, userstate); - message.State = UrlEncoder.Default.Encode(formater.Protect(properties)); - message.Parameters.Add(ExpectedStateParameter, userstate); - dataset.Add(GetStateOptions(), message); - return dataset; - } - } - - // Setup an event to check for expected state. - // The state gets set by the runtime after the 'MessageReceivedContext' - private static OpenIdConnectOptions GetStateOptions() - { - var options = new OpenIdConnectOptions(); - options.AuthenticationScheme = "OpenIdConnectHandlerTest"; - options.ConfigurationManager = TestUtilities.DefaultOpenIdConnectConfigurationManager; - options.ClientId = Guid.NewGuid().ToString(); - options.StateDataFormat = new AuthenticationPropertiesFormaterKeyValue(); - options.SignInScheme = "Cookies"; - options.Events = new OpenIdConnectEvents() - { - OnTokenResponseReceived = context => - { - context.HandleResponse(); - if (context.ProtocolMessage.State == null && !context.ProtocolMessage.Parameters.ContainsKey(ExpectedStateParameter)) - return Task.FromResult(null); - - if (context.ProtocolMessage.State == null || !context.ProtocolMessage.Parameters.ContainsKey(ExpectedStateParameter)) - Assert.True(false, "(context.ProtocolMessage.State=!= null || !context.ProtocolMessage.Parameters.ContainsKey(expectedState)"); - - Assert.Equal(context.ProtocolMessage.State, context.ProtocolMessage.Parameters[ExpectedStateParameter]); - return Task.FromResult(null); - } - }; - return options; - } - - private static TestServer CreateServer(OpenIdConnectOptions options, UrlEncoder encoder, OpenIdConnectHandler handler = null) - { - var builder = new WebHostBuilder() - .Configure(app => - { - app.UseMiddleware(Options.Create(options), encoder, handler); - app.Use(async (context, next) => - { - await next(); - }); - }) - .ConfigureServices(services => - { - services.AddWebEncoders(); - services.AddDataProtection(); - }); - return new TestServer(builder); - } - } -} diff --git a/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectMiddlewareForTestingAuthenticate.cs b/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectMiddlewareForTestingAuthenticate.cs deleted file mode 100644 index c73cf8f942..0000000000 --- a/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectMiddlewareForTestingAuthenticate.cs +++ /dev/null @@ -1,45 +0,0 @@ -// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; -using System.Text.Encodings.Web; -using Microsoft.AspNet.Authentication.OpenIdConnect; -using Microsoft.AspNet.Builder; -using Microsoft.AspNet.DataProtection; -using Microsoft.AspNet.Http; -using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Options; - -namespace Microsoft.AspNet.Authentication.Tests.OpenIdConnect -{ - - /// - /// pass a as the AuthenticationHandler - /// configured to handle certain messages. - /// - public class OpenIdConnectMiddlewareForTestingAuthenticate : OpenIdConnectMiddleware - { - OpenIdConnectHandler _handler; - - public OpenIdConnectMiddlewareForTestingAuthenticate( - RequestDelegate next, - IDataProtectionProvider dataProtectionProvider, - ILoggerFactory loggerFactory, - UrlEncoder encoder, - IServiceProvider services, - IOptions sharedOptions, - IOptions options, - HtmlEncoder htmlEncoder, - OpenIdConnectHandler handler = null - ) - : base(next, dataProtectionProvider, loggerFactory, encoder, services, sharedOptions, options, htmlEncoder) - { - _handler = handler; - } - - protected override AuthenticationHandler CreateHandler() - { - return _handler ?? base.CreateHandler(); - } - } -} diff --git a/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/TestUtilities.cs b/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/TestUtilities.cs index b5371965ca..e48aa66043 100644 --- a/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/TestUtilities.cs +++ b/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/TestUtilities.cs @@ -27,9 +27,9 @@ namespace Microsoft.AspNet.Authentication.Tests.OpenIdConnect { return new OpenIdConnectConfiguration() { - AuthorizationEndpoint = @"https://login.windows.net/common/oauth2/authorize", - EndSessionEndpoint = @"https://login.windows.net/common/oauth2/endsessionendpoint", - TokenEndpoint = @"https://login.windows.net/common/oauth2/token", + AuthorizationEndpoint = @"https://login.microsoftonline.com/common/oauth2/authorize", + EndSessionEndpoint = @"https://login.microsoftonline.com/common/oauth2/endsessionendpoint", + TokenEndpoint = @"https://login.microsoftonline.com/common/oauth2/token", }; } }