gitweb: Fix enable auth webserver component on app init

* Fixes the gitweb app initalization when there are no public
repositories and the apache2 configuration gitweb-freedombox-auth has
not been enabled before (this may happen when the git repositories or
apache2 configuration has been modified outside the plinth web UI).
* Fix comment in the gitweb-freedombox-auth.conf apache2 configuration
file.

Fixes #1928

Tests performed:
* The gitweb app unit and functional tests pass.
* Configured all gitweb repositories as private. Stopped the plinth
service. Disabled the apache2 configuration gitweb-freedombox-auth.
Then starting the plinth service succeeds and gitweb-freedombox-auth
configuration gets enabled.

Signed-off-by: Veiko Aasa <veiko17@disroot.org>
Reviewed-by: Joseph Nuthalapati <njoseph@riseup.net>

plinth/modules/gitweb/__init__.py

@@ -17,8 +17,10 @@ from plinth.modules.firewall.components import Firewall
 from plinth.modules.users.components import UsersAndGroups
 
 from .forms import is_repo_url
-from .manifest import (GIT_REPO_PATH,  # noqa, pylint: disable=unused-import
+from .manifest import (  # noqa, pylint: disable=unused-import
-    backup, clients)
+    GIT_REPO_PATH,
+    backup,
+    clients)
 
 version = 1
 
@@ -98,37 +100,9 @@ class GitwebApp(app_module.App):
         shortcut.login_required = login_required
         self.add(shortcut)
 
-    def get_repo_list(self):
-        """List all Git repositories and set Gitweb as public or private."""
-        repos = []
-        if os.path.exists(GIT_REPO_PATH):
-            for repo in os.listdir(GIT_REPO_PATH):
-                if not repo.endswith('.git') or repo.startswith('.'):
-                    continue
-
-                repo_info = {}
-                repo_info['name'] = repo[:-4]
-
-                private_file = os.path.join(GIT_REPO_PATH, repo, 'private')
-                if os.path.exists(private_file):
-                    repo_info['access'] = 'private'
-                else:
-                    repo_info['access'] = 'public'
-
-                progress_file = os.path.join(GIT_REPO_PATH, repo,
-                                             'clone_progress')
-                if os.path.exists(progress_file):
-                    with open(progress_file) as file_handle:
-                        clone_progress = file_handle.read()
-                        repo_info['clone_progress'] = clone_progress
-
-                repos.append(repo_info)
-
-        return sorted(repos, key=lambda repo: repo['name'])
-
     def update_service_access(self):
         """Update the frontpage shortcut and webserver auth requirement."""
-        repos = self.get_repo_list()
+        repos = get_repo_list()
         if have_public_repos(repos):
             self._enable_public_access()
         else:
@@ -158,12 +132,12 @@ class GitwebWebserverAuth(Webserver):
 
     def is_enabled(self):
         """Return if configuration is enabled or public access is enabled."""
-        repos = app.get_repo_list()
+        repos = get_repo_list()
         return have_public_repos(repos) or super().is_enabled()
 
     def enable(self):
         """Enable apache configuration only if no public repository exists."""
-        repos = app.get_repo_list()
+        repos = get_repo_list()
         if not have_public_repos(repos):
             super().enable()
 
@@ -212,6 +186,34 @@ def create_repo(repo, repo_description, owner, is_private):
         actions.superuser_run('gitweb', args)
 
 
+def get_repo_list():
+    """List all git repositories."""
+    repos = []
+    if os.path.exists(GIT_REPO_PATH):
+        for repo in os.listdir(GIT_REPO_PATH):
+            if not repo.endswith('.git') or repo.startswith('.'):
+                continue
+
+            repo_info = {}
+            repo_info['name'] = repo[:-4]
+
+            private_file = os.path.join(GIT_REPO_PATH, repo, 'private')
+            if os.path.exists(private_file):
+                repo_info['access'] = 'private'
+            else:
+                repo_info['access'] = 'public'
+
+            progress_file = os.path.join(GIT_REPO_PATH, repo, 'clone_progress')
+            if os.path.exists(progress_file):
+                with open(progress_file) as file_handle:
+                    clone_progress = file_handle.read()
+                    repo_info['clone_progress'] = clone_progress
+
+            repos.append(repo_info)
+
+    return sorted(repos, key=lambda repo: repo['name'])
+
+
 def repo_info(repo):
     """Get information about repository."""
     output = actions.run('gitweb', ['repo-info', '--name', repo])

