Skip to content

Commit

Permalink
Cleanups and Fixes for Emergency Access
Browse files Browse the repository at this point in the history
- Several cleanups and code optimizations for Emergency Access
- Fixed a race-condition regarding jobs for Emergency Access
- Some other small changes like `allow(clippy::)` removals

Fixes dani-garcia#2925
  • Loading branch information
BlackDex committed Nov 26, 2022
1 parent 7a76731 commit 27903cc
Show file tree
Hide file tree
Showing 10 changed files with 147 additions and 114 deletions.
2 changes: 1 addition & 1 deletion src/api/admin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ async fn invite_user(data: Json<InviteData>, _token: AdminToken, mut conn: DbCon
if CONFIG.mail_enabled() {
mail::send_invite(&user.email, &user.uuid, None, None, &CONFIG.invitation_org_name(), None).await
} else {
let invitation = Invitation::new(user.email.clone());
let invitation = Invitation::new(&user.email);
invitation.save(conn).await
}
}
Expand Down
157 changes: 83 additions & 74 deletions src/api/core/emergency_access.rs

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions src/api/core/organizations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,7 @@ async fn send_invite(
}

if !CONFIG.mail_enabled() {
let invitation = Invitation::new(email.clone());
let invitation = Invitation::new(&email);
invitation.save(&mut conn).await?;
}

Expand Down Expand Up @@ -797,7 +797,7 @@ async fn _reinvite_user(org_id: &str, user_org: &str, invited_by_email: &str, co
)
.await?;
} else {
let invitation = Invitation::new(user.email);
let invitation = Invitation::new(&user.email);
invitation.save(conn).await?;
}

