release: Merge default into stable for release preparation

docs/release-notes/release-notes-4.26.0.rst

@@ -0,0 +1,55 b''
+|RCE| 4.26.0 |RNS|
+------------------
+
+Release Date
+^^^^^^^^^^^^
+
+- 2021-08-06
+
+
+New Features
+^^^^^^^^^^^^
+
+
+
+General
+^^^^^^^
+
+- Caches: introduce invalidation as a safer ways to expire keys, deleting them are more problematic.
+- Caches: improved locking problems with distributed lock new cache backend.
+- Pull requests: optimize db transaction logic.
+  This should prevent potential problems with locking of pull-requests that have a lot of reviewers.
+- Pull requests: updates use retry logic in case of update is locked/fails for some concurrency issues.
+- Pull requests: allow forced state change to repo admins too.
+- SSH: handle subrepos better when using SSH communication.
+
+
+Security
+^^^^^^^^
+
+- Drafts comments: don't allow to view history for others than owner.
+- Validators: apply username validator to prevent bad values being searched in DB, and potential XSS payload sent via validators.
+
+
+Performance
+^^^^^^^^^^^
+
+- SSH: use pre-compiled backends for faster matching of vcs detection.
+- Routing: don't check channelstream connections for faster handling of this route.
+- Routing: skip vcsdetection for ops view so they are not checked against the vcs operations.
+
+
+Fixes
+^^^^^
+
+- Permissions: flush all users permissions when creating a new user group.
+- Repos: recover properly from bad extraction of repo_id from URL and DB calls.
+- Comments history: fixed fetching of history for comments
+- Pull requests: fix potential crash on providing a wrong order-by type column.
+- Caches: report damaged DB on key iterations too not only the GET call
+- API: added proper full permission flush on API calls when creating repos and repo groups.
+
+Upgrade notes
+^^^^^^^^^^^^^
+
+- Scheduled release 4.26.0.

.bumpversion.cfg

@@ -1,6 +1,5 b''
 [bumpversion]
-current_version = 4.25.2
+current_version = 4.26.0
 message = release: Bump version {current_version} to {new_version}
 
 [bumpversion:file:rhodecode/VERSION]
-

.release.cfg

@@ -5,25 +5,20 b' done = false'
 done = true
 
 [task:rc_tools_pinned]
-done = true
 
 [task:fixes_on_stable]
-done = true
 
 [task:pip2nix_generated]
-done = true
 
 [task:changelog_updated]
-done = true
 
 [task:generate_api_docs]
-done = true
+
+[task:updated_translation]
 
 [release]
-state = prepared
-version = 4.25.2
-
-[task:updated_translation]
+state = in_progress
+version = 4.26.0
 
 [task:generate_js_routes]
 

configs/production.ini

@@ -391,6 +391,8 b' rc_cache.cache_perms.expiration_time = 3'
 ; more Redis options: https://dogpilecache.sqlalchemy.org/en/latest/api.html#redis-backends
 #rc_cache.cache_perms.arguments.distributed_lock = true
 
+; auto-renew lock to prevent stale locks, slower but safer. Use only if problems happen
+#rc_cache.cache_perms.arguments.lock_auto_renewal = true
 
 ; ***************************************************
 ; `cache_repo` cache for file tree, Readme, RSS FEEDS
@@ -414,6 +416,8 b' rc_cache.cache_repo.expiration_time = 25'
 ; more Redis options: https://dogpilecache.sqlalchemy.org/en/latest/api.html#redis-backends
 #rc_cache.cache_repo.arguments.distributed_lock = true
 
+; auto-renew lock to prevent stale locks, slower but safer. Use only if problems happen
+#rc_cache.cache_repo.arguments.lock_auto_renewal = true
 
 ; ##############
 ; BEAKER SESSION

docs/release-notes/release-notes.rst

@@ -9,6 +9,7 b' Release Notes'
 .. toctree::
    :maxdepth: 1
 
+   release-notes-4.26.0.rst
    release-notes-4.25.2.rst
    release-notes-4.25.1.rst
    release-notes-4.25.0.rst

pkgs/python-packages.nix

