From c0f02637a9b75261b54b8dc9cebbe828953bdf29 Mon Sep 17 00:00:00 2001 From: Michael Mayer Date: Sat, 11 Oct 2025 11:24:10 +0200 Subject: [PATCH] Forms: Improve validation to support "+" signs in email addresses #5254 Signed-off-by: Michael Mayer --- frontend/src/common/form.js | 80 +++++++++-- frontend/tests/vitest/common/form.test.js | 156 +++++++++++++++++++++- pkg/clean/auth.go | 2 + pkg/clean/auth_test.go | 32 ++++- 4 files changed, 250 insertions(+), 20 deletions(-) diff --git a/frontend/src/common/form.js b/frontend/src/common/form.js index c5df3b0ad..cd222b29e 100644 --- a/frontend/src/common/form.js +++ b/frontend/src/common/form.js @@ -25,17 +25,21 @@ Additional information can be found in our Developer Guide: import { $gettext } from "common/gettext"; +// FormPropertyType enumerates supported types for form properties. export const FormPropertyType = Object.freeze({ String: "string", Number: "number", Object: "object", }); +// Form encapsulates a simple key/value form definition with helpers for type-checked assignments. export class Form { + // constructor optionally accepts an initial definition. constructor(definition) { this.definition = definition; } + // setValues assigns values in bulk while respecting the schema. setValues(values) { const def = this.getDefinition(); @@ -48,6 +52,7 @@ export class Form { return this; } + // getValues returns a map of current values. getValues() { const result = {}; const def = this.getDefinition(); @@ -59,6 +64,7 @@ export class Form { return result; } + // setValue updates a single value ensuring the type matches the definition. setValue(name, value) { const def = this.getDefinition(); @@ -73,6 +79,7 @@ export class Form { return this; } + // getValue fetches a single property value. getValue(name) { const def = this.getDefinition(); @@ -83,14 +90,17 @@ export class Form { } } + // setDefinition replaces the current form schema. setDefinition(definition) { this.definition = definition; } + // getDefinition returns the current schema or an empty object. getDefinition() { return this.definition ? this.definition : {}; } + // getOptions resolves the options array for select-style fields. getOptions(fieldName) { if ( this.definition && @@ -104,7 +114,9 @@ export class Form { } } +// rules centralizes reusable validation helpers and Vuetify rule factories used across the UI. export class rules { + // maxLen ensures that a string does not exceed the provided maximum length. static maxLen(v, max) { if (!v || typeof v !== "string" || max <= 0) { return true; @@ -113,6 +125,7 @@ export class rules { return v.length <= max; } + // minLen ensures that a string meets the minimum length. static minLen(v, min) { if (!v || typeof v !== "string" || min <= 0) { return true; @@ -121,6 +134,7 @@ export class rules { return v.length >= min; } + // isLat validates latitude values in decimal degrees. static isLat(v) { if (typeof v !== "string" || v === "") { return true; @@ -132,9 +146,10 @@ export class rules { return false; } - return -91 < lat < 91; + return lat >= -90 && lat <= 90; } + // isLng validates longitude values in decimal degrees. static isLng(v) { if (typeof v !== "string" || v === "") { return true; @@ -146,9 +161,10 @@ export class rules { return false; } - return -181 < lng < 181; + return lng >= -180 && lng <= 180; } + // isNumber validates that a value is a parsable number or empty. static isNumber(v) { if (typeof v !== "string" || v === "") { return true; @@ -157,6 +173,7 @@ export class rules { return !isNaN(Number(v)); } + // isNumberRange validates numeric strings within optional inclusive bounds. static isNumberRange(v, min, max) { if (typeof v !== "string" || !v || v === "-1") { return true; @@ -179,10 +196,12 @@ export class rules { return true; } + // isTime validates HH:MM:SS style times with any non-digit separator. static isTime(v) { return /^(2[0-3]|[0-1][0-9])\D[0-5][0-9]\D[0-5][0-9]$/.test(v); // 23:59:59 } + // isEmail verifies that strings match the backend email sanitizer rules while staying lenient for empty inputs. static isEmail(v) { if (typeof v !== "string" || v === "") { return true; @@ -190,9 +209,12 @@ export class rules { return false; } - return /^\w+([.-]?\w+)*@\w+([.-]?\w+)*(\.\w{2,32})+$/.test(v); + return /^[A-Za-z0-9.!#$%&'*+/=?^_`{|}~-]+@[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?(?:\.[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?)*$/.test( + v + ); } + // isUrl validates strings by length and URL parsing. static isUrl(v) { if (typeof v !== "string" || v === "") { return true; @@ -208,6 +230,7 @@ export class rules { return true; } + // lat returns Vuetify rule callbacks for latitude validation. static lat(required) { if (required) { return [(v) => !!v || $gettext("This field is required"), (v) => this.isLat(v) || $gettext("Invalid")]; @@ -216,6 +239,7 @@ export class rules { } } + // lng returns Vuetify rule callbacks for longitude validation. static lng(required) { if (required) { return [(v) => !!v || $gettext("This field is required"), (v) => this.isLng(v) || $gettext("Invalid")]; @@ -224,14 +248,16 @@ export class rules { } } + // time returns Vuetify rule callbacks enforcing HH:MM:SS format. static time(required) { if (required) { return [(v) => !!v || $gettext("This field is required"), (v) => this.isTime(v) || $gettext("Invalid time")]; } else { - return [(v) => this.isTime(v) || $gettext("Invalid time")]; + return [(v) => !v || this.isTime(v) || $gettext("Invalid time")]; } } + // email returns Vuetify rule callbacks for email validation. static email(required) { if (required) { return [ @@ -243,6 +269,7 @@ export class rules { } } + // url returns Vuetify rule callbacks for URL validation. static url(required) { if (required) { return [(v) => !!v || $gettext("This field is required"), (v) => !v || this.isUrl(v) || $gettext("Invalid URL")]; @@ -251,6 +278,7 @@ export class rules { } } + // text returns string length validators with optional localization label. static text(required, min, max, s) { if (!s) { s = $gettext("Text"); @@ -270,6 +298,7 @@ export class rules { } } + // number returns numeric validators with inclusive min/max checks. static number(required, min, max) { if (!min) { min = 0; @@ -279,20 +308,42 @@ export class rules { max = 2147483647; } + const minValidator = (v) => { + if (v === "" || v === undefined || v === null) { + return true; + } + + const value = Number(v); + + if (isNaN(value)) { + return $gettext("Invalid"); + } + + return value >= min || $gettext("Invalid"); + }; + + const maxValidator = (v) => { + if (v === "" || v === undefined || v === null) { + return true; + } + + const value = Number(v); + + if (isNaN(value)) { + return $gettext("Invalid"); + } + + return value <= max || $gettext("Invalid"); + }; + if (required) { - return [ - (v) => !!v || $gettext("This field is required"), - (v) => (this.isNumber(v) && v >= min) || $gettext("Invalid"), - (v) => (this.isNumber(v) && v <= max) || $gettext("Invalid"), - ]; + return [(v) => !!v || $gettext("This field is required"), minValidator, maxValidator]; } else { - return [ - (v) => (this.isNumber(v) && v >= min) || $gettext("Invalid"), - (v) => (this.isNumber(v) && v <= max) || $gettext("Invalid"), - ]; + return [minValidator, maxValidator]; } } + // country validates ISO-style country codes via length checks. static country(required) { if (required) { return [ @@ -308,6 +359,7 @@ export class rules { } } + // day validates day-of-month values between 1 and 31. static day(required) { if (required) { return [ @@ -319,6 +371,7 @@ export class rules { } } + // month validates month values between 1 and 12. static month(required) { if (required) { return [ @@ -330,6 +383,7 @@ export class rules { } } + // year validates year values using optional bounds (defaults 1800..current year). static year(required, min, max) { if (!min) { min = 1800; diff --git a/frontend/tests/vitest/common/form.test.js b/frontend/tests/vitest/common/form.test.js index 9cd27562f..4eee50b1e 100644 --- a/frontend/tests/vitest/common/form.test.js +++ b/frontend/tests/vitest/common/form.test.js @@ -1,6 +1,6 @@ import { describe, it, expect } from "vitest"; import "../fixtures"; -import { Form, FormPropertyType } from "common/form"; +import { Form, FormPropertyType, rules } from "common/form"; describe("common/form", () => { it("setting and getting definition", () => { @@ -127,4 +127,158 @@ describe("common/form", () => { expect(result[0].option).toBe(""); expect(result[0].label).toBe(""); }); + + describe("rules.isEmail", () => { + it("accepts representative valid addresses", () => { + const valid = [ + "user@example.com", + "user+news@example.com", + "user.name@sub-domain.example", + "user_name@example.co.uk", + "user@localhost", + ]; + + for (const addr of valid) { + expect(rules.isEmail(addr)).toBe(true); + } + }); + + it("rejects malformed addresses", () => { + const invalid = [ + "userexample.com", + "user@@example.com", + "user@", + "@example.com", + "user example@example.com", + "user@-example.com", + "user@example..com", + ]; + + for (const addr of invalid) { + expect(rules.isEmail(addr)).toBe(false); + } + }); + + it("ignores empty values", () => { + expect(rules.isEmail("")).toBe(true); + }); + }); + + describe("rules helpers", () => { + it("checks maxLen and minLen boundaries", () => { + expect(rules.maxLen("abc", 5)).toBe(true); + expect(rules.maxLen("abcdef", 5)).toBe(false); + expect(rules.minLen("abc", 2)).toBe(true); + expect(rules.minLen("a", 2)).toBe(false); + }); + + it("validates latitude and longitude ranges", () => { + expect(rules.isLat("45")).toBe(true); + expect(rules.isLat("91")).toBe(false); + expect(rules.isLng("-120")).toBe(true); + expect(rules.isLng("190")).toBe(false); + const [latRequired, latRange] = rules.lat(true); + expect(latRequired("")).toBe("This field is required"); + expect(latRange("91")).toBe("Invalid"); + expect(latRange("0")).toBe(true); + const [latOptional] = rules.lat(false); + expect(latOptional("")).toBe(true); + const [lngRequired, lngRange] = rules.lng(true); + expect(lngRequired("")).toBe("This field is required"); + expect(lngRange("200")).toBe("Invalid"); + expect(lngRange("-45")).toBe(true); + }); + + it("validates numeric strings and ranges", () => { + expect(rules.isNumber("123")).toBe(true); + expect(rules.isNumber("abc")).toBe(false); + expect(rules.isNumberRange("5", 1, 10)).toBe(true); + expect(rules.isNumberRange("0", 1, 10)).toBe(false); + expect(rules.isNumberRange("-1", 1, 10)).toBe(true); + const requiredNumber = rules.number(true, 1, 10); + expect(requiredNumber[0]("")).toBe("This field is required"); + expect(requiredNumber[1]("0")).toBe("Invalid"); + expect(requiredNumber[2]("11")).toBe("Invalid"); + expect(requiredNumber[1]("5")).toBe(true); + const optionalNumber = rules.number(false, 1, 10); + expect(optionalNumber[0]("")).toBe(true); + }); + + it("validates time values", () => { + expect(rules.isTime("23:59:59")).toBe(true); + expect(rules.isTime("24:00:00")).toBe(false); + const required = rules.time(true); + expect(required[0]("")).toBe("This field is required"); + expect(required[1]("23:59:59")).toBe(true); + expect(required[1]("25:00:00")).toBe("Invalid time"); + const optional = rules.time(false); + expect(optional[0]("")).toBe(true); + }); + + it("validates email and url rule wrappers", () => { + const requiredEmail = rules.email(true); + expect(requiredEmail[0]("")).toBe("This field is required"); + expect(requiredEmail[1]("bad")).toBe("Invalid address"); + expect(requiredEmail[1]("user@example.com")).toBe(true); + const optionalEmail = rules.email(false); + expect(optionalEmail[0]("")).toBe(true); + + expect(rules.isUrl("https://example.com")).toBe(true); + expect(rules.isUrl("ftp://example.com")).toBe(true); + const requiredUrl = rules.url(true); + expect(requiredUrl[0]("")).toBe("This field is required"); + expect(requiredUrl[1]("notaurl")).toBe("Invalid URL"); + expect(requiredUrl[1]("https://example.com")).toBe(true); + const optionalUrl = rules.url(false); + expect(optionalUrl[0]("")).toBe(true); + }); + + it("validates text length with labels", () => { + const requiredText = rules.text(true, 2, 4, "Name"); + expect(requiredText[0]("")).toBe("This field is required"); + expect(requiredText[1]("a")).toBe("%{s} is too short"); + expect(requiredText[2]("abcde")).toBe("%{s} is too long"); + expect(requiredText[1]("abc")).toBe(true); + const optionalText = rules.text(false, 2, 4, "Name"); + expect(optionalText[0]("a")).toBe("%{s} is too short"); + expect(optionalText[1]("abcde")).toBe("%{s} is too long"); + expect(optionalText[0]("abc")).toBe(true); + }); + + it("validates country, day, month, and year ranges", () => { + const requiredCountry = rules.country(true); + expect(requiredCountry[0]("")).toBe("This field is required"); + expect(requiredCountry[1]("D")).toBe("Invalid country"); + expect(requiredCountry[2]("USA")).toBe("Invalid country"); + expect(requiredCountry[1]("DE")).toBe(true); + const optionalCountry = rules.country(false); + expect(optionalCountry[0]("D")).toBe("Invalid country"); + expect(optionalCountry[1]("USA")).toBe("Invalid country"); + expect(optionalCountry[0]("DE")).toBe(true); + + const requiredDay = rules.day(true); + expect(requiredDay[0]("")).toBe("This field is required"); + expect(requiredDay[1]("0")).toBe("Invalid"); + expect(requiredDay[1]("32")).toBe("Invalid"); + expect(requiredDay[1]("15")).toBe(true); + const optionalDay = rules.day(false); + expect(optionalDay[0]("")).toBe(true); + + const requiredMonth = rules.month(true); + expect(requiredMonth[0]("")).toBe("This field is required"); + expect(requiredMonth[1]("0")).toBe("Invalid"); + expect(requiredMonth[1]("13")).toBe("Invalid"); + expect(requiredMonth[1]("6")).toBe(true); + const optionalMonth = rules.month(false); + expect(optionalMonth[0]("")).toBe(true); + + const requiredYear = rules.year(true, 1990, 2025); + expect(requiredYear[0]("")).toBe("This field is required"); + expect(requiredYear[1]("1989")).toBe("Invalid"); + expect(requiredYear[1]("2026")).toBe("Invalid"); + expect(requiredYear[1]("2000")).toBe(true); + const optionalYear = rules.year(false, 1990, 2025); + expect(optionalYear[0]("")).toBe(true); + }); + }); }); diff --git a/pkg/clean/auth.go b/pkg/clean/auth.go index f719274e6..c1a070af8 100644 --- a/pkg/clean/auth.go +++ b/pkg/clean/auth.go @@ -101,6 +101,8 @@ func Username(s string) string { } // Email returns the sanitized email with trimmed whitespace and in lowercase. +// It accepts common mailbox patterns such as plus addressing and single-label domains +// while rejecting inputs that do not match the backend validation regex. func Email(s string) string { // Empty or too long? if s == "" || reject(s, txt.ClipEmail) { diff --git a/pkg/clean/auth_test.go b/pkg/clean/auth_test.go index b45bafc49..ca48f065f 100644 --- a/pkg/clean/auth_test.go +++ b/pkg/clean/auth_test.go @@ -100,14 +100,34 @@ func TestUsername(t *testing.T) { } func TestEmail(t *testing.T) { - t.Run("Valid", func(t *testing.T) { - assert.Equal(t, "hello@photoprism.app", Email("hello@photoprism.app")) - }) - t.Run("Whitespace", func(t *testing.T) { - assert.Equal(t, "hello@photoprism.app", Email(" hello@photoprism.app ")) + t.Run("ValidExamples", func(t *testing.T) { + valid := []string{ + "user@example.com", + "user+news@example.com", + "user.name@sub-domain.example", + "user_name@example.co.uk", + "user@localhost", + " User@Example.COM ", + } + + for _, addr := range valid { + assert.Equal(t, strings.ToLower(strings.TrimSpace(addr)), Email(addr), addr) + } }) t.Run("Invalid", func(t *testing.T) { - assert.Equal(t, "", Email(" hello-photoprism ")) + invalid := []string{ + "userexample.com", + "user@@example.com", + "user@", + "@example.com", + "user example@example.com", + "user@-example.com", + "user@example..com", + } + + for _, addr := range invalid { + assert.Equal(t, "", Email(addr), addr) + } }) t.Run("Empty", func(t *testing.T) { assert.Equal(t, "", Email(""))