feat: DB operations can fail
This commit is contained in:
parent
dc239371e9
commit
7527ba7a09
6 changed files with 50 additions and 19 deletions
|
|
@ -2,6 +2,7 @@ use tokio::sync::RwLock;
|
||||||
|
|
||||||
use std::sync::Arc;
|
use std::sync::Arc;
|
||||||
|
|
||||||
|
use crate::db::error::BoxedError;
|
||||||
use crate::db::{DatabaseInterface, UserRef};
|
use crate::db::{DatabaseInterface, UserRef};
|
||||||
|
|
||||||
#[derive(Clone, Debug)]
|
#[derive(Clone, Debug)]
|
||||||
|
|
@ -26,13 +27,13 @@ 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(&self, user: &UserRef, password: &str) -> Result<bool, BoxedError> {
|
||||||
let Some(user) = self.get_user(user).await else {
|
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)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -2,6 +2,8 @@ use std::fmt;
|
||||||
|
|
||||||
use crate::db::UserRef;
|
use crate::db::UserRef;
|
||||||
|
|
||||||
|
pub type BoxedError = Box<dyn std::error::Error + Send + Sync + 'static>;
|
||||||
|
|
||||||
#[derive(Debug)]
|
#[derive(Debug)]
|
||||||
pub struct UserAlreadyExists(pub UserRef);
|
pub struct UserAlreadyExists(pub UserRef);
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1,17 +1,23 @@
|
||||||
use crate::db::error::UserAlreadyExists;
|
use crate::db::error::{BoxedError, 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>, BoxedError> {
|
||||||
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>, BoxedError> {
|
||||||
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>, BoxedError>;
|
||||||
async fn create_user(&mut self, user: User) -> Result<(), UserAlreadyExists>;
|
async fn create_user(
|
||||||
|
&mut self,
|
||||||
|
user: User,
|
||||||
|
) -> Result<Result<(), UserAlreadyExists>, BoxedError>;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,6 @@
|
||||||
use std::future::{Future, ready};
|
use std::future::{Future, ready};
|
||||||
|
|
||||||
use crate::db::error::UserAlreadyExists;
|
use crate::db::error::{BoxedError, 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>, BoxedError>> {
|
||||||
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>, BoxedError> {
|
||||||
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(()))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -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::BoxedError;
|
||||||
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(BoxedError),
|
||||||
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) => LdapResultCode::Unavailable,
|
||||||
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) => format!("Database error: {e}"),
|
||||||
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,17 @@ 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 as &dyn std::error::Error, "Database failure");
|
||||||
|
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 {
|
||||||
|
|
|
||||||
|
|
@ -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();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue