From 06a5a22032680162b9e3aae144de6b4997026491 Mon Sep 17 00:00:00 2001 From: Zack Fu Zi Xiang Date: Sun, 1 Sep 2024 00:51:05 +0800 Subject: [PATCH 1/7] feat: delete user --- ...22939e80ec2e75747503f0493d081ba43e4bd.json | 14 +++++++++ libs/client-api/src/http.rs | 13 +++++++++ libs/database/src/user.rs | 21 ++++++++++++++ src/api/user.rs | 12 +++++++- src/biz/user/user_info.rs | 4 +++ tests/user/delete.rs | 29 +++++++++++++++++++ 6 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 .sqlx/query-e6acbe78f0e8f776901c560088222939e80ec2e75747503f0493d081ba43e4bd.json diff --git a/.sqlx/query-e6acbe78f0e8f776901c560088222939e80ec2e75747503f0493d081ba43e4bd.json b/.sqlx/query-e6acbe78f0e8f776901c560088222939e80ec2e75747503f0493d081ba43e4bd.json new file mode 100644 index 000000000..2bb9c80d7 --- /dev/null +++ b/.sqlx/query-e6acbe78f0e8f776901c560088222939e80ec2e75747503f0493d081ba43e4bd.json @@ -0,0 +1,14 @@ +{ + "db_name": "PostgreSQL", + "query": "\n DELETE FROM auth.users WHERE id = $1\n ", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Uuid" + ] + }, + "nullable": [] + }, + "hash": "e6acbe78f0e8f776901c560088222939e80ec2e75747503f0493d081ba43e4bd" +} diff --git a/libs/client-api/src/http.rs b/libs/client-api/src/http.rs index 18314a6ea..fd944373a 100644 --- a/libs/client-api/src/http.rs +++ b/libs/client-api/src/http.rs @@ -738,6 +738,19 @@ impl Client { AppResponse::<()>::from_response(resp).await?.into_error() } + /// Deletes the user account and all associated data. + #[instrument(level = "info", skip_all, err)] + pub async fn delete_user(&self) -> Result<(), AppResponseError> { + let url = format!("{}/api/user", self.base_url); + let resp = self + .http_client_with_auth(Method::DELETE, &url) + .await? + .send() + .await?; + log_request_id(&resp); + AppResponse::<()>::from_response(resp).await?.into_error() + } + pub async fn get_snapshot_list( &self, workspace_id: &str, diff --git a/libs/database/src/user.rs b/libs/database/src/user.rs index 4ea6f7c99..599caa84e 100644 --- a/libs/database/src/user.rs +++ b/libs/database/src/user.rs @@ -240,3 +240,24 @@ pub async fn select_name_from_uuid(pool: &PgPool, user_uuid: &Uuid) -> Result Result<(), AppError> { + let res = sqlx::query!( + r#" + DELETE FROM auth.users WHERE id = $1 + "#, + user_uuid + ) + .execute(pool) + .await?; + + if res.rows_affected() != 1 { + return Err(AppError::RecordNotFound(format!( + "User with UUID {} not found", + user_uuid + ))); + } + + Ok(()) +} diff --git a/src/api/user.rs b/src/api/user.rs index f866223f1..af0ff84ce 100644 --- a/src/api/user.rs +++ b/src/api/user.rs @@ -1,4 +1,4 @@ -use crate::biz::user::user_info::{get_profile, get_user_workspace_info, update_user}; +use crate::biz::user::user_info::{delete_user, get_profile, get_user_workspace_info, update_user}; use crate::biz::user::user_verify::verify_token; use crate::state::AppState; use actix_web::web::{Data, Json}; @@ -16,6 +16,7 @@ pub fn user_scope() -> Scope { .service(web::resource("/update").route(web::post().to(update_user_handler))) .service(web::resource("/profile").route(web::get().to(get_user_profile_handler))) .service(web::resource("/workspace").route(web::get().to(get_user_workspace_info_handler))) + .service(web::resource("").route(web::delete().to(delete_user_handler))) } #[tracing::instrument(skip(state, path), err)] @@ -61,3 +62,12 @@ async fn update_user_handler( update_user(&state.pg_pool, auth.uuid()?, params).await?; Ok(AppResponse::Ok().into()) } + +#[tracing::instrument(skip(state), err)] +async fn delete_user_handler( + auth: Authorization, + state: Data, +) -> Result> { + delete_user(&state.pg_pool, auth.uuid()?).await?; + Ok(AppResponse::Ok().into()) +} diff --git a/src/biz/user/user_info.rs b/src/biz/user/user_info.rs index 470e39a45..6ae58dad0 100644 --- a/src/biz/user/user_info.rs +++ b/src/biz/user/user_info.rs @@ -74,3 +74,7 @@ pub async fn update_user( let metadata = params.metadata.map(|m| json!(m.into_inner())); Ok(database::user::update_user(pg_pool, &user_uuid, params.name, params.email, metadata).await?) } + +pub async fn delete_user(pg_pool: &PgPool, user_uuid: Uuid) -> Result<(), AppResponseError> { + Ok(database::user::delete_user(pg_pool, &user_uuid).await?) +} diff --git a/tests/user/delete.rs b/tests/user/delete.rs index c8324ed73..fe451a522 100644 --- a/tests/user/delete.rs +++ b/tests/user/delete.rs @@ -1,6 +1,35 @@ use client_api_test::*; use gotrue::params::{AdminDeleteUserParams, AdminUserParams}; +#[tokio::test] +async fn user_delete_self() { + let (client, user) = generate_unique_registered_user_client().await; + let admin_client = admin_user_client().await; + { + // user found before deletion + let search_result = admin_client + .admin_list_users(Some(&user.email)) + .await + .unwrap(); + let _target_user = search_result + .into_iter() + .find(|u| u.email == user.email) + .unwrap(); + } + + client.delete_user().await.unwrap(); + + { + // user cannot be found after deletion + let search_result = admin_client + .admin_list_users(Some(&user.email)) + .await + .unwrap(); + let target_user = search_result.into_iter().find(|u| u.email == user.email); + assert!(target_user.is_none(), "User should be deleted: {:?}", user); + } +} + #[tokio::test] async fn admin_delete_create_same_user_hard() { let (client, user) = generate_unique_registered_user_client().await; From 89f2c7003bcaedb98ad05b14e93e7c55c5acf808 Mon Sep 17 00:00:00 2001 From: Zack Fu Zi Xiang Date: Mon, 2 Sep 2024 11:14:04 +0800 Subject: [PATCH 2/7] feat: support user deletion --- dev.env | 5 +++ libs/app-error/src/lib.rs | 1 + src/api/user.rs | 65 +++++++++++++++++++++++++++++++++++++++ src/config/config.rs | 11 +++++++ 4 files changed, 82 insertions(+) diff --git a/dev.env b/dev.env index ee3683184..769e8cdc1 100644 --- a/dev.env +++ b/dev.env @@ -68,6 +68,11 @@ GOTRUE_EXTERNAL_DISCORD_ENABLED=false GOTRUE_EXTERNAL_DISCORD_CLIENT_ID= GOTRUE_EXTERNAL_DISCORD_SECRET= GOTRUE_EXTERNAL_DISCORD_REDIRECT_URI=http://localhost:9999/callback +# Apple OAuth2 +GOTRUE_EXTERNAL_APPLE_ENABLED=false +GOTRUE_EXTERNAL_APPLE_CLIENT_ID= +GOTRUE_EXTERNAL_APPLE_SECRET= +GOTRUE_EXTERNAL_APPLE_REDIRECT_URI=http://localhost:9999/callback # File Storage APPFLOWY_S3_USE_MINIO=true diff --git a/libs/app-error/src/lib.rs b/libs/app-error/src/lib.rs index e2a47176d..17e1ce257 100644 --- a/libs/app-error/src/lib.rs +++ b/libs/app-error/src/lib.rs @@ -317,6 +317,7 @@ pub enum ErrorCode { SqlxArgEncodingError = 1035, InvalidContentType = 1036, SingleUploadLimitExceeded = 1037, + AppleRevokeTokenError = 1038, } impl ErrorCode { diff --git a/src/api/user.rs b/src/api/user.rs index af0ff84ce..025bfdee1 100644 --- a/src/api/user.rs +++ b/src/api/user.rs @@ -4,8 +4,10 @@ use crate::state::AppState; use actix_web::web::{Data, Json}; use actix_web::Result; use actix_web::{web, Scope}; +use app_error::ErrorCode; use authentication::jwt::{Authorization, UserUuid}; use database_entity::dto::{AFUserProfile, AFUserWorkspaceInfo}; +use secrecy::ExposeSecret; use shared_entity::dto::auth_dto::{SignInTokenResponse, UpdateUserParams}; use shared_entity::response::AppResponseError; use shared_entity::response::{AppResponse, JsonAppResponse}; @@ -69,5 +71,68 @@ async fn delete_user_handler( state: Data, ) -> Result> { delete_user(&state.pg_pool, auth.uuid()?).await?; + + if is_apple_user(auth) { + if let Err(err) = revoke_apple_token( + &state.config.apple_oauth.client_id, + state.config.apple_oauth.client_secret.expose_secret(), + "TODO: get original apple during oauth", + ) + .await + { + tracing::warn!("revoke apple token failed: {:?}", err); + }; + } Ok(AppResponse::Ok().into()) } + +fn is_apple_user(auth: Authorization) -> bool { + if let Some(provider) = auth.claims.app_metadata.get("provider") { + if provider == "apple" { + return true; + } + }; + + if let Some(providers) = auth.claims.app_metadata.get("providers") { + if let Some(providers) = providers.as_array() { + for provider in providers { + if provider == "apple" { + return true; + } + } + } + } + + false +} + +/// Based on: https://developer.apple.com/documentation/sign_in_with_apple/revoke_tokens +async fn revoke_apple_token( + apple_client_id: &str, + apple_client_secret: &str, + apple_user_token: &str, +) -> Result<(), AppResponseError> { + let resp = reqwest::Client::new() + .post("https://appleid.apple.com/auth/revoke") + .form(&[ + ("client_id", apple_client_id), + ("client_secret", apple_client_secret), + ("token", apple_user_token), + ]) + .send() + .await?; + + let status = resp.status(); + if status.is_success() { + return Ok(()); + } + + let payload = resp.text().await?; + Err(AppResponseError::new( + ErrorCode::AppleRevokeTokenError, + format!( + "calling apple revoke, code: {}, message: {}", + status, payload + ), + )) +} diff --git a/src/config/config.rs b/src/config/config.rs index 349c8ec14..274ba86cc 100644 --- a/src/config/config.rs +++ b/src/config/config.rs @@ -22,6 +22,7 @@ pub struct Config { pub grpc_history: GrpcHistorySetting, pub collab: CollabSetting, pub mailer: MailerSetting, + pub apple_oauth: AppleOAuthSetting, } #[derive(serde::Deserialize, Clone, Debug)] @@ -32,6 +33,12 @@ pub struct MailerSetting { pub smtp_password: Secret, } +#[derive(serde::Deserialize, Clone, Debug)] +pub struct AppleOAuthSetting { + pub client_id: String, + pub client_secret: Secret, +} + #[derive(serde::Deserialize, Clone, Debug)] pub struct CasbinSetting { pub pool_size: u32, @@ -204,6 +211,10 @@ pub fn get_configuration() -> Result { smtp_username: get_env_var("APPFLOWY_MAILER_SMTP_USERNAME", "sender@example.com"), smtp_password: get_env_var("APPFLOWY_MAILER_SMTP_PASSWORD", "password").into(), }, + apple_oauth: AppleOAuthSetting { + client_id: get_env_var("APPFLOWY_APPLE_OAUTH_CLIENT_ID", ""), + client_secret: get_env_var("APPFLOWY_APPLE_OAUTH_CLIENT_SECRET", "").into(), + }, }; Ok(config) } From 25cec9982f41f625e5a285d79e60c1b17c391887 Mon Sep 17 00:00:00 2001 From: Zack Fu Zi Xiang Date: Mon, 2 Sep 2024 17:25:21 +0800 Subject: [PATCH 3/7] chore: revoke for apple user --- ...22939e80ec2e75747503f0493d081ba43e4bd.json | 14 ---- libs/client-api/src/http.rs | 21 ++++- libs/database/src/user.rs | 21 ----- libs/gotrue/src/api.rs | 2 +- libs/shared-entity/src/dto/auth_dto.rs | 6 ++ src/api/user.rs | 80 +++++++++++++++---- src/application.rs | 10 ++- src/biz/user/user_info.rs | 4 - src/biz/workspace/ops.rs | 2 +- src/state.rs | 9 ++- 10 files changed, 105 insertions(+), 64 deletions(-) delete mode 100644 .sqlx/query-e6acbe78f0e8f776901c560088222939e80ec2e75747503f0493d081ba43e4bd.json diff --git a/.sqlx/query-e6acbe78f0e8f776901c560088222939e80ec2e75747503f0493d081ba43e4bd.json b/.sqlx/query-e6acbe78f0e8f776901c560088222939e80ec2e75747503f0493d081ba43e4bd.json deleted file mode 100644 index 2bb9c80d7..000000000 --- a/.sqlx/query-e6acbe78f0e8f776901c560088222939e80ec2e75747503f0493d081ba43e4bd.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "\n DELETE FROM auth.users WHERE id = $1\n ", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Uuid" - ] - }, - "nullable": [] - }, - "hash": "e6acbe78f0e8f776901c560088222939e80ec2e75747503f0493d081ba43e4bd" -} diff --git a/libs/client-api/src/http.rs b/libs/client-api/src/http.rs index fd944373a..acd532ff1 100644 --- a/libs/client-api/src/http.rs +++ b/libs/client-api/src/http.rs @@ -1,5 +1,6 @@ use crate::notify::{ClientToken, TokenStateReceiver}; use app_error::AppError; +use client_api_entity::auth_dto::DeleteUserQuery; use client_api_entity::workspace_dto::QueryWorkspaceParam; use client_api_entity::AuthProvider; use client_api_entity::CollabType; @@ -738,15 +739,33 @@ impl Client { AppResponse::<()>::from_response(resp).await?.into_error() } - /// Deletes the user account and all associated data. #[instrument(level = "info", skip_all, err)] pub async fn delete_user(&self) -> Result<(), AppResponseError> { + let (provider_access_token, provider_refresh_token) = { + let token = self.token(); + let token_read = token.read(); + let token_resp = token_read + .as_ref() + .ok_or(AppResponseError::from(AppError::NotLoggedIn( + "token is empty".to_string(), + )))?; + ( + token_resp.provider_access_token.clone(), + token_resp.provider_refresh_token.clone(), + ) + }; + let url = format!("{}/api/user", self.base_url); let resp = self .http_client_with_auth(Method::DELETE, &url) .await? + .query(&DeleteUserQuery { + provider_access_token, + provider_refresh_token, + }) .send() .await?; + log_request_id(&resp); AppResponse::<()>::from_response(resp).await?.into_error() } diff --git a/libs/database/src/user.rs b/libs/database/src/user.rs index 599caa84e..4ea6f7c99 100644 --- a/libs/database/src/user.rs +++ b/libs/database/src/user.rs @@ -240,24 +240,3 @@ pub async fn select_name_from_uuid(pool: &PgPool, user_uuid: &Uuid) -> Result Result<(), AppError> { - let res = sqlx::query!( - r#" - DELETE FROM auth.users WHERE id = $1 - "#, - user_uuid - ) - .execute(pool) - .await?; - - if res.rows_affected() != 1 { - return Err(AppError::RecordNotFound(format!( - "User with UUID {} not found", - user_uuid - ))); - } - - Ok(()) -} diff --git a/libs/gotrue/src/api.rs b/libs/gotrue/src/api.rs index cfd77fbb5..14ac81370 100644 --- a/libs/gotrue/src/api.rs +++ b/libs/gotrue/src/api.rs @@ -14,7 +14,7 @@ use infra::reqwest::{check_response, from_body, from_response}; use reqwest::{Method, RequestBuilder}; use tracing::event; -#[derive(Clone)] +#[derive(Clone, Debug)] pub struct Client { client: reqwest::Client, pub base_url: String, diff --git a/libs/shared-entity/src/dto/auth_dto.rs b/libs/shared-entity/src/dto/auth_dto.rs index 464a6f217..6f5a270dd 100644 --- a/libs/shared-entity/src/dto/auth_dto.rs +++ b/libs/shared-entity/src/dto/auth_dto.rs @@ -65,3 +65,9 @@ pub struct SignInPasswordResponse { pub struct SignInTokenResponse { pub is_new: bool, } + +#[derive(Debug, serde::Deserialize, serde::Serialize)] +pub struct DeleteUserQuery { + pub provider_access_token: Option, + pub provider_refresh_token: Option, +} diff --git a/src/api/user.rs b/src/api/user.rs index 025bfdee1..98a10e9a4 100644 --- a/src/api/user.rs +++ b/src/api/user.rs @@ -1,4 +1,4 @@ -use crate::biz::user::user_info::{delete_user, get_profile, get_user_workspace_info, update_user}; +use crate::biz::user::user_info::{get_profile, get_user_workspace_info, update_user}; use crate::biz::user::user_verify::verify_token; use crate::state::AppState; use actix_web::web::{Data, Json}; @@ -7,8 +7,9 @@ use actix_web::{web, Scope}; use app_error::ErrorCode; use authentication::jwt::{Authorization, UserUuid}; use database_entity::dto::{AFUserProfile, AFUserWorkspaceInfo}; -use secrecy::ExposeSecret; -use shared_entity::dto::auth_dto::{SignInTokenResponse, UpdateUserParams}; +use gotrue::params::AdminDeleteUserParams; +use secrecy::{ExposeSecret, Secret}; +use shared_entity::dto::auth_dto::{DeleteUserQuery, SignInTokenResponse, UpdateUserParams}; use shared_entity::response::AppResponseError; use shared_entity::response::{AppResponse, JsonAppResponse}; @@ -69,24 +70,69 @@ async fn update_user_handler( async fn delete_user_handler( auth: Authorization, state: Data, -) -> Result> { - delete_user(&state.pg_pool, auth.uuid()?).await?; - - if is_apple_user(auth) { - if let Err(err) = revoke_apple_token( + query: web::Query, +) -> Result, actix_web::Error> { + let user_uuid = auth.uuid()?; + if is_apple_user(&auth) { + let query = query.into_inner(); + revoke_apple_user( &state.config.apple_oauth.client_id, - state.config.apple_oauth.client_secret.expose_secret(), - "TODO: get original apple during oauth", + &state.config.apple_oauth.client_secret, + query.provider_access_token, + query.provider_refresh_token, ) - .await - { - tracing::warn!("revoke apple token failed: {:?}", err); - }; + .await?; } + + let admin_token = state.gotrue_admin.token().await?; + let _ = &state + .gotrue_client + .admin_delete_user( + &admin_token, + &user_uuid.to_string(), + &AdminDeleteUserParams { + should_soft_delete: false, + }, + ) + .await + .map_err(AppResponseError::from)?; + Ok(AppResponse::Ok().into()) } -fn is_apple_user(auth: Authorization) -> bool { +async fn revoke_apple_user( + client_id: &str, + client_secret: &Secret, + apple_access_token: Option, + apple_refresh_token: Option, +) -> Result<(), AppResponseError> { + let (type_type_hint, token) = match apple_access_token { + Some(access_token) => ("access_token", access_token), + None => match apple_refresh_token { + Some(refresh_token) => ("refresh_token", refresh_token), + None => { + return Err(AppResponseError::new( + ErrorCode::InvalidRequest, + "apple email deletion must provide access_token or refresh_token", + )) + }, + }, + }; + + if let Err(err) = revoke_apple_token_http_call( + client_id, + client_secret.expose_secret(), + &token, + type_type_hint, + ) + .await + { + tracing::warn!("revoke apple token failed: {:?}", err); + }; + Ok(()) +} + +fn is_apple_user(auth: &Authorization) -> bool { if let Some(provider) = auth.claims.app_metadata.get("provider") { if provider == "apple" { return true; @@ -107,10 +153,11 @@ fn is_apple_user(auth: Authorization) -> bool { } /// Based on: https://developer.apple.com/documentation/sign_in_with_apple/revoke_tokens -async fn revoke_apple_token( +async fn revoke_apple_token_http_call( apple_client_id: &str, apple_client_secret: &str, apple_user_token: &str, + token_type_hint: &str, ) -> Result<(), AppResponseError> { let resp = reqwest::Client::new() .post("https://appleid.apple.com/auth/revoke") @@ -118,6 +165,7 @@ async fn revoke_apple_token( ("client_id", apple_client_id), ("client_secret", apple_client_secret), ("token", apple_user_token), + ("token_type_hint", token_type_hint), ]) .send() .await?; diff --git a/src/application.rs b/src/application.rs index 6086c9c88..35b65cf87 100644 --- a/src/application.rs +++ b/src/application.rs @@ -216,7 +216,7 @@ pub async fn init_state(config: &Config, rt_cmd_tx: CLCommandSender) -> Result Result Result { let admin_email = gotrue_setting.admin_email.as_str(); let password = gotrue_setting.admin_password.expose_secret(); - let gotrue_admin = GoTrueAdmin::new(admin_email.to_owned(), password.to_owned()); + let gotrue_admin = GoTrueAdmin::new( + admin_email.to_owned(), + password.to_owned(), + gotrue_client.clone(), + ); match gotrue_client .token(&Grant::Password(PasswordGrant { diff --git a/src/biz/user/user_info.rs b/src/biz/user/user_info.rs index 6ae58dad0..470e39a45 100644 --- a/src/biz/user/user_info.rs +++ b/src/biz/user/user_info.rs @@ -74,7 +74,3 @@ pub async fn update_user( let metadata = params.metadata.map(|m| json!(m.into_inner())); Ok(database::user::update_user(pg_pool, &user_uuid, params.name, params.email, metadata).await?) } - -pub async fn delete_user(pg_pool: &PgPool, user_uuid: Uuid) -> Result<(), AppResponseError> { - Ok(database::user::delete_user(pg_pool, &user_uuid).await?) -} diff --git a/src/biz/workspace/ops.rs b/src/biz/workspace/ops.rs index 052ba7688..0580ceb23 100644 --- a/src/biz/workspace/ops.rs +++ b/src/biz/workspace/ops.rs @@ -346,7 +346,7 @@ pub async fn invite_workspace_members( .begin() .await .context("Begin transaction to invite workspace members")?; - let admin_token = gotrue_admin.token(gotrue_client).await?; + let admin_token = gotrue_admin.token().await?; let inviter_name = database::user::select_name_from_uuid(pg_pool, inviter).await?; let workspace_name = diff --git a/src/state.rs b/src/state.rs index 45bb34a72..297308049 100644 --- a/src/state.rs +++ b/src/state.rs @@ -150,20 +150,23 @@ impl AppMetrics { #[derive(Debug, Clone)] pub struct GoTrueAdmin { + pub gotrue_client: gotrue::api::Client, pub admin_email: String, pub password: Secret, } impl GoTrueAdmin { - pub fn new(admin_email: String, password: String) -> Self { + pub fn new(admin_email: String, password: String, gotrue_client: gotrue::api::Client) -> Self { Self { admin_email, password: password.into(), + gotrue_client, } } - pub async fn token(&self, client: &gotrue::api::Client) -> Result { - let token = client + pub async fn token(&self) -> Result { + let token = self + .gotrue_client .token(&Grant::Password(PasswordGrant { email: self.admin_email.clone(), password: self.password.expose_secret().clone(), From fd22e1d472d53c406d2e8c6b5b2dc00e20a9625a Mon Sep 17 00:00:00 2001 From: Zack Fu Zi Xiang Date: Mon, 2 Sep 2024 21:13:33 +0800 Subject: [PATCH 4/7] chore: preserve provider token --- libs/client-api/src/http.rs | 14 ++++++++++++-- libs/client-api/src/native/retry.rs | 26 +++++++++++++++++--------- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/libs/client-api/src/http.rs b/libs/client-api/src/http.rs index acd532ff1..74981c381 100644 --- a/libs/client-api/src/http.rs +++ b/libs/client-api/src/http.rs @@ -273,13 +273,18 @@ impl Client { .split('&'); let mut refresh_token: Option<&str> = None; + let mut provider_token: Option = None; + let mut provider_refresh_token: Option = None; for param in key_value_pairs { match param.split_once('=') { Some(pair) => { let (k, v) = pair; if k == "refresh_token" { refresh_token = Some(v); - break; + } else if k == "provider_token" { + provider_token = Some(v.to_string()); + } else if k == "provider_refresh_token" { + provider_refresh_token = Some(v.to_string()); } }, None => warn!("param is not in key=value format: {}", param), @@ -287,13 +292,18 @@ impl Client { } let refresh_token = refresh_token.ok_or(url_missing_param("refresh_token"))?; - let new_token = self + let mut new_token = self .gotrue_client .token(&Grant::RefreshToken(RefreshTokenGrant { refresh_token: refresh_token.to_owned(), })) .await?; + // refresh endpoint does not return provider token + // so we need to set it manually to preserve this information + new_token.provider_access_token = provider_token; + new_token.provider_refresh_token = provider_refresh_token; + let (_user, new) = self.verify_token(&new_token.access_token).await?; self.token.write().set(new_token); Ok(new) diff --git a/libs/client-api/src/native/retry.rs b/libs/client-api/src/native/retry.rs index 3454de4fa..02078e7c4 100644 --- a/libs/client-api/src/native/retry.rs +++ b/libs/client-api/src/native/retry.rs @@ -47,18 +47,26 @@ impl Action for RefreshTokenAction { if let (Some(token), Some(gotrue_client)) = (weak_token.upgrade(), weak_gotrue_client.upgrade()) { - let refresh_token = token - .read() - .as_ref() - .ok_or(GoTrueError::NotLoggedIn( + let (refresh_token, provider_access_token, provider_refresh_token) = { + let mut token_write = token.write(); + let gotrue_resp_token = token_write.as_mut().ok_or(GoTrueError::NotLoggedIn( "fail to refresh user token".to_owned(), - ))? - .refresh_token - .as_str() - .to_owned(); - let access_token_resp = gotrue_client + ))?; + let refresh_token = gotrue_resp_token.refresh_token.as_str().to_owned(); + let provider_access_token = gotrue_resp_token.provider_access_token.take(); + let provider_refresh_token = gotrue_resp_token.provider_refresh_token.take(); + (refresh_token, provider_access_token, provider_refresh_token) + }; + + let mut access_token_resp = gotrue_client .token(&Grant::RefreshToken(RefreshTokenGrant { refresh_token })) .await?; + + // refresh does not preserve provider token and refresh token + // so we need to set it manually to preserve this information + access_token_resp.provider_access_token = provider_access_token; + access_token_resp.provider_refresh_token = provider_refresh_token; + token.write().set(access_token_resp); } Ok(()) From 95460924f84f4ead5dc3ec8a00e203a5b47dd45d Mon Sep 17 00:00:00 2001 From: Zack Fu Zi Xiang Date: Mon, 2 Sep 2024 21:20:32 +0800 Subject: [PATCH 5/7] fix: dont return error if fail to revoke for apple signed in user --- src/api/user.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/api/user.rs b/src/api/user.rs index 98a10e9a4..dbae51229 100644 --- a/src/api/user.rs +++ b/src/api/user.rs @@ -75,13 +75,16 @@ async fn delete_user_handler( let user_uuid = auth.uuid()?; if is_apple_user(&auth) { let query = query.into_inner(); - revoke_apple_user( + if let Err(err) = revoke_apple_user( &state.config.apple_oauth.client_id, &state.config.apple_oauth.client_secret, query.provider_access_token, query.provider_refresh_token, ) - .await?; + .await + { + tracing::warn!("revoke apple user failed: {:?}", err); + }; } let admin_token = state.gotrue_admin.token().await?; From de8f992377caa0b5fab7c34b802a7ef53e36a9c6 Mon Sep 17 00:00:00 2001 From: Zack Fu Zi Xiang Date: Tue, 3 Sep 2024 02:24:38 +0800 Subject: [PATCH 6/7] feat: admin frontend delete account button --- admin_frontend/src/ext/api.rs | 15 +++++++++++++++ admin_frontend/src/ext/entities.rs | 7 ------- admin_frontend/src/ext/error.rs | 7 ------- admin_frontend/src/web_api.rs | 13 ++++++++++++- .../templates/components/top_menu_bar.html | 7 +++++++ 5 files changed, 34 insertions(+), 15 deletions(-) diff --git a/admin_frontend/src/ext/api.rs b/admin_frontend/src/ext/api.rs index 126e4ed2c..b82428fb8 100644 --- a/admin_frontend/src/ext/api.rs +++ b/admin_frontend/src/ext/api.rs @@ -278,3 +278,18 @@ pub async fn verify_token_cloud( let _: SignInTokenResponse = from_json_response(resp).await?; Ok(()) } + +pub async fn delete_current_user( + access_token: &str, + appflowy_cloud_base_url: &str, +) -> Result<(), Error> { + let http_client = reqwest::Client::new(); + let url = format!("{}/api/user", appflowy_cloud_base_url); + let resp = http_client + .delete(url) + .header("Authorization", format!("Bearer {}", access_token)) + .send() + .await?; + check_response(resp).await?; + Ok(()) +} diff --git a/admin_frontend/src/ext/entities.rs b/admin_frontend/src/ext/entities.rs index 02c14e631..1c2a17ef0 100644 --- a/admin_frontend/src/ext/entities.rs +++ b/admin_frontend/src/ext/entities.rs @@ -27,13 +27,6 @@ pub struct WorkspaceMember { pub role: String, } -#[derive(Deserialize)] -pub struct WorkspaceUsageLimit { - pub total_blob_size: i64, - pub single_blob_size: i64, - pub member_count: i64, -} - #[derive(Deserialize, Serialize)] pub struct WorkspaceBlobUsage { pub consumed_capacity: u64, diff --git a/admin_frontend/src/ext/error.rs b/admin_frontend/src/ext/error.rs index 0942921fb..ee683d48b 100644 --- a/admin_frontend/src/ext/error.rs +++ b/admin_frontend/src/ext/error.rs @@ -1,13 +1,6 @@ use axum::response::{IntoResponse, Response}; -use serde::Deserialize; use shared_entity::response::AppResponseError; -#[derive(Deserialize, Debug)] -pub struct AppFlowyCloudError { - pub code: String, - pub message: String, -} - #[derive(Debug)] pub enum Error { NotOk(u16, String), // HTTP status code, payload diff --git a/admin_frontend/src/web_api.rs b/admin_frontend/src/web_api.rs index 6d06e8c0b..6062a7f7f 100644 --- a/admin_frontend/src/web_api.rs +++ b/admin_frontend/src/web_api.rs @@ -1,6 +1,7 @@ use crate::error::WebApiError; use crate::ext::api::{ - accept_workspace_invitation, invite_user_to_workspace, leave_workspace, verify_token_cloud, + accept_workspace_invitation, delete_current_user, invite_user_to_workspace, leave_workspace, + verify_token_cloud, }; use crate::models::{ WebApiAdminCreateUserRequest, WebApiChangePasswordRequest, WebApiCreateSSOProviderRequest, @@ -39,6 +40,7 @@ pub fn router() -> Router { .route("/workspace/:workspace_id/leave", post(leave_workspace_handler)) .route("/invite/:invite_id/accept", post(invite_accept_handler)) .route("/open_app", post(open_app_handler)) + .route("/delete-account", delete(delete_account_handler)) // admin .route("/admin/user", post(admin_add_user_handler)) @@ -115,6 +117,15 @@ async fn open_app_handler(session: UserSession) -> Result, + session: UserSession, +) -> Result> { + delete_current_user(&session.token.access_token, &state.appflowy_cloud_url).await?; + Ok(htmx_redirect("/web/login")) +} + // Invite another user, this will trigger email sending // to the target user async fn invite_handler( diff --git a/admin_frontend/templates/components/top_menu_bar.html b/admin_frontend/templates/components/top_menu_bar.html index 45e486038..65b71f292 100644 --- a/admin_frontend/templates/components/top_menu_bar.html +++ b/admin_frontend/templates/components/top_menu_bar.html @@ -10,6 +10,13 @@

  AppFlowy Cloud  

> {{ user.email|escape }} +
+ Delete Account +
From 14a1382d1c4b77ba16fee3ab867e6bfded85828e Mon Sep 17 00:00:00 2001 From: Zack Fu Zi Xiang Date: Tue, 3 Sep 2024 10:49:53 +0800 Subject: [PATCH 7/7] feat: call workspace delete when deleting user --- src/api/user.rs | 132 ++++--------------------------- src/api/workspace.rs | 9 ++- src/biz/user/mod.rs | 1 + src/biz/user/user_delete.rs | 152 ++++++++++++++++++++++++++++++++++++ src/biz/workspace/ops.rs | 8 +- 5 files changed, 180 insertions(+), 122 deletions(-) create mode 100644 src/biz/user/user_delete.rs diff --git a/src/api/user.rs b/src/api/user.rs index dbae51229..90871ab73 100644 --- a/src/api/user.rs +++ b/src/api/user.rs @@ -1,14 +1,12 @@ +use crate::biz::user::user_delete::delete_user; use crate::biz::user::user_info::{get_profile, get_user_workspace_info, update_user}; use crate::biz::user::user_verify::verify_token; use crate::state::AppState; use actix_web::web::{Data, Json}; use actix_web::Result; use actix_web::{web, Scope}; -use app_error::ErrorCode; use authentication::jwt::{Authorization, UserUuid}; use database_entity::dto::{AFUserProfile, AFUserWorkspaceInfo}; -use gotrue::params::AdminDeleteUserParams; -use secrecy::{ExposeSecret, Secret}; use shared_entity::dto::auth_dto::{DeleteUserQuery, SignInTokenResponse, UpdateUserParams}; use shared_entity::response::AppResponseError; use shared_entity::response::{AppResponse, JsonAppResponse}; @@ -73,117 +71,21 @@ async fn delete_user_handler( query: web::Query, ) -> Result, actix_web::Error> { let user_uuid = auth.uuid()?; - if is_apple_user(&auth) { - let query = query.into_inner(); - if let Err(err) = revoke_apple_user( - &state.config.apple_oauth.client_id, - &state.config.apple_oauth.client_secret, - query.provider_access_token, - query.provider_refresh_token, - ) - .await - { - tracing::warn!("revoke apple user failed: {:?}", err); - }; - } - - let admin_token = state.gotrue_admin.token().await?; - let _ = &state - .gotrue_client - .admin_delete_user( - &admin_token, - &user_uuid.to_string(), - &AdminDeleteUserParams { - should_soft_delete: false, - }, - ) - .await - .map_err(AppResponseError::from)?; - - Ok(AppResponse::Ok().into()) -} - -async fn revoke_apple_user( - client_id: &str, - client_secret: &Secret, - apple_access_token: Option, - apple_refresh_token: Option, -) -> Result<(), AppResponseError> { - let (type_type_hint, token) = match apple_access_token { - Some(access_token) => ("access_token", access_token), - None => match apple_refresh_token { - Some(refresh_token) => ("refresh_token", refresh_token), - None => { - return Err(AppResponseError::new( - ErrorCode::InvalidRequest, - "apple email deletion must provide access_token or refresh_token", - )) - }, - }, - }; - - if let Err(err) = revoke_apple_token_http_call( - client_id, - client_secret.expose_secret(), - &token, - type_type_hint, + let DeleteUserQuery { + provider_access_token, + provider_refresh_token, + } = query.into_inner(); + delete_user( + &state.pg_pool, + &state.bucket_storage, + &state.gotrue_client, + &state.gotrue_admin, + &state.config.apple_oauth, + auth, + user_uuid, + provider_access_token, + provider_refresh_token, ) - .await - { - tracing::warn!("revoke apple token failed: {:?}", err); - }; - Ok(()) -} - -fn is_apple_user(auth: &Authorization) -> bool { - if let Some(provider) = auth.claims.app_metadata.get("provider") { - if provider == "apple" { - return true; - } - }; - - if let Some(providers) = auth.claims.app_metadata.get("providers") { - if let Some(providers) = providers.as_array() { - for provider in providers { - if provider == "apple" { - return true; - } - } - } - } - - false -} - -/// Based on: https://developer.apple.com/documentation/sign_in_with_apple/revoke_tokens -async fn revoke_apple_token_http_call( - apple_client_id: &str, - apple_client_secret: &str, - apple_user_token: &str, - token_type_hint: &str, -) -> Result<(), AppResponseError> { - let resp = reqwest::Client::new() - .post("https://appleid.apple.com/auth/revoke") - .form(&[ - ("client_id", apple_client_id), - ("client_secret", apple_client_secret), - ("token", apple_user_token), - ("token_type_hint", token_type_hint), - ]) - .send() - .await?; - - let status = resp.status(); - if status.is_success() { - return Ok(()); - } - - let payload = resp.text().await?; - Err(AppResponseError::new( - ErrorCode::AppleRevokeTokenError, - format!( - "calling apple revoke, code: {}, message: {}", - status, payload - ), - )) + .await?; + Ok(AppResponse::Ok().into()) } diff --git a/src/api/workspace.rs b/src/api/workspace.rs index 1aa5144ec..62cd5af88 100644 --- a/src/api/workspace.rs +++ b/src/api/workspace.rs @@ -250,10 +250,13 @@ async fn delete_workspace_handler( workspace_id: web::Path, state: Data, ) -> Result>> { - let bucket_storage = &state.bucket_storage; - // TODO: add permission for workspace deletion - workspace::ops::delete_workspace_for_user(&state.pg_pool, &workspace_id, bucket_storage).await?; + workspace::ops::delete_workspace_for_user( + state.pg_pool.clone(), + *workspace_id, + state.bucket_storage.clone(), + ) + .await?; Ok(AppResponse::Ok().into()) } diff --git a/src/biz/user/mod.rs b/src/biz/user/mod.rs index c1ad70566..b2523ff7f 100644 --- a/src/biz/user/mod.rs +++ b/src/biz/user/mod.rs @@ -1,3 +1,4 @@ +pub mod user_delete; pub mod user_info; pub mod user_init; pub mod user_verify; diff --git a/src/biz/user/user_delete.rs b/src/biz/user/user_delete.rs new file mode 100644 index 000000000..01bd6ec86 --- /dev/null +++ b/src/biz/user/user_delete.rs @@ -0,0 +1,152 @@ +use std::sync::Arc; + +use crate::biz::workspace::ops::get_all_user_workspaces; +use crate::state::GoTrueAdmin; +use crate::{biz::workspace::ops::delete_workspace_for_user, config::config::AppleOAuthSetting}; +use app_error::ErrorCode; +use authentication::jwt::Authorization; +use database::file::s3_client_impl::S3BucketStorage; +use gotrue::params::AdminDeleteUserParams; +use secrecy::{ExposeSecret, Secret}; +use shared_entity::response::AppResponseError; +use uuid::Uuid; + +#[allow(clippy::too_many_arguments)] +pub async fn delete_user( + pg_pool: &sqlx::PgPool, + bucket_storage: &Arc, + gotrue_client: &gotrue::api::Client, + gotrue_admin: &GoTrueAdmin, + apple_oauth: &AppleOAuthSetting, + auth: Authorization, + user_uuid: Uuid, + provider_access_token: Option, + provider_refresh_token: Option, +) -> Result<(), AppResponseError> { + if is_apple_user(&auth) { + if let Err(err) = revoke_apple_user( + &apple_oauth.client_id, + &apple_oauth.client_secret, + provider_access_token, + provider_refresh_token, + ) + .await + { + tracing::warn!("revoke apple user failed: {:?}", err); + }; + } + + let admin_token = gotrue_admin.token().await?; + gotrue_client + .admin_delete_user( + &admin_token, + &user_uuid.to_string(), + &AdminDeleteUserParams { + should_soft_delete: false, + }, + ) + .await + .map_err(AppResponseError::from)?; + + // spawn tasks to delete all user workspace and object storage + let user_workspaces = get_all_user_workspaces(pg_pool, &user_uuid, false).await?; + let mut tasks = vec![]; + for workspace in user_workspaces { + let cloned_pg_pool = pg_pool.clone(); + tasks.push(tokio::spawn(delete_workspace_for_user( + cloned_pg_pool, + workspace.workspace_id, + bucket_storage.clone(), + ))); + } + for task in tasks { + task.await??; + } + + Ok(()) +} + +async fn revoke_apple_user( + client_id: &str, + client_secret: &Secret, + apple_access_token: Option, + apple_refresh_token: Option, +) -> Result<(), AppResponseError> { + let (type_type_hint, token) = match apple_access_token { + Some(access_token) => ("access_token", access_token), + None => match apple_refresh_token { + Some(refresh_token) => ("refresh_token", refresh_token), + None => { + return Err(AppResponseError::new( + ErrorCode::InvalidRequest, + "apple email deletion must provide access_token or refresh_token", + )) + }, + }, + }; + + if let Err(err) = revoke_apple_token_http_call( + client_id, + client_secret.expose_secret(), + &token, + type_type_hint, + ) + .await + { + tracing::warn!("revoke apple token failed: {:?}", err); + }; + Ok(()) +} + +fn is_apple_user(auth: &Authorization) -> bool { + if let Some(provider) = auth.claims.app_metadata.get("provider") { + if provider == "apple" { + return true; + } + }; + + if let Some(providers) = auth.claims.app_metadata.get("providers") { + if let Some(providers) = providers.as_array() { + for provider in providers { + if provider == "apple" { + return true; + } + } + } + } + + false +} + +/// Based on: https://developer.apple.com/documentation/sign_in_with_apple/revoke_tokens +async fn revoke_apple_token_http_call( + apple_client_id: &str, + apple_client_secret: &str, + apple_user_token: &str, + token_type_hint: &str, +) -> Result<(), AppResponseError> { + let resp = reqwest::Client::new() + .post("https://appleid.apple.com/auth/revoke") + .form(&[ + ("client_id", apple_client_id), + ("client_secret", apple_client_secret), + ("token", apple_user_token), + ("token_type_hint", token_type_hint), + ]) + .send() + .await?; + + let status = resp.status(); + if status.is_success() { + return Ok(()); + } + + let payload = resp.text().await?; + Err(AppResponseError::new( + ErrorCode::AppleRevokeTokenError, + format!( + "calling apple revoke, code: {}, message: {}", + status, payload + ), + )) +} diff --git a/src/biz/workspace/ops.rs b/src/biz/workspace/ops.rs index 3091781b0..2d841dd87 100644 --- a/src/biz/workspace/ops.rs +++ b/src/biz/workspace/ops.rs @@ -39,9 +39,9 @@ use crate::state::GoTrueAdmin; const MAX_COMMENT_LENGTH: usize = 5000; pub async fn delete_workspace_for_user( - pg_pool: &PgPool, - workspace_id: &Uuid, - bucket_storage: &Arc, + pg_pool: PgPool, + workspace_id: Uuid, + bucket_storage: Arc, ) -> Result<(), AppResponseError> { // remove files from s3 bucket_storage @@ -49,7 +49,7 @@ pub async fn delete_workspace_for_user( .await?; // remove from postgres - delete_from_workspace(pg_pool, workspace_id).await?; + delete_from_workspace(&pg_pool, &workspace_id).await?; // TODO: There can be a rare case where user uploads while workspace is being deleted. // We need some routine job to clean up these orphaned files.