From 223a5c5b3ad5313b08943b14ca61a0059a046d11 Mon Sep 17 00:00:00 2001 From: James Gunn Date: Tue, 19 Sep 2023 09:33:16 +0100 Subject: [PATCH] Add wildcard support to post logout redirect URIs (#722) --- .../Models/Application.cs | 32 ++++++++ .../Oidc/TeacherIdentityApplicationManager.cs | 80 +++++++++---------- .../Oidc/TeacherIdentityApplicationStore.cs | 28 ++++++- .../ModelTests/ApplicationTests.cs | 32 ++++++++ 4 files changed, 129 insertions(+), 43 deletions(-) create mode 100644 dotnet-authserver/tests/TeacherIdentity.AuthServer.Tests/ModelTests/ApplicationTests.cs diff --git a/dotnet-authserver/src/TeacherIdentity.AuthServer/Models/Application.cs b/dotnet-authserver/src/TeacherIdentity.AuthServer/Models/Application.cs index 5c0d07c88..dac582b03 100644 --- a/dotnet-authserver/src/TeacherIdentity.AuthServer/Models/Application.cs +++ b/dotnet-authserver/src/TeacherIdentity.AuthServer/Models/Application.cs @@ -1,5 +1,6 @@ using System.ComponentModel; using System.Text.Json; +using System.Text.RegularExpressions; using OpenIddict.Abstractions; using OpenIddict.EntityFrameworkCore.Models; @@ -8,6 +9,37 @@ namespace TeacherIdentity.AuthServer.Models; public class Application : OpenIddictEntityFrameworkCoreApplication { private const string EmptyJsonArray = "[]"; + private const string RedirectUriWildcardPlaceholder = "__"; + + public static bool MatchUriPattern(string pattern, string uri, bool ignorePath) + { + if (!Uri.TryCreate(pattern, UriKind.Absolute, out _)) + { + throw new ArgumentException("A valid absolute URI must be specified.", nameof(pattern)); + } + + if (!Uri.TryCreate(uri, UriKind.Absolute, out _)) + { + throw new ArgumentException("A valid absolute URI must be specified.", nameof(uri)); + } + + var normalizedPattern = ignorePath ? RemovePathAndQuery(pattern) : pattern; + var normalizedUri = ignorePath ? RemovePathAndQuery(uri) : uri; + + if (normalizedPattern.Equals(normalizedUri, StringComparison.Ordinal)) + { + return true; + } + + if (normalizedPattern.Contains(RedirectUriWildcardPlaceholder)) + { + return Regex.IsMatch(normalizedUri, $"^{Regex.Escape(normalizedPattern).Replace(RedirectUriWildcardPlaceholder, ".*")}$"); + } + + return false; + + static string RemovePathAndQuery(string address) => new Uri(address).GetLeftPart(UriPartial.Authority); + } public string? ServiceUrl { get; set; } diff --git a/dotnet-authserver/src/TeacherIdentity.AuthServer/Oidc/TeacherIdentityApplicationManager.cs b/dotnet-authserver/src/TeacherIdentity.AuthServer/Oidc/TeacherIdentityApplicationManager.cs index 0d016765f..7701b4f4d 100644 --- a/dotnet-authserver/src/TeacherIdentity.AuthServer/Oidc/TeacherIdentityApplicationManager.cs +++ b/dotnet-authserver/src/TeacherIdentity.AuthServer/Oidc/TeacherIdentityApplicationManager.cs @@ -1,4 +1,4 @@ -using System.Text.RegularExpressions; +using System.Runtime.CompilerServices; using Microsoft.Extensions.Options; using OpenIddict.Abstractions; using OpenIddict.Core; @@ -8,8 +8,6 @@ namespace TeacherIdentity.AuthServer.Oidc; public partial class TeacherIdentityApplicationManager : OpenIddictApplicationManager { - private const string RedirectUriWildcardPlaceholder = "__"; - public TeacherIdentityApplicationManager( IOpenIddictApplicationCache cache, ILogger> logger, @@ -21,6 +19,41 @@ public TeacherIdentityApplicationManager( public new TeacherIdentityApplicationStore Store => (TeacherIdentityApplicationStore)base.Store; + public override IAsyncEnumerable FindByPostLogoutRedirectUriAsync(string address, CancellationToken cancellationToken = default) + { + if (string.IsNullOrEmpty(address)) + { + throw new ArgumentException("The address cannot be null or empty.", nameof(address)); + } + + var applications = Options.CurrentValue.DisableEntityCaching ? + Store.FindByPostLogoutRedirectUriAsync(address, cancellationToken) : + Cache.FindByPostLogoutRedirectUriAsync(address, cancellationToken); + + if (Options.CurrentValue.DisableAdditionalFiltering) + { + return applications; + } + + return ExecuteAsync(cancellationToken); + + async IAsyncEnumerable ExecuteAsync([EnumeratorCancellation] CancellationToken cancellationToken) + { + await foreach (var application in applications) + { + var addresses = await Store.GetPostLogoutRedirectUrisAsync(application, cancellationToken); + + foreach (var pattern in addresses) + { + if (Application.MatchUriPattern(pattern, address, ignorePath: false)) + { + yield return application; + } + } + } + } + } + public override async ValueTask PopulateAsync(Application application, OpenIddictApplicationDescriptor descriptor, CancellationToken cancellationToken = default) { await base.PopulateAsync(application, descriptor, cancellationToken); @@ -52,31 +85,12 @@ public async ValueTask ValidateRedirectUriDomain(Application application, ArgumentNullException.ThrowIfNull(application); ArgumentException.ThrowIfNullOrEmpty(address); - if (!Uri.TryCreate(address, UriKind.Absolute, out var addressUri)) - { - return false; - } - - var addressAuthority = addressUri.GetLeftPart(UriPartial.Authority); - foreach (var uri in await Store.GetRedirectUrisAsync(application, cancellationToken)) { - var authority = new Uri(uri).GetLeftPart(UriPartial.Authority); - - if (authority.Equals(addressAuthority)) + if (Application.MatchUriPattern(uri, address, ignorePath: true)) { return true; } - - if (authority.Contains(RedirectUriWildcardPlaceholder)) - { - var pattern = $"^{Regex.Escape(authority).Replace(RedirectUriWildcardPlaceholder, ".*")}$"; - - if (Regex.IsMatch(addressAuthority, pattern)) - { - return true; - } - } } return false; @@ -84,8 +98,6 @@ public async ValueTask ValidateRedirectUriDomain(Application application, public override async ValueTask ValidateRedirectUriAsync(Application application, string address, CancellationToken cancellationToken = default) { - // This is a modified form of the standard implementation with support for a __ wildcard in a redirect URI - if (application is null) { throw new ArgumentNullException(nameof(application)); @@ -98,26 +110,10 @@ public override async ValueTask ValidateRedirectUriAsync(Application appli foreach (var uri in await Store.GetRedirectUrisAsync(application, cancellationToken)) { - // 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(uri, address, StringComparison.Ordinal)) + if (Application.MatchUriPattern(uri, address, ignorePath: false)) { return true; } - - if (uri.Contains(RedirectUriWildcardPlaceholder)) - { - var pattern = $"^{Regex.Escape(uri).Replace(RedirectUriWildcardPlaceholder, ".*")}$"; - - if (Regex.IsMatch(address, pattern)) - { - return true; - } - else - { - continue; - } - } } Logger.LogInformation(OpenIddictResources.GetResourceString(OpenIddictResources.ID6162), address, await GetClientIdAsync(application, cancellationToken)); diff --git a/dotnet-authserver/src/TeacherIdentity.AuthServer/Oidc/TeacherIdentityApplicationStore.cs b/dotnet-authserver/src/TeacherIdentity.AuthServer/Oidc/TeacherIdentityApplicationStore.cs index 323e534ab..87207781d 100644 --- a/dotnet-authserver/src/TeacherIdentity.AuthServer/Oidc/TeacherIdentityApplicationStore.cs +++ b/dotnet-authserver/src/TeacherIdentity.AuthServer/Oidc/TeacherIdentityApplicationStore.cs @@ -1,3 +1,4 @@ +using System.Runtime.CompilerServices; using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Options; using OpenIddict.EntityFrameworkCore; @@ -15,7 +16,32 @@ public TeacherIdentityApplicationStore( { } - public new TeacherIdentityServerDbContext Context => (TeacherIdentityServerDbContext)base.Context; + public new TeacherIdentityServerDbContext Context => base.Context; + + public override IAsyncEnumerable FindByRedirectUriAsync(string address, CancellationToken cancellationToken) + { + // It appears that this is never actually used by the library; + // should it ever be used the base implementation will need replacing with one that supports wildcards. + throw new NotImplementedException(); + } + + public override async IAsyncEnumerable FindByPostLogoutRedirectUriAsync(string address, [EnumeratorCancellation] CancellationToken cancellationToken) + { + var applications = Context.Set().AsAsyncEnumerable(); + + await foreach (var application in applications.WithCancellation(cancellationToken)) + { + var addresses = await GetPostLogoutRedirectUrisAsync(application, cancellationToken); + + foreach (var postLogoutRedirectUri in addresses) + { + if (Application.MatchUriPattern(postLogoutRedirectUri, address, ignorePath: false)) + { + yield return application; + } + } + } + } public ValueTask GetServiceUrlAsync(Application application) { diff --git a/dotnet-authserver/tests/TeacherIdentity.AuthServer.Tests/ModelTests/ApplicationTests.cs b/dotnet-authserver/tests/TeacherIdentity.AuthServer.Tests/ModelTests/ApplicationTests.cs new file mode 100644 index 000000000..f88c9145d --- /dev/null +++ b/dotnet-authserver/tests/TeacherIdentity.AuthServer.Tests/ModelTests/ApplicationTests.cs @@ -0,0 +1,32 @@ +using TeacherIdentity.AuthServer.Models; + +namespace TeacherIdentity.AuthServer.Tests.ModelTests; + +public class ApplicationTests +{ + [Theory] + // Exact match + [InlineData("https://localhost:3000/callback", "https://localhost:3000/callback", false, true)] + [InlineData("https://localhost:3000/callback", "https://localhost:3000/callback", true, true)] + // Scheme mismatch + [InlineData("https://localhost:3000/callback", "http://localhost:3000/callback", false, false)] + [InlineData("https://localhost:3000/callback", "http://localhost:3000/callback", true, false)] + // Path mismatch + [InlineData("https://localhost:3000/", "https://localhost:3000/callback", false, false)] + [InlineData("https://localhost:3000/", "https://localhost:3000/callback", true, true)] + // Wildcard domain + [InlineData("https://__.london.cloudapps.digital/callback", "https://reviewapp123.london.cloudapps.digital/callback", false, true)] + [InlineData("https://__.london.cloudapps.digital/callback", "https://reviewapp123.london.cloudapps.digital/callback", true, true)] + [InlineData("https://__.london.cloudapps.digital/", "https://reviewapp123.london.cloudapps.digital/callback", false, false)] + [InlineData("https://__.london.cloudapps.digital/", "https://reviewapp123.london.cloudapps.digital/callback", true, true)] + public void MatchUriPattern_ReturnsExpectedResult(string pattern, string uri, bool ignorePath, bool expectedResult) + { + // Arrange + + // Act + var result = Application.MatchUriPattern(pattern, uri, ignorePath); + + // Assert + Assert.Equal(expectedResult, result); + } +}