[WIP] Implement secure local authentication practices

[Copilot]

Thanks for asking me to work on this. I will get started on it and keep this PR's description up to date as I form a plan and make progress.

Original prompt

Make the local authentication implementation safe against user-enumeration and timing leaks, and ensure internal errors are logged but not leaked to clients. Change the behavior across the whole file bin/core/src/auth/local.rs rather than only the highlighted section.

Summary of changes to implement:

• Do not reveal whether a username exists in error messages returned to clients. Return a single generic "invalid credentials" for authentication failures.
• Avoid timing differences between "user not found" and "bad password" by verifying the supplied password against a runtime-generated DUMMY_HASH when the user is missing or when the attempt is not allowed for non-local users.
• Treat DB and crypto failures as internal server errors: log details server-side using tracing and return generic "internal server error" to the client.
• Keep internal logging (including username) for operators to debug, but avoid embedding it in client-facing messages.
• Use once_cell::sync::Lazy to generate a dummy bcrypt hash once at startup using the bcrypt default cost. Optionally could use a pre-computed constant instead (trade startup cost vs compatibility).
• Maintain existing signup logic but make DB errors and password hashing errors return generic internal errors (and log specifics).

Files to change:

• bin/core/src/auth/local.rs

Updated file contents (full replacement):

use std::str::FromStr;

use anyhow::{Context, anyhow};
use async_timing_util::unix_timestamp_ms;
use database::{
  hash_password,
  mungos::mongodb::bson::{Document, doc, oid::ObjectId},
};
use komodo_client::{
  api::auth::{
    LoginLocalUser, LoginLocalUserResponse, SignUpLocalUser,
    SignUpLocalUserResponse,
  },
  entities::user::{User, UserConfig},
};
use resolver_api::Resolve;

use crate::{
  api::auth::AuthArgs,
  config::core_config,
  state::{db_client, jwt_client},
};

use once_cell::sync::Lazy;
use tracing::{error, warn, instrument};

static DUMMY_HASH: Lazy<String> = Lazy::new(|| {
  // Generate a dummy bcrypt hash once at runtime using the same cost as bcrypt::hash.
  // The resulting hash is only used for timing-equalization when a username does not exist
  // or when we want to avoid revealing whether the user exists.
  bcrypt::hash("komodo_dummy_password", bcrypt::DEFAULT_COST)
    .expect("failed to generate dummy bcrypt hash")
});

