Admin improvements (#812)

* security and improvements

* rate limiting admin endpoints

* token in memory fix

* delete returns 404 if not found

* health behind auth

* admin warning if no token configured

* cargo fmt

* improvements

Author: tonylee08

Cargo.lock

@@ -32,6 +32,10 @@ bigdecimal = "0.4.9"
 chrono = "0.4.42"
 thiserror = "1.0"
 tokio-util = "0.7"
+subtle = "2.5"
+tracing.workspace = true
+governor = "0.6"
+secrecy = "0.8"
 
 [[bin]]
 name = "deepbook-server"

crates/server/Cargo.toml

@@ -29,9 +29,23 @@ pub async fn require_admin_auth(
             if state.is_valid_admin_token(token) {
                 Ok(next.run(req).await)
             } else {
-                Err(StatusCode::UNAUTHORIZED)
+                tracing::warn!("Invalid admin token provided");
+                auth_failure_rate_limited(&state)
             }
         }
-        _ => Err(StatusCode::UNAUTHORIZED),
+        _ => {
+            tracing::warn!("Missing or malformed admin authorization header");
+            auth_failure_rate_limited(&state)
+        }
+    }
+}
+
+/// Returns 429 if rate limit exceeded, otherwise 401
+fn auth_failure_rate_limited(state: &AppState) -> Result<Response, StatusCode> {
+    if !state.check_admin_rate_limit() {
+        tracing::warn!("Admin auth rate limit exceeded");
+        Err(StatusCode::TOO_MANY_REQUESTS)
+    } else {
+        Err(StatusCode::UNAUTHORIZED)
     }
 }

crates/server/src/admin/auth.rs

@@ -61,6 +61,14 @@ pub async fn create_pool(
     State(state): State<Arc<AppState>>,
     Json(payload): Json<CreatePoolRequest>,
 ) -> Result<Json<AdminResponse>, DeepBookError> {
+    tracing::info!(
+        action = "create_pool",
+        pool_id = %payload.pool_id,
+        pool_name = %payload.pool_name,
+        base_asset = %payload.base_asset_symbol,
+        quote_asset = %payload.quote_asset_symbol,
+        "Admin creating pool"
+    );
     state.writer().create_pool(payload).await?;
     Ok(Json(AdminResponse {
         status: "created".to_string(),
@@ -72,6 +80,15 @@ pub async fn update_pool(
     Path(pool_id): Path<String>,
     Json(payload): Json<UpdatePoolRequest>,
 ) -> Result<Json<AdminResponse>, DeepBookError> {
+    tracing::info!(
+        action = "update_pool",
+        pool_id = %pool_id,
+        pool_name = ?payload.pool_name,
+        min_size = ?payload.min_size,
+        lot_size = ?payload.lot_size,
+        tick_size = ?payload.tick_size,
+        "Admin updating pool"
+    );
     state.writer().update_pool(&pool_id, payload).await?;
     Ok(Json(AdminResponse {
         status: "updated".to_string(),
@@ -82,6 +99,11 @@ pub async fn delete_pool(
     State(state): State<Arc<AppState>>,
     Path(pool_id): Path<String>,
 ) -> Result<Json<AdminResponse>, DeepBookError> {
+    tracing::info!(
+        action = "delete_pool",
+        pool_id = %pool_id,
+        "Admin deleting pool"
+    );
     state.writer().delete_pool(&pool_id).await?;
     Ok(Json(AdminResponse {
         status: "deleted".to_string(),
@@ -92,6 +114,14 @@ pub async fn create_asset(
     State(state): State<Arc<AppState>>,
     Json(payload): Json<CreateAssetRequest>,
 ) -> Result<Json<AdminResponse>, DeepBookError> {
+    tracing::info!(
+        action = "create_asset",
+        asset_type = %payload.asset_type,
+        symbol = %payload.symbol,
+        name = %payload.name,
+        decimals = %payload.decimals,
+        "Admin creating asset"
+    );
     state.writer().create_asset(payload).await?;
     Ok(Json(AdminResponse {
         status: "created".to_string(),
@@ -102,6 +132,11 @@ pub async fn delete_asset(
     State(state): State<Arc<AppState>>,
     Path(asset_type): Path<String>,
 ) -> Result<Json<AdminResponse>, DeepBookError> {
+    tracing::info!(
+        action = "delete_asset",
+        asset_type = %asset_type,
+        "Admin deleting asset"
+    );
     state.writer().delete_asset(&asset_type).await?;
     Ok(Json(AdminResponse {
         status: "deleted".to_string(),

crates/server/src/admin/handlers.rs

@@ -14,12 +14,17 @@ use super::handlers;
 use crate::server::AppState;
 
 pub fn admin_routes(state: Arc<AppState>) -> Router<Arc<AppState>> {
-    Router::new()
-        .route("/health", get(handlers::admin_health))
+    // Authenticated routes
+    let protected = Router::new()
         .route("/pools", post(handlers::create_pool))
         .route("/pools/{pool_id}", put(handlers::update_pool))
         .route("/pools/{pool_id}", delete(handlers::delete_pool))
         .route("/assets", post(handlers::create_asset))
         .route("/assets/{asset_type}", delete(handlers::delete_asset))
-        .layer(from_fn_with_state(state, require_admin_auth))
+        .layer(from_fn_with_state(state, require_admin_auth));
+
+    // Health check is unauthenticated for load balancer probes
+    Router::new()
+        .route("/health", get(handlers::admin_health))
+        .merge(protected)
 }

crates/server/src/admin/routes.rs

@@ -21,9 +21,12 @@ use deepbook_schema::*;
 use diesel::dsl::count_star;
 use diesel::dsl::{max, min};
 use diesel::{ExpressionMethods, QueryDsl};
+use governor::{Quota, RateLimiter};
+use secrecy::{ExposeSecret, Secret};
 use serde::Deserialize;
 use serde_json::Value;
 use std::net::{IpAddr, Ipv4Addr};
+use std::num::NonZeroU32;
 use std::time::{SystemTime, UNIX_EPOCH};
 use std::{collections::HashMap, net::SocketAddr};
 use sui_pg_db::DbArgs;
@@ -111,6 +114,12 @@ pub const DEPOSITED_ASSETS_PATH: &str = "/deposited_assets/:balance_manager_ids"
 pub const COLLATERAL_EVENTS_PATH: &str = "/collateral_events";
 pub const GET_POINTS_PATH: &str = "/get_points";
 
+type AdminRateLimiter = RateLimiter<
+    governor::state::NotKeyed,
+    governor::state::InMemoryState,
+    governor::clock::DefaultClock,
+>;
+
 #[derive(Clone)]
 pub struct AppState {
     reader: Reader,
@@ -121,7 +130,8 @@ pub struct AppState {
     deepbook_package_id: String,
     deep_token_package_id: String,
     deep_treasury_id: String,
-    admin_tokens: Vec<String>,
+    admin_tokens: Vec<Secret<String>>,
+    admin_auth_limiter: Arc<AdminRateLimiter>,
     margin_package_id: Option<String>,
 }
 
@@ -147,15 +157,27 @@ impl AppState {
         .await?;
         let writer = Writer::new(database_url, args).await?;
 
-        let admin_tokens = admin_tokens
+        let admin_tokens: Vec<Secret<String>> = admin_tokens
             .map(|s| {
                 s.split(',')
                     .map(|t| t.trim().to_string())
                     .filter(|t| !t.is_empty())
+                    .map(Secret::new)
                     .collect()
             })
             .unwrap_or_default();
 
+        if admin_tokens.is_empty() {
+            tracing::warn!(
+                "No admin tokens configured (ADMIN_TOKENS env var). Admin endpoints will reject all requests."
+            );
+        }
+
+        // Rate limiter: 10 attempts per minute for admin auth failures
+        let admin_auth_limiter = Arc::new(RateLimiter::direct(Quota::per_minute(
+            NonZeroU32::new(10).unwrap(),
+        )));
+
         Ok(Self {
             reader,
             writer,
@@ -166,6 +188,7 @@ impl AppState {
             deep_token_package_id,
             deep_treasury_id,
             admin_tokens,
+            admin_auth_limiter,
             margin_package_id,
         })
     }
@@ -191,7 +214,14 @@ impl AppState {
     }
 
     pub fn is_valid_admin_token(&self, token: &str) -> bool {
-        self.admin_tokens.iter().any(|t| t == token)
+        use subtle::ConstantTimeEq;
+        self.admin_tokens
+            .iter()
+            .any(|t| t.expose_secret().as_bytes().ct_eq(token.as_bytes()).into())
+    }
+
+    pub fn check_admin_rate_limit(&self) -> bool {
+        self.admin_auth_limiter.check().is_ok()
     }
 }
 

crates/server/src/server.rs

@@ -4,11 +4,20 @@
 use crate::admin::handlers::{CreateAssetRequest, CreatePoolRequest, UpdatePoolRequest};
 use crate::error::DeepBookError;
 use deepbook_schema::schema;
-use diesel::{ExpressionMethods, QueryDsl};
+use diesel::{AsChangeset, ExpressionMethods, QueryDsl};
 use diesel_async::RunQueryDsl;
 use sui_pg_db::{Db, DbArgs};
 use url::Url;
 
+#[derive(AsChangeset)]
+#[diesel(table_name = schema::pools)]
+struct PoolChangeset {
+    pool_name: Option<String>,
+    min_size: Option<i64>,
+    lot_size: Option<i64>,
+    tick_size: Option<i64>,
+}
+
 #[derive(Clone)]
 pub struct Writer {
     db: Db,
@@ -56,31 +65,22 @@ impl Writer {
             .await
             .map_err(|e| DeepBookError::database(e.to_string()))?;
 
-        if let Some(name) = req.pool_name {
-            diesel::update(schema::pools::table.filter(schema::pools::pool_id.eq(id)))
-                .set(schema::pools::pool_name.eq(name))
-                .execute(&mut conn)
-                .await?;
-        }
-        if let Some(size) = req.min_size {
-            diesel::update(schema::pools::table.filter(schema::pools::pool_id.eq(id)))
-                .set(schema::pools::min_size.eq(size))
-                .execute(&mut conn)
-                .await?;
-        }
-        if let Some(size) = req.lot_size {
-            diesel::update(schema::pools::table.filter(schema::pools::pool_id.eq(id)))
-                .set(schema::pools::lot_size.eq(size))
-                .execute(&mut conn)
-                .await?;
-        }
-        if let Some(size) = req.tick_size {
+        let changeset = PoolChangeset {
+            pool_name: req.pool_name,
+            min_size: req.min_size,
+            lot_size: req.lot_size,
+            tick_size: req.tick_size,
+        };
+
+        let rows_affected =
             diesel::update(schema::pools::table.filter(schema::pools::pool_id.eq(id)))
-                .set(schema::pools::tick_size.eq(size))
+                .set(changeset)
                 .execute(&mut conn)
                 .await?;
-        }
 
+        if rows_affected == 0 {
+            return Err(DeepBookError::not_found(format!("pool {id}")));
+        }
         Ok(())
     }
 
@@ -91,10 +91,14 @@ impl Writer {
             .await
             .map_err(|e| DeepBookError::database(e.to_string()))?;
 
-        diesel::delete(schema::pools::table.filter(schema::pools::pool_id.eq(id)))
-            .execute(&mut conn)
-            .await?;
+        let rows_affected =
+            diesel::delete(schema::pools::table.filter(schema::pools::pool_id.eq(id)))
+                .execute(&mut conn)
+                .await?;
 
+        if rows_affected == 0 {
+            return Err(DeepBookError::not_found(format!("pool {id}")));
+        }
         Ok(())
     }
 
@@ -128,10 +132,14 @@ impl Writer {
             .await
             .map_err(|e| DeepBookError::database(e.to_string()))?;
 
-        diesel::delete(schema::assets::table.filter(schema::assets::asset_type.eq(id)))
-            .execute(&mut conn)
-            .await?;
+        let rows_affected =
+            diesel::delete(schema::assets::table.filter(schema::assets::asset_type.eq(id)))
+                .execute(&mut conn)
+                .await?;
 
+        if rows_affected == 0 {
+            return Err(DeepBookError::not_found(format!("asset {id}")));
+        }
         Ok(())
     }
 }