From b13e5b24407fc328d60e7d728d898caf22d36e0e Mon Sep 17 00:00:00 2001 From: Bryan Stitt Date: Mon, 3 Jul 2023 12:17:30 -0700 Subject: [PATCH] move more logs to tracing and possible stripe payment fix maybe RawValue does need to_string instead of just get --- web3_proxy/src/errors.rs | 34 +++++--- .../src/frontend/users/payment_stripe.rs | 77 ++++++------------- 2 files changed, 48 insertions(+), 63 deletions(-) diff --git a/web3_proxy/src/errors.rs b/web3_proxy/src/errors.rs index 5df1a75c..302eb4b0 100644 --- a/web3_proxy/src/errors.rs +++ b/web3_proxy/src/errors.rs @@ -146,6 +146,7 @@ pub enum Web3ProxyError { /// simple way to return an error message to the user and an anyhow to our logs #[display(fmt = "{}, {}, {:?}", _0, _1, _2)] StatusCode(StatusCode, Cow<'static, str>, Option), + StripeWebhookError(stripe::WebhookError), /// TODO: what should be attached to the timout? #[display(fmt = "{:?}", _0)] #[error(ignore)] @@ -498,7 +499,7 @@ impl Web3ProxyError { StatusCode::INTERNAL_SERVER_ERROR, JsonRpcErrorData { // TODO: is it safe to expose our io error strings? - message: err.to_string().into(), + message: format!("std io error: {}", err).into(), code: StatusCode::INTERNAL_SERVER_ERROR.as_u16().into(), data: None, }, @@ -674,7 +675,7 @@ impl Web3ProxyError { num_known, min_head_rpcs, } => { - error!("NotEnoughRpcs {}/{}", num_known, min_head_rpcs); + error!(%num_known, %min_head_rpcs, "NotEnoughRpcs"); ( StatusCode::BAD_GATEWAY, JsonRpcErrorData { @@ -689,7 +690,7 @@ impl Web3ProxyError { ) } Self::NotEnoughSoftLimit { available, needed } => { - error!("NotEnoughSoftLimit {}/{}", available, needed); + error!(available, needed, "NotEnoughSoftLimit"); ( StatusCode::BAD_GATEWAY, JsonRpcErrorData { @@ -742,7 +743,7 @@ impl Web3ProxyError { ) } Self::OriginNotAllowed(origin) => { - trace!("OriginNotAllowed origin={}", origin); + trace!(?origin, "OriginNotAllowed"); ( StatusCode::FORBIDDEN, JsonRpcErrorData { @@ -753,7 +754,7 @@ impl Web3ProxyError { ) } Self::ParseBytesError(err) => { - trace!("ParseBytesError err={:#?}", err); + trace!(?err, "ParseBytesError"); ( StatusCode::BAD_REQUEST, JsonRpcErrorData { @@ -764,7 +765,7 @@ impl Web3ProxyError { ) } Self::ParseMsgError(err) => { - trace!("ParseMsgError err={:#?}", err); + trace!(?err, "ParseMsgError"); ( StatusCode::BAD_REQUEST, JsonRpcErrorData { @@ -936,9 +937,22 @@ impl Web3ProxyError { }, ) } + Self::StripeWebhookError(err) => { + trace!(?err, "StripeWebhookError"); + ( + StatusCode::BAD_REQUEST, + JsonRpcErrorData { + message: format!("stripe webhook error: {}", err).into(), + code: StatusCode::BAD_REQUEST.as_u16().into(), + // TODO: include the stripe signature? anything else? + data: None, + }, + ) + } Self::Timeout(x) => ( StatusCode::REQUEST_TIMEOUT, JsonRpcErrorData { + // TODO: prettier message message: format!("request timed out: {:?}", x).into(), code: StatusCode::REQUEST_TIMEOUT.as_u16().into(), // TODO: include the actual id! @@ -946,11 +960,11 @@ impl Web3ProxyError { }, ), Self::UlidDecode(err) => { - // trace!(?err, "UlidDecodeError"); + trace!(?err, "UlidDecodeError"); ( StatusCode::BAD_REQUEST, JsonRpcErrorData { - message: format!("{}", err).into(), + message: format!("ulid decode error: {}", err).into(), code: StatusCode::BAD_REQUEST.as_u16().into(), data: None, }, @@ -1010,7 +1024,7 @@ impl Web3ProxyError { ) } Self::UserAgentNotAllowed(ua) => { - trace!("UserAgentNotAllowed ua={}", ua); + trace!(%ua, "UserAgentNotAllowed"); ( StatusCode::FORBIDDEN, JsonRpcErrorData { @@ -1033,7 +1047,7 @@ impl Web3ProxyError { ) } Self::WatchRecvError(err) => { - error!("WatchRecvError err={:#?}", err); + error!(?err, "WatchRecvError"); ( StatusCode::INTERNAL_SERVER_ERROR, JsonRpcErrorData { diff --git a/web3_proxy/src/frontend/users/payment_stripe.rs b/web3_proxy/src/frontend/users/payment_stripe.rs index 35a0a29f..00dcd92d 100644 --- a/web3_proxy/src/frontend/users/payment_stripe.rs +++ b/web3_proxy/src/frontend/users/payment_stripe.rs @@ -1,15 +1,11 @@ use crate::app::Web3ProxyApp; use crate::errors::{Web3ProxyError, Web3ProxyErrorContext, Web3ProxyResponse}; -use crate::frontend::authorization::{ - login_is_authorized, Authorization as Web3ProxyAuthorization, -}; use anyhow::Context; use axum::{ headers::{authorization::Bearer, Authorization}, response::IntoResponse, Extension, Json, TypedHeader, }; -use axum_client_ip::InsecureClientIp; use axum_macros::debug_handler; use entities::{ balance, increase_on_chain_balance_receipt, rpc_key, stripe_increase_balance_receipt, user, @@ -21,12 +17,12 @@ use migration::sea_orm::{ self, ActiveModelTrait, ColumnTrait, EntityTrait, QueryFilter, TransactionTrait, }; use migration::{Expr, OnConflict}; -use serde::{Deserialize, Serialize}; +use serde::Deserialize; use serde_json::json; use std::num::NonZeroU64; use std::sync::Arc; use stripe::Webhook; -use tracing::{debug, error, trace}; +use tracing::{error, info, trace}; /// `GET /user/balance/stripe` -- Use a bearer token to get the user's balance and spend. /// @@ -55,12 +51,9 @@ pub async fn user_stripe_deposits_get( Ok(Json(response).into_response()) } -// /// the JSON input to the `post_user` handler. -// /// TODO: what else can we update here? password hash? subscription to newsletter? -#[derive(Debug, Serialize, Deserialize)] +/// the JSON input to the `user_balance_stripe_post` handler. +#[derive(Debug, Deserialize)] pub struct StripePost { - // email: Option, - // referral_code: Option, data: Box, } @@ -69,38 +62,18 @@ pub struct StripePost { #[debug_handler] pub async fn user_balance_stripe_post( Extension(app): Extension>, - InsecureClientIp(ip): InsecureClientIp, + // InsecureClientIp(ip): InsecureClientIp, headers: HeaderMap, - bearer: Option>>, Json(payload): Json, ) -> Web3ProxyResponse { - // rate limit by bearer token **OR** IP address - let (_, _semaphore) = if let Some(TypedHeader(Authorization(bearer))) = bearer { - let (_, semaphore) = app.bearer_is_authorized(bearer).await?; + // TODO: (high) rate limits by IP address. login limiter is probably too low + // TODO: maybe instead, a bad stripe-header should ban the IP? or a good one should allow it? - // TODO: is handling this as internal fine? - let authorization = Web3ProxyAuthorization::internal(app.db_conn().ok().cloned())?; + // TODO: lower log level when done testing + info!(?payload, ?headers); - (authorization, Some(semaphore)) - } else { - let authorization = login_is_authorized(&app, ip).await?; - (authorization, None) - }; - - // let recipient_user_id: u64 = params - // .remove("user_id") - // .ok_or(Web3ProxyError::BadRouting)? - // .parse() - // .or(Err(Web3ProxyError::ParseAddressError))?; - - trace!(?payload); - - // Get the payload, and the header - // let payload = payload.data.get("data").ok_or(Web3ProxyError::BadRequest( - // "You have not provided a 'data' for the Stripe payload".into(), - // ))?; - - // TODO Get this from the header + // get the signature from the header + // the docs are inconsistent on the key, so we just check all of them let signature = if let Some(x) = headers.get("stripe-signature") { x } else if let Some(x) = headers.get("Stripe-Signature") { @@ -115,30 +88,28 @@ pub async fn user_balance_stripe_post( )); }; + let payload = + serde_json::to_string(&payload.data).web3_context("could not parse payload data")?; + let signature = signature .to_str() .web3_context("Could not parse stripe signature as byte-string")?; - // Now parse the payload and signature - // TODO: Move env variable elsewhere - let event = Webhook::construct_event( - payload.data.get(), - signature, - app.config - .stripe_api_key - .clone() - .web3_context("Stripe API key not found in config!")? - .as_str(), - ) - .context(Web3ProxyError::BadRequest( - "Could not parse the stripe webhook request!".into(), - ))?; + let secret = app + .config + .stripe_api_key + .clone() + .web3_context("Stripe API key not found in config!")?; + + let event = Webhook::construct_event(&payload, signature, secret.as_str())?; let intent = match event.data.object { stripe::EventObject::PaymentIntent(intent) => intent, _ => return Ok("Received irrelevant webhook".into_response()), }; - debug!("Found PaymentIntent Event: {:?}", intent); + + // TODO: lower log level when done testing + info!(?intent); if intent.status.as_str() != "succeeded" { return Ok("Received Webhook".into_response());