explicit relay of proxy errors to the frontend

[rhoerbe]

Durch the setup of a rocket chat server fronted by satosa (SAML2SAML) I encountered various configurations errors, like a certificate problem with the metadata, clock drag and incorrect metadata registration. However, the error messagefron the frontend is always "Authentication failed. Error id ....".

I think that - at least for QA systems - error messages should include the root cause. "Authentication failed may be also misleading when the error is not in the actual authentication process.

[c00kiemon5ter]

This is a complex issue; I totally agree with your reasoning.

Error handling in SATOSA needs a complete overhaul, IMO.

For start, exceptions that are caught and then raise other exceptions should follow the chain pattern (raise ... from ...) that python provides. Then, I would try to group errors in domains or subsystems and form a structure. Then, it should be easier to add or remove the needed exception/error classes. This would allow for better error messages and logging (which is by itself another big refactor).

# generic application error
# all custom error/exceptions should derive from this
class AppError(Exception):
  pass

class SomeFuncError(AppError):
  pass

# an error that describes errors in a subsystem
# an example would be AuthenticationError
# error messages would specify if it is due to invalid token or wrong password etc
# another example would be AuthorisationError/PermissionError
# error messages would specify if it is due to invalid token or wrong password etc
class SubSystemError(AppError):
  pass

# subsystems may provide lower level exceptions
# an example would be a BasicAuthenticationError
# either because no such user was found or because the password did not match
class SomeOtherFuncError(SubSystemError):
  pass

def some_other_func():
  raise SomeOtherFuncError("error in low level operation occured")

def some_func():
  try:
    some_other_func()
  except SubSystemError as e:
    raise SomeFuncError("failed to do some_func") from e

def main():
  try:
    some_func()
  except AppError as e:
    log.error("Application error", exc_info=settings.DEBUG)
  except Exception as e:
    log.exception("System error")

This allows application-errors to be logged by their error message only, unless the debug flag is set. When debug is set, one gets a full stacktrace to the point that caused the first exception that was not handled.

In general I would argue that always showing stacktraces is useful, but I am not opposed to hiding them for cosmetic reasons - however a mechanism to enable them should be in place, in order to ask for them when a user can reproduce a problem; and that often happens in QA forums or GH-issues.

[rhoerbe]

My guess was to structure exception classes in 2 main categories: (a) expected and well handled, and (b) other (like rare, hard to test and not foreseen).

(a) Should provide a short a clear message in the log (and possible the GUI). A stack trace is not included by default, because the message is complete with context. (b) Should include the stack trace, because the context is quite likely not understood.

If an exception is raise by a module and passed to the main loop, this would allow quite simply to adjust the amount of information per exception category to be printed by catching these 2 exception super classes.

E.g.: An malformed XML metadata file will cause pysaml2 to print a stack trace, but not the offending metadata source. The stacktrace will point to the plugin, but it is a lot of noise for the short information that metadatafile idp.xml is not well-formed xml in line 5 pos 33. I would say that an invalid metadata file is a well understood and expected error, and error messages should talk to system admins (who might not be SAML experts) as well as developers.

[peppelinux]

Wouldn't it be Better to merge all this issues to a single topic regarding the exception handling and representation to users?

Probably this Is also related to https://github.com/IdentityPython/SATOSA/pull/221

What do you think also about a jinja template for exception message rendering as a web page? I'll move in this, any ideas?