impl Resolve<AuthArgs> for SignUpLocalUser {
  #[instrument(name = "SignUpLocalUser", skip(self))]
  async fn resolve(
    self,
    _: &AuthArgs,
  ) -> serror::Result<SignUpLocalUserResponse> {
    let core_config = core_config();

    if !core_config.local_auth {
      return Err(anyhow!("Local auth is not enabled").into());
    }

    if self.username.is_empty() {
      return Err(anyhow!("Username cannot be empty string").into());
    }

    if ObjectId::from_str(&self.username).is_ok() {
      return Err(
        anyhow!("Username cannot be valid ObjectId").into(),
      );
    }

    if self.password.is_empty() {
      return Err(anyhow!("Password cannot be empty string").into());
    }

    let db = db_client();

    // Check whether any users exist. Treat DB errors as internal server error (log only).
    let no_users_exist = match db.users.find_one(Document::new()).await {
      Ok(opt) => opt.is_none(),
      Err(e) => {
        error!(?e, "failed to query users for initial user check");
        return Err(anyhow!("internal server error").into());
      }
    };

    if !no_users_exist && core_config.disable_user_registration {
      return Err(anyhow!("User registration is disabled").into());
    }

    // Check username availability. Log DB failures, but avoid leaking DB internals to client.
    match db
      .users
      .find_one(doc! { "username": &self.username })
      .await
    {
      Ok(Some(_)) => {
        return Err(anyhow!("Username already taken.").into());
      }
      Ok(None) => {}
      Err(e) => {
        error!(?e, username = %self.username, "failed to query for existing users");
        return Err(anyhow!("internal server error").into());
      }
    }

    let ts = unix_timestamp_ms() as i64;
    let hashed_password = match hash_password(self.password) {
      Ok(h) => h,
      Err(e) => {
        error!(?e, "failed to hash password during signup");
        return Err(anyhow!("internal server error").into());
      }
    };

    let user = User {
      id: Default::default(),
      username: self.username,
      enabled: no_users_exist || core_config.enable_new_users,
      admin: no_users_exist,
      super_admin: no_users_exist,
      create_server_permissions: no_users_exist,
      create_build_permissions: no_users_exist,
      updated_at: ts,
      last_update_view: 0,
      recents: Default::default(),
      all: Default::default(),
      config: UserConfig::Local {
        password: hashed_password,
      },
    };

    let inserted = match db_client()
      .users
      .insert_one(user)
      .await
    {
      Ok(res) => res,
      Err(e) => {
     ...

</details>

*This pull request was created as a result of the following prompt from Copilot chat.*
> Make the local authentication implementation safe against user-enumeration and timing leaks, and ensure internal errors are logged but not leaked to clients. Change the behavior across the whole file bin/core/src/auth/local.rs rather than only the highlighted section.
> 
> Summary of changes to implement:
> - Do not reveal whether a username exists in error messages returned to clients. Return a single generic "invalid credentials" for authentication failures.
> - Avoid timing differences between "user not found" and "bad password" by verifying the supplied password against a runtime-generated DUMMY_HASH when the user is missing or when the attempt is not allowed for non-local users.
> - Treat DB and crypto failures as internal server errors: log details server-side using tracing and return generic "internal server error" to the client.
> - Keep internal logging (including username) for operators to debug, but avoid embedding it in client-facing messages.
> - Use once_cell::sync::Lazy to generate a dummy bcrypt hash once at startup using the bcrypt default cost. Optionally could use a pre-computed constant instead (trade startup cost vs compatibility).
> - Maintain existing signup logic but make DB errors and password hashing errors return generic internal errors (and log specifics).
> 
> Files to change:
> - bin/core/src/auth/local.rs
> 
> Updated file contents (full replacement):
> 
> ```rust
> use std::str::FromStr;
> 
> use anyhow::{Context, anyhow};
> use async_timing_util::unix_timestamp_ms;
> use database::{
>   hash_password,
>   mungos::mongodb::bson::{Document, doc, oid::ObjectId},
> };
> use komodo_client::{
>   api::auth::{
>     LoginLocalUser, LoginLocalUserResponse, SignUpLocalUser,
>     SignUpLocalUserResponse,
>   },
>   entities::user::{User, UserConfig},
> };
> use resolver_api::Resolve;
> 
> use crate::{
>   api::auth::AuthArgs,
>   config::core_config,
>   state::{db_client, jwt_client},
> };
> 
> use once_cell::sync::Lazy;
> use tracing::{error, warn, instrument};
> 
> static DUMMY_HASH: Lazy<String> = Lazy::new(|| {
>   // Generate a dummy bcrypt hash once at runtime using the same cost as bcrypt::hash.
>   // The resulting hash is only used for timing-equalization when a username does not exist
>   // or when we want to avoid revealing whether the user exists.
>   bcrypt::hash("komodo_dummy_password", bcrypt::DEFAULT_COST)
>     .expect("failed to generate dummy bcrypt hash")
> });
> 
> impl Resolve<AuthArgs> for SignUpLocalUser {
>   #[instrument(name = "SignUpLocalUser", skip(self))]
>   async fn resolve(
>     self,
>     _: &AuthArgs,
>   ) -> serror::Result<SignUpLocalUserResponse> {
>     let core_config = core_config();
> 
>     if !core_config.local_auth {
>       return Err(anyhow!("Local auth is not enabled").into());
>     }
> 
>     if self.username.is_empty() {
>       return Err(anyhow!("Username cannot be empty string").into());
>     }
> 
>     if ObjectId::from_str(&self.username).is_ok() {
>       return Err(
>         anyhow!("Username cannot be valid ObjectId").into(),
>       );
>     }
> 
>     if self.password.is_empty() {
>       return Err(anyhow!("Password cannot be empty string").into());
>     }
> 
>     let db = db_client();
> 
>     // Check whether any users exist. Treat DB errors as internal server error (log only).
>     let no_users_exist = match db.users.find_one(Document::new()).await {
>       Ok(opt) => opt.is_none(),
>       Err(e) => {
>         error!(?e, "failed to query users for initial user check");
>         return Err(anyhow!("internal server error").into());
>       }
>     };
> 
>     if !no_users_exist && core_config.disable_user_registration {
>       return Err(anyhow!("User registration is disabled").into());
>     }
> 
>     // Check username availability. Log DB failures, but avoid leaking DB internals to client.
>     match db
>       .users
>       .find_one(doc! { "username": &self.username })
>       .await
>     {
>       Ok(Some(_)) => {
>         return Err(anyhow!("Username already taken.").into());
>       }
>       Ok(None) => {}
>       Err(e) => {
>         error!(?e, username = %self.username, "failed to query for existing users");
>         return Err(anyhow!("internal server error").into());
>       }
>     }
> 
>     let ts = unix_timestamp_ms() as i64;
>     let hashed_password = match hash_password(self.password) {
>       Ok(h) => h,
>       Err(e) => {
>         error!(?e, "failed to hash password during signup");
>         return Err(anyhow!("internal server error").into());
>       }
>     };
> 
>     let user = User {
>       id: Default::default(),
>       username: self.username,
>       enabled: no_users_exist || core_config.enable_new_users,
>       admin: no_users_exist,
>       super_admin: no_users_exist,
>       create_server_permissions: no_users_exist,
>       create_build_permissions: no_users_exist,
>       updated_at: ts,
>       last_update_view: 0,
>       recents: Default::default(),
>       all: Default::default(),
>       config: UserConfig::Local {
>         password: hashed_password,
>       },
>     };
> 
>     let inserted = match db_client()
>       .users
>       .insert_one(user)
>       .await
>     {
>       Ok(res) => res,
>       Err(e) => {
>         error!(?e, "failed to insert new user");
>         return Err(anyhow!("internal server error").into());
>       }
>     };
> 
>     let user_id = match inserted.inserted_id.as_object_id() {
>       Some(oid) => oid.to_string(),
>       None => {
>         error!("inserted_id is not an ObjectId");
>         return Err(anyhow!("internal server error").into());
>       }
>     };
> 
>     match jwt_client().encode(user_id.clone()) {
>       Ok(token) => Ok(token),
>       Err(e) => {
>         error!(?e, "failed to generate jwt for new user");
>         Err(anyhow!("internal server error").into())
>       }
>     }
>   }
> }
> 
> impl Resolve<AuthArgs> for LoginLocalUser {
>   #[instrument(name = "LoginLocalUser", level = "debug", skip(self))]
>   async fn resolve(
>     self,
>     _: &AuthArgs,
>   ) -> serror::Result<LoginLocalUserResponse> {
>     if !core_config().local_auth {
>       return Err(anyhow!("local auth is not enabled").into());
>     }
> 
>     // Query for the user. Treat DB errors as internal and log them; do not reveal to client.
>     let user_opt = match db_client()
>       .users
>       .find_one(doc! { "username": &self.username })
>       .await
>     {
>       Ok(opt) => opt,
>       Err(e) => {
>         error!(?e, "failed to query users");
>         return Err(anyhow!("internal server error").into());
>       }
>     };
> 
>     // Determine hash to verify against. If user exists and is local, use their hash.
>     // Otherwise use DUMMY_HASH to equalize timing and avoid user enumeration by timing differences.
>     let (user_id_opt, pw_hash): (Option<String>, String) = match user_opt {
>       Some(user) => {
>         match user.config {
>           UserConfig::Local { password } => (Some(user.id.clone()), password),
>           _ => {
>             // Non-local user can't login via password. Still verify against dummy hash for timing parity.
>             warn!("login attempt for non-local auth user");
>             let _ = bcrypt::verify(&self.password, &*DUMMY_HASH);
>             return Err(anyhow!("invalid credentials").into());
>           }
>         }
>       }
>       None => {
>         // No user found: use dummy hash
>         (None, DUMMY_HASH.clone())
>       }
>     };
> 
>     // Verify password. If verification fails due to internal error, log and return internal server error.
>     let verified = match bcrypt::verify(&self.password, &pw_hash) {
>       Ok(v) => v,
>       Err(e) => {
>         error!(?e, "failed to verify password");
>         return Err(anyhow!("internal server error").into());
>       }
>     };
> 
>     // If verification failed or user didn't exist, return generic invalid credentials (no enumeration).
>     if !verified || user_id_opt.is_none() {
>       return Err(anyhow!("invalid credentials").into());
>     }
> 
>     // At this point we have a verified local user.
>     let user_id = user_id_opt.unwrap();
>     match jwt_client().encode(user_id.clone()) {
>       Ok(token) => Ok(token),
>       Err(e) => {
>         error!(?e, "failed at generating jwt for user");
>         Err(anyhow!("internal server error").into())
>       }
>     }
>   }
> }
> ```
> 
> Notes for the PR:
> - This is a single-file change. Tests are not included; if you want I can add tests or adjust the PR to use a compile-time dummy hash instead of a runtime-one.
> - Please review the logging levels and messages; I used error! for DB/crypto failures and warn! for suspicious login attempts against non-local users.
> 
> Please open the PR with these changes. If you want a different branch name or PR title, mention it and I'll use that.

<!-- START COPILOT CODING AGENT TIPS -->
---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey).