Refine input validation

- Remove usage of JS core checkValidity() in favour of custom backend compliant validation
- Rewrite and refactor with removal of form tag in favour of section
- Scope styles
- Remove many uses of $nextTick
- Refine disabled state logic
- Translate account property constants

Signed-off-by: Christopher Ng <chrng8@gmail.com>

8 files changed, 157 insertions, 82 deletions

diff --git a/apps/settings/src/components/PersonalInfo/DisplayNameSection/DisplayName.vue b/apps/settings/src/components/PersonalInfo/DisplayNameSection/DisplayName.vue
index 3b15f1ea335..1ff7014ed73 100644
--- a/apps/settings/src/components/PersonalInfo/DisplayNameSection/DisplayName.vue
+++ b/apps/settings/src/components/PersonalInfo/DisplayNameSection/DisplayName.vue
@@ -48,6 +48,7 @@ import { showError } from '@nextcloud/dialogs'
 import debounce from 'debounce'
 
 import { savePrimaryDisplayName } from '../../../service/PersonalInfo/DisplayNameService'
+import { validateDisplayName } from '../../../utils/validate'
 
 // TODO Global avatar updating on events (e.g. updating the displayname) is currently being handled by global js, investigate using https://github.com/nextcloud/nextcloud-event-bus for global avatar updating
 
@@ -81,7 +82,7 @@ export default {
 		},
 
 		debounceDisplayNameChange: debounce(async function(displayName) {
-			if (this.$refs.displayName?.checkValidity() && this.isValid(displayName)) {
+			if (validateDisplayName(displayName)) {
 				await this.updatePrimaryDisplayName(displayName)
 			}
 		}, 500),
@@ -115,10 +116,6 @@ export default {
 			}
 		},
 
-		isValid(displayName) {
-			return displayName !== ''
-		},
-
 		onScopeChange(scope) {
 			this.$emit('update:scope', scope)
 		},
