From b1f18460d10d59ba97ad05f9c510d23179aa929e Mon Sep 17 00:00:00 2001 From: yenicelik Date: Fri, 10 Feb 2023 17:48:51 +0000 Subject: [PATCH] changes from review. will test now --- scripts/apply-migrations.sh | 6 +- scripts/manual-tests/16-change-user-tier.sh | 0 scripts/manual-tests/19-admin-imitate-user.sh | 0 web3_proxy/src/admin_queries.rs | 82 ++++++++++--------- .../change_user_admin_status.rs | 59 +++++++++---- web3_proxy/src/bin/web3_proxy_cli/main.rs | 2 +- web3_proxy/src/frontend/admin.rs | 6 +- web3_proxy/src/frontend/mod.rs | 2 +- 8 files changed, 93 insertions(+), 64 deletions(-) create mode 100644 scripts/manual-tests/16-change-user-tier.sh create mode 100644 scripts/manual-tests/19-admin-imitate-user.sh diff --git a/scripts/apply-migrations.sh b/scripts/apply-migrations.sh index 179ea9a5..3021239b 100644 --- a/scripts/apply-migrations.sh +++ b/scripts/apply-migrations.sh @@ -1,3 +1,3 @@ -sea-orm-cli migrate up - -# sea-orm-cli generate entity -t \ No newline at end of file +# sea-orm-cli migrate up +# sea-orm-cli generate entity -u mysql://root:dev_web3_proxy@127.0.0.1:13306/dev_web3_proxy -o entities/src --with-serde both +# sea-orm-cli generate entity -t \ No newline at end of file diff --git a/scripts/manual-tests/16-change-user-tier.sh b/scripts/manual-tests/16-change-user-tier.sh new file mode 100644 index 00000000..e69de29b diff --git a/scripts/manual-tests/19-admin-imitate-user.sh b/scripts/manual-tests/19-admin-imitate-user.sh new file mode 100644 index 00000000..e69de29b diff --git a/web3_proxy/src/admin_queries.rs b/web3_proxy/src/admin_queries.rs index 83305808..cfe7eb34 100644 --- a/web3_proxy/src/admin_queries.rs +++ b/web3_proxy/src/admin_queries.rs @@ -8,8 +8,10 @@ use axum::{ TypedHeader, }; use axum::response::{IntoResponse, Response}; -use entities::{admin, user, user_tier}; +use entities::{admin, login, user, user_tier}; use ethers::prelude::Address; +use ethers::types::Bytes; +use ethers::utils::keccak256; use hashbrown::HashMap; use http::StatusCode; use migration::sea_orm::{self, ActiveModelTrait, ColumnTrait, EntityTrait, IntoActiveModel, QueryFilter}; @@ -32,29 +34,13 @@ pub async fn query_admin_modify_usertier<'a>( // Quickly return if any of the input tokens are bad let user_address: Vec = params .get("user_address") - .ok_or_else(|| - FrontendErrorResponse::StatusCode( - StatusCode::BAD_REQUEST, - "Unable to find user_address key in request".to_string(), - None, - ) - )? + .ok_or_else(|| FrontendErrorResponse::BadRequest("Unable to find user_address key in request".to_string()))? .parse::
() - .map_err(|err| { - FrontendErrorResponse::StatusCode( - StatusCode::BAD_REQUEST, - "Unable to parse user_address as an Address".to_string(), - Some(err.into()), - ) - })? + .map_err(|_| FrontendErrorResponse::BadRequest("Unable to parse user_address as an Address".to_string()))? .to_fixed_bytes().into(); let user_tier_title = params .get("user_tier_title") - .ok_or_else(|| FrontendErrorResponse::StatusCode( - StatusCode::BAD_REQUEST, - "Unable to get the user_tier_title key from the request".to_string(), - None, - ))?; + .ok_or_else(||FrontendErrorResponse::BadRequest("Unable to get the user_tier_title key from the request".to_string()))?; // Prepare output body let mut response_body = HashMap::new(); @@ -78,22 +64,18 @@ pub async fn query_admin_modify_usertier<'a>( // Check if the caller is an admin (i.e. if he is in an admin table) let admin: admin::Model = admin::Entity::find() .filter(admin::Column::UserId.eq(caller_id)) - .one(&db_conn) + .one(db_replica.conn()) .await? - .ok_or(AccessDenied.into())?; + .ok_or(AccessDenied)?; // If we are here, that means an admin was found, and we can safely proceed // Fetch the admin, and the user let user: user::Model = user::Entity::find() .filter(user::Column::Address.eq(user_address)) - .one(&db_conn) + .one(db_replica.conn()) .await? - .ok_or(FrontendErrorResponse::StatusCode( - StatusCode::BAD_REQUEST, - "No user with this id found".to_string(), - None, - ))?; + .ok_or(FrontendErrorResponse::BadRequest("No user with this id found".to_string()))?; // Return early if the target user_tier_id is the same as the original user_tier_id response_body.insert( "user_tier_title", @@ -101,20 +83,16 @@ pub async fn query_admin_modify_usertier<'a>( ); // Now we can modify the user's tier - let new_user_tier: user_tier::Model = !user_tier::Entity::find() + let new_user_tier: user_tier::Model = user_tier::Entity::find() .filter(user_tier::Column::Title.eq(user_tier_title.clone())) - .one(&db_conn) + .one(db_replica.conn()) .await? - .ok_or(|| FrontendErrorResponse::StatusCode( - StatusCode::BAD_REQUEST, - "User Tier name was not found".to_string(), - None, - ))?; + .ok_or(FrontendErrorResponse::BadRequest("User Tier name was not found".to_string()))?; if user.user_tier_id == new_user_tier.id { info!("user already has that tier"); } else { - let mut user = user.into_active_model(); + let mut user = user.clone().into_active_model(); user.user_tier_id = sea_orm::Set(new_user_tier.id); @@ -123,11 +101,35 @@ pub async fn query_admin_modify_usertier<'a>( info!("user's tier changed"); } - // Finally, remove the user from redis - // TODO: Also remove the user from the redis - // redis_conn.zrem(); - // redis_conn.get::<_, u64>(&user.) // TODO: Where do i find the bearer token ... + // Query the login table, and get all bearer tokens by this user + let bearer_tokens = login::Entity::find() + .filter(login::Column::UserId.eq(user.id)) + .all(db_replica.conn()) + .await?; + // TODO: Remove from Redis + // Remove multiple items simultaneously, but this should be quick let's not prematurely optimize + let recent_user_id_key = format!("recent_users:id:{}", app.config.chain_id); + let salt = app + .config + .public_recent_ips_salt + .as_ref() + .expect("public_recent_ips_salt must exist in here"); + + // TODO: How do I remove the redis items (?) + for bearer_token in bearer_tokens { + let salted_user_id = format!("{}:{}", salt, bearer_token.user_id); + let hashed_user_id = Bytes::from(keccak256(salted_user_id.as_bytes())); + redis_conn + .zrem(&recent_user_id_key, hashed_user_id.to_string()) + .await?; + } + + // Now delete these tokens ... + login::Entity::delete_many() + .filter(login::Column::UserId.eq(user.id)) + .exec(&db_conn) + .await?; Ok(Json(&response_body).into_response()) diff --git a/web3_proxy/src/bin/web3_proxy_cli/change_user_admin_status.rs b/web3_proxy/src/bin/web3_proxy_cli/change_user_admin_status.rs index 1be5ea86..1ee46659 100644 --- a/web3_proxy/src/bin/web3_proxy_cli/change_user_admin_status.rs +++ b/web3_proxy/src/bin/web3_proxy_cli/change_user_admin_status.rs @@ -1,7 +1,9 @@ use anyhow::Context; use argh::FromArgs; use entities::{admin, login, user}; -use ethers::types::Address; +use ethers::types::{Address, Bytes}; +use ethers::utils::keccak256; +use http::StatusCode; use log::{debug, info}; use migration::sea_orm::{ self, ActiveModelTrait, ColumnTrait, DatabaseConnection, EntityTrait, ModelTrait, IntoActiveModel, @@ -34,17 +36,19 @@ impl ChangeUserAdminStatusSubCommand { .filter(user::Column::Address.eq(address.clone())) .one(db_conn) .await? - .context("No user found with that address")?; - - // Check if there is a record in the database - let mut admin = admin::Entity::find() - .filter(admin::Column::UserId.eq(address)) - .all(db_conn) - .await?; + .context(format!("No user with this id found {:?}", address))?; debug!("user: {:#?}", user); - match admin.pop() { + // Check if there is a record in the database + match admin::Entity::find() + .filter(admin::Column::UserId.eq(address)) + .one(db_conn) + .await? { + Some(old_admin) if !should_be_admin => { + // User is already an admin, but shouldn't be + old_admin.delete(db_conn).await?; + } None if should_be_admin => { // User is not an admin yet, but should be let new_admin = admin::ActiveModel { @@ -52,19 +56,42 @@ impl ChangeUserAdminStatusSubCommand { ..Default::default() }; new_admin.insert(db_conn).await?; - }, - Some(old_admin) if !should_be_admin => { - // User is already an admin, but shouldn't be - old_admin.delete(db_conn).await?; - }, - _ => {} + } + _ => { + // Do nothing in this case + debug!("no change needed for: {:#?}", user); + // Early return + return Ok(()); + } } + // Get the bearer tokens of this user and delete them ... + let bearer_tokens = login::Entity::find() + .filter(login::Column::UserId.eq(user.id)) + .all(db_conn) + .await?; + + // // TODO: Remove from Redis + // // Remove multiple items simultaneously, but this should be quick let's not prematurely optimize + // let recent_user_id_key = format!("recent_users:id:{}", app.config.chain_id); + // let salt = app + // .config + // .public_recent_ips_salt + // .as_ref() + // .expect("public_recent_ips_salt must exist in here"); + // + // // TODO: Also clear redis ... + // let salted_user_id = format!("{}:{}", salt, bearer_token.user_id); + // let hashed_user_id = Bytes::from(keccak256(salted_user_id.as_bytes())); + // redis_conn + // .zrem(&recent_user_id_key, hashed_user_id.to_string()) + // .await?; + // Remove any user logins from the database (incl. bearer tokens) let delete_result = login::Entity::delete_many() .filter(login::Column::UserId.eq(user.id)) .exec(db_conn) - .await; + .await?; debug!("cleared modified logins: {:?}", delete_result); diff --git a/web3_proxy/src/bin/web3_proxy_cli/main.rs b/web3_proxy/src/bin/web3_proxy_cli/main.rs index dcdbba61..def7dbf1 100644 --- a/web3_proxy/src/bin/web3_proxy_cli/main.rs +++ b/web3_proxy/src/bin/web3_proxy_cli/main.rs @@ -309,7 +309,7 @@ fn main() -> anyhow::Result<()> { SubCommand::ChangeUserTierByAddress(x) => { let db_url = cli_config .db_url - .expect("'--config' (with a db) or '--db-url' is required to run proxyd"); + .expect("'--config' (with a db) or '--db-url' is required to run change_user_admin_status"); let db_conn = get_db(db_url, 1, 1).await?; diff --git a/web3_proxy/src/frontend/admin.rs b/web3_proxy/src/frontend/admin.rs index 6532d312..8ef18785 100644 --- a/web3_proxy/src/frontend/admin.rs +++ b/web3_proxy/src/frontend/admin.rs @@ -17,7 +17,7 @@ use axum::{ response::IntoResponse, Extension, Json, TypedHeader, }; -use axum_client_ip::ClientIp; +use axum_client_ip::InsecureClientIp; use axum_macros::debug_handler; use chrono::{TimeZone, Utc}; use entities::sea_orm_active_enums::{LogLevel, Role}; @@ -67,7 +67,7 @@ pub async fn admin_change_user_roles( #[debug_handler] pub async fn admin_login_get( Extension(app): Extension>, - ClientIp(ip): ClientIp, + InsecureClientIp(ip): InsecureClientIp, Path(mut params): Path>, ) -> FrontendResult { // First check if the login is authorized @@ -229,7 +229,7 @@ pub async fn admin_login_get( #[debug_handler] pub async fn admin_login_post( Extension(app): Extension>, - ClientIp(ip): ClientIp, + InsecureClientIp(ip): InsecureClientIp, Query(query): Query, Json(payload): Json, ) -> FrontendResult { diff --git a/web3_proxy/src/frontend/mod.rs b/web3_proxy/src/frontend/mod.rs index 82b9bd7c..140e936c 100644 --- a/web3_proxy/src/frontend/mod.rs +++ b/web3_proxy/src/frontend/mod.rs @@ -172,7 +172,7 @@ pub async fn serve(port: u16, proxy_app: Arc) -> anyhow::Result<() .route("/admin/modify_role", get(admin::admin_change_user_roles)) .route("/admin/imitate-login/:user_address", get(admin::admin_login_get)) .route( - "/user/imitate-login/:user_address/:message_eip", + "/admin/imitate-login/:user_address/:message_eip", get(admin::admin_login_get), ) .route("/admin/imitate-login", post(admin::admin_login_post))