From afd08d42c7bdc6065690ab57eb8202c57c86df3b Mon Sep 17 00:00:00 2001 From: Sanchith Hegde <22217505+SanchithHegde@users.noreply.github.com> Date: Wed, 8 Mar 2023 14:34:22 +0530 Subject: [PATCH] refactor(authentication): authenticate merchant by API keys from API keys table (#712) --- crates/router/src/core/api_keys.rs | 59 +++++++++++--------- crates/router/src/core/errors.rs | 8 --- crates/router/src/db/api_keys.rs | 30 +++++++++- crates/router/src/routes/metrics.rs | 11 ++-- crates/router/src/scheduler/metrics.rs | 1 + crates/router/src/services/authentication.rs | 47 ++++++++++++++-- crates/storage_models/src/api_keys.rs | 6 ++ crates/storage_models/src/query/api_keys.rs | 14 ++++- 8 files changed, 129 insertions(+), 47 deletions(-) diff --git a/crates/router/src/core/api_keys.rs b/crates/router/src/core/api_keys.rs index de1d0d71f7..9cda9e9562 100644 --- a/crates/router/src/core/api_keys.rs +++ b/crates/router/src/core/api_keys.rs @@ -1,10 +1,8 @@ -use common_utils::{date_time, errors::CustomResult, fp_utils}; +use common_utils::date_time; use error_stack::{report, IntoReport, ResultExt}; -use masking::{PeekInterface, Secret, StrongSecret}; +use masking::{PeekInterface, StrongSecret}; use router_env::{instrument, tracing}; -#[cfg(feature = "kms")] -use crate::services::kms; use crate::{ configs::settings, consts, @@ -14,6 +12,8 @@ use crate::{ types::{api, storage, transformers::ForeignInto}, utils, }; +#[cfg(feature = "kms")] +use crate::{routes::metrics, services::kms}; pub static HASH_KEY: tokio::sync::OnceCell> = tokio::sync::OnceCell::const_new(); @@ -28,6 +28,10 @@ pub async fn get_hash_key( api_key_config.kms_encrypted_hash_key.clone(), ) .await + .map_err(|error| { + metrics::AWS_KMS_FAILURES.add(&metrics::CONTEXT, 1, &[]); + error + }) .change_context(errors::ApiErrorResponse::InternalServerError) .attach_printable("Failed to KMS decrypt API key hashing key")?; @@ -49,11 +53,13 @@ pub async fn get_hash_key( // Defining new types `PlaintextApiKey` and `HashedApiKey` in the hopes of reducing the possibility // of plaintext API key being stored in the data store. -pub struct PlaintextApiKey(Secret); +pub struct PlaintextApiKey(StrongSecret); + +#[derive(Debug, PartialEq, Eq)] pub struct HashedApiKey(String); impl PlaintextApiKey { - pub const HASH_KEY_LEN: usize = 32; + const HASH_KEY_LEN: usize = 32; const PREFIX_LEN: usize = 12; @@ -107,22 +113,6 @@ impl PlaintextApiKey { .to_string(), ) } - - pub fn verify_hash( - &self, - key: &[u8; Self::HASH_KEY_LEN], - stored_api_key: &HashedApiKey, - ) -> CustomResult<(), errors::ApiKeyError> { - // Converting both hashes to `blake3::Hash` since it provides constant-time equality checks - let provided_api_key_hash = blake3::keyed_hash(key, self.0.peek().as_bytes()); - let stored_api_key_hash = blake3::Hash::from_hex(&stored_api_key.0) - .into_report() - .change_context(errors::ApiKeyError::FailedToReadHashFromHex)?; - - fp_utils::when(provided_api_key_hash != stored_api_key_hash, || { - Err(errors::ApiKeyError::HashVerificationFailed).into_report() - }) - } } #[instrument(skip_all)] @@ -165,7 +155,7 @@ pub async fn retrieve_api_key( key_id: &str, ) -> RouterResponse { let api_key = store - .find_api_key_optional(key_id) + .find_api_key_by_key_id_optional(key_id) .await .change_context(errors::ApiErrorResponse::InternalServerError) // If retrieve failed .attach_printable("Failed to retrieve new API key")? @@ -224,12 +214,30 @@ pub async fn list_api_keys( Ok(ApplicationResponse::Json(api_keys)) } +impl From<&str> for PlaintextApiKey { + fn from(s: &str) -> Self { + Self(s.to_owned().into()) + } +} + +impl From for PlaintextApiKey { + fn from(s: String) -> Self { + Self(s.into()) + } +} + impl From for storage::HashedApiKey { fn from(hashed_api_key: HashedApiKey) -> Self { hashed_api_key.0.into() } } +impl From for HashedApiKey { + fn from(hashed_api_key: storage::HashedApiKey) -> Self { + Self(hashed_api_key.into_inner()) + } +} + #[cfg(test)] mod tests { #![allow(clippy::expect_used, clippy::unwrap_used)] @@ -251,8 +259,7 @@ mod tests { hashed_api_key.0.as_bytes() ); - plaintext_api_key - .verify_hash(hash_key.peek(), &hashed_api_key) - .unwrap(); + let new_hashed_api_key = plaintext_api_key.keyed_hash(hash_key.peek()); + assert_eq!(hashed_api_key, new_hashed_api_key) } } diff --git a/crates/router/src/core/errors.rs b/crates/router/src/core/errors.rs index f3bd6c4a7d..7c9bfa4230 100644 --- a/crates/router/src/core/errors.rs +++ b/crates/router/src/core/errors.rs @@ -414,11 +414,3 @@ pub enum WebhooksFlowError { #[error("Resource not found")] ResourceNotFound, } - -#[derive(Debug, thiserror::Error)] -pub enum ApiKeyError { - #[error("Failed to read API key hash from hexadecimal string")] - FailedToReadHashFromHex, - #[error("Failed to verify provided API key hash against stored API key hash")] - HashVerificationFailed, -} diff --git a/crates/router/src/db/api_keys.rs b/crates/router/src/db/api_keys.rs index 85aa26ee3f..3fc1730285 100644 --- a/crates/router/src/db/api_keys.rs +++ b/crates/router/src/db/api_keys.rs @@ -22,11 +22,16 @@ pub trait ApiKeyInterface { async fn revoke_api_key(&self, key_id: &str) -> CustomResult; - async fn find_api_key_optional( + async fn find_api_key_by_key_id_optional( &self, key_id: &str, ) -> CustomResult, errors::StorageError>; + async fn find_api_key_by_hash_optional( + &self, + hashed_api_key: storage::HashedApiKey, + ) -> CustomResult, errors::StorageError>; + async fn list_api_keys_by_merchant_id( &self, merchant_id: &str, @@ -69,7 +74,7 @@ impl ApiKeyInterface for Store { .into_report() } - async fn find_api_key_optional( + async fn find_api_key_by_key_id_optional( &self, key_id: &str, ) -> CustomResult, errors::StorageError> { @@ -80,6 +85,17 @@ impl ApiKeyInterface for Store { .into_report() } + async fn find_api_key_by_hash_optional( + &self, + hashed_api_key: storage::HashedApiKey, + ) -> CustomResult, errors::StorageError> { + let conn = pg_connection(&self.master_pool).await?; + storage::ApiKey::find_optional_by_hashed_api_key(&conn, hashed_api_key) + .await + .map_err(Into::into) + .into_report() + } + async fn list_api_keys_by_merchant_id( &self, merchant_id: &str, @@ -118,7 +134,7 @@ impl ApiKeyInterface for MockDb { Err(errors::StorageError::MockDbError)? } - async fn find_api_key_optional( + async fn find_api_key_by_key_id_optional( &self, _key_id: &str, ) -> CustomResult, errors::StorageError> { @@ -126,6 +142,14 @@ impl ApiKeyInterface for MockDb { Err(errors::StorageError::MockDbError)? } + async fn find_api_key_by_hash_optional( + &self, + _hashed_api_key: storage::HashedApiKey, + ) -> CustomResult, errors::StorageError> { + // [#172]: Implement function for `MockDb` + Err(errors::StorageError::MockDbError)? + } + async fn list_api_keys_by_merchant_id( &self, _merchant_id: &str, diff --git a/crates/router/src/routes/metrics.rs b/crates/router/src/routes/metrics.rs index d01e4c0090..c2a60311ca 100644 --- a/crates/router/src/routes/metrics.rs +++ b/crates/router/src/routes/metrics.rs @@ -5,11 +5,12 @@ use router_env::opentelemetry::{ Context, }; +use crate::create_counter; + pub static CONTEXT: Lazy = Lazy::new(Context::current); static GLOBAL_METER: Lazy = Lazy::new(|| global::meter("ROUTER_API")); -pub(crate) static HEALTH_METRIC: Lazy> = - Lazy::new(|| GLOBAL_METER.u64_counter("HEALTH_API").init()); - -pub(crate) static KV_MISS: Lazy> = - Lazy::new(|| GLOBAL_METER.u64_counter("KV_MISS").init()); +create_counter!(HEALTH_METRIC, GLOBAL_METER); // No. of health API hits +create_counter!(KV_MISS, GLOBAL_METER); // No. of KV misses +#[cfg(feature = "kms")] +create_counter!(AWS_KMS_FAILURES, GLOBAL_METER); // No. of AWS KMS API failures diff --git a/crates/router/src/scheduler/metrics.rs b/crates/router/src/scheduler/metrics.rs index aa4363c329..3fe40a6d32 100644 --- a/crates/router/src/scheduler/metrics.rs +++ b/crates/router/src/scheduler/metrics.rs @@ -11,6 +11,7 @@ static PT_METER: Lazy = Lazy::new(|| global::meter("PROCESS_TRACKER")); pub(crate) static CONSUMER_STATS: Lazy> = Lazy::new(|| PT_METER.f64_histogram("CONSUMER_OPS").init()); +#[macro_export] macro_rules! create_counter { ($name:ident, $meter:ident) => { pub(crate) static $name: Lazy> = diff --git a/crates/router/src/services/authentication.rs b/crates/router/src/services/authentication.rs index e56d3efbe1..15e5fe2ff9 100644 --- a/crates/router/src/services/authentication.rs +++ b/crates/router/src/services/authentication.rs @@ -1,11 +1,16 @@ use actix_web::http::header::HeaderMap; use api_models::{payment_methods::PaymentMethodListRequest, payments::PaymentsRequest}; use async_trait::async_trait; +use common_utils::date_time; use error_stack::{report, IntoReport, ResultExt}; use jsonwebtoken::{decode, Algorithm, DecodingKey, Validation}; +use masking::PeekInterface; use crate::{ - core::errors::{self, RouterResult}, + core::{ + api_keys, + errors::{self, RouterResult}, + }, db::StorageInterface, routes::{app::AppStateInfo, AppState}, services::api, @@ -38,11 +43,45 @@ where request_headers: &HeaderMap, state: &A, ) -> RouterResult { - let api_key = - get_api_key(request_headers).change_context(errors::ApiErrorResponse::Unauthorized)?; + let api_key = get_api_key(request_headers) + .change_context(errors::ApiErrorResponse::Unauthorized)? + .trim(); + if api_key.is_empty() { + return Err(errors::ApiErrorResponse::Unauthorized) + .into_report() + .attach_printable("API key is empty"); + } + + let api_key = api_keys::PlaintextApiKey::from(api_key); + let hash_key = { + let config = state.conf(); + api_keys::HASH_KEY + .get_or_try_init(|| api_keys::get_hash_key(&config.api_keys)) + .await? + }; + let hashed_api_key = api_key.keyed_hash(hash_key.peek()); + + let stored_api_key = state + .store() + .find_api_key_by_hash_optional(hashed_api_key.into()) + .await + .change_context(errors::ApiErrorResponse::InternalServerError) // If retrieve failed + .attach_printable("Failed to retrieve API key")? + .ok_or(report!(errors::ApiErrorResponse::Unauthorized)) // If retrieve returned `None` + .attach_printable("Merchant not authenticated")?; + + if stored_api_key + .expires_at + .map(|expires_at| expires_at < date_time::now()) + .unwrap_or(false) + { + return Err(report!(errors::ApiErrorResponse::Unauthorized)) + .attach_printable("API key has expired"); + } + state .store() - .find_merchant_account_by_api_key(api_key) + .find_merchant_account_by_merchant_id(&stored_api_key.merchant_id) .await .map_err(|e| { if e.current_context().is_db_not_found() { diff --git a/crates/storage_models/src/api_keys.rs b/crates/storage_models/src/api_keys.rs index 01df902e8e..650ea0c7fa 100644 --- a/crates/storage_models/src/api_keys.rs +++ b/crates/storage_models/src/api_keys.rs @@ -81,6 +81,12 @@ impl From for ApiKeyUpdateInternal { #[diesel(sql_type = diesel::sql_types::Text)] pub struct HashedApiKey(String); +impl HashedApiKey { + pub fn into_inner(self) -> String { + self.0 + } +} + impl From for HashedApiKey { fn from(hashed_api_key: String) -> Self { Self(hashed_api_key) diff --git a/crates/storage_models/src/query/api_keys.rs b/crates/storage_models/src/query/api_keys.rs index dde38dc6f8..4d1e317276 100644 --- a/crates/storage_models/src/query/api_keys.rs +++ b/crates/storage_models/src/query/api_keys.rs @@ -3,7 +3,7 @@ use router_env::{instrument, tracing}; use super::generics; use crate::{ - api_keys::{ApiKey, ApiKeyNew, ApiKeyUpdate, ApiKeyUpdateInternal}, + api_keys::{ApiKey, ApiKeyNew, ApiKeyUpdate, ApiKeyUpdateInternal, HashedApiKey}, errors, schema::api_keys::dsl, PgPooledConn, StorageResult, @@ -65,6 +65,18 @@ impl ApiKey { .await } + #[instrument(skip(conn))] + pub async fn find_optional_by_hashed_api_key( + conn: &PgPooledConn, + hashed_api_key: HashedApiKey, + ) -> StorageResult> { + generics::generic_find_one_optional::<::Table, _, _>( + conn, + dsl::hashed_api_key.eq(hashed_api_key), + ) + .await + } + #[instrument(skip(conn))] pub async fn find_by_merchant_id( conn: &PgPooledConn,