Skip to content

Commit

Permalink
Merge commit from fork
Browse files Browse the repository at this point in the history 
Fix IsLocalUrl in 7.0.x
  • Loading branch information
@josephdecock
josephdecock committed Jul 30, 2024
2 parents 1190605 + 4a861f2 commit 765116a
Show file tree
Hide file tree
Showing 4 changed files with 277 additions and 4 deletions.
20 changes: 18 additions & 2 deletions src/EntityFramework.Storage/Extensions/StringsExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ public static string CleanUrlPath(this string url)
[DebuggerStepThrough]
public static bool IsLocalUrl(this string url)
{
// This implementation is a copy of a https://github.com/dotnet/aspnetcore/blob/3f1acb59718cadf111a0a796681e3d3509bb3381/src/Mvc/Mvc.Core/src/Routing/UrlHelperBase.cs#L315
// We originally copied that code to avoid a dependency, but we could potentially remove this entirely by switching to the Microsoft.NET.Sdk.Web sdk.
if (string.IsNullOrEmpty(url))
{
return false;
Expand All @@ -153,7 +155,7 @@ public static bool IsLocalUrl(this string url)
// url doesn't start with "//" or "/\"
if (url[1] != '/' && url[1] != '\\')
{
return true;
return !HasControlCharacter(url.AsSpan(1));
}

return false;
Expand All @@ -171,13 +173,27 @@ public static bool IsLocalUrl(this string url)
// url doesn't start with "~//" or "~/\"
if (url[2] != '/' && url[2] != '\\')
{
return true;
return !HasControlCharacter(url.AsSpan(2));
}

return false;
}

return false;

static bool HasControlCharacter(ReadOnlySpan<char> readOnlySpan)
{
// URLs may not contain ASCII control characters.
for (var i = 0; i < readOnlySpan.Length; i++)
{
if (char.IsControl(readOnlySpan[i]))
{
return true;
}
}

return false;
}
}

[DebuggerStepThrough]
Expand Down
20 changes: 18 additions & 2 deletions src/IdentityServer/Extensions/StringsExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ public static string CleanUrlPath(this string url)
[DebuggerStepThrough]
public static bool IsLocalUrl(this string url)
{
// This implementation is a copy of a https://github.com/dotnet/aspnetcore/blob/3f1acb59718cadf111a0a796681e3d3509bb3381/src/Mvc/Mvc.Core/src/Routing/UrlHelperBase.cs#L315
// We originally copied that code to avoid a dependency, but we could potentially remove this entirely by switching to the Microsoft.NET.Sdk.Web sdk.
if (string.IsNullOrEmpty(url))
{
return false;
Expand All @@ -156,7 +158,7 @@ public static bool IsLocalUrl(this string url)
// url doesn't start with "//" or "/\"
if (url[1] != '/' && url[1] != '\\')
{
return true;
return !HasControlCharacter(url.AsSpan(1));
}

return false;
Expand All @@ -174,13 +176,27 @@ public static bool IsLocalUrl(this string url)
// url doesn't start with "~//" or "~/\"
if (url[2] != '/' && url[2] != '\\')
{
return true;
return !HasControlCharacter(url.AsSpan(2));
}

return false;
}

return false;

static bool HasControlCharacter(ReadOnlySpan<char> readOnlySpan)
{
// URLs may not contain ASCII control characters.
for (var i = 0; i < readOnlySpan.Length; i++)
{
if (char.IsControl(readOnlySpan[i]))
{
return true;
}
}

return false;
}
}

[DebuggerStepThrough]
Expand Down
62 changes: 62 additions & 0 deletions test/EntityFramework.Storage.UnitTests/IsLocalUrlTests.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
using System.Collections.Generic;
using FluentAssertions;
using Xunit;
using Duende.IdentityServer.EntityFramework.Extensions;

namespace UnitTests.Validation;

