Skip to content

Commit

Permalink
Revert "feat: RPC: Check for passwords in admin endpoints (#1921)" (#…
Browse files Browse the repository at this point in the history
…1927)

This reverts commit 08cc4e5.
  • Loading branch information
carneiro-cw authored Dec 20, 2024
1 parent 08cc4e5 commit 0d2f8ed
Show file tree
Hide file tree
Showing 11 changed files with 24 additions and 194 deletions.
10 changes: 0 additions & 10 deletions .github/workflows/e2e-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,3 @@ jobs:
concurrency:
group: ${{ github.workflow }}-clock-rocks-${{ github.ref || github.run_id }}
cancel-in-progress: true

e2e-admin-password:
name: E2E Admin Password
uses: ./.github/workflows/_setup-e2e.yml
with:
justfile_recipe: "e2e-admin-password"

concurrency:
group: ${{ github.workflow }}-admin-password-${{ github.ref || github.run_id }}
cancel-in-progress: true
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ describe("Leader & Follower change integration test", function () {
const version = await sendWithRetry("stratus_version", []);
expect(version).to.have.nested.property("git.commit");
expect(version.git.commit).to.be.a("string");
expect(version.git.commit.length).to.be.oneOf([7, 8]);
expect(version.git.commit).to.have.lengthOf(7);
});

it("Validate initial Follower state, health and version", async function () {
Expand All @@ -32,7 +32,7 @@ describe("Leader & Follower change integration test", function () {
const version = await sendWithRetry("stratus_version", []);
expect(version).to.have.nested.property("git.commit");
expect(version.git.commit).to.be.a("string");
expect(version.git.commit.length).to.be.oneOf([7, 8]);
expect(version.git.commit).to.have.lengthOf(7);
});

it("Change Leader to Leader should return false", async function () {
Expand Down
26 changes: 0 additions & 26 deletions e2e/test/admin/e2e-admin-password-disabled.test.ts

This file was deleted.

32 changes: 0 additions & 32 deletions e2e/test/admin/e2e-admin-password-enabled.test.ts

This file was deleted.

26 changes: 6 additions & 20 deletions e2e/test/helpers/rpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,7 @@ if (process.env.RPC_LOG) {
// Sends an RPC request to the blockchain, returning full response.
let requestId = 0;

export async function sendAndGetFullResponse(
method: string,
params: any[] = [],
headers: Record<string, string> = {},
): Promise<any> {
export async function sendAndGetFullResponse(method: string, params: any[] = []): Promise<any> {
for (let i = 0; i < params.length; ++i) {
const param = params[i];
if (param instanceof Account) {
Expand All @@ -134,14 +130,8 @@ export async function sendAndGetFullResponse(
console.log("REQ ->", JSON.stringify(payload));
}

// prepare headers
const requestHeaders = {
"Content-Type": "application/json",
...headers,
};

// execute request and log response
const response = await axios.post(providerUrl, payload, { headers: requestHeaders });
const response = await axios.post(providerUrl, payload, { headers: { "Content-Type": "application/json" } });
if (process.env.RPC_LOG) {
console.log("RESP <-", JSON.stringify(response.data));
}
Expand All @@ -150,19 +140,15 @@ export async function sendAndGetFullResponse(
}

// Sends an RPC request to the blockchain, returning its result field.
export async function send(method: string, params: any[] = [], headers: Record<string, string> = {}): Promise<any> {
const response = await sendAndGetFullResponse(method, params, headers);
export async function send(method: string, params: any[] = []): Promise<any> {
const response = await sendAndGetFullResponse(method, params);
return response.data.result;
}

// Sends an RPC request to the blockchain, returning its error field.
// Use it when you expect the RPC call to fail.
export async function sendAndGetError(
method: string,
params: any[] = [],
headers: Record<string, string> = {},
): Promise<any> {
const response = await sendAndGetFullResponse(method, params, headers);
export async function sendAndGetError(method: string, params: any[] = []): Promise<any> {
const response = await sendAndGetFullResponse(method, params);
return response.data.error;
}

Expand Down
21 changes: 1 addition & 20 deletions justfile
Original file line number Diff line number Diff line change
Expand Up @@ -159,25 +159,6 @@ e2e network="stratus" block_modes="automine" test="":
fi
done

# E2E: Execute admin password tests
e2e-admin-password:
#!/bin/bash
just build

# Start Stratus with password set
just _log "Running admin password tests with password set"
ADMIN_PASSWORD=test123 just run -a 0.0.0.0:3000 &
just _wait_for_stratus
cd e2e && npx hardhat test test/admin/e2e-admin-password-enabled.test.ts --network stratus
killport 3000

# Start Stratus without password set
just _log "Running admin password tests without password set"
just run -a 0.0.0.0:3000 &
just _wait_for_stratus
cd e2e && npx hardhat test test/admin/e2e-admin-password-disabled.test.ts --network stratus
killport 3000

# E2E: Starts and execute Hardhat tests in Hardhat
e2e-hardhat block-mode="automine" test="":
#!/bin/bash
Expand Down Expand Up @@ -618,4 +599,4 @@ stratus-test-coverage *args="":
-rm utils/deploy/deploy_02.log
*/

cargo llvm-cov report {{args}}
cargo llvm-cov report {{args}}
4 changes: 0 additions & 4 deletions src/eth/primitives/stratus_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,10 +192,6 @@ pub enum StratusError {
#[strum(props(kind = "server_state"))]
StratusNotFollower,

#[error("Incorrect password, cancelling operation.")]
#[strum(props(kind = "server_state"))]
InvalidPassword,

#[error("Stratus node is already in the process of changing mode.")]
#[strum(props(kind = "server_state"))]
ModeChangeInProgress,
Expand Down
35 changes: 0 additions & 35 deletions src/eth/rpc/rpc_http_middleware.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ use reqwest::header::HeaderMap;
use reqwest::header::HeaderValue;
use tower::Service;

use crate::eth::primitives::StratusError;
use crate::eth::rpc::RpcClientApp;
use crate::ext::not;

Expand All @@ -37,46 +36,12 @@ where

fn call(&mut self, mut request: HttpRequest<HttpBody>) -> Self::Future {
let client_app = parse_client_app(request.headers(), request.uri());
let authentication = parse_admin_password(request.headers());
request.extensions_mut().insert(client_app);
request.extensions_mut().insert(authentication);

Box::pin(self.service.call(request).map_err(Into::into))
}
}

#[derive(Debug, Clone)]
pub enum Authentication {
Admin,
None,
}

impl Authentication {
pub fn auth_admin(&self) -> Result<(), StratusError> {
if matches!(self, Authentication::Admin) {
return Ok(());
}
Err(StratusError::InvalidPassword)
}
}

/// Checks if the provided admin password is correct
fn parse_admin_password(headers: &HeaderMap<HeaderValue>) -> Authentication {
let real_pass = match std::env::var("ADMIN_PASSWORD") {
Ok(pass) if !pass.is_empty() => pass,
_ => return Authentication::Admin,
};

match headers
.get("Authorization")
.and_then(|val| val.to_str().ok())
.and_then(|val| val.strip_prefix("Password "))
{
Some(password) if password == real_pass => Authentication::Admin,
_ => Authentication::None,
}
}

/// Extracts the client application name from the `app` query parameter.
fn parse_client_app(headers: &HeaderMap<HeaderValue>, uri: &Uri) -> RpcClientApp {
fn try_query_params(uri: &Uri) -> Option<RpcClientApp> {
Expand Down
3 changes: 0 additions & 3 deletions src/eth/rpc/rpc_middleware.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,6 @@ impl<'a> RpcServiceT<'a> for RpcMiddleware {
_ => None,
};

let is_admin = request.extensions.is_admin();

let client = if let Some(tx_client) = tx.as_ref().and_then(|tx| tx.client.as_ref()) {
let val = tx_client.clone();
request.extensions_mut().insert(val);
Expand Down Expand Up @@ -117,7 +115,6 @@ impl<'a> RpcServiceT<'a> for RpcMiddleware {
rpc_tx_function = %tx.as_ref().map(|tx|tx.function).or_empty(),
rpc_tx_from = %tx.as_ref().and_then(|tx|tx.from).or_empty(),
rpc_tx_to = %tx.as_ref().and_then(|tx|tx.to).or_empty(),
is_admin = %is_admin,
"rpc request"
);

Expand Down
15 changes: 0 additions & 15 deletions src/eth/rpc/rpc_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use jsonrpsee::Extensions;
use rlp::Decodable;
use tracing::Span;

use super::rpc_http_middleware::Authentication;
use crate::eth::primitives::StratusError;
use crate::eth::rpc::rpc_client_app::RpcClientApp;
use crate::ext::type_basename;
Expand All @@ -16,12 +15,6 @@ pub trait RpcExtensionsExt {
/// Returns the client performing the JSON-RPC request.
fn rpc_client(&self) -> &RpcClientApp;

/// Returns current Authentication.
fn authentication(&self) -> &Authentication;

/// Returns wheather admin authentication suceeded.
fn is_admin(&self) -> bool;

/// Enters RpcMiddleware request span if present.
fn enter_middleware_span(&self) -> Option<EnteredWrap<'_>>;
}
Expand All @@ -31,14 +24,6 @@ impl RpcExtensionsExt for Extensions {
self.get::<RpcClientApp>().unwrap_or(&RpcClientApp::Unknown)
}

fn authentication(&self) -> &Authentication {
self.get::<Authentication>().unwrap_or(&Authentication::None)
}

fn is_admin(&self) -> bool {
matches!(self.authentication(), Authentication::Admin)
}

fn enter_middleware_span(&self) -> Option<EnteredWrap<'_>> {
self.get::<Span>().map(|s| s.enter()).map(EnteredWrap::new)
}
Expand Down
42 changes: 15 additions & 27 deletions src/eth/rpc/rpc_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,6 @@ fn stratus_reset(_: Params<'_>, ctx: Arc<RpcContext>, _: Extensions) -> Result<J
static MODE_CHANGE_SEMAPHORE: Lazy<Semaphore> = Lazy::new(|| Semaphore::new(1));

async fn stratus_change_to_leader(_: Params<'_>, ctx: Arc<RpcContext>, ext: Extensions) -> Result<JsonValue, StratusError> {
ext.authentication().auth_admin()?;
let permit = MODE_CHANGE_SEMAPHORE.try_acquire();
let _permit: SemaphorePermit = match permit {
Ok(permit) => permit,
Expand Down Expand Up @@ -371,7 +370,6 @@ async fn stratus_change_to_leader(_: Params<'_>, ctx: Arc<RpcContext>, ext: Exte
}

async fn stratus_change_to_follower(params: Params<'_>, ctx: Arc<RpcContext>, ext: Extensions) -> Result<JsonValue, StratusError> {
ext.authentication().auth_admin()?;
let permit = MODE_CHANGE_SEMAPHORE.try_acquire();
let _permit: SemaphorePermit = match permit {
Ok(permit) => permit,
Expand Down Expand Up @@ -425,8 +423,7 @@ async fn stratus_change_to_follower(params: Params<'_>, ctx: Arc<RpcContext>, ex
Ok(json!(true))
}

async fn stratus_init_importer(params: Params<'_>, ctx: Arc<RpcContext>, ext: Extensions) -> Result<JsonValue, StratusError> {
ext.authentication().auth_admin()?;
async fn stratus_init_importer(params: Params<'_>, ctx: Arc<RpcContext>, _: Extensions) -> Result<JsonValue, StratusError> {
let (params, external_rpc) = next_rpc_param::<String>(params.sequence())?;
let (params, external_rpc_ws) = next_rpc_param::<String>(params)?;
let (params, raw_external_rpc_timeout) = next_rpc_param::<String>(params)?;
Expand All @@ -452,8 +449,7 @@ async fn stratus_init_importer(params: Params<'_>, ctx: Arc<RpcContext>, ext: Ex
importer_config.init_follower_importer(ctx).await
}

fn stratus_shutdown_importer(_: Params<'_>, ctx: &RpcContext, ext: &Extensions) -> Result<JsonValue, StratusError> {
ext.authentication().auth_admin()?;
fn stratus_shutdown_importer(_: Params<'_>, ctx: &RpcContext, _: &Extensions) -> Result<JsonValue, StratusError> {
if GlobalState::get_node_mode() != NodeMode::Follower {
tracing::error!("node is currently not a follower");
return Err(StratusError::StratusNotFollower);
Expand All @@ -472,10 +468,8 @@ fn stratus_shutdown_importer(_: Params<'_>, ctx: &RpcContext, ext: &Extensions)
Ok(json!(true))
}

async fn stratus_change_miner_mode(params: Params<'_>, ctx: Arc<RpcContext>, ext: Extensions) -> Result<JsonValue, StratusError> {
ext.authentication().auth_admin()?;
async fn stratus_change_miner_mode(params: Params<'_>, ctx: Arc<RpcContext>, _: Extensions) -> Result<JsonValue, StratusError> {
let (_, mode_str) = next_rpc_param::<String>(params.sequence())?;

let mode = MinerMode::from_str(&mode_str).map_err(|e| {
tracing::error!(reason = ?e, "failed to parse miner mode");
StratusError::MinerModeParamInvalid
Expand Down Expand Up @@ -536,40 +530,34 @@ async fn change_miner_mode(new_mode: MinerMode, ctx: &RpcContext) -> Result<Json
Ok(json!(true))
}

fn stratus_enable_unknown_clients(_: Params<'_>, _: &RpcContext, ext: &Extensions) -> Result<bool, StratusError> {
ext.authentication().auth_admin()?;
fn stratus_enable_unknown_clients(_: Params<'_>, _: &RpcContext, _: &Extensions) -> bool {
GlobalState::set_unknown_client_enabled(true);
Ok(GlobalState::is_unknown_client_enabled())
GlobalState::is_unknown_client_enabled()
}

fn stratus_disable_unknown_clients(_: Params<'_>, _: &RpcContext, ext: &Extensions) -> Result<bool, StratusError> {
ext.authentication().auth_admin()?;
fn stratus_disable_unknown_clients(_: Params<'_>, _: &RpcContext, _: &Extensions) -> bool {
GlobalState::set_unknown_client_enabled(false);
Ok(GlobalState::is_unknown_client_enabled())
GlobalState::is_unknown_client_enabled()
}

fn stratus_enable_transactions(_: Params<'_>, _: &RpcContext, ext: &Extensions) -> Result<bool, StratusError> {
ext.authentication().auth_admin()?;
fn stratus_enable_transactions(_: Params<'_>, _: &RpcContext, _: &Extensions) -> bool {
GlobalState::set_transactions_enabled(true);
Ok(GlobalState::is_transactions_enabled())
GlobalState::is_transactions_enabled()
}

fn stratus_disable_transactions(_: Params<'_>, _: &RpcContext, ext: &Extensions) -> Result<bool, StratusError> {
ext.authentication().auth_admin()?;
fn stratus_disable_transactions(_: Params<'_>, _: &RpcContext, _: &Extensions) -> bool {
GlobalState::set_transactions_enabled(false);
Ok(GlobalState::is_transactions_enabled())
GlobalState::is_transactions_enabled()
}

fn stratus_enable_miner(_: Params<'_>, ctx: &RpcContext, ext: &Extensions) -> Result<bool, StratusError> {
ext.authentication().auth_admin()?;
fn stratus_enable_miner(_: Params<'_>, ctx: &RpcContext, _: &Extensions) -> bool {
ctx.miner.unpause();
Ok(true)
true
}

fn stratus_disable_miner(_: Params<'_>, ctx: &RpcContext, ext: &Extensions) -> Result<bool, StratusError> {
ext.authentication().auth_admin()?;
fn stratus_disable_miner(_: Params<'_>, ctx: &RpcContext, _: &Extensions) -> bool {
ctx.miner.pause();
Ok(false)
false
}

/// Returns the count of executed transactions waiting to enter the next block.
Expand Down

0 comments on commit 0d2f8ed

Please sign in to comment.