Skip to content

Commit

Permalink
feat: password hashing using argon2 (#36)
Browse files Browse the repository at this point in the history
  • Loading branch information
kentSarmiento committed Dec 26, 2023
1 parent 5aae6cd commit 3e7f493
Show file tree
Hide file tree
Showing 9 changed files with 124 additions and 57 deletions.
3 changes: 3 additions & 0 deletions .github/workflows/development.yml
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,11 @@ jobs:
needs: test
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3

# https://docs.deepsource.com/docs/analyzers-test-coverage#with-github-actions
- name: Use pull request HEAD commit for DeepSource
if: github.event_name == 'pull_request'
uses: actions/checkout@v3
with:
ref: ${{ github.event.pull_request.head.sha }}
Expand Down
39 changes: 39 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion link-for-later-axum/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const MONGODB_DATABASE_NAME_KEY: &str = "MONGODB_DATABASE_NAME";
#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
tracing_subscriber::fmt()
.with_max_level(tracing::Level::INFO)
.with_max_level(tracing::Level::DEBUG)
// disable printing the name of the module in every log line.
.with_target(false)
// disabling time is handy because CloudWatch will add the ingestion time.
Expand Down
1 change: 1 addition & 0 deletions link-for-later/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ repository = "https://github.com/kentSarmiento/link-for-later-service"
publish = false

[dependencies]
argon2 = "0.5.2"
axum = "0.7.2"
axum-extra = { version = "0.9.0", default-features = false, features=["typed-header"] }
bson = "2.8.1"
Expand Down
36 changes: 19 additions & 17 deletions link-for-later/src/controller/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,42 +10,44 @@ use crate::types::AppError;
#[allow(clippy::cognitive_complexity)]
impl IntoResponse for AppError {
fn into_response(self) -> Response {
let error_message = self.to_string();
tracing::info!("{}", error_message);
let (status, error_message) = match self {
Self::ServerError(ref e) => {
tracing::debug!("Server error: {}", e.to_string());
(StatusCode::INTERNAL_SERVER_ERROR, self.to_string())
tracing::debug!("{}: {}", error_message, e.to_string());
(StatusCode::INTERNAL_SERVER_ERROR, error_message)
}
Self::DatabaseError(ref e) => {
tracing::debug!("Database error: {}", e.to_string());
(StatusCode::INTERNAL_SERVER_ERROR, self.to_string())
tracing::debug!("{}: {}", error_message, e.to_string());
(StatusCode::INTERNAL_SERVER_ERROR, error_message)
}
Self::LinkNotFound(ref e) => {
tracing::debug!("Link not found: {}", e.to_string());
(StatusCode::NOT_FOUND, self.to_string())
tracing::debug!("{}: {}", error_message, e.to_string());
(StatusCode::NOT_FOUND, error_message)
}
Self::UserAlreadyExists(ref e) => {
tracing::debug!("User already exists: {}", e.to_string());
(StatusCode::BAD_REQUEST, self.to_string())
tracing::debug!("{}: {}", error_message, e.to_string());
(StatusCode::BAD_REQUEST, error_message)
}
Self::UserNotFound(ref e) => {
tracing::debug!("User not found: {}", e.to_string());
(StatusCode::BAD_REQUEST, self.to_string())
tracing::debug!("{}: {}", error_message, e.to_string());
(StatusCode::BAD_REQUEST, error_message)
}
Self::IncorrectPassword(ref e) => {
tracing::debug!("Incorrect password: {}", e.to_string());
(StatusCode::UNAUTHORIZED, self.to_string())
tracing::debug!("{}: {}", error_message, e.to_string());
(StatusCode::UNAUTHORIZED, error_message)
}
Self::AuthorizationError(ref e) => {
tracing::debug!("Authorization error: {}", e.to_string());
(StatusCode::UNAUTHORIZED, self.to_string())
tracing::debug!("{}: {}", error_message, e.to_string());
(StatusCode::UNAUTHORIZED, error_message)
}
Self::ValidationError(ref e) => {
tracing::debug!("Payload validation error: {}", e.to_string());
(StatusCode::BAD_REQUEST, self.to_string())
tracing::debug!("{}: {}", error_message, e.to_string());
(StatusCode::BAD_REQUEST, error_message)
}

#[cfg(test)]
Self::TestError => (StatusCode::INTERNAL_SERVER_ERROR, self.to_string()),
Self::TestError => (StatusCode::INTERNAL_SERVER_ERROR, error_message),
};

let body = Json(json!({
Expand Down
25 changes: 5 additions & 20 deletions link-for-later/src/controller/links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,7 @@ async fn list(State(app_state): State<AppState>, user: Claims) -> impl IntoRespo
.await
{
Ok(list) => Json(list).into_response(),
Err(e) => {
tracing::error!("Error: {}", e);
e.into_response()
}
Err(e) => e.into_response(),
}
}

Expand Down Expand Up @@ -69,10 +66,7 @@ async fn post(
.await
{
Ok(link) => (StatusCode::CREATED, Json(link)).into_response(),
Err(e) => {
tracing::error!("Error: {}", e);
e.into_response()
}
Err(e) => e.into_response(),
}
}

Expand All @@ -88,10 +82,7 @@ async fn get(
.await
{
Ok(link) => Json(link).into_response(),
Err(e) => {
tracing::error!("Error: {}", e);
e.into_response()
}
Err(e) => e.into_response(),
}
}

Expand Down Expand Up @@ -120,10 +111,7 @@ async fn put(
.await
{
Ok(link) => Json(link).into_response(),
Err(e) => {
tracing::error!("Error: {}", e);
e.into_response()
}
Err(e) => e.into_response(),
}
}

Expand All @@ -139,10 +127,7 @@ async fn delete(
.await
{
Ok(()) => StatusCode::NO_CONTENT.into_response(),
Err(e) => {
tracing::error!("Error: {}", e);
e.into_response()
}
Err(e) => e.into_response(),
}
}

Expand Down
10 changes: 2 additions & 8 deletions link-for-later/src/controller/users.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,7 @@ async fn register(
.await
{
Ok(_) => StatusCode::CREATED.into_response(),
Err(e) => {
tracing::error!("Error: {}", e);
e.into_response()
}
Err(e) => e.into_response(),
}
}

Expand All @@ -72,10 +69,7 @@ async fn login(
let response = AuthResponse::new(token.jwt());
(StatusCode::OK, Json(response)).into_response()
}
Err(e) => {
tracing::error!("Error: {}", e);
e.into_response()
}
Err(e) => e.into_response(),
}
}

Expand Down
38 changes: 31 additions & 7 deletions link-for-later/src/service/users.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
use argon2::{
password_hash::{rand_core::OsRng, PasswordHash, PasswordHasher, PasswordVerifier, SaltString},
Argon2,
};
use axum::async_trait;
use chrono::{DateTime, Duration, Utc};
use jsonwebtoken::{encode, EncodingKey, Header};
Expand Down Expand Up @@ -33,8 +37,16 @@ impl UsersService for ServiceProvider {
};

let now = Utc::now().to_rfc3339();
// TODO: secure password
let registered_user_info = UserInfoBuilder::from(user_info.clone())

let password_hash = Argon2::default()
.hash_password(
user_info.password().as_bytes(),
&SaltString::generate(&mut OsRng),
)
.map_err(|e| AppError::ServerError(format!("hash_password() {e:?}")))?
.to_string();

let registered_user_info = UserInfoBuilder::new(user_info.email(), &password_hash)
.created_at(&now)
.updated_at(&now)
.verified(true)
Expand All @@ -51,9 +63,11 @@ impl UsersService for ServiceProvider {
let user_query = UserQueryBuilder::new(user_info.email()).build();
let retrieved_user_info = users_repo.get(&user_query).await?;

if retrieved_user_info.password() != user_info.password() {
return Err(AppError::IncorrectPassword(user_info.email().to_owned()));
}
let parsed_hash = PasswordHash::new(retrieved_user_info.password())
.map_err(|e| AppError::ServerError(format!("PasswordHash::new() {e:?}")))?;
Argon2::default()
.verify_password(user_info.password().as_bytes(), &parsed_hash)
.map_err(|_| AppError::IncorrectPassword(user_info.email().to_owned()))?;

let timestamp = |timestamp: DateTime<Utc>| -> Result<usize> {
let timestamp: usize = timestamp
Expand Down Expand Up @@ -199,9 +213,14 @@ mod tests {
async fn test_login_user() {
let repo_query = UserQueryBuilder::new("[email protected]").build();
let user_to_login = UserInfoBuilder::new("[email protected]", "test").build();
let registered_user = UserInfoBuilder::new("[email protected]", "test").build();
let request_item = user_to_login.clone();

let password_hash = Argon2::default()
.hash_password(b"test", &SaltString::generate(&mut OsRng))
.unwrap()
.to_string();
let registered_user = UserInfoBuilder::new("[email protected]", &password_hash).build();

let mut mock_users_repo = MockUsersRepo::new();
mock_users_repo
.expect_get()
Expand Down Expand Up @@ -245,9 +264,14 @@ mod tests {
async fn test_login_user_incorrect_password() {
let repo_query = UserQueryBuilder::new("[email protected]").build();
let user_to_login = UserInfoBuilder::new("[email protected]", "incorrect").build();
let registered_user = UserInfoBuilder::new("[email protected]", "test").build();
let request_item = user_to_login.clone();

let password_hash = Argon2::default()
.hash_password(b"test", &SaltString::generate(&mut OsRng))
.unwrap()
.to_string();
let registered_user = UserInfoBuilder::new("[email protected]", &password_hash).build();

let mut mock_users_repo = MockUsersRepo::new();
mock_users_repo
.expect_get()
Expand Down
27 changes: 23 additions & 4 deletions link-for-later/tests/users.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
use argon2::{
password_hash::{rand_core::OsRng, PasswordHasher, SaltString},
Argon2,
};
use axum::{
body::Body,
http::{Request, StatusCode},
Expand Down Expand Up @@ -49,7 +53,7 @@ async fn test_register_user(#[values(DatabaseType::MongoDb)] db_type: DatabaseTy

let db_item = repository.get_user("[email protected]").await;
assert!(db_item.email == "[email protected]");
assert!(db_item.password == "test");
assert!(db_item.password != "test"); // verify password is not saved in plaintext
}

#[rstest]
Expand Down Expand Up @@ -132,7 +136,12 @@ async fn test_register_user_already_registered(
async fn test_login_user(#[values(DatabaseType::MongoDb)] db_type: DatabaseType) {
let repository = repository::new(&db_type);

repository.add_user("[email protected]", "test").await;
let password_hash = Argon2::default()
.hash_password(b"test", &SaltString::generate(&mut OsRng))
.unwrap()
.to_string();
repository.add_user("[email protected]", &password_hash).await;

let request = r#"{
"email": "[email protected]",
"password": "test"
Expand Down Expand Up @@ -196,7 +205,12 @@ async fn test_login_user_invalid_email(#[values(DatabaseType::MongoDb)] db_type:
async fn test_login_user_not_found(#[values(DatabaseType::MongoDb)] db_type: DatabaseType) {
let repository = repository::new(&db_type);

repository.add_user("[email protected]", "test").await;
let password_hash = Argon2::default()
.hash_password(b"test", &SaltString::generate(&mut OsRng))
.unwrap()
.to_string();
repository.add_user("[email protected]", &password_hash).await;

let request = r#"{
"email": "[email protected]",
"password": "test"
Expand Down Expand Up @@ -230,7 +244,12 @@ async fn test_login_user_incorrect_password(
) {
let repository = repository::new(&db_type);

repository.add_user("[email protected]", "test").await;
let password_hash = Argon2::default()
.hash_password(b"test", &SaltString::generate(&mut OsRng))
.unwrap()
.to_string();
repository.add_user("[email protected]", &password_hash).await;

let request = r#"{
"email": "[email protected]",
"password": "incorrect"
Expand Down

0 comments on commit 3e7f493

Please sign in to comment.