Reworks custom field value validation to check and return a 400 error code in more cases and support more URL looking items, not just some basic schemes
This commit is contained in:
Trenton H
parent
f4511893e1
commit
c626e96f0f
@ -2,6 +2,7 @@ import datetime
|
||||
import math
|
||||
import re
|
||||
import zoneinfo
|
||||
from decimal import Decimal
|
||||
|
||||
import magic
|
||||
from celery import states
|
||||
@ -9,7 +10,9 @@ from django.conf import settings
|
||||
from django.contrib.auth.models import Group
|
||||
from django.contrib.auth.models import User
|
||||
from django.contrib.contenttypes.models import ContentType
|
||||
from django.core.validators import URLValidator
|
||||
from django.core.validators import DecimalValidator
|
||||
from django.core.validators import MaxLengthValidator
|
||||
from django.core.validators import integer_validator
|
||||
from django.utils.crypto import get_random_string
|
||||
from django.utils.text import slugify
|
||||
from django.utils.translation import gettext as _
|
||||
@ -41,7 +44,7 @@ from documents.models import UiSettings
|
||||
from documents.parsers import is_mime_type_supported
|
||||
from documents.permissions import get_groups_with_only_permission
|
||||
from documents.permissions import set_permissions_for_object
|
||||
from documents.utils import ALL_URL_SCHEMES
|
||||
from documents.validators import uri_validator
|
||||
|
||||
|
||||
# https://www.django-rest-framework.org/api-guide/serializers/#example
|
||||
@ -490,17 +493,29 @@ class CustomFieldInstanceSerializer(serializers.ModelSerializer):
|
||||
|
||||
def validate(self, data):
|
||||
"""
|
||||
For some reason, URLField validation is not run against the value
|
||||
automatically. Force it to run against the value
|
||||
Probably because we're kind of doing it odd, validation from the model
|
||||
doesn't run against the field "value", so we have to re-create it here.
|
||||
|
||||
Don't like it, but it is better than returning an HTTP 500 when the database
|
||||
hates the value
|
||||
"""
|
||||
data = super().validate(data)
|
||||
field: CustomField = data["field"]
|
||||
if (
|
||||
field.data_type == CustomField.FieldDataType.URL
|
||||
and data["value"] is not None
|
||||
and len(data["value"]) > 0
|
||||
):
|
||||
URLValidator(schemes=ALL_URL_SCHEMES)(data["value"])
|
||||
if "value" in data and data["value"] is not None:
|
||||
if (
|
||||
field.data_type == CustomField.FieldDataType.URL
|
||||
and len(data["value"]) > 0
|
||||
):
|
||||
uri_validator(data["value"])
|
||||
elif field.data_type == CustomField.FieldDataType.INT:
|
||||
integer_validator(data["value"])
|
||||
elif field.data_type == CustomField.FieldDataType.MONETARY:
|
||||
DecimalValidator(max_digits=12, decimal_places=2)(
|
||||
Decimal(str(data["value"])),
|
||||
)
|
||||
elif field.data_type == CustomField.FieldDataType.STRING:
|
||||
MaxLengthValidator(limit_value=128)(data["value"])
|
||||
|
||||
return data
|
||||
|
||||
def reflect_doclinks(
|
||||
|
||||
@ -333,19 +333,17 @@ class TestCustomField(DirectoriesMixin, APITestCase):
|
||||
},
|
||||
format="json",
|
||||
)
|
||||
from pprint import pprint
|
||||
|
||||
pprint(resp.json())
|
||||
self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST)
|
||||
self.assertEqual(CustomFieldInstance.objects.count(), 0)
|
||||
self.assertEqual(len(doc.custom_fields.all()), 0)
|
||||
|
||||
def test_custom_field_value_validation(self):
|
||||
def test_custom_field_value_url_validation(self):
|
||||
"""
|
||||
GIVEN:
|
||||
- Document & custom field exist
|
||||
WHEN:
|
||||
- API request to set a field value
|
||||
- API request to set a field value to something which is or is not a link
|
||||
THEN:
|
||||
- HTTP 400 is returned
|
||||
- No field instance is created or attached to the document
|
||||
@ -360,31 +358,62 @@ class TestCustomField(DirectoriesMixin, APITestCase):
|
||||
name="Test Custom Field URL",
|
||||
data_type=CustomField.FieldDataType.URL,
|
||||
)
|
||||
custom_field_int = CustomField.objects.create(
|
||||
name="Test Custom Field INT",
|
||||
data_type=CustomField.FieldDataType.INT,
|
||||
)
|
||||
|
||||
for value in ["not a url", "file:"]:
|
||||
with self.subTest(f"Test value {value}"):
|
||||
resp = self.client.patch(
|
||||
f"/api/documents/{doc.id}/",
|
||||
data={
|
||||
"custom_fields": [
|
||||
{
|
||||
"field": custom_field_url.id,
|
||||
"value": value,
|
||||
},
|
||||
],
|
||||
},
|
||||
format="json",
|
||||
)
|
||||
|
||||
self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST)
|
||||
self.assertEqual(CustomFieldInstance.objects.count(), 0)
|
||||
self.assertEqual(len(doc.custom_fields.all()), 0)
|
||||
resp = self.client.patch(
|
||||
f"/api/documents/{doc.id}/",
|
||||
data={
|
||||
"custom_fields": [
|
||||
{
|
||||
"field": custom_field_url.id,
|
||||
"value": "not a url",
|
||||
"value": "tel:+1-816-555-1212",
|
||||
},
|
||||
],
|
||||
},
|
||||
format="json",
|
||||
)
|
||||
|
||||
self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST)
|
||||
self.assertEqual(CustomFieldInstance.objects.count(), 0)
|
||||
self.assertEqual(len(doc.custom_fields.all()), 0)
|
||||
self.assertEqual(resp.status_code, status.HTTP_200_OK)
|
||||
|
||||
self.assertRaises(
|
||||
Exception,
|
||||
self.client.patch,
|
||||
def test_custom_field_value_integer_validation(self):
|
||||
"""
|
||||
GIVEN:
|
||||
- Document & custom field exist
|
||||
WHEN:
|
||||
- API request to set a field value to something not an integer
|
||||
THEN:
|
||||
- HTTP 400 is returned
|
||||
- No field instance is created or attached to the document
|
||||
"""
|
||||
doc = Document.objects.create(
|
||||
title="WOW",
|
||||
content="the content",
|
||||
checksum="123",
|
||||
mime_type="application/pdf",
|
||||
)
|
||||
custom_field_int = CustomField.objects.create(
|
||||
name="Test Custom Field INT",
|
||||
data_type=CustomField.FieldDataType.INT,
|
||||
)
|
||||
|
||||
resp = self.client.patch(
|
||||
f"/api/documents/{doc.id}/",
|
||||
data={
|
||||
"custom_fields": [
|
||||
@ -397,6 +426,81 @@ class TestCustomField(DirectoriesMixin, APITestCase):
|
||||
format="json",
|
||||
)
|
||||
|
||||
self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST)
|
||||
self.assertEqual(CustomFieldInstance.objects.count(), 0)
|
||||
self.assertEqual(len(doc.custom_fields.all()), 0)
|
||||
|
||||
def test_custom_field_value_monetary_validation(self):
|
||||
"""
|
||||
GIVEN:
|
||||
- Document & custom field exist
|
||||
WHEN:
|
||||
- API request to set a field value to something not a valid monetary decimal
|
||||
THEN:
|
||||
- HTTP 400 is returned
|
||||
- No field instance is created or attached to the document
|
||||
"""
|
||||
doc = Document.objects.create(
|
||||
title="WOW",
|
||||
content="the content",
|
||||
checksum="123",
|
||||
mime_type="application/pdf",
|
||||
)
|
||||
custom_field_money = CustomField.objects.create(
|
||||
name="Test Custom Field MONETARY",
|
||||
data_type=CustomField.FieldDataType.MONETARY,
|
||||
)
|
||||
|
||||
resp = self.client.patch(
|
||||
f"/api/documents/{doc.id}/",
|
||||
data={
|
||||
"custom_fields": [
|
||||
{
|
||||
"field": custom_field_money.id,
|
||||
# Too many places past decimal
|
||||
"value": 12.123,
|
||||
},
|
||||
],
|
||||
},
|
||||
format="json",
|
||||
)
|
||||
|
||||
self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST)
|
||||
self.assertEqual(CustomFieldInstance.objects.count(), 0)
|
||||
self.assertEqual(len(doc.custom_fields.all()), 0)
|
||||
|
||||
def test_custom_field_value_short_text_validation(self):
|
||||
"""
|
||||
GIVEN:
|
||||
- Document & custom field exist
|
||||
WHEN:
|
||||
- API request to set a field value to a too long string
|
||||
THEN:
|
||||
- HTTP 400 is returned
|
||||
- No field instance is created or attached to the document
|
||||
"""
|
||||
doc = Document.objects.create(
|
||||
title="WOW",
|
||||
content="the content",
|
||||
checksum="123",
|
||||
mime_type="application/pdf",
|
||||
)
|
||||
custom_field_string = CustomField.objects.create(
|
||||
name="Test Custom Field STRING",
|
||||
data_type=CustomField.FieldDataType.STRING,
|
||||
)
|
||||
|
||||
resp = self.client.patch(
|
||||
f"/api/documents/{doc.id}/",
|
||||
data={
|
||||
"custom_fields": [
|
||||
{"field": custom_field_string.id, "value": "a" * 129},
|
||||
],
|
||||
},
|
||||
format="json",
|
||||
)
|
||||
|
||||
self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST)
|
||||
self.assertEqual(CustomFieldInstance.objects.count(), 0)
|
||||
self.assertEqual(len(doc.custom_fields.all()), 0)
|
||||
|
||||
|
||||
@ -40,381 +40,3 @@ def copy_file_with_basic_stats(
|
||||
|
||||
shutil.copy(source, dest)
|
||||
copy_basic_file_stats(source, dest)
|
||||
|
||||
|
||||
ALL_URL_SCHEMES = [
|
||||
"aaa",
|
||||
"aaas",
|
||||
"about",
|
||||
"acap",
|
||||
"acct",
|
||||
"acd",
|
||||
"acr",
|
||||
"adiumxtra",
|
||||
"adt",
|
||||
"afp",
|
||||
"afs",
|
||||
"aim",
|
||||
"amss",
|
||||
"android",
|
||||
"appdata",
|
||||
"apt",
|
||||
"ar",
|
||||
"ark",
|
||||
"at",
|
||||
"attachment",
|
||||
"aw",
|
||||
"barion",
|
||||
"bb",
|
||||
"beshare",
|
||||
"bitcoin",
|
||||
"bitcoincash",
|
||||
"blob",
|
||||
"bolo",
|
||||
"brid",
|
||||
"browserext",
|
||||
"cabal",
|
||||
"calculator",
|
||||
"callto",
|
||||
"cap",
|
||||
"cast",
|
||||
"casts",
|
||||
"chrome",
|
||||
"chrome-extension",
|
||||
"cid",
|
||||
"coap",
|
||||
"coap+tcp",
|
||||
"coap+ws",
|
||||
"coaps",
|
||||
"coaps+tcp",
|
||||
"coaps+ws",
|
||||
"com-eventbrite-attendee",
|
||||
"content",
|
||||
"content-type",
|
||||
"crid",
|
||||
"cstr",
|
||||
"cvs",
|
||||
"dab",
|
||||
"dat",
|
||||
"data",
|
||||
"dav",
|
||||
"dhttp",
|
||||
"diaspora",
|
||||
"dict",
|
||||
"did",
|
||||
"dis",
|
||||
"dlna-playcontainer",
|
||||
"dlna-playsingle",
|
||||
"dns",
|
||||
"dntp",
|
||||
"doi",
|
||||
"dpp",
|
||||
"drm",
|
||||
"drop",
|
||||
"dtmi",
|
||||
"dtn",
|
||||
"dvb",
|
||||
"dvx",
|
||||
"dweb",
|
||||
"ed2k",
|
||||
"eid",
|
||||
"elsi",
|
||||
"embedded",
|
||||
"ens",
|
||||
"ethereum",
|
||||
"example",
|
||||
"facetime",
|
||||
"fax",
|
||||
"feed",
|
||||
"feedready",
|
||||
"fido",
|
||||
"file",
|
||||
"filesystem",
|
||||
"finger",
|
||||
"first-run-pen-experience",
|
||||
"fish",
|
||||
"fm",
|
||||
"ftp",
|
||||
"fuchsia-pkg",
|
||||
"geo",
|
||||
"gg",
|
||||
"git",
|
||||
"gitoid",
|
||||
"gizmoproject",
|
||||
"go",
|
||||
"gopher",
|
||||
"graph",
|
||||
"grd",
|
||||
"gtalk",
|
||||
"h323",
|
||||
"ham",
|
||||
"hcap",
|
||||
"hcp",
|
||||
"http",
|
||||
"https",
|
||||
"hxxp",
|
||||
"hxxps",
|
||||
"hydrazone",
|
||||
"hyper",
|
||||
"iax",
|
||||
"icap",
|
||||
"icon",
|
||||
"im",
|
||||
"imap",
|
||||
"info",
|
||||
"iotdisco",
|
||||
"ipfs",
|
||||
"ipn",
|
||||
"ipns",
|
||||
"ipp",
|
||||
"ipps",
|
||||
"irc",
|
||||
"irc6",
|
||||
"ircs",
|
||||
"iris",
|
||||
"iris.beep",
|
||||
"iris.lwz",
|
||||
"iris.xpc",
|
||||
"iris.xpcs",
|
||||
"isostore",
|
||||
"itms",
|
||||
"jabber",
|
||||
"jar",
|
||||
"jms",
|
||||
"keyparc",
|
||||
"lastfm",
|
||||
"lbry",
|
||||
"ldap",
|
||||
"ldaps",
|
||||
"leaptofrogans",
|
||||
"lid",
|
||||
"lorawan",
|
||||
"lpa",
|
||||
"lvlt",
|
||||
"magnet",
|
||||
"mailserver",
|
||||
"mailto",
|
||||
"maps",
|
||||
"market",
|
||||
"matrix",
|
||||
"message",
|
||||
"microsoft.windows.camera",
|
||||
"microsoft.windows.camera.multipicker",
|
||||
"microsoft.windows.camera.picker",
|
||||
"mid",
|
||||
"mms",
|
||||
"modem",
|
||||
"mongodb",
|
||||
"moz",
|
||||
"ms-access",
|
||||
"ms-appinstaller",
|
||||
"ms-browser-extension",
|
||||
"ms-calculator",
|
||||
"ms-drive-to",
|
||||
"ms-enrollment",
|
||||
"ms-excel",
|
||||
"ms-eyecontrolspeech",
|
||||
"ms-gamebarservices",
|
||||
"ms-gamingoverlay",
|
||||
"ms-getoffice",
|
||||
"ms-help",
|
||||
"ms-infopath",
|
||||
"ms-inputapp",
|
||||
"ms-launchremotedesktop",
|
||||
"ms-lockscreencomponent-config",
|
||||
"ms-media-stream-id",
|
||||
"ms-meetnow",
|
||||
"ms-mixedrealitycapture",
|
||||
"ms-mobileplans",
|
||||
"ms-newsandinterests",
|
||||
"ms-officeapp",
|
||||
"ms-people",
|
||||
"ms-project",
|
||||
"ms-powerpoint",
|
||||
"ms-publisher",
|
||||
"ms-remotedesktop",
|
||||
"ms-remotedesktop-launch",
|
||||
"ms-restoretabcompanion",
|
||||
"ms-screenclip",
|
||||
"ms-screensketch",
|
||||
"ms-search",
|
||||
"ms-search-repair",
|
||||
"ms-secondary-screen-controller",
|
||||
"ms-secondary-screen-setup",
|
||||
"ms-settings",
|
||||
"ms-settings-airplanemode",
|
||||
"ms-settings-bluetooth",
|
||||
"ms-settings-camera",
|
||||
"ms-settings-cellular",
|
||||
"ms-settings-cloudstorage",
|
||||
"ms-settings-connectabledevices",
|
||||
"ms-settings-displays-topology",
|
||||
"ms-settings-emailandaccounts",
|
||||
"ms-settings-language",
|
||||
"ms-settings-location",
|
||||
"ms-settings-lock",
|
||||
"ms-settings-nfctransactions",
|
||||
"ms-settings-notifications",
|
||||
"ms-settings-power",
|
||||
"ms-settings-privacy",
|
||||
"ms-settings-proximity",
|
||||
"ms-settings-screenrotation",
|
||||
"ms-settings-wifi",
|
||||
"ms-settings-workplace",
|
||||
"ms-spd",
|
||||
"ms-stickers",
|
||||
"ms-sttoverlay",
|
||||
"ms-transit-to",
|
||||
"ms-useractivityset",
|
||||
"ms-virtualtouchpad",
|
||||
"ms-visio",
|
||||
"ms-walk-to",
|
||||
"ms-whiteboard",
|
||||
"ms-whiteboard-cmd",
|
||||
"ms-word",
|
||||
"msnim",
|
||||
"msrp",
|
||||
"msrps",
|
||||
"mss",
|
||||
"mt",
|
||||
"mtqp",
|
||||
"mumble",
|
||||
"mupdate",
|
||||
"mvn",
|
||||
"news",
|
||||
"nfs",
|
||||
"ni",
|
||||
"nih",
|
||||
"nntp",
|
||||
"notes",
|
||||
"num",
|
||||
"ocf",
|
||||
"oid",
|
||||
"onenote",
|
||||
"onenote-cmd",
|
||||
"opaquelocktoken",
|
||||
"openid",
|
||||
"openpgp4fpr",
|
||||
"otpauth",
|
||||
"p1",
|
||||
"pack",
|
||||
"palm",
|
||||
"paparazzi",
|
||||
"payment",
|
||||
"payto",
|
||||
"pkcs11",
|
||||
"platform",
|
||||
"pop",
|
||||
"pres",
|
||||
"prospero",
|
||||
"proxy",
|
||||
"pwid",
|
||||
"psyc",
|
||||
"pttp",
|
||||
"qb",
|
||||
"query",
|
||||
"quic-transport",
|
||||
"redis",
|
||||
"rediss",
|
||||
"reload",
|
||||
"res",
|
||||
"resource",
|
||||
"rmi",
|
||||
"rsync",
|
||||
"rtmfp",
|
||||
"rtmp",
|
||||
"rtsp",
|
||||
"rtsps",
|
||||
"rtspu",
|
||||
"sarif",
|
||||
"secondlife",
|
||||
"secret-token",
|
||||
"service",
|
||||
"session",
|
||||
"sftp",
|
||||
"sgn",
|
||||
"shc",
|
||||
"shttp (OBSOLETE)",
|
||||
"sieve",
|
||||
"simpleledger",
|
||||
"simplex",
|
||||
"sip",
|
||||
"sips",
|
||||
"skype",
|
||||
"smb",
|
||||
"smp",
|
||||
"sms",
|
||||
"smtp",
|
||||
"snews",
|
||||
"snmp",
|
||||
"soap.beep",
|
||||
"soap.beeps",
|
||||
"soldat",
|
||||
"spiffe",
|
||||
"spotify",
|
||||
"ssb",
|
||||
"ssh",
|
||||
"starknet",
|
||||
"steam",
|
||||
"stun",
|
||||
"stuns",
|
||||
"submit",
|
||||
"svn",
|
||||
"swh",
|
||||
"swid",
|
||||
"swidpath",
|
||||
"tag",
|
||||
"taler",
|
||||
"teamspeak",
|
||||
"tel",
|
||||
"teliaeid",
|
||||
"telnet",
|
||||
"tftp",
|
||||
"things",
|
||||
"thismessage",
|
||||
"tip",
|
||||
"tn3270",
|
||||
"tool",
|
||||
"turn",
|
||||
"turns",
|
||||
"tv",
|
||||
"udp",
|
||||
"unreal",
|
||||
"upt",
|
||||
"urn",
|
||||
"ut2004",
|
||||
"uuid-in-package",
|
||||
"v-event",
|
||||
"vemmi",
|
||||
"ventrilo",
|
||||
"ves",
|
||||
"videotex",
|
||||
"vnc",
|
||||
"view-source",
|
||||
"vscode",
|
||||
"vscode-insiders",
|
||||
"vsls",
|
||||
"w3",
|
||||
"wais",
|
||||
"web3",
|
||||
"wcr",
|
||||
"webcal",
|
||||
"web+ap",
|
||||
"wifi",
|
||||
"wpid",
|
||||
"ws",
|
||||
"wss",
|
||||
"wtai",
|
||||
"wyciwyg",
|
||||
"xcon",
|
||||
"xcon-userid",
|
||||
"xfire",
|
||||
"xmlrpc.beep",
|
||||
"xmlrpc.beeps",
|
||||
"xmpp",
|
||||
"xri",
|
||||
"ymsgr",
|
||||
"z39.50",
|
||||
"z39.50r",
|
||||
"z39.50s",
|
||||
] # pragma: no cover
|
||||
|
||||
29
src/documents/validators.py
Normal file
29
src/documents/validators.py
Normal file
@ -0,0 +1,29 @@
|
||||
from urllib.parse import urlparse
|
||||
|
||||
from django.core.exceptions import ValidationError
|
||||
from django.utils.translation import gettext_lazy as _
|
||||
|
||||
|
||||
def uri_validator(value) -> None:
|
||||
"""
|
||||
Raises a ValidationError if the given value does not parse as an
|
||||
URI looking thing, which we're defining as a scheme and either network
|
||||
location or path value
|
||||
"""
|
||||
try:
|
||||
parts = urlparse(value)
|
||||
if not parts.scheme:
|
||||
raise ValidationError(
|
||||
_(f"Unable to parse URI {value}, missing scheme"),
|
||||
params={"value": value},
|
||||
)
|
||||
elif not parts.netloc and not parts.path:
|
||||
raise ValidationError(
|
||||
_(f"Unable to parse URI {value}, missing net loction or path"),
|
||||
params={"value": value},
|
||||
)
|
||||
except Exception as e:
|
||||
raise ValidationError(
|
||||
_(f"Unable to parse URI {value}"),
|
||||
params={"value": value},
|
||||
) from e
|
||||
Loading…
x
Reference in New Issue
Block a user