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

Combine SpamCheckerApi with the more generic ModuleApi. #8464

Merged
merged 4 commits into from
Oct 7, 2020
Merged
Show file tree
Hide file tree
Changes from all 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/8464.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Combine `SpamCheckerApi` with the more generic `ModuleApi`.
9 changes: 3 additions & 6 deletions docs/spam_checker.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ able to be imported by the running Synapse.
The Python class is instantiated with two objects:

* Any configuration (see below).
* An instance of `synapse.spam_checker_api.SpamCheckerApi`.
* An instance of `synapse.module_api.ModuleApi`.

It then implements methods which return a boolean to alter behavior in Synapse.

Expand All @@ -26,11 +26,8 @@ well as some specific methods:
The details of the each of these methods (as well as their inputs and outputs)
are documented in the `synapse.events.spamcheck.SpamChecker` class.

The `SpamCheckerApi` class provides a way for the custom spam checker class to
call back into the homeserver internals. It currently implements the following
methods:

* `get_state_events_in_room`
The `ModuleApi` class provides a way for the custom spam checker class to
call back into the homeserver internals.

### Example

Expand Down
3 changes: 1 addition & 2 deletions synapse/app/homeserver.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@
from synapse.http.site import SynapseSite
from synapse.logging.context import LoggingContext
from synapse.metrics import METRICS_PREFIX, MetricsResource, RegistryProxy
from synapse.module_api import ModuleApi
from synapse.python_dependencies import check_requirements
from synapse.replication.http import REPLICATION_PREFIX, ReplicationRestResource
from synapse.replication.tcp.resource import ReplicationStreamProtocolFactory
Expand Down Expand Up @@ -106,7 +105,7 @@ def _listener_http(self, config: HomeServerConfig, listener_config: ListenerConf

additional_resources = listener_config.http_options.additional_resources
logger.debug("Configuring additional resources: %r", additional_resources)
module_api = ModuleApi(self, self.get_auth_handler())
module_api = self.get_module_api()
for path, resmodule in additional_resources.items():
handler_cls, config = load_module(resmodule)
handler = handler_cls(config, module_api)
Expand Down
5 changes: 3 additions & 2 deletions synapse/events/spamcheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,25 @@
import inspect
from typing import Any, Dict, List, Optional, Tuple

from synapse.spam_checker_api import RegistrationBehaviour, SpamCheckerApi
from synapse.spam_checker_api import RegistrationBehaviour
from synapse.types import Collection

MYPY = False
if MYPY:
import synapse.events
Copy link
Member

Choose a reason for hiding this comment

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

Why this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

well, because synapse.events is referenced at line 42. I'm not really sure why mypy wasn't complaining before, but I think including it here is more correct.

Copy link
Member

Choose a reason for hiding this comment

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

Looks like mypy checks this file. 🤷

import synapse.server


class SpamChecker:
def __init__(self, hs: "synapse.server.HomeServer"):
self.spam_checkers = [] # type: List[Any]
api = hs.get_module_api()

for module, config in hs.config.spam_checkers:
# Older spam checkers don't accept the `api` argument, so we
# try and detect support.
spam_args = inspect.getfullargspec(module)
if "api" in spam_args.args:
api = SpamCheckerApi(hs)
self.spam_checkers.append(module(config=config, api=api))
else:
self.spam_checkers.append(module(config=config))
Expand Down
3 changes: 1 addition & 2 deletions synapse/events/third_party_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

from synapse.events import EventBase
from synapse.events.snapshot import EventContext
from synapse.module_api import ModuleApi
from synapse.types import Requester, StateMap


Expand All @@ -40,7 +39,7 @@ def __init__(self, hs):

if module is not None:
self.third_party_rules = module(
config=config, module_api=ModuleApi(hs, hs.get_auth_handler()),
config=config, module_api=hs.get_module_api(),
)

async def check_event_allowed(
Expand Down
7 changes: 7 additions & 0 deletions synapse/handlers/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,14 @@ def __init__(self, hs):

self.bcrypt_rounds = hs.config.bcrypt_rounds

# we can't use hs.get_module_api() here, because to do so will create an
# import loop.
#
# TODO: refactor this class to separate the lower-level stuff that
# ModuleApi can use from the higher-level stuff that uses ModuleApi, as
# better way to break the loop
account_handler = ModuleApi(hs, self)

self.password_providers = [
module(config=config, account_handler=account_handler)
for module, config in hs.config.password_providers
Expand Down
29 changes: 28 additions & 1 deletion synapse/module_api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import logging
from typing import TYPE_CHECKING
from typing import TYPE_CHECKING, Iterable, Optional, Tuple

from twisted.internet import defer

from synapse.http.client import SimpleHttpClient
from synapse.http.site import SynapseRequest
from synapse.logging.context import make_deferred_yieldable, run_in_background
from synapse.storage.state import StateFilter
from synapse.types import UserID

if TYPE_CHECKING:
Expand Down Expand Up @@ -293,6 +294,32 @@ async def complete_sso_login_async(
registered_user_id, request, client_redirect_url,
)

@defer.inlineCallbacks
def get_state_events_in_room(
self, room_id: str, types: Iterable[Tuple[str, Optional[str]]]
) -> defer.Deferred:
"""Gets current state events for the given room.

(This is exposed for compatibility with the old SpamCheckerApi. We should
probably deprecate it and replace it with an async method in a subclass.)

Args:
room_id: The room ID to get state events in.
types: The event type and state key (using None
to represent 'any') of the room state to acquire.

Returns:
twisted.internet.defer.Deferred[list(synapse.events.FrozenEvent)]:
The filtered state events in the room.
"""
state_ids = yield defer.ensureDeferred(
self._store.get_filtered_current_state_ids(
room_id=room_id, state_filter=StateFilter.from_types(types)
)
)
state = yield defer.ensureDeferred(self._store.get_events(state_ids.values()))
return state.values()


class PublicRoomListManager:
"""Contains methods for adding to, removing from and querying whether a room
Expand Down
5 changes: 5 additions & 0 deletions synapse/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@
from synapse.handlers.user_directory import UserDirectoryHandler
from synapse.http.client import InsecureInterceptableContextFactory, SimpleHttpClient
from synapse.http.matrixfederationclient import MatrixFederationHttpClient
from synapse.module_api import ModuleApi
from synapse.notifier import Notifier
from synapse.push.action_generator import ActionGenerator
from synapse.push.pusherpool import PusherPool
Expand Down Expand Up @@ -656,6 +657,10 @@ def get_replication_streams(self) -> Dict[str, Stream]:
def get_federation_ratelimiter(self) -> FederationRateLimiter:
return FederationRateLimiter(self.clock, config=self.config.rc_federation)

@cache_in_self
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this should not be cached? Caching it means that different plugins could theoretically interfere with each other.

Copy link
Member Author

Choose a reason for hiding this comment

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

well, they can probably do that anyway, if they so desire, since most of the functions are thin wrappers. 🤷‍♂️

Copy link
Member

Choose a reason for hiding this comment

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

True, probably not worth trying to protect against without a big effort.

def get_module_api(self) -> ModuleApi:
return ModuleApi(self, self.get_auth_handler())

async def remove_pusher(self, app_id: str, push_key: str, user_id: str):
return await self.get_pusherpool().remove_pusher(app_id, push_key, user_id)

Expand Down
43 changes: 0 additions & 43 deletions synapse/spam_checker_api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,8 @@
# 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 logging
from enum import Enum

from twisted.internet import defer

from synapse.storage.state import StateFilter

MYPY = False
if MYPY:
import synapse.server

logger = logging.getLogger(__name__)


class RegistrationBehaviour(Enum):
"""
Expand All @@ -34,35 +23,3 @@ class RegistrationBehaviour(Enum):
ALLOW = "allow"
SHADOW_BAN = "shadow_ban"
DENY = "deny"


class SpamCheckerApi:
"""A proxy object that gets passed to spam checkers so they can get
access to rooms and other relevant information.
"""

def __init__(self, hs: "synapse.server.HomeServer"):
self.hs = hs

self._store = hs.get_datastore()

@defer.inlineCallbacks
def get_state_events_in_room(self, room_id: str, types: tuple) -> defer.Deferred:
"""Gets state events for the given room.

Args:
room_id: The room ID to get state events in.
types: The event type and state key (using None
to represent 'any') of the room state to acquire.

Returns:
twisted.internet.defer.Deferred[list(synapse.events.FrozenEvent)]:
The filtered state events in the room.
"""
state_ids = yield defer.ensureDeferred(
self._store.get_filtered_current_state_ids(
room_id=room_id, state_filter=StateFilter.from_types(types)
)
)
state = yield defer.ensureDeferred(self._store.get_events(state_ids.values()))
return state.values()
4 changes: 2 additions & 2 deletions tests/module_api/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
# 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.
from synapse.module_api import ModuleApi

from synapse.rest import admin
from synapse.rest.client.v1 import login, room

Expand All @@ -28,7 +28,7 @@ class ModuleApiTestCase(HomeserverTestCase):

def prepare(self, reactor, clock, homeserver):
self.store = homeserver.get_datastore()
self.module_api = ModuleApi(homeserver, homeserver.get_auth_handler())
self.module_api = homeserver.get_module_api()

def test_can_register_user(self):
"""Tests that an external module can register a user"""
Expand Down