Disable account related endpoints when using OAuth delegation

This commit is contained in:
Quentin Gliech 2023-05-10 16:08:43 +02:00 committed by Patrick Cloke
parent 5fe96082d0
commit 31691d6151
8 changed files with 243 additions and 19 deletions

View File

@ -274,6 +274,8 @@ class AuthHandler:
# response. # response.
self._extra_attributes: Dict[str, SsoLoginExtraAttributes] = {} self._extra_attributes: Dict[str, SsoLoginExtraAttributes] = {}
self.oauth_delegation_enabled = hs.config.auth.oauth_delegation_enabled
async def validate_user_via_ui_auth( async def validate_user_via_ui_auth(
self, self,
requester: Requester, requester: Requester,
@ -322,8 +324,12 @@ class AuthHandler:
LimitExceededError if the ratelimiter's failed request count for this LimitExceededError if the ratelimiter's failed request count for this
user is too high to proceed user is too high to proceed
""" """
if self.oauth_delegation_enabled:
raise SynapseError(
HTTPStatus.INTERNAL_SERVER_ERROR, "UIA shouldn't be used with MSC3861"
)
if not requester.access_token_id: if not requester.access_token_id:
raise ValueError("Cannot validate a user without an access token") raise ValueError("Cannot validate a user without an access token")
if can_skip_ui_auth and self._ui_auth_session_timeout: if can_skip_ui_auth and self._ui_auth_session_timeout:

View File

@ -27,6 +27,7 @@ from synapse.api.constants import LoginType
from synapse.api.errors import ( from synapse.api.errors import (
Codes, Codes,
InteractiveAuthIncompleteError, InteractiveAuthIncompleteError,
NotFoundError,
SynapseError, SynapseError,
ThreepidValidationError, ThreepidValidationError,
) )
@ -600,6 +601,9 @@ class ThreepidRestServlet(RestServlet):
# ThreePidBindRestServelet.PostBody with an `alias_generator` to handle # ThreePidBindRestServelet.PostBody with an `alias_generator` to handle
# `threePidCreds` versus `three_pid_creds`. # `threePidCreds` versus `three_pid_creds`.
async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
if self.hs.config.auth.oauth_delegation_enabled:
raise NotFoundError(errcode=Codes.UNRECOGNIZED)
if not self.hs.config.registration.enable_3pid_changes: if not self.hs.config.registration.enable_3pid_changes:
raise SynapseError( raise SynapseError(
400, "3PID changes are disabled on this server", Codes.FORBIDDEN 400, "3PID changes are disabled on this server", Codes.FORBIDDEN
@ -890,18 +894,20 @@ class AccountStatusRestServlet(RestServlet):
def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None: def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None:
if hs.config.worker.worker_app is None: if hs.config.worker.worker_app is None:
if not hs.config.auth.oauth_delegation_enabled:
EmailPasswordRequestTokenRestServlet(hs).register(http_server) EmailPasswordRequestTokenRestServlet(hs).register(http_server)
PasswordRestServlet(hs).register(http_server)
DeactivateAccountRestServlet(hs).register(http_server) DeactivateAccountRestServlet(hs).register(http_server)
PasswordRestServlet(hs).register(http_server)
EmailThreepidRequestTokenRestServlet(hs).register(http_server) EmailThreepidRequestTokenRestServlet(hs).register(http_server)
MsisdnThreepidRequestTokenRestServlet(hs).register(http_server) MsisdnThreepidRequestTokenRestServlet(hs).register(http_server)
AddThreepidEmailSubmitTokenServlet(hs).register(http_server) AddThreepidEmailSubmitTokenServlet(hs).register(http_server)
AddThreepidMsisdnSubmitTokenServlet(hs).register(http_server) AddThreepidMsisdnSubmitTokenServlet(hs).register(http_server)
ThreepidRestServlet(hs).register(http_server) ThreepidRestServlet(hs).register(http_server)
if hs.config.worker.worker_app is None: if hs.config.worker.worker_app is None:
ThreepidAddRestServlet(hs).register(http_server)
ThreepidBindRestServlet(hs).register(http_server) ThreepidBindRestServlet(hs).register(http_server)
ThreepidUnbindRestServlet(hs).register(http_server) ThreepidUnbindRestServlet(hs).register(http_server)
if not hs.config.auth.oauth_delegation_enabled:
ThreepidAddRestServlet(hs).register(http_server)
ThreepidDeleteRestServlet(hs).register(http_server) ThreepidDeleteRestServlet(hs).register(http_server)
WhoamiRestServlet(hs).register(http_server) WhoamiRestServlet(hs).register(http_server)

View File

@ -19,7 +19,7 @@ from typing import TYPE_CHECKING, List, Optional, Tuple
from pydantic import Extra, StrictStr from pydantic import Extra, StrictStr
from synapse.api import errors from synapse.api import errors
from synapse.api.errors import NotFoundError from synapse.api.errors import NotFoundError, UnrecognizedRequestError
from synapse.handlers.device import DeviceHandler from synapse.handlers.device import DeviceHandler
from synapse.http.server import HttpServer from synapse.http.server import HttpServer
from synapse.http.servlet import ( from synapse.http.servlet import (
@ -135,6 +135,7 @@ class DeviceRestServlet(RestServlet):
self.device_handler = handler self.device_handler = handler
self.auth_handler = hs.get_auth_handler() self.auth_handler = hs.get_auth_handler()
self._msc3852_enabled = hs.config.experimental.msc3852_enabled self._msc3852_enabled = hs.config.experimental.msc3852_enabled
self.oauth_delegation_enabled = hs.config.auth.oauth_delegation_enabled
async def on_GET( async def on_GET(
self, request: SynapseRequest, device_id: str self, request: SynapseRequest, device_id: str
@ -166,6 +167,9 @@ class DeviceRestServlet(RestServlet):
async def on_DELETE( async def on_DELETE(
self, request: SynapseRequest, device_id: str self, request: SynapseRequest, device_id: str
) -> Tuple[int, JsonDict]: ) -> Tuple[int, JsonDict]:
if self.oauth_delegation_enabled:
raise UnrecognizedRequestError(code=404)
requester = await self.auth.get_user_by_req(request) requester = await self.auth.get_user_by_req(request)
try: try:
@ -344,7 +348,10 @@ class ClaimDehydratedDeviceServlet(RestServlet):
def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None: def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None:
if hs.config.worker.worker_app is None: if (
hs.config.worker.worker_app is None
and not hs.config.auth.oauth_delegation_enabled
):
DeleteDevicesRestServlet(hs).register(http_server) DeleteDevicesRestServlet(hs).register(http_server)
DevicesRestServlet(hs).register(http_server) DevicesRestServlet(hs).register(http_server)
if hs.config.worker.worker_app is None: if hs.config.worker.worker_app is None:

View File

@ -17,9 +17,10 @@
import logging import logging
import re import re
from collections import Counter from collections import Counter
from http import HTTPStatus
from typing import TYPE_CHECKING, Any, Dict, Optional, Tuple from typing import TYPE_CHECKING, Any, Dict, Optional, Tuple
from synapse.api.errors import InvalidAPICallError, SynapseError from synapse.api.errors import Codes, InvalidAPICallError, SynapseError
from synapse.http.server import HttpServer from synapse.http.server import HttpServer
from synapse.http.servlet import ( from synapse.http.servlet import (
RestServlet, RestServlet,
@ -375,9 +376,29 @@ class SigningKeyUploadServlet(RestServlet):
user_id = requester.user.to_string() user_id = requester.user.to_string()
body = parse_json_object_from_request(request) body = parse_json_object_from_request(request)
if self.hs.config.experimental.msc3967_enabled: is_cross_signing_setup = (
if await self.e2e_keys_handler.is_cross_signing_set_up_for_user(user_id): await self.e2e_keys_handler.is_cross_signing_set_up_for_user(user_id)
)
# Before MSC3967 we required UIA both when setting up cross signing for the
# first time and when resetting the device signing key. With MSC3967 we only
# require UIA when resetting cross-signing, and not when setting up the first
# time. Because there is no UIA in MSC3861, for now we throw an error if the
# user tries to reset the device signing key when MSC3861 is enabled, but allow
# first-time setup.
if self.hs.config.auth.oauth_delegation_enabled:
# There is no way to reset the device signing key with MSC3861
if is_cross_signing_setup:
raise SynapseError(
HTTPStatus.NOT_IMPLEMENTED,
"Resetting cross signing keys is not yet supported with MSC3861",
Codes.UNRECOGNIZED,
)
# But first-time setup is fine
elif self.hs.config.experimental.msc3967_enabled:
# If we already have a master key then cross signing is set up and we require UIA to reset # If we already have a master key then cross signing is set up and we require UIA to reset
if is_cross_signing_setup:
await self.auth_handler.validate_user_via_ui_auth( await self.auth_handler.validate_user_via_ui_auth(
requester, requester,
request, request,
@ -387,6 +408,7 @@ class SigningKeyUploadServlet(RestServlet):
can_skip_ui_auth=False, can_skip_ui_auth=False,
) )
# Otherwise we don't require UIA since we are setting up cross signing for first time # Otherwise we don't require UIA since we are setting up cross signing for first time
else: else:
# Previous behaviour is to always require UIA but allow it to be skipped # Previous behaviour is to always require UIA but allow it to be skipped
await self.auth_handler.validate_user_via_ui_auth( await self.auth_handler.validate_user_via_ui_auth(

View File

@ -633,6 +633,9 @@ class CasTicketServlet(RestServlet):
def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None: def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None:
if hs.config.auth.oauth_delegation_enabled:
return
LoginRestServlet(hs).register(http_server) LoginRestServlet(hs).register(http_server)
if ( if (
hs.config.worker.worker_app is None hs.config.worker.worker_app is None

View File

@ -80,5 +80,8 @@ class LogoutAllRestServlet(RestServlet):
def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None: def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None:
if hs.config.auth.oauth_delegation_enabled:
return
LogoutRestServlet(hs).register(http_server) LogoutRestServlet(hs).register(http_server)
LogoutAllRestServlet(hs).register(http_server) LogoutAllRestServlet(hs).register(http_server)

View File

@ -955,6 +955,9 @@ def _calculate_registration_flows(
def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None: def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None:
if hs.config.auth.oauth_delegation_enabled:
return
if hs.config.worker.worker_app is None: if hs.config.worker.worker_app is None:
EmailRegisterRequestTokenRestServlet(hs).register(http_server) EmailRegisterRequestTokenRestServlet(hs).register(http_server)
MsisdnRegisterRequestTokenRestServlet(hs).register(http_server) MsisdnRegisterRequestTokenRestServlet(hs).register(http_server)

View File

@ -11,14 +11,27 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and # See the License for the specific language governing permissions and
# limitations under the License. # limitations under the License.
from typing import Any, Dict
from http import HTTPStatus
from typing import Any, Dict, Union
from unittest.mock import ANY, Mock from unittest.mock import ANY, Mock
from urllib.parse import parse_qs from urllib.parse import parse_qs
from signedjson.key import (
encode_verify_key_base64,
generate_signing_key,
get_verify_key,
)
from signedjson.sign import sign_json
from twisted.test.proto_helpers import MemoryReactor from twisted.test.proto_helpers import MemoryReactor
from synapse.api.errors import InvalidClientTokenError, OAuthInsufficientScopeError from synapse.api.errors import (
from synapse.rest.client import devices Codes,
InvalidClientTokenError,
OAuthInsufficientScopeError,
)
from synapse.rest.client import account, devices, keys, login, logout, register
from synapse.server import HomeServer from synapse.server import HomeServer
from synapse.types import JsonDict from synapse.types import JsonDict
from synapse.util import Clock from synapse.util import Clock
@ -57,6 +70,7 @@ DEVICE = "AABBCCDD"
MATRIX_DEVICE_SCOPE = "urn:matrix:org.matrix.msc2967.client:device:" + DEVICE MATRIX_DEVICE_SCOPE = "urn:matrix:org.matrix.msc2967.client:device:" + DEVICE
SUBJECT = "abc-def-ghi" SUBJECT = "abc-def-ghi"
USERNAME = "test-user" USERNAME = "test-user"
USER_ID = "@" + USERNAME + ":" + SERVER_NAME
async def get_json(url: str) -> JsonDict: async def get_json(url: str) -> JsonDict:
@ -84,7 +98,12 @@ async def get_json(url: str) -> JsonDict:
@skip_unless(HAS_AUTHLIB, "requires authlib") @skip_unless(HAS_AUTHLIB, "requires authlib")
class MSC3861OAuthDelegation(HomeserverTestCase): class MSC3861OAuthDelegation(HomeserverTestCase):
servlets = [ servlets = [
account.register_servlets,
devices.register_servlets, devices.register_servlets,
keys.register_servlets,
register.register_servlets,
login.register_servlets,
logout.register_servlets,
] ]
def default_config(self) -> Dict[str, Any]: def default_config(self) -> Dict[str, Any]:
@ -380,3 +399,158 @@ class MSC3861OAuthDelegation(HomeserverTestCase):
get_awaitable_result(self.auth.is_server_admin(requester)), False get_awaitable_result(self.auth.is_server_admin(requester)), False
) )
self.assertEqual(requester.device_id, DEVICE) self.assertEqual(requester.device_id, DEVICE)
def make_device_keys(self, user_id: str, device_id: str) -> JsonDict:
# We only generate a master key to simplify the test.
master_signing_key = generate_signing_key(device_id)
master_verify_key = encode_verify_key_base64(get_verify_key(master_signing_key))
return {
"master_key": sign_json(
{
"user_id": user_id,
"usage": ["master"],
"keys": {"ed25519:" + master_verify_key: master_verify_key},
},
user_id,
master_signing_key,
),
}
def test_cross_signing(self) -> None:
"""Try uploading device keys with OAuth delegation enabled."""
self.http_client.request = simple_async_mock(
return_value=FakeResponse.json(
code=200,
payload={
"active": True,
"sub": SUBJECT,
"scope": " ".join([MATRIX_USER_SCOPE, MATRIX_DEVICE_SCOPE]),
"username": USERNAME,
},
)
)
keys_upload_body = self.make_device_keys(USER_ID, DEVICE)
channel = self.make_request(
"POST",
"/_matrix/client/v3/keys/device_signing/upload",
keys_upload_body,
access_token="mockAccessToken",
)
self.assertEqual(channel.code, 200, channel.json_body)
channel = self.make_request(
"POST",
"/_matrix/client/v3/keys/device_signing/upload",
keys_upload_body,
access_token="mockAccessToken",
)
self.assertEqual(channel.code, HTTPStatus.NOT_IMPLEMENTED, channel.json_body)
def expect_unauthorized(
self, method: str, path: str, content: Union[bytes, str, JsonDict] = ""
) -> None:
channel = self.make_request(method, path, content, shorthand=False)
self.assertEqual(channel.code, 401, channel.json_body)
def expect_unrecognized(
self, method: str, path: str, content: Union[bytes, str, JsonDict] = ""
) -> None:
channel = self.make_request(method, path, content)
self.assertEqual(channel.code, 404, channel.json_body)
self.assertEqual(
channel.json_body["errcode"], Codes.UNRECOGNIZED, channel.json_body
)
def test_uia_endpoints(self) -> None:
"""Test that endpoints that were removed in MSC2964 are no longer available."""
# This is just an endpoint that should remain visible (but requires auth):
self.expect_unauthorized("GET", "/_matrix/client/v3/devices")
# This remains usable, but will require a uia scope:
self.expect_unauthorized(
"POST", "/_matrix/client/v3/keys/device_signing/upload"
)
def test_3pid_endpoints(self) -> None:
"""Test that 3pid account management endpoints that were removed in MSC2964 are no longer available."""
# Remains and requires auth:
self.expect_unauthorized("GET", "/_matrix/client/v3/account/3pid")
self.expect_unauthorized(
"POST",
"/_matrix/client/v3/account/3pid/bind",
{
"client_secret": "foo",
"id_access_token": "bar",
"id_server": "foo",
"sid": "bar",
},
)
self.expect_unauthorized("POST", "/_matrix/client/v3/account/3pid/unbind", {})
# These are gone:
self.expect_unrecognized(
"POST", "/_matrix/client/v3/account/3pid"
) # deprecated
self.expect_unrecognized("POST", "/_matrix/client/v3/account/3pid/add")
self.expect_unrecognized("POST", "/_matrix/client/v3/account/3pid/delete")
self.expect_unrecognized(
"POST", "/_matrix/client/v3/account/3pid/email/requestToken"
)
self.expect_unrecognized(
"POST", "/_matrix/client/v3/account/3pid/msisdn/requestToken"
)
def test_account_management_endpoints_removed(self) -> None:
"""Test that account management endpoints that were removed in MSC2964 are no longer available."""
self.expect_unrecognized("POST", "/_matrix/client/v3/account/deactivate")
self.expect_unrecognized("POST", "/_matrix/client/v3/account/password")
self.expect_unrecognized(
"POST", "/_matrix/client/v3/account/password/email/requestToken"
)
self.expect_unrecognized(
"POST", "/_matrix/client/v3/account/password/msisdn/requestToken"
)
def test_registration_endpoints_removed(self) -> None:
"""Test that registration endpoints that were removed in MSC2964 are no longer available."""
self.expect_unrecognized(
"GET", "/_matrix/client/v1/register/m.login.registration_token/validity"
)
self.expect_unrecognized("POST", "/_matrix/client/v3/register")
self.expect_unrecognized("GET", "/_matrix/client/v3/register")
self.expect_unrecognized("GET", "/_matrix/client/v3/register/available")
self.expect_unrecognized(
"POST", "/_matrix/client/v3/register/email/requestToken"
)
self.expect_unrecognized(
"POST", "/_matrix/client/v3/register/msisdn/requestToken"
)
def test_session_management_endpoints_removed(self) -> None:
"""Test that session management endpoints that were removed in MSC2964 are no longer available."""
self.expect_unrecognized("GET", "/_matrix/client/v3/login")
self.expect_unrecognized("POST", "/_matrix/client/v3/login")
self.expect_unrecognized("GET", "/_matrix/client/v3/login/sso/redirect")
self.expect_unrecognized("POST", "/_matrix/client/v3/logout")
self.expect_unrecognized("POST", "/_matrix/client/v3/logout/all")
self.expect_unrecognized("POST", "/_matrix/client/v3/refresh")
self.expect_unrecognized("GET", "/_matrix/static/client/login")
def test_device_management_endpoints_removed(self) -> None:
"""Test that device management endpoints that were removed in MSC2964 are no longer available."""
self.expect_unrecognized("POST", "/_matrix/client/v3/delete_devices")
self.expect_unrecognized("DELETE", "/_matrix/client/v3/devices/{DEVICE}")
def test_openid_endpoints_removed(self) -> None:
"""Test that OpenID id_token endpoints that were removed in MSC2964 are no longer available."""
self.expect_unrecognized(
"POST", "/_matrix/client/v3/user/{USERNAME}/openid/request_token"
)