Skip to content

Commit 9f79c49

Browse files
jagermanjagerman
committed
/user/{sid}/ban and .../unban endpoints
Allows adding bans, ban timeouts, and unbanning at both the room and server level.
1 parent 0aa366a commit 9f79c49

File tree

6 files changed

+326
-95
lines changed

6 files changed

+326
-95
lines changed

‎api.yaml‎

Lines changed: 87 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -692,9 +692,14 @@ paths:
692692
/user/{sessionId}/ban:
693693
post:
694694
tags: [Users]
695-
summary: Bans or unbans a user.
695+
summary: Bans a user.
696696
description: >
697-
Applies or removes a ban of a user from specific rooms, or from the server globally.
697+
Applies a ban of a user from specific rooms, or from the server globally.
698+
699+
700+
The invoking user must have moderator (or admin) permission in all given rooms when
701+
specifying `rooms`, and must be a global server moderator (or admin) if using the `global`
702+
parameter.
698703
699704
700705
Note that the given session ID does not have to exist: it is possible to preemptively ban
@@ -707,7 +712,7 @@ paths:
707712
parameters:
708713
- $ref: "#/components/parameters/pathSessionId"
709714
requestBody:
710-
description: Details of the ban to apply. To unban a user, specify a negative timeout.
715+
description: Details of the ban to apply.
711716
required: true
712717
content:
713718
application/json:
@@ -724,6 +729,12 @@ paths:
724729
be a moderator (or admin) of all of the given rooms.
725730
726731
732+
You can specify a single element list `["*"]` to ban the user from all rooms on
733+
the server to which the calling user has moderator permissions. This differs
734+
from a global ban in that it doesn't apply to non-moderator-permission rooms,
735+
nor does it apply to newly created rooms or non-room endpoints.
736+
737+
727738
Exclusive of `global`.
728739
global:
729740
type: boolean
@@ -732,6 +743,9 @@ paths:
732743
user must be a server-level moderator or admin.
733744
734745
746+
The ban applies immediately to all server requests as early as possible.
747+
748+
735749
Exclusive of `rooms`.
736750
timeout:
737751
type: number
@@ -740,14 +754,12 @@ paths:
740754
example: 86400
741755
description: >
742756
How long the ban should apply, in seconds. If there is an existing ban on the
743-
user in the given rooms or globally this updates the existing expiry to the
744-
given value. If omitted or `null` the ban does not expire.
757+
user in the given rooms this updates the existing expiry to the given value. If
758+
omitted or `null` then the ban does not expire.
759+
745760
746-
If this value is set to a negative value (`-1` is suggested) then any existing
747-
bans for this user are *removed* from the given rooms/server. Note, however,
748-
that server bans and room bans are independent: removing a server-level ban does
749-
not remove room-specific bans, and removing a room-level ban will not grant room
750-
access to a user who also has a server-level ban.
761+
May only be used with the `rooms` argument; timeouts are not supported on global
762+
bans.
751763
examples:
752764
tworooms:
753765
summary: "1-day ban from two rooms"
@@ -758,21 +770,77 @@ paths:
758770
summary: "Permanent server ban"
759771
value:
760772
global: true
761-
timeout: null
762-
delete_all: true
763-
unban:
764-
summary: "Unban a user from a room"
765-
value:
766-
rooms: ["lokinet"]
767-
global: false,
768-
timeout: -1
769773
responses:
770774
200:
771775
description: Ban applied successfully.
772776
content: {}
773777
403:
774778
description: >
775-
Permission denied. The user attempting to set the ban does not have moderator
779+
Permission denied. The user attempting to set the ban does not have the required
780+
moderator permissions for one or more of the given rooms (or server moderator permission
781+
for a global ban).
782+
content: {}
783+
/user/{sessionId}/unban:
784+
post:
785+
tags: [Users]
786+
summary: Removes a user ban.
787+
description: >
788+
Removes a room-specific or global ban of a user.
789+
790+
791+
Note that removing a room-specific ban does not affect an existing global ban, and removing
792+
a global ban does not affect existing room-specific bans.
793+
parameters:
794+
- $ref: "#/components/parameters/pathSessionId"
795+
requestBody:
796+
description: Details of the ban to remove.
797+
required: true
798+
content:
799+
application/json:
800+
schema:
801+
type: object
802+
properties:
803+
rooms:
804+
type: array
805+
items:
806+
$ref: "#/components/schemas/RoomToken"
807+
minItems: 1
808+
description: >
809+
List of room tokens from which the ban should be removed (if present). The
810+
invoking user must be a moderator (or admin) of all of the given rooms.
811+
812+
813+
You can specify a single element list `["*"]` to unban the user from all rooms
814+
on the server to which the calling user has moderator permissions. This isn't
815+
the same as removing a global ban; this just removes any room-specific bans that
816+
may currently apply from moderated rooms.
817+
818+
819+
Exclusive of `global`.
820+
global:
821+
type: boolean
822+
description: >
823+
If true then remove a global server ban of this user. The invoking user must be
824+
a server-level moderator or admin.
825+
826+
827+
Exclusive of `rooms`.
828+
examples:
829+
tworooms:
830+
summary: "Remove ban from two rooms"
831+
value:
832+
rooms: ["session", "lokinet"]
833+
permaban:
834+
summary: "Remove server ban"
835+
value:
836+
global: true
837+
responses:
838+
200:
839+
description: Ban removed successfully.
840+
content: {}
841+
403:
842+
description: >
843+
Permission denied. The user attempting to remove the ban does not have moderator
776844
permissions for one or more of the given rooms (or server moderator permission for a
777845
global ban).
778846
content: {}

