-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add is_public to groups table to allow for private groups #2582
Conversation
Prevent group API access to non-members for private groups Also make all the group code paths consistent with `requester_user_id` always being the User ID of the requesting user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PEP8 is sad
synapse/groups/groups_server.py:52:91: E501 line too long (100 > 90 characters)
synapse/groups/groups_server.py:160:91: E501 line too long (100 > 90 characters)
synapse/groups/groups_server.py:163:91: E501 line too long (116 > 90 characters)
synapse/groups/groups_server.py:182:91: E501 line too long (91 > 90 characters)
synapse/groups/groups_server.py:185:91: E501 line too long (116 > 90 characters)
synapse/groups/groups_server.py:223:91: E501 line too long (116 > 90 characters)
synapse/groups/groups_server.py:241:91: E501 line too long (116 > 90 characters)
synapse/groups/groups_server.py:277:91: E501 line too long (116 > 90 characters)
synapse/groups/groups_server.py:296:91: E501 line too long (116 > 90 characters)
synapse/groups/groups_server.py:626:91: E501 line too long (93 > 90 characters)
synapse/groups/groups_server.py:750:91: E501 line too long (97 > 90 characters)
synapse/rest/client/v2_alpha/groups.py:44:91: E501 line too long (100 > 90 characters)
synapse/rest/client/v2_alpha/groups.py:77:91: E501 line too long (100 > 90 characters)
synapse/rest/client/v2_alpha/groups.py:390:91: E501 line too long (98 > 90 characters)
synapse/rest/client/v2_alpha/groups.py:417:91: E501 line too long (93 > 90 characters)
synapse/groups/groups_server.py
Outdated
@@ -67,6 +67,10 @@ def check_group_is_ours(self, group_id, and_exists=False, and_is_admin=None): | |||
if and_exists and not group: | |||
raise SynapseError(404, "Unknown group") | |||
|
|||
is_user_in_group = yield self.store.is_user_in_group(requester_user_id, group_id) | |||
if is_user_in_group or not group.is_public: | |||
raise SynapseError(404, "Unknown group") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably only do this if the group exists, in particular this currently doesn't work well with create_group. (Speaking of which, you forgot to add requester_user_id to create_group, which is why its failing)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't that also be if not is_user_in_group...
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes o\
*/ | ||
|
||
-- whether non-members can access group APIs | ||
ALTER TABLE groups ADD COLUMN is_public BOOL DEFAULT 1 NOT NULL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll need to change the schema version in synapse.storage.prepare_database
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll need to add this to the scripts/synapse_port_db list of tables with boolean columns
2ca5d07
to
ee730ee
Compare
Adding a column with non-constant default not possible in sqlite3
ee730ee
to
69e8a05
Compare
(Ideally we should also add a sytest for this) |
Prevent group API access to non-members for private groups
Also make all the group code paths consistent with
requester_user_id
always being the User ID of the requesting user.