Commit dda4da55 authored by Szymon Zimnowoda's avatar Szymon Zimnowoda
Browse files

update err messages and codes

parent 778373a2
Showing with 63 additions and 21 deletions
+63 -21
......@@ -7,9 +7,10 @@ use crate::{
database_pool::{get_db_connection, remove_db, InitDb},
db_model::ItemBase,
error::Result,
file_api, forbidden, internal_error,
file_api, internal_error,
plugin_auth_crypto::DatabaseKey,
schema::{Schema, SchemaPropertyType},
unauthorized,
};
use lazy_static::lazy_static;
use rusqlite::types::ValueRef;
......@@ -360,7 +361,7 @@ pub fn check_owner(possible_owner: &str) -> Result<()> {
possible_owner,
hex::encode(possible_hash)
);
Err(forbidden! {
Err(unauthorized! {
"Unexpected owner",
})
}
......
......@@ -108,7 +108,6 @@ pub struct Oauth2Flow {
pub enum RegisterState {
VerifyEmailSent,
RegistrationComplete,
EnforcePasswordReset,
}
pub const POD_ACCOUNT: &str = "PodUserAccount";
......
......@@ -49,7 +49,7 @@ macro_rules! bad_request {
/// Shorthand for creating errors caused unauthorized access
#[macro_export]
macro_rules! forbidden {
macro_rules! unauthorized {
($($arg:tt)*) => {
$crate::error::Error {
context: None,
......
......@@ -11,7 +11,7 @@ use secrecy::{ExposeSecret, Secret, SecretString};
use serde_json::json;
use sha2::{Digest, Sha256};
use tokio::join;
use tracing::{debug, info, instrument};
use tracing::{debug, error, info, instrument};
use crate::api_model::{
AuthKey, ClientAuth, CreateItem, PodCredentials, PodOwner, RecoverRequest, RegisterResponse,
......@@ -29,9 +29,9 @@ use crate::error::{ErrorContext, Result};
use crate::plugin_auth_crypto::auth_to_database_key;
use crate::{
bad_request, database_api, database_utils, internal_api, internal_error, shared_state,
unauthorized,
};
#[instrument(fields(login=%body.login), skip_all)]
pub async fn register(
cli: &CliOptions,
init_db: &InitDb,
......@@ -39,10 +39,19 @@ pub async fn register(
) -> Result<RegisterResponse> {
let mut conn = shared_state::db_connection(init_db).await?;
if let Some(acc) = get_account_from_db(&mut conn, &body.login).await? {
Err(bad_request!(
"Account already exists, status {:?}",
error!(
"Trying to register on already existing account, state: {:?}",
acc.item.state
))
);
match acc.item.state {
RegisterState::VerifyEmailSent => Err(bad_request!(
"This email is already registered. Please verify the email using the code sent to your inbox."),
),
RegisterState::RegistrationComplete => Err(bad_request!(
"This email is already registered. Please log in."
)),
}
} else {
let mut rnd = rand::thread_rng();
......@@ -157,7 +166,7 @@ pub async fn verify(init_db: &InitDb, body: &RegisterVerifyAccountReq) -> Result
body.code.expose_secret(),
acc.item.code
);
Err(bad_request!("Invalid token provided"))
Err(unauthorized!("Invalid token provided"))
}
}
......@@ -169,6 +178,10 @@ pub async fn resend_verification_mail(
) -> Result<()> {
let mut conn = shared_state::db_connection(init_db).await?;
if let Some(mut acc) = get_account_from_db(&mut conn, &body.login).await? {
if acc.item.state != RegisterState::VerifyEmailSent {
return Err(bad_request!("Account is in state {:?}", acc.item.state));
}
validate_password(body.password.clone(), acc.item.password_hash.clone()).await?;
let mut rnd = rand::thread_rng();
......@@ -196,7 +209,7 @@ pub async fn resend_verification_mail(
// TODO: there is obvious race condition with parallel requests for the same
// account - ensure 1 request at a time happens
// 1 frontend - disable button, but what if you have 2 devices?
// 2 backend - some smart locking, creating TX will block whole database for that moment
// 2 backend - some smart locking, creating write-TX will block whole database for that moment
// - suboptimal
// there is race condition with /verify too
acc.item.code = code_str;
......@@ -218,11 +231,15 @@ pub async fn get_pod_keys(
let mut conn = shared_state::db_connection(init_db).await?;
let Some(acc) = get_account_from_db(&mut conn, &body.login).await? else {
return Err(bad_request!("Account does not exist"));
return Err(bad_request!("There is no account registered with this email. Please check your email or create an account."));
};
if acc.item.state != RegisterState::RegistrationComplete {
return Err(bad_request!("Account is not verified"));
error!(
"Account is not yet fully registered, state {:?}",
acc.item.state
);
return Err(bad_request!("This email is already registered. Please verify the email using the code sent to your inbox."));
}
validate_password(body.password.clone(), acc.item.password_hash.clone()).await?;
......@@ -281,7 +298,12 @@ async fn validate_password(password: Secret<String>, expected_hash: String) -> R
password.expose_secret().as_bytes(),
&PasswordHash::new(&expected_hash).expect("Invalid PHC string format"),
)
.map_err(|e| bad_request!("Invalid password {e}"))?;
.map_err(|e| match e {
argon2::password_hash::Error::Password => {
unauthorized!("Oops, wrong password! Please try again.")
}
_ => internal_error!("Error during password validation: {e}"),
})?;
Ok(())
})
.await?
......
......@@ -561,8 +561,8 @@ pub fn respond_with_result<T: Serialize>(result: Result<T>) -> actix_web::Result
| libpod::error::ErrorType::BadRequest(_) => StatusCode::BAD_REQUEST,
libpod::error::ErrorType::Any { code, msg: _ } => *code,
libpod::error::ErrorType::Unauthorized(_) => StatusCode::FORBIDDEN,
libpod::error::ErrorType::UnauthorizedDatabaseAccess(_)
libpod::error::ErrorType::Unauthorized(_)
| libpod::error::ErrorType::UnauthorizedDatabaseAccess(_)
| libpod::error::ErrorType::AeadEncryption(_) => StatusCode::UNAUTHORIZED,
libpod::error::ErrorType::Internal(_) => StatusCode::INTERNAL_SERVER_ERROR,
......
......@@ -70,7 +70,7 @@ async fn test_account_creation_flow(ctx: &mut TestDataForCreateAccount) {
.json::<String>()
.await
.unwrap()
.contains("Account already exists"),);
.contains("This email is already registered."),);
// Login is case insensitive
let res = ctx
......@@ -90,7 +90,7 @@ async fn test_account_creation_flow(ctx: &mut TestDataForCreateAccount) {
.json::<String>()
.await
.unwrap()
.contains("Account already exists"),);
.contains("This email is already registered."),);
// Login is invalid mail
let res = ctx
......@@ -124,7 +124,7 @@ async fn test_account_creation_flow(ctx: &mut TestDataForCreateAccount) {
)
.await;
assert_eq!(res.status(), StatusCode::BAD_REQUEST);
assert_eq!(res.status(), StatusCode::UNAUTHORIZED);
assert!(res
.json::<String>()
......@@ -159,13 +159,13 @@ async fn test_account_creation_flow(ctx: &mut TestDataForCreateAccount) {
)
.await;
assert_eq!(res.status(), StatusCode::BAD_REQUEST);
assert_eq!(res.status(), StatusCode::UNAUTHORIZED);
assert!(res
.json::<String>()
.await
.unwrap()
.contains("Invalid password"),);
.contains("Oops, wrong password! Please try again."),);
// with valid pass
let res = ctx
......@@ -339,6 +339,26 @@ async fn test_account_resend_email(ctx: &mut TestDataForCreateAccount) {
assert_eq!(res.status(), StatusCode::OK);
// cannot resend mail, if account is not in verify state
let res = ctx
.pod_client
.post_to(
json!({
"login": LOGIN,
"password": PASS
}),
"account/resend_mail",
)
.await;
assert_eq!(
res.status(),
StatusCode::BAD_REQUEST,
"The status is {}, body {:#}",
res.status(),
res.json::<Value>().await.unwrap()
);
// Can generate pod keys
// with valid pass
let res = ctx
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment