Merge commit from fork

- When using a form we were checking that only admin users can import projects,
  but that was done in the frontend, so it was easy to bypass that
  restriction.
- When using API V3, we were not doing any checks at all.

Ref GHSA-rmqq-mq6q-8hpg

Author: stsewd

readthedocs/api/v3/serializers.py

@@ -642,6 +642,7 @@ def validate_name(self, value):
 
     def validate(self, data):  # pylint: disable=arguments-renamed
         repo = data.get("repo")
+        user = self.context["request"].user
         try:
             # We are looking for an exact match of the repository URL entered
             # by the user and any of the known URLs (ssh, clone, html) we have
@@ -650,7 +651,9 @@ def validate(self, data):  # pylint: disable=arguments-renamed
             # If the `RemoteRepository` is found, we save it to link with
             # `Project` object after performing its creating.
             query = Q(ssh_url=repo) | Q(clone_url=repo) | Q(html_url=repo)
-            remote_repository = RemoteRepository.objects.get(query)
+            remote_repository = RemoteRepository.objects.for_project_linking(user).get(
+                query
+            )
             data.update(
                 {
                     "remote_repository": remote_repository,

readthedocs/api/v3/tests/test_projects.py

@@ -3,8 +3,9 @@
 import django_dynamic_fixture as fixture
 from django.test import override_settings
 from django.urls import reverse
+from django_dynamic_fixture import get
 
-from readthedocs.oauth.models import RemoteRepository
+from readthedocs.oauth.models import RemoteRepository, RemoteRepositoryRelation
 from readthedocs.projects.constants import SINGLE_VERSION_WITHOUT_TRANSLATIONS
 from readthedocs.projects.models import Project
 
@@ -400,6 +401,13 @@ def test_import_project_with_remote_repository(self):
             clone_url="https://github.com/rtfd/template",
             html_url="https://github.com/rtfd/template",
             ssh_url="[email protected]:rtfd/template.git",
+            private=False,
+        )
+        get(
+            RemoteRepositoryRelation,
+            remote_repository=remote_repository,
+            user=self.me,
+            admin=True,
         )
 
         data = {
@@ -421,6 +429,49 @@ def test_import_project_with_remote_repository(self):
         self.assertIsNotNone(project.remote_repository)
         self.assertEqual(project.remote_repository, remote_repository)
 
+    def test_import_project_with_remote_repository_from_other_user(self):
+        repo_url = "https://github.com/readthedocs/template"
+        remote_repository = get(
+            RemoteRepository,
+            full_name="readthedocs/template",
+            clone_url=repo_url,
+            html_url="https://github.com/readthedocs/template",
+            ssh_url="[email protected]:readthedocs/template.git",
+            private=False,
+        )
+
+        data = {
+            "name": "Test Project",
+            "repository": {
+                "url": repo_url,
+                "type": "git",
+            },
+        }
+
+        self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.token.key}")
+
+        response = self.client.post(reverse("projects-list"), data)
+        self.assertEqual(response.status_code, 201)
+        project = Project.objects.get(slug=response.data["slug"])
+        self.assertIsNone(project.remote_repository)
+
+        # The user has access to the repository but is not an admin,
+        # and the repository is private.
+        remote_repository.private = True
+        remote_repository.save()
+        get(
+            RemoteRepositoryRelation,
+            remote_repository=remote_repository,
+            user=self.me,
+            admin=False,
+        )
+
+        data["name"] = "Test Project 2"
+        response = self.client.post(reverse("projects-list"), data)
+        self.assertEqual(response.status_code, 201)
+        project = Project.objects.get(slug=response.data["slug"])
+        self.assertIsNone(project.remote_repository)
+
     def test_update_project(self):
         data = {
             "name": "Updated name",

readthedocs/oauth/querysets.py

@@ -1,6 +1,7 @@
 """Managers for OAuth models."""
 
 from django.db import models
+from django.db.models import Q
 
 from readthedocs.core.querysets import NoReprQuerySet
 
@@ -17,7 +18,20 @@ def api(self, user=None):
 
 
 class RemoteRepositoryQuerySet(RelatedUserQuerySet):
-    pass
+    def for_project_linking(self, user):
+        """
+        Return repositories that can be linked to a project by the given user.
+
+        Repositories can be imported if:
+
+        - The user has read or adming access to the repository on the VCS service.
+        - If the repository is private, the user must be an admin.
+        - If the repository is public, the user doesn't need to be an admin.
+        """
+        query = Q(remote_repository_relations__user=user) & (
+            Q(private=False) | Q(private=True, remote_repository_relations__admin=True)
+        )
+        return self.filter(query).distinct()
 
 
 class RemoteOrganizationQuerySet(RelatedUserQuerySet):

readthedocs/oauth/tests/__init__.py

@@ -0,0 +1,86 @@
+from django.contrib.auth.models import User
+from django.test import TestCase
+from django_dynamic_fixture import get
+
+from readthedocs.oauth.models import RemoteRepository, RemoteRepositoryRelation
+from readthedocs.projects.models import Project
+
+
+class TestRemoteRepositoryQuerysets(TestCase):
+    def setUp(self):
+        self.user = get(User)
+        self.project = get(
+            Project,
+            users=[self.user],
+        )
+
+        self.remote_repository_admin_public = get(
+            RemoteRepository,
+            private=False,
+        )
+        get(
+            RemoteRepositoryRelation,
+            user=self.user,
+            remote_repository=self.remote_repository_admin_public,
+            admin=True,
+        )
+
+        self.remote_repository_admin_private = get(
+            RemoteRepository,
+            private=True,
+        )
+        get(
+            RemoteRepositoryRelation,
+            user=self.user,
+            remote_repository=self.remote_repository_admin_private,
+            admin=True,
+        )
+
+        self.remote_repository_not_admin_public = get(
+            RemoteRepository,
+            private=False,
+        )
+        get(
+            RemoteRepositoryRelation,
+            user=self.user,
+            remote_repository=self.remote_repository_not_admin_public,
+            admin=False,
+        )
+
+        self.remote_repository_not_admin_private = get(
+            RemoteRepository,
+            private=True,
+        )
+        get(
+            RemoteRepositoryRelation,
+            user=self.user,
+            remote_repository=self.remote_repository_not_admin_private,
+            admin=False,
+        )
+
+        self.other_user = get(User)
+        self.remote_repository_other_user = get(
+            RemoteRepository,
+            private=False,
+        )
+        get(
+            RemoteRepositoryRelation,
+            user=self.other_user,
+            remote_repository=self.remote_repository_other_user,
+            admin=True,
+        )
+
+    def test_for_project_linking(self):
+        repositories = RemoteRepository.objects.for_project_linking(user=self.user)
+        self.assertEqual(repositories.count(), 3)
+        self.assertIn(self.remote_repository_admin_public, repositories)
+        self.assertIn(self.remote_repository_not_admin_public, repositories)
+        self.assertIn(self.remote_repository_admin_private, repositories)
+        self.assertNotIn(self.remote_repository_not_admin_private, repositories)
+        self.assertNotIn(self.remote_repository_other_user, repositories)
+
+        repositories = RemoteRepository.objects.for_project_linking(
+            user=self.other_user
+        )
+        self.assertEqual(repositories.count(), 1)
+        self.assertIn(self.remote_repository_other_user, repositories)

readthedocs/oauth/tests/test_querysets.py

@@ -362,9 +362,8 @@ def clean_remote_repository(self):
         if not remote_repo:
             return None
         try:
-            return RemoteRepository.objects.get(
+            return RemoteRepository.objects.for_project_linking(self.user).get(
                 pk=remote_repo,
-                users=self.user,
             )
         except RemoteRepository.DoesNotExist as exc:
             raise forms.ValidationError(_("Repository invalid")) from exc

readthedocs/projects/forms.py

@@ -162,11 +162,87 @@ def test_remote_repository_invalid_type(self):
         self.assertIn("remote_repository", form.errors)
 
     def test_remote_repository_invalid_id(self):
-        self.step_data["basics"]["remote_repository"] = 9
-        resp = self.post_step("basics")
-        self.assertEqual(resp.status_code, 200)
-        form = resp.context_data["form"]
-        self.assertIn("remote_repository", form.errors)
+        remote_repository_admin_public = get(
+            RemoteRepository,
+            private=False,
+        )
+        get(
+            RemoteRepositoryRelation,
+            user=self.user,
+            remote_repository=remote_repository_admin_public,
+            admin=True,
+        )
+
+        remote_repository_admin_private = get(
+            RemoteRepository,
+            private=True,
+        )
+        get(
+            RemoteRepositoryRelation,
+            user=self.user,
+            remote_repository=remote_repository_admin_private,
+            admin=True,
+        )
+
+        remote_repository_not_admin_public = get(
+            RemoteRepository,
+            private=False,
+        )
+        get(
+            RemoteRepositoryRelation,
+            user=self.user,
+            remote_repository=remote_repository_not_admin_public,
+            admin=False,
+        )
+
+        remote_repository_not_admin_private = get(
+            RemoteRepository,
+            private=True,
+        )
+        get(
+            RemoteRepositoryRelation,
+            user=self.user,
+            remote_repository=remote_repository_not_admin_private,
+            admin=False,
+        )
+
+        other_user = get(User)
+        remote_repository_other_user = get(
+            RemoteRepository,
+            private=False,
+        )
+        get(
+            RemoteRepositoryRelation,
+            user=other_user,
+            remote_repository=remote_repository_other_user,
+            admin=True,
+        )
+
+        invalid_remote_repos_pk = [
+            remote_repository_not_admin_private.pk,
+            remote_repository_other_user.pk,
+            # Doesn't exist
+            99,
+        ]
+        valid_remote_repos_pk = [
+            remote_repository_admin_private.pk,
+            remote_repository_admin_public.pk,
+            remote_repository_not_admin_public.pk,
+        ]
+
+        for remote_repo_pk in invalid_remote_repos_pk:
+            self.step_data["basics"]["remote_repository"] = remote_repo_pk
+            resp = self.post_step("basics")
+            self.assertEqual(resp.status_code, 200)
+            form = resp.context_data["form"]
+            self.assertIn("remote_repository", form.errors)
+
+        for remote_repo_pk in valid_remote_repos_pk:
+            self.step_data["basics"]["remote_repository"] = remote_repo_pk
+            resp = self.post_step("basics")
+            self.assertEqual(resp.status_code, 200)
+            form = resp.context_data["form"]
+            self.assertNotIn("remote_repository", form.errors)
 
     def test_remote_repository_is_not_added_for_wrong_user(self):
         user = get(User)