public class IsLocalUrlTests
{
private const string queryParameters = "?client_id=mvc.code" +
"&redirect_uri=https%3A%2F%2Flocalhost%3A44302%2Fsignin-oidc" +
"&response_type=code" +
"&scope=openid%20profile%20email%20custom.profile%20resource1.scope1%20resource2.scope1%20offline_access" +
"&code_challenge=LcJN1shWmezC0J5EU7QOi7N_amBuvMDb6PcTY0sB2YY" +
"&code_challenge_method=S256" +
"&response_mode=form_post" +
"&nonce=nonce" +
"&state=state";

public static IEnumerable<object[]> TestCases =>
new List<object[]>
{
new object[] { "/connect/authorize/callback" + queryParameters, true },
new object[] { "//evil.com/" + queryParameters, false },
// Tab character
new object[] { "/\t/evil.com/connect/authorize/callback" + queryParameters, false },
// Tabs and Spaces
new object[] { "/ \t/evil.com/connect/authorize/callback" + queryParameters, false },
new object[] { "/ \t/evil.com/connect/authorize/callback" + queryParameters, false },
new object[] { "/ \t/evil.com/connect/authorize/callback" + queryParameters, false },
new object[] { "/\t /evil.com/connect/authorize/callback" + queryParameters, false },
new object[] { "/\t /evil.com/connect/authorize/callback" + queryParameters, false },
new object[] { "/\t /evil.com/connect/authorize/callback" + queryParameters, false },
// Various new line related things
new object[] { "/\n/evil.com/" + queryParameters, false },
new object[] { "/\n\n/evil.com/" + queryParameters, false },
new object[] { "/\r/evil.com/" + queryParameters, false },
new object[] { "/\r\r/evil.com/" + queryParameters, false },
new object[] { "/\r\n/evil.com/" + queryParameters, false },
new object[] { "/\r\n\r\n/evil.com/" + queryParameters, false },
// Tabs and Newlines
new object[] { "/\t\n/evil.com/" + queryParameters, false },
new object[] { "/\t\n\n/evil.com/" + queryParameters, false },
new object[] { "/\t\r/evil.com/" + queryParameters, false },
new object[] { "/\t\r\r/evil.com/" + queryParameters, false },
new object[] { "/\t\r\n/evil.com/" + queryParameters, false },
new object[] { "/\t\r\n\r\n/evil.com/" + queryParameters, false },
new object[] { "/\n/evil.com\t/" + queryParameters, false },
new object[] { "/\n\n/evil.com\t/" + queryParameters, false },
new object[] { "/\r/evil.com\t/" + queryParameters, false },
new object[] { "/\r\r/evil.com\t/" + queryParameters, false },
new object[] { "/\r\n/evil.com\t/" + queryParameters, false },
new object[] { "/\r\n\r\n/evil.com\t/" + queryParameters, false },
};

[Theory]
[MemberData(nameof(TestCases))]
public void IsLocalUrl(string returnUrl, bool expected)
{
returnUrl.IsLocalUrl().Should().Be(expected);
}
}
179 changes: 179 additions & 0 deletions test/IdentityServer.UnitTests/Validation/IsLocalUrlTests.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
using System.Collections.Generic;
using Duende.IdentityServer.Configuration;
using Duende.IdentityServer.Extensions;
using Duende.IdentityServer.Services;
using Duende.IdentityServer.Validation;
using FluentAssertions;
using Microsoft.Extensions.Logging;
using UnitTests.Common;
using UnitTests.Endpoints.Authorize;
using Xunit;

namespace UnitTests.Validation;

