refactor(maestro): share cookie name + extraction across CLI and registry (NAP-273 F)
ci / rust (push) Successful in 6m45s
ci / rust (push) Successful in 6m45s
Promote `COOKIE_NAME` and a robust `extract_cookie` (iterates every `;`-pair, so it works on both `Cookie:` and `Set-Cookie:` headers regardless of position) into `maestro-protocol`. The registry re-exports them; the CLI drops its weaker `extract_cookie_value` (which only inspected the first pair) and its duplicate `COOKIE_NAME`. Both ends of the wire now agree on one definition. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -5,6 +5,7 @@ use std::path::PathBuf;
|
|||||||
use std::sync::{Arc, Mutex};
|
use std::sync::{Arc, Mutex};
|
||||||
use std::time::Duration;
|
use std::time::Duration;
|
||||||
|
|
||||||
|
use maestro_protocol::{extract_cookie, COOKIE_NAME};
|
||||||
use reqwest::header::{HeaderMap, HeaderValue, COOKIE, SET_COOKIE};
|
use reqwest::header::{HeaderMap, HeaderValue, COOKIE, SET_COOKIE};
|
||||||
use reqwest::{Method, Response, StatusCode};
|
use reqwest::{Method, Response, StatusCode};
|
||||||
use serde::de::DeserializeOwned;
|
use serde::de::DeserializeOwned;
|
||||||
@@ -12,8 +13,6 @@ use serde::Serialize;
|
|||||||
|
|
||||||
use crate::config::{CliConfig, ConfigError};
|
use crate::config::{CliConfig, ConfigError};
|
||||||
|
|
||||||
const COOKIE_NAME: &str = "maestro_session";
|
|
||||||
|
|
||||||
/// Time budget for establishing a TCP+TLS connection to the registry/daemon.
|
/// Time budget for establishing a TCP+TLS connection to the registry/daemon.
|
||||||
const CONNECT_TIMEOUT: Duration = Duration::from_secs(5);
|
const CONNECT_TIMEOUT: Duration = Duration::from_secs(5);
|
||||||
/// Total time budget for a single request (connect + send + receive). Bounds
|
/// Total time budget for a single request (connect + send + receive). Bounds
|
||||||
@@ -124,7 +123,7 @@ impl RegistryClient {
|
|||||||
let Some(raw) = headers.get(SET_COOKIE).and_then(|h| h.to_str().ok()) else {
|
let Some(raw) = headers.get(SET_COOKIE).and_then(|h| h.to_str().ok()) else {
|
||||||
return Ok(());
|
return Ok(());
|
||||||
};
|
};
|
||||||
let Some(val) = extract_cookie_value(raw, COOKIE_NAME) else {
|
let Some(val) = extract_cookie(raw, COOKIE_NAME) else {
|
||||||
return Ok(());
|
return Ok(());
|
||||||
};
|
};
|
||||||
let mut guard = self.cookie.lock().unwrap();
|
let mut guard = self.cookie.lock().unwrap();
|
||||||
@@ -178,32 +177,10 @@ async fn ok_or_body(resp: Response) -> Result<Response, HttpError> {
|
|||||||
Err(HttpError::ServerError { status, body })
|
Err(HttpError::ServerError { status, body })
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Extract `name=value` from a `Set-Cookie:` value. Ignores attributes.
|
|
||||||
pub fn extract_cookie_value<'a>(raw: &'a str, name: &str) -> Option<&'a str> {
|
|
||||||
let first = raw.split(';').next()?.trim();
|
|
||||||
let (k, v) = first.split_once('=')?;
|
|
||||||
(k.trim() == name).then(|| v.trim())
|
|
||||||
}
|
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn extracts_named_cookie_value() {
|
|
||||||
let raw = "maestro_session=abc.def.ghi; HttpOnly; SameSite=Strict; Path=/; Max-Age=2592000";
|
|
||||||
assert_eq!(
|
|
||||||
extract_cookie_value(raw, "maestro_session"),
|
|
||||||
Some("abc.def.ghi")
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn returns_none_for_other_cookie_name() {
|
|
||||||
let raw = "other=1; HttpOnly";
|
|
||||||
assert_eq!(extract_cookie_value(raw, "maestro_session"), None);
|
|
||||||
}
|
|
||||||
|
|
||||||
/// A peer that accepts the TCP connection but never sends a response must
|
/// A peer that accepts the TCP connection but never sends a response must
|
||||||
/// not hang the client forever: the request timeout has to fire (NAP-262).
|
/// not hang the client forever: the request timeout has to fire (NAP-262).
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
|
|||||||
@@ -463,6 +463,29 @@ pub struct JwtClaims {
|
|||||||
/// secret (NAP-266).
|
/// secret (NAP-266).
|
||||||
pub const REGISTRY_ISSUER: &str = "maestro-registry";
|
pub const REGISTRY_ISSUER: &str = "maestro-registry";
|
||||||
|
|
||||||
|
/// Name of the auth cookie the registry sets and the CLI persists. Defined
|
||||||
|
/// once here so both ends of the wire agree (NAP-273).
|
||||||
|
pub const COOKIE_NAME: &str = "maestro_session";
|
||||||
|
|
||||||
|
/// Find a named cookie's value inside a `Cookie:` / `Set-Cookie:` header value.
|
||||||
|
///
|
||||||
|
/// Iterates every `;`-separated `name=value` pair, so it works whether the
|
||||||
|
/// target pair is first or buried among other cookies or `Set-Cookie`
|
||||||
|
/// attributes (`HttpOnly`, `Path=/`, …). Returns the first match, or `None`.
|
||||||
|
/// Shared by the registry's inbound-cookie check and the CLI's `Set-Cookie`
|
||||||
|
/// capture (NAP-273).
|
||||||
|
pub fn extract_cookie<'a>(header_value: &'a str, name: &str) -> Option<&'a str> {
|
||||||
|
for part in header_value.split(';') {
|
||||||
|
let part = part.trim();
|
||||||
|
if let Some((k, v)) = part.split_once('=') {
|
||||||
|
if k.trim() == name {
|
||||||
|
return Some(v.trim());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
None
|
||||||
|
}
|
||||||
|
|
||||||
/// Pair-scoped attach credential. Issued by the registry inside
|
/// Pair-scoped attach credential. Issued by the registry inside
|
||||||
/// [`AttachCandidates::jwt`]; verified by the target daemon for direct
|
/// [`AttachCandidates::jwt`]; verified by the target daemon for direct
|
||||||
/// transports (T1/T2) and gated by `allow_inbound` after JWT verification.
|
/// transports (T1/T2) and gated by `allow_inbound` after JWT verification.
|
||||||
@@ -488,6 +511,20 @@ pub struct PairAttachClaims {
|
|||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn extract_cookie_finds_named_value_anywhere() {
|
||||||
|
// Cookie: header — pair may be buried among others.
|
||||||
|
let h = "foo=1; maestro_session=abc.def.ghi; bar=2";
|
||||||
|
assert_eq!(extract_cookie(h, COOKIE_NAME), Some("abc.def.ghi"));
|
||||||
|
assert_eq!(extract_cookie(h, "foo"), Some("1"));
|
||||||
|
assert_eq!(extract_cookie(h, "missing"), None);
|
||||||
|
// Set-Cookie: header — cookie first, attributes (no `=` or other keys)
|
||||||
|
// after it must be skipped, not matched.
|
||||||
|
let sc = "maestro_session=xyz; HttpOnly; SameSite=Strict; Path=/; Max-Age=2592000";
|
||||||
|
assert_eq!(extract_cookie(sc, COOKIE_NAME), Some("xyz"));
|
||||||
|
assert_eq!(extract_cookie("other=1; HttpOnly", COOKIE_NAME), None);
|
||||||
|
}
|
||||||
|
|
||||||
fn round_trip<T>(value: &T) -> T
|
fn round_trip<T>(value: &T) -> T
|
||||||
where
|
where
|
||||||
T: Serialize + for<'de> Deserialize<'de>,
|
T: Serialize + for<'de> Deserialize<'de>,
|
||||||
|
|||||||
@@ -28,8 +28,9 @@ use jsonwebtoken::{decode, encode, Algorithm, DecodingKey, EncodingKey, Header,
|
|||||||
use maestro_protocol::REGISTRY_ISSUER;
|
use maestro_protocol::REGISTRY_ISSUER;
|
||||||
use serde::{Deserialize, Serialize};
|
use serde::{Deserialize, Serialize};
|
||||||
|
|
||||||
/// Name of the auth cookie surfaced to web clients.
|
/// Name of the auth cookie surfaced to web clients. Defined in
|
||||||
pub const COOKIE_NAME: &str = "maestro_session";
|
/// `maestro-protocol` so the CLI agrees; re-exported here for local callers.
|
||||||
|
pub use maestro_protocol::{extract_cookie, COOKIE_NAME};
|
||||||
|
|
||||||
/// 30 days. Cookie is the second factor — long enough not to log users out;
|
/// 30 days. Cookie is the second factor — long enough not to log users out;
|
||||||
/// short enough to bound the blast radius if the secret rotates.
|
/// short enough to bound the blast radius if the secret rotates.
|
||||||
@@ -88,21 +89,6 @@ fn set_cookie_header(value: &str) -> String {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Find our cookie inside an inbound `Cookie:` header. Header may contain
|
|
||||||
/// many `name=value` pairs separated by `;`.
|
|
||||||
pub fn extract_cookie<'a>(header_value: &'a str, name: &str) -> Option<&'a str> {
|
|
||||||
for part in header_value.split(';') {
|
|
||||||
let part = part.trim();
|
|
||||||
if let Some(eq) = part.find('=') {
|
|
||||||
let (k, v) = part.split_at(eq);
|
|
||||||
if k == name {
|
|
||||||
return Some(&v[1..]);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
None
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Shared state for the auth middleware: just the signing secret.
|
/// Shared state for the auth middleware: just the signing secret.
|
||||||
#[derive(Debug, Clone)]
|
#[derive(Debug, Clone)]
|
||||||
pub struct AuthState {
|
pub struct AuthState {
|
||||||
@@ -175,14 +161,6 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn extract_cookie_finds_named_value() {
|
|
||||||
let h = "foo=1; maestro_session=abc.def.ghi; bar=2";
|
|
||||||
assert_eq!(extract_cookie(h, "maestro_session"), Some("abc.def.ghi"));
|
|
||||||
assert_eq!(extract_cookie(h, "foo"), Some("1"));
|
|
||||||
assert_eq!(extract_cookie(h, "missing"), None);
|
|
||||||
}
|
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn set_cookie_header_includes_attributes() {
|
fn set_cookie_header_includes_attributes() {
|
||||||
let h = set_cookie_header("abc");
|
let h = set_cookie_header("abc");
|
||||||
|
|||||||
Reference in New Issue
Block a user