Public Access
Move the plaintext exemption out of the test body it made too long
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>
This commit is contained in:
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user