UnknownLaunchpadError in what should be a LaunchpadAPIError causes user-visible oops (adding SSH key)
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
Canonical SSO provider |
Fix Released
|
Undecided
|
Maximiliano Bertacchini | ||
Launchpad itself |
Fix Released
|
High
|
Maximiliano Bertacchini |
Bug Description
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
add_lp_ssh_key calls http_request_
http_request_
Launchpad raises the exception like so:
raise SSHKeyAdditionE
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_
Originally reported in https:/
https:/
UnknownLaunchpa
File "django/
response = get_response(
File "django/
response = self._get_
File "django/
response = self.process_
File "django/
response = wrapped_
File "django/
return func(*args, **kwargs)
File "gargoyle/
return func(request, *args, **kwargs)
File "django/
return view_func(request, *args, **kwargs)
File "webui/
return view_func(request, *args, **kwargs)
File "webui/
return func(request, *args, **kwargs)
File "webui/
if import_
File "django/
return self.is_bound and not self.errors
File "django/
self.
File "django/
self.
File "django/
value = getattr(self, 'clean_%s' % name)()
File "webui/forms.py", line 67, in clean_ssh_key
dry_run=True)
File "webservices/
headers=
File "webservices/
raise _sshkey_
Related branches
- Daniel Manrique (community): Approve
-
Diff: 59 lines (+6/-6)2 files modifiedsrc/webservices/launchpad.py (+2/-2)
src/webservices/tests/test_launchpad.py (+4/-4)
- Colin Watson (community): Approve (code)
-
Diff: 112 lines (+38/-8)4 files modifiedlib/lp/registry/interfaces/ssh.py (+25/-1)
lib/lp/registry/model/person.py (+4/-6)
lib/lp/registry/tests/test_ssh.py (+7/-0)
lib/lp/testing/__init__.py (+2/-1)
- Colin Watson (community): Approve
-
Diff: 46 lines (+4/-4)3 files modifiedlib/lp/registry/interfaces/ssh.py (+2/-1)
lib/lp/registry/tests/test_ssh.py (+1/-1)
lib/lp/testing/__init__.py (+1/-2)
Changed in canonical-identity-provider: | |
assignee: | nobody → Maximiliano Bertacchini (maxiberta) |
Changed in canonical-identity-provider: | |
status: | New → In Progress |
Changed in canonical-identity-provider: | |
status: | In Progress → Fix Committed |
Changed in launchpad: | |
status: | New → In Progress |
importance: | Undecided → High |
assignee: | nobody → Maximiliano Bertacchini (maxiberta) |
tags: | added: api lp-registry |
Changed in canonical-identity-provider: | |
status: | Fix Committed → Fix Released |
Changed in launchpad: | |
status: | In Progress → Fix Committed |
Changed in launchpad: | |
status: | Fix Committed → In Progress |
tags: |
added: qa-ok removed: qa-needstesting |
Changed in launchpad: | |
status: | Fix Committed → Fix Released |
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?