[Management] Polish the access control
diff --git a/services/management/enclave/src/service.rs b/services/management/enclave/src/service.rs
index 9ae71da..a29ff69 100644
--- a/services/management/enclave/src/service.rs
+++ b/services/management/enclave/src/service.rs
@@ -24,10 +24,10 @@
use std::convert::TryInto;
use std::sync::Arc;
use teaclave_proto::teaclave_common::i32_from_task_status;
+use teaclave_proto::teaclave_frontend_service::*;
use teaclave_proto::teaclave_frontend_service::{
from_proto_file_ids, from_proto_ownership, to_proto_file_ids, to_proto_ownership,
};
-use teaclave_proto::teaclave_frontend_service::*;
use teaclave_proto::teaclave_management_service::{SaveLogsRequest, TeaclaveManagement};
use teaclave_proto::teaclave_storage_service::{
DeleteRequest, EnqueueRequest, GetKeysByPrefixRequest, GetRequest, PutRequest,
@@ -161,7 +161,6 @@
Ok(Response::new(response))
}
- // access control: user_id in owner_list
async fn register_fusion_output(
&self,
request: Request<RegisterFusionOutputRequest>,
@@ -214,7 +213,6 @@
Ok(Response::new(response))
}
- // access control: output_file.owner contains user_id
async fn get_output_file(
&self,
request: Request<GetOutputFileRequest>,
@@ -239,7 +237,6 @@
Ok(Response::new(response))
}
- // access control: input_file.owner contains user_id
async fn get_input_file(
&self,
request: Request<GetInputFileRequest>,
@@ -264,7 +261,7 @@
Ok(Response::new(response))
}
- // access_control: none
+ // access control: none
async fn register_function(
&self,
request: Request<RegisterFunctionRequest>,
@@ -335,8 +332,24 @@
request: Request<UpdateFunctionRequest>,
) -> TeaclaveServiceResponseResult<UpdateFunctionResponse> {
let user_id = get_request_user_id(&request)?;
+ let request = request.into_inner();
- let function = FunctionBuilder::try_from(request.into_inner())
+ let function_id = request
+ .function_id
+ .clone()
+ .try_into()
+ .map_err(|_| ManagementServiceError::InvalidFunctionId)?;
+ let function: Function = self
+ .read_from_db(&function_id)
+ .await
+ .map_err(|_| ManagementServiceError::InvalidFunctionId)?;
+
+ ensure!(
+ function.owner == user_id,
+ ManagementServiceError::PermissionDenied
+ );
+
+ let function = FunctionBuilder::try_from(request)
.map_err(tonic_error)?
.owner(user_id)
.build();
@@ -347,9 +360,6 @@
Ok(Response::new(response))
}
- // access control:
- // function.public || function.owner == user_id || request.role == PlatformAdmin ||
- // requested user_id in the user_allowlist
async fn get_function(
&self,
request: Request<GetFunctionRequest>,
@@ -381,8 +391,6 @@
}
}
- // access control:
- // function.public || request.role == PlatformAdmin || requested user_id in the user_allowlist
async fn get_function_usage_stats(
&self,
request: Request<GetFunctionUsageStatsRequest>,
@@ -423,7 +431,6 @@
Ok(Response::new(response))
}
- // access control: function.owner == user_id
async fn delete_function(
&self,
request: Request<DeleteFunctionRequest>,
@@ -450,8 +457,6 @@
Ok(Response::new(()))
}
- // access control: function.owner == user_id
- // disable function
// 1. `List functions` does not show this function
// 2. `Create new task` with the function id fails
async fn disable_function(
@@ -470,12 +475,10 @@
.await
.map_err(|_| ManagementServiceError::InvalidFunctionId)?;
- if role != UserRole::PlatformAdmin {
- ensure!(
- function.owner == user_id,
- ManagementServiceError::PermissionDenied
- );
- }
+ ensure!(
+ role == UserRole::PlatformAdmin || function.owner == user_id,
+ ManagementServiceError::PermissionDenied
+ );
let func_id = function.external_id().to_string();
// Updated function owner
@@ -516,26 +519,19 @@
Ok(Response::new(()))
}
- // access contro: user_id = request.user_id
async fn list_functions(
&self,
request: Request<ListFunctionsRequest>,
) -> TeaclaveServiceResponseResult<ListFunctionsResponse> {
- let mut request_user_id = request.get_ref().user_id.clone().into();
+ let request_user_id = request.get_ref().user_id.clone().into();
let current_user_id = get_request_user_id(&request)?;
let role = get_request_role(&request)?;
- if role != UserRole::PlatformAdmin {
- ensure!(
- request_user_id == current_user_id,
- ManagementServiceError::PermissionDenied
- );
- }
-
- if let UserRole::DataOwner(s) = &role {
- request_user_id = s.into();
- }
+ ensure!(
+ role == UserRole::PlatformAdmin || request_user_id == current_user_id,
+ ManagementServiceError::PermissionDenied
+ );
let u = User {
id: request_user_id,
@@ -620,7 +616,6 @@
Ok(Response::new(response))
}
- // access control: task.participants.contains(&user_id)
async fn get_task(
&self,
request: Request<GetTaskRequest>,
@@ -661,7 +656,7 @@
Ok(Response::new(response))
}
- // access control:
+ // prerequisite:
// 1) task.participants.contains(user_id)
// 2) task.status == Created
// 3) user can use the data:
@@ -723,7 +718,7 @@
Ok(Response::new(()))
}
- // access_control:
+ // prerequisite:
// 1) task status == Ready
// 2) user_id in task.participants
async fn approve_task(
@@ -759,7 +754,7 @@
Ok(Response::new(()))
}
- // access_control:
+ // prerequisite:
// 1) task status == Approved
// 2) user_id == task.creator
async fn invoke_task(
@@ -829,9 +824,6 @@
Ok(Response::new(()))
}
- // access_control:
- // 1) user_id == task.creator
- // 2) user_role == admin
async fn cancel_task(
&self,
request: Request<CancelTaskRequest>,
@@ -848,15 +840,10 @@
.await
.map_err(|_| ManagementServiceError::InvalidTaskId)?;
- match role {
- UserRole::PlatformAdmin => {}
- _ => {
- ensure!(
- ts.has_creator(&user_id),
- ManagementServiceError::PermissionDenied
- );
- }
- }
+ ensure!(
+ role == UserRole::PlatformAdmin || ts.has_creator(&user_id),
+ ManagementServiceError::PermissionDenied
+ );
match ts.status {
// need scheduler to cancel the task
@@ -891,7 +878,7 @@
Ok(Response::new(()))
}
- // access_control: none
+ // access control: none
async fn save_logs(
&self,
request: Request<SaveLogsRequest>,
@@ -917,8 +904,6 @@
Ok(Response::new(()))
}
- // access_control: only
- // user_role == admin
async fn query_audit_logs(
&self,
request: Request<QueryAuditLogsRequest>,
diff --git a/tests/functional/enclave/src/management_service.rs b/tests/functional/enclave/src/management_service.rs
index 7b73f94..f70c62a 100644
--- a/tests/functional/enclave/src/management_service.rs
+++ b/tests/functional/enclave/src/management_service.rs
@@ -255,9 +255,19 @@
.build();
let mut client = authorized_client("mock_user").await;
- let response = client.update_function(request).await;
+ let response = client.update_function(request.clone()).await;
assert!(response.is_ok());
assert!(original_id.to_string() == response.unwrap().into_inner().function_id);
+
+ let mock_id = ExternalID::try_from("function-00000000-0000-0000-0000-000000000006").unwrap();
+ let mut request_mock_id = request.clone();
+ request_mock_id.function_id = mock_id.to_string();
+ let response = client.update_function(request_mock_id).await;
+ assert!(response.is_err());
+
+ let mut client = authorized_client("another_mock_user").await;
+ let response = client.update_function(request).await;
+ assert!(response.is_err());
}
#[async_test_case]
