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

Allow admins to require a manual approval process before new accounts can be used (using MSC3866) #13556

Merged
merged 24 commits into from
Sep 29, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
f7c9743
Add experimental config options and constant for MSC3866
babolivier Aug 18, 2022
1eaff08
Add storage support for checking and updating a user's approval status
babolivier Aug 18, 2022
685f76f
Block new accounts after registering if configured to do so
babolivier Aug 18, 2022
5d08fe2
Block login if a user requires approval and the server is configured …
babolivier Aug 18, 2022
7b532a9
Change admin APIs to support checking and updating the approval statu…
babolivier Aug 18, 2022
eedaed1
Changelog
babolivier Aug 18, 2022
ffaea1e
Use a boolean in the database schema
babolivier Aug 30, 2022
0230200
Incorporate review
babolivier Aug 31, 2022
562aa7a
Merge branch 'develop' of github.com:matrix-org/synapse into babolivi…
babolivier Sep 21, 2022
868ab64
Incorporate review
babolivier Sep 21, 2022
836aa32
Merge branch 'develop' of github.com:matrix-org/synapse into babolivi…
babolivier Sep 21, 2022
8d091b4
Correctly filter on bools, not ints
babolivier Sep 22, 2022
116fc53
Merge branch 'develop' of github.com:matrix-org/synapse into babolivi…
babolivier Sep 22, 2022
a87d2f7
Don't create a new device if the new user needs approval
babolivier Sep 22, 2022
08d85f5
Test that we raise the error on SSO logins
babolivier Sep 22, 2022
7585098
Test that we don't register devices for users needing approval
babolivier Sep 22, 2022
75cf999
Lint
babolivier Sep 22, 2022
f4a7f16
Merge branch 'develop' of github.com:matrix-org/synapse into babolivi…
babolivier Sep 26, 2022
df0c887
Incorporate review
babolivier Sep 27, 2022
3f93dda
Fix test
babolivier Sep 29, 2022
577967c
Lint
babolivier Sep 29, 2022
7a5425a
Incorporate review
babolivier Sep 29, 2022
560e160
Incorporate latest change in the MSC
babolivier Sep 29, 2022
7d71712
Add comment to try to catch bool()ing NULLs in the future
babolivier Sep 29, 2022
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
11 changes: 11 additions & 0 deletions synapse/api/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,3 +269,14 @@ class PublicRoomsFilterFields:

GENERIC_SEARCH_TERM: Final = "generic_search_term"
ROOM_TYPES: Final = "room_types"


class ApprovalNoticeMedium:
"""Identifier for the medium this server will use to serve notice of approval for a
specific user's registration.

As defined in https://github.com/matrix-org/matrix-spec-proposals/blob/babolivier/m_not_approved/proposals/3866-user-not-approved-error.md
"""

NONE = "org.matrix.msc3866.none"
EMAIL = "org.matrix.msc3866.email"
14 changes: 14 additions & 0 deletions synapse/api/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,20 @@ def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict":
return cs_error(self.msg, self.errcode, **extra)


class NotApprovedError(SynapseError):
def __init__(
self,
msg: str,
approval_notice_medium: str,
):
super().__init__(
code=403,
msg=msg,
errcode=Codes.USER_AWAITING_APPROVAL,
additional_fields={"approval_notice_medium": approval_notice_medium},
)


