From ef8f40e734c8b5882b4cf80280c269a4728916a9 Mon Sep 17 00:00:00 2001 From: jordi fita mas Date: Mon, 3 Jul 2023 11:31:59 +0200 Subject: [PATCH] Create validation function for SQL domains and for phones MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When i wrote the functions to import contact, i already created a couple of “temporary” functions to validate whether the input given from the Excel files was correct according to the various domains used in the relations, so i can know whether i can import that data. I realized that i could do exactly the same when validating forms: check that the value conforms to the domain, in the exact same way, so i can make sure that the value will be accepted without duplicating the logic, at the expense of a call to the database. In an ideal world, i would use pg_input_is_valid, but this function is only available in PostgreSQL 16 and Debian 12 uses PostgreSQL 15. These functions are in the public schema because initially i wanted to use them to also validate email, which is needed in the login form, but then i recanted and kept the same email validation in Go, because something felt off about using the database for that particular form, but i do not know why. --- deploy/import_contact.sql | 44 +++++---------------------- deploy/input_is_valid.sql | 23 ++++++++++++++ deploy/input_is_valid_phone.sql | 24 +++++++++++++++ pkg/company.go | 4 +-- pkg/contacts.go | 18 ++++++----- pkg/form.go | 20 +++++-------- revert/input_is_valid.sql | 7 +++++ revert/input_is_valid_phone.sql | 7 +++++ sqitch.plan | 4 ++- test/input_is_valid.sql | 53 +++++++++++++++++++++++++++++++++ test/input_is_valid_phone.sql | 30 +++++++++++++++++++ verify/input_is_valid.sql | 7 +++++ verify/input_is_valid_phone.sql | 7 +++++ 13 files changed, 189 insertions(+), 59 deletions(-) create mode 100644 deploy/input_is_valid.sql create mode 100644 deploy/input_is_valid_phone.sql create mode 100644 revert/input_is_valid.sql create mode 100644 revert/input_is_valid_phone.sql create mode 100644 test/input_is_valid.sql create mode 100644 test/input_is_valid_phone.sql create mode 100644 verify/input_is_valid.sql create mode 100644 verify/input_is_valid_phone.sql diff --git a/deploy/import_contact.sql b/deploy/import_contact.sql index a5374a3..4e48da2 100644 --- a/deploy/import_contact.sql +++ b/deploy/import_contact.sql @@ -8,6 +8,8 @@ -- requires: contact_iban -- requires: contact_swift -- requires: contact_tax_details +-- requires: input_is_valid +-- requires: input_is_valid_phone begin; @@ -82,21 +84,6 @@ begin set tags = array_cat(contact.tags, excluded.tags) ; - -- TODO: use pg_input_is_valid with PostgreSQL 16 - create or replace function pg_temp.input_is_valid(input text, typename text) returns bool as - $func$ - begin - begin - execute format('select %L::%s', input, typename); - return true; - exception when others then - return false; - end; - end; - $func$ - language plpgsql - immutable; - insert into contact_tax_details (contact_id, business_name, vatin, address, city, province, postal_code, country_code) select contact_id, imported_contact.name, (country_code || vatin)::vatin, address, city, province, postal_code, country_code from imported_contact @@ -106,7 +93,7 @@ begin and length(city) > 1 and length(province) > 1 and postal_code ~ postal_code_regex - and pg_temp.input_is_valid(country_code || vatin, 'vatin') + and input_is_valid(country_code || vatin, 'vatin') on conflict (contact_id) do update set business_name = excluded.business_name , vatin = excluded.vatin @@ -121,7 +108,7 @@ begin select contact_id, email::email from imported_contact where contact_id is not null - and pg_temp.input_is_valid(email, 'email') + and input_is_valid(email, 'email') on conflict (contact_id) do update set email = excluded.email ; @@ -130,7 +117,7 @@ begin select contact_id, web::uri from imported_contact where contact_id is not null - and pg_temp.input_is_valid(web, 'uri') + and input_is_valid(web, 'uri') and length(web) > 1 on conflict (contact_id) do update set uri = excluded.uri @@ -140,7 +127,7 @@ begin select contact_id, iban::iban from imported_contact where contact_id is not null - and pg_temp.input_is_valid(iban, 'iban') + and input_is_valid(iban, 'iban') on conflict (contact_id) do update set iban = excluded.iban ; @@ -149,31 +136,16 @@ begin select contact_id, bic::bic from imported_contact where contact_id is not null - and pg_temp.input_is_valid(bic, 'bic') + and input_is_valid(bic, 'bic') on conflict (contact_id) do update set bic = excluded.bic ; - -- TODO: use pg_input_is_valid with PostgreSQL 16 - create or replace function pg_temp.phone_is_valid(phone text, country text) returns bool as - $func$ - begin - begin - perform parse_packed_phone_number(phone, country); - return true; - exception when others then - return false; - end; - end; - $func$ - language plpgsql - immutable; - insert into contact_phone (contact_id, phone) select contact_id, parse_packed_phone_number(phone, case when country_code = '' then 'ES' else country_code end) from imported_contact where contact_id is not null - and pg_temp.phone_is_valid(phone, case when country_code = '' then 'ES' else country_code end) + and input_is_valid_phone(phone, case when country_code = '' then 'ES' else country_code end) on conflict (contact_id) do update set phone = excluded.phone ; diff --git a/deploy/input_is_valid.sql b/deploy/input_is_valid.sql new file mode 100644 index 0000000..d0e1066 --- /dev/null +++ b/deploy/input_is_valid.sql @@ -0,0 +1,23 @@ +-- Deploy numerus:input_is_valid to pg +-- requires: schema_numerus +-- requires: roles + +begin; + +set search_path to public; + +create or replace function input_is_valid(input text, domname text) returns boolean as +$$ +begin + begin + execute format('select %L::%s', input, domname); + return true; + exception when others then + return false; + end; +end; +$$ +language plpgsql +stable; + +commit; diff --git a/deploy/input_is_valid_phone.sql b/deploy/input_is_valid_phone.sql new file mode 100644 index 0000000..cdc4a6f --- /dev/null +++ b/deploy/input_is_valid_phone.sql @@ -0,0 +1,24 @@ +-- Deploy numerus:input_is_valid_phone to pg +-- requires: schema_numerus +-- requires: roles +-- requires: extension_pg_libphonenumber + +begin; + +set search_path to public; + +create or replace function input_is_valid_phone(phone text, country text) returns boolean as +$$ +begin + begin + perform parse_packed_phone_number(phone, country); + return true; + exception when others then + return false; + end; +end; +$$ +language plpgsql +stable; + +commit; diff --git a/pkg/company.go b/pkg/company.go index 8c154f6..da9cc6d 100644 --- a/pkg/company.go +++ b/pkg/company.go @@ -275,10 +275,10 @@ func (form *taxDetailsForm) Validate(ctx context.Context, conn *Conn) bool { validator.CheckRequiredInput(form.BusinessName, gettext("Business name can not be empty.", form.locale)) validator.CheckInputMinLength(form.BusinessName, 2, gettext("Business name must have at least two letters.", form.locale)) if validator.CheckRequiredInput(form.VATIN, gettext("VAT number can not be empty.", form.locale)) { - validator.CheckValidVATINInput(form.VATIN, country, gettext("This value is not a valid VAT number.", form.locale)) + validator.CheckValidVATINInput(ctx, conn, form.VATIN, country, gettext("This value is not a valid VAT number.", form.locale)) } if validator.CheckRequiredInput(form.Phone, gettext("Phone can not be empty.", form.locale)) { - validator.CheckValidPhoneInput(form.Phone, country, gettext("This value is not a valid phone number.", form.locale)) + validator.CheckValidPhoneInput(ctx, conn, form.Phone, country, gettext("This value is not a valid phone number.", form.locale)) } if validator.CheckRequiredInput(form.Email, gettext("Email can not be empty.", form.locale)) { validator.CheckValidEmailInput(form.Email, gettext("This value is not a valid email. It should be like name@domain.com.", form.locale)) diff --git a/pkg/contacts.go b/pkg/contacts.go index 41558b8..6a156fd 100644 --- a/pkg/contacts.go +++ b/pkg/contacts.go @@ -380,10 +380,11 @@ func (form *contactForm) Validate(ctx context.Context, conn *Conn) bool { if validator.CheckValidSelectOption(form.Country, gettext("Selected country is not valid.", form.locale)) { country = form.Country.Selected[0] } - validator.CheckRequiredInput(form.BusinessName, gettext("Business name can not be empty.", form.locale)) - validator.CheckInputMinLength(form.BusinessName, 2, gettext("Business name must have at least two letters.", form.locale)) + if validator.CheckRequiredInput(form.BusinessName, gettext("Business name can not be empty.", form.locale)) { + validator.CheckInputMinLength(form.BusinessName, 2, gettext("Business name must have at least two letters.", form.locale)) + } if validator.CheckRequiredInput(form.VATIN, gettext("VAT number can not be empty.", form.locale)) { - validator.CheckValidVATINInput(form.VATIN, country, gettext("This value is not a valid VAT number.", form.locale)) + validator.CheckValidVATINInput(ctx, conn, form.VATIN, country, gettext("This value is not a valid VAT number.", form.locale)) } validator.CheckRequiredInput(form.Address, gettext("Address can not be empty.", form.locale)) validator.CheckRequiredInput(form.City, gettext("City can not be empty.", form.locale)) @@ -394,11 +395,12 @@ func (form *contactForm) Validate(ctx context.Context, conn *Conn) bool { } } - validator.CheckRequiredInput(form.Name, gettext("Name can not be empty.", form.locale)) - validator.CheckInputMinLength(form.Name, 2, gettext("Name must have at least two letters.", form.locale)) + if validator.CheckRequiredInput(form.Name, gettext("Name can not be empty.", form.locale)) { + validator.CheckInputMinLength(form.Name, 2, gettext("Name must have at least two letters.", form.locale)) + } if form.Phone.Val != "" { - validator.CheckValidPhoneInput(form.Phone, country, gettext("This value is not a valid phone number.", form.locale)) + validator.CheckValidPhoneInput(ctx, conn, form.Phone, country, gettext("This value is not a valid phone number.", form.locale)) } if form.Email.Val != "" { validator.CheckValidEmailInput(form.Email, gettext("This value is not a valid email. It should be like name@domain.com.", form.locale)) @@ -407,10 +409,10 @@ func (form *contactForm) Validate(ctx context.Context, conn *Conn) bool { validator.CheckValidURL(form.Web, gettext("This value is not a valid web address. It should be like https://domain.com/.", form.locale)) } if form.IBAN.Val != "" { - validator.CheckValidIBANInput(form.IBAN, gettext("This values is not a valid IBAN.", form.locale)) + validator.CheckValidIBANInput(ctx, conn, form.IBAN, gettext("This values is not a valid IBAN.", form.locale)) } if form.BIC.Val != "" { - validator.CheckValidBICInput(form.IBAN, gettext("This values is not a valid BIC.", form.locale)) + validator.CheckValidBICInput(ctx, conn, form.BIC, gettext("This values is not a valid BIC.", form.locale)) } return validator.AllOK() diff --git a/pkg/form.go b/pkg/form.go index 0d2610a..e7d3698 100644 --- a/pkg/form.go +++ b/pkg/form.go @@ -441,24 +441,20 @@ func (v *FormValidator) CheckValidEmailInput(field *InputField, message string) return v.checkInput(field, err == nil, message) } -func (v *FormValidator) CheckValidVATINInput(field *InputField, country string, message string) bool { - // TODO: actual VATIN validation - return v.checkInput(field, true, message) +func (v *FormValidator) CheckValidVATINInput(ctx context.Context, conn *Conn, field *InputField, country string, message string) bool { + return v.checkInput(field, conn.MustGetBool(ctx, "select input_is_valid($1 || $2, 'vatin')", country, field.Val), message) } -func (v *FormValidator) CheckValidPhoneInput(field *InputField, country string, message string) bool { - // TODO: actual phone validation - return v.checkInput(field, true, message) +func (v *FormValidator) CheckValidPhoneInput(ctx context.Context, conn *Conn, field *InputField, country string, message string) bool { + return v.checkInput(field, conn.MustGetBool(ctx, "select input_is_valid_phone($1, $2)", field.Val, country), message) } -func (v *FormValidator) CheckValidIBANInput(field *InputField, message string) bool { - // TODO: actual IBAN validation - return v.checkInput(field, true, message) +func (v *FormValidator) CheckValidIBANInput(ctx context.Context, conn *Conn, field *InputField, message string) bool { + return v.checkInput(field, conn.MustGetBool(ctx, "select input_is_valid($1, 'iban')", field.Val), message) } -func (v *FormValidator) CheckValidBICInput(field *InputField, message string) bool { - // TODO: actual BIC validation - return v.checkInput(field, true, message) +func (v *FormValidator) CheckValidBICInput(ctx context.Context, conn *Conn, field *InputField, message string) bool { + return v.checkInput(field, conn.MustGetBool(ctx, "select input_is_valid($1, 'bic')", field.Val), message) } func (v *FormValidator) CheckPasswordConfirmation(password *InputField, confirm *InputField, message string) bool { diff --git a/revert/input_is_valid.sql b/revert/input_is_valid.sql new file mode 100644 index 0000000..d08e811 --- /dev/null +++ b/revert/input_is_valid.sql @@ -0,0 +1,7 @@ +-- Revert numerus:input_is_valid from pg + +begin; + +drop function if exists public.input_is_valid(text, text); + +commit; diff --git a/revert/input_is_valid_phone.sql b/revert/input_is_valid_phone.sql new file mode 100644 index 0000000..6aaedbc --- /dev/null +++ b/revert/input_is_valid_phone.sql @@ -0,0 +1,7 @@ +-- Revert numerus:input_is_valid_phone from pg + +begin; + +drop function if exists public.input_is_valid_phone(text, text); + +commit; diff --git a/sqitch.plan b/sqitch.plan index aeb96cc..22e92c5 100644 --- a/sqitch.plan +++ b/sqitch.plan @@ -111,4 +111,6 @@ bic [schema_numerus] 2023-07-01T22:46:30Z jordi fita mas # A contact_swift [schema_numerus roles contact bic] 2023-07-01T23:03:13Z jordi fita mas # Add relation for contact’s SWIFT-BIC add_contact [add_contact@v0 tax_details contact_web contact_email contact_phone contact_iban contact_swift] 2023-06-29T11:10:15Z jordi fita mas # Change add contact to accept a tax_detail parameter and use the new relations for web, email, phone, iban, and swift edit_contact [edit_contact@v0 tax_details contact_web contact_email contact_phone contact_iban contact_swift] 2023-06-29T11:50:41Z jordi fita mas # Change edit_contact to require tax_details parameter and to use new relations for web, email, phone, iban, and swift -import_contact [schema_numerus roles contact contact_web contact_phone contact_email contact_iban contact_swift contact_tax_details] 2023-07-02T18:50:22Z jordi fita mas # Add functions to massively import customer data +input_is_valid [schema_public roles] 2023-07-03T08:42:46Z jordi fita mas # add function to check if input is valid for a domain +input_is_valid_phone [schema_public roles extension_pg_libphonenumber] 2023-07-03T08:59:36Z jordi fita mas # add function to validate phone number inputs +import_contact [schema_numerus roles contact contact_web contact_phone contact_email contact_iban contact_swift contact_tax_details input_is_valid input_is_valid_phone] 2023-07-02T18:50:22Z jordi fita mas # Add functions to massively import customer data diff --git a/test/input_is_valid.sql b/test/input_is_valid.sql new file mode 100644 index 0000000..5570980 --- /dev/null +++ b/test/input_is_valid.sql @@ -0,0 +1,53 @@ +-- Test input_is_valid +set client_min_messages to warning; +create extension if not exists pgtap; +reset client_min_messages; + +begin; + +select plan(36); + +set search_path to numerus, public; + +select has_function('public', 'input_is_valid', array ['text', 'text']); +select function_lang_is('public', 'input_is_valid', array ['text', 'text'], 'plpgsql'); +select function_returns('public', 'input_is_valid', array ['text', 'text'], 'boolean'); +select isnt_definer('public', 'input_is_valid', array ['text', 'text']); +select volatility_is('public', 'input_is_valid', array ['text', 'text'], 'stable'); +select function_privs_are('public', 'input_is_valid', array ['text', 'text'], 'guest', array ['EXECUTE']); +select function_privs_are('public', 'input_is_valid', array ['text', 'text'], 'invoicer', array ['EXECUTE']); +select function_privs_are('public', 'input_is_valid', array ['text', 'text'], 'admin', array ['EXECUTE']); +select function_privs_are('public', 'input_is_valid', array ['text', 'text'], 'authenticator', array ['EXECUTE']); + +select is( input_is_valid('123', 'integer'), true ); +select is( input_is_valid('abc', 'integer'), false ); +select is( input_is_valid('abc', 'email'), false ); +select is( input_is_valid('ESabc', 'vatin'), false ); +select is( input_is_valid('abc', 'iban'), false ); +select is( input_is_valid('abc', 'bic'), false ); +select is( input_is_valid('abc', 'text'), true ); +select is( input_is_valid('ES44444444A', 'vatin'), true ); +select is( input_is_valid('ES44444444A', 'text'), true ); +select is( input_is_valid('ES44444444A', 'email'), false ); +select is( input_is_valid('ES44444444A', 'iban'), false ); +select is( input_is_valid('ES44444444A', 'bic'), false ); +select is( input_is_valid('NL04RABO9373475770', 'iban'), true ); +select is( input_is_valid('NL04RABO9373475770', 'text'), true ); +select is( input_is_valid('ESNL04RABO9373475770', 'vatin'), false ); +select is( input_is_valid('NL04RABO9373475770', 'email'), false ); +select is( input_is_valid('NL04RABO9373475770', 'bic'), false ); +select is( input_is_valid('ARBNNL22', 'bic'), true ); +select is( input_is_valid('ARBNNL22', 'text'), true ); +select is( input_is_valid('ESARBNNL22', 'vatin'), false ); +select is( input_is_valid('ARBNNL22', 'email'), false ); +select is( input_is_valid('ARBNNL22', 'iban'), false ); +select is( input_is_valid('2023-05-12', 'text'), true ); +select is( input_is_valid('2023-05-12', 'date'), true ); +select is( input_is_valid('2023-05-12', 'integer'), false ); +select is( input_is_valid('', 'text'), true ); +select is( input_is_valid('', 'inexistent'), false ); + +select * +from finish(); + +rollback; diff --git a/test/input_is_valid_phone.sql b/test/input_is_valid_phone.sql new file mode 100644 index 0000000..e138634 --- /dev/null +++ b/test/input_is_valid_phone.sql @@ -0,0 +1,30 @@ +-- Test input_is_valid_phone +set client_min_messages to warning; +create extension if not exists pgtap; +reset client_min_messages; + +begin; + +select plan(12); + +set search_path to numerus, public; + +select has_function('public', 'input_is_valid_phone', array ['text', 'text']); +select function_lang_is('public', 'input_is_valid_phone', array ['text', 'text'], 'plpgsql'); +select function_returns('public', 'input_is_valid_phone', array ['text', 'text'], 'boolean'); +select isnt_definer('public', 'input_is_valid_phone', array ['text', 'text']); +select volatility_is('public', 'input_is_valid_phone', array ['text', 'text'], 'stable'); +select function_privs_are('public', 'input_is_valid_phone', array ['text', 'text'], 'guest', array ['EXECUTE']); +select function_privs_are('public', 'input_is_valid_phone', array ['text', 'text'], 'invoicer', array ['EXECUTE']); +select function_privs_are('public', 'input_is_valid_phone', array ['text', 'text'], 'admin', array ['EXECUTE']); +select function_privs_are('public', 'input_is_valid_phone', array ['text', 'text'], 'authenticator', array ['EXECUTE']); + + +select is( input_is_valid_phone('555-555-5555', 'US'), true ); +select is( input_is_valid_phone('555-555-5555555555', 'US'), false ); +select is( input_is_valid_phone('555-555-55555555555', 'US'), false ); + +select * +from finish(); + +rollback; diff --git a/verify/input_is_valid.sql b/verify/input_is_valid.sql new file mode 100644 index 0000000..1e05dba --- /dev/null +++ b/verify/input_is_valid.sql @@ -0,0 +1,7 @@ +-- Verify numerus:input_is_valid on pg + +begin; + +select has_function_privilege('public.input_is_valid(text, text)', 'execute'); + +rollback; diff --git a/verify/input_is_valid_phone.sql b/verify/input_is_valid_phone.sql new file mode 100644 index 0000000..f6c7fc1 --- /dev/null +++ b/verify/input_is_valid_phone.sql @@ -0,0 +1,7 @@ +-- Verify numerus:input_is_valid_phone on pg + +begin; + +select has_function_privilege('public.input_is_valid_phone(text, text)', 'execute'); + +rollback;