Skip to content

Commit

Permalink
core: Fix archive user updating their keys when using autoconfig
Browse files Browse the repository at this point in the history
Previously a user's key fingerprint would be updated in the database but
not in the GPG keyring if the user already existed. This change will add
the key unconditionally if it is missing in the GPG keyring and ensure
it and the DB are in sync again.

This allows one key adding multiple subkeys.
  • Loading branch information
ximion committed Feb 29, 2024
1 parent f622aed commit fdb5204
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 13 deletions.
4 changes: 2 additions & 2 deletions src/laniakea/archive/uploadermgr.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,14 @@ def import_key_file_for_uploader(uploader: ArchiveUploader, fname: T.PathUnion):
archive_log.info('UPLOADER-ADDED-GPG: %s: %s', uploader.email, ', '.join(fingerprints))


def retrieve_uploader_fingerprints() -> list[str]:
def retrieve_uploader_fingerprints(*, only_primary=True) -> list[str]:
"""List all fingerprints for keys that have archive upload access."""

lconf = LocalConfig()
keyring_dir = lconf.uploaders_keyring_dir
os.makedirs(keyring_dir, exist_ok=True)

return list_gpg_fingerprints(keyring_dir)
return list_gpg_fingerprints(keyring_dir, only_primary=only_primary)


def delete_uploader_key(fingerprint: str):
Expand Down
2 changes: 1 addition & 1 deletion src/laniakea/utils/gpg.py
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ def sign(


def list_gpg_fingerprints(gpghome: T.PathUnion, *, only_primary=True) -> list[str]:
"""List all primary key fingerprints from the keyring set by :gpghome"""
"""List all key fingerprints from the keyring set by :gpghome"""

args = [
'/usr/bin/gpg',
Expand Down
36 changes: 26 additions & 10 deletions src/lkadmin/archive.py
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ def update_uploaders(dir_path, auto=False, no_confirm=False):
if not no_confirm:
data_src = git_url if git_url else dir_path
proceed_answer = Confirm.ask(
'Update users with data from {}? This will DELETE and users not present in this directory!'.format(data_src)
'Update users with data from {}? This will DELETE all users not present in this directory!'.format(data_src)
)
if not proceed_answer:
return
Expand All @@ -643,7 +643,7 @@ def update_uploaders(dir_path, auto=False, no_confirm=False):
delete_expired_uploader_keys()

# get all active key fingerprints
uploader_fprs = set(retrieve_uploader_fingerprints())
uploader_fprs = set(retrieve_uploader_fingerprints(only_primary=False))
used_fprs = set()

valid_users_found = False
Expand Down Expand Up @@ -671,7 +671,7 @@ def update_uploaders(dir_path, auto=False, no_confirm=False):
session.add(user)
user.name = uconf.get('name', '')
user.alias = uconf.get('alias', None)
user.pgp_fingerprints = fingerprints
user.pgp_fingerprints = []
user.is_human = uconf.get('is_human', True)
user.allow_source_uploads = uconf.get('allow_source_uploads', True)
user.allow_binary_uploads = uconf.get('allow_binary_uploads', False)
Expand All @@ -695,15 +695,28 @@ def update_uploaders(dir_path, auto=False, no_confirm=False):

# update GPG keys
for fpr in fingerprints:
used_fprs.add(fpr)
# check if we already have the key for existing users
# Check if the key already exists in the global GPG keyring (and skip adding it again)
if fpr in uploader_fprs:
continue
# check if the key was already added to the current user in the current import cycle
if fpr in user.pgp_fingerprints:
continue
log.info('Importing key %s for %s', fpr, email)
import_key_file_for_uploader(user, os.path.join(uconf_root, fpr + '.asc'))
key_fname = os.path.join(uconf_root, fpr + '.asc')
if os.path.isfile(key_fname):
# we have a file for this ID, import it!
log.info('Importing key %s for %s', fpr, email)
import_key_file_for_uploader(user, key_fname)

# this operation may have added multiple new keys to the trusted set, add them
# to the known-keys set for uploaders
uploader_fprs = set(retrieve_uploader_fingerprints(only_primary=False))

# update the database key list based on what the GPG keyring has available
for fpr in fingerprints:
# add the fingerprint to the active set
used_fprs.add(fpr)

# update database entries based on available keys
if fpr in uploader_fprs and fpr not in user.pgp_fingerprints:
log.debug('Allowing key %s for %s', fpr, email)
user.pgp_fingerprints.append(fpr)

if valid_users_found:
# only remove stuff if we found at least one valid user in the new dataset, as safety precaution
Expand All @@ -713,6 +726,9 @@ def update_uploaders(dir_path, auto=False, no_confirm=False):
log.info('Removing user %s', user.email)
session.delete(user)

# get a list of only primary key fingerprints, to potentially delete
uploader_fprs = set(retrieve_uploader_fingerprints(only_primary=True))

for orphan_fpr in uploader_fprs - used_fprs:
log.warning('Found orphaned key %s in archive uploader keyring - deleting it.', orphan_fpr)
delete_uploader_key(orphan_fpr)

0 comments on commit fdb5204

Please sign in to comment.