def cs_error(msg: str, code: str = Codes.UNKNOWN, **kwargs: Any) -> "JsonDict":
"""Utility method for constructing an error response for client-server
interactions.
Expand Down
14 changes: 10 additions & 4 deletions synapse/rest/client/login.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,14 @@

from typing_extensions import TypedDict

from synapse.api.errors import Codes, InvalidClientTokenError, LoginError, SynapseError
from synapse.api.constants import ApprovalNoticeMedium
from synapse.api.errors import (
Codes,
InvalidClientTokenError,
LoginError,
NotApprovedError,
SynapseError,
)
from synapse.api.ratelimiting import Ratelimiter
from synapse.api.urls import CLIENT_API_PREFIX
from synapse.appservice import ApplicationService
Expand Down Expand Up @@ -229,10 +236,9 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, LoginResponse]:
if self._require_approval:
approved = await self.auth_handler.is_user_approved(result["user_id"])
if not approved:
raise SynapseError(
code=403,
errcode=Codes.USER_AWAITING_APPROVAL,
raise NotApprovedError(
msg="This account is pending approval by a server administrator.",
approval_notice_medium=ApprovalNoticeMedium.NONE,
)

well_known_data = self._well_known_builder.get_well_known()
Expand Down
12 changes: 8 additions & 4 deletions synapse/rest/client/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,15 @@
import synapse
import synapse.api.auth
import synapse.types
from synapse.api.constants import APP_SERVICE_REGISTRATION_TYPE, LoginType
from synapse.api.constants import (
APP_SERVICE_REGISTRATION_TYPE,
ApprovalNoticeMedium,
LoginType,
)
from synapse.api.errors import (
Codes,
InteractiveAuthIncompleteError,
NotApprovedError,
SynapseError,
ThreepidValidationError,
UnrecognizedRequestError,
Expand Down Expand Up @@ -740,10 +745,9 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
)

if self._require_approval:
raise SynapseError(
code=403,
errcode=Codes.USER_AWAITING_APPROVAL,
raise NotApprovedError(
msg="This account needs to be approved by an administrator before it can be used.",
approval_notice_medium=ApprovalNoticeMedium.NONE,
)

return 200, return_dict
Expand Down
13 changes: 12 additions & 1 deletion synapse/storage/databases/main/registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,11 +191,22 @@ def get_user_by_id_txn(txn: LoggingTransaction) -> Optional[Dict[str, Any]]:

return rows[0]

return await self.db_pool.runInteraction(
row = await self.db_pool.runInteraction(
desc="get_user_by_id",
func=get_user_by_id_txn,
)

if row is not None:
# If we're using SQLite our boolean values will be integers. Because we
# present some of this data as is to e.g. server admins via REST APIs, we
# want to make sure we're returning the right type of data.
boolean_columns = ["admin", "deactivated", "shadow_banned", "approved"]
for column in boolean_columns:
if not isinstance(row[column], bool):
row[column] = bool(row[column])
babolivier marked this conversation as resolved.
Show resolved Hide resolved

return row

async def get_userinfo_by_id(self, user_id: str) -> Optional[UserInfo]:
"""Get a UserInfo object for a user by user ID.

Expand Down
13 changes: 8 additions & 5 deletions tests/rest/admin/test_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@
from twisted.test.proto_helpers import MemoryReactor

import synapse.rest.admin
from synapse.api.constants import UserTypes, LoginType
from synapse.api.constants import ApprovalNoticeMedium, LoginType, UserTypes
from synapse.api.errors import Codes, HttpResponseException, ResourceLimitError
from synapse.api.room_versions import RoomVersions
from synapse.rest.client import devices, login, logout, profile, room, sync, register
from synapse.rest.client import devices, login, logout, profile, register, room, sync
from synapse.rest.media.v1.filepath import MediaFilePaths
from synapse.server import HomeServer
from synapse.types import JsonDict, UserID
Expand Down Expand Up @@ -2647,6 +2647,9 @@ def test_approve_account(self) -> None:
)
self.assertEqual(403, channel.code, channel.result)
self.assertEqual(Codes.USER_AWAITING_APPROVAL, channel.json_body["errcode"])
self.assertEqual(
ApprovalNoticeMedium.NONE, channel.json_body["approval_notice_medium"]
)

# Get user
channel = self.make_request(
Expand All @@ -2656,7 +2659,7 @@ def test_approve_account(self) -> None:
)

self.assertEqual(200, channel.code, msg=channel.json_body)
self.assertFalse(channel.json_body["approved"])
self.assertIs(False, channel.json_body["approved"])

