From d1c82b78111e9f2e503062571a52371b807f3356 Mon Sep 17 00:00:00 2001 From: "Nathan.fooo" <86001920+appflowy@users.noreply.github.com> Date: Mon, 18 Mar 2024 19:34:44 +0800 Subject: [PATCH] chore: enable access control by env (#394) --- .github/workflows/integration_test.yml | 1 - .github/workflows/push_latest_docker.yml | 2 +- Cargo.toml | 4 +-- deploy.env | 1 + dev.env | 3 +- docker-compose.yml | 3 +- src/application.rs | 10 +++--- src/biz/casbin/access_control.rs | 39 +++++++++++++++------- src/biz/casbin/collab_ac.rs | 41 +++++++++++++----------- src/main.rs | 31 ++---------------- src/middleware/access_control_mw.rs | 7 ++++ 11 files changed, 70 insertions(+), 72 deletions(-) diff --git a/.github/workflows/integration_test.yml b/.github/workflows/integration_test.yml index 3f0aa5b6..35ed205c 100644 --- a/.github/workflows/integration_test.yml +++ b/.github/workflows/integration_test.yml @@ -54,7 +54,6 @@ jobs: run: | sed -i '/image: appflowyinc\/appflowy_cloud:/d' docker-compose.yml sed -i '/image: appflowyinc\/admin_frontend:/d' docker-compose.yml - sed -i 's/FEATURES: "disable_access_control"/FEATURES: ""/g' docker-compose.yml cat docker-compose.yml - name: Run Docker-Compose diff --git a/.github/workflows/push_latest_docker.yml b/.github/workflows/push_latest_docker.yml index 32d3b458..f6892125 100644 --- a/.github/workflows/push_latest_docker.yml +++ b/.github/workflows/push_latest_docker.yml @@ -91,7 +91,7 @@ jobs: labels: ${{ steps.meta.outputs.labels }} provenance: false build-args: | - FEATURES=disable_access_control + FEATURES= - name: Logout from Docker Hub if: always() diff --git a/Cargo.toml b/Cargo.toml index 94a45793..61906588 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,7 +39,7 @@ secrecy = { version = "0.8", features = ["serde"] } rand = { version = "0.8", features = ["std_rng"] } anyhow = "1.0.79" thiserror = "1.0.56" -reqwest = { workspace = true, default-features = false, features = ["json", "rustls-tls", "cookies"] } +reqwest = { workspace = true, features = ["json", "rustls-tls", "cookies"] } unicode-segmentation = "1.10" lazy_static = "1.4.0" fancy-regex = "0.11.0" @@ -182,8 +182,6 @@ collab-document = { git = "https://github.com/AppFlowy-IO/AppFlowy-Collab", rev [features] custom_env= [] -# This feature will be removed once the cpu spike issue is resolved -disable_access_control = [] ai_enable = [] # Comment the above and uncomment the below to use local version of collab by cloning the repo and placing it in libs folder diff --git a/deploy.env b/deploy.env index 320c2f61..2ab57c8b 100644 --- a/deploy.env +++ b/deploy.env @@ -6,6 +6,7 @@ APPFLOWY_GOTRUE_BASE_URL=http://gotrue:9999 ## URL that connects to the postgres docker container APPFLOWY_DATABASE_URL=postgres://postgres:password@postgres:5432/postgres +APPFLOWY_ACCESS_CONTROL=true # admin frontend ## URL that connects to redis docker container diff --git a/dev.env b/dev.env index 14dbf96d..54864591 100644 --- a/dev.env +++ b/dev.env @@ -1,6 +1,7 @@ # gotrue URL that the appflowy service will use to connect to gotrue APPFLOWY_GOTRUE_BASE_URL=http://localhost:9999 APPFLOWY_DATABASE_URL=postgres://postgres:password@localhost:5432/postgres +APPFLOWY_ACCESS_CONTROL=true # This file is used to set the environment variables for local development # Copy this file to .env and change the values as needed @@ -85,4 +86,4 @@ GF_SECURITY_ADMIN_PASSWORD=password CLOUDFLARE_TUNNEL_TOKEN= # AppFlowy AI -OPENAI_API_KEY= \ No newline at end of file +OPENAI_API_KEY= diff --git a/docker-compose.yml b/docker-compose.yml index 48a4c131..5f153940 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -106,11 +106,12 @@ services: - APPFLOWY_S3_BUCKET=${APPFLOWY_S3_BUCKET} - APPFLOWY_S3_REGION=${APPFLOWY_S3_REGION} - APPFLOWY_AI_URL=${APPFLOWY_AI_URL} + - APPFLOWY_ACCESS_CONTROL=${APPFLOWY_ACCESS_CONTROL} build: context: . dockerfile: Dockerfile args: - FEATURES: "disable_access_control" + FEATURES: "" image: appflowyinc/appflowy_cloud:${BACKEND_VERSION:-latest} admin_frontend: diff --git a/src/application.rs b/src/application.rs index 4673269c..16a89fd0 100644 --- a/src/application.rs +++ b/src/application.rs @@ -4,7 +4,7 @@ use crate::api::file_storage::file_storage_scope; use crate::api::user::user_scope; use crate::api::workspace::{collab_scope, workspace_scope}; use crate::api::ws::ws_scope; -use crate::biz::casbin::access_control::AccessControl; +use crate::biz::casbin::access_control::{enable_access_control, AccessControl}; use crate::biz::casbin::RealtimeCollabAccessControlImpl; use crate::biz::collab::access_control::{ @@ -164,9 +164,6 @@ fn get_certificate_and_server_key(config: &Config) -> Option<(Secret, Se pub async fn init_state(config: &Config, rt_cmd_tx: RTCommandSender) -> Result { // Print the feature flags - if cfg!(feature = "disable_access_control") { - info!("Access control is disabled"); - } let metrics = AppMetrics::new(); @@ -199,7 +196,10 @@ pub async fn init_state(config: &Config, rt_cmd_tx: RTCommandSender) -> Result, act: &ActionType, ) -> Result<(), AppError> { - if cfg!(feature = "disable_access_control") { - Ok(()) - } else { + if enable_access_control() { let result = self.enforcer.update_policy(uid, obj, act).await; let _ = self.change_tx.send(AccessControlChange::UpdatePolicy { uid: *uid, oid: obj.object_id().to_string(), }); result + } else { + Ok(()) } } pub async fn remove_policy(&self, uid: &i64, obj: &ObjectType<'_>) -> Result<(), AppError> { - if cfg!(feature = "disable_access_control") { - Ok(()) - } else { + if enable_access_control() { self.enforcer.remove_policy(uid, obj).await?; let _ = self.change_tx.send(AccessControlChange::RemovePolicy { uid: *uid, oid: obj.object_id().to_string(), }); Ok(()) + } else { + Ok(()) } } @@ -117,17 +118,17 @@ impl AccessControl { where A: ToACAction, { - if cfg!(feature = "disable_access_control") { - Ok(true) - } else { + if enable_access_control() { self.enforcer.enforce_policy(uid, obj, act).await + } else { + Ok(true) } } } -/// policy in db: -/// p = 1, 123, 1 (1 mean AFRole::Owner) -/// p = 1, 456, 50 (50 mean AFAccessLevel::FullAccess) +/// policy: +/// p = sub=uid, obj=object_id, act=role_id +/// p = sub=uid, obj=object_id, act=access_level /// /// role_definition in db: /// g = _, _ @@ -354,3 +355,17 @@ impl FromACAction for AFRole { Self::from(action) } } + +lazy_static! { + static ref ENABLE_ACCESS_CONTROL: bool = { + match std::env::var("APPFLOWY_ACCESS_CONTROL") { + Ok(value) => value.eq_ignore_ascii_case("true") || value.eq("1"), + Err(_) => false, + } + }; +} + +#[inline] +pub fn enable_access_control() -> bool { + *ENABLE_ACCESS_CONTROL +} diff --git a/src/biz/casbin/collab_ac.rs b/src/biz/casbin/collab_ac.rs index 4b75b396..2b7d329d 100644 --- a/src/biz/casbin/collab_ac.rs +++ b/src/biz/casbin/collab_ac.rs @@ -1,4 +1,6 @@ -use crate::biz::casbin::access_control::{AccessControl, AccessControlChange, Action}; +use crate::biz::casbin::access_control::{ + enable_access_control, AccessControl, AccessControlChange, Action, +}; use crate::biz::casbin::access_control::{ActionType, ObjectType}; use crate::biz::collab::access_control::CollabAccessControl; use app_error::AppError; @@ -107,27 +109,28 @@ impl RealtimeCollabAccessControlImpl { oid: &str, required_action: Action, ) -> Result { - if cfg!(feature = "disable_access_control") { - return Ok(true); - } - let key = cache_key(*uid, oid); - // Check if the action is already cached - if let Some(action) = self.action_by_oid.get(&key) { - return Ok(*action >= required_action); - } + if enable_access_control() { + let key = cache_key(*uid, oid); + // Check if the action is already cached + if let Some(action) = self.action_by_oid.get(&key) { + return Ok(*action >= required_action); + } - // Not in cache, enforce access control - let is_permitted = self - .access_control - .enforce(uid, &ObjectType::Collab(oid), &required_action) - .await?; + // Not in cache, enforce access control + let is_permitted = self + .access_control + .enforce(uid, &ObjectType::Collab(oid), &required_action) + .await?; - if is_permitted { - // Permission granted, cache the action - self.action_by_oid.insert(key, required_action); + if is_permitted { + // Permission granted, cache the action + self.action_by_oid.insert(key, required_action); + } + + Ok(is_permitted) + } else { + Ok(true) } - - Ok(is_permitted) } } diff --git a/src/main.rs b/src/main.rs index e17ba7f4..979f683b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,7 +1,6 @@ use appflowy_cloud::application::{init_state, Application}; use appflowy_cloud::config::config::get_configuration; use appflowy_cloud::telemetry::init_subscriber; -use tracing::info; #[actix_web::main] async fn main() -> anyhow::Result<()> { @@ -21,34 +20,8 @@ async fn main() -> anyhow::Result<()> { get_configuration().map_err(|e| anyhow::anyhow!("Failed to read configuration: {}", e))?; init_subscriber(&conf.app_env, filters); - // If current build is debug and the feature "custom_env" is not enabled, load from .env - // otherwise, load from .env.without_nginx. - if cfg!(debug_assertions) { - #[cfg(not(feature = "custom_env"))] - { - info!("custom_env is disable, load from .env"); - dotenvy::dotenv().ok(); - } - - #[cfg(feature = "custom_env")] - { - match dotenvy::from_filename(".env.without_nginx") { - Ok(_) => { - info!("custom_env is enabled, load from .env.without_nginx"); - }, - Err(err) => { - tracing::error!( - "Failed to load .env.without_nginx: {}, fallback to .env file", - err - ); - dotenvy::dotenv().ok(); - }, - } - } - } else { - // In release, always load from .env - dotenvy::dotenv().ok(); - } + // Load environment variables from .env file + dotenvy::dotenv().ok(); let (tx, rx) = tokio::sync::mpsc::channel(1000); let state = init_state(&conf, tx) diff --git a/src/middleware/access_control_mw.rs b/src/middleware/access_control_mw.rs index 36b5cc9c..fe6aed44 100644 --- a/src/middleware/access_control_mw.rs +++ b/src/middleware/access_control_mw.rs @@ -18,6 +18,7 @@ use std::future::{ready, Ready}; use std::sync::Arc; use tracing::error; +use crate::biz::casbin::access_control::enable_access_control; use crate::state::AppState; use app_error::AppError; use uuid::Uuid; @@ -118,6 +119,12 @@ where forward_ready!(service); fn call(&self, mut req: ServiceRequest) -> Self::Future { + // If the access control is not enabled, skip the access control + if !enable_access_control() { + let fut = self.service.call(req); + return Box::pin(fut); + } + let path = req.match_pattern().map(|pattern| { // Create ResourceDef will cause memory leak, so we use the cache to store the ResourceDef let mut path = req.match_info().clone();