Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Require AppserviceRegistrationType #9548

Merged
merged 17 commits into from
Apr 12, 2021
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/9548.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/register now requires a `body.type` value of `m.login.appservice` when registering appservice users.
Half-Shot marked this conversation as resolved.
Show resolved Hide resolved
5 changes: 5 additions & 0 deletions synapse/api/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ class LoginType:
DUMMY = "m.login.dummy"


# This is not a login type as we cannot login with it, however
# the spec says that this is a valid registration type for appservices.
Half-Shot marked this conversation as resolved.
Show resolved Hide resolved
AppserviceRegistrationType = "m.login.application_service"
clokep marked this conversation as resolved.
Show resolved Hide resolved


class EventTypes:
Member = "m.room.member"
Create = "m.room.create"
Expand Down
23 changes: 16 additions & 7 deletions synapse/rest/client/v2_alpha/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

import hmac
import logging
import random
Expand All @@ -22,7 +21,7 @@
import synapse
import synapse.api.auth
import synapse.types
from synapse.api.constants import LoginType
from synapse.api.constants import AppserviceRegistrationType, LoginType
from synapse.api.errors import (
Codes,
InteractiveAuthIncompleteError,
Expand Down Expand Up @@ -428,15 +427,20 @@ async def on_POST(self, request):
raise SynapseError(400, "Invalid username")
desired_username = body["username"]

appservice = None
if self.auth.has_access_token(request):
appservice = self.auth.get_appservice_by_req(request)

# fork off as soon as possible for ASes which have completely
# different registration flows to normal users

# == Application Service Registration ==
if appservice:
if body.get("type") == AppserviceRegistrationType:
if not self.auth.has_access_token(request):
Half-Shot marked this conversation as resolved.
Show resolved Hide resolved
raise SynapseError(
400,
"Appservice token must be provided when using a type of m.login.application_service",
)

# Verify the AS
self.auth.get_appservice_by_req(request)

# Set the desired user according to the AS API (which uses the
# 'user' key not 'username'). Since this is a new addition, we'll
# fallback to 'username' if they gave one.
Expand All @@ -457,6 +461,11 @@ async def on_POST(self, request):
)

return 200, result
elif self.auth.has_access_token(request):
raise SynapseError(
400,
"A type of m.login.application_service must be provided when registering as an appservice",
Half-Shot marked this conversation as resolved.
Show resolved Hide resolved
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

passing an access token does not necessarily mean that you are claiming to be an appservice?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there is a case currently where you can register with an access token without being an appservice?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason for keeping this elif around is that previously we used the access token to determine if the request was on the behalf of an appservice. Rather than throwing that away I wanted to throw an explicit error for people running non-compliant appservices (hence the very specific error message).

If we throw this statement away, the token will be ignored and requests that previously successfully created an appservice user may either fail with a somewhat unhelpful error (like "you didn't specify a password"), or may succeed in creating a non-appservice user which would probably do more harm than good.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the point is more that the error message is ... unclear.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggested a rephrasing.


# == Normal User Registration == (everyone else)
if not self._registration_enabled:
Expand Down
31 changes: 27 additions & 4 deletions tests/rest/client/v2_alpha/test_register.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,14 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

import datetime
import json
import os

import pkg_resources

import synapse.rest.admin
from synapse.api.constants import LoginType
from synapse.api.constants import AppserviceRegistrationType, LoginType
from synapse.api.errors import Codes
from synapse.appservice import ApplicationService
from synapse.rest.client.v1 import login, logout
Expand Down Expand Up @@ -59,7 +58,9 @@ def test_POST_appservice_registration_valid(self):
)

self.hs.get_datastore().services_cache.append(appservice)
request_data = json.dumps({"username": "as_user_kermit"})
request_data = json.dumps(
{"username": "as_user_kermit", "type": AppserviceRegistrationType}
)

channel = self.make_request(
b"POST", self.url + b"?access_token=i_am_an_app_service", request_data
Expand All @@ -69,9 +70,31 @@ def test_POST_appservice_registration_valid(self):
det_data = {"user_id": user_id, "home_server": self.hs.hostname}
self.assertDictContainsSubset(det_data, channel.json_body)

def test_POST_appservice_registration_no_type(self):
as_token = "i_am_an_app_service"

appservice = ApplicationService(
as_token,
self.hs.config.server_name,
id="1234",
namespaces={"users": [{"regex": r"@as_user.*", "exclusive": True}]},
sender="@as:test",
)

self.hs.get_datastore().services_cache.append(appservice)
request_data = json.dumps({"username": "as_user_kermit"})

channel = self.make_request(
b"POST", self.url + b"?access_token=i_am_an_app_service", request_data
)

self.assertEquals(channel.result["code"], b"400", channel.result)

def test_POST_appservice_registration_invalid(self):
self.appservice = None # no application service exists
request_data = json.dumps({"username": "kermit"})
request_data = json.dumps(
{"username": "kermit", "type": AppserviceRegistrationType}
)
channel = self.make_request(
b"POST", self.url + b"?access_token=i_am_an_app_service", request_data
)
Expand Down
23 changes: 12 additions & 11 deletions tests/test_mau.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

import json

from synapse.api.constants import LoginType
from synapse.api.constants import AppserviceRegistrationType, LoginType
from synapse.api.errors import Codes, HttpResponseException, SynapseError
from synapse.appservice import ApplicationService
from synapse.rest.client.v2_alpha import register, sync
Expand Down Expand Up @@ -113,7 +113,7 @@ def test_as_ignores_mau(self):
)
)

self.create_user("as_kermit4", token=as_token)
self.create_user("as_kermit4", token=as_token, appservice=True)

def test_allowed_after_a_month_mau(self):
# Create and sync so that the MAU counts get updated
Expand Down Expand Up @@ -232,19 +232,20 @@ def test_tracked_but_not_limited(self):
self.reactor.advance(100)
self.assertEqual(2, self.successResultOf(count))

def create_user(self, localpart, token=None):
request_data = json.dumps(
{
"username": localpart,
"password": "monkey",
"auth": {"type": LoginType.DUMMY},
}
)
def create_user(self, localpart, token=None, appservice=False):
request_data = {
"username": localpart,
"password": "monkey",
"auth": {"type": LoginType.DUMMY},
}

if appservice:
request_data["type"] = AppserviceRegistrationType

channel = self.make_request(
"POST",
"/register",
request_data,
json.dumps(request_data),
Half-Shot marked this conversation as resolved.
Show resolved Hide resolved
access_token=token,
)

Expand Down