public class IsLocalUrlTests
{
private const string queryParameters = "?client_id=mvc.code" +
"&redirect_uri=https%3A%2F%2Flocalhost%3A44302%2Fsignin-oidc" +
"&response_type=code" +
"&scope=openid%20profile%20email%20custom.profile%20resource1.scope1%20resource2.scope1%20offline_access" +
"&code_challenge=LcJN1shWmezC0J5EU7QOi7N_amBuvMDb6PcTY0sB2YY" +
"&code_challenge_method=S256" +
"&response_mode=form_post" +
"&nonce=nonce" +
"&state=state";

public static IEnumerable<object[]> TestCases =>
new List<object[]>
{
new object[] { "/connect/authorize/callback" + queryParameters, true },
new object[] { "//evil.com/" + queryParameters, false },
// Tab character
new object[] { "/\t/evil.com/connect/authorize/callback" + queryParameters, false },
// Tabs and Spaces
new object[] { "/ \t/evil.com/connect/authorize/callback" + queryParameters, false },
new object[] { "/ \t/evil.com/connect/authorize/callback" + queryParameters, false },
new object[] { "/ \t/evil.com/connect/authorize/callback" + queryParameters, false },
new object[] { "/\t /evil.com/connect/authorize/callback" + queryParameters, false },
new object[] { "/\t /evil.com/connect/authorize/callback" + queryParameters, false },
new object[] { "/\t /evil.com/connect/authorize/callback" + queryParameters, false },
// Various new line related things
new object[] { "/\n/evil.com/" + queryParameters, false },
new object[] { "/\n\n/evil.com/" + queryParameters, false },
new object[] { "/\r/evil.com/" + queryParameters, false },
new object[] { "/\r\r/evil.com/" + queryParameters, false },
new object[] { "/\r\n/evil.com/" + queryParameters, false },
new object[] { "/\r\n\r\n/evil.com/" + queryParameters, false },
// Tabs and Newlines
new object[] { "/\t\n/evil.com/" + queryParameters, false },
new object[] { "/\t\n\n/evil.com/" + queryParameters, false },
new object[] { "/\t\r/evil.com/" + queryParameters, false },
new object[] { "/\t\r\r/evil.com/" + queryParameters, false },
new object[] { "/\t\r\n/evil.com/" + queryParameters, false },
new object[] { "/\t\r\n\r\n/evil.com/" + queryParameters, false },
new object[] { "/\n/evil.com\t/" + queryParameters, false },
new object[] { "/\n\n/evil.com\t/" + queryParameters, false },
new object[] { "/\r/evil.com\t/" + queryParameters, false },
new object[] { "/\r\r/evil.com\t/" + queryParameters, false },
new object[] { "/\r\n/evil.com\t/" + queryParameters, false },
new object[] { "/\r\n\r\n/evil.com\t/" + queryParameters, false },
};

[Theory]
[MemberData(nameof(TestCases))]
public async void GetAuthorizationContextAsync(string returnUrl, bool expected)
{
var interactionService = new DefaultIdentityServerInteractionService(null, null, null, null, null, null, null,
GetReturnUrlParser(), new LoggerFactory().CreateLogger<DefaultIdentityServerInteractionService>());
var actual = await interactionService.GetAuthorizationContextAsync(returnUrl);
if (expected)
{
actual.Should().NotBeNull();
}
else
{
actual.Should().BeNull();
}
}

[Theory]
[MemberData(nameof(TestCases))]
public void IsLocalUrl(string returnUrl, bool expected)
{
returnUrl.IsLocalUrl().Should().Be(expected);
}

[Theory]
[MemberData(nameof(TestCases))]
public void GetIdentityServerRelativeUrl(string returnUrl, bool expected)
{
var serverUrls = new MockServerUrls
{
Origin = "https://localhost:5001",
BasePath = "/"
};
var actual = serverUrls.GetIdentityServerRelativeUrl(returnUrl);
if (expected)
{
actual.Should().NotBeNull();
}
else
{
actual.Should().BeNull();
}
}

[Theory]
[MemberData(nameof(TestCases))]
public async void OidcReturnUrlParser_ParseAsync(string returnUrl, bool expected)
{
var oidcParser = GetOidcReturnUrlParser();
var actual = await oidcParser.ParseAsync(returnUrl);
if (expected)
{
actual.Should().NotBeNull();
}
else
{
actual.Should().BeNull();
}
}

[Theory]
[MemberData(nameof(TestCases))]
public void OidcReturnUrlParser_IsValidReturnUrl(string returnUrl, bool expected)
{
var oidcParser = GetOidcReturnUrlParser();
oidcParser.IsValidReturnUrl(returnUrl).Should().Be(expected);
}


[Theory]
[MemberData(nameof(TestCases))]
public void ReturnUrlParser_IsValidReturnUrl(string returnUrl, bool expected)
{
var parser = GetReturnUrlParser();
parser.IsValidReturnUrl(returnUrl).Should().Be(expected);
}

[Theory]
[MemberData(nameof(TestCases))]
public async void ReturnUrlParser_ParseAsync(string returnUrl, bool expected)
{
var parser = GetReturnUrlParser();
var actual = await parser.ParseAsync(returnUrl);
if (expected)
{
actual.Should().NotBeNull();
}
else
{
actual.Should().BeNull();
}
}

private static ReturnUrlParser GetReturnUrlParser()
{
var oidcParser = GetOidcReturnUrlParser();
var parser = new ReturnUrlParser(new IReturnUrlParser[] { oidcParser });
return parser;
}

private static OidcReturnUrlParser GetOidcReturnUrlParser()
{
return new OidcReturnUrlParser(
new IdentityServerOptions(),
new StubAuthorizeRequestValidator
{
Result = new AuthorizeRequestValidationResult
(
new ValidatedAuthorizeRequest()
)
},
new MockUserSession(),
new MockServerUrls(),
new LoggerFactory().CreateLogger<OidcReturnUrlParser>());
}


}

0 comments on commit 765116a

Please sign in to comment.