diff --git a/bin/core/src/auth/jwt.rs b/bin/core/src/auth/jwt.rs index 4ddd33cbd..5252fda26 100644 --- a/bin/core/src/auth/jwt.rs +++ b/bin/core/src/auth/jwt.rs @@ -64,14 +64,14 @@ impl JwtClient { exp, }; let jwt = encode(&self.header, &claims, &self.encoding_key) - .context("failed at signing claim")?; + .context("Failed at signing claim")?; Ok(JwtResponse { user_id, jwt }) } pub fn decode(&self, jwt: &str) -> anyhow::Result { decode::(jwt, &self.decoding_key, &self.validation) .map(|res| res.claims) - .context("failed to decode token claims") + .context("Failed to decode token claims") } pub async fn create_exchange_token( diff --git a/bin/core/src/auth/local.rs b/bin/core/src/auth/local.rs index 227f84de7..d5668938e 100644 --- a/bin/core/src/auth/local.rs +++ b/bin/core/src/auth/local.rs @@ -14,7 +14,7 @@ use komodo_client::{ use rate_limit::WithFailureRateLimit; use reqwest::StatusCode; use resolver_api::Resolve; -use serror::AddStatusCode as _; +use serror::{AddStatusCode as _, AddStatusCodeError}; use crate::{ api::auth::AuthArgs, @@ -58,7 +58,10 @@ async fn sign_up_local_user( db.users.find_one(Document::new()).await?.is_none(); if !no_users_exist && config.disable_user_registration { - return Err(anyhow!("User registration is disabled").into()); + return Err( + anyhow!("User registration is disabled") + .status_code(StatusCode::UNAUTHORIZED), + ); } if db @@ -68,7 +71,14 @@ async fn sign_up_local_user( .context("Failed to query for existing users")? .is_some() { - return Err(anyhow!("Username already taken.").into()); + // When user registration is enabled, there is no way around allowing + // potential attackers to gain some insight about which usernames exist + // if they are allowed to register accounts. Since this can be easily inferred, + // might as well be clear. The auth rate limiter is critical here. + return Err( + anyhow!("Username already taken.") + .status_code(StatusCode::BAD_REQUEST), + ); } let ts = unix_timestamp_ms() as i64; @@ -125,7 +135,10 @@ async fn login_local_user( req: LoginLocalUser, ) -> serror::Result { if !core_config().local_auth { - return Err(anyhow!("Local auth is not enabled").into()); + return Err( + anyhow!("Local auth is not enabled") + .status_code(StatusCode::UNAUTHORIZED), + ); } validate_username(&req.username) @@ -135,30 +148,34 @@ async fn login_local_user( .users .find_one(doc! { "username": &req.username }) .await - .context("failed at db query for users")? - .with_context(|| { - format!("did not find user with username {}", req.username) - })?; + .context("Failed at db query for users")? + .context("Invalid login credentials") + .status_code(StatusCode::UNAUTHORIZED)?; let UserConfig::Local { password: user_pw_hash, } = user.config else { return Err( - anyhow!("Non-local auth users can not log in with a password") - .into(), + anyhow!("Invalid login credentials") + .status_code(StatusCode::UNAUTHORIZED), ); }; let verified = bcrypt::verify(req.password, &user_pw_hash) - .context("failed at verify password")?; + .context("Invalid login credentials") + .status_code(StatusCode::UNAUTHORIZED)?; if !verified { - return Err(anyhow!("invalid credentials").into()); + return Err( + anyhow!("Invalid login credentials") + .status_code(StatusCode::UNAUTHORIZED), + ); } jwt_client() .encode(user.id) + // This is in internal error (500), not auth error .context("Failed to generate JWT for user") .map_err(Into::into) } diff --git a/bin/core/src/auth/mod.rs b/bin/core/src/auth/mod.rs index 5fa43b8e3..b8b7ed4d6 100644 --- a/bin/core/src/auth/mod.rs +++ b/bin/core/src/auth/mod.rs @@ -59,23 +59,21 @@ pub async fn get_user_id_from_headers( ) { (Some(jwt), _, _) => { // USE JWT - let jwt = jwt.to_str().context("jwt is not str")?; - auth_jwt_get_user_id(jwt) - .await - .context("failed to authenticate jwt") + let jwt = jwt.to_str().context("JWT is not valid UTF-8")?; + auth_jwt_get_user_id(jwt).await } (None, Some(key), Some(secret)) => { // USE API KEY / SECRET - let key = key.to_str().context("key is not str")?; - let secret = secret.to_str().context("secret is not str")?; - auth_api_key_get_user_id(key, secret) - .await - .context("failed to authenticate api key") + let key = + key.to_str().context("X-API-KEY is not valid UTF-8")?; + let secret = + secret.to_str().context("X-API-SECRET is not valid UTF-8")?; + auth_api_key_get_user_id(key, secret).await } _ => { // AUTH FAIL Err(anyhow!( - "must attach either AUTHORIZATION header with jwt OR pass X-API-KEY and X-API-SECRET" + "Must attach either AUTHORIZATION header with jwt OR pass X-API-KEY and X-API-SECRET" )) } } @@ -85,22 +83,26 @@ pub async fn authenticate_check_enabled( headers: &HeaderMap, ) -> anyhow::Result { let user_id = get_user_id_from_headers(headers).await?; - let user = get_user(&user_id).await?; + let user = get_user(&user_id) + .await + .map_err(|_| anyhow!("Invalid user credentials"))?; if user.enabled { Ok(user) } else { - Err(anyhow!("User not enabled")) + Err(anyhow!("Invalid user credentials")) } } pub async fn auth_jwt_get_user_id( jwt: &str, ) -> anyhow::Result { - let claims: JwtClaims = jwt_client().decode(jwt)?; + let claims: JwtClaims = jwt_client() + .decode(jwt) + .map_err(|_| anyhow!("Invalid user credentials"))?; if claims.exp > unix_timestamp_ms() { Ok(claims.id) } else { - Err(anyhow!("token has expired")) + Err(anyhow!("Invalid user credentials")) } } @@ -119,19 +121,19 @@ pub async fn auth_api_key_get_user_id( .api_keys .find_one(doc! { "key": key }) .await - .context("failed to query db")? - .context("no api key matching key")?; + .context("Failed to query db")? + .context("Invalid user credentials")?; if key.expires != 0 && key.expires < komodo_timestamp() { - return Err(anyhow!("api key expired")); + return Err(anyhow!("Invalid user credentials")); } if bcrypt::verify(secret, &key.secret) - .context("failed to verify secret hash")? + .map_err(|_| anyhow!("Invalid user credentials"))? { // secret matches Ok(key.user_id) } else { // secret mismatch - Err(anyhow!("invalid api secret")) + Err(anyhow!("Invalid user credentials")) } } @@ -148,6 +150,6 @@ async fn check_enabled(user_id: String) -> anyhow::Result { if user.enabled { Ok(user) } else { - Err(anyhow!("user not enabled")) + Err(anyhow!("Invalid user credentials")) } } diff --git a/bin/core/src/helpers/query.rs b/bin/core/src/helpers/query.rs index a03dc5dfc..6cd8c53aa 100644 --- a/bin/core/src/helpers/query.rs +++ b/bin/core/src/helpers/query.rs @@ -56,8 +56,8 @@ pub async fn get_user(user: &str) -> anyhow::Result { .users .find_one(id_or_username_filter(user)) .await - .context("failed to query mongo for user")? - .with_context(|| format!("no user found with {user}")) + .context("Failed to query mongo for user")? + .with_context(|| format!("No user found matching '{user}'")) } pub async fn get_server_with_state(