Merge branch 'master' into release

aspnet · Feb 22, 2021 · 78c8866 · 78c8866
2 parents 7942cff + 1a07400
commit 78c8866
Show file tree

Hide file tree

Showing 12 changed files with 278 additions and 112 deletions.
diff --git a/src/Microsoft.Owin.Security.OpenIdConnect/Microsoft.Owin.Security.OpenIdConnect.csproj b/src/Microsoft.Owin.Security.OpenIdConnect/Microsoft.Owin.Security.OpenIdConnect.csproj
@@ -86,6 +86,7 @@
     <Compile Include="GlobalSuppressions.cs" />
     <Compile Include="Notifications\AuthorizationCodeReceivedNotification.cs" />
     <Compile Include="Notifications\TokenResponseReceivedNotification.cs" />
+    <Compile Include="OAuthConstants.cs" />
     <Compile Include="OpenIdConnectAuthenticationDefaults.cs" />
     <Compile Include="OpenIdConnectAuthenticationExtensions.cs" />
     <Compile Include="OpenidConnectAuthenticationHandler.cs" />

diff --git a/src/Microsoft.Owin.Security.OpenIdConnect/OAuthConstants.cs b/src/Microsoft.Owin.Security.OpenIdConnect/OAuthConstants.cs
@@ -0,0 +1,30 @@
+// 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.
+
+namespace Microsoft.Owin.Security.OpenIdConnect
+{
+    [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1704:IdentifiersShouldBeSpelledCorrectly", MessageId = "Auth",
+        Justification = "OAuth2 is a valid word.")]
+    internal static class OAuthConstants
+    {
+        /// <summary>
+        /// code_verifier defined in https://tools.ietf.org/html/rfc7636
+        /// </summary>
+        public const string CodeVerifierKey = "code_verifier";
+
+        /// <summary>
+        /// code_challenge defined in https://tools.ietf.org/html/rfc7636
+        /// </summary>
+        public const string CodeChallengeKey = "code_challenge";
+
+        /// <summary>
+        /// code_challenge_method defined in https://tools.ietf.org/html/rfc7636
+        /// </summary>
+        public const string CodeChallengeMethodKey = "code_challenge_method";
+
+        /// <summary>
+        /// S256 defined in https://tools.ietf.org/html/rfc7636
+        /// </summary>
+        public const string CodeChallengeMethodS256 = "S256";
+    }
+}
diff --git a/src/Microsoft.Owin.Security.OpenIdConnect/OpenIdConnectAuthenticationOptions.cs b/src/Microsoft.Owin.Security.OpenIdConnect/OpenIdConnectAuthenticationOptions.cs
@@ -47,6 +47,7 @@ public OpenIdConnectAuthenticationOptions()
         /// <para>TokenValidationParameters: new <see cref="TokenValidationParameters"/> with AuthenticationType = authenticationType.</para>
         /// <para>UseTokenLifetime: true.</para>
         /// <para>RedeemCode: false.</para>
+        /// <para>UsePkce: true.</para>
         /// </remarks>
         /// <param name="authenticationType"> will be used to when creating the <see cref="System.Security.Claims.ClaimsIdentity"/> for the AuthenticationType property.</param>
         [SuppressMessage("Microsoft.Globalization", "CA1303:Do not pass literals as localized parameters", MessageId = "Microsoft.Owin.Security.OpenIdConnect.OpenIdConnectAuthenticationOptions.set_Caption(System.String)", Justification = "Not a LOC field")]
@@ -71,6 +72,7 @@ public OpenIdConnectAuthenticationOptions(string authenticationType)
             UseTokenLifetime = true;
             CookieManager = new CookieManager();
             RedeemCode = false;
+            UsePkce = true;
         }
 
         /// <summary>
@@ -322,5 +324,15 @@ public bool UseTokenLifetime
         /// This property is set to <c>false</c> by default.
         /// </summary>
         public bool RedeemCode { get; set; }
