Improve SAML error messages (#8248)

This commit is contained in:
Patrick Cloke 2020-09-14 09:05:36 -04:00 committed by GitHub
parent 04cc249b43
commit 6605470bfb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 178 additions and 185 deletions

View file

@ -114,6 +114,20 @@ request to
with the query parameters from the original link, presented as a URL-encoded form. See the file with the query parameters from the original link, presented as a URL-encoded form. See the file
itself for more details. itself for more details.
Updated Single Sign-on HTML Templates
-------------------------------------
The ``saml_error.html`` template was removed from Synapse and replaced with the
``sso_error.html`` template. If your Synapse is configured to use SAML and a
custom ``sso_redirect_confirm_template_dir`` configuration then any customisations
of the ``saml_error.html`` template will need to be merged into the ``sso_error.html``
template. These templates are similar, but the parameters are slightly different:
* The ``msg`` parameter should be renamed to ``error_description``.
* There is no longer a ``code`` parameter for the response code.
* A string ``error`` parameter is available that includes a short hint of why a
user is seeing the error page.
Upgrading to v1.18.0 Upgrading to v1.18.0
==================== ====================

1
changelog.d/8248.feature Normal file
View file

@ -0,0 +1 @@
Consolidate the SSO error template across all configuration.

View file

@ -1485,11 +1485,14 @@ trusted_key_servers:
# At least one of `sp_config` or `config_path` must be set in this section to # At least one of `sp_config` or `config_path` must be set in this section to
# enable SAML login. # enable SAML login.
# #
# (You will probably also want to set the following options to `false` to # You will probably also want to set the following options to `false` to
# disable the regular login/registration flows: # disable the regular login/registration flows:
# * enable_registration # * enable_registration
# * password_config.enabled # * password_config.enabled
# #
# You will also want to investigate the settings under the "sso" configuration
# section below.
#
# Once SAML support is enabled, a metadata file will be exposed at # Once SAML support is enabled, a metadata file will be exposed at
# https://<server>:<port>/_matrix/saml2/metadata.xml, which you may be able to # https://<server>:<port>/_matrix/saml2/metadata.xml, which you may be able to
# use to configure your SAML IdP with. Alternatively, you can manually configure # use to configure your SAML IdP with. Alternatively, you can manually configure
@ -1612,31 +1615,6 @@ saml2_config:
# - attribute: department # - attribute: department
# value: "sales" # value: "sales"
# Directory in which Synapse will try to find the template files below.
# If not set, default templates from within the Synapse package will be used.
#
# DO NOT UNCOMMENT THIS SETTING unless you want to customise the templates.
# If you *do* uncomment it, you will need to make sure that all the templates
# below are in the directory.
#
# Synapse will look for the following templates in this directory:
#
# * HTML page to display to users if something goes wrong during the
# authentication process: 'saml_error.html'.
#
# When rendering, this template is given the following variables:
# * code: an HTML error code corresponding to the error that is being
# returned (typically 400 or 500)
#
# * msg: a textual message describing the error.
#
# The variables will automatically be HTML-escaped.
#
# You can see the default templates at:
# https://github.com/matrix-org/synapse/tree/master/synapse/res/templates
#
#template_dir: "res/templates"
# OpenID Connect integration. The following settings can be used to make Synapse # OpenID Connect integration. The following settings can be used to make Synapse
# use an OpenID Connect Provider for authentication, instead of its internal # use an OpenID Connect Provider for authentication, instead of its internal

View file

@ -169,10 +169,6 @@ class SAML2Config(Config):
saml2_config.get("saml_session_lifetime", "15m") saml2_config.get("saml_session_lifetime", "15m")
) )
self.saml2_error_html_template = self.read_templates(
["saml_error.html"], saml2_config.get("template_dir")
)[0]
def _default_saml_config_dict( def _default_saml_config_dict(
self, required_attributes: set, optional_attributes: set self, required_attributes: set, optional_attributes: set
): ):
@ -225,11 +221,14 @@ class SAML2Config(Config):
# At least one of `sp_config` or `config_path` must be set in this section to # At least one of `sp_config` or `config_path` must be set in this section to
# enable SAML login. # enable SAML login.
# #
# (You will probably also want to set the following options to `false` to # You will probably also want to set the following options to `false` to
# disable the regular login/registration flows: # disable the regular login/registration flows:
# * enable_registration # * enable_registration
# * password_config.enabled # * password_config.enabled
# #
# You will also want to investigate the settings under the "sso" configuration
# section below.
#
# Once SAML support is enabled, a metadata file will be exposed at # Once SAML support is enabled, a metadata file will be exposed at
# https://<server>:<port>/_matrix/saml2/metadata.xml, which you may be able to # https://<server>:<port>/_matrix/saml2/metadata.xml, which you may be able to
# use to configure your SAML IdP with. Alternatively, you can manually configure # use to configure your SAML IdP with. Alternatively, you can manually configure
@ -351,31 +350,6 @@ class SAML2Config(Config):
# value: "staff" # value: "staff"
# - attribute: department # - attribute: department
# value: "sales" # value: "sales"
# Directory in which Synapse will try to find the template files below.
# If not set, default templates from within the Synapse package will be used.
#
# DO NOT UNCOMMENT THIS SETTING unless you want to customise the templates.
# If you *do* uncomment it, you will need to make sure that all the templates
# below are in the directory.
#
# Synapse will look for the following templates in this directory:
#
# * HTML page to display to users if something goes wrong during the
# authentication process: 'saml_error.html'.
#
# When rendering, this template is given the following variables:
# * code: an HTML error code corresponding to the error that is being
# returned (typically 400 or 500)
#
# * msg: a textual message describing the error.
#
# The variables will automatically be HTML-escaped.
#
# You can see the default templates at:
# https://github.com/matrix-org/synapse/tree/master/synapse/res/templates
#
#template_dir: "res/templates"
""" % { """ % {
"config_dir_path": config_dir_path "config_dir_path": config_dir_path
} }

