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

NEST-501: Updating ZAP to 2.15.0, regex patterns for LogsShouldBeEmptyAsync(), giving access to the ZAP log, Swagger auto-discovery for OpenAPI scan #388

Merged
merged 24 commits into from
Jul 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
e9adca7
Updating ZAP to 2.15.0
Piedone Jul 8, 2024
86b53c7
False positive analyzer violation
Piedone Jul 8, 2024
6e7d9d4
Adding AssertAppLogsCanContainWarningsAndCacheFolderErrorsAsync for a…
Piedone Jul 9, 2024
3d7509e
Merge remote-tracking branch 'origin/dev' into issue/NEST-501
Piedone Jul 9, 2024
a6c88c9
Using regex for permitted error lines
Piedone Jul 10, 2024
6cd5b25
Fix build error
Piedone Jul 10, 2024
71c315a
Ability to access the ZAP log too
Piedone Jul 10, 2024
d34c543
The default ZAP log level should be Info
Piedone Jul 10, 2024
a2b6f7f
No need to lower the ZAP log level's casing for the CLI
Piedone Jul 10, 2024
ec81ba1
MD code styling
Piedone Jul 10, 2024
a12424c
Doc about logging the URL
Piedone Jul 10, 2024
3ae5571
Fixing that ZAP couldn't create the home directory in GHA
Piedone Jul 10, 2024
4dd7535
Docs
Piedone Jul 10, 2024
6352f80
Merge remote-tracking branch 'origin/dev' into issue/NEST-501
Piedone Jul 11, 2024
2d06176
Adding Swagger to auto-discover OpenAPI endpoints
Piedone Jul 11, 2024
1c15acb
Fixing Swagger analyzer violation
Piedone Jul 11, 2024
66f9fb8
Making deliberate ErrorController requests not show up as ZAP warnings
Piedone Jul 14, 2024
4be77f9
Notes on Debug ZapLogLevel
Piedone Jul 14, 2024
baecd18
Updating xUnit to 2.9.0
Piedone Jul 14, 2024
58150cb
Updating samples too
Piedone Jul 14, 2024
4a2fa64
Deprecating MaxParallelTests
Piedone Jul 14, 2024
414441d
Fixing DisplayActiveScanRuleRuntimesScript
Piedone Jul 15, 2024
a08f611
YAML code styling
Piedone Jul 15, 2024
6364c10
Merge remote-tracking branch 'origin/dev' into issue/NEST-501
Piedone Jul 17, 2024
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
2 changes: 1 addition & 1 deletion Lombiq.Tests.UI.Samples/Lombiq.Tests.UI.Samples.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

<ItemGroup>
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.7.2" />
<PackageReference Include="xunit.runner.visualstudio" Version="2.8.0">
<PackageReference Include="xunit.runner.visualstudio" Version="2.8.2">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
Expand Down
24 changes: 12 additions & 12 deletions Lombiq.Tests.UI.Samples/UITestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
using Lombiq.Tests.UI.Extensions;
using Lombiq.Tests.UI.Samples.Helpers;
using Lombiq.Tests.UI.Services;
using Shouldly;
using System;
using System.Threading.Tasks;
using Xunit.Abstractions;
Expand Down Expand Up @@ -87,17 +86,18 @@ protected override Task ExecuteTestAsync(
// assertions: By default, the Orchard logs and the browser logs (where e.g. JavaScript errors show
// up) are checked and if there are any errors, the test will fail. You can also enable the checking of
// accessibility rules as we'll see later. Maybe not all of the default checks are suitable for you.
// Then it's simple to override them; here we change which log entries cause the tests to fail. We use
// the trick of making expected error messages not look like real errors.
configuration.AssertAppLogsAsync = async webApplicationInstance =>
(await webApplicationInstance.GetLogOutputAsync())
.ReplaceOrdinalIgnoreCase(
"|Lombiq.TrainingDemo.Services.DemoBackgroundTask|ERROR|Expected non-error",
"|Lombiq.TrainingDemo.Services.DemoBackgroundTask|EXPECTED_ERROR|Expected non-error")
.ReplaceOrdinalIgnoreCase(
"|OrchardCore.Media.Core.DefaultMediaFileStoreCacheFileProvider|ERROR|Error deleting cache folder",
"|OrchardCore.Media.Core.DefaultMediaFileStoreCacheFileProvider|EXPECTED_ERROR|Error deleting cache folder")
.ShouldNotContain("|ERROR|");
// Then it's simple to override them; here we change which log entries cause the tests to fail, and
// allow warnings and certain errors.
// Note that this is just for demonstration; you could use
// OrchardCoreUITestExecutorConfiguration.AssertAppLogsCanContainWarningsAndCacheFolderErrorsAsync which
// provides this configuration built-in.
configuration.AssertAppLogsAsync = webApplicationInstance =>
webApplicationInstance.LogsShouldBeEmptyAsync(
canContainWarnings: true,
permittedErrorLinePatterns:
[
"OrchardCore.Media.Core.DefaultMediaFileStoreCacheFileProvider|ERROR|Error deleting cache folder",
]);

if (changeConfigurationAsync != null) await changeConfigurationAsync(configuration);
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Lombiq.HelpfulLibraries.AspNetCore.Mvc;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using OrchardCore.Modules;

Expand All @@ -14,5 +15,6 @@ public class ApplicationInfoController : ControllerBase
public ApplicationInfoController(IApplicationContext applicationContext) => _applicationContext = applicationContext;

[HttpGet]
[ProducesResponseType<int>(StatusCodes.Status200OK)]
public IActionResult Get() => Ok(_applicationContext.GetApplicationInfo());
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public IActionResult Index()
}

