Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make IsRedirectAllowedUrlAsync of IAppUrlProvider async. #18492

Merged
merged 6 commits into from
Jan 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -122,40 +122,40 @@
return localizer;
}

protected RedirectResult RedirectSafely(string returnUrl, string? returnUrlHash = null)
protected virtual async Task<RedirectResult> RedirectSafelyAsync(string returnUrl, string? returnUrlHash = null)
{
return Redirect(GetRedirectUrl(returnUrl, returnUrlHash));
return Redirect(await GetRedirectUrlAsync(returnUrl, returnUrlHash));

Check warning on line 127 in framework/src/Volo.Abp.AspNetCore.Mvc.UI/Volo/Abp/AspNetCore/Mvc/UI/RazorPages/AbpPageModel.cs

View check run for this annotation

Codecov / codecov/patch

framework/src/Volo.Abp.AspNetCore.Mvc.UI/Volo/Abp/AspNetCore/Mvc/UI/RazorPages/AbpPageModel.cs#L127

Added line #L127 was not covered by tests
}

protected virtual string GetRedirectUrl(string returnUrl, string? returnUrlHash = null)
protected virtual async Task<string> GetRedirectUrlAsync(string returnUrl, string? returnUrlHash = null)
{
returnUrl = NormalizeReturnUrl(returnUrl);
returnUrl = await NormalizeReturnUrlAsync(returnUrl);

Check warning on line 132 in framework/src/Volo.Abp.AspNetCore.Mvc.UI/Volo/Abp/AspNetCore/Mvc/UI/RazorPages/AbpPageModel.cs

View check run for this annotation

Codecov / codecov/patch

framework/src/Volo.Abp.AspNetCore.Mvc.UI/Volo/Abp/AspNetCore/Mvc/UI/RazorPages/AbpPageModel.cs#L132

Added line #L132 was not covered by tests

if (!returnUrlHash.IsNullOrWhiteSpace())
{
returnUrl = returnUrl + returnUrlHash;
returnUrl += returnUrlHash;

Check warning on line 136 in framework/src/Volo.Abp.AspNetCore.Mvc.UI/Volo/Abp/AspNetCore/Mvc/UI/RazorPages/AbpPageModel.cs

View check run for this annotation

Codecov / codecov/patch

framework/src/Volo.Abp.AspNetCore.Mvc.UI/Volo/Abp/AspNetCore/Mvc/UI/RazorPages/AbpPageModel.cs#L136

Added line #L136 was not covered by tests
}

return returnUrl;
}

private string NormalizeReturnUrl(string returnUrl)
protected virtual async Task<string> NormalizeReturnUrlAsync(string returnUrl)
{
if (returnUrl.IsNullOrEmpty())
{
return GetAppHomeUrl();
return await GetAppHomeUrlAsync();

Check warning on line 146 in framework/src/Volo.Abp.AspNetCore.Mvc.UI/Volo/Abp/AspNetCore/Mvc/UI/RazorPages/AbpPageModel.cs

View check run for this annotation

Codecov / codecov/patch

framework/src/Volo.Abp.AspNetCore.Mvc.UI/Volo/Abp/AspNetCore/Mvc/UI/RazorPages/AbpPageModel.cs#L146

Added line #L146 was not covered by tests
}

if (Url.IsLocalUrl(returnUrl) || AppUrlProvider.IsRedirectAllowedUrl(returnUrl))
if (Url.IsLocalUrl(returnUrl) || await AppUrlProvider.IsRedirectAllowedUrlAsync(returnUrl))
{
return returnUrl;
}

return GetAppHomeUrl();
return await GetAppHomeUrlAsync();

Check warning on line 154 in framework/src/Volo.Abp.AspNetCore.Mvc.UI/Volo/Abp/AspNetCore/Mvc/UI/RazorPages/AbpPageModel.cs

View check run for this annotation

Codecov / codecov/patch

framework/src/Volo.Abp.AspNetCore.Mvc.UI/Volo/Abp/AspNetCore/Mvc/UI/RazorPages/AbpPageModel.cs#L154

Added line #L154 was not covered by tests
}

