From aa79131335e2831d5f360c23b53344aa347bd51f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Chalet?= Date: Wed, 5 Sep 2018 18:24:04 +0200 Subject: [PATCH] Introduce OpenIddictServerBuilder.SetRequestCachingPolicy() --- .../Managers/OpenIddictScopeManager.cs | 5 ++ .../Internal/OpenIddictServerInitializer.cs | 5 ++ ...OpenIddictServerProvider.Authentication.cs | 7 +-- .../OpenIddictServerProvider.Session.cs | 7 +-- .../OpenIddictServerBuilder.cs | 18 +++++++ .../OpenIddictServerOptions.cs | 10 ++++ .../OpenIddictServerInitializerTests.cs | 24 +++++++++ ...ddictServerProviderTests.Authentication.cs | 11 ++++- .../OpenIddictServerProviderTests.Session.cs | 11 ++++- .../OpenIddictServerBuilderTests.cs | 49 +++++++++++++++++++ 10 files changed, 133 insertions(+), 14 deletions(-) diff --git a/src/OpenIddict.Core/Managers/OpenIddictScopeManager.cs b/src/OpenIddict.Core/Managers/OpenIddictScopeManager.cs index 80849fd0..38179591 100644 --- a/src/OpenIddict.Core/Managers/OpenIddictScopeManager.cs +++ b/src/OpenIddict.Core/Managers/OpenIddictScopeManager.cs @@ -211,6 +211,11 @@ namespace OpenIddict.Core public virtual async Task> FindByNamesAsync( ImmutableArray names, CancellationToken cancellationToken = default) { + if (names.IsDefaultOrEmpty) + { + return ImmutableArray.Create(); + } + if (names.Any(name => string.IsNullOrEmpty(name))) { throw new ArgumentException("Scope names cannot be null or empty.", nameof(names)); diff --git a/src/OpenIddict.Server/Internal/OpenIddictServerInitializer.cs b/src/OpenIddict.Server/Internal/OpenIddictServerInitializer.cs index 8544329f..0cad4746 100644 --- a/src/OpenIddict.Server/Internal/OpenIddictServerInitializer.cs +++ b/src/OpenIddict.Server/Internal/OpenIddictServerInitializer.cs @@ -152,6 +152,11 @@ namespace OpenIddict.Server.Internal "The token endpoint must be enabled to use the authorization code, client credentials, password and refresh token flows."); } + if (options.EnableRequestCaching && options.RequestCachingPolicy == null) + { + throw new InvalidOperationException("A caching policy must be specified when enabling request caching."); + } + if (options.RevocationEndpointPath.HasValue && options.DisableTokenStorage) { throw new InvalidOperationException("The revocation endpoint cannot be enabled when token storage is disabled."); diff --git a/src/OpenIddict.Server/Internal/OpenIddictServerProvider.Authentication.cs b/src/OpenIddict.Server/Internal/OpenIddictServerProvider.Authentication.cs index f78cb1af..39a0ee06 100644 --- a/src/OpenIddict.Server/Internal/OpenIddictServerProvider.Authentication.cs +++ b/src/OpenIddict.Server/Internal/OpenIddictServerProvider.Authentication.cs @@ -15,7 +15,6 @@ using AspNet.Security.OpenIdConnect.Server; using JetBrains.Annotations; using Microsoft.AspNetCore.Diagnostics; using Microsoft.AspNetCore.WebUtilities; -using Microsoft.Extensions.Caching.Distributed; using Microsoft.Extensions.Logging; using Microsoft.IdentityModel.Tokens; using Newtonsoft.Json; @@ -469,11 +468,7 @@ namespace OpenIddict.Server.Internal // to avoid collisions with the other types of cached requests. var key = OpenIddictConstants.Environment.AuthorizationRequest + context.Request.RequestId; - await options.Cache.SetAsync(key, stream.ToArray(), new DistributedCacheEntryOptions - { - AbsoluteExpiration = context.Options.SystemClock.UtcNow + TimeSpan.FromMinutes(30), - SlidingExpiration = TimeSpan.FromMinutes(10) - }); + await options.Cache.SetAsync(key, stream.ToArray(), options.RequestCachingPolicy); // Create a new authorization request containing only the request_id parameter. var address = QueryHelpers.AddQueryString( diff --git a/src/OpenIddict.Server/Internal/OpenIddictServerProvider.Session.cs b/src/OpenIddict.Server/Internal/OpenIddictServerProvider.Session.cs index 9a3279d4..10cad9a8 100644 --- a/src/OpenIddict.Server/Internal/OpenIddictServerProvider.Session.cs +++ b/src/OpenIddict.Server/Internal/OpenIddictServerProvider.Session.cs @@ -12,7 +12,6 @@ using AspNet.Security.OpenIdConnect.Server; using JetBrains.Annotations; using Microsoft.AspNetCore.Diagnostics; using Microsoft.AspNetCore.WebUtilities; -using Microsoft.Extensions.Caching.Distributed; using Microsoft.Extensions.Logging; using Microsoft.IdentityModel.Tokens; using Newtonsoft.Json; @@ -187,11 +186,7 @@ namespace OpenIddict.Server.Internal // to avoid collisions with the other types of cached requests. var key = OpenIddictConstants.Environment.LogoutRequest + context.Request.RequestId; - await options.Cache.SetAsync(key, stream.ToArray(), new DistributedCacheEntryOptions - { - AbsoluteExpiration = context.Options.SystemClock.UtcNow + TimeSpan.FromMinutes(30), - SlidingExpiration = TimeSpan.FromMinutes(10) - }); + await options.Cache.SetAsync(key, stream.ToArray(), options.RequestCachingPolicy); // Create a new logout request containing only the request_id parameter. var address = QueryHelpers.AddQueryString( diff --git a/src/OpenIddict.Server/OpenIddictServerBuilder.cs b/src/OpenIddict.Server/OpenIddictServerBuilder.cs index 9b12a209..316f03cd 100644 --- a/src/OpenIddict.Server/OpenIddictServerBuilder.cs +++ b/src/OpenIddict.Server/OpenIddictServerBuilder.cs @@ -18,6 +18,7 @@ using AspNet.Security.OpenIdConnect.Primitives; using JetBrains.Annotations; using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Caching.Distributed; using Microsoft.IdentityModel.Tokens; using OpenIddict.Server; @@ -741,6 +742,23 @@ namespace Microsoft.Extensions.DependencyInjection public OpenIddictServerBuilder SetRefreshTokenLifetime([CanBeNull] TimeSpan? lifetime) => Configure(options => options.RefreshTokenLifetime = lifetime); + /// + /// Sets the caching policy used to determine how long the authorization and + /// end session requests should be cached by the distributed cache implementation. + /// Note: the specified policy is only used when request caching is explicitly enabled. + /// + /// The request caching policy. + /// The . + public OpenIddictServerBuilder SetRequestCachingPolicy([NotNull] DistributedCacheEntryOptions policy) + { + if (policy == null) + { + throw new ArgumentNullException(nameof(policy)); + } + + return Configure(options => options.RequestCachingPolicy = policy); + } + /// /// Sets the issuer address, which is used as the base address /// for the endpoint URIs returned from the discovery endpoint. diff --git a/src/OpenIddict.Server/OpenIddictServerOptions.cs b/src/OpenIddict.Server/OpenIddictServerOptions.cs index 7f1d5a52..50b61cbd 100644 --- a/src/OpenIddict.Server/OpenIddictServerOptions.cs +++ b/src/OpenIddict.Server/OpenIddictServerOptions.cs @@ -113,6 +113,16 @@ namespace OpenIddict.Server /// public RandomNumberGenerator RandomNumberGenerator { get; set; } = RandomNumberGenerator.Create(); + /// + /// Gets or sets the caching policy used to determine how long the authorization + /// and end session requests should be cached by the distributed cache implementation. + /// + public DistributedCacheEntryOptions RequestCachingPolicy { get; set; } = new DistributedCacheEntryOptions + { + AbsoluteExpirationRelativeToNow = TimeSpan.FromHours(1), + SlidingExpiration = TimeSpan.FromMinutes(30) + }; + /// /// Gets the OAuth2/OpenID Connect scopes enabled for this application. /// diff --git a/test/OpenIddict.Server.Tests/Internal/OpenIddictServerInitializerTests.cs b/test/OpenIddict.Server.Tests/Internal/OpenIddictServerInitializerTests.cs index fa1d503d..fe9b50bd 100644 --- a/test/OpenIddict.Server.Tests/Internal/OpenIddictServerInitializerTests.cs +++ b/test/OpenIddict.Server.Tests/Internal/OpenIddictServerInitializerTests.cs @@ -164,6 +164,30 @@ namespace OpenIddict.Server.Internal.Tests "client credentials, password and refresh token flows.", exception.Message); } + [Fact] + public async Task PostConfigure_ThrowsAnExceptionWhenCachingPolicyIsNullAndRequestCachingEnabled() + { + // Arrange + var server = CreateAuthorizationServer(builder => + { + builder.EnableAuthorizationEndpoint("/connect/authorize") + .AllowImplicitFlow() + .EnableRequestCaching(); + + builder.Configure(options => options.RequestCachingPolicy = null); + }); + + var client = new OpenIdConnectClient(server.CreateClient()); + + // Act and assert + var exception = await Assert.ThrowsAsync(delegate + { + return client.GetAsync("/"); + }); + + Assert.Equal("A caching policy must be specified when enabling request caching.", exception.Message); + } + [Fact] public async Task PostConfigure_ThrowsAnExceptionWhenTokenStorageIsDisabled() { diff --git a/test/OpenIddict.Server.Tests/Internal/OpenIddictServerProviderTests.Authentication.cs b/test/OpenIddict.Server.Tests/Internal/OpenIddictServerProviderTests.Authentication.cs index c18fbe7c..c25ec6e5 100644 --- a/test/OpenIddict.Server.Tests/Internal/OpenIddictServerProviderTests.Authentication.cs +++ b/test/OpenIddict.Server.Tests/Internal/OpenIddictServerProviderTests.Authentication.cs @@ -4,6 +4,7 @@ * the license and the contributors participating to this project. */ +using System; using System.Collections.Immutable; using System.IO; using System.Security.Cryptography; @@ -828,6 +829,12 @@ namespace OpenIddict.Server.Internal.Tests builder.EnableRequestCaching(); + builder.SetRequestCachingPolicy(new DistributedCacheEntryOptions + { + AbsoluteExpirationRelativeToNow = TimeSpan.FromDays(42), + SlidingExpiration = TimeSpan.FromSeconds(42) + }); + builder.Configure(options => options.RandomNumberGenerator = generator.Object); }); @@ -850,7 +857,9 @@ namespace OpenIddict.Server.Internal.Tests cache.Verify(mock => mock.SetAsync( OpenIddictConstants.Environment.AuthorizationRequest + identifier, It.IsAny(), - It.IsAny(), + It.Is(options => + options.AbsoluteExpirationRelativeToNow == TimeSpan.FromDays(42) && + options.SlidingExpiration == TimeSpan.FromSeconds(42)), It.IsAny()), Times.Once()); generator.Verify(mock => mock.GetBytes(It.Is(bytes => bytes.Length == 256 / 8)), Times.Once()); diff --git a/test/OpenIddict.Server.Tests/Internal/OpenIddictServerProviderTests.Session.cs b/test/OpenIddict.Server.Tests/Internal/OpenIddictServerProviderTests.Session.cs index 9a591b1c..4058b050 100644 --- a/test/OpenIddict.Server.Tests/Internal/OpenIddictServerProviderTests.Session.cs +++ b/test/OpenIddict.Server.Tests/Internal/OpenIddictServerProviderTests.Session.cs @@ -4,6 +4,7 @@ * the license and the contributors participating to this project. */ +using System; using System.Collections.Immutable; using System.Security.Cryptography; using System.Threading; @@ -136,6 +137,12 @@ namespace OpenIddict.Server.Internal.Tests builder.EnableRequestCaching(); + builder.SetRequestCachingPolicy(new DistributedCacheEntryOptions + { + AbsoluteExpirationRelativeToNow = TimeSpan.FromDays(42), + SlidingExpiration = TimeSpan.FromSeconds(42) + }); + builder.Configure(options => options.RandomNumberGenerator = generator.Object); }); @@ -156,7 +163,9 @@ namespace OpenIddict.Server.Internal.Tests cache.Verify(mock => mock.SetAsync( OpenIddictConstants.Environment.LogoutRequest + identifier, It.IsAny(), - It.IsAny(), + It.Is(options => + options.AbsoluteExpirationRelativeToNow == TimeSpan.FromDays(42) && + options.SlidingExpiration == TimeSpan.FromSeconds(42)), It.IsAny()), Times.Once()); generator.Verify(mock => mock.GetBytes(It.Is(bytes => bytes.Length == 256 / 8)), Times.Once()); diff --git a/test/OpenIddict.Server.Tests/OpenIddictServerBuilderTests.cs b/test/OpenIddict.Server.Tests/OpenIddictServerBuilderTests.cs index 0d417a4a..c0689983 100644 --- a/test/OpenIddict.Server.Tests/OpenIddictServerBuilderTests.cs +++ b/test/OpenIddict.Server.Tests/OpenIddictServerBuilderTests.cs @@ -12,6 +12,7 @@ using System.Threading.Tasks; using AspNet.Security.OpenIdConnect.Primitives; using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Caching.Distributed; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using Microsoft.IdentityModel.Tokens; @@ -713,6 +714,54 @@ namespace OpenIddict.Server.Tests Assert.Null(options.RefreshTokenLifetime); } + [Fact] + public void SetRequestCachingPolicy_ThrowsAnExceptionForNullPolicy() + { + // Arrange + var services = CreateServices(); + var builder = CreateBuilder(services); + + // Act and assert + var exception = Assert.Throws(() => builder.SetRequestCachingPolicy(null)); + + Assert.Equal("policy", exception.ParamName); + } + + [Fact] + public void SetRequestCachingPolicy_PolicyIsUpdated() + { + // Arrange + var services = CreateServices(); + var builder = CreateBuilder(services); + + var policy = new DistributedCacheEntryOptions + { + AbsoluteExpirationRelativeToNow = TimeSpan.FromDays(42), + SlidingExpiration = TimeSpan.FromSeconds(42) + }; + + // Act + builder.SetRequestCachingPolicy(policy); + + var options = GetOptions(services); + + // Assert + Assert.Same(policy, options.RequestCachingPolicy); + } + + [Fact] + public void SetIssuer_ThrowsAnExceptionForNullIssuer() + { + // Arrange + var services = CreateServices(); + var builder = CreateBuilder(services); + + // Act and assert + var exception = Assert.Throws(() => builder.SetIssuer(null)); + + Assert.Equal("address", exception.ParamName); + } + [Fact] public void SetIssuer_AddressIsReplaced() {