fix(station): re-check the requester after preparing a request

core/station/impl/src/models/request_policy.rs

@@ -166,14 +166,19 @@ pub mod request_policy_test_utils {
         core::CallContext,
         errors::RequestPolicyError,
         models::{
-            request_policy_rule::RequestPolicyRule, request_specifier::RequestSpecifier,
+            permission::{Allow, Permission},
+            request_policy_rule::RequestPolicyRule,
+            request_specifier::RequestSpecifier,
+            resource::{Resource, UserResourceAction},
             AddNamedRuleOperationInput, AddRequestPolicyOperationInput, AddUserOperationInput,
             MetadataItem, ADMIN_GROUP_ID,
         },
+        repositories::permission::PERMISSION_REPOSITORY,
         services::{NAMED_RULE_SERVICE, REQUEST_POLICY_SERVICE, REQUEST_SERVICE, USER_SERVICE},
     };
     use candid::Principal;
-    use orbit_essentials::model::ModelValidator;
+    use orbit_essentials::model::{ModelKey, ModelValidator};
+    use orbit_essentials::repository::Repository;
     use station_api::CreateRequestInput;
     use uuid::Uuid;
 
@@ -222,6 +227,12 @@ pub mod request_policy_test_utils {
             })
             .expect("Failed to add user");
 
+        let permission = Permission::new(
+            Allow::user_groups(vec![*ADMIN_GROUP_ID]),
+            Resource::User(UserResourceAction::Create),
+        );
+        PERMISSION_REPOSITORY.insert(permission.key(), permission);
+
         let ctx = CallContext::new(user.identities[0]);
 
         let request = REQUEST_SERVICE

core/station/impl/src/services/request.rs