‎sogs/routes/users.py‎

Lines changed: 119 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,73 @@
1212
users = Blueprint('users', __name__)
1313

1414

15+
def extract_rooms_or_global(req, admin=True):
16+
"""
17+
Extracts the rooms / global parameters from the request body checking them for validity and
18+
expanding them as appropriate.
19+
20+
Throws a flask abort on failure, returns (rooms, global) which will be either ([list of Rooms],
21+
None) for a room operation or (None, True) for a global operation.
22+
23+
admin specifies whether we require admin permission (if True) or just moderator permission,
24+
either in all rooms specified, or globally. (Similarly it affects what `rooms=['*']` expands
25+
to).
26+
"""
27+
28+
room_tokens, global_ = req.get('rooms'), req.get('global', False)
29+
30+
if room_tokens and not isinstance(room_tokens, list):
31+
app.logger.warning("Invalid request: rooms must be a list")
32+
abort(http.BAD_REQUEST)
33+
34+
if room_tokens and global_:
35+
app.logger.warning("Invalid moderator request: cannot specify both 'rooms' and 'global'")
36+
abort(http.BAD_REQUEST)
37+
38+
if not room_tokens and not global_:
39+
app.logger.warning("Invalid moderator request: neither 'rooms' nor 'global' specified")
40+
abort(http.BAD_REQUEST)
41+
42+
if room_tokens:
43+
if len(room_tokens) > 1 and '*' in room_tokens:
44+
app.logger.warning("Invalid moderator request: room '*' must be the only rooms value")
45+
abort(http.BAD_REQUEST)
46+
47+
if room_tokens == ['*']:
48+
room_tokens = None
49+
50+
try:
51+
rooms = mroom.get_rooms_with_permission(
52+
g.user, tokens=room_tokens, moderator=True, admin=admin
53+
)
54+
except Exception as e:
55+
# This is almost certainly a bad room token passed in:
56+
app.logger.warning(f"Cannot get rooms for adding a moderator: {e}")
57+
abort(http.NOT_FOUND)
58+
59+
if room_tokens:
60+
if len(rooms) != len(room_tokens):
61+
abort(http.FORBIDDEN)
62+
elif not rooms:
63+
abort(http.FORBIDDEN)
64+
65+
return (rooms, None)
66+
67+
if not g.user.global_moderator or (admin and not g.user.global_admin):
68+
abort(http.FORBIDDEN)
69+
70+
return (None, True)
71+
72+
1573
@users.post("/user/<SessionID:sid>/moderator")
1674
@auth.user_required
1775
def set_mod(sid):
1876

1977
user = User(session_id=sid)
2078

2179
req = request.json
22-
room_tokens, global_mod = req.get('rooms'), req.get('global', False)
2380

24-
if room_tokens and not isinstance(room_tokens, list):
25-
app.logger.warning("Invalid request: room_tokens must be a list")
26-
abort(http.BAD_REQUEST)
81+
rooms, global_mod = extract_rooms_or_global(req)
2782

