fix(shares): remove orphan shares

* a share without associated grant cannot exists (clen / trigger added)
    * reflect changes in documentatiion
This commit is contained in:
Edouard Vanbelle
2026-06-01 13:08:57 +02:00
parent 287365fed9
commit 657056d7aa
2 changed files with 116 additions and 7 deletions
+55 -7
View File
@@ -1,18 +1,23 @@
# Share Integration # Share Integration
OxiCloud supports public file and folder sharing through signed share links. A share can be public, password-protected, time-limited, or scoped by permissions. OxiCloud supports public file and folder sharing through signed share links. A share can be public, password-protected, or time-limited.
> **Where permission and expiration live now.** Both the granted permissions and the expiration timestamp are stored on the `storage.access_grants` row that represents the share, not on the share row itself. They are evaluated by the same `AuthorizationEngine` that handles user and group grants — see [ReBAC Authorization](/architecture/rebac-authorization). The `storage.shares` row keeps only the token-side metadata (public token, password hash, item name, access count).
## What a Share Contains ## What a Share Contains
A share record tracks: A share record (`storage.shares`) tracks:
- The shared item ID and whether it is a file or folder - The shared item ID and whether it is a file or folder
- A public token used in the share URL - A public token used in the share URL
- Optional password protection - Optional password protection (hash only — never plaintext)
- Optional expiration timestamp
- Permissions for read, write, and reshare
- The creator and access count - The creator and access count
What used to live on the share row but is now resolved through ReBAC:
- **Expiration** → `access_grants.expires_at`. The cascade query filters expired grants inline (`expires_at IS NULL OR expires_at > NOW()`), so an expired share fails the same path a revoked user grant fails. No separate "is this share expired" check.
- **Permission scope** → `access_grants.permission` rows. **For security, public share-link grants are restricted to `read` only** (the equivalent of the `viewer` role). Anyone holding the token can view but not modify, comment, share, or delete. To grant write or share access to a specific recipient, create a per-user or per-group grant instead of a share link.
## Public and Private Routes ## Public and Private Routes
### Authenticated management routes ### Authenticated management routes
@@ -61,14 +66,57 @@ Share metadata is persisted separately from the file content itself. The shared
3. If the share is password protected, the client verifies the password first 3. If the share is password protected, the client verifies the password first
4. Access is counted and the shared resource is returned according to the granted permissions 4. Access is counted and the shared resource is returned according to the granted permissions
## Lifecycle & cleanup
Because permissions and expiry now live on `access_grants`, every share is represented by two correlated rows: one in `storage.shares` (token metadata) and one or more in `storage.access_grants` (`subject_type='token'`, `subject_id=share.id`). Two triggers keep them in sync — one per direction — so neither side can outlive the other.
### Share deletion → grant cleanup
Deleting a share row (`DELETE FROM storage.shares` via `DELETE /api/shares/{id}`) fires the `trg_cleanup_grants_token` trigger declared in `migrations/20260520000000_rebac_access_grants.sql`. That trigger removes every `access_grants` row whose `subject_type='token'` and `subject_id=share.id`, in the same transaction. The token becomes unreachable immediately — no stale grants left behind.
The same pattern runs when the underlying resource is deleted: `trg_cleanup_grants_folder` / `trg_cleanup_grants_file` clean up the grants, and any share row referencing a deleted resource is then garbage-collected by the reverse trigger described below.
### Grant revocation → share row cleanup
`DELETE /api/grants/{grant_id}` on the **last** grant of a token row removes the matching `storage.shares` row, atomically and in the same transaction. The `trg_cleanup_share_on_grant_delete` trigger declared in `migrations/20260612000001_share_grant_reverse_cascade.sql` watches `access_grants` for `DELETE` events with `subject_type='token'` and deletes the paired share row **iff no other grants for the same `subject_id` still exist**:
```sql
AFTER DELETE ON storage.access_grants:
IF OLD.subject_type = 'token' THEN
DELETE FROM storage.shares
WHERE id = OLD.subject_id
AND NOT EXISTS (SELECT 1 FROM storage.access_grants
WHERE subject_type = 'token'
AND subject_id = OLD.subject_id);
```
The `NOT EXISTS` guard makes it safe in two important cases:
- **Multi-grant tokens** — if a token had several permission rows (e.g. read+share, were that ever to be allowed), revoking one leaves the share row intact. Only the final revocation triggers cleanup.
- **Forward-cascade re-entry** — when the original DELETE comes from `storage.shares`, the forward trigger is already deleting these grant rows. The reverse trigger then tries to delete a share row that's already gone, finds no row, and the statement is a no-op. No recursion.
Net effect: revoking the last grant on a token via the grants API and deleting the share via `DELETE /api/shares/{id}` are now equivalent — both end in a clean state with zero rows on either side.
### Resource deletion
Both triggers compose cleanly with resource lifecycle:
- A folder/file delete → `trg_cleanup_grants_*` removes the grants → `trg_cleanup_share_on_grant_delete` removes the share rows that just lost their last grant. One delete on the resource cleans up everything downstream in a single transaction.
### Pre-existing orphans
The `20260612000001` migration also runs a one-shot `DELETE FROM storage.shares WHERE NOT EXISTS (… token grants)` to garbage-collect any orphans that accumulated before the reverse trigger existed.
## Security Notes ## Security Notes
- Passwords are stored as hashes, never as plaintext - Passwords are stored as hashes, never as plaintext
- Expired shares are rejected before content access - Expired shares are rejected before content access by the engine's inline `expires_at` check — no separate code path
- Permissions are checked per action, not only when the share is created - Permissions are checked per action by the `AuthorizationEngine`, not just when the share is created. Revoking a grant takes effect immediately (subject to the 30 s group-expansion cache for user-grant checks; token-grant checks have no cache layer)
- Public share grants are server-side restricted to `read` regardless of what the request asked for — see the rebac-authorization doc for the role-to-permissions expansion
## Related Pages ## Related Pages
- [ReBAC Authorization](/architecture/rebac-authorization) — how grants, permissions, expiry, and cascades work
- [OIDC / SSO](/config/oidc) - [OIDC / SSO](/config/oidc)
- [Admin Settings](/config/admin-settings) - [Admin Settings](/config/admin-settings)
- [Internal Architecture](/architecture/) - [Internal Architecture](/architecture/)
@@ -0,0 +1,61 @@
-- ════════════════════════════════════════════════════════════════════════════
-- Share-link orphan cleanup: reverse cascade access_grants → storage.shares
-- ════════════════════════════════════════════════════════════════════════════
-- Today every share-link has two correlated rows:
-- 1. `storage.shares` — token + password hash + counters
-- 2. `storage.access_grants` — permission rows for subject_type='token'
--
-- The forward direction is already wired (see 20260520000000_rebac_access_grants.sql):
-- DELETE storage.shares → trg_cleanup_grants_token → access_grants gone.
--
-- The reverse direction was not. A caller hitting `DELETE /api/grants/{id}`
-- on the last grant of a token would leave the storage.shares row stranded:
-- the token still resolves to "no access" (the cascade query finds no rows),
-- but the metadata row accumulates and never garbage-collects.
--
-- This migration adds an AFTER DELETE trigger on access_grants that, when
-- the deleted row's `subject_type='token'`, deletes the storage.shares row
-- iff no other grants reference that subject_id. Per-permission revokes
-- (deleting one of several grants for the same token) are unaffected.
CREATE OR REPLACE FUNCTION storage.cleanup_share_on_last_token_grant_delete()
RETURNS trigger AS $$
BEGIN
IF OLD.subject_type = 'token' THEN
-- DELETE is a no-op when the share row is already gone — e.g. when
-- the original DELETE came from `storage.shares`, the forward
-- cascade (`trg_cleanup_grants_token`) is already deleting these
-- grant rows. The `NOT EXISTS` guard also makes the trigger safe
-- for multi-grant tokens: the share only goes away when its last
-- grant does.
DELETE FROM storage.shares s
WHERE s.id = OLD.subject_id
AND NOT EXISTS (
SELECT 1 FROM storage.access_grants ag
WHERE ag.subject_type = 'token'
AND ag.subject_id = OLD.subject_id
);
END IF;
RETURN OLD;
END;
$$ LANGUAGE plpgsql;
DROP TRIGGER IF EXISTS trg_cleanup_share_on_grant_delete ON storage.access_grants;
CREATE TRIGGER trg_cleanup_share_on_grant_delete
AFTER DELETE ON storage.access_grants
FOR EACH ROW
EXECUTE FUNCTION storage.cleanup_share_on_last_token_grant_delete();
COMMENT ON FUNCTION storage.cleanup_share_on_last_token_grant_delete() IS
'Reverse cascade: deletes storage.shares row when its last token grant is removed. Pairs with trg_cleanup_grants_token (forward direction).';
-- ── One-shot sweep of pre-existing orphans ─────────────────────────────────
-- Any share row that already has zero matching grants is dead weight. The
-- forward trigger never had a chance to fire on these (they ended up
-- grant-less via `DELETE /api/grants/{id}` calls predating this trigger).
DELETE FROM storage.shares s
WHERE NOT EXISTS (
SELECT 1 FROM storage.access_grants ag
WHERE ag.subject_type = 'token'
AND ag.subject_id = s.id
);