diff options
author | Étienne Loks <etienne.loks@iggdrasil.net> | 2023-04-11 12:27:23 +0200 |
---|---|---|
committer | Étienne Loks <etienne.loks@iggdrasil.net> | 2023-04-17 15:47:16 +0200 |
commit | 367059ddef14a495e277f68ceaf3455c092f839d (patch) | |
tree | ae625ff0265fecd122946c71d3a2d6afefae4817 | |
parent | ff5aee7158bd46e4ae22bc431adadd7060a6e277 (diff) | |
download | Ishtar-367059ddef14a495e277f68ceaf3455c092f839d.tar.bz2 Ishtar-367059ddef14a495e277f68ceaf3455c092f839d.zip |
bandit checker: mark false security issues - fix security issues (low severity)
25 files changed, 176 insertions, 113 deletions
diff --git a/archaeological_context_records/tests.py b/archaeological_context_records/tests.py index 28aa2208b..7a0b41f50 100644 --- a/archaeological_context_records/tests.py +++ b/archaeological_context_records/tests.py @@ -261,11 +261,12 @@ class ContextRecordInit(OperationInitTest): return self.create_context_record(force=force, user=user)[0] def tearDown(self): + # nosec: quick and dirty cleanup do not care to catch exceptions if hasattr(self, "context_records"): for cr in self.context_records: try: cr.delete() - except: + except: # nosec pass self.context_records = [] super(ContextRecordInit, self).tearDown() @@ -955,7 +956,8 @@ class ContextRecordPermissionTest(ContextRecordInit, TestCase): self.alt_user.user_permissions.add( Permission.objects.get(codename="change_own_contextrecord") ) - self.alt_username2, self.alt_password2, self.alt_user2 = create_user( + # nosec: hard coded password for test purposes + self.alt_username2, self.alt_password2, self.alt_user2 = create_user( # nosec username="luke", password="iamyourfather" ) profile = UserProfile.objects.create( diff --git a/archaeological_finds/tests.py b/archaeological_finds/tests.py index 1a62dce4b..467c7d1e1 100644 --- a/archaeological_finds/tests.py +++ b/archaeological_finds/tests.py @@ -145,18 +145,19 @@ class FindInit(ContextRecordInit): def tearDown(self): super(FindInit, self).tearDown() + # nosec: quick and dirty cleanup do not care to catch exceptions if hasattr(self, "finds"): for f in self.finds: try: f.delete() - except: + except: # nosec pass self.finds = [] if hasattr(self, "base_finds"): for f in self.base_finds: try: f.delete() - except: + except: # nosec pass self.base_find = [] @@ -1700,7 +1701,8 @@ class FindPermissionTest(FindInit, TestCase): self.alt_user.user_permissions.add( Permission.objects.get(codename="change_own_find", content_type=ct_find) ) - self.alt_username2, self.alt_password2, self.alt_user2 = create_user( + # nosec: hard coded password for test purposes + self.alt_username2, self.alt_password2, self.alt_user2 = create_user( # nosec username="luke", password="iamyourfather" ) profile = UserProfile.objects.create( diff --git a/archaeological_operations/models.py b/archaeological_operations/models.py index ee2023f5c..57fc4676f 100644 --- a/archaeological_operations/models.py +++ b/archaeological_operations/models.py @@ -3005,7 +3005,7 @@ class AdministrativeAct(DocumentItem, BaseHistorizedItem, OwnPerms, ValueGetter) else: try: self._get_index() - except: + except ValidationError: pass super(AdministrativeAct, self).save(*args, **kwargs) if hasattr(self, "associated_file") and self.associated_file: diff --git a/archaeological_operations/tests.py b/archaeological_operations/tests.py index 8d134e519..605f4e46f 100644 --- a/archaeological_operations/tests.py +++ b/archaeological_operations/tests.py @@ -19,7 +19,8 @@ from bs4 import BeautifulSoup import json import datetime -from subprocess import Popen, PIPE +# nosec: call an explicit bin for testing purpose +from subprocess import Popen, PIPE # nosec from io import BytesIO import tempfile import locale @@ -113,7 +114,8 @@ from ishtar_common.serializers import restore_serialized class FileInit(object): def login_as_superuser(self): - self.client.login(username="username", password="tralala") + # nosec: hard coded password for test purposes + self.client.login(username="username", password="tralala") # nosec def create_file(self): self.extra_models, self.model_list = {}, [] @@ -1659,7 +1661,8 @@ class ParcelTest(ImportTest, TestCase): ) def init_operation_parcels_tests(self): - username, password, user = create_user( + # nosec: hard coded password for test purposes + username, password, user = create_user( # nosec username="Gandalf", password="ushallpass" ) user.user_permissions.add( @@ -1980,10 +1983,11 @@ class OperationInitTest(object): def tearDown(self): # cleanup for further test + # nosec: quick and dirty cleanup do not care to catch exceptions if hasattr(self, "user"): try: self.user.delete() - except: + except: # nosec pass self.user = None # all try/except is necessary for bad migrations on main... @@ -1992,14 +1996,14 @@ class OperationInitTest(object): for ope in self.operations: try: ope.delete() - except: + except: # nosec pass self.operations = [] if hasattr(self, "parcels"): for p in self.parcels: try: p.delete() - except: + except: # nosec pass self.parcels = [] @@ -2347,8 +2351,9 @@ class OperationTest(TestCase, OperationInitTest): ) self.assertEqual(response.status_code, 200) f = BytesIO(response.content) + # nosec: call an explicit bin for testing purpose filetype = ( - Popen("/usr/bin/file -b --mime -", shell=True, stdout=PIPE, stdin=PIPE) + Popen("/usr/bin/file -b --mime -", shell=True, stdout=PIPE, stdin=PIPE) # nosec .communicate(f.read(1024))[0] .strip() ) @@ -2840,7 +2845,8 @@ class OperationSearchTest(TestCase, OperationInitTest, SearchText): self.alt_user.user_permissions.add( Permission.objects.get(codename="change_own_operation") ) - self.alt_username2, self.alt_password2, self.alt_user2 = create_user( + # nosec: hard coded password for test purposes + self.alt_username2, self.alt_password2, self.alt_user2 = create_user( # nosec username="luke", password="iamyourfather" ) profile = UserProfile.objects.create( @@ -3314,7 +3320,8 @@ class OperationPermissionTest(TestCase, OperationInitTest): self.alt_user.user_permissions.add( Permission.objects.get(codename="change_own_operation") ) - self.alt_username2, self.alt_password2, self.alt_user2 = create_user( + # nosec: hard coded password for test purposes + self.alt_username2, self.alt_password2, self.alt_user2 = create_user( # nosec username="luke", password="iamyourfather" ) profile_type = ProfileType.objects.get(txt_idx="collaborator") diff --git a/archaeological_operations/utils.py b/archaeological_operations/utils.py index ba4f17358..a571228b8 100644 --- a/archaeological_operations/utils.py +++ b/archaeological_operations/utils.py @@ -460,11 +460,11 @@ def parse_insee(value): for value in values: try: town = Town.objects.get(numero_insee=value) - towns.append(town) - except: + except Town.DoesNotExist: # sys.stderr.write('Numero INSEE : %s non existant en base' # % value) continue + towns.append(town) return towns @@ -482,8 +482,7 @@ def parse_parcels(parcel_str, insee_code=None, owner=None): if insee_code: town = parse_insee(insee_code) # manage only one town at a time - assert len(town) < 2 - if not town: + if len(town) >= 2 or not town: return parcels town = town[0] m = PARCEL_YEAR_REGEXP.match(parcel_str) diff --git a/archaeological_warehouse/tests.py b/archaeological_warehouse/tests.py index e211e3a2d..54432f9a5 100644 --- a/archaeological_warehouse/tests.py +++ b/archaeological_warehouse/tests.py @@ -616,7 +616,7 @@ class ContainerTest(FindInit, TestCase): self.assertEqual(len(c), 1) # unaccent ct2_label = ct2.label - assert "e" in ct2_label + self.assertIn("e", ct2_label) ct2_label = ct2_label.replace("e", "é") full_path = "{} 35000 {} Test".format(ct2_label, ct.label) response = client.get(url, {"term": full_path}) diff --git a/archaeological_warehouse/views.py b/archaeological_warehouse/views.py index b6796d083..982d7dea2 100644 --- a/archaeological_warehouse/views.py +++ b/archaeological_warehouse/views.py @@ -179,8 +179,9 @@ def autocomplete_container(request, warehouse_id=None): while not index_is_ok and index is not None: for idx, v in enumerate(values): try: - assert unaccent_splitted[index + idx] == v - except (ValueError, AssertionError): + if unaccent_splitted[index + idx] != v: + raise ValueError() + except ValueError: break index_is_ok = True if not index_is_ok: diff --git a/changelog/en/changelog_1.md b/changelog/en/changelog_1.md index 835bb2181..908636336 100644 --- a/changelog/en/changelog_1.md +++ b/changelog/en/changelog_1.md @@ -1,29 +1,33 @@ -v4.0.44 - 2023-04-07 +v4.0.44 - 2023-04-13 -------------------- ### Features/improvements ### - Display of a changelog with alert display when updates are made -- Load task refactoring - manage external_id regen with tasks - Containers: manage history -- Update and fix translations (refs #5578, refs #5579, refs #5581) - Security: - Manage expiration of passwords - Manage strong password policy (ISHTAR_STRONG_PASSWORD_POLICY) with "Each character type" validator - Add default auth validator - Default timeout for session is set to 5 days - Optional security for login attempt: loging, deactivate account after many failed login. - - Force using 128 bits salt for password hasher ### Bug fixes ### - Json fields: fix bad save of multi values - Cascade update from warehouse to containers (refs #5432) - Sheets: fix history view with associated geo - Containers: remove division search -- Importer export: fix pre_importer call - Image detail: do not display Modify link when not relevant (refs #5438) - Fix french label for geo types (refs #5577) - Fix permissions for treatments requests (refs #5441) +### Technical ### +- Load task refactoring - manage external_id regen with tasks +- Update and fix translations (refs #5578, refs #5579, refs #5581) +- Importer export: fix pre_importer call +- Force using 128 bits salt for password hasher +- Mark false security issues detected by bandit +- Fix low severity security issues detected by bandit + v4.0.43 - 2023-03-17 -------------------- diff --git a/changelog/fr/changelog_1.md b/changelog/fr/changelog_1.md index 4e47eed56..cdd0d50a3 100644 --- a/changelog/fr/changelog_1.md +++ b/changelog/fr/changelog_1.md @@ -1,4 +1,4 @@ -v4.0.44 - 2023-04-06 +v4.0.44 - 2023-04-13 -------------------- ### Fonctionnalités/améliorations ### @@ -25,6 +25,8 @@ v4.0.44 - 2023-04-06 - Mise à jour et correction des traductions (#5578, #5579, #5581) - Export d'importeur : correction de l'appel au pre_importer - Utilisation d'une longueur de sel de 128 bits pour le hachage des mots de passe +- Marquage de faux problèmes de sécurité détectés par l'outil "bandit" +- Correction de problèmes de sécurité de faible gravité v4.0.43 - 2023-03-17 -------------------- diff --git a/ishtar_common/admin.py b/ishtar_common/admin.py index ab24ff58b..c824b36f5 100644 --- a/ishtar_common/admin.py +++ b/ishtar_common/admin.py @@ -998,8 +998,8 @@ class ImportGEOJSONActionAdmin(object): json_file = json_file_obj.read() try: dct = json.loads(json_file) - assert "features" in dct - assert dct["features"] + if "features" not in dct or not dct["features"]: + raise ValueError() except (ValueError, AssertionError): error = _("Bad geojson file") return self.import_geojson_error( diff --git a/ishtar_common/data_importer.py b/ishtar_common/data_importer.py index ae3c8387a..796a75699 100644 --- a/ishtar_common/data_importer.py +++ b/ishtar_common/data_importer.py @@ -371,8 +371,9 @@ class YearFormater(Formater): return try: value = int(value) - assert value > 0 and value < (datetime.date.today().year + 30) - except (ValueError, AssertionError): + if value <= 0 or value > (datetime.date.today().year + 30): + raise ValueError() + except ValueError: raise ValueError(_('"%(value)s" is not a valid date') % {"value": value}) return value @@ -384,8 +385,9 @@ class YearNoFuturFormater(Formater): return try: value = int(value) - assert value > 0 and value < datetime.date.today().year - except (ValueError, AssertionError): + if value <= 0 or value > datetime.date.today().year: + raise ValueError() + except ValueError: raise ValueError(_('"%(value)s" is not a valid date') % {"value": value}) return value @@ -674,7 +676,7 @@ class DateFormater(Formater): for date_format in self.date_formats: try: return datetime.datetime.strptime(value, date_format).date() - except: + except ValueError: continue raise ValueError(_('"%(value)s" is not a valid date') % {"value": value}) @@ -1013,7 +1015,8 @@ class Importer(object): self.current_csv_line = None self.conservative_import = conservative_import # for a conservative_import UNICITY_KEYS should be defined - assert not self.conservative_import or bool(self.UNICITY_KEYS) + if self.conservative_import and not bool(self.UNICITY_KEYS): + raise ValueError("A conservative import should have unicity key defined") self.DB_TARGETS = {} self.match_table = {} self.concats = set() @@ -1097,7 +1100,8 @@ class Importer(object): (further exploitation by web interface) - user: associated user """ - assert output in ("silent", "cli", "db") + if output not in ("silent", "cli", "db"): + raise ValueError("initialize called with a bad output option") vals = [] for idx_line, line in enumerate(table): if self.skip_lines > idx_line: @@ -1356,7 +1360,8 @@ class Importer(object): for idx_col, val in enumerate(line): try: self._row_processing(c_row, idx_col, idx_line, val, data) - except: + # nosec: no catch to force continue processing of lines + except: # nosec pass self.validity.append(c_row) @@ -2288,8 +2293,8 @@ class Importer(object): target_name = field.name elif rel_model == obj.__class__: item_name = field.name - assert target_name is not None - assert item_name is not None + if target_name is None or item_name is None: + raise IntegrityError(f"Configuration error for attribute {attr}.") inter_model.objects.get_or_create( **{item_name: obj, target_name: v} ) diff --git a/ishtar_common/forms_common.py b/ishtar_common/forms_common.py index bcc5a28be..f031b280f 100644 --- a/ishtar_common/forms_common.py +++ b/ishtar_common/forms_common.py @@ -320,8 +320,9 @@ class NewImportForm(BaseImportForm): value = self.cleaned_data.get("imported_images_link", None) if value: try: - assert is_downloadable(value) - except (AssertionError, requests.exceptions.RequestException): + if not is_downloadable(value): + raise forms.ValidationError("") + except (requests.exceptions.RequestException, forms.ValidationError): raise forms.ValidationError( _("Invalid link or no file is available for this link.") ) @@ -378,18 +379,21 @@ class NewImportGISForm(BaseImportForm): if value: try: ext = value.name.lower().split(".")[-1] - assert ext in ("zip", "gpkg", "csv") + if ext not in ("zip", "gpkg", "csv"): + raise forms.ValidationError("") if ext == "zip": zip_file = zipfile.ZipFile(value) - assert not zip_file.testzip() + if zip_file.testzip(): + raise forms.ValidationError("") has_correct_file = False for name in zip_file.namelist(): in_ext = name.lower().split(".")[-1] if in_ext in ("shp", "gpkg"): has_correct_file = True break - assert has_correct_file - except AssertionError: + if not has_correct_file: + raise forms.ValidationError("") + except forms.ValidationError: raise forms.ValidationError( _("GIS file must be a zip containing a ShapeFile or GeoPackage file.") ) diff --git a/ishtar_common/ignf_utils.py b/ishtar_common/ignf_utils.py index 94429d458..6be83407f 100644 --- a/ishtar_common/ignf_utils.py +++ b/ishtar_common/ignf_utils.py @@ -1,4 +1,5 @@ -import xml.etree.ElementTree as ET +# nosec: parsing only used by programmer to generate previous dict from a trusted source +import xml.etree.ElementTree as ET # nosec # from the bellow script diff --git a/ishtar_common/libreoffice.py b/ishtar_common/libreoffice.py index 482364a73..7dbb36e30 100644 --- a/ishtar_common/libreoffice.py +++ b/ishtar_common/libreoffice.py @@ -202,7 +202,7 @@ class UnoCalc(UnoClient): if k.startswith("get"): try: print(k, getattr(validation, k)()) - except: + except (AttributeError, TypeError): pass def test_image(self): diff --git a/ishtar_common/models.py b/ishtar_common/models.py index f7baebfe4..ba317998f 100644 --- a/ishtar_common/models.py +++ b/ishtar_common/models.py @@ -36,7 +36,9 @@ import string import tempfile import time from io import BytesIO -from subprocess import Popen, PIPE +# nosec: only script inside the script directory can be executed +# script directory is not web available +from subprocess import Popen, PIPE # nosec from PIL import Image from markdown import markdown from ooopy.OOoPy import OOoPy @@ -45,7 +47,8 @@ import ooopy.Transforms as OOTransforms import uuid import zipfile from urllib.parse import urlencode -from xml.etree import ElementTree as ET +# nosec: ElementTree used to create XML not for parsing +from xml.etree import ElementTree as ET # nosec from django.apps import apps from django.conf import settings @@ -408,9 +411,7 @@ def is_unique(cls, field): # unique validator for models def func(value): query = {field: value} - try: - assert cls.objects.filter(**query).count() == 0 - except AssertionError: + if cls.objects.filter(**query).count() != 0: raise ValidationError(_("This item already exists.")) return func @@ -922,9 +923,8 @@ class RelationsViews(models.Model): Check view or table properly created with settings on the profile :return: True if table or view updated """ - assert cls.CREATE_SQL - assert cls.DELETE_SQL - assert cls.CREATE_TABLE_SQL + if not cls.CREATE_SQL or not cls.DELETE_SQL or not cls.CREATE_TABLE_SQL: + raise NotImplementedError("CREATE_SQL or DELETE_SQL or CREATE_TABLE_SQL is missing.") profile = get_current_profile(force=True) table_type = "" with connection.cursor() as cursor: @@ -2456,7 +2456,8 @@ def documentation_get_gender_values(): class BaseGenderedType(ValueGetter): def get_values(self, prefix="", **kwargs): dct = super(BaseGenderedType, self).get_values(prefix=prefix, **kwargs) - assert hasattr(self, "grammatical_gender") + if not hasattr(self, "grammatical_gender"): + raise NotImplementedError("This model should have a grammatical_gender field") dct[prefix + "grammatical_gender"] = self.grammatical_gender return dct @@ -5144,6 +5145,7 @@ class AdministrationTask(models.Model): script_name = None # only script inside the script directory can be executed + # script directory is not web available for name in os.listdir(script_dir): if name == self.script.path: if os.path.isfile(os.path.join(script_dir, name)): @@ -5165,7 +5167,9 @@ class AdministrationTask(models.Model): self.finished_date = datetime.datetime.now() try: - session = Popen([script_name], stdout=PIPE, stderr=PIPE) + # nosec: only script inside the script directory can be executed + # this script directory is not web available + session = Popen([script_name], stdout=PIPE, stderr=PIPE) # nosec stdout, stderr = session.communicate() except OSError as e: self.state = "FE" diff --git a/ishtar_common/models_common.py b/ishtar_common/models_common.py index fd12f19be..1e6da2b7d 100644 --- a/ishtar_common/models_common.py +++ b/ishtar_common/models_common.py @@ -1356,10 +1356,11 @@ class HistoricalRecords(BaseHistoricalRecords): def create_historical_record(self, instance, history_type, using=None): try: history_modifier = getattr(instance, "history_modifier", None) - assert history_modifier - except (User.DoesNotExist, AssertionError): + except User.DoesNotExist: # on batch removing of users, user could have disappeared return + if not history_modifier: + return history_date = getattr(instance, "_history_date", datetime.datetime.now()) history_change_reason = getattr(instance, "changeReason", None) force = getattr(instance, "_force_history", False) @@ -1550,7 +1551,8 @@ class BaseHistorizedItem( """ Get a "step" previous state of the item """ - assert step or date + if not step and not date: + raise AttributeError("Need to provide step or date") historized = self.history.all() item = None if step: @@ -1720,7 +1722,8 @@ class BaseHistorizedItem( or not self.last_modified: self.last_modified = datetime.datetime.now() if not getattr(self, "skip_history_when_saving", False): - assert hasattr(self, "history_modifier") + if not hasattr(self, "history_modifier"): + raise NotImplementedError("Should have a history_modifier field.") if created: self.history_creator = self.history_modifier # external ID can have related item not available before save @@ -3751,7 +3754,8 @@ class QuickAction: self.target = target self.module = module self.is_popup = is_popup - assert self.target in ("one", "many", None) + if self.target not in ("one", "many", None): + raise AttributeError("target must be one, many or None") def is_available(self, user, session=None, obj=None): if self.module and not getattr(get_current_profile(), self.module): diff --git a/ishtar_common/models_imports.py b/ishtar_common/models_imports.py index 76121d846..e91a94868 100644 --- a/ishtar_common/models_imports.py +++ b/ishtar_common/models_imports.py @@ -49,12 +49,15 @@ from django.template.defaultfilters import slugify from django.utils.functional import cached_property from django.utils.translation import ugettext_lazy as _, pgettext_lazy -try: - assert settings.USE_LIBREOFFICE - from ishtar_common.libreoffice import UnoCalc - from com.sun.star.awt.FontSlant import ITALIC -except (AssertionError, ImportError): - UnoCalc = None +UnoCalc = None +ITALIC = None +if settings.USE_LIBREOFFICE: + try: + from ishtar_common.libreoffice import UnoCalc + from com.sun.star.awt.FontSlant import ITALIC + except ImportError: + pass + from ishtar_common.model_managers import SlugModelManager from ishtar_common.utils import ( diff --git a/ishtar_common/models_rest.py b/ishtar_common/models_rest.py index 411404325..05b1206d1 100644 --- a/ishtar_common/models_rest.py +++ b/ishtar_common/models_rest.py @@ -10,12 +10,14 @@ from django.contrib.postgres.fields import ArrayField from django.core.files import File from django.utils.text import slugify -try: - assert settings.USE_LIBREOFFICE - from ishtar_common.libreoffice import UnoCalc - from com.sun.star.awt.FontSlant import ITALIC -except (AssertionError, ImportError): - UnoCalc = None +UnoCalc = None +ITALIC = None +if settings.USE_LIBREOFFICE: + try: + from ishtar_common.libreoffice import UnoCalc + from com.sun.star.awt.FontSlant import ITALIC + except ImportError: + pass from django.apps import apps from django.template import loader diff --git a/ishtar_common/rest.py b/ishtar_common/rest.py index e489e5f80..08fc2dc36 100644 --- a/ishtar_common/rest.py +++ b/ishtar_common/rest.py @@ -32,7 +32,8 @@ class SearchAPIView(APIView): permission_classes = (permissions.IsAuthenticated, IpModelPermission) def __init__(self, **kwargs): - assert self.model is not None + if not self.model: + raise NotImplementedError("model attribute is not defined") super(SearchAPIView, self).__init__(**kwargs) def search_model_query(self, request): @@ -99,9 +100,10 @@ class FacetAPIView(APIView): select_forms = [] def __init__(self, **kwargs): - assert self.models - assert self.select_forms - assert len(self.models) == len(self.select_forms) + if not self.models or not self.select_forms: + raise NotImplementedError("models or select_forms attribute is not defined") + if len(self.models) != len(self.select_forms): + raise NotImplementedError("len of models and select_forms must be equals") super().__init__(**kwargs) def get(self, request, format=None): @@ -223,7 +225,8 @@ class GetAPIView(generics.RetrieveAPIView): model = None def __init__(self, **kwargs): - assert self.model is not None + if self.model is None: + raise NotImplementedError("model attribute is not defined") super().__init__(**kwargs) def search_model_query(self, request): diff --git a/ishtar_common/tests.py b/ishtar_common/tests.py index 97002f13d..7328a5f95 100644 --- a/ishtar_common/tests.py +++ b/ishtar_common/tests.py @@ -171,7 +171,8 @@ def create_superuser(): return username, password, user -def create_user(username="username678", password="dcbqj756aaa456!@%"): +# nosec: hard coded password used for test +def create_user(username="username678", password="dcbqj756aaa456!@%"): # nosec q = User.objects.filter(username=username) if q.count(): return username, password, q.all()[0] @@ -245,7 +246,8 @@ class SearchText: SEARCH_URL = "" def _test_search(self, client, result, context=""): - assert self.SEARCH_URL + if not self.SEARCH_URL: + raise NotImplementedError("No SEARCH_URL defined.") for q, expected_result in result: search = {"search_vector": q} response = client.get(reverse(self.SEARCH_URL), search) diff --git a/ishtar_common/utils.py b/ishtar_common/utils.py index 91591e0b2..1757612ef 100644 --- a/ishtar_common/utils.py +++ b/ishtar_common/utils.py @@ -36,7 +36,8 @@ import requests from secretary import Renderer as MainSecretaryRenderer, UndefinedSilently import shutil import string -import subprocess +# nosec: no user input +import subprocess # nosec import sys import tempfile import time @@ -751,7 +752,8 @@ def get_random_item_image_link(request): if not total: return "" - image_nb = random.randint(0, total - 1) + # nosec: not used for security/cryptographic purposes + image_nb = random.randint(0, total - 1) # nosec return _get_image_link(q.all()[image_nb]) @@ -1413,7 +1415,8 @@ def generate_relation_graph( svg_tmp_name = tempdir + os.path.sep + "relations.svg" with open(svg_tmp_name, "w") as svg_file: - popen = subprocess.Popen(args, stdout=svg_file) + # nosec: no user input + popen = subprocess.Popen(args, stdout=svg_file) # nosec popen.wait() # scale image if necessary @@ -2132,7 +2135,8 @@ def generate_pdf_preview(item, save=True, tempdir=None, page_number=None): item.associated_file.path, preview_tmp_name) try: - popen = subprocess.Popen(args) + # nosec: no user input + popen = subprocess.Popen(args) # nosec popen.wait(timeout=5) except subprocess.SubprocessError: return @@ -2183,7 +2187,7 @@ def create_osm_town(rel_id, name, numero_insee=None): retry += 1 try: geojson = response.json() - except: + except requests.JSONDecodeError: requests.get(OSM_REFRESH_URL.format(rel_id)) time.sleep(3) if not geojson: @@ -2194,7 +2198,7 @@ def create_osm_town(rel_id, name, numero_insee=None): try: geojson_simplify = response.json() geojson = geojson_simplify - except: + except requests.JSONDecodeError: pass default = {"name": name} if not numero_insee: diff --git a/ishtar_common/utils_migrations.py b/ishtar_common/utils_migrations.py index ec0b46509..1aead83e8 100644 --- a/ishtar_common/utils_migrations.py +++ b/ishtar_common/utils_migrations.py @@ -96,7 +96,7 @@ def reinit_last_modified(apps, app_name, models): model = apps.get_model(app_name, model_name) try: historical_model = apps.get_model(app_name, "Historical" + model_name) - except: + except LookupError: continue for item in model.objects.all(): q = historical_model.objects.filter(id=item.pk).order_by("-history_date") @@ -192,7 +192,7 @@ def migrate_created_field(apps, app_name, model_names): model = apps.get_model(app_name, model_name) try: model_history = apps.get_model(app_name, "Historical" + model_name) - except: + except LookupError: continue for item in model.objects.all(): q = model_history.objects.filter(id=item.pk).order_by("history_date") diff --git a/ishtar_common/views.py b/ishtar_common/views.py index ac4e995d1..ba9be495a 100644 --- a/ishtar_common/views.py +++ b/ishtar_common/views.py @@ -1256,8 +1256,9 @@ class QRCodeView(DynamicModelView, IshtarMixin, LoginRequiredMixin, View): model = self.get_model(kwargs) try: item = model.objects.get(pk=kwargs.get("pk")) - assert hasattr(item, "qrcode") - except (model.DoesNotExist, AssertionError): + except model.DoesNotExist: + raise Http404() + if not hasattr(item, "qrcode"): raise Http404() if not item.qrcode or not item.qrcode.name: @@ -2191,8 +2192,9 @@ class DocumentEditView(DocumentFormMixin, UpdateView): kwargs = super(DocumentEditView, self).get_form_kwargs() try: document = models.Document.objects.get(pk=self.kwargs.get("pk")) - assert check_permission(self.request, "document/edit", document.pk) - except (AssertionError, models.Document.DoesNotExist): + except models.Document.DoesNotExist: + raise Http404() + if not check_permission(self.request, "document/edit", document.pk): raise Http404() initial = {} for k in ( @@ -2475,7 +2477,8 @@ class QAItemForm(IshtarMixin, LoginRequiredMixin, FormView): return self.model.get_quick_action_by_url(self.base_url) def pre_dispatch(self, request, *args, **kwargs): - assert self.model + if not self.model: + raise NotImplementedError("No attribute model defined.") pks = [int(pk) for pk in kwargs.get("pks").split("-")] self.items = list(self.model.objects.filter(pk__in=pks)) if not self.items: @@ -2836,8 +2839,10 @@ class GeoEditView(GeoFormMixin, UpdateView): kwargs = super(GeoEditView, self).get_form_kwargs() try: geo = models.GeoVectorData.objects.get(pk=self.kwargs.get("pk")) - assert check_permission(self.request, "geo/edit", geo.pk) - except (AssertionError, models.GeoVectorData.DoesNotExist): + except models.GeoVectorData.DoesNotExist: + raise Http404() + + if not check_permission(self.request, "geo/edit", geo.pk): raise Http404() initial = {} diff --git a/ishtar_common/views_item.py b/ishtar_common/views_item.py index 5947a6798..b2b924992 100644 --- a/ishtar_common/views_item.py +++ b/ishtar_common/views_item.py @@ -9,7 +9,8 @@ import json import logging import re import requests -import subprocess +# nosec: no user input used +import subprocess # nosec from tempfile import NamedTemporaryFile import unidecode @@ -405,12 +406,13 @@ def show_item(model, name, extra_dct=None, model_for_perms=None): dct["IS_HISTORY"] = True if item.get_last_history_date() != date: item = item.get_previous(date=date) - assert item is not None + if item is None: + raise ValueError("No previous history entry.") dct["previous"] = item._previous dct["next"] = item._next else: date = None - except (ValueError, AssertionError): + except ValueError: return HttpResponse("", content_type="text/plain") if not date: historized = item.history.all() @@ -466,7 +468,8 @@ def show_item(model, name, extra_dct=None, model_for_perms=None): html_source.name, ] try: - subprocess.check_call( + # nosec: no user input + subprocess.check_call( # nosec pandoc_args, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL ) except subprocess.CalledProcessError: @@ -1011,7 +1014,7 @@ def _manage_facet_search(model, dct, and_reqs): rel = getattr(model, base_k).field.related_model if not hasattr(rel, "label") and hasattr(rel, "cached_label"): lbl_name = "__cached_label__" - except: + except AttributeError: pass suffix = ( "{}icontains".format(lbl_name) @@ -2311,7 +2314,8 @@ def get_item( try: start = int(request_items.get("start")) page_nb = start // row_nb + 1 - assert page_nb >= 1 + if page_nb < 1: + raise ValueError("Page number is not relevant.") except (TypeError, ValueError, AssertionError): start = 0 page_nb = 1 diff --git a/ishtar_common/wizards.py b/ishtar_common/wizards.py index 030bb4af2..8dcb16b70 100644 --- a/ishtar_common/wizards.py +++ b/ishtar_common/wizards.py @@ -587,9 +587,10 @@ class Wizard(IshtarWizard): fields.pop("DELETE") multi = len(fields) > 1 if multi: - assert hasattr(frm, "base_model") or hasattr( - frm, "base_models" - ), "Must define a base_model(s) for " + str(frm.__class__) + if not hasattr(frm, "base_model") and not hasattr(frm, "base_models"): + raise NotImplementedError( + f"Must define a base_model(s) for {frm.__class__}" + ) for frm in form.forms: if not frm.is_valid(): continue @@ -703,7 +704,8 @@ class Wizard(IshtarWizard): continue vals = k.split("__") - assert len(vals) == 2, "Only one level of dependant item is managed" + if len(vals) != 2: + raise NotImplementedError("Only one level of dependant item is managed") dependant_item, key = vals if dependant_item not in other_objs: other_objs[dependant_item] = {} @@ -906,9 +908,10 @@ class Wizard(IshtarWizard): model = related_model.through # not m2m -> foreign key if not hasattr(related_model, "clear"): - assert hasattr( - model, "MAIN_ATTR" - ), "Must define a MAIN_ATTR for " + str(model.__class__) + if not hasattr(model, "MAIN_ATTR"): + raise NotImplementedError( + f"Must define a MAIN_ATTR for {model.__class__}." + ) value[getattr(model, "MAIN_ATTR")] = obj # check old links @@ -1112,7 +1115,7 @@ class Wizard(IshtarWizard): idx = items[-2] try: int(idx) - except: + except ValueError: continue if items[-1] == "DELETE": to_delete.add(idx) @@ -1710,7 +1713,8 @@ class DeletionWizard(Wizard): hasattr(self, "model") and hasattr(self.model, "TABLE_COLS") ): self.fields = self.model.TABLE_COLS - assert self.model + if not self.model: + raise NotImplementedError("Missing model attribute") super(DeletionWizard, self).__init__(*args, **kwargs) def get_formated_datas(self, forms): @@ -1785,7 +1789,8 @@ class MultipleDeletionWizard(MultipleItemWizard): hasattr(self, "model") and hasattr(self.model, "TABLE_COLS") ): self.fields = self.model.TABLE_COLS - assert self.model + if not self.model: + raise NotImplementedError("Missing model attribute") super(MultipleDeletionWizard, self).__init__(*args, **kwargs) def get_template_names(self): |