Skip to content

Commit 047577d

Browse files
committed
Make future permissions restore room defaults
If we change permission to write=false, then schedule a future that restores write=true, we end up (after arriving at the future) with an explicit write=true permission for the user. This seems slightly wrong because the positive permission is actually *more* permissive than what the user *probably* had which was just write permission inherited from the room. This won't matter hugely right away, but could show up later if the room default_write value gets changed (at which point this disciplined user would have write access). This changes the future to set the override to null if the future would be setting the room default as a permission. So, in the described situation, we'd set `write=null` in the override row rather than `write=true`, and so the user would end up with write permissions via the default, rather than via a specific permission grant.
1 parent c7690c6 commit 047577d

File tree

1 file changed

+19
-5
lines changed

1 file changed

+19
-5
lines changed

sogs/cleanup.py

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,17 +94,31 @@ def expire_nonce_history():
9494
def apply_permission_updates():
9595
with db.transaction():
9696
now = time.time()
97+
# This update gets a bit complicated; basically what we want to do is:
98+
# - if a future permission row is to be applied, then any `null` permission should not
99+
# change the current permission (i.e. preserve the current override value if an override
100+
# row exists, otherwise insert a null).
101+
# - for non-null permissions if the permission being applied equals the room's default then
102+
# we want to update the override value to null regardless of what it is now, because that
103+
# is almost always what was intended.
104+
# The: `CASE WHEN f.perm IS NULL THEN o.perm ELSE NULLIF(f.perm, r.perm) END`s below make
105+
# this happen: if our future is null we use the current override value (null if we have no
106+
# override), otherwise if the future and room defaults and equal we specifically set null;
107+
# otherwise we set the future value.
97108
num_perms = query(
98109
"""
99110
INSERT INTO user_permission_overrides (room, "user", read, write, upload)
100-
SELECT room, "user", read, write, upload
101-
FROM user_permission_futures
111+
SELECT f.room, f."user",
112+
CASE WHEN f.read IS NULL THEN o.read ELSE NULLIF(f.read, r.read) END,
113+
CASE WHEN f.write IS NULL THEN o.write ELSE NULLIF(f.write, r.write) END,
114+
CASE WHEN f.upload IS NULL THEN o.upload ELSE NULLIF(f.upload, r.upload) END
115+
FROM user_permission_futures f
116+
JOIN rooms r ON f.room = r.id
117+
LEFT JOIN user_permission_overrides o ON f.room = o.room AND f."user" = o."user"
102118
WHERE at <= :now
103119
ORDER BY at
104120
ON CONFLICT (room, "user") DO UPDATE SET
105-
read = COALESCE(excluded.read, user_permission_overrides.read),
106-
write = COALESCE(excluded.write, user_permission_overrides.write),
107-
upload = COALESCE(excluded.upload, user_permission_overrides.upload)
121+
read = excluded.read, write = excluded.write, upload = excluded.upload
108122
""",
109123
now=now,
110124
).rowcount

0 commit comments

Comments
 (0)