DatasetActionAuthorizer::check_action_allowed(): use DatasetID instea…

…d of DatasetHandle
kamu-data · Jan 16, 2025 · 6d4cf77 · 6d4cf77
1 parent f48701b
commit 6d4cf77
Show file tree

Hide file tree

Showing 80 changed files with 440 additions and 162 deletions.
diff --git a/src/adapter/auth-oso-rebac/src/oso_dataset_authorizer.rs b/src/adapter/auth-oso-rebac/src/oso_dataset_authorizer.rs
@@ -61,10 +61,8 @@ impl OsoDatasetAuthorizer {
 
     async fn dataset_resource(
         &self,
-        dataset_handle: &odf::DatasetHandle,
+        dataset_id: &odf::DatasetID,
     ) -> Result<DatasetResource, InternalError> {
-        let dataset_id = &dataset_handle.id;
-
         let dataset_resource = self
             .oso_resource_service
             .dataset_resource(dataset_id)
@@ -86,14 +84,14 @@ impl OsoDatasetAuthorizer {
 
 #[async_trait::async_trait]
 impl DatasetActionAuthorizer for OsoDatasetAuthorizer {
-    #[tracing::instrument(level = "debug", skip_all, fields(%dataset_handle, ?action))]
+    #[tracing::instrument(level = "debug", skip_all, fields(%dataset_id, ?action))]
     async fn check_action_allowed(
         &self,
-        dataset_handle: &odf::DatasetHandle,
+        dataset_id: &odf::DatasetID,
         action: DatasetAction,
     ) -> Result<(), DatasetActionUnauthorizedError> {
         let (user_actor, dataset_resource) =
-            try_join!(self.user_actor(), self.dataset_resource(dataset_handle))?;
+            try_join!(self.user_actor(), self.dataset_resource(dataset_id))?;
 
         match self
             .kamu_auth_oso
@@ -104,7 +102,7 @@ impl DatasetActionAuthorizer for OsoDatasetAuthorizer {
                 AccessError::Forbidden(
                     DatasetActionNotEnoughPermissionsError {
                         action,
-                        dataset_ref: dataset_handle.as_local_ref(),
+                        dataset_ref: dataset_id.as_local_ref(),
                     }
                     .into(),
                 ),
@@ -119,7 +117,7 @@ impl DatasetActionAuthorizer for OsoDatasetAuthorizer {
         dataset_handle: &odf::DatasetHandle,
     ) -> Result<HashSet<DatasetAction>, InternalError> {
         let (user_actor, dataset_resource) =
-            try_join!(self.user_actor(), self.dataset_resource(dataset_handle))?;
+            try_join!(self.user_actor(), self.dataset_resource(&dataset_handle.id))?;
 
         self.kamu_auth_oso
             .get_allowed_actions(user_actor, dataset_resource)

diff --git a/src/adapter/auth-oso-rebac/tests/tests/test_oso_dataset_authorizer.rs b/src/adapter/auth-oso-rebac/tests/tests/test_oso_dataset_authorizer.rs
@@ -54,12 +54,12 @@ async fn test_owner_can_read_and_write_private_dataset() {
 
     let read_result = harness
         .dataset_authorizer
-        .check_action_allowed(&dataset_handle, DatasetAction::Read)
+        .check_action_allowed(&dataset_handle.id, DatasetAction::Read)
         .await;
 
     let write_result = harness
         .dataset_authorizer
-        .check_action_allowed(&dataset_handle, DatasetAction::Write)
+        .check_action_allowed(&dataset_handle.id, DatasetAction::Write)
         .await;
 
     let allowed_actions = harness
@@ -87,12 +87,12 @@ async fn test_guest_can_read_but_not_write_public_dataset() {
 
     let read_result = harness
         .dataset_authorizer
-        .check_action_allowed(&dataset_handle, DatasetAction::Read)
+        .check_action_allowed(&dataset_handle.id, DatasetAction::Read)
         .await;
 
     let write_result = harness
         .dataset_authorizer
-        .check_action_allowed(&dataset_handle, DatasetAction::Write)
+        .check_action_allowed(&dataset_handle.id, DatasetAction::Write)
         .await;
 
     let allowed_actions = harness

diff --git a/src/adapter/graphql/src/queries/datasets/dataset_metadata.rs b/src/adapter/graphql/src/queries/datasets/dataset_metadata.rs
@@ -179,7 +179,7 @@ impl DatasetMetadata {
         Ok(upstream_dependencies)
     }
 
-    // TODO: Convert to collection
+    // TODO: Convert to connection (page_based_connection!)
     // TODO: Private Datasets: tests
     /// Current downstream dependencies of a dataset
     async fn current_downstream_dependencies(

diff --git a/src/adapter/graphql/src/utils.rs b/src/adapter/graphql/src/utils.rs
@@ -11,7 +11,7 @@ use async_graphql::{Context, ErrorExtensions};
 use internal_error::*;
 use kamu_accounts::{CurrentAccountSubject, GetAccessTokenError, LoggedAccount};
 use kamu_core::auth::DatasetActionUnauthorizedError;
-use kamu_core::{DatasetRegistry, ResolvedDataset};
+use kamu_core::{auth, DatasetRegistry, ResolvedDataset};
 use kamu_datasets::DatasetEnvVarsConfig;
 use {kamu_task_system as ts, opendatafabric as odf};
 
@@ -109,32 +109,27 @@ pub(crate) async fn check_dataset_read_access(
     ctx: &Context<'_>,
     dataset_handle: &odf::DatasetHandle,
 ) -> Result<(), GqlError> {
-    let dataset_action_authorizer =
-        from_catalog_n!(ctx, dyn kamu_core::auth::DatasetActionAuthorizer);
-
-    dataset_action_authorizer
-        .check_action_allowed(dataset_handle, kamu_core::auth::DatasetAction::Read)
-        .await
-        .map_err(|e| match e {
-            DatasetActionUnauthorizedError::Access(_) => GqlError::Gql(
-                async_graphql::Error::new("Dataset access error")
-                    .extend_with(|_, eev| eev.set("alias", dataset_handle.alias.to_string())),
-            ),
-            DatasetActionUnauthorizedError::Internal(e) => GqlError::Internal(e),
-        })?;
-
-    Ok(())
+    check_dataset_access(ctx, dataset_handle, auth::DatasetAction::Read).await
 }
 
 pub(crate) async fn check_dataset_write_access(
     ctx: &Context<'_>,
     dataset_handle: &odf::DatasetHandle,
 ) -> Result<(), GqlError> {
-    let dataset_action_authorizer =
-        from_catalog_n!(ctx, dyn kamu_core::auth::DatasetActionAuthorizer);
+    check_dataset_access(ctx, dataset_handle, auth::DatasetAction::Write).await
+}
+
+////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+
+async fn check_dataset_access(
+    ctx: &Context<'_>,
+    dataset_handle: &odf::DatasetHandle,
+    action: auth::DatasetAction,
+) -> Result<(), GqlError> {
+    let dataset_action_authorizer = from_catalog_n!(ctx, dyn auth::DatasetActionAuthorizer);
 
     dataset_action_authorizer
-        .check_action_allowed(dataset_handle, kamu_core::auth::DatasetAction::Write)
+        .check_action_allowed(&dataset_handle.id, action)
         .await
         .map_err(|e| match e {
             DatasetActionUnauthorizedError::Access(_) => make_dataset_access_error(dataset_handle),

diff --git a/src/adapter/graphql/tests/tests/test_gql_account_flow_triggers.rs b/src/adapter/graphql/tests/tests/test_gql_account_flow_triggers.rs
@@ -641,6 +641,7 @@ impl FlowTriggerHarness {
                     .with_consumer_filter(messaging_outbox::ConsumerFilter::AllConsumers),
             )
             .bind::<dyn Outbox, OutboxImmediateImpl>()
+            .add::<DidGeneratorDefault>()
             .add_value(TenancyConfig::MultiTenant)
             .add_builder(DatasetRepositoryLocalFs::builder().with_root(datasets_dir))
             .bind::<dyn DatasetRepository, DatasetRepositoryLocalFs>()

diff --git a/src/adapter/graphql/tests/tests/test_gql_data.rs b/src/adapter/graphql/tests/tests/test_gql_data.rs
@@ -59,6 +59,7 @@ async fn create_catalog_with_local_workspace(
         let mut b = dill::CatalogBuilder::new();
 
         b.add::<DependencyGraphServiceImpl>()
+            .add::<DidGeneratorDefault>()
             .add::<InMemoryDatasetDependencyRepository>()
             .add_value(current_account_subject)
             .add_value(predefined_accounts_config)

diff --git a/src/adapter/graphql/tests/tests/test_gql_dataset_env_vars.rs b/src/adapter/graphql/tests/tests/test_gql_dataset_env_vars.rs
@@ -23,6 +23,7 @@ use kamu_core::{
     CreateDatasetFromSnapshotUseCase,
     CreateDatasetResult,
     DatasetRepository,
+    DidGeneratorDefault,
     TenancyConfig,
 };
 use kamu_datasets::DatasetEnvVarsConfig;
@@ -336,6 +337,7 @@ impl DatasetEnvVarsHarness {
             let mut b = dill::CatalogBuilder::new();
 
             b.add::<DummyOutboxImpl>()
+                .add::<DidGeneratorDefault>()
                 .add_value(DatasetEnvVarsConfig::sample())
                 .add_value(TenancyConfig::SingleTenant)
                 .add_builder(DatasetRepositoryLocalFs::builder().with_root(datasets_dir))

diff --git a/src/adapter/graphql/tests/tests/test_gql_dataset_flow_configs.rs b/src/adapter/graphql/tests/tests/test_gql_dataset_flow_configs.rs
@@ -24,6 +24,7 @@ use kamu_core::{
     CreateDatasetFromSnapshotUseCase,
     CreateDatasetResult,
     DatasetRepository,
+    DidGeneratorDefault,
     TenancyConfig,
 };
 use kamu_datasets_inmem::InMemoryDatasetDependencyRepository;
@@ -581,6 +582,7 @@ impl FlowConfigHarness {
             let mut b = dill::CatalogBuilder::new();
 
             b.add::<DummyOutboxImpl>()
+                .add::<DidGeneratorDefault>()
                 .add_value(TenancyConfig::SingleTenant)
                 .add_builder(DatasetRepositoryLocalFs::builder().with_root(datasets_dir))
                 .bind::<dyn DatasetRepository, DatasetRepositoryLocalFs>()

diff --git a/src/adapter/graphql/tests/tests/test_gql_dataset_flow_runs.rs b/src/adapter/graphql/tests/tests/test_gql_dataset_flow_runs.rs
@@ -40,6 +40,7 @@ use kamu_core::{
     DatasetIntervalIncrement,
     DatasetLifecycleMessage,
     DatasetRepository,
+    DidGeneratorDefault,
     PullResult,
     ResetResult,
     TenancyConfig,
@@ -3113,6 +3114,7 @@ impl FlowRunsHarness {
                     .with_consumer_filter(messaging_outbox::ConsumerFilter::AllConsumers),
             )
             .bind::<dyn Outbox, OutboxImmediateImpl>()
+            .add::<DidGeneratorDefault>()
             .add_value(TenancyConfig::SingleTenant)
             .add_builder(DatasetRepositoryLocalFs::builder().with_root(datasets_dir))
             .bind::<dyn DatasetRepository, DatasetRepositoryLocalFs>()

diff --git a/src/adapter/graphql/tests/tests/test_gql_dataset_flow_triggers.rs b/src/adapter/graphql/tests/tests/test_gql_dataset_flow_triggers.rs
@@ -24,6 +24,7 @@ use kamu_core::{
     CreateDatasetFromSnapshotUseCase,
     CreateDatasetResult,
     DatasetRepository,
+    DidGeneratorDefault,
     TenancyConfig,
 };
 use kamu_datasets_inmem::InMemoryDatasetDependencyRepository;
@@ -1063,6 +1064,7 @@ impl FlowTriggerHarness {
             let mut b = dill::CatalogBuilder::new();
 
             b.add::<DummyOutboxImpl>()
+                .add::<DidGeneratorDefault>()
                 .add_value(TenancyConfig::SingleTenant)
                 .add_builder(DatasetRepositoryLocalFs::builder().with_root(datasets_dir))
                 .bind::<dyn DatasetRepository, DatasetRepositoryLocalFs>()

diff --git a/src/adapter/graphql/tests/tests/test_gql_datasets.rs b/src/adapter/graphql/tests/tests/test_gql_datasets.rs
@@ -805,6 +805,7 @@ impl GraphQLDatasetsHarness {
             let mut b = dill::CatalogBuilder::new();
 
             b.add::<SystemTimeSourceDefault>()
+                .add::<DidGeneratorDefault>()
                 .add_builder(
                     OutboxImmediateImpl::builder()
                         .with_consumer_filter(messaging_outbox::ConsumerFilter::AllConsumers),

diff --git a/src/adapter/graphql/tests/tests/test_gql_metadata.rs b/src/adapter/graphql/tests/tests/test_gql_metadata.rs
@@ -34,6 +34,7 @@ async fn test_current_push_sources() {
         let mut b = CatalogBuilder::new();
 
         b.add_value(RunInfoDir::new(tempdir.path().join("run")))
+            .add::<DidGeneratorDefault>()
             .add::<DummyOutboxImpl>()
             .add_value(TenancyConfig::SingleTenant)
             .add_builder(DatasetRepositoryLocalFs::builder().with_root(datasets_dir))

diff --git a/src/adapter/graphql/tests/tests/test_gql_metadata_chain.rs b/src/adapter/graphql/tests/tests/test_gql_metadata_chain.rs
@@ -529,6 +529,7 @@ impl GraphQLMetadataChainHarness {
             let mut b = dill::CatalogBuilder::new();
 
             b.add::<SystemTimeSourceDefault>()
+                .add::<DidGeneratorDefault>()
                 .add::<DummyOutboxImpl>()
                 .add::<CreateDatasetUseCaseImpl>()
                 .add::<CreateDatasetFromSnapshotUseCaseImpl>()

diff --git a/src/adapter/graphql/tests/tests/test_gql_remote_statuses.rs b/src/adapter/graphql/tests/tests/test_gql_remote_statuses.rs
@@ -143,6 +143,7 @@ impl PushStatusesTestHarness {
             let mut b = CatalogBuilder::new();
 
             b.add_value(RunInfoDir::new(tempdir.path().join("run")))
+                .add::<DidGeneratorDefault>()
                 .add::<DummyOutboxImpl>()
                 .add_builder(DatasetRepositoryLocalFs::builder().with_root(datasets_dir))
                 .bind::<dyn DatasetRepository, DatasetRepositoryLocalFs>()

diff --git a/src/adapter/graphql/tests/tests/test_gql_search.rs b/src/adapter/graphql/tests/tests/test_gql_search.rs
@@ -28,6 +28,7 @@ async fn test_search_query() {
     std::fs::create_dir(&datasets_dir).unwrap();
 
     let cat = dill::CatalogBuilder::new()
+        .add::<DidGeneratorDefault>()
         .add::<SystemTimeSourceDefault>()
         .add::<DummyOutboxImpl>()
         .add::<DependencyGraphServiceImpl>()

diff --git a/src/adapter/http/Cargo.toml b/src/adapter/http/Cargo.toml
@@ -104,6 +104,7 @@ http-body-util = { optional = true, version = "0.1" }
 container-runtime = { workspace = true }
 init-on-startup = { workspace = true }
 kamu = { workspace = true, features = ["testing"] }
+kamu-core = { workspace = true, features = ["testing"] }
 kamu-accounts = { workspace = true, features = ["testing"] }
 kamu-accounts-inmem = { workspace = true }
 kamu-accounts-services = { workspace = true }

diff --git a/src/adapter/http/src/data/ingest_handler.rs b/src/adapter/http/src/data/ingest_handler.rs
@@ -131,7 +131,7 @@ pub async fn dataset_ingest_handler(
     // Authorization check
     let authorizer = catalog.get_one::<dyn DatasetActionAuthorizer>().unwrap();
     authorizer
-        .check_action_allowed(target.get_handle(), auth::DatasetAction::Write)
+        .check_action_allowed(&target.get_handle().id, auth::DatasetAction::Write)
         .await
         .map_err(|e| match e {
             DatasetActionUnauthorizedError::Access(_) => ApiError::new_forbidden(),

diff --git a/src/adapter/http/src/middleware/dataset_authorization_layer.rs b/src/adapter/http/src/middleware/dataset_authorization_layer.rs
@@ -131,7 +131,7 @@ where
                         {
                             Ok(dataset_handle) => {
                                 if let Err(err) = dataset_action_authorizer
-                                    .check_action_allowed(&dataset_handle, action)
+                                    .check_action_allowed(&dataset_handle.id, action)
                                     .await
                                 {
                                     if let Err(err_result) = Self::check_logged_in(catalog) {

diff --git a/src/adapter/http/tests/harness/client_side_harness.rs b/src/adapter/http/tests/harness/client_side_harness.rs
@@ -75,6 +75,8 @@ impl ClientSideHarness {
 
         let mut b = dill::CatalogBuilder::new();
 
+        b.add::<DidGeneratorDefault>();
+
         b.add_value(RunInfoDir::new(run_info_dir));
         b.add_value(CacheDir::new(cache_dir));
         b.add_value(RemoteReposDir::new(repos_dir));

diff --git a/src/adapter/http/tests/harness/server_side_harness.rs b/src/adapter/http/tests/harness/server_side_harness.rs
@@ -130,10 +130,10 @@ pub(crate) fn create_web_user_catalog(
         let mut mock_dataset_action_authorizer = MockDatasetActionAuthorizer::new();
         mock_dataset_action_authorizer
             .expect_check_action_allowed()
-            .returning(|dataset_handle, action| {
+            .returning(|dataset_id, action| {
                 if action == DatasetAction::Write {
                     Err(MockDatasetActionAuthorizer::denying_error(
-                        dataset_handle,
+                        dataset_id.as_local_ref(),
                         action,
                     ))
                 } else {

diff --git a/src/adapter/http/tests/harness/server_side_local_fs_harness.rs b/src/adapter/http/tests/harness/server_side_local_fs_harness.rs
@@ -40,7 +40,13 @@ use kamu::{
 };
 use kamu_accounts::testing::MockAuthenticationService;
 use kamu_accounts::{Account, AuthenticationService};
-use kamu_core::{CompactionExecutor, CompactionPlanner, DatasetRegistry, TenancyConfig};
+use kamu_core::{
+    CompactionExecutor,
+    CompactionPlanner,
+    DatasetRegistry,
+    DidGeneratorDefault,
+    TenancyConfig,
+};
 use kamu_datasets_inmem::InMemoryDatasetDependencyRepository;
 use kamu_datasets_services::DependencyGraphServiceImpl;
 use messaging_outbox::DummyOutboxImpl;
@@ -99,6 +105,7 @@ impl ServerSideLocalFsHarness {
             let base_url_rest = format!("http://{}", listener.local_addr().unwrap());
 
             b.add_value(RunInfoDir::new(run_info_dir))
+                .add::<DidGeneratorDefault>()
                 .add_value(CacheDir::new(cache_dir))
                 .add::<DummyOutboxImpl>()
                 .add_value(time_source.clone())

diff --git a/src/adapter/http/tests/harness/server_side_s3_harness.rs b/src/adapter/http/tests/harness/server_side_s3_harness.rs
@@ -44,7 +44,7 @@ use kamu::{
 };
 use kamu_accounts::testing::MockAuthenticationService;
 use kamu_accounts::{Account, AuthenticationService};
-use kamu_core::{DatasetRegistry, TenancyConfig};
+use kamu_core::{DatasetRegistry, DidGeneratorDefault, TenancyConfig};
 use kamu_datasets_inmem::InMemoryDatasetDependencyRepository;
 use kamu_datasets_services::DependencyGraphServiceImpl;
 use messaging_outbox::DummyOutboxImpl;
@@ -97,6 +97,7 @@ impl ServerSideS3Harness {
             let mut b = dill::CatalogBuilder::new();
 
             b.add_value(time_source.clone())
+                .add::<DidGeneratorDefault>()
                 .add_value(RunInfoDir::new(run_info_dir))
                 .bind::<dyn SystemTimeSource, SystemTimeSourceStub>()
                 .add::<DummyOutboxImpl>()