From c50f872bb7ada1ba333f09e034fb2d20df0aa56a Mon Sep 17 00:00:00 2001 From: Zack Fu Zi Xiang Date: Thu, 22 Feb 2024 10:48:10 +0800 Subject: [PATCH] Revert "Merge pull request #328 from AppFlowy-IO/workspace-add-email" This reverts commit 5e7794646a4dce935b27541d60aa9084574ba6b8, reversing changes made to d5869742c269cd8d4669e4f5e9154302dbad95d6. --- ...36f77d96a83d462bf815f13ebe70d88fada3.json} | 4 +- ...6943c7ba23eac358cae6d36537991a8f8975.json} | 4 +- admin_frontend/src/web_app.rs | 2 +- libs/client-api/src/http.rs | 40 ++++----- libs/database/src/collab/collab_db_ops.rs | 8 +- libs/database/src/workspace.rs | 1 - libs/gotrue/src/api.rs | 26 ++---- libs/gotrue/src/params.rs | 6 -- src/api/workspace.rs | 12 +-- src/application.rs | 82 +++++-------------- src/biz/user.rs | 28 ++----- src/biz/utils.rs | 17 ---- src/biz/workspace/ops.rs | 54 ++---------- src/state.rs | 2 - tests/access_control/collab_ac_test.rs | 19 +++-- tests/access_control/member_ac_test.rs | 10 +-- tests/access_control/mod.rs | 22 ----- tests/access_control/user_ac_test.rs | 21 ++++- tests/gotrue/admin.rs | 4 +- tests/workspace/member_crud.rs | 52 +++++------- 20 files changed, 133 insertions(+), 281 deletions(-) rename .sqlx/{query-f18d6e075a522b0ce5935351dd57ab0dda4d8b4ed3881c2ad0bc09c07c43e6fe.json => query-5144da61424400216e10d9327f5136f77d96a83d462bf815f13ebe70d88fada3.json} (74%) rename .sqlx/{query-f409626142553d4496d15b5dfa7da8a5a238da86f56c930c09a261f2efa1f55c.json => query-eff32b7631cb1b715776c66749706943c7ba23eac358cae6d36537991a8f8975.json} (73%) diff --git a/.sqlx/query-f18d6e075a522b0ce5935351dd57ab0dda4d8b4ed3881c2ad0bc09c07c43e6fe.json b/.sqlx/query-5144da61424400216e10d9327f5136f77d96a83d462bf815f13ebe70d88fada3.json similarity index 74% rename from .sqlx/query-f18d6e075a522b0ce5935351dd57ab0dda4d8b4ed3881c2ad0bc09c07c43e6fe.json rename to .sqlx/query-5144da61424400216e10d9327f5136f77d96a83d462bf815f13ebe70d88fada3.json index 61361749..3d251f0a 100644 --- a/.sqlx/query-f18d6e075a522b0ce5935351dd57ab0dda4d8b4ed3881c2ad0bc09c07c43e6fe.json +++ b/.sqlx/query-5144da61424400216e10d9327f5136f77d96a83d462bf815f13ebe70d88fada3.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n INSERT INTO af_collab_snapshot (oid, blob, len, encrypt, workspace_id)\n VALUES ($1, $2, $3, $4, $5)\n RETURNING sid AS snapshot_id, oid AS object_id, created_at\n ", + "query": "\n INSERT INTO af_collab_snapshot (oid, blob, len, encrypt, workspace_id) \n VALUES ($1, $2, $3, $4, $5)\n RETURNING sid AS snapshot_id, oid AS object_id, created_at\n ", "describe": { "columns": [ { @@ -34,5 +34,5 @@ false ] }, - "hash": "f18d6e075a522b0ce5935351dd57ab0dda4d8b4ed3881c2ad0bc09c07c43e6fe" + "hash": "5144da61424400216e10d9327f5136f77d96a83d462bf815f13ebe70d88fada3" } diff --git a/.sqlx/query-f409626142553d4496d15b5dfa7da8a5a238da86f56c930c09a261f2efa1f55c.json b/.sqlx/query-eff32b7631cb1b715776c66749706943c7ba23eac358cae6d36537991a8f8975.json similarity index 73% rename from .sqlx/query-f409626142553d4496d15b5dfa7da8a5a238da86f56c930c09a261f2efa1f55c.json rename to .sqlx/query-eff32b7631cb1b715776c66749706943c7ba23eac358cae6d36537991a8f8975.json index 4e14b48b..2f4c119e 100644 --- a/.sqlx/query-f409626142553d4496d15b5dfa7da8a5a238da86f56c930c09a261f2efa1f55c.json +++ b/.sqlx/query-eff32b7631cb1b715776c66749706943c7ba23eac358cae6d36537991a8f8975.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n SELECT sid as \"snapshot_id\", oid as \"object_id\", created_at\n FROM af_collab_snapshot\n WHERE oid = $1 AND deleted_at IS NULL\n ORDER BY created_at DESC;\n ", + "query": "\n SELECT sid as \"snapshot_id\", oid as \"object_id\", created_at\n FROM af_collab_snapshot \n WHERE oid = $1 AND deleted_at IS NULL\n ORDER BY created_at DESC;\n ", "describe": { "columns": [ { @@ -30,5 +30,5 @@ false ] }, - "hash": "f409626142553d4496d15b5dfa7da8a5a238da86f56c930c09a261f2efa1f55c" + "hash": "eff32b7631cb1b715776c66749706943c7ba23eac358cae6d36537991a8f8975" } diff --git a/admin_frontend/src/web_app.rs b/admin_frontend/src/web_app.rs index d8873f63..4e3796d0 100644 --- a/admin_frontend/src/web_app.rs +++ b/admin_frontend/src/web_app.rs @@ -146,7 +146,7 @@ pub async fn admin_users_handler( ) -> Result, WebAppError> { let users = state .gotrue_client - .admin_list_user(&session.token.access_token, None) + .admin_list_user(&session.token.access_token) .await .map_or_else( |err| { diff --git a/libs/client-api/src/http.rs b/libs/client-api/src/http.rs index 6d364d51..91a8fcc0 100644 --- a/libs/client-api/src/http.rs +++ b/libs/client-api/src/http.rs @@ -408,32 +408,20 @@ impl Client { email: &str, password: &str, ) -> Result { - let user = self - .gotrue_client - .admin_add_user( - &self.access_token()?, - &AdminUserParams { - email: email.to_owned(), - password: Some(password.to_owned()), - email_confirm: true, - ..Default::default() - }, - ) - .await?; - Ok(user) - } - - // filter is postgre sql like filter - #[instrument(level = "debug", skip_all, err)] - pub async fn admin_list_users( - &self, - filter: Option<&str>, - ) -> Result, AppResponseError> { - let user = self - .gotrue_client - .admin_list_user(&self.access_token()?, filter) - .await?; - Ok(user.users) + Ok( + self + .gotrue_client + .admin_add_user( + &self.access_token()?, + &AdminUserParams { + email: email.to_owned(), + password: Some(password.to_owned()), + email_confirm: true, + ..Default::default() + }, + ) + .await?, + ) } /// Only expose this method for testing diff --git a/libs/database/src/collab/collab_db_ops.rs b/libs/database/src/collab/collab_db_ops.rs index a84117a3..f4acc1d8 100644 --- a/libs/database/src/collab/collab_db_ops.rs +++ b/libs/database/src/collab/collab_db_ops.rs @@ -318,7 +318,7 @@ pub async fn should_create_snapshot<'a, E: Executor<'a, Database = Postgres>>( ) -> Result { let hours = Utc::now() - Duration::hours(SNAPSHOT_PER_HOUR); let latest_snapshot_time: Option> = sqlx::query_scalar( - "SELECT created_at FROM af_collab_snapshot + "SELECT created_at FROM af_collab_snapshot WHERE oid = $1 ORDER BY created_at DESC LIMIT 1", ) .bind(oid) @@ -344,7 +344,7 @@ pub(crate) async fn create_snapshot_and_maintain_limit<'a>( let snapshot_meta = sqlx::query_as!( AFSnapshotMeta, r#" - INSERT INTO af_collab_snapshot (oid, blob, len, encrypt, workspace_id) + INSERT INTO af_collab_snapshot (oid, blob, len, encrypt, workspace_id) VALUES ($1, $2, $3, $4, $5) RETURNING sid AS snapshot_id, oid AS object_id, created_at "#, @@ -360,7 +360,7 @@ pub(crate) async fn create_snapshot_and_maintain_limit<'a>( // When a new snapshot is created that surpasses the preset limit, older snapshots will be deleted to maintain the limit sqlx::query( r#" - DELETE FROM af_collab_snapshot + DELETE FROM af_collab_snapshot WHERE oid = $1 AND sid NOT IN ( SELECT sid FROM af_collab_snapshot WHERE oid = $1 ORDER BY created_at DESC LIMIT $2) "#, ) @@ -403,7 +403,7 @@ pub async fn get_all_collab_snapshot_meta( AFSnapshotMeta, r#" SELECT sid as "snapshot_id", oid as "object_id", created_at - FROM af_collab_snapshot + FROM af_collab_snapshot WHERE oid = $1 AND deleted_at IS NULL ORDER BY created_at DESC; "#, diff --git a/libs/database/src/workspace.rs b/libs/database/src/workspace.rs index 4ca10707..00141945 100644 --- a/libs/database/src/workspace.rs +++ b/libs/database/src/workspace.rs @@ -390,7 +390,6 @@ pub async fn select_workspace<'a, E: Executor<'a, Database = Postgres>>( .await?; Ok(workspace) } - #[inline] pub async fn update_updated_at_of_workspace<'a, E: Executor<'a, Database = Postgres>>( executor: E, diff --git a/libs/gotrue/src/api.rs b/libs/gotrue/src/api.rs index d7d487a5..292d2301 100644 --- a/libs/gotrue/src/api.rs +++ b/libs/gotrue/src/api.rs @@ -1,7 +1,7 @@ use super::grant::Grant; use crate::params::{ AdminDeleteUserParams, AdminUserParams, CreateSSOProviderParams, GenerateLinkParams, - GenerateLinkResponse, InviteUserParams, MagicLinkParams, + GenerateLinkResponse, MagicLinkParams, }; use anyhow::Context; use gotrue_entity::dto::{ @@ -139,14 +139,12 @@ impl Client { pub async fn admin_list_user( &self, access_token: &str, - filter: Option<&str>, ) -> Result { let url = format!("{}/admin/users", self.base_url); - let mut req = self.http_client_with_auth(Method::GET, &url, access_token); - if let Some(filter) = filter { - req = req.query(&[("filter", filter)]); - } - let resp = req.send().await?; + let resp = self + .http_client_with_auth(Method::GET, &url, access_token) + .send() + .await?; to_gotrue_result(resp).await } @@ -193,20 +191,6 @@ impl Client { to_gotrue_result(resp).await } - pub async fn admin_invite_user( - &self, - access_token: &str, - admin_user_params: &InviteUserParams, - ) -> Result { - let url = format!("{}/invite", self.base_url); - let resp = self - .http_client_with_auth(Method::POST, &url, access_token) - .json(&admin_user_params) - .send() - .await?; - to_gotrue_result(resp).await - } - pub async fn admin_add_user( &self, access_token: &str, diff --git a/libs/gotrue/src/params.rs b/libs/gotrue/src/params.rs index 92988b3d..3a1dd390 100644 --- a/libs/gotrue/src/params.rs +++ b/libs/gotrue/src/params.rs @@ -8,12 +8,6 @@ pub struct AdminDeleteUserParams { pub should_soft_delete: bool, } -#[derive(Default, Serialize)] -pub struct InviteUserParams { - pub email: String, - pub data: serde_json::Value, -} - #[derive(Debug, Default, Deserialize, Serialize)] pub struct AdminUserParams { pub aud: String, diff --git a/src/api/workspace.rs b/src/api/workspace.rs index 59598046..790ad7f0 100644 --- a/src/api/workspace.rs +++ b/src/api/workspace.rs @@ -194,13 +194,15 @@ async fn add_workspace_members_handler( state: Data, ) -> Result> { let create_members = payload.into_inner(); - - let role_by_uid = - workspace::ops::add_workspace_members(&state, &user_uuid, &workspace_id, create_members.0) - .await?; + let role_by_uid = workspace::ops::add_workspace_members( + &state.pg_pool, + &user_uuid, + &workspace_id, + create_members.0, + ) + .await?; for (uid, role) in role_by_uid { - // TODO: use one single query to insert all roles state .workspace_access_control .insert_workspace_role(&uid, &workspace_id, role) diff --git a/src/application.rs b/src/application.rs index 20867b47..40bd5eea 100644 --- a/src/application.rs +++ b/src/application.rs @@ -1,32 +1,5 @@ use crate::api::metrics::metrics_scope; -use crate::component::auth::HEADER_TOKEN; -use crate::config::config::{Config, DatabaseSetting, GoTrueSetting, S3Setting}; -use crate::middleware::request_id::RequestIdMiddleware; -use crate::self_signed::create_self_signed_certificate; -use crate::state::{AppState, UserCache}; -use actix_identity::IdentityMiddleware; -use actix_session::storage::RedisSessionStore; -use actix_session::SessionMiddleware; -use actix_web::cookie::Key; -use actix_web::{dev::Server, web, web::Data, App, HttpServer}; - -use actix::Actor; -use anyhow::{Context, Error}; -use app_error::AppError; -use gotrue::grant::{Grant, PasswordGrant}; -use openssl::ssl::{SslAcceptor, SslAcceptorBuilder, SslFiletype, SslMethod}; -use openssl::x509::X509; -use secrecy::{ExposeSecret, Secret}; -use snowflake::Snowflake; -use sqlx::{postgres::PgPoolOptions, PgPool}; -use std::net::TcpListener; -use std::sync::Arc; -use std::time::Duration; - -use tokio::sync::RwLock; -use tracing::info; - use crate::api::file_storage::file_storage_scope; use crate::api::user::user_scope; use crate::api::workspace::{collab_scope, workspace_scope}; @@ -37,11 +10,32 @@ use crate::biz::collab::storage::init_collab_storage; use crate::biz::pg_listener::PgListeners; use crate::biz::user::RealtimeUserImpl; use crate::biz::workspace::access_control::WorkspaceHttpAccessControl; +use crate::component::auth::HEADER_TOKEN; +use crate::config::config::{Config, DatabaseSetting, GoTrueSetting, S3Setting}; use crate::middleware::access_control_mw::WorkspaceAccessControl; use crate::middleware::metrics_mw::MetricsMiddleware; -use crate::state::AppMetrics; +use crate::middleware::request_id::RequestIdMiddleware; +use crate::self_signed::create_self_signed_certificate; +use crate::state::{AppMetrics, AppState, UserCache}; +use actix::Actor; +use actix_identity::IdentityMiddleware; +use actix_session::storage::RedisSessionStore; +use actix_session::SessionMiddleware; +use actix_web::cookie::Key; +use actix_web::{dev::Server, web, web::Data, App, HttpServer}; +use anyhow::{Context, Error}; use database::file::bucket_s3_impl::S3BucketStorage; +use openssl::ssl::{SslAcceptor, SslAcceptorBuilder, SslFiletype, SslMethod}; +use openssl::x509::X509; use realtime::collaborate::CollabServer; +use secrecy::{ExposeSecret, Secret}; +use snowflake::Snowflake; +use sqlx::{postgres::PgPoolOptions, PgPool}; +use std::net::TcpListener; +use std::sync::Arc; +use std::time::Duration; +use tokio::sync::RwLock; +use tracing::info; pub struct Application { port: u16, @@ -173,12 +167,6 @@ pub async fn init_state(config: &Config) -> Result { let gotrue_client = get_gotrue_client(&config.gotrue).await?; setup_admin_account(&gotrue_client, &pg_pool, &config.gotrue).await?; - // Gotrue Admin - let gotrue_admin = GoTrueAdmin::new( - config.gotrue.admin_email.clone(), - config.gotrue.admin_password.clone(), - ); - // Redis info!("Connecting to Redis..."); let redis_client = get_redis_client(config.redis_uri.expose_secret()).await?; @@ -219,7 +207,6 @@ pub async fn init_state(config: &Config) -> Result { users: Arc::new(users), id_gen: Arc::new(RwLock::new(Snowflake::new(1))), gotrue_client, - gotrue_admin, redis_client, collab_storage, collab_access_control, @@ -231,31 +218,6 @@ pub async fn init_state(config: &Config) -> Result { }) } -#[derive(Debug, Clone)] -pub struct GoTrueAdmin { - pub admin_email: String, - pub password: Secret, -} - -impl GoTrueAdmin { - pub fn new(admin_email: String, password: String) -> Self { - Self { - admin_email, - password: password.into(), - } - } - - pub async fn token(&self, client: &gotrue::api::Client) -> Result { - let token = client - .token(&Grant::Password(PasswordGrant { - email: self.admin_email.clone(), - password: self.password.expose_secret().clone(), - })) - .await?; - Ok(token.access_token) - } -} - async fn setup_admin_account( gotrue_client: &gotrue::api::Client, pg_pool: &PgPool, diff --git a/src/biz/user.rs b/src/biz/user.rs index 0f5a501e..743881aa 100644 --- a/src/biz/user.rs +++ b/src/biz/user.rs @@ -1,5 +1,4 @@ use anyhow::{Context, Result}; -use gotrue_entity::dto::User; use crate::biz::workspace::access_control::WorkspaceAccessControl; use crate::state::AppState; @@ -27,30 +26,15 @@ use workspace_template::{WorkspaceTemplate, WorkspaceTemplateBuilder}; #[instrument(skip_all, err)] pub async fn verify_token(access_token: &str, state: &AppState) -> Result { let user = state.gotrue_client.user_info(access_token).await?; + let user_uuid = uuid::Uuid::parse_str(&user.id)?; + let name = name_from_user_metadata(&user.user_metadata); + let mut txn = state .pg_pool .begin() .await .context("acquire transaction to verify token")?; - let is_new = create_user_if_not_exists(&user, &mut txn, state).await; - txn - .commit() - .await - .context("fail to commit transaction to verify token")?; - - is_new -} - -/// Returns if user is new -pub async fn create_user_if_not_exists( - user: &User, - txn: &mut Transaction<'_, sqlx::Postgres>, - state: &AppState, -) -> Result { - let user_uuid = uuid::Uuid::parse_str(&user.id)?; - let name = name_from_user_metadata(&user.user_metadata); - // To prevent concurrent creation of the same user with the same workspace resources, we lock // the user row when `verify_token` is called. This means that if multiple requests try to // create the same user simultaneously, the first request will acquire the lock, create the user, @@ -83,12 +67,16 @@ pub async fn create_user_if_not_exists( create_workspace_for_user( new_uid, &workspace_id, - txn, + &mut txn, vec![GetStartedDocumentTemplate], state, ) .await?; } + txn + .commit() + .await + .context("fail to commit transaction to verify token")?; Ok(is_new) } diff --git a/src/biz/utils.rs b/src/biz/utils.rs index 36029784..36f003e5 100644 --- a/src/biz/utils.rs +++ b/src/biz/utils.rs @@ -1,4 +1,3 @@ -use app_error::AppError; use std::pin::Pin; use std::task::{Context, Poll}; use tokio::io::{self, AsyncRead, ReadBuf}; @@ -43,19 +42,3 @@ where &self.reader } } - -pub async fn check_user_exists( - admin_token: &str, - gotrue_client: &gotrue::api::Client, - email: &str, -) -> Result { - let users = gotrue_client - .admin_list_user(admin_token, Some(email)) - .await?; - for user in users.users { - if user.email == email { - return Ok(true); - } - } - Ok(false) -} diff --git a/src/biz/workspace/ops.rs b/src/biz/workspace/ops.rs index ea3016fa..503da55d 100644 --- a/src/biz/workspace/ops.rs +++ b/src/biz/workspace/ops.rs @@ -9,7 +9,6 @@ use database::workspace::{ select_workspace_member_list, update_updated_at_of_workspace, upsert_workspace_member, }; use database_entity::dto::{AFAccessLevel, AFRole, AFWorkspace}; -use gotrue::params::InviteUserParams; use shared_entity::dto::workspace_dto::{CreateWorkspaceMember, WorkspaceMemberChangeset}; use shared_entity::response::AppResponseError; use sqlx::{types::uuid, PgPool}; @@ -18,10 +17,6 @@ use std::ops::DerefMut; use tracing::instrument; use uuid::Uuid; -use crate::biz::user::create_user_if_not_exists; -use crate::biz::utils::check_user_exists; -use crate::state::AppState; - pub async fn delete_workspace_for_user( pg_pool: &PgPool, workspace_id: &Uuid, @@ -101,52 +96,16 @@ pub async fn open_workspace( /// #[instrument(level = "debug", skip_all, err)] pub async fn add_workspace_members( - state: &AppState, + pg_pool: &PgPool, _user_uuid: &Uuid, workspace_id: &Uuid, members: Vec, ) -> Result, AppError> { - // Invite user to workspace if user is not registered - let admin_token = state.gotrue_admin.token(&state.gotrue_client).await?; - let mut txn = state - .pg_pool + let mut txn = pg_pool .begin() .await .context("Begin transaction to insert workspace members")?; - let gotrue_client = &state.gotrue_client; - for member in members.iter() { - let user_exists = check_user_exists(&admin_token, gotrue_client, &member.email).await?; - if !user_exists { - let user = gotrue_client - .admin_invite_user( - &admin_token, - &InviteUserParams { - email: member.email.clone(), - ..Default::default() - }, - ) - .await?; - let is_new = create_user_if_not_exists(&user, &mut txn, state).await?; - if !is_new { - tracing::error!("User should be new but is not new. User: {:?}", user); - } - } - } - let role_by_uid = add_workspace_members_db(workspace_id, members, &mut txn).await?; - - txn - .commit() - .await - .context("Commit transaction to insert workspace members")?; - Ok(role_by_uid) -} - -pub async fn add_workspace_members_db( - workspace_id: &Uuid, - members: Vec, - txn: &mut sqlx::Transaction<'_, sqlx::Postgres>, -) -> Result, AppError> { let mut role_by_uid = HashMap::new(); for member in members.into_iter() { let access_level = match &member.role { @@ -160,11 +119,16 @@ pub async fn add_workspace_members_db( // "Failed to get uid from email {} when adding workspace members", // member.email // ))?; - insert_workspace_member_with_txn(txn, workspace_id, &member.email, member.role.clone()).await?; - upsert_collab_member_with_txn(uid, workspace_id.to_string(), &access_level, txn).await?; + insert_workspace_member_with_txn(&mut txn, workspace_id, &member.email, member.role.clone()) + .await?; + upsert_collab_member_with_txn(uid, workspace_id.to_string(), &access_level, &mut txn).await?; role_by_uid.insert(uid, member.role); } + txn + .commit() + .await + .context("Commit transaction to insert workspace members")?; Ok(role_by_uid) } diff --git a/src/state.rs b/src/state.rs index fcf85706..54685a23 100644 --- a/src/state.rs +++ b/src/state.rs @@ -1,4 +1,3 @@ -use crate::application::GoTrueAdmin; use crate::biz::casbin::{CollabAccessControlImpl, WorkspaceAccessControlImpl}; use crate::biz::collab::storage::CollabPostgresDBStorage; use crate::biz::pg_listener::PgListeners; @@ -30,7 +29,6 @@ pub struct AppState { pub users: Arc, pub id_gen: Arc>, pub gotrue_client: gotrue::api::Client, - pub gotrue_admin: GoTrueAdmin, pub redis_client: RedisClient, pub collab_storage: Arc, pub collab_access_control: CollabAccessControlImpl, diff --git a/tests/access_control/collab_ac_test.rs b/tests/access_control/collab_ac_test.rs index aeef6733..e4519edd 100644 --- a/tests/access_control/collab_ac_test.rs +++ b/tests/access_control/collab_ac_test.rs @@ -1,6 +1,7 @@ use crate::access_control::*; use actix_http::Method; use anyhow::{anyhow, Context}; +use appflowy_cloud::biz; use appflowy_cloud::biz::casbin::access_control::{Action, ActionType, ObjectType}; use database_entity::dto::{AFAccessLevel, AFRole}; @@ -41,7 +42,10 @@ async fn test_collab_access_control(pool: PgPool) -> anyhow::Result<()> { role: AFRole::Guest, }, ]; - let _ = add_workspace_members_in_tx(&pool, &workspace.workspace_id, members).await; + let _ = + biz::workspace::ops::add_workspace_members(&pool, &user.uuid, &workspace.workspace_id, members) + .await + .context("adding users to workspace")?; // user that created the workspace should have full access assert_access_level( @@ -144,15 +148,18 @@ async fn test_collab_access_control_access_http_method(pool: PgPool) -> anyhow:: .next() .ok_or(anyhow!("workspace should be created"))?; - let _ = add_workspace_members_in_tx( + let _ = biz::workspace::ops::add_workspace_members( &pool, + &guest.uuid, &workspace.workspace_id, vec![CreateWorkspaceMember { email: guest.email, role: AFRole::Guest, }], ) - .await; + .await + .context("adding users to workspace") + .unwrap(); for method in [Method::GET, Method::POST, Method::PUT, Method::DELETE] { assert_can_access_http_method( @@ -236,15 +243,17 @@ async fn test_collab_access_control_send_receive_collab_update(pool: PgPool) -> .next() .ok_or(anyhow!("workspace should be created"))?; - let _ = add_workspace_members_in_tx( + let _ = biz::workspace::ops::add_workspace_members( &pool, + &guest.uuid, &workspace.workspace_id, vec![CreateWorkspaceMember { email: guest.email, role: AFRole::Guest, }], ) - .await; + .await + .context("adding users to workspace")?; // Need to wait for the listener(spawn_listen_on_workspace_member_change) to receive the event sleep(Duration::from_secs(2)).await; diff --git a/tests/access_control/member_ac_test.rs b/tests/access_control/member_ac_test.rs index 297ffd20..a4cb616d 100644 --- a/tests/access_control/member_ac_test.rs +++ b/tests/access_control/member_ac_test.rs @@ -1,6 +1,5 @@ use crate::access_control::{ - add_workspace_members_in_tx, assert_workspace_role, assert_workspace_role_error, create_user, - setup_access_control, + assert_workspace_role, assert_workspace_role_error, create_user, setup_access_control, }; use anyhow::{anyhow, Context}; use app_error::ErrorCode; @@ -34,16 +33,17 @@ async fn test_workspace_access_control_get_role(pool: PgPool) -> anyhow::Result< .await; let member = create_user(&pool).await?; - - let _ = add_workspace_members_in_tx( + let _ = biz::workspace::ops::add_workspace_members( &pool, + &member.uuid, &workspace.workspace_id, vec![CreateWorkspaceMember { email: member.email.clone(), role: AFRole::Member, }], ) - .await; + .await + .context("adding users to workspace")?; assert_workspace_role( &workspace_access_control, diff --git a/tests/access_control/mod.rs b/tests/access_control/mod.rs index 00207aab..a677c536 100644 --- a/tests/access_control/mod.rs +++ b/tests/access_control/mod.rs @@ -1,21 +1,18 @@ use actix_http::Method; use anyhow::{Context, Error}; use app_error::ErrorCode; -use appflowy_cloud::biz; use appflowy_cloud::biz::casbin::{CollabAccessControlImpl, WorkspaceAccessControlImpl}; use appflowy_cloud::biz::workspace::access_control::WorkspaceAccessControl; use client_api_test_util::setup_log; use database_entity::dto::{AFAccessLevel, AFRole}; use lazy_static::lazy_static; use realtime::collaborate::CollabAccessControl; -use shared_entity::dto::workspace_dto::CreateWorkspaceMember; use snowflake::Snowflake; use sqlx::PgPool; use std::sync::Arc; use casbin::{CoreApi, DefaultModel, Enforcer}; use dashmap::DashMap; -use std::collections::HashMap; use std::time::Duration; use tokio::sync::RwLock; use tokio::time::{interval, timeout}; @@ -310,22 +307,3 @@ pub async fn assert_can_access_http_method( timeout(timeout_duration, operation).await?; Ok(()) } - -pub async fn add_workspace_members_in_tx( - pool: &PgPool, - workspace_id: &Uuid, - members: Vec, -) -> HashMap { - let mut txn = pool - .begin() - .await - .expect("acquire transaction to add workspace members"); - let res = biz::workspace::ops::add_workspace_members_db(workspace_id, members, &mut txn) - .await - .expect("add workspace members"); - txn - .commit() - .await - .expect("commit transaction to add workspace members"); - res -} diff --git a/tests/access_control/user_ac_test.rs b/tests/access_control/user_ac_test.rs index da6642ce..93bfe455 100644 --- a/tests/access_control/user_ac_test.rs +++ b/tests/access_control/user_ac_test.rs @@ -1,6 +1,8 @@ use crate::access_control::*; use anyhow::anyhow; +use appflowy_cloud::biz; use appflowy_cloud::biz::casbin::access_control::{Action, ObjectType, ToCasbinAction}; + use casbin::CoreApi; use database_entity::dto::{AFAccessLevel, AFRole}; @@ -94,7 +96,14 @@ async fn test_add_users_to_workspace(pool: PgPool) -> anyhow::Result<()> { role: AFRole::Guest, }, ]; - let _ = add_workspace_members_in_tx(&pool, &workspace.workspace_id, members).await; + let _ = biz::workspace::ops::add_workspace_members( + &pool, + &user_main.uuid, + &workspace.workspace_id, + members, + ) + .await + .context("adding users to workspace")?; let enforcer = setup_enforcer(&pool).await?; { // Owner @@ -223,8 +232,14 @@ async fn test_reload_policy_after_adding_user_to_workspace(pool: PgPool) -> anyh email: user_member.email.clone(), role: AFRole::Member, }]; - - let _ = add_workspace_members_in_tx(&pool, &workspace.workspace_id, members).await; + let _ = biz::workspace::ops::add_workspace_members( + &pool, + &user_owner.uuid, + &workspace.workspace_id, + members, + ) + .await + .context("adding users to workspace")?; assert!(!enforcer .enforce(( diff --git a/tests/gotrue/admin.rs b/tests/gotrue/admin.rs index 28e91179..5eda6a46 100644 --- a/tests/gotrue/admin.rs +++ b/tests/gotrue/admin.rs @@ -47,7 +47,7 @@ async fn admin_user_create_list_edit_delete() { // list users let users = gotrue_client - .admin_list_user(&admin_token.access_token, None) + .admin_list_user(&admin_token.access_token) .await .unwrap() .users; @@ -94,7 +94,7 @@ async fn admin_user_create_list_edit_delete() { .unwrap(); let users = gotrue_client - .admin_list_user(&admin_token.access_token, None) + .admin_list_user(&admin_token.access_token) .await .unwrap() .users; diff --git a/tests/workspace/member_crud.rs b/tests/workspace/member_crud.rs index 4350b5de..7a4b599c 100644 --- a/tests/workspace/member_crud.rs +++ b/tests/workspace/member_crud.rs @@ -1,5 +1,5 @@ use app_error::ErrorCode; -use client_api_test_util::{admin_user_client, TestClient}; +use client_api_test_util::TestClient; use database_entity::dto::AFRole; use shared_entity::dto::workspace_dto::CreateWorkspaceMember; @@ -36,6 +36,25 @@ async fn add_duplicate_workspace_members() { .await; } +#[tokio::test] +async fn add_not_exist_workspace_members() { + let c1 = TestClient::new_user_without_ws_conn().await; + let workspace_id = c1.workspace_id().await; + let email = format!("{}@appflowy.io", uuid::Uuid::new_v4()); + let err = c1 + .api_client + .add_workspace_members( + workspace_id, + vec![CreateWorkspaceMember { + email, + role: AFRole::Member, + }], + ) + .await + .unwrap_err(); + + assert_eq!(err.code, ErrorCode::RecordNotFound); +} #[tokio::test] async fn update_workspace_member_role_not_enough_permission() { let c1 = TestClient::new_user_without_ws_conn().await; @@ -234,34 +253,3 @@ async fn get_user_workspace_info_after_open_workspace() { workspace_id_c1 ); } - -#[tokio::test] -async fn add_workspace_member_not_exists() { - let c1 = TestClient::new_user_without_ws_conn().await; - let workspace_id_c1 = c1.workspace_id().await; - let non_exist_email = format!("{}@appflowy.io", uuid::Uuid::new_v4()); - c1.api_client - .add_workspace_members( - &workspace_id_c1, - vec![CreateWorkspaceMember { - email: non_exist_email.clone(), - role: AFRole::Member, - }], - ) - .await - .expect("should be able to add member whose email not exists yet"); - - let admin_client = admin_user_client().await; - let users = admin_client - .admin_list_users(Some(&non_exist_email)) - .await - .expect("should be able to list users"); - println!("---------non_exist_email: {}", non_exist_email); - let non_exist_user = users - .iter() - .find(|u| u.email == non_exist_email) - .expect("should find the user"); - - // since user does not exist and is just created, it should not be confirmed yet - assert!(non_exist_user.confirmed_at.is_none()); -}