Merge pull request #7058 from matrix-org/babolivier/saml_error_html
SAML2: render a comprehensible error page if something goes wrong
This commit is contained in:
commit
6640460d05
|
@ -0,0 +1 @@
|
||||||
|
Render a configurable and comprehensible error page if something goes wrong during the SAML2 authentication process.
|
|
@ -1347,6 +1347,13 @@ saml2_config:
|
||||||
#
|
#
|
||||||
#grandfathered_mxid_source_attribute: upn
|
#grandfathered_mxid_source_attribute: upn
|
||||||
|
|
||||||
|
# Path to a file containing HTML content to serve in case an error happens
|
||||||
|
# when the user gets redirected from the SAML IdP back to Synapse.
|
||||||
|
# If no file is provided, this defaults to some minimalistic HTML telling the
|
||||||
|
# user that something went wrong and they should try authenticating again.
|
||||||
|
#
|
||||||
|
#error_html_path: /path/to/static/content/saml_error.html
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
# Enable CAS for registration and login.
|
# Enable CAS for registration and login.
|
||||||
|
|
|
@ -27,6 +27,18 @@ DEFAULT_USER_MAPPING_PROVIDER = (
|
||||||
"synapse.handlers.saml_handler.DefaultSamlMappingProvider"
|
"synapse.handlers.saml_handler.DefaultSamlMappingProvider"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
SAML2_ERROR_DEFAULT_HTML = """
|
||||||
|
<html>
|
||||||
|
<body>
|
||||||
|
<p>Oops! Something went wrong</p>
|
||||||
|
<p>
|
||||||
|
Try logging in again from your Matrix client and if the problem persists
|
||||||
|
please contact the server's administrator.
|
||||||
|
</p>
|
||||||
|
</body>
|
||||||
|
</html>
|
||||||
|
"""
|
||||||
|
|
||||||
|
|
||||||
def _dict_merge(merge_dict, into_dict):
|
def _dict_merge(merge_dict, into_dict):
|
||||||
"""Do a deep merge of two dicts
|
"""Do a deep merge of two dicts
|
||||||
|
@ -160,6 +172,13 @@ class SAML2Config(Config):
|
||||||
saml2_config.get("saml_session_lifetime", "5m")
|
saml2_config.get("saml_session_lifetime", "5m")
|
||||||
)
|
)
|
||||||
|
|
||||||
|
if "error_html_path" in config:
|
||||||
|
self.saml2_error_html_content = self.read_file(
|
||||||
|
config["error_html_path"], "saml2_config.error_html_path",
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
self.saml2_error_html_content = SAML2_ERROR_DEFAULT_HTML
|
||||||
|
|
||||||
def _default_saml_config_dict(
|
def _default_saml_config_dict(
|
||||||
self, required_attributes: set, optional_attributes: set
|
self, required_attributes: set, optional_attributes: set
|
||||||
):
|
):
|
||||||
|
@ -325,6 +344,13 @@ class SAML2Config(Config):
|
||||||
# The default is 'uid'.
|
# The default is 'uid'.
|
||||||
#
|
#
|
||||||
#grandfathered_mxid_source_attribute: upn
|
#grandfathered_mxid_source_attribute: upn
|
||||||
|
|
||||||
|
# Path to a file containing HTML content to serve in case an error happens
|
||||||
|
# when the user gets redirected from the SAML IdP back to Synapse.
|
||||||
|
# If no file is provided, this defaults to some minimalistic HTML telling the
|
||||||
|
# user that something went wrong and they should try authenticating again.
|
||||||
|
#
|
||||||
|
#error_html_path: /path/to/static/content/saml_error.html
|
||||||
""" % {
|
""" % {
|
||||||
"config_dir_path": config_dir_path
|
"config_dir_path": config_dir_path
|
||||||
}
|
}
|
||||||
|
|
|
@ -23,6 +23,7 @@ from saml2.client import Saml2Client
|
||||||
|
|
||||||
from synapse.api.errors import SynapseError
|
from synapse.api.errors import SynapseError
|
||||||
from synapse.config import ConfigError
|
from synapse.config import ConfigError
|
||||||
|
from synapse.http.server import finish_request
|
||||||
from synapse.http.servlet import parse_string
|
from synapse.http.servlet import parse_string
|
||||||
from synapse.module_api import ModuleApi
|
from synapse.module_api import ModuleApi
|
||||||
from synapse.types import (
|
from synapse.types import (
|
||||||
|
@ -73,6 +74,8 @@ 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)
|
||||||
|
|
||||||
|
self._error_html_content = hs.config.saml2_error_html_content
|
||||||
|
|
||||||
def handle_redirect_request(self, client_redirect_url):
|
def handle_redirect_request(self, client_redirect_url):
|
||||||
"""Handle an incoming request to /login/sso/redirect
|
"""Handle an incoming request to /login/sso/redirect
|
||||||
|
|
||||||
|
@ -114,7 +117,22 @@ class SamlHandler:
|
||||||
# the dict.
|
# the dict.
|
||||||
self.expire_sessions()
|
self.expire_sessions()
|
||||||
|
|
||||||
user_id = await self._map_saml_response_to_user(resp_bytes, relay_state)
|
try:
|
||||||
|
user_id = await self._map_saml_response_to_user(resp_bytes, relay_state)
|
||||||
|
except Exception as e:
|
||||||
|
# If decoding the response or mapping it to a user failed, then log the
|
||||||
|
# error and tell the user that something went wrong.
|
||||||
|
logger.error(e)
|
||||||
|
|
||||||
|
request.setResponseCode(400)
|
||||||
|
request.setHeader(b"Content-Type", b"text/html; charset=utf-8")
|
||||||
|
request.setHeader(
|
||||||
|
b"Content-Length", b"%d" % (len(self._error_html_content),)
|
||||||
|
)
|
||||||
|
request.write(self._error_html_content.encode("utf8"))
|
||||||
|
finish_request(request)
|
||||||
|
return
|
||||||
|
|
||||||
self._auth_handler.complete_sso_login(user_id, request, relay_state)
|
self._auth_handler.complete_sso_login(user_id, request, relay_state)
|
||||||
|
|
||||||
async def _map_saml_response_to_user(self, resp_bytes, client_redirect_url):
|
async def _map_saml_response_to_user(self, resp_bytes, client_redirect_url):
|
||||||
|
|
|
@ -210,7 +210,7 @@ class LoggingContext(object):
|
||||||
class Sentinel(object):
|
class Sentinel(object):
|
||||||
"""Sentinel to represent the root context"""
|
"""Sentinel to represent the root context"""
|
||||||
|
|
||||||
__slots__ = ["previous_context", "alive", "request", "scope"]
|
__slots__ = ["previous_context", "alive", "request", "scope", "tag"]
|
||||||
|
|
||||||
def __init__(self) -> None:
|
def __init__(self) -> None:
|
||||||
# Minimal set for compatibility with LoggingContext
|
# Minimal set for compatibility with LoggingContext
|
||||||
|
@ -218,6 +218,7 @@ class LoggingContext(object):
|
||||||
self.alive = None
|
self.alive = None
|
||||||
self.request = None
|
self.request = None
|
||||||
self.scope = None
|
self.scope = None
|
||||||
|
self.tag = None
|
||||||
|
|
||||||
def __str__(self):
|
def __str__(self):
|
||||||
return "sentinel"
|
return "sentinel"
|
||||||
|
@ -511,7 +512,7 @@ class PreserveLoggingContext(object):
|
||||||
|
|
||||||
__slots__ = ["current_context", "new_context", "has_parent"]
|
__slots__ = ["current_context", "new_context", "has_parent"]
|
||||||
|
|
||||||
def __init__(self, new_context: Optional[LoggingContext] = None) -> None:
|
def __init__(self, new_context: Optional[LoggingContextOrSentinel] = None) -> None:
|
||||||
if new_context is None:
|
if new_context is None:
|
||||||
self.new_context = LoggingContext.sentinel # type: LoggingContextOrSentinel
|
self.new_context = LoggingContext.sentinel # type: LoggingContextOrSentinel
|
||||||
else:
|
else:
|
||||||
|
|
|
@ -14,7 +14,11 @@
|
||||||
# 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 synapse.http.server import DirectServeResource, wrap_html_request_handler
|
from synapse.http.server import (
|
||||||
|
DirectServeResource,
|
||||||
|
finish_request,
|
||||||
|
wrap_html_request_handler,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
class SAML2ResponseResource(DirectServeResource):
|
class SAML2ResponseResource(DirectServeResource):
|
||||||
|
@ -24,8 +28,20 @@ class SAML2ResponseResource(DirectServeResource):
|
||||||
|
|
||||||
def __init__(self, hs):
|
def __init__(self, hs):
|
||||||
super().__init__()
|
super().__init__()
|
||||||
|
self._error_html_content = hs.config.saml2_error_html_content
|
||||||
self._saml_handler = hs.get_saml_handler()
|
self._saml_handler = hs.get_saml_handler()
|
||||||
|
|
||||||
|
async def _async_render_GET(self, request):
|
||||||
|
# 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.
|
||||||
|
# In this case, just tell the user that something went wrong and they should
|
||||||
|
# try to authenticate again.
|
||||||
|
request.setResponseCode(400)
|
||||||
|
request.setHeader(b"Content-Type", b"text/html; charset=utf-8")
|
||||||
|
request.setHeader(b"Content-Length", b"%d" % (len(self._error_html_content),))
|
||||||
|
request.write(self._error_html_content.encode("utf8"))
|
||||||
|
finish_request(request)
|
||||||
|
|
||||||
@wrap_html_request_handler
|
@wrap_html_request_handler
|
||||||
async def _async_render_POST(self, request):
|
async def _async_render_POST(self, request):
|
||||||
return await self._saml_handler.handle_saml_response(request)
|
return await self._saml_handler.handle_saml_response(request)
|
||||||
|
|
|
@ -29,7 +29,11 @@ from twisted.internet import defer
|
||||||
|
|
||||||
from synapse.api.errors import StoreError
|
from synapse.api.errors import StoreError
|
||||||
from synapse.config.database import DatabaseConnectionConfig
|
from synapse.config.database import DatabaseConnectionConfig
|
||||||
from synapse.logging.context import LoggingContext, make_deferred_yieldable
|
from synapse.logging.context import (
|
||||||
|
LoggingContext,
|
||||||
|
LoggingContextOrSentinel,
|
||||||
|
make_deferred_yieldable,
|
||||||
|
)
|
||||||
from synapse.metrics.background_process_metrics import run_as_background_process
|
from synapse.metrics.background_process_metrics import run_as_background_process
|
||||||
from synapse.storage.background_updates import BackgroundUpdater
|
from synapse.storage.background_updates import BackgroundUpdater
|
||||||
from synapse.storage.engines import BaseDatabaseEngine, PostgresEngine, Sqlite3Engine
|
from synapse.storage.engines import BaseDatabaseEngine, PostgresEngine, Sqlite3Engine
|
||||||
|
@ -543,7 +547,9 @@ class Database(object):
|
||||||
Returns:
|
Returns:
|
||||||
Deferred: The result of func
|
Deferred: The result of func
|
||||||
"""
|
"""
|
||||||
parent_context = LoggingContext.current_context()
|
parent_context = (
|
||||||
|
LoggingContext.current_context()
|
||||||
|
) # type: Optional[LoggingContextOrSentinel]
|
||||||
if parent_context == LoggingContext.sentinel:
|
if parent_context == LoggingContext.sentinel:
|
||||||
logger.warning(
|
logger.warning(
|
||||||
"Starting db connection from sentinel context: metrics will be lost"
|
"Starting db connection from sentinel context: metrics will be lost"
|
||||||
|
|
Loading…
Reference in New Issue