Skip to content

Commit

Permalink
fix(masking): mask email while logging SQL query (#4436)
Browse files Browse the repository at this point in the history
  • Loading branch information
dracarys18 committed Apr 23, 2024
1 parent 5ce0535 commit 4c81a66
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 51 deletions.
8 changes: 6 additions & 2 deletions crates/diesel_models/src/query/user.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use async_bb8_diesel::AsyncRunQueryDsl;
use common_utils::pii;
use diesel::{
associations::HasTable, debug_query, result::Error as DieselError, ExpressionMethods,
JoinOnDsl, QueryDsl,
Expand Down Expand Up @@ -26,7 +27,10 @@ impl UserNew {
}

impl User {
pub async fn find_by_user_email(conn: &PgPooledConn, user_email: &str) -> StorageResult<Self> {
pub async fn find_by_user_email(
conn: &PgPooledConn,
user_email: &pii::Email,
) -> StorageResult<Self> {
generics::generic_find_one::<<Self as HasTable>::Table, _, _>(
conn,
users_dsl::email.eq(user_email.to_owned()),
Expand Down Expand Up @@ -62,7 +66,7 @@ impl User {

pub async fn update_by_user_email(
conn: &PgPooledConn,
user_email: &str,
user_email: &pii::Email,
user_update: UserUpdate,
) -> StorageResult<Self> {
generics::generic_update_with_unique_predicate_get_result::<
Expand Down
41 changes: 26 additions & 15 deletions crates/router/src/core/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ pub async fn signin_without_invite_checks(
) -> UserResponse<user_api::DashboardEntryResponse> {
let user_from_db: domain::UserFromStorage = state
.store
.find_user_by_email(request.email.clone().expose().expose().as_str())
.find_user_by_email(&request.email)
.await
.map_err(|e| {
if e.current_context().is_db_not_found() {
Expand Down Expand Up @@ -134,7 +134,7 @@ pub async fn signin(
) -> UserResponse<user_api::SignInResponse> {
let user_from_db: domain::UserFromStorage = state
.store
.find_user_by_email(request.email.clone().expose().expose().as_str())
.find_user_by_email(&request.email)
.await
.map_err(|e| {
if e.current_context().is_db_not_found() {
Expand Down Expand Up @@ -177,10 +177,7 @@ pub async fn connect_account(
state: AppState,
request: user_api::ConnectAccountRequest,
) -> UserResponse<user_api::ConnectAccountResponse> {
let find_user = state
.store
.find_user_by_email(request.email.clone().expose().expose().as_str())
.await;
let find_user = state.store.find_user_by_email(&request.email).await;

if let Ok(found_user) = find_user {
let user_from_db: domain::UserFromStorage = found_user.into();
Expand Down Expand Up @@ -340,7 +337,7 @@ pub async fn forgot_password(

let user_from_db = state
.store
.find_user_by_email(user_email.get_secret().expose().as_str())
.find_user_by_email(&user_email.into_inner())
.await
.map_err(|e| {
if e.current_context().is_db_not_found() {
Expand Down Expand Up @@ -389,7 +386,9 @@ pub async fn reset_password(
let user = state
.store
.update_user_by_email(
email_token.get_email(),
&email_token
.get_email()
.change_context(UserErrors::InternalServerError)?,
storage_user::UserUpdate::AccountUpdate {
name: None,
password: Some(hash_password),
Expand Down Expand Up @@ -462,7 +461,7 @@ pub async fn invite_user(

let invitee_user = state
.store
.find_user_by_email(invitee_email.clone().get_secret().expose().as_str())
.find_user_by_email(&invitee_email.clone().into_inner())
.await;

if let Ok(invitee_user) = invitee_user {
Expand Down Expand Up @@ -684,7 +683,7 @@ async fn handle_invitation(
let invitee_email = domain::UserEmail::from_pii_email(request.email.clone())?;
let invitee_user = state
.store
.find_user_by_email(invitee_email.clone().get_secret().expose().as_str())
.find_user_by_email(&invitee_email.into_inner())
.await;

if let Ok(invitee_user) = invitee_user {
Expand Down Expand Up @@ -882,7 +881,7 @@ pub async fn resend_invite(
let invitee_email = domain::UserEmail::from_pii_email(request.email)?;
let user: domain::UserFromStorage = state
.store
.find_user_by_email(invitee_email.clone().get_secret().expose().as_str())
.find_user_by_email(&invitee_email.clone().into_inner())
.await
.map_err(|e| {
if e.current_context().is_db_not_found() {
Expand Down Expand Up @@ -949,7 +948,11 @@ pub async fn accept_invite_from_email(

let user: domain::UserFromStorage = state
.store
.find_user_by_email(email_token.get_email())
.find_user_by_email(
&email_token
.get_email()
.change_context(UserErrors::InternalServerError)?,
)
.await
.change_context(UserErrors::InternalServerError)?
.into();
Expand Down Expand Up @@ -1326,7 +1329,11 @@ pub async fn verify_email_without_invite_checks(
auth::blacklist::check_email_token_in_blacklist(&state, &token).await?;
let user = state
.store
.find_user_by_email(email_token.get_email())
.find_user_by_email(
&email_token
.get_email()
.change_context(UserErrors::InternalServerError)?,
)
.await
.change_context(UserErrors::InternalServerError)?;
let user = state
Expand Down Expand Up @@ -1362,7 +1369,11 @@ pub async fn verify_email(

let user = state
.store
.find_user_by_email(email_token.get_email())
.find_user_by_email(
&email_token
.get_email()
.change_context(UserErrors::InternalServerError)?,
)
.await
.change_context(UserErrors::InternalServerError)?;

Expand Down Expand Up @@ -1411,7 +1422,7 @@ pub async fn send_verification_mail(
let user_email = domain::UserEmail::try_from(req.email)?;
let user = state
.store
.find_user_by_email(user_email.clone().get_secret().expose().as_str())
.find_user_by_email(&user_email.into_inner())
.await
.map_err(|e| {
if e.current_context().is_db_not_found() {
Expand Down
8 changes: 1 addition & 7 deletions crates/router/src/core/user_role.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use api_models::{user as user_api, user_role as user_role_api};
use diesel_models::{enums::UserStatus, user_role::UserRoleUpdate};
use error_stack::{report, ResultExt};
use masking::ExposeInterface;
use router_env::logger;

use crate::{
Expand Down Expand Up @@ -230,12 +229,7 @@ pub async fn delete_user_role(
) -> UserResponse<()> {
let user_from_db: domain::UserFromStorage = state
.store
.find_user_by_email(
domain::UserEmail::from_pii_email(request.email)?
.get_secret()
.expose()
.as_str(),
)
.find_user_by_email(&domain::UserEmail::from_pii_email(request.email)?.into_inner())
.await
.map_err(|e| {
if e.current_context().is_db_not_found() {
Expand Down
6 changes: 3 additions & 3 deletions crates/router/src/db/kafka_store.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::sync::Arc;

use common_enums::enums::MerchantStorageScheme;
use common_utils::errors::CustomResult;
use common_utils::{errors::CustomResult, pii};
use data_models::payments::{
payment_attempt::PaymentAttemptInterface, payment_intent::PaymentIntentInterface,
};
Expand Down Expand Up @@ -2269,7 +2269,7 @@ impl UserInterface for KafkaStore {

async fn find_user_by_email(
&self,
user_email: &str,
user_email: &pii::Email,
) -> CustomResult<storage::User, errors::StorageError> {
self.diesel_store.find_user_by_email(user_email).await
}
Expand All @@ -2293,7 +2293,7 @@ impl UserInterface for KafkaStore {

async fn update_user_by_email(
&self,
user_email: &str,
user_email: &pii::Email,
user: storage::UserUpdate,
) -> CustomResult<storage::User, errors::StorageError> {
self.diesel_store
Expand Down
29 changes: 11 additions & 18 deletions crates/router/src/db/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use super::MockDb;
use crate::{
connection,
core::errors::{self, CustomResult},
pii,
services::Store,
};
pub mod sample_data;
Expand All @@ -20,7 +21,7 @@ pub trait UserInterface {

async fn find_user_by_email(
&self,
user_email: &str,
user_email: &pii::Email,
) -> CustomResult<storage::User, errors::StorageError>;

async fn find_user_by_id(
Expand All @@ -36,7 +37,7 @@ pub trait UserInterface {

async fn update_user_by_email(
&self,
user_email: &str,
user_email: &pii::Email,
user: storage::UserUpdate,
) -> CustomResult<storage::User, errors::StorageError>;

Expand Down Expand Up @@ -68,7 +69,7 @@ impl UserInterface for Store {
#[instrument(skip_all)]
async fn find_user_by_email(
&self,
user_email: &str,
user_email: &pii::Email,
) -> CustomResult<storage::User, errors::StorageError> {
let conn = connection::pg_connection_write(self).await?;
storage::User::find_by_user_email(&conn, user_email)
Expand Down Expand Up @@ -102,7 +103,7 @@ impl UserInterface for Store {
#[instrument(skip_all)]
async fn update_user_by_email(
&self,
user_email: &str,
user_email: &pii::Email,
user: storage::UserUpdate,
) -> CustomResult<storage::User, errors::StorageError> {
let conn = connection::pg_connection_write(self).await?;
Expand Down Expand Up @@ -168,20 +169,16 @@ impl UserInterface for MockDb {

async fn find_user_by_email(
&self,
user_email: &str,
user_email: &pii::Email,
) -> CustomResult<storage::User, errors::StorageError> {
let users = self.users.lock().await;
let user_email_pii: common_utils::pii::Email = user_email
.to_string()
.try_into()
.map_err(|_| errors::StorageError::MockDbError)?;
users
.iter()
.find(|user| user.email == user_email_pii)
.find(|user| user.email.eq(user_email))
.cloned()
.ok_or(
errors::StorageError::ValueNotFound(format!(
"No user available for email = {user_email}"
"No user available for email = {user_email:?}"
))
.into(),
)
Expand Down Expand Up @@ -246,17 +243,13 @@ impl UserInterface for MockDb {

async fn update_user_by_email(
&self,
user_email: &str,
user_email: &pii::Email,
update_user: storage::UserUpdate,
) -> CustomResult<storage::User, errors::StorageError> {
let mut users = self.users.lock().await;
let user_email_pii: common_utils::pii::Email = user_email
.to_string()
.try_into()
.map_err(|_| errors::StorageError::MockDbError)?;
users
.iter_mut()
.find(|user| user.email == user_email_pii)
.find(|user| user.email.eq(user_email))
.map(|user| {
*user = match &update_user {
storage::UserUpdate::VerifyUser => storage::User {
Expand All @@ -282,7 +275,7 @@ impl UserInterface for MockDb {
})
.ok_or(
errors::StorageError::ValueNotFound(format!(
"No user available for user_email = {user_email}"
"No user available for user_email = {user_email:?}"
))
.into(),
)
Expand Down
9 changes: 6 additions & 3 deletions crates/router/src/services/email/types.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use api_models::user::dashboard_metadata::ProdIntent;
use common_utils::errors::CustomResult;
use common_utils::{
errors::{self, CustomResult},
pii,
};
use error_stack::ResultExt;
use external_services::email::{EmailContents, EmailData, EmailError};
use masking::{ExposeInterface, PeekInterface, Secret};
Expand Down Expand Up @@ -167,8 +170,8 @@ impl EmailToken {
jwt::generate_jwt(&token_payload, settings).await
}

pub fn get_email(&self) -> &str {
self.email.as_str()
pub fn get_email(&self) -> CustomResult<pii::Email, errors::ParsingError> {
pii::Email::try_from(self.email.clone())
}

pub fn get_merchant_id(&self) -> Option<&str> {
Expand Down
2 changes: 1 addition & 1 deletion crates/router/src/types/domain/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ impl NewUser {
pub async fn check_if_already_exists_in_db(&self, state: AppState) -> UserResult<()> {
if state
.store
.find_user_by_email(self.get_email().into_inner().expose().expose().as_str())
.find_user_by_email(&self.get_email().into_inner())
.await
.is_ok()
{
Expand Down
4 changes: 2 additions & 2 deletions crates/router/src/utils/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use api_models::user as user_api;
use common_utils::errors::CustomResult;
use diesel_models::{enums::UserStatus, user_role::UserRole};
use error_stack::ResultExt;
use masking::{ExposeInterface, Secret};
use masking::Secret;

use crate::{
core::errors::{StorageError, UserErrors, UserResult},
Expand Down Expand Up @@ -180,7 +180,7 @@ pub async fn get_user_from_db_by_email(
) -> CustomResult<UserFromStorage, StorageError> {
state
.store
.find_user_by_email(email.get_secret().expose().as_str())
.find_user_by_email(&email.into_inner())
.await
.map(UserFromStorage::from)
}
Expand Down

0 comments on commit 4c81a66

Please sign in to comment.