@@ -1883,7 +1883,7 b' self: super: {'
     };
   };
   "rhodecode-enterprise-ce" = super.buildPythonPackage {
-    name = "rhodecode-enterprise-ce-4.25.2";
+    name = "rhodecode-enterprise-ce-4.26.0";
     buildInputs = [
       self."pytest"
       self."py"

rhodecode/VERSION

@@ -1,1 +1,1 b''
-4.25.2
  No newline at end of file
+4.26.0
  No newline at end of file

rhodecode/apps/_base/subscribers.py

@@ -41,13 +41,14 b' def trigger_user_permission_flush(event)'
     automatic flush of permission caches, so the users affected receive new permissions
     Right Away
     """
-
+    invalidate = True
     affected_user_ids = set(event.user_ids)
     for user_id in affected_user_ids:
         for cache_namespace_uid_tmpl in cache_namespaces:
             cache_namespace_uid = cache_namespace_uid_tmpl.format(user_id)
-            del_keys = rc_cache.clear_cache_namespace('cache_perms', cache_namespace_uid)
-            log.debug('Deleted %s cache keys for user_id: %s and namespace %s',
+            del_keys = rc_cache.clear_cache_namespace(
+                'cache_perms', cache_namespace_uid, invalidate=invalidate)
+            log.debug('Invalidated %s cache keys for user_id: %s and namespace %s',
                       del_keys, user_id, cache_namespace_uid)
 
 

rhodecode/apps/admin/views/user_groups.py

@@ -247,8 +247,7 b' class AdminUserGroupsView(BaseAppView, D'
                     % user_group_name, category='error')
             raise HTTPFound(h.route_path('user_groups_new'))
 
-        affected_user_ids = [self._rhodecode_user.user_id]
-        PermissionModel().trigger_permission_flush(affected_user_ids)
+        PermissionModel().trigger_permission_flush()
 
         raise HTTPFound(
             h.route_path('edit_user_group', user_group_id=user_group_id))

rhodecode/apps/repository/__init__.py

@@ -173,7 +173,7 b' def includeme(config):'
 
     config.add_route(
         name='repo_commit_comment_history_view',
-        pattern='/{repo_name:.*?[^/]}/changeset/{commit_id}/comment/{comment_history_id}/history_view', repo_route=True)
+        pattern='/{repo_name:.*?[^/]}/changeset/{commit_id}/comment/{comment_id}/history_view/{comment_history_id}', repo_route=True)
     config.add_view(
         RepoCommitsView,
         attr='repo_commit_comment_history_view',

rhodecode/apps/repository/views/repo_changelog.py

@@ -72,7 +72,7 b' class RepoChangelogView(RepoAppView):'
             h.flash(msg, category='error')
             raise HTTPNotFound()
         except RepositoryError as e:
-            h.flash(safe_str(h.escape(e)), category='error')
+            h.flash(h.escape(safe_str(e)), category='error')
             raise HTTPNotFound()
 
     def _graph(self, repo, commits, prev_data=None, next_data=None):
@@ -238,14 +238,14 b' class RepoChangelogView(RepoAppView):'
                 f_path=f_path, commit_id=commit_id)
 
         except EmptyRepositoryError as e:
-            h.flash(safe_str(h.escape(e)), category='warning')
+            h.flash(h.escape(safe_str(e)), category='warning')
             raise HTTPFound(
                 h.route_path('repo_summary', repo_name=self.db_repo_name))
         except HTTPFound:
             raise
         except (RepositoryError, CommitDoesNotExistError, Exception) as e:
             log.exception(safe_str(e))
-            h.flash(safe_str(h.escape(e)), category='error')
+            h.flash(h.escape(safe_str(e)), category='error')
 
             if commit_id:
                 # from single commit page, we redirect to main commits

rhodecode/apps/repository/views/repo_commits.py

@@ -539,9 +539,10 b' class RepoCommitsView(RepoAppView):'
     @CSRFRequired()
     def repo_commit_comment_history_view(self):
         c = self.load_default_context()
+        comment_id = self.request.matchdict['comment_id']
         comment_history_id = self.request.matchdict['comment_history_id']
 
-        comment = ChangesetComment.get_or_404(comment_history_id)
+        comment = ChangesetComment.get_or_404(comment_id)
         comment_owner = (comment.author.user_id == self._rhodecode_db_user.user_id)
         if comment.draft and not comment_owner:
             # if we see draft comments history, we only allow this for owner

rhodecode/apps/repository/views/repo_compare.py

@@ -70,7 +70,7 b' class RepoCompareView(RepoAppView):'
 
         except RepositoryError as e:
             log.exception(safe_str(e))
-            h.flash(safe_str(h.escape(e)), category='warning')
+            h.flash(h.escape(safe_str(e)), category='warning')
             if not partial:
                 raise HTTPFound(
                     h.route_path('repo_summary', repo_name=repo.repo_name))

rhodecode/apps/repository/views/repo_files.py

@@ -186,7 +186,7 b' class RepoFilesView(RepoAppView):'
             h.flash(msg, category='error')
             raise HTTPNotFound()
         except RepositoryError as e:
-            h.flash(safe_str(h.escape(e)), category='error')
+            h.flash(h.escape(safe_str(e)), category='error')
             raise HTTPNotFound()
 
     def _get_filenode_or_redirect(self, commit_obj, path):
@@ -206,7 +206,7 b' class RepoFilesView(RepoAppView):'
             raise HTTPNotFound()
         except RepositoryError as e:
             log.warning('Repository error while fetching filenode `%s`. Err:%s', path, e)
-            h.flash(safe_str(h.escape(e)), category='error')
+            h.flash(h.escape(safe_str(e)), category='error')
             raise HTTPNotFound()
 
         return file_node
@@ -733,7 +733,7 b' class RepoFilesView(RepoAppView):'
                                           c.commit.raw_id, f_path)
 
         except RepositoryError as e:
-            h.flash(safe_str(h.escape(e)), category='error')
+            h.flash(h.escape(safe_str(e)), category='error')
             raise HTTPNotFound()
 
         if self.request.environ.get('HTTP_X_PJAX'):
@@ -927,7 +927,7 b' class RepoFilesView(RepoAppView):'
                 _d, _f = ScmModel().get_quick_filter_nodes(repo_name, _commit_id, _f_path)
             except (RepositoryError, CommitDoesNotExistError, Exception) as e:
                 log.exception(safe_str(e))
-                h.flash(safe_str(h.escape(e)), category='error')
+                h.flash(h.escape(safe_str(e)), category='error')
                 raise HTTPFound(h.route_path(
                     'repo_files', repo_name=self.db_repo_name,
                     commit_id='tip', f_path='/'))
@@ -1444,7 +1444,7 b' class RepoFilesView(RepoAppView):'
                       'contain .. in the path'), category='warning')
             raise HTTPFound(default_redirect_url)
         except (NodeError, NodeAlreadyExistsError) as e:
-            h.flash(_(h.escape(e)), category='error')
+            h.flash(h.escape(safe_str(e)), category='error')
         except Exception:
             log.exception('Error occurred during commit')
             h.flash(_('Error occurred during commit'), category='error')

rhodecode/lib/_vendor/redis_lock/__init__.py

@@ -10,7 +10,7 b' from redis import StrictRedis'
 __version__ = '3.7.0'
 
 loggers = {
-    k: getLogger("rhodecode" + ".".join((__name__, k)))
+    k: getLogger("rhodecode." + ".".join((__name__, k)))
     for k in [
         "acquire",
         "refresh.thread.start",
@@ -221,10 +221,11 b' class Lock(object):'
         """
         logger = loggers["acquire"]
 
-        logger.debug("Getting %r ...", self._name)
+        logger.debug("Getting acquire on %r ...", self._name)
 
         if self._held:
-            raise AlreadyAcquired("Already acquired from this Lock instance.")
+            owner_id = self.get_owner_id()
+            raise AlreadyAcquired("Already acquired from this Lock instance. Lock id: {}".format(owner_id))
 
         if not blocking and timeout is not None:
             raise TimeoutNotUsable("Timeout cannot be used if blocking=False")

rhodecode/lib/middleware/vcs.py

@@ -166,6 +166,9 b' def detect_vcs_request(environ, backends'
         # static files no detection
         '_static',
 
+        # skip ops ping
+        '_admin/ops/ping',
+
         # full channelstream connect should be VCS skipped
         '_admin/channelstream/connect',
     ]

rhodecode/lib/rc_cache/backends.py

@@ -33,6 +33,8 b' from dogpile.cache.backends import redis'
 from dogpile.cache.backends.file import NO_VALUE, compat, FileLock
 from dogpile.cache.util import memoized_property
 
+from pyramid.settings import asbool
+
 from rhodecode.lib.memory_lru_dict import LRUDict, LRUDictDebug
 
 
@@ -224,6 +226,16 b' class FileNamespaceBackend(PickleSeriali'
 
 
 class BaseRedisBackend(redis_backend.RedisBackend):
+    key_prefix = ''
+
+    def __init__(self, arguments):
+        super(BaseRedisBackend, self).__init__(arguments)
+        self._lock_timeout = self.lock_timeout
+        self._lock_auto_renewal = asbool(arguments.pop("lock_auto_renewal", True))
+
+        if self._lock_auto_renewal and not self._lock_timeout:
+            # set default timeout for auto_renewal
+            self._lock_timeout = 30
 
     def _create_client(self):
         args = {}
@@ -287,17 +299,10 b' class BaseRedisBackend(redis_backend.Red'
 
     def get_mutex(self, key):
         if self.distributed_lock:
-            import redis_lock
             lock_key = redis_backend.u('_lock_{0}').format(key)
             log.debug('Trying to acquire Redis lock for key %s', lock_key)
-            lock = redis_lock.Lock(
-                redis_client=self.client,
-                name=lock_key,
-                expire=self.lock_timeout,
-                auto_renewal=False,
-                strict=True,
-            )
-            return lock
+            return get_mutex_lock(self.client, lock_key, self._lock_timeout,
+                                  auto_renewal=self._lock_auto_renewal)
         else:
             return None
 
@@ -310,3 +315,40 b' class RedisPickleBackend(PickleSerialize'
 class RedisMsgPackBackend(MsgPackSerializer, BaseRedisBackend):
     key_prefix = 'redis_msgpack_backend'
     pass
+
+
+def get_mutex_lock(client, lock_key, lock_timeout, auto_renewal=False):
+    import redis_lock
+
+    class _RedisLockWrapper(object):
+        """LockWrapper for redis_lock"""
+
+        @classmethod
+        def get_lock(cls):
+            return redis_lock.Lock(
+                redis_client=client,
+                name=lock_key,
+                expire=lock_timeout,
+                auto_renewal=auto_renewal,
+                strict=True,
+            )
+
+        def __init__(self):
+            self.lock = self.get_lock()
+
+        def acquire(self, wait=True):
+            try:
+                return self.lock.acquire(wait)
+            except redis_lock.AlreadyAcquired:
+                return False
+            except redis_lock.AlreadyStarted:
+                # refresh thread exists, but it also means we acquired the lock
+                return True
+
+        def release(self):
+            try:
+                self.lock.release()
+            except redis_lock.NotAcquired:
+                pass
+
+    return _RedisLockWrapper()

rhodecode/lib/rc_cache/utils.py

@@ -261,12 +261,15 b' def get_or_create_region(region_name, re'
     return region_obj
 
 
-def clear_cache_namespace(cache_region, cache_namespace_uid):
+def clear_cache_namespace(cache_region, cache_namespace_uid, invalidate=False):
     region = get_or_create_region(cache_region, cache_namespace_uid)
     cache_keys = region.backend.list_keys(prefix=cache_namespace_uid)
     num_delete_keys = len(cache_keys)
-    if num_delete_keys:
-        region.delete_multi(cache_keys)
+    if invalidate:
+        region.invalidate(hard=False)
+    else:
+        if num_delete_keys:
+            region.delete_multi(cache_keys)
     return num_delete_keys
 
 

rhodecode/model/pull_request.py

@@ -345,14 +345,14 b' class PullRequestModel(BaseModel):'
         if only_created:
             q = q.filter(PullRequest.pull_request_state == PullRequest.STATE_CREATED)
 
-        if order_by:
-            order_map = {
-                'name_raw': PullRequest.pull_request_id,
-                'id': PullRequest.pull_request_id,
-                'title': PullRequest.title,
-                'updated_on_raw': PullRequest.updated_on,
-                'target_repo': PullRequest.target_repo_id
-            }
+        order_map = {
+            'name_raw': PullRequest.pull_request_id,
+            'id': PullRequest.pull_request_id,
+            'title': PullRequest.title,
+            'updated_on_raw': PullRequest.updated_on,
+            'target_repo': PullRequest.target_repo_id
+        }
+        if order_by and order_by in order_map:
             if order_dir == 'asc':
                 q = q.order_by(order_map[order_by].asc())
             else:
@@ -499,13 +499,13 b' class PullRequestModel(BaseModel):'
                 pull_request_alias.description.ilike(like_expression),
             ))
 