+
+        /// <summary>
+        /// Enables or disables the use of the Proof Key for Code Exchange (PKCE) standard.
+        /// This only applies when the <see cref="ResponseType"/> is set to <see cref="OpenIdConnectResponseType.Code"/>.
+        /// See https://tools.ietf.org/html/rfc7636.
+        /// The default value is `true`.
+        /// </summary>
+        [SuppressMessage("Microsoft.Naming", "CA1704:IdentifiersShouldBeSpelledCorrectly", MessageId = "Pkce",
+        Justification = "Pkce is a valid acronym.")]
+        public bool UsePkce { get; set; }
     }
-}
+}
diff --git a/src/Microsoft.Owin.Security.OpenIdConnect/OpenidConnectAuthenticationHandler.cs b/src/Microsoft.Owin.Security.OpenIdConnect/OpenidConnectAuthenticationHandler.cs
@@ -16,6 +16,7 @@
 using Microsoft.IdentityModel.Protocols.OpenIdConnect;
 using Microsoft.IdentityModel.Tokens;
 using Microsoft.Owin.Logging;
+using Microsoft.Owin.Security.DataHandler.Encoder;
 using Microsoft.Owin.Security.Infrastructure;
 using Microsoft.Owin.Security.Notifications;
 
@@ -155,10 +156,31 @@ protected override async Task ApplyResponseChallengeAsync()
                     RequestType = OpenIdConnectRequestType.Authentication,
                     Resource = Options.Resource,
                     ResponseType = Options.ResponseType,
-                    Scope = Options.Scope,
-                    State = OpenIdConnectAuthenticationDefaults.AuthenticationPropertiesKey + "=" + Uri.EscapeDataString(Options.StateDataFormat.Protect(properties)),
+                    Scope = Options.Scope
                 };
 
+                // https://tools.ietf.org/html/rfc7636
+                if (Options.UsePkce && Options.ResponseType == OpenIdConnectResponseType.Code)
+                {
+                    using (RandomNumberGenerator randomNumberGenerator = RandomNumberGenerator.Create())
+                    using (HashAlgorithm hash = SHA256.Create())
+                    {
+                        byte[] bytes = new byte[32];
+                        randomNumberGenerator.GetBytes(bytes);
+                        string codeVerifier = TextEncodings.Base64Url.Encode(bytes);
+
+                        // Store this for use during the code redemption.
+                        properties.Dictionary.Add(OAuthConstants.CodeVerifierKey, codeVerifier);
+                        byte[] challengeBytes = hash.ComputeHash(Encoding.UTF8.GetBytes(codeVerifier));
+                        string codeChallenge = TextEncodings.Base64Url.Encode(challengeBytes);
+
+                        openIdConnectMessage.Parameters.Add(OAuthConstants.CodeChallengeKey, codeChallenge);
+                        openIdConnectMessage.Parameters.Add(OAuthConstants.CodeChallengeMethodKey, OAuthConstants.CodeChallengeMethodS256);
+                    }
+                }
+
+                openIdConnectMessage.State = OpenIdConnectAuthenticationDefaults.AuthenticationPropertiesKey + "=" + Uri.EscapeDataString(Options.StateDataFormat.Protect(properties));
+
                 // Omitting the response_mode parameter when it already corresponds to the default
                 // response_mode used for the specified response_type is recommended by the specifications.
                 // See http://openid.net/specs/oauth-v2-multiple-response-types-1_0.html#ResponseModes
@@ -397,6 +419,15 @@ protected override async Task<AuthenticationTicket> AuthenticateCoreAsync()
                         RedirectUri = tokenEndpointRequest.RedirectUri,
                         TokenEndpointRequest = tokenEndpointRequest
                     };
