ssh keys are not linked if the key doesn't have a comment

The problem code is this
        try:
            kind, keytext, comment = sshkey.split(' ', 2)
        except ValueError:
            raise SSHKeyAdditionError
        if not (kind and keytext and comment):
            raise SSHKeyAdditionError

The error could be explicit and state what parts are missing that are required.

furthermore i suggest to default comment to the currently logged-in user.
Think:

class SSHKeyAdditionError(Exception):
  def __init__(self, what):
    self.what = what
  def __str__(self):
    if self.what == "kind":
      return u"Unknown key type"
    elif self.what == "keytext":
      return u"Bad keytext"
    elif self.what == "comment":
      return u"Missing comment"
    else:
      return u"Internal error"
class SSHKey(object):
  def __init__(self, stuff):
    if not stuff or len(stuff) < 2:
      raise ValueError
    stuff.reverse()
    self._data = {"kind" : stuff.pop(),
                  "keytext" : stuff.pop(),
                  "comment" : stuff or "get_calling_username()"
                 }
    # get_calling_username() returns None or u"" for invalid or not
    # found user
  def __getitem__(self, key):
    return self._data[key]
  def validate(self):
    if len(self._data) != 3:
      self.error = ""
      return False
    for key in ["kind", "keytext", "comment"]:
      tmp = self._data.get(key)
      if not tmp:
        self.error = key
        return False
    return True

sshkey="id-dsa 0x0815=="
try:
    userkey = SSHKey(sshkey.split(None, 2))
except ValueError:
    raise SSHKeyAdditionError("")
if not userkey.validate():
    raise SSHKeyAdditionError(userkey.error)

thanks,

thinking about it, it would be even easier to just default comment to kind or "id" and not try to lookup the user, but i don't know the surrounding code, admittedly.

HTH,

I agree about falling back to a default. A user can use the same comment for many keys. It would be easier to add a rule to make a comment then to ask for one or tell the user to hack the key.

    if comment is None:
        comment = '%s key' % person.name

person.name is launchpad id. The code is in SSHKeySet.new() in lib/lp/registry/model/person.py

careful!
if not comment or not len(comment.strip()):
  comment = '%s key' % person.name

Think about "a b== "

better validate version:
  def validate(self):
    if len(self._data) != 3:
      self.error = ""
      return False
    for key in ["kind", "keytext", "comment"]:
      tmp = self._data.get(key)
      if not tmp or not tmp.strip(): # reject empty or just spaces!
        self.error = key
        return False
    return True

gah! not len(tmp.strip()) ;)