feat: DB operations can fail

This commit is contained in:
selfhoster selfhoster 2026-09-01 23:27:02 +02:00
commit 0e19117ca4
6 changed files with 66 additions and 19 deletions

View file

@ -2,6 +2,7 @@ use tokio::sync::RwLock;
use std::sync::Arc; use std::sync::Arc;
use crate::db::error::BoxableError;
use crate::db::{DatabaseInterface, UserRef}; use crate::db::{DatabaseInterface, UserRef};
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
@ -26,13 +27,17 @@ impl<D: DatabaseInterface> Database<D> {
/// TODO: we may want to make sure the method runs in constant time /// TODO: we may want to make sure the method runs in constant time
/// to avoid leaking information about existing users... /// to avoid leaking information about existing users...
/// or maybe we do not care. /// or maybe we do not care.
pub async fn check_password(&self, user: &UserRef, password: &str) -> bool { pub async fn check_password(
let Some(user) = self.get_user(user).await else { &self,
user: &UserRef,
password: &str,
) -> Result<bool, Box<dyn BoxableError>> {
let Some(user) = self.get_user(user).await? else {
tracing::debug!("check_password: User not found {user}"); tracing::debug!("check_password: User not found {user}");
return false; return Ok(false);
}; };
tracing::debug!("Comparing {} and {}", user.password, password); tracing::debug!("Comparing {} and {}", user.password, password);
user.password == password Ok(user.password == password)
} }
} }

View file

@ -1,6 +1,21 @@
use ldap3_proto::LdapResultCode;
use std::fmt; use std::fmt;
use crate::db::UserRef; use crate::db::UserRef;
use crate::ldap::LdapReturnError;
pub trait BoxableError: std::error::Error + tracing::Value + Sync + Send + 'static {}
impl<T: ?Sized + BoxableError> LdapReturnError for T {
fn code(&self) -> LdapResultCode {
LdapResultCode::Unavailable
}
fn message(&self) -> String {
self.to_string()
}
}
#[derive(Debug)] #[derive(Debug)]
pub struct UserAlreadyExists(pub UserRef); pub struct UserAlreadyExists(pub UserRef);

View file

@ -1,17 +1,23 @@
use crate::db::error::UserAlreadyExists; use crate::db::error::{BoxableError, UserAlreadyExists};
use crate::db::{Database, User, UserRef}; use crate::db::{Database, User, UserRef};
impl<D: DatabaseInterface> DatabaseInterface for Database<D> { impl<D: DatabaseInterface> DatabaseInterface for Database<D> {
async fn get_user(&self, user: &UserRef) -> Option<User> { async fn get_user(&self, user: &UserRef) -> Result<Option<User>, Box<dyn BoxableError>> {
self.inner.read().await.get_user(user).await self.inner.read().await.get_user(user).await
} }
async fn create_user(&mut self, user: User) -> Result<(), UserAlreadyExists> { async fn create_user(
&mut self,
user: User,
) -> Result<Result<(), UserAlreadyExists>, Box<dyn BoxableError>> {
self.inner.write().await.create_user(user).await self.inner.write().await.create_user(user).await
} }
} }
pub trait DatabaseInterface { pub trait DatabaseInterface {
async fn get_user(&self, user: &UserRef) -> Option<User>; async fn get_user(&self, user: &UserRef) -> Result<Option<User>, Box<dyn BoxableError>>;
async fn create_user(&mut self, user: User) -> Result<(), UserAlreadyExists>; async fn create_user(
&mut self,
user: User,
) -> Result<Result<(), UserAlreadyExists>, Box<dyn BoxableError>>;
} }

View file

@ -1,6 +1,6 @@
use std::future::{Future, ready}; use std::future::{Future, ready};
use crate::db::error::UserAlreadyExists; use crate::db::error::{BoxableError, UserAlreadyExists};
use crate::db::{Database, DatabaseInterface, Group, User, UserRef}; use crate::db::{Database, DatabaseInterface, Group, User, UserRef};
#[derive(Clone, Debug, Default)] #[derive(Clone, Debug, Default)]
@ -16,23 +16,29 @@ impl MemoryDatabase {
} }
impl DatabaseInterface for MemoryDatabase { impl DatabaseInterface for MemoryDatabase {
fn get_user(&self, req_user: &UserRef) -> impl Future<Output = Option<User>> { fn get_user(
&self,
req_user: &UserRef,
) -> impl Future<Output = Result<Option<User>, Box<dyn BoxableError>>> {
for user in &self.users { for user in &self.users {
if user.username == req_user.username && user.domain == req_user.domain { if user.username == req_user.username && user.domain == req_user.domain {
return ready(Some(user.clone())); return ready(Ok(Some(user.clone())));
} }
} }
ready(None) ready(Ok(None))
} }
async fn create_user(&mut self, user: User) -> Result<(), UserAlreadyExists> { async fn create_user(
&mut self,
user: User,
) -> Result<Result<(), UserAlreadyExists>, Box<dyn BoxableError>> {
let user_ref = user.user_ref(); let user_ref = user.user_ref();
if self.get_user(&user_ref).await.is_some() { if self.get_user(&user_ref).await?.is_some() {
return Err(UserAlreadyExists(user_ref)); return Ok(Err(UserAlreadyExists(user_ref)));
} }
self.users.push(user); self.users.push(user);
Ok(()) Ok(Ok(()))
} }
} }

View file

@ -1,13 +1,15 @@
use ldap3_proto::proto::{LdapBindCred, LdapBindRequest, LdapBindResponse, LdapOp, LdapResult}; use ldap3_proto::proto::{LdapBindCred, LdapBindRequest, LdapBindResponse, LdapOp, LdapResult};
use ldap3_proto::{LdapMsg, LdapResultCode}; use ldap3_proto::{LdapMsg, LdapResultCode};
use crate::db::error::BoxableError;
use crate::db::{Database, DatabaseInterface}; use crate::db::{Database, DatabaseInterface};
use crate::ldap::{ use crate::ldap::{
Dn, InvalidDnError, LdapReturnError, LdapStream, LdapStreamError, NotUserDnError, Dn, InvalidDnError, LdapReturnError, LdapStream, LdapStreamError, NotUserDnError,
}; };
#[derive(Debug)]
pub enum BindError { pub enum BindError {
// TODO: DB Error Db(Box<dyn BoxableError>),
InvalidCredentials, InvalidCredentials,
InvalidDn(InvalidDnError), InvalidDn(InvalidDnError),
NotUserDn(NotUserDnError), NotUserDn(NotUserDnError),
@ -20,6 +22,7 @@ impl BindError {
stream: &mut LdapStream, stream: &mut LdapStream,
msgid: i32, msgid: i32,
) -> Result<(), LdapStreamError> { ) -> Result<(), LdapStreamError> {
tracing::debug!(error=?self, "Bind failed");
let resp_msg = LdapMsg { let resp_msg = LdapMsg {
msgid, msgid,
op: LdapOp::BindResponse(LdapBindResponse { op: LdapOp::BindResponse(LdapBindResponse {
@ -48,6 +51,7 @@ impl From<InvalidDnError> for BindError {
impl LdapReturnError for BindError { impl LdapReturnError for BindError {
fn code(&self) -> LdapResultCode { fn code(&self) -> LdapResultCode {
match self { match self {
Self::Db(e) => e.code(),
Self::InvalidCredentials => LdapResultCode::InvalidCredentials, Self::InvalidCredentials => LdapResultCode::InvalidCredentials,
Self::InvalidDn(e) => e.code(), Self::InvalidDn(e) => e.code(),
Self::NotUserDn(e) => e.code(), Self::NotUserDn(e) => e.code(),
@ -57,6 +61,7 @@ impl LdapReturnError for BindError {
fn message(&self) -> String { fn message(&self) -> String {
match self { match self {
Self::Db(e) => e.message(),
Self::InvalidCredentials => "Wrong username or password".to_string(), Self::InvalidCredentials => "Wrong username or password".to_string(),
Self::InvalidDn(e) => e.message(), Self::InvalidDn(e) => e.message(),
Self::NotUserDn(e) => e.message(), Self::NotUserDn(e) => e.message(),
@ -117,7 +122,16 @@ pub async fn op_bind<D: DatabaseInterface>(
} }
}; };
if db.check_password(&user_ref, &password).await { let success = match db.check_password(&user_ref, &password).await {
Ok(success) => success,
Err(e) => {
tracing::error!(error = e, "Database failure");
BindError::Db(e).error_message(stream, msgid).await?;
return Ok(None);
}
};
if success {
bind_success(stream, msgid).await?; bind_success(stream, msgid).await?;
Ok(Some(dn)) Ok(Some(dn))
} else { } else {

View file

@ -22,6 +22,7 @@ async fn create_dummy_users<D: DatabaseInterface>(db: &mut Database<D>) {
mail: format!("{domain}@{domain}.localhost"), mail: format!("{domain}@{domain}.localhost"),
}) })
.await .await
.unwrap()
.unwrap(); .unwrap();
} }
} }