diff --git a/crates/core/ras-auth-core/src/authorize.rs b/crates/core/ras-auth-core/src/authorize.rs index 0ba21c3..f0d1236 100644 --- a/crates/core/ras-auth-core/src/authorize.rs +++ b/crates/core/ras-auth-core/src/authorize.rs @@ -1,10 +1,8 @@ //! Shared request-authorization pipeline for generated services. //! -//! Every service macro (REST, file, JSON-RPC, bidirectional WebSocket) used -//! to inline its own copy of the credential → CSRF → authenticate → -//! permission-group sequence. These helpers are the single implementation; -//! generated code maps the returned [`AuthorizeError`] to its own protocol's -//! response shape. +//! REST and file services share credential, CSRF, authentication, and permission +//! checks here. Generated code maps [`AuthorizeError`] to protocol responses. +//! JSON-RPC and WebSocket services also use the permission-group helpers. use crate::{ AuthError, AuthProvider, AuthTransportConfig, AuthenticatedUser, Caller, @@ -17,7 +15,7 @@ use http::HeaderMap; pub enum AuthorizeError { /// No usable credential was found in the request MissingCredential, - /// Double-submit CSRF validation failed for a cookie credential + /// CSRF validation failed for a cookie credential CsrfValidationFailed, /// The credential did not authenticate AuthenticationFailed(AuthError), @@ -37,7 +35,7 @@ pub enum AuthorizeError { /// requirement — `WITH_PERMISSIONS([])`, i.e. no groups or only empty groups. /// An empty group mixed with non-empty siblings (`["admin"] | []`) is ignored /// here rather than treated as a blanket grant; the service macros reject that -/// shape at compile time, and this runtime guard is the belt-and-suspenders. +/// shape at compile time; direct callers receive the same protection. pub fn check_permission_groups

( provider: &P, user: &AuthenticatedUser, @@ -83,8 +81,8 @@ pub fn user_satisfies_permission_groups(user: &AuthenticatedUser, groups: &[Vec< /// generated REST and file-service servers. /// /// `method` is the HTTP method, used to scope CSRF validation to unsafe -/// requests. Errors are ordered so no work happens for unauthenticated -/// callers: the request body has not been touched when this returns `Err`. +/// requests. This helper does not read the body; callers should authorize before +/// buffering or parsing it. pub async fn authorize_request

( method: &str, headers: &HeaderMap, diff --git a/crates/core/ras-auth-core/src/lib.rs b/crates/core/ras-auth-core/src/lib.rs index e50ab5d..a049169 100644 --- a/crates/core/ras-auth-core/src/lib.rs +++ b/crates/core/ras-auth-core/src/lib.rs @@ -1,4 +1,4 @@ -//! Authentication and authorization traits for JSON-RPC services. +//! Authentication, authorization, and HTTP credential handling for RAS services. mod authorize; mod transport; @@ -101,12 +101,8 @@ pub struct AuthenticatedUser { /// an unsafe method all resolve to anonymous). /// * [`Caller::Authenticated`] — a valid credential was presented. /// -/// Deliberately **not** `Serialize`/`Deserialize`: a `Caller` represents a -/// *resolved* identity and must only be produced by [`resolve_caller`], never -/// reconstructed from request input. The `#[must_use]` attribute flags a -/// discarded [`resolve_caller`] result; note it cannot catch a handler that -/// receives `caller` as a parameter and never reads it (Rust applies `must_use` -/// to discarded expression results, not to unused bindings). +/// A `Caller` represents a resolved identity, so it cannot be deserialized from +/// request input. Construct it from trusted authentication results. #[must_use] #[derive(Debug, Clone)] pub enum Caller { @@ -160,8 +156,7 @@ pub type AuthFuture<'a, T = AuthenticatedUser> = /// Trait for implementing authentication providers. /// -/// This trait allows for flexible authentication mechanisms while providing -/// a consistent interface for the JSON-RPC service layer. +/// Services use this interface to validate tokens and check permissions. pub trait AuthProvider: Send + Sync + 'static { /// Validates a token and returns the authenticated user. /// diff --git a/crates/core/ras-transport-core/src/error.rs b/crates/core/ras-transport-core/src/error.rs index 5c340e9..30bdf6f 100644 --- a/crates/core/ras-transport-core/src/error.rs +++ b/crates/core/ras-transport-core/src/error.rs @@ -1,9 +1,7 @@ //! Typed transport error. //! -//! Generated clients return `Result` instead of the old -//! `Box`, so callers can match on the -//! failure mode (connection vs. HTTP status vs. (de)serialization vs. a -//! JSON-RPC application error). +//! Generated clients expose distinct connection, HTTP status, serialization, +//! body, and JSON-RPC errors so callers can handle each failure separately. use thiserror::Error; diff --git a/crates/core/ras-transport-core/src/lib.rs b/crates/core/ras-transport-core/src/lib.rs index 451a3fa..cbe8aa6 100644 --- a/crates/core/ras-transport-core/src/lib.rs +++ b/crates/core/ras-transport-core/src/lib.rs @@ -1,27 +1,17 @@ //! HTTP transport abstraction for generated Rust Agent Stack clients. //! -//! Generated REST / JSON-RPC / File clients dispatch through the -//! [`HttpTransport`] trait instead of hard-coding `reqwest::Client`. Two impls -//! ship here: [`ReqwestTransport`] (production, `reqwest` feature) and -//! [`AxumTestTransport`] (in-process, wraps `axum_test::TestServer`, native + -//! `axum-test` feature) so clients can be exercised end-to-end against a server -//! with no sockets. +//! Generated REST, JSON-RPC, and file clients use [`HttpTransport`]. The +//! `reqwest` feature provides a network adapter; the native `axum-test` feature +//! provides an in-process adapter for exercising clients without sockets. //! //! # Relationship to `WebSocketTransport` //! -//! This trait is the HTTP sibling of the `WebSocketTransport` abstraction in -//! `ras-jsonrpc-bidirectional-client` -//! (`crates/rpc/bidirectional/ras-jsonrpc-bidirectional-client/src/lib.rs`). -//! Both follow the same dyn-dispatch + conditional-`Send` pattern: -//! `#[cfg_attr(target_arch = "wasm32", async_trait(?Send))]` together with the -//! [`TransportThreadBounds`] marker, so a single `Arc` works on both -//! native and wasm targets. They are intentionally separate: bidirectional RPC -//! is WebSocket (full-duplex frames), not request/response HTTP, and must not -//! be routed through `HttpTransport`. +//! `ras-jsonrpc-bidirectional-client` owns the separate `WebSocketTransport` +//! abstraction because full-duplex frames have a different lifecycle from HTTP +//! requests. Both transport traits support native and WASM implementations. //! -//! On wasm, request bodies cannot be streamed (the fetch API has no streaming -//! request body), so [`RequestBody::Stream`] is collected before sending; -//! response bodies still stream. +//! `ReqwestTransport` streams request and response bodies on native targets +//! and buffers both on WASM. `AxumTestTransport` buffers both bodies. use std::pin::Pin; @@ -56,8 +46,6 @@ pub use axum_test_transport::AxumTestTransport; /// `::ras_transport_core::http::Method` etc. without a direct dependency. pub use http; -// --- Thread-bound marker, mirroring the bidirectional-client precedent. --- - /// Marker for the thread bounds a transport (and its streams) must satisfy. /// /// `Send + Sync` on native; unconstrained on wasm (single-threaded). @@ -74,8 +62,6 @@ pub trait TransportThreadBounds {} #[cfg(target_arch = "wasm32")] impl TransportThreadBounds for T {} -// --- Byte stream alias, conditionally `Send`. --- - /// A streaming sequence of body chunks. `Send` on native, not on wasm. #[cfg(not(target_arch = "wasm32"))] pub type ByteStream = Pin> + Send>>; @@ -102,8 +88,6 @@ where Box::pin(stream) } -// --- The transport trait. --- - /// Abstraction over the wire transport used by a generated HTTP client. /// /// See the [crate-level docs](crate) for the relationship with @@ -120,24 +104,14 @@ pub trait HttpTransport: TransportThreadBounds { -> Result; } -// --- Query / JSON helpers. --- - -/// Serialize a single query value to its `application/x-www-form-urlencoded` -/// form for one `key`, returning zero or more `(key, value)` pairs. +/// Convert a query value into decoded `(key, value)` pairs for form encoding. /// -/// Serializing one key/value at a time (rather than a whole struct) means a -/// `Vec` produces repeated keys and `#[serde(rename = ...)]` enum variants -/// encode by their rename — matching reqwest's old `.query()` behavior exactly. -/// `Option::None` produces no pairs. +/// Sequences produce repeated keys, enum variants honor `#[serde(rename)]`, +/// and `Option::None` produces no pairs. Encode with [`serialize_query_pairs`]. pub fn serialize_query_value( key: &str, value: &T, ) -> Result, TransportError> { - // We collect each scalar value into its own `(key, encoded_value)` entry so - // that scalars produce one pair, sequences produce repeated keys, and - // `None` produces nothing. Each scalar is rendered through - // `serde_urlencoded` (one key=value pair) so enum `#[serde(rename)]`s and - // numeric/bool formatting match reqwest's old `.query()` byte-for-byte. let mut collector = QueryValueCollector { values: Vec::new() }; value .serialize(&mut collector) @@ -152,12 +126,9 @@ pub fn serialize_query_value( /// Serialize several `(key, value)` query parameters and join them into a /// single query string (without a leading `?`). Empty result if no pairs. /// -/// The final byte-level encoding is delegated to `serde_urlencoded` (the same -/// crate reqwest's `.query()` uses internally), so the produced wire query -/// string matches the pre-transport reqwest client exactly — including its -/// `application/x-www-form-urlencoded` unreserved set (`*` stays raw, `~` -/// becomes `%7E`, space becomes `+`). Keys are emitted in order, so repeated -/// keys (from `Vec`) preserve their sequence. +/// Uses `application/x-www-form-urlencoded` encoding: `*` stays raw, `~` +/// becomes `%7E`, and space becomes `+`. Pair order, including repeated keys, +/// is preserved. /// /// Returns [`TransportError::Serialize`] on encoding failure rather than /// silently yielding an empty string — generated clients append the result @@ -210,11 +181,8 @@ fn hex_digit(nibble: u8) -> char { } } -// --- internal helpers --- - -/// Render one scalar value through `serde_urlencoded` and return the decoded -/// value string (the part after `=` of a single `k=v` pair), so enum renames -/// and scalar formatting match reqwest's old `.query()` exactly. +/// Preserve form-serializer scalar formatting and enum renames while returning +/// a decoded value for [`serialize_query_pairs`]. fn encode_scalar(value: &T) -> Result { use serde::ser::Error as _; // serde_urlencoded serializes a sequence of (key, value) tuples. diff --git a/crates/core/ras-transport-core/src/reqwest_transport.rs b/crates/core/ras-transport-core/src/reqwest_transport.rs index 7fc9925..5816799 100644 --- a/crates/core/ras-transport-core/src/reqwest_transport.rs +++ b/crates/core/ras-transport-core/src/reqwest_transport.rs @@ -64,7 +64,7 @@ impl HttpTransport for ReqwestTransport { RequestBody::Stream(stream) => builder.body(reqwest::Body::wrap_stream(stream)), #[cfg(target_arch = "wasm32")] RequestBody::Stream(mut stream) => { - // wasm fetch cannot stream request bodies; collect first. + // The WASM adapter sends buffered request bodies. let mut buf = bytes::BytesMut::new(); while let Some(chunk) = stream.next().await { buf.extend_from_slice(&chunk?); @@ -77,9 +77,7 @@ impl HttpTransport for ReqwestTransport { let status = resp.status(); let headers = resp.headers().clone(); - // Native streams the response body; wasm reqwest lacks the `stream` - // feature, so collect into a single chunk (response streaming on wasm - // is bounded by the fetch implementation regardless). + // The WASM reqwest dependency lacks response streaming support. #[cfg(not(target_arch = "wasm32"))] let body_stream = byte_stream_from( resp.bytes_stream() diff --git a/crates/core/ras-transport-core/tests/query_serialization.rs b/crates/core/ras-transport-core/tests/query_serialization.rs index 953d4e8..bb9c19e 100644 --- a/crates/core/ras-transport-core/tests/query_serialization.rs +++ b/crates/core/ras-transport-core/tests/query_serialization.rs @@ -58,10 +58,7 @@ fn pairs_are_percent_encoded() { #[test] fn encoding_matches_reqwests_urlencoded_unreserved_set() { - // reqwest's `.query()` delegates to serde_urlencoded, whose unreserved set - // is `[A-Za-z0-9*-._]` (space -> `+`). Regression coverage for the two - // characters where the previous hand-rolled encoder diverged: - // `~` must be percent-encoded (`%7E`), and `*` must stay raw (`*`). + // Form encoding escapes `~` as `%7E` and leaves `*` raw. let pairs = serialize_query_value("q", &"~").unwrap(); assert_eq!(serialize_query_pairs(&pairs).unwrap(), "q=%7E"); diff --git a/crates/identity/ras-identity-local/src/lib.rs b/crates/identity/ras-identity-local/src/lib.rs index fbaeb93..8dbd7dd 100644 --- a/crates/identity/ras-identity-local/src/lib.rs +++ b/crates/identity/ras-identity-local/src/lib.rs @@ -29,7 +29,7 @@ pub struct LocalUser { pub metadata: Option, } -/// Redacting `Debug` so the Argon2 `password_hash` never lands in logs (L1). +/// Redacting `Debug` so the Argon2 `password_hash` never lands in logs. impl fmt::Debug for LocalUser { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("LocalUser") diff --git a/crates/identity/ras-identity-oauth2/src/client.rs b/crates/identity/ras-identity-oauth2/src/client.rs index e1be1f2..e5d43ec 100644 --- a/crates/identity/ras-identity-oauth2/src/client.rs +++ b/crates/identity/ras-identity-oauth2/src/client.rs @@ -52,7 +52,7 @@ impl OAuth2HttpTransport for ReqwestOAuth2HttpTransport { if !response.status().is_success() { // Never log or propagate the raw provider response body — it can - // contain tokens or other sensitive material (L1). Status only. + // contain tokens or other sensitive material. Status only. let status = response.status(); error!("Token exchange failed with status {}", status); return Err(OAuth2Error::TokenExchangeFailed(format!( @@ -84,7 +84,7 @@ impl OAuth2HttpTransport for ReqwestOAuth2HttpTransport { .map_err(log_upstream_error)?; if !response.status().is_success() { - // Status only; the raw body may echo the bearer token (L1). + // Status only; the raw body may echo the bearer token. let status = response.status(); error!("User info request failed with status {}", status); return Err(OAuth2Error::UserInfoFailed(format!( @@ -150,7 +150,7 @@ impl PkceChallenge { /// `provider_config.auth_params` nor caller-supplied `additional_params` may /// override them — many providers honour the last occurrence of a duplicated /// query parameter, so an injected second `redirect_uri` / `state` / PKCE value -/// would be an authorization-code-theft or CSRF vector (H1). +/// would be an authorization-code-theft or CSRF vector. const RESERVED_AUTH_PARAMS: &[&str] = &[ "response_type", "client_id", @@ -204,12 +204,11 @@ pub struct OAuth2Client { } impl OAuth2Client { - /// Infallible constructor. + /// Create a client with bounded HTTP timeouts. /// - /// Panics if the HTTP client cannot be built. It never silently falls back - /// to an unbounded (timeout-less) client — a hung token/userinfo endpoint - /// would otherwise stall the flow indefinitely (M6). Use [`Self::try_new`] - /// to handle the (near-impossible) build error yourself. + /// # Panics + /// Panics if the HTTP client cannot be built. Use [`Self::try_new`] to + /// handle construction errors. pub fn new( state_store: Arc, state_ttl_seconds: u64, @@ -277,7 +276,7 @@ impl OAuth2Client { additional_params: HashMap, binding: Option, ) -> OAuth2Result<(String, String)> { - // Reject reserved-parameter overrides before doing any work (H1). + // Reject reserved-parameter overrides before doing any work. reject_reserved_params(provider_config.auth_params.keys(), "provider auth_params")?; reject_reserved_params(additional_params.keys(), "additional_params")?; @@ -463,7 +462,7 @@ struct IdTokenClaims { } /// Subject (`sub`) claim of an id_token, used to bind it to the userinfo -/// response so a confused-deputy userinfo cannot change the account (M6). +/// response so a confused-deputy userinfo cannot change the account. pub(crate) fn id_token_subject(id_token: &str) -> OAuth2Result> { Ok(decode_id_token_claims(id_token)?.sub) } @@ -480,8 +479,8 @@ fn decode_id_token_claims(id_token: &str) -> OAuth2Result { .map_err(|_| OAuth2Error::InvalidIdToken("invalid JSON payload".to_string())) } -/// Validate the mandatory id_token claims: issuer (when configured), -/// audience, expiry, and the nonce echoed from the authorization request. +/// Validate the id_token issuer, audience, expiry, subject, and expected nonce. +/// Accepting an id_token requires a configured provider issuer. /// /// 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 @@ -515,7 +514,7 @@ pub(crate) fn validate_id_token_claims( // Issuer is fail-closed: an id_token whose issuer is unverified cannot be // trusted to identify the account, so accepting one without a configured - // `issuer` is refused rather than silently skipped (M6). + // `issuer` is refused rather than silently skipped. let Some(expected_issuer) = &provider_config.issuer else { return Err(OAuth2Error::InvalidIdToken( "provider `issuer` must be configured to accept id_tokens".to_string(), @@ -566,7 +565,7 @@ pub(crate) fn validate_id_token_claims( } // `sub` is REQUIRED by OIDC Core §2. Refuse an id_token without it so the - // userinfo <-> id_token subject binding (M6) cannot silently no-op on a + // userinfo <-> id_token subject binding cannot silently no-op on a // token that carries no subject. match claims.sub.as_deref() { Some(sub) if !sub.trim().is_empty() => {} @@ -1006,7 +1005,7 @@ mod tests { })); assert!(validate_id_token_claims(&config, &good, Some("nonce-1")).is_ok()); - // An otherwise-valid id_token with no `sub` is rejected (M6): the + // An otherwise-valid id_token with no `sub` is rejected: the // userinfo binding must never run against an absent subject. let no_sub = fake_id_token(serde_json::json!({ "iss": "https://issuer.test", @@ -1025,7 +1024,7 @@ mod tests { })); assert!(validate_id_token_claims(&config, &aud_single_array, None).is_ok()); - // Multi-audience token requires azp == client_id (M6). + // Multi-audience token requires azp == client_id. let aud_array_with_azp = fake_id_token(serde_json::json!({ "iss": "https://issuer.test", "sub": "subject-1", @@ -1076,7 +1075,7 @@ mod tests { #[test] fn id_token_without_configured_issuer_is_rejected() { - // issuer is None on provider_config() -> fail closed (M6). + // issuer is None on provider_config() -> fail closed. let config = provider_config(); assert!(config.issuer.is_none()); let exp = chrono::Utc::now().timestamp() + 600; diff --git a/crates/identity/ras-identity-oauth2/src/config.rs b/crates/identity/ras-identity-oauth2/src/config.rs index a0c5081..09fee2a 100644 --- a/crates/identity/ras-identity-oauth2/src/config.rs +++ b/crates/identity/ras-identity-oauth2/src/config.rs @@ -17,8 +17,8 @@ pub struct OAuth2ProviderConfig { pub token_endpoint: String, pub userinfo_endpoint: Option, /// Expected `iss` claim of id_tokens returned by this provider - /// (e.g. `https://accounts.google.com`). When set, callbacks carrying - /// an id_token with a different issuer are rejected. + /// (e.g. `https://accounts.google.com`). Required when a callback returns + /// an id_token; missing or mismatched issuers are rejected. #[serde(default)] pub issuer: Option, pub redirect_uri: String, @@ -74,7 +74,7 @@ fn is_https(url: &str) -> bool { .is_some_and(|scheme| scheme.eq_ignore_ascii_case("https://")) } -/// Manual `Debug` that redacts `client_secret` so it never lands in logs (L1). +/// Manual `Debug` that redacts `client_secret` so it never lands in logs. impl fmt::Debug for OAuth2ProviderConfig { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("OAuth2ProviderConfig") diff --git a/crates/identity/ras-identity-oauth2/src/provider.rs b/crates/identity/ras-identity-oauth2/src/provider.rs index 8806b04..75408dd 100644 --- a/crates/identity/ras-identity-oauth2/src/provider.rs +++ b/crates/identity/ras-identity-oauth2/src/provider.rs @@ -140,7 +140,7 @@ impl OAuth2Provider { ) -> OAuth2Result { // Always bind by default: an unbound flow lets an attacker start a flow // and trick a victim into completing it, joining the attacker's app - // session to the victim's IdP identity (M2). + // session to the victim's IdP identity. let binding = uuid::Uuid::new_v4().to_string(); self.start_flow_bound(provider_id, additional_params, Some(binding)) .await @@ -212,7 +212,7 @@ impl OAuth2Provider { // Bind the userinfo response to the id_token: identity is derived from // userinfo, so a wrong/confused userinfo endpoint must not be able to - // change the account when an id_token established the subject (M6). + // change the account when an id_token established the subject. // Fail closed if the id_token carries no `sub` (validate_id_token_claims // already requires it, but this must never silently pass). When a custom // `subject_field` is configured the resolved identity subject is a @@ -417,13 +417,13 @@ mod tests { assert!(url.contains("response_type=code")); assert!(url.contains("client_id=test_client_id")); assert!(!state.is_empty()); - // Default start_flow always binds (M2). + // Default start_flow always binds. assert!(binding.is_some_and(|b| !b.is_empty())); } _ => panic!("Expected AuthorizationUrl response"), } - // StartFlow payloads are no longer routed through verify() + // Flow initiation returns a URL, so it cannot satisfy identity verification. let payload = serde_json::json!({ "type": "StartFlow", "provider_id": "google", diff --git a/crates/identity/ras-identity-oauth2/src/tests.rs b/crates/identity/ras-identity-oauth2/src/tests.rs index 76d5f6e..4a0172d 100644 --- a/crates/identity/ras-identity-oauth2/src/tests.rs +++ b/crates/identity/ras-identity-oauth2/src/tests.rs @@ -235,14 +235,14 @@ mod integration_tests { assert!(url.contains("/authorize")); assert!(url.contains("response_type=code")); assert!(url.contains("code_challenge")); - // start_flow binds by default (M2). + // start_flow binds by default. assert!(binding.is_some()); (state, binding) } _ => panic!("Expected authorization URL"), }; - // StartFlow payloads are no longer routed through verify() + // Flow initiation returns a URL, so it cannot satisfy identity verification. let start_payload = serde_json::json!({ "type": "StartFlow", "provider_id": "mock_provider" @@ -252,7 +252,7 @@ mod integration_tests { Err(ras_identity_core::IdentityError::UnsupportedMethod) )); - // Simulate callback — echo the binding captured at start (M2). + // Simulate callback — echo the binding captured at start. let callback_payload = serde_json::json!({ "type": "Callback", "provider_id": "mock_provider", diff --git a/crates/identity/ras-identity-session/src/lib.rs b/crates/identity/ras-identity-session/src/lib.rs index c73b50a..516be32 100644 --- a/crates/identity/ras-identity-session/src/lib.rs +++ b/crates/identity/ras-identity-session/src/lib.rs @@ -122,7 +122,7 @@ pub struct SessionConfig { pub iss: Option, /// Expected token audience. Encoded into new tokens and verified on /// `verify_session`; a token for a different `aud` is rejected. This is the - /// cross-service confused-deputy guard (M3). Required unless + /// cross-service confused-deputy guard. Required unless /// `require_iss_aud` is false. pub aud: Option, /// When true (default), validation fails if `iss` or `aud` is `None`. @@ -135,7 +135,7 @@ pub struct SessionConfig { pub max_sessions_per_user: usize, } -/// Redacting `Debug` so `jwt_secret` never lands in logs (L1). +/// Redacting `Debug` so `jwt_secret` never lands in logs. impl std::fmt::Debug for SessionConfig { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("SessionConfig") @@ -615,7 +615,7 @@ impl SessionService { } // Cross-service confused-deputy guard: reject tokens minted for a - // different issuer/audience when this service configures them (M3). + // different issuer/audience when this service configures them. if let Some(expected_iss) = &self.config.iss && claims.iss.as_deref() != Some(expected_iss.as_str()) { @@ -841,7 +841,7 @@ mod tests { #[tokio::test] async fn token_for_one_audience_is_rejected_by_another_service() { - // Two services share a secret but configure different audiences (M3). + // Two services share a secret but configure different audiences. let service_a = SessionService::new(test_config().with_audience("svc-a")).unwrap(); let local = LocalUserProvider::new(); local @@ -872,9 +872,7 @@ mod tests { #[tokio::test] async fn permissions_are_frozen_into_the_token_snapshot() { - // Names the documented freeze behavior (M3): the permission set is - // copied into the JWT at begin_session and returned verbatim on verify; - // it is not reloaded. If a reload is ever added, this test must change. + // Verification returns the permissions captured at session creation. let permissions_provider = Arc::new(StaticPermissions::new(vec!["read".to_string()])); let service = SessionService::new(test_config()) .unwrap() diff --git a/crates/observability/ras-observability-otel/src/lib.rs b/crates/observability/ras-observability-otel/src/lib.rs index 593f037..13218aa 100644 --- a/crates/observability/ras-observability-otel/src/lib.rs +++ b/crates/observability/ras-observability-otel/src/lib.rs @@ -105,7 +105,6 @@ impl UsageTracker for OtelUsageTracker { ) { let user_agent = user_agent(headers); - // Log the request match user { Some(u) => { info!( @@ -128,7 +127,6 @@ impl UsageTracker for OtelUsageTracker { } } - // Record metrics self.metrics.increment_requests_started(context); } } @@ -192,26 +190,20 @@ impl OtelSetupBuilder { /// Build and initialize OpenTelemetry pub fn build(self) -> Result> { - // Create or use existing Prometheus registry let prometheus_registry = self.prometheus_registry.unwrap_or_default(); - // Create Prometheus exporter let prometheus_exporter = opentelemetry_prometheus::exporter() .with_registry(prometheus_registry.clone()) .build()?; - // Build meter provider let meter_provider = SdkMeterProvider::builder() .with_reader(prometheus_exporter) .build(); - // Set as global provider global::set_meter_provider(meter_provider.clone()); - // Create meter let meter = global::meter(self.service_name); - // Create metrics let metrics = Arc::new(OtelMetrics::new(&meter)); Ok(OtelSetup { diff --git a/crates/rest/ras-rest-macro/src/client.rs b/crates/rest/ras-rest-macro/src/client.rs index 9332234..0f0f6e1 100644 --- a/crates/rest/ras-rest-macro/src/client.rs +++ b/crates/rest/ras-rest-macro/src/client.rs @@ -348,12 +348,8 @@ fn generate_client_method_with_timeout( }; } - // Build query-string handling. Required params are always serialized; - // `Option` params are skipped when `None`. Values are run through - // `ras_transport_core::serialize_query_value`, which mirrors reqwest's old - // serde-backed `.query()` behavior: `Vec` produces repeated keys and - // enum serde renames are honored. The collected (decoded) pairs are - // percent-encoded and appended to the URL via `serialize_query_pairs`. + // Preserve repeated keys and serde renames; absent optional parameters + // must not add a query pair. let query_handling = if query_params.is_empty() { quote! {} } else { @@ -440,10 +436,8 @@ fn generate_client_method_with_timeout( let __response = self.transport.execute(__request).await?; let __response = __response.error_for_status().await?; let __bytes = __response.bytes().await?; - // A 204/304 (or otherwise empty) success body deserializes as JSON - // `null` so `Option` / `serde_json::Value` responses resolve to - // `None` / `Null` instead of failing with an EOF error. (The server - // now emits an empty body for 204/304 per RFC 9110.) + // Treat an empty successful response as JSON null so optional + // responses resolve to None rather than a deserialization error. let __result = if __bytes.is_empty() { ::ras_transport_core::deserialize_json(b"null")? } else { diff --git a/crates/rest/ras-rest-macro/src/lib.rs b/crates/rest/ras-rest-macro/src/lib.rs index eed77d4..b190874 100644 --- a/crates/rest/ras-rest-macro/src/lib.rs +++ b/crates/rest/ras-rest-macro/src/lib.rs @@ -315,24 +315,20 @@ fn parse_doc_comment_attr(attr: syn::Attribute, entry_kind: &str) -> syn::Result impl Parse for ServiceDefinition { fn parse(input: syn::parse::ParseStream) -> syn::Result { - // Parse the opening brace let content; syn::braced!(content in input); - // Parse service_name: Ident let _ = content.parse::()?; // "service_name" let _ = content.parse::()?; let service_name = content.parse::()?; let _ = content.parse::()?; - // Parse base_path: "string" let _ = content.parse::()?; // "base_path" let _ = content.parse::()?; let base_path_lit = content.parse::()?; let base_path = base_path_lit.value(); let _ = content.parse::()?; - // Parse optional fields (openapi, serve_docs, docs_path, ui_theme, body_limit) let mut openapi = None; let mut static_hosting = static_hosting::StaticHostingConfig::default(); let mut body_limit = None; @@ -340,7 +336,6 @@ impl Parse for ServiceDefinition { let mut require_json_content_type = true; let mut docs_require_auth = false; - // Parse optional fields while content.peek(Ident) { let field_name = content.fork().parse::()?; @@ -348,7 +343,6 @@ impl Parse for ServiceDefinition { let _ = content.parse::()?; // "openapi" let _ = content.parse::()?; - // Parse openapi value - can be true/false or { output: "path" } if content.peek(syn::LitBool) { let enabled = content.parse::()?; if enabled.value() { @@ -358,7 +352,6 @@ impl Parse for ServiceDefinition { let openapi_content; syn::braced!(openapi_content in content); - // Parse output: "path" let _ = openapi_content.parse::()?; // "output" let _ = openapi_content.parse::()?; let path = openapi_content.parse::()?; @@ -418,7 +411,6 @@ impl Parse for ServiceDefinition { } } - // Parse endpoints: [...] let _ = content.parse::()?; // "endpoints" let _ = content.parse::()?; @@ -430,7 +422,6 @@ impl Parse for ServiceDefinition { let endpoint = endpoints_content.parse::()?; endpoints.push(endpoint); - // Handle optional trailing comma if endpoints_content.peek(Token![,]) { let _ = endpoints_content.parse::()?; } @@ -534,7 +525,6 @@ impl Parse for EndpointDefinition { fn parse(input: syn::parse::ParseStream) -> syn::Result { let docs = parse_doc_comment_attrs(input.call(syn::Attribute::parse_outer)?, "endpoint")?; - // Parse HTTP method (GET, POST, PUT, DELETE, PATCH) let method_ident = input.parse::()?; let method = match method_ident.to_string().as_str() { "GET" => HttpMethod::Get, @@ -550,20 +540,17 @@ impl Parse for EndpointDefinition { } }; - // Parse auth requirement (UNAUTHORIZED, OPTIONAL_AUTH, or WITH_PERMISSIONS([...])) let auth = if input.peek(syn::Ident) { let auth_ident = input.parse::()?; match auth_ident.to_string().as_str() { "UNAUTHORIZED" => AuthRequirement::Unauthorized, "OPTIONAL_AUTH" => AuthRequirement::OptionalAuth, "WITH_PERMISSIONS" => { - // Parse ([...] | [...] | ...) let perms_content; syn::parenthesized!(perms_content in input); let mut permission_groups = Vec::new(); - // Parse first permission group let first_group_content; syn::bracketed!(first_group_content in perms_content); @@ -578,7 +565,6 @@ impl Parse for EndpointDefinition { } permission_groups.push(first_group); - // Parse additional permission groups separated by | while perms_content.peek(Token![|]) { let _ = perms_content.parse::()?; @@ -625,22 +611,18 @@ impl Parse for EndpointDefinition { )); }; - // Parse path with potential path parameters (e.g., users/{id: String}/posts/{post_id: i32}) let (path, path_params, handler_name_parts) = parse_endpoint_path(input)?; - // Parse query parameters if present (? param1:Type & param2:Type) let mut query_params = Vec::new(); if input.peek(Token![?]) { let _ = input.parse::()?; query_params = parse_query_params(input)?; } - // Generate handler name based on method and path let method_str = method.as_str().to_lowercase(); let path_str = handler_name_parts.join("_"); let handler_name = syn::parse_str::(&format!("{}_{}", method_str, path_str))?; - // Parse (RequestType) - optional for GET/DELETE let request_type = if input.peek(syn::token::Paren) { let request_content; syn::parenthesized!(request_content in input); @@ -653,7 +635,6 @@ impl Parse for EndpointDefinition { None }; - // Parse -> ResponseType let _ = input.parse::]>()?; let response_type = input.parse::()?; @@ -805,7 +786,6 @@ fn generate_service_code(service_def: ServiceDefinition) -> syn::Result syn::Result syn::Result {} AuthRequirement::OptionalAuth => { @@ -853,21 +828,18 @@ fn generate_service_code(service_def: ServiceDefinition) -> syn::Result syn::Result syn::Result syn::Result proc_macro2::TokenStream { let body_limit_tokens = effective_body_limit_tokens(endpoint); - // Handle authentication if required match &endpoint.auth { AuthRequirement::Unauthorized => { - // Build argument list for unauthorized endpoint let mut args = Vec::new(); // Opt-in request headers (before path params) @@ -2108,7 +2068,6 @@ fn generate_handler_body( args.push(quote! { headers.clone() }); } - // Add path parameters if endpoint.path_params.len() == 1 { args.push(quote! { path_params }); } else { @@ -2118,13 +2077,11 @@ fn generate_handler_body( } } - // Add query parameters for query_param in &endpoint.query_params { let param_name = &query_param.name; args.push(quote! { query_params.#param_name }); } - // Handle JSON body extraction with error handling let json_handling = if endpoint.request_type.is_some() { args.push(quote! { body }); generate_body_extraction(require_json, &body_limit_tokens, method, path) @@ -2135,14 +2092,12 @@ fn generate_handler_body( quote! { #json_handling - // Call usage tracker if configured (for unauthorized endpoints, headers come from handler params) if let Some(tracker) = &with_usage_tracker { let tracker_headers = ras_auth_core::redact_sensitive_headers_for_auth_transport(&headers, &auth_transport); tracker(&tracker_headers, None, #method, #path).await; } - // Track duration let start_time = std::time::Instant::now(); let result = match service.#handler_name(#(#args),*).await { @@ -2154,7 +2109,6 @@ fn generate_handler_body( Err(rest_error) => { use axum::response::IntoResponse; - // Log internal error if present if let Some(internal) = &rest_error.internal_error { ras_rest_core::tracing::error!(error = ?internal, "Request failed with status {}", rest_error.status); } @@ -2171,7 +2125,6 @@ fn generate_handler_body( }, }; - // Call duration tracker if configured let duration = start_time.elapsed(); if let Some(tracker) = &with_method_duration_tracker { tracker(#method, #path, None, duration).await; @@ -2189,7 +2142,6 @@ fn generate_handler_body( args.push(quote! { headers.clone() }); } - // Add path parameters if endpoint.path_params.len() == 1 { args.push(quote! { path_params }); } else { @@ -2199,13 +2151,11 @@ fn generate_handler_body( } } - // Add query parameters for query_param in &endpoint.query_params { let param_name = &query_param.name; args.push(quote! { query_params.#param_name }); } - // Handle JSON body extraction with error handling let json_handling = if endpoint.request_type.is_some() { args.push(quote! { body }); generate_body_extraction(require_json, &body_limit_tokens, method, path) @@ -2228,14 +2178,12 @@ fn generate_handler_body( #json_handling - // Call usage tracker if configured if let Some(tracker) = &with_usage_tracker { let tracker_headers = ras_auth_core::redact_sensitive_headers_for_auth_transport(&headers, &auth_transport); tracker(&tracker_headers, __ras_caller_user.as_ref(), #method, #path).await; } - // Track duration let start_time = std::time::Instant::now(); let result = match service.#handler_name(#(#args),*).await { @@ -2263,7 +2211,6 @@ fn generate_handler_body( }, }; - // Call duration tracker if configured let duration = start_time.elapsed(); if let Some(tracker) = &with_method_duration_tracker { tracker(#method, #path, __ras_caller_user.as_ref(), duration).await; @@ -2273,7 +2220,6 @@ fn generate_handler_body( } } AuthRequirement::WithPermissions(_) => { - // Build argument list for authenticated endpoint let mut args = vec![quote! { &user }]; // Opt-in request headers (after the user, before path params) @@ -2281,7 +2227,6 @@ fn generate_handler_body( args.push(quote! { headers.clone() }); } - // Add path parameters if endpoint.path_params.len() == 1 { args.push(quote! { path_params }); } else { @@ -2291,13 +2236,11 @@ fn generate_handler_body( } } - // Add query parameters for query_param in &endpoint.query_params { let param_name = &query_param.name; args.push(quote! { query_params.#param_name }); } - // Handle JSON body extraction with error handling let json_handling = if endpoint.request_type.is_some() { args.push(quote! { body }); generate_body_extraction(require_json, &body_limit_tokens, method, path) @@ -2322,14 +2265,12 @@ fn generate_handler_body( // Read and parse the body only after auth has succeeded #json_handling - // Call usage tracker if configured if let Some(tracker) = &with_usage_tracker { let tracker_headers = ras_auth_core::redact_sensitive_headers_for_auth_transport(&headers, &auth_transport); tracker(&tracker_headers, Some(&user), #method, #path).await; } - // Track duration let start_time = std::time::Instant::now(); let result = match service.#handler_name(#(#args),*).await { @@ -2341,7 +2282,6 @@ fn generate_handler_body( Err(rest_error) => { use axum::response::IntoResponse; - // Log internal error if present if let Some(internal) = &rest_error.internal_error { ras_rest_core::tracing::error!(error = ?internal, "Request failed with status {}", rest_error.status); } @@ -2358,7 +2298,6 @@ fn generate_handler_body( }, }; - // Call duration tracker if configured let duration = start_time.elapsed(); if let Some(tracker) = &with_method_duration_tracker { tracker(#method, #path, Some(&user), duration).await; diff --git a/crates/rest/ras-rest-macro/src/openapi.rs b/crates/rest/ras-rest-macro/src/openapi.rs index e92fae6..9575815 100644 --- a/crates/rest/ras-rest-macro/src/openapi.rs +++ b/crates/rest/ras-rest-macro/src/openapi.rs @@ -24,7 +24,6 @@ pub fn generate_openapi_code( ); let endpoint_info_struct_name = quote::format_ident!("{}OpenApiEndpointInfo", service_name); - // Generate the output path based on config let output_path_code = match config { OpenApiConfig::Enabled => { let service_name_lower = service_name.to_string().to_lowercase(); @@ -39,7 +38,6 @@ pub fn generate_openapi_code( } }; - // Collect unique types for schema generation let mut unique_types = std::collections::HashMap::new(); for endpoint in &service_def.endpoints { if let Some(request_type) = &endpoint.request_type { @@ -51,14 +49,12 @@ pub fn generate_openapi_code( let response_type_str = quote!(#response_type).to_string(); unique_types.insert(response_type_str, quote!(#response_type)); - // Add path parameter types for path_param in &endpoint.path_params { let param_type = &path_param.param_type; let param_type_str = quote!(#param_type).to_string(); unique_types.insert(param_type_str, quote!(#param_type)); } - // Add query parameter types for query_param in &endpoint.query_params { let param_type = &query_param.param_type; let param_type_str = quote!(#param_type).to_string(); @@ -89,7 +85,6 @@ pub fn generate_openapi_code( } } - // Helper function to sanitize type names for OpenAPI component names let sanitize_type_name = |type_name: &str| -> String { if type_name == "()" { "Unit".to_string() @@ -105,7 +100,6 @@ pub fn generate_openapi_code( } }; - // Generate schema generation functions let schema_fns: Vec = unique_types .iter() .map(|(type_name, type_tokens)| { @@ -138,7 +132,6 @@ pub fn generate_openapi_code( }) .collect(); - // Generate schema collection code let schema_insertions: Vec = unique_types .keys() .map(|type_name| { @@ -163,7 +156,6 @@ pub fn generate_openapi_code( }) .collect(); - // Generate endpoint info structs let endpoint_infos: Vec = service_def .endpoints .iter() @@ -189,11 +181,9 @@ pub fn generate_openapi_code( let auth_required = matches!(endpoint.auth, AuthRequirement::WithPermissions(_)); // OPTIONAL_AUTH advertises an *optional* security requirement. let auth_optional = matches!(endpoint.auth, AuthRequirement::OptionalAuth); - // Flatten permission groups for OpenAPI documentation let permissions = match &endpoint.auth { AuthRequirement::Unauthorized | AuthRequirement::OptionalAuth => vec![], AuthRequirement::WithPermissions(groups) => { - // For OpenAPI docs, flatten all permission groups into a single list groups.iter().flatten().cloned().collect() } }; @@ -354,11 +344,9 @@ pub fn generate_openapi_code( fn fix_schema_refs(value: &mut serde_json::Value, schemas: &mut serde_json::Map) { match value { serde_json::Value::Object(obj) => { - // Extract nested definitions and move them to top-level schemas if let Some(defs) = obj.remove("definitions") { if let serde_json::Value::Object(defs_obj) = defs { for (name, schema) in defs_obj { - // Recursively fix the definition before adding it let mut schema_copy = schema.clone(); fix_schema_refs(&mut schema_copy, schemas); schemas.insert(name, schema_copy); @@ -366,11 +354,9 @@ pub fn generate_openapi_code( } } - // Extract $defs and move them to top-level schemas if let Some(defs) = obj.remove("$defs") { if let serde_json::Value::Object(defs_obj) = defs { for (name, schema) in defs_obj { - // Recursively fix the definition before adding it let mut schema_copy = schema.clone(); fix_schema_refs(&mut schema_copy, schemas); schemas.insert(name, schema_copy); @@ -378,10 +364,8 @@ pub fn generate_openapi_code( } } - // Fix $ref strings to point to components/schemas if let Some(ref_val) = obj.get_mut("$ref") { if let serde_json::Value::String(ref_str) = ref_val { - // Replace any reference to definitions or $defs with components/schemas if ref_str.starts_with("#/definitions/") { let name = ref_str.trim_start_matches("#/definitions/"); *ref_str = format!("#/components/schemas/{}", name); @@ -395,7 +379,6 @@ pub fn generate_openapi_code( // Remove $schema field as it's not needed in OpenAPI obj.remove("$schema"); - // Recursively process all values for (_, v) in obj.iter_mut() { fix_schema_refs(v, schemas); } @@ -413,24 +396,20 @@ pub fn generate_openapi_code( fn normalize_nullable_properties(value: &mut serde_json::Value) { match value { serde_json::Value::Object(obj) => { - // Process properties object if it exists if let Some(properties) = obj.get_mut("properties") { if let serde_json::Value::Object(props) = properties { for (_, prop_value) in props.iter_mut() { if let serde_json::Value::Object(prop_obj) = prop_value { - // Check if this property has type: ["string", "null"] pattern if let Some(type_val) = prop_obj.get("type") { if let serde_json::Value::Array(type_array) = type_val { if type_array.len() == 2 { let null_value = serde_json::Value::String("null".to_string()); if type_array.contains(&null_value) { - // Find the non-null type let non_null_type = type_array.iter() .find(|t| **t != null_value) .cloned(); if let Some(actual_type) = non_null_type { - // Replace with the non-null type and add nullable: true prop_obj.insert("type".to_string(), actual_type); prop_obj.insert("nullable".to_string(), serde_json::Value::Bool(true)); } @@ -439,18 +418,15 @@ pub fn generate_openapi_code( } } } - // Recursively process nested objects normalize_nullable_properties(prop_value); } } } - // Process definitions object if it exists if let Some(definitions) = obj.get_mut("definitions") { normalize_nullable_properties(definitions); } - // Process any other nested objects for (_, v) in obj.iter_mut() { normalize_nullable_properties(v); } @@ -468,19 +444,16 @@ pub fn generate_openapi_code( fn fix_option_types(value: &mut serde_json::Value) { match value { serde_json::Value::Object(obj) => { - // Fix type: ["string", "null"] pattern if let Some(type_val) = obj.get("type") { if let serde_json::Value::Array(type_array) = type_val { if type_array.len() == 2 { let null_value = serde_json::Value::String("null".to_string()); if type_array.contains(&null_value) { - // Find the non-null type let non_null_type = type_array.iter() .find(|t| **t != null_value) .cloned(); if let Some(actual_type) = non_null_type { - // Replace with the non-null type and add nullable: true obj.insert("type".to_string(), actual_type); obj.insert("nullable".to_string(), serde_json::Value::Bool(true)); } @@ -489,10 +462,8 @@ pub fn generate_openapi_code( } } - // Fix anyOf that includes {"type": "null"} if let Some(any_of) = obj.get_mut("anyOf") { if let serde_json::Value::Array(any_of_array) = any_of { - // Check if this is an Option type pattern (one real type + null) if any_of_array.len() == 2 { let has_null = any_of_array.iter().any(|item| { if let serde_json::Value::Object(item_obj) = item { @@ -506,7 +477,6 @@ pub fn generate_openapi_code( }); if has_null { - // Find the non-null schema let non_null_schema = any_of_array.iter().find(|item| { if let serde_json::Value::Object(item_obj) = item { if let Some(type_val) = item_obj.get("type") { @@ -521,7 +491,6 @@ pub fn generate_openapi_code( }).cloned(); if let Some(schema) = non_null_schema { - // Replace anyOf with the non-null schema and add nullable obj.remove("anyOf"); if let serde_json::Value::Object(schema_obj) = schema { for (key, val) in schema_obj { @@ -535,7 +504,6 @@ pub fn generate_openapi_code( } } - // Recursively process all nested objects for (_, v) in obj.iter_mut() { fix_option_types(v); } @@ -549,7 +517,6 @@ pub fn generate_openapi_code( } } - // Generate schema functions for each type #(#schema_fns)* /// Generate OpenAPI 3.0 document for this service @@ -562,13 +529,10 @@ pub fn generate_openapi_code( #(#endpoint_infos),* ]; - // Generate schemas for all unique types let mut schemas = HashMap::new(); - // Insert all the generated schemas #(#schema_insertions)* - // Fix all schema references and flatten nested definitions let mut final_schemas = serde_json::Map::new(); for (name, mut schema) in schemas { fix_schema_refs(&mut schema, &mut final_schemas); @@ -576,7 +540,6 @@ pub fn generate_openapi_code( final_schemas.insert(name, schema); } - // Group endpoints by path to create OpenAPI paths let mut paths = serde_json::Map::new(); for endpoint in &endpoints { @@ -621,10 +584,8 @@ pub fn generate_openapi_code( } }); - // Add parameters (path and query parameters) let mut parameters = vec![]; - // Add path parameters for (param_name, param_type) in &endpoint.path_params { parameters.push(json!({ "name": param_name, @@ -637,9 +598,7 @@ pub fn generate_openapi_code( })); } - // Add query parameters for (param_name, param_type) in &endpoint.query_params { - // Check if the type is Option to determine if it's required let is_optional = param_type.starts_with("Option_") || param_type.starts_with("Option<") || param_type.starts_with("Option <"); parameters.push(json!({ "name": param_name, @@ -665,7 +624,6 @@ pub fn generate_openapi_code( operation["x-ras-canonical-path"] = json!(endpoint.canonical_path); } - // Add request body for non-GET methods if endpoint.method != "GET" && endpoint.request_type_name != "Unit" { operation["requestBody"] = json!({ "description": "Request body", @@ -680,7 +638,6 @@ pub fn generate_openapi_code( }); } - // Add security requirements if auth is required if endpoint.auth_required { operation["security"] = json!([{ "bearerAuth": [] @@ -698,7 +655,6 @@ pub fn generate_openapi_code( operation["security"] = json!([{}, { "bearerAuth": [] }]); } - // Add the operation to the path item path_item[method_lower] = operation; } @@ -728,7 +684,6 @@ pub fn generate_openapi_code( let doc = #openapi_fn_name(); let output_path = #output_path_code; - // Create parent directories if they don't exist if let Some(parent) = std::path::Path::new(&output_path).parent() { std::fs::create_dir_all(parent)?; } @@ -762,7 +717,6 @@ fn permission_groups_tokens(groups: &[Vec]) -> TokenStream { pub fn generate_schema_impl_checks(service_def: &ServiceDefinition) -> TokenStream { let mut unique_types = HashMap::new(); - // Collect unique request and response types for endpoint in &service_def.endpoints { if let Some(request_type) = &endpoint.request_type { unique_types.insert(quote!(#request_type).to_string(), quote!(#request_type)); @@ -771,13 +725,11 @@ pub fn generate_schema_impl_checks(service_def: &ServiceDefinition) -> TokenStre let response_type = &endpoint.response_type; unique_types.insert(quote!(#response_type).to_string(), quote!(#response_type)); - // Add path parameter types for path_param in &endpoint.path_params { let param_type = &path_param.param_type; unique_types.insert(quote!(#param_type).to_string(), quote!(#param_type)); } - // Add query parameter types for query_param in &endpoint.query_params { let param_type = &query_param.param_type; unique_types.insert(quote!(#param_type).to_string(), quote!(#param_type)); diff --git a/crates/rest/ras-rest-macro/tests/http_integration.rs b/crates/rest/ras-rest-macro/tests/http_integration.rs index 935fa0a..80684d4 100644 --- a/crates/rest/ras-rest-macro/tests/http_integration.rs +++ b/crates/rest/ras-rest-macro/tests/http_integration.rs @@ -1351,7 +1351,7 @@ async fn test_body_is_not_parsed_before_auth() { .await; assert_eq!(response.status_code().as_u16(), 401); - // With valid credentials the malformed body is now parsed and rejected. + // Valid credentials allow body parsing, which rejects the malformed payload. let response = server .post("/api/v1/users") .authorization_bearer("admin-token") diff --git a/crates/rest/ras-rest-macro/tests/xm_feedback_hardening_test.rs b/crates/rest/ras-rest-macro/tests/xm_feedback_hardening_test.rs index bf495fd..ebf779f 100644 --- a/crates/rest/ras-rest-macro/tests/xm_feedback_hardening_test.rs +++ b/crates/rest/ras-rest-macro/tests/xm_feedback_hardening_test.rs @@ -1,15 +1,12 @@ -//! Regression tests for the `rest_service!` hardening prompted by the XM -//! device-integration feedback: +//! HTTP request, response, and service-configuration contracts for `rest_service!`: //! //! * Content-Type enforcement (strict `application/json`, opt-out). //! * `413` vs `400` split for over-limit vs unreadable bodies. -//! * `204 No Content` no longer carries a serialized body. +//! * `204 No Content` responses have an empty body. //! * Per-endpoint `body_limit` override. //! * Opt-in request-header parameter for handlers. //! * Startup assertion when a permissioned service has no auth provider. //! * `docs_require_auth` gate on the docs / openapi routes. -//! -//! Each of these fails against the pre-hardening macro. use axum_test::TestServer; use ras_auth_core::{AuthError, AuthProvider, AuthenticatedUser}; diff --git a/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-client/src/client.rs b/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-client/src/client.rs index 1429557..c46d72d 100644 --- a/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-client/src/client.rs +++ b/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-client/src/client.rs @@ -96,7 +96,6 @@ impl Client { *state = ClientState::Connecting; drop(state); - // Connect transport let mut transport = self.transport.write().await; transport .connect() @@ -104,14 +103,12 @@ impl Client { .map_err(|e| ClientError::connection(format!("Failed to connect: {}", e)))?; drop(transport); - // Set up message handling let (shutdown_tx, shutdown_rx) = oneshot::channel(); let (message_tx, message_rx) = mpsc::channel(self.config.message_buffer_size); *self.shutdown_tx.write().await = Some(shutdown_tx); *self.message_tx.write().await = Some(message_tx); - // Start message handling task self.start_message_handler(message_rx, shutdown_rx).await?; // Wait for the server's ConnectionEstablished message before @@ -158,23 +155,19 @@ impl Client { *state = ClientState::Disconnected; drop(state); - // Send shutdown signal if let Some(shutdown_tx) = self.shutdown_tx.write().await.take() { let _ = shutdown_tx.send(()); } - // Disconnect transport let mut transport = self.transport.write().await; transport .disconnect() .await .map_err(|e| ClientError::connection(format!("Failed to disconnect: {}", e)))?; - // Clear connection state *self.connection_id.write().await = None; *self.message_tx.write().await = None; - // Fail all pending requests let pending_ids: Vec = self .pending_requests .iter() @@ -220,7 +213,6 @@ impl Client { created_at: Instant::now(), }; - // Check if we're over the pending request limit if self.pending_requests.len() >= self.config.max_pending_requests { return Err(ClientError::internal("Too many pending requests")); } @@ -280,7 +272,6 @@ impl Client { self.subscriptions.insert(topic.to_string(), subscription); - // Send subscription message let message = BidirectionalMessage::Subscribe { topics: vec![topic.to_string()], }; @@ -300,7 +291,6 @@ impl Client { self.subscriptions.remove(topic); - // Send unsubscription message let message = BidirectionalMessage::Unsubscribe { topics: vec![topic.to_string()], }; @@ -364,8 +354,6 @@ impl Client { .collect() } - // Internal helper methods - async fn send_message(&self, message: BidirectionalMessage) -> ClientResult<()> { if let Some(tx) = self.message_tx.read().await.as_ref() { tx.send(message) @@ -398,13 +386,11 @@ impl Client { loop { tokio::select! { - // Handle shutdown signal _ = &mut shutdown_rx => { debug!("Message handler received shutdown signal"); break; } - // Handle outgoing messages message = message_rx.recv() => { if let Some(message) = message { let mut transport = transport.write().await; @@ -417,7 +403,6 @@ impl Client { } } - // Handle incoming messages _ = receive_interval.tick() => { let transport_clone = Arc::clone(&transport); let mut transport = transport_clone.write().await; @@ -439,7 +424,6 @@ impl Client { ).await; } Ok(None) => { - // No message available, continue } Err(e) => { error!("Failed to receive message: {}", e); @@ -470,13 +454,11 @@ impl Client { } } BidirectionalMessage::ServerNotification(notification) => { - // Handle notification with registered handlers if let Some(handler) = context.notification_handlers.get(¬ification.method) { handler(¬ification.method, ¬ification.params); } } BidirectionalMessage::Broadcast(broadcast) => { - // Handle broadcast to subscribed topics if let Some(subscription) = context.subscriptions.get(&broadcast.topic) { (subscription.value().handler)(&broadcast.method, &broadcast.params); } @@ -506,13 +488,11 @@ impl Client { .await; } BidirectionalMessage::Request(request) => { - // Handle incoming RPC request from server if let Some(_id) = &request.id { if let Some(handler) = context.rpc_request_handlers.get(&request.method) { debug!("Handling RPC request: {}", request.method); let response = handler(request).await; - // Send response back to server let response_message = BidirectionalMessage::Response(response); let tx = context.message_tx.read().await.clone(); if let Some(tx) = tx @@ -522,7 +502,6 @@ impl Client { } } else { warn!("No handler registered for RPC method: {}", request.method); - // Send method not found error let error_response = JsonRpcResponse::error( ras_jsonrpc_types::JsonRpcError::new( -32601, @@ -837,9 +816,7 @@ mod tests { #[tokio::test] async fn builder_jwt_in_query_params_and_full_setters() { - // Exercise every with_* setter so each path is colored. We don't - // auto-connect (no server), but the resulting config must reflect - // each option. + // Builder options must survive construction without a connection. let custom = ReconnectConfig::default(); let client = ClientBuilder::new("ws://localhost:8080") .with_jwt_token("tok".into()) diff --git a/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-client/src/config.rs b/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-client/src/config.rs index 35f8f4b..eb22790 100644 --- a/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-client/src/config.rs +++ b/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-client/src/config.rs @@ -292,7 +292,7 @@ mod tests { // Second delay should be larger due to backoff assert!(delay2 > delay1); - // Should not exceed max delay (now properly capped) + // Jitter must not exceed the maximum delay. let delay_large = config.calculate_delay(100); assert!( delay_large <= config.max_delay, diff --git a/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-client/src/wasm.rs b/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-client/src/wasm.rs index aa2bd14..ef6d09a 100644 --- a/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-client/src/wasm.rs +++ b/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-client/src/wasm.rs @@ -61,7 +61,6 @@ impl WasmWebSocketTransport { let connection_state = Arc::clone(&self.connection_state); let connect_tx = Arc::new(Mutex::new(Some(connect_tx))); - // Handle connection open { let connection_state = Arc::clone(&connection_state); let connect_tx = Arc::clone(&connect_tx); @@ -75,7 +74,6 @@ impl WasmWebSocketTransport { onopen_callback.forget(); } - // Handle messages { let message_queue = Arc::clone(&message_queue); let onmessage_callback = Closure::wrap(Box::new(move |event: MessageEvent| { @@ -87,7 +85,6 @@ impl WasmWebSocketTransport { onmessage_callback.forget(); } - // Handle errors { let connection_state = Arc::clone(&connection_state); let connect_tx = Arc::clone(&connect_tx); @@ -102,7 +99,6 @@ impl WasmWebSocketTransport { onerror_callback.forget(); } - // Handle connection close { let connection_state = Arc::clone(&connection_state); let connect_tx = Arc::clone(&connect_tx); @@ -125,14 +121,12 @@ impl WasmWebSocketTransport { fn parse_message_event(event: &MessageEvent) -> ClientResult { let data = event.data(); - // Handle text messages if let Some(text) = data.as_string() { let message: BidirectionalMessage = serde_json::from_str(&text).map_err(ClientError::Json)?; return Ok(message); } - // Handle binary messages (ArrayBuffer or Blob) if let Ok(array_buffer) = data.dyn_into::() { let uint8_array = Uint8Array::new(&array_buffer); let bytes = uint8_array.to_vec(); @@ -166,7 +160,6 @@ impl WasmWebSocketTransport { #[async_trait(?Send)] impl WebSocketTransport for WasmWebSocketTransport { async fn connect(&mut self) -> ClientResult<()> { - // Check if already connected { let state = self.connection_state.lock().unwrap(); if matches!( @@ -191,16 +184,12 @@ impl WebSocketTransport for WasmWebSocketTransport { // Set binary type to arraybuffer for better binary message handling websocket.set_binary_type(BinaryType::Arraybuffer); - // Set up connection completion channel let (connect_tx, connect_rx) = oneshot::channel(); - // Set up event handlers self.setup_event_handlers(&websocket, connect_tx)?; - // Store the WebSocket *self.websocket.lock().unwrap() = Some(websocket); - // Wait for connection to complete or fail connect_rx .await .map_err(|_| ClientError::internal("Connection channel closed"))? @@ -212,7 +201,6 @@ impl WebSocketTransport for WasmWebSocketTransport { if let Some(websocket) = websocket { *self.connection_state.lock().unwrap() = WasmConnectionState::Closing; - // Close the WebSocket connection websocket.close().map_err(|e| { ClientError::javascript(format!("Failed to close WebSocket: {:?}", e)) })?; @@ -231,7 +219,6 @@ impl WebSocketTransport for WasmWebSocketTransport { } async fn receive(&mut self) -> ClientResult> { - // Check connection state { let state = self.connection_state.lock().unwrap(); match *state { @@ -248,7 +235,6 @@ impl WebSocketTransport for WasmWebSocketTransport { } } - // Try to get a message from the queue let message = self.message_queue.lock().unwrap().pop_front(); Ok(message) } @@ -273,7 +259,6 @@ impl std::fmt::Debug for WasmWebSocketTransport { } } -// Utility functions for WASM environment pub mod utils { use wasm_bindgen::prelude::*; diff --git a/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-server/src/error.rs b/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-server/src/error.rs index 3cae4df..a45c02a 100644 --- a/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-server/src/error.rs +++ b/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-server/src/error.rs @@ -59,7 +59,7 @@ impl ServerError { /// The [`Display`](std::fmt::Display) impl keeps full detail for server logs; /// this is what goes on the wire (JSON-RPC error message / upgrade HTTP body) /// so handler internals, DSNs, auth specifics, or `AuthError` fields never - /// leak to clients (H3). Mirrors `FileError::client_message`. + /// leak to clients. Mirrors `FileError::client_message`. pub fn client_message(&self) -> &'static str { match self { ServerError::AuthenticationFailed(_) => "Authentication failed", @@ -161,7 +161,7 @@ mod tests { #[test] fn client_message_never_leaks_internal_detail() { - // Handler internals must not reach the client (H3). + // Handler internals must not reach the client. let internal = ServerError::Internal("database password is hunter2".into()); assert_eq!(internal.client_message(), "Internal error"); assert!(!internal.client_message().contains("hunter2")); diff --git a/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-server/src/handler.rs b/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-server/src/handler.rs index fef9acc..e266140 100644 --- a/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-server/src/handler.rs +++ b/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-server/src/handler.rs @@ -77,7 +77,6 @@ pub trait MessageHandler: Send + Sync + 'static { topics: Vec, context: Arc, ) -> ServerResult<()> { - // Default implementation unsubscribes the connection from each requested topic. for topic in topics { context.unsubscribe(&topic).await; } @@ -102,14 +101,12 @@ pub trait MessageHandler: Send + Sync + 'static { /// Handle ping message async fn on_ping(&self, _context: Arc) -> ServerResult<()> { - // Default implementation records the ping at debug level. debug!("Received ping"); Ok(()) } /// Handle pong message async fn on_pong(&self, _context: Arc) -> ServerResult<()> { - // Default implementation records the pong at debug level. debug!("Received pong"); Ok(()) } @@ -455,12 +452,10 @@ impl WebSocketHandler { self.context.id ); - // Notify handler of connection if let Err(e) = self.handler.on_connect(self.context.clone()).await { error!("Error in on_connect handler: {}", e); } - // Send connection established message let established_msg = BidirectionalMessage::ConnectionEstablished { connection_id: self.context.id, }; @@ -516,7 +511,6 @@ impl WebSocketHandler { let idle_deadline = tokio::time::sleep(idle_timeout.unwrap_or(Duration::from_secs(0))); tokio::pin!(idle_deadline); - // Main message handling loop loop { tokio::select! { // Re-validate credentials so revoked/expired tokens are @@ -564,7 +558,6 @@ impl WebSocketHandler { break; } - // Handle incoming WebSocket messages msg = socket.recv() => { if let Some(timeout) = idle_timeout { idle_deadline @@ -589,7 +582,6 @@ impl WebSocketHandler { } } - // Handle outgoing messages msg = self.message_rx.recv() => { match msg { Some(OutboundMessage { message, topic }) => { @@ -624,12 +616,10 @@ impl WebSocketHandler { // Return this connection's subscription slots to the service pool self.context.release_all_subscriptions().await; - // Notify handler of disconnection if let Err(e) = self.handler.on_disconnect(self.context.clone(), None).await { error!("Error in on_disconnect handler: {}", e); } - // Send connection closed message let closed_msg = BidirectionalMessage::ConnectionClosed { connection_id: self.context.id, reason: None, @@ -672,7 +662,6 @@ impl WebSocketHandler { )); } debug!("Received binary message ({} bytes)", data.len()); - // Try to parse as UTF-8 text match String::from_utf8(data) { Ok(text) => self.handle_text_message(text, socket).await, Err(_) => { @@ -706,12 +695,10 @@ impl WebSocketHandler { text: String, socket: &mut S, ) -> ServerResult<()> { - // Try to parse as BidirectionalMessage first if let Ok(msg) = serde_json::from_str::(&text) { return self.handle_bidirectional_message(msg, socket).await; } - // Try to parse as JSON-RPC request if let Ok(request) = serde_json::from_str::(&text) { return self.handle_jsonrpc_request(request, socket).await; } @@ -736,7 +723,6 @@ impl WebSocketHandler { ) -> ServerResult<()> { match msg { BidirectionalMessage::Request(request) => { - // Handle as JSON-RPC request self.handle_jsonrpc_request(request, _socket).await } BidirectionalMessage::Subscribe { topics } => { @@ -807,7 +793,6 @@ impl WebSocketHandler { .await { Ok(Some(response)) => { - // Send response back to client let response_msg = BidirectionalMessage::Response(response); self.send_message(socket, response_msg).await } @@ -852,7 +837,7 @@ fn jsonrpc_error_from_server_error(error: &ServerError) -> JsonRpcError { }; // Send only a generic per-class message; the full error was already logged - // server-side by the caller. Never interpolate handler/AuthError Display (H3). + // server-side by the caller. Never interpolate handler/AuthError Display. JsonRpcError::new(code, error.client_message().to_string(), None) } @@ -867,7 +852,7 @@ mod tests { #[test] fn jsonrpc_error_from_server_error_sends_generic_message_not_handler_detail() { // Handler error carrying a secret -> client sees only a generic message, - // stable code preserved, no data field (H3). + // stable code preserved, no data field. let err = ServerError::Internal("database password is hunter2".into()); let jsonrpc = jsonrpc_error_from_server_error(&err); assert_eq!(jsonrpc.code, error_codes::INTERNAL_ERROR); @@ -1216,7 +1201,7 @@ mod tests { let error = error_response.error.as_ref().expect("JSON-RPC error"); assert_eq!(error.code, ras_jsonrpc_types::error_codes::INVALID_REQUEST); // Message is the generic per-class string; the handler's detail - // ("bad request") stays server-side (H3). + // ("bad request") stays server-side. assert_eq!(error.message, "Invalid request"); let success_response = match &messages[2] { diff --git a/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-server/src/upgrade.rs b/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-server/src/upgrade.rs index 53c37d9..216188f 100644 --- a/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-server/src/upgrade.rs +++ b/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-server/src/upgrade.rs @@ -83,7 +83,7 @@ impl WebSocketUpgrade { Err(e) => { // Log the real error server-side; the HTTP body gets only a // generic per-class message so AuthError internals (required/has - // permission lists, Internal(...) strings) never leak (H3). + // permission lists, Internal(...) strings) never leak. error!("Authentication failed during WebSocket upgrade: {}", e); Err((e.to_status_code(), e.client_message().to_string())) } @@ -133,7 +133,7 @@ fn extract_auth_token_from_headers(headers: &HeaderMap) -> Option { // Only `Authorization: Bearer ` is a bearer token, matching the HTTP // transport (`ras_auth_core::extract_auth_credential`). A raw value or any // other scheme (`Basic ...`) is NOT a token, and a present-but-malformed - // Authorization header does not fall through to a weaker transport (M5). + // Authorization header does not fall through to a weaker transport. if let Some(auth_header) = headers.get("authorization") { let Ok(auth_str) = auth_header.to_str() else { return None; @@ -211,7 +211,7 @@ fn get_header_value(headers: &HeaderMap, name: &str) -> Option { /// These headers are entirely client-controllable and there is no trusted-proxy /// allowlist here, so the result is a *claim*, not a verified address. It is /// exposed as connection metadata only and must never drive an authorization, -/// rate-limit, or audit decision as-is (M5). +/// rate-limit, or audit decision as-is. fn extract_client_ip_from_headers(headers: &HeaderMap) -> Option { let ip_headers = [ "x-forwarded-for", @@ -319,7 +319,7 @@ mod tests { #[test] fn rejects_raw_authorization_value_as_token() { - // A bare value with no `Bearer ` scheme is not a token (M5). + // A bare value with no `Bearer ` scheme is not a token. let mut headers = HeaderMap::new(); headers.insert("authorization", HeaderValue::from_static("raw-token")); @@ -511,7 +511,7 @@ mod tests { .expect_err("auth failure is propagated"); assert_eq!(error.to_status_code(), StatusCode::UNAUTHORIZED); - // Display keeps detail for logs; client_message stays generic (H3). + // Display keeps detail for logs; client_message stays generic. assert_eq!(error.to_string(), "Authentication failed: Token expired"); assert_eq!(error.client_message(), "Authentication failed"); assert_eq!(provider.tokens(), vec!["expired-token".to_string()]); diff --git a/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-server/tests/manager_unit.rs b/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-server/tests/manager_unit.rs index b5121a6..ba44891 100644 --- a/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-server/tests/manager_unit.rs +++ b/crates/rpc/bidirectional/ras-jsonrpc-bidirectional-server/tests/manager_unit.rs @@ -261,7 +261,7 @@ async fn default_impl_is_equivalent_to_new() { #[tokio::test] async fn broadcast_to_full_channel_does_not_block_map_access() { // A slow consumer with a full bounded channel must not wedge the manager: - // if shard guards were held across the send await (the old behavior), + // if shard guards were held across the send await, // the concurrent remove_connection below would deadlock and trip the // timeout. tokio::time::timeout(std::time::Duration::from_secs(5), async { diff --git a/crates/rpc/ras-jsonrpc-core/src/lib.rs b/crates/rpc/ras-jsonrpc-core/src/lib.rs index 368a659..9f1bfe6 100644 --- a/crates/rpc/ras-jsonrpc-core/src/lib.rs +++ b/crates/rpc/ras-jsonrpc-core/src/lib.rs @@ -1,8 +1,7 @@ -//! Core authentication and authorization traits for JSON-RPC services. +//! Runtime facade for generated JSON-RPC services. //! -//! This crate provides the authentication and authorization traits used by the -//! `ras-jsonrpc-macro` procedural macro to generate type-safe JSON-RPC services -//! with axum integration. +//! Re-exports authentication, protocol, version-migration, and tracing APIs so +//! generated server code can use a single runtime dependency. // Re-export authentication types from ras-auth-core pub use ras_auth_core::*; @@ -128,7 +127,7 @@ mod tests { assert_eq!(error.code, error_codes::INSUFFICIENT_PERMISSIONS); assert_eq!(error.message, "Insufficient permissions"); - // `required` is advertised; the caller's grant set is never echoed (M1). + // `required` is advertised; the caller's grant set is never echoed. assert_eq!( error.data, Some(json!({ diff --git a/crates/rpc/ras-jsonrpc-macro/src/lib.rs b/crates/rpc/ras-jsonrpc-macro/src/lib.rs index 0633108..fbcc4d6 100644 --- a/crates/rpc/ras-jsonrpc-macro/src/lib.rs +++ b/crates/rpc/ras-jsonrpc-macro/src/lib.rs @@ -194,17 +194,14 @@ fn parse_doc_comment_attr(attr: syn::Attribute, entry_kind: &str) -> syn::Result impl Parse for ServiceDefinition { fn parse(input: syn::parse::ParseStream) -> syn::Result { - // Parse the opening brace let content; syn::braced!(content in input); - // Parse service_name: Ident let _ = content.parse::()?; // "service_name" let _ = content.parse::()?; let service_name = content.parse::()?; let _ = content.parse::()?; - // Check if openrpc field is present let mut openrpc = None; let mut explorer = None; let mut feature_gated = false; @@ -212,7 +209,6 @@ impl Parse for ServiceDefinition { let mut body_limit = None; let mut docs_require_auth = false; - // Parse optional fields until we hit "methods" while content.peek(Ident) { let field_name = content.fork().parse::()?; if field_name == "methods" { @@ -223,7 +219,6 @@ impl Parse for ServiceDefinition { let _ = content.parse::()?; if field_name == "openrpc" { - // Parse openrpc value - can be true/false or { output: "path" } if content.peek(syn::LitBool) { let enabled = content.parse::()?; if enabled.value() { @@ -233,14 +228,12 @@ impl Parse for ServiceDefinition { let openrpc_content; syn::braced!(openrpc_content in content); - // Parse output: "path" let _ = openrpc_content.parse::()?; // "output" let _ = openrpc_content.parse::()?; let path = openrpc_content.parse::()?; openrpc = Some(OpenRpcConfig::WithPath(path.value())); } } else if field_name == "explorer" { - // Parse explorer value - can be true/false or { path: "/custom-path" } if content.peek(syn::LitBool) { let enabled = content.parse::()?; if enabled.value() { @@ -250,7 +243,6 @@ impl Parse for ServiceDefinition { let explorer_content; syn::braced!(explorer_content in content); - // Parse path: "/custom-path" let _ = explorer_content.parse::()?; // "path" let _ = explorer_content.parse::()?; let path = explorer_content.parse::()?; @@ -278,7 +270,6 @@ impl Parse for ServiceDefinition { let _ = content.parse::()?; } - // Parse methods: [...] let _ = content.parse::()?; // "methods" let _ = content.parse::()?; @@ -290,7 +281,6 @@ impl Parse for ServiceDefinition { let method = methods_content.parse::()?; methods.push(method); - // Handle optional trailing comma if methods_content.peek(Token![,]) { let _ = methods_content.parse::()?; } @@ -313,20 +303,17 @@ impl Parse for MethodDefinition { fn parse(input: syn::parse::ParseStream) -> syn::Result { let docs = parse_doc_comment_attrs(input.call(syn::Attribute::parse_outer)?, "method")?; - // Parse auth requirement (UNAUTHORIZED, OPTIONAL_AUTH, or WITH_PERMISSIONS([...])) let auth = if input.peek(syn::Ident) { let auth_ident = input.parse::()?; match auth_ident.to_string().as_str() { "UNAUTHORIZED" => AuthRequirement::Unauthorized, "OPTIONAL_AUTH" => AuthRequirement::OptionalAuth, "WITH_PERMISSIONS" => { - // Parse ([...] | [...] | ...) let perms_content; syn::parenthesized!(perms_content in input); let mut permission_groups = Vec::new(); - // Parse first permission group let first_group_content; syn::bracketed!(first_group_content in perms_content); @@ -341,7 +328,6 @@ impl Parse for MethodDefinition { } permission_groups.push(first_group); - // Parse additional permission groups separated by | while perms_content.peek(Token![|]) { let _ = perms_content.parse::()?; @@ -388,15 +374,12 @@ impl Parse for MethodDefinition { )); }; - // Parse method name let name = input.parse::()?; - // Parse (RequestType) let request_content; syn::parenthesized!(request_content in input); let request_type = request_content.parse::()?; - // Parse -> ResponseType let _ = input.parse::]>()?; let response_type = input.parse::()?; @@ -521,7 +504,6 @@ fn generate_service_code(service_def: ServiceDefinition) -> syn::Result syn::Result static_hosting::StaticHostingConfig::default(), }; - // JSON-RPC services in this macro expose the explorer next to a single endpoint. - // The static host generator still accepts a base path for future reuse. + // The explorer and RPC endpoint share the service root. static_hosting::generate_static_hosting_code( &explorer_config, &service_def.service_name, @@ -632,7 +613,7 @@ fn generate_server_code(service_def: &ServiceDefinition) -> proc_macro2::TokenSt let explorer_enabled = service_def.explorer.is_some() && service_def.openrpc.is_some(); - // Content-Type gate (#1): reject a non-`application/json` body with 415 before + // Content-Type gate: reject a non-`application/json` body with 415 before // parsing. Requiring `application/json` forces a CORS preflight for // cross-origin requests, closing the simple-request CSRF shape. let content_type_gate = if service_def.require_json_content_type { @@ -670,11 +651,11 @@ fn generate_server_code(service_def: &ServiceDefinition) -> proc_macro2::TokenSt quote! {} }; - // Body-size cap (#6a): apply as a DefaultBodyLimit layer so an over-limit body + // Body-size cap: apply as a DefaultBodyLimit layer so an over-limit body // is rejected by the extractor before the handler runs. let body_limit_value = service_def.body_limit.unwrap_or(DEFAULT_BODY_LIMIT); - // Startup assertion (#6c): a service with any WITH_PERMISSIONS method (or a + // Startup assertion: a service with any WITH_PERMISSIONS method (or a // gated explorer) needs an auth provider, else every such call silently fails // authentication at runtime. Fail the build instead. let any_route_requires_auth = service_def @@ -698,10 +679,8 @@ fn generate_server_code(service_def: &ServiceDefinition) -> proc_macro2::TokenSt quote! {} }; - // Explorer route integration (#4): merge the explorer/openrpc routes, gated - // behind authentication when `docs_require_auth` is set. The default explorer - // routes function stays public and unchanged; gating is applied here (where - // the built service, and thus the auth config, is in scope) via a layer. + // Apply the explorer's auth policy where the service auth configuration + // is available. let explorer_route_integration = if explorer_enabled { let service_name_lower = service_name_str.to_lowercase(); let explorer_routes_fn_str = [&service_name_lower, "_explorer_routes"].concat(); @@ -759,7 +738,6 @@ fn generate_server_code(service_def: &ServiceDefinition) -> proc_macro2::TokenSt quote! {} }; - // Generate trait methods let trait_methods = service_def.methods.iter().map(|method| { let method_name = &method.name; let request_type = &method.request_type; @@ -806,7 +784,6 @@ fn generate_server_code(service_def: &ServiceDefinition) -> proc_macro2::TokenSt quote! { matches!(request.method.as_str(), #(#optional_auth_wire_names)|*) } }; - // Generate method dispatch logic for the JSON-RPC handler let method_dispatch = service_def .methods .iter() @@ -969,20 +946,17 @@ fn generate_server_code(service_def: &ServiceDefinition) -> proc_macro2::TokenSt let mut router = axum::Router::new(); - // Add the JSON-RPC endpoint router = router.route(&base_url, rpc_handler); - // Bound the request body size for the JSON-RPC endpoint (#6a). + // Bound the request body size for the JSON-RPC endpoint. router = router.layer(axum::extract::DefaultBodyLimit::max(#body_limit_value)); - // Include explorer routes if explorer is enabled #explorer_route_integration Ok(router) } async fn handle_request(&self, headers: axum::http::HeaderMap, body: String) -> ras_jsonrpc_types::JsonRpcResponse { - // Parse JSON-RPC request let request: ras_jsonrpc_types::JsonRpcRequest = match serde_json::from_str(&body) { Ok(req) => req, Err(__ras_json_err) => { @@ -1000,7 +974,6 @@ fn generate_server_code(service_def: &ServiceDefinition) -> proc_macro2::TokenSt let request_id = request.id.clone(); - // Validate JSON-RPC version if request.jsonrpc != "2.0" { return ras_jsonrpc_types::JsonRpcResponse::error(ras_jsonrpc_types::JsonRpcError::invalid_request(), request_id); } @@ -1059,7 +1032,6 @@ fn generate_server_code(service_def: &ServiceDefinition) -> proc_macro2::TokenSt } }; - // Call usage tracker if configured if let Some(tracker) = &self.usage_tracker { let user_ref = authenticated_user.as_ref(); let tracker_headers = @@ -1067,7 +1039,6 @@ fn generate_server_code(service_def: &ServiceDefinition) -> proc_macro2::TokenSt tracker(&tracker_headers, user_ref, &request).await; } - // Dispatch method match request.method.as_str() { #(#method_dispatch)* _ => ras_jsonrpc_types::JsonRpcResponse::error( @@ -1136,7 +1107,7 @@ fn jsonrpc_auth_check_code( let provider = self.auth_provider.as_ref().expect("auth provider required for WITH_PERMISSIONS methods"); if let Err(error) = ras_jsonrpc_core::check_permission_groups(provider.as_ref(), user, &required_permission_groups) { // Only `required` is surfaced to the client; the caller's - // full grant set (`has`) stays server-side (M1). + // full grant set (`has`) stays server-side. let required = match error { ras_jsonrpc_core::AuthError::InsufficientPermissions { required, .. } => required, _ => Vec::new(), diff --git a/crates/rpc/ras-jsonrpc-macro/src/openrpc.rs b/crates/rpc/ras-jsonrpc-macro/src/openrpc.rs index fd71564..b83bbaa 100644 --- a/crates/rpc/ras-jsonrpc-macro/src/openrpc.rs +++ b/crates/rpc/ras-jsonrpc-macro/src/openrpc.rs @@ -24,7 +24,6 @@ pub fn generate_openrpc_code( ); let method_info_struct_name = quote::format_ident!("{}OpenRpcMethodInfo", service_name); - // Generate the output path based on config let output_path_code = match config { OpenRpcConfig::Enabled => { let service_name_lower = service_name.to_string().to_lowercase(); @@ -39,7 +38,6 @@ pub fn generate_openrpc_code( } }; - // Generate unique function names for each service let flatten_fn_name = quote::format_ident!( "_flatten_schema_defs_{}", service_name.to_string().to_lowercase() @@ -53,7 +51,6 @@ pub fn generate_openrpc_code( service_name.to_string().to_lowercase() ); - // Collect unique types for schema generation let mut unique_types = std::collections::HashMap::new(); for method in &service_def.methods { let request_type = &method.request_type; @@ -76,7 +73,6 @@ pub fn generate_openrpc_code( } } - // Generate schema generation functions let schema_fns: Vec = unique_types .iter() .map(|(type_name, type_tokens)| { @@ -102,7 +98,6 @@ pub fn generate_openrpc_code( }) }); - // Extract $defs and flatten them let mut extracted_defs = std::collections::HashMap::new(); let flattened_schema = #flatten_fn_name(schema_value, &mut extracted_defs); (flattened_schema, extracted_defs) @@ -112,7 +107,6 @@ pub fn generate_openrpc_code( }) .collect(); - // Generate schema collection code let schema_insertions: Vec = unique_types .keys() .map(|type_name| { @@ -135,12 +129,9 @@ pub fn generate_openrpc_code( ); quote! { let (schema, defs) = #fn_name(); - // Sanitize the type name by removing spaces let sanitized_name = #type_name.to_string().replace(" ", ""); schemas.insert(sanitized_name, schema); - // Merge extracted defs into the main schemas collection for (def_name, def_schema) in defs { - // Also sanitize def names let sanitized_def_name = def_name.replace(" ", ""); schemas.insert(sanitized_def_name, def_schema); } @@ -149,7 +140,6 @@ pub fn generate_openrpc_code( }) .collect(); - // Generate method info structs let method_infos: Vec = service_def .methods .iter() @@ -165,11 +155,9 @@ pub fn generate_openrpc_code( }; let auth_required = matches!(method.auth, AuthRequirement::WithPermissions(_)); let auth_optional = matches!(method.auth, AuthRequirement::OptionalAuth); - // Flatten permission groups for OpenRPC documentation let permissions = match &method.auth { AuthRequirement::Unauthorized | AuthRequirement::OptionalAuth => vec![], AuthRequirement::WithPermissions(groups) => { - // For OpenRPC docs, flatten all permission groups into a single list groups.iter().flatten().cloned().collect() } }; @@ -266,18 +254,15 @@ pub fn generate_openrpc_code( extracted_defs: &mut std::collections::HashMap ) -> serde_json::Value { if let Some(obj) = schema.as_object_mut() { - // If this schema has $defs, extract them if let Some(defs) = obj.remove("$defs") { if let Some(defs_obj) = defs.as_object() { for (def_name, def_schema) in defs_obj { - // Recursively flatten nested $defs in the extracted definitions let flattened_def = #flatten_fn_name(def_schema.clone(), extracted_defs); extracted_defs.insert(def_name.clone(), flattened_def); } } } - // Update all $ref paths to point to components/schemas #update_refs_fn_name(&mut schema); } @@ -313,7 +298,6 @@ pub fn generate_openrpc_code( /// Generate example value from schema fn #generate_example_fn_name(schema: &serde_json::Value, schemas: &std::collections::HashMap) -> serde_json::Value { - // Check if schema has examples field if let Some(examples) = schema.get("examples") { if let Some(arr) = examples.as_array() { if let Some(first) = arr.first() { @@ -322,12 +306,10 @@ pub fn generate_openrpc_code( } } - // Check if schema has example field (singular) if let Some(example) = schema.get("example") { return example.clone(); } - // Check for $ref if let Some(ref_str) = schema.get("$ref").and_then(|v| v.as_str()) { if let Some(ref_name) = ref_str.strip_prefix("#/components/schemas/") { if let Some(ref_schema) = schemas.get(ref_name) { @@ -348,7 +330,6 @@ pub fn generate_openrpc_code( } } - // Generate based on type match schema.get("type").and_then(|v| v.as_str()) { Some("string") => serde_json::json!("example_string"), Some("number") | Some("integer") => serde_json::json!(42), @@ -376,7 +357,6 @@ pub fn generate_openrpc_code( } } - // Generate schema functions for each type #(#schema_fns)* /// Generate OpenRPC document for this service @@ -389,20 +369,15 @@ pub fn generate_openrpc_code( #(#method_infos),* ]; - // Generate schemas for all unique types let mut schemas = HashMap::new(); - // Insert all the generated schemas #(#schema_insertions)* let openrpc_methods: Vec = methods.iter().map(|method| { let mut params = vec![]; - // Add request parameter only if not unit type if method.request_type_name != "()" { - // Sanitize the type name for schema reference let sanitized_request_type = method.request_type_name.replace(" ", ""); - // Get the schema for the request type to generate an example let example = if let Some(schema) = schemas.get(&sanitized_request_type) { #generate_example_fn_name(schema, &schemas) } else { @@ -451,14 +426,11 @@ pub fn generate_openrpc_code( extensions.insert("x-ras-canonical-method".to_string(), json!(method.canonical_method)); } - // Generate example pairing for the method let mut examples = vec![]; if method.request_type_name != "()" { - // Sanitize type names for schema lookups let sanitized_request_type = method.request_type_name.replace(" ", ""); let sanitized_response_type = method.response_type_name.replace(" ", ""); - // Get the schema for the request type to generate an example let request_example = if let Some(schema) = schemas.get(&sanitized_request_type) { #generate_example_fn_name(schema, &schemas) } else { @@ -483,7 +455,6 @@ pub fn generate_openrpc_code( })); } - // Sanitize the response type name for schema reference let sanitized_response_type = method.response_type_name.replace(" ", ""); let method_summary = method .summary @@ -506,7 +477,6 @@ pub fn generate_openrpc_code( // Note: Examples are intentionally omitted as they're optional in OpenRPC // and can cause validation issues with some validators - // Add extensions to the method object if let Some(obj) = method_obj.as_object_mut() { if let Some(description) = &method.description { obj.insert("description".to_string(), json!(description)); @@ -573,7 +543,6 @@ pub fn generate_openrpc_code( let doc = #openrpc_fn_name(); let output_path = #output_path_code; - // Create parent directories if they don't exist if let Some(parent) = std::path::Path::new(&output_path).parent() { std::fs::create_dir_all(parent)?; } @@ -606,7 +575,6 @@ fn permission_groups_tokens(groups: &[Vec]) -> TokenStream { pub fn generate_schema_impl_checks(service_def: &ServiceDefinition) -> TokenStream { let mut unique_types = HashMap::new(); - // Collect unique request and response types for method in &service_def.methods { let request_type = &method.request_type; let response_type = &method.response_type; diff --git a/crates/rpc/ras-jsonrpc-macro/tests/http_integration.rs b/crates/rpc/ras-jsonrpc-macro/tests/http_integration.rs index b253053..a192b1a 100644 --- a/crates/rpc/ras-jsonrpc-macro/tests/http_integration.rs +++ b/crates/rpc/ras-jsonrpc-macro/tests/http_integration.rs @@ -263,8 +263,7 @@ fn create_test_server() -> axum_test::TestServer { .unwrap() } -// `auth_cookie` now always installs a default double-submit CSRF config (H2), -// so there is no cookie-without-CSRF server to construct. +// Cookie auth installs double-submit CSRF protection by default. fn create_cookie_test_server() -> axum_test::TestServer { let builder = TestServiceBuilder::new(TestServiceImpl) .base_url("/rpc") @@ -432,7 +431,7 @@ async fn test_cookie_auth_coexists_with_bearer_tokens() { "id": 1 }); - // Cookie auth on a POST now requires the double-submit CSRF header (H2). + // Cookie auth on a POST requires the double-submit CSRF header. let response: Value = server .post("/rpc") .add_header( diff --git a/crates/rpc/ras-jsonrpc-macro/tests/http_status_codes_test.rs b/crates/rpc/ras-jsonrpc-macro/tests/http_status_codes_test.rs index 91fe277..b93f394 100644 --- a/crates/rpc/ras-jsonrpc-macro/tests/http_status_codes_test.rs +++ b/crates/rpc/ras-jsonrpc-macro/tests/http_status_codes_test.rs @@ -190,7 +190,7 @@ async fn test_403_does_not_leak_callers_permission_set() { let app = test_app(); // A user holding an internal permission probes an admin method. The 403 must - // not echo back the caller's grant set (M1) — `hidden:internal` must not + // not echo back the caller's grant set — `hidden:internal` must not // appear anywhere in the response body. let response = make_jsonrpc_request( app.clone(), diff --git a/crates/rpc/ras-jsonrpc-macro/tests/xm_feedback_parity_test.rs b/crates/rpc/ras-jsonrpc-macro/tests/xm_feedback_parity_test.rs index 79f895e..cb8e160 100644 --- a/crates/rpc/ras-jsonrpc-macro/tests/xm_feedback_parity_test.rs +++ b/crates/rpc/ras-jsonrpc-macro/tests/xm_feedback_parity_test.rs @@ -1,5 +1,4 @@ -//! Regression tests for the `jsonrpc_service!` hardening that brings it to parity -//! with the `rest_service!` changes prompted by the XM device-integration feedback: +//! HTTP request and service-configuration contracts for `jsonrpc_service!`: //! //! * Content-Type enforcement (strict `application/json`, opt-out). //! * Service-level `body_limit`. diff --git a/crates/rpc/ras-jsonrpc-types/src/lib.rs b/crates/rpc/ras-jsonrpc-types/src/lib.rs index 131e14b..137b008 100644 --- a/crates/rpc/ras-jsonrpc-types/src/lib.rs +++ b/crates/rpc/ras-jsonrpc-types/src/lib.rs @@ -189,7 +189,7 @@ impl JsonRpcError { /// Only `required` is included in the error data so clients know what to /// request. The caller's actual grant set is deliberately never echoed back /// — surfacing it would let any authenticated user enumerate their own (and - /// internal) permission names by probing privileged methods (M1). + /// internal) permission names by probing privileged methods. pub fn insufficient_permissions(required: Vec) -> Self { Self::new( error_codes::INSUFFICIENT_PERMISSIONS, @@ -284,7 +284,7 @@ mod tests { assert_eq!(err.code, error_codes::INSUFFICIENT_PERMISSIONS); let data = err.data.unwrap(); assert_eq!(data["required"], serde_json::json!(["admin"])); - // The caller's grant set must never be surfaced (M1). + // The caller's grant set must never be surfaced. assert!(data.get("has").is_none()); } diff --git a/crates/specs/ras-openrpc-types/src/validation.rs b/crates/specs/ras-openrpc-types/src/validation.rs index d2cda11..c4111d3 100644 --- a/crates/specs/ras-openrpc-types/src/validation.rs +++ b/crates/specs/ras-openrpc-types/src/validation.rs @@ -37,14 +37,13 @@ impl ValidateUnique for Vec { } } -/// Validate URL format +/// Check for a nonempty URL-like value with a scheme separator, `/`, or `localhost` prefix. +/// This is a shape check, not a full URL parser. pub fn validate_url(url: &str) -> OpenRpcResult<()> { - // Basic URL validation - checks for scheme presence if url.is_empty() { return Err(OpenRpcError::invalid_url("URL cannot be empty")); } - // Check if it looks like a URL (has scheme or is relative) if !url.contains("://") && !url.starts_with('/') && !url.starts_with("localhost") { return Err(OpenRpcError::invalid_url(format!( "Invalid URL format: {}", @@ -55,13 +54,13 @@ pub fn validate_url(url: &str) -> OpenRpcResult<()> { Ok(()) } -/// Validate email format +/// Check for nonempty local/domain parts and a dot in the domain. +/// This does not validate the full email address grammar. pub fn validate_email(email: &str) -> OpenRpcResult<()> { if email.is_empty() { return Err(OpenRpcError::invalid_email("Email cannot be empty")); } - // Basic email validation - must contain @ and domain part if !email.contains('@') || email.starts_with('@') || email.ends_with('@') { return Err(OpenRpcError::invalid_email(format!( "Invalid email format: {}", @@ -77,7 +76,6 @@ pub fn validate_email(email: &str) -> OpenRpcResult<()> { ))); } - // Domain must contain at least one dot if !parts[1].contains('.') { return Err(OpenRpcError::invalid_email(format!( "Invalid email domain: {}", @@ -88,13 +86,13 @@ pub fn validate_email(email: &str) -> OpenRpcResult<()> { Ok(()) } -/// Validate semver version format +/// Check that the first two dot-separated version components are `u32` values. +/// Remaining components are unchecked; this is not full SemVer validation. pub fn validate_semver(version: &str) -> OpenRpcResult<()> { if version.is_empty() { return Err(OpenRpcError::validation("Version cannot be empty")); } - // Basic semver pattern check: major.minor.patch let parts: Vec<&str> = version.split('.').collect(); if parts.len() < 2 { return Err(OpenRpcError::validation(format!( @@ -103,7 +101,6 @@ pub fn validate_semver(version: &str) -> OpenRpcResult<()> { ))); } - // Check that major and minor are numeric for (i, part) in parts.iter().take(2).enumerate() { if part.parse::().is_err() { let component = if i == 0 { "major" } else { "minor" }; @@ -147,9 +144,8 @@ pub fn validate_component_key(key: &str) -> OpenRpcResult<()> { Ok(()) } -/// Validate JSON-RPC error code (must be integer, certain ranges reserved) +/// Reject application error codes in the reserved range `-32768..=-32000`. pub fn validate_error_code(code: i64) -> OpenRpcResult<()> { - // Pre-defined error codes are reserved: -32768 to -32000 if (-32768..=-32000).contains(&code) { return Err(OpenRpcError::validation(format!( "Error code {} is in reserved range (-32768 to -32000)", @@ -171,14 +167,13 @@ pub fn validate_param_structure(param_structure: &str) -> OpenRpcResult<()> { } } -/// Validate method name (must be unique within methods array) +/// Reject empty names, spaces, and the reserved `rpc.` prefix except `rpc.discover`. +/// Collection-level validation handles uniqueness. pub fn validate_method_name(name: &str) -> OpenRpcResult<()> { if name.is_empty() { return Err(OpenRpcError::validation("Method name cannot be empty")); } - // Method names should be valid JSON-RPC method names - // Basic validation - no spaces, not starting with rpc. unless it's rpc.discover if name.contains(' ') { return Err(OpenRpcError::validation(format!( "Method name '{}' cannot contain spaces", @@ -196,7 +191,8 @@ pub fn validate_method_name(name: &str) -> OpenRpcResult<()> { Ok(()) } -/// Validate content descriptor name (must be unique within params array when by-name) +/// Reject empty content descriptor names and names containing spaces. +/// This helper does not check uniqueness within a parameter collection. pub fn validate_content_descriptor_name(name: &str) -> OpenRpcResult<()> { if name.is_empty() { return Err(OpenRpcError::validation( @@ -204,7 +200,6 @@ pub fn validate_content_descriptor_name(name: &str) -> OpenRpcResult<()> { )); } - // Names should be valid parameter names (no spaces, valid identifier-like) if name.contains(' ') { return Err(OpenRpcError::validation(format!( "Content descriptor name '{}' cannot contain spaces", diff --git a/examples/bidirectional-chat/server/src/config.rs b/examples/bidirectional-chat/server/src/config.rs index 6a292f9..428bd78 100644 --- a/examples/bidirectional-chat/server/src/config.rs +++ b/examples/bidirectional-chat/server/src/config.rs @@ -419,7 +419,7 @@ impl Config { /// Apply direct environment variable overrides fn apply_env_overrides(&mut self) -> Result<()> { - // Handle legacy environment variables for backward compatibility + // Deployment variables override the structured configuration. if let Ok(host) = std::env::var("HOST") { info!("Using HOST environment variable"); self.server.host = host.parse().context("Invalid HOST value")?; diff --git a/examples/bidirectional-chat/server/src/main.rs b/examples/bidirectional-chat/server/src/main.rs index caa715c..1b2b503 100644 --- a/examples/bidirectional-chat/server/src/main.rs +++ b/examples/bidirectional-chat/server/src/main.rs @@ -2350,10 +2350,7 @@ mod tests { response_by_id(&messages, "send-before-join").expect("send_message error response"); let error = error_response.error.as_ref().expect("send_message error"); assert_eq!(error.code, ras_jsonrpc_types::error_codes::INTERNAL_ERROR); - // Handler error detail is no longer forwarded to the client (H3); the - // wire message is generic and the real reason is logged server-side. - // (A production app should return client-facing errors in an Ok response - // rather than via a handler `Err`.) + // Handler errors expose a generic message; details stay in server logs. assert_eq!(error.message, "Internal error"); let join_response = @@ -2408,8 +2405,7 @@ mod tests { let second_send = response_by_id(&messages, "send-2").expect("second send response"); let error = second_send.error.as_ref().expect("rate limit error"); assert_eq!(error.code, ras_jsonrpc_types::error_codes::INTERNAL_ERROR); - // Handler error detail (the rate-limit reason) is sanitized on the wire - // (H3) and logged server-side instead. + // The rate-limit reason stays in server logs. assert_eq!(error.message, "Internal error"); let after_limit = diff --git a/examples/bidirectional-chat/server/tests/auth_lifecycle_tests.rs b/examples/bidirectional-chat/server/tests/auth_lifecycle_tests.rs index 3d44452..52120b3 100644 --- a/examples/bidirectional-chat/server/tests/auth_lifecycle_tests.rs +++ b/examples/bidirectional-chat/server/tests/auth_lifecycle_tests.rs @@ -1,7 +1,7 @@ -//! Chat server auth and lifecycle integration tests +//! Auth and lifecycle tests for a locally wired chat service fixture. //! //! These tests cover: -//! - In-memory server startup and health checks +//! - In-memory fixture startup and health checks //! - Login and registration flows //! - Permission-bearing session creation //! - Concurrent login handling diff --git a/examples/bidirectional-chat/server/tests/server_tests.rs b/examples/bidirectional-chat/server/tests/server_tests.rs index 261e281..7204cf5 100644 --- a/examples/bidirectional-chat/server/tests/server_tests.rs +++ b/examples/bidirectional-chat/server/tests/server_tests.rs @@ -1,9 +1,6 @@ -//! Integration tests for the bidirectional chat server +//! Chat configuration, persistence, and health-router fixture tests. //! -//! These tests cover: -//! - Server startup and health checks -//! - Configuration validation -//! - Persistence behavior +//! The health fixture does not construct the application server. use anyhow::Result; use axum::Router; @@ -22,7 +19,7 @@ struct TestServer { } impl TestServer { - /// Start a test server with the given configuration + /// Start the isolated health router; application configuration is unused. async fn start(_config: Config) -> Result { let health_router = Router::new().route("/health", axum::routing::get(|| async { "OK" })); diff --git a/examples/file-service-wasm/file-service-backend/src/storage.rs b/examples/file-service-wasm/file-service-backend/src/storage.rs index 31fe14c..a1d6185 100644 --- a/examples/file-service-wasm/file-service-backend/src/storage.rs +++ b/examples/file-service-wasm/file-service-backend/src/storage.rs @@ -285,7 +285,7 @@ mod tests { .await .expect("save file"); - // A truncated id must not match by prefix (the old behavior). + // A truncated id must not match by prefix. let prefix = &saved.id[..8]; assert!(storage.get_file(prefix, None).await.is_err()); diff --git a/examples/oauth2-demo/server/src/main.rs b/examples/oauth2-demo/server/src/main.rs index 115446f..d7db52e 100644 --- a/examples/oauth2-demo/server/src/main.rs +++ b/examples/oauth2-demo/server/src/main.rs @@ -100,7 +100,7 @@ pub struct CallbackQuery { /// /// Deliberately carries no client-controlled parameter map: forwarding an /// arbitrary map into the authorize URL let a caller inject reserved OAuth -/// parameters (H1/H4). Any extra IdP parameters are hardcoded server-side in +/// parameters. Any extra IdP parameters are hardcoded server-side in /// `create_oauth2_provider`. #[derive(Debug, Serialize, Deserialize)] pub struct StartOAuth2Request { @@ -192,7 +192,7 @@ fn create_session_service(config: &AppConfig) -> Result { let session_config = SessionConfig { jwt_secret: config.jwt_secret.clone(), jwt_ttl: chrono::Duration::hours(24), - // Enabled so logout/revocation actually invalidates a session (H4). + // Enabled so logout/revocation actually invalidates a session. enforce_active_sessions: true, algorithm: JwtAlgorithm::HS256, iss: Some("oauth2-demo".to_string()), @@ -217,7 +217,7 @@ async fn index_handler() -> Html<&'static str> { /// Handler to start the OAuth2 flow. /// /// `start_flow` generates a login-CSRF binding; we store it in an HttpOnly -/// cookie that the browser returns on the same-site callback (M2/H4). +/// cookie that the browser returns on the same-site callback. async fn start_oauth2_handler( State(state): State, Json(request): Json, @@ -285,7 +285,7 @@ async fn oauth2_callback_handler( .state .ok_or_else(|| "Missing state parameter in callback".to_string())?; - // Echo the login-CSRF binding read back from the cookie (M2/H4). + // Echo the login-CSRF binding read back from the cookie. let binding = read_binding_cookie(&headers); // Complete the OAuth2 flow @@ -315,7 +315,7 @@ async fn oauth2_callback_handler( // never sent to the server (no access-log entry) and are not included in the // `Referer` header. success.html moves it into sessionStorage and clears the // fragment immediately. A production app should prefer a Set-Cookie session - // (which the library now pairs with CSRF) over any URL-based delivery. + // with CSRF protection over any URL-based delivery. Ok(( response_headers, Redirect::to(&format!("/success#token={}", token)), @@ -404,7 +404,7 @@ async fn main() -> Result<()> { .route("/api-docs", get(api_docs_handler)) .nest_service("/static", ServeDir::new("../static")) .layer( - // Restrict CORS to the demo's own origin rather than `Any` (H4). + // Restrict CORS to the demo's own origin rather than `Any`. CorsLayer::new() .allow_origin(HeaderValue::from_static(DEMO_ORIGIN)) .allow_methods([Method::GET, Method::POST]) @@ -467,7 +467,7 @@ mod static_page_tests { #[test] fn success_page_reads_token_from_fragment_and_clears_url() { // Token is read from the URL fragment (never sent to the server) and the - // URL is scrubbed immediately (H4). + // URL is scrubbed immediately. assert!(SUCCESS_HTML.contains("window.location.hash")); assert!(SUCCESS_HTML.contains("history.replaceState")); // It must NOT read the token from the query string anymore. diff --git a/examples/oauth2-demo/server/src/permissions.rs b/examples/oauth2-demo/server/src/permissions.rs index eafafcc..1e235fa 100644 --- a/examples/oauth2-demo/server/src/permissions.rs +++ b/examples/oauth2-demo/server/src/permissions.rs @@ -31,7 +31,7 @@ impl GoogleOAuth2Permissions { // Only an IdP-verified email may drive privilege decisions. An // unverified email address is attacker-controllable (the IdP never - // confirmed the user owns it), so it must never grant admin (H4/M6). + // confirmed the user owns it), so it must never grant admin. let email_verified = identity .metadata .as_ref() @@ -238,7 +238,7 @@ mod tests { #[tokio::test] async fn unverified_admin_email_is_not_granted_admin() { - // The security fix (H4/M6): an UNVERIFIED @example.com address must not + // An unverified @example.com address must not // receive admin, since the IdP never confirmed the user owns it. let provider = GoogleOAuth2Permissions::new(); let identity = create_test_identity("42", Some("attacker@example.com"), Some(false));