protected virtual string GetAppHomeUrl()
protected virtual Task<string> GetAppHomeUrlAsync()
{
return "~/"; //TODO: ???
return Task.FromResult("~/"); //TODO: ???

Check warning on line 159 in framework/src/Volo.Abp.AspNetCore.Mvc.UI/Volo/Abp/AspNetCore/Mvc/UI/RazorPages/AbpPageModel.cs

View check run for this annotation

Codecov / codecov/patch

framework/src/Volo.Abp.AspNetCore.Mvc.UI/Volo/Abp/AspNetCore/Mvc/UI/RazorPages/AbpPageModel.cs#L159

Added line #L159 was not covered by tests
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.DependencyInjection;
Expand Down Expand Up @@ -104,14 +105,14 @@
return localizer;
}

protected virtual RedirectResult RedirectSafely(string returnUrl, string? returnUrlHash = null)
protected virtual async Task<RedirectResult> RedirectSafelyAsync(string returnUrl, string? returnUrlHash = null)
{
return Redirect(GetRedirectUrl(returnUrl, returnUrlHash));
return Redirect(await GetRedirectUrlAsync(returnUrl, returnUrlHash));

Check warning on line 110 in framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AbpController.cs

View check run for this annotation

Codecov / codecov/patch

framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AbpController.cs#L110

Added line #L110 was not covered by tests
}

