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

Do not allow a deactivated user to login via SSO. #7240

Merged
merged 6 commits into from
Apr 9, 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/7240.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Do not allow a deactivated user to login via SSO.
7 changes: 7 additions & 0 deletions synapse/config/sso.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +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.
import os
from typing import Any, Dict

import pkg_resources
Expand All @@ -36,6 +37,12 @@ def read_config(self, config, **kwargs):
template_dir = pkg_resources.resource_filename("synapse", "res/templates",)

self.sso_redirect_confirm_template_dir = template_dir
self.sso_account_deactivated_template = self.read_file(
os.path.join(
self.sso_redirect_confirm_template_dir, "sso_account_deactivated.html"
),
"sso_account_deactivated_template",
)

self.sso_client_whitelist = sso_config.get("client_whitelist") or []

Expand Down
34 changes: 30 additions & 4 deletions synapse/handlers/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,9 @@ def __init__(self, hs):
self._sso_auth_confirm_template = load_jinja2_templates(
hs.config.sso_redirect_confirm_template_dir, ["sso_auth_confirm.html"],
)[0]
self._sso_account_deactivated_template = (
hs.config.sso_account_deactivated_template
)

self._server_name = hs.config.server_name

Expand Down Expand Up @@ -644,9 +647,6 @@ def check_user_exists(self, user_id: str):
Returns:
defer.Deferred: (unicode) canonical_user_id, or None if zero or
multiple matches

Raises:
UserDeactivatedError if a user is found but is deactivated.
"""
res = yield self._find_user_id_and_pwd_hash(user_id)
if res is not None:
Expand Down Expand Up @@ -1099,7 +1099,7 @@ def complete_sso_ui_auth(
request.write(html_bytes)
finish_request(request)

def complete_sso_login(
async def complete_sso_login(
self,
registered_user_id: str,
request: SynapseRequest,
Expand All @@ -1113,6 +1113,32 @@ def complete_sso_login(
client_redirect_url: The URL to which to redirect the user at the end of the
process.
"""
# If the account has been deactivated, do not proceed with the login
# flow.
deactivated = await self.store.get_user_deactivated_status(registered_user_id)
if deactivated:
html = self._sso_account_deactivated_template.encode("utf-8")

request.setResponseCode(403)
request.setHeader(b"Content-Type", b"text/html; charset=utf-8")
request.setHeader(b"Content-Length", b"%d" % (len(html),))
request.write(html)
finish_request(request)
return

self._complete_sso_login(registered_user_id, request, client_redirect_url)

