github-starred/actual
2
0
Fork 0
mirror of https://github.com/actualbudget/actual.git synced 2026-11-12 03:42:19 -06:00
Code Issues 3.0k Packages Projects Releases 58 Wiki Activity

[PR #6339] [CLOSED] Fix non-owner users being able to delete server budget files #104149

New Issue
Closed
opened 2026-05-30 05:09:40 -05:00 by GiteaMirror · 0 comments
GiteaMirror commented 2026-05-30 05:09:40 -05:00
Owner
Copy Link
Copy Source

📋 Pull Request Information

Original PR: https://github.com/actualbudget/actual/pull/6339
Author: @MatissJanis
Created: 12/8/2025
Status: ❌ Closed

Base: master ← Head: matiss/fix-budget-delete-permissions

📝 Commits (1)

  • de42f46 Fix non-owner users being able to delete server budget files

📊 Changes

6 files changed (+161 additions, -18 deletions)

View changed files

📝 packages/desktop-client/src/budgetfiles/budgetfilesSlice.ts (+28 -2)
📝 packages/desktop-client/src/components/modals/manager/DeleteFileModal.tsx (+30 -7)
📝 packages/loot-core/src/server/budgetfiles/app.ts (+23 -4)
📝 packages/sync-server/src/app-sync.test.ts (+60 -4)
📝 packages/sync-server/src/app-sync.ts (+14 -1)
➕ upcoming-release-notes/6339.md (+6 -0)

📄 Description

Summary

This PR fixes issue #6334 where non-owner users were able to delete shared budgets from the server.

Problem

Previously, the /delete-user-file endpoint in the sync server only verified that a file exists before allowing deletion. It did not check if the requesting user is the file owner or an admin, allowing any user with access to the budget to delete it from the server.

Solution

Server-side

  • Added ownership check to /delete-user-file endpoint in app-sync.ts
  • Only the file owner or server admins can delete files from the server
  • Returns 403 Forbidden with appropriate error details for non-owners

Client-side

  • Updated deleteBudget action to handle permission errors and show notifications
  • Added UI feedback in DeleteFileModal:
    • Non-owners see "Only the file owner can delete it from the server"
    • The "Delete file from all devices" button is disabled for non-owners
    • Local deletion is still allowed for everyone

Tests

  • Added test: owner can delete their file
  • Added test: non-owner receives 403 forbidden
  • Added test: admin can delete any file

Testing

  1. Set up multiuser mode with OpenID
  2. Create a budget with user A and share it with user B
  3. As user B, try to delete the budget from the server
  4. Verify user B sees the disabled button and explanatory text
  5. Verify user B can still delete locally
  6. Verify user A (owner) can delete from server

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/actualbudget/actual/pull/6339 **Author:** [@MatissJanis](https://github.com/MatissJanis) **Created:** 12/8/2025 **Status:** ❌ Closed **Base:** `master` ← **Head:** `matiss/fix-budget-delete-permissions` --- ### 📝 Commits (1) - [`de42f46`](https://github.com/actualbudget/actual/commit/de42f469b0a05bbeb25a5ff0d26e59fd206ae16b) Fix non-owner users being able to delete server budget files ### 📊 Changes **6 files changed** (+161 additions, -18 deletions) <details> <summary>View changed files</summary> 📝 `packages/desktop-client/src/budgetfiles/budgetfilesSlice.ts` (+28 -2) 📝 `packages/desktop-client/src/components/modals/manager/DeleteFileModal.tsx` (+30 -7) 📝 `packages/loot-core/src/server/budgetfiles/app.ts` (+23 -4) 📝 `packages/sync-server/src/app-sync.test.ts` (+60 -4) 📝 `packages/sync-server/src/app-sync.ts` (+14 -1) ➕ `upcoming-release-notes/6339.md` (+6 -0) </details> ### 📄 Description ## Summary This PR fixes [issue #6334](https://github.com/actualbudget/actual/issues/6334) where non-owner users were able to delete shared budgets from the server. ## Problem Previously, the `/delete-user-file` endpoint in the sync server only verified that a file exists before allowing deletion. It did not check if the requesting user is the file owner or an admin, allowing any user with access to the budget to delete it from the server. ## Solution ### Server-side - Added ownership check to `/delete-user-file` endpoint in `app-sync.ts` - Only the file owner or server admins can delete files from the server - Returns 403 Forbidden with appropriate error details for non-owners ### Client-side - Updated `deleteBudget` action to handle permission errors and show notifications - Added UI feedback in `DeleteFileModal`: - Non-owners see "Only the file owner can delete it from the server" - The "Delete file from all devices" button is disabled for non-owners - Local deletion is still allowed for everyone ### Tests - Added test: owner can delete their file - Added test: non-owner receives 403 forbidden - Added test: admin can delete any file ## Testing 1. Set up multiuser mode with OpenID 2. Create a budget with user A and share it with user B 3. As user B, try to delete the budget from the server 4. Verify user B sees the disabled button and explanatory text 5. Verify user B can still delete locally 6. Verify user A (owner) can delete from server --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-05-30 05:09:40 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
AI generated
API
bank sync
budgeting
bug
can’t replicate
dependencies
docker
documentation
electron
experimental feature
feature
feedback
goal templates
good first issue
help wanted
importers
maintenance
needs info
needs testing
needs triage
needs votes
openid
payees
pull-request
Mirrored from GitHub Pull Request
 regression
reports
responsive
rules
schedules
server
✨ merged
split transactions
tech debt
tech-support
theme
transaction import
transaction reconciliation
transactions
translations
upstream
user interface
✅ approved
wontfix
No labels pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
GiteaMirror ninjasurge (Kaehlen Rog)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/actual#104149