diff --git a/.config/nextest.toml b/.config/nextest.toml
new file mode 100644
index 0000000..20d6266
--- /dev/null
+++ b/.config/nextest.toml
@@ -0,0 +1,26 @@
+# cargo-nextest configuration. Shared by local runs and CI.
+#
+# Flakiness policy: no retries anywhere. A test that needs a retry is a bug
+# in the test; fix the test. `slow-timeout` turns a hang into a failure with
+# a name attached instead of a stalled job.
+
+[profile.default]
+slow-timeout = { period = "30s", terminate-after = 4 } # 2 min hard cap per test
+fail-fast = false
+
+[profile.ci]
+slow-timeout = { period = "30s", terminate-after = 6 } # 3 min on slower runners
+fail-fast = false
+failure-output = "immediate-final"
+status-level = "fail"
+final-status-level = "slow"
+
+[profile.ci.junit]
+path = "junit.xml"
+
+# Repeated-run profile for flake hunting: same policy, quieter output.
+[profile.stress]
+slow-timeout = { period = "30s", terminate-after = 4 }
+fail-fast = false
+status-level = "fail"
+final-status-level = "fail"
diff --git a/.github/scripts/coverage_summary.py b/.github/scripts/coverage_summary.py
new file mode 100755
index 0000000..72408d4
--- /dev/null
+++ b/.github/scripts/coverage_summary.py
@@ -0,0 +1,129 @@
+#!/usr/bin/env python3
+"""Render a Markdown coverage table from `cargo llvm-cov report --json`.
+
+Usage: coverage_summary.py HEAD.json [BASE.json]
+
+Rows are grouped per workspace crate (derived from the source path), with the
+line-coverage delta against BASE when given. Files under `target/` and
+`examples/`/`tests/` fixtures are still counted (they are part of the
+workspace) but examples are listed separately so library crates stay visible.
+"""
+import json
+import os
+import re
+import sys
+from collections import defaultdict
+
+CRATE_RE = re.compile(
+ r"(?:^|/)(crates/rpc/bidirectional/[^/]+|crates/[^/]+/[^/]+|examples/[^/]+(?:/[^/]+)?|tests/playwright/fixtures/[^/]+)/"
+)
+
+
+def load(path):
+ if not path or not os.path.exists(path):
+ return None
+ with open(path) as f:
+ data = json.load(f)
+ return data.get("data", [None])[0]
+
+
+def per_crate(report):
+ """-> {crate: (covered_lines, total_lines)}"""
+ out = defaultdict(lambda: [0, 0])
+ if not report:
+ return out
+ for f in report.get("files", []):
+ m = CRATE_RE.search(f["filename"])
+ crate = m.group(1) if m else "(other)"
+ lines = f["summary"]["lines"]
+ out[crate][0] += lines["covered"]
+ out[crate][1] += lines["count"]
+ return out
+
+
+def pct(cov, tot):
+ return 100.0 * cov / tot if tot else 0.0
+
+
+def fmt_delta(d):
+ if d is None:
+ return ""
+ if abs(d) < 0.005:
+ return "±0.00"
+ return f"{d:+.2f}"
+
+
+def main():
+ head = load(sys.argv[1])
+ base = load(sys.argv[2]) if len(sys.argv) > 2 else None
+ if head is None:
+ print("Coverage report unavailable.")
+ return
+
+ hc = per_crate(head)
+ bc = per_crate(base) if base else None
+
+ total_lines = head["totals"]["lines"]
+ total_pct = total_lines["percent"]
+ total_delta = None
+ if base:
+ total_delta = total_pct - base["totals"]["lines"]["percent"]
+
+ print("### Test coverage (lines)")
+ print()
+ headline = f"**Total: {total_pct:.2f}%**"
+ if total_delta is not None:
+ headline += f" ({fmt_delta(total_delta)} vs base)"
+ print(headline, f"— {total_lines['covered']}/{total_lines['count']} lines")
+ print()
+ cols = "| Crate | Lines | Coverage |" + (" Δ |" if base else "")
+ print(cols)
+ print("|---|---:|---:|" + ("---:|" if base else ""))
+
+ def rows(prefix):
+ for crate in sorted(k for k in hc if k.startswith(prefix)):
+ cov, tot = hc[crate]
+ p = pct(cov, tot)
+ line = f"| `{crate}` | {cov}/{tot} | {p:.2f}% |"
+ if base:
+ if crate in bc and bc[crate][1]:
+ d = p - pct(*bc[crate])
+ line += f" {fmt_delta(d)} |"
+ else:
+ line += " new |"
+ print(line)
+
+ rows("crates/")
+ if any(k.startswith(("examples/", "tests/")) for k in hc):
+ print("| **Examples and fixtures** | | |" + (" |" if base else ""))
+ rows("examples/")
+ rows("tests/")
+ if "(other)" in hc:
+ rows("(other)")
+
+ # Files whose coverage dropped the most, to make regressions actionable.
+ if base:
+ base_files = {f["filename"]: f["summary"]["lines"] for f in base.get("files", [])}
+ drops = []
+ for f in head.get("files", []):
+ b = base_files.get(f["filename"])
+ if not b or not b["count"]:
+ continue
+ d = f["summary"]["lines"]["percent"] - b["percent"]
+ if d <= -1.0:
+ drops.append((d, f["filename"], f["summary"]["lines"]["percent"]))
+ if drops:
+ print()
+ print("Files with coverage drops ≥ 1 point
")
+ print()
+ print("| File | Coverage | Δ |")
+ print("|---|---:|---:|")
+ for d, name, p in sorted(drops)[:25]:
+ short = name.split("/rust-api-stack/", 1)[-1]
+ print(f"| `{short}` | {p:.2f}% | {fmt_delta(d)} |")
+ print()
+ print(" ")
+
+
+if __name__ == "__main__":
+ main()
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 4da35d4..aa052f6 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -161,13 +161,22 @@ jobs:
steps:
- uses: actions/checkout@v6.0.2
- uses: dtolnay/rust-toolchain@stable
+ - uses: taiki-e/install-action@nextest
- uses: Swatinem/rust-cache@v2
- name: Build tests
- run: cargo test --workspace --all-targets --all-features --no-run --locked
- - name: Run tests
- run: cargo test --workspace --all-targets --all-features --locked
+ run: cargo nextest run --workspace --all-targets --all-features --locked --no-run
+ - name: Run tests (nextest, no retries)
+ run: cargo nextest run --workspace --all-targets --all-features --locked --profile ci
- name: Run doctests
run: cargo test --doc --workspace --all-features --locked
+ - name: Upload JUnit report
+ if: always()
+ uses: actions/upload-artifact@v7.0.1
+ with:
+ name: junit-tests
+ path: target/nextest/ci/junit.xml
+ if-no-files-found: ignore
+ retention-days: 14
feature-matrix:
name: Feature matrix
@@ -327,28 +336,71 @@ jobs:
coverage:
name: Coverage report
runs-on: ubuntu-latest
- needs: [test]
+ permissions:
+ contents: read
+ pull-requests: write
steps:
- uses: actions/checkout@v6.0.2
+ with:
+ fetch-depth: 0
- uses: dtolnay/rust-toolchain@stable
with:
components: llvm-tools-preview
- - uses: taiki-e/install-action@cargo-llvm-cov
+ - uses: taiki-e/install-action@v2
+ with:
+ tool: cargo-llvm-cov,nextest
- uses: Swatinem/rust-cache@v2
- - name: Generate coverage (lcov)
- run: cargo llvm-cov --workspace --all-targets --all-features --locked --lcov --output-path lcov.info
- - name: Print summary
- run: cargo llvm-cov report --summary-only
+
+ - name: Coverage for this ref
+ run: |
+ cargo llvm-cov nextest --workspace --all-targets --all-features --locked \
+ --lcov --output-path lcov.info
+ cargo llvm-cov report --json --output-path coverage-head.json
+ cargo llvm-cov report --summary-only | tee coverage-head.txt
+
+ - name: Coverage for base (PR only)
+ if: github.event_name == 'pull_request'
+ run: |
+ base=$(git merge-base "origin/${{ github.base_ref }}" HEAD)
+ git stash --include-untracked -q || true
+ git checkout -q "$base"
+ cargo llvm-cov nextest --workspace --all-targets --all-features --locked \
+ --json --output-path coverage-base.json || echo '{}' > coverage-base.json
+ git checkout -q -
+ git stash pop -q || true
+
+ - name: Build coverage summary
+ id: summary
+ shell: bash
+ run: |
+ python3 .github/scripts/coverage_summary.py \
+ coverage-head.json \
+ "${{ github.event_name == 'pull_request' && 'coverage-base.json' || '' }}" \
+ > coverage-summary.md
+ cat coverage-summary.md >> "$GITHUB_STEP_SUMMARY"
+
+ - name: Comment on PR
+ if: github.event_name == 'pull_request'
+ env:
+ GH_TOKEN: ${{ github.token }}
+ PR: ${{ github.event.pull_request.number }}
+ run: |
+ marker=''
+ body="$(printf '%s\n' "$marker"; cat coverage-summary.md)"
+ existing=$(gh api "repos/${{ github.repository }}/issues/$PR/comments" \
+ --jq ".[] | select(.body | startswith(\"$marker\")) | .id" | head -1)
+ if [ -n "$existing" ]; then
+ gh api -X PATCH "repos/${{ github.repository }}/issues/comments/$existing" -f body="$body" >/dev/null
+ else
+ gh pr comment "$PR" --body "$body"
+ fi
+
- name: Upload coverage artifact
uses: actions/upload-artifact@v7.0.1
with:
name: coverage-lcov
- path: lcov.info
+ path: |
+ lcov.info
+ coverage-head.json
+ coverage-summary.md
retention-days: 30
- # Optional: enable Codecov upload by adding a CODECOV_TOKEN secret and
- # uncommenting. Without the token the run still succeeds and the lcov
- # artifact above remains the source of truth.
- # - uses: codecov/codecov-action@v4
- # with:
- # files: lcov.info
- # fail_ci_if_error: false
diff --git a/.github/workflows/stress.yml b/.github/workflows/stress.yml
new file mode 100644
index 0000000..f4bc257
--- /dev/null
+++ b/.github/workflows/stress.yml
@@ -0,0 +1,50 @@
+name: Flake detector
+
+# Runs the whole suite several times at different parallelism levels. Any
+# failure here is a real flake (retries are disabled everywhere), so it is
+# a bug in the test, not noise. Weekly, plus on demand.
+
+on:
+ schedule:
+ - cron: "17 3 * * 1"
+ workflow_dispatch:
+ inputs:
+ iterations:
+ description: Runs per parallelism level
+ default: "5"
+ required: false
+
+env:
+ CARGO_TERM_COLOR: always
+ CARGO_INCREMENTAL: 0
+
+jobs:
+ stress:
+ name: Repeated runs (threads=${{ matrix.threads }})
+ runs-on: ubuntu-latest
+ strategy:
+ fail-fast: false
+ matrix:
+ threads: [1, 4, 16]
+ steps:
+ - uses: actions/checkout@v6.0.2
+ - uses: dtolnay/rust-toolchain@stable
+ - uses: taiki-e/install-action@nextest
+ - uses: Swatinem/rust-cache@v2
+ - name: Build tests
+ run: cargo nextest run --workspace --all-targets --all-features --locked --no-run
+ - name: Run repeatedly
+ shell: bash
+ run: |
+ n="${{ github.event.inputs.iterations || '5' }}"
+ failed=0
+ for i in $(seq 1 "$n"); do
+ echo "::group::run $i/$n (threads=${{ matrix.threads }})"
+ if ! cargo nextest run --workspace --all-targets --all-features --locked \
+ --profile stress --test-threads "${{ matrix.threads }}"; then
+ failed=$((failed + 1))
+ fi
+ echo "::endgroup::"
+ done
+ echo "failed runs: $failed / $n" | tee -a "$GITHUB_STEP_SUMMARY"
+ exit "$failed"
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0b0e4c7..05d37c3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,108 @@ All notable changes to this project will be documented in this file.
## [Unreleased]
+### Fixed - 2026-09-05 (audit remediation, fifth review pass)
+- **`ConnectionContext::subscribe` is the only subscription mutation, and it is checked** (`ras-jsonrpc-bidirectional-server`, breaking). The context now carries the service's `SubscriptionPolicy` (limits, shared accounting, manager) and `subscribe` enforces topic length and the per-connection cap under the connection's write guard, reserves a global slot atomically, and mirrors into the manager, returning `BidirectionalError` on refusal instead of `()`. `unsubscribe` releases the slot and the manager entry. The `info` field is no longer public, so there is no unchecked path: a handler subscribing from `on_connect`, `handle_request` or anywhere else is limited and counted exactly like a client-driven subscribe, and teardown releases only what was reserved, so the counter cannot underflow. The handler-level reconciliation (`sync_subscriptions`) and `WebSocketHandler::with_subscription_limits` / `with_subscription_accounting` are gone; attach the policy with `ConnectionContext::with_subscription_policy` (the service does this automatically). Regression test subscribes greedily from both `on_connect` and `handle_subscribe` and asserts exactly the cap was accepted and the counter returns to zero.
+
+### Fixed - 2026-09-05 (audit remediation, fourth review pass)
+- **Subscription limits are enforced by the server, independent of the manager** (`ras-jsonrpc-bidirectional-server`). The service now owns a `SubscriptionAccounting` counter shared by all of its connections (`WebSocketService::subscription_accounting`, a required method). After every `handle_subscribe`, the handler reconciles the context against the configured limits itself: topic length and the per-connection cap from the context, the global cap by atomic reservation on the shared counter. Only accepted topics are handed to the manager, so a permissive custom `ConnectionManager` supplied via `build_with_manager`, a `DefaultConnectionManager::new()` built with different limits, or a greedy custom `MessageHandler` writing straight into the context can no longer exceed the service's caps. Slots are released on unsubscribe, on re-validation drops, and on disconnect. `DefaultConnectionManager` keeps its own enforcement as a second line. Covered by an integration test that plugs a fully permissive custom manager and a greedy handler into `build_with_manager`; it fails with the server-side check removed.
+
+### Fixed - 2026-09-05 (audit remediation, third review pass)
+- **Subscription limits are a manager invariant** (`ras-jsonrpc-bidirectional-server`, `-types`). `DefaultConnectionManager::add_subscription` now enforces topic length, the per-connection cap and the global cap itself, reserving the global slot with an atomic counter, and returns the new `BidirectionalError::SubscriptionLimitReached` (or `InvalidTopic`) when a cap would be exceeded. The handler's pre-check is only a fast path for a friendly error response; after any `handle_subscribe`, including a custom one that writes straight to the connection context, the handler mirrors into the manager and rolls back whatever the manager refuses. `DefaultConnectionManager::with_subscription_limits` sets the caps; both builders pass their `subscription_limits` through. Custom `ConnectionManager` implementations must enforce their own caps.
+- **Subscription state and index are updated as one unit** (`ras-jsonrpc-bidirectional-server`). `add_subscription` and `remove_subscription` hold the connection's entry guard while touching both the per-connection state and the topic index, so a concurrent add/remove of the same pair can no longer leave a stale index entry (and its accounting) behind. Covered by a 500-round interleaving test that fails against the previous ordering.
+- **Connection admission has no advisory fallback** (`ras-jsonrpc-bidirectional-server`, breaking for custom `WebSocketService` implementations). `WebSocketService::connection_permits` is now a required method; admission takes a permit from it or runs unbounded when it returns `None`. The checked-then-added path is gone. `max_connections` is informational.
+- `sanitize_log_detail` keeps its 256-byte contract: the ellipsis marker is counted inside the budget (`ras-auth-core`).
+
+### Fixed - 2026-09-05 (audit remediation, second review pass)
+- **Revocation race closed with an egress gate** (`ras-jsonrpc-bidirectional-server`). A `broadcast_to_topic` that snapshotted the topic index in the window between a re-validation's permission change and the subscription's removal could still deliver. Every message the manager routes on a topic is now tagged with that topic in the connection's outbound queue (`OutboundMessage`, `ChannelMessageSender::send_on_topic`), and the handler loop re-checks the subscription immediately before writing to the socket, dropping the message if the subscription is gone. Re-authorization removes the subscription from the connection context before the manager index, so the gate is authoritative from the first instant. `ChannelMessageSender::new` now takes an `mpsc::Sender`; code that reads the raw queue must unwrap `.message`.
+- **Connection admission is atomic** (`ras-jsonrpc-bidirectional-server`). `BuiltWebSocketService` holds a `tokio::sync::Semaphore` sized to `max_connections`; a permit is taken before the upgrade and held for the connection's lifetime, so concurrent upgrades cannot overshoot the cap. Custom `WebSocketService` implementations can expose the same via the new `connection_permits()` method; without it the previous advisory check applies.
+- **Zero durations no longer panic** (`ras-jsonrpc-bidirectional-server`). A zero `auth_revalidation_interval` falls back to the 30 s default with a warning; a zero keepalive ping interval or idle timeout disables that half of the keepalive with a warning. Previously either reached `tokio::time::interval_at` and panicked the connection task.
+- **Rejection detail is sanitized before logging** (`ras-auth-core`, `ras-rest-macro`, `ras-file-macro`). Generated handlers pass extractor rejection text through the new `ras_auth_core::sanitize_log_detail` (re-exported from `ras-rest-core` and `ras-file-core`), which replaces control characters and truncates to 256 bytes, so a crafted path or query value cannot inject log lines or amplify log volume.
+- `chacha20` refreshed from the yanked 0.10.1 to 0.10.2 in `Cargo.lock` (stale entry; nothing in the workspace depended on it).
+
+### Changed - 2026-09-05 (audit remediation, second review pass)
+- **`max_connections` is bounded by default** (`ras-jsonrpc-bidirectional-server`, breaking for deployments that relied on unbounded). `WebSocketServiceBuilder` and the generated `Builder` default to `DEFAULT_MAX_CONNECTIONS` (10 000); pass `max_connections(None)` explicitly to lift the cap. The generated builder also gained `max_message_size`, `subscription_limits`, `keepalive` and `on_permission_change` passthroughs, so every hardening knob is reachable without dropping to the low-level builder.
+- **Global subscription cap** (`ras-jsonrpc-bidirectional-server`, `-types`). `SubscriptionLimits::max_total_subscriptions` (default 100 000, `0` disables) bounds (connection, topic) pairs across the whole manager, so many connections cannot multiply the per-connection allowance. `ConnectionManager` gained a defaulted `total_subscription_count()` (returns 0, disabling the cap, unless the manager tracks it; `DefaultConnectionManager` tracks it exactly with an atomic counter).
+
+### Security - 2026-09-05 (audit remediation, one PR)
+Remediates every finding from the September 2026 security review plus the follow-up gap sweep: 24 issues across the WebSocket, session, identity and auth-core crates, each with a regression test named by its ID (W1–W5, C1–C2, S1–S5, I1–I9, A1–A2, F1–F2). Version bumps in this set:
+
+| Crate | From | To | Why |
+|---|---|---|---|
+| `ras-auth-core` | 0.2.0 | 0.3.0 | `AuthError` no longer `Serialize`; CSRF constructors renamed (A1, A2) |
+| `ras-identity-session` | 0.3.0 | 0.4.0 | `iss`/`aud` required by default; `SessionConfig::new` signature (S2) |
+| `ras-identity-local` | 0.2.1 | 0.3.0 | `password_hash` not serialized; `LocalAuthPayload` changes (I1, I2) |
+| `ras-identity-oauth2` | 0.2.0 | 0.3.0 | secret not serialized; `code` optional; https enforced; `add_provider` fallible (I1, I5, I9) |
+| `ras-jsonrpc-bidirectional-server` | 0.2.0 | 0.3.0 | `AuthRevalidation` gained a field; new service options (W1–W4) |
+| `ras-jsonrpc-bidirectional-client` | 0.2.0 | 0.3.0 | `AuthConfig::JwtParams` removed (C1) |
+| `ras-jsonrpc-bidirectional-macro` | 0.2.0 | 0.2.1 | generated handler routes permissions through the provider (W5) |
+| `ras-file-core` | 0.2.0 | 0.2.1 | `sanitize_filename`, `attachment()` encoding (F1) |
+| `ras-file-macro` | 0.2.0 | 0.2.1 | filename sanitization, generic rejection bodies (F1, F2) |
+| `ras-rest-macro` | 0.3.0 | 0.3.1 | generic path/query rejection bodies (F2) |
+
+Dependent crates and examples had their path-dependency version specs updated to match.
+
+
+### Fixed - 2026-09-05 (WebSocket hardening — `ras-jsonrpc-bidirectional-server`, `-macro`, `-client`)
+- **W1: subscriptions no longer survive permission revocation** (`ras-jsonrpc-bidirectional-server`). On every successful credential re-validation the handler re-runs `authorize_subscribe` for each held topic against the refreshed user and drops the ones no longer authorized, in both the connection context and the manager's topic index. Previously only the cached user was refreshed, so a downgraded connection kept receiving topic broadcasts until it disconnected. The new `PermissionChangePolicy` (`WebSocketService::on_permission_change`, builder field `on_permission_change`) selects `DropSubscriptions` (default) or `Close`, which closes the socket whenever the permission set changes so the client must re-authenticate.
+- **Subscriptions made through the default `handle_subscribe` now reach `broadcast_to_topic`.** The handler loop mirrors subscribe/unsubscribe changes from the connection context into the connection manager's topic index; previously the two stores were never reconciled, so topics accepted by `authorize_subscribe` were invisible to manager-driven broadcasts.
+- **W2: the inbound message limit is enforced at the transport** (`ras-jsonrpc-bidirectional-server`). `handle_upgrade` now sets `max_message_size` and `max_frame_size` on the Axum upgrade from `WebSocketService::max_message_size()`. Previously the 1 MiB check ran only after tungstenite had buffered the whole frame under its 64 MiB default, so any client could force 64 MiB allocations per message.
+- **W5: WebSocket permission checks route through `AuthProvider::check_permissions`** (`ras-jsonrpc-bidirectional-macro`). The generated handler now carries an optional `Arc` (`with_auth_provider`, set automatically by the generated builder) and uses `ras_auth_core::check_permission_groups`, so providers with wildcard, hierarchical or dynamic permission semantics behave identically over WebSocket, REST and JSON-RPC. Handlers built by hand without a provider fall back to plain set membership as before. The insufficient-permissions error now uses `JsonRpcError::insufficient_permissions` (code from `error_codes`, `required` only in `data`).
+- **Browser clients can now authenticate** (`ras-jsonrpc-bidirectional-client`, `-server`). The WASM transport never sent a token at all, and the server never selected a subprotocol, so a browser offering `token.` had its upgrade rejected. The client now offers `ras-jsonrpc` plus `token.` (`ClientConfig::get_subprotocols`), the server parses comma-separated `Sec-WebSocket-Protocol` lists and selects `ras-jsonrpc` (`WS_SUBPROTOCOL`), so the token is read but never echoed in the response.
+
+### Added - 2026-09-05 (WebSocket hardening)
+- **W3: `SubscriptionLimits`** (`ras-jsonrpc-bidirectional-server`) — `WebSocketService::subscription_limits()` / builder field `subscription_limits`. Defaults: 64 topics per message, 256 per connection, 256-byte topic names. An over-limit `Subscribe` is answered with an invalid-params error and leaves the connection open; the service's `handle_subscribe` never sees it.
+- **W4: `KeepaliveConfig`** (`ras-jsonrpc-bidirectional-server`) — `WebSocketService::keepalive()` / builder field `keepalive`. The server pings every 30 s and closes a connection that produces no inbound frame for 90 s (browsers and tungstenite answer pings automatically). Either half can be disabled with `None`. `max_connections` stays unbounded by default; production deployments should set it.
+- `WS_SUBPROTOCOL` / `WS_TOKEN_SUBPROTOCOL_PREFIX` constants exported from `ras-jsonrpc-bidirectional-server` and `-client`.
+
+### Changed - 2026-09-05 (WebSocket hardening — breaking, `ras-jsonrpc-bidirectional-client` 0.3.0)
+- **C1: `AuthConfig::JwtParams` removed.** It placed the token in the URL query string, where it enters proxy logs, browser history and tracing spans, and the bundled server never read it from there. Use `AuthConfig::JwtHeader` (header on native, subprotocol in browsers). `ClientBuilder::with_jwt_in_header` is now a deprecated no-op.
+- **C2: `AuthConfig::CustomParams` are percent-encoded** and emitted in sorted key order. Previously keys and values were concatenated raw.
+- `AuthRevalidation` gained the `on_permission_change` field (`ras-jsonrpc-bidirectional-server` 0.3.0); `WebSocketHandler` gained `with_connection_manager`, `with_subscription_limits`, `with_keepalive`.
+
+### Changed - 2026-09-05 (`ras-identity-session` hardening, S1–S5)
+- **`iss`/`aud` are now required by default (S2). Breaking.** `SessionConfig` gained `require_iss_aud: bool` (default `true`); `SessionConfig::validate` (and therefore `SessionService::new`) fails when either `iss` or `aud` is `None`. `SessionConfig::new` now takes the issuer and audience: `SessionConfig::new(secret, iss, aud)`. Single-service deployments that never share a secret can opt out with `SessionConfig::new_unscoped(secret)` or `.allow_unscoped_tokens()`. Struct-literal callers must add the new `require_iss_aud` and `max_sessions_per_user` fields. Examples (`bidirectional-chat`, `oauth2-demo`, `google_oauth2`) and the identity READMEs now set a real issuer/audience.
+- **Stricter `jwt_secret` validation (S3).** In addition to the 32-byte minimum, a secret is rejected when it contains fewer than 10 distinct byte values, a run of 8 or more identical bytes, or (case-insensitive substring) any of `change-me`, `changeme`, `secret`, `password`, `example`, `placeholder`, `test-secret`, `dev-secret`, `insecure`, `12345678`, `abcdefgh`, `your-secret`. Placeholder secrets in the example configs, `.env.example`, READMEs and test fixtures were replaced with random hex values..
+- **`begin_session`/`verify_session` no longer sweep the session store inline (S1).** The previous implementation took the `active_sessions` write lock and walked the whole map on every call, before the token was even decoded. Expired entries are now pruned lazily at most once per 60 s (a cheap atomic check on the hot path) and by `start_cleanup_task`, which should be started whenever `enforce_active_sessions` is on.
+
+### Added - 2026-09-05 (`ras-identity-session` hardening)
+- **`nbf` claim (S4).** `JwtClaims` gained an optional `nbf: Option` (serde default, omitted when `None`). `verify_session` rejects a token whose `iat` or `nbf` is more than `CLOCK_SKEW_LEEWAY_SECS` (60 s) in the future with `SessionError::InvalidSession`.
+- **Per-user session cap (S5).** `SessionConfig::max_sessions_per_user` (default `DEFAULT_MAX_SESSIONS_PER_USER` = 32, builder `with_max_sessions_per_user`, must be ≥ 1). When `enforce_active_sessions` is on and a user already holds that many sessions, `begin_session` evicts their oldest sessions (by `iat`) before inserting the new one, so a credential-stuffing loop cannot grow the in-memory store without bound.
+- `SessionConfig::new_unscoped`, `SessionConfig::allow_unscoped_tokens`, `SessionConfig::with_max_sessions_per_user`, and the `CLOCK_SKEW_LEEWAY_SECS` / `DEFAULT_MAX_SESSIONS_PER_USER` constants. `Debug` for `SessionConfig` shows the two new fields (secret still redacted).
+
+### Added - 2026-09-05 (identity provider hardening I1–I9)
+- **`OAuth2ProviderConfig::metadata_claims: Vec` (I8).** Allow-list of additional userinfo claims copied into `VerifiedIdentity.metadata` (and therefore the session JWT). Defaults to empty via `#[serde(default)]`; previously *every* extra claim the IdP returned was merged into metadata.
+- **`OAuth2ProviderConfig::allow_insecure_endpoints: bool` and `OAuth2ProviderConfig::validate()` (I9).** Authorization/token/userinfo endpoints must be `https://`; `validate()` rejects anything else with `OAuth2Error::ConfigError` unless the flag (serde default `false`) is set. Only enable it for a local mock IdP.
+- **`OAuth2Error::ProviderDenied { error }` and `OAuth2Error::InvalidCallback` (I5).** A callback carrying `error=…` (e.g. `access_denied`) now maps to `ProviderDenied` with only the standardized error code; `error_description` is logged at `warn` server-side and never echoed. A callback with neither `code` nor `error` returns `InvalidCallback`.
+- **`ras_identity_local::MAX_PASSWORD_BYTES` (1024) and `LocalUserError::{PasswordTooLong, HashTaskFailed}` (I4).** `add_user` rejects longer passwords with `PasswordTooLong`; `verify` rejects them with the usual `InvalidCredentials` so nothing about the account is revealed.
+- `InMemoryStateStore::len()` / `is_empty()` accessors.
+- `ras-identity-oauth2` now depends on `subtle` (workspace dep).
+
+### Changed - 2026-09-05 (identity provider hardening I1–I9)
+- **`LocalUser.password_hash` is no longer serialized (I1a, breaking).** The field carries `#[serde(skip_serializing)]`; `Serialize` output omits it entirely. `Deserialize` still requires it. Anything that persisted `LocalUser` via serde must now store the hash separately.
+- **`OAuth2ProviderConfig.client_secret` is no longer serialized (I1b, breaking).** Same treatment: dumped configs never contain the secret; deserialization still requires it.
+- **`LocalAuthPayload` no longer derives `Serialize` and has a redacting `Debug` (I2, breaking).** `{:?}` prints `password: "[REDACTED]"`. Nothing in the workspace serialized the payload; build the login JSON with `serde_json::json!` instead.
+- **Argon2 runs on the blocking pool and outside the users lock (I4).** `add_user` and `verify` clone the stored hash out of the `RwLock` and run `hash_password` / `verify_password` in `tokio::task::spawn_blocking`; the read lock is no longer held for the duration of a hash and the async executor is no longer stalled. The concurrency semaphore and the sentinel-hash timing behaviour for unknown users are unchanged.
+- **`InMemoryStateStore` evicts instead of refusing at capacity, and sweeps at most every 10 s (I3).** `store` no longer returns `TooManyPendingFlows` when `max_states` is reached: it force-sweeps expired flows and, if still full, evicts the pending flow closest to expiry. The opportunistic expired-state sweep is rate-limited to once per 10 seconds instead of an O(n) `retain` on every call (`cleanup_expired` still sweeps unconditionally). Production deployments should rate-limit flow starts at the edge; see the type docs. `OAuth2Error::TooManyPendingFlows` is now `#[deprecated]` (kept for custom `OAuth2StateStore` implementations).
+- **`AuthorizationResponse.code` and `OAuth2AuthPayload::Callback { code }` are `Option` (I5, breaking).** A legitimate `error=access_denied` redirect carries no code and is no longer an `InvalidPayload`.
+- **`OAuth2Error::HttpError` displays a fixed `"upstream request failed"` (I6).** The underlying `reqwest::Error` (which embeds the request URL) is logged at `warn` at the transport and remains reachable via `source()`, but no longer reaches `IdentityError::ProviderError` strings. Undecodable token/userinfo responses likewise log the reqwest error and surface a fixed message.
+- **OAuth2 session-binding comparison is constant-time (I7).** `handle_callback` compares the stored binding against the callback value with `subtle::ConstantTimeEq`; semantics (missing or mismatched value → `InvalidState`, unbound flow ignores the callback value) are unchanged.
+- **Provider construction validates every provider config (I9, breaking).** `OAuth2Provider::try_new` returns `ConfigError` for a non-`https://` endpoint (unless `allow_insecure_endpoints`), `OAuth2Provider::new` panics with `invalid OAuth2 configuration` (consistent with the existing `OAuth2Client::new` panic), and **`OAuth2Provider::add_provider` now returns `OAuth2Result<()>`**. The in-crate mock-IdP tests set `allow_insecure_endpoints: true`.
+- `OAuth2ProviderConfig` gained two fields, so struct literals must add `metadata_claims: Vec::new()` and `allow_insecure_endpoints: false` (updated: `examples/oauth2-demo`, the crate's `google_oauth2` example and README).
+
+### Changed - 2026-09-05 (security hardening — `ras-auth-core`, `ras-file-core`, `ras-file-macro`, `ras-rest-macro`)
+- **Weak CSRF modes are renamed `dangerous_*` and warn when paired with cookie auth (A1).** `ras-auth-core`: `CsrfConfig::header_presence_only` → `CsrfConfig::dangerous_header_presence_only`, `CsrfConfig::with_expected_value` → `CsrfConfig::dangerous_static_value`. Neither mode binds the token to the session (presence-only relies entirely on restrictive credentialed CORS; a static value is a shared process-wide secret). The old names remain as `#[deprecated]` thin wrappers for one release. `AuthTransportConfig::with_cookie` / `with_csrf` now emit a `tracing::warn!` when cookie auth is combined with either mode, and `AuthTransportConfig::validate` warns as a fallback for struct-literal configs (rate-limited to once per distinct weak config per process, since `validate` runs on every request). New `CsrfConfig::dangerous_mode()` reports which weak mode, if any, is active. `ras-auth-core` gains a direct `tracing` dependency. The `pub` fields on `CsrfConfig` (`header_name`, `expected_value`, `cookie_name`) are left public so struct-literal construction keeps compiling; a literal that clears `cookie_name` still goes through the `validate` warning path. README and the `identity-and-sessions` book chapter document the new names.
+- **`AuthError` is no longer `Serialize`/`Deserialize`, and its `Display` no longer lists the caller's permissions (A2).** `ras-auth-core`: the derives are removed (nothing in the workspace serialized `AuthError`; generated servers already map it to a generic per-class message). `AuthError::InsufficientPermissions`'s `Display` now reads `Insufficient permissions: required [...], caller holds N permission(s)` — the `has` field is retained for server-side logging via `Debug`. **Breaking** for any downstream that serialized `AuthError` directly; map to a wire type of your own instead.
+- **`DownloadResponse::attachment` escapes properly and emits an RFC 5987 `filename*` (F1).** `ras-file-core`: `"` and `\` are backslash-escaped in the quoted `filename="..."` form (previously `"` was stripped and `\` passed through), control characters are stripped, non-ASCII is replaced by `_` in the legacy form, and a `filename*=UTF-8''` parameter carries the original Unicode name. Tests that assert the exact `Content-Disposition` string need updating (in-repo: `ras-file-macro` e2e, `file-service-example`, `file-service-backend`).
+
+### Added - 2026-09-05 (security hardening)
+- **`ras_file_core::sanitize_filename(&str) -> String`** and **`ras_file_core::MAX_FILENAME_BYTES`** (F1). Reduces an untrusted filename to a single safe path component: keeps only the final component (split on both `/` and `\`), strips NUL and other control characters, maps dots-only names (`.`, `..`) and empty results to `"upload"`, and truncates to 255 bytes on a UTF-8 char boundary. Unicode is preserved.
+- `ras-file-core` now depends on and re-exports `tracing` (`ras_file_core::tracing`) so generated `file_service!` code can log without consumers declaring a direct `tracing` dependency.
+
+### Fixed - 2026-09-05 (security hardening)
+- **Upload filenames are sanitized before they reach the handler (F1).** `file_service!`: the multipart `filename=` parameter is passed through `ras_file_core::sanitize_filename` before `IncomingFile::file_name()` sees it, so a handler that joins the name onto a directory cannot be steered by `../` or `..\` segments. `filename: required` / `forbidden` policies are still evaluated on the raw presence of the parameter.
+- **axum rejection bodies are no longer echoed to the client (F2).** `file_service!`: `Multipart` extractor rejections, multipart parse errors, and `Path` extraction failures previously returned axum's own text (e.g. `Invalid boundary ...`, or the offending path value). They now return fixed messages — `invalid multipart request`, `invalid multipart body`, `invalid path parameters` — and log the axum detail at `warn`. `rest_service!`: `Path` and `axum_extra::Query` extractors previously used axum's default plain-text rejection, which echoes the offending value and target type (``Cannot parse `abc` to a `i32` ``). Generated handlers now take those extractors as `Result<_, Rejection>` and return `400` with the JSON body `{"error": "Invalid path parameters"}` / `{"error": "Invalid query parameters"}`, logging the detail at `warn` in line with the existing rejection-logging convention. Note: the `VersionMigration` error `Display` is still echoed on a `400` — that message is application-authored, like `RestError::message`, and unchanged.
+
### Changed - 2026-08-18 (`rest_service!` hardening — device-integration feedback)
- **`rest_service!` now requires `application/json` on bodied endpoints by default.** A request whose `Content-Type` is not `application/json` (parameters like `; charset=utf-8` are allowed) is rejected with `415 Unsupported Media Type` before the body is read. This forces a CORS preflight for cross-origin requests, closing the simple-request CSRF shape (a cross-origin `text/plain` POST), and matches `file_service!`, which already validated. **Breaking:** clients that POST/PUT/PATCH a body without an `application/json` content type now get `415`; opt out per-service with `require_json_content_type: false`. Rides in the already-unreleased `ras-rest-macro` `0.3.0`.
diff --git a/Cargo.lock b/Cargo.lock
index 820a7b2..854cf4f 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -457,9 +457,9 @@ checksum = "613afe47fcd5fac7ccf1db93babcb082c5994d996f20b8b159f2ad1658eb5724"
[[package]]
name = "chacha20"
-version = "0.10.1"
+version = "0.10.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "d524456ba66e72eb8b115ff89e01e497f8e6d11d78b70b1aa13c0fbd97540a81"
+checksum = "65c35e4b699c7e15ccbe7ee35c005e4fc0a278d22238a2857e6ce2dadeda1b06"
dependencies = [
"cfg-if",
"cpufeatures 0.3.0",
@@ -2687,7 +2687,7 @@ dependencies = [
[[package]]
name = "ras-auth-core"
-version = "0.2.0"
+version = "0.3.0"
dependencies = [
"cookie",
"http",
@@ -2696,11 +2696,12 @@ dependencies = [
"subtle",
"thiserror 2.0.18",
"tokio",
+ "tracing",
]
[[package]]
name = "ras-file-core"
-version = "0.2.0"
+version = "0.2.1"
dependencies = [
"bytes",
"futures-core",
@@ -2708,11 +2709,12 @@ dependencies = [
"http",
"ras-auth-core",
"thiserror 2.0.18",
+ "tracing",
]
[[package]]
name = "ras-file-macro"
-version = "0.2.0"
+version = "0.2.1"
dependencies = [
"async-trait",
"axum",
@@ -2748,7 +2750,7 @@ dependencies = [
[[package]]
name = "ras-identity-local"
-version = "0.2.1"
+version = "0.3.0"
dependencies = [
"argon2",
"async-trait",
@@ -2761,7 +2763,7 @@ dependencies = [
[[package]]
name = "ras-identity-oauth2"
-version = "0.2.0"
+version = "0.3.0"
dependencies = [
"async-trait",
"axum",
@@ -2775,6 +2777,7 @@ dependencies = [
"serde",
"serde_json",
"sha2",
+ "subtle",
"thiserror 2.0.18",
"tokio",
"tracing",
@@ -2785,7 +2788,7 @@ dependencies = [
[[package]]
name = "ras-identity-session"
-version = "0.3.0"
+version = "0.4.0"
dependencies = [
"async-trait",
"base64 0.22.1",
@@ -2804,7 +2807,7 @@ dependencies = [
[[package]]
name = "ras-jsonrpc-bidirectional-client"
-version = "0.2.0"
+version = "0.3.0"
dependencies = [
"anyhow",
"async-trait",
@@ -2833,7 +2836,7 @@ dependencies = [
[[package]]
name = "ras-jsonrpc-bidirectional-macro"
-version = "0.2.0"
+version = "0.2.1"
dependencies = [
"anyhow",
"async-trait",
@@ -2863,7 +2866,7 @@ dependencies = [
[[package]]
name = "ras-jsonrpc-bidirectional-server"
-version = "0.2.0"
+version = "0.3.0"
dependencies = [
"async-trait",
"axum",
@@ -2878,6 +2881,7 @@ dependencies = [
"serde_json",
"thiserror 2.0.18",
"tokio",
+ "tokio-tungstenite 0.26.2",
"tracing",
]
@@ -3010,7 +3014,7 @@ dependencies = [
[[package]]
name = "ras-rest-macro"
-version = "0.3.0"
+version = "0.3.1"
dependencies = [
"async-trait",
"axum",
diff --git a/README.md b/README.md
index 27faeb2..4c0400f 100644
--- a/README.md
+++ b/README.md
@@ -343,15 +343,27 @@ breakage before a pull request.
cargo fmt --all -- --check
cargo clippy --workspace --all-targets --all-features --locked -- -D warnings
-# Tests and doctests
-cargo test --workspace --all-targets --all-features --no-run --locked
-cargo test --workspace --all-targets --all-features --locked
+# Tests (cargo-nextest, as in CI) and doctests
+cargo nextest run --workspace --all-targets --all-features --locked
cargo test --doc --workspace --all-features --locked
+# Coverage (cargo-llvm-cov); CI posts the same table on every pull request
+cargo llvm-cov nextest --workspace --all-targets --all-features --locked
+
# Documentation
RUSTDOCFLAGS="-D warnings" cargo doc --workspace --all-features --no-deps --locked
```
+Tests run under [cargo-nextest](https://nexte.st) with retries disabled
+(`.config/nextest.toml`): a test that only passes on a retry is a bug in the
+test. A weekly `Flake detector` workflow runs the whole suite five times at
+three parallelism levels and can be triggered by hand from the Actions tab.
+Per-test hangs are capped at two minutes locally and three in CI.
+
+`cargo test` still works for one-off runs; nextest is preferred because it
+isolates each test in its own process, reports per-test timings, and is what
+CI and the coverage job execute.
+
### Documentation Hygiene
CI also checks that each Cargo package has a local README target and that local
diff --git a/crates/core/ras-auth-core/Cargo.toml b/crates/core/ras-auth-core/Cargo.toml
index 3d485bd..f27402b 100644
--- a/crates/core/ras-auth-core/Cargo.toml
+++ b/crates/core/ras-auth-core/Cargo.toml
@@ -1,6 +1,6 @@
[package]
name = "ras-auth-core"
-version = "0.2.0"
+version = "0.3.0"
edition = "2024"
rust-version = "1.88"
description = "Core authentication and authorization traits for Rust Agent Stack services"
@@ -15,6 +15,7 @@ http = { workspace = true }
serde = { workspace = true }
serde_json = { workspace = true }
subtle = { workspace = true }
+tracing = { workspace = true }
thiserror = { workspace = true }
[dev-dependencies]
diff --git a/crates/core/ras-auth-core/README.md b/crates/core/ras-auth-core/README.md
index 713c7df..225a9b3 100644
--- a/crates/core/ras-auth-core/README.md
+++ b/crates/core/ras-auth-core/README.md
@@ -141,8 +141,15 @@ The default cookie is `HttpOnly`, `Secure`, `SameSite=Lax`, `Path=/`, and uses a
`CsrfConfig::default()` uses a double-submit token: issue a CSRF cookie with
`csrf_cookie_header_value(...)`, then have browser clients echo the same token in
the `x-ras-csrf` header on cookie-authenticated `POST`, `PUT`, `PATCH`, and
-`DELETE` requests. Use `header_presence_only(...)` only behind restrictive
-credentialed CORS where a presence-only custom header is an intentional tradeoff.
+`DELETE` requests.
+
+Two weaker modes exist and are named to discourage casual use:
+`CsrfConfig::dangerous_header_presence_only(...)` (only requires the custom
+header to be present; sound only behind restrictive credentialed CORS) and
+`CsrfConfig::dangerous_static_value(...)` (a single process-wide value that is
+not bound to a session). Configuring cookie auth with either logs a
+`tracing::warn!`. The former names `header_presence_only` and
+`with_expected_value` are deprecated aliases and will be removed.
## Usage
diff --git a/crates/core/ras-auth-core/src/lib.rs b/crates/core/ras-auth-core/src/lib.rs
index 18f24bb..e50ab5d 100644
--- a/crates/core/ras-auth-core/src/lib.rs
+++ b/crates/core/ras-auth-core/src/lib.rs
@@ -13,8 +13,41 @@ use thiserror::Error;
pub use authorize::*;
pub use transport::*;
+/// Maximum length of an attacker-influenced string included in a log line.
+pub const MAX_LOG_DETAIL_BYTES: usize = 256;
+
+/// Make an untrusted string safe to place in a log record.
+///
+/// Control characters (including newlines, which enable log injection) are
+/// replaced with `?`, and the result is truncated to
+/// [`MAX_LOG_DETAIL_BYTES`] on a UTF-8 boundary, the trailing `…` marker
+/// included in that budget. Use it for
+/// any request-derived detail, such as extractor rejection text, before it
+/// reaches `tracing`.
+pub fn sanitize_log_detail(raw: &str) -> String {
+ let mut out: String = raw
+ .chars()
+ .map(|c| if c.is_control() { '?' } else { c })
+ .collect();
+ const MARKER: char = '…';
+ if out.len() > MAX_LOG_DETAIL_BYTES {
+ let mut cut = MAX_LOG_DETAIL_BYTES - MARKER.len_utf8();
+ while !out.is_char_boundary(cut) {
+ cut -= 1;
+ }
+ out.truncate(cut);
+ out.push(MARKER);
+ }
+ out
+}
+
/// Errors that can occur during authentication or authorization.
-#[derive(Debug, Error, Clone, Serialize, Deserialize)]
+///
+/// Deliberately **not** `Serialize`/`Deserialize`: this is a server-side
+/// diagnostic type. [`AuthError::InsufficientPermissions`] carries the caller's
+/// full permission set in `has`, which must never be sent over the wire. Map
+/// to a generic per-class client message instead (as the generated servers do).
+#[derive(Debug, Error, Clone)]
pub enum AuthError {
/// The provided token is invalid or malformed.
#[error("Invalid token")]
@@ -25,7 +58,11 @@ pub enum AuthError {
TokenExpired,
/// The token does not have the required permissions.
- #[error("Insufficient permissions: required {required:?}, has {has:?}")]
+ ///
+ /// The `Display` output names the required set and only the *count* of
+ /// permissions the caller holds; `has` is retained for server-side logging
+ /// via `Debug`.
+ #[error("Insufficient permissions: required {required:?}, caller holds {} permission(s)", has.len())]
InsufficientPermissions {
required: Vec,
has: Vec,
@@ -223,6 +260,19 @@ mod tests {
}
}
+ #[test]
+ fn log_detail_strips_control_chars_and_truncates() {
+ let injected = "bad value\n[ERROR] forged line\r\x1b[31m";
+ let cleaned = sanitize_log_detail(injected);
+ assert!(!cleaned.contains('\n') && !cleaned.contains('\r') && !cleaned.contains('\x1b'));
+ assert!(cleaned.starts_with("bad value?[ERROR] forged line"));
+
+ let long = "é".repeat(MAX_LOG_DETAIL_BYTES);
+ let cut = sanitize_log_detail(&long);
+ assert!(cut.ends_with('…'));
+ assert!(cut.len() <= MAX_LOG_DETAIL_BYTES);
+ }
+
#[test]
fn check_permissions_allows_user_when_all_required_permissions_are_present() {
let provider = TestAuthProvider;
@@ -303,28 +353,47 @@ mod tests {
}
#[test]
- fn auth_error_serializes_structured_permission_details() {
+ fn a2_insufficient_permissions_display_does_not_leak_held_permissions() {
let error = AuthError::InsufficientPermissions {
required: vec!["admin".to_string()],
- has: vec!["user".to_string()],
+ has: vec!["user".to_string(), "billing:read".to_string()],
};
- let value = serde_json::to_value(&error).expect("serialize auth error");
- assert_eq!(
- value,
- json!({
- "InsufficientPermissions": {
- "required": ["admin"],
- "has": ["user"]
- }
- })
- );
+ let display = error.to_string();
+ assert!(display.contains("required [\"admin\"]"), "{display}");
+ assert!(display.contains("holds 2 permission(s)"), "{display}");
+ assert!(!display.contains("user"), "{display}");
+ assert!(!display.contains("billing:read"), "{display}");
- let decoded: AuthError = serde_json::from_value(value).expect("deserialize auth error");
- let AuthError::InsufficientPermissions { required, has } = decoded else {
- panic!("expected insufficient permissions");
- };
- assert_eq!(required, vec!["admin"]);
- assert_eq!(has, vec!["user"]);
+ // `has` is kept for server-side logging through `Debug`.
+ let debug = format!("{error:?}");
+ assert!(debug.contains("billing:read"), "{debug}");
+ }
+
+ /// `AuthError` must not implement `Serialize` so it can never be emitted
+ /// on the wire by accident. Compile-time check via autoref specialization:
+ /// the `IsSerialize` impl on `Probe` wins when `T: Serialize`; otherwise
+ /// method lookup falls back to the `NotSerialize` impl on `&Probe`.
+ #[test]
+ fn a2_auth_error_is_not_serializable() {
+ struct Probe(std::marker::PhantomData);
+ trait NotSerialize {
+ fn is_serialize(&self) -> bool {
+ false
+ }
+ }
+ impl NotSerialize for &Probe {}
+ trait IsSerialize {
+ fn is_serialize(&self) -> bool {
+ true
+ }
+ }
+ impl IsSerialize for Probe {}
+
+ let auth_error = &Probe::(std::marker::PhantomData);
+ assert!(!auth_error.is_serialize());
+ // Sanity check that the probe detects a serializable type.
+ let user = &Probe::(std::marker::PhantomData);
+ assert!(user.is_serialize());
}
}
diff --git a/crates/core/ras-auth-core/src/transport.rs b/crates/core/ras-auth-core/src/transport.rs
index 79dca80..92bb7bb 100644
--- a/crates/core/ras-auth-core/src/transport.rs
+++ b/crates/core/ras-auth-core/src/transport.rs
@@ -377,16 +377,29 @@ impl CsrfConfig {
}
}
- /// Require the custom header to carry an exact value.
+ /// Require the custom header to carry a single, static, process-wide value.
///
- /// This is intended for callers that validate a session-specific CSRF token
- /// outside of the default double-submit cookie flow.
- pub fn with_expected_value(mut self, expected_value: impl Into) -> Self {
+ /// **Dangerous.** A static value is not bound to a session: any attacker
+ /// who learns it once (from a leaked bundle, a shared client, or a single
+ /// captured request) can forge unsafe cookie-authenticated requests for
+ /// every user until the value is rotated. This disables the double-submit
+ /// cookie check. Prefer [`Self::default`] for browser sessions.
+ pub fn dangerous_static_value(mut self, expected_value: impl Into) -> Self {
self.expected_value = Some(expected_value.into());
self.cookie_name = None;
self
}
+ /// Deprecated alias for [`Self::dangerous_static_value`].
+ #[deprecated(
+ since = "0.3.0",
+ note = "renamed to `dangerous_static_value`; a static CSRF value is not \
+ bound to a session and is a weak CSRF defense"
+ )]
+ pub fn with_expected_value(self, expected_value: impl Into) -> Self {
+ self.dangerous_static_value(expected_value)
+ }
+
/// Require the custom header to match this CSRF cookie.
pub fn with_cookie_name(mut self, cookie_name: impl Into) -> Self {
self.cookie_name = Some(cookie_name.into());
@@ -396,9 +409,12 @@ impl CsrfConfig {
/// Require only a non-empty custom header.
///
- /// This mode depends on restrictive credentialed CORS and is not a complete
- /// CSRF defense by itself. Prefer [`Self::default`] for browser sessions.
- pub fn header_presence_only(header_name: HeaderName) -> Self {
+ /// **Dangerous.** This mode relies entirely on the browser refusing to send
+ /// a custom header cross-origin without a successful CORS preflight. It is
+ /// only sound behind a restrictive credentialed CORS policy and is not a
+ /// complete CSRF defense by itself. Prefer [`Self::default`] for browser
+ /// sessions.
+ pub fn dangerous_header_presence_only(header_name: HeaderName) -> Self {
Self {
header_name,
expected_value: None,
@@ -406,6 +422,46 @@ impl CsrfConfig {
}
}
+ /// Deprecated alias for [`Self::dangerous_header_presence_only`].
+ #[deprecated(
+ since = "0.3.0",
+ note = "renamed to `dangerous_header_presence_only`; presence-only CSRF \
+ depends on restrictive CORS and is a weak CSRF defense"
+ )]
+ pub fn header_presence_only(header_name: HeaderName) -> Self {
+ Self::dangerous_header_presence_only(header_name)
+ }
+
+ /// Whether this configuration uses one of the weak, opt-in modes
+ /// ([`Self::dangerous_static_value`] or
+ /// [`Self::dangerous_header_presence_only`]) rather than the default
+ /// double-submit cookie check.
+ ///
+ /// Returns the mode name for logging, or `None` for the double-submit mode.
+ pub fn dangerous_mode(&self) -> Option<&'static str> {
+ match (&self.expected_value, &self.cookie_name) {
+ (Some(_), _) => Some("static_value"),
+ (None, None) => Some("header_presence_only"),
+ (None, Some(_)) => None,
+ }
+ }
+
+ /// Emit a `warn!` if this CSRF config is in a weak mode. Called from the
+ /// [`AuthTransportConfig`] builders (once per construction) and, as a
+ /// fallback for struct-literal construction, once per process from
+ /// [`AuthTransportConfig::validate`].
+ fn warn_if_dangerous(&self) {
+ if let Some(mode) = self.dangerous_mode() {
+ tracing::warn!(
+ csrf_mode = mode,
+ csrf_header = %self.header_name,
+ "cookie auth is configured with a weak CSRF mode \
+ (`CsrfConfig::dangerous_*`); this is not a complete CSRF defense. \
+ Prefer the default double-submit cookie mode for browser sessions"
+ );
+ }
+ }
+
/// Build a `Set-Cookie` header value for the double-submit CSRF token.
///
/// The CSRF cookie is intentionally not `HttpOnly` so browser clients can
@@ -424,7 +480,7 @@ impl CsrfConfig {
pub fn validate(&self) -> Result<(), AuthTransportError> {
// A CORS-safelisted or browser-controlled header name provides zero CSRF
// protection (it is sent automatically cross-origin), so reject it —
- // otherwise `header_presence_only(HeaderName::from_static("accept"))`
+ // otherwise `dangerous_header_presence_only(HeaderName::from_static("accept"))`
// would produce a config that passes validation but never blocks a
// forged request.
let header = self.header_name.as_str();
@@ -546,15 +602,29 @@ impl AuthTransportConfig {
if self.csrf.is_none() {
self.csrf = Some(CsrfConfig::default());
}
+ self.warn_if_weak_csrf();
self
}
/// Enable CSRF protection for cookie-authenticated unsafe requests.
+ ///
+ /// Passing a `CsrfConfig::dangerous_*` mode together with cookie auth logs
+ /// a `warn!` at construction time.
pub fn with_csrf(mut self, csrf: CsrfConfig) -> Self {
self.csrf = Some(csrf);
+ self.warn_if_weak_csrf();
self
}
+ /// Log a warning when cookie auth is paired with a weak CSRF mode.
+ fn warn_if_weak_csrf(&self) {
+ if self.cookie.is_some()
+ && let Some(csrf) = &self.csrf
+ {
+ csrf.warn_if_dangerous();
+ }
+ }
+
/// Disable bearer-token extraction.
pub fn without_bearer(mut self) -> Self {
self.bearer = false;
@@ -589,6 +659,26 @@ impl AuthTransportConfig {
csrf.validate()?;
}
+ // `validate` runs on every request, so the weak-mode warning is
+ // rate-limited here to once per distinct weak config per process. The
+ // builders (`with_cookie`, `with_csrf`) warn unconditionally at
+ // construction time; this is the fallback for struct-literal configs.
+ if self.cookie.is_some()
+ && let Some(csrf) = &self.csrf
+ && let Some(mode) = csrf.dangerous_mode()
+ {
+ static WEAK_CSRF_WARNED: std::sync::Mutex> =
+ std::sync::Mutex::new(Vec::new());
+ let key = (csrf.header_name.as_str().to_string(), mode);
+ let mut warned = WEAK_CSRF_WARNED
+ .lock()
+ .unwrap_or_else(|poisoned| poisoned.into_inner());
+ if !warned.contains(&key) {
+ warned.push(key);
+ csrf.warn_if_dangerous();
+ }
+ }
+
Ok(())
}
}
@@ -950,7 +1040,7 @@ mod tests {
fn csrf_expected_value_mode_does_not_require_csrf_cookie() {
let config = AuthTransportConfig::default()
.with_cookie(AuthCookieConfig::default())
- .with_csrf(CsrfConfig::default().with_expected_value("csrf-token"));
+ .with_csrf(CsrfConfig::default().dangerous_static_value("csrf-token"));
let cookie = AuthCredential::new("cookie-token", AuthTokenSource::Cookie);
let headers = headers(&[(DEFAULT_CSRF_HEADER, "csrf-token")]);
@@ -1016,8 +1106,9 @@ mod tests {
"cookie",
"origin",
] {
- let csrf =
- CsrfConfig::header_presence_only(HeaderName::from_bytes(name.as_bytes()).unwrap());
+ let csrf = CsrfConfig::dangerous_header_presence_only(
+ HeaderName::from_bytes(name.as_bytes()).unwrap(),
+ );
let error = csrf.validate().expect_err(name);
assert!(
matches!(error, AuthTransportError::InvalidCsrfConfig(_)),
@@ -1026,7 +1117,8 @@ mod tests {
}
// A genuinely custom header (forces a CORS preflight) is accepted.
- let ok = CsrfConfig::header_presence_only(HeaderName::from_static("x-csrf-token"));
+ let ok =
+ CsrfConfig::dangerous_header_presence_only(HeaderName::from_static("x-csrf-token"));
assert!(ok.validate().is_ok());
}
@@ -1085,4 +1177,129 @@ mod tests {
HeaderValue::from_static("[REDACTED]")
);
}
+
+ /// Minimal `tracing` subscriber that records the messages of `WARN` events.
+ /// Kept dependency-free (no `tracing-subscriber`) since it only needs to
+ /// capture a handful of events for the A1 regression tests.
+ struct WarnCapture(std::sync::Mutex>);
+
+ impl tracing::Subscriber for WarnCapture {
+ fn enabled(&self, metadata: &tracing::Metadata<'_>) -> bool {
+ *metadata.level() <= tracing::Level::WARN
+ }
+ fn new_span(&self, _: &tracing::span::Attributes<'_>) -> tracing::span::Id {
+ tracing::span::Id::from_u64(1)
+ }
+ fn record(&self, _: &tracing::span::Id, _: &tracing::span::Record<'_>) {}
+ fn record_follows_from(&self, _: &tracing::span::Id, _: &tracing::span::Id) {}
+ fn event(&self, event: &tracing::Event<'_>) {
+ struct Msg(String);
+ impl tracing::field::Visit for Msg {
+ fn record_debug(
+ &mut self,
+ field: &tracing::field::Field,
+ value: &dyn std::fmt::Debug,
+ ) {
+ self.0.push_str(&format!("{}={:?} ", field.name(), value));
+ }
+ }
+ let mut msg = Msg(String::new());
+ event.record(&mut msg);
+ self.0.lock().unwrap().push(msg.0);
+ }
+ fn enter(&self, _: &tracing::span::Id) {}
+ fn exit(&self, _: &tracing::span::Id) {}
+ }
+
+ fn capture_warnings(f: impl FnOnce()) -> Vec {
+ let capture = std::sync::Arc::new(WarnCapture(std::sync::Mutex::new(Vec::new())));
+ tracing::subscriber::with_default(capture.clone(), f);
+ capture.0.lock().unwrap().clone()
+ }
+
+ #[test]
+ fn a1_dangerous_modes_are_reported_and_deprecated_aliases_still_work() {
+ let presence =
+ CsrfConfig::dangerous_header_presence_only(HeaderName::from_static("x-csrf-token"));
+ assert_eq!(presence.dangerous_mode(), Some("header_presence_only"));
+
+ let static_value = CsrfConfig::default().dangerous_static_value("shared-secret");
+ assert_eq!(static_value.dangerous_mode(), Some("static_value"));
+
+ assert_eq!(CsrfConfig::default().dangerous_mode(), None);
+ assert_eq!(
+ CsrfConfig::default()
+ .with_cookie_name("__Host-other")
+ .dangerous_mode(),
+ None
+ );
+
+ // The deprecated names remain as thin wrappers for one release.
+ #[allow(deprecated)]
+ let legacy_presence =
+ CsrfConfig::header_presence_only(HeaderName::from_static("x-csrf-token"));
+ assert_eq!(legacy_presence, presence);
+ #[allow(deprecated)]
+ let legacy_static = CsrfConfig::default().with_expected_value("shared-secret");
+ assert_eq!(legacy_static, static_value);
+ }
+
+ #[test]
+ fn a1_cookie_auth_with_weak_csrf_mode_warns_at_construction() {
+ let warnings = capture_warnings(|| {
+ let _ = AuthTransportConfig::default()
+ .with_cookie(AuthCookieConfig::default())
+ .with_csrf(CsrfConfig::dangerous_header_presence_only(
+ HeaderName::from_static("x-csrf-token"),
+ ));
+ });
+ assert_eq!(warnings.len(), 1, "{warnings:?}");
+ assert!(warnings[0].contains("csrf_mode=\"header_presence_only\""));
+ assert!(warnings[0].contains("weak CSRF mode"));
+
+ // Ordering does not matter: csrf first, then cookie.
+ let warnings = capture_warnings(|| {
+ let _ = AuthTransportConfig::default()
+ .with_csrf(CsrfConfig::default().dangerous_static_value("shared-secret"))
+ .with_cookie(AuthCookieConfig::default());
+ });
+ assert_eq!(warnings.len(), 1, "{warnings:?}");
+ assert!(warnings[0].contains("csrf_mode=\"static_value\""));
+
+ // Weak CSRF without cookie auth is irrelevant (bearer-only) — no warning.
+ let warnings = capture_warnings(|| {
+ let _ = AuthTransportConfig::default().with_csrf(
+ CsrfConfig::dangerous_header_presence_only(HeaderName::from_static("x-csrf-token")),
+ );
+ });
+ assert!(warnings.is_empty(), "{warnings:?}");
+
+ // The default double-submit mode never warns.
+ let warnings = capture_warnings(|| {
+ let _ = AuthTransportConfig::default().with_cookie(AuthCookieConfig::default());
+ });
+ assert!(warnings.is_empty(), "{warnings:?}");
+ }
+
+ #[test]
+ fn a1_struct_literal_weak_csrf_warns_from_validate_once() {
+ // Struct-literal construction bypasses the builders, so `validate`
+ // warns as a fallback — but only once per distinct weak config per
+ // process, since it runs on every request. A header name unique to
+ // this test keeps it independent of test ordering.
+ let config = AuthTransportConfig {
+ bearer: true,
+ cookie: Some(AuthCookieConfig::default()),
+ csrf: Some(CsrfConfig::dangerous_header_presence_only(
+ HeaderName::from_static("x-a1-struct-literal-csrf"),
+ )),
+ };
+ let warnings = capture_warnings(|| {
+ config.validate().unwrap();
+ config.validate().unwrap();
+ config.validate().unwrap();
+ });
+ assert_eq!(warnings.len(), 1, "{warnings:?}");
+ assert!(warnings[0].contains("csrf_mode=\"header_presence_only\""));
+ }
}
diff --git a/crates/core/ras-observability-core/Cargo.toml b/crates/core/ras-observability-core/Cargo.toml
index f9fc64e..e4d241b 100644
--- a/crates/core/ras-observability-core/Cargo.toml
+++ b/crates/core/ras-observability-core/Cargo.toml
@@ -10,7 +10,7 @@ homepage = "https://github.com/JedimEmO/rust-api-stack"
readme = "README.md"
[dependencies]
-ras-auth-core = { path = "../ras-auth-core", version = "0.2.0" }
+ras-auth-core = { path = "../ras-auth-core", version = "0.3.0" }
async-trait = { workspace = true }
serde = { workspace = true }
axum = { workspace = true }
diff --git a/crates/identity/ras-identity-local/Cargo.toml b/crates/identity/ras-identity-local/Cargo.toml
index 6f0e980..aa09c26 100644
--- a/crates/identity/ras-identity-local/Cargo.toml
+++ b/crates/identity/ras-identity-local/Cargo.toml
@@ -1,6 +1,6 @@
[package]
name = "ras-identity-local"
-version = "0.2.1"
+version = "0.3.0"
edition = "2024"
rust-version = "1.88"
description = "Local username/password authentication provider with Argon2 hashing"
diff --git a/crates/identity/ras-identity-local/README.md b/crates/identity/ras-identity-local/README.md
index c10982e..d27da3f 100644
--- a/crates/identity/ras-identity-local/README.md
+++ b/crates/identity/ras-identity-local/README.md
@@ -65,13 +65,15 @@ provider
)
.await?;
+let session_config = SessionConfig::new(
+ "fd2f56e597efef86b80c5484eb5247f4139b33bcdcb60dab", // openssl rand -hex 32
+ "my-service", // iss
+ "my-service", // aud
+)?;
let session_service = Arc::new(SessionService::new(SessionConfig {
- jwt_secret: "use-at-least-32-bytes-of-random-secret".to_string(),
jwt_ttl: Duration::hours(1),
- enforce_active_sessions: true,
algorithm: JwtAlgorithm::HS256,
- iss: None,
- aud: None,
+ ..session_config
})?);
session_service.register_provider(Box::new(provider)).await;
diff --git a/crates/identity/ras-identity-local/src/lib.rs b/crates/identity/ras-identity-local/src/lib.rs
index 2af4f3b..fbaeb93 100644
--- a/crates/identity/ras-identity-local/src/lib.rs
+++ b/crates/identity/ras-identity-local/src/lib.rs
@@ -14,9 +14,15 @@ use std::fmt;
use std::sync::Arc;
use tokio::sync::RwLock;
+/// Maximum accepted password length in bytes (I4). Longer inputs are rejected before
+/// hashing so a client cannot make the server burn Argon2 time on multi-megabyte inputs.
+pub const MAX_PASSWORD_BYTES: usize = 1024;
+
#[derive(Clone, Serialize, Deserialize)]
pub struct LocalUser {
pub username: String,
+ /// Argon2 PHC string. Never serialized (I1a); still required on deserialize.
+ #[serde(skip_serializing)]
pub password_hash: String,
pub email: Option,
pub display_name: Option,
@@ -36,12 +42,22 @@ impl fmt::Debug for LocalUser {
}
}
-#[derive(Debug, Serialize, Deserialize)]
+#[derive(Deserialize)]
pub struct LocalAuthPayload {
pub username: String,
pub password: String,
}
+/// Redacting `Debug` so the plaintext password never lands in logs (I2).
+impl fmt::Debug for LocalAuthPayload {
+ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+ f.debug_struct("LocalAuthPayload")
+ .field("username", &self.username)
+ .field("password", &"[REDACTED]")
+ .finish()
+ }
+}
+
/// Errors returned when managing local users.
#[derive(Debug)]
pub enum LocalUserError {
@@ -49,6 +65,10 @@ pub enum LocalUserError {
UserAlreadyExists { username: String },
/// Password hashing failed while creating the user.
PasswordHash(argon2::password_hash::Error),
+ /// The password exceeds [`MAX_PASSWORD_BYTES`].
+ PasswordTooLong { max_bytes: usize },
+ /// The blocking hashing task was cancelled or panicked.
+ HashTaskFailed,
}
impl fmt::Display for LocalUserError {
@@ -58,6 +78,10 @@ impl fmt::Display for LocalUserError {
write!(f, "user '{username}' already exists")
}
Self::PasswordHash(error) => write!(f, "failed to hash password: {error}"),
+ Self::PasswordTooLong { max_bytes } => {
+ write!(f, "password exceeds maximum length of {max_bytes} bytes")
+ }
+ Self::HashTaskFailed => write!(f, "password hashing task failed"),
}
}
}
@@ -66,7 +90,9 @@ impl Error for LocalUserError {
fn source(&self) -> Option<&(dyn Error + 'static)> {
match self {
Self::PasswordHash(error) => Some(error),
- Self::UserAlreadyExists { .. } => None,
+ Self::UserAlreadyExists { .. }
+ | Self::PasswordTooLong { .. }
+ | Self::HashTaskFailed => None,
}
}
}
@@ -98,6 +124,12 @@ impl LocalUserProvider {
email: Option,
display_name: Option,
) -> Result<(), LocalUserError> {
+ if password.len() > MAX_PASSWORD_BYTES {
+ return Err(LocalUserError::PasswordTooLong {
+ max_bytes: MAX_PASSWORD_BYTES,
+ });
+ }
+
{
let users = self.users.read().await;
if users.contains_key(&username) {
@@ -105,11 +137,15 @@ impl LocalUserProvider {
}
}
- let argon2 = Argon2::default();
- let salt = SaltString::generate(&mut OsRng);
- let password_hash = argon2
- .hash_password(password.as_bytes(), &salt)?
- .to_string();
+ // Argon2 is CPU-bound; keep it off the async executor (I4).
+ let password_hash = tokio::task::spawn_blocking(move || {
+ let salt = SaltString::generate(&mut OsRng);
+ Argon2::default()
+ .hash_password(password.as_bytes(), &salt)
+ .map(|hash| hash.to_string())
+ })
+ .await
+ .map_err(|_| LocalUserError::HashTaskFailed)??;
let user = LocalUser {
username: username.clone(),
@@ -139,23 +175,40 @@ impl LocalUserProvider {
self.semaphore.clone().acquire_owned().await.map_err(|_| {
IdentityError::ProviderError("local auth limiter closed".to_string())
})?;
- let users = self.users.read().await;
+
+ // Reject oversized passwords before spending Argon2 time on them (I4). Same error
+ // as a wrong password so nothing is leaked about the account.
+ if password.len() > MAX_PASSWORD_BYTES {
+ return Err(IdentityError::InvalidCredentials);
+ }
// Verify missing users against a fixed sentinel hash to keep timing consistent.
const SENTINEL_HASH: &str = "$argon2id$v=19$m=19456,t=2,p=1$9QsJRKgzJkKaOUvlp7gl2Q$qmE3qIFBNJ6nZYbLYXEI2uo0zZc7T0Q8LU1ZsqsZ3QE";
- let (user, password_hash) = if let Some(user) = users.get(username) {
- (Some(user.clone()), user.password_hash.as_str())
- } else {
- (None, SENTINEL_HASH)
+ // Clone the stored hash out of the lock so verification never holds it (I4).
+ let (user, password_hash) = {
+ let users = self.users.read().await;
+ match users.get(username) {
+ Some(user) => (Some(user.clone()), user.password_hash.clone()),
+ None => (None, SENTINEL_HASH.to_string()),
+ }
};
- let parsed_hash = PasswordHash::new(password_hash)
- .map_err(|e| IdentityError::ProviderError(e.to_string()))?;
-
- let password_valid = Argon2::default()
- .verify_password(password.as_bytes(), &parsed_hash)
- .is_ok();
+ // Argon2 is CPU-bound; run it on the blocking pool (I4).
+ let password = password.to_string();
+ let password_valid = tokio::task::spawn_blocking(move || {
+ let parsed_hash = PasswordHash::new(&password_hash)
+ .map_err(|e| IdentityError::ProviderError(e.to_string()))?;
+ Ok::(
+ Argon2::default()
+ .verify_password(password.as_bytes(), &parsed_hash)
+ .is_ok(),
+ )
+ })
+ .await
+ .map_err(|_| {
+ IdentityError::ProviderError("password verification task failed".to_string())
+ })??;
// Only succeed if both user exists AND password is valid.
if password_valid {
@@ -217,6 +270,77 @@ mod tests {
assert!(debug.contains("alice"));
}
+ #[test]
+ fn i1a_local_user_serialize_omits_password_hash() {
+ let user = LocalUser {
+ username: "alice".to_string(),
+ password_hash: "$argon2id$v=19$m=19456,t=2,p=1$secretsecret$hashhashhash".to_string(),
+ email: Some("alice@example.com".to_string()),
+ display_name: None,
+ metadata: None,
+ };
+ let json = serde_json::to_value(&user).unwrap();
+ assert!(json.get("password_hash").is_none());
+ assert!(!json.to_string().contains("hashhashhash"));
+ assert_eq!(json["username"], "alice");
+
+ // Deserialize still requires the hash.
+ let full = serde_json::json!({
+ "username": "alice",
+ "password_hash": "$argon2id$x",
+ "email": null,
+ "display_name": null,
+ "metadata": null
+ });
+ let parsed: LocalUser = serde_json::from_value(full).unwrap();
+ assert_eq!(parsed.password_hash, "$argon2id$x");
+ assert!(serde_json::from_value::(json).is_err());
+ }
+
+ #[test]
+ fn i2_login_payload_debug_redacts_password() {
+ let payload = LocalAuthPayload {
+ username: "alice".to_string(),
+ password: "hunter2-super-secret".to_string(),
+ };
+ let debug = format!("{payload:?}");
+ assert!(!debug.contains("hunter2"));
+ assert!(debug.contains("[REDACTED]"));
+ assert!(debug.contains("alice"));
+ }
+
+ #[tokio::test]
+ async fn i4_oversized_password_rejected_before_hashing() {
+ let provider = setup_test_provider().await;
+
+ let too_long = "x".repeat(MAX_PASSWORD_BYTES + 1);
+ let result = provider
+ .add_user("bob".to_string(), too_long.clone(), None, None)
+ .await;
+ assert!(matches!(
+ result,
+ Err(LocalUserError::PasswordTooLong { max_bytes }) if max_bytes == MAX_PASSWORD_BYTES
+ ));
+
+ let result = provider
+ .verify(serde_json::json!({ "username": "testuser", "password": too_long }))
+ .await;
+ assert!(matches!(result, Err(IdentityError::InvalidCredentials)));
+
+ // Exactly at the limit is still accepted.
+ let at_limit = "y".repeat(MAX_PASSWORD_BYTES);
+ provider
+ .add_user("carol".to_string(), at_limit.clone(), None, None)
+ .await
+ .unwrap();
+ assert!(
+ provider
+ .verify(serde_json::json!({ "username": "carol", "password": at_limit }))
+ .await
+ .is_ok()
+ );
+ }
+
async fn setup_test_provider() -> LocalUserProvider {
let provider = LocalUserProvider::new();
diff --git a/crates/identity/ras-identity-oauth2/Cargo.toml b/crates/identity/ras-identity-oauth2/Cargo.toml
index 1bf4249..4726095 100644
--- a/crates/identity/ras-identity-oauth2/Cargo.toml
+++ b/crates/identity/ras-identity-oauth2/Cargo.toml
@@ -1,6 +1,6 @@
[package]
name = "ras-identity-oauth2"
-version = "0.2.0"
+version = "0.3.0"
edition = "2024"
rust-version = "1.88"
description = "OAuth2 authentication provider with Google support, PKCE, and state management"
@@ -28,10 +28,11 @@ base64 = { workspace = true }
sha2 = { workspace = true }
rand = { workspace = true }
url = { workspace = true }
+subtle = { workspace = true }
[dev-dependencies]
axum-test = { workspace = true }
tracing-subscriber = { workspace = true }
# For the example
-ras-identity-session = { path = "../ras-identity-session", version = "0.3.0" }
+ras-identity-session = { path = "../ras-identity-session", version = "0.4.0" }
diff --git a/crates/identity/ras-identity-oauth2/README.md b/crates/identity/ras-identity-oauth2/README.md
index b9d8f78..e308035 100644
--- a/crates/identity/ras-identity-oauth2/README.md
+++ b/crates/identity/ras-identity-oauth2/README.md
@@ -45,6 +45,10 @@ let google_config = OAuth2ProviderConfig {
auth_params: HashMap::new(),
use_pkce: true,
user_info_mapping: None,
+ // Extra userinfo claims to copy into session metadata (and the JWT). Empty = none.
+ metadata_claims: Vec::new(),
+ // Endpoints must be https:// unless this is set (local mock IdP only).
+ allow_insecure_endpoints: false,
};
// Create OAuth2 configuration
@@ -67,7 +71,11 @@ use ras_identity_session::{SessionConfig, SessionService};
// Register with session service. The provider is cheap to clone; keep one
// handle for flow initiation and register the other for verification.
-let session_config = SessionConfig::new("use-at-least-32-bytes-of-random-secret")?;
+let session_config = SessionConfig::new(
+ "50f60a216877ea8c01d90deebfaf37ee95297d744bca0931", // openssl rand -hex 32
+ "my-service", // iss
+ "my-service", // aud
+)?;
let session_service = SessionService::new(session_config)?;
session_service.register_provider(Box::new(oauth2_provider.clone())).await;
@@ -128,6 +136,8 @@ For a non-browser flow where login CSRF does not apply, use
- `auth_params`: Additional authorization parameters
- `use_pkce`: Enable PKCE for enhanced security
- `user_info_mapping`: Custom field mapping for user info
+- `metadata_claims`: Allow-list of additional userinfo claims copied into the identity metadata (and therefore the session JWT). Defaults to empty; only `picture` and `email_verified` are ever propagated without it
+- `allow_insecure_endpoints`: Permit non-`https://` endpoint URLs. Defaults to `false`; `OAuth2Provider::new`/`try_new`/`add_provider` reject insecure endpoints unless set. Only enable for a local mock IdP
### OAuth2Config
diff --git a/crates/identity/ras-identity-oauth2/examples/google_oauth2.rs b/crates/identity/ras-identity-oauth2/examples/google_oauth2.rs
index 114a340..fc00319 100644
--- a/crates/identity/ras-identity-oauth2/examples/google_oauth2.rs
+++ b/crates/identity/ras-identity-oauth2/examples/google_oauth2.rs
@@ -38,6 +38,8 @@ async fn main() -> Result<(), Box> {
auth_params: HashMap::new(),
use_pkce: true, // Enable PKCE for security
user_info_mapping: None, // Use default mapping
+ metadata_claims: Vec::new(),
+ allow_insecure_endpoints: false,
};
// Create OAuth2 configuration
@@ -53,8 +55,12 @@ async fn main() -> Result<(), Box> {
let oauth2_provider = OAuth2Provider::new(oauth2_config, state_store);
// Create session service
- let session_config =
- SessionConfig::new("oauth2-example-secret-that-is-long-enough-for-tests").unwrap();
+ let session_config = SessionConfig::new(
+ "50f60a216877ea8c01d90deebfaf37ee95297d744bca0931", // openssl rand -hex 32
+ "google-oauth2-example",
+ "google-oauth2-example",
+ )
+ .unwrap();
let session_service = SessionService::new(session_config).unwrap();
// Register OAuth2 provider with session service
@@ -168,6 +174,8 @@ mod tests {
auth_params: HashMap::new(),
use_pkce: true,
user_info_mapping: None,
+ metadata_claims: Vec::new(),
+ allow_insecure_endpoints: false,
};
let config = OAuth2Config::new().add_provider(google_config);
diff --git a/crates/identity/ras-identity-oauth2/src/client.rs b/crates/identity/ras-identity-oauth2/src/client.rs
index 824b725..e1be1f2 100644
--- a/crates/identity/ras-identity-oauth2/src/client.rs
+++ b/crates/identity/ras-identity-oauth2/src/client.rs
@@ -11,7 +11,8 @@ use sha2::{Digest, Sha256};
use std::collections::HashMap;
use std::sync::Arc;
use std::time::Duration;
-use tracing::{debug, error, info};
+use subtle::ConstantTimeEq;
+use tracing::{debug, error, info, warn};
use url::Url;
#[async_trait::async_trait]
@@ -41,7 +42,13 @@ impl OAuth2HttpTransport for ReqwestOAuth2HttpTransport {
token_endpoint: &str,
params: &HashMap,
) -> OAuth2Result {
- let response = self.client.post(token_endpoint).form(params).send().await?;
+ let response = self
+ .client
+ .post(token_endpoint)
+ .form(params)
+ .send()
+ .await
+ .map_err(log_upstream_error)?;
if !response.status().is_success() {
// Never log or propagate the raw provider response body — it can
@@ -53,10 +60,11 @@ impl OAuth2HttpTransport for ReqwestOAuth2HttpTransport {
)));
}
- let token_response: TokenResponse = response
- .json()
- .await
- .map_err(|e| OAuth2Error::InvalidTokenResponse(e.to_string()))?;
+ let token_response: TokenResponse = response.json().await.map_err(|e| {
+ // reqwest decode errors embed the request URL; keep that in the log only (I6).
+ warn!(error = %e, "token endpoint returned an undecodable response");
+ OAuth2Error::InvalidTokenResponse("undecodable token response".to_string())
+ })?;
info!("Successfully exchanged code for tokens");
Ok(token_response)
@@ -72,7 +80,8 @@ impl OAuth2HttpTransport for ReqwestOAuth2HttpTransport {
.get(userinfo_endpoint)
.bearer_auth(access_token)
.send()
- .await?;
+ .await
+ .map_err(log_upstream_error)?;
if !response.status().is_success() {
// Status only; the raw body may echo the bearer token (L1).
@@ -83,10 +92,10 @@ impl OAuth2HttpTransport for ReqwestOAuth2HttpTransport {
)));
}
- let user_info: UserInfoResponse = response
- .json()
- .await
- .map_err(|e| OAuth2Error::InvalidUserInfoResponse(e.to_string()))?;
+ let user_info: UserInfoResponse = response.json().await.map_err(|e| {
+ warn!(error = %e, "userinfo endpoint returned an undecodable response");
+ OAuth2Error::InvalidUserInfoResponse("undecodable userinfo response".to_string())
+ })?;
debug!(
"Successfully retrieved user info for subject: {}",
@@ -352,30 +361,35 @@ impl OAuth2Client {
}
// When the flow was bound to a browser session, the callback must
- // present the identical binding value (login-CSRF guard).
- if state.binding.is_some() && state.binding != callback_response.binding {
+ // present the identical binding value (login-CSRF guard). Compared in
+ // constant time so the binding cannot be recovered byte-by-byte (I7).
+ if let Some(expected) = &state.binding
+ && !binding_matches(expected, callback_response.binding.as_deref())
+ {
return Err(OAuth2Error::InvalidState);
}
- // Check for errors in callback
+ // Check for errors in callback. Only the standardized error code is
+ // surfaced; the free-text description stays in the server log (I5).
if let Some(error) = &callback_response.error {
- let error_desc = callback_response
- .error_description
- .as_deref()
- .unwrap_or("No description");
- return Err(OAuth2Error::CallbackError(format!(
- "{}: {}",
- error, error_desc
- )));
+ warn!(
+ provider = %provider_config.provider_id,
+ error = %error,
+ error_description = callback_response.error_description.as_deref().unwrap_or(""),
+ "OAuth2 provider returned an error on callback"
+ );
+ return Err(OAuth2Error::ProviderDenied {
+ error: error.clone(),
+ });
}
+ let Some(code) = callback_response.code.as_deref() else {
+ return Err(OAuth2Error::InvalidCallback);
+ };
+
// Exchange authorization code for tokens
let token_response = self
- .exchange_code(
- provider_config,
- &callback_response.code,
- state.code_verifier.as_deref(),
- )
+ .exchange_code(provider_config, code, state.code_verifier.as_deref())
.await?;
// Validate id_token claims when the provider returned one. The token
@@ -472,6 +486,26 @@ fn decode_id_token_claims(id_token: &str) -> OAuth2Result {
/// The signature is not verified: the token was received directly from the
/// token endpoint over TLS, which OIDC Core §3.1.3.7 permits as a substitute
/// for signature validation in the authorization-code flow.
+/// Log a transport-level failure at `warn` (the `reqwest::Error` carries the
+/// request URL) and hand back the fixed-message error variant (I6).
+fn log_upstream_error(error: reqwest::Error) -> OAuth2Error {
+ warn!(error = %error, "upstream OAuth2 request failed");
+ OAuth2Error::HttpError(error)
+}
+
+/// Constant-time comparison of the stored session binding against the value
+/// presented on callback (I7). A missing callback value never matches.
+fn binding_matches(expected: &str, presented: Option<&str>) -> bool {
+ match presented {
+ Some(presented) => {
+ // `ct_eq` on slices short-circuits on length, but the length of the
+ // binding is not secret (it is a UUID or caller-chosen value).
+ expected.as_bytes().ct_eq(presented.as_bytes()).into()
+ }
+ None => false,
+ }
+}
+
pub(crate) fn validate_id_token_claims(
provider_config: &OAuth2ProviderConfig,
id_token: &str,
@@ -638,6 +672,8 @@ mod tests {
auth_params: HashMap::new(),
use_pkce: true,
user_info_mapping: None,
+ metadata_claims: Vec::new(),
+ allow_insecure_endpoints: false,
}
}
@@ -747,7 +783,7 @@ mod tests {
.handle_callback(
&wrong_provider,
AuthorizationResponse {
- code: "auth-code".to_string(),
+ code: Some("auth-code".to_string()),
state,
error: None,
error_description: None,
@@ -762,7 +798,7 @@ mod tests {
}
#[tokio::test]
- async fn handle_callback_returns_provider_callback_error_without_transport_call() {
+ async fn i5_handle_callback_maps_provider_error_to_fixed_variant_without_description() {
let state_store = Arc::new(InMemoryStateStore::new());
let transport = Arc::new(RecordingTransport::new());
let client = client_with_transport(state_store, transport.clone());
@@ -773,29 +809,80 @@ mod tests {
.await
.unwrap();
+ // A legitimate denial carries no code at all.
let error = client
.handle_callback(
&provider_config,
AuthorizationResponse {
- code: "ignored-code".to_string(),
+ code: None,
state,
error: Some("access_denied".to_string()),
- error_description: Some("user denied consent".to_string()),
+ error_description: Some("user denied consent