# Approve user
channel = self.make_request(
Expand All @@ -2667,7 +2670,7 @@ def test_approve_account(self) -> None:
)

self.assertEqual(200, channel.code, msg=channel.json_body)
self.assertTrue(channel.json_body["approved"])
self.assertIs(True, channel.json_body["approved"])

# Check that the user is now approved
channel = self.make_request(
Expand All @@ -2677,7 +2680,7 @@ def test_approve_account(self) -> None:
)

self.assertEqual(200, channel.code, msg=channel.json_body)
self.assertTrue(channel.json_body["approved"])
self.assertIs(True, channel.json_body["approved"])

@override_config(
{
Expand Down
5 changes: 4 additions & 1 deletion tests/rest/client/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from twisted.web.resource import Resource

import synapse.rest.admin
from synapse.api.constants import LoginType
from synapse.api.constants import ApprovalNoticeMedium, LoginType
from synapse.api.errors import Codes
from synapse.handlers.ui_auth.checkers import UserInteractiveAuthChecker
from synapse.rest.client import account, auth, devices, login, logout, register
Expand Down Expand Up @@ -587,6 +587,9 @@ def test_sso_not_approved(self) -> None:
login_resp = self.helper.login_via_oidc("username", expected_status=403)

self.assertEqual(login_resp["errcode"], Codes.USER_AWAITING_APPROVAL)
self.assertEqual(
ApprovalNoticeMedium.NONE, login_resp["approval_notice_medium"]
)

# Check that we didn't register a device for the user during the login attempt.
devices = self.get_success(
Expand Down
8 changes: 7 additions & 1 deletion tests/rest/client/test_login.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
from twisted.web.resource import Resource

import synapse.rest.admin
from synapse.api.constants import LoginType
from synapse.api.constants import ApprovalNoticeMedium, LoginType
from synapse.api.errors import Codes
from synapse.appservice import ApplicationService
from synapse.rest.client import devices, login, logout, register
Expand Down Expand Up @@ -431,6 +431,9 @@ def test_require_approval(self) -> None:
)
self.assertEqual(403, channel.code, channel.result)
self.assertEqual(Codes.USER_AWAITING_APPROVAL, channel.json_body["errcode"])
self.assertEqual(
ApprovalNoticeMedium.NONE, channel.json_body["approval_notice_medium"]
)

params = {
"type": LoginType.PASSWORD,
Expand All @@ -440,6 +443,9 @@ def test_require_approval(self) -> None:
channel = self.make_request("POST", LOGIN_URL, params)
self.assertEqual(403, channel.code, channel.result)
self.assertEqual(Codes.USER_AWAITING_APPROVAL, channel.json_body["errcode"])
self.assertEqual(
ApprovalNoticeMedium.NONE, channel.json_body["approval_notice_medium"]
)


@skip_unless(has_saml2 and HAS_OIDC, "Requires SAML2 and OIDC")
Expand Down
9 changes: 8 additions & 1 deletion tests/rest/client/test_register.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@
from twisted.test.proto_helpers import MemoryReactor

import synapse.rest.admin
from synapse.api.constants import APP_SERVICE_REGISTRATION_TYPE, LoginType
from synapse.api.constants import (
APP_SERVICE_REGISTRATION_TYPE,
ApprovalNoticeMedium,
LoginType,
)
from synapse.api.errors import Codes
from synapse.appservice import ApplicationService
from synapse.rest.client import account, account_validity, login, logout, register, sync
Expand Down Expand Up @@ -787,6 +791,9 @@ def test_require_approval(self) -> None:
)
self.assertEqual(403, channel.code, channel.result)
self.assertEqual(Codes.USER_AWAITING_APPROVAL, channel.json_body["errcode"])
self.assertEqual(
ApprovalNoticeMedium.NONE, channel.json_body["approval_notice_medium"]
)


class AccountValidityTestCase(unittest.HomeserverTestCase):
Expand Down