[Route("api/InteractiveMode/IsInteractive")]
[HttpGet]
public IActionResult IsInteractive() => Json(_interactiveModeStatusAccessor.Enabled);

public IActionResult Continue()
Expand Down
3 changes: 3 additions & 0 deletions Lombiq.Tests.UI.Shortcuts/Lombiq.Tests.UI.Shortcuts.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@
<PackageReference Include="OrchardCore.Abstractions" Version="1.8.0" />
<PackageReference Include="OrchardCore.ResourceManagement.Abstractions" Version="1.8.0" />
<PackageReference Include="OrchardCore.Workflows" Version="1.8.0" />
<!-- With 6.6.2 and possible earlier versions we'd get an exception that the Tenants API doesn't comply with the
recommendations, see https://github.com/OrchardCMS/OrchardCore/issues/16430. -->
<PackageReference Include="Swashbuckle.AspNetCore" Version="6.5.0" />
</ItemGroup>

<ItemGroup Condition="'$(NuGetBuild)' != 'true'">
Expand Down
8 changes: 8 additions & 0 deletions Lombiq.Tests.UI.Shortcuts/Manifest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,11 @@
"OrchardCore.Workflows.Http",
]
)]

[assembly: Feature(
Id = Swagger,
Name = "Swagger - Shortcuts - Lombiq UI Testing Toolbox",
Category = "Development",
Description = "WARNING: Only enable this feature in the UI testing environment. Provides a Swagger endpoint to " +
"generate a JSON OpenAPI definition for the web APIs available in the app. Used in security scanning."
)]
1 change: 1 addition & 0 deletions Lombiq.Tests.UI.Shortcuts/ShortcutsFeatureIds.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@ public static class ShortcutsFeatureIds
public const string Default = Area;
public const string FeatureToggleTestBench = Default + ".FeatureToggleTestBench";
public const string MediaCachePurge = Default + ".MediaCachePurge";
public const string Swagger = Default + ".Swagger";
public const string Workflows = Default + ".Workflows";
}
15 changes: 15 additions & 0 deletions Lombiq.Tests.UI.Shortcuts/Startup.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
using Lombiq.HelpfulLibraries.AspNetCore.Extensions;
using Lombiq.Tests.UI.Shortcuts.Services;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.OpenApi.Models;
using OrchardCore.Modules;
using System;

namespace Lombiq.Tests.UI.Shortcuts;

Expand All @@ -13,3 +17,14 @@ public override void ConfigureServices(IServiceCollection services)
services.AddAsyncResultFilter<ApplicationInfoInjectingFilter>();
}
}

