UnknownLaunchpadError in what should be a LaunchpadAPIError causes user-visible oops (adding SSH key)

[Daniel Manrique]

if we get a 400 when adding an SSH key, we should treat it as SSO treats a LaunchpadAPIError which bubbles the error as a form validation error.

This case was a genuinely-borked SSH public key payload, but showing the user the error as a form validation would be clearer than sentry-IDing on them as we do now.

The code that triggered this is in SSO src/webui/forms.py ClaimSSHKeyForm.clean_ssh_key, which calls add_lp_ssh_key(..., dry_run=True) and nicely handles a LaunchpadAPIError, but since what's raised here is UnknownLaunchpadError, it barfs.

add_lp_ssh_key calls http_request_with_retry and correctly tries to raise LaunchpadAPIError for non-200 responses, but before it can get a response (which is done in an if construct, not a try/catch), http_request_with_retry raises the UnknownLaunchpadError exception.

http_request_with_retry checks if the error is one of a known list (check _sshkey_placeholder_exception, and the given data below should result in a SSHKeyAdditionDataError).

Launchpad raises the exception like so:
   raise SSHKeyAdditionError(
                    "Invalid SSH key data: '%s' (%s)" % (sshkey, e))

But our matching regex doesn't match that (tested with the below message sample):

    MATCH = re.compile(
        r"^Invalid SSH key data: '.*?'$")

Mainly because there's stuff outside the single quotes in the raised exception.

    This regex works: r"^Invalid SSH key data: '.*?' \(.*?\)$"

That should solve the problem: http_request_with_retry retries for UnknownLaunchpadError (ultimately raising that same one again, which is not handled by clean_ssh_key resulting in the oops), but for the other "known" exceptions, it just raises them without retrying, so it'd raise SSHKeyAdditionError. This one is a subclass of LaunchpadAPIError, so it would be nicely handled by clean_ssh_key.

Originally reported in https://bugs.launchpad.net/canonical-identity-provider/+bug/1777494 (Private for the time being).

https://sentry.ols.canonical.com/canonical/sso/issues/669/ (Canonical internal link, sorry).

UnknownLaunchpadError: <UnknownLaunchpadError code: 400 message: 'Invalid SSH key data: 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQDm91Jdtbq/huGvoUiY80Fo0kEcUYbD5xqosE7z0+xKux7+T/AvreujM7ZDOzEnkuaDJWsZruXwBdkuZvvBHlcX+NqtChsoOw hgHabSTTJ48PYYaJ7khaPXg/c+fE15eIMIBp6/l9rAKWqjym7A+WOrqA9JERR10naOolLIbpqn6cEi8f70wdQbJjzwgH3Jg4NrtWUeDmkBNn1MTEZ0zGlmD5VXhKtUYi+V5zvu By/7WyRnoLhNA38/eHwfRwbe+RfuDgn9F7nOHdlDMkoKHnfnDuMFn6q+Jt+0MBnJ8ANDu5Vg8EN7XqTUzRCdQjw87z4bx1yLQVtMmZK4v7CRdV4eyc0iCsrYpDO5fvZERzMopZ hI+VRGLrdMMElpgPYCYfoqgdA2r0dFBV0c0Vys0wevQD+60ywNzpr2BiVXPMndjHQ++1SHXJahmvq7XQ6gx8/5FeEV7F1/kBlD0n/D//jLXJeln+bBKXZfihJMm8I2gX2hMO/H OXtuhxbm0yvu43pAMyfZLBXssYt17g0zhXqKzvLc83xWl8nYaV11XmxFpcldj8J38jJlzvnX9CUlmxEgDxRQ5biNpKX1XRgNDHM/bKtDTQT5XfKHyJeGJ9cnIdUKv3Z8KhSDdh QKGIXK1R3wj2D3u5aiSNmbch35jmdJbJOjicc0zEV3uOhFVM7jDw==' (Incorrect padding)'>
  File "django/core/handlers/exception.py", line 41, in inner
    response = get_response(request)
  File "django/core/handlers/base.py", line 249, in _legacy_get_response
    response = self._get_response(request)
  File "django/core/handlers/base.py", line 187, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "django/core/handlers/base.py", line 185, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "django/utils/decorators.py", line 185, in inner
    return func(*args, **kwargs)
  File "gargoyle/decorators.py", line 29, in wrapped
    return func(request, *args, **kwargs)
  File "django/contrib/auth/decorators.py", line 23, in _wrapped_view
    return view_func(request, *args, **kwargs)
  File "webui/decorators.py", line 135, in _wrapped_view
    return view_func(request, *args, **kwargs)
  File "webui/decorators.py", line 161, in wrapper
    return func(request, *args, **kwargs)
  File "webui/views/account.py", line 524, in ssh_keys
    if import_key_form.is_valid():
  File "django/forms/forms.py", line 183, in is_valid
    return self.is_bound and not self.errors
  File "django/forms/forms.py", line 175, in errors
    self.full_clean()
  File "django/forms/forms.py", line 384, in full_clean
    self._clean_fields()
  File "django/forms/forms.py", line 405, in _clean_fields
    value = getattr(self, 'clean_%s' % name)()
  File "webui/forms.py", line 67, in clean_ssh_key
    dry_run=True)
  File "webservices/launchpad.py", line 196, in add_lp_ssh_key
    headers=headers, opener=_lp_api_opener())
  File "webservices/launchpad.py", line 240, in http_request_with_retry
    raise _sshkey_placeholder_exception(e.code, e.read())

[Colin Watson (cjwatson)]

Ew. Can we at a minimum add something to Launchpad to warn that this error message is regex-matched by SSO, and ideally figure out some way to make the handling more robust against what ought to be innocent changes to Launchpad?

[Launchpad QA Bot (lpqabot)]

Fixed in stable r18689 <http://bazaar.launchpad.net/~launchpad-pqm/launchpad/stable/revision/18689>.

[Launchpad QA Bot (lpqabot)]

Fixed in stable r18693 <http://bazaar.launchpad.net/~launchpad-pqm/launchpad/stable/revision/18693>.