documents: store allowed ips addresses in CIDR format

Note: this requires a data migration, the document index should be recreated. * Fixes slow OAI-PHM requests that returns 500 errors. * Fixes slow document api requests. Co-Authored-by: Johnny Mariéthoz <[email protected]>
Garfield-fr · Feb 9, 2022 · 1dece3b · 1dece3b
1 parent cb7fad2
commit 1dece3b
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 5 deletions.
diff --git a/sonar/modules/documents/mappings/v7/documents/document-v1.0.0.json b/sonar/modules/documents/mappings/v7/documents/document-v1.0.0.json
@@ -106,6 +106,9 @@
             "type": "text"
           },
           "ips": {
+            "type": "ip_range"
+          },
+          "allowedIps": {
             "type": "keyword"
           }
         }

diff --git a/sonar/modules/utils.py b/sonar/modules/utils.py
@@ -297,7 +297,8 @@ def get_ips_list(ranges):
 
     :param list ranges: List of ranges.
     :returns: List of IP addresses.
-    :rtype: list
+    :rtype: list of cidr ips
+            (https://en.wikipedia.org/wiki/Classless_Inter-Domain_Routing)
     """
     ip_set = IPSet()
 
@@ -315,7 +316,7 @@ def get_ips_list(ranges):
         except Exception:
             pass
 
-    return [str(ip) for ip in ip_set]
+    return [str(ip.cidr) for ip in ip_set.iter_cidrs()]
 
 
 def file_download_ui(pid, record, _record_file_factory=None, **kwargs):

diff --git a/tests/api/documents/test_documents_query.py b/tests/api/documents/test_documents_query.py
@@ -71,6 +71,26 @@ def test_masked_document(db, client, organisation, document, es_clear):
     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', view='global'))
+    assert res.status_code == 200
+    assert res.json['hits']['total']['value'] == 1
+
+    # Masked for external IPs, IP is allowed
+    organisation['allowedIps'] = '127.0.0.*'
+    organisation.commit()
+    db.session.commit()
+    organisation.reindex()
+    document.reindex()
+    res = client.get(url_for('invenio_records_rest.doc_list', view='global'))
+    assert res.status_code == 200
+    assert res.json['hits']['total']['value'] == 1
+
     # Masked for external IPs, IP is allowed
     organisation['allowedIps'] = '127.0.0.1'
     organisation.commit()
@@ -80,3 +100,13 @@ def test_masked_document(db, client, organisation, document, es_clear):
     res = client.get(url_for('invenio_records_rest.doc_list', view='global'))
     assert res.status_code == 200
     assert res.json['hits']['total']['value'] == 1
+
+    # Masked for external IPs, IP is not allowed
+    organisation['allowedIps'] = '192.168.1.1'
+    organisation.commit()
+    db.session.commit()
+    organisation.reindex()
+    document.reindex()
+    res = client.get(url_for('invenio_records_rest.doc_list', view='global'))
+    assert res.status_code == 200
+    assert res.json['hits']['total']['value'] == 0
diff --git a/tests/ui/test_utils.py b/tests/ui/test_utils.py
@@ -232,6 +232,6 @@ def test_get_current_ip(app):
 def test_get_ips_list():
     """Test get IP list."""
     ranges = ['127.0.0.1', '192.168.1.3-5', '12.13.14.15/32']
-    assert get_ips_list(ranges) == [
-        '12.13.14.15', '127.0.0.1', '192.168.1.3', '192.168.1.4', '192.168.1.5'
-    ]
+    assert set(get_ips_list(ranges)) == set([
+        '12.13.14.15/32', '127.0.0.1/32', '192.168.1.3/32', '192.168.1.4/31'
+    ])