@@ -327,8 +327,19 @@ impl RequestService {
         ctx: &CallContext,
     ) -> ServiceResult<Request> {
         let requester = self.user_service.get_user_by_identity(&ctx.caller())?;
+        let resource = Resource::from(&input);
         let mut request = RequestFactory::create_request(requester.id, input).await?;
 
+        // Preparing the request can await an external validation call, so the requester is
+        // re-checked against the current state before the request is stored.
+        let current_ctx = CallContext::new(ctx.caller());
+        let requester_is_current_and_active = current_ctx
+            .user()
+            .is_some_and(|user| user.id == requester.id && user.is_active());
+        if !requester_is_current_and_active || !Authorization::is_allowed(&current_ctx, &resource) {
+            return Err(RequestError::Unauthorized.into());
+        }
+
         // Different request types may have different validation rules.
         request.validate()?;
 
@@ -571,12 +582,12 @@ mod tests {
         models::{
             account_test_utils::mock_account,
             asset_test_utils::mock_asset,
-            permission::Allow,
+            permission::{Allow, Permission},
             request_policy_rule::RequestPolicyRule,
             request_policy_test_utils::mock_request_policy,
             request_specifier::{RequestSpecifier, UserSpecifier},
             request_test_utils::mock_request,
-            resource::ResourceIds,
+            resource::{AccountResourceAction, ResourceAction, ResourceIds},
             user_test_utils::mock_user,
             AddAccountOperationInput, AddAddressBookEntryOperation,
             AddAddressBookEntryOperationInput, AddAssetOperationInput, AddUserOperation,
@@ -585,8 +596,9 @@ mod tests {
             TransferOperation, TransferOperationInput, User, UserGroup, UserStatus, ADMIN_GROUP_ID,
         },
         repositories::{
-            request_policy::REQUEST_POLICY_REPOSITORY, AccountRepository, AssetRepository,
-            NOTIFICATION_REPOSITORY, USER_GROUP_REPOSITORY, USER_REPOSITORY,
+            permission::PERMISSION_REPOSITORY, request_policy::REQUEST_POLICY_REPOSITORY,
+            AccountRepository, AssetRepository, NOTIFICATION_REPOSITORY, USER_GROUP_REPOSITORY,
+            USER_REPOSITORY,
         },
         services::{AccountService, ASSET_SERVICE},
     };
@@ -625,6 +637,14 @@ mod tests {
 
         USER_REPOSITORY.insert(user.to_key(), user.clone());
 
+        for resource in [
+            Resource::AddressBook(ResourceAction::Create),
+            Resource::Account(AccountResourceAction::Transfer(ResourceId::Any)),
+        ] {
+            let permission = Permission::new(Allow::user_groups(vec![*ADMIN_GROUP_ID]), resource);
+            PERMISSION_REPOSITORY.insert(permission.key(), permission);
+        }
+
         let call_context = CallContext::new(caller_principal);
 
         TestContext {
@@ -847,6 +867,92 @@ mod tests {
         assert!(!request.approvals.is_empty());
     }
 
+    #[tokio::test]
+    async fn create_request_refuses_requester_deactivated_after_authorization() {
+        let ctx = setup();
+
+        let input = CreateRequestInput {
+            operation: station_api::RequestOperationInput::AddAddressBookEntry(
+                station_api::AddAddressBookEntryOperationInput {
+                    address_owner: "".to_owned(),
+                    address: "abc".to_owned(),
+                    blockchain: "icp".to_owned(),
+                    metadata: vec![],
+                    labels: vec![],
+                    address_format: AddressFormat::ICPAccountIdentifier.to_string(),
+                },
+            ),
+            title: None,
+            summary: None,
+            execution_plan: None,
+            expiration_dt: None,
+            deduplication_key: None,
+            tags: None,
+        };
+
+        ctx.service
+            .create_request(input.clone(), &ctx.call_context)
+            .await
+            .expect("an active requester can create the request");
+        assert_eq!(ctx.repository.list().len(), 1);
+
+        let mut deactivated_user = ctx.caller_user.clone();
+        deactivated_user.status = UserStatus::Inactive;
+        USER_REPOSITORY.insert(deactivated_user.to_key(), deactivated_user);
+
+        let error = ctx
+            .service
+            .create_request(input, &ctx.call_context)
+            .await
+            .expect_err("a deactivated requester cannot create the request");
+
+        assert_eq!(error.code, "UNAUTHORIZED");
+        assert_eq!(ctx.repository.list().len(), 1);
+    }
+
+    #[tokio::test]
+    async fn create_request_refuses_deactivated_requester_with_public_permission() {
+        let ctx = setup();
+
+        let permission = Permission::new(
+            Allow::public(),
+            Resource::AddressBook(ResourceAction::Create),
+        );
+        PERMISSION_REPOSITORY.insert(permission.key(), permission);
+
+        let mut deactivated_user = ctx.caller_user.clone();
+        deactivated_user.status = UserStatus::Inactive;
+        USER_REPOSITORY.insert(deactivated_user.to_key(), deactivated_user);
+
+        let input = CreateRequestInput {
+            operation: station_api::RequestOperationInput::AddAddressBookEntry(
+                station_api::AddAddressBookEntryOperationInput {
+                    address_owner: "".to_owned(),
+                    address: "abc".to_owned(),
+                    blockchain: "icp".to_owned(),
+                    metadata: vec![],
+                    labels: vec![],
+                    address_format: AddressFormat::ICPAccountIdentifier.to_string(),
+                },
+            ),
+            title: None,
+            summary: None,
+            execution_plan: None,
+            expiration_dt: None,
+            deduplication_key: None,
+            tags: None,
+        };
+
+        let error = ctx
+            .service
+            .create_request(input, &ctx.call_context)
+            .await
+            .expect_err("a deactivated requester cannot create the request");
+
+        assert_eq!(error.code, "UNAUTHORIZED");
+        assert!(ctx.repository.list().is_empty());
+    }
+
     #[tokio::test]
     async fn user_can_cancel_their_own_pending_request() {
         let ctx = setup();

tests/canister/api/spec.did

@@ -10,6 +10,8 @@ type StoreNumberInput = record {
 service : {
   call : (canister_id : principal, method_name : text, arg : blob, total_calls : nat64) -> ();
   validate_number : (input : StoreNumberInput) -> (ValidationResponse);
+  validate_when_released : () -> (ValidationResponse);
+  release_validation : () -> ();
   store_number : (input : StoreNumberInput) -> ();
   get_number : () -> (nat64) query;
   noop : () -> ();

tests/canister/impl/src/lib.rs

@@ -6,6 +6,7 @@ use ic_cdk::{id, query, update};
 
 thread_local! {
     static NUMBER: std::cell::RefCell<u64> = const { std::cell::RefCell::new(0) };
+    static VALIDATION_RELEASED: std::cell::RefCell<bool> = const { std::cell::RefCell::new(false) };
     static RELEASED: std::cell::RefCell<bool> = const { std::cell::RefCell::new(false) };
 }
 
@@ -45,6 +46,21 @@ async fn validate_number(input: StoreNumberInput) -> ValidationResponse {
     }
 }
 
+#[update]
+async fn validate_when_released() -> ValidationResponse {
+    while !VALIDATION_RELEASED.with(|released| *released.borrow()) {
+        let _ = ic_cdk::call::<_, ()>(id(), "noop", ((),)).await;
+    }
+    ValidationResponse::Ok("Released".to_string())
+}
+
+#[update]
+async fn release_validation() {
+    VALIDATION_RELEASED.with(|released| {
+        *released.borrow_mut() = true;
+    });
+}
+
 #[update]
 async fn store_number(input: StoreNumberInput) {
     NUMBER.with(|n| {

tests/integration/src/external_canister_tests.rs

@@ -1,10 +1,10 @@
 use crate::setup::{create_canister, setup_new_env, WALLET_ADMIN_USER};
 use crate::utils::{
     add_user, bump_time_to_avoid_ratelimit, canister_status, deploy_test_canister, execute_request,
-    get_core_canister_health_status, get_request, get_system_info, hash, submit_request,
-    submit_request_approval, submit_request_raw, submit_request_with_expected_trap, update_raw,
-    upload_canister_chunks_to_asset_canister, user_test_id, wait_for_request, ADMIN_GROUP_ID,
-    COUNTER_WAT,
+    expect_await_call_result, get_core_canister_health_status, get_request, get_system_info, hash,
+    submit_request, submit_request_approval, submit_request_raw, submit_request_with_expected_trap,
+    update_raw, upload_canister_chunks_to_asset_canister, user_test_id, wait_for_request,
+    ADMIN_GROUP_ID, COUNTER_WAT,
 };
 use crate::TestEnv;
 use candid::{Encode, Principal};
@@ -21,15 +21,16 @@ use station_api::{
     ChangeExternalCanisterOperationInput, CreateExternalCanisterOperationInput,
     CreateExternalCanisterOperationKindAddExistingDTO,
     CreateExternalCanisterOperationKindCreateNewDTO, CreateExternalCanisterOperationKindDTO,
-    EditPermissionOperationInput, ExecutionMethodResourceTargetDTO,
-    ExternalCanisterCallPermissionDTO, ExternalCanisterCallRequestPolicyRuleInput,
-    ExternalCanisterIdDTO, ExternalCanisterPermissionsCreateInput,
-    ExternalCanisterRequestPoliciesCreateInput, HealthStatus, ListRequestsInput,
-    ListRequestsOperationTypeDTO, ListRequestsResponse, PruneExternalCanisterOperationInput,
-    PruneExternalCanisterResourceDTO, QuorumDTO, RequestApprovalStatusDTO, RequestOperationDTO,
+    CreateRequestInput, CreateRequestResponse, EditPermissionOperationInput,
+    EditUserOperationInput, ExecutionMethodResourceTargetDTO, ExternalCanisterCallPermissionDTO,
+    ExternalCanisterCallRequestPolicyRuleInput, ExternalCanisterIdDTO,
+    ExternalCanisterPermissionsCreateInput, ExternalCanisterRequestPoliciesCreateInput,
+    HealthStatus, ListRequestsInput, ListRequestsOperationTypeDTO, ListRequestsResponse,
+    PruneExternalCanisterOperationInput, PruneExternalCanisterResourceDTO, QuorumDTO,
+    RequestApprovalStatusDTO, RequestExecutionScheduleDTO, RequestOperationDTO,
     RequestOperationInput, RequestPolicyRuleDTO, RequestSpecifierDTO, RequestStatusDTO,
     RestoreExternalCanisterOperationInput, Snapshot, SnapshotExternalCanisterOperationInput,
-    UserSpecifierDTO, ValidationMethodResourceTargetDTO,
+    UserSpecifierDTO, UserStatusDTO, ValidationMethodResourceTargetDTO,
 };
 use std::str::FromStr;
 use std::time::Duration;
@@ -1068,6 +1069,107 @@ fn call_external_canister_test() {
     );
 }
 
+#[test]
+fn create_call_external_canister_request_refuses_requester_deactivated_during_validation() {
+    let TestEnv {
+        env, canister_ids, ..
+    } = setup_new_env();
+
+    let test_canister_id = deploy_test_canister(&env, canister_ids.station);
+
+    let requester = user_test_id(0);
+    let requester_user = add_user(&env, requester, vec![], canister_ids.station);
+
+    let validation_method = CanisterMethodDTO {
+        canister_id: test_canister_id,
+        method_name: "validate_when_released".to_string(),
+    };
+    let execution_method = CanisterMethodDTO {
+        canister_id: test_canister_id,
+        method_name: "noop".to_string(),
+    };
+    execute_request(
+        &env,
+        WALLET_ADMIN_USER,
+        canister_ids.station,
+        RequestOperationInput::EditPermission(EditPermissionOperationInput {
+            resource: station_api::ResourceDTO::ExternalCanister(
+                station_api::ExternalCanisterResourceActionDTO::Call(
+                    CallExternalCanisterResourceTargetDTO {
+                        validation_method: ValidationMethodResourceTargetDTO::ValidationMethod(
+                            validation_method.clone(),
+                        ),
+                        execution_method: ExecutionMethodResourceTargetDTO::ExecutionMethod(
+                            execution_method.clone(),
+                        ),
+                    },
+                ),
+            ),
+            auth_scope: Some(station_api::AuthScopeDTO::Authenticated),
+            user_groups: None,
+            users: None,
+        }),
+    )
+    .unwrap();
+
+    // the validation method does not reply until it is released, which keeps the request creation pending
+    let create_request_input = CreateRequestInput {
+        operation: RequestOperationInput::CallExternalCanister(
+            CallExternalCanisterOperationInput {
+                validation_method: Some(validation_method),
+                execution_method,
+                arg: None,
+                execution_method_cycles: None,
+            },
+        ),
+        title: None,
+        summary: None,
+        execution_plan: Some(RequestExecutionScheduleDTO::Immediate),
+        expiration_dt: None,
+        deduplication_key: None,
+        tags: None,
+    };
+    let message_id = env
+        .submit_call(
+            canister_ids.station,
+            requester,
+            "create_request",
+            Encode!(&create_request_input).unwrap(),
+        )
+        .unwrap();
+    env.tick();
+    env.tick();
+
+    execute_request(
+        &env,
+        WALLET_ADMIN_USER,
+        canister_ids.station,
+        RequestOperationInput::EditUser(EditUserOperationInput {
+            id: requester_user.id,
+            name: None,
+            identities: None,
+            groups: None,
+            status: Some(UserStatusDTO::Inactive),
+            cancel_pending_requests: None,
+        }),
+    )
+    .unwrap();
+
+    update_candid_as::<_, ((),)>(
+        &env,
+        test_canister_id,
+        Principal::anonymous(),
+        "release_validation",
+        ((),),
+    )
+    .unwrap();
+
+    let (result,) = expect_await_call_result::<(ApiResult<CreateRequestResponse>,)>(
+        env.await_call(message_id).unwrap(),
+    );
+    assert_eq!(result.unwrap_err().code, "UNAUTHORIZED");
+}
+
 #[test]
 fn create_external_canister_with_too_many_cycles() {
     let TestEnv {