From 1cf6efa7405655233d94c28d9800703c763b24a5 Mon Sep 17 00:00:00 2001 From: vishalsodani Date: Tue, 18 Oct 2022 10:48:45 +0530 Subject: [PATCH 1/6] Add new error when using /keys without masterkey set --- meilisearch-http/src/extractors/authentication/error.rs | 3 +++ meilisearch-http/src/extractors/authentication/mod.rs | 2 +- meilisearch-types/src/error.rs | 4 ++++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/meilisearch-http/src/extractors/authentication/error.rs b/meilisearch-http/src/extractors/authentication/error.rs index bb78c53d0..7fa0319b8 100644 --- a/meilisearch-http/src/extractors/authentication/error.rs +++ b/meilisearch-http/src/extractors/authentication/error.rs @@ -9,6 +9,8 @@ pub enum AuthenticationError { // Triggered on configuration error. #[error("An internal error has occurred. `Irretrievable state`.")] IrretrievableState, + #[error("Meilisearch is running without a master key. To access this API endpoint, you must have set a master key at launch.")] + MissingMasterKey, } impl ErrorCode for AuthenticationError { @@ -17,6 +19,7 @@ impl ErrorCode for AuthenticationError { AuthenticationError::MissingAuthorizationHeader => Code::MissingAuthorizationHeader, AuthenticationError::InvalidToken => Code::InvalidToken, AuthenticationError::IrretrievableState => Code::Internal, + AuthenticationError::MissingMasterKey => Code::MissingMasterKey, } } } diff --git a/meilisearch-http/src/extractors/authentication/mod.rs b/meilisearch-http/src/extractors/authentication/mod.rs index f6feabbbd..b9e2f711a 100644 --- a/meilisearch-http/src/extractors/authentication/mod.rs +++ b/meilisearch-http/src/extractors/authentication/mod.rs @@ -57,7 +57,7 @@ impl GuardedData { }), None => Err(AuthenticationError::IrretrievableState.into()), }, - None => Err(AuthenticationError::MissingAuthorizationHeader.into()), + None => Err(AuthenticationError::MissingMasterKey.into()), } } diff --git a/meilisearch-types/src/error.rs b/meilisearch-types/src/error.rs index 56ac65f9e..147207aec 100644 --- a/meilisearch-types/src/error.rs +++ b/meilisearch-types/src/error.rs @@ -144,6 +144,7 @@ pub enum Code { InvalidStore, InvalidToken, MissingAuthorizationHeader, + MissingMasterKey, NoSpaceLeftOnDevice, DumpNotFound, TaskNotFound, @@ -231,6 +232,9 @@ impl Code { MissingAuthorizationHeader => { ErrCode::authentication("missing_authorization_header", StatusCode::UNAUTHORIZED) } + MissingMasterKey => { + ErrCode::authentication("missing_master_key", StatusCode::UNAUTHORIZED) + } TaskNotFound => ErrCode::invalid("task_not_found", StatusCode::NOT_FOUND), DumpNotFound => ErrCode::invalid("dump_not_found", StatusCode::NOT_FOUND), NoSpaceLeftOnDevice => { From f0ecacb58d9eee1599679fbfe90ea41bc93c9086 Mon Sep 17 00:00:00 2001 From: vishalsodani Date: Tue, 25 Oct 2022 22:41:48 +0530 Subject: [PATCH 2/6] add implementation for no master key set and fix tests --- meilisearch-auth/src/lib.rs | 15 ++++++ .../src/extractors/authentication/mod.rs | 21 ++++++--- meilisearch-http/tests/auth/api_keys.rs | 46 +++++++++---------- 3 files changed, 53 insertions(+), 29 deletions(-) diff --git a/meilisearch-auth/src/lib.rs b/meilisearch-auth/src/lib.rs index 43183d4cf..d27d98b4d 100644 --- a/meilisearch-auth/src/lib.rs +++ b/meilisearch-auth/src/lib.rs @@ -173,13 +173,28 @@ impl AuthController { pub struct AuthFilter { pub search_rules: SearchRules, pub allow_index_creation: bool, + master_key_missing: bool, } +impl AuthFilter { + pub fn with_no_master_key() -> AuthFilter { + AuthFilter { + search_rules: SearchRules::default(), + allow_index_creation: true, + master_key_missing: true, + } + } + + pub fn is_missing_master_key(&self) -> bool { + self.master_key_missing + } +} impl Default for AuthFilter { fn default() -> Self { Self { search_rules: SearchRules::default(), allow_index_creation: true, + master_key_missing: false, } } } diff --git a/meilisearch-http/src/extractors/authentication/mod.rs b/meilisearch-http/src/extractors/authentication/mod.rs index b9e2f711a..18093b666 100644 --- a/meilisearch-http/src/extractors/authentication/mod.rs +++ b/meilisearch-http/src/extractors/authentication/mod.rs @@ -50,14 +50,20 @@ impl GuardedData { { match Self::authenticate(auth, String::new(), None).await? { Some(filters) => match data { - Some(data) => Ok(Self { - data, - filters, - _marker: PhantomData, - }), + Some(data) => { + if filters.is_missing_master_key() { + Err(AuthenticationError::MissingMasterKey.into()) + } else { + Ok(Self { + data, + filters, + _marker: PhantomData, + }) + } + } None => Err(AuthenticationError::IrretrievableState.into()), }, - None => Err(AuthenticationError::MissingMasterKey.into()), + None => Err(AuthenticationError::MissingAuthorizationHeader.into()), } } @@ -171,6 +177,9 @@ pub mod policies { token: &str, index: Option<&str>, ) -> Option { + if auth.get_master_key().is_none() && is_keys_action(A) { + return Some(AuthFilter::with_no_master_key()); + } // authenticate if token is the master key. // master key can only have access to keys routes. // if master key is None only keys routes are inaccessible. diff --git a/meilisearch-http/tests/auth/api_keys.rs b/meilisearch-http/tests/auth/api_keys.rs index 7fdf2f129..9223f4a6b 100644 --- a/meilisearch-http/tests/auth/api_keys.rs +++ b/meilisearch-http/tests/auth/api_keys.rs @@ -1400,13 +1400,13 @@ async fn error_patch_api_key_indexes_invalid_parameters() { #[actix_rt::test] async fn error_access_api_key_routes_no_master_key_set() { - let mut server = Server::new().await; + let server = Server::new().await; let expected_response = json!({ - "message": "The Authorization header is missing. It must use the bearer authorization method.", - "code": "missing_authorization_header", + "message": "Meilisearch is running without a master key. To access this API endpoint, you must have set a master key at launch.", + "code": "missing_master_key", "type": "auth", - "link": "https://docs.meilisearch.com/errors#missing_authorization_header" + "link": "https://docs.meilisearch.com/errors#missing_master_key" }); let expected_code = 401; @@ -1430,32 +1430,32 @@ async fn error_access_api_key_routes_no_master_key_set() { assert_eq!(expected_code, code, "{:?}", &response); assert_eq!(response, expected_response); - server.use_api_key("MASTER_KEY"); + // server.use_api_key("MASTER_KEY"); - let expected_response = json!({"message": "The provided API key is invalid.", - "code": "invalid_api_key", - "type": "auth", - "link": "https://docs.meilisearch.com/errors#invalid_api_key" - }); - let expected_code = 403; + // let expected_response = json!({"message": "The provided API key is invalid.", + // "code": "invalid_api_key", + // "type": "auth", + // "link": "https://docs.meilisearch.com/errors#invalid_api_key" + // }); + // let expected_code = 403; - let (response, code) = server.add_api_key(json!({})).await; + // let (response, code) = server.add_api_key(json!({})).await; - assert_eq!(expected_code, code, "{:?}", &response); - assert_eq!(response, expected_response); + // assert_eq!(expected_code, code, "{:?}", &response); + // assert_eq!(response, expected_response); - let (response, code) = server.patch_api_key("content", json!({})).await; + // let (response, code) = server.patch_api_key("content", json!({})).await; - assert_eq!(expected_code, code, "{:?}", &response); - assert_eq!(response, expected_response); + // assert_eq!(expected_code, code, "{:?}", &response); + // assert_eq!(response, expected_response); - let (response, code) = server.get_api_key("content").await; + // let (response, code) = server.get_api_key("content").await; - assert_eq!(expected_code, code, "{:?}", &response); - assert_eq!(response, expected_response); + // assert_eq!(expected_code, code, "{:?}", &response); + // assert_eq!(response, expected_response); - let (response, code) = server.list_api_keys().await; + // let (response, code) = server.list_api_keys().await; - assert_eq!(expected_code, code, "{:?}", &response); - assert_eq!(response, expected_response); + // assert_eq!(expected_code, code, "{:?}", &response); + // assert_eq!(response, expected_response); } From 9cf3ff72a3c2debf0af09438cf4cbf7edc181e0b Mon Sep 17 00:00:00 2001 From: vishalsodani Date: Thu, 27 Oct 2022 12:56:18 +0530 Subject: [PATCH 3/6] fix checking of master key as per review comment --- meilisearch-auth/src/lib.rs | 15 ---------- .../src/extractors/authentication/mod.rs | 28 +++++++++---------- 2 files changed, 13 insertions(+), 30 deletions(-) diff --git a/meilisearch-auth/src/lib.rs b/meilisearch-auth/src/lib.rs index d27d98b4d..43183d4cf 100644 --- a/meilisearch-auth/src/lib.rs +++ b/meilisearch-auth/src/lib.rs @@ -173,28 +173,13 @@ impl AuthController { pub struct AuthFilter { pub search_rules: SearchRules, pub allow_index_creation: bool, - master_key_missing: bool, } -impl AuthFilter { - pub fn with_no_master_key() -> AuthFilter { - AuthFilter { - search_rules: SearchRules::default(), - allow_index_creation: true, - master_key_missing: true, - } - } - - pub fn is_missing_master_key(&self) -> bool { - self.master_key_missing - } -} impl Default for AuthFilter { fn default() -> Self { Self { search_rules: SearchRules::default(), allow_index_creation: true, - master_key_missing: false, } } } diff --git a/meilisearch-http/src/extractors/authentication/mod.rs b/meilisearch-http/src/extractors/authentication/mod.rs index 18093b666..7497d6377 100644 --- a/meilisearch-http/src/extractors/authentication/mod.rs +++ b/meilisearch-http/src/extractors/authentication/mod.rs @@ -48,22 +48,23 @@ impl GuardedData { where P: Policy + 'static, { + let auth_clone = auth.clone(); + let master_key: Option<&String> = auth_clone.get_master_key(); + match Self::authenticate(auth, String::new(), None).await? { Some(filters) => match data { - Some(data) => { - if filters.is_missing_master_key() { - Err(AuthenticationError::MissingMasterKey.into()) - } else { - Ok(Self { - data, - filters, - _marker: PhantomData, - }) - } - } + Some(data) => Ok(Self { + data, + filters, + _marker: PhantomData, + }), + None => Err(AuthenticationError::IrretrievableState.into()), }, - None => Err(AuthenticationError::MissingAuthorizationHeader.into()), + None => match master_key { + Some(_) => Err(AuthenticationError::MissingAuthorizationHeader.into()), + None => Err(AuthenticationError::MissingMasterKey.into()), + }, } } @@ -177,9 +178,6 @@ pub mod policies { token: &str, index: Option<&str>, ) -> Option { - if auth.get_master_key().is_none() && is_keys_action(A) { - return Some(AuthFilter::with_no_master_key()); - } // authenticate if token is the master key. // master key can only have access to keys routes. // if master key is None only keys routes are inaccessible. From 03ba830ab20e46acdbdd3186d5621cddd7c62cad Mon Sep 17 00:00:00 2001 From: vishalsodani Date: Thu, 27 Oct 2022 12:59:28 +0530 Subject: [PATCH 4/6] uncomment tests --- meilisearch-http/tests/auth/api_keys.rs | 40 ++++++++++++------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/meilisearch-http/tests/auth/api_keys.rs b/meilisearch-http/tests/auth/api_keys.rs index 9223f4a6b..1ea6ebb94 100644 --- a/meilisearch-http/tests/auth/api_keys.rs +++ b/meilisearch-http/tests/auth/api_keys.rs @@ -1400,7 +1400,7 @@ async fn error_patch_api_key_indexes_invalid_parameters() { #[actix_rt::test] async fn error_access_api_key_routes_no_master_key_set() { - let server = Server::new().await; + let mut server = Server::new().await; let expected_response = json!({ "message": "Meilisearch is running without a master key. To access this API endpoint, you must have set a master key at launch.", @@ -1430,32 +1430,32 @@ async fn error_access_api_key_routes_no_master_key_set() { assert_eq!(expected_code, code, "{:?}", &response); assert_eq!(response, expected_response); - // server.use_api_key("MASTER_KEY"); + server.use_api_key("MASTER_KEY"); - // let expected_response = json!({"message": "The provided API key is invalid.", - // "code": "invalid_api_key", - // "type": "auth", - // "link": "https://docs.meilisearch.com/errors#invalid_api_key" - // }); - // let expected_code = 403; + let expected_response = json!({"message": "The provided API key is invalid.", + "code": "invalid_api_key", + "type": "auth", + "link": "https://docs.meilisearch.com/errors#invalid_api_key" + }); + let expected_code = 403; - // let (response, code) = server.add_api_key(json!({})).await; + let (response, code) = server.add_api_key(json!({})).await; - // assert_eq!(expected_code, code, "{:?}", &response); - // assert_eq!(response, expected_response); + assert_eq!(expected_code, code, "{:?}", &response); + assert_eq!(response, expected_response); - // let (response, code) = server.patch_api_key("content", json!({})).await; + let (response, code) = server.patch_api_key("content", json!({})).await; - // assert_eq!(expected_code, code, "{:?}", &response); - // assert_eq!(response, expected_response); + assert_eq!(expected_code, code, "{:?}", &response); + assert_eq!(response, expected_response); - // let (response, code) = server.get_api_key("content").await; + let (response, code) = server.get_api_key("content").await; - // assert_eq!(expected_code, code, "{:?}", &response); - // assert_eq!(response, expected_response); + assert_eq!(expected_code, code, "{:?}", &response); + assert_eq!(response, expected_response); - // let (response, code) = server.list_api_keys().await; + let (response, code) = server.list_api_keys().await; - // assert_eq!(expected_code, code, "{:?}", &response); - // assert_eq!(response, expected_response); + assert_eq!(expected_code, code, "{:?}", &response); + assert_eq!(response, expected_response); } From 89c40c83c303ac3ebce9dd79eaea2a84b9342917 Mon Sep 17 00:00:00 2001 From: vishalsodani Date: Thu, 27 Oct 2022 14:08:29 +0530 Subject: [PATCH 5/6] refactor code to avoid cloning --- meilisearch-http/src/extractors/authentication/mod.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/meilisearch-http/src/extractors/authentication/mod.rs b/meilisearch-http/src/extractors/authentication/mod.rs index 7497d6377..d36db02c8 100644 --- a/meilisearch-http/src/extractors/authentication/mod.rs +++ b/meilisearch-http/src/extractors/authentication/mod.rs @@ -48,8 +48,7 @@ impl GuardedData { where P: Policy + 'static, { - let auth_clone = auth.clone(); - let master_key: Option<&String> = auth_clone.get_master_key(); + let missing_master_key = auth.get_master_key().is_none(); match Self::authenticate(auth, String::new(), None).await? { Some(filters) => match data { @@ -61,10 +60,10 @@ impl GuardedData { None => Err(AuthenticationError::IrretrievableState.into()), }, - None => match master_key { - Some(_) => Err(AuthenticationError::MissingAuthorizationHeader.into()), - None => Err(AuthenticationError::MissingMasterKey.into()), - }, + None if missing_master_key => { + Err(AuthenticationError::MissingMasterKey.into()) + } + None => Err(AuthenticationError::MissingAuthorizationHeader.into()), } } From 689bef7ad2d364db0bbd4652aee88a2b5ad980c5 Mon Sep 17 00:00:00 2001 From: vishalsodani Date: Thu, 27 Oct 2022 14:09:38 +0530 Subject: [PATCH 6/6] fmt the code --- meilisearch-http/src/extractors/authentication/mod.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/meilisearch-http/src/extractors/authentication/mod.rs b/meilisearch-http/src/extractors/authentication/mod.rs index d36db02c8..4107a6194 100644 --- a/meilisearch-http/src/extractors/authentication/mod.rs +++ b/meilisearch-http/src/extractors/authentication/mod.rs @@ -60,9 +60,7 @@ impl GuardedData { None => Err(AuthenticationError::IrretrievableState.into()), }, - None if missing_master_key => { - Err(AuthenticationError::MissingMasterKey.into()) - } + None if missing_master_key => Err(AuthenticationError::MissingMasterKey.into()), None => Err(AuthenticationError::MissingAuthorizationHeader.into()), } }