diff --git a/crates/router/src/compatibility/stripe/errors.rs b/crates/router/src/compatibility/stripe/errors.rs index cb545ed20f..2b4c4cb575 100644 --- a/crates/router/src/compatibility/stripe/errors.rs +++ b/crates/router/src/compatibility/stripe/errors.rs @@ -384,7 +384,6 @@ impl From for StripeErrorCode { errors::ApiErrorResponse::PreconditionFailed { message } => { Self::PreconditionFailed { message } } - errors::ApiErrorResponse::BadCredentials => Self::Unauthorized, errors::ApiErrorResponse::InvalidDataValue { field_name } => Self::ParameterMissing { field_name: field_name.to_owned(), param: field_name.to_owned(), diff --git a/crates/router/src/core/errors.rs b/crates/router/src/core/errors.rs index 7d4c141949..9af3f0b33c 100644 --- a/crates/router/src/core/errors.rs +++ b/crates/router/src/core/errors.rs @@ -13,7 +13,7 @@ use router_env::opentelemetry::metrics::MetricsError; use storage_models::errors as storage_errors; pub use self::api_error_response::ApiErrorResponse; -pub(crate) use self::utils::{ApiClientErrorExt, ConnectorErrorExt, StorageErrorExt}; +pub(crate) use self::utils::{ConnectorErrorExt, StorageErrorExt}; use crate::services; pub type RouterResult = CustomResult; pub type RouterResponse = CustomResult, ApiErrorResponse>; @@ -246,22 +246,8 @@ pub enum ApiClientError { #[error("Failed to decode response")] ResponseDecodingFailed, - #[error("Server responded with Bad Request")] - BadRequestReceived(bytes::Bytes), - #[error("Server responded with Unauthorized")] - UnauthorizedReceived(bytes::Bytes), - #[error("Server responded with Forbidden")] - ForbiddenReceived, - #[error("Server responded with Not Found")] - NotFoundReceived(bytes::Bytes), - #[error("Server responded with Method Not Allowed")] - MethodNotAllowedReceived, #[error("Server responded with Request Timeout")] RequestTimeoutReceived, - #[error("Server responded with Unprocessable Entity")] - UnprocessableEntityReceived(bytes::Bytes), - #[error("Server responded with Too Many Requests")] - TooManyRequestsReceived, #[error("Server responded with Internal Server Error")] InternalServerErrorReceived, diff --git a/crates/router/src/core/errors/api_error_response.rs b/crates/router/src/core/errors/api_error_response.rs index 213ee325b2..aca5a7dfe7 100644 --- a/crates/router/src/core/errors/api_error_response.rs +++ b/crates/router/src/core/errors/api_error_response.rs @@ -19,15 +19,9 @@ pub enum ErrorType { pub enum ApiErrorResponse { #[error( error_type = ErrorType::InvalidRequestError, code = "IR_01", - message = "API key not provided. Provide API key in the Authorization header, using api-key (e.g api-key: API_KEY)." + message = "API key not provided or invalid API key used. Provide API key in the Authorization header or create new API key, using api-key (e.g api-key: API_KEY)." )] Unauthorized, - #[error( - error_type = ErrorType::InvalidRequestError, code = "IR_02", - message = "Access forbidden, invalid API key was used. Please create your new API key from \ - the Dashboard Settings section." - )] - BadCredentials, #[error(error_type = ErrorType::InvalidRequestError, code = "IR_03", message = "Unrecognized request URL.")] InvalidRequestUrl, #[error(error_type = ErrorType::InvalidRequestError, code = "IR_04", message = "The HTTP method is not applicable for this API.")] @@ -148,20 +142,18 @@ impl actix_web::ResponseError for ApiErrorResponse { use reqwest::StatusCode; match self { - Self::Unauthorized | Self::BadCredentials | Self::InvalidEphermeralKey => { - StatusCode::UNAUTHORIZED - } // 401 - Self::InvalidRequestUrl => StatusCode::NOT_FOUND, // 404 - Self::InvalidHttpMethod => StatusCode::METHOD_NOT_ALLOWED, // 405 + Self::Unauthorized | Self::InvalidEphermeralKey => StatusCode::UNAUTHORIZED, // 401 + Self::InvalidRequestUrl => StatusCode::NOT_FOUND, // 404 + Self::InvalidHttpMethod => StatusCode::METHOD_NOT_ALLOWED, // 405 Self::MissingRequiredField { .. } | Self::InvalidDataValue { .. } => { StatusCode::BAD_REQUEST } // 400 Self::InvalidDataFormat { .. } | Self::InvalidRequestData { .. } => { StatusCode::UNPROCESSABLE_ENTITY } // 422 - Self::RefundAmountExceedsPaymentAmount => StatusCode::BAD_REQUEST, // 400 - Self::MaximumRefundCount => StatusCode::BAD_REQUEST, // 400 - Self::PreconditionFailed { .. } => StatusCode::BAD_REQUEST, // 400 + Self::RefundAmountExceedsPaymentAmount => StatusCode::BAD_REQUEST, // 400 + Self::MaximumRefundCount => StatusCode::BAD_REQUEST, // 400 + Self::PreconditionFailed { .. } => StatusCode::BAD_REQUEST, // 400 Self::PaymentAuthorizationFailed { .. } | Self::PaymentAuthenticationFailed { .. } diff --git a/crates/router/src/core/errors/utils.rs b/crates/router/src/core/errors/utils.rs index 65095ef3bc..a4374ad09e 100644 --- a/crates/router/src/core/errors/utils.rs +++ b/crates/router/src/core/errors/utils.rs @@ -36,27 +36,6 @@ impl StorageErrorExt for error_stack::Report { } } -pub(crate) trait ApiClientErrorExt { - fn to_unsuccessful_processing_step_response( - self, - ) -> error_stack::Report; -} - -impl ApiClientErrorExt for error_stack::Report { - fn to_unsuccessful_processing_step_response( - self, - ) -> error_stack::Report { - let data = match self.current_context() { - errors::ApiClientError::BadRequestReceived(bytes) - | errors::ApiClientError::UnauthorizedReceived(bytes) - | errors::ApiClientError::NotFoundReceived(bytes) - | errors::ApiClientError::UnprocessableEntityReceived(bytes) => Some(bytes.clone()), - _ => None, - }; - self.change_context(errors::ConnectorError::ProcessingStepFailed(data)) - } -} - pub(crate) trait ConnectorErrorExt { fn to_refund_failed_response(self) -> error_stack::Report; fn to_payment_failed_response(self) -> error_stack::Report; diff --git a/crates/router/src/services/api.rs b/crates/router/src/services/api.rs index c5bc263ef7..1da2bd8a3a 100644 --- a/crates/router/src/services/api.rs +++ b/crates/router/src/services/api.rs @@ -18,9 +18,7 @@ pub use self::request::{Method, Request, RequestBuilder}; use crate::{ configs::settings::Connectors, core::{ - errors::{ - self, ApiClientErrorExt, CustomResult, RouterResponse, RouterResult, StorageErrorExt, - }, + errors::{self, CustomResult, RouterResponse, RouterResult, StorageErrorExt}, payments, }, db::StorageInterface, @@ -173,7 +171,8 @@ where logger::debug!(?response); Ok(response) } - Err(error) => Err(error.to_unsuccessful_processing_step_response()), + Err(error) => Err(error + .change_context(errors::ConnectorError::ProcessingStepFailed(None))), } } None => Ok(router_data), @@ -205,7 +204,6 @@ async fn send_request( logger::debug!(method=?request.method, headers=?request.headers, payload=?request.payload, ?request); let url = &request.url; let should_bypass_proxy = client::proxy_bypass_urls(&state.conf.locker).contains(url); - // TODO propogate error for request timeout let client = client::create_client( &state.conf.proxy, should_bypass_proxy, @@ -247,8 +245,11 @@ async fn send_request( Method::Put => client.put(url).add_headers(headers).send().await, Method::Delete => client.delete(url).add_headers(headers).send().await, } + .map_err(|error| match error { + error if error.is_timeout() => errors::ApiClientError::RequestTimeoutReceived, + _ => errors::ApiClientError::RequestNotSent, + }) .into_report() - .change_context(errors::ApiClientError::RequestNotSent) .attach_printable("Unable to send request to connector") } @@ -306,18 +307,15 @@ async fn handle_response( _ => errors::ApiClientError::UnexpectedServerResponse, }; Err(report!(error).attach_printable("Client error response received")) - */ + */ Ok(Err(Response { response: bytes, status_code, })) } - _ => { - // FIXME: may need to understand redirects - Err(report!(errors::ApiClientError::UnexpectedServerResponse) - .attach_printable("Unexpected response from server")) - } + _ => Err(report!(errors::ApiClientError::UnexpectedServerResponse) + .attach_printable("Unexpected response from server")), } })? .await @@ -541,24 +539,16 @@ pub async fn authenticate_merchant<'a>( authenticate_by_api_key(store, api_key).await } - MerchantAuthentication::MerchantId(merchant_id) => { - store - .find_merchant_account_by_merchant_id(&merchant_id) - .await - .map_err(|error| { - // TODO: The BadCredentials error is too specific for api keys, and inappropriate for AdminApiKey/MerchantID - // https://juspay.atlassian.net/browse/ORCA-366 - error.to_not_found_response(errors::ApiErrorResponse::BadCredentials) - }) - } + MerchantAuthentication::MerchantId(merchant_id) => store + .find_merchant_account_by_merchant_id(&merchant_id) + .await + .map_err(|error| error.to_not_found_response(errors::ApiErrorResponse::Unauthorized)), MerchantAuthentication::AdminApiKey => { let admin_api_key = get_api_key(request).change_context(errors::ApiErrorResponse::Unauthorized)?; if admin_api_key != "test_admin" { - // TODO: The BadCredentials error is too specific for api keys, and inappropriate - // for AdminApiKey/MerchantID - Err(report!(errors::ApiErrorResponse::BadCredentials) + Err(report!(errors::ApiErrorResponse::Unauthorized) .attach_printable("Admin Authentication Failure"))?; } @@ -599,7 +589,7 @@ pub async fn authenticate_connector<'a>( ConnectorAuthentication::MerchantId(merchant_id) => store .find_merchant_account_by_merchant_id(merchant_id) .await - .map_err(|error| error.to_not_found_response(errors::ApiErrorResponse::BadCredentials)), + .map_err(|error| error.to_not_found_response(errors::ApiErrorResponse::Unauthorized)), } } @@ -627,7 +617,7 @@ pub(crate) async fn authenticate_eph_key<'a>( let ek = store .get_ephemeral_key(api_key) .await - .change_context(errors::ApiErrorResponse::BadCredentials)?; + .change_context(errors::ApiErrorResponse::Unauthorized)?; utils::when( ek.customer_id.ne(&customer_id), Err(report!(errors::ApiErrorResponse::InvalidEphermeralKey)), @@ -657,7 +647,7 @@ pub async fn authenticate_by_api_key( store .find_merchant_account_by_api_key(api_key) .await - .change_context(errors::ApiErrorResponse::BadCredentials) + .change_context(errors::ApiErrorResponse::Unauthorized) .attach_printable("Merchant not authenticated") } @@ -668,7 +658,7 @@ async fn authenticate_by_publishable_key( store .find_merchant_account_by_publishable_key(publishable_key) .await - .change_context(errors::ApiErrorResponse::BadCredentials) + .change_context(errors::ApiErrorResponse::Unauthorized) .attach_printable("Merchant not authenticated") } diff --git a/crates/router/src/services/api/client.rs b/crates/router/src/services/api/client.rs index f2c7dc4a3b..653f30e442 100644 --- a/crates/router/src/services/api/client.rs +++ b/crates/router/src/services/api/client.rs @@ -100,7 +100,6 @@ pub(super) fn create_client( .attach_printable_lazy(|| "Error with client library") } -// TODO: Move to env variable pub(super) fn proxy_bypass_urls(locker: &Locker) -> Vec { let locker_host = locker.host.to_owned(); let basilisk_host = locker.basilisk_host.to_owned(); diff --git a/crates/router/src/services/api/request.rs b/crates/router/src/services/api/request.rs index f7f3e58532..e7c3d8c357 100644 --- a/crates/router/src/services/api/request.rs +++ b/crates/router/src/services/api/request.rs @@ -1,4 +1,4 @@ -use std::str::FromStr; +use std::{collections, str::FromStr}; use error_stack::{IntoReport, ResultExt}; use masking::Secret; @@ -10,7 +10,7 @@ use crate::{ logger, }; -pub(crate) type Headers = Vec<(String, String)>; +pub(crate) type Headers = collections::HashSet<(String, String)>; #[derive( Clone, Copy, Debug, Eq, PartialEq, Deserialize, Serialize, strum::Display, strum::EnumString, @@ -46,7 +46,7 @@ impl Request { Self { method, url: String::from(url), - headers: Vec::new(), + headers: collections::HashSet::new(), payload: None, content_type: None, certificate: None, @@ -60,7 +60,7 @@ impl Request { pub fn add_header(&mut self, header: &str, value: &str) { self.headers - .push((String::from(header), String::from(value))); + .insert((String::from(header), String::from(value))); } pub fn add_content_type(&mut self, content_type: ContentType) { @@ -91,7 +91,7 @@ impl RequestBuilder { Self { method: Method::Get, url: String::with_capacity(1024), - headers: Vec::new(), + headers: std::collections::HashSet::new(), payload: None, content_type: None, certificate: None, @@ -110,14 +110,13 @@ impl RequestBuilder { } pub fn header(mut self, header: &str, value: &str) -> Self { - self.headers.push((header.into(), value.into())); + self.headers.insert((header.into(), value.into())); self } pub fn headers(mut self, headers: Vec<(String, String)>) -> Self { - // Fixme add union property - let mut h = headers.into_iter().map(|(h, v)| (h, v)).collect(); - self.headers.append(&mut h); + let mut h = headers.into_iter().map(|(h, v)| (h, v)); + self.headers.extend(&mut h); self } diff --git a/crates/router/src/services/encryption.rs b/crates/router/src/services/encryption.rs index 66aa735fc3..b2da15d15f 100644 --- a/crates/router/src/services/encryption.rs +++ b/crates/router/src/services/encryption.rs @@ -234,7 +234,7 @@ pub fn decrypt(mut data: Vec, key: &[u8]) -> CustomResult &str { - let key_identifier = "1"; //TODO https://github.com/juspay/orca/issues/46 + let key_identifier = "1"; // [#46]: Fetch this value from redis or external sources if key_identifier == "1" { &keys.locker_key_identifier1 } else {