fix(documents): allow anonymous document listing

* DocumentPermission.list now always allows listing, following the
  rero-ils any_user model
* search_factory applies the masked/IP visibility filter to any
  non-privileged request, independent of the view parameter
* default the no-view anonymous scope to global (all organisations)
* logged_user documents.list permission reflects the now-public
  document listing
* add tests for anonymous list/read with masked and IP-restricted
  documents
* Closes #884

Co-Authored-by: Bertrand Zuchuat <bertrand.zuchuat@rero.ch>

Author: Garfield-fr

sonar/modules/documents/permissions.py

@@ -3,7 +3,6 @@
 
 """Permissions for documents."""
 
-from flask import request
 from invenio_files_rest.models import Bucket, ObjectVersion
 from invenio_pidstore.errors import PIDDoesNotExistError
 from invenio_pidstore.models import PersistentIdentifier
@@ -26,14 +25,9 @@ def list(cls, user, record=None):
         :param record: Record to check.
         :returns: True is action can be done.
         """
-
-        # Documents are accessible in public view, but eventually filtered
-        # later by organisation
-        if request.args.get("view"):
-            return True
-
-        # Only for moderators users.
-        return bool(user and user.is_moderator and current_organisation)
+        # Listing is always allowed. Anonymous and non-moderator users are
+        # restricted to the safe (non-masked) scope in the search factory.
+        return True
 
     @classmethod
     def create(cls, user, record=None):

sonar/modules/documents/query.py

@@ -44,9 +44,10 @@ def search_factory(self, search, query_parser=None):
         return (search, urlkwargs)
 
     view = request.args.get("view")
+    is_privileged = current_user_record and current_user_record.is_moderator
 
-    # Public search
-    if view:
+    # Public search: anonymous or non-moderator users, with or without a view.
+    if not is_privileged:
         # Don't display masked records
         search = search.filter(
             "bool",
@@ -64,17 +65,26 @@ def search_factory(self, search, query_parser=None):
             ],
         )
 
-        # Filter record by organisation view.
-        if view != current_app.config.get("SONAR_APP_DEFAULT_ORGANISATION"):
+        # Filter record by organisation view. No view means the global scope
+        # (all organisations).
+        if view and view != current_app.config.get("SONAR_APP_DEFAULT_ORGANISATION"):
             search = search.filter("term", organisation__pid=view)
 
         # Filter collection
         if request.args.get("collection_view"):
             search = search.filter("term", collections__pid=request.args["collection_view"])
-    # Admin
+    # Moderator/admin
     else:
+        if view:
+            # Filter record by organisation view.
+            if view != current_app.config.get("SONAR_APP_DEFAULT_ORGANISATION"):
+                search = search.filter("term", organisation__pid=view)
+
+            # Filter collection
+            if request.args.get("collection_view"):
+                search = search.filter("term", collections__pid=request.args["collection_view"])
         # Filters records by user's organisation
-        if not current_user_record.is_superuser:
+        elif not current_user_record.is_superuser:
             search = search.filter("term", organisation__pid=current_organisation["pid"])
 
     return (search, urlkwargs)

tests/api/documents/test_documents_permissions.py

@@ -27,9 +27,10 @@ def test_list(
     make_document(organisation=None, with_file=True)
     make_document(organisation="org", with_file=True)
 
-    # Not logged
+    # Not logged: public, global scope, no view required.
     res = client.get(url_for("invenio_records_rest.doc_list"))
-    assert res.status_code == 401
+    assert res.status_code == 200
+    assert res.json["hits"]["total"]["value"] == 2
 
     # Not logged but permission checks disabled
     app.config.update(SONAR_APP_DISABLE_PERMISSION_CHECKS=True)
@@ -38,15 +39,17 @@ def test_list(
     assert res.json["hits"]["total"]["value"] == 2
     app.config.update(SONAR_APP_DISABLE_PERMISSION_CHECKS=False)
 
-    # Logged as user
+    # Logged as user: same public, global scope as anonymous.
     login_user_via_session(client, email=user["email"])
     res = client.get(url_for("invenio_records_rest.doc_list"))
-    assert res.status_code == 403
+    assert res.status_code == 200
+    assert res.json["hits"]["total"]["value"] == 2
 
-    # Logged as submitter
+    # Logged as submitter: same public, global scope as anonymous.
     login_user_via_session(client, email=submitter["email"])
     res = client.get(url_for("invenio_records_rest.doc_list"))
-    assert res.status_code == 403
+    assert res.status_code == 200
+    assert res.json["hits"]["total"]["value"] == 2
 
     # Logged as moderator
     login_user_via_session(client, email=moderator["email"])

tests/api/documents/test_documents_query.py

@@ -110,3 +110,48 @@ def test_masked_document(db, client, organisation, document, search_clear):
     res = client.get(url_for("invenio_records_rest.doc_list", view="global"))
     assert res.status_code == 200
     assert res.json["hits"]["total"]["value"] == 0
+
+
+def test_anonymous_document_list_without_view(db, client, organisation, document, search_clear):
+    """Test anonymous document list defaults to the global, non-masked scope."""
+    # No view: behaves like the global view for anonymous users.
+    res = client.get(url_for("invenio_records_rest.doc_list"))
+    assert res.status_code == 200
+    assert res.json["hits"]["total"]["value"] == 1
+
+    # Masked documents are excluded, with or without a view.
+    document["masked"] = "masked_for_all"
+    document.commit()
+    document.reindex()
+    db.session.commit()
+    res = client.get(url_for("invenio_records_rest.doc_list"))
+    assert res.status_code == 200
+    assert res.json["hits"]["total"]["value"] == 0
+
+    # Masked for external IPs, IP is not allowed.
+    document["masked"] = "masked_for_external_ips"
+    document.commit()
+    document.reindex()
+    db.session.commit()
+    res = client.get(url_for("invenio_records_rest.doc_list"))
+    assert res.status_code == 200
+    assert res.json["hits"]["total"]["value"] == 0
+
+    # Masked for external IPs, IP is allowed.
+    organisation["allowedIps"] = "127.0.0.1/32"
+    organisation.commit()
+    db.session.commit()
+    organisation.reindex()
+    document.reindex()
+    res = client.get(url_for("invenio_records_rest.doc_list"))
+    assert res.status_code == 200
+    assert res.json["hits"]["total"]["value"] == 1
+
+    # Org-scoped view still filters by organisation for anonymous users.
+    res = client.get(url_for("invenio_records_rest.doc_list", view="org"))
+    assert res.status_code == 200
+    assert res.json["hits"]["total"]["value"] == 1
+
+    res = client.get(url_for("invenio_records_rest.doc_list", view="org2"))
+    assert res.status_code == 200
+    assert res.json["hits"]["total"]["value"] == 0

tests/ui/test_views.py

@@ -123,7 +123,7 @@ def test_logged_user(app, client, superuser, admin, moderator, submitter, user):
     assert not res.json["metadata"]["permissions"]["organisations"]["add"]
     assert not res.json["metadata"]["permissions"]["users"]["add"]
     assert res.json["metadata"]["permissions"]["deposits"]["add"]
-    assert not res.json["metadata"]["permissions"]["documents"]["list"]
+    assert res.json["metadata"]["permissions"]["documents"]["list"]
     assert not res.json["metadata"]["permissions"]["organisations"]["list"]
     assert res.json["metadata"]["permissions"]["users"]["list"]
     assert res.json["metadata"]["permissions"]["deposits"]["list"]
@@ -136,7 +136,7 @@ def test_logged_user(app, client, superuser, admin, moderator, submitter, user):
     assert not res.json["metadata"]["permissions"]["organisations"]["add"]
     assert not res.json["metadata"]["permissions"]["users"]["add"]
     assert not res.json["metadata"]["permissions"]["deposits"]["add"]
-    assert not res.json["metadata"]["permissions"]["documents"]["list"]
+    assert res.json["metadata"]["permissions"]["documents"]["list"]
     assert not res.json["metadata"]["permissions"]["organisations"]["list"]
     assert res.json["metadata"]["permissions"]["users"]["list"]
     assert not res.json["metadata"]["permissions"]["deposits"]["list"]