mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 14:53:34 +00:00
fix: address review feedback (auth 503, token expiry, CORS PATCH)
- DB auth errors now return 503 instead of 401 so outages are distinguishable from invalid tokens (serrrfirat H3) - Cap expires_in_days to 36500 before i64 cast to prevent negative duration from u64 overflow (serrrfirat H1) - Add PATCH to CORS allowed methods for profile/user update endpoints (Copilot) - Stop leaking panic details in CatchPanicLayer response body Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
e5e3335eb9
commit
8eb32c7370
+22
-11
@@ -156,7 +156,11 @@ impl DbAuthenticator {
|
||||
}
|
||||
|
||||
/// Authenticate a token against the database, using cache when possible.
|
||||
pub async fn authenticate(&self, candidate: &str) -> Option<UserIdentity> {
|
||||
///
|
||||
/// Returns `Ok(Some(identity))` on success, `Ok(None)` if the token is
|
||||
/// not found, or `Err(())` if the database is unreachable (so the caller
|
||||
/// can return 503 instead of 401).
|
||||
pub async fn authenticate(&self, candidate: &str) -> Result<Option<UserIdentity>, ()> {
|
||||
let hash = hash_token(candidate);
|
||||
|
||||
// Check cache first (promotes to most-recent on hit)
|
||||
@@ -164,7 +168,7 @@ impl DbAuthenticator {
|
||||
let mut cache = self.cache.write().await;
|
||||
if let Some((identity, inserted_at)) = cache.get(&hash) {
|
||||
if inserted_at.elapsed().as_secs() < Self::CACHE_TTL_SECS {
|
||||
return Some(identity.clone());
|
||||
return Ok(Some(identity.clone()));
|
||||
}
|
||||
// Expired — remove stale entry
|
||||
cache.pop(&hash);
|
||||
@@ -174,10 +178,10 @@ impl DbAuthenticator {
|
||||
// Cache miss or expired — query DB
|
||||
let (token_record, user_record) = match self.store.authenticate_token(&hash).await {
|
||||
Ok(Some(pair)) => pair,
|
||||
Ok(None) => return None,
|
||||
Ok(None) => return Ok(None),
|
||||
Err(e) => {
|
||||
tracing::warn!(error = %e, "DB auth lookup failed");
|
||||
return None;
|
||||
tracing::error!(error = %e, "DB auth lookup failed, returning 503");
|
||||
return Err(());
|
||||
}
|
||||
};
|
||||
|
||||
@@ -202,7 +206,7 @@ impl DbAuthenticator {
|
||||
cache.put(hash, (identity.clone(), Instant::now()));
|
||||
}
|
||||
|
||||
Some(identity)
|
||||
Ok(Some(identity))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -331,11 +335,18 @@ pub async fn auth_middleware(
|
||||
}
|
||||
|
||||
// 2. Fall back to DB-backed token lookup.
|
||||
if let Some(ref db_auth) = auth.db_auth
|
||||
&& let Some(identity) = db_auth.authenticate(tok).await
|
||||
{
|
||||
request.extensions_mut().insert(identity);
|
||||
return next.run(request).await;
|
||||
if let Some(ref db_auth) = auth.db_auth {
|
||||
match db_auth.authenticate(tok).await {
|
||||
Ok(Some(identity)) => {
|
||||
request.extensions_mut().insert(identity);
|
||||
return next.run(request).await;
|
||||
}
|
||||
Err(()) => {
|
||||
return (StatusCode::SERVICE_UNAVAILABLE, "Database unavailable")
|
||||
.into_response();
|
||||
}
|
||||
Ok(None) => {}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -34,13 +34,12 @@ pub async fn tokens_create_handler(
|
||||
))?
|
||||
.to_string();
|
||||
|
||||
let expires_in_days = body
|
||||
let expires_in_days: Option<i64> = body
|
||||
.get("expires_in_days")
|
||||
.and_then(|v| v.as_u64())
|
||||
.map(|d| d.min(36500));
|
||||
.map(|d| d.min(36500) as i64);
|
||||
|
||||
let expires_at =
|
||||
expires_in_days.map(|days| chrono::Utc::now() + chrono::Duration::days(days as i64));
|
||||
let expires_at = expires_in_days.map(|days| chrono::Utc::now() + chrono::Duration::days(days));
|
||||
|
||||
// Generate 32 random bytes for the token.
|
||||
// Hash the hex-encoded plaintext (what the user sends as Bearer token),
|
||||
|
||||
@@ -613,6 +613,7 @@ pub async fn start_server(
|
||||
axum::http::Method::GET,
|
||||
axum::http::Method::POST,
|
||||
axum::http::Method::PUT,
|
||||
axum::http::Method::PATCH,
|
||||
axum::http::Method::DELETE,
|
||||
])
|
||||
.allow_headers(AllowHeaders::list([
|
||||
@@ -640,9 +641,7 @@ pub async fn start_server(
|
||||
axum::http::Response::builder()
|
||||
.status(axum::http::StatusCode::INTERNAL_SERVER_ERROR)
|
||||
.header("content-type", "text/plain")
|
||||
.body(axum::body::Body::from(format!(
|
||||
"Internal Server Error: {detail}"
|
||||
)))
|
||||
.body(axum::body::Body::from("Internal Server Error"))
|
||||
.unwrap_or_else(|_| {
|
||||
axum::http::Response::new(axum::body::Body::from("Internal Server Error"))
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user