refactor(workspace): pass workspace object instead of id to service methods
- Replace workspace_id parameter with Workspace object reference in all workspace service methods - Remove redundant find_workspace_by_id calls that were duplicated in each method - Update all method signatures across approval, audit, billing, branding, core, settings and stats modules - Modify SQL queries to bind ws.id instead of separate workspace_id parameter - Add Workspace import to all affected modules - Adjust method calls in API handlers to pass workspace object instead of id - Consolidate workspace retrieval logic to single location per operation flow
This commit is contained in:
@@ -5,35 +5,27 @@ use uuid::Uuid;
|
||||
|
||||
use crate::error::AppError;
|
||||
use crate::models::common::{EventType, Role};
|
||||
use crate::models::workspaces::WorkspaceWebhook;
|
||||
use crate::models::workspaces::{Workspace, WorkspaceWebhook};
|
||||
use crate::service::WorkspaceService;
|
||||
use crate::session::Session;
|
||||
|
||||
use super::util::{clamp_limit_offset, ensure_affected, required_text};
|
||||
|
||||
/// Validate webhook URL for SSRF protection
|
||||
fn validate_webhook_url(url_str: &str) -> Result<(), AppError> {
|
||||
let url = Url::parse(url_str).map_err(|_| AppError::BadRequest("Invalid URL format".into()))?;
|
||||
|
||||
// Only allow HTTPS
|
||||
if url.scheme() != "https" {
|
||||
return Err(AppError::BadRequest(
|
||||
"Webhook URL must use HTTPS protocol".into(),
|
||||
));
|
||||
}
|
||||
|
||||
let host = url
|
||||
.host_str()
|
||||
.ok_or_else(|| AppError::BadRequest("URL must have a host".into()))?;
|
||||
|
||||
// Reject IP addresses directly (require domain names)
|
||||
if host.parse::<IpAddr>().is_ok() {
|
||||
return Err(AppError::BadRequest(
|
||||
"Webhook URL must use a domain name, not an IP address".into(),
|
||||
));
|
||||
}
|
||||
|
||||
// Reject localhost and common local domains
|
||||
let host_lower = host.to_lowercase();
|
||||
if host_lower == "localhost"
|
||||
|| host_lower.ends_with(".localhost")
|
||||
@@ -47,14 +39,11 @@ fn validate_webhook_url(url_str: &str) -> Result<(), AppError> {
|
||||
"Webhook URL cannot point to localhost or internal domains".into(),
|
||||
));
|
||||
}
|
||||
|
||||
// Reject metadata endpoints (AWS, GCP, Azure)
|
||||
if host == "169.254.169.254" || host == "metadata.google.internal" {
|
||||
return Err(AppError::BadRequest(
|
||||
"Webhook URL cannot point to cloud metadata endpoints".into(),
|
||||
));
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -78,12 +67,11 @@ impl WorkspaceService {
|
||||
pub async fn workspace_webhooks(
|
||||
&self,
|
||||
ctx: &Session,
|
||||
workspace_id: Uuid,
|
||||
ws: &Workspace,
|
||||
limit: i64,
|
||||
offset: i64,
|
||||
) -> Result<Vec<WorkspaceWebhook>, AppError> {
|
||||
let user_uid = ctx.user().ok_or(AppError::Unauthorized)?;
|
||||
let ws = self.find_workspace_by_id(workspace_id).await?;
|
||||
self.ensure_workspace_role_at_least(user_uid, &ws, Role::Admin)
|
||||
.await?;
|
||||
let (limit, offset) = clamp_limit_offset(limit, offset);
|
||||
@@ -92,7 +80,7 @@ impl WorkspaceService {
|
||||
last_delivery_status, last_delivery_at, created_by, created_at, updated_at \
|
||||
FROM workspace_webhook WHERE workspace_id = $1 ORDER BY created_at DESC LIMIT $2 OFFSET $3",
|
||||
)
|
||||
.bind(workspace_id)
|
||||
.bind(ws.id)
|
||||
.bind(limit)
|
||||
.bind(offset)
|
||||
.fetch_all(self.ctx.db.reader())
|
||||
@@ -103,11 +91,10 @@ impl WorkspaceService {
|
||||
pub async fn workspace_create_webhook(
|
||||
&self,
|
||||
ctx: &Session,
|
||||
workspace_id: Uuid,
|
||||
ws: &Workspace,
|
||||
params: CreateWebhookParams,
|
||||
) -> Result<WorkspaceWebhook, AppError> {
|
||||
let user_uid = ctx.user().ok_or(AppError::Unauthorized)?;
|
||||
let ws = self.find_workspace_by_id(workspace_id).await?;
|
||||
self.ensure_workspace_role_at_least(user_uid, &ws, Role::Admin)
|
||||
.await?;
|
||||
|
||||
@@ -141,7 +128,7 @@ impl WorkspaceService {
|
||||
last_delivery_status, last_delivery_at, created_by, created_at, updated_at",
|
||||
)
|
||||
.bind(Uuid::now_v7())
|
||||
.bind(workspace_id)
|
||||
.bind(ws.id)
|
||||
.bind(&url)
|
||||
.bind(¶ms.secret_ciphertext)
|
||||
.bind(¶ms.events)
|
||||
@@ -159,12 +146,11 @@ impl WorkspaceService {
|
||||
pub async fn workspace_update_webhook(
|
||||
&self,
|
||||
ctx: &Session,
|
||||
workspace_id: Uuid,
|
||||
ws: &Workspace,
|
||||
webhook_id: Uuid,
|
||||
params: UpdateWebhookParams,
|
||||
) -> Result<WorkspaceWebhook, AppError> {
|
||||
let user_uid = ctx.user().ok_or(AppError::Unauthorized)?;
|
||||
let ws = self.find_workspace_by_id(workspace_id).await?;
|
||||
self.ensure_workspace_role_at_least(user_uid, &ws, Role::Admin)
|
||||
.await?;
|
||||
|
||||
@@ -174,7 +160,7 @@ impl WorkspaceService {
|
||||
FROM workspace_webhook WHERE id = $1 AND workspace_id = $2",
|
||||
)
|
||||
.bind(webhook_id)
|
||||
.bind(workspace_id)
|
||||
.bind(ws.id)
|
||||
.fetch_optional(self.ctx.db.reader())
|
||||
.await
|
||||
.map_err(AppError::Database)?
|
||||
@@ -186,7 +172,6 @@ impl WorkspaceService {
|
||||
.map(|u| u.trim().to_string())
|
||||
.unwrap_or(current.url);
|
||||
|
||||
// Validate URL if it was updated
|
||||
if params.url.is_some() {
|
||||
validate_webhook_url(&url)?;
|
||||
}
|
||||
@@ -219,7 +204,7 @@ impl WorkspaceService {
|
||||
.bind(active)
|
||||
.bind(now)
|
||||
.bind(webhook_id)
|
||||
.bind(workspace_id)
|
||||
.bind(ws.id)
|
||||
.fetch_one(&mut *txn)
|
||||
.await
|
||||
.map_err(AppError::Database)?;
|
||||
@@ -231,11 +216,10 @@ impl WorkspaceService {
|
||||
pub async fn workspace_delete_webhook(
|
||||
&self,
|
||||
ctx: &Session,
|
||||
workspace_id: Uuid,
|
||||
ws: &Workspace,
|
||||
webhook_id: Uuid,
|
||||
) -> Result<(), AppError> {
|
||||
let user_uid = ctx.user().ok_or(AppError::Unauthorized)?;
|
||||
let ws = self.find_workspace_by_id(workspace_id).await?;
|
||||
self.ensure_workspace_role_at_least(user_uid, &ws, Role::Admin)
|
||||
.await?;
|
||||
|
||||
@@ -255,7 +239,7 @@ impl WorkspaceService {
|
||||
let result =
|
||||
sqlx::query("DELETE FROM workspace_webhook WHERE id = $1 AND workspace_id = $2")
|
||||
.bind(webhook_id)
|
||||
.bind(workspace_id)
|
||||
.bind(ws.id)
|
||||
.execute(&mut *txn)
|
||||
.await
|
||||
.map_err(AppError::Database)?;
|
||||
|
||||
Reference in New Issue
Block a user