-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Conversation
…eate_support_user
…eate_support_user
…eate_support_user
…apse into neilj/create_support_user
…eate_support_user
…eate_support_user
What will the support user be for? |
@rubo77 it's an off by default feature to aid trouble shooting. |
what is "an off"? (sorry, I am not native english) So what is the user doing if activated? what is this change for? is it connected with another PR? |
@rubo77 This feature is useful if you are supporting someone's homeserver and want to log in as a user to trouble shoot without polluting the user directory or monthly active user counts. The specific driver is for Modular though this could be used in any context where one party is providing support to another. by 'an off by default' - I mean that it is not expected that most synapse admins will want this behaviour and by default it will not be enabled. |
This seems a bit dangerous: as an admin You could very comfortable spy on others with this feature. What about encrypted rooms? Is it impossible to join those as support user? |
To be clear, the user is still visible in any room it joins - so in that regard it is no different from a regular user signing up and joining rooms - I don't see an opportunity for spying. The difference is that the support user does not appear in search results and does not contribute to monthly active user limits. |
…sary filters on removing support users from user directory
…eate_support_user
synapse/handlers/register.py
Outdated
# the room is never created, though this seems unlikely and | ||
# recoverable from given the support user being involved in the first | ||
# place. | ||
if (self.hs.config.autocreate_auto_join_rooms and not is_support): |
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.
nit: parens are redundant here
synapse/storage/registration.py
Outdated
@@ -184,6 +185,9 @@ def register(self, user_id, token=None, password_hash=None, | |||
appservice_id (str): The ID of the appservice registering the user. | |||
create_profile_with_displayname (unicode): Optionally create a profile for | |||
the user, setting their displayname to the given value | |||
admin (boolean): is an admin user? | |||
user_type (synapse.api.constants import UserTypes): type of 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.
user_type (synapse.api.constants import UserTypes): type of user | |
user_type (synapse.api.constants.UserTypes|None): type of user |
synapse/storage/registration.py
Outdated
@@ -247,6 +253,7 @@ def _register( | |||
"is_guest": 1 if make_guest else 0, | |||
"appservice_id": appservice_id, | |||
"admin": 1 if admin else 0, | |||
"user_type": user_type |
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.
trailing comma would be good here as well
* limitations under the License. | ||
*/ | ||
|
||
/* Record the user_type of the user, initially this column is used to identify |
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.
This is kinda stating the obvious and will be unhelpful if we ever add more user types. How about:
The type of the user: NULL for a regular user, or one of the constants in synapse.api.constants.UserTypes
tests/handlers/test_register.py
Outdated
@@ -43,10 +43,6 @@ def setUp(self): | |||
self.addCleanup, | |||
expire_access_token=True, | |||
) | |||
self.macaroon_generator = Mock( |
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.
it seems to be a way of checking the access token is correctly passed through by get_or_create_user
. So let me rephrase: is it necessary to remove these and weaken the result _token
tests below?
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.
nearly there!
@@ -65,6 +66,8 @@ def request_registration( | |||
mac.update(password.encode('utf8')) | |||
mac.update(b"\x00") | |||
mac.update(b"admin" if admin else b"notadmin") | |||
mac.update(b"\x00") |
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.
as per my comments in #synapse-dev, it would be nice to preserve backwards-compatibility for users of this API who just want to make a regular user.
Co-Authored-By: neilisfragile <neil@matrix.org>
Co-Authored-By: neilisfragile <neil@matrix.org>
Co-Authored-By: neilisfragile <neil@matrix.org>
docs/admin_api/register_api.rst
Outdated
@@ -60,6 +60,7 @@ each separated by NULs. For an example of generation in Python:: | |||
mac.update(b"\x00") | |||
mac.update(b"admin" if admin else b"notadmin") | |||
mac.update(b"\x00") |
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.
this needs to be conditional :/
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.
lgtm
Synapse 0.34.1rc1 (2019-01-08) ============================== Features -------- - Special-case a support user for use in verifying behaviour of a given server. The support user does not appear in user directory or monthly active user counts. ([\#4141](#4141), [\#4344](#4344)) - Support for serving .well-known files ([\#4262](#4262)) - Rework SAML2 authentication ([\#4265](#4265), [\#4267](#4267)) - SAML2 authentication: Initialise user display name from SAML2 data ([\#4272](#4272)) - Synapse can now have its conditional/extra dependencies installed by pip. This functionality can be used by using `pip install matrix-synapse[feature]`, where feature is a comma separated list with the possible values `email.enable_notifs`, `matrix-synapse-ldap3`, `postgres`, `resources.consent`, `saml2`, `url_preview`, and `test`. If you want to install all optional dependencies, you can use "all" instead. ([\#4298](#4298), [\#4325](#4325), [\#4327](#4327)) - Add routes for reading account data. ([\#4303](#4303)) - Add opt-in support for v2 rooms ([\#4307](#4307)) - Add a script to generate a clean config file ([\#4315](#4315)) - Return server data in /login response ([\#4319](#4319)) Bugfixes -------- - Fix contains_url check to be consistent with other instances in code-base and check that value is an instance of string. ([\#3405](#3405)) - Fix CAS login when username is not valid in an MXID ([\#4264](#4264)) - Send CORS headers for /media/config ([\#4279](#4279)) - Add 'sandbox' to CSP for media reprository ([\#4284](#4284)) - Make the new landing page prettier. ([\#4294](#4294)) - Fix deleting E2E room keys when using old SQLite versions. ([\#4295](#4295)) - The metric synapse_admin_mau:current previously did not update when config.mau_stats_only was set to True ([\#4305](#4305)) - Fixed per-room account data filters ([\#4309](#4309)) - Fix indentation in default config ([\#4313](#4313)) - Fix synapse:latest docker upload ([\#4316](#4316)) - Fix test_metric.py compatibility with prometheus_client 0.5. Contributed by Maarten de Vries <maarten@de-vri.es>. ([\#4317](#4317)) - Avoid packaging _trial_temp directory in -py3 debian packages ([\#4326](#4326)) - Check jinja version for consent resource ([\#4327](#4327)) - fix NPE in /messages by checking if all events were filtered out ([\#4330](#4330)) - Fix `python -m synapse.config` on Python 3. ([\#4356](#4356)) Deprecations and Removals ------------------------- - Remove the deprecated v1/register API on Python 2. It was never ported to Python 3. ([\#4334](#4334)) Internal Changes ---------------- - Getting URL previews of IP addresses no longer fails on Python 3. ([\#4215](#4215)) - drop undocumented dependency on dateutil ([\#4266](#4266)) - Update the example systemd config to use a virtualenv ([\#4273](#4273)) - Update link to kernel DCO guide ([\#4274](#4274)) - Make isort tox check print diff when it fails ([\#4283](#4283)) - Log room_id in Unknown room errors ([\#4297](#4297)) - Documentation improvements for coturn setup. Contributed by Krithin Sitaram. ([\#4333](#4333)) - Update pull request template to use absolute links ([\#4341](#4341)) - Update README to not lie about required restart when updating TLS certificates ([\#4343](#4343)) - Update debian packaging for compatibility with transitional package ([\#4349](#4349)) - Fix command hint to generate a config file when trying to start without a config file ([\#4353](#4353)) - Add better logging for unexpected errors while sending transactions ([\#4358](#4358))
When guest_access changes from allowed to forbidden all local guest users should be kicked from the room. This did not happen when revocation was received from federation on a worker. Presumably broken in #4141
Creates a support user and excludes it from mau counting and the user dir.
I'm not very happy with the user dir exclusions since it means finding 6 places where the index is added to/updated and filtering, thoughts on better impls much appreciated