[Feature("Lombiq.Tests.UI.Shortcuts.Swagger")]
public class SwaggerStartup : StartupBase
{
public override void ConfigureServices(IServiceCollection services) =>
services.AddSwaggerGen(swaggerGenOptions =>
swaggerGenOptions.SwaggerDoc("v1", new OpenApiInfo { Title = "Orchard Core API", Version = "v1" }));

public override void Configure(IApplicationBuilder app, IEndpointRouteBuilder routes, IServiceProvider serviceProvider) =>
app.UseSwagger();
}
4 changes: 4 additions & 0 deletions Lombiq.Tests.UI/Docs/SecurityScanning.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,7 @@ This is because the Docker installation is configured to use Windows images, whi
- You can find out which rules take the most time by adding a script displaying each rules' runtime with `YamlDocumentExtensions.AddDisplayActiveScanRuleRuntimesScriptAfterActiveScan()`.
- The ["Cross Site Scripting (DOM Based)" active scan rule](https://www.zaproxy.org/docs/desktop/addons/dom-xss-active-scan-rule/), unlike other rules, launches browsers and thus will take 1-2 orders of magnitude more time than other scans, usually causing the bulk of an active scan's runtime. Also see [the official docs](https://www.zaproxy.org/docs/desktop/addons/dom-xss-active-scan-rule/). You can tune it so it completes faster but still produces acceptable results for your app. You can do this from the Automation Framework plan's YAML file (see the samples on how you can use a custom one), or with `SecurityScanConfiguration.ConfigureXssActiveScanRule()`.
- In CI workflows, you might want to restrict how many scans run in parallel, if you have more than one. You can use [xUnit's `[Collection]` attributes](https://xunit.net/docs/running-tests-in-parallel#parallelism-in-test-frameworks) to have e.g. only two collections for such tests, thus allowing only two parallel scans.
- The test fails due to Orchard Core exceptions caused by ZAP being logged, but you have no idea how those happened?
- If you app uses NLog for logging, make it log the URL too with the [AspNetRequest Url Layout Renderer](https://github.com/NLog/NLog/wiki/AspNetRequest-Url-Layout-Renderer): `${aspnet-request-url:IncludeQueryString=true}`. This will help you pinpoint the ZAP request that caused the exception. We recommend adding this before `${aspnet-traceidentifier}`, so the usual format of log messages is otherwise preserved.
- Set `SecurityScanningConfiguration.CreateReportOnTestFailAlways` to `true` to get a report even if the security scan passes. In the report, you may find details even about ignored alerts.
- Increase the ZAP log level to `Debug` via `SecurityScanningConfiguration.ZapLogLevel`. This will include the ZAP log in the failure dump, providing you with more information about what ZAP did exactly, including the URLs of the requests it sent. Note that this slows down the security scan considerably (can even double the runtime), so use it only when necessary.
35 changes: 17 additions & 18 deletions Lombiq.Tests.UI/Extensions/WebApplicationInstanceExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text.RegularExpressions;
using System.Threading;
using System.Threading.Tasks;

Expand All @@ -14,39 +15,37 @@ public static class WebApplicationInstanceExtensions
/// Asserting that the logs should be empty. When they aren't the Shouldly exception will contain the logs'
/// contents.
/// </summary>
/// <param name="permittedErrorLines">
/// <param name="permittedErrorLinePatterns">
/// If not <see langword="null"/> or empty, each line is split and any lines containing <c>|ERROR|</c> will be
/// ignored if they contain any string from this collection (case-insensitive).
/// ignored if they regex match any string from this collection (case-insensitive).
/// </param>
public static async Task LogsShouldBeEmptyAsync(
this IWebApplicationInstance webApplicationInstance,
bool canContainWarnings = false,
ICollection<string> permittedErrorLines = null,
ICollection<string> permittedErrorLinePatterns = null,
CancellationToken cancellationToken = default)
{
if (cancellationToken == default) cancellationToken = CancellationToken.None;
permittedErrorLines ??= [];
permittedErrorLinePatterns ??= [];

var logOutput = await webApplicationInstance.GetLogOutputAsync(cancellationToken);

if (canContainWarnings)
{
logOutput.ShouldNotContain("|FATAL|");
logOutput.ShouldNotContain("|FATAL|");

var errorLines = logOutput
.SplitByNewLines()
.Where(line => line.Contains("|ERROR|"));
var lines = logOutput.SplitByNewLines();

if (permittedErrorLines.Count != 0)
{
errorLines = errorLines.Where(line => !permittedErrorLines.Any(line.ContainsOrdinalIgnoreCase));
}
var errorLines = lines.Where(line => line.Contains("|ERROR|"));

errorLines.ShouldBeEmpty();
if (permittedErrorLinePatterns.Count != 0)
{
errorLines = errorLines.Where(line =>
!permittedErrorLinePatterns.Any(pattern => Regex.IsMatch(line, pattern, RegexOptions.IgnoreCase | RegexOptions.Compiled)));
}
else

errorLines.ShouldBeEmpty();

if (!canContainWarnings)
{
logOutput.ShouldBeEmpty();
lines.Where(line => line.Contains("|WARNING|")).ShouldBeEmpty();
}
}

Expand Down
2 changes: 1 addition & 1 deletion Lombiq.Tests.UI/Lombiq.Tests.UI.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
<PackageReference Include="Shouldly" Version="4.2.1" />
<PackageReference Include="System.Linq.Async" Version="6.0.1" />
<PackageReference Include="TWP.Selenium.Axe.Html" Version="1.0.0" />
<PackageReference Include="xunit" Version="2.8.0" />
<PackageReference Include="xunit" Version="2.9.0" />
<PackageReference Include="YamlDotNet" Version="15.1.4" />
</ItemGroup>

Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,20 @@
- parameters:
action: add
type: active
type: standalone
engine: 'ECMAScript : Graal.js'
name: displayRuleRuntimes
target: ''
inline: var extAscan = control.getExtensionLoader().getExtension(\n org.zaproxy.zap.extension.ascan.ExtensionActiveScan.NAME);\n\
# It's easier to copy this as-is between this file and the ZAP GUI.
# yamllint disable
inline: "var extAscan = control.getExtensionLoader().getExtension(\n org.zaproxy.zap.extension.ascan.ExtensionActiveScan.NAME);\n\
\nif (extAscan != null) {\n var lastScan = extAscan.getLastScan();\n if (lastScan\
\ != null) {\n var hps = lastScan.getHostProcesses().toArray();\n for\
\ (var i=0; i < hps.length; i++) {\n var hp = hps[i];\n var plugins\
\ = hp.getCompleted().toArray();\n for (var j=0; j < plugins.length; j++)\
\ {\n var plugin = plugins[j];\n var timeTaken = plugin.getTimeFinished().getTime()\n\
\ - plugin.getTimeStarted().getTime();\n print(plugin.getName()\
\ + \\\t\ + timeTaken);\n }\n }\n }\n}\n
\ + \"\\t\" + timeTaken);\n }\n }\n }\n}"
# yamllint enable
name: script
type: script
- parameters:
Expand Down
3 changes: 3 additions & 0 deletions Lombiq.Tests.UI/SecurityScanning/SecurityScanConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,9 @@ internal async Task ApplyToPlanAsync(YamlDocument yamlDocument, UITestContext co
{
yamlDocument.SetStartUrl(StartUri);

var openApiJob = yamlDocument.GetJobByType("openapi");
openApiJob?.GetOrCreateParameters().SetMappingChild("targetUrl", StartUri.ToString());

foreach (var uri in _additionalUris)
{
yamlDocument.AddUrl(uri.IsAbsoluteUri ? uri : context.GetAbsoluteUri(uri.OriginalString));
Expand Down
4 changes: 3 additions & 1 deletion Lombiq.Tests.UI/SecurityScanning/SecurityScanResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ namespace Lombiq.Tests.UI.SecurityScanning;
public class SecurityScanResult
{
public string ReportsDirectoryPath { get; }
public string ZapLogPath { get; set; }
public SarifLog SarifLog { get; }

public SecurityScanResult(string reportsDirectoryPath, SarifLog sarifLog)
public SecurityScanResult(string reportsDirectoryPath, string zapLogPath, SarifLog sarifLog)
{
ReportsDirectoryPath = reportsDirectoryPath;
ZapLogPath = zapLogPath;
SarifLog = sarifLog;
}
}
34 changes: 34 additions & 0 deletions Lombiq.Tests.UI/SecurityScanning/SecurityScanningConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,34 @@ public class SecurityScanningConfiguration
/// </remarks>
internal bool CreateReportAlways { get; set; }

/// <summary>
/// Gets or sets a value indicating whether to save a report to the failure dump for every scan if the test fails,
/// even if the security scan itself passes.
/// </summary>
// This is just a temporary solution until CreateReportAlways will work in
// https://github.com/Lombiq/UI-Testing-Toolbox/issues/323. Then it needs to be deprecated.
public bool CreateReportOnTestFailAlways
{
get => CreateReportAlways;
set => CreateReportAlways = value;
}

/// <summary>
/// Gets or sets a delegate that may modify the deserialized representation of the ZAP Automation Framework plan in
/// YAML.
/// </summary>
public Func<UITestContext, YamlDocument, Task> ZapAutomationFrameworkPlanModifier { get; set; }

/// <summary>
/// Gets or sets the log level for the ZAP security scanning. Defaults to <see cref="ZapLogLevel.Info"/>. These log
/// levels correspond to <see href="https://logging.apache.org/log4j/2.x/manual/customloglevels.html">Log4j's
/// standard log levels</see>. Also see <see
/// href="https://www.zaproxy.org/faq/how-do-you-configure-zap-logging/">the docs on ZAP's logging</see>. Note that
/// using a log level more granular than <see cref="ZapLogLevel.Info"/> (like <see cref="ZapLogLevel.Debug"/> slows
/// down the security scan considerably (can even double the runtime), so use it only when necessary.
/// </summary>
public ZapLogLevel ZapLogLevel { get; set; } = ZapLogLevel.Info;

/// <summary>
/// Gets or sets a delegate to run assertions on the <see cref="SarifLog"/> when security scanning happens.
/// </summary>
Expand All @@ -43,3 +65,15 @@ public class SecurityScanningConfiguration
Details = result,
});
}

public enum ZapLogLevel
{
Off,
Fatal,
Error,
Warn,
Info,
Debug,
Trace,
All,
}
Loading