2883
mod, admin, visible = (
2984
None if arg is None else bool(arg)
@@ -51,39 +106,11 @@ def set_mod(sid):
51106
# (False, True) -- removes admin, adds mod
52107
# (False, None) -- removes admin
53108

54-
with db.transaction():
55-
if room_tokens:
56-
if visible is None:
57-
visible = True
58-
59-
if global_mod:
60-
app.logger.warning(
61-
"Invalid moderator request: cannot specify both 'rooms' and 'global'"
62-
)
63-
abort(http.BAD_REQUEST)
64-
65-
if len(room_tokens) > 1 and '*' in room_tokens:
66-
app.logger.warning(
67-
"Invalid moderator request: room '*' must be the only rooms value"
68-
)
69-
abort(http.BAD_REQUEST)
70-
71-
if room_tokens == ['*']:
72-
room_tokens = None
73-
74-
try:
75-
rooms = mroom.get_rooms_with_permission(g.user, tokens=room_tokens, admin=True)
76-
except Exception as e:
77-
# This is almost certainly a bad room token passed in:
78-
app.logger.warning(f"Cannot get rooms for adding a moderator: {e}")
79-
abort(http.BAD_REQUEST)
80-
81-
if room_tokens is not None:
82-
if len(rooms) != len(room_tokens):
83-
abort(http.FORBIDDEN)
84-
elif not rooms:
85-
abort(http.FORBIDDEN)
109+
if rooms:
110+
if visible is None:
111+
visible = True
86112

113+
with db.transaction():
87114
for room in rooms:
88115
if (admin, mod) in ((True, None), (None, True)):
89116
room.set_moderator(user, added_by=g.user, admin=admin, visible=visible)
@@ -98,18 +125,63 @@ def set_mod(sid):
98125
app.logger.error("Internal error: unhandled mod/admin room case")
99126
raise RuntimeError("Internal error: unhandled mod/admin room case")
100127

101-
else: # global mod
102-
if visible is None:
103-
visible = False
104-
105-
if (admin, mod) in ((True, None), (None, True)):
106-
user.set_moderator(added_by=g.user, admin=admin, visible=visible)
107-
elif (admin, mod) == (None, False):
108-
user.remove_moderator(removed_by=g.user)
109-
elif (admin, mod) == (False, None):
110-
user.remove_moderator(removed_by=g.user, remove_admin_only=True)
111-
elif (admin, mod) == (False, True):
128+
else: # global mod
129+
if visible is None:
130+
visible = False
131+
132+
if (admin, mod) in ((True, None), (None, True)):
133+
user.set_moderator(added_by=g.user, admin=admin, visible=visible)
134+
elif (admin, mod) == (None, False):
135+
user.remove_moderator(removed_by=g.user)
136+
elif (admin, mod) == (False, None):
137+
user.remove_moderator(removed_by=g.user, remove_admin_only=True)
138+
elif (admin, mod) == (False, True):
139+
with db.transaction():
112140
user.remove_moderator(removed_by=g.user, remove_admin_only=True)
113141
user.set_moderator(added_by=g.user, admin=bool(admin), visible=visible)
114142

115143
return jsonify({})
144+
145+
146+
@users.post("/user/<SessionID:sid>/ban")
147+
@auth.user_required
148+
def ban_user(sid):
149+
150+
user = User(session_id=sid)
151+
req = request.json
152+
rooms, global_ban = extract_rooms_or_global(req, admin=False)
153+
154+
timeout = req.get('timeout')
155+
if timeout is not None and not isinstance(timeout, int) and not isinstance(timeout, float):
156+
app.logger.warning("Invalid ban request: timeout must be numeric")
157+
abort(http.BAD_REQUEST)
158+
159+
if timeout and global_ban:
160+
app.logger.warning("Invalid ban request: global server bans do not support timeouts")
161+
abort(http.BAD_REQUEST)
162+
163+
if rooms:
164+
with db.transaction():
165+
for room in rooms:
166+
room.ban_user(to_ban=user, mod=g.user, timeout=timeout)
167+
else:
168+
user.ban(banned_by=g.user)
169+
170+
return {}
171+
172+
173+
@users.post("/user/<SessionID:sid>/unban")
174+
@auth.user_required
175+
def unban_user(sid):
176+
177+
user = User(session_id=sid)
178+
rooms, global_ban = extract_rooms_or_global(request.json, admin=False)
179+
180+
if rooms:
181+
with db.transaction():
182+
for room in rooms:
183+
room.unban_user(to_unban=user, mod=g.user)
184+
else:
185+
user.unban(unbanned_by=g.user)
186+
187+
return {}

‎tests/conftest.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,16 @@ def room(db):
7575
return Room.create('test-room', name='Test room', description='Test suite testing room')
7676

7777

78+
@pytest.fixture
79+
def room2(db):
80+
"""
81+
Creates a test room, typically used with `room` when two separate rooms are needed. Note that
82+
`mod` and `admin` (if used) are only a mod and admin of `room`, not `room2`
83+
"""
84+
85+
return Room.create('room2', name='Room 2', description='Test suite testing room2')
86+
87+
7888
@pytest.fixture
7989
def user(db):
8090
"""

‎tests/test_room_routes.py‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,8 @@
99
from request import sogs_get, sogs_post, sogs_put
1010

1111

12-
def test_list(client, room, user, user2, admin, mod, global_mod, global_admin):
12+
def test_list(client, room, room2, user, user2, admin, mod, global_mod, global_admin):
1313

14-
room2 = Room.create('room2', name='Room 2', description='Test suite testing room2')
1514
room2.default_write = False
1615
room2.default_upload = False
1716

0 commit comments

Comments
 (0)