View file

@ -131,10 +131,10 @@ class OidcHandler:
def _render_error( def _render_error(
self, request, error: str, error_description: Optional[str] = None self, request, error: str, error_description: Optional[str] = None
) -> None: ) -> None:
"""Renders the error template and respond with it. """Render the error template and respond to the request with it.
This is used to show errors to the user. The template of this page can This is used to show errors to the user. The template of this page can
be found under ``synapse/res/templates/sso_error.html``. be found under `synapse/res/templates/sso_error.html`.
Args: Args:
request: The incoming request from the browser. request: The incoming request from the browser.

View file

@ -21,9 +21,10 @@ import saml2
import saml2.response import saml2.response
from saml2.client import Saml2Client from saml2.client import Saml2Client
from synapse.api.errors import AuthError, SynapseError from synapse.api.errors import SynapseError
from synapse.config import ConfigError from synapse.config import ConfigError
from synapse.config.saml2_config import SamlAttributeRequirement from synapse.config.saml2_config import SamlAttributeRequirement
from synapse.http.server import respond_with_html
from synapse.http.servlet import parse_string from synapse.http.servlet import parse_string
from synapse.http.site import SynapseRequest from synapse.http.site import SynapseRequest
from synapse.module_api import ModuleApi from synapse.module_api import ModuleApi
@ -41,6 +42,10 @@ if TYPE_CHECKING:
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
class MappingException(Exception):
"""Used to catch errors when mapping the SAML2 response to a user."""
@attr.s @attr.s
class Saml2SessionData: class Saml2SessionData:
"""Data we track about SAML2 sessions""" """Data we track about SAML2 sessions"""
@ -68,6 +73,7 @@ class SamlHandler:
hs.config.saml2_grandfathered_mxid_source_attribute hs.config.saml2_grandfathered_mxid_source_attribute
) )
self._saml2_attribute_requirements = hs.config.saml2.attribute_requirements self._saml2_attribute_requirements = hs.config.saml2.attribute_requirements
self._error_template = hs.config.sso_error_template
# plugin to do custom mapping from saml response to mxid # plugin to do custom mapping from saml response to mxid
self._user_mapping_provider = hs.config.saml2_user_mapping_provider_class( self._user_mapping_provider = hs.config.saml2_user_mapping_provider_class(
@ -84,6 +90,25 @@ class SamlHandler:
# a lock on the mappings # a lock on the mappings
self._mapping_lock = Linearizer(name="saml_mapping", clock=self._clock) self._mapping_lock = Linearizer(name="saml_mapping", clock=self._clock)
def _render_error(
self, request, error: str, error_description: Optional[str] = None
) -> None:
"""Render the error template and respond to the request with it.
This is used to show errors to the user. The template of this page can
be found under `synapse/res/templates/sso_error.html`.
Args:
request: The incoming request from the browser.
We'll respond with an HTML page describing the error.
error: A technical identifier for this error.
error_description: A human-readable description of the error.
"""
html = self._error_template.render(
error=error, error_description=error_description
)
respond_with_html(request, 400, html)
def handle_redirect_request( def handle_redirect_request(
self, client_redirect_url: bytes, ui_auth_session_id: Optional[str] = None self, client_redirect_url: bytes, ui_auth_session_id: Optional[str] = None
) -> bytes: ) -> bytes:
@ -134,49 +159,6 @@ class SamlHandler:
# the dict. # the dict.
self.expire_sessions() self.expire_sessions()
# Pull out the user-agent and IP from the request.
user_agent = request.requestHeaders.getRawHeaders(b"User-Agent", default=[b""])[
0
].decode("ascii", "surrogateescape")
ip_address = self.hs.get_ip_from_request(request)
user_id, current_session = await self._map_saml_response_to_user(
resp_bytes, relay_state, user_agent, ip_address
)
# Complete the interactive auth session or the login.
if current_session and current_session.ui_auth_session_id:
await self._auth_handler.complete_sso_ui_auth(
user_id, current_session.ui_auth_session_id, request
)
else:
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,
user_agent: str,
ip_address: str,
) -> Tuple[str, Optional[Saml2SessionData]]:
"""
Given a sample response, retrieve the cached session and user for it.
Args:
resp_bytes: The SAML response.
client_redirect_url: The redirect URL passed in by the client.
user_agent: The user agent of the client making the request.
ip_address: The IP address of the client making the request.
Returns:
Tuple of the user ID and SAML session associated with this response.
Raises:
SynapseError if there was a problem with the response.
RedirectException: some mapping providers may raise this if they need
to redirect to an interstitial page.
"""
try: try:
saml2_auth = self._saml_client.parse_authn_request_response( saml2_auth = self._saml_client.parse_authn_request_response(
resp_bytes, resp_bytes,
@ -189,12 +171,23 @@ class SamlHandler:
# in the (user-visible) exception message, so let's log the exception here # in the (user-visible) exception message, so let's log the exception here
# so we can track down the session IDs later. # so we can track down the session IDs later.
logger.warning(str(e)) logger.warning(str(e))
raise SynapseError(400, "Unexpected SAML2 login.") self._render_error(
request, "unsolicited_response", "Unexpected SAML2 login."
)
return
except Exception as e: except Exception as e:
raise SynapseError(400, "Unable to parse SAML2 response: %s." % (e,)) self._render_error(
request,
"invalid_response",
"Unable to parse SAML2 response: %s." % (e,),
)
return
if saml2_auth.not_signed: if saml2_auth.not_signed:
raise SynapseError(400, "SAML2 response was not signed.") self._render_error(
request, "unsigned_respond", "SAML2 response was not signed."
)
return
logger.debug("SAML2 response: %s", saml2_auth.origxml) logger.debug("SAML2 response: %s", saml2_auth.origxml)
for assertion in saml2_auth.assertions: for assertion in saml2_auth.assertions:
@ -213,15 +206,73 @@ class SamlHandler:
saml2_auth.in_response_to, None saml2_auth.in_response_to, None
) )
# Ensure that the attributes of the logged in user meet the required
# attributes.
for requirement in self._saml2_attribute_requirements: for requirement in self._saml2_attribute_requirements:
_check_attribute_requirement(saml2_auth.ava, requirement) if not _check_attribute_requirement(saml2_auth.ava, requirement):
self._render_error(
request, "unauthorised", "You are not authorised to log in here."
)
return
# Pull out the user-agent and IP from the request.
user_agent = request.requestHeaders.getRawHeaders(b"User-Agent", default=[b""])[
0
].decode("ascii", "surrogateescape")
ip_address = self.hs.get_ip_from_request(request)
# Call the mapper to register/login the user
try:
user_id = await self._map_saml_response_to_user(
saml2_auth, relay_state, user_agent, ip_address
)
except MappingException as e:
logger.exception("Could not map user")
self._render_error(request, "mapping_error", str(e))
return
# Complete the interactive auth session or the login.
if current_session and current_session.ui_auth_session_id:
await self._auth_handler.complete_sso_ui_auth(
user_id, current_session.ui_auth_session_id, request
)
else:
await self._auth_handler.complete_sso_login(user_id, request, relay_state)
async def _map_saml_response_to_user(
self,
saml2_auth: saml2.response.AuthnResponse,
client_redirect_url: str,
user_agent: str,
ip_address: str,
) -> str:
"""
Given a SAML response, retrieve the user ID for it and possibly register the user.
Args:
saml2_auth: The parsed SAML2 response.
client_redirect_url: The redirect URL passed in by the client.
user_agent: The user agent of the client making the request.
ip_address: The IP address of the client making the request.
Returns:
The user ID associated with this response.
Raises:
MappingException if there was a problem mapping the response to a user.
RedirectException: some mapping providers may raise this if they need
to redirect to an interstitial page.
"""
remote_user_id = self._user_mapping_provider.get_remote_user_id( remote_user_id = self._user_mapping_provider.get_remote_user_id(
saml2_auth, client_redirect_url saml2_auth, client_redirect_url
) )
if not remote_user_id: if not remote_user_id:
raise Exception("Failed to extract remote user id from SAML response") raise MappingException(
"Failed to extract remote user id from SAML response"
)
with (await self._mapping_lock.queue(self._auth_provider_id)): with (await self._mapping_lock.queue(self._auth_provider_id)):
# first of all, check if we already have a mapping for this user # first of all, check if we already have a mapping for this user
@ -235,7 +286,7 @@ class SamlHandler:
) )
if registered_user_id is not None: if registered_user_id is not None:
logger.info("Found existing mapping %s", registered_user_id) logger.info("Found existing mapping %s", registered_user_id)
return registered_user_id, current_session return registered_user_id
# backwards-compatibility hack: see if there is an existing user with a # backwards-compatibility hack: see if there is an existing user with a
# suitable mapping from the uid # suitable mapping from the uid
@ -260,7 +311,7 @@ class SamlHandler:
await self._datastore.record_user_external_id( await self._datastore.record_user_external_id(
self._auth_provider_id, remote_user_id, registered_user_id self._auth_provider_id, remote_user_id, registered_user_id
) )
return registered_user_id, current_session return registered_user_id
# Map saml response to user attributes using the configured mapping provider # Map saml response to user attributes using the configured mapping provider
for i in range(1000): for i in range(1000):
@ -277,7 +328,7 @@ class SamlHandler:
localpart = attribute_dict.get("mxid_localpart") localpart = attribute_dict.get("mxid_localpart")
if not localpart: if not localpart:
raise Exception( raise MappingException(
"Error parsing SAML2 response: SAML mapping provider plugin " "Error parsing SAML2 response: SAML mapping provider plugin "
"did not return a mxid_localpart value" "did not return a mxid_localpart value"
) )
@ -294,8 +345,8 @@ class SamlHandler:
else: else:
# Unable to generate a username in 1000 iterations # Unable to generate a username in 1000 iterations
# Break and return error to the user # Break and return error to the user
raise SynapseError( raise MappingException(
500, "Unable to generate a Matrix ID from the SAML response" "Unable to generate a Matrix ID from the SAML response"
) )
logger.info("Mapped SAML user to local part %s", localpart) logger.info("Mapped SAML user to local part %s", localpart)
@ -310,7 +361,7 @@ class SamlHandler:
await self._datastore.record_user_external_id( await self._datastore.record_user_external_id(
self._auth_provider_id, remote_user_id, registered_user_id self._auth_provider_id, remote_user_id, registered_user_id
) )
return registered_user_id, current_session return registered_user_id
def expire_sessions(self): def expire_sessions(self):
expire_before = self._clock.time_msec() - self._saml2_session_lifetime expire_before = self._clock.time_msec() - self._saml2_session_lifetime
@ -323,11 +374,11 @@ class SamlHandler:
del self._outstanding_requests_dict[reqid] del self._outstanding_requests_dict[reqid]
def _check_attribute_requirement(ava: dict, req: SamlAttributeRequirement): def _check_attribute_requirement(ava: dict, req: SamlAttributeRequirement) -> bool:
values = ava.get(req.attribute, []) values = ava.get(req.attribute, [])
for v in values: for v in values:
if v == req.value: if v == req.value:
return return True
logger.info( logger.info(
"SAML2 attribute %s did not match required value '%s' (was '%s')", "SAML2 attribute %s did not match required value '%s' (was '%s')",
@ -335,7 +386,7 @@ def _check_attribute_requirement(ava: dict, req: SamlAttributeRequirement):
req.value, req.value,
values, values,
) )
raise AuthError(403, "You are not authorized to log in here.") return False
DOT_REPLACE_PATTERN = re.compile( DOT_REPLACE_PATTERN = re.compile(
@ -390,7 +441,7 @@ class DefaultSamlMappingProvider:
return saml_response.ava["uid"][0] return saml_response.ava["uid"][0]
except KeyError: except KeyError:
logger.warning("SAML2 response lacks a 'uid' attestation") logger.warning("SAML2 response lacks a 'uid' attestation")
raise SynapseError(400, "'uid' not in SAML2 response") raise MappingException("'uid' not in SAML2 response")
def saml_response_to_user_attributes( def saml_response_to_user_attributes(
self, self,

View file

@ -1,52 +0,0 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title>SSO login error</title>
</head>
<body>
{# a 403 means we have actively rejected their login #}
{% if code == 403 %}
<p>You are not allowed to log in here.</p>
{% else %}
<p>
There was an error during authentication:
</p>
<div id="errormsg" style="margin:20px 80px">{{ msg }}</div>
<p>
If you are seeing this page after clicking a link sent to you via email, make
sure you only click the confirmation link once, and that you open the
validation link in the same client you're logging in from.
</p>
<p>
Try logging in again from your Matrix client and if the problem persists
please contact the server's administrator.
</p>
<script type="text/javascript">
// Error handling to support Auth0 errors that we might get through a GET request
// to the validation endpoint. If an error is provided, it's either going to be
// located in the query string or in a query string-like URI fragment.
// We try to locate the error from any of these two locations, but if we can't
// we just don't print anything specific.
let searchStr = "";
if (window.location.search) {
// window.location.searchParams isn't always defined when
// window.location.search is, so it's more reliable to parse the latter.
searchStr = window.location.search;
} else if (window.location.hash) {
// Replace the # with a ? so that URLSearchParams does the right thing and
// doesn't parse the first parameter incorrectly.
searchStr = window.location.hash.replace("#", "?");
}
// We might end up with no error in the URL, so we need to check if we have one
// to print one.
let errorDesc = new URLSearchParams(searchStr).get("error_description")
if (errorDesc) {
document.getElementById("errormsg").innerText = errorDesc;
}
</script>
{% endif %}
</body>
</html>

View file

@ -5,14 +5,49 @@
<title>SSO error</title> <title>SSO error</title>
</head> </head>
<body> <body>
<p>Oops! Something went wrong during authentication.</p> {# If an error of unauthorised is returned it means we have actively rejected their login #}
{% if error == "unauthorised" %}
<p>You are not allowed to log in here.</p>
{% else %}
<p>
There was an error during authentication:
</p>
<div id="errormsg" style="margin:20px 80px">{{ error_description }}</div>
<p>
If you are seeing this page after clicking a link sent to you via email, make
sure you only click the confirmation link once, and that you open the
validation link in the same client you're logging in from.
</p>
<p> <p>
Try logging in again from your Matrix client and if the problem persists Try logging in again from your Matrix client and if the problem persists
please contact the server's administrator. please contact the server's administrator.
</p> </p>
<p>Error: <code>{{ error }}</code></p> <p>Error: <code>{{ error }}</code></p>
{% if error_description %}
<pre><code>{{ error_description }}</code></pre> <script type="text/javascript">
{% endif %} // Error handling to support Auth0 errors that we might get through a GET request
// to the validation endpoint. If an error is provided, it's either going to be
// located in the query string or in a query string-like URI fragment.
// We try to locate the error from any of these two locations, but if we can't
// we just don't print anything specific.
let searchStr = "";
if (window.location.search) {
// window.location.searchParams isn't always defined when
// window.location.search is, so it's more reliable to parse the latter.
searchStr = window.location.search;
} else if (window.location.hash) {
// Replace the # with a ? so that URLSearchParams does the right thing and
// doesn't parse the first parameter incorrectly.
searchStr = window.location.hash.replace("#", "?");
}
// We might end up with no error in the URL, so we need to check if we have one
// to print one.
let errorDesc = new URLSearchParams(searchStr).get("error_description")
if (errorDesc) {
document.getElementById("errormsg").innerText = errorDesc;
}
</script>
{% endif %}
</body> </body>
</html> </html>

View file

@ -13,10 +13,8 @@
# 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 twisted.python import failure
from synapse.api.errors import SynapseError from synapse.http.server import DirectServeHtmlResource
from synapse.http.server import DirectServeHtmlResource, return_html_error
class SAML2ResponseResource(DirectServeHtmlResource): class SAML2ResponseResource(DirectServeHtmlResource):
@ -27,21 +25,15 @@ class SAML2ResponseResource(DirectServeHtmlResource):
def __init__(self, hs): def __init__(self, hs):
super().__init__() super().__init__()
self._saml_handler = hs.get_saml_handler() self._saml_handler = hs.get_saml_handler()
self._error_html_template = hs.config.saml2.saml2_error_html_template
async def _async_render_GET(self, request): async def _async_render_GET(self, request):
# We're not expecting any GET request on that resource if everything goes right, # We're not expecting any GET request on that resource if everything goes right,
# but some IdPs sometimes end up responding with a 302 redirect on this endpoint. # but some IdPs sometimes end up responding with a 302 redirect on this endpoint.
# In this case, just tell the user that something went wrong and they should # In this case, just tell the user that something went wrong and they should
# try to authenticate again. # try to authenticate again.
f = failure.Failure( self._saml_handler._render_error(
SynapseError(400, "Unexpected GET request on /saml2/authn_response") request, "unexpected_get", "Unexpected GET request on /saml2/authn_response"
) )
return_html_error(f, request, self._error_html_template)
async def _async_render_POST(self, request): async def _async_render_POST(self, request):
try: await self._saml_handler.handle_saml_response(request)
await self._saml_handler.handle_saml_response(request)
except Exception:
f = failure.Failure()
return_html_error(f, request, self._error_html_template)