From 338c1a8647bfcf89c72fcc2e0d175e1de12f27c6 Mon Sep 17 00:00:00 2001
From: Jaap-Jan de Wit | DodoTech <jaap-jan@dodotech.dev>
Date: Sat, 1 Aug 2026 14:55:32 +0200
Subject: [PATCH] Move the plaintext exemption out of the test body it made too
 long
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

MA0051: TheWholeSlice reached 68 lines against a limit of 60, because the last commit put a
nine-line paragraph and a five-line call in the middle of it. dotnet format --verify-no-changes
runs the analysers, so the build stopped there and never reached the two fixes that paragraph
was explaining.

The explanation was worth keeping and the place was wrong. It is a fact about one call, not
about the slice, and this file already keeps its steps in named methods under a "Steps"
heading. SignInToTheStackAsync now holds both, which leaves the test body reading as the
sequence it is meant to be — sign in, enroll, unlock, write, read elsewhere — rather than a
sequence with an essay in it.

Nothing about the behaviour changed: same call, same configureOidc, same exemption claimed
by the same single caller that starts the provider it is talking to.

I should also say how this reached CI, since the answer is not that it was hard to catch. I
ran the format gate locally before the last push and read the exit code of a pipeline it was
piped into rather than the tool's own, so a failing command reported as passing. Run again
against the tool's exit status it is 0, and the end-to-end suite still passes in the Alpine
container that reproduces the runner.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../M1VerticalSliceTests.cs                   | 42 ++++++++++++-------
 1 file changed, 27 insertions(+), 15 deletions(-)

diff --git a/tests/DodoSSH.SystemTests/M1VerticalSliceTests.cs b/tests/DodoSSH.SystemTests/M1VerticalSliceTests.cs
index b71bbdc..9f88548 100644
--- a/tests/DodoSSH.SystemTests/M1VerticalSliceTests.cs
+++ b/tests/DodoSSH.SystemTests/M1VerticalSliceTests.cs
@@ -61,21 +61,7 @@ public sealed class M1VerticalSliceTests(DevStack stack) : IClassFixture<DevStac
         var account = DevStack.RealmUser;
         var browser = new ScriptedBrowser(account.Username, account.Password);
 
-        // The plaintext exemption is stated here rather than inferred from the address, and that is the
-        // whole point of stating it. ServerConnection allows an http authority only when it is loopback,
-        // which is a sound rule and not one this suite can rely on: Testcontainers reports the host it
-        // can actually be reached at, so a developer running the tests directly gets localhost and passes,
-        // while the same suite inside a container gets the bridge gateway — 172.17.0.1 — and is refused.
-        // That is the product being right. 172.17.0.1 is not loopback, and a client that quietly accepted
-        // plaintext metadata from a routable address would be a real weakness for everyone who is not a
-        // test. So the test says out loud that it accepts plaintext from the throwaway Keycloak it started
-        // itself, and the rule stays as strict as it was for everybody else.
-        using var connection = await ServerConnection.SignInAsync(
-            stack.ApiBaseUrl,
-            browser,
-            TimeProvider.System,
-            Token,
-            configureOidc: options => options with { RequireHttpsMetadata = false });
+        using var connection = await SignInToTheStackAsync(browser);
 
         AssertDiscoveredFromTheServer(connection);
 
@@ -127,6 +113,32 @@ public sealed class M1VerticalSliceTests(DevStack stack) : IClassFixture<DevStac
 
     // ---- Steps ----
 
+    /// <summary>Signs in against the Keycloak this suite started for itself.</summary>
+    /// <remarks>
+    /// <para>
+    /// The plaintext exemption is stated here rather than inherited from the shape of an address, and
+    /// saying it out loud is the point. <see cref="ServerConnection"/> allows an <c>http</c> authority
+    /// only when it is loopback — a sound rule, and not one this suite can lean on. Testcontainers
+    /// reports the host a caller can actually reach it at, so running these tests directly yields
+    /// <c>localhost</c> and passes, while running them inside a container yields the bridge gateway
+    /// <c>172.17.0.1</c> and is refused.
+    /// </para>
+    /// <para>
+    /// That refusal is the product being correct. 172.17.0.1 is genuinely not loopback, and a client
+    /// that quietly accepted plaintext metadata from a routable address would be a real weakness for
+    /// everybody who is not a test. So the exemption is claimed here, by the one caller that knows it
+    /// started the provider itself and that it lives for the length of one test, and the rule stays
+    /// exactly as strict for everyone else.
+    /// </para>
+    /// </remarks>
+    private Task<ServerConnection> SignInToTheStackAsync(ScriptedBrowser browser) =>
+        ServerConnection.SignInAsync(
+            stack.ApiBaseUrl,
+            browser,
+            TimeProvider.System,
+            Token,
+            configureOidc: options => options with { RequireHttpsMetadata = false });
+
     /// <remarks>
     /// The user typed one server URL. Everything about the identity provider — the authority, the client
     /// id, the scopes — came back from the server, which is the whole onboarding story.