OIDC cleanup

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)

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
         /// <param name="principal">The principal in which tokens are saved.</param>
         /// <param name="message">The OpenID Connect response.</param>
         /// <param name="saveRefreshToken">A <see cref="bool"/> indicating whether the refresh token should be stored.</param>
-        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));
             }
         }
 

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();
         }
 
-        /// <summary>
-        /// Gets or sets the expected audience for any received JWT token.
-        /// </summary>
-        /// <value>
-        /// The expected audience for any received JWT token.
-        /// </value>
-        public string Audience { get; set; }
-
         /// <summary>
         /// Gets or sets the Authority to use when making OpenIdConnect calls.
         /// </summary>
@@ -141,7 +134,7 @@ namespace Microsoft.AspNet.Builder
         /// <summary>
         /// Gets or sets the method used to redirect the user agent to the identity provider.
         /// </summary>
-        public OpenIdConnectRedirectBehavior AuthenticationMethod { get; set; }
+        public OpenIdConnectRedirectBehavior AuthenticationMethod { get; set; } = OpenIdConnectRedirectBehavior.RedirectGet;
 
         /// <summary>
         /// Gets or sets the 'resource'.
@@ -190,5 +183,13 @@ namespace Microsoft.AspNet.Builder
         /// This is disabled by default.
         /// </summary>
         public bool UseTokenLifetime { get; set; }
+
+        /// <summary>
+        /// 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.
+        /// </summary>
+        public bool SkipUnrecognizedRequests { get; set; } = false;
     }
 }

src/Microsoft.AspNet.Authentication/RemoteAuthenticationOptions.cs

@@ -11,7 +11,7 @@ namespace Microsoft.AspNet.Builder
     public class RemoteAuthenticationOptions : AuthenticationOptions
     {
         /// <summary>
-        /// 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.
         /// </summary>
         /// <value>
         /// The back channel timeout.
@@ -50,7 +50,7 @@ namespace Microsoft.AspNet.Builder
 
         /// <summary>
         /// Defines whether access and refresh tokens should be stored in the
-        /// <see cref="ClaimsPrincipal"/> after a successful authentication.
+        /// <see cref="ClaimsPrincipal"/> after a successful authorization with the remote provider.
         /// This property is set to <c>false</c> by default to reduce
         /// the size of the final authentication cookie.
         /// </summary>

test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectHandlerForTestingAuthenticate.cs

@@ -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
-{
-    /// <summary>
-    ///  Allows for custom processing of ApplyResponseChallenge, ApplyResponseGrant and AuthenticateCore
-    /// </summary>
-    public class OpenIdConnectHandlerForTestingAuthenticate : OpenIdConnectHandler
-    {
-        public OpenIdConnectHandlerForTestingAuthenticate() : base(null, null)
-        {
-        }
-
-        protected override Task<OpenIdConnectMessage> RedeemAuthorizationCodeAsync(string authorizationCode, string redirectUri)
-        {
-            var jsonResponse = new JObject();
-            jsonResponse.Add(OpenIdConnectParameterNames.IdToken, "test token");
-            return Task.FromResult(new OpenIdConnectMessage(jsonResponse));
-        }
-
-        protected override Task<AuthenticationTicket> 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));
-        }
-    }
-}

test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectHandlerTests.cs

@@ -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
-{
-    /// <summary>
-    /// These tests are designed to test OpenIdConnectAuthenticationHandler.
-    /// </summary>
-    public class OpenIdConnectHandlerTests
-    {
-        private const string nonceForOpenIdConnect = "abc";
-        private static SecurityToken specCompliantOpenIdConnect = new JwtSecurityToken("issuer", "audience", new List<Claim> { 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<OpenIdConnectOptions, OpenIdConnectMessage> AuthenticateCoreStateDataSet
-        {
-            get
-            {
-                var formater = new AuthenticationPropertiesFormaterKeyValue();
-                var properties = new AuthenticationProperties();
-                var dataset = new TheoryData<OpenIdConnectOptions, OpenIdConnectMessage>();
-
-                // 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<object>(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<object>(null);
-                }
-            };
-            return options;
-        }
-
-        private static TestServer CreateServer(OpenIdConnectOptions options, UrlEncoder encoder, OpenIdConnectHandler handler = null)
-        {
-            var builder = new WebHostBuilder()
-                .Configure(app =>
-                {
-                    app.UseMiddleware<OpenIdConnectMiddlewareForTestingAuthenticate>(Options.Create(options), encoder, handler);
-                    app.Use(async (context, next) =>
-                    {
-                        await next();
-                    });
-                })
-                .ConfigureServices(services =>
-                {
-                    services.AddWebEncoders();
-                    services.AddDataProtection();
-                });
-            return new TestServer(builder);
-        }
-    }
-}

test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectMiddlewareForTestingAuthenticate.cs

@@ -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
-{
-
-    /// <summary>
-    /// pass a <see cref="OpenIdConnectHandler"/> as the AuthenticationHandler
-    /// configured to handle certain messages.
-    /// </summary>
-    public class OpenIdConnectMiddlewareForTestingAuthenticate : OpenIdConnectMiddleware
-    {
-        OpenIdConnectHandler _handler;
-
-        public OpenIdConnectMiddlewareForTestingAuthenticate(
-            RequestDelegate next,
-            IDataProtectionProvider dataProtectionProvider,
-            ILoggerFactory loggerFactory,
-            UrlEncoder encoder,
-            IServiceProvider services,
-            IOptions<SharedAuthenticationOptions> sharedOptions,
-            IOptions<OpenIdConnectOptions> options,
-            HtmlEncoder htmlEncoder,
-            OpenIdConnectHandler handler = null
-            )
-        : base(next, dataProtectionProvider, loggerFactory, encoder, services, sharedOptions, options, htmlEncoder)
-        {
-            _handler = handler;
-        }
-
-        protected override AuthenticationHandler<OpenIdConnectOptions> CreateHandler()
-        {
-            return _handler ?? base.CreateHandler();
-        }
-    }
-}

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",
+                    AuthorizationEndpoint = @"https://login.microsoftonline.com/common/oauth2/authorize",
-                    EndSessionEndpoint = @"https://login.windows.net/common/oauth2/endsessionendpoint",
+                    EndSessionEndpoint = @"https://login.microsoftonline.com/common/oauth2/endsessionendpoint",
-                    TokenEndpoint = @"https://login.windows.net/common/oauth2/token",
+                    TokenEndpoint = @"https://login.microsoftonline.com/common/oauth2/token",
                 };
             }
         }