# HG changeset patch # User Mads Kiilerich # Date 1547159458 -3600 # Node ID 16df4993b4426688b364315734f1f7603bd37fd2 # Parent a93b8a544f83ead9d9698622d000357c455a66a3 scm: don't try to get IP address from web request in model Remove a layering violation and make functions more reusable when they no longer depend on global state. At this level, the IP address (and information about the current user) is only used for hooks logging push / pull operations. Arguably, IP address logging only belongs in an HTTP access log, not in the log of push/pull operations. But as long as we have IP addresses in the logs, we have to provide it. The (good?) alternative would be to drop IP address from the push / pull logs ... diff -r a93b8a544f83 -r 16df4993b442 kallithea/controllers/admin/gists.py --- a/kallithea/controllers/admin/gists.py Thu Jan 10 03:35:01 2019 +0100 +++ b/kallithea/controllers/admin/gists.py Thu Jan 10 23:30:58 2019 +0100 @@ -118,6 +118,7 @@ gist = GistModel().create( description=form_result['description'], owner=request.authuser.user_id, + ip_addr=request.ip_addr, gist_mapping=nodes, gist_type=gist_type, lifetime=form_result['lifetime'] @@ -215,7 +216,8 @@ GistModel().update( gist=c.gist, description=rpost['description'], - owner=c.gist.owner, + owner=c.gist.owner, # FIXME: request.authuser.user_id ? + ip_addr=request.ip_addr, gist_mapping=nodes, gist_type=c.gist.gist_type, lifetime=rpost['lifetime'] diff -r a93b8a544f83 -r 16df4993b442 kallithea/controllers/admin/repos.py --- a/kallithea/controllers/admin/repos.py Thu Jan 10 03:35:01 2019 +0100 +++ b/kallithea/controllers/admin/repos.py Thu Jan 10 23:30:58 2019 +0100 @@ -501,7 +501,7 @@ c.active = 'remote' if request.POST: try: - ScmModel().pull_changes(repo_name, request.authuser.username) + ScmModel().pull_changes(repo_name, request.authuser.username, request.ip_addr) h.flash(_('Pulled from remote location'), category='success') except Exception as e: log.error(traceback.format_exc()) diff -r a93b8a544f83 -r 16df4993b442 kallithea/controllers/api/api.py --- a/kallithea/controllers/api/api.py Thu Jan 10 03:35:01 2019 +0100 +++ b/kallithea/controllers/api/api.py Thu Jan 10 23:30:58 2019 +0100 @@ -206,6 +206,7 @@ try: ScmModel().pull_changes(repo.repo_name, request.authuser.username, + request.ip_addr, clone_uri=Optional.extract(clone_uri)) return dict( msg='Pulled from `%s`' % repo.repo_name, @@ -2269,6 +2270,7 @@ gist = GistModel().create(description=description, owner=owner, + ip_addr=request.ip_addr, gist_mapping=files, gist_type=gist_type, lifetime=lifetime) diff -r a93b8a544f83 -r 16df4993b442 kallithea/controllers/files.py --- a/kallithea/controllers/files.py Thu Jan 10 03:35:01 2019 +0100 +++ b/kallithea/controllers/files.py Thu Jan 10 23:30:58 2019 +0100 @@ -326,7 +326,9 @@ } } self.scm_model.delete_nodes( - user=request.authuser.user_id, repo=c.db_repo, + user=request.authuser.user_id, + ip_addr=request.ip_addr, + repo=c.db_repo, message=message, nodes=nodes, parent_cs=c.cs, @@ -389,6 +391,7 @@ self.scm_model.commit_change(repo=c.db_repo_scm_instance, repo_name=repo_name, cs=c.cs, user=request.authuser.user_id, + ip_addr=request.ip_addr, author=author, message=message, content=content, f_path=f_path) h.flash(_('Successfully committed to %s') % f_path, @@ -450,7 +453,9 @@ } } self.scm_model.create_nodes( - user=request.authuser.user_id, repo=c.db_repo, + user=request.authuser.user_id, + ip_addr=request.ip_addr, + repo=c.db_repo, message=message, nodes=nodes, parent_cs=c.cs, diff -r a93b8a544f83 -r 16df4993b442 kallithea/model/gist.py --- a/kallithea/model/gist.py Thu Jan 10 03:35:01 2019 +0100 +++ b/kallithea/model/gist.py Thu Jan 10 23:30:58 2019 +0100 @@ -97,7 +97,7 @@ cs = repo.scm_instance.get_changeset(revision) return cs, [n for n in cs.get_node('/')] - def create(self, description, owner, gist_mapping, + def create(self, description, owner, ip_addr, gist_mapping, gist_type=Gist.GIST_PUBLIC, lifetime=-1): """ @@ -159,7 +159,9 @@ scm_instance_no_cache=lambda: repo, )) ScmModel().create_nodes( - user=owner.user_id, repo=fake_repo, + user=owner.user_id, + ip_addr=ip_addr, + repo=fake_repo, message=message, nodes=processed_mapping, trigger_push_hook=False @@ -181,7 +183,7 @@ log.error(traceback.format_exc()) raise - def update(self, gist, description, owner, gist_mapping, gist_type, + def update(self, gist, description, owner, ip_addr, gist_mapping, gist_type, lifetime): gist = Gist.guess_instance(gist) gist_repo = gist.scm_instance @@ -226,6 +228,7 @@ ScmModel().update_nodes( user=owner.user_id, + ip_addr=ip_addr, repo=fake_repo, message=message, nodes=gist_mapping_op, diff -r a93b8a544f83 -r 16df4993b442 kallithea/model/scm.py --- a/kallithea/model/scm.py Thu Jan 10 03:35:01 2019 +0100 +++ b/kallithea/model/scm.py Thu Jan 10 23:30:58 2019 +0100 @@ -327,20 +327,11 @@ repo.fork = fork return repo - def _handle_rc_scm_extras(self, username, repo_name, repo_alias, + def _handle_rc_scm_extras(self, username, ip_addr, repo_name, repo_alias, action=None): from kallithea import CONFIG - from kallithea.lib.base import _get_ip_addr - try: - from tg import request - environ = request.environ - except TypeError: - # we might use this outside of request context, let's fake the - # environ data - from webob import Request - environ = Request.blank('').environ extras = { - 'ip': _get_ip_addr(environ), + 'ip': ip_addr, 'username': username, 'action': action or 'push_local', 'repository': repo_name, @@ -349,7 +340,7 @@ } _set_extras(extras) - def _handle_push(self, repo, username, action, repo_name, revisions): + def _handle_push(self, repo, username, ip_addr, action, repo_name, revisions): """ Handle that the repository has changed. Adds an action log entry with the new revisions, and the head revision @@ -360,7 +351,7 @@ :param repo_name: name of repo :param revisions: list of revisions that we pushed """ - self._handle_rc_scm_extras(username, repo_name, repo_alias=repo.alias, action=action) + self._handle_rc_scm_extras(username, ip_addr, repo_name, repo_alias=repo.alias, action=action) process_pushed_raw_ids(revisions) # also calls mark_for_invalidation def _get_IMC_module(self, scm_type): @@ -380,7 +371,7 @@ raise Exception('Invalid scm_type, must be one of hg,git got %s' % (scm_type,)) - def pull_changes(self, repo, username, clone_uri=None): + def pull_changes(self, repo, username, ip_addr, clone_uri=None): """ Pull from "clone URL" or fork origin. """ @@ -400,11 +391,12 @@ # TODO: extract fetched revisions ... somehow ... self._handle_push(repo, username=username, + ip_addr=ip_addr, action='push_remote', repo_name=repo_name, revisions=[]) else: - self._handle_rc_scm_extras(username, dbrepo.repo_name, + self._handle_rc_scm_extras(username, ip_addr, dbrepo.repo_name, repo.alias, action='push_remote') repo.pull(clone_uri) @@ -413,7 +405,7 @@ log.error(traceback.format_exc()) raise - def commit_change(self, repo, repo_name, cs, user, author, message, + def commit_change(self, repo, repo_name, cs, user, ip_addr, author, message, content, f_path): """ Commit a change to a single file @@ -444,6 +436,7 @@ self.mark_for_invalidation(repo_name) self._handle_push(repo, username=user.username, + ip_addr=ip_addr, action='push_local', repo_name=repo_name, revisions=[tip.raw_id]) @@ -485,7 +478,7 @@ return _dirs, _files - def create_nodes(self, user, repo, message, nodes, parent_cs=None, + def create_nodes(self, user, ip_addr, repo, message, nodes, parent_cs=None, author=None, trigger_push_hook=True): """ Commits specified nodes to repo. @@ -549,12 +542,13 @@ if trigger_push_hook: self._handle_push(scm_instance, username=user.username, + ip_addr=ip_addr, action='push_local', repo_name=repo.repo_name, revisions=[tip.raw_id]) return tip - def update_nodes(self, user, repo, message, nodes, parent_cs=None, + def update_nodes(self, user, ip_addr, repo, message, nodes, parent_cs=None, author=None, trigger_push_hook=True): """ Commits specified nodes to repo. Again. @@ -609,11 +603,12 @@ if trigger_push_hook: self._handle_push(scm_instance, username=user.username, + ip_addr=ip_addr, action='push_local', repo_name=repo.repo_name, revisions=[tip.raw_id]) - def delete_nodes(self, user, repo, message, nodes, parent_cs=None, + def delete_nodes(self, user, ip_addr, repo, message, nodes, parent_cs=None, author=None, trigger_push_hook=True): """ Deletes specified nodes from repo. @@ -668,6 +663,7 @@ if trigger_push_hook: self._handle_push(scm_instance, username=user.username, + ip_addr=ip_addr, action='push_local', repo_name=repo.repo_name, revisions=[tip.raw_id]) diff -r a93b8a544f83 -r 16df4993b442 kallithea/tests/base.py --- a/kallithea/tests/base.py Thu Jan 10 03:35:01 2019 +0100 +++ b/kallithea/tests/base.py Thu Jan 10 23:30:58 2019 +0100 @@ -45,8 +45,8 @@ 'HG_FORK', 'GIT_FORK', 'TEST_USER_ADMIN_LOGIN', 'TEST_USER_ADMIN_PASS', 'TEST_USER_ADMIN_EMAIL', 'TEST_USER_REGULAR_LOGIN', 'TEST_USER_REGULAR_PASS', 'TEST_USER_REGULAR_EMAIL', 'TEST_USER_REGULAR2_LOGIN', - 'TEST_USER_REGULAR2_PASS', 'TEST_USER_REGULAR2_EMAIL', 'TEST_HG_REPO', - 'TEST_HG_REPO_CLONE', 'TEST_HG_REPO_PULL', 'TEST_GIT_REPO', + 'TEST_USER_REGULAR2_PASS', 'TEST_USER_REGULAR2_EMAIL', 'IP_ADDR', + 'TEST_HG_REPO', 'TEST_HG_REPO_CLONE', 'TEST_HG_REPO_PULL', 'TEST_GIT_REPO', 'TEST_GIT_REPO_CLONE', 'TEST_GIT_REPO_PULL', 'HG_REMOTE_REPO', 'GIT_REMOTE_REPO', 'HG_TEST_REVISION', 'GIT_TEST_REVISION', ] @@ -67,6 +67,8 @@ TEST_USER_REGULAR2_PASS = 'test12' TEST_USER_REGULAR2_EMAIL = 'test_regular2@example.com' +IP_ADDR = '127.0.0.127' + HG_REPO = u'vcs_test_hg' GIT_REPO = u'vcs_test_git' diff -r a93b8a544f83 -r 16df4993b442 kallithea/tests/fixture.py --- a/kallithea/tests/fixture.py Thu Jan 10 03:35:01 2019 +0100 +++ b/kallithea/tests/fixture.py Thu Jan 10 23:30:58 2019 +0100 @@ -42,7 +42,7 @@ from kallithea.lib.db_manage import DbManage from kallithea.lib.vcs.backends.base import EmptyChangeset from kallithea.tests.base import invalidate_all_caches, GIT_REPO, HG_REPO, \ - TESTS_TMP_PATH, TEST_USER_ADMIN_LOGIN, TEST_USER_REGULAR_LOGIN, TEST_USER_ADMIN_EMAIL + TESTS_TMP_PATH, TEST_USER_ADMIN_LOGIN, TEST_USER_REGULAR_LOGIN, TEST_USER_ADMIN_EMAIL, IP_ADDR log = logging.getLogger(__name__) @@ -261,7 +261,7 @@ } form_data.update(kwargs) gist = GistModel().create( - description=form_data['description'], owner=form_data['owner'], + description=form_data['description'], owner=form_data['owner'], ip_addr=IP_ADDR, gist_mapping=form_data['gist_mapping'], gist_type=form_data['gist_type'], lifetime=form_data['lifetime'] ) @@ -302,7 +302,9 @@ } } cs = ScmModel().create_nodes( - user=TEST_USER_ADMIN_LOGIN, repo=repo, + user=TEST_USER_ADMIN_LOGIN, + ip_addr=IP_ADDR, + repo=repo, message=message, nodes=nodes, parent_cs=_cs, @@ -311,7 +313,9 @@ else: cs = ScmModel().commit_change( repo=repo.scm_instance, repo_name=repo.repo_name, - cs=parent, user=TEST_USER_ADMIN_LOGIN, + cs=parent, + user=TEST_USER_ADMIN_LOGIN, + ip_addr=IP_ADDR, author=author, message=message, content=content, diff -r a93b8a544f83 -r 16df4993b442 kallithea/tests/functional/test_admin_gists.py --- a/kallithea/tests/functional/test_admin_gists.py Thu Jan 10 03:35:01 2019 +0100 +++ b/kallithea/tests/functional/test_admin_gists.py Thu Jan 10 23:30:58 2019 +0100 @@ -11,7 +11,7 @@ f_name: {'content': content} } owner = User.get_by_username(owner) - gist = GistModel().create(description, owner=owner, + gist = GistModel().create(description, owner=owner, ip_addr=IP_ADDR, gist_mapping=gist_mapping, gist_type=gist_type, lifetime=lifetime) Session().commit()