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

Commit

Permalink
Rip out half-implemented m.login.saml2 support (#4265)
Browse files Browse the repository at this point in the history
* Rip out half-implemented m.login.saml2 support

This was implemented in an odd way that left most of the work to the client, in
a way that I really didn't understand. It's going to be a pain to maintain, so
let's start by ripping it out.

* drop undocumented dependency on dateutil

It turns out we were relying on dateutil being pulled in transitively by
pysaml2. There's no need for that bloat.
  • Loading branch information
richvdh authored and hawkowl committed Dec 6, 2018
1 parent 9a3e24a commit b0c24a6
Show file tree
Hide file tree
Showing 7 changed files with 4 additions and 169 deletions.
1 change: 1 addition & 0 deletions changelog.d/4265.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Rework SAML2 authentication
3 changes: 1 addition & 2 deletions synapse/config/homeserver.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
from .registration import RegistrationConfig
from .repository import ContentRepositoryConfig
from .room_directory import RoomDirectoryConfig
from .saml2 import SAML2Config
from .server import ServerConfig
from .server_notices_config import ServerNoticesConfig
from .spam_checker import SpamCheckerConfig
Expand All @@ -45,7 +44,7 @@
class HomeServerConfig(TlsConfig, ServerConfig, DatabaseConfig, LoggingConfig,
RatelimitConfig, ContentRepositoryConfig, CaptchaConfig,
VoipConfig, RegistrationConfig, MetricsConfig, ApiConfig,
AppServiceConfig, KeyConfig, SAML2Config, CasConfig,
AppServiceConfig, KeyConfig, CasConfig,
JWTConfig, PasswordConfig, EmailConfig,
WorkerConfig, PasswordAuthProviderConfig, PushConfig,
SpamCheckerConfig, GroupsConfig, UserDirectoryConfig,
Expand Down
55 changes: 0 additions & 55 deletions synapse/config/saml2.py

This file was deleted.

29 changes: 0 additions & 29 deletions synapse/handlers/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -327,35 +327,6 @@ def check_recaptcha(self, ip, private_key, challenge, response):
else:
logger.info("Valid captcha entered from %s", ip)

@defer.inlineCallbacks
def register_saml2(self, localpart):
"""
Registers email_id as SAML2 Based Auth.
"""
if types.contains_invalid_mxid_characters(localpart):
raise SynapseError(
400,
"User ID can only contain characters a-z, 0-9, or '=_-./'",
)
yield self.auth.check_auth_blocking()
user = UserID(localpart, self.hs.hostname)
user_id = user.to_string()

yield self.check_user_id_not_appservice_exclusive(user_id)
token = self.macaroon_gen.generate_access_token(user_id)
try:
yield self.store.register(
user_id=user_id,
token=token,
password_hash=None,
create_profile_with_localpart=user.localpart,
)
except Exception as e:
yield self.store.add_access_token_to_user(user_id, token)
# Ignore Registration errors
logger.exception(e)
defer.returnValue((user_id, token))

@defer.inlineCallbacks
def register_email(self, threepidCreds):
"""
Expand Down
1 change: 0 additions & 1 deletion synapse/python_dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@
"pillow>=3.1.2": ["PIL"],
"sortedcontainers>=1.4.4": ["sortedcontainers"],
"psutil>=2.0.0": ["psutil>=2.0.0"],
"pysaml2>=3.0.0": ["saml2"],
"pymacaroons-pynacl>=0.9.3": ["pymacaroons"],
"msgpack-python>=0.4.2": ["msgpack"],
"phonenumbers>=8.2.0": ["phonenumbers"],
Expand Down
69 changes: 2 additions & 67 deletions synapse/rest/client/v1/login.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@

from six.moves import urllib

from canonicaljson import json
from saml2 import BINDING_HTTP_POST, config
from saml2.client import Saml2Client

from twisted.internet import defer
from twisted.web.client import PartialDownloadError

Expand Down Expand Up @@ -81,16 +77,13 @@ def login_id_thirdparty_from_phone(identifier):

class LoginRestServlet(ClientV1RestServlet):
PATTERNS = client_path_patterns("/login$")
SAML2_TYPE = "m.login.saml2"
CAS_TYPE = "m.login.cas"
SSO_TYPE = "m.login.sso"
TOKEN_TYPE = "m.login.token"
JWT_TYPE = "m.login.jwt"

def __init__(self, hs):
super(LoginRestServlet, self).__init__(hs)
self.idp_redirect_url = hs.config.saml2_idp_redirect_url
self.saml2_enabled = hs.config.saml2_enabled
self.jwt_enabled = hs.config.jwt_enabled
self.jwt_secret = hs.config.jwt_secret
self.jwt_algorithm = hs.config.jwt_algorithm
Expand All @@ -103,8 +96,6 @@ def on_GET(self, request):
flows = []
if self.jwt_enabled:
flows.append({"type": LoginRestServlet.JWT_TYPE})
if self.saml2_enabled:
flows.append({"type": LoginRestServlet.SAML2_TYPE})
if self.cas_enabled:
flows.append({"type": LoginRestServlet.SSO_TYPE})

Expand Down Expand Up @@ -134,18 +125,8 @@ def on_OPTIONS(self, request):
def on_POST(self, request):
login_submission = parse_json_object_from_request(request)
try:
if self.saml2_enabled and (login_submission["type"] ==
LoginRestServlet.SAML2_TYPE):
relay_state = ""
if "relay_state" in login_submission:
relay_state = "&RelayState=" + urllib.parse.quote(
login_submission["relay_state"])
result = {
"uri": "%s%s" % (self.idp_redirect_url, relay_state)
}
defer.returnValue((200, result))
elif self.jwt_enabled and (login_submission["type"] ==
LoginRestServlet.JWT_TYPE):
if self.jwt_enabled and (login_submission["type"] ==
LoginRestServlet.JWT_TYPE):
result = yield self.do_jwt_login(login_submission)
defer.returnValue(result)
elif login_submission["type"] == LoginRestServlet.TOKEN_TYPE:
Expand Down Expand Up @@ -345,50 +326,6 @@ def _register_device(self, user_id, login_submission):
)


class SAML2RestServlet(ClientV1RestServlet):
PATTERNS = client_path_patterns("/login/saml2", releases=())

def __init__(self, hs):
super(SAML2RestServlet, self).__init__(hs)
self.sp_config = hs.config.saml2_config_path
self.handlers = hs.get_handlers()

@defer.inlineCallbacks
def on_POST(self, request):
saml2_auth = None
try:
conf = config.SPConfig()
conf.load_file(self.sp_config)
SP = Saml2Client(conf)
saml2_auth = SP.parse_authn_request_response(
request.args['SAMLResponse'][0], BINDING_HTTP_POST)
except Exception as e: # Not authenticated
logger.exception(e)
if saml2_auth and saml2_auth.status_ok() and not saml2_auth.not_signed:
username = saml2_auth.name_id.text
handler = self.handlers.registration_handler
(user_id, token) = yield handler.register_saml2(username)
# Forward to the RelayState callback along with ava
if 'RelayState' in request.args:
request.redirect(urllib.parse.unquote(
request.args['RelayState'][0]) +
'?status=authenticated&access_token=' +
token + '&user_id=' + user_id + '&ava=' +
urllib.quote(json.dumps(saml2_auth.ava)))
finish_request(request)
defer.returnValue(None)
defer.returnValue((200, {"status": "authenticated",
"user_id": user_id, "token": token,
"ava": saml2_auth.ava}))
elif 'RelayState' in request.args:
request.redirect(urllib.parse.unquote(
request.args['RelayState'][0]) +
'?status=not_authenticated')
finish_request(request)
defer.returnValue(None)
defer.returnValue((200, {"status": "not_authenticated"}))


class CasRedirectServlet(RestServlet):
PATTERNS = client_path_patterns("/login/(cas|sso)/redirect")

Expand Down Expand Up @@ -517,8 +454,6 @@ def parse_cas_response(self, cas_response_body):

def register_servlets(hs, http_server):
LoginRestServlet(hs).register(http_server)
if hs.config.saml2_enabled:
SAML2RestServlet(hs).register(http_server)
if hs.config.cas_enabled:
CasRedirectServlet(hs).register(http_server)
CasTicketServlet(hs).register(http_server)
15 changes: 0 additions & 15 deletions tests/handlers/test_register.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,21 +129,6 @@ def test_register_mau_blocked(self):
with self.assertRaises(ResourceLimitError):
yield self.handler.register(localpart="local_part")

@defer.inlineCallbacks
def test_register_saml2_mau_blocked(self):
self.hs.config.limit_usage_by_mau = True
self.store.get_monthly_active_count = Mock(
return_value=defer.succeed(self.lots_of_users)
)
with self.assertRaises(ResourceLimitError):
yield self.handler.register_saml2(localpart="local_part")

self.store.get_monthly_active_count = Mock(
return_value=defer.succeed(self.hs.config.max_mau_value)
)
with self.assertRaises(ResourceLimitError):
yield self.handler.register_saml2(localpart="local_part")

@defer.inlineCallbacks
def test_auto_create_auto_join_rooms(self):
room_alias_str = "#room:test"
Expand Down

0 comments on commit b0c24a6

Please sign in to comment.