Admin API for creating new users #3415

hawkowl · 2018-06-20T13:46:47Z

This has an accompanying sytest change: matrix-org/sytest#453

richvdh

Hum. Given this is making an API which is incompatible to both the existing APIs, I think we should take the opportunity to move this to a completely separate endpoint.

It fits badly under /r0/register, since (a) it skips user-interactive auth, and (b) it's synapse specific.

In short, I'm afraid I would prefer making (and documenting) yet a third version, and deprecating the existing APIs, with a view to deleting them in short order.

hawkowl · 2018-07-18T12:41:40Z

This moves the disparate and theoretically-insecure APIs from v1 and r0 into an admin path, including nonce use to prevent replay attacks.

This has an accompanying sytest change: matrix-org/sytest#453

richvdh

generally looks great.

Could we have a doc in docs/admin-api to document the new API?

richvdh · 2018-07-18T21:45:34Z

synapse/rest/client/v1/register.py

@@ -128,12 +128,10 @@ def on_POST(self, request):
            login_type = register_json["type"]

            is_application_server = login_type == LoginType.APPLICATION_SERVICE
-            is_using_shared_secret = login_type == LoginType.SHARED_SECRET


can we deprecate this for now, rather than remove it, please?

richvdh · 2018-07-18T21:45:57Z

synapse/rest/client/v2_alpha/register.py

@@ -259,16 +258,6 @@ def on_POST(self, request):
        if desired_username is not None:
            desired_username = desired_username.lower()

-        # == Shared Secret Registration == (e.g. create new user scripts)
-        if 'mac' in body:


richvdh · 2018-07-18T21:47:53Z

synapse/rest/client/v2_alpha/register.py

            guest_access_token = params.get("guest_access_token", None)

            if desired_username is not None:
                desired_username = desired_username.lower()

            (registered_user_id, _) = yield self.registration_handler.register(
                localpart=desired_username,
-                password=new_password,


I don't think this is correct.

richvdh · 2018-07-18T21:49:34Z

tests/utils.py

@@ -71,6 +71,8 @@ def setup_test_homeserver(name="test", datastore=None, config=None, reactor=None
        config.user_directory_search_all_users = False
        config.user_consent_server_notice_content = None
        config.block_events_without_consent_error = None
+        config.media_storage_providers = []


I'm a bit confused as to what's going on here?

Importing the admin APIs loads the media storage providers code, which tries to iterate over this (and you can't iterate over a mock)

sigh. ok. thanks.

richvdh · 2018-07-18T21:54:57Z

synapse/rest/client/v1/admin.py

@@ -63,6 +65,116 @@ def on_GET(self, request, user_id):
        defer.returnValue((200, ret))


+class UserRegisterServlet(ClientV1RestServlet):
+    PATTERNS = client_path_patterns("/admin/register")
+    SALT_TIMEOUT = 60


can we have a comment to explain what this does?

richvdh · 2018-07-18T21:55:54Z

synapse/rest/client/v1/admin.py

+
+        if nonce not in self.nonces:
+            raise SynapseError(
+                400, "nonce cannot be reused",


how do we know it's being reused?

Changed the comment to be a bit more accurate

richvdh · 2018-07-18T21:57:35Z

synapse/rest/client/v1/admin.py

+        super(UserRegisterServlet, self).__init__(hs)
+        self.handlers = hs.get_handlers()
+        self.reactor = hs.get_reactor()
+        self.nonces = {}


can we have a comment for this, please? what does it map to and from?

richvdh · 2018-07-18T21:59:13Z

synapse/secrets.py

+# limitations under the License.
+
+"""
+Secrets generation -- shh!


ok, but can we define what it does?

fixed

richvdh

lgtm modulo nits

richvdh · 2018-07-19T10:56:36Z

docs/admin_api/register_api.rst

+Shared-Secret Registration
+==========================
+
+This API allows for the creation of users in an administrative and non-interactive way. This is generally used for bootstrapping a Synapse instance with administrator accounts.


can we wrap at 80 chars please?

richvdh · 2018-07-19T10:57:30Z

docs/admin_api/register_api.rst

+     "device_id": "device_id_here"
+    }
+
+The MAC is the hex digest output of the HMAC-SHA1 algorithm, with the key being the shared secret and the content being the nonce, user, password, and either the string "admin" or "notadmin", each separated by NULLs. For an example of generation in Python::


richvdh · 2018-07-19T10:58:36Z

synapse/rest/client/v1/admin.py

+    """
+    Attributes:
+         NONCE_TIMEOUT (int): Seconds until a generated nonce won't be accepted
+         nonces (dict): The nonces that we will accept. A dict of nonce to the


so it's dict[str, int] ?

richvdh · 2018-07-19T11:01:00Z

tests/utils.py

@@ -71,6 +71,8 @@ def setup_test_homeserver(name="test", datastore=None, config=None, reactor=None
        config.user_directory_search_all_users = False
        config.user_consent_server_notice_content = None
        config.block_events_without_consent_error = None
+        config.media_storage_providers = []


sigh. ok. thanks.

hawkowl requested a review from a team June 22, 2018 09:06

richvdh self-assigned this Jun 22, 2018

richvdh reviewed Jun 22, 2018

View reviewed changes

richvdh assigned hawkowl and unassigned richvdh Jun 22, 2018

hawkowl changed the title ~~Update Shared Secret registration in v2 and make the create script point at it~~ [WIP] Admin API for creating new users Jul 17, 2018

hawkowl changed the title ~~[WIP] Admin API for creating new users~~ Admin API for creating new users Jul 18, 2018

hawkowl requested a review from a team July 18, 2018 12:39

richvdh previously requested changes Jul 18, 2018

View reviewed changes

make a new admin api for register

31f89bb

hawkowl force-pushed the hawkowl/register-user-improv branch from 3334d48 to 31f89bb Compare July 19, 2018 06:25

hawkowl added 3 commits July 19, 2018 16:32

make things a bit clearer

1365887

make the docstring better

0ffd1a7

registration API docs

09e8c9d

hawkowl requested a review from a team July 19, 2018 07:12

richvdh approved these changes Jul 19, 2018

View reviewed changes

nit fixes

c57aa33

hawkowl merged commit e1a237e into develop Jul 20, 2018

hawkowl deleted the hawkowl/register-user-improv branch July 20, 2018 12:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Admin API for creating new users #3415

Admin API for creating new users #3415

hawkowl commented Jun 20, 2018 •

edited

Loading

richvdh left a comment •

edited

Loading

hawkowl commented Jul 18, 2018

richvdh left a comment

richvdh Jul 18, 2018

hawkowl Jul 19, 2018

richvdh Jul 18, 2018

richvdh Jul 18, 2018

hawkowl Jul 19, 2018

richvdh Jul 18, 2018

hawkowl Jul 19, 2018

richvdh Jul 19, 2018

richvdh Jul 18, 2018

hawkowl Jul 19, 2018

richvdh Jul 18, 2018

hawkowl Jul 19, 2018

richvdh Jul 18, 2018

hawkowl Jul 19, 2018

richvdh Jul 18, 2018

richvdh left a comment

richvdh Jul 19, 2018

richvdh Jul 19, 2018

richvdh Jul 19, 2018

richvdh Jul 19, 2018

Admin API for creating new users #3415

Admin API for creating new users #3415

Conversation

hawkowl commented Jun 20, 2018 • edited Loading

richvdh left a comment • edited Loading

Choose a reason for hiding this comment

hawkowl commented Jul 18, 2018

richvdh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richvdh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hawkowl commented Jun 20, 2018 •

edited

Loading

richvdh left a comment •

edited

Loading