Skip to content

Commit

Permalink
refactor(users): Use domain email type in user DB functions (#6699)
Browse files Browse the repository at this point in the history
  • Loading branch information
ThisIsMani authored Nov 29, 2024
1 parent 96393ff commit 55fe82f
Show file tree
Hide file tree
Showing 12 changed files with 68 additions and 67 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,11 @@ pub enum ApiErrorResponse {
LinkConfigurationError { message: String },
#[error(error_type = ErrorType::InvalidRequestError, code = "IR_41", message = "Payout validation failed")]
PayoutFailed { data: Option<serde_json::Value> },
#[error(
error_type = ErrorType::InvalidRequestError, code = "IR_42",
message = "Cookies are not found in the request"
)]
CookieNotFound,

#[error(error_type = ErrorType::InvalidRequestError, code = "WE_01", message = "Failed to authenticate the webhook")]
WebhookAuthenticationFailed,
Expand Down Expand Up @@ -627,6 +632,9 @@ impl ErrorSwitch<api_models::errors::types::ApiErrorResponse> for ApiErrorRespon
Self::PayoutFailed { data } => {
AER::BadRequest(ApiError::new("IR", 41, "Payout failed while processing with connector.", Some(Extra { data: data.clone(), ..Default::default()})))
},
Self::CookieNotFound => {
AER::Unauthorized(ApiError::new("IR", 42, "Cookies are not found in the request", None))
},

