From c8c59440949c3787b27e32cccbb94fe366c4e422 Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Mon, 20 Feb 2023 09:25:29 +0100 Subject: [PATCH] Authentication: is_index_authorized takes into account API key indexes even with a tenant token --- meilisearch-auth/src/lib.rs | 43 ++++++++++++++++--- .../src/analytics/segment_analytics.rs | 4 +- meilisearch/src/routes/indexes/mod.rs | 7 +-- meilisearch/src/routes/indexes/search.rs | 8 +--- meilisearch/src/routes/mod.rs | 11 +++-- meilisearch/src/routes/swap_indexes.rs | 4 +- meilisearch/src/routes/tasks.rs | 14 ++---- 7 files changed, 56 insertions(+), 35 deletions(-) diff --git a/meilisearch-auth/src/lib.rs b/meilisearch-auth/src/lib.rs index 9f9b15f38..a7a350b40 100644 --- a/meilisearch-auth/src/lib.rs +++ b/meilisearch-auth/src/lib.rs @@ -88,9 +88,11 @@ impl AuthController { let mut filters = AuthFilter::default(); let key = self.get_key(uid)?; + filters.key_authorized_indexes = SearchRules::Set(key.indexes.into_iter().collect()); + filters.search_rules = match search_rules { Some(search_rules) => search_rules, - None => SearchRules::Set(key.indexes.into_iter().collect()), + None => filters.key_authorized_indexes.clone(), }; filters.allow_index_creation = self.is_key_authorized(uid, Action::IndexesAdd, None)?; @@ -160,13 +162,42 @@ impl AuthController { } pub struct AuthFilter { - pub search_rules: SearchRules, + search_rules: SearchRules, + key_authorized_indexes: SearchRules, pub allow_index_creation: bool, } impl Default for AuthFilter { fn default() -> Self { - Self { search_rules: SearchRules::default(), allow_index_creation: true } + Self { + search_rules: SearchRules::default(), + key_authorized_indexes: SearchRules::default(), + allow_index_creation: true, + } + } +} + +impl AuthFilter { + pub fn is_index_authorized(&self, index: &str) -> bool { + self.key_authorized_indexes.is_index_authorized(index) + && self.search_rules.is_index_authorized(index) + } + + pub fn get_index_search_rules(&self, index: &str) -> Option { + if !self.is_index_authorized(index) { + return None; + } + self.search_rules.get_index_search_rules(index) + } + + /// Return the list of indexes such that `self.is_index_authorized(index) == true`, + /// or `None` if all indexes satisfy this condition. + /// + /// FIXME: this works only when there are no tenant tokens, otherwise it ignores the rules of the API key. + /// + /// It is better to use `is_index_authorized` when possible. + pub fn authorized_indexes(&self) -> Option> { + self.search_rules.authorized_indexes() } } @@ -185,7 +216,7 @@ impl Default for SearchRules { } impl SearchRules { - pub fn is_index_authorized(&self, index: &str) -> bool { + fn is_index_authorized(&self, index: &str) -> bool { match self { Self::Set(set) => { set.contains("*") @@ -200,7 +231,7 @@ impl SearchRules { } } - pub fn get_index_search_rules(&self, index: &str) -> Option { + fn get_index_search_rules(&self, index: &str) -> Option { match self { Self::Set(_) => { if self.is_index_authorized(index) { @@ -221,7 +252,7 @@ impl SearchRules { /// 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> { + fn authorized_indexes(&self) -> Option> { match self { SearchRules::Set(set) => { if set.contains("*") { diff --git a/meilisearch/src/analytics/segment_analytics.rs b/meilisearch/src/analytics/segment_analytics.rs index 818b06240..4d4b7cd06 100644 --- a/meilisearch/src/analytics/segment_analytics.rs +++ b/meilisearch/src/analytics/segment_analytics.rs @@ -9,7 +9,7 @@ use actix_web::HttpRequest; use byte_unit::Byte; use http::header::CONTENT_TYPE; use index_scheduler::IndexScheduler; -use meilisearch_auth::{AuthController, SearchRules}; +use meilisearch_auth::{AuthController, AuthFilter}; use meilisearch_types::InstanceUid; use once_cell::sync::Lazy; use regex::Regex; @@ -401,7 +401,7 @@ impl Segment { auth_controller: AuthController, ) { if let Ok(stats) = - create_all_stats(index_scheduler.into(), auth_controller, &SearchRules::default()) + create_all_stats(index_scheduler.into(), auth_controller, &AuthFilter::default()) { // Replace the version number with the prototype name if any. let version = if let Some(prototype) = crate::prototype_name() { diff --git a/meilisearch/src/routes/indexes/mod.rs b/meilisearch/src/routes/indexes/mod.rs index b58bef0e9..ee0a9dea0 100644 --- a/meilisearch/src/routes/indexes/mod.rs +++ b/meilisearch/src/routes/indexes/mod.rs @@ -89,11 +89,11 @@ pub async fn list_indexes( index_scheduler: GuardedData, Data>, paginate: AwebQueryParameter, ) -> Result { - let search_rules = &index_scheduler.filters().search_rules; + let filters = index_scheduler.filters(); let indexes: Vec<_> = index_scheduler.indexes()?; let indexes = indexes .into_iter() - .filter(|(name, _)| search_rules.is_index_authorized(name)) + .filter(|(name, _)| filters.is_index_authorized(name)) .map(|(name, index)| IndexView::new(name, &index)) .collect::, _>>()?; @@ -120,7 +120,8 @@ pub async fn create_index( ) -> Result { let IndexCreateRequest { primary_key, uid } = body.into_inner(); - let allow_index_creation = index_scheduler.filters().search_rules.is_index_authorized(&uid); + // FIXME: allow_index_creation? + let allow_index_creation = index_scheduler.filters().is_index_authorized(&uid); if allow_index_creation { analytics.publish( "Index Created".to_string(), diff --git a/meilisearch/src/routes/indexes/search.rs b/meilisearch/src/routes/indexes/search.rs index 3fb413c43..eef092d02 100644 --- a/meilisearch/src/routes/indexes/search.rs +++ b/meilisearch/src/routes/indexes/search.rs @@ -159,9 +159,7 @@ pub async fn search_with_url_query( let mut query: SearchQuery = params.into_inner().into(); // Tenant token search_rules. - if let Some(search_rules) = - index_scheduler.filters().search_rules.get_index_search_rules(&index_uid) - { + if let Some(search_rules) = index_scheduler.filters().get_index_search_rules(&index_uid) { add_search_rules(&mut query, search_rules); } @@ -193,9 +191,7 @@ pub async fn search_with_post( debug!("search called with params: {:?}", query); // Tenant token search_rules. - if let Some(search_rules) = - index_scheduler.filters().search_rules.get_index_search_rules(&index_uid) - { + if let Some(search_rules) = index_scheduler.filters().get_index_search_rules(&index_uid) { add_search_rules(&mut query, search_rules); } diff --git a/meilisearch/src/routes/mod.rs b/meilisearch/src/routes/mod.rs index 0f367c9f4..20b9f73f9 100644 --- a/meilisearch/src/routes/mod.rs +++ b/meilisearch/src/routes/mod.rs @@ -237,10 +237,9 @@ async fn get_stats( analytics: web::Data, ) -> Result { analytics.publish("Stats Seen".to_string(), json!({ "per_index_uid": false }), Some(&req)); - let search_rules = &index_scheduler.filters().search_rules; + let filters = index_scheduler.filters(); - let stats = - create_all_stats((*index_scheduler).clone(), (*auth_controller).clone(), search_rules)?; + let stats = create_all_stats((*index_scheduler).clone(), (*auth_controller).clone(), filters)?; debug!("returns: {:?}", stats); Ok(HttpResponse::Ok().json(stats)) @@ -249,19 +248,19 @@ async fn get_stats( pub fn create_all_stats( index_scheduler: Data, auth_controller: AuthController, - search_rules: &meilisearch_auth::SearchRules, + filters: &meilisearch_auth::AuthFilter, ) -> Result { let mut last_task: Option = None; let mut indexes = BTreeMap::new(); let mut database_size = 0; let processing_task = index_scheduler.get_tasks_from_authorized_indexes( Query { statuses: Some(vec![Status::Processing]), limit: Some(1), ..Query::default() }, - search_rules.authorized_indexes(), + filters.authorized_indexes(), )?; // accumulate the size of each indexes let processing_index = processing_task.first().and_then(|task| task.index_uid()); for (name, index) in index_scheduler.indexes()? { - if !search_rules.is_index_authorized(&name) { + if !filters.is_index_authorized(&name) { continue; } diff --git a/meilisearch/src/routes/swap_indexes.rs b/meilisearch/src/routes/swap_indexes.rs index c4177e900..c4e204c09 100644 --- a/meilisearch/src/routes/swap_indexes.rs +++ b/meilisearch/src/routes/swap_indexes.rs @@ -42,7 +42,7 @@ pub async fn swap_indexes( }), Some(&req), ); - let search_rules = &index_scheduler.filters().search_rules; + let filters = index_scheduler.filters(); let mut swaps = vec![]; for SwapIndexesPayload { indexes } in params.into_iter() { @@ -53,7 +53,7 @@ pub async fn swap_indexes( return Err(MeilisearchHttpError::SwapIndexPayloadWrongLength(indexes).into()); } }; - if !search_rules.is_index_authorized(lhs) || !search_rules.is_index_authorized(rhs) { + if !filters.is_index_authorized(lhs) || !filters.is_index_authorized(rhs) { return Err(AuthenticationError::InvalidToken.into()); } swaps.push(IndexSwap { indexes: (lhs.to_string(), rhs.to_string()) }); diff --git a/meilisearch/src/routes/tasks.rs b/meilisearch/src/routes/tasks.rs index 49f3aac66..e0252317b 100644 --- a/meilisearch/src/routes/tasks.rs +++ b/meilisearch/src/routes/tasks.rs @@ -319,7 +319,7 @@ async fn cancel_tasks( let tasks = index_scheduler.get_task_ids_from_authorized_indexes( &index_scheduler.read_txn()?, &query, - &index_scheduler.filters().search_rules.authorized_indexes(), + &index_scheduler.filters().authorized_indexes(), )?; let task_cancelation = KindWithContent::TaskCancelation { query: format!("?{}", req.query_string()), tasks }; @@ -364,7 +364,7 @@ async fn delete_tasks( let tasks = index_scheduler.get_task_ids_from_authorized_indexes( &index_scheduler.read_txn()?, &query, - &index_scheduler.filters().search_rules.authorized_indexes(), + &index_scheduler.filters().authorized_indexes(), )?; let task_deletion = KindWithContent::TaskDeletion { query: format!("?{}", req.query_string()), tasks }; @@ -398,10 +398,7 @@ async fn get_tasks( let query = params.into_query(); let mut tasks_results: Vec = index_scheduler - .get_tasks_from_authorized_indexes( - query, - index_scheduler.filters().search_rules.authorized_indexes(), - )? + .get_tasks_from_authorized_indexes(query, index_scheduler.filters().authorized_indexes())? .into_iter() .map(|t| TaskView::from_task(&t)) .collect(); @@ -440,10 +437,7 @@ async fn get_task( let query = index_scheduler::Query { uids: Some(vec![task_uid]), ..Query::default() }; if let Some(task) = index_scheduler - .get_tasks_from_authorized_indexes( - query, - index_scheduler.filters().search_rules.authorized_indexes(), - )? + .get_tasks_from_authorized_indexes(query, index_scheduler.filters().authorized_indexes())? .first() { let task_view = TaskView::from_task(task);