matrix-org · clokep · Aug 18, 2021 · Aug 5, 2021 · Aug 11, 2021 · Aug 11, 2021
@@ -0,0 +1 @@
+Display an error on User-Interactive Authentication fallback pages when authentication fails. Contributed by Callum Brown.
@@ -112,6 +112,14 @@ environment variable.
 See [using a forward proxy with Synapse documentation](setup/forward_proxy.md) for
 details.
 
+## User-interactive authentication fallback templates can now display errors
+
+This may affect you if you make use of custom HTML templates for the
+[reCAPTCHA](../synapse/res/templates/recaptcha.html) or
+[terms](../synapse/res/templates/terms.html) fallback pages.
+
+The template is now provided an `error` variable if the authentication
+process failed. See the default templates linked above for an example.
 
 # Upgrading to v1.39.0
 

@@ -627,23 +627,28 @@ async def check_ui_auth(
 
  async def add_oob_auth(
  self, stagetype: str, authdict: Dict[str, Any], clientip: str
- ) -> bool:
+ ) -> None:
  """
  Adds the result of out-of-band authentication into an existing auth
  session. Currently used for adding the result of fallback auth.
+
+ Raises:
+ LoginError if the stagetype is unknown or the session is missing.
+ LoginError is raised by check_auth if authentication fails.
  """
  if stagetype not in self.checkers:
- raise LoginError(400, "", Codes.MISSING_PARAM)
+ raise LoginError(
+ 400, f"Unknown UIA stage type: {stagetype}", Codes.INVALID_PARAM
+ )
  if "session" not in authdict:
- raise LoginError(400, "", Codes.MISSING_PARAM)
+ raise LoginError(400, "Missing session ID", Codes.MISSING_PARAM)
 
+ # If authentication fails a LoginError is raised. Otherwise, store
+ # the successful result.
  result = await self.checkers[stagetype].check_auth(authdict, clientip)
- if result:
- await self.store.mark_ui_auth_stage_complete(
- authdict["session"], stagetype, result
- )
- return True
- return False
+ await self.store.mark_ui_auth_stage_complete(
+ authdict["session"], stagetype, result
+ )
 
  def get_session_id(self, clientdict: Dict[str, Any]) -> Optional[str]:
  """

@@ -49,7 +49,7 @@ async def check_auth(self, authdict: dict, clientip: str) -> Any:
  clientip: The IP address of the client.
 
  Raises:
- SynapseError if authentication failed
+ LoginError if authentication failed.
 
  Returns:
  The result of authentication (to pass back to the client?)
@@ -131,7 +131,9 @@ async def check_auth(self, authdict: dict, clientip: str) -> Any:
  )
  if resp_body["success"]:
  return True
- raise LoginError(401, "", errcode=Codes.UNAUTHORIZED)
+ raise LoginError(
+ 401, "Captcha authentication failed", errcode=Codes.UNAUTHORIZED
+ )
 
 
 class _BaseThreepidAuthChecker:
@@ -191,7 +193,9 @@ async def _check_threepid(self, medium: str, authdict: dict) -> dict:
  raise AssertionError("Unrecognized threepid medium: %s" % (medium,))
 
  if not threepid:
- raise LoginError(401, "", errcode=Codes.UNAUTHORIZED)
+ raise LoginError(
+ 401, "Unable to get validated threepid", errcode=Codes.UNAUTHORIZED
+ )
 
  if threepid["medium"] != medium:
  raise LoginError(

@@ -16,6 +16,9 @@
 <body>
 <form id="registrationForm" method="post" action="{{ myurl }}">
  <div>
+ {% if error is defined %}
+ <p class="error"><strong>Error: {{ error }}</strong></p>
+ {% endif %}
  <p>
  Hello! We need to prevent computer programs and other automated
  things from creating accounts on this server.

@@ -8,6 +8,9 @@
 <body>
 <form id="registrationForm" method="post" action="{{ myurl }}">
  <div>
+ {% if error is defined %}
+ <p class="error"><strong>Error: {{ error }}</strong></p>
+ {% endif %}
  <p>
  Please click the button below if you agree to the
  <a href="{{ terms_url }}">privacy policy of this homeserver.</a>

@@ -16,7 +16,7 @@
 from typing import TYPE_CHECKING
 
 from synapse.api.constants import LoginType
-from synapse.api.errors import SynapseError
+from synapse.api.errors import LoginError, SynapseError
 from synapse.api.urls import CLIENT_API_PREFIX
 from synapse.http.server import respond_with_html
 from synapse.http.servlet import RestServlet, parse_string
@@ -95,29 +95,32 @@ async def on_POST(self, request, stagetype):
 
  authdict = {"response": response, "session": session}
 
- success = await self.auth_handler.add_oob_auth(
- LoginType.RECAPTCHA, authdict, request.getClientIP()
- )
-
- if success:
- html = self.success_template.render()
- else:
+ try:
+ await self.auth_handler.add_oob_auth(
+ LoginType.RECAPTCHA, authdict, request.getClientIP()
+ )
+ except LoginError as e:
+ # Authentication failed, let user try again
  html = self.recaptcha_template.render(
  session=session,
  myurl="%s/r0/auth/%s/fallback/web"
  % (CLIENT_API_PREFIX, LoginType.RECAPTCHA),
  sitekey=self.hs.config.recaptcha_public_key,
+ error=e.msg,
  )
+ else:
+ # No LoginError was raised, so authentication was successful
+ html = self.success_template.render()
+
  elif stagetype == LoginType.TERMS:
  authdict = {"session": session}
 
- success = await self.auth_handler.add_oob_auth(
- LoginType.TERMS, authdict, request.getClientIP()
- )
-
- if success:
- html = self.success_template.render()
- else:
+ try:
+ await self.auth_handler.add_oob_auth(
+ LoginType.TERMS, authdict, request.getClientIP()
+ )
+ except LoginError as e:
+ # Authentication failed, let user try again
  html = self.terms_template.render(
  session=session,
  terms_url="%s_matrix/consent?v=%s"
@@ -127,10 +130,16 @@ async def on_POST(self, request, stagetype):
  ),
  myurl="%s/r0/auth/%s/fallback/web"
  % (CLIENT_API_PREFIX, LoginType.TERMS),
+ error=e.msg,
  )
+ else:
+ # No LoginError was raised, so authentication was successful
+ html = self.success_template.render()
+
  elif stagetype == LoginType.SSO:
  # The SSO fallback workflow should not post here,
  raise SynapseError(404, "Fallback SSO auth does not support POST requests.")
+
  else:
  raise SynapseError(404, "Unknown auth stage type")
 

@@ -57,4 +57,8 @@ textarea, input {
 
  background-color: #f8f8f8;
  border: 1px #ccc solid;
-}
+}
+
+.error {
+ color: red;
+}