From b628821d6013dafdc9287ed777e576e173f4811d Mon Sep 17 00:00:00 2001
From: Tobin Feldman-Fitzthum <tobin@ibm.com>
Date: Wed, 4 Dec 2024 12:05:18 -0600
Subject: [PATCH] plugins: minor cleanup of api-server code

Since the path doesn't necessarily represent a plugin, let's not call it
a plugin and sub_path. Instead, let's just say base_path and
additional_path.

I think this clarifies the relationship between the built-in features
and the plugins.

Signed-off-by: Tobin Feldman-Fitzthum <tobin@ibm.com>
---
 kbs/src/api_server.rs | 41 ++++++++++++++++++++++-------------------
 kbs/src/error.rs      |  8 ++++----
 2 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/kbs/src/api_server.rs b/kbs/src/api_server.rs
index 694f5cdd0..cdef86e97 100644
--- a/kbs/src/api_server.rs
+++ b/kbs/src/api_server.rs
@@ -100,7 +100,7 @@ impl ApiServer {
                     .wrap(middleware::Logger::default())
                     .app_data(web::Data::new(api_server))
                     .service(
-                        web::resource([kbs_path!("{plugin}{sub_path:.*}")])
+                        web::resource([kbs_path!("{base_path}{additional_path:.*}")])
                             .route(web::get().to(api))
                             .route(web::post().to(api)),
                     )
@@ -138,22 +138,23 @@ pub(crate) async fn api(
     core: web::Data<ApiServer>,
 ) -> Result<HttpResponse> {
     let query = request.query_string();
-    let plugin_name = request
+    let base_path = request
         .match_info()
-        .get("plugin")
-        .ok_or(Error::IllegalAccessedPath {
-            path: request.path().to_string(),
-        })?;
-    let sub_path = request
-        .match_info()
-        .get("sub_path")
-        .ok_or(Error::IllegalAccessedPath {
+        .get("base_path")
+        .ok_or(Error::InvalidRequestPath {
             path: request.path().to_string(),
         })?;
+    let additional_path =
+        request
+            .match_info()
+            .get("additional_path")
+            .ok_or(Error::InvalidRequestPath {
+                path: request.path().to_string(),
+            })?;
 
-    let end_point = format!("{plugin_name}{sub_path}");
+    let endpoint = format!("{base_path}{additional_path}");
 
-    match plugin_name {
+    match base_path {
         #[cfg(feature = "as")]
         "auth" if request.method() == Method::POST => core
             .attestation_service
@@ -188,6 +189,8 @@ pub(crate) async fn api(
 
             Ok(HttpResponse::Ok().content_type("text/xml").body(policy))
         }
+        // If the base_path cannot be served by any of the above built-in
+        // functions, try fulfilling the request via the PluginManager.
         plugin_name => {
             let plugin = core
                 .plugin_manager
@@ -198,20 +201,20 @@ pub(crate) async fn api(
 
             let body = body.to_vec();
             if plugin
-                .validate_auth(&body, query, sub_path, request.method())
+                .validate_auth(&body, query, additional_path, request.method())
                 .await
                 .map_err(|e| Error::PluginInternalError { source: e })?
             {
-                // Plugin calls needs to be authorized by the admin auth
+                // Plugin calls need to be authorized by the admin auth
                 core.admin_auth.validate_auth(&request)?;
                 let response = plugin
-                    .handle(&body, query, sub_path, request.method())
+                    .handle(&body, query, additional_path, request.method())
                     .await
                     .map_err(|e| Error::PluginInternalError { source: e })?;
 
                 Ok(HttpResponse::Ok().content_type("text/xml").body(response))
             } else {
-                // Plugin calls needs to be authorized by the Token and policy
+                // Plugin calls need to be authorized by the Token and policy
                 let token = core
                     .get_attestation_token(&request)
                     .await
@@ -222,16 +225,16 @@ pub(crate) async fn api(
                 let claim_str = serde_json::to_string(&claims)?;
 
                 // TODO: add policy filter support for other plugins
-                if !core.policy_engine.evaluate(&end_point, &claim_str).await? {
+                if !core.policy_engine.evaluate(&endpoint, &claim_str).await? {
                     return Err(Error::PolicyDeny);
                 }
 
                 let response = plugin
-                    .handle(&body, query, sub_path, request.method())
+                    .handle(&body, query, additional_path, request.method())
                     .await
                     .map_err(|e| Error::PluginInternalError { source: e })?;
                 if plugin
-                    .encrypted(&body, query, sub_path, request.method())
+                    .encrypted(&body, query, additional_path, request.method())
                     .await
                     .map_err(|e| Error::PluginInternalError { source: e })?
                 {
diff --git a/kbs/src/error.rs b/kbs/src/error.rs
index 344c86f07..d66bceeed 100644
--- a/kbs/src/error.rs
+++ b/kbs/src/error.rs
@@ -37,8 +37,8 @@ pub enum Error {
         source: anyhow::Error,
     },
 
-    #[error("Accessed path {path} is illegal")]
-    IllegalAccessedPath { path: String },
+    #[error("Request path {path} is invalid")]
+    InvalidRequestPath { path: String },
 
     #[error("JWE failed")]
     JweError {
@@ -95,7 +95,7 @@ impl ResponseError for Error {
 
         // Per the KBS protocol, errors should yield 401 or 404 reponses
         let mut res = match self {
-            Error::IllegalAccessedPath { .. } | Error::PluginNotFound { .. } => {
+            Error::InvalidRequestPath { .. } | Error::PluginNotFound { .. } => {
                 HttpResponse::NotFound()
             }
             _ => HttpResponse::Unauthorized(),
@@ -114,7 +114,7 @@ mod tests {
     use super::Error;
 
     #[rstest]
-    #[case(Error::IllegalAccessedPath{path: "test".into()})]
+    #[case(Error::InvalidRequestPath{path: "test".into()})]
     #[case(Error::PluginNotFound{plugin_name: "test".into()})]
     fn into_error_response(#[case] err: Error) {
         let _ = actix_web::ResponseError::error_response(&err);