mirror of
				https://github.com/meilisearch/meilisearch.git
				synced 2025-10-30 23:46:28 +00:00 
			
		
		
		
	Fix(auth): Forbid index creation on alternates routes
Forbid index creation on alternates routes when the action `index.create` is not given fix #2024
This commit is contained in:
		
				
					committed by
					
						 Maxime Legendre
						Maxime Legendre
					
				
			
			
				
	
			
			
			
						parent
						
							845d3114ea
						
					
				
				
					commit
					a845cd8880
				
			| @@ -69,6 +69,11 @@ impl AuthController { | ||||
|             if !key.indexes.iter().any(|i| i.as_str() == "*") { | ||||
|                 filters.indexes = Some(key.indexes); | ||||
|             } | ||||
|  | ||||
|             filters.allow_index_creation = key | ||||
|                 .actions | ||||
|                 .iter() | ||||
|                 .any(|&action| action == Action::IndexesAdd || action == Action::All); | ||||
|         } | ||||
|  | ||||
|         Ok(filters) | ||||
| @@ -118,9 +123,18 @@ impl AuthController { | ||||
|     } | ||||
| } | ||||
|  | ||||
| #[derive(Default)] | ||||
| pub struct AuthFilter { | ||||
|     pub indexes: Option<Vec<String>>, | ||||
|     pub allow_index_creation: bool, | ||||
| } | ||||
|  | ||||
| impl Default for AuthFilter { | ||||
|     fn default() -> Self { | ||||
|         Self { | ||||
|             indexes: None, | ||||
|             allow_index_creation: true, | ||||
|         } | ||||
|     } | ||||
| } | ||||
|  | ||||
| pub fn generate_key(master_key: &[u8], uid: &str) -> String { | ||||
|   | ||||
| @@ -173,6 +173,7 @@ pub async fn add_documents( | ||||
|         &req, | ||||
|     ); | ||||
|  | ||||
|     let allow_index_creation = meilisearch.filters().allow_index_creation; | ||||
|     let task = document_addition( | ||||
|         extract_mime_type(&req)?, | ||||
|         meilisearch, | ||||
| @@ -180,6 +181,7 @@ pub async fn add_documents( | ||||
|         params.primary_key, | ||||
|         body, | ||||
|         IndexDocumentsMethod::ReplaceDocuments, | ||||
|         allow_index_creation, | ||||
|     ) | ||||
|     .await?; | ||||
|  | ||||
| @@ -203,6 +205,7 @@ pub async fn update_documents( | ||||
|         &req, | ||||
|     ); | ||||
|  | ||||
|     let allow_index_creation = meilisearch.filters().allow_index_creation; | ||||
|     let task = document_addition( | ||||
|         extract_mime_type(&req)?, | ||||
|         meilisearch, | ||||
| @@ -210,6 +213,7 @@ pub async fn update_documents( | ||||
|         params.into_inner().primary_key, | ||||
|         body, | ||||
|         IndexDocumentsMethod::UpdateDocuments, | ||||
|         allow_index_creation, | ||||
|     ) | ||||
|     .await?; | ||||
|  | ||||
| @@ -223,6 +227,7 @@ async fn document_addition( | ||||
|     primary_key: Option<String>, | ||||
|     body: Payload, | ||||
|     method: IndexDocumentsMethod, | ||||
|     allow_index_creation: bool, | ||||
| ) -> Result<SummarizedTaskView, ResponseError> { | ||||
|     let format = match mime_type | ||||
|         .as_ref() | ||||
| @@ -250,6 +255,7 @@ async fn document_addition( | ||||
|         primary_key, | ||||
|         method, | ||||
|         format, | ||||
|         allow_index_creation, | ||||
|     }; | ||||
|  | ||||
|     let task = meilisearch.register_update(index_uid, update).await?.into(); | ||||
|   | ||||
| @@ -34,9 +34,12 @@ macro_rules! make_setting_route { | ||||
|                     $attr: Setting::Reset, | ||||
|                     ..Default::default() | ||||
|                 }; | ||||
|  | ||||
|                 let allow_index_creation = meilisearch.filters().allow_index_creation; | ||||
|                 let update = Update::Settings { | ||||
|                     settings, | ||||
|                     is_deletion: true, | ||||
|                     allow_index_creation, | ||||
|                 }; | ||||
|                 let task: SummarizedTaskView = meilisearch | ||||
|                     .register_update(index_uid.into_inner(), update) | ||||
| @@ -66,9 +69,11 @@ macro_rules! make_setting_route { | ||||
|                     ..Default::default() | ||||
|                 }; | ||||
|  | ||||
|                 let allow_index_creation = meilisearch.filters().allow_index_creation; | ||||
|                 let update = Update::Settings { | ||||
|                     settings, | ||||
|                     is_deletion: false, | ||||
|                     allow_index_creation, | ||||
|                 }; | ||||
|                 let task: SummarizedTaskView = meilisearch | ||||
|                     .register_update(index_uid.into_inner(), update) | ||||
| @@ -272,9 +277,11 @@ pub async fn update_all( | ||||
|         Some(&req), | ||||
|     ); | ||||
|  | ||||
|     let allow_index_creation = meilisearch.filters().allow_index_creation; | ||||
|     let update = Update::Settings { | ||||
|         settings, | ||||
|         is_deletion: false, | ||||
|         allow_index_creation, | ||||
|     }; | ||||
|     let task: SummarizedTaskView = meilisearch | ||||
|         .register_update(index_uid.into_inner(), update) | ||||
| @@ -300,9 +307,11 @@ pub async fn delete_all( | ||||
| ) -> Result<HttpResponse, ResponseError> { | ||||
|     let settings = Settings::cleared().into_unchecked(); | ||||
|  | ||||
|     let allow_index_creation = data.filters().allow_index_creation; | ||||
|     let update = Update::Settings { | ||||
|         settings, | ||||
|         is_deletion: true, | ||||
|         allow_index_creation, | ||||
|     }; | ||||
|     let task: SummarizedTaskView = data | ||||
|         .register_update(index_uid.into_inner(), update) | ||||
|   | ||||
| @@ -496,3 +496,135 @@ async fn list_authorized_tasks_no_index_restriction() { | ||||
|     // key should have access on `test` index. | ||||
|     assert!(response.iter().any(|task| task["indexUid"] == "test")); | ||||
| } | ||||
|  | ||||
| #[actix_rt::test] | ||||
| async fn error_creating_index_without_action() { | ||||
|     let mut server = Server::new_auth().await; | ||||
|     server.use_api_key("MASTER_KEY"); | ||||
|  | ||||
|     // create key with access on all indexes. | ||||
|     let content = json!({ | ||||
|         "indexes": ["*"], | ||||
|         "actions": ALL_ACTIONS.iter().cloned().filter(|a| *a != "indexes.create").collect::<Vec<_>>(), | ||||
|         "expiresAt": "2050-11-13T00:00:00Z" | ||||
|     }); | ||||
|     let (response, code) = server.add_api_key(content).await; | ||||
|     assert_eq!(code, 201); | ||||
|     assert!(response["key"].is_string()); | ||||
|  | ||||
|     // use created key. | ||||
|     let key = response["key"].as_str().unwrap(); | ||||
|     server.use_api_key(&key); | ||||
|  | ||||
|     let expected_error = json!({ | ||||
|         "message": "Index `test` not found.", | ||||
|         "code": "index_not_found", | ||||
|         "type": "invalid_request", | ||||
|         "link": "https://docs.meilisearch.com/errors#index_not_found" | ||||
|     }); | ||||
|  | ||||
|     // try to create a index via add documents route | ||||
|     let index = server.index("test"); | ||||
|     let documents = json!([ | ||||
|         { | ||||
|             "id": 1, | ||||
|             "content": "foo", | ||||
|         } | ||||
|     ]); | ||||
|  | ||||
|     let (response, code) = index.add_documents(documents, None).await; | ||||
|     assert_eq!(code, 202, "{:?}", response); | ||||
|     let task_id = response["uid"].as_u64().unwrap(); | ||||
|  | ||||
|     let response = index.wait_task(task_id).await; | ||||
|     assert_eq!(response["status"], "failed"); | ||||
|     assert_eq!(response["error"], expected_error.clone()); | ||||
|  | ||||
|     // try to create a index via add settings route | ||||
|     let settings = json!({ "distinctAttribute": "test"}); | ||||
|  | ||||
|     let (response, code) = index.update_settings(settings).await; | ||||
|     assert_eq!(code, 202); | ||||
|     let task_id = response["uid"].as_u64().unwrap(); | ||||
|  | ||||
|     let response = index.wait_task(task_id).await; | ||||
|  | ||||
|     assert_eq!(response["status"], "failed"); | ||||
|     assert_eq!(response["error"], expected_error.clone()); | ||||
|  | ||||
|     // try to create a index via add specialized settings route | ||||
|     let (response, code) = index.update_distinct_attribute(json!("test")).await; | ||||
|     assert_eq!(code, 202); | ||||
|     let task_id = response["uid"].as_u64().unwrap(); | ||||
|  | ||||
|     let response = index.wait_task(task_id).await; | ||||
|  | ||||
|     assert_eq!(response["status"], "failed"); | ||||
|     assert_eq!(response["error"], expected_error.clone()); | ||||
| } | ||||
|  | ||||
| #[actix_rt::test] | ||||
| async fn lazy_create_index() { | ||||
|     let mut server = Server::new_auth().await; | ||||
|     server.use_api_key("MASTER_KEY"); | ||||
|  | ||||
|     // create key with access on all indexes. | ||||
|     let content = json!({ | ||||
|         "indexes": ["*"], | ||||
|         "actions": ["*"], | ||||
|         "expiresAt": "2050-11-13T00:00:00Z" | ||||
|     }); | ||||
|  | ||||
|     let (response, code) = server.add_api_key(content).await; | ||||
|     assert_eq!(code, 201); | ||||
|     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("test"); | ||||
|     let documents = json!([ | ||||
|         { | ||||
|             "id": 1, | ||||
|             "content": "foo", | ||||
|         } | ||||
|     ]); | ||||
|  | ||||
|     let (response, code) = index.add_documents(documents, None).await; | ||||
|     assert_eq!(code, 202, "{:?}", response); | ||||
|     let task_id = response["uid"].as_u64().unwrap(); | ||||
|  | ||||
|     index.wait_task(task_id).await; | ||||
|  | ||||
|     let (response, code) = index.get_task(task_id).await; | ||||
|     assert_eq!(code, 200); | ||||
|     assert_eq!(response["status"], "succeeded"); | ||||
|  | ||||
|     // try to create a index via add settings route | ||||
|     let index = server.index("test1"); | ||||
|     let settings = json!({ "distinctAttribute": "test"}); | ||||
|  | ||||
|     let (response, code) = index.update_settings(settings).await; | ||||
|     assert_eq!(code, 202); | ||||
|     let task_id = response["uid"].as_u64().unwrap(); | ||||
|  | ||||
|     index.wait_task(task_id).await; | ||||
|  | ||||
|     let (response, code) = index.get_task(task_id).await; | ||||
|     assert_eq!(code, 200); | ||||
|     assert_eq!(response["status"], "succeeded"); | ||||
|  | ||||
|     // try to create a index via add specialized settings route | ||||
|     let index = server.index("test2"); | ||||
|     let (response, code) = index.update_distinct_attribute(json!("test")).await; | ||||
|     assert_eq!(code, 202); | ||||
|     let task_id = response["uid"].as_u64().unwrap(); | ||||
|  | ||||
|     index.wait_task(task_id).await; | ||||
|  | ||||
|     let (response, code) = index.get_task(task_id).await; | ||||
|     assert_eq!(code, 200); | ||||
|     assert_eq!(response["status"], "succeeded"); | ||||
| } | ||||
|   | ||||
| @@ -2,29 +2,12 @@ mod api_keys; | ||||
| mod authorization; | ||||
| mod payload; | ||||
|  | ||||
| use crate::common::server::default_settings; | ||||
| use crate::common::server::TEST_TEMP_DIR; | ||||
| use crate::common::Server; | ||||
| use actix_web::http::StatusCode; | ||||
|  | ||||
| use serde_json::{json, Value}; | ||||
| use tempfile::TempDir; | ||||
|  | ||||
| impl Server { | ||||
|     pub async fn new_auth() -> Self { | ||||
|         let dir = TempDir::new().unwrap(); | ||||
|  | ||||
|         if cfg!(windows) { | ||||
|             std::env::set_var("TMP", TEST_TEMP_DIR.path()); | ||||
|         } else { | ||||
|             std::env::set_var("TMPDIR", TEST_TEMP_DIR.path()); | ||||
|         } | ||||
|  | ||||
|         let mut options = default_settings(dir.path()); | ||||
|         options.master_key = Some("MASTER_KEY".to_string()); | ||||
|  | ||||
|         Self::new_with_options(options).await | ||||
|     } | ||||
|  | ||||
|     pub fn use_api_key(&mut self, api_key: impl AsRef<str>) { | ||||
|         self.service.api_key = Some(api_key.as_ref().to_string()); | ||||
|     } | ||||
|   | ||||
| @@ -50,6 +50,33 @@ impl Server { | ||||
|         } | ||||
|     } | ||||
|  | ||||
|     pub async fn new_auth() -> Self { | ||||
|         let dir = TempDir::new().unwrap(); | ||||
|  | ||||
|         if cfg!(windows) { | ||||
|             std::env::set_var("TMP", TEST_TEMP_DIR.path()); | ||||
|         } else { | ||||
|             std::env::set_var("TMPDIR", TEST_TEMP_DIR.path()); | ||||
|         } | ||||
|  | ||||
|         let mut options = default_settings(dir.path()); | ||||
|         options.master_key = Some("MASTER_KEY".to_string()); | ||||
|  | ||||
|         let meilisearch = setup_meilisearch(&options).unwrap(); | ||||
|         let auth = AuthController::new(&options.db_path, &options.master_key).unwrap(); | ||||
|         let service = Service { | ||||
|             meilisearch, | ||||
|             auth, | ||||
|             options, | ||||
|             api_key: None, | ||||
|         }; | ||||
|  | ||||
|         Server { | ||||
|             service, | ||||
|             _dir: Some(dir), | ||||
|         } | ||||
|     } | ||||
|  | ||||
|     pub async fn new_with_options(options: Opt) -> Self { | ||||
|         let meilisearch = setup_meilisearch(&options).unwrap(); | ||||
|         let auth = AuthController::new(&options.db_path, &options.master_key).unwrap(); | ||||
|   | ||||
| @@ -17,3 +17,4 @@ cc 3a01c78db082434b8a4f8914abf0d1059d39f4426d16df20d72e1bd7ebb94a6a # shrinks to | ||||
| cc c450806df3921d1e6fe9b6af93d999e8196d0175b69b64f1810802582421e94a # shrinks to task = Task { id: 0, index_uid: IndexUid("a"), content: CreateIndex { primary_key: Some("") }, events: [] }, index_exists = false, index_op_fails = false, any_int = 0 | ||||
| cc fb6b98947cbdbdee05ed3c0bf2923aad2c311edc276253642eb43a0c0ec4888a # shrinks to task = Task { id: 0, index_uid: IndexUid("A"), content: CreateIndex { primary_key: Some("") }, events: [] }, index_exists = false, index_op_fails = true, any_int = 0 | ||||
| cc 1aa59d8e22484e9915efbb5818e1e1ab684aa61b166dc82130d6221663ba00bf # shrinks to task = Task { id: 0, index_uid: IndexUid("a"), content: DocumentDeletion(Clear), events: [] }, index_exists = true, index_op_fails = false, any_int = 0 | ||||
| cc 2e8644e6397b5f76e0b79f961fa125e2f45f42f26e03c453c9a174dfb427500d # shrinks to task = Task { id: 0, index_uid: IndexUid("0"), content: SettingsUpdate { settings: Settings { displayed_attributes: NotSet, searchable_attributes: NotSet, filterable_attributes: NotSet, sortable_attributes: NotSet, ranking_rules: NotSet, stop_words: NotSet, synonyms: NotSet, distinct_attribute: NotSet, _kind: PhantomData }, is_deletion: false, allow_index_creation: false }, events: [] }, index_exists = false, index_op_fails = false, any_int = 0 | ||||
|   | ||||
| @@ -74,11 +74,13 @@ impl From<Update> for TaskContent { | ||||
|                 primary_key, | ||||
|                 // document count is unknown for legacy updates | ||||
|                 documents_count: 0, | ||||
|                 allow_index_creation: true, | ||||
|             }, | ||||
|             Update::Settings(settings) => TaskContent::SettingsUpdate { | ||||
|                 settings, | ||||
|                 // There is no way to know now, so we assume it isn't | ||||
|                 is_deletion: false, | ||||
|                 allow_index_creation: true, | ||||
|             }, | ||||
|             Update::ClearDocuments => TaskContent::DocumentDeletion(DocumentDeletion::Clear), | ||||
|         } | ||||
|   | ||||
| @@ -119,6 +119,7 @@ pub enum Update { | ||||
|         settings: Settings<Unchecked>, | ||||
|         /// Indicates whether the update was a deletion | ||||
|         is_deletion: bool, | ||||
|         allow_index_creation: bool, | ||||
|     }, | ||||
|     DocumentAddition { | ||||
|         #[derivative(Debug = "ignore")] | ||||
| @@ -126,6 +127,7 @@ pub enum Update { | ||||
|         primary_key: Option<String>, | ||||
|         method: IndexDocumentsMethod, | ||||
|         format: DocumentAdditionFormat, | ||||
|         allow_index_creation: bool, | ||||
|     }, | ||||
|     DeleteIndex, | ||||
|     CreateIndex { | ||||
| @@ -340,15 +342,18 @@ where | ||||
|             Update::Settings { | ||||
|                 settings, | ||||
|                 is_deletion, | ||||
|                 allow_index_creation, | ||||
|             } => TaskContent::SettingsUpdate { | ||||
|                 settings, | ||||
|                 is_deletion, | ||||
|                 allow_index_creation, | ||||
|             }, | ||||
|             Update::DocumentAddition { | ||||
|                 mut payload, | ||||
|                 primary_key, | ||||
|                 format, | ||||
|                 method, | ||||
|                 allow_index_creation, | ||||
|             } => { | ||||
|                 let mut buffer = Vec::new(); | ||||
|                 while let Some(bytes) = payload.next().await { | ||||
| @@ -380,6 +385,7 @@ where | ||||
|                     merge_strategy: method, | ||||
|                     primary_key, | ||||
|                     documents_count, | ||||
|                     allow_index_creation, | ||||
|                 } | ||||
|             } | ||||
|             Update::DeleteIndex => TaskContent::IndexDeletion, | ||||
|   | ||||
| @@ -188,13 +188,18 @@ where | ||||
|                 content_uuid, | ||||
|                 merge_strategy, | ||||
|                 primary_key, | ||||
|                 allow_index_creation, | ||||
|                 .. | ||||
|             } => { | ||||
|                 let primary_key = primary_key.clone(); | ||||
|                 let content_uuid = *content_uuid; | ||||
|                 let method = *merge_strategy; | ||||
|  | ||||
|                 let index = self.get_or_create_index(index_uid, task.id).await?; | ||||
|                 let index = if *allow_index_creation { | ||||
|                     self.get_or_create_index(index_uid, task.id).await? | ||||
|                 } else { | ||||
|                     self.get_index(index_uid.into_inner()).await? | ||||
|                 }; | ||||
|                 let file_store = self.file_store.clone(); | ||||
|                 let result = spawn_blocking(move || { | ||||
|                     index.update_documents(method, content_uuid, primary_key, file_store) | ||||
| @@ -227,8 +232,9 @@ where | ||||
|             TaskContent::SettingsUpdate { | ||||
|                 settings, | ||||
|                 is_deletion, | ||||
|                 allow_index_creation, | ||||
|             } => { | ||||
|                 let index = if *is_deletion { | ||||
|                 let index = if *is_deletion || !*allow_index_creation { | ||||
|                     self.get_index(index_uid.into_inner()).await? | ||||
|                 } else { | ||||
|                     self.get_or_create_index(index_uid, task.id).await? | ||||
| @@ -503,8 +509,8 @@ mod test { | ||||
|  | ||||
|                 match &task.content { | ||||
|                     // an unexisting index should trigger an index creation in the folllowing cases: | ||||
|                     TaskContent::DocumentAddition { .. } | ||||
|                     | TaskContent::SettingsUpdate { is_deletion: false, .. } | ||||
|                     TaskContent::DocumentAddition { allow_index_creation: true, .. } | ||||
|                     | TaskContent::SettingsUpdate { allow_index_creation: true, is_deletion: false, .. } | ||||
|                     | TaskContent::IndexCreation { .. } if !index_exists => { | ||||
|                         index_store | ||||
|                             .expect_create() | ||||
| @@ -566,6 +572,8 @@ mod test { | ||||
|                     || (!index_exists && matches!(task.content, TaskContent::IndexDeletion | ||||
|                                                                 | TaskContent::DocumentDeletion(_) | ||||
|                                                                 | TaskContent::SettingsUpdate { is_deletion: true, ..} | ||||
|                                                                 | TaskContent::SettingsUpdate { allow_index_creation: false, ..} | ||||
|                                                                 | TaskContent::DocumentAddition { allow_index_creation: false, ..} | ||||
|                                                                 | TaskContent::IndexUpdate { .. } )) | ||||
|                 { | ||||
|                     assert!(result.is_err(), "{:?}", result); | ||||
|   | ||||
| @@ -134,12 +134,14 @@ pub enum TaskContent { | ||||
|         merge_strategy: IndexDocumentsMethod, | ||||
|         primary_key: Option<String>, | ||||
|         documents_count: usize, | ||||
|         allow_index_creation: bool, | ||||
|     }, | ||||
|     DocumentDeletion(DocumentDeletion), | ||||
|     SettingsUpdate { | ||||
|         settings: Settings<Unchecked>, | ||||
|         /// Indicates whether the task was a deletion | ||||
|         is_deletion: bool, | ||||
|         allow_index_creation: bool, | ||||
|     }, | ||||
|     IndexDeletion, | ||||
|     IndexCreation { | ||||
|   | ||||
		Reference in New Issue
	
	Block a user