From 3e1bbc626765433c055d8ad8ad34d8d8de37c8a6 Mon Sep 17 00:00:00 2001 From: Brock Allen Date: Tue, 20 Apr 2021 17:28:08 -0400 Subject: [PATCH 1/8] allow for full URLs to be validated by return url parser (cherry picked from commit 7479605ffd4a51dad471712f9b866470324534bd) --- .../Services/Default/OidcReturnUrlParser.cs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs b/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs index 1df94b023..7f3969dcb 100644 --- a/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs +++ b/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs @@ -1,4 +1,4 @@ -// Copyright (c) Duende Software. All rights reserved. +// Copyright (c) Duende Software. All rights reserved. // See LICENSE in the project root for license information. @@ -10,6 +10,7 @@ using Duende.IdentityServer.Models; using Duende.IdentityServer.Stores; using Duende.IdentityServer.Validation; +using Microsoft.AspNetCore.Http; namespace Duende.IdentityServer.Services { @@ -17,17 +18,20 @@ internal class OidcReturnUrlParser : IReturnUrlParser { private readonly IAuthorizeRequestValidator _validator; private readonly IUserSession _userSession; + private readonly IHttpContextAccessor _httpContextAccessor; private readonly ILogger _logger; private readonly IAuthorizationParametersMessageStore _authorizationParametersMessageStore; public OidcReturnUrlParser( IAuthorizeRequestValidator validator, IUserSession userSession, + IHttpContextAccessor httpContextAccessor, ILogger logger, IAuthorizationParametersMessageStore authorizationParametersMessageStore = null) { _validator = validator; _userSession = userSession; + _httpContextAccessor = httpContextAccessor; _logger = logger; _authorizationParametersMessageStore = authorizationParametersMessageStore; } @@ -59,6 +63,12 @@ public async Task ParseAsync(string returnUrl) public bool IsValidReturnUrl(string returnUrl) { + var host = _httpContextAccessor.HttpContext.GetIdentityServerHost(); + if (returnUrl.StartsWith(host, StringComparison.OrdinalIgnoreCase)) + { + returnUrl = returnUrl.Substring(host.Length); + } + if (returnUrl.IsLocalUrl()) { var index = returnUrl.IndexOf('?'); From 10957e1dba6f51cf2cf066e1ec3533efa4fc6ce1 Mon Sep 17 00:00:00 2001 From: Brock Allen Date: Wed, 21 Apr 2021 11:07:26 -0400 Subject: [PATCH 2/8] add tests to exercise host comparison --- .../Services/Default/OidcReturnUrlParser.cs | 29 ++++- .../Default/OidcReturnUrlParserTests.cs | 120 ++++++++++++++++++ 2 files changed, 143 insertions(+), 6 deletions(-) create mode 100644 test/IdentityServer.UnitTests/Services/Default/OidcReturnUrlParserTests.cs diff --git a/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs b/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs index 7f3969dcb..1e77c1961 100644 --- a/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs +++ b/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs @@ -63,18 +63,35 @@ public async Task ParseAsync(string returnUrl) public bool IsValidReturnUrl(string returnUrl) { - var host = _httpContextAccessor.HttpContext.GetIdentityServerHost(); - if (returnUrl.StartsWith(host, StringComparison.OrdinalIgnoreCase)) + if (returnUrl != null) { - returnUrl = returnUrl.Substring(host.Length); + if (!Uri.TryCreate(returnUrl, UriKind.RelativeOrAbsolute, out _)) + { + return false; + } + + var host = _httpContextAccessor.HttpContext.GetIdentityServerHost(); + if (returnUrl.StartsWith(host, StringComparison.OrdinalIgnoreCase) == true) + { + returnUrl = returnUrl.Substring(host.Length); + } } if (returnUrl.IsLocalUrl()) { - var index = returnUrl.IndexOf('?'); - if (index >= 0) { - returnUrl = returnUrl.Substring(0, index); + var index = returnUrl.IndexOf('?'); + if (index >= 0) + { + returnUrl = returnUrl.Substring(0, index); + } + } + { + var index = returnUrl.IndexOf('#'); + if (index >= 0) + { + returnUrl = returnUrl.Substring(0, index); + } } if (returnUrl.EndsWith(Constants.ProtocolRoutePaths.Authorize, StringComparison.Ordinal) || diff --git a/test/IdentityServer.UnitTests/Services/Default/OidcReturnUrlParserTests.cs b/test/IdentityServer.UnitTests/Services/Default/OidcReturnUrlParserTests.cs new file mode 100644 index 000000000..b493437b6 --- /dev/null +++ b/test/IdentityServer.UnitTests/Services/Default/OidcReturnUrlParserTests.cs @@ -0,0 +1,120 @@ +// Copyright (c) Duende Software. All rights reserved. +// See LICENSE in the project root for license information. + +using Duende.IdentityServer.Services; +using FluentAssertions; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Logging; +using Xunit; + +namespace UnitTests.Services.Default +{ + public class OidcReturnUrlParserTests + { + private OidcReturnUrlParser _subject; + + DefaultHttpContext _httpContext = new DefaultHttpContext(); + + public OidcReturnUrlParserTests() + { + _httpContext.Request.Scheme = "https"; + _httpContext.Request.Host = new HostString("server"); + + _subject = new OidcReturnUrlParser( + null, null, + new HttpContextAccessor { HttpContext = _httpContext }, + new LoggerFactory().CreateLogger()); + } + + [Theory] + [InlineData("/connect/authorize")] + [InlineData("/connect/authorize?foo=f1&bar=b1")] + [InlineData("/connect/authorize/callback")] + [InlineData("/connect/authorize/callback?foo=f1&bar=b1")] + [InlineData("/foo/connect/authorize")] + [InlineData("/foo/connect/authorize/callback")] + public void IsValidReturnUrl_accepts_authorize_or_authorizecallback(string url) + { + var valid = _subject.IsValidReturnUrl(url); + valid.Should().BeTrue(); + } + + [Theory] + [InlineData(default(string))] + [InlineData("")] + [InlineData("/")] + [InlineData("/path")] + [InlineData("//connect/authorize")] + [InlineData("/connect/authorizex")] + [InlineData("/connect")] + [InlineData("/connect/token")] + [InlineData("/authorize")] + [InlineData("/foo?/connect/authorize")] + [InlineData("/foo#/connect/authorize")] + [InlineData("/foo?#/connect/authorize")] + [InlineData("/foo#?/connect/authorize")] + [InlineData("//server/connect/authorize")] + public void IsValidReturnUrl_rejects_non_authorize_or_authorizecallback(string url) + { + var valid = _subject.IsValidReturnUrl(url); + valid.Should().BeFalse(); + } + + [Theory] + [InlineData("https://server/connect/authorize")] + [InlineData("HTTPS://server/connect/authorize")] + [InlineData("https://SERVER/connect/authorize")] + [InlineData("https://server/foo/connect/authorize")] + public void IsValidReturnUrl_accepts_urls_with_current_host(string url) + { + var valid = _subject.IsValidReturnUrl(url); + valid.Should().BeTrue(); + } + + [Theory] + [InlineData("http://server/connect/authorize")] + [InlineData("https:\\/server/connect/authorize")] + [InlineData("https:\\\\server/connect/authorize")] + [InlineData("https://foo/connect/authorize")] + [InlineData("https://serverhttps://server/connect/authorize")] + [InlineData("https://serverfoo/connect/authorize")] + [InlineData("https://server//foo/connect/authorize")] + [InlineData("https://server:443/connect/authorize")] + public void IsValidReturnUrl_rejects_urls_with_incorrect_current_host(string url) + { + var valid = _subject.IsValidReturnUrl(url); + valid.Should().BeFalse(); + } + + + [Theory] + [InlineData("https://server:443/connect/authorize")] + [InlineData("HTTPS://server:443/connect/authorize")] + [InlineData("https://SERVER:443/connect/authorize")] + public void IsValidReturnUrl_accepts_urls_with_current_port(string url) + { + _httpContext.Request.Host = new HostString("server:443"); + + var valid = _subject.IsValidReturnUrl(url); + valid.Should().BeTrue(); + } + + [Theory] + [InlineData("https://server/connect/authorize")] + [InlineData("https://server:80/connect/authorize")] + [InlineData("https://server:4/connect/authorize")] + [InlineData("https://foo:443/connect/authorize")] + [InlineData("https://server:4433/connect/authorize")] + [InlineData("https://server:443https://server:443/connect/authorize")] + [InlineData("https://serverfoo:443/connect/authorize")] + [InlineData("https://server:443foo/connect/authorize")] + [InlineData("https://server:443//foo/connect/authorize")] + public void IsValidReturnUrl_rejects_urls_with_incorrect_current_port(string url) + { + _httpContext.Request.Host = new HostString("server:443"); + + var valid = _subject.IsValidReturnUrl(url); + valid.Should().BeFalse(); + } + } +} From 78aff0ec7a249d7401979482eb0a30dc944f4ce6 Mon Sep 17 00:00:00 2001 From: Brock Allen Date: Wed, 21 Apr 2021 11:10:14 -0400 Subject: [PATCH 3/8] log that url failed to parse --- src/IdentityServer/Services/Default/OidcReturnUrlParser.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs b/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs index 1e77c1961..cc7ef8c4e 100644 --- a/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs +++ b/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs @@ -67,6 +67,7 @@ public bool IsValidReturnUrl(string returnUrl) { if (!Uri.TryCreate(returnUrl, UriKind.RelativeOrAbsolute, out _)) { + _logger.LogTrace("returnUrl is not valid"); return false; } From ef2ae49e2572ad3f1b4b22d6e12a5324620bd985 Mon Sep 17 00:00:00 2001 From: Brock Allen Date: Fri, 23 Apr 2021 10:36:48 -0400 Subject: [PATCH 4/8] add feature flag AllowHostInReturnUrl --- .../Options/UserInteractionOptions.cs | 5 +++++ .../Services/Default/OidcReturnUrlParser.cs | 6 +++++- .../Default/OidcReturnUrlParserTests.cs | 19 +++++++++++++++++-- 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/IdentityServer/Configuration/DependencyInjection/Options/UserInteractionOptions.cs b/src/IdentityServer/Configuration/DependencyInjection/Options/UserInteractionOptions.cs index a4af41f91..b94437d62 100644 --- a/src/IdentityServer/Configuration/DependencyInjection/Options/UserInteractionOptions.cs +++ b/src/IdentityServer/Configuration/DependencyInjection/Options/UserInteractionOptions.cs @@ -106,5 +106,10 @@ public class UserInteractionOptions /// The device verification user code parameter. /// public string DeviceVerificationUserCodeParameter { get; set; } = Constants.UIConstants.DefaultRoutePathParams.UserCode; + + /// + /// Flag that allows return URL validation to accept full URL that includes the IdentityServer scheme and host name. Defaults to false. + /// + public bool AllowHostInReturnUrl { get; set; } } } diff --git a/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs b/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs index cc7ef8c4e..5c2b79f22 100644 --- a/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs +++ b/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs @@ -11,11 +11,13 @@ using Duende.IdentityServer.Stores; using Duende.IdentityServer.Validation; using Microsoft.AspNetCore.Http; +using Duende.IdentityServer.Configuration; namespace Duende.IdentityServer.Services { internal class OidcReturnUrlParser : IReturnUrlParser { + private readonly IdentityServerOptions _options; private readonly IAuthorizeRequestValidator _validator; private readonly IUserSession _userSession; private readonly IHttpContextAccessor _httpContextAccessor; @@ -23,12 +25,14 @@ internal class OidcReturnUrlParser : IReturnUrlParser private readonly IAuthorizationParametersMessageStore _authorizationParametersMessageStore; public OidcReturnUrlParser( + IdentityServerOptions options, IAuthorizeRequestValidator validator, IUserSession userSession, IHttpContextAccessor httpContextAccessor, ILogger logger, IAuthorizationParametersMessageStore authorizationParametersMessageStore = null) { + _options = options; _validator = validator; _userSession = userSession; _httpContextAccessor = httpContextAccessor; @@ -63,7 +67,7 @@ public async Task ParseAsync(string returnUrl) public bool IsValidReturnUrl(string returnUrl) { - if (returnUrl != null) + if (_options.UserInteraction.AllowHostInReturnUrl && returnUrl != null) { if (!Uri.TryCreate(returnUrl, UriKind.RelativeOrAbsolute, out _)) { diff --git a/test/IdentityServer.UnitTests/Services/Default/OidcReturnUrlParserTests.cs b/test/IdentityServer.UnitTests/Services/Default/OidcReturnUrlParserTests.cs index b493437b6..bd771c4f1 100644 --- a/test/IdentityServer.UnitTests/Services/Default/OidcReturnUrlParserTests.cs +++ b/test/IdentityServer.UnitTests/Services/Default/OidcReturnUrlParserTests.cs @@ -1,6 +1,7 @@ // Copyright (c) Duende Software. All rights reserved. // See LICENSE in the project root for license information. +using Duende.IdentityServer.Configuration; using Duende.IdentityServer.Services; using FluentAssertions; using Microsoft.AspNetCore.Http; @@ -12,7 +13,8 @@ namespace UnitTests.Services.Default public class OidcReturnUrlParserTests { private OidcReturnUrlParser _subject; - + + IdentityServerOptions _options = new IdentityServerOptions(); DefaultHttpContext _httpContext = new DefaultHttpContext(); public OidcReturnUrlParserTests() @@ -21,6 +23,7 @@ public OidcReturnUrlParserTests() _httpContext.Request.Host = new HostString("server"); _subject = new OidcReturnUrlParser( + _options, null, null, new HttpContextAccessor { HttpContext = _httpContext }, new LoggerFactory().CreateLogger()); @@ -67,10 +70,19 @@ public void IsValidReturnUrl_rejects_non_authorize_or_authorizecallback(string u [InlineData("https://server/foo/connect/authorize")] public void IsValidReturnUrl_accepts_urls_with_current_host(string url) { + _options.UserInteraction.AllowHostInReturnUrl = true; var valid = _subject.IsValidReturnUrl(url); valid.Should().BeTrue(); } - + + [Fact] + public void IsValidReturnUrl_when_AllowHostInReturnUrl_disabled_rejects_urls_with_current_host() + { + _options.UserInteraction.AllowHostInReturnUrl = false; + var valid = _subject.IsValidReturnUrl("https://server/connect/authorize"); + valid.Should().BeFalse(); + } + [Theory] [InlineData("http://server/connect/authorize")] [InlineData("https:\\/server/connect/authorize")] @@ -82,6 +94,7 @@ public void IsValidReturnUrl_accepts_urls_with_current_host(string url) [InlineData("https://server:443/connect/authorize")] public void IsValidReturnUrl_rejects_urls_with_incorrect_current_host(string url) { + _options.UserInteraction.AllowHostInReturnUrl = true; var valid = _subject.IsValidReturnUrl(url); valid.Should().BeFalse(); } @@ -93,6 +106,7 @@ public void IsValidReturnUrl_rejects_urls_with_incorrect_current_host(string url [InlineData("https://SERVER:443/connect/authorize")] public void IsValidReturnUrl_accepts_urls_with_current_port(string url) { + _options.UserInteraction.AllowHostInReturnUrl = true; _httpContext.Request.Host = new HostString("server:443"); var valid = _subject.IsValidReturnUrl(url); @@ -111,6 +125,7 @@ public void IsValidReturnUrl_accepts_urls_with_current_port(string url) [InlineData("https://server:443//foo/connect/authorize")] public void IsValidReturnUrl_rejects_urls_with_incorrect_current_port(string url) { + _options.UserInteraction.AllowHostInReturnUrl = true; _httpContext.Request.Host = new HostString("server:443"); var valid = _subject.IsValidReturnUrl(url); From bd569d674680310d3a042e3549631659013d5472 Mon Sep 17 00:00:00 2001 From: Brock Allen Date: Fri, 23 Apr 2021 10:40:43 -0400 Subject: [PATCH 5/8] rename feature flag to origin --- .../Options/UserInteractionOptions.cs | 4 ++-- .../Services/Default/OidcReturnUrlParser.cs | 2 +- .../Services/Default/OidcReturnUrlParserTests.cs | 10 +++++----- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/IdentityServer/Configuration/DependencyInjection/Options/UserInteractionOptions.cs b/src/IdentityServer/Configuration/DependencyInjection/Options/UserInteractionOptions.cs index b94437d62..ef6bafd46 100644 --- a/src/IdentityServer/Configuration/DependencyInjection/Options/UserInteractionOptions.cs +++ b/src/IdentityServer/Configuration/DependencyInjection/Options/UserInteractionOptions.cs @@ -108,8 +108,8 @@ public class UserInteractionOptions public string DeviceVerificationUserCodeParameter { get; set; } = Constants.UIConstants.DefaultRoutePathParams.UserCode; /// - /// Flag that allows return URL validation to accept full URL that includes the IdentityServer scheme and host name. Defaults to false. + /// Flag that allows return URL validation to accept full URL that includes the IdentityServer origin. Defaults to false. /// - public bool AllowHostInReturnUrl { get; set; } + public bool AllowOriginInReturnUrl { get; set; } } } diff --git a/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs b/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs index 5c2b79f22..7fa1cc3b0 100644 --- a/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs +++ b/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs @@ -67,7 +67,7 @@ public async Task ParseAsync(string returnUrl) public bool IsValidReturnUrl(string returnUrl) { - if (_options.UserInteraction.AllowHostInReturnUrl && returnUrl != null) + if (_options.UserInteraction.AllowOriginInReturnUrl && returnUrl != null) { if (!Uri.TryCreate(returnUrl, UriKind.RelativeOrAbsolute, out _)) { diff --git a/test/IdentityServer.UnitTests/Services/Default/OidcReturnUrlParserTests.cs b/test/IdentityServer.UnitTests/Services/Default/OidcReturnUrlParserTests.cs index bd771c4f1..8da4710f1 100644 --- a/test/IdentityServer.UnitTests/Services/Default/OidcReturnUrlParserTests.cs +++ b/test/IdentityServer.UnitTests/Services/Default/OidcReturnUrlParserTests.cs @@ -70,7 +70,7 @@ public void IsValidReturnUrl_rejects_non_authorize_or_authorizecallback(string u [InlineData("https://server/foo/connect/authorize")] public void IsValidReturnUrl_accepts_urls_with_current_host(string url) { - _options.UserInteraction.AllowHostInReturnUrl = true; + _options.UserInteraction.AllowOriginInReturnUrl = true; var valid = _subject.IsValidReturnUrl(url); valid.Should().BeTrue(); } @@ -78,7 +78,7 @@ public void IsValidReturnUrl_accepts_urls_with_current_host(string url) [Fact] public void IsValidReturnUrl_when_AllowHostInReturnUrl_disabled_rejects_urls_with_current_host() { - _options.UserInteraction.AllowHostInReturnUrl = false; + _options.UserInteraction.AllowOriginInReturnUrl = false; var valid = _subject.IsValidReturnUrl("https://server/connect/authorize"); valid.Should().BeFalse(); } @@ -94,7 +94,7 @@ public void IsValidReturnUrl_when_AllowHostInReturnUrl_disabled_rejects_urls_wit [InlineData("https://server:443/connect/authorize")] public void IsValidReturnUrl_rejects_urls_with_incorrect_current_host(string url) { - _options.UserInteraction.AllowHostInReturnUrl = true; + _options.UserInteraction.AllowOriginInReturnUrl = true; var valid = _subject.IsValidReturnUrl(url); valid.Should().BeFalse(); } @@ -106,7 +106,7 @@ public void IsValidReturnUrl_rejects_urls_with_incorrect_current_host(string url [InlineData("https://SERVER:443/connect/authorize")] public void IsValidReturnUrl_accepts_urls_with_current_port(string url) { - _options.UserInteraction.AllowHostInReturnUrl = true; + _options.UserInteraction.AllowOriginInReturnUrl = true; _httpContext.Request.Host = new HostString("server:443"); var valid = _subject.IsValidReturnUrl(url); @@ -125,7 +125,7 @@ public void IsValidReturnUrl_accepts_urls_with_current_port(string url) [InlineData("https://server:443//foo/connect/authorize")] public void IsValidReturnUrl_rejects_urls_with_incorrect_current_port(string url) { - _options.UserInteraction.AllowHostInReturnUrl = true; + _options.UserInteraction.AllowOriginInReturnUrl = true; _httpContext.Request.Host = new HostString("server:443"); var valid = _subject.IsValidReturnUrl(url); From ca63d9a72f2ab24e507362c240a5ac51f88181d9 Mon Sep 17 00:00:00 2001 From: Brock Allen Date: Fri, 23 Apr 2021 10:54:45 -0400 Subject: [PATCH 6/8] use GetOrigin not GetHost to deal with unicode --- .../Services/Default/OidcReturnUrlParser.cs | 6 +++--- .../Services/Default/OidcReturnUrlParserTests.cs | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs b/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs index 7fa1cc3b0..73b351f5a 100644 --- a/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs +++ b/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs @@ -75,10 +75,10 @@ public bool IsValidReturnUrl(string returnUrl) return false; } - var host = _httpContextAccessor.HttpContext.GetIdentityServerHost(); - if (returnUrl.StartsWith(host, StringComparison.OrdinalIgnoreCase) == true) + var origin = _httpContextAccessor.HttpContext.GetIdentityServerOrigin(); + if (returnUrl.StartsWith(origin, StringComparison.OrdinalIgnoreCase) == true) { - returnUrl = returnUrl.Substring(host.Length); + returnUrl = returnUrl.Substring(origin.Length); } } diff --git a/test/IdentityServer.UnitTests/Services/Default/OidcReturnUrlParserTests.cs b/test/IdentityServer.UnitTests/Services/Default/OidcReturnUrlParserTests.cs index 8da4710f1..438e9d4b7 100644 --- a/test/IdentityServer.UnitTests/Services/Default/OidcReturnUrlParserTests.cs +++ b/test/IdentityServer.UnitTests/Services/Default/OidcReturnUrlParserTests.cs @@ -5,6 +5,7 @@ using Duende.IdentityServer.Services; using FluentAssertions; using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Xunit; @@ -19,6 +20,10 @@ public class OidcReturnUrlParserTests public OidcReturnUrlParserTests() { + var services = new ServiceCollection(); + services.AddSingleton(_options); + _httpContext.RequestServices = services.BuildServiceProvider(); + _httpContext.Request.Scheme = "https"; _httpContext.Request.Host = new HostString("server"); @@ -100,6 +105,16 @@ public void IsValidReturnUrl_rejects_urls_with_incorrect_current_host(string url } + [Fact] + public void IsValidReturnUrl_accepts_urls_with_unicode() + { + _options.UserInteraction.AllowOriginInReturnUrl = true; + _httpContext.Request.Host = new HostString("грант.рф"); + + var valid = _subject.IsValidReturnUrl("https://грант.рф/connect/authorize"); + valid.Should().BeTrue(); + } + [Theory] [InlineData("https://server:443/connect/authorize")] [InlineData("HTTPS://server:443/connect/authorize")] From 6f20c01318c39bba613941d096d92c608fd4daf7 Mon Sep 17 00:00:00 2001 From: Brock Allen Date: Fri, 23 Apr 2021 11:56:34 -0400 Subject: [PATCH 7/8] due to ASCII restrictions in response headers, need to use API that returns punycode --- src/IdentityServer/Services/Default/OidcReturnUrlParser.cs | 6 +++--- .../Services/Default/OidcReturnUrlParserTests.cs | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs b/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs index 73b351f5a..7fa1cc3b0 100644 --- a/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs +++ b/src/IdentityServer/Services/Default/OidcReturnUrlParser.cs @@ -75,10 +75,10 @@ public bool IsValidReturnUrl(string returnUrl) return false; } - var origin = _httpContextAccessor.HttpContext.GetIdentityServerOrigin(); - if (returnUrl.StartsWith(origin, StringComparison.OrdinalIgnoreCase) == true) + var host = _httpContextAccessor.HttpContext.GetIdentityServerHost(); + if (returnUrl.StartsWith(host, StringComparison.OrdinalIgnoreCase) == true) { - returnUrl = returnUrl.Substring(origin.Length); + returnUrl = returnUrl.Substring(host.Length); } } diff --git a/test/IdentityServer.UnitTests/Services/Default/OidcReturnUrlParserTests.cs b/test/IdentityServer.UnitTests/Services/Default/OidcReturnUrlParserTests.cs index 438e9d4b7..0efdebd52 100644 --- a/test/IdentityServer.UnitTests/Services/Default/OidcReturnUrlParserTests.cs +++ b/test/IdentityServer.UnitTests/Services/Default/OidcReturnUrlParserTests.cs @@ -7,6 +7,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using System; using Xunit; namespace UnitTests.Services.Default @@ -111,7 +112,7 @@ public void IsValidReturnUrl_accepts_urls_with_unicode() _options.UserInteraction.AllowOriginInReturnUrl = true; _httpContext.Request.Host = new HostString("грант.рф"); - var valid = _subject.IsValidReturnUrl("https://грант.рф/connect/authorize"); + var valid = _subject.IsValidReturnUrl("https://xn--80af5akm.xn--p1ai/connect/authorize"); valid.Should().BeTrue(); } From cd575293eca60166c8aa62e32719eb0fc707dc1a Mon Sep 17 00:00:00 2001 From: Brock Allen Date: Fri, 23 Apr 2021 11:58:47 -0400 Subject: [PATCH 8/8] remove unneeded code in test --- .../Services/Default/OidcReturnUrlParserTests.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/IdentityServer.UnitTests/Services/Default/OidcReturnUrlParserTests.cs b/test/IdentityServer.UnitTests/Services/Default/OidcReturnUrlParserTests.cs index 0efdebd52..50e887735 100644 --- a/test/IdentityServer.UnitTests/Services/Default/OidcReturnUrlParserTests.cs +++ b/test/IdentityServer.UnitTests/Services/Default/OidcReturnUrlParserTests.cs @@ -21,10 +21,6 @@ public class OidcReturnUrlParserTests public OidcReturnUrlParserTests() { - var services = new ServiceCollection(); - services.AddSingleton(_options); - _httpContext.RequestServices = services.BuildServiceProvider(); - _httpContext.Request.Scheme = "https"; _httpContext.Request.Host = new HostString("server");