Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions radicale/radicale_custom_rights.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@ def authorization(self, user, path):
segments = sane_path.split("/")
owner_id = segments[0]

# 1. Owner always gets full permissions
# 1. Owner always gets full permissions. "D"/"d" are deny-delete bits
# in Radicale, so the owner must never receive them.
if user.lower() == owner_id.lower():
if len(segments) == 1:
return "RW"
return "RWDrwd"
return "rw"

# 2. Accessing someone else's space
if len(segments) < 2:
Expand Down Expand Up @@ -61,9 +62,9 @@ def authorization(self, user, path):
has_write = bool(rw_flag) or role in ('WRITE', 'ADMIN')
if role in ('READ', 'FREE_BUSY'):
has_write = False
if len(segments) > 2:
return "rwd" if has_write else "rd"
return "rw" if has_write else "r"
# "d" denies deleting the collection itself (Radicale checks it
# against the collection path), so only the owner can delete it.
return "rwd" if has_write else "rd"

return ""
except Exception:
Expand Down
40 changes: 20 additions & 20 deletions server/test/issue192_sharee_deny_delete_permission.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,13 @@

const test = require('node:test');
const assert = require('node:assert/strict');
const fs = require('node:fs');
const path = require('node:path');
const { spawnSync } = require('node:child_process');

const pyRightsPath = path.join(__dirname, '../../radicale/radicale_custom_rights.py');