-        if order_by:
-            order_map = {
-                'name_raw': pull_request_alias.pull_request_id,
-                'title': pull_request_alias.title,
-                'updated_on_raw': pull_request_alias.updated_on,
-                'target_repo': pull_request_alias.target_repo_id
-            }
+        order_map = {
+            'name_raw': pull_request_alias.pull_request_id,
+            'title': pull_request_alias.title,
+            'updated_on_raw': pull_request_alias.updated_on,
+            'target_repo': pull_request_alias.target_repo_id
+        }
+        if order_by and order_by in order_map:
             if order_dir == 'asc':
                 q = q.order_by(order_map[order_by].asc())
             else:
@@ -585,13 +585,14 b' class PullRequestModel(BaseModel):'
                 PullRequest.title.ilike(like_expression),
                 PullRequest.description.ilike(like_expression),
             ))
-        if order_by:
-            order_map = {
-                'name_raw': PullRequest.pull_request_id,
-                'title': PullRequest.title,
-                'updated_on_raw': PullRequest.updated_on,
-                'target_repo': PullRequest.target_repo_id
-            }
+
+        order_map = {
+            'name_raw': PullRequest.pull_request_id,
+            'title': PullRequest.title,
+            'updated_on_raw': PullRequest.updated_on,
+            'target_repo': PullRequest.target_repo_id
+        }
+        if order_by and order_by in order_map:
             if order_dir == 'asc':
                 q = q.order_by(order_map[order_by].asc())
             else:
@@ -665,13 +666,13 b' class PullRequestModel(BaseModel):'
                 pull_request_alias.description.ilike(like_expression),
             ))
 
-        if order_by:
-            order_map = {
-                'name_raw': pull_request_alias.pull_request_id,
-                'title': pull_request_alias.title,
-                'updated_on_raw': pull_request_alias.updated_on,
-                'target_repo': pull_request_alias.target_repo_id
-            }
+        order_map = {
+            'name_raw': pull_request_alias.pull_request_id,
+            'title': pull_request_alias.title,
+            'updated_on_raw': pull_request_alias.updated_on,
+            'target_repo': pull_request_alias.target_repo_id
+        }
+        if order_by and order_by in order_map:
             if order_dir == 'asc':
                 q = q.order_by(order_map[order_by].asc())
             else:

rhodecode/model/repo.py

@@ -125,13 +125,15 b' class RepoModel(BaseModel):'
         :param repo_name:
         :return: repo object if matched else None
         """
-
+        _repo_id = None
         try:
             _repo_id = self._extract_id_from_repo_name(repo_name)
             if _repo_id:
                 return self.get(_repo_id)
         except Exception:
             log.exception('Failed to extract repo_name from URL')
+            if _repo_id:
+                Session().rollback()
 
         return None
 

rhodecode/model/scm.py

@@ -285,7 +285,8 b' class ScmModel(BaseModel):'
             repo.update_commit_cache(config=config, cs_cache=None)
             if delete:
                 cache_namespace_uid = 'cache_repo.{}'.format(repo_id)
-                rc_cache.clear_cache_namespace('cache_repo', cache_namespace_uid)
+                rc_cache.clear_cache_namespace(
+                    'cache_repo', cache_namespace_uid, invalidate=True)
 
     def toggle_following_repo(self, follow_repo_id, user_id):
 

rhodecode/public/js/rhodecode/routes.js

@@ -289,7 +289,7 b' function registerRCRoutes() {'
     pyroutes.register('repo_commit_comment_create', '/%(repo_name)s/changeset/%(commit_id)s/comment/create', ['repo_name', 'commit_id']);
     pyroutes.register('repo_commit_comment_delete', '/%(repo_name)s/changeset/%(commit_id)s/comment/%(comment_id)s/delete', ['repo_name', 'commit_id', 'comment_id']);
     pyroutes.register('repo_commit_comment_edit', '/%(repo_name)s/changeset/%(commit_id)s/comment/%(comment_id)s/edit', ['repo_name', 'commit_id', 'comment_id']);
-    pyroutes.register('repo_commit_comment_history_view', '/%(repo_name)s/changeset/%(commit_id)s/comment/%(comment_history_id)s/history_view', ['repo_name', 'commit_id', 'comment_history_id']);
+    pyroutes.register('repo_commit_comment_history_view', '/%(repo_name)s/changeset/%(commit_id)s/comment/%(comment_id)s/history_view/%(comment_history_id)s', ['repo_name', 'commit_id', 'comment_id', 'comment_history_id']);
     pyroutes.register('repo_commit_comment_preview', '/%(repo_name)s/changeset/%(commit_id)s/comment/preview', ['repo_name', 'commit_id']);
     pyroutes.register('repo_commit_data', '/%(repo_name)s/changeset-data/%(commit_id)s', ['repo_name', 'commit_id']);
     pyroutes.register('repo_commit_download', '/%(repo_name)s/changeset-download/%(commit_id)s', ['repo_name', 'commit_id']);

rhodecode/public/js/src/rhodecode/comments.js

@@ -572,7 +572,8 b' var CommentsController = function() {'
            'repo_commit_comment_history_view',
            {
                'repo_name': templateContext.repo_name,
-               'commit_id': comment_id,
+               'commit_id': null, // We don't need to check the commit data here...
+               'comment_id': comment_id,
                'comment_history_id': comment_history_id,
            }
        );