plinth/modules/gitweb/data/etc/apache2/conf-available/gitweb-freedombox-auth.conf

@@ -1,7 +1,7 @@
 ##
 ## Limit access to gitweb web interface. Only users belonging to 'admin' or
 ## 'git-access' groups are allowed to view the web interface. This configuration
-## is to be enabled when there is at least one public git project.
+## is to be enabled when there are no public git projects.
 ##
 <Directory /usr/share/gitweb>
     Include includes/freedombox-single-sign-on.conf

plinth/modules/gitweb/forms.py

@@ -92,7 +92,7 @@ class CreateRepoForm(forms.Form):
         if repo_name.endswith('.git'):
             repo_name = repo_name[:-4]
 
-        for repo in gitweb.app.get_repo_list():
+        for repo in gitweb.get_repo_list():
             if repo_name == repo['name']:
                 raise ValidationError(
                     _('A repository with this name already exists.'))
@@ -124,7 +124,7 @@ class EditRepoForm(CreateRepoForm):
         if name.endswith('.git'):
             name = name[:-4]
 
-        for repo in gitweb.app.get_repo_list():
+        for repo in gitweb.get_repo_list():
             if name == repo['name']:
                 raise ValidationError(
                     _('A repository with this name already exists.'))

plinth/modules/gitweb/tests/test_views.py

@@ -60,14 +60,17 @@ def action_run(*args, **kwargs):
 @pytest.fixture(autouse=True)
 def gitweb_patch():
     """Patch gitweb."""
-    with patch('plinth.modules.gitweb.app') as app_patch, \
+    with patch('plinth.modules.gitweb.get_repo_list') as get_repo_list, \
+            patch('plinth.modules.gitweb.app') as gitweb_app, \
             patch('plinth.actions.superuser_run', side_effect=action_run), \
             patch('plinth.actions.run', side_effect=action_run):
-        app_patch.get_repo_list.return_value = [{
+        get_repo_list.return_value = [{
             'name': EXISTING_REPOS[0]['name']
         }, {
             'name': EXISTING_REPOS[1]['name']
         }]
+        gitweb_app.update_service_access.return_value = None
+
         yield
 
 
@@ -148,8 +151,8 @@ def test_create_repo_failed_view(rf):
     """Test that repo creation failure sends correct error message."""
     general_error_message = "An error occurred while creating the repository."
     error_description = 'some error'
-    with patch('plinth.modules.gitweb.create_repo', side_effect=ActionError(
+    with patch('plinth.modules.gitweb.create_repo',
-            'gitweb', '', error_description)):
+               side_effect=ActionError('gitweb', '', error_description)):
         form_data = {
             'gitweb-name': 'something_other',
             'gitweb-description': '',

plinth/modules/gitweb/views.py

@@ -28,7 +28,7 @@ class GitwebAppView(views.AppView):
     def get_context_data(self, *args, **kwargs):
         """Add repositories to the context data."""
         context = super().get_context_data(*args, **kwargs)
-        repos = gitweb.app.get_repo_list()
+        repos = gitweb.get_repo_list()
         context['repos'] = repos
         context['cloning'] = any('clone_progress' in repo for repo in repos)
         context['refresh_page_sec'] = 3 if context['cloning'] else None
@@ -92,7 +92,7 @@ class EditRepoView(SuccessMessageMixin, FormView):
     def get_initial(self):
         """Load information about repository being edited."""
         name = self.kwargs['name']
-        for repo in gitweb.app.get_repo_list():
+        for repo in gitweb.get_repo_list():
             if repo['name'] == name and 'clone_progress' not in repo:
                 break
         else:
@@ -126,7 +126,7 @@ def delete(request, name):
     On GET, display a confirmation page.
     On POST, delete the repository.
     """
-    for repo in gitweb.app.get_repo_list():
+    for repo in gitweb.get_repo_list():
         if repo['name'] == name and 'clone_progress' not in repo:
             break
     else: