cschlaefke/Shuffle
1
0
Fork 0
Code Issues 33 Pull requests 2 Projects Releases Packages Wiki Activity Actions

test(auth): KTO-01 Playwright-E2E (api, web, throttle) und CI-Job e2e #42

Merged
cschlaefke merged 9 commits from feature/38-auth-e2e into main 2026-11-03 10:42:02 +00:00
Conversation 2 Commits 9 Files changed 38 +3877 -1
cschlaefke commented 2026-11-03 09:46:00 +00:00
Owner
Copy link

Zusammenfassung

E2E-Prüfung der Akzeptanzkriterien von KTO-01 (#1) gegen echtes GoTrue und die gebaute Flutter-Web-App (T3) plus CI-Job e2e auf docker-host mit Extra-GoTrue-Container für die Drosselung (D2). Kein App-Code geändert.

Closes #38

Spec: docs/specs/1-auth-registrierung-anmeldung.md (§ 7.1, T3, D2)

Umfang

  • e2e/: Playwright-Projekte api (52 Tests), web (21), throttle (4), spike (3, nur lokal, protokolliert SMTP-Ausfall-Verhalten); Hilfen für GoTrue, Mailpit, Datenbank (nur lokale Hosts) und auth-config.ts mit AUTH-CFG-Bezug.
  • .forgejo/workflows/ci.yml: Job e2e (nach supabase-tests), .forgejo/scripts/e2e-run.sh, e2e/support/tcp-forward.mjs (Relay vom dind-Gateway auf 127.0.0.1), e2e/support/start-throttle-gotrue.sh.
  • Doku: docs/auth-konfiguration.md § 7, supabase/README.md.

Abdeckung

Krit. 1–11 sind mit API- und/oder Web-Tests abgedeckt, inkl. Regression AUTH-CFG-24 (Krit. 4), Body-Format der Trigger-Fehler (Krit. 10), Einmaligkeit/Ablauf Reset-Link (Krit. 8), Abmeldung anderer Sitzungen (Krit. 9), echte Drosselung Login #31 -> 429 mit zweiter IP unbeeinflusst (Krit. 7).
Lokal: alle drei Projekte grün, 3 Gesamtläufe in Folge.

Befunde

  • #37 bestätigt: Bei SMTP-Ausfall liefern /recover, /resend, /signup für bekannte und unbekannte Adressen unterschiedliche Status (500 vs. 200). Ergebnis in Issue #37, Entscheidung steht aus.
  • #41 (a11y): Semantics-Baum faltet Titel/Text/Knopf zu einem Knoten (E2E-Hilfe pressFirstButton als Workaround).
  • Spec Krit. 9: expires_at in die Vergangenheit setzen wirkt nicht (Ablauf kommt aus dem JWT-exp); der Test verschiebt die Browser-Uhr um 2 h.

Definition of Done

  • Tests je Kriterium, lokal grün
  • Doku aktualisiert
  • CI-Job e2e auf dem echten Runner grün (erster Lauf ausstehend; nur dort prüfbar: supabase start/db reset gegen dind, Port-Veröffentlichung, supabase status -o env-Schlüsselnamen, Artefakt-Upload, IPv4/IPv6 bei localhost:3000)
  • Android/iOS/echter Screenreader nicht Teil dieses PRs

Risiken

  • Job führt PR-Code mit Zugriff auf den dind-Daemon aus: nur für PRs aus dem eigenen Repo (head.repo.full_name == github.repository). supabase-tests hat dieselbe Lücke (Folge-Issue).
  • e2e und supabase-tests teilen Container-Namen/Ports im dind; parallele Läufe verschiedener PRs können kollidieren.
  • Throttle-Test: Token-Bucket 0,5/s, erste 429 kann bis zu zwei Versuche später kommen; auf CI-Hardware Flakiness beobachten.

So testest du es lokal

supabase start -x studio,imgproxy,edge-runtime,logflare,vector && supabase db reset
cd e2e && npm ci && npx playwright install chromium && npx playwright test --project=api
# web-Projekt: Flutter-Web-Build nach app/build/web, siehe e2e/README.md

🤖 Generated with Claude Code

## Zusammenfassung E2E-Prüfung der Akzeptanzkriterien von KTO-01 (#1) gegen echtes GoTrue und die gebaute Flutter-Web-App (T3) plus CI-Job `e2e` auf `docker-host` mit Extra-GoTrue-Container für die Drosselung (D2). Kein App-Code geändert. Closes #38 Spec: `docs/specs/1-auth-registrierung-anmeldung.md` (§ 7.1, T3, D2) ## Umfang - `e2e/`: Playwright-Projekte `api` (52 Tests), `web` (21), `throttle` (4), `spike` (3, nur lokal, protokolliert SMTP-Ausfall-Verhalten); Hilfen für GoTrue, Mailpit, Datenbank (nur lokale Hosts) und `auth-config.ts` mit AUTH-CFG-Bezug. - `.forgejo/workflows/ci.yml`: Job `e2e` (nach `supabase-tests`), `.forgejo/scripts/e2e-run.sh`, `e2e/support/tcp-forward.mjs` (Relay vom dind-Gateway auf 127.0.0.1), `e2e/support/start-throttle-gotrue.sh`. - Doku: `docs/auth-konfiguration.md` § 7, `supabase/README.md`. ## Abdeckung Krit. 1–11 sind mit API- und/oder Web-Tests abgedeckt, inkl. Regression AUTH-CFG-24 (Krit. 4), Body-Format der Trigger-Fehler (Krit. 10), Einmaligkeit/Ablauf Reset-Link (Krit. 8), Abmeldung anderer Sitzungen (Krit. 9), echte Drosselung Login #31 -> 429 mit zweiter IP unbeeinflusst (Krit. 7). Lokal: alle drei Projekte grün, 3 Gesamtläufe in Folge. ## Befunde - **#37 bestätigt:** Bei SMTP-Ausfall liefern `/recover`, `/resend`, `/signup` für bekannte und unbekannte Adressen unterschiedliche Status (500 vs. 200). Ergebnis in Issue #37, Entscheidung steht aus. - **#41 (a11y):** Semantics-Baum faltet Titel/Text/Knopf zu einem Knoten (E2E-Hilfe `pressFirstButton` als Workaround). - **Spec Krit. 9:** `expires_at` in die Vergangenheit setzen wirkt nicht (Ablauf kommt aus dem JWT-`exp`); der Test verschiebt die Browser-Uhr um 2 h. ## Definition of Done - [x] Tests je Kriterium, lokal grün - [x] Doku aktualisiert - [ ] CI-Job `e2e` auf dem echten Runner grün (**erster Lauf ausstehend**; nur dort prüfbar: `supabase start`/`db reset` gegen dind, Port-Veröffentlichung, `supabase status -o env`-Schlüsselnamen, Artefakt-Upload, IPv4/IPv6 bei `localhost:3000`) - [ ] Android/iOS/echter Screenreader nicht Teil dieses PRs ## Risiken - Job führt PR-Code mit Zugriff auf den dind-Daemon aus: nur für PRs aus dem eigenen Repo (`head.repo.full_name == github.repository`). `supabase-tests` hat dieselbe Lücke (Folge-Issue). - `e2e` und `supabase-tests` teilen Container-Namen/Ports im dind; parallele Läufe verschiedener PRs können kollidieren. - Throttle-Test: Token-Bucket 0,5/s, erste 429 kann bis zu zwei Versuche später kommen; auf CI-Hardware Flakiness beobachten. ## So testest du es lokal ```bash supabase start -x studio,imgproxy,edge-runtime,logflare,vector && supabase db reset cd e2e && npm ci && npx playwright install chromium && npx playwright test --project=api # web-Projekt: Flutter-Web-Build nach app/build/web, siehe e2e/README.md ``` 🤖 Generated with [Claude Code](https://claude.com/claude-code)
cschlaefke added 5 commits 2026-11-03 09:46:01 +00:00 
Refs #38

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Refs #38

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Refs #38

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Refs #38

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
ci(e2e): add Playwright e2e job on docker-host with throttle GoTrue and artifacts
Some checks failed
CI / flutter (pull_request) Successful in 1m15s
Details
CI / web-build (pull_request) Successful in 54s
Details
CI / supabase (pull_request) Successful in 3s
Details
CI / supabase-tests (pull_request) Successful in 30s
Details
CI / e2e (pull_request) Failing after 4m3s
Details
23816377a5 
Refs #38

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
cschlaefke commented 2026-11-03 09:51:48 +00:00
Author
Owner
Copy link

Review (code-reviewer) – PR #42 / Issue #38 (T3 + D2)

Grundlage: git diff main...HEAD (35 Dateien, app/ unverändert → Flutter-Checks nicht betroffen), Spec docs/specs/1-auth-registrierung-anmeldung.md § 7.1, T3/D2.

Selbst ausgeführt (lokal): bash -n für .forgejo/scripts/e2e-run.sh und e2e/support/start-throttle-gotrue.sh ok · node --check e2e/support/tcp-forward.mjs ok · npx tsc --noEmit -p e2e ok · npx playwright test --list: 73 Tests (api 52 + web 21), mit THROTTLE_GOTRUE_URL/SPIKE_GOTRUE_URL 80. Die Tests selbst habe ich nicht gegen einen Stack laufen lassen. shellcheck und actionlint sind lokal nicht installiert.

Secrets: app/env/ci.example.json ist in diesem PR unverändert und enthält nur Platzhalter (public-anon-key-placeholder, öffentliche Legal-URLs, TERMS_VERSION). Die JWTs in e2e/support/auth-config.ts:22-27 sind dekodiert die öffentlichen CLI-Demo-Schlüssel (iss: supabase-demo, role: anon bzw. service_role). Echte Secrets sind nicht im Diff.

Blocker

  1. e2e/support/db.ts:10-15: Der Host-Guard lässt sich umgehen. assertLocal prüft new URL(url).hostname, pg verbindet aber über pg-connection-string, und das wertet den Query-Parameter ?host= aus. Selbst geprüft:

    postgresql://postgres:postgres@127.0.0.1:54322/postgres?host=db.example.com
  new URL().hostname = 127.0.0.1   ->  pg verbindet zu db.example.com

    Damit gilt „läuft nie gegen Nicht-Lokal“ nicht mehr, und der Helfer schreibt in auth.users.
    Vorschlag: den Host aus demselben Parser nehmen, den pg nutzt: import { parse } from 'pg-connection-string'; const cfg = parse(env.dbUrl);. cfg.host gegen die Allowlist prüfen und den Client aus cfg bauen. Alternativ URLs mit Query-Parametern (host, hostaddr) ablehnen.
    Dazu ein Nit: new URL(...).hostname liefert für IPv6 [::1] mit Klammern, der Eintrag '::1' greift also nie. host.docker.internal ist ein DNS-Name; die Begründung als Kommentar ergänzen oder den Eintrag streichen.

  2. e2e/tests/api/krit04-enumeration.spec.ts:23,32: Race durch das globale countUsers(). Mit fullyParallel und 2 (CI) bzw. 4 Workern legen andere Tests (auch das Projekt throttle) zwischen den beiden Zählungen Nutzer an. Der Test wird dann sporadisch rot, und retries: 1 verdeckt das nur.
    Vorschlag: die Absicht der Spec („kein neuer Nutzer“) je Adresse prüfen: select count(*) from auth.users where email = $1 ergibt 1, und es gibt keine Zeile mit id = again.body.id.

Sollte

  1. .forgejo/workflows/ci.yml:197: Der Fork-Guard schützt nicht gegen bösartige Forks (blockiert diesen PR nicht). Bei on: pull_request kommt die Workflow-Datei aus dem PR-Head. Ein Fork kann das if: also einfach entfernen. Außerdem führt supabase-tests Fork-Code ohnehin ungeschützt auf docker-host aus. Dieser PR vergrößert das Risiko nicht, der Kommentar (ci.yml ~189-192) und docs/auth-konfiguration.md § 7 versprechen aber einen Schutz, den es so nicht gibt. Die wirksame Kontrolle liegt in den Repo- bzw. Runner-Einstellungen: Freigabe von Fork-Workflows, oder das Label docker-host nur für vertrauenswürdige Läufe. Vorschlag: Kommentar und Doku entsprechend korrigieren, Folge-Issue für supabase-tests und die Runner-Policy anlegen.
  2. e2e/tests/throttle/krit07-drosselung.spec.ts:75: Login #31 muss hier ohne Toleranz 429 liefern. Test 1 (Z. 42-46) erlaubt zwei Versuche Spielraum, dieser Test keinen. Bei 0,5 Token/s und 30 bcrypt-Logins über Relay und dind (vier Throttle-Tests laufen parallel) füllt der Bucket schnell 1-2 Token nach. Vorschlag: einen gemeinsamen Helfer exhaustUntil429(client, maxExtra) nutzen. Die Toleranz besser aus der gemessenen Dauer ableiten (ceil(elapsed_s * 0.5) + 1) statt fest auf 2 setzen; das gilt auch für Test 1. Ähnlich, aber seltener: Z. 49 (richtiger Login direkt nach der 429 → 429) kann rot werden, wenn der Bucket kurz vor 1 stand.
  3. e2e/tests/web/krit05-sitzung.spec.ts:43-46: Die Gegenprobe prüft nichts. goto('/login') und direkt danach toHaveURL(/\/login$/) ist erfüllt, bevor Flutter überhaupt startet. Vorschlag: erst enableSemantics(page) und expect(getByRole('button', { name: 'Sign in', exact: true })).toBeVisible() abwarten, dann die URL prüfen, so wie in krit09-abmeldung.spec.ts:36-40.
  4. e2e/support/mailpit.ts expectMailCount: Das feste 2-s-Fenster kann unter CI-Last falsch grün sein. Das betrifft die Abwesenheitsnachweise in Krit. 4, 6, 8 und 11. Vorschlag: ein Sentinel-Muster. Danach eine bekannte Mail an dieselbe Adresse auslösen (z. B. recover bei bestätigten Konten), auf sie warten und dann die exakte Gesamtzahl prüfen. Wo das nicht geht (unbekannte Adressen), das Fenster beibehalten und so kommentieren.
  5. Kein Locality-Guard für SUPABASE_URL und MAILPIT_URL. GoTrueClient.createConfirmedUser (e2e/support/gotrue.ts) schickt den Service-Role-Key an /admin/users. Außerdem ist der Demo-Service-Key als Fallback hinterlegt. Zeigt SUPABASE_URL versehentlich auf Staging, werden dort Nutzer angelegt bzw. der Key gesendet. Vorschlag: assertLocal (in korrigierter Form, siehe 1) in einem globalSetup auf alle URLs aus env anwenden, außer throttle/spike mit Allowlist.
  6. e2e/tests/web/krit04-06-07-anmeldung.spec.ts:111 (QUE-06 Krit. 3, „nur per Tastatur“): Das E-Mail-Feld wird per fillField geklickt. Damit ist nicht belegt, dass der Ablauf nur mit der Tastatur geht. Vorschlag: vom Seitenanfang per Tab ins erste Feld, keyboard.type, dann weiter wie bisher.
  7. .forgejo/workflows/ci.yml:227 und :239-240: Die Maskierung kommt zu spät. supabase start gibt die Schlüssel sehr wahrscheinlich unmaskiert ins Log aus, bevor add-mask läuft. Das Risiko ist gering, weil es Demo-Schlüssel sind, aber der Kommentar „nur maskiert weiterreichen“ stimmt so nicht. Vorschlag: Kommentar anpassen oder die Ausgabe von start mit > /dev/null unterdrücken (Fehler dann über status prüfen).
  8. .forgejo/scripts/e2e-run.sh:16-21: Die Warteschleife schlägt nie fehl. Sie prüft nur, ob der lokale Relay lauscht, nicht ob das Ziel erreichbar ist. Vorschlag: je Dienst einen HTTP-Probe durch den Relay (curl -fsS http://127.0.0.1:54321/auth/v1/health -H "apikey: $SUPABASE_ANON_KEY", :9998/health, :54324/api/v1/info) mit || exit 1 nach dem Timeout.
  9. .forgejo/workflows/ci.yml:211: Fehlt xz, steht xz in $missing, und apt-get install xz schlägt fehl, weil das Paket xz-utils heißt. Vorschlag: Werkzeug und Paket getrennt abbilden (xz → xz-utils).
  10. Keine concurrency-Gruppe: Parallele Läufe (zwei PRs, PR plus Push auf main) teilen sich dind, Container-Namen und Ports. start-throttle-gotrue.sh macht docker rm -f shuffle_auth_throttle und kann damit den Container eines anderen Laufs entfernen. supabase stop beendet ebenfalls fremde Stacks. Vorschlag: concurrency: { group: docker-host-dind } für supabase-tests und e2e; ob Forgejo das unterstützt, prüfen. Sonst die Runner-Kapazität auf 1 setzen und das dokumentieren.

Nit

  • e2e/support/web.ts:129-130 und die Tests (krit04-06-07-anmeldung.spec.ts:28,70, krit08-reset.spec.ts:92, krit05-sitzung.spec.ts:27-32): baseURL: 'http://localhost:3000' ist hart kodiert, WEB_URL wird ignoriert. freshContext ist definiert, wird aber nicht benutzt. Bitte vereinheitlichen.
  • e2e/tests/api/config.spec.ts:48-50: Der Titel nennt AUTH-CFG-10, geprüft wird nur sign_in_sign_ups (AUTH-CFG-11). token_refresh = 150 fehlt; daraus ergeben sich die 0,5 Token/s, die der Throttle-Test voraussetzt.
  • e2e/tests/web/krit09-abmeldung.spec.ts:53,57-59: Object.values(...)[0] hängt von der Reihenfolge der Storage-Schlüssel ab (z. B. code-verifier). Die Poll-Bedingung .not.toBe(before) ist auch erfüllt, wenn der Wert undefined ist (Sitzung gelöscht). Bitte zusätzlich auf „truthy“ prüfen. Der Kommentar „Sitzung B: ein zweites Gerät in eigenem Kontext“ (Z. 17) beschreibt nicht, was der Test tut: Es gibt nur einen Browser-Kontext, A ist die API.
  • e2e/support/start-throttle-gotrue.sh:27: grep '^supabase_auth_' | head -n1 nimmt bei mehreren lokalen Projekten einen beliebigen Container. Vorschlag: Standard supabase_auth_shuffle (project_id). API_EXTERNAL_URL wird nicht explizit gesetzt, wie es D2 vorsieht, sondern geerbt. Für throttle ist das unkritisch (keine Mails), sollte aber im Kommentar stehen.
  • e2e/playwright.config.ts:49: reuseExistingServer: true auch in CI. Üblich ist !process.env.CI, damit nie gegen einen fremden oder alten Server getestet wird.
  • BIND_ADDR=0.0.0.0 (ci.yml, Extra-GoTrue): Der Container mit kopierter Env (JWT-Secret, DB-URL, alles Demo-Werte) ist für die Dauer des Jobs auf allen dind-Interfaces erreichbar. Das entspricht der CLI selbst; ein Satz dazu in der Doku genügt.
  • Artefakte (playwright-report, test-results): Traces enthalten den Anon-Key (öffentlich) sowie Tokens und Passwörter der zufälligen Testkonten. add-mask wirkt dort nicht. Das ist unkritisch, sollte aber in der Doku stehen, weil das Repo öffentlich ist.

Positiv

  • Saubere Abdeckung der Matrix 7.1: Regressionstest AUTH-CFG-24 mit identities leer, ID ≠ echte ID, keine Mail und altes Passwort; exaktes Trigger-Body-Format (Krit. 10) inkl. x-sb-error-code; Codepoint-Grenzen mit Emoji; Ablauf über recovery_sent_at/confirmation_sent_at inkl. Grenzfall „60 s vor Ablauf gültig“.
  • Krit. 9 (web): Die Browser-Uhr zu verschieben (shiftBrowserClock) ist eine gute, begründete Lösung, samt Gegenprobe.
  • Krit. 7 web per page.route mit CORS-Header und begründet; der echte Limiter wird separat gegen den Extra-Container getestet; R13 eingehalten (nie X-Forwarded-For gegen die CLI).
  • Mailpit wird nie global geleert, Mails werden je Adresse gefiltert, die Adressen sind zufällig.
  • config.spec.ts bindet auth-config.ts, config.toml und die Doku zusammen.
  • CI: Schlüssel zur Laufzeit, Env-Datei in RUNNER_TEMP statt im Repo, if: always()-Cleanup in der richtigen Reihenfolge (Extra-Container vor supabase stop), trap für den Relay, Artefakte nur bei Fehlern mit 7 Tagen Aufbewahrung.
  • Spike #37: nachvollziehbar dokumentiert, nicht blockierend.

Nur auf dem echten Runner prüfbar

Payload-Feld pull_request.head.repo.full_name in Forgejo und welche Workflow-Datei bei Fork-PRs gilt · Schlüsselnamen in supabase status -o env (CLI 3.0.0: ANON_KEY/SERVICE_ROLE_KEY) · ob supabase start die Schlüssel ins Log schreibt · apt-get und Root für playwright install --with-deps sowie Node und Flutter im docker-host-Image · Port-Veröffentlichung im dind (0.0.0.0:9998, Health über DOCKER_GW) · IPv4/IPv6 bei localhost:3000 (serve) · forgejo/upload-artifact@v4 · Timing der Drosselung auf CI-Hardware · Kollisionen zwischen Läufen und ob Forgejo concurrency unterstützt · Cleanup bei Abbruch (Cancel).

Fazit: CHANGES REQUESTED. Grund sind Blocker 1 (Host-Guard in db.ts) und Blocker 2 (Race beim globalen Nutzerzähler in Krit. 4). Der Merge bleibt bei Chris.

## Review (code-reviewer) – PR #42 / Issue #38 (T3 + D2) Grundlage: `git diff main...HEAD` (35 Dateien, `app/` unverändert → Flutter-Checks nicht betroffen), Spec `docs/specs/1-auth-registrierung-anmeldung.md` § 7.1, T3/D2. **Selbst ausgeführt (lokal):** `bash -n` für `.forgejo/scripts/e2e-run.sh` und `e2e/support/start-throttle-gotrue.sh` ok · `node --check e2e/support/tcp-forward.mjs` ok · `npx tsc --noEmit -p e2e` ok · `npx playwright test --list`: 73 Tests (api 52 + web 21), mit `THROTTLE_GOTRUE_URL`/`SPIKE_GOTRUE_URL` 80. Die Tests selbst habe ich nicht gegen einen Stack laufen lassen. shellcheck und actionlint sind lokal nicht installiert. **Secrets:** `app/env/ci.example.json` ist in diesem PR unverändert und enthält nur Platzhalter (`public-anon-key-placeholder`, öffentliche Legal-URLs, `TERMS_VERSION`). Die JWTs in `e2e/support/auth-config.ts:22-27` sind dekodiert die öffentlichen CLI-Demo-Schlüssel (`iss: supabase-demo`, `role: anon` bzw. `service_role`). Echte Secrets sind nicht im Diff. --- ### Blocker 1. **`e2e/support/db.ts:10-15`: Der Host-Guard lässt sich umgehen.** `assertLocal` prüft `new URL(url).hostname`, `pg` verbindet aber über `pg-connection-string`, und das wertet den Query-Parameter `?host=` aus. Selbst geprüft: ``` postgresql://postgres:postgres@127.0.0.1:54322/postgres?host=db.example.com new URL().hostname = 127.0.0.1 -> pg verbindet zu db.example.com ``` Damit gilt „läuft nie gegen Nicht-Lokal“ nicht mehr, und der Helfer schreibt in `auth.users`. **Vorschlag:** den Host aus demselben Parser nehmen, den `pg` nutzt: `import { parse } from 'pg-connection-string'; const cfg = parse(env.dbUrl);`. `cfg.host` gegen die Allowlist prüfen und den `Client` aus `cfg` bauen. Alternativ URLs mit Query-Parametern (`host`, `hostaddr`) ablehnen. Dazu ein Nit: `new URL(...).hostname` liefert für IPv6 `[::1]` mit Klammern, der Eintrag `'::1'` greift also nie. `host.docker.internal` ist ein DNS-Name; die Begründung als Kommentar ergänzen oder den Eintrag streichen. 2. **`e2e/tests/api/krit04-enumeration.spec.ts:23,32`: Race durch das globale `countUsers()`.** Mit `fullyParallel` und 2 (CI) bzw. 4 Workern legen andere Tests (auch das Projekt `throttle`) zwischen den beiden Zählungen Nutzer an. Der Test wird dann sporadisch rot, und `retries: 1` verdeckt das nur. **Vorschlag:** die Absicht der Spec („kein neuer Nutzer“) je Adresse prüfen: `select count(*) from auth.users where email = $1` ergibt 1, und es gibt keine Zeile mit `id = again.body.id`. ### Sollte 3. **`.forgejo/workflows/ci.yml:197`: Der Fork-Guard schützt nicht gegen bösartige Forks (blockiert diesen PR nicht).** Bei `on: pull_request` kommt die Workflow-Datei aus dem PR-Head. Ein Fork kann das `if:` also einfach entfernen. Außerdem führt `supabase-tests` Fork-Code ohnehin ungeschützt auf `docker-host` aus. Dieser PR vergrößert das Risiko nicht, der Kommentar (ci.yml ~189-192) und `docs/auth-konfiguration.md` § 7 versprechen aber einen Schutz, den es so nicht gibt. Die wirksame Kontrolle liegt in den Repo- bzw. Runner-Einstellungen: Freigabe von Fork-Workflows, oder das Label `docker-host` nur für vertrauenswürdige Läufe. **Vorschlag:** Kommentar und Doku entsprechend korrigieren, Folge-Issue für `supabase-tests` und die Runner-Policy anlegen. 4. **`e2e/tests/throttle/krit07-drosselung.spec.ts:75`: Login #31 muss hier ohne Toleranz 429 liefern.** Test 1 (Z. 42-46) erlaubt zwei Versuche Spielraum, dieser Test keinen. Bei 0,5 Token/s und 30 bcrypt-Logins über Relay und dind (vier Throttle-Tests laufen parallel) füllt der Bucket schnell 1-2 Token nach. **Vorschlag:** einen gemeinsamen Helfer `exhaustUntil429(client, maxExtra)` nutzen. Die Toleranz besser aus der gemessenen Dauer ableiten (`ceil(elapsed_s * 0.5) + 1`) statt fest auf 2 setzen; das gilt auch für Test 1. Ähnlich, aber seltener: Z. 49 (richtiger Login direkt nach der 429 → 429) kann rot werden, wenn der Bucket kurz vor 1 stand. 5. **`e2e/tests/web/krit05-sitzung.spec.ts:43-46`: Die Gegenprobe prüft nichts.** `goto('/login')` und direkt danach `toHaveURL(/\/login$/)` ist erfüllt, bevor Flutter überhaupt startet. **Vorschlag:** erst `enableSemantics(page)` und `expect(getByRole('button', { name: 'Sign in', exact: true })).toBeVisible()` abwarten, dann die URL prüfen, so wie in `krit09-abmeldung.spec.ts:36-40`. 6. **`e2e/support/mailpit.ts` `expectMailCount`: Das feste 2-s-Fenster kann unter CI-Last falsch grün sein.** Das betrifft die Abwesenheitsnachweise in Krit. 4, 6, 8 und 11. **Vorschlag:** ein Sentinel-Muster. Danach eine bekannte Mail an dieselbe Adresse auslösen (z. B. `recover` bei bestätigten Konten), auf sie warten und dann die exakte Gesamtzahl prüfen. Wo das nicht geht (unbekannte Adressen), das Fenster beibehalten und so kommentieren. 7. **Kein Locality-Guard für `SUPABASE_URL` und `MAILPIT_URL`.** `GoTrueClient.createConfirmedUser` (`e2e/support/gotrue.ts`) schickt den Service-Role-Key an `/admin/users`. Außerdem ist der Demo-Service-Key als Fallback hinterlegt. Zeigt `SUPABASE_URL` versehentlich auf Staging, werden dort Nutzer angelegt bzw. der Key gesendet. **Vorschlag:** `assertLocal` (in korrigierter Form, siehe 1) in einem `globalSetup` auf alle URLs aus `env` anwenden, außer `throttle`/`spike` mit Allowlist. 8. **`e2e/tests/web/krit04-06-07-anmeldung.spec.ts:111` (QUE-06 Krit. 3, „nur per Tastatur“):** Das E-Mail-Feld wird per `fillField` geklickt. Damit ist nicht belegt, dass der Ablauf nur mit der Tastatur geht. **Vorschlag:** vom Seitenanfang per `Tab` ins erste Feld, `keyboard.type`, dann weiter wie bisher. 9. **`.forgejo/workflows/ci.yml:227` und `:239-240`: Die Maskierung kommt zu spät.** `supabase start` gibt die Schlüssel sehr wahrscheinlich unmaskiert ins Log aus, bevor `add-mask` läuft. Das Risiko ist gering, weil es Demo-Schlüssel sind, aber der Kommentar „nur maskiert weiterreichen“ stimmt so nicht. **Vorschlag:** Kommentar anpassen oder die Ausgabe von `start` mit `> /dev/null` unterdrücken (Fehler dann über `status` prüfen). 10. **`.forgejo/scripts/e2e-run.sh:16-21`: Die Warteschleife schlägt nie fehl.** Sie prüft nur, ob der lokale Relay lauscht, nicht ob das Ziel erreichbar ist. **Vorschlag:** je Dienst einen HTTP-Probe durch den Relay (`curl -fsS http://127.0.0.1:54321/auth/v1/health -H "apikey: $SUPABASE_ANON_KEY"`, `:9998/health`, `:54324/api/v1/info`) mit `|| exit 1` nach dem Timeout. 11. **`.forgejo/workflows/ci.yml:211`:** Fehlt `xz`, steht `xz` in `$missing`, und `apt-get install xz` schlägt fehl, weil das Paket `xz-utils` heißt. **Vorschlag:** Werkzeug und Paket getrennt abbilden (`xz` → `xz-utils`). 12. **Keine `concurrency`-Gruppe:** Parallele Läufe (zwei PRs, PR plus Push auf main) teilen sich dind, Container-Namen und Ports. `start-throttle-gotrue.sh` macht `docker rm -f shuffle_auth_throttle` und kann damit den Container eines anderen Laufs entfernen. `supabase stop` beendet ebenfalls fremde Stacks. **Vorschlag:** `concurrency: { group: docker-host-dind }` für `supabase-tests` und `e2e`; ob Forgejo das unterstützt, prüfen. Sonst die Runner-Kapazität auf 1 setzen und das dokumentieren. ### Nit - `e2e/support/web.ts:129-130` und die Tests (`krit04-06-07-anmeldung.spec.ts:28,70`, `krit08-reset.spec.ts:92`, `krit05-sitzung.spec.ts:27-32`): `baseURL: 'http://localhost:3000'` ist hart kodiert, `WEB_URL` wird ignoriert. `freshContext` ist definiert, wird aber nicht benutzt. Bitte vereinheitlichen. - `e2e/tests/api/config.spec.ts:48-50`: Der Titel nennt AUTH-CFG-10, geprüft wird nur `sign_in_sign_ups` (AUTH-CFG-11). `token_refresh = 150` fehlt; daraus ergeben sich die 0,5 Token/s, die der Throttle-Test voraussetzt. - `e2e/tests/web/krit09-abmeldung.spec.ts:53,57-59`: `Object.values(...)[0]` hängt von der Reihenfolge der Storage-Schlüssel ab (z. B. code-verifier). Die Poll-Bedingung `.not.toBe(before)` ist auch erfüllt, wenn der Wert `undefined` ist (Sitzung gelöscht). Bitte zusätzlich auf „truthy“ prüfen. Der Kommentar „Sitzung B: ein zweites Gerät in eigenem Kontext“ (Z. 17) beschreibt nicht, was der Test tut: Es gibt nur einen Browser-Kontext, A ist die API. - `e2e/support/start-throttle-gotrue.sh:27`: `grep '^supabase_auth_' | head -n1` nimmt bei mehreren lokalen Projekten einen beliebigen Container. **Vorschlag:** Standard `supabase_auth_shuffle` (project_id). `API_EXTERNAL_URL` wird nicht explizit gesetzt, wie es D2 vorsieht, sondern geerbt. Für `throttle` ist das unkritisch (keine Mails), sollte aber im Kommentar stehen. - `e2e/playwright.config.ts:49`: `reuseExistingServer: true` auch in CI. Üblich ist `!process.env.CI`, damit nie gegen einen fremden oder alten Server getestet wird. - `BIND_ADDR=0.0.0.0` (ci.yml, Extra-GoTrue): Der Container mit kopierter Env (JWT-Secret, DB-URL, alles Demo-Werte) ist für die Dauer des Jobs auf allen dind-Interfaces erreichbar. Das entspricht der CLI selbst; ein Satz dazu in der Doku genügt. - Artefakte (`playwright-report`, `test-results`): Traces enthalten den Anon-Key (öffentlich) sowie Tokens und Passwörter der zufälligen Testkonten. `add-mask` wirkt dort nicht. Das ist unkritisch, sollte aber in der Doku stehen, weil das Repo öffentlich ist. ### Positiv - Saubere Abdeckung der Matrix 7.1: Regressionstest AUTH-CFG-24 mit `identities` leer, ID ≠ echte ID, keine Mail und altes Passwort; exaktes Trigger-Body-Format (Krit. 10) inkl. `x-sb-error-code`; Codepoint-Grenzen mit Emoji; Ablauf über `recovery_sent_at`/`confirmation_sent_at` inkl. Grenzfall „60 s vor Ablauf gültig“. - Krit. 9 (web): Die Browser-Uhr zu verschieben (`shiftBrowserClock`) ist eine gute, begründete Lösung, samt Gegenprobe. - Krit. 7 web per `page.route` mit CORS-Header und begründet; der echte Limiter wird separat gegen den Extra-Container getestet; R13 eingehalten (nie `X-Forwarded-For` gegen die CLI). - Mailpit wird nie global geleert, Mails werden je Adresse gefiltert, die Adressen sind zufällig. - `config.spec.ts` bindet `auth-config.ts`, `config.toml` und die Doku zusammen. - CI: Schlüssel zur Laufzeit, Env-Datei in `RUNNER_TEMP` statt im Repo, `if: always()`-Cleanup in der richtigen Reihenfolge (Extra-Container vor `supabase stop`), `trap` für den Relay, Artefakte nur bei Fehlern mit 7 Tagen Aufbewahrung. - Spike #37: nachvollziehbar dokumentiert, nicht blockierend. ### Nur auf dem echten Runner prüfbar Payload-Feld `pull_request.head.repo.full_name` in Forgejo und welche Workflow-Datei bei Fork-PRs gilt · Schlüsselnamen in `supabase status -o env` (CLI 3.0.0: `ANON_KEY`/`SERVICE_ROLE_KEY`) · ob `supabase start` die Schlüssel ins Log schreibt · `apt-get` und Root für `playwright install --with-deps` sowie Node und Flutter im `docker-host`-Image · Port-Veröffentlichung im dind (`0.0.0.0:9998`, Health über `DOCKER_GW`) · IPv4/IPv6 bei `localhost:3000` (`serve`) · `forgejo/upload-artifact@v4` · Timing der Drosselung auf CI-Hardware · Kollisionen zwischen Läufen und ob Forgejo `concurrency` unterstützt · Cleanup bei Abbruch (Cancel). --- **Fazit: CHANGES REQUESTED.** Grund sind Blocker 1 (Host-Guard in `db.ts`) und Blocker 2 (Race beim globalen Nutzerzähler in Krit. 4). Der Merge bleibt bei Chris.
cschlaefke added 2 commits 2026-11-03 09:55:52 +00:00 
Refs #38

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
test(auth): address review findings for e2e
Some checks failed
CI / flutter (pull_request) Successful in 1m15s
Details
CI / web-build (pull_request) Successful in 53s
Details
CI / supabase (pull_request) Successful in 3s
Details
CI / supabase-tests (pull_request) Successful in 30s
Details
CI / e2e (pull_request) Failing after 3m56s
Details
aa421b12b0 
Refs #38

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
cschlaefke added 1 commit 2026-11-03 10:11:19 +00:00
ci(e2e): provide mail templates under remote docker daemon
Some checks failed
CI / flutter (pull_request) Successful in 1m12s
Details
CI / web-build (pull_request) Successful in 53s
Details
CI / supabase (pull_request) Successful in 3s
Details
CI / supabase-tests (pull_request) Successful in 30s
Details
CI / e2e (pull_request) Failing after 53s
Details
354fdd4e41 
Refs #38

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
cschlaefke added 1 commit 2026-11-03 10:17:44 +00:00
ci(e2e): seed mail templates in dind before supabase start
All checks were successful
CI / flutter (pull_request) Successful in 1m15s
Details
CI / web-build (pull_request) Successful in 56s
Details
CI / supabase (pull_request) Successful in 3s
Details
CI / supabase-tests (pull_request) Successful in 31s
Details
CI / e2e (pull_request) Successful in 4m34s
Details
15ccae2b52 
Refs #38

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
cschlaefke added the
status/ready-for-merge
 label 2026-11-03 10:40:40 +00:00
cschlaefke commented 2026-11-03 10:40:46 +00:00
Author
Owner
Copy link

CI-Ergebnis (Commit 15ccae2)

Alle 5 Jobs grün: flutter (1m15s), web-build (56s), supabase (3s), supabase-tests (31s), e2e (4m34s). Im E2E-Lauf auf dem Runner: 84 Tests bestanden (api, web, throttle), keine Wiederholungen, keine Ausfälle.

Erkenntnisse aus den drei CI-Läufen

  1. Erster Lauf: 25 Tests rot, alle mit derselben Ursache. Die Mailvorlagen (supabase/templates/*.html) fehlten im Docker-in-Docker-Daemon (Bind-Mount auf einen Pfad des Job-Containers). Die CLI liefert die Vorlagen über Kong aus; GoTrue verschickte dadurch ein Verzeichnislisting statt der Vorlage, der Betreff kam weiter aus config.toml. Das ist der stille Fallback R5 der Spec, hier real aufgetreten.
  2. Zweiter Lauf: nachträgliches Ersetzen der Verzeichnisse + Neustart von Kong scheiterte (Mount-Ziel im Container ist ein Verzeichnis).
  3. Dritter Lauf: Vorlagen werden vor supabase start im dind angelegt (.forgejo/scripts/remote-templates.sh seed) und nach db reset geprüft (verify). Damit grün.

Review

code-reviewer: 2 Blocker (Host-Guard in db.ts umgehbar, Race im Krit.-4-Test) und wichtige Punkte behoben (aa421b1, da8305c). Fork-PR-Risiko des docker-host-Runners: Folge-Issue #43. Spec Krit. 9 (Rezept mit expires_at) korrigiert.

Offen / Hinweise

  • Befund #37 (Konten-Aufdeckung bei SMTP-Ausfall) bestätigt, Entscheidung durch Chris steht aus. Befund #41 (a11y, Semantics-Baum).
  • concurrency-Gruppe auf Job-Ebene: Wirkung bei Forgejo 8.0.3 / Runner v5.0.4 ungeprüft; parallele PR-Läufe könnten kollidieren.
  • Nicht getestet: Android/iOS, echter Screenreader.
  • Für die Produktion: Vorlagen-Auslieferung per URL zuverlässig bereitstellen und per Testmail prüfen (D1), sonst greift der stille Fallback.

Merge durch Chris. Mit dem Merge ist Issue #38 geschlossen.

## CI-Ergebnis (Commit `15ccae2`) Alle 5 Jobs grün: `flutter` (1m15s), `web-build` (56s), `supabase` (3s), `supabase-tests` (31s), **`e2e` (4m34s)**. Im E2E-Lauf auf dem Runner: **84 Tests bestanden** (api, web, throttle), keine Wiederholungen, keine Ausfälle. ## Erkenntnisse aus den drei CI-Läufen 1. Erster Lauf: 25 Tests rot, alle mit derselben Ursache. Die Mailvorlagen (`supabase/templates/*.html`) fehlten im Docker-in-Docker-Daemon (Bind-Mount auf einen Pfad des Job-Containers). Die CLI liefert die Vorlagen über Kong aus; GoTrue verschickte dadurch ein Verzeichnislisting statt der Vorlage, der Betreff kam weiter aus `config.toml`. Das ist der stille Fallback R5 der Spec, hier real aufgetreten. 2. Zweiter Lauf: nachträgliches Ersetzen der Verzeichnisse + Neustart von Kong scheiterte (Mount-Ziel im Container ist ein Verzeichnis). 3. Dritter Lauf: Vorlagen werden **vor** `supabase start` im dind angelegt (`.forgejo/scripts/remote-templates.sh seed`) und nach `db reset` geprüft (`verify`). Damit grün. ## Review `code-reviewer`: 2 Blocker (Host-Guard in `db.ts` umgehbar, Race im Krit.-4-Test) und wichtige Punkte behoben (`aa421b1`, `da8305c`). Fork-PR-Risiko des `docker-host`-Runners: Folge-Issue #43. Spec Krit. 9 (Rezept mit `expires_at`) korrigiert. ## Offen / Hinweise - Befund #37 (Konten-Aufdeckung bei SMTP-Ausfall) bestätigt, Entscheidung durch Chris steht aus. Befund #41 (a11y, Semantics-Baum). - `concurrency`-Gruppe auf Job-Ebene: Wirkung bei Forgejo 8.0.3 / Runner v5.0.4 ungeprüft; parallele PR-Läufe könnten kollidieren. - Nicht getestet: Android/iOS, echter Screenreader. - Für die Produktion: Vorlagen-Auslieferung per URL zuverlässig bereitstellen und per Testmail prüfen (D1), sonst greift der stille Fallback. Merge durch Chris. Mit dem Merge ist Issue #38 geschlossen.
cschlaefke merged commit 0f7eee80a9 into main 2026-11-03 10:42:02 +00:00
cschlaefke deleted branch feature/38-auth-e2e 2026-11-03 10:42:02 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
epic/katalog

Epic KAT: Spielekatalog

  
epic/konto

Epic KTO: Konto & Profil

  
epic/partien

Epic PAR: Partien & Statistik

  
epic/querschnitt

Epic QUE: Querschnitt

  
epic/sammlung

Epic SAM: Sammlung & Listen

  
needs-clarification

Fachliche Frage offen

  
prio/could

könnte

  
prio/must

MVP – muss

  
prio/should

sollte

  
size/l

3 Tage

  
size/m

1–3 Tage

  
size/s

< 1 Tag

  
size/xl

Mehr als 3 Tage

  
status/in-progress

In Umsetzung

  
status/ready-for-merge

Review bestanden, Merge durch Chris

  
status/spec-review

Spec wartet auf Freigabe

  
type/bug

Fehler

  
type/epic

Epic

  
type/story

Fachliche User Story

  
type/tech

Technische Aufgabe

No labels
epic/katalog
epic/konto
epic/partien
epic/querschnitt
epic/sammlung
needs-clarification
prio/could
prio/must
prio/should
size/l
size/m
size/s
size/xl
status/in-progress
status/ready-for-merge
status/spec-review
type/bug
type/epic
type/story
type/tech
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: cschlaefke/Shuffle#42
No description provided.
Delete branch "feature/38-auth-e2e"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?