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..4d60ed733 100644 --- a/dotnet-authserver/src/TeacherIdentity.AuthServer/Oidc/TeacherIdentityApplicationManager.cs +++ b/dotnet-authserver/src/TeacherIdentity.AuthServer/Oidc/TeacherIdentityApplicationManager.cs @@ -1,4 +1,3 @@ -using System.Text.RegularExpressions; using Microsoft.Extensions.Options; using OpenIddict.Abstractions; using OpenIddict.Core; @@ -8,8 +7,6 @@ namespace TeacherIdentity.AuthServer.Oidc; public partial class TeacherIdentityApplicationManager : OpenIddictApplicationManager { - private const string RedirectUriWildcardPlaceholder = "__"; - public TeacherIdentityApplicationManager( IOpenIddictApplicationCache cache, ILogger> logger, @@ -52,31 +49,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 +62,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 +74,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/src/TeacherIdentity.AuthServer/Pages/Admin/AddClient.cshtml.cs b/dotnet-authserver/src/TeacherIdentity.AuthServer/Pages/Admin/AddClient.cshtml.cs index fc7e6225d..ea5cec765 100644 --- a/dotnet-authserver/src/TeacherIdentity.AuthServer/Pages/Admin/AddClient.cshtml.cs +++ b/dotnet-authserver/src/TeacherIdentity.AuthServer/Pages/Admin/AddClient.cshtml.cs @@ -54,7 +54,7 @@ public AddClientModel( [ModelBinder(BinderType = typeof(MultiLineStringModelBinder))] public string[]? RedirectUris { get; set; } - [Display(Name = "Post logout redirect URIs", Description = "Enter one per line")] + [Display(Name = "Post logout redirect URIs", Description = "Enter one per line. You can use '__' as a wildcard.")] [ModelBinder(BinderType = typeof(MultiLineStringModelBinder))] public string[]? PostLogoutRedirectUris { get; set; } diff --git a/dotnet-authserver/src/TeacherIdentity.AuthServer/Pages/Admin/EditClient.cshtml.cs b/dotnet-authserver/src/TeacherIdentity.AuthServer/Pages/Admin/EditClient.cshtml.cs index 5370cf93b..ba02eb5f8 100644 --- a/dotnet-authserver/src/TeacherIdentity.AuthServer/Pages/Admin/EditClient.cshtml.cs +++ b/dotnet-authserver/src/TeacherIdentity.AuthServer/Pages/Admin/EditClient.cshtml.cs @@ -60,7 +60,7 @@ public EditClientModel( [ModelBinder(BinderType = typeof(MultiLineStringModelBinder))] public string[]? RedirectUris { get; set; } - [Display(Name = "Post logout redirect URIs", Description = "Enter one per line")] + [Display(Name = "Post logout redirect URIs", Description = "Enter one per line. You can use '__' as a wildcard.")] [ModelBinder(BinderType = typeof(MultiLineStringModelBinder))] public string[]? PostLogoutRedirectUris { get; set; } 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); + } +}