From 4741e031ed292468d6f5c04dfec313e5cfa16240 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Chalet?= Date: Wed, 6 Sep 2017 16:32:36 +0200 Subject: [PATCH] Introduce OpenIddictApplicationManager.ValidateLogoutRedirectUriAsync() and allow multiple clients to share the same logout redirect URL --- .../Managers/OpenIddictApplicationManager.cs | 72 +++++++++++++++---- .../Stores/IOpenIddictApplicationStore.cs | 19 +++-- .../Stores/OpenIddictApplicationStore.cs | 24 +++++-- src/OpenIddict/OpenIddictProvider.Session.cs | 20 +++--- .../OpenIddictProviderTests.Session.cs | 22 +++--- 5 files changed, 108 insertions(+), 49 deletions(-) diff --git a/src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs b/src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs index 9e917f21..bcef21ac 100644 --- a/src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs +++ b/src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs @@ -154,17 +154,31 @@ namespace OpenIddict.Core } /// - /// Retrieves an application using its post_logout_redirect_uri. + /// Retrieves all the applications associated with the specified post_logout_redirect_uri. /// - /// The post_logout_redirect_uri associated with the application. + /// The post_logout_redirect_uri associated with the applications. /// The that can be used to abort the operation. /// /// A that can be used to monitor the asynchronous operation, whose result - /// returns the client application corresponding to the post_logout_redirect_uri. + /// returns the client applications corresponding to the specified post_logout_redirect_uri. /// - public virtual Task FindByLogoutRedirectUri(string url, CancellationToken cancellationToken) + public virtual Task FindByLogoutRedirectUri(string address, CancellationToken cancellationToken) { - return Store.FindByLogoutRedirectUri(url, cancellationToken); + return Store.FindByLogoutRedirectUri(address, cancellationToken); + } + + /// + /// Retrieves all the applications associated with the specified redirect_uri. + /// + /// The redirect_uri associated with the applications. + /// The that can be used to abort the operation. + /// + /// A that can be used to monitor the asynchronous operation, whose result + /// returns the client applications corresponding to the specified redirect_uri. + /// + public virtual Task FindByRedirectUri(string address, CancellationToken cancellationToken) + { + return Store.FindByRedirectUri(address, cancellationToken); } /// @@ -469,9 +483,8 @@ namespace OpenIddict.Core var address = await Store.GetRedirectUriAsync(application, cancellationToken); if (!string.IsNullOrEmpty(address)) { - Uri uri; // Ensure the redirect_uri is a valid and absolute URL. - if (!Uri.TryCreate(address, UriKind.Absolute, out uri)) + if (!Uri.TryCreate(address, UriKind.Absolute, out Uri uri)) { throw new ArgumentException("The redirect_uri must be an absolute URL."); } @@ -487,9 +500,8 @@ namespace OpenIddict.Core address = await Store.GetLogoutRedirectUriAsync(application, cancellationToken); if (!string.IsNullOrEmpty(address)) { - Uri uri; // Ensure the post_logout_redirect_uri is a valid and absolute URL. - if (!Uri.TryCreate(address, UriKind.Absolute, out uri)) + if (!Uri.TryCreate(address, UriKind.Absolute, out Uri uri)) { throw new ArgumentException("The post_logout_redirect_uri must be an absolute URL."); } @@ -502,6 +514,34 @@ namespace OpenIddict.Core } } + /// + /// Validates the specified post_logout_redirect_uri. + /// + /// The address that should be compared to the post_logout_redirect_uri stored in the database. + /// The that can be used to abort the operation. + /// + /// A that can be used to monitor the asynchronous operation, whose result + /// returns a boolean indicating whether the post_logout_redirect_uri was valid. + /// + public virtual async Task ValidateLogoutRedirectUriAsync(string address, CancellationToken cancellationToken) + { + // Warning: SQL engines like Microsoft SQL Server are known to use case-insensitive lookups by default. + // To ensure a case-sensitive comparison is used, string.Equals(Ordinal) is manually called here. + foreach (var application in await Store.FindByLogoutRedirectUri(address, cancellationToken)) + { + // Note: the post_logout_redirect_uri must be compared using case-sensitive "Simple String Comparison". + if (string.Equals(address, await Store.GetLogoutRedirectUriAsync(application, cancellationToken), StringComparison.Ordinal)) + { + return true; + } + } + + Logger.LogWarning("Client validation failed because '{PostLogoutRedirectUri}' " + + "was not a valid post_logout_redirect_uri.", address); + + return false; + } + /// /// Validates the redirect_uri associated with an application. /// @@ -519,15 +559,17 @@ namespace OpenIddict.Core throw new ArgumentNullException(nameof(application)); } - if (!string.Equals(address, await Store.GetRedirectUriAsync(application, cancellationToken), StringComparison.Ordinal)) + // Note: the redirect_uri must be compared using case-sensitive "Simple String Comparison". + // See http://openid.net/specs/openid-connect-core-1_0.html#AuthRequest for more information. + if (string.Equals(address, await Store.GetRedirectUriAsync(application, cancellationToken), StringComparison.Ordinal)) { - Logger.LogWarning("Client validation failed because {RedirectUri} was not a valid redirect_uri " + - "for {Client}", address, await GetDisplayNameAsync(application, cancellationToken)); - - return false; + return true; } - return true; + Logger.LogWarning("Client validation failed because '{RedirectUri}' was not a valid redirect_uri " + + "for '{Client}'.", address, await GetDisplayNameAsync(application, cancellationToken)); + + return false; } /// diff --git a/src/OpenIddict.Core/Stores/IOpenIddictApplicationStore.cs b/src/OpenIddict.Core/Stores/IOpenIddictApplicationStore.cs index 1a212ff0..972340bf 100644 --- a/src/OpenIddict.Core/Stores/IOpenIddictApplicationStore.cs +++ b/src/OpenIddict.Core/Stores/IOpenIddictApplicationStore.cs @@ -60,15 +60,26 @@ namespace OpenIddict.Core Task FindByClientIdAsync(string identifier, CancellationToken cancellationToken); /// - /// Retrieves an application using its post_logout_redirect_uri. + /// Retrieves all the applications associated with the specified post_logout_redirect_uri. /// - /// The post_logout_redirect_uri associated with the application. + /// The post_logout_redirect_uri associated with the applications. /// The that can be used to abort the operation. /// /// A that can be used to monitor the asynchronous operation, whose result - /// returns the client application corresponding to the post_logout_redirect_uri. + /// returns the client applications corresponding to the specified post_logout_redirect_uri. /// - Task FindByLogoutRedirectUri(string url, CancellationToken cancellationToken); + Task FindByLogoutRedirectUri(string address, CancellationToken cancellationToken); + + /// + /// Retrieves all the applications associated with the specified redirect_uri. + /// + /// The redirect_uri associated with the applications. + /// The that can be used to abort the operation. + /// + /// A that can be used to monitor the asynchronous operation, whose result + /// returns the client applications corresponding to the specified redirect_uri. + /// + Task FindByRedirectUri(string address, CancellationToken cancellationToken); /// /// Retrieves the client identifier associated with an application. diff --git a/src/OpenIddict.EntityFrameworkCore/Stores/OpenIddictApplicationStore.cs b/src/OpenIddict.EntityFrameworkCore/Stores/OpenIddictApplicationStore.cs index 264489db..e6aaa966 100644 --- a/src/OpenIddict.EntityFrameworkCore/Stores/OpenIddictApplicationStore.cs +++ b/src/OpenIddict.EntityFrameworkCore/Stores/OpenIddictApplicationStore.cs @@ -156,17 +156,31 @@ namespace OpenIddict.EntityFrameworkCore } /// - /// Retrieves an application using its post_logout_redirect_uri. + /// Retrieves all the applications associated with the specified post_logout_redirect_uri. /// - /// The post_logout_redirect_uri associated with the application. + /// The post_logout_redirect_uri associated with the applications. /// The that can be used to abort the operation. /// /// A that can be used to monitor the asynchronous operation, whose result - /// returns the client application corresponding to the post_logout_redirect_uri. + /// returns the client applications corresponding to the specified post_logout_redirect_uri. /// - public virtual Task FindByLogoutRedirectUri(string url, CancellationToken cancellationToken) + public virtual Task FindByLogoutRedirectUri(string address, CancellationToken cancellationToken) { - return Applications.SingleOrDefaultAsync(application => application.LogoutRedirectUri == url, cancellationToken); + return Applications.Where(application => application.LogoutRedirectUri == address).ToArrayAsync(cancellationToken); + } + + /// + /// Retrieves all the applications associated with the specified redirect_uri. + /// + /// The redirect_uri associated with the applications. + /// The that can be used to abort the operation. + /// + /// A that can be used to monitor the asynchronous operation, whose result + /// returns the client applications corresponding to the specified redirect_uri. + /// + public virtual Task FindByRedirectUri(string address, CancellationToken cancellationToken) + { + return Applications.Where(application => application.RedirectUri == address).ToArrayAsync(cancellationToken); } /// diff --git a/src/OpenIddict/OpenIddictProvider.Session.cs b/src/OpenIddict/OpenIddictProvider.Session.cs index e8e6c5f6..66066bdb 100644 --- a/src/OpenIddict/OpenIddictProvider.Session.cs +++ b/src/OpenIddict/OpenIddictProvider.Session.cs @@ -82,21 +82,17 @@ namespace OpenIddict public override async Task ValidateLogoutRequest([NotNull] ValidateLogoutRequestContext context) { // If an optional post_logout_redirect_uri was provided, validate it. - if (!string.IsNullOrEmpty(context.PostLogoutRedirectUri)) + if (!string.IsNullOrEmpty(context.PostLogoutRedirectUri) && + !await Applications.ValidateLogoutRedirectUriAsync(context.PostLogoutRedirectUri, context.HttpContext.RequestAborted)) { - var application = await Applications.FindByLogoutRedirectUri(context.PostLogoutRedirectUri, context.HttpContext.RequestAborted); - if (application == null) - { - Logger.LogError("The logout request was rejected because the client application corresponding " + - "to the specified post_logout_redirect_uri was not found in the database: " + - "'{PostLogoutRedirectUri}'.", context.PostLogoutRedirectUri); + Logger.LogError("The logout request was rejected because the specified post_logout_redirect_uri " + + "was invalid: '{PostLogoutRedirectUri}'.", context.PostLogoutRedirectUri); - context.Reject( - error: OpenIdConnectConstants.Errors.InvalidClient, - description: "Invalid post_logout_redirect_uri."); + context.Reject( + error: OpenIdConnectConstants.Errors.InvalidClient, + description: "Invalid post_logout_redirect_uri."); - return; - } + return; } context.Validate(); diff --git a/test/OpenIddict.Tests/OpenIddictProviderTests.Session.cs b/test/OpenIddict.Tests/OpenIddictProviderTests.Session.cs index 2e692099..e6dbc6a1 100644 --- a/test/OpenIddict.Tests/OpenIddictProviderTests.Session.cs +++ b/test/OpenIddict.Tests/OpenIddictProviderTests.Session.cs @@ -64,8 +64,8 @@ namespace OpenIddict.Tests // Arrange var manager = CreateApplicationManager(instance => { - instance.Setup(mock => mock.FindByLogoutRedirectUri("http://www.fabrikam.com/path", It.IsAny())) - .ReturnsAsync(value: null); + instance.Setup(mock => mock.ValidateLogoutRedirectUriAsync("http://www.fabrikam.com/path", It.IsAny())) + .ReturnsAsync(false); }); var server = CreateAuthorizationServer(builder => @@ -85,7 +85,7 @@ namespace OpenIddict.Tests Assert.Equal(OpenIdConnectConstants.Errors.InvalidClient, response.Error); Assert.Equal("Invalid post_logout_redirect_uri.", response.ErrorDescription); - Mock.Get(manager).Verify(mock => mock.FindByLogoutRedirectUri("http://www.fabrikam.com/path", It.IsAny()), Times.Once()); + Mock.Get(manager).Verify(mock => mock.ValidateLogoutRedirectUriAsync("http://www.fabrikam.com/path", It.IsAny()), Times.Once()); } [Fact] @@ -98,10 +98,8 @@ namespace OpenIddict.Tests { builder.Services.AddSingleton(CreateApplicationManager(instance => { - var application = new OpenIddictApplication(); - - instance.Setup(mock => mock.FindByLogoutRedirectUri("http://www.fabrikam.com/path", It.IsAny())) - .ReturnsAsync(application); + instance.Setup(mock => mock.ValidateLogoutRedirectUriAsync("http://www.fabrikam.com/path", It.IsAny())) + .ReturnsAsync(true); })); builder.Services.AddSingleton(cache.Object); @@ -138,10 +136,8 @@ namespace OpenIddict.Tests { builder.Services.AddSingleton(CreateApplicationManager(instance => { - var application = new OpenIddictApplication(); - - instance.Setup(mock => mock.FindByLogoutRedirectUri("http://www.fabrikam.com/path", It.IsAny())) - .ReturnsAsync(application); + instance.Setup(mock => mock.ValidateLogoutRedirectUriAsync("http://www.fabrikam.com/path", It.IsAny())) + .ReturnsAsync(true); })); }); @@ -166,8 +162,8 @@ namespace OpenIddict.Tests { builder.Services.AddSingleton(CreateApplicationManager(instance => { - instance.Setup(mock => mock.FindByLogoutRedirectUri("http://www.fabrikam.com/path", It.IsAny())) - .ReturnsAsync(value: null); + instance.Setup(mock => mock.ValidateLogoutRedirectUriAsync("http://www.fabrikam.com/path", It.IsAny())) + .ReturnsAsync(false); })); builder.EnableAuthorizationEndpoint("/logout-status-code-middleware");