From 474d4ec498860b489cf226b17d4b63d1b0df99e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Renault?= Date: Thu, 1 Dec 2022 15:09:43 +0100 Subject: [PATCH 01/13] Add tests for the index patterns --- meilisearch/tests/auth/authorization.rs | 347 ++++++++++++++++++++++-- 1 file changed, 330 insertions(+), 17 deletions(-) diff --git a/meilisearch/tests/auth/authorization.rs b/meilisearch/tests/auth/authorization.rs index 8ef2d108d..6013274ef 100644 --- a/meilisearch/tests/auth/authorization.rs +++ b/meilisearch/tests/auth/authorization.rs @@ -77,12 +77,14 @@ static INVALID_RESPONSE: Lazy = Lazy::new(|| { }) }); +const MASTER_KEY: &str = "MASTER_KEY"; + #[actix_rt::test] async fn error_access_expired_key() { use std::{thread, time}; let mut server = Server::new_auth().await; - server.use_api_key("MASTER_KEY"); + server.use_api_key(MASTER_KEY); let content = json!({ "indexes": ["products"], @@ -111,7 +113,7 @@ async fn error_access_expired_key() { #[actix_rt::test] async fn error_access_unauthorized_index() { let mut server = Server::new_auth().await; - server.use_api_key("MASTER_KEY"); + server.use_api_key(MASTER_KEY); let content = json!({ "indexes": ["sales"], @@ -144,7 +146,7 @@ async fn error_access_unauthorized_action() { for ((method, route), action) in AUTHORIZATIONS.iter() { // create a new API key letting only the needed action. - server.use_api_key("MASTER_KEY"); + server.use_api_key(MASTER_KEY); let content = json!({ "indexes": ["products"], @@ -168,7 +170,7 @@ async fn error_access_unauthorized_action() { #[actix_rt::test] async fn access_authorized_master_key() { let mut server = Server::new_auth().await; - server.use_api_key("MASTER_KEY"); + server.use_api_key(MASTER_KEY); // master key must have access to all routes. for ((method, route), _) in AUTHORIZATIONS.iter() { @@ -185,7 +187,7 @@ async fn access_authorized_restricted_index() { for ((method, route), actions) in AUTHORIZATIONS.iter() { for action in actions { // create a new API key letting only the needed action. - server.use_api_key("MASTER_KEY"); + server.use_api_key(MASTER_KEY); let content = json!({ "indexes": ["products"], @@ -222,7 +224,7 @@ async fn access_authorized_no_index_restriction() { for ((method, route), actions) in AUTHORIZATIONS.iter() { for action in actions { // create a new API key letting only the needed action. - server.use_api_key("MASTER_KEY"); + server.use_api_key(MASTER_KEY); let content = json!({ "indexes": ["*"], @@ -255,7 +257,7 @@ async fn access_authorized_no_index_restriction() { #[actix_rt::test] async fn access_authorized_stats_restricted_index() { let mut server = Server::new_auth().await; - server.use_admin_key("MASTER_KEY").await; + server.use_admin_key(MASTER_KEY).await; // create index `test` let index = server.index("test"); @@ -295,7 +297,7 @@ async fn access_authorized_stats_restricted_index() { #[actix_rt::test] async fn access_authorized_stats_no_index_restriction() { let mut server = Server::new_auth().await; - server.use_admin_key("MASTER_KEY").await; + server.use_admin_key(MASTER_KEY).await; // create index `test` let index = server.index("test"); @@ -335,7 +337,7 @@ async fn access_authorized_stats_no_index_restriction() { #[actix_rt::test] async fn list_authorized_indexes_restricted_index() { let mut server = Server::new_auth().await; - server.use_admin_key("MASTER_KEY").await; + server.use_admin_key(MASTER_KEY).await; // create index `test` let index = server.index("test"); @@ -376,7 +378,7 @@ async fn list_authorized_indexes_restricted_index() { #[actix_rt::test] async fn list_authorized_indexes_no_index_restriction() { let mut server = Server::new_auth().await; - server.use_admin_key("MASTER_KEY").await; + server.use_admin_key(MASTER_KEY).await; // create index `test` let index = server.index("test"); @@ -414,10 +416,194 @@ async fn list_authorized_indexes_no_index_restriction() { assert!(response.iter().any(|index| index["uid"] == "test")); } +#[actix_rt::test] +async fn access_authorized_index_patterns() { + let mut server = Server::new_auth().await; + server.use_admin_key(MASTER_KEY).await; + + // create products_1 index + let index_1 = server.index("products_1"); + let (response, code) = index_1.create(Some("id")).await; + assert_eq!(202, code, "{:?}", &response); + + // create products index + let index_ = server.index("products"); + let (response, code) = index_.create(Some("id")).await; + assert_eq!(202, code, "{:?}", &response); + + // create key with all document access on indices with product_* pattern. + let content = json!({ + "indexes": ["products_*"], + "actions": ["documents.*"], + "expiresAt": (OffsetDateTime::now_utc() + Duration::hours(1)).format(&Rfc3339).unwrap(), + }); + + // Register the key + let (response, code) = server.add_api_key(content).await; + assert_eq!(201, code, "{:?}", &response); + assert!(response["key"].is_string()); + + // use created key. + let key = response["key"].as_str().unwrap(); + server.use_api_key(&key); + + // refer to products_1 and products with modified api key. + let index_1 = server.index("products_1"); + + let index_ = server.index("products"); + + // try to create a index via add documents route + let documents = json!([ + { + "id": 1, + "content": "foo", + } + ]); + + // Adding document to products_1 index. Should succeed with 202 + let (response, code) = index_1.add_documents(documents.clone(), None).await; + assert_eq!(202, code, "{:?}", &response); + let task_id = response["taskUid"].as_u64().unwrap(); + + // Adding document to products index. Should Fail with 403 -- invalid_api_key + let (response, code) = index_.add_documents(documents, None).await; + assert_eq!(403, code, "{:?}", &response); + + server.use_api_key(MASTER_KEY); + + // refer to products_1 with modified api key. + let index_1 = server.index("products_1"); + + index_1.wait_task(task_id).await; + + let (response, code) = index_1.get_task(task_id).await; + assert_eq!(200, code, "{:?}", &response); + assert_eq!(response["status"], "succeeded"); +} + +#[actix_rt::test] +async fn raise_error_non_authorized_index_patterns() { + let mut server = Server::new_auth().await; + server.use_admin_key(MASTER_KEY).await; + + // create products_1 index + let product_1_index = server.index("products_1"); + let (response, code) = product_1_index.create(Some("id")).await; + assert_eq!(202, code, "{:?}", &response); + + // create products_2 index + let product_2_index = server.index("products_2"); + let (response, code) = product_2_index.create(Some("id")).await; + assert_eq!(202, code, "{:?}", &response); + + // create test index + let test_index = server.index("test"); + let (response, code) = test_index.create(Some("id")).await; + assert_eq!(202, code, "{:?}", &response); + + // create key with all document access on indices with product_* pattern. + let content = json!({ + "indexes": ["products_*"], + "actions": ["documents.*"], + "expiresAt": (OffsetDateTime::now_utc() + Duration::hours(1)).format(&Rfc3339).unwrap(), + }); + + // Register the key + let (response, code) = server.add_api_key(content).await; + assert_eq!(201, code, "{:?}", &response); + assert!(response["key"].is_string()); + + // use created key. + let key = response["key"].as_str().unwrap(); + server.use_api_key(&key); + + // refer to products_1 and products_2 with modified api key. + let product_1_index = server.index("products_1"); + let product_2_index = server.index("products_2"); + + // refer to test index + let test_index = server.index("test"); + + // try to create a index via add documents route + let documents = json!([ + { + "id": 1, + "content": "foo", + } + ]); + + // Adding document to products_1 index. Should succeed with 202 + let (response, code) = product_1_index.add_documents(documents.clone(), None).await; + assert_eq!(202, code, "{:?}", &response); + let task1_id = response["taskUid"].as_u64().unwrap(); + + // Adding document to products_2 index. Should succeed with 202 + let (response, code) = product_2_index.add_documents(documents.clone(), None).await; + assert_eq!(202, code, "{:?}", &response); + let task2_id = response["taskUid"].as_u64().unwrap(); + + // Adding document to test index. Should Fail with 403 -- invalid_api_key + let (response, code) = test_index.add_documents(documents, None).await; + assert_eq!(403, code, "{:?}", &response); + + server.use_api_key(MASTER_KEY); + + // refer to products_1 with modified api key. + let product_1_index = server.index("products_1"); + // refer to products_2 with modified api key. + let product_2_index = server.index("products_2"); + + product_1_index.wait_task(task1_id).await; + product_2_index.wait_task(task2_id).await; + + let (response, code) = product_1_index.get_task(task1_id).await; + assert_eq!(200, code, "{:?}", &response); + assert_eq!(response["status"], "succeeded"); + + let (response, code) = product_1_index.get_task(task2_id).await; + assert_eq!(200, code, "{:?}", &response); + assert_eq!(response["status"], "succeeded"); +} + +#[actix_rt::test] +async fn pattern_indexes() { + // Create server with master key + let mut server = Server::new_auth().await; + server.use_admin_key(MASTER_KEY).await; + + // index.* constraints on products_* index pattern + let content = json!({ + "indexes": ["products_*"], + "actions": ["indexes.*"], + "expiresAt": (OffsetDateTime::now_utc() + Duration::hours(1)).format(&Rfc3339).unwrap(), + }); + + // Generate and use the api key + let (response, code) = server.add_api_key(content).await; + assert_eq!(201, code, "{:?}", &response); + let key = response["key"].as_str().expect("Key is not string"); + server.use_api_key(&key); + + // Create Index products_1 using generated api key + let products_1 = server.index("products_1"); + let (response, code) = products_1.create(Some("id")).await; + assert_eq!(202, code, "{:?}", &response); + + // Fail to create products_* using generated api key + let products_1 = server.index("products_*"); + let (response, code) = products_1.create(Some("id")).await; + assert_eq!(400, code, "{:?}", &response); + + // Fail to create test_1 using generated api key + let products_1 = server.index("test_1"); + let (response, code) = products_1.create(Some("id")).await; + assert_eq!(403, code, "{:?}", &response); +} + #[actix_rt::test] async fn list_authorized_tasks_restricted_index() { let mut server = Server::new_auth().await; - server.use_admin_key("MASTER_KEY").await; + server.use_admin_key(MASTER_KEY).await; // create index `test` let index = server.index("test"); @@ -446,7 +632,6 @@ async fn list_authorized_tasks_restricted_index() { let (response, code) = server.service.get("/tasks").await; assert_eq!(200, code, "{:?}", &response); - println!("{}", response); let response = response["results"].as_array().unwrap(); // key should have access on `products` index. assert!(response.iter().any(|task| task["indexUid"] == "products")); @@ -458,7 +643,7 @@ async fn list_authorized_tasks_restricted_index() { #[actix_rt::test] async fn list_authorized_tasks_no_index_restriction() { let mut server = Server::new_auth().await; - server.use_admin_key("MASTER_KEY").await; + server.use_admin_key(MASTER_KEY).await; // create index `test` let index = server.index("test"); @@ -499,7 +684,7 @@ async fn list_authorized_tasks_no_index_restriction() { #[actix_rt::test] async fn error_creating_index_without_action() { let mut server = Server::new_auth().await; - server.use_api_key("MASTER_KEY"); + server.use_api_key(MASTER_KEY); // create key with access on all indexes. let content = json!({ @@ -587,7 +772,7 @@ async fn lazy_create_index() { ]; for content in contents { - server.use_api_key("MASTER_KEY"); + server.use_api_key(MASTER_KEY); let (response, code) = server.add_api_key(content).await; assert_eq!(201, code, "{:?}", &response); assert!(response["key"].is_string()); @@ -643,14 +828,114 @@ async fn lazy_create_index() { } } +#[actix_rt::test] +async fn lazy_create_index_from_pattern() { + let mut server = Server::new_auth().await; + + // create key with access on all indexes. + let contents = vec![ + json!({ + "indexes": ["products_*"], + "actions": ["*"], + "expiresAt": "2050-11-13T00:00:00Z" + }), + json!({ + "indexes": ["products_*"], + "actions": ["indexes.*", "documents.*", "settings.*", "tasks.*"], + "expiresAt": "2050-11-13T00:00:00Z" + }), + json!({ + "indexes": ["products_*"], + "actions": ["indexes.create", "documents.add", "settings.update", "tasks.get"], + "expiresAt": "2050-11-13T00:00:00Z" + }), + ]; + + for content in contents { + server.use_api_key(MASTER_KEY); + let (response, code) = server.add_api_key(content).await; + assert_eq!(201, code, "{:?}", &response); + assert!(response["key"].is_string()); + + // use created key. + let key = response["key"].as_str().unwrap(); + server.use_api_key(&key); + + // try to create a index via add documents route + let index = server.index("products_1"); + let test = server.index("test"); + let documents = json!([ + { + "id": 1, + "content": "foo", + } + ]); + + let (response, code) = index.add_documents(documents.clone(), None).await; + assert_eq!(202, code, "{:?}", &response); + let task_id = response["taskUid"].as_u64().unwrap(); + + index.wait_task(task_id).await; + + let (response, code) = index.get_task(task_id).await; + assert_eq!(200, code, "{:?}", &response); + assert_eq!(response["status"], "succeeded"); + + // Fail to create test index + let (response, code) = test.add_documents(documents, None).await; + assert_eq!(403, code, "{:?}", &response); + + // try to create a index via add settings route + let index = server.index("products_2"); + let settings = json!({ "distinctAttribute": "test"}); + + let (response, code) = index.update_settings(settings).await; + assert_eq!(202, code, "{:?}", &response); + let task_id = response["taskUid"].as_u64().unwrap(); + + index.wait_task(task_id).await; + + let (response, code) = index.get_task(task_id).await; + assert_eq!(200, code, "{:?}", &response); + assert_eq!(response["status"], "succeeded"); + + // Fail to create test index + + let index = server.index("test"); + let settings = json!({ "distinctAttribute": "test"}); + + let (response, code) = index.update_settings(settings).await; + assert_eq!(403, code, "{:?}", &response); + + // try to create a index via add specialized settings route + let index = server.index("products_3"); + let (response, code) = index.update_distinct_attribute(json!("test")).await; + assert_eq!(202, code, "{:?}", &response); + let task_id = response["taskUid"].as_u64().unwrap(); + + index.wait_task(task_id).await; + + let (response, code) = index.get_task(task_id).await; + assert_eq!(200, code, "{:?}", &response); + assert_eq!(response["status"], "succeeded"); + + // Fail to create test index + let index = server.index("test"); + let settings = json!({ "distinctAttribute": "test"}); + + let (response, code) = index.update_settings(settings).await; + assert_eq!(403, code, "{:?}", &response); + } +} + #[actix_rt::test] async fn error_creating_index_without_index() { let mut server = Server::new_auth().await; - server.use_api_key("MASTER_KEY"); + server.use_api_key(MASTER_KEY); // create key with access on all indexes. let content = json!({ - "indexes": ["unexpected"], + "indexes": ["unexpected","products_*"], "actions": ["*"], "expiresAt": "2050-11-13T00:00:00Z" }); @@ -690,4 +975,32 @@ async fn error_creating_index_without_index() { let index = server.index("test3"); let (response, code) = index.create(None).await; assert_eq!(403, code, "{:?}", &response); + + // try to create a index via add documents route + let index = server.index("products"); + let documents = json!([ + { + "id": 1, + "content": "foo", + } + ]); + + let (response, code) = index.add_documents(documents, None).await; + assert_eq!(403, code, "{:?}", &response); + + // try to create a index via add settings route + let index = server.index("products"); + let settings = json!({ "distinctAttribute": "test"}); + let (response, code) = index.update_settings(settings).await; + assert_eq!(403, code, "{:?}", &response); + + // try to create a index via add specialized settings route + let index = server.index("products"); + let (response, code) = index.update_distinct_attribute(json!("test")).await; + assert_eq!(403, code, "{:?}", &response); + + // try to create a index via create index route + let index = server.index("products"); + let (response, code) = index.create(None).await; + assert_eq!(403, code, "{:?}", &response); } From 0b08413c98cee79798de9c2a59c677832a29820f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Renault?= Date: Thu, 1 Dec 2022 15:48:23 +0100 Subject: [PATCH 02/13] Introduce the IndexUidPattern type --- meilisearch-types/src/index_uid_pattern.rs | 97 ++++++++++++++++++++++ meilisearch-types/src/lib.rs | 1 + 2 files changed, 98 insertions(+) create mode 100644 meilisearch-types/src/index_uid_pattern.rs diff --git a/meilisearch-types/src/index_uid_pattern.rs b/meilisearch-types/src/index_uid_pattern.rs new file mode 100644 index 000000000..1f82e83db --- /dev/null +++ b/meilisearch-types/src/index_uid_pattern.rs @@ -0,0 +1,97 @@ +use std::error::Error; +use std::fmt; +use std::str::FromStr; + +use serde::{Deserialize, Serialize}; + +use crate::error::{Code, ErrorCode}; +use crate::index_uid::{IndexUid, IndexUidFormatError}; + +/// An index uid pattern is composed of only ascii alphanumeric characters, - and _, between 1 and 400 +/// bytes long and optionally ending with a *. +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)] +pub struct IndexUidPattern( + #[cfg_attr(feature = "test-traits", proptest(regex("[a-zA-Z0-9_-]{1,400}\\*?")))] String, +); + +impl IndexUidPattern { + /// Returns wether this index uid matches this index uid pattern. + pub fn matches(&self, uid: &IndexUid) -> bool { + self.matches_str(uid.as_str()) + } + + /// Returns wether this string matches this index uid pattern. + pub fn matches_str(&self, uid: &str) -> bool { + match self.0.strip_suffix('*') { + Some(prefix) => uid.starts_with(prefix), + None => self.0 == uid, + } + } +} + +impl std::ops::Deref for IndexUidPattern { + type Target = str; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl TryFrom for IndexUidPattern { + type Error = IndexUidPatternFormatError; + + fn try_from(uid: String) -> Result { + let result = match uid.strip_suffix('*') { + Some(prefix) => IndexUid::from_str(prefix).map(|_| IndexUidPattern(uid)), + None => IndexUid::try_from(uid).map(IndexUid::into_inner).map(IndexUidPattern), + }; + + match result { + Ok(index_uid_pattern) => Ok(index_uid_pattern), + Err(IndexUidFormatError { invalid_uid }) => { + Err(IndexUidPatternFormatError { invalid_uid }) + } + } + } +} + +impl FromStr for IndexUidPattern { + type Err = IndexUidPatternFormatError; + + fn from_str(uid: &str) -> Result { + uid.to_string().try_into() + } +} + +impl From for String { + fn from(IndexUidPattern(uid): IndexUidPattern) -> Self { + uid + } +} + +#[derive(Debug)] +pub struct IndexUidPatternFormatError { + pub invalid_uid: String, +} + +impl fmt::Display for IndexUidPatternFormatError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "`{}` is not a valid index uid pattern. Index uid patterns \ + can be an integer or a string containing only alphanumeric \ + characters, hyphens (-), underscores (_), and \ + optionally end with a star (*).", + self.invalid_uid, + ) + } +} + +impl Error for IndexUidPatternFormatError {} + +impl ErrorCode for IndexUidPatternFormatError { + fn error_code(&self) -> Code { + // TODO should I return a new error code? + Code::InvalidIndexUid + } +} diff --git a/meilisearch-types/src/lib.rs b/meilisearch-types/src/lib.rs index c7f7ca7f5..22caa8114 100644 --- a/meilisearch-types/src/lib.rs +++ b/meilisearch-types/src/lib.rs @@ -2,6 +2,7 @@ pub mod compression; pub mod document_formats; pub mod error; pub mod index_uid; +pub mod index_uid_pattern; pub mod keys; pub mod settings; pub mod star_or; From 29961b8c6b5cb3104aae407cb1949d9301b83dc6 Mon Sep 17 00:00:00 2001 From: Kerollmops Date: Wed, 25 Jan 2023 14:41:36 +0100 Subject: [PATCH 03/13] Make it work with the dumps --- dump/src/reader/compat/v5_to_v6.rs | 2 +- dump/src/reader/v6/mod.rs | 2 +- meilisearch-types/src/index_uid_pattern.rs | 5 +++++ 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/dump/src/reader/compat/v5_to_v6.rs b/dump/src/reader/compat/v5_to_v6.rs index 237381414..51858450e 100644 --- a/dump/src/reader/compat/v5_to_v6.rs +++ b/dump/src/reader/compat/v5_to_v6.rs @@ -183,7 +183,7 @@ impl CompatV5ToV6 { .map(|index| match index { v5::StarOr::Star => v6::StarOr::Star, v5::StarOr::Other(uid) => { - v6::StarOr::Other(v6::IndexUid::new_unchecked(uid.as_str())) + v6::StarOr::Other(v6::IndexUidPattern::new_unchecked(uid.as_str())) } }) .collect(), diff --git a/dump/src/reader/v6/mod.rs b/dump/src/reader/v6/mod.rs index edf552452..77d7a52bc 100644 --- a/dump/src/reader/v6/mod.rs +++ b/dump/src/reader/v6/mod.rs @@ -35,7 +35,7 @@ pub type PaginationSettings = meilisearch_types::settings::PaginationSettings; // everything related to the api keys pub type Action = meilisearch_types::keys::Action; pub type StarOr = meilisearch_types::star_or::StarOr; -pub type IndexUid = meilisearch_types::index_uid::IndexUid; +pub type IndexUidPattern = meilisearch_types::index_uid_pattern::IndexUidPattern; // everything related to the errors pub type ResponseError = meilisearch_types::error::ResponseError; diff --git a/meilisearch-types/src/index_uid_pattern.rs b/meilisearch-types/src/index_uid_pattern.rs index 1f82e83db..8cb50fee9 100644 --- a/meilisearch-types/src/index_uid_pattern.rs +++ b/meilisearch-types/src/index_uid_pattern.rs @@ -10,11 +10,16 @@ use crate::index_uid::{IndexUid, IndexUidFormatError}; /// An index uid pattern is composed of only ascii alphanumeric characters, - and _, between 1 and 400 /// bytes long and optionally ending with a *. #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)] +#[cfg_attr(feature = "test-traits", derive(proptest_derive::Arbitrary))] pub struct IndexUidPattern( #[cfg_attr(feature = "test-traits", proptest(regex("[a-zA-Z0-9_-]{1,400}\\*?")))] String, ); impl IndexUidPattern { + pub fn new_unchecked(s: impl AsRef) -> Self { + Self(s.as_ref().to_string()) + } + /// Returns wether this index uid matches this index uid pattern. pub fn matches(&self, uid: &IndexUid) -> bool { self.matches_str(uid.as_str()) From 184b8afd9e479651cc6063e7fab2d6bc3f3a0d9a Mon Sep 17 00:00:00 2001 From: Kerollmops Date: Wed, 25 Jan 2023 14:42:03 +0100 Subject: [PATCH 04/13] Make it work in the CreateApiKey struct --- meilisearch-types/src/keys.rs | 11 ++++++----- meilisearch/src/routes/api_key.rs | 1 + meilisearch/src/routes/mod.rs | 2 ++ 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/meilisearch-types/src/keys.rs b/meilisearch-types/src/keys.rs index b41bb06b6..50afa755c 100644 --- a/meilisearch-types/src/keys.rs +++ b/meilisearch-types/src/keys.rs @@ -12,15 +12,15 @@ use uuid::Uuid; use crate::error::deserr_codes::*; use crate::error::{unwrap_any, Code, DeserrError, ErrorCode, TakeErrorMessage}; -use crate::index_uid::{IndexUid, IndexUidFormatError}; +use crate::index_uid_pattern::{IndexUidPattern, IndexUidPatternFormatError}; use crate::star_or::StarOr; pub type KeyId = Uuid; -impl MergeWithError for DeserrError { +impl MergeWithError for DeserrError { fn merge( _self_: Option, - other: IndexUidFormatError, + other: IndexUidPatternFormatError, merge_location: deserr::ValuePointerRef, ) -> std::result::Result { DeserrError::error::( @@ -47,10 +47,11 @@ pub struct CreateApiKey { #[deserr(error = DeserrError)] pub actions: Vec, #[deserr(error = DeserrError)] - pub indexes: Vec>, + pub indexes: Vec>, #[deserr(error = DeserrError, default = None, from(&String) = parse_expiration_date -> TakeErrorMessage)] pub expires_at: Option, } + impl CreateApiKey { pub fn to_key(self) -> Key { let CreateApiKey { description, name, uid, actions, indexes, expires_at } = self; @@ -108,7 +109,7 @@ pub struct Key { pub name: Option, pub uid: KeyId, pub actions: Vec, - pub indexes: Vec>, + pub indexes: Vec>, #[serde(with = "time::serde::rfc3339::option")] pub expires_at: Option, #[serde(with = "time::serde::rfc3339")] diff --git a/meilisearch/src/routes/api_key.rs b/meilisearch/src/routes/api_key.rs index 76912bbaa..b43398da1 100644 --- a/meilisearch/src/routes/api_key.rs +++ b/meilisearch/src/routes/api_key.rs @@ -61,6 +61,7 @@ pub struct ListApiKeys { #[deserr(error = DeserrError, default = PAGINATION_DEFAULT_LIMIT(), from(&String) = parse_usize_take_error_message -> TakeErrorMessage)] pub limit: usize, } + impl ListApiKeys { fn as_pagination(self) -> Pagination { Pagination { offset: self.offset, limit: self.limit } diff --git a/meilisearch/src/routes/mod.rs b/meilisearch/src/routes/mod.rs index 2e619540a..5111478c9 100644 --- a/meilisearch/src/routes/mod.rs +++ b/meilisearch/src/routes/mod.rs @@ -56,6 +56,7 @@ where { Ok(Some(input.parse()?)) } + pub fn from_string_to_option_take_error_message( input: &str, ) -> Result, TakeErrorMessage> @@ -90,6 +91,7 @@ impl From for SummarizedTaskView { } } } + pub struct Pagination { pub offset: usize, pub limit: usize, From ec7de4bae7730ad14921bbb6e00273fe62f50f5b Mon Sep 17 00:00:00 2001 From: Kerollmops Date: Wed, 25 Jan 2023 16:12:40 +0100 Subject: [PATCH 05/13] Make it work for any all routes including stats and index swaps --- meilisearch-auth/src/lib.rs | 21 ++++++++++++++++----- meilisearch-auth/src/store.rs | 24 +++++++++++++++++++++--- 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/meilisearch-auth/src/lib.rs b/meilisearch-auth/src/lib.rs index 8d4a7f2b7..c81f9f20b 100644 --- a/meilisearch-auth/src/lib.rs +++ b/meilisearch-auth/src/lib.rs @@ -8,6 +8,7 @@ use std::path::Path; use std::sync::Arc; use error::{AuthControllerError, Result}; +use meilisearch_types::index_uid_pattern::IndexUidPattern; use meilisearch_types::keys::{Action, CreateApiKey, Key, PatchApiKey}; use meilisearch_types::star_or::StarOr; use serde::{Deserialize, Serialize}; @@ -141,9 +142,7 @@ impl AuthController { .get_expiration_date(uid, action, None)? .or(match index { // else check if the key has access to the requested index. - Some(index) => { - self.store.get_expiration_date(uid, action, Some(index.as_bytes()))? - } + Some(index) => self.store.get_expiration_date(uid, action, Some(index))?, // or to any index if no index has been requested. None => self.store.prefix_first_expiration_date(uid, action)?, }) { @@ -196,8 +195,20 @@ impl Default for SearchRules { impl SearchRules { pub fn is_index_authorized(&self, index: &str) -> bool { match self { - Self::Set(set) => set.contains("*") || set.contains(index), - Self::Map(map) => map.contains_key("*") || map.contains_key(index), + Self::Set(set) => { + set.contains("*") + || set.contains(index) + || set + .iter() // We must store the IndexUidPattern in the Set + .any(|pattern| IndexUidPattern::new_unchecked(pattern).matches_str(index)) + } + Self::Map(map) => { + map.contains_key("*") + || map.contains_key(index) + || map + .keys() // We must store the IndexUidPattern in the Map + .any(|pattern| IndexUidPattern::new_unchecked(pattern).matches_str(index)) + } } } diff --git a/meilisearch-auth/src/store.rs b/meilisearch-auth/src/store.rs index b3f9ed672..d1c2562c1 100644 --- a/meilisearch-auth/src/store.rs +++ b/meilisearch-auth/src/store.rs @@ -9,6 +9,7 @@ use std::str; use std::sync::Arc; use hmac::{Hmac, Mac}; +use meilisearch_types::index_uid_pattern::IndexUidPattern; use meilisearch_types::keys::KeyId; use meilisearch_types::milli; use meilisearch_types::milli::heed::types::{ByteSlice, DecodeIgnore, SerdeJson}; @@ -210,11 +211,28 @@ impl HeedAuthStore { &self, uid: Uuid, action: Action, - index: Option<&[u8]>, + index: Option<&str>, ) -> Result>> { let rtxn = self.env.read_txn()?; - let tuple = (&uid, &action, index); - Ok(self.action_keyid_index_expiration.get(&rtxn, &tuple)?) + let tuple = (&uid, &action, index.map(|s| s.as_bytes())); + match self.action_keyid_index_expiration.get(&rtxn, &tuple)? { + Some(expiration) => Ok(Some(expiration)), + None => { + let tuple = (&uid, &action, None); + for result in self.action_keyid_index_expiration.prefix_iter(&rtxn, &tuple)? { + let ((_, _, index_uid_pattern), expiration) = result?; + if let Some((pattern, index)) = index_uid_pattern.zip(index) { + let index_uid_pattern = str::from_utf8(pattern)?.to_string(); + // TODO I shouldn't unwrap here but rather return an internal error + let pattern = IndexUidPattern::try_from(index_uid_pattern).unwrap(); + if pattern.matches_str(index) { + return Ok(Some(expiration)); + } + } + } + Ok(None) + } + } } pub fn prefix_first_expiration_date( From d563ed8a39a81b4aebb0899be3963100bac66aba Mon Sep 17 00:00:00 2001 From: Kerollmops Date: Wed, 25 Jan 2023 17:22:32 +0100 Subject: [PATCH 06/13] Making it work with index uid patterns --- Cargo.lock | 1 + meilisearch-auth/Cargo.toml | 1 + meilisearch-auth/src/lib.rs | 57 +++++++------------ meilisearch-types/src/index_uid_pattern.rs | 29 ++++++++-- meilisearch-types/src/keys.rs | 8 +-- .../src/extractors/authentication/mod.rs | 5 +- 6 files changed, 53 insertions(+), 48 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dd7d828da..0fc88e802 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2535,6 +2535,7 @@ dependencies = [ "base64 0.13.1", "enum-iterator", "hmac", + "maplit", "meilisearch-types", "rand", "roaring", diff --git a/meilisearch-auth/Cargo.toml b/meilisearch-auth/Cargo.toml index 383be69cf..a42cbae02 100644 --- a/meilisearch-auth/Cargo.toml +++ b/meilisearch-auth/Cargo.toml @@ -7,6 +7,7 @@ edition = "2021" base64 = "0.13.1" enum-iterator = "1.1.3" hmac = "0.12.1" +maplit = "1.0.2" meilisearch-types = { path = "../meilisearch-types" } rand = "0.8.5" roaring = { version = "0.10.0", features = ["serde"] } diff --git a/meilisearch-auth/src/lib.rs b/meilisearch-auth/src/lib.rs index c81f9f20b..287dda0ce 100644 --- a/meilisearch-auth/src/lib.rs +++ b/meilisearch-auth/src/lib.rs @@ -8,6 +8,7 @@ use std::path::Path; use std::sync::Arc; use error::{AuthControllerError, Result}; +use maplit::hashset; use meilisearch_types::index_uid_pattern::IndexUidPattern; use meilisearch_types::keys::{Action, CreateApiKey, Key, PatchApiKey}; use meilisearch_types::star_or::StarOr; @@ -75,31 +76,12 @@ impl AuthController { search_rules: Option, ) -> Result { let mut filters = AuthFilter::default(); - let key = self - .store - .get_api_key(uid)? - .ok_or_else(|| AuthControllerError::ApiKeyNotFound(uid.to_string()))?; + let key = self.get_key(uid)?; - if !key.indexes.iter().any(|i| i == &StarOr::Star) { - filters.search_rules = match search_rules { - // Intersect search_rules with parent key authorized indexes. - Some(search_rules) => SearchRules::Map( - key.indexes - .into_iter() - .filter_map(|index| { - search_rules.get_index_search_rules(index.deref()).map( - |index_search_rules| { - (String::from(index), Some(index_search_rules)) - }, - ) - }) - .collect(), - ), - None => SearchRules::Set(key.indexes.into_iter().map(String::from).collect()), - }; - } else if let Some(search_rules) = search_rules { - filters.search_rules = search_rules; - } + filters.search_rules = match search_rules { + Some(search_rules) => search_rules, + None => SearchRules::Set(key.indexes.into_iter().collect()), + }; filters.allow_index_creation = self.is_key_authorized(uid, Action::IndexesAdd, None)?; @@ -182,13 +164,13 @@ impl Default for AuthFilter { #[derive(Debug, Serialize, Deserialize, Clone)] #[serde(untagged)] pub enum SearchRules { - Set(HashSet), - Map(HashMap>), + Set(HashSet), + Map(HashMap>), } impl Default for SearchRules { fn default() -> Self { - Self::Set(Some("*".to_string()).into_iter().collect()) + Self::Set(hashset! { IndexUidPattern::all() }) } } @@ -198,16 +180,12 @@ impl SearchRules { Self::Set(set) => { set.contains("*") || set.contains(index) - || set - .iter() // We must store the IndexUidPattern in the Set - .any(|pattern| IndexUidPattern::new_unchecked(pattern).matches_str(index)) + || set.iter().any(|pattern| pattern.matches_str(index)) } Self::Map(map) => { map.contains_key("*") || map.contains_key(index) - || map - .keys() // We must store the IndexUidPattern in the Map - .any(|pattern| IndexUidPattern::new_unchecked(pattern).matches_str(index)) + || map.keys().any(|pattern| pattern.matches_str(index)) } } } @@ -215,21 +193,26 @@ impl SearchRules { pub fn get_index_search_rules(&self, index: &str) -> Option { match self { Self::Set(set) => { - if set.contains("*") || set.contains(index) { + if self.is_index_authorized(index) { Some(IndexSearchRules::default()) } else { None } } Self::Map(map) => { - map.get(index).or_else(|| map.get("*")).map(|isr| isr.clone().unwrap_or_default()) + // We must take the most retrictive rule of this index uid patterns set of rules. + map.iter() + .filter(|(pattern, _)| pattern.matches_str(index)) + .max_by_key(|(pattern, _)| (pattern.is_exact(), pattern.len())) + .map(|(_, rule)| rule.clone()) + .flatten() } } } /// Return the list of indexes such that `self.is_index_authorized(index) == true`, /// or `None` if all indexes satisfy this condition. - pub fn authorized_indexes(&self) -> Option> { + pub fn authorized_indexes(&self) -> Option> { match self { SearchRules::Set(set) => { if set.contains("*") { @@ -250,7 +233,7 @@ impl SearchRules { } impl IntoIterator for SearchRules { - type Item = (String, IndexSearchRules); + type Item = (IndexUidPattern, IndexSearchRules); type IntoIter = Box>; fn into_iter(self) -> Self::IntoIter { diff --git a/meilisearch-types/src/index_uid_pattern.rs b/meilisearch-types/src/index_uid_pattern.rs index 8cb50fee9..bc12da351 100644 --- a/meilisearch-types/src/index_uid_pattern.rs +++ b/meilisearch-types/src/index_uid_pattern.rs @@ -1,7 +1,10 @@ +use std::borrow::Borrow; use std::error::Error; use std::fmt; +use std::ops::Deref; use std::str::FromStr; +use deserr::DeserializeFromValue; use serde::{Deserialize, Serialize}; use crate::error::{Code, ErrorCode}; @@ -9,17 +12,25 @@ use crate::index_uid::{IndexUid, IndexUidFormatError}; /// An index uid pattern is composed of only ascii alphanumeric characters, - and _, between 1 and 400 /// bytes long and optionally ending with a *. -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)] -#[cfg_attr(feature = "test-traits", derive(proptest_derive::Arbitrary))] -pub struct IndexUidPattern( - #[cfg_attr(feature = "test-traits", proptest(regex("[a-zA-Z0-9_-]{1,400}\\*?")))] String, -); +#[derive(Serialize, Deserialize, DeserializeFromValue, Debug, Clone, PartialEq, Eq, Hash)] +#[deserr(from(&String) = FromStr::from_str -> IndexUidPatternFormatError)] +pub struct IndexUidPattern(String); impl IndexUidPattern { pub fn new_unchecked(s: impl AsRef) -> Self { Self(s.as_ref().to_string()) } + /// Matches any index name. + pub fn all() -> Self { + IndexUidPattern::from_str("*").unwrap() + } + + /// Returns `true` if the pattern matches a specific index name. + pub fn is_exact(&self) -> bool { + !self.0.ends_with('*') + } + /// Returns wether this index uid matches this index uid pattern. pub fn matches(&self, uid: &IndexUid) -> bool { self.matches_str(uid.as_str()) @@ -34,7 +45,7 @@ impl IndexUidPattern { } } -impl std::ops::Deref for IndexUidPattern { +impl Deref for IndexUidPattern { type Target = str; fn deref(&self) -> &Self::Target { @@ -42,6 +53,12 @@ impl std::ops::Deref for IndexUidPattern { } } +impl Borrow for IndexUidPattern { + fn borrow(&self) -> &str { + &self.0 + } +} + impl TryFrom for IndexUidPattern { type Error = IndexUidPatternFormatError; diff --git a/meilisearch-types/src/keys.rs b/meilisearch-types/src/keys.rs index 50afa755c..a9e2e0889 100644 --- a/meilisearch-types/src/keys.rs +++ b/meilisearch-types/src/keys.rs @@ -47,7 +47,7 @@ pub struct CreateApiKey { #[deserr(error = DeserrError)] pub actions: Vec, #[deserr(error = DeserrError)] - pub indexes: Vec>, + pub indexes: Vec, #[deserr(error = DeserrError, default = None, from(&String) = parse_expiration_date -> TakeErrorMessage)] pub expires_at: Option, } @@ -109,7 +109,7 @@ pub struct Key { pub name: Option, pub uid: KeyId, pub actions: Vec, - pub indexes: Vec>, + pub indexes: Vec, #[serde(with = "time::serde::rfc3339::option")] pub expires_at: Option, #[serde(with = "time::serde::rfc3339")] @@ -127,7 +127,7 @@ impl Key { description: Some("Use it for anything that is not a search operation. Caution! Do not expose it on a public frontend".to_string()), uid, actions: vec![Action::All], - indexes: vec![StarOr::Star], + indexes: vec![IndexUidPattern::all()], expires_at: None, created_at: now, updated_at: now, @@ -142,7 +142,7 @@ impl Key { description: Some("Use it to search from the frontend".to_string()), uid, actions: vec![Action::Search], - indexes: vec![StarOr::Star], + indexes: vec![IndexUidPattern::all()], expires_at: None, created_at: now, updated_at: now, diff --git a/meilisearch/src/extractors/authentication/mod.rs b/meilisearch/src/extractors/authentication/mod.rs index 8944b60d3..f1efdf9aa 100644 --- a/meilisearch/src/extractors/authentication/mod.rs +++ b/meilisearch/src/extractors/authentication/mod.rs @@ -230,7 +230,10 @@ pub mod policies { } } - return auth.get_key_filters(uid, Some(data.claims.search_rules)).ok(); + match auth.get_key_filters(uid, Some(data.claims.search_rules)) { + Ok(auth) if auth.search_rules.is_index_authorized() => Some(auth), + _ => None, + } } None From a36b1dbd7054de44af48af813c2823648d4b190c Mon Sep 17 00:00:00 2001 From: Kerollmops Date: Wed, 1 Feb 2023 18:21:45 +0100 Subject: [PATCH 07/13] Fix the tasks with the new patterns --- dump/src/reader/compat/v5_to_v6.rs | 6 ++-- dump/src/reader/v6/mod.rs | 1 - index-scheduler/src/lib.rs | 30 ++++++++++++++----- meilisearch-auth/src/lib.rs | 4 +-- meilisearch-auth/src/store.rs | 3 +- meilisearch-types/src/index_uid_pattern.rs | 5 ++++ meilisearch-types/src/keys.rs | 1 - .../src/extractors/authentication/mod.rs | 17 ++++++----- 8 files changed, 41 insertions(+), 26 deletions(-) diff --git a/dump/src/reader/compat/v5_to_v6.rs b/dump/src/reader/compat/v5_to_v6.rs index 51858450e..3b348492d 100644 --- a/dump/src/reader/compat/v5_to_v6.rs +++ b/dump/src/reader/compat/v5_to_v6.rs @@ -181,10 +181,8 @@ impl CompatV5ToV6 { .indexes .into_iter() .map(|index| match index { - v5::StarOr::Star => v6::StarOr::Star, - v5::StarOr::Other(uid) => { - v6::StarOr::Other(v6::IndexUidPattern::new_unchecked(uid.as_str())) - } + v5::StarOr::Star => v6::IndexUidPattern::all(), + v5::StarOr::Other(uid) => v6::IndexUidPattern::new_unchecked(uid.as_str()), }) .collect(), expires_at: key.expires_at, diff --git a/dump/src/reader/v6/mod.rs b/dump/src/reader/v6/mod.rs index 77d7a52bc..f0ad81116 100644 --- a/dump/src/reader/v6/mod.rs +++ b/dump/src/reader/v6/mod.rs @@ -34,7 +34,6 @@ pub type PaginationSettings = meilisearch_types::settings::PaginationSettings; // everything related to the api keys pub type Action = meilisearch_types::keys::Action; -pub type StarOr = meilisearch_types::star_or::StarOr; pub type IndexUidPattern = meilisearch_types::index_uid_pattern::IndexUidPattern; // everything related to the errors diff --git a/index-scheduler/src/lib.rs b/index-scheduler/src/lib.rs index 0b9e856d2..3599ac36a 100644 --- a/index-scheduler/src/lib.rs +++ b/index-scheduler/src/lib.rs @@ -43,6 +43,7 @@ use file_store::FileStore; use meilisearch_types::error::ResponseError; use meilisearch_types::heed::types::{OwnedType, SerdeBincode, SerdeJson, Str}; use meilisearch_types::heed::{self, Database, Env, RoTxn}; +use meilisearch_types::index_uid_pattern::IndexUidPattern; use meilisearch_types::milli; use meilisearch_types::milli::documents::DocumentsBatchBuilder; use meilisearch_types::milli::update::IndexerConfig; @@ -617,7 +618,7 @@ impl IndexScheduler { &self, rtxn: &RoTxn, query: &Query, - authorized_indexes: &Option>, + authorized_indexes: &Option>, ) -> Result { let mut tasks = self.get_task_ids(rtxn, query)?; @@ -635,7 +636,7 @@ impl IndexScheduler { let all_indexes_iter = self.index_tasks.iter(rtxn)?; for result in all_indexes_iter { let (index, index_tasks) = result?; - if !authorized_indexes.contains(&index.to_owned()) { + if !authorized_indexes.iter().any(|p| p.matches_str(index)) { tasks -= index_tasks; } } @@ -655,7 +656,7 @@ impl IndexScheduler { pub fn get_tasks_from_authorized_indexes( &self, query: Query, - authorized_indexes: Option>, + authorized_indexes: Option>, ) -> Result> { let rtxn = self.env.read_txn()?; @@ -2503,7 +2504,11 @@ mod tests { let query = Query { index_uids: Some(vec!["catto".to_owned()]), ..Default::default() }; let tasks = index_scheduler - .get_task_ids_from_authorized_indexes(&rtxn, &query, &Some(vec!["doggo".to_owned()])) + .get_task_ids_from_authorized_indexes( + &rtxn, + &query, + &Some(vec![IndexUidPattern::new_unchecked("doggo")]), + ) .unwrap(); // we have asked for only the tasks associated with catto, but are only authorized to retrieve the tasks // associated with doggo -> empty result @@ -2511,7 +2516,11 @@ mod tests { let query = Query::default(); let tasks = index_scheduler - .get_task_ids_from_authorized_indexes(&rtxn, &query, &Some(vec!["doggo".to_owned()])) + .get_task_ids_from_authorized_indexes( + &rtxn, + &query, + &Some(vec![IndexUidPattern::new_unchecked("doggo")]), + ) .unwrap(); // we asked for all the tasks, but we are only authorized to retrieve the doggo tasks // -> only the index creation of doggo should be returned @@ -2522,7 +2531,10 @@ mod tests { .get_task_ids_from_authorized_indexes( &rtxn, &query, - &Some(vec!["catto".to_owned(), "doggo".to_owned()]), + &Some(vec![ + IndexUidPattern::new_unchecked("catto"), + IndexUidPattern::new_unchecked("doggo"), + ]), ) .unwrap(); // we asked for all the tasks, but we are only authorized to retrieve the doggo and catto tasks @@ -2570,7 +2582,11 @@ mod tests { let query = Query { canceled_by: Some(vec![task_cancelation.uid]), ..Query::default() }; let tasks = index_scheduler - .get_task_ids_from_authorized_indexes(&rtxn, &query, &Some(vec!["doggo".to_string()])) + .get_task_ids_from_authorized_indexes( + &rtxn, + &query, + &Some(vec![IndexUidPattern::new_unchecked("doggo")]), + ) .unwrap(); // Return only 1 because the user is not authorized to see task 2 snapshot!(snapshot_bitmap(&tasks), @"[1,]"); diff --git a/meilisearch-auth/src/lib.rs b/meilisearch-auth/src/lib.rs index 287dda0ce..6b8b6ef9e 100644 --- a/meilisearch-auth/src/lib.rs +++ b/meilisearch-auth/src/lib.rs @@ -3,7 +3,6 @@ pub mod error; mod store; use std::collections::{HashMap, HashSet}; -use std::ops::Deref; use std::path::Path; use std::sync::Arc; @@ -11,7 +10,6 @@ use error::{AuthControllerError, Result}; use maplit::hashset; use meilisearch_types::index_uid_pattern::IndexUidPattern; use meilisearch_types::keys::{Action, CreateApiKey, Key, PatchApiKey}; -use meilisearch_types::star_or::StarOr; use serde::{Deserialize, Serialize}; pub use store::open_auth_store_env; use store::{generate_key_as_hexa, HeedAuthStore}; @@ -192,7 +190,7 @@ impl SearchRules { pub fn get_index_search_rules(&self, index: &str) -> Option { match self { - Self::Set(set) => { + Self::Set(_) => { if self.is_index_authorized(index) { Some(IndexSearchRules::default()) } else { diff --git a/meilisearch-auth/src/store.rs b/meilisearch-auth/src/store.rs index d1c2562c1..79a1631a4 100644 --- a/meilisearch-auth/src/store.rs +++ b/meilisearch-auth/src/store.rs @@ -14,7 +14,6 @@ use meilisearch_types::keys::KeyId; use meilisearch_types::milli; use meilisearch_types::milli::heed::types::{ByteSlice, DecodeIgnore, SerdeJson}; use meilisearch_types::milli::heed::{Database, Env, EnvOpenOptions, RwTxn}; -use meilisearch_types::star_or::StarOr; use sha2::Sha256; use time::OffsetDateTime; use uuid::fmt::Hyphenated; @@ -126,7 +125,7 @@ impl HeedAuthStore { } } - let no_index_restriction = key.indexes.contains(&StarOr::Star); + let no_index_restriction = key.indexes.iter().any(|p| p.matches_all()); for action in actions { if no_index_restriction { // If there is no index restriction we put None. diff --git a/meilisearch-types/src/index_uid_pattern.rs b/meilisearch-types/src/index_uid_pattern.rs index bc12da351..88e0292f2 100644 --- a/meilisearch-types/src/index_uid_pattern.rs +++ b/meilisearch-types/src/index_uid_pattern.rs @@ -26,6 +26,11 @@ impl IndexUidPattern { IndexUidPattern::from_str("*").unwrap() } + /// Returns `true` if it matches any index. + pub fn matches_all(&self) -> bool { + self.0 == "*" + } + /// Returns `true` if the pattern matches a specific index name. pub fn is_exact(&self) -> bool { !self.0.ends_with('*') diff --git a/meilisearch-types/src/keys.rs b/meilisearch-types/src/keys.rs index a9e2e0889..f594640d9 100644 --- a/meilisearch-types/src/keys.rs +++ b/meilisearch-types/src/keys.rs @@ -13,7 +13,6 @@ use uuid::Uuid; use crate::error::deserr_codes::*; use crate::error::{unwrap_any, Code, DeserrError, ErrorCode, TakeErrorMessage}; use crate::index_uid_pattern::{IndexUidPattern, IndexUidPatternFormatError}; -use crate::star_or::StarOr; pub type KeyId = Uuid; diff --git a/meilisearch/src/extractors/authentication/mod.rs b/meilisearch/src/extractors/authentication/mod.rs index f1efdf9aa..4836679a9 100644 --- a/meilisearch/src/extractors/authentication/mod.rs +++ b/meilisearch/src/extractors/authentication/mod.rs @@ -199,6 +199,9 @@ pub mod policies { token: &str, index: Option<&str>, ) -> Option { + // Tenant token will always define an index. + let index = index?; + // Only search action can be accessed by a tenant token. if A != actions::SEARCH { return None; @@ -206,7 +209,7 @@ pub mod policies { let uid = extract_key_id(token)?; // check if parent key is authorized to do the action. - if auth.is_key_authorized(uid, Action::Search, index).ok()? { + if auth.is_key_authorized(uid, Action::Search, Some(index)).ok()? { // Check if tenant token is valid. let key = auth.generate_key(uid)?; let data = decode::( @@ -217,10 +220,8 @@ pub mod policies { .ok()?; // Check index access if an index restriction is provided. - if let Some(index) = index { - if !data.claims.search_rules.is_index_authorized(index) { - return None; - } + if !data.claims.search_rules.is_index_authorized(index) { + return None; } // Check if token is expired. @@ -230,10 +231,10 @@ pub mod policies { } } - match auth.get_key_filters(uid, Some(data.claims.search_rules)) { - Ok(auth) if auth.search_rules.is_index_authorized() => Some(auth), + return match auth.get_key_filters(uid, Some(data.claims.search_rules)) { + Ok(auth) if auth.search_rules.is_index_authorized(index) => Some(auth), _ => None, - } + }; } None From 7b4b57ecc82563fbc842e1d1eee25b8a9468896b Mon Sep 17 00:00:00 2001 From: Kerollmops Date: Wed, 8 Feb 2023 14:54:05 +0100 Subject: [PATCH 08/13] Fix the current tests --- dump/src/lib.rs | 7 +++---- meilisearch-types/src/index_uid_pattern.rs | 1 + meilisearch/tests/auth/api_keys.rs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/dump/src/lib.rs b/dump/src/lib.rs index 7a7b9a5b7..8f7c28b8a 100644 --- a/dump/src/lib.rs +++ b/dump/src/lib.rs @@ -203,12 +203,11 @@ pub(crate) mod test { use big_s::S; use maplit::btreeset; - use meilisearch_types::index_uid::IndexUid; + use meilisearch_types::index_uid_pattern::IndexUidPattern; use meilisearch_types::keys::{Action, Key}; use meilisearch_types::milli::update::Setting; use meilisearch_types::milli::{self}; use meilisearch_types::settings::{Checked, Settings}; - use meilisearch_types::star_or::StarOr; use meilisearch_types::tasks::{Details, Status}; use serde_json::{json, Map, Value}; use time::macros::datetime; @@ -341,7 +340,7 @@ pub(crate) mod test { name: Some(S("doggos_key")), uid: Uuid::from_str("9f8a34da-b6b2-42f0-939b-dbd4c3448655").unwrap(), actions: vec![Action::DocumentsAll], - indexes: vec![StarOr::Other(IndexUid::from_str("doggos").unwrap())], + indexes: vec![IndexUidPattern::from_str("doggos").unwrap()], expires_at: Some(datetime!(4130-03-14 12:21 UTC)), created_at: datetime!(1960-11-15 0:00 UTC), updated_at: datetime!(2022-11-10 0:00 UTC), @@ -351,7 +350,7 @@ pub(crate) mod test { name: Some(S("master_key")), uid: Uuid::from_str("4622f717-1c00-47bb-a494-39d76a49b591").unwrap(), actions: vec![Action::All], - indexes: vec![StarOr::Star], + indexes: vec![IndexUidPattern::all()], expires_at: None, created_at: datetime!(0000-01-01 00:01 UTC), updated_at: datetime!(1964-05-04 17:25 UTC), diff --git a/meilisearch-types/src/index_uid_pattern.rs b/meilisearch-types/src/index_uid_pattern.rs index 88e0292f2..9f49c06ea 100644 --- a/meilisearch-types/src/index_uid_pattern.rs +++ b/meilisearch-types/src/index_uid_pattern.rs @@ -69,6 +69,7 @@ impl TryFrom for IndexUidPattern { fn try_from(uid: String) -> Result { let result = match uid.strip_suffix('*') { + Some("") => Ok(IndexUidPattern(uid)), Some(prefix) => IndexUid::from_str(prefix).map(|_| IndexUidPattern(uid)), None => IndexUid::try_from(uid).map(IndexUid::into_inner).map(IndexUidPattern), }; diff --git a/meilisearch/tests/auth/api_keys.rs b/meilisearch/tests/auth/api_keys.rs index 0a14107a8..abc0aea53 100644 --- a/meilisearch/tests/auth/api_keys.rs +++ b/meilisearch/tests/auth/api_keys.rs @@ -386,7 +386,7 @@ async fn error_add_api_key_invalid_index_uids() { meili_snap::snapshot!(code, @"400 Bad Request"); meili_snap::snapshot!(meili_snap::json_string!(response, { ".createdAt" => "[ignored]", ".updatedAt" => "[ignored]" }), @r###" { - "message": "`invalid index # / \\name with spaces` is not a valid index uid. Index uid can be an integer or a string containing only alphanumeric characters, hyphens (-) and underscores (_). at `.indexes[0]`.", + "message": "`invalid index # / \\name with spaces` is not a valid index uid pattern. Index uid patterns can be an integer or a string containing only alphanumeric characters, hyphens (-), underscores (_), and optionally end with a star (*). at `.indexes[0]`.", "code": "invalid_api_key_indexes", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid-api-key-indexes" From c690c4fec4f7eda2f32037aaa1ac2c038e4d8fb4 Mon Sep 17 00:00:00 2001 From: Kerollmops Date: Wed, 8 Feb 2023 18:19:51 +0100 Subject: [PATCH 09/13] Added and modified the current API Key and Tenant Token tests --- meilisearch/tests/auth/tenant_token.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/meilisearch/tests/auth/tenant_token.rs b/meilisearch/tests/auth/tenant_token.rs index af3e7c2a5..81e161b5a 100644 --- a/meilisearch/tests/auth/tenant_token.rs +++ b/meilisearch/tests/auth/tenant_token.rs @@ -82,6 +82,11 @@ static ACCEPTED_KEYS: Lazy> = Lazy::new(|| { "actions": ["search"], "expiresAt": (OffsetDateTime::now_utc() + Duration::days(1)).format(&Rfc3339).unwrap() }), + json!({ + "indexes": ["sal*", "prod*"], + "actions": ["search"], + "expiresAt": (OffsetDateTime::now_utc() + Duration::days(1)).format(&Rfc3339).unwrap() + }), ] }); @@ -104,6 +109,11 @@ static REFUSED_KEYS: Lazy> = Lazy::new(|| { "actions": ["*"], "expiresAt": (OffsetDateTime::now_utc() + Duration::days(1)).format(&Rfc3339).unwrap() }), + json!({ + "indexes": ["prod*", "p*"], + "actions": ["*"], + "expiresAt": (OffsetDateTime::now_utc() + Duration::days(1)).format(&Rfc3339).unwrap() + }), json!({ "indexes": ["products"], "actions": ["search"], @@ -245,6 +255,10 @@ async fn search_authorized_simple_token() { "searchRules" => json!(["sales"]), "exp" => Value::Null }, + hashmap! { + "searchRules" => json!(["sa*"]), + "exp" => Value::Null + }, ]; compute_authorized_search!(tenant_tokens, {}, 5); @@ -351,11 +365,19 @@ async fn filter_search_authorized_filter_token() { }), "exp" => json!((OffsetDateTime::now_utc() + Duration::hours(1)).unix_timestamp()) }, + hashmap! { + "searchRules" => json!({ + "*": {}, + "sal*": {"filter": ["color = blue"]} + }), + "exp" => json!((OffsetDateTime::now_utc() + Duration::hours(1)).unix_timestamp()) + }, ]; compute_authorized_search!(tenant_tokens, "color = yellow", 1); } +/// Tests that those Tenant Token are incompatible with the REFUSED_KEYS defined above. #[actix_rt::test] async fn error_search_token_forbidden_parent_key() { let tenant_tokens = vec![ @@ -383,6 +405,10 @@ async fn error_search_token_forbidden_parent_key() { "searchRules" => json!(["sales"]), "exp" => json!((OffsetDateTime::now_utc() + Duration::hours(1)).unix_timestamp()) }, + hashmap! { + "searchRules" => json!(["sali*", "s*", "sales*"]), + "exp" => json!((OffsetDateTime::now_utc() + Duration::hours(1)).unix_timestamp()) + }, ]; compute_forbidden_search!(tenant_tokens, REFUSED_KEYS); From 764df24b7d6ff98cbf9ca77fa103acb9cc69ff25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Renault?= Date: Thu, 9 Feb 2023 13:21:20 +0100 Subject: [PATCH 10/13] Make clippy happy (again) --- meilisearch-auth/src/lib.rs | 3 +-- meilisearch/tests/auth/authorization.rs | 8 ++++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/meilisearch-auth/src/lib.rs b/meilisearch-auth/src/lib.rs index c27b71e74..9f9b15f38 100644 --- a/meilisearch-auth/src/lib.rs +++ b/meilisearch-auth/src/lib.rs @@ -214,8 +214,7 @@ impl SearchRules { map.iter() .filter(|(pattern, _)| pattern.matches_str(index)) .max_by_key(|(pattern, _)| (pattern.is_exact(), pattern.len())) - .map(|(_, rule)| rule.clone()) - .flatten() + .and_then(|(_, rule)| rule.clone()) } } } diff --git a/meilisearch/tests/auth/authorization.rs b/meilisearch/tests/auth/authorization.rs index 15d4d96fb..69a74b022 100644 --- a/meilisearch/tests/auth/authorization.rs +++ b/meilisearch/tests/auth/authorization.rs @@ -445,7 +445,7 @@ async fn access_authorized_index_patterns() { // use created key. let key = response["key"].as_str().unwrap(); - server.use_api_key(&key); + server.use_api_key(key); // refer to products_1 and products with modified api key. let index_1 = server.index("products_1"); @@ -515,7 +515,7 @@ async fn raise_error_non_authorized_index_patterns() { // use created key. let key = response["key"].as_str().unwrap(); - server.use_api_key(&key); + server.use_api_key(key); // refer to products_1 and products_2 with modified api key. let product_1_index = server.index("products_1"); @@ -582,7 +582,7 @@ async fn pattern_indexes() { let (response, code) = server.add_api_key(content).await; assert_eq!(201, code, "{:?}", &response); let key = response["key"].as_str().expect("Key is not string"); - server.use_api_key(&key); + server.use_api_key(key); // Create Index products_1 using generated api key let products_1 = server.index("products_1"); @@ -859,7 +859,7 @@ async fn lazy_create_index_from_pattern() { // use created key. let key = response["key"].as_str().unwrap(); - server.use_api_key(&key); + server.use_api_key(key); // try to create a index via add documents route let index = server.index("products_1"); From 47748395dcfcbc78acb2838ebcf3cb5afa4b871b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Renault?= Date: Mon, 13 Feb 2023 17:20:08 +0100 Subject: [PATCH 11/13] Update an authentication comment Co-authored-by: Many the fish --- meilisearch/src/extractors/authentication/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meilisearch/src/extractors/authentication/mod.rs b/meilisearch/src/extractors/authentication/mod.rs index 4836679a9..84b598689 100644 --- a/meilisearch/src/extractors/authentication/mod.rs +++ b/meilisearch/src/extractors/authentication/mod.rs @@ -199,7 +199,7 @@ pub mod policies { token: &str, index: Option<&str>, ) -> Option { - // Tenant token will always define an index. + // A tenant token only has access to the search route which always defines an index. let index = index?; // Only search action can be accessed by a tenant token. From 4b1cd10653f0e9d7d9db5945caf4abd8b7e21758 Mon Sep 17 00:00:00 2001 From: Kerollmops Date: Mon, 13 Feb 2023 17:26:34 +0100 Subject: [PATCH 12/13] Return an internal error when index pattern should be valid --- meilisearch-auth/src/store.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/meilisearch-auth/src/store.rs b/meilisearch-auth/src/store.rs index 459234065..cc5dcdfb5 100644 --- a/meilisearch-auth/src/store.rs +++ b/meilisearch-auth/src/store.rs @@ -5,6 +5,7 @@ use std::convert::{TryFrom, TryInto}; use std::fs::create_dir_all; use std::path::Path; use std::str; +use std::str::FromStr; use std::sync::Arc; use hmac::{Hmac, Mac}; @@ -18,7 +19,7 @@ use time::OffsetDateTime; use uuid::fmt::Hyphenated; use uuid::Uuid; -use super::error::Result; +use super::error::{AuthControllerError, Result}; use super::{Action, Key}; const AUTH_STORE_SIZE: usize = 1_073_741_824; //1GiB @@ -225,9 +226,9 @@ impl HeedAuthStore { for result in self.action_keyid_index_expiration.prefix_iter(&rtxn, &tuple)? { let ((_, _, index_uid_pattern), expiration) = result?; if let Some((pattern, index)) = index_uid_pattern.zip(index) { - let index_uid_pattern = str::from_utf8(pattern)?.to_string(); - // TODO I shouldn't unwrap here but rather return an internal error - let pattern = IndexUidPattern::try_from(index_uid_pattern).unwrap(); + let index_uid_pattern = str::from_utf8(pattern)?; + let pattern = IndexUidPattern::from_str(index_uid_pattern) + .map_err(|e| AuthControllerError::Internal(Box::new(e)))?; if pattern.matches_str(index) { return Ok(Some(expiration)); } From 7f3ae40204aa861440349ec3d95f48a207ab2289 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Renault?= Date: Tue, 14 Feb 2023 17:09:20 +0100 Subject: [PATCH 13/13] Remove a useless comment regarding the index pattern error code --- meilisearch-types/src/index_uid_pattern.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/meilisearch-types/src/index_uid_pattern.rs b/meilisearch-types/src/index_uid_pattern.rs index 9f49c06ea..99537b22f 100644 --- a/meilisearch-types/src/index_uid_pattern.rs +++ b/meilisearch-types/src/index_uid_pattern.rs @@ -119,7 +119,6 @@ impl Error for IndexUidPatternFormatError {} impl ErrorCode for IndexUidPatternFormatError { fn error_code(&self) -> Code { - // TODO should I return a new error code? Code::InvalidIndexUid } }