Skip to content
This repository was archived by the owner on Dec 13, 2018. It is now read-only.

Replace INonceCache by IDistributedCache #293

Merged
merged 1 commit into from
Jun 29, 2015
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -12,6 +12,7 @@
using Microsoft.AspNet.Http;
using Microsoft.AspNet.Http.Authentication;
using Microsoft.AspNet.Http.Features.Authentication;
using Microsoft.Framework.Caching.Distributed;
using Microsoft.Framework.Internal;
using Microsoft.Framework.Logging;
using Microsoft.IdentityModel.Protocols;
Expand Down Expand Up @@ -149,13 +150,18 @@ protected override async Task<bool> HandleUnauthorizedAsync([NotNull] ChallengeC
if (Options.ProtocolValidator.RequireNonce)
{
message.Nonce = Options.ProtocolValidator.GenerateNonce();
if (Options.NonceCache != null)
if (Options.CacheNonces)
{
if (!Options.NonceCache.TryAddNonce(message.Nonce))
if (await Options.NonceCache.GetAsync(message.Nonce) != null)
{
Logger.LogError(Resources.OIDCH_0033_TryAddNonceFailed, message.Nonce);
throw new OpenIdConnectProtocolException(string.Format(CultureInfo.InvariantCulture, Resources.OIDCH_0033_TryAddNonceFailed, message.Nonce));
Logger.LogError(Resources.OIDCH_0033_NonceAlreadyExists, message.Nonce);
throw new OpenIdConnectProtocolException(string.Format(CultureInfo.InvariantCulture, Resources.OIDCH_0033_NonceAlreadyExists, message.Nonce));
}

await Options.NonceCache.SetAsync(message.Nonce, new byte[0], new DistributedCacheEntryOptions
{
AbsoluteExpirationRelativeToNow = Options.ProtocolValidator.NonceLifetime
});
}
else
{
Expand Down Expand Up @@ -389,11 +395,16 @@ public override async Task<AuthenticationTicket> AuthenticateAsync()
}

string nonce = jwt.Payload.Nonce;
if (Options.NonceCache != null)
if (Options.CacheNonces)
{
// if the nonce cannot be removed, it was used
if (!Options.NonceCache.TryRemoveNonce(nonce))
if (await Options.NonceCache.GetAsync(nonce) != null)
{
await Options.NonceCache.RemoveAsync(nonce);
}
else
{
// If the nonce cannot be removed, it was
// already used and MUST be rejected.
nonce = null;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@
using Microsoft.AspNet.Builder;
using Microsoft.AspNet.DataProtection;
using Microsoft.AspNet.Http;
using Microsoft.Framework.Caching.Distributed;
using Microsoft.Framework.DependencyInjection;
using Microsoft.Framework.Internal;
using Microsoft.Framework.Logging;
using Microsoft.Framework.OptionsModel;
using Microsoft.IdentityModel.Protocols;
using Microsoft.Framework.Internal;
using Microsoft.Framework.WebEncoders;
using Microsoft.IdentityModel.Protocols;

namespace Microsoft.AspNet.Authentication.OpenIdConnect
{
Expand All @@ -42,6 +44,7 @@ public OpenIdConnectAuthenticationMiddleware(
[NotNull] IDataProtectionProvider dataProtectionProvider,
[NotNull] ILoggerFactory loggerFactory,
[NotNull] IUrlEncoder encoder,
[NotNull] IServiceProvider services,
[NotNull] IOptions<ExternalAuthenticationOptions> externalOptions,
[NotNull] IOptions<OpenIdConnectAuthenticationOptions> options,
ConfigureOptions<OpenIdConnectAuthenticationOptions> configureOptions = null)
Expand Down Expand Up @@ -125,6 +128,13 @@ public OpenIdConnectAuthenticationMiddleware(
Options.ConfigurationManager = new ConfigurationManager<OpenIdConnectConfiguration>(Options.MetadataAddress, httpClient);
}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Silent failure/fallback for misconfiguration is hard to debug. I'd expect something more like:

if  (Options.CacheNonces && Options.NonceCache == null)
{
  Options.NonceCache = services.GetRequiredService<IDistributedCache>(); // throws
}

Or maybe GetServices and check for null to throw your own error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed using the first option 😄

if (Options.CacheNonces && Options.NonceCache == null)
{
// Use the global distributed cache if the user has not provided his own instance.
// Note: GetRequiredService will throw an exception if caching services have not been registered.
Options.NonceCache = services.GetRequiredService<IDistributedCache>();
}
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Net.Http;
using Microsoft.AspNet.Http;
using Microsoft.AspNet.Http.Authentication;
using Microsoft.Framework.Caching.Distributed;
using Microsoft.Framework.Internal;
using Microsoft.IdentityModel.Protocols;

Expand Down Expand Up @@ -173,7 +174,13 @@ public string Caption
/// recommends adding a nonce to a request as a mitigation against replay attacks when requesting id_tokens.
/// By default the runtime uses cookies with unique names generated from a hash of the nonce.
/// </summary>
public INonceCache NonceCache { get; set; }
public IDistributedCache NonceCache { get; set; }

/// <summary>
/// Gets or sets the value indicating whether nonces should be stored in the distributed cache or not.
/// The default value, <c>false</c>, is used to store nonces in client cookies.
/// </summary>
public bool CacheNonces { get; set; }

/// <summary>
/// Gets or sets the <see cref="OpenIdConnectAuthenticationNotifications"/> to notify when processing OpenIdConnect messages.
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@
<data name="OIDCH_0032_UsingCurrentUriRedirectUri" xml:space="preserve">
<value>OIDCH_0032: using the CurrentUri for 'local redirect' post authentication: '{0}'.</value>
</data>
<data name="OIDCH_0033_TryAddNonceFailed" xml:space="preserve">
<value>OIDCH_0033: ProtocolValidator.RequireNonce == true. Options.NonceCache.TryAddNonce returned false. This usually indicates the nonce is not unique or has been used. The nonce is: '{0}'.</value>
<data name="OIDCH_0033_NonceAlreadyExists" xml:space="preserve">
<value>OIDCH_0033: ProtocolValidator.RequireNonce == true. The generated nonce already exists: this usually indicates the nonce is not unique or has been used. The nonce is: '{0}'.</value>
</data>
<data name="OIDCH_0034_RedirectToIdentityProviderNotificationHandledResponse" xml:space="preserve">
<value>OIDCH_0034: redirectToIdentityProviderNotification.HandledResponse</value>
Expand Down Expand Up @@ -222,4 +222,4 @@
<data name="OIDCH_0020_IdTokenReceived" xml:space="preserve">
<value>OIDCH_0020: 'id_token' received: '{0}'</value>
</data>
</root>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"description": "ASP.NET 5 middleware that enables an application to support OpenIdConnect authentication workflow.",
"dependencies": {
"Microsoft.AspNet.Authentication": "1.0.0-*",
"Microsoft.Framework.Caching.Abstractions": "1.0.0-*",
"Microsoft.Framework.NotNullAttribute.Sources": { "type": "build", "version": "1.0.0-*" },
"Microsoft.IdentityModel.Protocol.Extensions": "2.0.0-beta5-*"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,5 @@ public static IServiceCollection ConfigureClaimsTransformation([NotNull] this IS
{
return services.Configure<ClaimsTransformationOptions>(o => o.Transformer = new ClaimsTransformer { TransformAsyncDelegate = asyncTransform });
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,7 @@ private static TestServer CreateServer(IOptions<OpenIdConnectAuthenticationOptio
},
services =>
{
services.AddAuthentication();
services.AddWebEncoders();
services.AddDataProtection();
}
Expand All @@ -456,6 +457,7 @@ private static TestServer CreateServer(CustomConfigureOptions configureOptions,
},
services =>
{
services.AddAuthentication();
services.AddWebEncoders();
services.AddDataProtection();
}
Expand Down Expand Up @@ -571,11 +573,12 @@ public CustomOpenIdConnectAuthenticationMiddleware(
IDataProtectionProvider dataProtectionProvider,
ILoggerFactory loggerFactory,
IUrlEncoder encoder,
IServiceProvider services,
IOptions<ExternalAuthenticationOptions> externalOptions,
IOptions<OpenIdConnectAuthenticationOptions> options,
ConfigureOptions<OpenIdConnectAuthenticationOptions> configureOptions = null
)
: base(next, dataProtectionProvider, loggerFactory, encoder, externalOptions, options, configureOptions)
: base(next, dataProtectionProvider, loggerFactory, encoder, services, externalOptions, options, configureOptions)
{
Logger = (loggerFactory as CustomLoggerFactory).Logger;
}
Expand Down