Merge pull request #21 from kitconcept/ree-solr-security-fix

Fix solr search security problem with individual users

news/20.bugfix

@@ -0,0 +1 @@
+Fix solr search security problem with individual users @reebalazs

src/kitconcept/solr/services/solr.py

@@ -47,6 +47,10 @@ def escape(term):
     return term
 
 
+def replace_colon(term):
+    return term.replace(":", "$")
+
+
 def security_filter():
     user = getSecurityManager().getUser()
     roles = user.getRoles()
@@ -60,7 +64,10 @@ def security_filter():
             roles = roles + groups
     roles.append("user:%s" % user.getId())
     # Roles with spaces need to be quoted
-    roles = ['"%s"' % escape(r) if " " in r else escape(r) for r in roles]
+    roles = [
+        '"%s"' % escape(replace_colon(r)) if " " in r else escape(replace_colon(r))
+        for r in roles
+    ]
     return "allowedRolesAndUsers:(%s)" % " OR ".join(roles)
 
 

tests/services/roles_and_users/conftest.py

@@ -0,0 +1,62 @@
+from plone.app.testing import SITE_OWNER_NAME
+from plone.app.testing import SITE_OWNER_PASSWORD
+from typing import List
+
+import pytest
+
+
+@pytest.fixture
+def contents() -> List:
+    return [
+        {
+            "_container": "",
+            "type": "Document",
+            "id": "document1",
+            "title": "My Document about Noam Chomsky",
+            "_transitions": ["publish"],
+        },
+        {
+            "_container": "",
+            "type": "Document",
+            "id": "document2",
+            "title": "My Private Document about Noam Chomsky",
+        },
+    ]
+
+
+@pytest.fixture
+def user_credentials() -> tuple:
+    return "user2", "averylongpasswordbutnotthatlong"
+
+
+@pytest.fixture
+def member_as_user1_credentials() -> tuple:
+    return "member_as_user1", "averylongpasswordbutnotthatlong"
+
+
+@pytest.fixture
+def users(user_credentials, member_as_user1_credentials) -> List:
+    return [
+        {
+            "username": user_credentials[0],
+            "email": "[email protected]",
+            "password": user_credentials[1],
+            "roles": ["Member"],
+        },
+        {
+            "username": member_as_user1_credentials[0],
+            "email": "[email protected]",
+            "password": member_as_user1_credentials[1],
+            "roles": ["Member"],
+        },
+    ]
+
+
+@pytest.fixture
+def users_credentials_role(user_credentials, member_as_user1_credentials) -> dict:
+    return {
+        "manager": (SITE_OWNER_NAME, SITE_OWNER_PASSWORD),
+        "member": user_credentials,
+        "anonymous": None,
+        "member_as_user1": member_as_user1_credentials,
+    }

tests/services/roles_and_users/test_roles_and_users.py

@@ -0,0 +1,51 @@
+from plone.app.testing import setRoles
+
+import pytest
+import transaction
+
+
+@pytest.fixture()
+def member_request(request_factory, user_credentials):
+    request = request_factory()
+    request.auth = user_credentials
+    yield request
+    request.auth = ()
+
+
+class TestEndpointRolesAndUsers:
+    url = "/@solr?q=chomsky"
+
+    @pytest.fixture
+    def api_request(self, request_factory, users_credentials_role, portal, roles):
+        def func(role: str) -> dict:
+            req = request_factory()
+            credentials = users_credentials_role.get(role, None)
+            if credentials:
+                req.auth = credentials
+                # setting the role is explicitly needed for these tests
+                setRoles(portal, credentials[0], roles)
+                transaction.commit()
+            return req
+
+        return func
+
+    @pytest.fixture(autouse=True)
+    def _init(self, portal_with_content):
+        self.portal = portal_with_content
+
+
+class TestEndpointRolesAndUsersNoPermission(TestEndpointRolesAndUsers):
+    @pytest.mark.parametrize(
+        "role,roles,path,expected",
+        [
+            ("anonymous", ["Anonymous"], "/plone/document2", False),
+            ("member_as_user1", ["user:test_user_1_"], "/plone/document2", True),
+            ("member_as_user1", ["Reader"], "/plone/document2", True),
+        ],
+    )
+    def test_paths(
+        self, api_request, all_path_string, role: str, path: str, expected: bool
+    ):
+        data = api_request(role).get(self.url).json()
+        path_strings = all_path_string(data)
+        assert (path in path_strings) is expected