Skip to content

middleware/auth.rs (JWT issuance and validation) has zero test coverage despite gating every authenticated endpoint #43

Description

@abayomicornelius

Overview

src/middleware/auth.rs — the file responsible for issuing and validating every JWT in the system, i.e. the single most security-critical piece of logic in the entire codebase — has zero test coverage. There is no #[cfg(test)] mod tests block anywhere in the file, and no test elsewhere in the repository exercises issue_jwt or decode_jwt directly.

This stands in sharp contrast to the rest of the codebase, which is otherwise reasonably well-tested: services/escrow.rs has 6 unit tests covering its authorization matrix, services/batch.rs has 5 tests including duplicate/invalid-input rejection, services/tx_hash.rs has 8 tests including cross-checked independent hash computation, services/soroban.rs has 4 tests for XDR construction, services/reconciliation.rs has 4 unit tests plus 2 #[ignore]d integration tests, and services/payment_request.rs/services/subscription.rs each have their own test modules. middleware/auth.rs is the one piece of code every single authenticated request in the system passes through, and it has none.

Concretely untested behavior includes:

// src/middleware/auth.rs:58-83
pub fn issue_jwt(user_id: Uuid, email: &str, secret: &str, expiry_hours: i64) -> Result<String, AppError> { ... }

// src/middleware/auth.rs:86-101
pub fn decode_jwt(token: &str, secret: &str) -> Result<JwtClaims, AppError> {
    let mut validation = Validation::new(Algorithm::HS256);
    validation.validate_exp = true;
    let data = decode::<JwtClaims>(token, &DecodingKey::from_secret(secret.as_bytes()), &validation)
        .map_err(|e| match e.kind() {
            jsonwebtoken::errors::ErrorKind::ExpiredSignature => AppError::TokenExpired,
            _ => AppError::InvalidToken,
        })?;
    Ok(data.claims)
}

None of the following properties — each a real, security-relevant behavior of this function — are pinned by any test today: that a token signed with the wrong secret is rejected as InvalidToken; that an expired token specifically maps to TokenExpired (a distinct error variant from InvalidToken, per error.rs's status_and_code, both currently mapping to 401 but semantically different and presumably intended to let a client distinguish "log in again" from "your token was tampered with"); that a token with a tampered payload/signature is rejected; that a token signed with a different algorithm (e.g. HS384, or the classic alg: none attack) is rejected outright by Validation::new(Algorithm::HS256)'s enforcement rather than silently accepted; that issue_jwt's exp/iat claims are actually set to what the function's signature promises (expiry_hours from now); and that the AuthUser extractor (middleware/auth.rs:24-55) correctly rejects a missing Authorization header, a non-Bearer scheme, and a sub claim that isn't a valid UUID.

A regression in any of this logic — e.g. an accidental algorithm downgrade, a broken expiry check, or a sub-parsing bug that silently authenticates as the wrong user — would ship with zero test signal today.

Requirements

  • Add a #[cfg(test)] mod tests block to src/middleware/auth.rs covering issue_jwt and decode_jwt directly (both are plain, synchronous, non-DB functions — no async runtime or database needed, unlike most of this codebase's other tests).
  • Cover at minimum: round-trip issue→decode succeeds and recovers the original user_id/email; decoding with the wrong secret fails as InvalidToken; decoding a token whose exp is in the past fails as TokenExpired specifically (not InvalidToken); decoding a token signed with a different algorithm is rejected; a tampered token (flip a byte in the signature) is rejected.
  • Add a focused test (using axum::http::Request/Parts construction, or axum-test/similar if a test-only dependency is warranted) for the AuthUser extractor's rejection paths: missing header, malformed scheme, non-UUID sub.

Acceptance Criteria

  • issue_jwt/decode_jwt have direct unit tests covering the cases listed above, runnable with plain cargo test (no DATABASE_URL/#[ignore] needed, since none of this logic touches the database).
  • The TokenExpired vs InvalidToken distinction is specifically pinned by a test (this is the kind of subtle branch that's easy to accidentally collapse into one variant during a refactor with no test to catch it).
  • AuthUser::from_request_parts's three rejection paths (missing header, bad scheme, invalid sub) each have a test.
  • cargo test continues to pass without requiring a live Postgres instance for these new tests specifically.

Additional Notes

Why this is worth a dedicated issue rather than folding into a generic "add more tests" effort

The absence of tests here isn't part of a blanket "this repo has no tests" gap (it doesn't — see the file list above) — it's a specific, sharp gap in exactly the one file where a silent regression has the highest blast radius (every authenticated endpoint in the API, i.e. everything except registration, login, and the couple of intentionally-public routes). That makes it worth tracking and fixing on its own rather than as a line item in a broader testing sweep.

Testing strategy

  • issue_jwt/decode_jwt tests need no mocks or fixtures beyond a hardcoded test secret (≥32 chars, matching Config::from_env's own validation) and chrono::Duration manipulation for the expiry cases (e.g. issue with expiry_hours = -1 to produce an already-expired token deterministically, rather than sleeping in a test).

Cross-references

  • Complements, but doesn't overlap, the (separately filed) testing gap in services/payment.rs — that covers the money-movement path; this covers the authentication path that gates every other endpoint.

Metadata

Metadata

Assignees

No one assigned

    Labels

    GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardOfficial Campaign | FWC26Campaign: Official Campaign | FWC26Third CampaignCampaign: Third CampaignsecuritySecurity concerntestingTest coverage relatedvery hardVery difficult / senior-level bounty issue

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions