From 0e5f4a0b34b0cefd2ba38bec559a90cca935ed28 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Sep 25 2015 23:34:04 +0000 Subject: [PATCH 1/6] Make gitolite pick up multiple keys per user To implement this, we write keys_/.pub, where i is an integer value relating to the line number of the specific key. Signed-off-by: Patrick Uiterwijk --- diff --git a/files/load_from_disk.py b/files/load_from_disk.py index 6837b1d..5f0feb6 100644 --- a/files/load_from_disk.py +++ b/files/load_from_disk.py @@ -57,6 +57,7 @@ def main(folder, debug=False): username=user, fullname=user, default_email='%s@fedoraproject.org' % user, + keydir=pagure.APP.config.get('GITOLITE_KEYDIR', None), ) pagure.SESSION.commit() except SQLAlchemyError, err: diff --git a/pagure/__init__.py b/pagure/__init__.py index 7aec556..d3878c6 100644 --- a/pagure/__init__.py +++ b/pagure/__init__.py @@ -160,59 +160,6 @@ def is_repo_admin(repo_obj): ) or (user in usergrps) -def generate_gitolite_acls(): - """ Generate the gitolite configuration file for all repos - """ - pagure.lib.git.write_gitolite_acls( - SESSION, APP.config['GITOLITE_CONFIG']) - - gitolite_folder = APP.config.get('GITOLITE_HOME', None) - gitolite_version = APP.config.get('GITOLITE_VERSION', 3) - if gitolite_folder: - if gitolite_version == 2: - cmd = 'GL_RC=%s GL_BINDIR=%s gl-compile-conf' % ( - APP.config.get('GL_RC'), APP.config.get('GL_BINDIR') - ) - elif gitolite_version == 3: - cmd = 'HOME=%s gitolite compile && HOME=%s gitolite trigger '\ - 'POST_COMPILE' % ( - APP.config.get('GITOLITE_HOME'), - APP.config.get('GITOLITE_HOME') - ) - else: - raise pagure.exceptions.PagureException( - 'Non-supported gitolite version "%s"' % gitolite_version - ) - subprocess.Popen( - cmd, - shell=True, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - cwd=gitolite_folder - ) - # We need to do this because gitolite will also try to recreate the authorized_keys - # file, but it will ignore any keyfiles with more then a single line. So it will - # never create a authorized_keys file with more than one key for any single user. - generate_authorized_key_file() - - -def generate_gitolite_key(user, key): # pragma: no cover - """ Generate the gitolite ssh key file for the specified user - """ - gitolite_keydir = APP.config.get('GITOLITE_KEYDIR', None) - if gitolite_keydir: - keyfile = os.path.join(gitolite_keydir, '%s.pub' % user) - with open(keyfile, 'w') as stream: - # If we do more then one line, gitolite will ignore the key file. - # Symptom: WARNING: keydir/.pub does not contain exactly 1 line; ignoring - # Let us make sure we at least have the users first key in there until - # we manually recreate the authorized_keys file (should happen almost - # the same time, but to prevent issues in the most trivial case where - # a user just has a single key, we also use the gitolite system as - # fallback). - stream.write(key.split('\n')[0]) - - def generate_authorized_key_file(): # pragma: no cover """ Regenerate the `authorized_keys` file used by gitolite. """ @@ -305,7 +252,8 @@ def set_user(return_url): username=flask.g.fas_user.username, fullname=flask.g.fas_user.fullname, default_email=flask.g.fas_user.email, - ssh_key=flask.g.fas_user.get('ssh_key') + ssh_key=flask.g.fas_user.get('ssh_key'), + keydir=APP.config.get('GITOLITE_KEYDIR', None), ) SESSION.commit() except SQLAlchemyError, err: diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 3c2fcd2..7ba6349 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -152,6 +152,36 @@ def search_user(session, username=None, email=None, token=None, pattern=None): return output +def create_user_ssh_keys_on_disk(user, gitolite_keydir): + if gitolite_keydir: + # First remove any old keyfiles for the user + # Assumption: we populated the keydir. This means that files + # will be in 0/.pub, ..., and not in any deeper + # directory structures. Also, this means that if a user + # had 5 lines, they will be up to at most keys_4/.pub, + # meaning that if a user is not in keys_/.pub, with + # i being any integer, the user is most certainly not in + # keys_/.pub. + i = 0 + keyline_file = os.path.join(gitolite_keydir, + 'keys_%i' % i, + '%s.pub' % user.user) + while os.path.exists(keyline_file): + os.path.rm(keyline_file) + i += 1 + keyline_file = os.path.join(gitolite_keydir, + 'keys_%i' % i, + '%s.pub' % user.user) + + # Now let's create new keyfiles for the user + keys = user.public_ssh_key.split('\n') + for i in range(len(keys)): + keyline_dir = os.path.join(gitolite_keydir, 'keys_%i' % i) + if not os.path.exists(keyline_dir): + os.mkdir(keyline_dir) + keyfile = os.path.join(keyline_dir, '%s.pub' % user.user) + with open(keyfile, 'w') as stream: + stream.write(keys[i].strip().encode('UTF-8')) def add_issue_comment(session, issue, comment, user, ticketfolder, notify=True, redis=None): @@ -1774,7 +1804,7 @@ def get_pull_request_flag_by_uid(session, flag_uid): def set_up_user(session, username, fullname, default_email, - emails=None, ssh_key=None): + emails=None, ssh_key=None, keydir=None): ''' Set up a new user into the database or update its information. ''' user = search_user(session, username=username) if not user: @@ -1800,7 +1830,7 @@ def set_up_user(session, username, fullname, default_email, add_email_to_user(session, user, email) if ssh_key and not user.public_ssh_key: - user.public_ssh_key = ssh_key + update_user_ssh(session, user, ssh_key, keydir) return user @@ -1816,18 +1846,18 @@ def add_email_to_user(session, user, user_email): session.flush() -def update_user_ssh(session, user, ssh_key): +def update_user_ssh(session, user, ssh_key, keydir): ''' Set up a new user into the database or update its information. ''' if isinstance(user, basestring): user = __get_user(session, user) message = 'Nothing to update' - ssh_key = ssh_key.strip().replace('\n', '') \ - if ssh_key and ssh_key.strip() else None - if ssh_key != user.public_ssh_key: user.public_ssh_key = ssh_key + if keydir: + create_user_ssh_keys_on_disk(user, keydir) + pagure.lib.git.generate_gitolite_acls() session.add(user) session.flush() message = 'Public ssh key updated' diff --git a/pagure/lib/git.py b/pagure/lib/git.py index 36e6e36..1a9726c 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -120,6 +120,39 @@ def write_gitolite_acls(session, configfile): stream.write(row + '\n') +def generate_gitolite_acls(): + """ Generate the gitolite configuration file for all repos + """ + pagure.lib.git.write_gitolite_acls( + pagure.SESSION, pagure.APP.config['GITOLITE_CONFIG']) + + gitolite_folder = pagure.APP.config.get('GITOLITE_HOME', None) + gitolite_version = pagure.APP.config.get('GITOLITE_VERSION', 3) + if gitolite_folder: + if gitolite_version == 2: + cmd = 'GL_RC=%s GL_BINDIR=%s gl-compile-conf' % ( + pagure.APP.config.get('GL_RC'), + pagure.APP.config.get('GL_BINDIR') + ) + elif gitolite_version == 3: + cmd = 'HOME=%s gitolite compile && HOME=%s gitolite trigger '\ + 'POST_COMPILE' % ( + pagure.APP.config.get('GITOLITE_HOME'), + pagure.APP.config.get('GITOLITE_HOME') + ) + else: + raise pagure.exceptions.PagureException( + 'Non-supported gitolite version "%s"' % gitolite_version + ) + subprocess.Popen( + cmd, + shell=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + cwd=gitolite_folder + ) + + def update_git(obj, repo, repofolder, objtype='ticket'): """ Update the given issue in its git. @@ -307,6 +340,7 @@ def get_user_from_json(session, jsondata, key='user'): fullname=fullname or username, default_email=default_email, emails=useremails, + keydir=pagure.APP.config.get('GITOLITE_KEYDIR', None), ) session.commit() diff --git a/pagure/ui/admin.py b/pagure/ui/admin.py index 1b1b7b5..f9ebb02 100644 --- a/pagure/ui/admin.py +++ b/pagure/ui/admin.py @@ -16,8 +16,8 @@ from sqlalchemy.exc import SQLAlchemyError import pagure.exceptions import pagure.forms import pagure.lib +import pagure.lib.git from pagure import (APP, SESSION, - generate_gitolite_acls, generate_authorized_key_file, is_admin, admin_session_timedout) # pylint: disable=E1101 @@ -63,7 +63,7 @@ def admin_generate_acl(): form = pagure.forms.ConfirmationForm() if form.validate_on_submit(): try: - generate_gitolite_acls() + pagure.lib.git.generate_gitolite_acls() flask.flash('Gitolite ACLs updated') except pagure.exceptions.PagureException, err: flask.flash(str(err), 'error') diff --git a/pagure/ui/app.py b/pagure/ui/app.py index a2145e3..0de7c7c 100644 --- a/pagure/ui/app.py +++ b/pagure/ui/app.py @@ -15,11 +15,11 @@ from sqlalchemy.exc import SQLAlchemyError import pagure.exceptions import pagure.lib +import pagure.lib.git import pagure.forms import pagure.ui.filters from pagure import (APP, SESSION, cla_required, - generate_gitolite_acls, generate_gitolite_key, - generate_authorized_key_file, authenticated, + authenticated, admin_session_timedout) @@ -358,7 +358,7 @@ def new_project(): requestfolder=APP.config['REQUESTS_FOLDER'], ) SESSION.commit() - generate_gitolite_acls() + pagure.lib.git.generate_gitolite_acls() flask.flash(message) return flask.redirect(flask.url_for('view_repo', repo=name)) except pagure.exceptions.PagureException, err: @@ -397,9 +397,9 @@ def user_settings(): SESSION, user=user, ssh_key=ssh_key, + keydir=APP.config.get('GITOLITE_KEYDIR', None), ) if message != 'Nothing to update': - generate_gitolite_key(user.user, ssh_key) generate_authorized_key_file() SESSION.commit() flask.flash(message) diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index 8e5787b..bbfab9a 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -21,7 +21,7 @@ import pagure.lib import pagure.lib.git import pagure.forms from pagure import (APP, REDIS, SESSION, LOG, cla_required, - is_repo_admin, generate_gitolite_acls) + is_repo_admin) # pylint: disable=E1101 @@ -728,7 +728,7 @@ def fork_project(repo, username=None): user=flask.g.fas_user.username) SESSION.commit() - generate_gitolite_acls() + pagure.lib.git.generate_gitolite_acls() flask.flash(message) return flask.redirect( flask.url_for( diff --git a/pagure/ui/groups.py b/pagure/ui/groups.py index 2dc4bcd..7e3c5ca 100644 --- a/pagure/ui/groups.py +++ b/pagure/ui/groups.py @@ -15,6 +15,7 @@ from sqlalchemy.exc import SQLAlchemyError import pagure import pagure.forms import pagure.lib +import pagure.lib.git # pylint: disable=E1101 @@ -76,7 +77,7 @@ def view_group(group): is_admin=pagure.is_admin(), ) pagure.SESSION.commit() - pagure.generate_gitolite_acls() + pagure.lib.git.generate_gitolite_acls() flask.flash(msg) except pagure.exceptions.PagureException, err: pagure.SESSION.rollback() @@ -124,7 +125,7 @@ def group_user_delete(user, group): is_admin=pagure.is_admin() ) pagure.SESSION.commit() - pagure.generate_gitolite_acls() + pagure.lib.git.generate_gitolite_acls() flask.flash( 'User `%s` removed from the group `%s`' % (user, group)) except pagure.exceptions.PagureException, err: @@ -173,7 +174,7 @@ def group_delete(group): pagure.SESSION.delete(group_obj) pagure.SESSION.commit() - pagure.generate_gitolite_acls() + pagure.lib.git.generate_gitolite_acls() flask.flash( 'Group `%s` has been deleted' % (group)) diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index 6b9c4c2..ee5d548 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -1048,7 +1048,7 @@ def remove_user(repo, userid, username=None): break try: SESSION.commit() - pagure.generate_gitolite_acls() + pagure.lib.git.generate_gitolite_acls() flask.flash('User removed') except SQLAlchemyError as err: # pragma: no cover SESSION.rollback() @@ -1094,7 +1094,7 @@ def add_user(repo, username=None): user=flask.g.fas_user.username, ) SESSION.commit() - pagure.generate_gitolite_acls() + pagure.lib.git.generate_gitolite_acls() flask.flash(msg) return flask.redirect( flask.url_for( @@ -1150,7 +1150,7 @@ def add_group_project(repo, username=None): user=flask.g.fas_user.username, ) SESSION.commit() - pagure.generate_gitolite_acls() + pagure.lib.git.generate_gitolite_acls() flask.flash(msg) return flask.redirect( flask.url_for( diff --git a/tests/test.git/HEAD b/tests/test.git/HEAD new file mode 100644 index 0000000..cb089cd --- /dev/null +++ b/tests/test.git/HEAD @@ -0,0 +1 @@ +ref: refs/heads/master diff --git a/tests/test.git/config b/tests/test.git/config new file mode 100644 index 0000000..d5f2df9 --- /dev/null +++ b/tests/test.git/config @@ -0,0 +1,4 @@ +[core] + bare = true + repositoryformatversion = 0 + filemode = true diff --git a/tests/test.git/description b/tests/test.git/description new file mode 100644 index 0000000..498b267 --- /dev/null +++ b/tests/test.git/description @@ -0,0 +1 @@ +Unnamed repository; edit this file 'description' to name the repository. diff --git a/tests/test.git/hooks/README.sample b/tests/test.git/hooks/README.sample new file mode 100755 index 0000000..d125ec8 --- /dev/null +++ b/tests/test.git/hooks/README.sample @@ -0,0 +1,5 @@ +#!/bin/sh +# +# Place appropriately named executable hook scripts into this directory +# to intercept various actions that git takes. See `git help hooks` for +# more information. diff --git a/tests/test.git/info/exclude b/tests/test.git/info/exclude new file mode 100644 index 0000000..6d05881 --- /dev/null +++ b/tests/test.git/info/exclude @@ -0,0 +1,2 @@ +# File patterns to ignore; see `git help ignore` for more information. +# Lines that start with '#' are comments. diff --git a/tests/test2.git/HEAD b/tests/test2.git/HEAD new file mode 100644 index 0000000..cb089cd --- /dev/null +++ b/tests/test2.git/HEAD @@ -0,0 +1 @@ +ref: refs/heads/master diff --git a/tests/test2.git/config b/tests/test2.git/config new file mode 100644 index 0000000..d5f2df9 --- /dev/null +++ b/tests/test2.git/config @@ -0,0 +1,4 @@ +[core] + bare = true + repositoryformatversion = 0 + filemode = true diff --git a/tests/test2.git/description b/tests/test2.git/description new file mode 100644 index 0000000..498b267 --- /dev/null +++ b/tests/test2.git/description @@ -0,0 +1 @@ +Unnamed repository; edit this file 'description' to name the repository. diff --git a/tests/test2.git/hooks/README.sample b/tests/test2.git/hooks/README.sample new file mode 100755 index 0000000..d125ec8 --- /dev/null +++ b/tests/test2.git/hooks/README.sample @@ -0,0 +1,5 @@ +#!/bin/sh +# +# Place appropriately named executable hook scripts into this directory +# to intercept various actions that git takes. See `git help hooks` for +# more information. diff --git a/tests/test2.git/info/exclude b/tests/test2.git/info/exclude new file mode 100644 index 0000000..6d05881 --- /dev/null +++ b/tests/test2.git/info/exclude @@ -0,0 +1,2 @@ +# File patterns to ignore; see `git help ignore` for more information. +# Lines that start with '#' are comments. diff --git a/tests/test_progit_lib.py b/tests/test_progit_lib.py index fdf0b4f..0d2a172 100644 --- a/tests/test_progit_lib.py +++ b/tests/test_progit_lib.py @@ -997,7 +997,8 @@ class PagureLibtests(tests.Modeltests): session=self.session, username='skvidal', fullname='Seth', - default_email='skvidal@fp.o' + default_email='skvidal@fp.o', + keydir=pagure.APP.config.get('GITOLITE_KEYDIR', None), ) self.session.commit() @@ -1018,7 +1019,8 @@ class PagureLibtests(tests.Modeltests): session=self.session, username='skvidal', fullname='Seth V', - default_email='skvidal@fp.o' + default_email='skvidal@fp.o', + keydir=pagure.APP.config.get('GITOLITE_KEYDIR', None), ) self.session.commit() # Nothing changed @@ -1034,7 +1036,8 @@ class PagureLibtests(tests.Modeltests): session=self.session, username='skvidal', fullname='Seth', - default_email='svidal@fp.o' + default_email='svidal@fp.o', + keydir=pagure.APP.config.get('GITOLITE_KEYDIR', None), ) self.session.commit() # Email added @@ -1052,13 +1055,13 @@ class PagureLibtests(tests.Modeltests): user = pagure.lib.search_user(self.session, username='foo') self.assertEqual(user.public_ssh_key, None) - msg = pagure.lib.update_user_ssh(self.session, user, 'blah') + msg = pagure.lib.update_user_ssh(self.session, user, 'blah', keydir=None) self.assertEqual(msg, 'Public ssh key updated') - msg = pagure.lib.update_user_ssh(self.session, user, 'blah') + msg = pagure.lib.update_user_ssh(self.session, user, 'blah', keydir=None) self.assertEqual(msg, 'Nothing to update') - msg = pagure.lib.update_user_ssh(self.session, 'foo', None) + msg = pagure.lib.update_user_ssh(self.session, 'foo', None, keydir=None) self.assertEqual(msg, 'Public ssh key updated') def test_avatar_url(self): From 63cbb9bae653dae09fbed92c50b4d2e0fe8f1798 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Sep 25 2015 23:34:04 +0000 Subject: [PATCH 2/6] Regenerate user key files Signed-off-by: Patrick Uiterwijk --- diff --git a/pagure/__init__.py b/pagure/__init__.py index d3878c6..41fb351 100644 --- a/pagure/__init__.py +++ b/pagure/__init__.py @@ -48,6 +48,7 @@ if 'PAGURE_CONFIG' in os.environ: import pagure.lib +import pagure.lib.git import pagure.mail_logging import pagure.doc_utils import pagure.forms @@ -160,39 +161,16 @@ def is_repo_admin(repo_obj): ) or (user in usergrps) -def generate_authorized_key_file(): # pragma: no cover - """ Regenerate the `authorized_keys` file used by gitolite. +def generate_user_key_files(): + """ Regenerate the key files used by gitolite. """ gitolite_home = APP.config.get('GITOLITE_HOME', None) if gitolite_home: users = pagure.lib.search_user(SESSION) - - authorized_file = os.path.join( - gitolite_home, '.ssh', 'authorized_keys') - with open(authorized_file, 'w') as stream: - stream.write('# gitolite start\n') - gitolite_version = APP.config.get('GITOLITE_VERSION', 3) - for user in users: - if not user.public_ssh_key: - continue - if gitolite_version == 2: - row = 'command="/usr/bin/gl-auth-command %s",' \ - 'no-port-forwarding,no-X11-forwarding,'\ - 'no-agent-forwarding,no-pty %s' % ( - user.user, user.public_ssh_key.strip()) - elif gitolite_version == 3: - row = 'command="HOME=%s '\ - '/usr/share/gitolite3/gitolite-shell %s",' \ - 'no-port-forwarding,no-X11-forwarding,'\ - 'no-agent-forwarding,no-pty %s' % ( - gitolite_home, user.user, - user.public_ssh_key.strip()) - else: - raise pagure.exceptions.PagureException( - 'Non-supported gitolite version "%s"' % - gitolite_version) - stream.write(row.encode('utf-8') + '\n') - stream.write('# gitolite end\n') + for user in users: + pagure.lib.update_user_ssh(SESSION, user, user.public_ssh_key, + APP.config.get('GITOLITE_KEYDIR', None)) + pagure.lib.git.generate_gitolite_acls() def cla_required(function): diff --git a/pagure/templates/admin_index.html b/pagure/templates/admin_index.html index e445eb3..3fd5302 100644 --- a/pagure/templates/admin_index.html +++ b/pagure/templates/admin_index.html @@ -19,7 +19,7 @@
  • + value="Re-generate user ssh key files"> {{ form.csrf_token }}
    • diff --git a/pagure/ui/admin.py b/pagure/ui/admin.py index f9ebb02..2588f5b 100644 --- a/pagure/ui/admin.py +++ b/pagure/ui/admin.py @@ -17,7 +17,7 @@ import pagure.exceptions import pagure.forms import pagure.lib import pagure.lib.git -from pagure import (APP, SESSION, +from pagure import (APP, SESSION, generate_user_key_files, is_admin, admin_session_timedout) # pylint: disable=E1101 @@ -73,12 +73,13 @@ def admin_generate_acl(): @APP.route('/admin/ssh', methods=['POST']) @admin_required def admin_refresh_ssh(): - """ Regenerate the gitolite ACL file. """ + """ Regenerate the user key files. """ form = pagure.forms.ConfirmationForm() if form.validate_on_submit(): try: - generate_authorized_key_file() - flask.flash('Authorized file updated') + generate_user_key_files() + pagure.lib.git.generate_gitolite_acls() + flask.flash('User key files regenerated') except pagure.exceptions.PagureException, err: flask.flash(str(err), 'error') return flask.redirect(flask.url_for('admin_index')) diff --git a/pagure/ui/app.py b/pagure/ui/app.py index 0de7c7c..74604d6 100644 --- a/pagure/ui/app.py +++ b/pagure/ui/app.py @@ -399,8 +399,6 @@ def user_settings(): ssh_key=ssh_key, keydir=APP.config.get('GITOLITE_KEYDIR', None), ) - if message != 'Nothing to update': - generate_authorized_key_file() SESSION.commit() flask.flash(message) return flask.redirect( diff --git a/tests/test_progit_flask_ui_admin.py b/tests/test_progit_flask_ui_admin.py index f177e0e..0057be0 100644 --- a/tests/test_progit_flask_ui_admin.py +++ b/tests/test_progit_flask_ui_admin.py @@ -72,7 +72,7 @@ class PagureFlaskAdmintests(tests.Modeltests): self.assertTrue('

    Admin section

    ' in output.data) self.assertTrue('Re-generate gitolite ACLs file' in output.data) self.assertTrue( - 'Re-generate ssh authorized_key file' in output.data) + 'Re-generate user ssh key files' in output.data) @patch('pagure.lib.git.write_gitolite_acls') def test_admin_generate_acl(self, wga): @@ -101,7 +101,7 @@ class PagureFlaskAdmintests(tests.Modeltests): self.assertTrue('

    Admin section

    ' in output.data) self.assertTrue('Re-generate gitolite ACLs file' in output.data) self.assertTrue( - 'Re-generate ssh authorized_key file' in output.data) + 'Re-generate user ssh key files' in output.data) self.assertFalse( '
  • Gitolite ACLs updated
    • ' in output.data) @@ -116,12 +116,12 @@ class PagureFlaskAdmintests(tests.Modeltests): self.assertTrue('

    Admin section

    ' in output.data) self.assertTrue('Re-generate gitolite ACLs file' in output.data) self.assertTrue( - 'Re-generate ssh authorized_key file' in output.data) + 'Re-generate user ssh key files' in output.data) self.assertTrue( '
  • Gitolite ACLs updated
    • ' in output.data) - @patch('pagure.generate_authorized_key_file') + @patch('pagure.generate_user_key_files') def test_admin_refresh_ssh(self, gakf): """ Test the admin_refresh_ssh endpoint. """ gakf.return_value = True @@ -148,7 +148,7 @@ class PagureFlaskAdmintests(tests.Modeltests): self.assertTrue('

    Admin section

    ' in output.data) self.assertTrue('Re-generate gitolite ACLs file' in output.data) self.assertTrue( - 'Re-generate ssh authorized_key file' in output.data) + 'Re-generate user ssh key files' in output.data) self.assertFalse( '
  • Authorized file updated
    • ' in output.data) @@ -163,9 +163,9 @@ class PagureFlaskAdmintests(tests.Modeltests): self.assertTrue('

    Admin section

    ' in output.data) self.assertTrue('Re-generate gitolite ACLs file' in output.data) self.assertTrue( - 'Re-generate ssh authorized_key file' in output.data) + 'Re-generate user ssh key files' in output.data) self.assertTrue( - '
  • Authorized file updated
    • ' + '
  • User key files regenerated
    • ' in output.data) def test_admin_generate_hook_token(self): @@ -193,7 +193,7 @@ class PagureFlaskAdmintests(tests.Modeltests): self.assertTrue('

    Admin section

    ' in output.data) self.assertTrue('Re-generate gitolite ACLs file' in output.data) self.assertTrue( - 'Re-generate ssh authorized_key file' in output.data) + 'Re-generate user ssh key files' in output.data) self.assertTrue( 'Re-generate hook-token for every projects' in output.data) @@ -207,7 +207,7 @@ class PagureFlaskAdmintests(tests.Modeltests): self.assertTrue('

    Admin section

    ' in output.data) self.assertTrue('Re-generate gitolite ACLs file' in output.data) self.assertTrue( - 'Re-generate ssh authorized_key file' in output.data) + 'Re-generate user ssh key files' in output.data) self.assertTrue( 'Re-generate hook-token for every projects' in output.data) self.assertTrue( From eb310ac267d601f101e96fd0b379c5bdfc2e772f Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Sep 27 2015 19:22:20 +0000 Subject: [PATCH 3/6] Adjust update_user_ssh to always write the key and drop the returned message The way this function is called is such that checking if the key provided is the same as the key in place does not actually make sense. So we'll just update the key in place with the provided one and adjust it in the filesystem as well. --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 7ba6349..b42fe42 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -1851,18 +1851,11 @@ def update_user_ssh(session, user, ssh_key, keydir): if isinstance(user, basestring): user = __get_user(session, user) - message = 'Nothing to update' - - if ssh_key != user.public_ssh_key: - user.public_ssh_key = ssh_key - if keydir: - create_user_ssh_keys_on_disk(user, keydir) - pagure.lib.git.generate_gitolite_acls() - session.add(user) - session.flush() - message = 'Public ssh key updated' - - return message + user.public_ssh_key = ssh_key + if keydir and user.public_ssh_key: + create_user_ssh_keys_on_disk(user, keydir) + session.add(user) + session.flush() def avatar_url(username, size=64, default='retro'): From 11f8ab78038319c03f423c60560de77ad38a891a Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Sep 27 2015 19:23:46 +0000 Subject: [PATCH 4/6] "os.path.rm" does not exist, "os.unlink" does though --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index b42fe42..c3f2477 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -167,7 +167,7 @@ def create_user_ssh_keys_on_disk(user, gitolite_keydir): 'keys_%i' % i, '%s.pub' % user.user) while os.path.exists(keyline_file): - os.path.rm(keyline_file) + os.unlink(keyline_file) i += 1 keyline_file = os.path.join(gitolite_keydir, 'keys_%i' % i, From f365aefe6aff3392f629cbd38cca59d2f466573a Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Sep 27 2015 19:24:27 +0000 Subject: [PATCH 5/6] Adjust the UI to only update the ssh key when it makes sense to do so --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index c3f2477..1559fac 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -152,6 +152,7 @@ def search_user(session, username=None, email=None, token=None, pattern=None): return output + def create_user_ssh_keys_on_disk(user, gitolite_keydir): if gitolite_keydir: # First remove any old keyfiles for the user @@ -183,6 +184,7 @@ def create_user_ssh_keys_on_disk(user, gitolite_keydir): with open(keyfile, 'w') as stream: stream.write(keys[i].strip().encode('UTF-8')) + def add_issue_comment(session, issue, comment, user, ticketfolder, notify=True, redis=None): ''' Add a comment to an issue. ''' diff --git a/pagure/ui/app.py b/pagure/ui/app.py index 74604d6..a0ab3db 100644 --- a/pagure/ui/app.py +++ b/pagure/ui/app.py @@ -393,13 +393,16 @@ def user_settings(): ssh_key = form.ssh_key.data try: - message = pagure.lib.update_user_ssh( - SESSION, - user=user, - ssh_key=ssh_key, - keydir=APP.config.get('GITOLITE_KEYDIR', None), - ) - SESSION.commit() + message = 'Nothing to update' + if user.public_ssh_key != ssh_key: + pagure.lib.update_user_ssh( + SESSION, + user=user, + ssh_key=ssh_key, + keydir=APP.config.get('GITOLITE_KEYDIR', None), + ) + SESSION.commit() + message = 'Public ssh key updated' flask.flash(message) return flask.redirect( flask.url_for('view_user', username=user.user)) From 1b47d54fa1cc645bf54aa16c033e3a3a734051f4 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Sep 27 2015 19:25:04 +0000 Subject: [PATCH 6/6] Adjust unit-tests for the change in behaviour in update_user_ssh --- diff --git a/tests/test_progit_lib.py b/tests/test_progit_lib.py index 0d2a172..0f00f10 100644 --- a/tests/test_progit_lib.py +++ b/tests/test_progit_lib.py @@ -1056,13 +1056,16 @@ class PagureLibtests(tests.Modeltests): self.assertEqual(user.public_ssh_key, None) msg = pagure.lib.update_user_ssh(self.session, user, 'blah', keydir=None) - self.assertEqual(msg, 'Public ssh key updated') + user = pagure.lib.search_user(self.session, username='foo') + self.assertEqual(user.public_ssh_key, 'blah') msg = pagure.lib.update_user_ssh(self.session, user, 'blah', keydir=None) - self.assertEqual(msg, 'Nothing to update') + user = pagure.lib.search_user(self.session, username='foo') + self.assertEqual(user.public_ssh_key, 'blah') msg = pagure.lib.update_user_ssh(self.session, 'foo', None, keydir=None) - self.assertEqual(msg, 'Public ssh key updated') + user = pagure.lib.search_user(self.session, username='foo') + self.assertEqual(user.public_ssh_key, None) def test_avatar_url(self): """ Test the avatar_url of pagure.lib. """