gitweb: Don't use privileged action feature to run as different user

- Instead implement running specific commands inside the privileged action as a
specific user.

Tests:

- Gitweb functional tests and unit tests work.

- Running various operations such as clone, create, set branch, rename, etc. all
result in repositories (and all their contents) owned by www-data:www-data.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Joseph Nuthalapati <njoseph@riseup.net>

plinth/conftest.py

@@ -6,6 +6,7 @@ pytest configuration for all tests.
 import importlib
 import os
 import pathlib
+import subprocess
 from unittest.mock import patch
 
 import pytest
@@ -191,6 +192,18 @@ def fixture_mock_privileged(request):
             module.__dict__[name] = wrapper
 
 
+@pytest.fixture(name='mock_run_as_user')
+def fixture_mock_run_as_user():
+    """A fixture to override action_utils.run_as_user."""
+
+    def _bypass_runuser(*args, username, **kwargs):
+        return subprocess.run(*args, **kwargs)
+
+    with patch('plinth.action_utils.run_as_user') as mock:
+        mock.side_effect = _bypass_runuser
+        yield
+
+
 @pytest.fixture(name='splinter_screenshot_dir', scope='session')
 def fixture_splinter_screenshot_dir(request):
     """Set default screenshot directory to ./screenshots.

plinth/modules/gitweb/__init__.py

@@ -14,7 +14,7 @@ from plinth.package import Packages
 
 from . import manifest, privileged
 from .forms import is_repo_url
-from .manifest import GIT_REPO_PATH, REPO_DIR_OWNER
+from .manifest import GIT_REPO_PATH
 
 _description = [
     _('Git is a distributed version-control system for tracking changes in '
@@ -217,7 +217,7 @@ def get_repo_list():
 
 def repo_info(repo):
     """Get information about repository."""
-    info = privileged.repo_info(repo, _run_as_user=REPO_DIR_OWNER)
+    info = privileged.repo_info(repo)
     if info['access'] == 'private':
         info['is_private'] = True
     else:
@@ -248,5 +248,4 @@ def edit_repo(form_initial, form_cleaned):
             privileged.set_repo_access(repo, 'public')
 
     if form_cleaned['default_branch'] != form_initial['default_branch']:
-        privileged.set_default_branch(repo, form_cleaned['default_branch'],
+        privileged.set_default_branch(repo, form_cleaned['default_branch'])
-                                      _run_as_user=REPO_DIR_OWNER)

plinth/modules/gitweb/forms.py

@@ -14,12 +14,11 @@ from django.utils.translation import gettext_lazy as _
 from plinth.modules import gitweb
 
 from . import privileged
-from .manifest import REPO_DIR_OWNER
 
 
 def _get_branches(repo):
     """Get all the branches in the repository."""
-    branch_data = privileged.get_branches(repo, _run_as_user=REPO_DIR_OWNER)
+    branch_data = privileged.get_branches(repo)
     default_branch = branch_data['default_branch']
     branches = branch_data['branches']
 

plinth/modules/gitweb/privileged.py

@@ -199,9 +199,11 @@ def _get_default_branch(repo):
     """Get default branch of the repository."""
     repo_path = GIT_REPO_PATH / repo
 
-    return subprocess.check_output(
+    return action_utils.run_as_user(
         ['git', '-C',
-         str(repo_path), 'symbolic-ref', '--short', 'HEAD']).decode().strip()
+         str(repo_path), 'symbolic-ref', '--short', 'HEAD'],
+        username=REPO_DIR_OWNER, check=True,
+        stdout=subprocess.PIPE).stdout.decode().strip()
 
 
 def _get_repo_description(repo):
@@ -267,11 +269,12 @@ def _set_access_status(repo, status):
 
 def _get_branches(repo):
     """Return list of the branches in the repository."""
-    output = subprocess.check_output(
+    process = action_utils.run_as_user(
         ['git', '-C', repo, 'branch', '--format=%(refname:short)'],
-        cwd=GIT_REPO_PATH)
+        cwd=GIT_REPO_PATH, username=REPO_DIR_OWNER, check=True,
+        stdout=subprocess.PIPE)
 
-    return output.decode().strip().split()
+    return process.stdout.decode().strip().split()
 
 
 @privileged
@@ -299,10 +302,9 @@ def set_default_branch(name: str, branch: str):
     if branch not in _get_branches(repo):
         raise ValueError('No such branch')
 
-    subprocess.check_call([
+    action_utils.run_as_user(
-        'git', '-C', repo, 'symbolic-ref', 'HEAD',
+        ['git', '-C', repo, 'symbolic-ref', 'HEAD', f'refs/heads/{branch}'],
-        "refs/heads/{}".format(branch)
+        cwd=GIT_REPO_PATH, check=True, username=REPO_DIR_OWNER)
-    ], cwd=GIT_REPO_PATH)
 
 
 @privileged

plinth/modules/gitweb/tests/test_privileged.py

@@ -16,16 +16,16 @@ REPO_DATA = {
     'access': 'private',
 }
 
-pytestmark = pytest.mark.usefixtures('mock_privileged')
+pytestmark = pytest.mark.usefixtures('mock_privileged', 'mock_run_as_user')
 privileged_modules_to_mock = ['plinth.modules.gitweb.privileged']
 git_installed = pytest.mark.skipif(not pathlib.Path('/usr/bin/git').exists(),
                                    reason='git is not installed')
 
 
 @pytest.fixture(autouse=True)
-def fixture_set_repo_path(tmpdir):
+def fixture_set_repo_path(tmp_path):
     """Set a repository path in the actions module."""
-    privileged.GIT_REPO_PATH = tmpdir
+    privileged.GIT_REPO_PATH = tmp_path
 
 
 @pytest.fixture(name='existing_repo')