Fix taking into account the blacklisted patterns

Basically, we were checking if a new project matches the blacklist
before taking into account arguments such as: is user namespace
enforced, or is the project private and so on, so patterns in the
blacklist such as docs/* would not be matched if the user was named
'docs' and user_namespace was enabled on the instance.

We're adding tests to this to ensure we don't run into this again.

Fixes https://pagure.io/pagure/issue/4726

Signed-off-by: Pierre-Yves Chibon <pingou@pingoured.fr>

pagure/lib/query.py

@@ -1621,14 +1621,6 @@ def new_project(
 
     Is an async operation, and returns task ID.
     """
-    ns_name = name if not namespace else "%s/%s" % (namespace, name)
-    matched = any(map(functools.partial(fnmatch.fnmatch, ns_name), blacklist))
-    if matched:
-        raise pagure.exceptions.ProjectBlackListedException(
-            'No project "%s" are allowed to be created due to potential '
-            "conflicts in URLs with pagure itself" % ns_name
-        )
-
     user_obj = get_user(session, user)
     allowed_prefix = allowed_prefix + [grp for grp in user_obj.groups]
     if user_ns:
@@ -1646,6 +1638,15 @@ def new_project(
             "name of a group of which you are a member."
         )
 
+    path = name if not namespace else "%s/%s" % (namespace, name)
+
+    matched = any(map(functools.partial(fnmatch.fnmatch, path), blacklist))
+    if matched:
+        raise pagure.exceptions.ProjectBlackListedException(
+            'No project "%s" are allowed to be created due to potential '
+            "conflicts in URLs with pagure itself" % path
+        )
+
     if len(name) == 40 and prevent_40_chars:
         # We must block project with a name <foo>/<bar> where the length
         # of <bar> is exactly 40 characters long as this would otherwise
@@ -1658,10 +1659,6 @@ def new_project(
             "the `/`"
         )
 
-    path = name
-    if namespace:
-        path = "%s/%s" % (namespace, name)
-
     # Repo exists in the DB
     repo = _get_project(session, name, namespace=namespace)
     # this is leaking private repos but we're leaking them anyway if we fail

tests/test_pagure_flask_ui_app.py

@@ -2702,6 +2702,101 @@ class PagureFlaskAppNewProjecttests(tests.Modeltests):
             )
         )
 
+    @patch.dict("pagure.config.config", {"USER_NAMESPACE": True})
+    def test_new_project_user_namespaced(self):
+        """ Test the new_project with a user namespaced enabled.
+        """
+
+        user = tests.FakeUser(username="foo")
+        with tests.user_set(self.app.application, user):
+            output = self.app.get("/new/")
+            self.assertEqual(output.status_code, 200)
+            output_text = output.get_data(as_text=True)
+
+            csrf_token = self.get_csrf(output=output)
+
+            data = {
+                "description": "Project #1",
+                "name": "project-1",
+                "csrf_token": csrf_token,
+            }
+
+            output = self.app.post("/new/", data=data, follow_redirects=True)
+            self.assertEqual(output.status_code, 200)
+            output_text = output.get_data(as_text=True)
+            self.assertIn(
+                "<title>Overview - foo/project-1 - Pagure</title>", output_text
+            )
+            self.assertIn(
+                '<div class="projectinfo my-3">\nProject #1', output_text
+            )
+            self.assertIn("<p>This repo is brand new!</p>", output_text)
+
+        # After
+        projects = pagure.lib.query.search_projects(self.session)
+        self.assertEqual(len(projects), 1)
+        self.assertTrue(
+            os.path.exists(
+                os.path.join(self.path, "repos", "foo", "project-1.git")
+            )
+        )
+        self.assertTrue(
+            os.path.exists(
+                os.path.join(
+                    self.path, "repos", "tickets", "foo", "project-1.git"
+                )
+            )
+        )
+        self.assertTrue(
+            os.path.exists(
+                os.path.join(
+                    self.path, "repos", "docs", "foo", "project-1.git"
+                )
+            )
+        )
+        self.assertTrue(
+            os.path.exists(
+                os.path.join(
+                    self.path, "repos", "requests", "foo", "project-1.git"
+                )
+            )
+        )
+
+    @patch.dict("pagure.config.config", {"USER_NAMESPACE": True})
+    def test_new_project_user_namespaced_invalid_user(self):
+        """ Test the new_project with a user namespaced enabled.
+        """
+        tests.create_user(self.session, "docs", "evil docs", ["docs@bar.com"])
+
+        user = tests.FakeUser(username="docs")
+        with tests.user_set(self.app.application, user):
+            output = self.app.get("/new/")
+            self.assertEqual(output.status_code, 200)
+            output_text = output.get_data(as_text=True)
+
+            csrf_token = self.get_csrf(output=output)
+
+            data = {
+                "description": "Project #1",
+                "name": "project-1",
+                "csrf_token": csrf_token,
+            }
+
+            output = self.app.post("/new/", data=data, follow_redirects=True)
+            self.assertEqual(output.status_code, 200)
+            output_text = output.get_data(as_text=True)
+            self.assertIn("<title>New project - Pagure</title>", output_text)
+            self.assertIn(
+                "</i> No project &#34;docs/project-1&#34; are allowed to be "
+                "created due to potential conflicts in URLs with pagure "
+                "itself</div>",
+                output_text,
+            )
+
+        # After
+        projects = pagure.lib.query.search_projects(self.session)
+        self.assertEqual(len(projects), 0)
+
 
 if __name__ == "__main__":
     unittest.main(verbosity=2)

tests/test_pagure_lib.py

@@ -1568,7 +1568,7 @@ class PagureLibtests(tests.Modeltests):
             name="static",
             repospanner_region=None,
             blacklist=["space/*"],
-            allowed_prefix=[],
+            allowed_prefix=["space"],
             description="description for static",
             parent_id=None,
         )