Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport ASP.NET Core PKCE Support to OpenIdConnectAuthenticationHandler #389

Merged
merged 6 commits into from
Dec 10, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down
30 changes: 30 additions & 0 deletions src/Microsoft.Owin.Security.OpenIdConnect/OAuthConstants.cs
Original file line number Diff line number Diff line change
@@ -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";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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: false.</para>
Tratcher marked this conversation as resolved.
Show resolved Hide resolved
/// </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")]
Expand All @@ -71,6 +72,7 @@ public OpenIdConnectAuthenticationOptions(string authenticationType)
UseTokenLifetime = true;
CookieManager = new CookieManager();
RedeemCode = false;
UsePkce = false;
}

/// <summary>
Expand Down Expand Up @@ -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`.
rzontar marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
[SuppressMessage("Microsoft.Naming", "CA1704:IdentifiersShouldBeSpelledCorrectly", MessageId = "Pkce",
Justification = "Pkce is a valid acronym.")]
public bool UsePkce { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -155,10 +156,36 @@ 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. See RunAuthorizationCodeReceivedEventAsync.
properties.Dictionary.Add(OAuthConstants.CodeVerifierKey, codeVerifier);


rzontar marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down Expand Up @@ -397,6 +424,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)
{
Expand Down
18 changes: 10 additions & 8 deletions tests/Katana.Sandbox.WebServer/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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",
/*
Expand All @@ -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 =>
Expand All @@ -166,6 +167,7 @@ public void Configuration(IAppBuilder app)
// n.HandleCodeRedemption(message);
// }
//}
UsePkce = true,
});

/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
// 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.Linq;
using System.Net;
using System.Net.Http;
using System.Threading.Tasks;
using System.Xml.Linq;
using Microsoft.IdentityModel.Protocols.OpenIdConnect;
using Microsoft.Owin.Security.Cookies;
using Microsoft.Owin.Security.OpenIdConnect;
using Microsoft.Owin.Testing;
using Owin;
using Xunit;
using Xunit.Extensions;

namespace Microsoft.Owin.Security.Tests.OpenIdConnect
{
public class OpenIdConnectMiddlewareTests
{
[Theory]
[InlineData(true)]
[InlineData(false)]
public async Task ChallengeIncludesPkceIfRequested(bool include)
{
var options = new OpenIdConnectAuthenticationOptions()
{
Authority = "https://demo.identityserver.io",
ClientId = "Test Client Id",
ClientSecret = "Test Client Secret",
UsePkce = include,
ResponseType = OpenIdConnectResponseType.Code
};
var server = CreateServer(
app => app.UseOpenIdConnectAuthentication(options),
context =>
{
context.Authentication.Challenge("OpenIdConnect");
return true;
});

var transaction = await SendAsync(server, "http://example.com/challenge");

var res = transaction.Response;
Assert.Equal(HttpStatusCode.Redirect, res.StatusCode);
Assert.NotNull(res.Headers.Location);

if (include)
{
Assert.Contains("code_challenge=", res.Headers.Location.Query);
Assert.Contains("code_challenge_method=S256", res.Headers.Location.Query);
}
else
{
Assert.DoesNotContain("code_challenge=", res.Headers.Location.Query);
Assert.DoesNotContain("code_challenge_method=", res.Headers.Location.Query);
}
}

[Theory]
[InlineData(OpenIdConnectResponseType.Token)]
[InlineData(OpenIdConnectResponseType.IdToken)]
[InlineData(OpenIdConnectResponseType.CodeIdToken)]
public async Task ChallengeDoesNotIncludePkceForOtherResponseTypes(string responseType)
{
var options = new OpenIdConnectAuthenticationOptions()
{
Authority = "https://demo.identityserver.io",
ClientId = "Test Client Id",
ClientSecret = "Test Client Secret",
UsePkce = true,
ResponseType = responseType
};
var server = CreateServer(
app => app.UseOpenIdConnectAuthentication(options),
context =>
{
context.Authentication.Challenge("OpenIdConnect");
return true;
});

var transaction = await SendAsync(server, "http://example.com/challenge");

var res = transaction.Response;
Assert.Equal(HttpStatusCode.Redirect, res.StatusCode);
Assert.NotNull(res.Headers.Location);

Assert.DoesNotContain("code_challenge=", res.Headers.Location.Query);
Assert.DoesNotContain("code_challenge_method=", res.Headers.Location.Query);
}


private static TestServer CreateServer(Action<IAppBuilder> configure, Func<IOwinContext, bool> handler)
{
return TestServer.Create(app =>
{
app.Properties["host.AppName"] = "OpenIdConnect.Owin.Security.Tests";
app.UseCookieAuthentication(new CookieAuthenticationOptions
{
AuthenticationType = "External"
});
app.SetDefaultSignInAsAuthenticationType("External");
if (configure != null)
{
configure(app);
}
app.Use(async (context, next) =>
{
if (handler == null || !handler(context))
{
await next();
}
});
});
}

private static async Task<Transaction> SendAsync(TestServer server, string uri, string cookieHeader = null)
{
var request = new HttpRequestMessage(HttpMethod.Get, uri);
if (!string.IsNullOrEmpty(cookieHeader))
{
request.Headers.Add("Cookie", cookieHeader);
}
var transaction = new Transaction
{
Request = request,
Response = await server.HttpClient.SendAsync(request),
};
if (transaction.Response.Headers.Contains("Set-Cookie"))
{
transaction.SetCookie = transaction.Response.Headers.GetValues("Set-Cookie").ToList();
}
transaction.ResponseText = await transaction.Response.Content.ReadAsStringAsync();

if (transaction.Response.Content != null &&
transaction.Response.Content.Headers.ContentType != null &&
transaction.Response.Content.Headers.ContentType.MediaType == "text/xml")
{
transaction.ResponseElement = XElement.Parse(transaction.ResponseText);
}
return transaction;
}

private class Transaction
{
public HttpRequestMessage Request { get; set; }
public HttpResponseMessage Response { get; set; }
public IList<string> SetCookie { get; set; }
public string ResponseText { get; set; }
public XElement ResponseElement { get; set; }

public string AuthenticationCookieValue
{
get
{
if (SetCookie != null && SetCookie.Count > 0)
{
var authCookie = SetCookie.SingleOrDefault(c => c.Contains(".AspNet.External="));
if (authCookie != null)
{
return authCookie.Substring(0, authCookie.IndexOf(';'));
}
}

return null;
}
}

public string FindClaimValue(string claimType)
{
XElement claim = ResponseElement.Elements("claim").SingleOrDefault(elt => elt.Attribute("type").Value == claimType);
if (claim == null)
{
return null;
}
return claim.Attribute("value").Value;
}
}
}
}
2 changes: 2 additions & 0 deletions tests/Microsoft.Owin.Security.Tests/packages.config
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
<packages>
<package id="Microsoft.IdentityModel.JsonWebTokens" version="5.3.0" targetFramework="net45" />
<package id="Microsoft.IdentityModel.Logging" version="5.3.0" targetFramework="net45" />
<package id="Microsoft.IdentityModel.Protocols" version="5.3.0" targetFramework="net45" />
<package id="Microsoft.IdentityModel.Protocols.OpenIdConnect" version="5.3.0" targetFramework="net45" />
<package id="Microsoft.IdentityModel.Tokens" version="5.3.0" targetFramework="net45" />
<package id="Newtonsoft.Json" version="10.0.1" targetFramework="net45" />
<package id="Owin" version="1.0" targetFramework="net45" />
Expand Down