From 45f1827fb7ed57a4e24025194fd35683c73593dd Mon Sep 17 00:00:00 2001 From: Steven Hammerton Date: Wed, 4 Nov 2015 23:32:30 +0000 Subject: [PATCH 1/7] Add service URL to CAS config --- synapse/config/cas.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/synapse/config/cas.py b/synapse/config/cas.py index a337ae6ca..3f22fbfb4 100644 --- a/synapse/config/cas.py +++ b/synapse/config/cas.py @@ -27,10 +27,12 @@ class CasConfig(Config): if cas_config: self.cas_enabled = cas_config.get("enabled", True) self.cas_server_url = cas_config["server_url"] + self.cas_service_url = cas_config["service_url"] self.cas_required_attributes = cas_config.get("required_attributes", {}) else: self.cas_enabled = False self.cas_server_url = None + self.cas_service_url = None self.cas_required_attributes = {} def default_config(self, config_dir_path, server_name, **kwargs): @@ -39,6 +41,7 @@ class CasConfig(Config): #cas_config: # enabled: true # server_url: "https://cas-server.com" + # ticket_redirect_url: "https://homesever.domain.com:8448" # #required_attributes: # # name: value """ From 414a4a71b4421a376feb4e3e4ec5ae997fa289b2 Mon Sep 17 00:00:00 2001 From: Steven Hammerton Date: Thu, 5 Nov 2015 14:01:12 +0000 Subject: [PATCH 2/7] Allow hs to do CAS login completely and issue the client with a login token that can be redeemed for the usual successful login response --- synapse/config/cas.py | 2 +- synapse/handlers/auth.py | 76 ++++++++++++++++- synapse/rest/client/v1/login.py | 145 +++++++++++++++++++++++++++++++- 3 files changed, 218 insertions(+), 5 deletions(-) diff --git a/synapse/config/cas.py b/synapse/config/cas.py index 3f22fbfb4..326e40584 100644 --- a/synapse/config/cas.py +++ b/synapse/config/cas.py @@ -41,7 +41,7 @@ class CasConfig(Config): #cas_config: # enabled: true # server_url: "https://cas-server.com" - # ticket_redirect_url: "https://homesever.domain.com:8448" + # service_url: "https://homesever.domain.com:8448" # #required_attributes: # # name: value """ diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 1b11dbdff..7a85883aa 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -18,7 +18,7 @@ from twisted.internet import defer from ._base import BaseHandler from synapse.api.constants import LoginType from synapse.types import UserID -from synapse.api.errors import LoginError, Codes +from synapse.api.errors import AuthError, LoginError, Codes from synapse.util.async import run_on_reactor from twisted.web.client import PartialDownloadError @@ -46,6 +46,13 @@ class AuthHandler(BaseHandler): } self.bcrypt_rounds = hs.config.bcrypt_rounds self.sessions = {} + self.INVALID_TOKEN_HTTP_STATUS = 401 + self._KNOWN_CAVEAT_PREFIXES = set([ + "gen = ", + "type = ", + "time < ", + "user_id = ", + ]) @defer.inlineCallbacks def check_auth(self, flows, clientdict, clientip): @@ -297,10 +304,11 @@ class AuthHandler(BaseHandler): defer.returnValue((user_id, access_token, refresh_token)) @defer.inlineCallbacks - def login_with_cas_user_id(self, user_id): + def login_with_user_id(self, user_id): """ Authenticates the user with the given user ID, - intended to have been captured from a CAS response + it is intended that the authentication of the user has + already been verified by other mechanism (e.g. CAS) Args: user_id (str): User ID @@ -393,6 +401,17 @@ class AuthHandler(BaseHandler): )) return m.serialize() + def generate_short_term_login_token(self, user_id): + macaroon = self._generate_base_macaroon(user_id) + macaroon.add_first_party_caveat("type = login") + now = self.hs.get_clock().time_msec() + expiry = now + (2 * 60 * 1000) + macaroon.add_first_party_caveat("time < %d" % (expiry,)) + return macaroon.serialize() + + def validate_short_term_login_token_and_get_user_id(self, login_token): + return self._validate_macaroon_and_get_user_id(login_token, "login", True) + def _generate_base_macaroon(self, user_id): macaroon = pymacaroons.Macaroon( location=self.hs.config.server_name, @@ -402,6 +421,57 @@ class AuthHandler(BaseHandler): macaroon.add_first_party_caveat("user_id = %s" % (user_id,)) return macaroon + def _validate_macaroon_and_get_user_id(self, macaroon_str, + macaroon_type, validate_expiry): + try: + macaroon = pymacaroons.Macaroon.deserialize(macaroon_str) + user_id = self._get_user_from_macaroon(macaroon) + v = pymacaroons.Verifier() + v.satisfy_exact("gen = 1") + v.satisfy_exact("type = " + macaroon_type) + v.satisfy_exact("user_id = " + user_id) + if validate_expiry: + v.satisfy_general(self._verify_expiry) + + v.verify(macaroon, self.hs.config.macaroon_secret_key) + + v = pymacaroons.Verifier() + v.satisfy_general(self._verify_recognizes_caveats) + v.verify(macaroon, self.hs.config.macaroon_secret_key) + return user_id + except (pymacaroons.exceptions.MacaroonException, TypeError, ValueError): + raise AuthError( + self.INVALID_TOKEN_HTTP_STATUS, "Invalid token", + errcode=Codes.UNKNOWN_TOKEN + ) + + def _get_user_from_macaroon(self, macaroon): + user_prefix = "user_id = " + for caveat in macaroon.caveats: + if caveat.caveat_id.startswith(user_prefix): + return caveat.caveat_id[len(user_prefix):] + raise AuthError( + self.INVALID_TOKEN_HTTP_STATUS, "No user_id found in token", + errcode=Codes.UNKNOWN_TOKEN + ) + + def _verify_expiry(self, caveat): + prefix = "time < " + if not caveat.startswith(prefix): + return False + expiry = int(caveat[len(prefix):]) + now = self.hs.get_clock().time_msec() + return now < expiry + + def _verify_recognizes_caveats(self, caveat): + first_space = caveat.find(" ") + if first_space < 0: + return False + second_space = caveat.find(" ", first_space + 1) + if second_space < 0: + return False + return caveat[:second_space + 1] in self._KNOWN_CAVEAT_PREFIXES + @defer.inlineCallbacks def set_password(self, user_id, newpassword): password_hash = self.hash(newpassword) diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index 4ea06c143..5a2cedacb 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -22,6 +22,7 @@ from base import ClientV1RestServlet, client_path_pattern import simplejson as json import urllib +import urlparse import logging from saml2 import BINDING_HTTP_POST @@ -39,6 +40,7 @@ class LoginRestServlet(ClientV1RestServlet): PASS_TYPE = "m.login.password" SAML2_TYPE = "m.login.saml2" CAS_TYPE = "m.login.cas" + TOKEN_TYPE = "m.login.token" def __init__(self, hs): super(LoginRestServlet, self).__init__(hs) @@ -58,6 +60,7 @@ class LoginRestServlet(ClientV1RestServlet): flows.append({"type": LoginRestServlet.CAS_TYPE}) if self.password_enabled: flows.append({"type": LoginRestServlet.PASS_TYPE}) + flows.append({"type": LoginRestServlet.TOKEN_TYPE}) return (200, {"flows": flows}) def on_OPTIONS(self, request): @@ -83,6 +86,7 @@ class LoginRestServlet(ClientV1RestServlet): "uri": "%s%s" % (self.idp_redirect_url, relay_state) } defer.returnValue((200, result)) + # TODO Delete this after all CAS clients switch to token login instead elif self.cas_enabled and (login_submission["type"] == LoginRestServlet.CAS_TYPE): # TODO: get this from the homeserver rather than creating a new one for @@ -96,6 +100,9 @@ class LoginRestServlet(ClientV1RestServlet): body = yield http_client.get_raw(uri, args) result = yield self.do_cas_login(body) defer.returnValue(result) + elif login_submission["type"] == LoginRestServlet.TOKEN_TYPE: + result = yield self.do_token_login(login_submission) + defer.returnValue(result) else: raise SynapseError(400, "Bad login type.") except KeyError: @@ -131,6 +138,26 @@ class LoginRestServlet(ClientV1RestServlet): defer.returnValue((200, result)) + @defer.inlineCallbacks + def do_token_login(self, login_submission): + token = login_submission['token'] + auth_handler = self.handlers.auth_handler + user_id = ( + yield auth_handler.validate_short_term_login_token_and_get_user_id(token) + ) + user_id, access_token, refresh_token = ( + yield auth_handler.login_with_user_id(user_id) + ) + result = { + "user_id": user_id, # may have changed + "access_token": access_token, + "refresh_token": refresh_token, + "home_server": self.hs.hostname, + } + + defer.returnValue((200, result)) + + # TODO Delete this after all CAS clients switch to token login instead @defer.inlineCallbacks def do_cas_login(self, cas_response_body): user, attributes = self.parse_cas_response(cas_response_body) @@ -152,7 +179,7 @@ class LoginRestServlet(ClientV1RestServlet): user_exists = yield auth_handler.does_user_exist(user_id) if user_exists: user_id, access_token, refresh_token = ( - yield auth_handler.login_with_cas_user_id(user_id) + yield auth_handler.login_with_user_id(user_id) ) result = { "user_id": user_id, # may have changed @@ -173,6 +200,7 @@ class LoginRestServlet(ClientV1RestServlet): defer.returnValue((200, result)) + # TODO Delete this after all CAS clients switch to token login instead def parse_cas_response(self, cas_response_body): root = ET.fromstring(cas_response_body) if not root.tag.endswith("serviceResponse"): @@ -243,6 +271,7 @@ class SAML2RestServlet(ClientV1RestServlet): defer.returnValue((200, {"status": "not_authenticated"})) +# TODO Delete this after all CAS clients switch to token login instead class CasRestServlet(ClientV1RestServlet): PATTERN = client_path_pattern("/login/cas") @@ -254,6 +283,118 @@ class CasRestServlet(ClientV1RestServlet): return (200, {"serverUrl": self.cas_server_url}) +class CasRedirectServlet(ClientV1RestServlet): + PATTERN = client_path_pattern("/login/cas/redirect") + + def __init__(self, hs): + super(CasRedirectServlet, self).__init__(hs) + self.cas_server_url = hs.config.cas_server_url + self.cas_service_url = hs.config.cas_service_url + + def on_GET(self, request): + args = request.args + if "redirectUrl" not in args: + return (400, "Redirect URL not specified for CAS auth") + clientRedirectUrlParam = urllib.urlencode({ + "redirectUrl": args["redirectUrl"][0] + }) + hsRedirectUrl = self.cas_service_url + "/_matrix/client/api/v1/login/cas/ticket" + serviceParam = urllib.urlencode({ + "service": "%s?%s" % (hsRedirectUrl, clientRedirectUrlParam) + }) + request.redirect("%s?%s" % (self.cas_server_url, serviceParam)) + request.finish() + defer.returnValue(None) + + +class CasTicketServlet(ClientV1RestServlet): + PATTERN = client_path_pattern("/login/cas/ticket") + + def __init__(self, hs): + super(CasTicketServlet, self).__init__(hs) + self.cas_server_url = hs.config.cas_server_url + self.cas_service_url = hs.config.cas_service_url + self.cas_required_attributes = hs.config.cas_required_attributes + + @defer.inlineCallbacks + def on_GET(self, request): + clientRedirectUrl = request.args["redirectUrl"][0] + # TODO: get this from the homeserver rather than creating a new one for + # each request + http_client = SimpleHttpClient(self.hs) + uri = self.cas_server_url + "/proxyValidate" + args = { + "ticket": request.args["ticket"], + "service": self.cas_service_url + } + body = yield http_client.get_raw(uri, args) + result = yield self.handle_cas_response(request, body, clientRedirectUrl) + defer.returnValue(result) + + @defer.inlineCallbacks + def handle_cas_response(self, request, cas_response_body, clientRedirectUrl): + user, attributes = self.parse_cas_response(cas_response_body) + + for required_attribute, required_value in self.cas_required_attributes.items(): + # If required attribute was not in CAS Response - Forbidden + if required_attribute not in attributes: + raise LoginError(401, "Unauthorized", errcode=Codes.UNAUTHORIZED) + + # Also need to check value + if required_value is not None: + actual_value = attributes[required_attribute] + # If required attribute value does not match expected - Forbidden + if required_value != actual_value: + raise LoginError(401, "Unauthorized", errcode=Codes.UNAUTHORIZED) + + user_id = UserID.create(user, self.hs.hostname).to_string() + auth_handler = self.handlers.auth_handler + user_exists = yield auth_handler.does_user_exist(user_id) + if not user_exists: + user_id, ignored = ( + yield self.handlers.registration_handler.register(localpart=user) + ) + + login_token = auth_handler.generate_short_term_login_token(user_id) + redirectUrl = self.add_login_token_to_redirect_url(clientRedirectUrl, login_token) + request.redirect(redirectUrl) + request.finish() + defer.returnValue(None) + + def add_login_token_to_redirect_url(self, url, token): + url_parts = list(urlparse.urlparse(url)) + query = dict(urlparse.parse_qsl(url_parts[4])) + query.update({"loginToken": token}) + url_parts[4] = urllib.urlencode(query) + return urlparse.urlunparse(url_parts) + + def parse_cas_response(self, cas_response_body): + root = ET.fromstring(cas_response_body) + if not root.tag.endswith("serviceResponse"): + raise LoginError(401, "Invalid CAS response", errcode=Codes.UNAUTHORIZED) + if not root[0].tag.endswith("authenticationSuccess"): + raise LoginError(401, "Unsuccessful CAS response", errcode=Codes.UNAUTHORIZED) + for child in root[0]: + if child.tag.endswith("user"): + user = child.text + if child.tag.endswith("attributes"): + attributes = {} + for attribute in child: + # ElementTree library expands the namespace in attribute tags + # to the full URL of the namespace. + # See (https://docs.python.org/2/library/xml.etree.elementtree.html) + # We don't care about namespace here and it will always be encased in + # curly braces, so we remove them. + if "}" in attribute.tag: + attributes[attribute.tag.split("}")[1]] = attribute.text + else: + attributes[attribute.tag] = attribute.text + if user is None or attributes is None: + raise LoginError(401, "Invalid CAS response", errcode=Codes.UNAUTHORIZED) + + return (user, attributes) + + def _parse_json(request): try: content = json.loads(request.content.read()) @@ -269,5 +410,7 @@ def register_servlets(hs, 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) CasRestServlet(hs).register(http_server) # TODO PasswordResetRestServlet(hs).register(http_server) From 0b31223c7a9a43ecb6fd80e3d1e5424ad8cdd835 Mon Sep 17 00:00:00 2001 From: Steven Hammerton Date: Thu, 5 Nov 2015 21:32:47 +0000 Subject: [PATCH 3/7] Updates to fallback CAS login to do new token login --- synapse/static/client/login/js/login.js | 38 ++++++++----------------- 1 file changed, 12 insertions(+), 26 deletions(-) diff --git a/synapse/static/client/login/js/login.js b/synapse/static/client/login/js/login.js index ab8b4d44e..bfb738603 100644 --- a/synapse/static/client/login/js/login.js +++ b/synapse/static/client/login/js/login.js @@ -17,12 +17,11 @@ var submitPassword = function(user, pwd) { }).error(errorFunc); }; -var submitCas = function(ticket, service) { - console.log("Logging in with cas..."); +var submitToken = function(loginToken) { + console.log("Logging in with login token..."); var data = { - type: "m.login.cas", - ticket: ticket, - service: service, + type: "m.login.token", + token: loginToken }; $.post(matrixLogin.endpoint, JSON.stringify(data), function(response) { show_login(); @@ -41,23 +40,10 @@ var errorFunc = function(err) { } }; -var getCasURL = function(cb) { - $.get(matrixLogin.endpoint + "/cas", function(response) { - var cas_url = response.serverUrl; - - cb(cas_url); - }).error(errorFunc); -}; - - var gotoCas = function() { - getCasURL(function(cas_url) { - var this_page = window.location.origin + window.location.pathname; - - var redirect_url = cas_url + "/login?service=" + encodeURIComponent(this_page); - - window.location.replace(redirect_url); - }); + var this_page = window.location.origin + window.location.pathname; + var redirect_url = matrixLogin.endpoint + "/cas/redirect?redirectUrl=" + encodeURIComponent(this_page); + window.location.replace(redirect_url); } var setFeedbackString = function(text) { @@ -111,7 +97,7 @@ var fetch_info = function(cb) { matrixLogin.onLoad = function() { fetch_info(function() { - if (!try_cas()) { + if (!try_token()) { show_login(); } }); @@ -148,20 +134,20 @@ var parseQsFromUrl = function(query) { return result; }; -var try_cas = function() { +var try_token = function() { var pos = window.location.href.indexOf("?"); if (pos == -1) { return false; } var qs = parseQsFromUrl(window.location.href.substr(pos+1)); - var ticket = qs.ticket; + var loginToken = qs.loginToken; - if (!ticket) { + if (!loginToken) { return false; } - submitCas(ticket, location.origin); + submitToken(loginToken); return true; }; From dd2eb49385f4b7d3bba94597a1fadb04bdeda0a4 Mon Sep 17 00:00:00 2001 From: Steven Hammerton Date: Wed, 11 Nov 2015 11:12:35 +0000 Subject: [PATCH 4/7] Share more code between macaroon validation --- synapse/api/auth.py | 19 +++++++------- synapse/handlers/auth.py | 55 +++++----------------------------------- 2 files changed, 17 insertions(+), 57 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 3e891a619..7fbbd8917 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -587,7 +587,10 @@ class Auth(object): def _get_user_from_macaroon(self, macaroon_str): try: macaroon = pymacaroons.Macaroon.deserialize(macaroon_str) - self._validate_macaroon(macaroon) + self.validate_macaroon( + macaroon, "access", + [lambda c: c == "guest = true", lambda c: c.startswith("time < ")] + ) user_prefix = "user_id = " user = None @@ -635,26 +638,24 @@ class Auth(object): errcode=Codes.UNKNOWN_TOKEN ) - def _validate_macaroon(self, macaroon): + def validate_macaroon(self, macaroon, type_string, additional_validation_functions): v = pymacaroons.Verifier() v.satisfy_exact("gen = 1") - v.satisfy_exact("type = access") + v.satisfy_exact("type = " + type_string) v.satisfy_general(lambda c: c.startswith("user_id = ")) - v.satisfy_general(self._verify_expiry) - v.satisfy_exact("guest = true") + + for validation_function in additional_validation_functions: + v.satisfy_general(validation_function) v.verify(macaroon, self.hs.config.macaroon_secret_key) v = pymacaroons.Verifier() v.satisfy_general(self._verify_recognizes_caveats) v.verify(macaroon, self.hs.config.macaroon_secret_key) - def _verify_expiry(self, caveat): + def verify_expiry(self, caveat): prefix = "time < " if not caveat.startswith(prefix): return False - # TODO(daniel): Enable expiry check when clients actually know how to - # refresh tokens. (And remember to enable the tests) - return True expiry = int(caveat[len(prefix):]) now = self.hs.get_clock().time_msec() return now < expiry diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 7a85883aa..01976a575 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -47,12 +47,6 @@ class AuthHandler(BaseHandler): self.bcrypt_rounds = hs.config.bcrypt_rounds self.sessions = {} self.INVALID_TOKEN_HTTP_STATUS = 401 - self._KNOWN_CAVEAT_PREFIXES = set([ - "gen = ", - "type = ", - "time < ", - "user_id = ", - ]) @defer.inlineCallbacks def check_auth(self, flows, clientdict, clientip): @@ -410,7 +404,13 @@ class AuthHandler(BaseHandler): return macaroon.serialize() def validate_short_term_login_token_and_get_user_id(self, login_token): - return self._validate_macaroon_and_get_user_id(login_token, "login", True) + try: + macaroon = pymacaroons.Macaroon.deserialize(login_token) + auth_api = self.hs.get_auth() + auth_api.validate_macaroon(macaroon, "login", [auth_api.verify_expiry]) + return self._get_user_from_macaroon(macaroon) + except (pymacaroons.exceptions.MacaroonException, TypeError, ValueError): + raise AuthError(401, "Invalid token", errcode=Codes.UNKNOWN_TOKEN) def _generate_base_macaroon(self, user_id): macaroon = pymacaroons.Macaroon( @@ -421,30 +421,6 @@ class AuthHandler(BaseHandler): macaroon.add_first_party_caveat("user_id = %s" % (user_id,)) return macaroon - def _validate_macaroon_and_get_user_id(self, macaroon_str, - macaroon_type, validate_expiry): - try: - macaroon = pymacaroons.Macaroon.deserialize(macaroon_str) - user_id = self._get_user_from_macaroon(macaroon) - v = pymacaroons.Verifier() - v.satisfy_exact("gen = 1") - v.satisfy_exact("type = " + macaroon_type) - v.satisfy_exact("user_id = " + user_id) - if validate_expiry: - v.satisfy_general(self._verify_expiry) - - v.verify(macaroon, self.hs.config.macaroon_secret_key) - - v = pymacaroons.Verifier() - v.satisfy_general(self._verify_recognizes_caveats) - v.verify(macaroon, self.hs.config.macaroon_secret_key) - return user_id - except (pymacaroons.exceptions.MacaroonException, TypeError, ValueError): - raise AuthError( - self.INVALID_TOKEN_HTTP_STATUS, "Invalid token", - errcode=Codes.UNKNOWN_TOKEN - ) - def _get_user_from_macaroon(self, macaroon): user_prefix = "user_id = " for caveat in macaroon.caveats: @@ -455,23 +431,6 @@ class AuthHandler(BaseHandler): errcode=Codes.UNKNOWN_TOKEN ) - def _verify_expiry(self, caveat): - prefix = "time < " - if not caveat.startswith(prefix): - return False - expiry = int(caveat[len(prefix):]) - now = self.hs.get_clock().time_msec() - return now < expiry - - def _verify_recognizes_caveats(self, caveat): - first_space = caveat.find(" ") - if first_space < 0: - return False - second_space = caveat.find(" ", first_space + 1) - if second_space < 0: - return False - return caveat[:second_space + 1] in self._KNOWN_CAVEAT_PREFIXES - @defer.inlineCallbacks def set_password(self, user_id, newpassword): password_hash = self.hash(newpassword) From 2b779af10fe5c39f6119acddb5290be2b2a5930f Mon Sep 17 00:00:00 2001 From: Steven Hammerton Date: Wed, 11 Nov 2015 11:20:23 +0000 Subject: [PATCH 5/7] Minor review fixes --- synapse/handlers/auth.py | 8 ++++---- synapse/rest/client/v1/login.py | 23 ++++++++++------------- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 01976a575..be157e2bb 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -298,11 +298,11 @@ class AuthHandler(BaseHandler): defer.returnValue((user_id, access_token, refresh_token)) @defer.inlineCallbacks - def login_with_user_id(self, user_id): + def get_login_tuple_for_user_id(self, user_id): """ - Authenticates the user with the given user ID, - it is intended that the authentication of the user has - already been verified by other mechanism (e.g. CAS) + Gets login tuple for the user with the given user ID. + The user is assumed to have been authenticated by some other + machanism (e.g. CAS) Args: user_id (str): User ID diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index 5a2cedacb..78c542a94 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -146,7 +146,7 @@ class LoginRestServlet(ClientV1RestServlet): yield auth_handler.validate_short_term_login_token_and_get_user_id(token) ) user_id, access_token, refresh_token = ( - yield auth_handler.login_with_user_id(user_id) + yield auth_handler.get_login_tuple_for_user_id(user_id) ) result = { "user_id": user_id, # may have changed @@ -179,7 +179,7 @@ class LoginRestServlet(ClientV1RestServlet): user_exists = yield auth_handler.does_user_exist(user_id) if user_exists: user_id, access_token, refresh_token = ( - yield auth_handler.login_with_user_id(user_id) + yield auth_handler.get_login_tuple_for_user_id(user_id) ) result = { "user_id": user_id, # may have changed @@ -304,7 +304,6 @@ class CasRedirectServlet(ClientV1RestServlet): }) request.redirect("%s?%s" % (self.cas_server_url, serviceParam)) request.finish() - defer.returnValue(None) class CasTicketServlet(ClientV1RestServlet): @@ -318,21 +317,19 @@ class CasTicketServlet(ClientV1RestServlet): @defer.inlineCallbacks def on_GET(self, request): - clientRedirectUrl = request.args["redirectUrl"][0] - # TODO: get this from the homeserver rather than creating a new one for - # each request - http_client = SimpleHttpClient(self.hs) + client_redirect_url = request.args["redirectUrl"][0] + http_client = self.hs.get_simple_http_client() uri = self.cas_server_url + "/proxyValidate" args = { "ticket": request.args["ticket"], "service": self.cas_service_url } body = yield http_client.get_raw(uri, args) - result = yield self.handle_cas_response(request, body, clientRedirectUrl) + result = yield self.handle_cas_response(request, body, client_redirect_url) defer.returnValue(result) @defer.inlineCallbacks - def handle_cas_response(self, request, cas_response_body, clientRedirectUrl): + def handle_cas_response(self, request, cas_response_body, client_redirect_url): user, attributes = self.parse_cas_response(cas_response_body) for required_attribute, required_value in self.cas_required_attributes.items(): @@ -351,15 +348,15 @@ class CasTicketServlet(ClientV1RestServlet): auth_handler = self.handlers.auth_handler user_exists = yield auth_handler.does_user_exist(user_id) if not user_exists: - user_id, ignored = ( + user_id, _ = ( yield self.handlers.registration_handler.register(localpart=user) ) login_token = auth_handler.generate_short_term_login_token(user_id) - redirectUrl = self.add_login_token_to_redirect_url(clientRedirectUrl, login_token) - request.redirect(redirectUrl) + redirect_url = self.add_login_token_to_redirect_url(client_redirect_url, + login_token) + request.redirect(redirect_url) request.finish() - defer.returnValue(None) def add_login_token_to_redirect_url(self, url, token): url_parts = list(urlparse.urlparse(url)) From ffdc8e5e1ccc4aab4b11f7da06c8695ba6b20111 Mon Sep 17 00:00:00 2001 From: Steven Hammerton Date: Wed, 11 Nov 2015 14:26:47 +0000 Subject: [PATCH 6/7] Snakes not camels --- synapse/rest/client/v1/login.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index 78c542a94..0171f6c01 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -295,14 +295,14 @@ class CasRedirectServlet(ClientV1RestServlet): args = request.args if "redirectUrl" not in args: return (400, "Redirect URL not specified for CAS auth") - clientRedirectUrlParam = urllib.urlencode({ + client_redirect_url_param = urllib.urlencode({ "redirectUrl": args["redirectUrl"][0] }) - hsRedirectUrl = self.cas_service_url + "/_matrix/client/api/v1/login/cas/ticket" - serviceParam = urllib.urlencode({ - "service": "%s?%s" % (hsRedirectUrl, clientRedirectUrlParam) + hs_redirect_url = self.cas_service_url + "/_matrix/client/api/v1/login/cas/ticket" + service_param = urllib.urlencode({ + "service": "%s?%s" % (hs_redirect_url, client_redirect_url_param) }) - request.redirect("%s?%s" % (self.cas_server_url, serviceParam)) + request.redirect("%s?%s" % (self.cas_server_url, service_param)) request.finish() From f20d064e05b1641162f36303139a611a97b6890e Mon Sep 17 00:00:00 2001 From: Steven Hammerton Date: Tue, 17 Nov 2015 10:58:05 +0000 Subject: [PATCH 7/7] Always check guest = true in macaroons --- synapse/api/auth.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 7fbbd8917..8111b3442 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -589,7 +589,7 @@ class Auth(object): macaroon = pymacaroons.Macaroon.deserialize(macaroon_str) self.validate_macaroon( macaroon, "access", - [lambda c: c == "guest = true", lambda c: c.startswith("time < ")] + [lambda c: c.startswith("time < ")] ) user_prefix = "user_id = " @@ -643,6 +643,7 @@ class Auth(object): v.satisfy_exact("gen = 1") v.satisfy_exact("type = " + type_string) v.satisfy_general(lambda c: c.startswith("user_id = ")) + v.satisfy_exact("guest = true") for validation_function in additional_validation_functions: v.satisfy_general(validation_function)