enha: reject unknown clients in the middleware (#1939)

e2e/hardhat.config.ts

@@ -57,7 +57,7 @@ const config: HardhatUserConfig = {
             },
         },
         stratus: {
-            url: `http://localhost:${STRATUS_PORT}?app=e2e`,
+            url: `http://localhost:${STRATUS_PORT}`,
             accounts: {
                 mnemonic: ACCOUNTS_MNEMONIC,
             },

e2e/test/automine/e2e-json-rpc.test.ts

@@ -1,6 +1,7 @@
 import { expect } from "chai";
 import { keccak256 } from "ethers";
-import { Block, Bytes, TransactionReceipt } from "web3-types";
+import { JsonRpcProvider } from "ethers";
+import { Block, Bytes } from "web3-types";
 
 import { ALICE, BOB } from "../helpers/account";
 import { isStratus } from "../helpers/network";
@@ -39,6 +40,64 @@ describe("JSON-RPC", () => {
         });
     });
 
+    describe("Unknown Clients", () => {
+        before(async () => {
+            if (isStratus) {
+                // Ensure clean state
+                await send("stratus_enableUnknownClients");
+            }
+        });
+
+        it("disabling unknown clients blocks requests without client identification", async function () {
+            if (!isStratus) {
+                this.skip();
+                return;
+            }
+
+            await send("stratus_disableUnknownClients");
+
+            // Request without client identification should fail
+            const error = await sendAndGetError("eth_blockNumber");
+            expect(error.code).eq(1003);
+
+            // Requests with client identification should succeed
+            const validHeaders = {
+                "x-app": "test-client",
+                "x-stratus-app": "test-client",
+                "x-client": "test-client",
+                "x-stratus-client": "test-client",
+            };
+
+            for (const [header, value] of Object.entries(validHeaders)) {
+                const headers = { [header]: value };
+                const result = await send("eth_blockNumber", [], headers);
+                expect(result).to.match(/^0x[0-9a-f]+$/);
+            }
+
+            // URL parameters should also work
+            const validUrlParams = ["app=test-client", "client=test-client"];
+            for (const param of validUrlParams) {
+                const providerWithParam = new JsonRpcProvider(`http://localhost:3000?${param}`);
+                const blockNumber = await providerWithParam.getBlockNumber();
+                expect(blockNumber).to.be.a("number");
+            }
+        });
+
+        it("enabling unknown clients allows all requests", async function () {
+            if (!isStratus) {
+                this.skip();
+                return;
+            }
+
+            await send("stratus_disableUnknownClients");
+            await send("stratus_enableUnknownClients");
+
+            // Request without client identification should now succeed
+            const blockNumber = await send("eth_blockNumber");
+            expect(blockNumber).to.match(/^0x[0-9a-f]+$/);
+        });
+    });
+
     describe("Metadata", () => {
         it("eth_chainId", async () => {
             (await sendExpect("eth_chainId")).eq(CHAIN_ID);
@@ -291,7 +350,7 @@ describe("JSON-RPC", () => {
 
                 // Record timestamp in contract
                 const tx = await contract.recordTimestamp();
-                const receipt = await tx.wait();
+                const receipt: TransactionReceipt = await tx.wait();
 
                 // Get the timestamp from contract event
                 const event = receipt.logs[0];

justfile

@@ -201,7 +201,7 @@ e2e-stratus block-mode="automine" test="":
     just build
 
     just _log "Starting Stratus"
-    just run -a 0.0.0.0:3000 --block-mode {{block-mode}} > stratus.log &
+    RUST_LOG=debug just run -a 0.0.0.0:3000 --block-mode {{block-mode}} > stratus.log &
 
     just _wait_for_stratus
 

src/eth/primitives/stratus_error.rs

@@ -1,4 +1,9 @@
+use futures::future::BoxFuture;
+use jsonrpsee::server::middleware::rpc::layer::ResponseFuture;
 use jsonrpsee::types::ErrorObjectOwned;
+use jsonrpsee::types::Id;
+use jsonrpsee::MethodResponse;
+use jsonrpsee::ResponsePayload;
 use stratus_macros::ErrorCode;
 
 use crate::alias::JsonValue;
@@ -271,6 +276,12 @@ impl StratusError {
             _ => JsonValue::Null,
         }
     }
+
+    pub fn to_response_future<'a>(self, id: Id<'_>) -> ResponseFuture<BoxFuture<'a, MethodResponse>> {
+        let response = ResponsePayload::<()>::error(StratusError::RPC(RpcError::ClientMissing));
+        let method_response = MethodResponse::response(id, response, u32::MAX as usize);
+        ResponseFuture::ready(method_response)
+    }
 }
 
 // -----------------------------------------------------------------------------

src/eth/rpc/mod.rs

@@ -18,6 +18,5 @@ use rpc_middleware::RpcMiddleware;
 use rpc_parser::next_rpc_param;
 use rpc_parser::next_rpc_param_or_default;
 use rpc_parser::parse_rpc_rlp;
-use rpc_server::reject_unknown_client;
 pub use rpc_server::serve_rpc;
 pub use rpc_subscriptions::RpcSubscriptions;

src/eth/rpc/rpc_method_wrapper.rs

@@ -5,7 +5,6 @@ use jsonrpsee::types::Params;
 use jsonrpsee::Extensions;
 
 use crate::eth::primitives::StratusError;
-use crate::eth::rpc::reject_unknown_client;
 use crate::eth::rpc::rpc_parser::RpcExtensionsExt;
 use crate::eth::rpc::RpcContext;
 
@@ -24,7 +23,6 @@ cfg_if! {
             F: Fn(Params<'_>, Arc<RpcContext>, &Extensions) -> Result<T, StratusError> + Clone,
         {
             move |params, ctx, extensions| {
-                reject_unknown_client(extensions.rpc_client())?;
                 function(params, ctx, &extensions).inspect_err(|e| metrify_stratus_error(e, &extensions, method_name))
             }
         }
@@ -34,7 +32,6 @@ cfg_if! {
             F: Fn(Params<'_>, Arc<RpcContext>, &Extensions) -> Result<T, StratusError> + Clone,
         {
             move |params, ctx, extensions| {
-                reject_unknown_client(extensions.rpc_client())?;
                 function(params, ctx, &extensions)
             }
         }

src/eth/rpc/rpc_middleware.rs

@@ -11,6 +11,7 @@ use jsonrpsee::server::middleware::rpc::RpcServiceT;
 #[cfg(feature = "metrics")]
 use jsonrpsee::server::ConnectionGuard;
 use jsonrpsee::types::error::INTERNAL_ERROR_CODE;
+use jsonrpsee::types::Id;
 use jsonrpsee::types::Params;
 use jsonrpsee::MethodResponse;
 use pin_project::pin_project;
@@ -28,6 +29,8 @@ use crate::eth::primitives::Bytes;
 use crate::eth::primitives::CallInput;
 use crate::eth::primitives::Hash;
 use crate::eth::primitives::Nonce;
+use crate::eth::primitives::RpcError;
+use crate::eth::primitives::StratusError;
 use crate::eth::primitives::TransactionInput;
 use crate::eth::rpc::next_rpc_param;
 use crate::eth::rpc::parse_rpc_rlp;
@@ -42,6 +45,7 @@ use crate::infra::metrics;
 use crate::infra::tracing::new_cid;
 use crate::infra::tracing::SpanExt;
 use crate::infra::tracing::TracingExt;
+use crate::GlobalState;
 
 // -----------------------------------------------------------------------------
 // Request handling
@@ -139,17 +143,27 @@ impl<'a> RpcServiceT<'a> for RpcMiddleware {
         drop(middleware_enter);
         request.extensions_mut().insert(span);
 
+        let id = request.id.to_string();
+        let future_response = reject_unknown_client(&client, request.id.clone()).unwrap_or(self.service.call(request));
         RpcResponse {
             client,
-            id: request.id.to_string(),
+            id,
             method: method.to_string(),
             tx,
             start: Instant::now(),
-            future_response: self.service.call(request),
+            future_response,
         }
     }
 }
 
+/// Returns an error JSON-RPC response if the client is not allowed to perform the current operation.
+fn reject_unknown_client<'a>(client: &RpcClientApp, id: Id<'_>) -> Option<ResponseFuture<BoxFuture<'a, MethodResponse>>> {
+    if client.is_unknown() && !GlobalState::is_unknown_client_enabled() {
+        return Some(StratusError::RPC(RpcError::ClientMissing).to_response_future(id));
+    }
+    None
+}
+
 // -----------------------------------------------------------------------------
 // Response handling
 // -----------------------------------------------------------------------------

src/eth/rpc/rpc_server.rs

@@ -64,7 +64,6 @@ use crate::eth::rpc::next_rpc_param;
 use crate::eth::rpc::next_rpc_param_or_default;
 use crate::eth::rpc::parse_rpc_rlp;
 use crate::eth::rpc::rpc_parser::RpcExtensionsExt;
-use crate::eth::rpc::RpcClientApp;
 use crate::eth::rpc::RpcContext;
 use crate::eth::rpc::RpcHttpMiddleware;
 use crate::eth::rpc::RpcMiddleware;
@@ -601,9 +600,7 @@ fn stratus_state(_: Params<'_>, ctx: &RpcContext, _: &Extensions) -> Result<Json
     Ok(GlobalState::get_global_state_as_json(ctx))
 }
 
-async fn stratus_get_subscriptions(_: Params<'_>, ctx: Arc<RpcContext>, ext: Extensions) -> Result<JsonValue, StratusError> {
-    reject_unknown_client(ext.rpc_client())?;
-
+async fn stratus_get_subscriptions(_: Params<'_>, ctx: Arc<RpcContext>, _: Extensions) -> Result<JsonValue, StratusError> {
     // NOTE: this is a workaround for holding only one lock at a time
     let pending_txs = serde_json::to_value(ctx.subs.new_heads.read().await.values().collect_vec()).expect_infallible();
     let new_heads = serde_json::to_value(ctx.subs.pending_txs.read().await.values().collect_vec()).expect_infallible();
@@ -1090,8 +1087,6 @@ async fn eth_subscribe(params: Params<'_>, pending: PendingSubscriptionSink, ctx
     drop(middleware_enter);
 
     async move {
-        reject_unknown_client(ext.rpc_client())?;
-
         // parse params
         let client = ext.rpc_client();
         let (params, event) = match next_rpc_param::<String>(params.sequence()) {
@@ -1169,18 +1164,6 @@ fn eth_get_storage_at(params: Params<'_>, ctx: Arc<RpcContext>, ext: &Extensions
     Ok(hex_num_zero_padded(slot.value.as_u256()))
 }
 
-// -----------------------------------------------------------------------------
-// Request helpers
-// -----------------------------------------------------------------------------
-
-/// Returns an error JSON-RPC response if the client is not allowed to perform the current operation.
-pub(super) fn reject_unknown_client(client: &RpcClientApp) -> Result<(), StratusError> {
-    if client.is_unknown() && not(GlobalState::is_unknown_client_enabled()) {
-        return Err(RpcError::ClientMissing.into());
-    }
-    Ok(())
-}
-
 // -----------------------------------------------------------------------------
 // Response helpers
 // -----------------------------------------------------------------------------