kallithea Changeset - 16df4993b442

Changeset - 16df4993b442

Parent rev.

Child rev.

[Not reviewed]

default

0 9 0

Mads Kiilerich - 7 years ago 2019-01-10 23:30:58
mads@kiilerich.com

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 ...

9 files changed with 47 insertions and 33 deletions:

kallithea/controllers/admin/gists.py

kallithea/controllers/admin/repos.py

kallithea/controllers/api/api.py

kallithea/controllers/files.py

kallithea/model/gist.py

kallithea/model/scm.py

kallithea/tests/base.py

kallithea/tests/fixture.py

kallithea/tests/functional/test_admin_gists.py

0 comments (0 inline, 0 general)

kallithea/controllers/admin/gists.py

➞

Show inline comments

@@ @@ -118,6 +118,7 @@ class GistsController(BaseController): @@
             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 @@ class GistsController(BaseController): @@
                 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']

kallithea/controllers/admin/repos.py

➞

Show inline comments

@@ @@ -501,7 +501,7 @@ class ReposController(BaseRepoController @@
         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())

kallithea/controllers/api/api.py

➞

Show inline comments

@@ @@ -206,6 +206,7 @@ class ApiController(JSONRPCController): @@
         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 @@ class ApiController(JSONRPCController): @@
             gist = GistModel().create(description=description,
                                       owner=owner,
                                       ip_addr=request.ip_addr,
                                       gist_mapping=files,
                                       gist_type=gist_type,
                                       lifetime=lifetime)

kallithea/controllers/files.py

➞

Show inline comments

@@ @@ -326,7 +326,9 @@ class FilesController(BaseRepoController @@
+                    }
+                }
                 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 @@ class FilesController(BaseRepoController @@
                 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 @@ class FilesController(BaseRepoController @@
+                    }
+                }
                 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,

kallithea/model/gist.py

➞

Show inline comments

@@ @@ -97,7 +97,7 @@ class GistModel(object): @@
         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 @@ class GistModel(object): @@
             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 @@ class GistModel(object): @@
             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 @@ class GistModel(object): @@
         ScmModel().update_nodes(
             user=owner.user_id,
             ip_addr=ip_addr,
             repo=fake_repo,
             message=message,
             nodes=gist_mapping_op,

kallithea/model/scm.py

➞

Show inline comments

@@ @@ -327,20 +327,11 @@ class ScmModel(object): @@
         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 @@ class ScmModel(object): @@
+        }
         _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 @@ class ScmModel(object): @@
         :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 @@ class ScmModel(object): @@
         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 @@ class ScmModel(object): @@
                 # 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 @@ class ScmModel(object): @@
             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 @@ class ScmModel(object): @@
             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 @@ class ScmModel(object): @@
         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 @@ class ScmModel(object): @@
         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 @@ class ScmModel(object): @@
         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 @@ class ScmModel(object): @@
         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])

kallithea/tests/base.py

➞

Show inline comments

@@ @@ -45,8 +45,8 @@ __all__ = [ @@
     '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_LOGIN = 'test_regular @@
 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'

kallithea/tests/fixture.py

➞

Show inline comments

@@ @@ -42,7 +42,7 @@ from kallithea.lib.auth import AuthUser @@
 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 @@ class Fixture(object): @@
+        }
         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 @@ class Fixture(object): @@
+                }
+            }
             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 @@ class Fixture(object): @@
         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,

kallithea/tests/functional/test_admin_gists.py

➞

Show inline comments

@@ @@ -11,7 +11,7 @@ def _create_gist(f_name, content='some g @@
         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()

0 comments (0 inline, 0 general)