From a74fb87d1e3600b55c5e5db90b7fe857ef252ac1 Mon Sep 17 00:00:00 2001 From: Tamo Date: Mon, 24 Jun 2024 19:00:53 +0200 Subject: [PATCH] start introducing new error messages --- .../src/extractors/authentication/mod.rs | 109 ++++++++------ meilisearch/tests/auth/errors.rs | 134 +++++++++++++++++- 2 files changed, 200 insertions(+), 43 deletions(-) diff --git a/meilisearch/src/extractors/authentication/mod.rs b/meilisearch/src/extractors/authentication/mod.rs index 007e2be40..c3c38c27f 100644 --- a/meilisearch/src/extractors/authentication/mod.rs +++ b/meilisearch/src/extractors/authentication/mod.rs @@ -12,6 +12,8 @@ use futures::Future; use meilisearch_auth::{AuthController, AuthFilter}; use meilisearch_types::error::{Code, ResponseError}; +use self::policies::AuthError; + pub struct GuardedData { data: D, filters: AuthFilter, @@ -35,12 +37,12 @@ impl GuardedData { let missing_master_key = auth.get_master_key().is_none(); match Self::authenticate(auth, token, index).await? { - Some(filters) => match data { + Ok(filters) => match data { Some(data) => Ok(Self { data, filters, _marker: PhantomData }), None => Err(AuthenticationError::IrretrievableState.into()), }, - None if missing_master_key => Err(AuthenticationError::MissingMasterKey.into()), - None => Err(AuthenticationError::InvalidToken.into()), + Err(_) if missing_master_key => Err(AuthenticationError::MissingMasterKey.into()), + Err(e) => Err(ResponseError::from_msg(e.to_string(), Code::InvalidApiKey)), } } @@ -51,12 +53,12 @@ impl GuardedData { let missing_master_key = auth.get_master_key().is_none(); match Self::authenticate(auth, String::new(), None).await? { - Some(filters) => match data { + Ok(filters) => match data { Some(data) => Ok(Self { data, filters, _marker: PhantomData }), None => Err(AuthenticationError::IrretrievableState.into()), }, - None if missing_master_key => Err(AuthenticationError::MissingMasterKey.into()), - None => Err(AuthenticationError::MissingAuthorizationHeader.into()), + Err(_) if missing_master_key => Err(AuthenticationError::MissingMasterKey.into()), + Err(_) => Err(AuthenticationError::MissingAuthorizationHeader.into()), } } @@ -64,7 +66,7 @@ impl GuardedData { auth: Data, token: String, index: Option, - ) -> Result, ResponseError> + ) -> Result, ResponseError> where P: Policy + 'static, { @@ -127,7 +129,7 @@ pub trait Policy { auth: Data, token: &str, index: Option<&str>, - ) -> Option; + ) -> Result; } pub mod policies { @@ -144,11 +146,32 @@ pub mod policies { enum TenantTokenOutcome { NotATenantToken, - Invalid, - Expired, Valid(Uuid, SearchRules), } + #[derive(thiserror::Error, Debug)] + pub enum AuthError { + #[error("Tenant token expired. Was valid up to `{exp}` and we're now `{now}`")] + ExpiredTenantToken { exp: i64, now: i64 }, + #[error("The provided API key is invalid.")] + InvalidApiKey, + #[error("The provided tenant token is invalid.")] + InvalidTenantToken, + #[error("Could not decode tenant token, {0}")] + CouldNotDecodeTenantToken(jsonwebtoken::errors::Error), + } + + impl From for AuthError { + fn from(error: jsonwebtoken::errors::Error) -> Self { + use jsonwebtoken::errors::ErrorKind; + + match error.kind() { + ErrorKind::InvalidToken => AuthError::InvalidTenantToken, + _ => AuthError::CouldNotDecodeTenantToken(error), + } + } + } + fn tenant_token_validation() -> Validation { let mut validation = Validation::default(); validation.validate_exp = false; @@ -158,15 +181,15 @@ pub mod policies { } /// Extracts the key id used to sign the payload, without performing any validation. - fn extract_key_id(token: &str) -> Option { + fn extract_key_id(token: &str) -> Result { let mut validation = tenant_token_validation(); validation.insecure_disable_signature_validation(); let dummy_key = DecodingKey::from_secret(b"secret"); - let token_data = decode::(token, &dummy_key, &validation).ok()?; + let token_data = decode::(token, &dummy_key, &validation)?; // get token fields without validating it. let Claims { api_key_uid, .. } = token_data.claims; - Some(api_key_uid) + Ok(api_key_uid) } fn is_keys_action(action: u8) -> bool { @@ -187,76 +210,78 @@ pub mod policies { auth: Data, token: &str, index: Option<&str>, - ) -> Option { + ) -> Result { // authenticate if token is the master key. // Without a master key, all routes are accessible except the key-related routes. if auth.get_master_key().map_or_else(|| !is_keys_action(A), |mk| mk == token) { - return Some(AuthFilter::default()); + return Ok(AuthFilter::default()); } let (key_uuid, search_rules) = match ActionPolicy::::authenticate_tenant_token(&auth, token) { - TenantTokenOutcome::Valid(key_uuid, search_rules) => { + Ok(TenantTokenOutcome::Valid(key_uuid, search_rules)) => { (key_uuid, Some(search_rules)) } - TenantTokenOutcome::Expired => return None, - TenantTokenOutcome::Invalid => return None, - TenantTokenOutcome::NotATenantToken => { - (auth.get_optional_uid_from_encoded_key(token.as_bytes()).ok()??, None) - } + Ok(TenantTokenOutcome::NotATenantToken) + | Err(AuthError::InvalidTenantToken) => ( + auth.get_optional_uid_from_encoded_key(token.as_bytes()) + .map_err(|_e| AuthError::InvalidApiKey)? + .ok_or(AuthError::InvalidApiKey)?, + None, + ), + Err(e) => return Err(e), }; // check that the indexes are allowed - let action = Action::from_repr(A)?; - let auth_filter = auth.get_key_filters(key_uuid, search_rules).ok()?; + let action = Action::from_repr(A).ok_or(AuthError::InvalidTenantToken)?; + let auth_filter = auth + .get_key_filters(key_uuid, search_rules) + .map_err(|_e| AuthError::InvalidTenantToken)?; if auth.is_key_authorized(key_uuid, action, index).unwrap_or(false) && index.map(|index| auth_filter.is_index_authorized(index)).unwrap_or(true) { - return Some(auth_filter); + return Ok(auth_filter); } - None + Err(AuthError::InvalidApiKey) } } impl ActionPolicy { - fn authenticate_tenant_token(auth: &AuthController, token: &str) -> TenantTokenOutcome { + fn authenticate_tenant_token( + auth: &AuthController, + token: &str, + ) -> Result { // Only search action can be accessed by a tenant token. if A != actions::SEARCH { - return TenantTokenOutcome::NotATenantToken; + return Ok(TenantTokenOutcome::NotATenantToken); } - let uid = if let Some(uid) = extract_key_id(token) { - uid - } else { - return TenantTokenOutcome::NotATenantToken; - }; + let uid = extract_key_id(token)?; // Check if tenant token is valid. let key = if let Some(key) = auth.generate_key(uid) { key } else { - return TenantTokenOutcome::Invalid; + /// Only happens when no master key has been set + return Err(AuthError::InvalidTenantToken); }; - let data = if let Ok(data) = decode::( + let data = decode::( token, &DecodingKey::from_secret(key.as_bytes()), &tenant_token_validation(), - ) { - data - } else { - return TenantTokenOutcome::Invalid; - }; + )?; // Check if token is expired. if let Some(exp) = data.claims.exp { - if OffsetDateTime::now_utc().unix_timestamp() > exp { - return TenantTokenOutcome::Expired; + let now = OffsetDateTime::now_utc().unix_timestamp(); + if now > exp { + return Err(AuthError::ExpiredTenantToken { exp, now }); } } - TenantTokenOutcome::Valid(uid, data.claims.search_rules) + Ok(TenantTokenOutcome::Valid(uid, data.claims.search_rules)) } } diff --git a/meilisearch/tests/auth/errors.rs b/meilisearch/tests/auth/errors.rs index 581243a0a..4c65fce98 100644 --- a/meilisearch/tests/auth/errors.rs +++ b/meilisearch/tests/auth/errors.rs @@ -1,7 +1,10 @@ +use actix_web::test; +use http::StatusCode; +use jsonwebtoken::{EncodingKey, Header}; use meili_snap::*; use uuid::Uuid; -use crate::common::Server; +use crate::common::{Server, Value}; use crate::json; #[actix_rt::test] @@ -436,3 +439,132 @@ async fn patch_api_keys_unknown_field() { } "###); } + +async fn send_request_with_custom_auth( + app: impl actix_web::dev::Service< + actix_http::Request, + Response = actix_web::dev::ServiceResponse, + Error = actix_web::Error, + >, + url: &str, + auth: &str, +) -> (Value, StatusCode) { + let req = test::TestRequest::get().uri(url).insert_header(("Authorization", auth)).to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + let body = test::read_body(res).await; + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + + (response, status_code) +} + +#[actix_rt::test] +async fn invalid_auth_format() { + let server = Server::new_auth().await; + let app = server.init_web_app().await; + + let req = test::TestRequest::get().uri("/indexes/dog/documents").to_request(); + let res = test::call_service(&app, req).await; + let status_code = res.status(); + let body = test::read_body(res).await; + let response: Value = serde_json::from_slice(&body).unwrap_or_default(); + snapshot!(status_code, @"401 Unauthorized"); + snapshot!(response, @r###" + { + "message": "The Authorization header is missing. It must use the bearer authorization method.", + "code": "missing_authorization_header", + "type": "auth", + "link": "https://docs.meilisearch.com/errors#missing_authorization_header" + } + "###); + + let (response, status_code) = + send_request_with_custom_auth(&app, "/indexes/dog/documents", "Bearer").await; + snapshot!(status_code, @"403 Forbidden"); + snapshot!(response, @r###" + { + "message": "The provided API key is invalid.", + "code": "invalid_api_key", + "type": "auth", + "link": "https://docs.meilisearch.com/errors#invalid_api_key" + } + "###); + + let (response, status_code) = + send_request_with_custom_auth(&app, "/indexes/dog/documents", "Bearer kefir").await; + snapshot!(status_code, @"403 Forbidden"); + snapshot!(response, @r###" + { + "message": "The provided API key is invalid.", + "code": "invalid_api_key", + "type": "auth", + "link": "https://docs.meilisearch.com/errors#invalid_api_key" + } + "###); + + // The tenant token won't be recognized at all if we're not on a search route + let claims = json!({ "tamo": "kefir" }); + let jwt = jsonwebtoken::encode(&Header::default(), &claims, &EncodingKey::from_secret(b"tamo")) + .unwrap(); + let (response, status_code) = + send_request_with_custom_auth(&app, "/indexes/dog/documents", &format!("Bearer {jwt}")) + .await; + snapshot!(status_code, @"403 Forbidden"); + snapshot!(response, @r###" + { + "message": "The provided API key is invalid.", + "code": "invalid_api_key", + "type": "auth", + "link": "https://docs.meilisearch.com/errors#invalid_api_key" + } + "###); + + let claims = json!({ "tamo": "kefir" }); + let jwt = jsonwebtoken::encode(&Header::default(), &claims, &EncodingKey::from_secret(b"tamo")) + .unwrap(); + let (response, status_code) = + send_request_with_custom_auth(&app, "/indexes/dog/search", &format!("Bearer {jwt}")).await; + snapshot!(status_code, @"403 Forbidden"); + snapshot!(response, @r###" + { + "message": "Could not decode tenant token, JSON error: missing field `searchRules` at line 1 column 16", + "code": "invalid_api_key", + "type": "auth", + "link": "https://docs.meilisearch.com/errors#invalid_api_key" + } + "###); + + // The error messages are not ideal but that's expected since we cannot _yet_ use deserr + let claims = json!({ "searchRules": "kefir" }); + let jwt = jsonwebtoken::encode(&Header::default(), &claims, &EncodingKey::from_secret(b"tamo")) + .unwrap(); + let (response, status_code) = + send_request_with_custom_auth(&app, "/indexes/dog/search", &format!("Bearer {jwt}")).await; + snapshot!(status_code, @"403 Forbidden"); + snapshot!(response, @r###" + { + "message": "Could not decode tenant token, JSON error: data did not match any variant of untagged enum SearchRules at line 1 column 23", + "code": "invalid_api_key", + "type": "auth", + "link": "https://docs.meilisearch.com/errors#invalid_api_key" + } + "###); + + let uuid = Uuid::nil(); + let claims = json!({ "searchRules": ["kefir"], "apiKeyUid": uuid.to_string() }); + let jwt = jsonwebtoken::encode(&Header::default(), &claims, &EncodingKey::from_secret(b"tamo")) + .unwrap(); + let (response, status_code) = + send_request_with_custom_auth(&app, "/indexes/dog/search", &format!("Bearer {jwt}")).await; + snapshot!(status_code, @"403 Forbidden"); + snapshot!(response, @r###" + { + "message": "Could not decode tenant token, InvalidSignature", + "code": "invalid_api_key", + "type": "auth", + "link": "https://docs.meilisearch.com/errors#invalid_api_key" + } + "###); + + // ~~ For the next tests we first need to retrieve an API key +}