test('Issue #192 — Read-Write Sharee Deny-Delete Permission Enforcement', async (t) => {
await t.test('1. Static source analysis: radicale_custom_rights.py emits "rw" for collection and "rwd" for item sharee access', () => {
const code = fs.readFileSync(pyRightsPath, 'utf8');
assert.match(
code,
/return\s+["']rwd["']\s+if\s+has_write\s+else\s+["']rd["']/,
'radicale_custom_rights.py must emit item level "rwd" for write sharees and "rd" for read sharees'
);
});

await t.test('2. Python unit execution: Rights.authorization returns correct collection vs item permissions', () => {
await t.test('Rights.authorization: sharees carry deny-delete "d" on the collection, the owner never gets "D"/"d"', () => {
const pythonScript = `
import sys
import os
Expand Down Expand Up @@ -77,22 +67,32 @@ try:

rights = radicale_custom_rights.Rights()

# Owner access check (collection level)
# Radicale 3.4 collection DELETE (app/delete.py, permit_delete_collection=True)
# needs "w" on the collection path and is refused if "d" is on that path.
def collection_delete_allowed(perm):
return 'w' in perm and 'd' not in perm

# Owner: no deny bits, may delete her own calendar
owner_perm = rights.authorization('owner1', 'owner1/cal1')
assert owner_perm == 'RWDrwd', f"Owner expected 'RWDrwd', got {owner_perm!r}"
assert owner_perm == 'rw', f"Owner expected 'rw', got {owner_perm!r}"
assert collection_delete_allowed(owner_perm), f"Owner must be able to delete own calendar, got {owner_perm!r}"
owner_item_perm = rights.authorization('owner1', 'owner1/cal1/item.ics')
assert 'd' not in owner_item_perm and 'D' not in owner_item_perm, f"Owner item expected no deny bits, got {owner_item_perm!r}"

# Read-write sharee access check (collection level must NOT include 'd' or 'D')
# Read-write sharee: may write, must NOT delete the owner's collection
rw_sharee_col_perm = rights.authorization('sharee_write', 'owner1/cal1')
assert 'd' not in rw_sharee_col_perm and 'D' not in rw_sharee_col_perm, f"Read-write sharee collection expected 'rw', got {rw_sharee_col_perm!r}"
assert rw_sharee_col_perm == 'rw', f"Read-write sharee collection expected 'rw', got {rw_sharee_col_perm!r}"

# Read-write sharee access check (item level must include 'd')
assert rw_sharee_col_perm == 'rwd', f"Read-write sharee collection expected 'rwd', got {rw_sharee_col_perm!r}"
assert not collection_delete_allowed(rw_sharee_col_perm), f"Read-write sharee could delete owner's calendar: {rw_sharee_col_perm!r}"
rw_sharee_item_perm = rights.authorization('sharee_write', 'owner1/cal1/item.ics')
assert rw_sharee_item_perm == 'rwd', f"Read-write sharee item expected 'rwd', got {rw_sharee_item_perm!r}"

# Read-only sharee access check
# Read-only sharee
ro_sharee_col_perm = rights.authorization('sharee_read', 'owner1/cal1')
assert ro_sharee_col_perm == 'r', f"Read-only sharee collection expected 'r', got {ro_sharee_col_perm!r}"
assert ro_sharee_col_perm == 'rd', f"Read-only sharee collection expected 'rd', got {ro_sharee_col_perm!r}"
assert not collection_delete_allowed(ro_sharee_col_perm)

# Stranger
assert rights.authorization('stranger', 'owner1/cal1') == ''

print("ALL_PYTHON_RIGHTS_TESTS_PASSED")
finally:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ test('Issue #201 — Prevent Unauthenticated & Sharee Permanent Calendar Deletio
}
});

await t.test('1. Python unit execution: Rights.authorization returns "RWDrwd" for owner and "rw"/"r" for sharees on collection level', () => {
await t.test('1. Python unit execution: Rights.authorization returns "rw" for owner and deny-delete "rwd"/"rd" for sharees on collection level', () => {
const pythonScript = `
import sys
import os
Expand Down Expand Up @@ -117,24 +117,23 @@ try:

rights = radicale_custom_rights.Rights()

# Owner access check at collection level (must have 'D' and 'd' to delete collection/items)
# In Radicale "D"/"d" are deny-delete bits (checked against the collection
# path on a collection DELETE). The owner must not carry them; sharees must.
owner_perm = rights.authorization('owner1', 'owner1/cal1')
assert 'D' in owner_perm and 'd' in owner_perm, f"Owner expected collection delete permission 'D' and 'd', got {owner_perm!r}"
assert owner_perm == 'RWDrwd', f"Owner expected 'RWDrwd', got {owner_perm!r}"
assert 'D' not in owner_perm and 'd' not in owner_perm, f"Owner must not carry deny-delete bits, got {owner_perm!r}"
assert owner_perm == 'rw', f"Owner expected 'rw', got {owner_perm!r}"

# Write sharee access check at collection level (must NOT contain 'd' or 'D' at collection level)
# Write sharee at collection level must carry 'd' so it cannot delete the collection
rw_sharee_col_perm = rights.authorization('sharee_write', 'owner1/cal1')
assert 'd' not in rw_sharee_col_perm and 'D' not in rw_sharee_col_perm, f"Write sharee collection level must NOT contain 'd' or 'D', got {rw_sharee_col_perm!r}"
assert rw_sharee_col_perm == 'rw', f"Write sharee collection level expected 'rw', got {rw_sharee_col_perm!r}"
assert rw_sharee_col_perm == 'rwd', f"Write sharee collection level expected 'rwd', got {rw_sharee_col_perm!r}"

# Write sharee access check at item level (must contain 'd' to delete events inside collection)
# Write sharee at item level keeps write access to events
rw_sharee_item_perm = rights.authorization('sharee_write', 'owner1/cal1/event1.ics')
assert 'd' in rw_sharee_item_perm, f"Write sharee item level expected 'd', got {rw_sharee_item_perm!r}"
assert rw_sharee_item_perm == 'rwd', f"Write sharee item level expected 'rwd', got {rw_sharee_item_perm!r}"

# Read sharee access check at collection level
# Read sharee at collection level
ro_sharee_col_perm = rights.authorization('sharee_read', 'owner1/cal1')
assert ro_sharee_col_perm == 'r', f"Read sharee collection level expected 'r', got {ro_sharee_col_perm!r}"
assert ro_sharee_col_perm == 'rd', f"Read sharee collection level expected 'rd', got {ro_sharee_col_perm!r}"

print("ALL_ISSUE201_PYTHON_RIGHTS_TESTS_PASSED")
finally:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,13 @@ try:
rights = radicale_custom_rights.Rights({})

# 1. Sharee uppercase
assert rights.authorization("USER1", "user2/shared-read") == "r", "USER1 should get read permission"
assert rights.authorization("USER1", "user2/shared-read") == "rd", "USER1 should get read permission"

# 2. Owner uppercase in path
assert rights.authorization("user1", "USER2/shared-read") == "r", "user1 should get read permission on USER2 path"
assert rights.authorization("user1", "USER2/shared-read") == "rd", "user1 should get read permission on USER2 path"

# 3. Mixed case both
assert rights.authorization("User1", "User2/shared-write") == "rw", "User1 should get read-write permission on User2 path"
assert rights.authorization("User1", "User2/shared-write") == "rwd", "User1 should get read-write permission on User2 path"

print("OK")
finally:
Expand Down
31 changes: 24 additions & 7 deletions server/test/python/test_radicale_custom_rights.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ def strip_path(path):

import radicale_custom_rights


def collection_delete_allowed(permissions):
"""Radicale 3.4 DELETE on a calendar collection (radicale/app/delete.py,
permit_delete_collection=True): requires "w" on the collection path and
is refused when the deny-delete bit "d" is present on that same path."""
return "w" in permissions and "d" not in permissions

class TestRadicaleCustomRights(unittest.TestCase):
def setUp(self):
self.temp_dir = tempfile.mkdtemp()
Expand Down Expand Up @@ -107,28 +114,38 @@ def test_owner_principal_root(self):
self.assertEqual(perm_case, "RW")

def test_owner_calendar_subpath(self):
# Radicale's "D"/"d" are deny-delete bits: the owner must keep the
# ability to delete her own calendar collection and its items.
perm = self.rights.authorization("user1", "user1/my-cal")
self.assertEqual(perm, "RWDrwd")
self.assertEqual(perm, "rw")
self.assertTrue(collection_delete_allowed(perm))
perm_item = self.rights.authorization("user1", "user1/my-cal/item.ics")
self.assertEqual(perm_item, "rw")

def test_other_user_principal_root_access_denied(self):
perm = self.rights.authorization("user1", "user2")
self.assertEqual(perm, "")

def test_sharee_read_only_access(self):
perm = self.rights.authorization("user1", "user2/shared-read")
self.assertEqual(perm, "r")
self.assertEqual(perm, "rd")
self.assertFalse(collection_delete_allowed(perm))
perm_item = self.rights.authorization("user1", "user2/shared-read/item.ics")
self.assertEqual(perm_item, "rd")

def test_sharee_write_access(self):
# Issue #192: a read-write sharee must not be able to delete the
# owner's calendar collection, so the collection path carries "d".
perm = self.rights.authorization("user1", "user2/shared-write")
self.assertEqual(perm, "rw")
self.assertEqual(perm, "rwd")
self.assertFalse(collection_delete_allowed(perm))
perm_item = self.rights.authorization("user1", "user2/shared-write/item.ics")
self.assertEqual(perm_item, "rwd")

def test_sharee_admin_access(self):
perm = self.rights.authorization("user1", "user2/shared-admin")
self.assertEqual(perm, "rw")
self.assertEqual(perm, "rwd")
self.assertFalse(collection_delete_allowed(perm))
perm_item = self.rights.authorization("user1", "user2/shared-admin/item.ics")
self.assertEqual(perm_item, "rwd")

Expand All @@ -148,15 +165,15 @@ def test_missing_db_returns_empty(self):
def test_sharee_case_insensitive_access(self):
# Case-insensitive sharee username matching
perm_upper_sharee = self.rights.authorization("USER1", "user2/shared-read")
self.assertEqual(perm_upper_sharee, "r")
self.assertEqual(perm_upper_sharee, "rd")
perm_mixed_sharee = self.rights.authorization("User1", "user2/shared-write/item.ics")
self.assertEqual(perm_mixed_sharee, "rwd")

# Case-insensitive calendar owner matching in path
perm_upper_owner = self.rights.authorization("user1", "USER2/shared-read")
self.assertEqual(perm_upper_owner, "r")
self.assertEqual(perm_upper_owner, "rd")
perm_mixed_both = self.rights.authorization("USER1", "User2/shared-write")
self.assertEqual(perm_mixed_both, "rw")
self.assertEqual(perm_mixed_both, "rwd")

if __name__ == "__main__":
unittest.main()
Loading