protected virtual string GetRedirectUrl(string returnUrl, string? returnUrlHash = null)
protected virtual async Task<string> GetRedirectUrlAsync(string returnUrl, string? returnUrlHash = null)
{
returnUrl = NormalizeReturnUrl(returnUrl);
returnUrl = await NormalizeReturnUrlAsync(returnUrl);

Check warning on line 115 in framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AbpController.cs

View check run for this annotation

Codecov / codecov/patch

framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AbpController.cs#L115

Added line #L115 was not covered by tests

if (!returnUrlHash.IsNullOrWhiteSpace())
{
Expand All @@ -121,23 +122,23 @@
return returnUrl;
}

protected virtual string NormalizeReturnUrl(string returnUrl)
protected virtual async Task<string> NormalizeReturnUrlAsync(string returnUrl)
{
if (returnUrl.IsNullOrEmpty())
{
return GetAppHomeUrl();
return await GetAppHomeUrlAsync();

Check warning on line 129 in framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AbpController.cs

View check run for this annotation

Codecov / codecov/patch

framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AbpController.cs#L129

Added line #L129 was not covered by tests
}

if (Url.IsLocalUrl(returnUrl) || AppUrlProvider.IsRedirectAllowedUrl(returnUrl))
if (Url.IsLocalUrl(returnUrl) || await AppUrlProvider.IsRedirectAllowedUrlAsync(returnUrl))
{
return returnUrl;
}

return GetAppHomeUrl();
return await GetAppHomeUrlAsync();

Check warning on line 137 in framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AbpController.cs

View check run for this annotation

Codecov / codecov/patch

framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AbpController.cs#L137

Added line #L137 was not covered by tests
}

protected virtual string GetAppHomeUrl()
protected virtual Task<string> GetAppHomeUrlAsync()
{
return Url.Content("~/");
return Task.FromResult(Url.Content("~/"));

Check warning on line 142 in framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AbpController.cs

View check run for this annotation

Codecov / codecov/patch

framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AbpController.cs#L142

Added line #L142 was not covered by tests
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,46 +22,46 @@
}

[HttpGet]
public virtual ActionResult Login(string returnUrl = "", string returnUrlHash = "")
public virtual async Task<ActionResult> LoginAsync(string returnUrl = "", string returnUrlHash = "")
{
if (CurrentUser.IsAuthenticated)
{
return RedirectSafely(returnUrl, returnUrlHash);
return await RedirectSafelyAsync(returnUrl, returnUrlHash);

Check warning on line 29 in framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Authentication/ChallengeAccountController.cs

View check run for this annotation

Codecov / codecov/patch

framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Authentication/ChallengeAccountController.cs#L29

Added line #L29 was not covered by tests
}

return Challenge(new AuthenticationProperties { RedirectUri = GetRedirectUrl(returnUrl, returnUrlHash) }, ChallengeAuthenticationSchemas);
return Challenge(new AuthenticationProperties { RedirectUri = await GetRedirectUrlAsync(returnUrl, returnUrlHash) }, ChallengeAuthenticationSchemas);

Check warning on line 32 in framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Authentication/ChallengeAccountController.cs

View check run for this annotation

Codecov / codecov/patch

framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Authentication/ChallengeAccountController.cs#L32

Added line #L32 was not covered by tests
}

[HttpGet]
public virtual async Task<ActionResult> Logout(string returnUrl = "", string returnUrlHash = "")
public virtual async Task<ActionResult> LogoutAsync(string returnUrl = "", string returnUrlHash = "")
{
await HttpContext.SignOutAsync();

if (HttpContext.User.Identity?.AuthenticationType == AuthenticationType)
{
return RedirectSafely(returnUrl, returnUrlHash);
return await RedirectSafelyAsync(returnUrl, returnUrlHash);

Check warning on line 42 in framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Authentication/ChallengeAccountController.cs

View check run for this annotation

Codecov / codecov/patch

framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Authentication/ChallengeAccountController.cs#L42

Added line #L42 was not covered by tests
}

return SignOut(new AuthenticationProperties { RedirectUri = GetRedirectUrl(returnUrl, returnUrlHash) }, ChallengeAuthenticationSchemas);
return SignOut(new AuthenticationProperties { RedirectUri = await GetRedirectUrlAsync(returnUrl, returnUrlHash) }, ChallengeAuthenticationSchemas);

Check warning on line 45 in framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Authentication/ChallengeAccountController.cs

View check run for this annotation

Codecov / codecov/patch

framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Authentication/ChallengeAccountController.cs#L45

Added line #L45 was not covered by tests
}

[HttpGet]
public virtual async Task<IActionResult> FrontChannelLogout(string sid)
public virtual async Task<IActionResult> FrontChannelLogoutAsync(string sid)
{
if (User.Identity != null && User.Identity.IsAuthenticated)
{
var currentSid = User.FindFirst("sid")?.Value ?? string.Empty;
if (string.Equals(currentSid, sid, StringComparison.Ordinal))
{
await Logout();
await LogoutAsync();

Check warning on line 56 in framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Authentication/ChallengeAccountController.cs

View check run for this annotation

Codecov / codecov/patch

framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Authentication/ChallengeAccountController.cs#L56

Added line #L56 was not covered by tests
}
}

return NoContent();
}

[HttpGet]
public virtual Task<IActionResult> AccessDenied()
public virtual Task<IActionResult> AccessDeniedAsync()
{
return Task.FromResult<IActionResult>(Challenge(
new AuthenticationProperties
Expand All @@ -78,9 +78,9 @@
}

[HttpGet]
public virtual async Task<ActionResult> Challenge(string returnUrl = "", string returnUrlHash = "")
public virtual async Task<ActionResult> ChallengeAsync(string returnUrl = "", string returnUrlHash = "")
{
await HttpContext.SignOutAsync();
return Challenge(new AuthenticationProperties { RedirectUri = GetRedirectUrl(returnUrl, returnUrlHash) }, ChallengeAuthenticationSchemas);
return Challenge(new AuthenticationProperties { RedirectUri = await GetRedirectUrlAsync(returnUrl, returnUrlHash) }, ChallengeAuthenticationSchemas);

Check warning on line 84 in framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Authentication/ChallengeAccountController.cs

View check run for this annotation

Codecov / codecov/patch

framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/Authentication/ChallengeAccountController.cs#L84

Added line #L84 was not covered by tests
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ public class AppUrlProvider : IAppUrlProvider, ITransientDependency
{
protected AppUrlOptions Options { get; }
protected IMultiTenantUrlProvider MultiTenantUrlProvider { get; }

public ILogger<AppUrlProvider> Logger { get; set; }

public AppUrlProvider(
Expand All @@ -37,9 +36,14 @@ await GetConfiguredUrl(
);
}

public bool IsRedirectAllowedUrl(string url)
public virtual async Task<bool> IsRedirectAllowedUrlAsync(string url)
{
var allow = Options.RedirectAllowedUrls.Any(x => url.StartsWith(x, StringComparison.CurrentCultureIgnoreCase));
var redirectAllowedUrls = new List<string>();
foreach (var redirectAllowedUrl in Options.RedirectAllowedUrls)
{
redirectAllowedUrls.Add((await NormalizeUrlAsync(redirectAllowedUrl))!);
}
var allow = redirectAllowedUrls.Any(x => url.StartsWith(x, StringComparison.CurrentCultureIgnoreCase));
if (!allow)
{
Logger.LogError($"Invalid RedirectUrl: {url}, Use {nameof(AppUrlProvider)} to configure it!");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ public interface IAppUrlProvider

Task<string?> GetUrlOrNullAsync([NotNull] string appName, string? urlName = null);

bool IsRedirectAllowedUrl(string url);
Task<bool> IsRedirectAllowedUrlAsync(string url);

Task<string?> NormalizeUrlAsync(string? url);
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using Microsoft.Extensions.DependencyInjection;
using Shouldly;
using Volo.Abp.MultiTenancy;
using Volo.Abp.MultiTenancy.ConfigurationStore;
using Volo.Abp.Testing;
using Volo.Abp.UI.Navigation.Urls;
using Xunit;
Expand All @@ -15,6 +16,8 @@ public class AppUrlProvider_Tests : AbpIntegratedTest<AbpUiNavigationTestModule>
private readonly IAppUrlProvider _appUrlProvider;
private readonly ICurrentTenant _currentTenant;

private readonly Guid _tenantAId = Guid.NewGuid();

public AppUrlProvider_Tests()
{
_appUrlProvider = ServiceProvider.GetRequiredService<AppUrlProvider>();
Expand All @@ -35,12 +38,22 @@ protected override void AfterAddApplication(IServiceCollection services)
options.RedirectAllowedUrls.AddRange(new List<string>()
{
"https://wwww.volosoft.com",
"https://wwww.aspnetzero.com"
"https://wwww.aspnetzero.com",
"https://{{tenantName}}.abp.io",
"https://{{tenantId}}.abp.io"
});

options.Applications["BLAZOR"].RootUrl = "https://{{tenantId}}.abp.io";
options.Applications["BLAZOR"].Urls["PasswordReset"] = "account/reset-password";
});

services.Configure<AbpDefaultTenantStoreOptions>(options =>
{
options.Tenants = new TenantConfiguration[]
{
new(_tenantAId, "community")
};
});
}

[Fact]
Expand All @@ -64,14 +77,13 @@ public async Task GetUrlAsync()
url.ShouldBe("https://community.abp.io/account/reset-password");
}

var tenantId = Guid.NewGuid();
using (_currentTenant.Change(tenantId))
using (_currentTenant.Change(_tenantAId))
{
var url = await _appUrlProvider.GetUrlAsync("BLAZOR");
url.ShouldBe($"https://{tenantId}.abp.io");
url.ShouldBe($"https://{_tenantAId}.abp.io");

url = await _appUrlProvider.GetUrlAsync("BLAZOR", "PasswordReset");
url.ShouldBe($"https://{tenantId}.abp.io/account/reset-password");
url.ShouldBe($"https://{_tenantAId}.abp.io/account/reset-password");
}

await Assert.ThrowsAsync<AbpException>(async () =>
Expand All @@ -87,9 +99,27 @@ public async Task GetUrlOrNullAsync()
}

[Fact]
public void IsRedirectAllowedUrl()
public async Task IsRedirectAllowedUrlAsync()
{
_appUrlProvider.IsRedirectAllowedUrl("https://community.abp.io").ShouldBeFalse();
_appUrlProvider.IsRedirectAllowedUrl("https://wwww.volosoft.com").ShouldBeTrue();
(await _appUrlProvider.IsRedirectAllowedUrlAsync("https://community.abp.io")).ShouldBeFalse();
(await _appUrlProvider.IsRedirectAllowedUrlAsync("https://wwww.volosoft.com")).ShouldBeTrue();

using (_currentTenant.Change(null))
{
(await _appUrlProvider.IsRedirectAllowedUrlAsync("https://www.abp.io")).ShouldBeFalse();
(await _appUrlProvider.IsRedirectAllowedUrlAsync("https://abp.io")).ShouldBeTrue();
}

using (_currentTenant.Change(_tenantAId, "community"))
{
(await _appUrlProvider.IsRedirectAllowedUrlAsync("https://community.abp.io")).ShouldBeTrue();
(await _appUrlProvider.IsRedirectAllowedUrlAsync("https://community2.abp.io")).ShouldBeFalse();
}

using (_currentTenant.Change(_tenantAId))
{
(await _appUrlProvider.IsRedirectAllowedUrlAsync($"https://{_tenantAId}.abp.io")).ShouldBeTrue();
(await _appUrlProvider.IsRedirectAllowedUrlAsync($"https://{Guid.NewGuid()}.abp.io")).ShouldBeFalse();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ await IdentitySecurityLogManager.SaveAsync(new IdentitySecurityLogContext()
// Clear the dynamic claims cache.
await IdentityDynamicClaimsPrincipalContributorCache.ClearAsync(user.Id, user.TenantId);

return RedirectSafely(ReturnUrl, ReturnUrlHash);
return await RedirectSafelyAsync(ReturnUrl, ReturnUrlHash);
}

public override async Task<IActionResult> OnPostExternalLogin(string provider)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,28 +18,28 @@ public class LoggedOutModel : AccountPageModel
[BindProperty(SupportsGet = true)]
public string PostLogoutRedirectUri { get; set; }

public virtual Task<IActionResult> OnGetAsync()
public virtual async Task<IActionResult> OnGetAsync()
{
NormalizeUrl();
return Task.FromResult<IActionResult>(Page());
await NormalizeUrlAsync();
return Page();
}

public virtual Task<IActionResult> OnPostAsync()
public virtual async Task<IActionResult> OnPostAsync()
{
NormalizeUrl();
return Task.FromResult<IActionResult>(Page());
await NormalizeUrlAsync();
return Page();
}

protected virtual void NormalizeUrl()
protected virtual async Task NormalizeUrlAsync()
{
if (!PostLogoutRedirectUri.IsNullOrWhiteSpace())
{
PostLogoutRedirectUri = Url.Content(GetRedirectUrl(PostLogoutRedirectUri));
PostLogoutRedirectUri = Url.Content(await GetRedirectUrlAsync(PostLogoutRedirectUri));
}

if(!SignOutIframeUrl.IsNullOrWhiteSpace())
{
SignOutIframeUrl = Url.Content(GetRedirectUrl(SignOutIframeUrl));
SignOutIframeUrl = Url.Content(await GetRedirectUrlAsync(SignOutIframeUrl));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ await IdentitySecurityLogManager.SaveAsync(new IdentitySecurityLogContext()
// Clear the dynamic claims cache.
await IdentityDynamicClaimsPrincipalContributorCache.ClearAsync(user.Id, user.TenantId);

return RedirectSafely(ReturnUrl, ReturnUrlHash);
return await RedirectSafelyAsync(ReturnUrl, ReturnUrlHash);
}

/// <summary>
Expand Down Expand Up @@ -237,7 +237,7 @@ await IdentitySecurityLogManager.SaveAsync(new IdentitySecurityLogContext()
await IdentityDynamicClaimsPrincipalContributorCache.ClearAsync(user.Id, user.TenantId);
}

return RedirectSafely(returnUrl, returnUrlHash);
return await RedirectSafelyAsync(returnUrl, returnUrlHash);
}

//TODO: Handle other cases for result!
Expand Down Expand Up @@ -279,7 +279,7 @@ await IdentitySecurityLogManager.SaveAsync(new IdentitySecurityLogContext()
// Clear the dynamic claims cache.
await IdentityDynamicClaimsPrincipalContributorCache.ClearAsync(user.Id, user.TenantId);

return RedirectSafely(returnUrl, returnUrlHash);
return await RedirectSafelyAsync(returnUrl, returnUrlHash);
}

protected virtual async Task ReplaceEmailToUsernameOfInputIfNeeds()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ await IdentitySecurityLogManager.SaveAsync(new IdentitySecurityLogContext()
await SignInManager.SignOutAsync();
if (ReturnUrl != null)
{
return RedirectSafely(ReturnUrl, ReturnUrlHash);
return await RedirectSafelyAsync(ReturnUrl, ReturnUrlHash);
}

if (await SettingProvider.IsTrueAsync(AccountSettingNames.EnableLocalLogin))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public virtual async Task<IActionResult> OnGetAsync()
{
if (!Url.IsLocalUrl(ReturnUrl) &&
!ReturnUrl.StartsWith(UriHelper.BuildAbsolute(Request.Scheme, Request.Host, Request.PathBase).RemovePostFix("/")) &&
!AppUrlProvider.IsRedirectAllowedUrl(ReturnUrl))
!await AppUrlProvider.IsRedirectAllowedUrlAsync(ReturnUrl))
{
ReturnUrl = null;
}
Expand Down
Loading
Loading