refactor(router): api-key routes refactoring (#600)

crates/router/src/routes/api_keys.rs

@@ -1,13 +1,9 @@
 use actix_web::{web, HttpRequest, Responder};
-use error_stack::{IntoReport, ResultExt};
 use router_env::{instrument, tracing, Flow};
 
 use super::app::AppState;
 use crate::{
-    core::{
+    core::api_keys,
-        api_keys,
-        errors::{self, RouterResult},
-    },
     services::{api, authentication as auth},
     types::api as api_types,
 };
@@ -18,7 +14,7 @@ use crate::{
 /// displayed only once on creation, so ensure you store it securely.
 #[utoipa::path(
     post,
-    path = "/api_keys",
+    path = "/api_keys/{merchant_id)",
     request_body= CreateApiKeyRequest,
     responses(
         (status = 200, description = "API Key created", body = CreateApiKeyResponse),
@@ -31,17 +27,18 @@ use crate::{
 pub async fn api_key_create(
     state: web::Data<AppState>,
     req: HttpRequest,
+    path: web::Path<String>,
     json_payload: web::Json<api_types::CreateApiKeyRequest>,
 ) -> impl Responder {
     let payload = json_payload.into_inner();
+    let merchant_id = path.into_inner();
 
     api::server_wrap(
         state.get_ref(),
         &req,
         payload,
         |state, _, payload| async {
-            let merchant_id = get_merchant_id_header(&req)?;
+            api_keys::create_api_key(&*state.store, payload, merchant_id.clone()).await
-            api_keys::create_api_key(&*state.store, payload, merchant_id).await
         },
         &auth::AdminApiAuth,
     )
@@ -53,7 +50,7 @@ pub async fn api_key_create(
 /// Retrieve information about the specified API Key.
 #[utoipa::path(
     get,
-    path = "/api_keys/{key_id}",
+    path = "/api_keys/{merchant_id}/{key_id}",
     params (("key_id" = String, Path, description = "The unique identifier for the API Key")),
     responses(
         (status = 200, description = "API Key retrieved", body = RetrieveApiKeyResponse),
@@ -66,9 +63,9 @@ pub async fn api_key_create(
 pub async fn api_key_retrieve(
     state: web::Data<AppState>,
     req: HttpRequest,
-    path: web::Path<String>,
+    path: web::Path<(String, String)>,
 ) -> impl Responder {
-    let key_id = path.into_inner();
+    let (key_id, _merchant_id) = path.into_inner();
 
     api::server_wrap(
         state.get_ref(),
@@ -85,7 +82,7 @@ pub async fn api_key_retrieve(
 /// Update information for the specified API Key.
 #[utoipa::path(
     post,
-    path = "/api_keys/{key_id}",
+    path = "/api_keys/{merchant_id}/{key_id}",
     request_body = UpdateApiKeyRequest,
     params (("key_id" = String, Path, description = "The unique identifier for the API Key")),
     responses(
@@ -99,10 +96,10 @@ pub async fn api_key_retrieve(
 pub async fn api_key_update(
     state: web::Data<AppState>,
     req: HttpRequest,
-    path: web::Path<String>,
+    path: web::Path<(String, String)>,
     json_payload: web::Json<api_types::UpdateApiKeyRequest>,
 ) -> impl Responder {
-    let key_id = path.into_inner();
+    let (key_id, _merchant_id) = path.into_inner();
     let payload = json_payload.into_inner();
 
     api::server_wrap(
@@ -121,7 +118,7 @@ pub async fn api_key_update(
 /// authenticating with our APIs.
 #[utoipa::path(
     delete,
-    path = "/api_keys/{key_id}",
+    path = "/api_keys/{merchant_id)/{key_id}",
     params (("key_id" = String, Path, description = "The unique identifier for the API Key")),
     responses(
         (status = 200, description = "API Key revoked", body = RevokeApiKeyResponse),
@@ -134,9 +131,9 @@ pub async fn api_key_update(
 pub async fn api_key_revoke(
     state: web::Data<AppState>,
     req: HttpRequest,
-    path: web::Path<String>,
+    path: web::Path<(String, String)>,
 ) -> impl Responder {
-    let key_id = path.into_inner();
+    let (key_id, _merchant_id) = path.into_inner();
 
     api::server_wrap(
         state.get_ref(),
@@ -153,7 +150,7 @@ pub async fn api_key_revoke(
 /// List all API Keys associated with your merchant account.
 #[utoipa::path(
     get,
-    path = "/api_keys/list",
+    path = "/api_keys/{merchant_id}/list",
     params(
         ("limit" = Option<i64>, Query, description = "The maximum number of API Keys to include in the response"),
         ("skip" = Option<i64>, Query, description = "The number of API Keys to skip when retrieving the list of API keys."),
@@ -168,42 +165,22 @@ pub async fn api_key_revoke(
 pub async fn api_key_list(
     state: web::Data<AppState>,
     req: HttpRequest,
+    path: web::Path<String>,
     query: web::Query<api_types::ListApiKeyConstraints>,
 ) -> impl Responder {
     let list_api_key_constraints = query.into_inner();
     let limit = list_api_key_constraints.limit;
     let offset = list_api_key_constraints.skip;
+    let merchant_id = path.into_inner();
 
     api::server_wrap(
         state.get_ref(),
         &req,
-        (&req, limit, offset),
+        (limit, offset, merchant_id),
-        |state, _, (req, limit, offset)| async move {
+        |state, _, (limit, offset, merchant_id)| async move {
-            let merchant_id = get_merchant_id_header(req)?;
             api_keys::list_api_keys(&*state.store, merchant_id, limit, offset).await
         },
         &auth::AdminApiAuth,
     )
     .await
 }
-
-fn get_merchant_id_header(req: &HttpRequest) -> RouterResult<String> {
-    use crate::headers::X_MERCHANT_ID;
-
-    req.headers()
-        .get(X_MERCHANT_ID)
-        .ok_or_else(|| errors::ApiErrorResponse::InvalidRequestData {
-            message: format!("Missing header: `{X_MERCHANT_ID}`"),
-        })
-        .into_report()?
-        .to_str()
-        .into_report()
-        .change_context(errors::ApiErrorResponse::InvalidDataValue {
-            field_name: X_MERCHANT_ID,
-        })
-        .attach_printable(
-            "Failed to convert header value to string, \
-             possibly contains non-printable or non-ASCII characters",
-        )
-        .map(|s| s.to_owned())
-}

crates/router/src/routes/app.rs

@@ -340,7 +340,7 @@ pub struct ApiKeys;
 #[cfg(feature = "olap")]
 impl ApiKeys {
     pub fn server(state: AppState) -> Scope {
-        web::scope("/api_keys")
+        web::scope("/api_keys/{merchant_id}")
             .app_data(web::Data::new(state))
             .service(web::resource("").route(web::post().to(api_key_create)))
             .service(web::resource("/list").route(web::get().to(api_key_list)))

crates/router/src/services/authentication.rs

@@ -5,7 +5,7 @@ use error_stack::{report, IntoReport, ResultExt};
 use jsonwebtoken::{decode, Algorithm, DecodingKey, Validation};
 
 use crate::{
-    core::errors::{self, RouterResult, StorageErrorExt},
+    core::errors::{self, RouterResult},
     db::StorageInterface,
     routes::{app::AppStateInfo, AppState},
     services::api,
@@ -44,8 +44,13 @@ where
             .store()
             .find_merchant_account_by_api_key(api_key)
             .await
-            .change_context(errors::ApiErrorResponse::Unauthorized)
+            .map_err(|e| {
-            .attach_printable("Merchant not authenticated")
+                if e.current_context().is_db_not_found() {
+                    e.change_context(errors::ApiErrorResponse::Unauthorized)
+                } else {
+                    e.change_context(errors::ApiErrorResponse::InternalServerError)
+                }
+            })
     }
 }
 
@@ -87,7 +92,13 @@ impl AuthenticateAndFetch<storage::MerchantAccount, AppState> for MerchantIdAuth
             .store
             .find_merchant_account_by_merchant_id(self.0.as_ref())
             .await
-            .map_err(|error| error.to_not_found_response(errors::ApiErrorResponse::Unauthorized))
+            .map_err(|e| {
+                if e.current_context().is_db_not_found() {
+                    e.change_context(errors::ApiErrorResponse::Unauthorized)
+                } else {
+                    e.change_context(errors::ApiErrorResponse::InternalServerError)
+                }
+            })
     }
 }
 
@@ -107,8 +118,13 @@ impl AuthenticateAndFetch<storage::MerchantAccount, AppState> for PublishableKey
             .store
             .find_merchant_account_by_publishable_key(publishable_key)
             .await
-            .change_context(errors::ApiErrorResponse::Unauthorized)
+            .map_err(|e| {
-            .attach_printable("Merchant not authenticated")
+                if e.current_context().is_db_not_found() {
+                    e.change_context(errors::ApiErrorResponse::Unauthorized)
+                } else {
+                    e.change_context(errors::ApiErrorResponse::InternalServerError)
+                }
+            })
     }
 }