From 8c8e88fc9943158f38014546b92bf08c30d59e81 Mon Sep 17 00:00:00 2001 From: Florian Merz Date: Thu, 24 Sep 2020 14:02:05 +0200 Subject: [PATCH] handle inactive users --- .../2020-02-14-142048_groupslist/up.sql | 13 ++- migrations/2020-08-25-110359_nonnda/up.sql | 1 - src/api/sudo.rs | 11 ++ src/db/internal/user.rs | 37 ++++++ src/db/operations/users.rs | 41 ++++++- tests/api/inactive.rs | 106 ++++++++++++++++++ tests/api/mod.rs | 1 + tests/helpers/users.rs | 4 + 8 files changed, 207 insertions(+), 7 deletions(-) create mode 100644 tests/api/inactive.rs diff --git a/migrations/2020-02-14-142048_groupslist/up.sql b/migrations/2020-02-14-142048_groupslist/up.sql index 9bc408c..6d617a5 100644 --- a/migrations/2020-02-14-142048_groupslist/up.sql +++ b/migrations/2020-02-14-142048_groupslist/up.sql @@ -1 +1,12 @@ -CREATE VIEW groups_list AS SELECT groups.name, groups.typ, groups.trust, count(memberships.user_uuid) as members_count FROM groups JOIN memberships ON groups.group_id = memberships.group_id GROUP BY groups.group_id; \ No newline at end of file +CREATE VIEW groups_list AS +SELECT + groups.name, + groups.typ, + groups.trust, + count(memberships.user_uuid) AS members_count +FROM + GROUPS + JOIN memberships ON groups.group_id = memberships.group_id +GROUP BY + groups.group_id; + diff --git a/migrations/2020-08-25-110359_nonnda/up.sql b/migrations/2020-08-25-110359_nonnda/up.sql index b14e35f..e4ab982 100644 --- a/migrations/2020-08-25-110359_nonnda/up.sql +++ b/migrations/2020-08-25-110359_nonnda/up.sql @@ -12,4 +12,3 @@ FROM users_staff u WHERE p.user_uuid = u.user_uuid; - diff --git a/src/api/sudo.rs b/src/api/sudo.rs index d65373a..cb81b62 100644 --- a/src/api/sudo.rs +++ b/src/api/sudo.rs @@ -193,6 +193,16 @@ async fn delete_inactive_group( .map_err(ApiError::GenericBadRequest) } +#[guard(Staff, Admin, Medium)] +async fn delete_inactive_users( + pool: web::Data, + scope_and_user: ScopeAndUser, +) -> Result { + operations::users::delete_inactive_users(&pool, &scope_and_user) + .map(|_| HttpResponse::Ok().json("")) + .map_err(ApiError::GenericBadRequest) +} + #[guard(Staff, Admin, Medium)] async fn subscribe_nda_mailing_list( pool: web::Data, @@ -272,6 +282,7 @@ pub fn sudo_app() -> impl HttpServiceFactory { web::resource("/member/{group_name}/{user_uuid}") .route(web::delete().to(remove_member::)), ) + .service(web::resource("/user/inactive").route(web::delete().to(delete_inactive_users))) .service(web::resource("/user/{uuid}").route(web::delete().to(delete_user))) .service(web::resource("/user/uuids/staff").route(web::get().to(all_staff_uuids))) .service(web::resource("/user/uuids/members").route(web::get().to(all_member_uuids))) diff --git a/src/db/internal/user.rs b/src/db/internal/user.rs index ee4f5b5..640990e 100644 --- a/src/db/internal/user.rs +++ b/src/db/internal/user.rs @@ -81,6 +81,9 @@ pub fn user_profile_by_user_id( } pub fn delete_user(connection: &PgConnection, user: &User) -> Result<(), Error> { + diesel::delete(schema::requests::table) + .filter(schema::requests::user_uuid.eq(user.user_uuid)) + .execute(connection)?; diesel::delete(schema::invitations::table) .filter(schema::invitations::user_uuid.eq(user.user_uuid)) .execute(connection)?; @@ -420,3 +423,37 @@ pub fn all_members(connection: &PgConnection) -> Result, Error> { .get_results::(connection) .map_err(Into::into) } + +use diesel::pg::expression::dsl::array; +use diesel::pg::types::sql_types::Array; +use diesel::pg::types::sql_types::Jsonb; +use diesel::pg::Pg; +use diesel::sql_types::Text; + +sql_function! { + fn jsonb_extract_path(from_json: Jsonb, path_elems: Array) -> Jsonb +} + +diesel_infix_operator!(ExtrPath, " #> ", Jsonb, backend: Pg); + +fn extr_path(left: T, right: U) -> ExtrPath +where + T: Expression, + U: Expression, +{ + ExtrPath::new(left, right) +} + +pub fn all_inactive(connection: &PgConnection) -> Result, Error> { + schema::profiles::table + .filter( + extr_path( + schema::profiles::profile, + array::(("active".to_string(), "value".to_string())), + ) + .eq(serde_json::Value::from(false)), + ) + .select(schema::profiles::user_uuid) + .get_results::(connection) + .map_err(Into::into) +} diff --git a/src/db/operations/users.rs b/src/db/operations/users.rs index f1f82dd..2bb946c 100644 --- a/src/db/operations/users.rs +++ b/src/db/operations/users.rs @@ -1,3 +1,4 @@ +use crate::cis::operations::send_groups_to_cis; use crate::db::internal; use crate::db::logs::log_comment_body; use crate::db::operations::members::revoke_memberships_by_trust; @@ -17,6 +18,7 @@ use cis_client::AsyncCisClientTrait; use cis_profile::schema::Profile; use dino_park_gate::scope::ScopeAndUser; use failure::Error; +use log::info; use std::sync::Arc; use uuid::Uuid; @@ -116,17 +118,20 @@ pub async fn update_user_cache( profile: &Profile, cis_client: Arc, ) -> Result<(), Error> { + let user_uuid = Uuid::parse_str(&profile.uuid.value.clone().ok_or(PacksError::NoUuid)?)?; + if profile.active.value == Some(false) { + return delete_user(pool, &User { user_uuid }); + } let connection = pool.get()?; let new_trust = trust_for_profile(&profile); - let uuid = Uuid::parse_str(&profile.uuid.value.clone().ok_or(PacksError::NoUuid)?)?; - let old_profile = internal::user::user_profile_by_uuid_maybe(&connection, &uuid)?; + let old_profile = internal::user::user_profile_by_uuid_maybe(&connection, &user_uuid)?; internal::user::update_user_cache(&connection, profile)?; if let Some(old_profile) = old_profile { let old_trust = trust_for_profile(&old_profile.profile); drop(connection); let remove_groups = RemoveGroups { - user: User { user_uuid: uuid }, + user: User { user_uuid }, group_names: &[], force: true, notify: true, @@ -142,7 +147,12 @@ pub async fn update_user_cache( ) .await?; } + } else if let Some(ref groups) = profile.access_information.mozilliansorg.values { + if !groups.0.is_empty() { + send_groups_to_cis(pool, cis_client, &user_uuid).await?; + } } + Ok(()) } @@ -156,13 +166,32 @@ pub fn user_profile_by_uuid(pool: &Pool, user_uuid: &Uuid) -> Result Result<(), Error> { + let connection = pool.get()?; + let host = internal::user::user_by_id(&connection, &scope_and_user.user_id)?; + ONLY_ADMINS.run(&RuleContext::minimal( + pool, + &scope_and_user, + "", + &host.user_uuid, + ))?; + let inactive_uuids = internal::user::all_inactive(&connection)?; + drop(connection); + info!("deleting {} users", inactive_uuids.len()); + for user_uuid in inactive_uuids { + delete_user(pool, &User { user_uuid })?; + info!("delete user {}", user_uuid); + } + Ok(()) +} + pub fn get_all_member_uuids( pool: &Pool, scope_and_user: &ScopeAndUser, ) -> Result, Error> { let connection = pool.get()?; let host = internal::user::user_by_id(&connection, &scope_and_user.user_id)?; - SEARCH_USERS.run(&RuleContext::minimal( + ONLY_ADMINS.run(&RuleContext::minimal( pool, &scope_and_user, "", @@ -174,7 +203,7 @@ pub fn get_all_member_uuids( pub fn get_all_staff_uuids(pool: &Pool, scope_and_user: &ScopeAndUser) -> Result, Error> { let connection = pool.get()?; let host = internal::user::user_by_id(&connection, &scope_and_user.user_id)?; - SEARCH_USERS.run(&RuleContext::minimal( + ONLY_ADMINS.run(&RuleContext::minimal( pool, scope_and_user, "", @@ -182,3 +211,5 @@ pub fn get_all_staff_uuids(pool: &Pool, scope_and_user: &ScopeAndUser) -> Result ))?; internal::user::all_staff(&connection) } + +pub use internal::user::update_user_cache as _update_user_cache; diff --git a/tests/api/inactive.rs b/tests/api/inactive.rs new file mode 100644 index 0000000..d51a8b5 --- /dev/null +++ b/tests/api/inactive.rs @@ -0,0 +1,106 @@ +use crate::helpers::api::*; +use crate::helpers::db::get_pool; +use crate::helpers::db::reset; +use crate::helpers::misc::read_json; +use crate::helpers::misc::test_app_and_cis; +use crate::helpers::misc::Soa; +use crate::helpers::sudo::add_to_group; +use crate::helpers::users::basic_user; +use crate::helpers::users::user_id; +use actix_web::test; +use actix_web::App; +use cis_client::getby::GetBy; +use cis_client::AsyncCisClientTrait; +use dino_park_packs::db::operations::users::_update_user_cache; +use dino_park_packs::db::operations::users::update_user_cache; +use failure::Error; +use serde_json::json; +use std::sync::Arc; + +#[actix_rt::test] +async fn update_inactive() -> Result<(), Error> { + reset()?; + let (service, cis_client) = test_app_and_cis().await; + let cis_client = Arc::new(cis_client); + let app = App::new().service(service); + let mut app = test::init_service(app).await; + + let host_user = basic_user(1, true); + let mut staff_user_1 = basic_user(2, true); + let mut staff_user_2 = basic_user(3, true); + let host = Soa::from(&host_user).aal_medium(); + + let res = post( + &mut app, + "/groups/api/v1/groups", + json!({ "name": "inactive-test", "description": "a group", "trust": "Staff" }), + &host.clone().creator(), + ) + .await; + assert!(res.status().is_success()); + + let res = get(&mut app, "/groups/api/v1/groups", &host).await; + assert!(res.status().is_success()); + assert_eq!(read_json(res).await["groups"][0]["typ"], "Closed"); + + add_to_group(&mut app, &host, &staff_user_1, "inactive-test").await; + add_to_group(&mut app, &host, &staff_user_2, "inactive-test").await; + + let res = get(&mut app, "/groups/api/v1/members/inactive-test", &host).await; + assert!(res.status().is_success()); + let members = read_json(res).await; + assert_eq!(members["members"].as_array().map(|a| a.len()), Some(3)); + + let pool = get_pool(); + staff_user_2.active.value = Some(false); + update_user_cache(&pool, &staff_user_2, Arc::clone(&cis_client)).await?; + + let res = get(&mut app, "/groups/api/v1/members/inactive-test", &host).await; + assert!(res.status().is_success()); + let members = read_json(res).await; + assert_eq!(members["members"].as_array().map(|a| a.len()), Some(2)); + + // updating an inactive profile must not fail + update_user_cache(&pool, &staff_user_2, Arc::clone(&cis_client)).await?; + + let mut staff_user_2_reactivated = cis_client + .get_user_by(&user_id(&staff_user_2), &GetBy::Uuid, None) + .await?; + // enabling a user again with groups resets groups to db state + staff_user_2_reactivated.active.value = Some(true); + update_user_cache(&pool, &staff_user_2_reactivated, Arc::clone(&cis_client)).await?; + assert_eq!( + cis_client + .get_user_by(&user_id(&staff_user_2), &GetBy::Uuid, None) + .await? + .access_information + .mozilliansorg + .values + .map(|kv| kv.0.is_empty()), + Some(true) + ); + + staff_user_1.active.value = Some(false); + let connection = pool.get()?; + _update_user_cache(&connection, &staff_user_1)?; + + let res = get(&mut app, "/groups/api/v1/members/inactive-test", &host).await; + assert!(res.status().is_success()); + let members = read_json(res).await; + assert_eq!(members["members"].as_array().map(|a| a.len()), Some(2)); + + let res = delete( + &mut app, + "/groups/api/v1/sudo/user/inactive", + &host.clone().admin(), + ) + .await; + assert!(res.status().is_success()); + + let res = get(&mut app, "/groups/api/v1/members/inactive-test", &host).await; + assert!(res.status().is_success()); + let members = read_json(res).await; + assert_eq!(members["members"].as_array().map(|a| a.len()), Some(1)); + + Ok(()) +} diff --git a/tests/api/mod.rs b/tests/api/mod.rs index 7683520..a85ca43 100644 --- a/tests/api/mod.rs +++ b/tests/api/mod.rs @@ -7,6 +7,7 @@ mod errors; mod expiration; mod groups; mod import; +mod inactive; mod invitations; mod join; mod requests; diff --git a/tests/helpers/users.rs b/tests/helpers/users.rs index 8ec25f6..fc0ff34 100644 --- a/tests/helpers/users.rs +++ b/tests/helpers/users.rs @@ -30,6 +30,10 @@ pub fn user_uuid(p: &Profile) -> String { p.uuid.value.clone().unwrap() } +pub fn user_id(p: &Profile) -> String { + p.user_id.value.clone().unwrap() +} + pub fn user_email(p: &Profile) -> String { p.primary_email.value.clone().unwrap() }