Expand Down
1 change: 0 additions & 1 deletion src/api/icons.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,6 @@ mod tests {

use cached::proc_macro::cached;
#[cached(key = "String", convert = r#"{ domain.to_string() }"#, size = 16, time = 60)]
#[allow(clippy::unused_async)] // This is needed because cached causes a false-positive here.
async fn is_domain_blacklisted(domain: &str) -> bool {
if CONFIG.icon_blacklist_non_global_ips() {
if let Ok(s) = lookup_host((domain, 0)).await {
Expand Down
12 changes: 6 additions & 6 deletions src/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,17 +177,17 @@ pub struct EmergencyAccessInviteJwtClaims {
pub sub: String,

pub email: String,
pub emer_id: Option<String>,
pub grantor_name: Option<String>,
pub grantor_email: Option<String>,
pub emer_id: String,
pub grantor_name: String,
pub grantor_email: String,
}

pub fn generate_emergency_access_invite_claims(
uuid: String,
email: String,
emer_id: Option<String>,
grantor_name: Option<String>,
grantor_email: Option<String>,
emer_id: String,
grantor_name: String,
grantor_email: String,
) -> EmergencyAccessInviteJwtClaims {
let time_now = Utc::now().naive_utc();
let expire_hours = i64::from(CONFIG.invitation_expiration_hours());
Expand Down
1 change: 0 additions & 1 deletion src/db/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ macro_rules! generate_connections {

impl DbPool {
// For the given database URL, guess its type, run migrations, create pool, and return it
#[allow(clippy::diverging_sub_expression)]
pub fn from_config() -> Result<Self, Error> {
let url = CONFIG.database_url();
let conn_type = DbConnType::from_url(&url)?;
Expand Down
63 changes: 45 additions & 18 deletions src/db/models/emergency_access.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use serde_json::Value;
use super::User;

db_object! {
#[derive(Debug, Identifiable, Queryable, Insertable, AsChangeset)]
#[derive(Identifiable, Queryable, Insertable, AsChangeset)]
#[diesel(table_name = emergency_access)]
#[diesel(treat_none_as_null = true)]
#[diesel(primary_key(uuid))]
Expand All @@ -27,14 +27,14 @@ db_object! {
/// Local methods

impl EmergencyAccess {
pub fn new(grantor_uuid: String, email: Option<String>, status: i32, atype: i32, wait_time_days: i32) -> Self {
pub fn new(grantor_uuid: String, email: String, status: i32, atype: i32, wait_time_days: i32) -> Self {
let now = Utc::now().naive_utc();

Self {
uuid: crate::util::get_uuid(),
grantor_uuid,
grantee_uuid: None,
email,
email: Some(email),
status,
atype,
wait_time_days,
Expand Down Expand Up @@ -87,7 +87,6 @@ impl EmergencyAccess {
})
}

#[allow(clippy::manual_map)]
pub async fn to_json_grantee_details(&self, conn: &mut DbConn) -> Value {
let grantee_user = if let Some(grantee_uuid) = self.grantee_uuid.as_deref() {
Some(User::find_by_uuid(grantee_uuid, conn).await.expect("Grantee user not found."))
Expand All @@ -110,7 +109,7 @@ impl EmergencyAccess {
}
}

#[derive(Copy, Clone, PartialEq, Eq, num_derive::FromPrimitive)]
#[derive(Copy, Clone)]
pub enum EmergencyAccessType {
View = 0,
Takeover = 1,
Expand All @@ -126,18 +125,6 @@ impl EmergencyAccessType {
}
}

impl PartialEq<i32> for EmergencyAccessType {
fn eq(&self, other: &i32) -> bool {
*other == *self as i32
}
}

impl PartialEq<EmergencyAccessType> for i32 {
fn eq(&self, other: &EmergencyAccessType) -> bool {
*self == *other as i32
}
}

pub enum EmergencyAccessStatus {
Invited = 0,
Accepted = 1,
Expand Down Expand Up @@ -189,6 +176,45 @@ impl EmergencyAccess {
}
}

pub async fn update_access_status_and_save(
&mut self,
status: i32,
date: &NaiveDateTime,
conn: &mut DbConn,
) -> EmptyResult {
// Update the grantee so that it will refresh it's status.
User::update_uuid_revision(self.grantee_uuid.as_ref().expect("Error getting grantee"), conn).await;
self.status = status;
self.updated_at = date.to_owned();

db_run! {conn: {
crate::util::retry(|| {
diesel::update(emergency_access::table.filter(emergency_access::uuid.eq(&self.uuid)))
.set((emergency_access::status.eq(status), emergency_access::updated_at.eq(date)))
.execute(conn)
}, 10)
.map_res("Error updating emergency access status")
}}
}

pub async fn update_last_notification_date_and_save(
&mut self,
date: &NaiveDateTime,
conn: &mut DbConn,
) -> EmptyResult {
self.last_notification_at = Some(date.to_owned());
self.updated_at = date.to_owned();

db_run! {conn: {
crate::util::retry(|| {
diesel::update(emergency_access::table.filter(emergency_access::uuid.eq(&self.uuid)))
.set((emergency_access::last_notification_at.eq(date), emergency_access::updated_at.eq(date)))
.execute(conn)
}, 10)
.map_res("Error updating emergency access status")
}}
}

pub async fn delete_all_by_user(user_uuid: &str, conn: &mut DbConn) -> EmptyResult {
for ea in Self::find_all_by_grantor_uuid(user_uuid, conn).await {
ea.delete(conn).await?;
Expand Down Expand Up @@ -233,10 +259,11 @@ impl EmergencyAccess {
}}
}

pub async fn find_all_recoveries(conn: &mut DbConn) -> Vec<Self> {
pub async fn find_all_recoveries_initiated(conn: &mut DbConn) -> Vec<Self> {
db_run! { conn: {
emergency_access::table
.filter(emergency_access::status.eq(EmergencyAccessStatus::RecoveryInitiated as i32))
.filter(emergency_access::recovery_initiated_at.is_not_null())
.load::<EmergencyAccessDb>(conn).expect("Error loading emergency_access").from_db()
}}
}
Expand Down
2 changes: 1 addition & 1 deletion src/db/models/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ impl User {
}

impl Invitation {
pub fn new(email: String) -> Self {
pub fn new(email: &str) -> Self {
let email = email.to_lowercase();
Self {
email,
Expand Down
1 change: 0 additions & 1 deletion src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,6 @@ impl<S> MapResult<S> for Option<S> {
}
}

#[allow(clippy::unnecessary_wraps)]
const fn _has_source<T>(e: T) -> Option<T> {
Some(e)
}
Expand Down
18 changes: 9 additions & 9 deletions src/mail.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,16 +256,16 @@ pub async fn send_invite(
pub async fn send_emergency_access_invite(
address: &str,
uuid: &str,
emer_id: Option<String>,
grantor_name: Option<String>,
grantor_email: Option<String>,
emer_id: &str,
grantor_name: &str,
grantor_email: &str,
) -> EmptyResult {
let claims = generate_emergency_access_invite_claims(
uuid.to_string(),
String::from(uuid),
String::from(address),
emer_id.clone(),
grantor_name.clone(),
grantor_email,
String::from(emer_id),
String::from(grantor_name),
String::from(grantor_email),
);

let invite_token = encode_jwt(&claims);
Expand All @@ -275,7 +275,7 @@ pub async fn send_emergency_access_invite(
json!({
"url": CONFIG.domain(),
"img_src": CONFIG._smtp_img_src(),
"emer_id": emer_id.unwrap_or_else(|| "_".to_string()),
"emer_id": emer_id,
"email": percent_encode(address.as_bytes(), NON_ALPHANUMERIC).to_string(),
"grantor_name": grantor_name,
"token": invite_token,
Expand Down Expand Up @@ -328,7 +328,7 @@ pub async fn send_emergency_access_recovery_initiated(
address: &str,
grantee_name: &str,
atype: &str,
wait_time_days: &str,
wait_time_days: &i32,
) -> EmptyResult {
let (subject, body_html, body_text) = get_text(
"email/emergency_access_recovery_initiated",
Expand Down

0 comments on commit 27903cc

Please sign in to comment.