From 140d98917c9c1db3bf4ac3dcde0829c7a5bddb1f Mon Sep 17 00:00:00 2001 From: Seb Ringrose Date: Tue, 5 May 2026 12:25:38 +0100 Subject: [PATCH 1/2] feat(webhooks): allow org members read-only access to org webhooks Org members previously had no visibility into webhooks owned by the organization, leaving them unable to spot delivery failures and notify admins. Add a read-only path: list_webhooks and get_webhook fall through to is_org_member when no direct or own-resource permission applies. Mutating handlers (create/update/delete/rotate-secret) keep the existing can_manage_org_resource check, so writes still require owner/admin. Frontend: NotificationSettings gains a readOnly prop that hides email and low-balance toggles, the Add Webhook button, and the per-row Switch/Edit/Delete controls. MyOrganization now always renders the panel and passes readOnly={!canManage}, so members see the same webhook list (URL, events, scope, enabled state, auto-disable warning) without any controls or secrets. --- .../notifications/NotificationSettings.tsx | 132 ++++++----- .../organizations/MyOrganization.test.tsx | 12 +- .../features/organizations/MyOrganization.tsx | 8 +- dwctl/src/api/handlers/webhooks.rs | 207 +++++++++++++++++- 4 files changed, 296 insertions(+), 63 deletions(-) diff --git a/dashboard/src/components/features/notifications/NotificationSettings.tsx b/dashboard/src/components/features/notifications/NotificationSettings.tsx index 27ba61ec2..dbd339b4d 100644 --- a/dashboard/src/components/features/notifications/NotificationSettings.tsx +++ b/dashboard/src/components/features/notifications/NotificationSettings.tsx @@ -149,12 +149,20 @@ interface NotificationSettingsProps { showPlatformScope?: boolean; /** Whether the userId refers to an organization */ isOrganization?: boolean; + /** + * Render webhook list as read-only — hides email & low-balance toggles + * and all webhook mutation controls. Used to let regular org members see + * the org's webhooks (and any delivery failures) without being able to + * change them. + */ + readOnly?: boolean; } export const NotificationSettings: React.FC = ({ userId, showPlatformScope = false, isOrganization = false, + readOnly = false, }) => { const { data: user, refetch: refetchUser } = useUser(userId); const updateUserMutation = useUpdateUser(); @@ -355,9 +363,11 @@ export const NotificationSettings: React.FC = ({ <>

- Notifications + {readOnly ? "Webhooks" : "Notifications"}

+ {!readOnly && ( + <> {/* Email Section */}
Email @@ -446,10 +456,15 @@ export const NotificationSettings: React.FC = ({
+ + )} + {/* Webhooks Section */} -
- Webhooks -
+ {!readOnly && ( +
+ Webhooks +
+ )}
@@ -458,19 +473,23 @@ export const NotificationSettings: React.FC = ({

- Receive HTTP callbacks when events occur + {readOnly + ? "HTTP callbacks sent when events occur. Contact an admin to make changes." + : "Receive HTTP callbacks when events occur"}

- + {!readOnly && ( + + )}
{/* Webhook List */} @@ -541,53 +560,56 @@ export const NotificationSettings: React.FC = ({ )} -
- - handleWebhookToggle(webhook) - } - aria-label={`Toggle webhook ${webhook.url}`} - /> - - - - - Edit webhook - - - - - - Delete webhook - -
+ {!readOnly && ( +
+ + handleWebhookToggle(webhook) + } + aria-label={`Toggle webhook ${webhook.url}`} + /> + + + + + Edit webhook + + + + + + Delete webhook + +
+ )} ))} ) : (
- No webhooks configured. Add one to receive HTTP - notifications. + {readOnly + ? "No webhooks configured." + : "No webhooks configured. Add one to receive HTTP notifications."}
)} diff --git a/dashboard/src/components/features/organizations/MyOrganization.test.tsx b/dashboard/src/components/features/organizations/MyOrganization.test.tsx index 3cf7f9bcf..17c41c4ec 100644 --- a/dashboard/src/components/features/organizations/MyOrganization.test.tsx +++ b/dashboard/src/components/features/organizations/MyOrganization.test.tsx @@ -101,7 +101,7 @@ describe("MyOrganization", () => { ).toBeInTheDocument(); }); - it("does not render notification settings for regular members", async () => { + it("renders read-only webhooks view for regular members", async () => { server.use(userWithOrg("member")); const { container } = render(, { wrapper: createWrapper(), @@ -113,9 +113,19 @@ describe("MyOrganization", () => { ).toBeInTheDocument(); }); + // Read-only view: heading is "Webhooks", and admin-only controls are absent. + expect( + within(container).getByRole("heading", { name: "Webhooks" }), + ).toBeInTheDocument(); expect( within(container).queryByRole("heading", { name: "Notifications" }), ).not.toBeInTheDocument(); + expect( + within(container).queryByRole("button", { name: "Add webhook" }), + ).not.toBeInTheDocument(); + expect( + within(container).queryByRole("switch", { name: "Email notifications" }), + ).not.toBeInTheDocument(); }); it("passes the org ID to NotificationSettings", async () => { diff --git a/dashboard/src/components/features/organizations/MyOrganization.tsx b/dashboard/src/components/features/organizations/MyOrganization.tsx index c34472e3e..84885509c 100644 --- a/dashboard/src/components/features/organizations/MyOrganization.tsx +++ b/dashboard/src/components/features/organizations/MyOrganization.tsx @@ -63,9 +63,11 @@ export function MyOrganization() { readOnly={!canManage} /> - {canManage && ( - - )} + ); } diff --git a/dwctl/src/api/handlers/webhooks.rs b/dwctl/src/api/handlers/webhooks.rs index 8d6edc87c..e8b053ea1 100644 --- a/dwctl/src/api/handlers/webhooks.rs +++ b/dwctl/src/api/handlers/webhooks.rs @@ -61,10 +61,10 @@ pub async fn list_webhooks( if !can_read_all && !can_read_own { let mut conn = state.db.read().acquire().await.map_err(|e| Error::Database(e.into()))?; - let can_org = permissions::can_manage_org_resource(¤t_user, target_user_id, &mut conn) + let is_member = permissions::is_org_member(¤t_user, target_user_id, &mut conn) .await .map_err(Error::Database)?; - if !can_org { + if !is_member { return Err(Error::InsufficientPermissions { required: Permission::Any(vec![ Permission::Allow(Resource::Webhooks, Operation::ReadAll), @@ -258,10 +258,10 @@ pub async fn get_webhook( if !can_read_all && !can_read_own { let mut conn = state.db.read().acquire().await.map_err(|e| Error::Database(e.into()))?; - let can_org = permissions::can_manage_org_resource(¤t_user, target_user_id, &mut conn) + let is_member = permissions::is_org_member(¤t_user, target_user_id, &mut conn) .await .map_err(Error::Database)?; - if !can_org { + if !is_member { return Err(Error::InsufficientPermissions { required: Permission::Any(vec![ Permission::Allow(Resource::Webhooks, Operation::ReadAll), @@ -859,4 +859,203 @@ mod tests { response.assert_status_ok(); } + + // ── Read-only org member webhook access ──────────────────────────────── + // + // Members of an org (any role, including plain "member") can list and read + // the org's webhooks so they can spot delivery failures and notify admins, + // but they cannot mutate them. + + /// Helper: create a webhook for an org via the API as the owner, returning + /// the created webhook (with secret). + async fn create_org_webhook( + app: &axum_test::TestServer, + owner: &crate::api::models::users::UserResponse, + org_id: UserId, + url: &str, + ) -> WebhookWithSecretResponse { + let response = app + .post(&format!("/admin/api/v1/users/{}/webhooks", org_id)) + .add_header(&add_auth_headers(owner)[0].0, &add_auth_headers(owner)[0].1) + .add_header(&add_auth_headers(owner)[1].0, &add_auth_headers(owner)[1].1) + .json(&json!({ "url": url })) + .await; + response.assert_status(StatusCode::CREATED); + response.json() + } + + #[sqlx::test] + #[test_log::test] + async fn test_org_member_can_list_org_webhooks_without_secrets(pool: PgPool) { + let (app, _bg_services) = create_test_app(pool.clone(), false).await; + let alice = create_test_user(&pool, Role::StandardUser).await; + let bob = create_test_user(&pool, Role::StandardUser).await; + let org = create_test_org(&pool, alice.id).await; + add_org_member(&pool, org.id, bob.id, "member").await; + + create_org_webhook(&app, &alice, org.id, "https://example.com/hook").await; + + // Bob (plain member) lists the org's webhooks + let response = app + .get(&format!("/admin/api/v1/users/{}/webhooks", org.id)) + .add_header(&add_auth_headers(&bob)[0].0, &add_auth_headers(&bob)[0].1) + .add_header(&add_auth_headers(&bob)[1].0, &add_auth_headers(&bob)[1].1) + .await; + + response.assert_status_ok(); + let webhooks: Vec = response.json(); + assert_eq!(webhooks.len(), 1); + assert_eq!(webhooks[0].url, "https://example.com/hook"); + + // Secrets must never appear in member-visible responses. WebhookResponse + // has no `secret` field, but check the raw JSON too as a belt-and-braces + // guard against future struct changes. + let raw: serde_json::Value = response.json(); + let item = &raw.as_array().unwrap()[0]; + assert!(item.get("secret").is_none(), "list response must not include secret"); + } + + #[sqlx::test] + #[test_log::test] + async fn test_org_member_can_get_single_org_webhook(pool: PgPool) { + let (app, _bg_services) = create_test_app(pool.clone(), false).await; + let alice = create_test_user(&pool, Role::StandardUser).await; + let bob = create_test_user(&pool, Role::StandardUser).await; + let org = create_test_org(&pool, alice.id).await; + add_org_member(&pool, org.id, bob.id, "member").await; + + let created = create_org_webhook(&app, &alice, org.id, "https://example.com/hook").await; + + let response = app + .get(&format!("/admin/api/v1/users/{}/webhooks/{}", org.id, created.id)) + .add_header(&add_auth_headers(&bob)[0].0, &add_auth_headers(&bob)[0].1) + .add_header(&add_auth_headers(&bob)[1].0, &add_auth_headers(&bob)[1].1) + .await; + + response.assert_status_ok(); + let raw: serde_json::Value = response.json(); + assert!(raw.get("secret").is_none(), "get response must not include secret"); + } + + #[sqlx::test] + #[test_log::test] + async fn test_non_member_cannot_list_org_webhooks(pool: PgPool) { + let (app, _bg_services) = create_test_app(pool.clone(), false).await; + let alice = create_test_user(&pool, Role::StandardUser).await; + let outsider = create_test_user(&pool, Role::StandardUser).await; + let org = create_test_org(&pool, alice.id).await; + + create_org_webhook(&app, &alice, org.id, "https://example.com/hook").await; + + let response = app + .get(&format!("/admin/api/v1/users/{}/webhooks", org.id)) + .add_header(&add_auth_headers(&outsider)[0].0, &add_auth_headers(&outsider)[0].1) + .add_header(&add_auth_headers(&outsider)[1].0, &add_auth_headers(&outsider)[1].1) + .await; + + response.assert_status_forbidden(); + } + + #[sqlx::test] + #[test_log::test] + async fn test_org_member_cannot_create_org_webhook(pool: PgPool) { + let (app, _bg_services) = create_test_app(pool.clone(), false).await; + let alice = create_test_user(&pool, Role::StandardUser).await; + let bob = create_test_user(&pool, Role::StandardUser).await; + let org = create_test_org(&pool, alice.id).await; + add_org_member(&pool, org.id, bob.id, "member").await; + + let response = app + .post(&format!("/admin/api/v1/users/{}/webhooks", org.id)) + .add_header(&add_auth_headers(&bob)[0].0, &add_auth_headers(&bob)[0].1) + .add_header(&add_auth_headers(&bob)[1].0, &add_auth_headers(&bob)[1].1) + .json(&json!({ "url": "https://example.com/hook" })) + .await; + + response.assert_status_forbidden(); + } + + #[sqlx::test] + #[test_log::test] + async fn test_org_member_cannot_update_org_webhook(pool: PgPool) { + let (app, _bg_services) = create_test_app(pool.clone(), false).await; + let alice = create_test_user(&pool, Role::StandardUser).await; + let bob = create_test_user(&pool, Role::StandardUser).await; + let org = create_test_org(&pool, alice.id).await; + add_org_member(&pool, org.id, bob.id, "member").await; + + let created = create_org_webhook(&app, &alice, org.id, "https://example.com/hook").await; + + let response = app + .patch(&format!("/admin/api/v1/users/{}/webhooks/{}", org.id, created.id)) + .add_header(&add_auth_headers(&bob)[0].0, &add_auth_headers(&bob)[0].1) + .add_header(&add_auth_headers(&bob)[1].0, &add_auth_headers(&bob)[1].1) + .json(&json!({ "enabled": false })) + .await; + + response.assert_status_forbidden(); + } + + #[sqlx::test] + #[test_log::test] + async fn test_org_member_cannot_delete_org_webhook(pool: PgPool) { + let (app, _bg_services) = create_test_app(pool.clone(), false).await; + let alice = create_test_user(&pool, Role::StandardUser).await; + let bob = create_test_user(&pool, Role::StandardUser).await; + let org = create_test_org(&pool, alice.id).await; + add_org_member(&pool, org.id, bob.id, "member").await; + + let created = create_org_webhook(&app, &alice, org.id, "https://example.com/hook").await; + + let response = app + .delete(&format!("/admin/api/v1/users/{}/webhooks/{}", org.id, created.id)) + .add_header(&add_auth_headers(&bob)[0].0, &add_auth_headers(&bob)[0].1) + .add_header(&add_auth_headers(&bob)[1].0, &add_auth_headers(&bob)[1].1) + .await; + + response.assert_status_forbidden(); + } + + #[sqlx::test] + #[test_log::test] + async fn test_org_member_cannot_rotate_org_webhook_secret(pool: PgPool) { + let (app, _bg_services) = create_test_app(pool.clone(), false).await; + let alice = create_test_user(&pool, Role::StandardUser).await; + let bob = create_test_user(&pool, Role::StandardUser).await; + let org = create_test_org(&pool, alice.id).await; + add_org_member(&pool, org.id, bob.id, "member").await; + + let created = create_org_webhook(&app, &alice, org.id, "https://example.com/hook").await; + + let response = app + .post(&format!("/admin/api/v1/users/{}/webhooks/{}/rotate-secret", org.id, created.id)) + .add_header(&add_auth_headers(&bob)[0].0, &add_auth_headers(&bob)[0].1) + .add_header(&add_auth_headers(&bob)[1].0, &add_auth_headers(&bob)[1].1) + .await; + + response.assert_status_forbidden(); + } + + #[sqlx::test] + #[test_log::test] + async fn test_org_admin_can_still_manage_webhooks(pool: PgPool) { + // Regression guard: existing owner/admin write path must keep working + // after we relaxed the read-permission fallback from can_manage_org_resource + // (owner/admin only) to is_org_member (any role). + let (app, _bg_services) = create_test_app(pool.clone(), false).await; + let alice = create_test_user(&pool, Role::StandardUser).await; + let bob = create_test_user(&pool, Role::StandardUser).await; + let org = create_test_org(&pool, alice.id).await; + add_org_member(&pool, org.id, bob.id, "admin").await; + + // Bob (org admin) creates a webhook for the org. + let response = app + .post(&format!("/admin/api/v1/users/{}/webhooks", org.id)) + .add_header(&add_auth_headers(&bob)[0].0, &add_auth_headers(&bob)[0].1) + .add_header(&add_auth_headers(&bob)[1].0, &add_auth_headers(&bob)[1].1) + .json(&json!({ "url": "https://example.com/hook" })) + .await; + + response.assert_status(StatusCode::CREATED); + } } From 513833db74dcff6b7aff9c93e7d29c89f590f49d Mon Sep 17 00:00:00 2001 From: Seb Ringrose Date: Tue, 5 May 2026 12:51:21 +0100 Subject: [PATCH 2/2] chore(webhooks): address PR review feedback - Update OpenAPI descriptions on list_webhooks and get_webhook to mention org-member read access, so API consumers aren't misled by stale docs. - Inline comments now explain the three-tier authorization (admin / own / org member) so the rule is documented next to the check. - Skip the useUser fetch in NotificationSettings when readOnly: every consumer of the user object lives inside the email/low-balance section, which read-only callers don't render. Adds an `enabled` option to useUser, mirroring the existing useUsers pattern, so a member viewing the org's webhooks doesn't fire a needless GET /users/{org_id}. --- dashboard/src/api/control-layer/hooks.ts | 8 +++++--- .../notifications/NotificationSettings.tsx | 5 ++++- dwctl/src/api/handlers/webhooks.rs | 15 +++++++++++---- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/dashboard/src/api/control-layer/hooks.ts b/dashboard/src/api/control-layer/hooks.ts index 34dc9c005..36cb271f1 100644 --- a/dashboard/src/api/control-layer/hooks.ts +++ b/dashboard/src/api/control-layer/hooks.ts @@ -82,11 +82,13 @@ export function useUsers(options?: UsersQuery & { enabled?: boolean }) { } -export function useUser(id: string, options?: { include?: string }) { +export function useUser(id: string, options?: { include?: string; enabled?: boolean }) { + const { enabled = true, include } = options || {}; return useQuery({ - queryKey: queryKeys.users.byId(id, options?.include), - queryFn: () => dwctlApi.users.get(id, options), + queryKey: queryKeys.users.byId(id, include), + queryFn: () => dwctlApi.users.get(id, { include }), staleTime: 30 * 1000, // 30 seconds - matches useTransactions to keep balance in sync + enabled, }); } diff --git a/dashboard/src/components/features/notifications/NotificationSettings.tsx b/dashboard/src/components/features/notifications/NotificationSettings.tsx index dbd339b4d..192ea6d66 100644 --- a/dashboard/src/components/features/notifications/NotificationSettings.tsx +++ b/dashboard/src/components/features/notifications/NotificationSettings.tsx @@ -164,7 +164,10 @@ export const NotificationSettings: React.FC = ({ isOrganization = false, readOnly = false, }) => { - const { data: user, refetch: refetchUser } = useUser(userId); + // Skip the user fetch in read-only mode — every consumer of `user` is in + // the email/low-balance section, which read-only callers don't render. Also + // avoids a needless 403 if a member lacks permission to read the org user. + const { data: user, refetch: refetchUser } = useUser(userId, { enabled: !readOnly }); const updateUserMutation = useUpdateUser(); const updateOrgMutation = useUpdateOrganization(); const navigate = useNavigate(); diff --git a/dwctl/src/api/handlers/webhooks.rs b/dwctl/src/api/handlers/webhooks.rs index e8b053ea1..872631881 100644 --- a/dwctl/src/api/handlers/webhooks.rs +++ b/dwctl/src/api/handlers/webhooks.rs @@ -27,7 +27,7 @@ use crate::{ path = "/users/{user_id}/webhooks", tag = "webhooks", summary = "List webhooks", - description = "List all webhooks for a user. Users can list their own webhooks; admins can list any user's webhooks.", + description = "List all webhooks for a user. Users can list their own webhooks; admins can list any user's webhooks; members of an organization can list webhooks owned by that organization (read-only).", params( ("user_id" = uuid::Uuid, Path, description = "User ID"), ), @@ -54,7 +54,13 @@ pub async fn list_webhooks( UserIdOrCurrent::Id(id) => id, }; - // Check permissions: can read all webhooks OR read own webhooks + // Allowed if any of: + // - admin permission (ReadAll) + // - reading own webhooks (ReadOwn) + // - target is an organization the caller belongs to (any role) — read-only + // access for org members, so they can spot delivery failures and notify + // admins. Mutating handlers stay restricted to owner/admin via + // can_manage_org_resource. let can_read_all = permissions::has_permission(¤t_user, Resource::Webhooks, Operation::ReadAll); let can_read_own = target_user_id == current_user.id && permissions::has_permission(¤t_user, Resource::Webhooks, Operation::ReadOwn); @@ -222,7 +228,7 @@ pub async fn create_webhook( path = "/users/{user_id}/webhooks/{webhook_id}", tag = "webhooks", summary = "Get webhook", - description = "Get a specific webhook. Secret is not included in the response.", + description = "Get a specific webhook. Users can read their own webhooks; admins can read any user's webhooks; members of an organization can read webhooks owned by that organization (read-only). Secret is not included in the response.", params( ("user_id" = uuid::Uuid, Path, description = "User ID"), ("webhook_id" = uuid::Uuid, Path, description = "Webhook ID"), @@ -251,7 +257,8 @@ pub async fn get_webhook( UserIdOrCurrent::Id(id) => id, }; - // Check permissions + // Same authorization model as list_webhooks: admin (ReadAll), owner + // (ReadOwn), or member of the target organization (read-only). let can_read_all = permissions::has_permission(¤t_user, Resource::Webhooks, Operation::ReadAll); let can_read_own = target_user_id == current_user.id && permissions::has_permission(¤t_user, Resource::Webhooks, Operation::ReadOwn);