chore: API export tests, auth log gating, Makefile hooks dedupe #35
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| -- 027_fix_superadmin_baseline.sql | ||
| -- Purpose: Normalize superadmin baseline so login and permissions behave consistently. | ||
| -- - Ensure canonical email/username/full_name | ||
| -- - Ensure family, membership, current_family_id, and default ledger exist | ||
| -- - Keep password_hash as Argon2 known value (idempotent) | ||
|
|
||
| -- Canonical IDs used across seeds | ||
| DO $$ | ||
| DECLARE | ||
| v_user_id UUID := '550e8400-e29b-41d4-a716-446655440000'; | ||
| v_family_id UUID := '650e8400-e29b-41d4-a716-446655440000'; | ||
| v_ledger_id UUID := '750e8400-e29b-41d4-a716-446655440000'; | ||
| v_email TEXT := 'superadmin@jive.money'; | ||
| v_name TEXT := 'Super Admin'; | ||
| v_hash TEXT := '$argon2id$v=19$m=19456,t=2,p=1$VnRaV3dqQ3I5emZLc0tXSQ$B5q+BXWvBzVNFLCCPfyqxqhYf2Kx0Mmdz4HDUX9+KMI'; | ||
| BEGIN | ||
| -- Ensure user exists and normalize fields | ||
| INSERT INTO users (id, email, username, full_name, name, password_hash, is_active, email_verified, created_at, updated_at) | ||
| VALUES (v_user_id, v_email, 'superadmin', v_name, v_name, v_hash, true, true, NOW(), NOW()) | ||
| ON CONFLICT (id) DO UPDATE SET | ||
| email = EXCLUDED.email, | ||
| username = 'superadmin', | ||
| full_name = COALESCE(users.full_name, EXCLUDED.full_name), | ||
| name = COALESCE(users.name, EXCLUDED.name), | ||
| password_hash = EXCLUDED.password_hash, | ||
| is_active = true, | ||
| email_verified = true, | ||
| updated_at = NOW(); | ||
|
|
||
| -- Ensure family exists with owner_id and baseline fields | ||
| INSERT INTO families (id, name, owner_id, currency, timezone, locale, invite_code, created_at, updated_at) | ||
| VALUES (v_family_id, 'Admin Family', v_user_id, 'CNY', 'Asia/Shanghai', 'zh-CN', 'ADM1NFAM', NOW(), NOW()) | ||
| ON CONFLICT (id) DO UPDATE SET | ||
| owner_id = v_user_id, | ||
| name = 'Admin Family', | ||
| updated_at = NOW(); | ||
|
|
||
| -- Ensure membership (owner) | ||
| INSERT INTO family_members (family_id, user_id, role, joined_at) | ||
| VALUES (v_family_id, v_user_id, 'owner', NOW()) | ||
| ON CONFLICT (family_id, user_id) DO UPDATE SET | ||
| role = 'owner'; | ||
|
|
||
| -- Ensure current_family_id set on user | ||
| UPDATE users SET current_family_id = v_family_id, updated_at = NOW() | ||
| WHERE id = v_user_id AND (current_family_id IS NULL OR current_family_id <> v_family_id); | ||
|
|
||
| -- Ensure default ledger exists | ||
| INSERT INTO ledgers (id, family_id, name, currency, created_by, is_default, is_active, created_at, updated_at) | ||
| VALUES (v_ledger_id, v_family_id, 'Admin Ledger', 'CNY', v_user_id, true, true, NOW(), NOW()) | ||
| ON CONFLICT (id) DO NOTHING; | ||
| END $$; | ||
|
|
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
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
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -112,18 +112,19 @@ pub async fn register_with_preferences( | |||||
| sqlx::query( | ||||||
| r#" | ||||||
| INSERT INTO users ( | ||||||
| id, email, name, full_name, password_hash, | ||||||
| country, preferred_currency, preferred_language, | ||||||
| preferred_timezone, preferred_date_format, | ||||||
| avatar_url, avatar_style, avatar_color, avatar_background, | ||||||
| created_at, updated_at | ||||||
| ) | ||||||
| VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16) | ||||||
| "#, | ||||||
| ) | ||||||
| .bind(user_id) | ||||||
| .bind(&req.email) | ||||||
| .bind(&req.name) | ||||||
| .bind(&req.name) | ||||||
|
There was a problem hiding this comment. The same field
Suggested change
|
||||||
| .bind(&password_hash) | ||||||
| .bind(&req.country) | ||||||
| .bind(&req.currency) | ||||||
|
|
||||||
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| #[cfg(test)] | ||
| mod tests { | ||
| use axum::{routing::get, Router}; | ||
| use http::{header, Request, StatusCode}; | ||
| use hyper::Body; | ||
| use tower::ServiceExt; // for `oneshot` | ||
|
|
||
| use jive_money_api::handlers::transactions::export_transactions_csv_stream; | ||
| use jive_money_api::auth::Claims; | ||
|
|
||
| use crate::fixtures::{create_test_pool, create_test_user, create_test_family}; | ||
|
|
||
| async fn bearer_for(user_id: uuid::Uuid, family_id: uuid::Uuid) -> String { | ||
| let claims = Claims::new(user_id, format!("{}@example.com", user_id), Some(family_id)); | ||
| format!("Bearer {}", claims.to_token().unwrap()) | ||
| } | ||
|
|
||
| // User A should not export data for User B's family (403) | ||
| #[tokio::test] | ||
| async fn export_cross_family_forbidden() { | ||
| let pool = create_test_pool().await; | ||
|
|
||
| // Create two users and two families (each user owns their own family) | ||
| let user_a = create_test_user(&pool).await; | ||
| let user_b = create_test_user(&pool).await; | ||
| let family_a = create_test_family(&pool, user_a.id).await; | ||
| let family_b = create_test_family(&pool, user_b.id).await; | ||
|
|
||
| // Token for user A bound to family A | ||
| let token_a_family_a = bearer_for(user_a.id, family_a.id).await; | ||
|
|
||
| // Minimal router with CSV export endpoint | ||
| let app = Router::new() | ||
| .route("/api/v1/transactions/export.csv", get(export_transactions_csv_stream)) | ||
| .with_state(pool.clone()); | ||
|
|
||
| // Try to export for family B while token is bound to family A. | ||
| // The handler reads family_id from token claims, not from query, so the | ||
| // access check should fail before any data is returned. | ||
| let req = Request::builder() | ||
| .method("GET") | ||
| .uri("/api/v1/transactions/export.csv") | ||
| .header(header::AUTHORIZATION, token_a_family_a) | ||
| .body(Body::empty()) | ||
| .unwrap(); | ||
|
|
||
| let resp = app.clone().oneshot(req).await.unwrap(); | ||
| assert_eq!(resp.status(), StatusCode::FORBIDDEN); | ||
|
|
||
| // Control: user B exporting their own family's data should pass auth (may be empty CSV) | ||
| let token_b_family_b = bearer_for(user_b.id, family_b.id).await; | ||
| let req = Request::builder() | ||
| .method("GET") | ||
| .uri("/api/v1/transactions/export.csv") | ||
| .header(header::AUTHORIZATION, token_b_family_b) | ||
| .body(Body::empty()) | ||
| .unwrap(); | ||
| let resp = app.clone().oneshot(req).await.unwrap(); | ||
| assert_eq!(resp.status(), StatusCode::OK); | ||
| } | ||
|
Comment on lines
+20
to
+60
There was a problem hiding this comment. This test appears to have a logical flaw. It creates a valid token for The test's name I've suggested a rewrite that correctly implements the intended cross-family access check and also makes the control case more direct by having user A access their own family's data. async fn export_cross_family_forbidden() {
let pool = create_test_pool().await;
// Create two users and two families (each user owns their own family)
let user_a = create_test_user(&pool).await;
let user_b = create_test_user(&pool).await;
let family_a = create_test_family(&pool, user_a.id).await;
let family_b = create_test_family(&pool, user_b.id).await;
// Minimal router with CSV export endpoint
let app = Router::new()
.route("/api/v1/transactions/export.csv", get(export_transactions_csv_stream))
.with_state(pool.clone());
// Test case: User A tries to export data for User B's family (should be forbidden)
// A token is created for User A, but associated with Family B.
// The backend should detect that User A is not a member of Family B and deny access.
let token_a_for_family_b = bearer_for(user_a.id, family_b.id).await;
let req = Request::builder()
.method("GET")
.uri("/api/v1/transactions/export.csv")
.header(header::AUTHORIZATION, token_a_for_family_b)
.body(Body::empty())
.unwrap();
let resp = app.clone().oneshot(req).await.unwrap();
assert_eq!(resp.status(), StatusCode::FORBIDDEN);
// Control Case: User A exporting their own family's data should be allowed
let token_a_for_family_a = bearer_for(user_a.id, family_a.id).await;
let req = Request::builder()
.method("GET")
.uri("/api/v1/transactions/export.csv")
.header(header::AUTHORIZATION, token_a_for_family_a)
.body(Body::empty())
.unwrap();
let resp = app.clone().oneshot(req).await.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
} |
||
| } | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
|| trueat the end of this command will suppress any errors from themigrate_local.shscript, causing themaketarget to succeed even if the migration fails. This can hide issues like a database connection problem and lead to a confusing state where subsequent steps fail without a clear root cause. It's better to let the script fail and report the error, which makes debugging easier.