feat(auth): retire x-user-id dev shim — jwt is the only auth path - #17
Merged
Conversation
Phase 1.d.2. Better Auth is deployed (waveflow-web 1.c.2c) and lazy auto-provisioning landed (PR #16), so the legacy x-user-id header shim has no remaining job. This commit deletes it end-to-end. Production code: - config: drop dev_auth_enabled + auth_disabled_at_boot. The WAVEFLOW_JWT_* triple is now required at boot — a partial / missing config fails fast instead of silently 503-ing every request. - AppState: jwt_verifier becomes Arc<JwtVerifier> (no Option), reflecting the boot guarantee. - middleware: drop require_user_id, parse_x_user_id_header, X_USER_ID_HEADER + the shim-fallback branch in authenticate. Single flow: missing bearer → 401, bad token → 401, valid token → lazy-provision + UserId extension. - api/users.rs: deleted entirely. POST /api/v1/users had only ever existed to seed the shim; lazy provision now lands the row on first authenticated request. - api/mod.rs: drop users module + reject_dev_auth_disabled helper. Every /api/v1/* gets the single authenticate layer. - api/{libraries,playlists,profiles,tracks}.rs: openapi header params switch from x-user-id to Authorization Bearer. - db::users: drop the now-unused create helper. Migration: - 20260531000000_users_external_id_not_null.sql: deletes any rows with NULL external_id (dev artifacts that no JWT could ever authenticate against) then ALTER COLUMN ... SET NOT NULL. Tests: - support.rs: rebuilt around JwksHarness. spawn_app stays as a no-auth-needed helper for probe tests; new spawn_authenticated + spawn_two_authenticated bundle harness + bearer mint + lazy-provision warm-up + users.id lookup. - jwt_middleware.rs: drop the *_with_shim variants + the prod-gate 503 test (the boot fail-fast covers that case now). - profiles / libraries / tracks / playlists: every test that used .header("x-user-id", ...) + mint_user(POST /users) now uses spawn_authenticated + .bearer_auth. Tenancy tests share one app instance + one JWKS harness for both callers. - openapi.rs: assert the spec no longer advertises /api/v1/users. Docs: - README + CLAUDE.md: bullet rewritten to describe JWT-only. - .env.example: drop WAVEFLOW_DEV_AUTH; add the JWT triple with the default Better Auth dev URLs. Net -678 LOC. cargo check + cargo fmt clean locally; integration tests need a live Postgres so CI will validate them. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughLe PR implémente Phase 1.d.2 : suppression du shim dev ChangesJWT-Only Authentication Migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/support.rs (1)
73-83:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAjouter une barrière de disponibilité avant de retourner l’URL
Line 73 lance le serveur dans une tâche, puis Line 82 renvoie l’URL immédiatement. Les appels faits juste après peuvent partir avant que le listener accepte des connexions, ce qui introduit de la flakiness (connection refused) dans les tests d’intégration.
💡 Correctif proposé
tokio::spawn(async move { axum::serve( listener, app(config, state).into_make_service_with_connect_info::<SocketAddr>(), ) .await .unwrap(); }); - format!("http://{addr}") + let base = format!("http://{addr}"); + let client = reqwest::Client::new(); + for _ in 0..50 { + match client.get(format!("{base}/health")).send().await { + Ok(resp) if resp.status().is_success() => return base, + _ => tokio::time::sleep(std::time::Duration::from_millis(20)).await, + } + } + panic!("test server not ready: {base}"); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/support.rs` around lines 73 - 83, Le serveur est lancé de façon asynchrone avec tokio::spawn en appelant axum::serve(app(config, state)...), puis l’URL est retournée immédiatement via format!("http://{addr}"), ce qui provoque des erreurs de connexion racey; fixez-le en ajoutant une disponibilité (readiness) : créez un signal (oneshot channel ou tokio::sync::Notify) que la tâche spawnée émet une fois que le listener/serveur est prêt (par ex. après le bind/accept ou juste avant d’appeler .await sur axum::serve), et attendez ce signal dans la fonction appelante avant de retourner format!("http://{addr}"), en modifiant la closure où axum::serve est appelée et le point qui retourne l’URL pour utiliser ce signal.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/api/profiles.rs`:
- Around line 129-130: La réponse 409 dans create_profile contient encore la
mention obsolète "X-User-Id" dans sa description et/ou dans le body retourné;
remplacez toute occurrence par une formulation correcte relative à
l'authentification JWT (p.ex. "user id from JWT / authenticated user id") ou
supprimez la référence si inutile, et mettez à jour la description et le payload
retourné dans le décorateur/annotation de response (la déclaration 409 autour de
create_profile et les blocs similaires autour des lignes 149-159) pour qu'ils
reflètent le contrat JWT-only.
---
Outside diff comments:
In `@tests/support.rs`:
- Around line 73-83: Le serveur est lancé de façon asynchrone avec tokio::spawn
en appelant axum::serve(app(config, state)...), puis l’URL est retournée
immédiatement via format!("http://{addr}"), ce qui provoque des erreurs de
connexion racey; fixez-le en ajoutant une disponibilité (readiness) : créez un
signal (oneshot channel ou tokio::sync::Notify) que la tâche spawnée émet une
fois que le listener/serveur est prêt (par ex. après le bind/accept ou juste
avant d’appeler .await sur axum::serve), et attendez ce signal dans la fonction
appelante avant de retourner format!("http://{addr}"), en modifiant la closure
où axum::serve est appelée et le point qui retourne l’URL pour utiliser ce
signal.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1552f142-6655-4b68-8a63-c96c82c0f901
📒 Files selected for processing (22)
.env.exampleCLAUDE.mdREADME.mdmigrations/20260531000000_users_external_id_not_null.sqlsrc/api/libraries.rssrc/api/mod.rssrc/api/playlists.rssrc/api/profiles.rssrc/api/tracks.rssrc/api/users.rssrc/config.rssrc/db.rssrc/lib.rssrc/main.rssrc/middleware.rstests/jwt_middleware.rstests/libraries.rstests/openapi.rstests/playlists.rstests/profiles.rstests/support.rstests/tracks.rs
💤 Files with no reviewable changes (1)
- src/api/users.rs
… handler CI surfaced two leftovers from PR #17: - tests/ready.rs::users_external_id_check_rejects_blank asserted NULL external_id is allowed, but the 1.d.2 migration set the column NOT NULL. Rename to users_external_id_rejects_null_and_blank and assert NULL trips SQLSTATE 23502 (not_null_violation) on top of the existing blank-string CHECK case. - src/api/profiles.rs create_profile: openapi 409 description + inline comment + response body all still referenced 'X-User-Id' by name. Rewrite to mention the authenticated user id resolved by the middleware. 409 stays defensive (race between the middleware's lazy-provision and a hypothetical concurrent users row delete; unreachable today but cheap to keep). Skipped: CR's outside-diff finding on tests/support.rs about spawning axum::serve without a readiness signal. TcpListener::bind returns once the socket is in LISTEN state, so the kernel queues incoming SYN packets in the accept backlog — a client connect() succeeds even before axum::serve reaches accept(). No race-induced failure (just a few ms of latency); CI run on PR #17 was clean on the integration suite, and a oneshot signal would add code for a problem TCP semantics already solve. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
This was referenced May 31, 2026
This was referenced Jun 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 1.d.2. Better Auth is deployed (
waveflow-web1.c.2c) and lazy auto-provisioning landed (#16 +waveflow-web1.c.3b), so the legacyX-User-Idheader shim has no remaining job. This commit deletes it end-to-end.Net diff: −678 LOC.
Production code
config: dropdev_auth_enabled+auth_disabled_at_boot. JWT triple required at boot.AppState:jwt_verifierbecomesArc<JwtVerifier>(noOption).middleware: droprequire_user_id,parse_x_user_id_header,X_USER_ID_HEADER+ the shim-fallback branch inauthenticate.UserIdextension.src/api/users.rsdeleted entirely.POST /api/v1/usersonly existed to seed the shim. Lazy provision lands the row on first authenticated request.api/mod.rs: dropusersmodule +reject_dev_auth_disabledhelper./api/v1/*gets the singleauthenticatelayer — no forks.api/{libraries,playlists,profiles,tracks}.rs: openapi headers switch fromx-user-idtoAuthorization Bearer.db::users: drop the now-unusedcreatehelper.POST /usershandler.Migration
20260531000000_users_external_id_not_null.sql—DELETE FROM users WHERE external_id IS NULL(dev artifacts that no JWT could ever authenticate against), thenALTER COLUMN external_id SET NOT NULL. Safe because the server hasn't deployed yet — no production rows to backfill.Tests
tests/support.rsJwksHarness.spawn_appstays as a no-auth-needed helper for probe tests. Newspawn_authenticated+spawn_two_authenticatedbundle harness + bearer mint + lazy-provision warm-up +users.idlookup, so a test bootstrap is one line.tests/jwt_middleware.rs*_with_shimvariants + the prod-gate 503 test (the boot fail-fast covers that case now).tests/profiles.rs/libraries.rs/tracks.rs/playlists.rs.header("x-user-id", …)+mint_user(POST /users)rewrote to.bearer_auth(&auth.token)+spawn_authenticated. Tenancy tests share one app + one JWKS harness for both callers.tests/openapi.rs/api/v1/users.Docs
README.md+CLAUDE.md— bullets rewritten to describe JWT-only..env.example— dropWAVEFLOW_DEV_AUTH; add the requiredWAVEFLOW_JWT_JWKS_URL/_ISSUER/_AUDIENCEtriple with the Better Auth dev URLs.Test plan
cargo check --workspace --all-targets— cleancargo fmt --all --check— cleancargo test— integration tests need a live Postgres so CI validatesManual smoke after merge:
WAVEFLOW_JWT_*unset →cargo runpanics with the missing-knob messageWAVEFLOW_JWT_*set → boot OK,/api/v1/profilesreturns 401 with no Bearer, 200 + empty list with a valid one minted off the matching Better Auth instanceWhat this is NOT
auth::JwtVerifieretc.). Those tests are unchanged.jsonwebtokenor any JWKS code — that's the only auth code path now./healthor/ready(still unauthenticated probes).Summary by CodeRabbit
Authentification
Changements de rupture
Base de données
Documentation
Tests