Return 400 instead of 500 in case of not ObjectID arg (#2889)

CHANGELOG.md

@@ -16,6 +16,7 @@
 - Add a dict of URIs to replace in a RDF graph at harvest time [#2884](https://github.com/opendatateam/udata/pull/2884)
 - Fix duplicate recipients in new comments mail [#2886](https://github.com/opendatateam/udata/pull/2886)
 - Add type to resource csv adapter [#2888](https://github.com/opendatateam/udata/pull/2888)
+- Return 400 instead of 500 in case of not ObjectID arg in API [#2889](https://github.com/opendatateam/udata/pull/2889)
 
 ## 6.1.6 (2023-07-19)
 

udata/core/dataset/api.py

@@ -21,6 +21,7 @@
 import logging
 from datetime import datetime
 
+from bson.objectid import ObjectId
 from flask import request, current_app, abort, redirect, url_for, make_response
 from flask_security import current_user
 from mongoengine.queryset.visitor import Q
@@ -118,8 +119,12 @@ def parse_filters(datasets, args):
         if args.get('featured'):
             datasets = datasets.filter(featured=args['featured'])
         if args.get('organization'):
+            if not ObjectId.is_valid(args['organization']):
+                api.abort(400, 'Organization arg must be an identifier')
             datasets = datasets.filter(organization=args['organization'])
         if args.get('owner'):
+            if not ObjectId.is_valid(args['owner']):
+                api.abort(400, 'Owner arg must be an identifier')
             datasets = datasets.filter(owner=args['owner'])
         if args.get('format'):
             datasets = datasets.filter(resources__format=args['format'])

udata/core/reuse/api.py

@@ -1,3 +1,4 @@
+from bson.objectid import ObjectId
 from datetime import datetime
 
 from flask import request
@@ -60,6 +61,8 @@ def parse_filters(reuses, args):
             phrase_query = ' '.join([f'"{elem}"' for elem in args['q'].split(' ')])
             reuses = reuses.search_text(phrase_query)
         if args.get('dataset'):
+            if not ObjectId.is_valid(args['dataset']):
+                api.abort(400, 'Dataset arg must be an identifier')
             reuses = reuses.filter(datasets=args['dataset'])
         if args.get('featured'):
             reuses = reuses.filter(featured=args['featured'])
@@ -70,8 +73,12 @@ def parse_filters(reuses, args):
         if args.get('tag'):
             reuses = reuses.filter(tags=args['tag'])
         if args.get('organization'):
+            if not ObjectId.is_valid(args['organization']):
+                api.abort(400, 'Organization arg must be an identifier')
             reuses = reuses.filter(organization=args['organization'])
         if args.get('owner'):
+            if not ObjectId.is_valid(args['owner']):
+                api.abort(400, 'Owner arg must be an identifier')
             reuses = reuses.filter(owner=args['owner'])
         return reuses
 

udata/tests/api/test_datasets_api.py

@@ -207,12 +207,18 @@ def test_dataset_api_list_with_filters(self):
         self.assertEqual(len(response.json['data']), 1)
         self.assertEqual(response.json['data'][0]['id'], str(owner_dataset.id))
 
+        response = self.get(url_for('api.datasets', owner='owner-id'))
+        self.assert400(response)
+
         # filter on organization
         response = self.get(url_for('api.datasets', organization=org.id))
         self.assert200(response)
         self.assertEqual(len(response.json['data']), 1)
         self.assertEqual(response.json['data'][0]['id'], str(org_dataset.id))
 
+        response = self.get(url_for('api.datasets', organization='org-id'))
+        self.assert400(response)
+
         # filter on schema
         response = self.get(url_for('api.datasets', schema='my-schema'))
         self.assert200(response)

udata/tests/api/test_reuses_api.py

@@ -84,6 +84,12 @@ def test_reuse_api_list_with_filters(self, api):
         assert len(response.json['data']) == 1
         assert response.json['data'][0]['id'] == str(org_reuse.id)
 
+        response = api.get(url_for('api.reuses', owner='owner-id'))
+        assert400(response)
+
+        response = api.get(url_for('api.reuses', organization='org-id'))
+        assert400(response)
+
     def test_reuse_api_get(self, api):
         '''It should fetch a reuse from the API'''
         reuse = ReuseFactory()