Skip to content

test(auth): add auth subsystem black-box test suite - #32

Merged
chadcrum merged 1 commit into
dcm-project:mainfrom
chadcrum:flpath-4482-auth-subsystem-tests
Jul 24, 2026
Merged

test(auth): add auth subsystem black-box test suite#32
chadcrum merged 1 commit into
dcm-project:mainfrom
chadcrum:flpath-4482-auth-subsystem-tests

Conversation

@chadcrum

@chadcrum chadcrum commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Black-box subsystem tests for the auth layer added in #24. Exercises the full authentication middleware against real containers (Keycloak 26, PostgreSQL, control-plane with AUTH_DISABLED=false).

27 Ginkgo test cases covering:

  • Health endpoint auth bypass
  • Proxy secret validation (correct/wrong/missing)
  • JWT precedence when both Bearer and proxy headers present (valid and invalid)
  • Admin actor seeding on startup
  • JIT actor provisioning (preferred username, subject fallback, cache hit)
  • Full OIDC round-trip (Keycloak ROPC -> JWT Bearer -> API access)
  • Client credentials grant (service account JIT provisioning)
  • Actor status enforcement (suspended/deactivated -> 403, reactivation -> 200, with cache TTL wait)
  • RFC 7807 error response format (401/403)
  • Concurrent first-login race condition (10 goroutines, unique constraint handling)

Stack

Docker compose with non-conflicting ports (25432, 28180, 28080) to coexist with dev stacks:

  • quay.io/sclorg/postgresql-16-c9s - test database with direct access for actor status manipulation
  • quay.io/keycloak/keycloak:26.0 - OIDC provider, reuses existing deploy/keycloak/realm-export.json
  • control-plane built from Containerfile with AUTH_CACHE_TTL=2s for testable status changes

CI

Adds auth-subsystem parallel job to .github/workflows/subsystem.yaml using existing black-box.yaml reusable workflow.

Resolves FLPATH-4482

@chadcrum
chadcrum force-pushed the flpath-4482-auth-subsystem-tests branch 2 times, most recently from eeb74c1 to a9cf2c4 Compare July 22, 2026 23:20
@chadcrum
chadcrum marked this pull request as ready for review July 22, 2026 23:32
@chadcrum
chadcrum requested a review from a team as a code owner July 22, 2026 23:32
@chadcrum
chadcrum requested review from gciavarrini, gpb88, hardengl, jenniferubah, jordigilh, machacekondra, testetson22 and y-first and removed request for a team July 22, 2026 23:32
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

test(auth): add black-box auth subsystem test suite (Keycloak/Postgres/compose)

🧪 Tests ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Add Ginkgo black-box tests covering auth middleware, JIT provisioning, and OIDC flows.
• Provide docker-compose stack (Keycloak 26 + Postgres + control-plane) for realistic auth testing.
• Wire a new CI job and Make targets to run the auth subsystem suite in parallel.
Diagram

graph TD
  A["GitHub Actions: subsystem.yaml"] --> B["Reusable workflow: black-box.yaml"] --> C["Make targets: auth-subsystem-test-*" ] --> D["docker-compose: auth stack"] --> E["control-plane (AUTH enabled)"] --> F["Ginkgo suite (subsystem tag)"]
  D --> G[("PostgreSQL test DB")] --> F
  D --> H(["Keycloak 26 OIDC"]) --> F
  subgraph Legend
    direction LR
    _cfg["CI/Config"] ~~~ _svc(["Service"]) ~~~ _db[("Database")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use Testcontainers (Go) instead of docker-compose
  • ➕ Better isolation per test run (ephemeral networks/ports) and reduced port-collision risk
  • ➕ Programmatic readiness checks and logs capture on failure
  • ➕ Potentially easier parallelization/sharding at suite level
  • ➖ More implementation complexity and higher maintenance than a single compose file
  • ➖ Harder to reuse existing shared 'black-box' CI workflow expectations (up/test/down targets)
  • ➖ May increase CI runtime due to per-suite container lifecycle overhead
2. Split into smaller suites (proxy path vs OIDC vs status/race)
  • ➕ Faster feedback by running subsets locally/CI and easier pinpointing of failures
  • ➕ Allows different timeouts/retries per suite (OIDC often needs longer)
  • ➖ More CI wiring and potential duplication of setup helpers
  • ➖ Risk of over-partitioning and increasing overall pipeline time

Recommendation: Current approach (compose-based black-box suite invoked via Make + shared workflow) is the best fit given existing CI patterns and the need to validate real Keycloak/Postgres/control-plane integration. If flakes/port conflicts appear, consider migrating the stack management to Testcontainers or adding suite sharding as a follow-up rather than expanding the compose complexity further.

Files changed (14) +921 / -0

Tests (11) +895 / -0
docker-compose.yamlCompose stack for auth black-box testing +78/-0

Compose stack for auth black-box testing

• Adds a dedicated test stack with Postgres, Keycloak 26 (realm import), and a control-plane build configured with auth enabled and short cache TTL. Exposes non-conflicting ports and includes health checks for service readiness.

test/subsystem/auth/docker-compose.yaml

suite_test.goSubsystem test suite bootstrap and readiness checks +70/-0

Subsystem test suite bootstrap and readiness checks

• Creates the Ginkgo suite entrypoint, configures URLs/secrets via env with defaults, opens a direct Postgres connection, and waits for the API health endpoint before running tests.

test/subsystem/auth/suite_test.go

helpers_test.goKeycloak, HTTP, and DB helper utilities for black-box tests +234/-0

Keycloak, HTTP, and DB helper utilities for black-box tests

• Adds helpers for Keycloak admin/user token flows (ROPC + client credentials), user lifecycle via Keycloak admin API, HTTP request option builders, RFC7807 problem parsing, and DB utilities for actor identity/status assertions.

test/subsystem/auth/helpers_test.go

health_test.goVerify health endpoint bypasses authentication +35/-0

Verify health endpoint bypasses authentication

• Adds tests ensuring /health returns 200 without credentials and remains accessible even with invalid Bearer tokens or proxy secrets.

test/subsystem/auth/health_test.go

proxy_secret_test.goValidate proxy secret auth and credential precedence +75/-0

Validate proxy secret auth and credential precedence

• Covers missing/invalid proxy-secret cases, required subject handling, and identity precedence rules when both Bearer and proxy headers are present (including invalid Bearer overriding valid proxy secret).

test/subsystem/auth/proxy_secret_test.go

admin_seed_test.goAssert admin actor is seeded on startup +28/-0

Assert admin actor is seeded on startup

• Verifies control-plane startup seeds a single active admin actor and binds it to the configured admin subject.

test/subsystem/auth/admin_seed_test.go

jit_test.goTest JIT provisioning via proxy-secret headers +81/-0

Test JIT provisioning via proxy-secret headers

• Ensures first request provisions an actor, preferred username is honored, subject fallback is used when absent, and repeated requests do not create duplicate identities.

test/subsystem/auth/jit_test.go

oidc_test.goTest OIDC round-trip and client credentials JWT path +77/-0

Test OIDC round-trip and client credentials JWT path

• Validates real Keycloak-issued JWTs can authenticate and trigger JIT provisioning, rejects tampered JWTs, and confirms service-account provisioning via client-credentials tokens.

test/subsystem/auth/oidc_test.go

status_test.goEnforce actor status (active/suspended/deactivated) with cache TTL +88/-0

Enforce actor status (active/suspended/deactivated) with cache TTL

• Checks that active actors can access APIs, suspended/deactivated actors receive 403 with appropriate detail, and reactivation restores access after cache expiration delays.

test/subsystem/auth/status_test.go

error_format_test.goValidate RFC 7807 error responses for 401/403 +53/-0

Validate RFC 7807 error responses for 401/403

• Ensures unauthenticated and forbidden responses return application/problem+json with expected fields and details, including status-based denial messaging after actor status changes.

test/subsystem/auth/error_format_test.go

race_test.goCover concurrent JIT provisioning and username collision handling +76/-0

Cover concurrent JIT provisioning and username collision handling

• Verifies concurrent first-login requests for the same subject succeed without creating duplicate identities, and checks username collisions across different subjects result in 409 with a specific problem detail.

test/subsystem/auth/race_test.go

Other (3) +26 / -0
subsystem.yamlAdd auth-subsystem black-box CI job +12/-0

Add auth-subsystem black-box CI job

• Introduces an auth-subsystem job using the shared black-box reusable workflow. Configures make targets for up/test/down and declares required container images (Go toolset, UBI minimal, Postgres 16, Keycloak 26).

.github/workflows/subsystem.yaml

MakefileInclude auth Make targets +1/-0

Include auth Make targets

• Adds inclusion of make/auth.mk so auth subsystem targets are available to CI and local runs.

Makefile

auth.mkAdd auth subsystem up/test/down make targets +13/-0

Add auth subsystem up/test/down make targets

• Defines make targets to start and stop the auth docker-compose stack and run Ginkgo with the subsystem build tag against test/subsystem/auth.

make/auth.mk

@qodo-code-review

qodo-code-review Bot commented Jul 22, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Health poll no timeout ✓ Resolved 🐞 Bug ☼ Reliability
Description
BeforeSuite uses http.Get (default client with no timeout) inside Eventually, so a stuck
connection/DNS resolution can block forever and bypass the 120s Eventually timeout. This can wedge
CI runs indefinitely.
Code

test/subsystem/auth/suite_test.go[R46-56]

+	Eventually(func() error {
+		resp, err := http.Get(apiURL + "/health")
+		if err != nil {
+			return err
+		}
+		defer resp.Body.Close()
+		if resp.StatusCode != http.StatusOK {
+			return fmt.Errorf("health check returned %d", resp.StatusCode)
+		}
+		return nil
+	}).WithTimeout(120 * time.Second).WithPolling(2 * time.Second).Should(Succeed())
Evidence
The code defines a timeout-configured httpClient but uses http.Get in the health polling loop;
http.Get has no client timeout and can block longer than the Eventually timeout window.

test/subsystem/auth/suite_test.go[19-27]
test/subsystem/auth/suite_test.go[46-56]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The suite health readiness loop uses `http.Get`, which uses `http.DefaultClient` with no timeout. If the call blocks, Gomega’s `Eventually` timeout cannot interrupt it.

### Issue Context
The suite already defines `httpClient := &http.Client{Timeout: 10 * time.Second}` but doesn’t use it for the readiness check.

### Fix
Replace `http.Get(...)` with `httpClient.Get(...)`, or build a request with context deadline and execute with `httpClient.Do(req)`.

### Fix Focus Areas
- test/subsystem/auth/suite_test.go[19-27]
- test/subsystem/auth/suite_test.go[46-56]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Compose project collision ✓ Resolved 🐞 Bug ☼ Reliability
Description
auth-subsystem-test-up/down runs docker/podman compose without overriding COMPOSE_PROJECT_NAME,
so it inherits the repo default (control-plane) and shares the same project namespace as the dev
stack. Because both stacks reuse service/volume names (e.g., postgres_data),
auth-subsystem-test-down (down -v) can tear down or delete dev-stack volumes/data when both are
present.
Code

make/auth.mk[R4-8]

+auth-subsystem-test-up:
+	$(COMPOSE) -f test/subsystem/$(AUTH_DOMAIN)/docker-compose.yaml up -d --build
+
+auth-subsystem-test-down:
+	$(COMPOSE) -f test/subsystem/$(AUTH_DOMAIN)/docker-compose.yaml down -v
Evidence
The Makefile exports COMPOSE_PROJECT_NAME with default control-plane, and the new auth targets
do not override it, so compose resources are created in that same project namespace. Both the dev
compose and auth subsystem compose define overlapping service names and a postgres_data volume;
running down -v in the shared project can remove volumes belonging to the other stack.

Makefile[15-33]
make/auth.mk[1-13]
test/subsystem/auth/docker-compose.yaml[4-78]
deploy/compose.yaml[19-94]
deploy/compose.yaml[200-202]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`make auth-subsystem-test-up` / `make auth-subsystem-test-down` uses the globally-exported `COMPOSE_PROJECT_NAME` (defaulting to `control-plane`). Since the auth subsystem compose file reuses common service + volume names (e.g., `control-plane`, `postgres`, `keycloak`, `postgres_data`), it can collide with the dev stack and `down -v` can remove dev volumes.

### Issue Context
The Makefile exports `COMPOSE_PROJECT_NAME`, and the new auth targets don’t override it. Both the dev compose and the auth subsystem compose define `postgres_data`.

### Fix
For the auth subsystem make targets, force an isolated compose project name, e.g.:
- Prefix the compose command with `COMPOSE_PROJECT_NAME=auth-subsystem` (portable across docker-compose / docker compose / podman-compose)
- Or add `-p auth-subsystem` if supported by your compose implementation
Apply this to both `up` and `down` targets.

### Fix Focus Areas
- make/auth.mk[4-8]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Hardcoded Keycloak admin creds ✗ Dismissed 🐞 Bug ≡ Correctness
Description
getKeycloakAdminToken always authenticates using admin/admin, but the auth test compose stack
allows KEYCLOAK_ADMIN_PASSWORD to be overridden; any non-default value will break the test suite.
This reduces portability across environments and makes configuration changes brittle.
Code

test/subsystem/auth/helpers_test.go[R21-29]

+func getKeycloakAdminToken() string {
+	GinkgoHelper()
+	tokenURL := keycloakURL + "/realms/master/protocol/openid-connect/token"
+	resp, err := httpClient.PostForm(tokenURL, url.Values{
+		"grant_type": {"password"},
+		"client_id":  {"admin-cli"},
+		"username":   {"admin"},
+		"password":   {"admin"},
+	})
Evidence
The compose file explicitly supports overriding the Keycloak admin password via env, while the
helper hardcodes admin as the password, creating a direct configuration mismatch.

test/subsystem/auth/docker-compose.yaml[26-33]
test/subsystem/auth/helpers_test.go[21-38]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Keycloak admin credentials are hardcoded in the test helper, but the compose stack supports overriding the admin password.

### Issue Context
Compose uses `${KEYCLOAK_ADMIN_PASSWORD:-admin}`, while `getKeycloakAdminToken()` always posts password `admin`.

### Fix
- Add suite-level variables (e.g., `keycloakAdminUser`, `keycloakAdminPassword`) initialized via `envOrDefault(...)` in `BeforeSuite`.
- Update `getKeycloakAdminToken()` to use those variables instead of hardcoding.

### Fix Focus Areas
- test/subsystem/auth/helpers_test.go[21-38]
- test/subsystem/auth/suite_test.go[34-40]
- test/subsystem/auth/docker-compose.yaml[26-33]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Unpinned CI references ✗ Dismissed 🐞 Bug ⛨ Security
Description
The new auth-subsystem workflow job references a reusable workflow by the mutable @main ref and
pulls multiple container images tagged :latest, which makes CI behavior non-reproducible and
increases supply-chain exposure to upstream changes. A change in the referenced workflow or tags can
break or alter CI without any commit in this repo.
Code

.github/workflows/subsystem.yaml[R10-20]

+  auth-subsystem:
+    uses: dcm-project/shared-workflows/.github/workflows/black-box.yaml@main
+    with:
+      up-target: auth-subsystem-test-up
+      test-target: auth-subsystem-test
+      down-target: auth-subsystem-test-down
+      images: >-
+        registry.access.redhat.com/ubi9/go-toolset:1.25.5
+        registry.access.redhat.com/ubi9/ubi-minimal:latest
+        quay.io/sclorg/postgresql-16-c9s:latest
+        quay.io/keycloak/keycloak:26.0
Evidence
The auth-subsystem job explicitly uses @main for the reusable workflow and includes
ubi-minimal:latest and postgresql-16-c9s:latest in its image list, both of which are mutable
references.

.github/workflows/subsystem.yaml[10-20]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The auth subsystem CI job uses mutable references:
- reusable workflow `.../black-box.yaml@main`
- images with `:latest` tags
This reduces determinism and complicates provenance verification.

### Issue Context
The new job is introduced in `.github/workflows/subsystem.yaml`.

### Fix
- Pin the reusable workflow to an immutable ref (commit SHA or a version tag).
- Replace `:latest` image tags with pinned versions and ideally immutable digests (e.g., `image@sha256:...`).

### Fix Focus Areas
- .github/workflows/subsystem.yaml[10-20]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

5. Sleep-based cache waits ✗ Dismissed 🐞 Bug ☼ Reliability
Description
Several tests use fixed time.Sleep(3 * time.Second) delays to wait for auth cache expiry after DB
status changes, which bakes in timing assumptions and can become flaky or unnecessarily slow under
load. Use Eventually polling for the expected API result instead of fixed sleeps.
Code

test/subsystem/auth/status_test.go[R46-70]

+	It("returns 403 for a suspended actor", func() {
+		updateActorStatus(kcUserID, "suspended")
+		time.Sleep(3 * time.Second)
+
+		resp := doRequest(http.MethodGet, "/catalog-items",
+			withProxySecret(proxySecret, kcUserID))
+		Expect(resp.StatusCode).To(Equal(http.StatusForbidden))
+
+		problem := readProblemResponse(resp)
+		Expect(problem.Detail).To(Equal("account suspended"))
+	})
+
+	It("allows access after reactivating a suspended actor", func() {
+		updateActorStatus(kcUserID, "suspended")
+		time.Sleep(3 * time.Second)
+
+		resp := doRequest(http.MethodGet, "/catalog-items",
+			withProxySecret(proxySecret, kcUserID))
+		Expect(resp.StatusCode).To(Equal(http.StatusForbidden))
+		problem := readProblemResponse(resp)
+		Expect(problem.Detail).To(Equal("account suspended"))
+
+		updateActorStatus(kcUserID, "active")
+		time.Sleep(3 * time.Second)
+
Evidence
time.Sleep(3 * time.Second) is used in multiple places after changing actor status, rather than
polling for the externally observable behavior change.

test/subsystem/auth/status_test.go[46-70]
test/subsystem/auth/error_format_test.go[39-41]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Tests rely on fixed sleeps to wait for cached auth state to refresh. Fixed sleeps slow the suite and can be flaky when the system is slow.

### Issue Context
The stack sets `AUTH_CACHE_TTL=2s`, and tests sleep 3s after status changes.

### Fix
Replace `time.Sleep(...)` with a bounded `Eventually` that repeatedly calls the API until it returns the expected status (403/200), with a reasonable timeout and polling interval.

### Fix Focus Areas
- test/subsystem/auth/status_test.go[46-70]
- test/subsystem/auth/error_format_test.go[39-41]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread make/auth.mk Outdated
Comment thread test/subsystem/auth/suite_test.go
Comment thread test/subsystem/auth/helpers_test.go
Comment thread .github/workflows/subsystem.yaml
Comment thread test/subsystem/auth/status_test.go
Add Ginkgo black-box tests against real Keycloak/PostgreSQL/control-plane
containers covering the auth middleware: proxy-secret validation, JIT
actor provisioning, OIDC JWT round-trips, actor status enforcement,
RFC 7807 error formats, and concurrent first-login races.

Signed-off-by: Chad Crum <1462069+chadcrum@users.noreply.github.com>
Co-Authored-By: Claude <noreply@anthropic.com>
@chadcrum
chadcrum force-pushed the flpath-4482-auth-subsystem-tests branch from 34d0e18 to ad51d8e Compare July 24, 2026 18:27
@chadcrum
chadcrum merged commit 5a0e1f5 into dcm-project:main Jul 24, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants