Skip to content

Fix admin redirects with conventional site routing #15

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.

Sign up for GitHub

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

Open
MortenFriisSiteImprove wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix admin redirects with conventional site routing #15

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions SiteImprove.Optimizely.Plugin/Controllers/SiteimproveAdminController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public ActionResult Save(bool recheck, bool latestUI, string apiUser, string api

_settingsRepo.SaveToken(settings.Token, settings.Recheck, settings.LatestUI, settings.ApiUser, settings.ApiKey, settings.UrlMap);

return RedirectToAction("Index");
return Redirect(_moduleResourceResolver.ResolvePath(Constants.SiteImproveModuleName, "SiteimproveAdmin"));
}

private static bool IsWebUrl(string value)
Expand All @@ -99,10 +99,10 @@ public ActionResult EnablePrepublishCheck(bool enablePrepublishCheck = false)

if (!success)
{
return RedirectToAction("Index", new { prepublishError = true });
return Redirect(_moduleResourceResolver.ResolvePath(Constants.SiteImproveModuleName, "SiteimproveAdmin") + "?prepublishError=true");
}

return RedirectToAction("Index");
return Redirect(_moduleResourceResolver.ResolvePath(Constants.SiteImproveModuleName, "SiteimproveAdmin"));
}
}
}
4 changes: 2 additions & 2 deletions TESTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ dotnet restore tests/Plugin.Tests/Plugin.Tests.csproj --locked-mode
dotnet test tests/Plugin.Tests/Plugin.Tests.csproj --configuration Release --no-restore -p:GeneratePackageOnBuild=false --logger 'trx;LogFilePrefix=backend' --results-directory test-results/backend
```

The 56 backend cases run on both .NET 6 and .NET 8. The tests build and reference
The 64 backend cases run on both .NET 6 and .NET 8. The tests build and reference
the actual plugin project and locked Optimizely dependencies. They do not establish
compatibility with every CMS 12 release. The plugin still targets .NET 6, which
produces an end-of-support build warning; this PR does not change that target.
Expand All @@ -24,7 +24,7 @@ produces an end-of-support build warning; this PR does not change that target.
| Page URL controller | Revision and language; direct Block request returns HTTP 400 without a typed Page lookup. |
| URL mapping | Host/scheme/port matching; preservation of page paths and query strings; independent sites and language paths; missing URLs/sites and resolver errors. |
| Settings | First save, update and reload; URL maps; token reuse; automatic/manual renewal; failed renewal preserves configuration; concurrent first requests generate one token; unavailable service does not create a blank record. |
| Admin actions | Save inputs, valid and duplicate URL maps, empty mapping rows, and Prepublish enablement outcome. |
| Admin actions | Save inputs, valid and duplicate URL maps, empty mapping rows, and Prepublish enablement outcome; module redirects with and without a conventional route. |
| Authorization | Real ASP.NET middleware and production policy/controller attributes; all four configured roles; anonymous and unauthorized callers; protected reads and writes. |
| Publish events | Recheck URL/token; disabled rechecks, Blocks, missing URLs and background events; start-page recrawl transition; shutdown/reinitialization avoid duplicate subscriptions. |
| Overlay selection | Exactly one script matching the latest-interface setting. |
Expand Down
42 changes: 38 additions & 4 deletions tests/Plugin.Tests/AuthorizationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Moq;
using SiteImprove.Optimizely.Plugin;
using SiteImprove.Optimizely.Plugin.Controllers;
using SiteImprove.Optimizely.Plugin.Helper;
using SiteImprove.Optimizely.Plugin.Infrastructure;
Expand All @@ -23,7 +24,7 @@ namespace Plugin.Tests;

public class AuthorizationTests
{
private static TestServer Server()
private static TestServer Server(bool conventionalRoute = true, bool prepublishSuccess = true)
{
return new TestServer(new WebHostBuilder().ConfigureServices(services => {
services.AddLogging();
Expand All @@ -36,13 +37,22 @@ private static TestServer Server()
settings.Setup(x => x.GetToken()).Returns("fixture-token");
settings.Setup(x => x.GetSetting()).Returns(new Settings { Token = "fixture-token" });
services.AddSingleton(settings.Object);
services.AddSingleton(Mock.Of<ISiteimproveHelper>());
services.AddSingleton(Mock.Of<IModuleResourceResolver>());
var helper = new Mock<ISiteimproveHelper>();
helper.Setup(x => x.EnablePrepublishCheck(It.IsAny<string>(), It.IsAny<string>())).Returns(prepublishSuccess);
services.AddSingleton(helper.Object);
var resolver = new Mock<IModuleResourceResolver>();
resolver.Setup(x => x.ResolvePath(Constants.SiteImproveModuleName, "SiteimproveAdmin"))
.Returns("/custom-ui/SiteImprove.Optimizely.Plugin/SiteimproveAdmin");
services.AddSingleton(resolver.Object);
}).Configure(app => {
app.UseRouting();
app.UseAuthentication();
app.UseAuthorization();
app.UseEndpoints(endpoints => endpoints.MapControllerRoute("default", "{controller}/{action}"));
app.UseEndpoints(endpoints => {
if (conventionalRoute)
endpoints.MapControllerRoute("default", "{controller}/{action}/{id?}");
endpoints.MapControllerRoute("module", "custom-ui/SiteImprove.Optimizely.Plugin/{controller}/{action=Index}");
});
}));
}

Expand Down Expand Up @@ -78,6 +88,30 @@ public async Task Anonymous_and_unauthorized_users_cannot_read_or_change_protect
Assert.Equal(HttpStatusCode.Forbidden, (await client.GetAsync("/Siteimprove/IsAuthorized")).StatusCode);
}

[Theory]
[InlineData(false, "Save", false, true)]
[InlineData(true, "Save", false, true)]
[InlineData(false, "EnablePrepublishCheck", true, true)]
[InlineData(true, "EnablePrepublishCheck", true, true)]
[InlineData(false, "EnablePrepublishCheck", true, false)]
[InlineData(true, "EnablePrepublishCheck", true, false)]
[InlineData(false, "EnablePrepublishCheck", false, true)]
[InlineData(true, "EnablePrepublishCheck", false, true)]
public async Task Admin_posts_redirect_to_the_module_under_either_route_setup(
bool conventionalRoute, string action, bool enable, bool success)
{
using var server = Server(conventionalRoute, success);
using var client = server.CreateClient();
client.DefaultRequestHeaders.Add("X-Fixture-Role", "CmsAdmins");
const string moduleUrl = "/custom-ui/SiteImprove.Optimizely.Plugin/SiteimproveAdmin";
var response = await client.PostAsync(moduleUrl + "/" + action,
new FormUrlEncodedContent(new Dictionary<string, string> {
["enablePrepublishCheck"] = enable.ToString()
}));
Assert.Equal(HttpStatusCode.Redirect, response.StatusCode);
Assert.Equal(moduleUrl + (success ? "" : "?prepublishError=true"), response.Headers.Location?.OriginalString);
}

private sealed class FixtureAuthentication : AuthenticationHandler<AuthenticationSchemeOptions>
{
#if NET6_0
Expand Down
18 changes: 12 additions & 6 deletions tests/Plugin.Tests/ControllerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Microsoft.Extensions.DependencyInjection;
using Moq;
using Newtonsoft.Json.Linq;
using SiteImprove.Optimizely.Plugin;
using SiteImprove.Optimizely.Plugin.Controllers;
using SiteImprove.Optimizely.Plugin.Helper;
using SiteImprove.Optimizely.Plugin.Models;
Expand Down Expand Up @@ -50,7 +51,14 @@ public void Direct_block_request_returns_bad_request_without_resolving_a_page_ur
helper.Verify(x => x.GetExternalUrl(It.IsAny<PageData>()), Times.Never);
}

private SiteimproveAdminController Admin() => new(settings.Object, helper.Object, Mock.Of<IModuleResourceResolver>());
private const string AdminUrl = "/custom-ui/SiteImprove.Optimizely.Plugin/SiteimproveAdmin";

private SiteimproveAdminController Admin()
{
var resolver = new Mock<IModuleResourceResolver>();
resolver.Setup(x => x.ResolvePath(Constants.SiteImproveModuleName, "SiteimproveAdmin")).Returns(AdminUrl);
return new(settings.Object, helper.Object, resolver.Object);
}

[Fact]
public void Saving_settings_preserves_token_and_filters_invalid_and_duplicate_mappings()
Expand All @@ -62,7 +70,7 @@ public void Saving_settings_preserves_token_and_filters_invalid_and_duplicate_ma
new KeyValuePair<string, string>("/relative", "https://public.example"),
new KeyValuePair<string, string>("https://other.example", "/relative")
};
Assert.IsType<RedirectToActionResult>(Admin().Save(true, false, "fixture-user", "fixture-key", map));
Assert.Equal(AdminUrl, Assert.IsType<RedirectResult>(Admin().Save(true, false, "fixture-user", "fixture-key", map)).Url);
settings.Verify(x => x.SaveToken("existing-token", true, false, "fixture-user", "fixture-key",
It.Is<Dictionary<string, string>>(m => m.Count == 1 && m["https://cms.example"] == "https://public.example")), Times.Once);
}
Expand All @@ -82,9 +90,7 @@ public void Enabling_prepublish_reports_the_service_outcome(bool success)
{
settings.Setup(x => x.GetSetting()).Returns(new Settings { ApiUser = "fixture-user", ApiKey = "fixture-key" });
helper.Setup(x => x.EnablePrepublishCheck("fixture-user", "fixture-key")).Returns(success);
var result = Assert.IsType<RedirectToActionResult>(Admin().EnablePrepublishCheck(true));
Assert.Equal("Index", result.ActionName);
if (!success) Assert.Equal(true, result.RouteValues["prepublishError"]);
else Assert.Null(result.RouteValues);
var result = Assert.IsType<RedirectResult>(Admin().EnablePrepublishCheck(true));
Assert.Equal(AdminUrl + (success ? "" : "?prepublishError=true"), result.Url);
}
}
Loading