def _complete_sso_login(
self,
registered_user_id: str,
request: SynapseRequest,
client_redirect_url: str,
):
"""
The synchronous portion of complete_sso_login.

This exists purely for backwards compatibility of synapse.module_api.ModuleApi.
"""
# Create a login token
login_token = self.macaroon_gen.generate_short_term_login_token(
registered_user_id
Expand Down
2 changes: 1 addition & 1 deletion synapse/handlers/cas_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,6 @@ async def handle_ticket(
localpart=localpart, default_display_name=user_display_name
)

self._auth_handler.complete_sso_login(
await self._auth_handler.complete_sso_login(
registered_user_id, request, client_redirect_url
)
2 changes: 1 addition & 1 deletion synapse/handlers/saml_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ async def handle_saml_response(self, request):
)

else:
self._auth_handler.complete_sso_login(user_id, request, relay_state)
await self._auth_handler.complete_sso_login(user_id, request, relay_state)

async def _map_saml_response_to_user(
self, resp_bytes: str, client_redirect_url: str
Expand Down
22 changes: 21 additions & 1 deletion synapse/module_api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,13 +220,33 @@ def complete_sso_login(
want their access token sent to `client_redirect_url`, or redirect them to that
URL with a token directly if the URL matches with one of the whitelisted clients.

This is deprecated in favor of complete_sso_login_async.

Args:
registered_user_id: The MXID that has been registered as a previous step of
of this SSO login.
request: The request to respond to.
client_redirect_url: The URL to which to offer to redirect the user (or to
redirect them directly if whitelisted).
"""
self._auth_handler._complete_sso_login(
registered_user_id, request, client_redirect_url,
)

async def complete_sso_login_async(
self, registered_user_id: str, request: SynapseRequest, client_redirect_url: str
):
"""Complete a SSO login by redirecting the user to a page to confirm whether they
want their access token sent to `client_redirect_url`, or redirect them to that
URL with a token directly if the URL matches with one of the whitelisted clients.

Args:
registered_user_id: The MXID that has been registered as a previous step of
of this SSO login.
request: The request to respond to.
client_redirect_url: The URL to which to offer to redirect the user (or to
redirect them directly if whitelisted).
"""
self._auth_handler.complete_sso_login(
await self._auth_handler.complete_sso_login(
registered_user_id, request, client_redirect_url,
)
10 changes: 10 additions & 0 deletions synapse/res/templates/sso_account_deactivated.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title>SSO account deactivated</title>
</head>
<body>
<p>This account has been deactivated.</p>
</body>
</html>
42 changes: 39 additions & 3 deletions tests/rest/client/v1/test_login.py
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ def _delete_device(self, access_token, user_id, password, device_id):
self.assertEquals(channel.code, 200, channel.result)


class CASRedirectConfirmTestCase(unittest.HomeserverTestCase):
class CASTestCase(unittest.HomeserverTestCase):

servlets = [
login.register_servlets,
Expand All @@ -274,6 +274,9 @@ def make_homeserver(self, reactor, clock):
"service_url": "https://matrix.goodserver.com:8448",
}

cas_user_id = "username"
self.user_id = "@%s:test" % cas_user_id

async def get_raw(uri, args):
"""Return an example response payload from a call to the `/proxyValidate`
endpoint of a CAS server, copied from
Expand All @@ -282,10 +285,11 @@ async def get_raw(uri, args):
This needs to be returned by an async function (as opposed to set as the
mock's return value) because the corresponding Synapse code awaits on it.
"""
return """
return (
"""
<cas:serviceResponse xmlns:cas='http://www.yale.edu/tp/cas'>
<cas:authenticationSuccess>
<cas:user>username</cas:user>
<cas:user>%s</cas:user>
<cas:proxyGrantingTicket>PGTIOU-84678-8a9d...</cas:proxyGrantingTicket>
<cas:proxies>
<cas:proxy>https://proxy2/pgtUrl</cas:proxy>
Expand All @@ -294,6 +298,8 @@ async def get_raw(uri, args):
</cas:authenticationSuccess>
</cas:serviceResponse>
"""
% cas_user_id
)

mocked_http_client = Mock(spec=["get_raw"])
mocked_http_client.get_raw.side_effect = get_raw
Expand All @@ -304,6 +310,9 @@ async def get_raw(uri, args):

return self.hs

def prepare(self, reactor, clock, hs):
self.deactivate_account_handler = hs.get_deactivate_account_handler()

def test_cas_redirect_confirm(self):
"""Tests that the SSO login flow serves a confirmation page before redirecting a
user to the redirect URL.
Expand Down Expand Up @@ -370,3 +379,30 @@ def _test_redirect(self, redirect_url):
self.assertEqual(channel.code, 302)
location_headers = channel.headers.getRawHeaders("Location")
self.assertEqual(location_headers[0][: len(redirect_url)], redirect_url)

@override_config({"sso": {"client_whitelist": ["https://legit-site.com/"]}})
def test_deactivated_user(self):
"""Logging in as a deactivated account should error."""
redirect_url = "https://legit-site.com/"

# First login (to create the user).
self._test_redirect(redirect_url)

# Deactivate the account.
self.get_success(
self.deactivate_account_handler.deactivate_account(self.user_id, False)
)

# Request the CAS ticket.
cas_ticket_url = (
"/_matrix/client/r0/login/cas/ticket?redirectUrl=%s&ticket=ticket"
% (urllib.parse.quote(redirect_url))
)

# Get Synapse to call the fake CAS and serve the template.
request, channel = self.make_request("GET", cas_ticket_url)
self.render(request)

# Because the user is deactivated they are served an error template.
self.assertEqual(channel.code, 403)
self.assertIn(b"SSO account deactivated", channel.result["body"])