+
+                    // PKCE https://tools.ietf.org/html/rfc7636#section-4.5
+                    string codeVerifier;
+                    if (properties.Dictionary.TryGetValue(OAuthConstants.CodeVerifierKey, out codeVerifier))
+                    {
+                        tokenEndpointRequest.Parameters.Add(OAuthConstants.CodeVerifierKey, codeVerifier);
+                        properties.Dictionary.Remove(OAuthConstants.CodeVerifierKey);
+                    }
+
                     await Options.Notifications.AuthorizationCodeReceived(authorizationCodeReceivedNotification);
                     if (authorizationCodeReceivedNotification.HandledResponse)
                     {

diff --git a/tests/Katana.Sandbox.WebServer/Startup.cs b/tests/Katana.Sandbox.WebServer/Startup.cs
@@ -10,6 +10,7 @@
 using System.Security.Principal;
 using System.Threading.Tasks;
 using Katana.Sandbox.WebServer;
+using Microsoft.IdentityModel.Protocols.OpenIdConnect;
 using Microsoft.Owin;
 using Microsoft.Owin.Extensions;
 using Microsoft.Owin.Host.SystemWeb;
@@ -135,9 +136,9 @@ public void Configuration(IAppBuilder app)
 
             app.UseOpenIdConnectAuthentication(new Microsoft.Owin.Security.OpenIdConnect.OpenIdConnectAuthenticationOptions()
             {
-                // https://github.com/IdentityServer/IdentityServer4.Demo/blob/master/src/IdentityServer4Demo/Config.cs
-                ClientId = "hybrid",
-                ClientSecret = "secret", // for code flow
+                // https://github.com/IdentityServer/IdentityServer4.Demo/blob/main/src/IdentityServer4Demo/Config.cs
+                ClientId = "interactive.confidential.short", // client requires pkce
+                ClientSecret = "secret",
                 Authority = "https://demo.identityserver.io/",
                 RedirectUri = "https://localhost:44318/signin-oidc",
                 /*
@@ -146,11 +147,11 @@ public void Configuration(IAppBuilder app)
                 ClientSecret = Environment.GetEnvironmentVariable("oidc:clientsecret"),*/
                 // CookieManager = new SystemWebCookieManager(),
                 CookieManager = new SameSiteCookieManager(),
-                //ResponseType = "code",
-                //ResponseMode = "query",
-                //SaveTokens = true,
-                //Scope = "openid profile offline_access",
-                //RedeemCode = true,
+                ResponseType = OpenIdConnectResponseType.Code,
+                ResponseMode = OpenIdConnectResponseMode.Query,
+                SaveTokens = true,
+                Scope = "openid profile offline_access",
+                RedeemCode = true,
                 //Notifications = new Microsoft.Owin.Security.OpenIdConnect.OpenIdConnectAuthenticationNotifications
                 //{
                 //    AuthorizationCodeReceived = async n =>
@@ -166,6 +167,7 @@ public void Configuration(IAppBuilder app)
                 //        n.HandleCodeRedemption(message);
                 //    }
                 //}
+                UsePkce = true,
             });
 
             /*

diff --git a/tests/Microsoft.Owin.Hosting.Tests/Containers/CastleWindsorContainerTests.cs b/tests/Microsoft.Owin.Hosting.Tests/Containers/CastleWindsorContainerTests.cs
diff --git a/tests/Microsoft.Owin.Hosting.Tests/Containers/NinjectContainerTests.cs b/tests/Microsoft.Owin.Hosting.Tests/Containers/NinjectContainerTests.cs
diff --git a/tests/Microsoft.Owin.Hosting.Tests/Microsoft.Owin.Hosting.Tests.csproj b/tests/Microsoft.Owin.Hosting.Tests/Microsoft.Owin.Hosting.Tests.csproj
@@ -50,14 +50,6 @@
       <SpecificVersion>False</SpecificVersion>
       <HintPath>..\..\packages\Castle.Core.3.2.2\lib\net45\Castle.Core.dll</HintPath>
     </Reference>
-    <Reference Include="Castle.Windsor, Version=3.2.0.0, Culture=neutral, PublicKeyToken=407dd0808d44fbdc, processorArchitecture=MSIL">
-      <SpecificVersion>False</SpecificVersion>
-      <HintPath>..\..\packages\Castle.Windsor.3.2.1\lib\net45\Castle.Windsor.dll</HintPath>
-    </Reference>
-    <Reference Include="Ninject, Version=3.0.0.0, Culture=neutral, PublicKeyToken=c7192dc5380945e7, processorArchitecture=MSIL">
-      <SpecificVersion>False</SpecificVersion>
-      <HintPath>..\..\packages\Ninject.3.0.1.10\lib\net45-full\Ninject.dll</HintPath>
-    </Reference>
     <Reference Include="Owin, Version=1.0.0.0, Culture=neutral, PublicKeyToken=f0ebd12fd5e55cc5, processorArchitecture=MSIL">
       <SpecificVersion>False</SpecificVersion>
       <HintPath>..\..\packages\Owin.1.0\lib\net40\Owin.dll</HintPath>
@@ -93,14 +85,12 @@
     <Compile Include="ServerFactories\StaticServerFactory.cs" />
     <Compile Include="ServerFactories\InstanceServerFactory.cs" />
     <Compile Include="Containers\AutofacContainerTests.cs" />
-    <Compile Include="Containers\CastleWindsorContainerTests.cs" />
     <Compile Include="Containers\ContainerTestsBase.cs" />
     <Compile Include="Containers\DefaultServicesTests.cs" />
     <Compile Include="Containers\ReplaceDefaultServiceWithTextFileTests.cs" />
     <Compile Include="ServerFactoryLoaderTests.cs" />
     <Compile Include="GlobalSuppressions.cs" />
     <Compile Include="HostingEngineTests.cs" />
-    <Compile Include="Containers\NinjectContainerTests.cs" />
     <Compile Include="Properties\AssemblyInfo.cs" />
     <Compile Include="ServerFactories\OwinServerFactory.cs" />
     <Compile Include="ServerFactoryAdapterTests.cs" />

diff --git a/tests/Microsoft.Owin.Hosting.Tests/packages.config b/tests/Microsoft.Owin.Hosting.Tests/packages.config
@@ -2,8 +2,6 @@
 <packages>
   <package id="Autofac" version="3.3.0" targetFramework="net45" />
   <package id="Castle.Core" version="3.2.2" targetFramework="net45" />
-  <package id="Castle.Windsor" version="3.2.1" targetFramework="net45" />
-  <package id="Ninject" version="3.0.1.10" targetFramework="net45" />
   <package id="Owin" version="1.0" targetFramework="net45" />
   <package id="Shouldly" version="1.1.1.1" targetFramework="net45" />
   <package id="structuremap" version="2.6.4.1" targetFramework="net45" />

diff --git a/tests/Microsoft.Owin.Security.Tests/Microsoft.Owin.Security.Tests.csproj b/tests/Microsoft.Owin.Security.Tests/Microsoft.Owin.Security.Tests.csproj
@@ -43,6 +43,12 @@
       <HintPath>..\..\packages\Microsoft.IdentityModel.Logging.5.3.0\lib\net45\Microsoft.IdentityModel.Logging.dll</HintPath>
       <Private>True</Private>
     </Reference>
+    <Reference Include="Microsoft.IdentityModel.Protocols, Version=5.3.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
+      <HintPath>..\..\packages\Microsoft.IdentityModel.Protocols.5.3.0\lib\net45\Microsoft.IdentityModel.Protocols.dll</HintPath>
+    </Reference>
+    <Reference Include="Microsoft.IdentityModel.Protocols.OpenIdConnect, Version=5.3.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
+      <HintPath>..\..\packages\Microsoft.IdentityModel.Protocols.OpenIdConnect.5.3.0\lib\net45\Microsoft.IdentityModel.Protocols.OpenIdConnect.dll</HintPath>
+    </Reference>
     <Reference Include="Microsoft.IdentityModel.Tokens, Version=5.3.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
       <HintPath>..\..\packages\Microsoft.IdentityModel.Tokens.5.3.0\lib\net45\Microsoft.IdentityModel.Tokens.dll</HintPath>
       <Private>True</Private>
@@ -89,6 +95,7 @@
     <Compile Include="Facebook\FacebookMiddlewareTests.cs" />
     <Compile Include="GlobalSuppressions.cs" />
     <Compile Include="Google\GoogleOAuth2MiddlewareTests.cs" />
+    <Compile Include="OpenIdConnect\OpenIdConnectMiddlewareTests.cs" />
     <Compile Include="MicrosoftAccount\MicrosoftAccountMiddlewareTests.cs" />
     <Compile Include="OAuth\OAuth2AuthorizationCustomGrantTests.cs" />
     <Compile Include="OAuth\OAuth2BearerTokenTests.cs" />