@@ -131,8 +128,18 @@ export default {
 		display: grid;
 		align-items: center;
 
-		input[type=text] {
+		input {
 			grid-area: 1 / 1;
+			height: 34px;
+			width: 100%;
+			margin: 3px 3px 3px 0;
+			padding: 7px 6px;
+			cursor: text;
+			font-family: var(--font-face);
+			border: 1px solid var(--color-border-dark);
+			border-radius: var(--border-radius);
+			background-color: var(--color-main-background);
+			color: var(--color-main-text);
 		}
 
 		.displayname__actions-container {
diff --git a/apps/settings/src/components/PersonalInfo/DisplayNameSection/DisplayNameSection.vue b/apps/settings/src/components/PersonalInfo/DisplayNameSection/DisplayNameSection.vue
index 100e7fad876..05b4836b615 100644
--- a/apps/settings/src/components/PersonalInfo/DisplayNameSection/DisplayNameSection.vue
+++ b/apps/settings/src/components/PersonalInfo/DisplayNameSection/DisplayNameSection.vue
@@ -20,29 +20,25 @@
 -->
 
 <template>
-	<form
-		ref="form"
-		class="section"
-		@submit.stop.prevent="() => {}">
+	<section>
 		<HeaderBar
 			:account-property="accountProperty"
 			label-for="displayname"
 			:is-editable="displayNameChangeSupported"
-			:is-valid-form="isValidForm"
+			:is-valid-section="isValidSection"
 			:handle-scope-change="savePrimaryDisplayNameScope"
 			:scope.sync="primaryDisplayName.scope" />
 
 		<template v-if="displayNameChangeSupported">
 			<DisplayName
-				:scope.sync="primaryDisplayName.scope"
 				:display-name.sync="primaryDisplayName.value"
-				@update:display-name="onUpdateDisplayName" />
+				:scope.sync="primaryDisplayName.scope" />
 		</template>
 
 		<span v-else>
 			{{ primaryDisplayName.value || t('settings', 'No full name set') }}
 		</span>
-	</form>
+	</section>
 </template>
 
 <script>
@@ -53,6 +49,7 @@ import HeaderBar from '../shared/HeaderBar'
 
 import { ACCOUNT_PROPERTY_READABLE_ENUM } from '../../../constants/AccountPropertyConstants'
 import { savePrimaryDisplayNameScope } from '../../../service/PersonalInfo/DisplayNameService'
+import { validateDisplayName } from '../../../utils/validate'
 
 const { displayNames: { primaryDisplayName } } = loadState('settings', 'personalInfoParameters', {})
 const { displayNameChangeSupported } = loadState('settings', 'accountParameters', {})
@@ -69,31 +66,24 @@ export default {
 		return {
 			accountProperty: ACCOUNT_PROPERTY_READABLE_ENUM.DISPLAYNAME,
 			displayNameChangeSupported,
-			isValidForm: true,
 			primaryDisplayName,
 			savePrimaryDisplayNameScope,
 		}
 	},
 
-	mounted() {
-		this.$nextTick(() => this.updateFormValidity())
-	},
-
-	methods: {
-		onUpdateDisplayName() {
-			this.$nextTick(() => this.updateFormValidity())
-		},
-
-		updateFormValidity() {
-			this.isValidForm = this.$refs.form?.checkValidity()
+	computed: {
+		isValidSection() {
+			return validateDisplayName(this.primaryDisplayName.value)
 		},
 	},
 }
 </script>
 
 <style lang="scss" scoped>
-	form::v-deep button {
-		&:disabled {
+	section {
+		padding: 10px 10px;
+
+		&::v-deep button:disabled {
 			cursor: default;
 		}
 	}
diff --git a/apps/settings/src/components/PersonalInfo/EmailSection/Email.vue b/apps/settings/src/components/PersonalInfo/EmailSection/Email.vue
index 6b839ccdb55..036c35425a2 100644
--- a/apps/settings/src/components/PersonalInfo/EmailSection/Email.vue
+++ b/apps/settings/src/components/PersonalInfo/EmailSection/Email.vue
@@ -59,6 +59,7 @@
 					<ActionButton
 						:aria-label="deleteEmailLabel"
 						:close-after-click="true"
+						:disabled="deleteDisabled"
 						icon="icon-delete"
 						@click.stop.prevent="deleteEmail">
 						{{ deleteEmailLabel }}
@@ -83,6 +84,7 @@ import FederationControl from '../shared/FederationControl'
 
 import { ACCOUNT_PROPERTY_READABLE_ENUM } from '../../../constants/AccountPropertyConstants'
 import { savePrimaryEmail, saveAdditionalEmail, saveAdditionalEmailScope, updateAdditionalEmail, removeAdditionalEmail } from '../../../service/PersonalInfo/EmailService'
+import { validateEmail } from '../../../utils/validate'
 
 export default {
 	name: 'Email',
@@ -126,9 +128,13 @@ export default {
 	computed: {
 		deleteDisabled() {
 			if (this.primary) {
-				return this.email === ''
+				// Disable for empty primary email as there is nothing to delete
+				// OR when initialEmail (reflects server state) and email (current input) are not the same
+				return this.email === '' || this.initialEmail !== this.email
+			} else if (this.initialEmail !== '') {
+				return this.initialEmail !== this.email
 			}
-			return this.email !== '' && !this.isValid(this.email)
+			return false
 		},
 
 		deleteEmailLabel() {
@@ -159,6 +165,7 @@ export default {
 
 	mounted() {
 		if (!this.primary && this.initialEmail === '') {
+			// $nextTick is needed here, otherwise it may not always work https://stackoverflow.com/questions/51922767/autofocus-input-on-mount-vue-ios/63485725#63485725
 			this.$nextTick(() => this.$refs.email?.focus())
 		}
 	},
@@ -170,7 +177,7 @@ export default {
 		},
 
 		debounceEmailChange: debounce(async function(email) {
-			if (this.$refs.email?.checkValidity() || email === '') {
+			if (validateEmail(email) || email === '') {
 				if (this.primary) {
 					await this.updatePrimaryEmail(email)
 				} else {
@@ -282,10 +289,6 @@ export default {
 			}
 		},
 
-		isValid(email) {
-			return /^\S+$/.test(email)
-		},
-
 		onScopeChange(scope) {
 			this.$emit('update:scope', scope)
 		},
@@ -298,8 +301,18 @@ export default {
 		display: grid;
 		align-items: center;
 
-		input[type=email] {
+		input {
 			grid-area: 1 / 1;
+			height: 34px;
+			width: 100%;
+			margin: 3px 3px 3px 0;
+			padding: 7px 6px;
+			cursor: text;
+			font-family: var(--font-face);
+			border: 1px solid var(--color-border-dark);
+			border-radius: var(--border-radius);
+			background-color: var(--color-main-background);
+			color: var(--color-main-text);
 		}
 
 		.email__actions-container {
diff --git a/apps/settings/src/components/PersonalInfo/EmailSection/EmailSection.vue b/apps/settings/src/components/PersonalInfo/EmailSection/EmailSection.vue
index 7b4a7b8f4eb..a78bae03ed7 100644
--- a/apps/settings/src/components/PersonalInfo/EmailSection/EmailSection.vue
+++ b/apps/settings/src/components/PersonalInfo/EmailSection/EmailSection.vue
@@ -20,17 +20,14 @@
 -->
 
 <template>
-	<form
-		ref="form"
-		class="section"
-		@submit.stop.prevent="() => {}">
+	<section>
 		<HeaderBar
 			:account-property="accountProperty"
 			label-for="email"
 			:handle-scope-change="savePrimaryEmailScope"
 			:is-editable="displayNameChangeSupported"
 			:is-multi-value-supported="true"
-			:is-valid-form="isValidForm"
+			:is-valid-section="isValidSection"
 			:scope.sync="primaryEmail.scope"
 			@add-additional="onAddAdditionalEmail" />
 
@@ -52,7 +49,7 @@
 		<span v-else>
 			{{ primaryEmail.value || t('settings', 'No email address set') }}
 		</span>
-	</form>
+	</section>
 </template>
 
 <script>
@@ -64,6 +61,7 @@ import HeaderBar from '../shared/HeaderBar'
 
 import { ACCOUNT_PROPERTY_READABLE_ENUM, DEFAULT_ADDITIONAL_EMAIL_SCOPE } from '../../../constants/AccountPropertyConstants'
 import { savePrimaryEmail, savePrimaryEmailScope, removeAdditionalEmail } from '../../../service/PersonalInfo/EmailService'
+import { validateEmail } from '../../../utils/validate'
 
 const { emails: { additionalEmails, primaryEmail } } = loadState('settings', 'personalInfoParameters', {})
 const { displayNameChangeSupported } = loadState('settings', 'accountParameters', {})
@@ -81,13 +79,24 @@ export default {
 			accountProperty: ACCOUNT_PROPERTY_READABLE_ENUM.EMAIL,
 			additionalEmails,
 			displayNameChangeSupported,
-			isValidForm: true,
 			primaryEmail,
 			savePrimaryEmailScope,
 		}
 	},
 
 	computed: {
+		firstAdditionalEmail() {
+			if (this.additionalEmails.length) {
+				return this.additionalEmails[0].value
+			}
+			return null
+		},
+
+		isValidSection() {
+			return validateEmail(this.primaryEmail.value)
+				&& this.additionalEmails.map(({ value }) => value).every(validateEmail)
+		},
+
 		primaryEmailValue: {
 			get() {
 				return this.primaryEmail.value
@@ -96,41 +105,25 @@ export default {
 				this.primaryEmail.value = value
 			},
 		},
-
-		firstAdditionalEmail() {
-			if (this.additionalEmails.length) {
-				return this.additionalEmails[0].value
-			}
-			return null
-		},
-	},
-
-	mounted() {
-		this.$nextTick(() => this.updateFormValidity())
 	},
 
 	methods: {
 		onAddAdditionalEmail() {
-			if (this.$refs.form?.checkValidity()) {
+			if (this.isValidSection) {
 				this.additionalEmails.push({ value: '', scope: DEFAULT_ADDITIONAL_EMAIL_SCOPE })
-				this.$nextTick(() => this.updateFormValidity())
 			}
 		},
 
 		onDeleteAdditionalEmail(index) {
 			this.$delete(this.additionalEmails, index)
-			this.$nextTick(() => this.updateFormValidity())
 		},
 
 		async onUpdateEmail() {
-			this.$nextTick(() => this.updateFormValidity())
-
 			if (this.primaryEmailValue === '' && this.firstAdditionalEmail) {
 				const deletedEmail = this.firstAdditionalEmail
 				await this.deleteFirstAdditionalEmail()
 				this.primaryEmailValue = deletedEmail
 				await this.updatePrimaryEmail()
-				this.$nextTick(() => this.updateFormValidity())
 			}
 		},
 
@@ -166,17 +159,15 @@ export default {
 				this.logger.error(errorMessage, error)
 			}
 		},
-
-		updateFormValidity() {
-			this.isValidForm = this.$refs.form?.checkValidity()
-		},
 	},
 }
 </script>
 
 <style lang="scss" scoped>
-	form::v-deep button {
-		&:disabled {
+	section {
+		padding: 10px 10px;
+
+		&::v-deep button:disabled {
 			cursor: default;
 		}
 	}
diff --git a/apps/settings/src/components/PersonalInfo/shared/FederationControl.vue b/apps/settings/src/components/PersonalInfo/shared/FederationControl.vue
index a48e5b62969..b94c3e38760 100644
--- a/apps/settings/src/components/PersonalInfo/shared/FederationControl.vue
+++ b/apps/settings/src/components/PersonalInfo/shared/FederationControl.vue
@@ -87,7 +87,7 @@ export default {
 
 	data() {
 		return {
-			accountPropertyLowerCase: this.accountProperty.toLowerCase(),
+			accountPropertyLowerCase: this.accountProperty.toLocaleLowerCase(),
 			initialScope: this.scope,
 		}
 	},
diff --git a/apps/settings/src/components/PersonalInfo/shared/HeaderBar.vue b/apps/settings/src/components/PersonalInfo/shared/HeaderBar.vue
index dcbbc2d09d7..ab5afe060c8 100644
--- a/apps/settings/src/components/PersonalInfo/shared/HeaderBar.vue
+++ b/apps/settings/src/components/PersonalInfo/shared/HeaderBar.vue
@@ -22,7 +22,8 @@
 <template>
 	<h3>
 		<label :for="labelFor">
-			{{ t('settings', accountProperty) }}
+			<!-- Already translated as required by prop validator -->
+			{{ accountProperty }}
 		</label>
 
 		<FederationControl
@@ -35,7 +36,7 @@
 		<template v-if="isEditable && isMultiValueSupported">
 			<AddButton
 				class="add-button"
-				:disabled="!isValidForm"
+				:disabled="!isValidSection"
 				@click.stop.prevent="onAddAdditional" />
 		</template>
 	</h3>
@@ -73,7 +74,7 @@ export default {
 			type: Boolean,
 			default: false,
 		},
-		isValidForm: {
+		isValidSection: {
 			type: Boolean,
 			default: true,
 		},
@@ -106,6 +107,18 @@ export default {
 </script>
 
 <style lang="scss" scoped>
+	h3 {
+    display: inline-flex;
+    width: 100%;
+    margin: 12px 0 0 0;
+		font-size: 16px;
+    color: var(--color-text-light);
+
+		label {
+			cursor: pointer;
+		}
+	}
+
 	.federation-control {
 		margin: -12px 0 0 8px;
 	}
diff --git a/apps/settings/src/constants/AccountPropertyConstants.js b/apps/settings/src/constants/AccountPropertyConstants.js
index a1c2b4814ca..19d4814e1c0 100644
--- a/apps/settings/src/constants/AccountPropertyConstants.js
+++ b/apps/settings/src/constants/AccountPropertyConstants.js
@@ -24,6 +24,8 @@
  * SYNC to be kept in sync with lib/public/Accounts/IAccountManager.php
  */
 
+import { translate as t } from '@nextcloud/l10n'
+
 /** Enum of account properties */
 export const ACCOUNT_PROPERTY_ENUM = Object.freeze({
 	ADDRESS: 'address',
@@ -36,16 +38,16 @@ export const ACCOUNT_PROPERTY_ENUM = Object.freeze({
 	WEBSITE: 'website',
 })
 
-/** Enum of account properties to human readable account properties */
+/** Enum of account properties to human readable account property names */
 export const ACCOUNT_PROPERTY_READABLE_ENUM = Object.freeze({
-	ADDRESS: 'Address',
-	AVATAR: 'Avatar',
-	DISPLAYNAME: 'Full name',
-	EMAIL: 'Email',
-	EMAIL_COLLECTION: 'Additional Email',
-	PHONE: 'Phone',
-	TWITTER: 'Twitter',
-	WEBSITE: 'Website',
+	ADDRESS: t('settings', 'Address'),
+	AVATAR: t('settings', 'Avatar'),
+	DISPLAYNAME: t('settings', 'Full name'),
+	EMAIL: t('settings', 'Email'),
+	EMAIL_COLLECTION: t('settings', 'Additional email'),
+	PHONE: t('settings', 'Phone number'),
+	TWITTER: t('settings', 'Twitter'),
+	WEBSITE: t('settings', 'Website'),
 })
 
 /** Enum of scopes */
@@ -71,9 +73,6 @@ export const PROPERTY_READABLE_SUPPORTED_SCOPES_ENUM = Object.freeze({
 /** Scope suffix */
 export const SCOPE_SUFFIX = 'Scope'
 
-/** Default additional email scope */
-export const DEFAULT_ADDITIONAL_EMAIL_SCOPE = SCOPE_ENUM.LOCAL
-
 /**
  * Enum of scope names to properties
  *
@@ -105,3 +104,14 @@ export const SCOPE_PROPERTY_ENUM = Object.freeze({
 		iconClass: 'icon-link',
 	},
 })
+
+/** Default additional email scope */
+export const DEFAULT_ADDITIONAL_EMAIL_SCOPE = SCOPE_ENUM.LOCAL
+
+/**
+ * Email validation regex
+ *
+ * *Sourced from https://github.com/mpyw/FILTER_VALIDATE_EMAIL.js/blob/71e62ca48841d2246a1b531e7e84f5a01f15e615/src/regexp/ascii.ts*
+ */
+// eslint-disable-next-line no-control-regex
+export const VALIDATE_EMAIL_REGEX = /^(?!(?:(?:\x22?\x5C[\x00-\x7E]\x22?)|(?:\x22?[^\x5C\x22]\x22?)){255,})(?!(?:(?:\x22?\x5C[\x00-\x7E]\x22?)|(?:\x22?[^\x5C\x22]\x22?)){65,}@)(?:(?:[\x21\x23-\x27\x2A\x2B\x2D\x2F-\x39\x3D\x3F\x5E-\x7E]+)|(?:\x22(?:[\x01-\x08\x0B\x0C\x0E-\x1F\x21\x23-\x5B\x5D-\x7F]|(?:\x5C[\x00-\x7F]))*\x22))(?:\.(?:(?:[\x21\x23-\x27\x2A\x2B\x2D\x2F-\x39\x3D\x3F\x5E-\x7E]+)|(?:\x22(?:[\x01-\x08\x0B\x0C\x0E-\x1F\x21\x23-\x5B\x5D-\x7F]|(?:\x5C[\x00-\x7F]))*\x22)))*@(?:(?:(?!.*[^.]{64,})(?:(?:(?:xn--)?[a-z0-9]+(?:-+[a-z0-9]+)*\.){1,126}){1,}(?:(?:[a-z][a-z0-9]*)|(?:(?:xn--)[a-z0-9]+))(?:-+[a-z0-9]+)*)|(?:\[(?:(?:IPv6:(?:(?:[a-f0-9]{1,4}(?::[a-f0-9]{1,4}){7})|(?:(?!(?:.*[a-f0-9][:\]]){7,})(?:[a-f0-9]{1,4}(?::[a-f0-9]{1,4}){0,5})?::(?:[a-f0-9]{1,4}(?::[a-f0-9]{1,4}){0,5})?)))|(?:(?:IPv6:(?:(?:[a-f0-9]{1,4}(?::[a-f0-9]{1,4}){5}:)|(?:(?!(?:.*[a-f0-9]:){5,})(?:[a-f0-9]{1,4}(?::[a-f0-9]{1,4}){0,3})?::(?:[a-f0-9]{1,4}(?::[a-f0-9]{1,4}){0,3}:)?)))?(?:(?:25[0-5])|(?:2[0-4][0-9])|(?:1[0-9]{2})|(?:[1-9]?[0-9]))(?:\.(?:(?:25[0-5])|(?:2[0-4][0-9])|(?:1[0-9]{2})|(?:[1-9]?[0-9]))){3}))\]))$/i
diff --git a/apps/settings/src/utils/validate.js b/apps/settings/src/utils/validate.js
new file mode 100644
index 00000000000..abea6250f3f
--- /dev/null
+++ b/apps/settings/src/utils/validate.js
@@ -0,0 +1,51 @@
+/**
+ * @copyright 2021, Christopher Ng <chrng8@gmail.com>
+ *
+ * @author Christopher Ng <chrng8@gmail.com>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+import { VALIDATE_EMAIL_REGEX } from '../constants/AccountPropertyConstants'
+
+/**
+ * Validate the display name input
+ *
+ * @param {string} input the input
+ * @returns {boolean}
+ */
+export function validateDisplayName(input) {
+	return input !== ''
+}
+
+/**
+ * Validate the email input
+ *
+ * *Compliant with PHP core FILTER_VALIDATE_EMAIL validator*
+ *
+ * *Reference implementation https://github.com/mpyw/FILTER_VALIDATE_EMAIL.js/blob/71e62ca48841d2246a1b531e7e84f5a01f15e615/src/index.ts*
+ *
+ * @param {string} input the input
+ * @returns {boolean}
+ */
+export function validateEmail(input) {
+	return typeof input === 'string'
+		&& VALIDATE_EMAIL_REGEX.test(input)
+		&& input.slice(-1) !== '\n'
+		&& input.length <= 320
+		&& encodeURIComponent(input).replace(/%../g, 'x').length <= 320
+}