Self::WebhookAuthenticationFailed => {
AER::Unauthorized(ApiError::new("WE", 1, "Webhook authentication failed", None))
Expand Down
3 changes: 2 additions & 1 deletion crates/router/src/compatibility/stripe/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,8 @@ impl From<errors::ApiErrorResponse> for StripeErrorCode {
| errors::ApiErrorResponse::GenericUnauthorized { .. }
| errors::ApiErrorResponse::AccessForbidden { .. }
| errors::ApiErrorResponse::InvalidCookie
| errors::ApiErrorResponse::InvalidEphemeralKey => Self::Unauthorized,
| errors::ApiErrorResponse::InvalidEphemeralKey
| errors::ApiErrorResponse::CookieNotFound => Self::Unauthorized,
errors::ApiErrorResponse::InvalidRequestUrl
| errors::ApiErrorResponse::InvalidHttpMethod
| errors::ApiErrorResponse::InvalidCardIin
Expand Down
44 changes: 14 additions & 30 deletions crates/router/src/core/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ pub async fn signin_token_only_flow(
) -> UserResponse<user_api::TokenResponse> {
let user_from_db: domain::UserFromStorage = state
.global_store
.find_user_by_email(&request.email)
.find_user_by_email(&domain::UserEmail::from_pii_email(request.email)?)
.await
.to_not_found_response(UserErrors::InvalidCredentials)?
.into();
Expand All @@ -191,7 +191,10 @@ pub async fn connect_account(
request: user_api::ConnectAccountRequest,
auth_id: Option<String>,
) -> UserResponse<user_api::ConnectAccountResponse> {
let find_user = state.global_store.find_user_by_email(&request.email).await;
let find_user = state
.global_store
.find_user_by_email(&domain::UserEmail::from_pii_email(request.email.clone())?)
.await;

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

let user_from_db = state
.global_store
.find_user_by_email(&user_email.into_inner())
.find_user_by_email(&user_email)
.await
.map_err(|e| {
if e.current_context().is_db_not_found() {
Expand Down Expand Up @@ -453,11 +456,7 @@ pub async fn reset_password_token_only_flow(

let user_from_db: domain::UserFromStorage = state
.global_store
.find_user_by_email(
&email_token
.get_email()
.change_context(UserErrors::InternalServerError)?,
)
.find_user_by_email(&email_token.get_email()?)
.await
.change_context(UserErrors::InternalServerError)?
.into();
Expand Down Expand Up @@ -564,10 +563,7 @@ async fn handle_invitation(
}

let invitee_email = domain::UserEmail::from_pii_email(request.email.clone())?;
let invitee_user = state
.global_store
.find_user_by_email(&invitee_email.into_inner())
.await;
let invitee_user = state.global_store.find_user_by_email(&invitee_email).await;

if let Ok(invitee_user) = invitee_user {
handle_existing_user_invitation(
Expand Down Expand Up @@ -958,7 +954,7 @@ pub async fn resend_invite(
let invitee_email = domain::UserEmail::from_pii_email(request.email)?;
let user: domain::UserFromStorage = state
.global_store
.find_user_by_email(&invitee_email.clone().into_inner())
.find_user_by_email(&invitee_email)
.await
.map_err(|e| {
if e.current_context().is_db_not_found() {
Expand Down Expand Up @@ -1065,11 +1061,7 @@ pub async fn accept_invite_from_email_token_only_flow(

let user_from_db: domain::UserFromStorage = state
.global_store
.find_user_by_email(
&email_token
.get_email()
.change_context(UserErrors::InternalServerError)?,
)
.find_user_by_email(&email_token.get_email()?)
.await
.change_context(UserErrors::InternalServerError)?
.into();
Expand Down Expand Up @@ -1525,11 +1517,7 @@ pub async fn verify_email_token_only_flow(

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

Expand Down Expand Up @@ -1572,7 +1560,7 @@ pub async fn send_verification_mail(
let user_email = domain::UserEmail::try_from(req.email)?;
let user = state
.global_store
.find_user_by_email(&user_email.into_inner())
.find_user_by_email(&user_email)
.await
.map_err(|e| {
if e.current_context().is_db_not_found() {
Expand Down Expand Up @@ -1669,11 +1657,7 @@ pub async fn user_from_email(

let user_from_db: domain::UserFromStorage = state
.global_store
.find_user_by_email(
&email_token
.get_email()
.change_context(UserErrors::InternalServerError)?,
)
.find_user_by_email(&email_token.get_email()?)
.await
.change_context(UserErrors::InternalServerError)?
.into();
Expand Down Expand Up @@ -2379,7 +2363,7 @@ pub async fn sso_sign(
// TODO: Use config to handle not found error
let user_from_db: domain::UserFromStorage = state
.global_store
.find_user_by_email(&email.into_inner())
.find_user_by_email(&email)
.await
.map(Into::into)
.to_not_found_response(UserErrors::UserNotFound)?;
Expand Down
2 changes: 1 addition & 1 deletion crates/router/src/core/user_role.rs
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ pub async fn delete_user_role(
) -> UserResponse<()> {
let user_from_db: domain::UserFromStorage = state
.global_store
.find_user_by_email(&domain::UserEmail::from_pii_email(request.email)?.into_inner())
.find_user_by_email(&domain::UserEmail::from_pii_email(request.email)?)
.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
Expand Up @@ -3,7 +3,7 @@ use std::sync::Arc;
use common_enums::enums::MerchantStorageScheme;
use common_utils::{
errors::CustomResult,
id_type, pii,
id_type,
types::{keymanager::KeyManagerState, theme::ThemeLineage},
};
use diesel_models::{
Expand Down Expand Up @@ -2983,7 +2983,7 @@ impl UserInterface for KafkaStore {

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

async fn update_user_by_email(
&self,
user_email: &pii::Email,
user_email: &domain::UserEmail,
user: storage::UserUpdate,
) -> CustomResult<storage::User, errors::StorageError> {
self.diesel_store
Expand Down
23 changes: 11 additions & 12 deletions crates/router/src/db/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,10 @@ use error_stack::report;
use masking::Secret;
use router_env::{instrument, tracing};

use super::MockDb;
use super::{domain, MockDb};
use crate::{
connection,
core::errors::{self, CustomResult},
pii,
services::Store,
};
pub mod sample_data;
Expand All @@ -22,7 +21,7 @@ pub trait UserInterface {

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

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

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

Expand Down Expand Up @@ -70,10 +69,10 @@ impl UserInterface for Store {
#[instrument(skip_all)]
async fn find_user_by_email(
&self,
user_email: &pii::Email,
user_email: &domain::UserEmail,
) -> CustomResult<storage::User, errors::StorageError> {
let conn = connection::pg_connection_read(self).await?;
storage::User::find_by_user_email(&conn, user_email)
storage::User::find_by_user_email(&conn, user_email.get_inner())
.await
.map_err(|error| report!(errors::StorageError::from(error)))
}
Expand Down Expand Up @@ -104,11 +103,11 @@ impl UserInterface for Store {
#[instrument(skip_all)]
async fn update_user_by_email(
&self,
user_email: &pii::Email,
user_email: &domain::UserEmail,
user: storage::UserUpdate,
) -> CustomResult<storage::User, errors::StorageError> {
let conn = connection::pg_connection_write(self).await?;
storage::User::update_by_user_email(&conn, user_email, user)
storage::User::update_by_user_email(&conn, user_email.get_inner(), user)
.await
.map_err(|error| report!(errors::StorageError::from(error)))
}
Expand Down Expand Up @@ -171,12 +170,12 @@ impl UserInterface for MockDb {

async fn find_user_by_email(
&self,
user_email: &pii::Email,
user_email: &domain::UserEmail,
) -> CustomResult<storage::User, errors::StorageError> {
let users = self.users.lock().await;
users
.iter()
.find(|user| user.email.eq(user_email))
.find(|user| user.email.eq(user_email.get_inner()))
.cloned()
.ok_or(
errors::StorageError::ValueNotFound(format!(
Expand Down Expand Up @@ -253,13 +252,13 @@ impl UserInterface for MockDb {

async fn update_user_by_email(
&self,
user_email: &pii::Email,
user_email: &domain::UserEmail,
update_user: storage::UserUpdate,
) -> CustomResult<storage::User, errors::StorageError> {
let mut users = self.users.lock().await;
users
.iter_mut()
.find(|user| user.email.eq(user_email))
.find(|user| user.email.eq(user_email.get_inner()))
.map(|user| {
*user = match &update_user {
storage::UserUpdate::VerifyUser => storage::User {
Expand Down
14 changes: 9 additions & 5 deletions crates/router/src/services/authentication.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2548,7 +2548,8 @@ where
T: serde::de::DeserializeOwned,
A: SessionStateInfo + Sync,
{
let cookie_token_result = get_cookie_from_header(headers).and_then(cookies::parse_cookie);
let cookie_token_result =
get_cookie_from_header(headers).and_then(cookies::get_jwt_from_cookies);
let auth_header_token_result = get_jwt_from_authorization_header(headers);
let force_cookie = state.conf().user.force_cookies;

Expand Down Expand Up @@ -3115,7 +3116,7 @@ pub fn is_ephemeral_auth<A: SessionStateInfo + Sync + Send>(
pub fn is_jwt_auth(headers: &HeaderMap) -> bool {
headers.get(headers::AUTHORIZATION).is_some()
|| get_cookie_from_header(headers)
.and_then(cookies::parse_cookie)
.and_then(cookies::get_jwt_from_cookies)
.is_ok()
}

Expand Down Expand Up @@ -3177,10 +3178,13 @@ pub fn get_jwt_from_authorization_header(headers: &HeaderMap) -> RouterResult<&s
}

pub fn get_cookie_from_header(headers: &HeaderMap) -> RouterResult<&str> {
headers
let cookie = headers
.get(cookies::get_cookie_header())
.and_then(|header_value| header_value.to_str().ok())
.ok_or(errors::ApiErrorResponse::InvalidCookie.into())
.ok_or(report!(errors::ApiErrorResponse::CookieNotFound))?;

cookie
.to_str()
.change_context(errors::ApiErrorResponse::InvalidCookie)
}

pub fn strip_jwt_token(token: &str) -> RouterResult<&str> {
Expand Down
6 changes: 3 additions & 3 deletions crates/router/src/services/authentication/cookies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,16 @@ pub fn remove_cookie_response() -> UserResponse<()> {
Ok(ApplicationResponse::JsonWithHeaders(((), header)))
}

pub fn parse_cookie(cookies: &str) -> RouterResult<String> {
pub fn get_jwt_from_cookies(cookies: &str) -> RouterResult<String> {
Cookie::split_parse(cookies)
.find_map(|cookie| {
cookie
.ok()
.filter(|parsed_cookie| parsed_cookie.name() == JWT_TOKEN_COOKIE_NAME)
.map(|parsed_cookie| parsed_cookie.value().to_owned())
})
.ok_or(report!(ApiErrorResponse::InvalidCookie))
.attach_printable("Cookie Parsing Failed")
.ok_or(report!(ApiErrorResponse::InvalidJwtToken))
.attach_printable("Unable to find JWT token in cookies")
}

#[cfg(feature = "olap")]
Expand Down
10 changes: 6 additions & 4 deletions crates/router/src/services/authorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,12 @@ where
i64::try_from(token.exp).change_context(ApiErrorResponse::InternalServerError)?;
let cache_ttl = token_expiry - common_utils::date_time::now_unix_timestamp();

set_role_info_in_cache(state, &token.role_id, &role_info, cache_ttl)
.await
.map_err(|e| logger::error!("Failed to set role info in cache {e:?}"))
.ok();
if cache_ttl > 0 {
set_role_info_in_cache(state, &token.role_id, &role_info, cache_ttl)
.await
.map_err(|e| logger::error!("Failed to set role info in cache {e:?}"))
.ok();
}
Ok(role_info)
}

Expand Down
11 changes: 5 additions & 6 deletions crates/router/src/services/email/types.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
use api_models::user::dashboard_metadata::ProdIntent;
use common_enums::EntityType;
use common_utils::{
errors::{self, CustomResult},
pii,
};
use common_utils::{errors::CustomResult, pii};
use error_stack::ResultExt;
use external_services::email::{EmailContents, EmailData, EmailError};
use masking::{ExposeInterface, Secret};
Expand Down Expand Up @@ -183,7 +180,7 @@ impl EmailToken {
entity: Option<Entity>,
flow: domain::Origin,
settings: &configs::Settings,
) -> CustomResult<String, UserErrors> {
) -> UserResult<String> {
let expiration_duration = std::time::Duration::from_secs(consts::EMAIL_TOKEN_TIME_IN_SECS);
let exp = jwt::generate_exp(expiration_duration)?.as_secs();
let token_payload = Self {
Expand All @@ -195,8 +192,10 @@ impl EmailToken {
jwt::generate_jwt(&token_payload, settings).await
}

pub fn get_email(&self) -> CustomResult<pii::Email, errors::ParsingError> {
pub fn get_email(&self) -> UserResult<domain::UserEmail> {
pii::Email::try_from(self.email.clone())
.change_context(UserErrors::InternalServerError)
.and_then(domain::UserEmail::from_pii_email)
}

pub fn get_entity(&self) -> Option<&Entity> {
Expand Down
Loading

0 comments on commit 55fe82f

Please sign in to comment.