Merge pull request #50698 from nextcloud/backport/50678/stable29

[stable29] fix(AccountProperty): better validation of twitter and fediverse handles

apps/settings/lib/Controller/UsersController.php

@@ -304,6 +304,7 @@ protected function canAdminChangeUserPasswords(): bool {
 	 * @NoAdminRequired
 	 * @NoSubAdminRequired
 	 * @PasswordConfirmationRequired
+	 * @UserRateThrottle(limit=50, period=600)
 	 *
 	 * @param string|null $avatarScope
 	 * @param string|null $displayname

apps/settings/src/components/PersonalInfo/FediverseSection.vue

@@ -21,30 +21,40 @@
 -->
 
 <template>
-	<AccountPropertySection v-bind.sync="fediverse"
+	<AccountPropertySection v-bind.sync="value"
+		:readable="readable"
+		:on-validate="onValidate"
 		:placeholder="t('settings', 'Your handle')" />
 </template>
 
-<script>
+<script setup lang="ts">
+import type { AccountProperties } from '../../constants/AccountPropertyConstants.js'
 import { loadState } from '@nextcloud/initial-state'
-
-import AccountPropertySection from './shared/AccountPropertySection.vue'
-
+import { t } from '@nextcloud/l10n'
+import { ref } from 'vue'
 import { NAME_READABLE_ENUM } from '../../constants/AccountPropertyConstants.js'
 
-const { fediverse } = loadState('settings', 'personalInfoParameters', {})
-
-export default {
-	name: 'FediverseSection',
-
-	components: {
-		AccountPropertySection,
-	},
+import AccountPropertySection from './shared/AccountPropertySection.vue'
 
-	data() {
-		return {
-			fediverse: { ...fediverse, readable: NAME_READABLE_ENUM[fediverse.name] },
-		}
-	},
+const { fediverse } = loadState<AccountProperties>('settings', 'personalInfoParameters', {})
+
+const value = ref({ ...fediverse })
+const readable = NAME_READABLE_ENUM[fediverse.name]
+
+/**
+ * Validate a fediverse handle
+ * @param text The potential fediverse handle
+ */
+function onValidate(text: string): boolean {
+	const result = text.match(/^@?([^@/]+)@([^@/]+)$/)
+	if (result === null) {
+		return false
+	}
+
+	try {
+		return URL.parse(`https://${result[2]}/`) !== null
+	} catch {
+		return false
+	}
 }
 </script>

apps/settings/src/components/PersonalInfo/TwitterSection.vue

@@ -21,30 +21,31 @@
 -->
 
 <template>
-	<AccountPropertySection v-bind.sync="twitter"
+	<AccountPropertySection v-bind.sync="value"
+		:readable="readable"
+		:on-validate="onValidate"
 		:placeholder="t('settings', 'Your X (formerly Twitter) handle')" />
 </template>
 
-<script>
-import { loadState } from '@nextcloud/initial-state'
+<script setup lang="ts">
+import type { AccountProperties } from '../../constants/AccountPropertyConstants.js'
 
+import { loadState } from '@nextcloud/initial-state'
+import { t } from '@nextcloud/l10n'
+import { ref } from 'vue'
+import { NAME_READABLE_ENUM } from '../../constants/AccountPropertyConstants.ts'
 import AccountPropertySection from './shared/AccountPropertySection.vue'
 
-import { NAME_READABLE_ENUM } from '../../constants/AccountPropertyConstants.js'
-
-const { twitter } = loadState('settings', 'personalInfoParameters', {})
-
-export default {
-	name: 'TwitterSection',
+const { twitter } = loadState<AccountProperties>('settings', 'personalInfoParameters', {})
 
-	components: {
-		AccountPropertySection,
-	},
+const value = ref({ ...twitter })
+const readable = NAME_READABLE_ENUM[twitter.name]
 
-	data() {
-		return {
-			twitter: { ...twitter, readable: NAME_READABLE_ENUM[twitter.name] },
-		}
-	},
+/**
+ * Validate that the text might be a twitter handle
+ * @param text The potential twitter handle
+ */
+function onValidate(text: string): boolean {
+	return text.match(/^@?([a-zA-Z0-9_]{2,15})$/) !== null
 }
 </script>

apps/settings/src/constants/AccountPropertyConstants.js → apps/settings/src/constants/AccountPropertyConstants.ts

@@ -121,12 +121,12 @@ export const ACCOUNT_SETTING_PROPERTY_READABLE_ENUM = Object.freeze({
 })
 
 /** Enum of scopes */
-export const SCOPE_ENUM = Object.freeze({
-	PRIVATE: 'v2-private',
-	LOCAL: 'v2-local',
-	FEDERATED: 'v2-federated',
-	PUBLISHED: 'v2-published',
-})
+export enum SCOPE_ENUM {
+	PRIVATE = 'v2-private',
+	LOCAL = 'v2-local',
+	FEDERATED = 'v2-federated',
+	PUBLISHED = 'v2-published',
+}
 
 /** Enum of readable account properties to supported scopes */
 export const PROPERTY_READABLE_SUPPORTED_SCOPES_ENUM = Object.freeze({
@@ -197,11 +197,11 @@ export const SCOPE_PROPERTY_ENUM = Object.freeze({
 export const DEFAULT_ADDITIONAL_EMAIL_SCOPE = SCOPE_ENUM.LOCAL
 
 /** Enum of verification constants, according to IAccountManager */
-export const VERIFICATION_ENUM = Object.freeze({
-	NOT_VERIFIED: 0,
-	VERIFICATION_IN_PROGRESS: 1,
-	VERIFIED: 2,
-})
+export enum VERIFICATION_ENUM {
+	NOT_VERIFIED = 0,
+	VERIFICATION_IN_PROGRESS = 1,
+	VERIFIED = 2,
+}
 
 /**
  * Email validation regex
@@ -210,3 +210,12 @@ export const VERIFICATION_ENUM = Object.freeze({
  */
 // 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
+
+export interface IAccountProperty {
+	name: string
+	value: string
+	scope: SCOPE_ENUM
+	verified: VERIFICATION_ENUM
+}
+
+export type AccountProperties = Record<(typeof ACCOUNT_PROPERTY_ENUM)[keyof (typeof ACCOUNT_PROPERTY_ENUM)], IAccountProperty>

apps/settings/src/service/PersonalInfo/EmailService.js

@@ -26,7 +26,7 @@ import { generateOcsUrl } from '@nextcloud/router'
 import { confirmPassword } from '@nextcloud/password-confirmation'
 import '@nextcloud/password-confirmation/dist/style.css'
 
-import { ACCOUNT_PROPERTY_ENUM, SCOPE_SUFFIX } from '../../constants/AccountPropertyConstants.js'
+import { ACCOUNT_PROPERTY_ENUM, SCOPE_SUFFIX } from '../../constants/AccountPropertyConstants.ts'
 
 /**
  * Save the primary email of the user

apps/settings/src/service/PersonalInfo/PersonalInfoService.js

@@ -26,7 +26,7 @@ import { generateOcsUrl } from '@nextcloud/router'
 import { confirmPassword } from '@nextcloud/password-confirmation'
 import '@nextcloud/password-confirmation/dist/style.css'
 
-import { SCOPE_SUFFIX } from '../../constants/AccountPropertyConstants.js'
+import { SCOPE_SUFFIX } from '../../constants/AccountPropertyConstants.ts'
 
 /**
  * Save the primary account property value for the user

apps/settings/src/utils/validate.js

@@ -26,7 +26,7 @@
  * TODO add nice validation errors for Profile page settings modal
  */
 
-import { VALIDATE_EMAIL_REGEX } from '../constants/AccountPropertyConstants.js'
+import { VALIDATE_EMAIL_REGEX } from '../constants/AccountPropertyConstants.ts'
 
 /**
  * Validate the email input

apps/settings/tests/UserMigration/AccountMigratorTest.php

@@ -31,6 +31,7 @@
 use OCP\Accounts\IAccountManager;
 use OCP\AppFramework\App;
 use OCP\IAvatarManager;
+use OCP\IConfig;
 use OCP\IUserManager;
 use OCP\UserMigration\IExportDestination;
 use OCP\UserMigration\IImportSource;
@@ -69,8 +70,11 @@ class AccountMigratorTest extends TestCase {
 	private const REGEX_CONFIG_FILE = '/^' . Application::APP_ID . '\/' . '[a-z]+\.json' . '$/';
 
 	protected function setUp(): void {
+		parent::setUp();
+
 		$app = new App(Application::APP_ID);
 		$container = $app->getContainer();
+		$container->get(IConfig::class)->setSystemValue('has_internet_connection', false);
 
 		$this->userManager = $container->get(IUserManager::class);
 		$this->avatarManager = $container->get(IAvatarManager::class);
@@ -81,6 +85,11 @@ protected function setUp(): void {
 		$this->output = $this->createMock(OutputInterface::class);
 	}
 
+	protected function tearDown(): void {
+		\OCP\Server::get(IConfig::class)->setSystemValue('has_internet_connection', true);
+		parent::tearDown();
+	}
+
 	public function dataImportExportAccount(): array {
 		return array_map(
 			function (string $filename) {

apps/settings/tests/UserMigration/assets/account-complex.json

@@ -1 +1 @@
-{"displayname":{"name":"displayname","value":"Steve Smith","scope":"v2-local","verified":"0","verificationData":""},"address":{"name":"address","value":"123 Water St","scope":"v2-local","verified":"0","verificationData":""},"website":{"name":"website","value":"https:\/\/example.org","scope":"v2-local","verified":"0","verificationData":""},"email":{"name":"email","value":"steve@example.org","scope":"v2-federated","verified":"0","verificationData":""},"avatar":{"name":"avatar","value":"","scope":"v2-local","verified":"0","verificationData":""},"phone":{"name":"phone","value":"+12178515387","scope":"v2-private","verified":"0","verificationData":""},"twitter":{"name":"twitter","value":"steve","scope":"v2-federated","verified":"0","verificationData":""},"fediverse":{"name":"fediverse","value":"@steve@floss.social","scope":"v2-federated","verified":"0","verificationData":""},"organisation":{"name":"organisation","value":"Mytery Machine","scope":"v2-private","verified":"0","verificationData":""},"role":{"name":"role","value":"Manager","scope":"v2-private","verified":"0","verificationData":""},"headline":{"name":"headline","value":"I am Steve","scope":"v2-local","verified":"0","verificationData":""},"biography":{"name":"biography","value":"Lorem ipsum dolor sit amet, consectetur adipiscing elit. Mauris porttitor ullamcorper dictum. Sed fermentum ut ligula scelerisque semper. Aliquam interdum convallis tellus eu dapibus. Integer in justo sollicitudin, hendrerit ligula sit amet, blandit sem.\n\nSuspendisse consectetur ultrices accumsan. Quisque sagittis bibendum lectus ut placerat. Mauris tincidunt ornare neque, et pulvinar tortor porttitor eu.","scope":"v2-local","verified":"0","verificationData":""},"profile_enabled":{"name":"profile_enabled","value":"1","scope":"v2-local","verified":"0","verificationData":""},"additional_mail":[{"name":"additional_mail","value":"steve@example.com","scope":"v2-published","verified":"0","verificationData":""},{"name":"additional_mail","value":"steve@earth.world","scope":"v2-local","verified":"0","verificationData":""}]}
+{"displayname":{"name":"displayname","value":"Steve Smith","scope":"v2-local","verified":"0","verificationData":""},"address":{"name":"address","value":"123 Water St","scope":"v2-local","verified":"0","verificationData":""},"website":{"name":"website","value":"https:\/\/example.org","scope":"v2-local","verified":"0","verificationData":""},"email":{"name":"email","value":"steve@example.org","scope":"v2-federated","verified":"0","verificationData":""},"avatar":{"name":"avatar","value":"","scope":"v2-local","verified":"0","verificationData":""},"phone":{"name":"phone","value":"+12178515387","scope":"v2-private","verified":"0","verificationData":""},"twitter":{"name":"twitter","value":"steve","scope":"v2-federated","verified":"0","verificationData":""},"fediverse":{"name":"fediverse","value":"steve@floss.social","scope":"v2-federated","verified":"0","verificationData":""},"organisation":{"name":"organisation","value":"Mytery Machine","scope":"v2-private","verified":"0","verificationData":""},"role":{"name":"role","value":"Manager","scope":"v2-private","verified":"0","verificationData":""},"headline":{"name":"headline","value":"I am Steve","scope":"v2-local","verified":"0","verificationData":""},"biography":{"name":"biography","value":"Lorem ipsum dolor sit amet, consectetur adipiscing elit. Mauris porttitor ullamcorper dictum. Sed fermentum ut ligula scelerisque semper. Aliquam interdum convallis tellus eu dapibus. Integer in justo sollicitudin, hendrerit ligula sit amet, blandit sem.\n\nSuspendisse consectetur ultrices accumsan. Quisque sagittis bibendum lectus ut placerat. Mauris tincidunt ornare neque, et pulvinar tortor porttitor eu.","scope":"v2-local","verified":"0","verificationData":""},"profile_enabled":{"name":"profile_enabled","value":"1","scope":"v2-local","verified":"0","verificationData":""},"additional_mail":[{"name":"additional_mail","value":"steve@example.com","scope":"v2-published","verified":"0","verificationData":""},{"name":"additional_mail","value":"steve@earth.world","scope":"v2-local","verified":"0","verificationData":""}]}

build/integration/features/bootstrap/BasicStructure.php

@@ -147,7 +147,11 @@ public function sendingTo($verb, $url) {
 	 * @return string
 	 */
 	public function getOCSResponse($response) {
-		return simplexml_load_string($response->getBody())->meta[0]->statuscode;
+		$body = simplexml_load_string((string)$response->getBody());
+		if ($body === false) {
+			throw new \RuntimeException('Could not parse OCS response, body is not valid XML');
+		}
+		return $body->meta[0]->statuscode;
 	}
 
 	/**

build/integration/features/bootstrap/FeatureContext.php

@@ -32,9 +32,16 @@
  * Features context.
  */
 class FeatureContext implements Context, SnippetAcceptingContext {
+	use AppConfiguration;
 	use ContactsMenu;
 	use ExternalStorage;
 	use Search;
 	use WebDav;
 	use Trashbin;
+
+	protected function resetAppConfigs(): void {
+		$this->deleteServerConfig('bruteForce', 'whitelist_0');
+		$this->deleteServerConfig('bruteForce', 'whitelist_1');
+		$this->deleteServerConfig('bruteforcesettings', 'apply_allowlist_to_ratelimit');
+	}
 }

build/integration/features/provisioning-v1.feature

@@ -1,6 +1,9 @@
 Feature: provisioning
 	Background:
 		Given using api version "1"
+		Given parameter "whitelist_0" of app "bruteForce" is set to "127.0.0.1"
+		Given parameter "whitelist_1" of app "bruteForce" is set to "::1"
+		Given parameter "apply_allowlist_to_ratelimit" of app "bruteforcesettings" is set to "true"
 
 	Scenario: Getting an not existing user
 		Given As an "admin"
@@ -597,6 +600,7 @@ Feature: provisioning
 			| settings |
 			| sharebymail |
 			| systemtags |
+			| testing |
 			| theming |
 			| twofactor_backupcodes |
 			| updatenotification |
@@ -621,6 +625,7 @@ Feature: provisioning
 		And the HTTP status code should be "200"
 
 	Scenario: enable an app
+		Given invoking occ with "app:disable testing"
 		Given As an "admin"
 		And app "testing" is disabled
 		When sending "POST" to "/cloud/apps/testing"
@@ -635,12 +640,14 @@ Feature: provisioning
 		And the HTTP status code should be "200"
 
 	Scenario: disable an app
+		Given invoking occ with "app:enable testing"
 		Given As an "admin"
 		And app "testing" is enabled
 		When sending "DELETE" to "/cloud/apps/testing"
 		Then the OCS status code should be "100"
 		And the HTTP status code should be "200"
 		And app "testing" is disabled
+		Given invoking occ with "app:enable testing"
 
 	Scenario: disable an user
 		Given As an "admin"

build/psalm-baseline.xml

@@ -2907,6 +2907,14 @@
       <code><![CDATA[?IImage]]></code>
     </InvalidReturnType>
   </file>
+  <file src="lib/private/Profile/Actions/FediverseAction.php">
+    <NoValue>
+      <code><![CDATA[$instance]]></code>
+    </NoValue>
+    <RedundantCondition>
+      <code><![CDATA[$instance === '']]></code>
+    </RedundantCondition>
+  </file>
   <file src="lib/private/RedisFactory.php">
     <InvalidArgument>
       <code><![CDATA[\RedisCluster::OPT_SLAVE_FAILOVER]]></code>

cypress/e2e/settings/personal-info.cy.ts

@@ -115,18 +115,26 @@ const checkSettingsVisibility = (property: string, defaultVisibility: Visibility
 	}) */
 }
 
-const genericProperties = ['Location', 'X (formerly Twitter)', 'Fediverse']
+const genericProperties = [
+	['Location', 'Berlin'],
+	['X (formerly Twitter)', 'nextclouders'],
+	['Fediverse', 'nextcloud@mastodon.xyz'],
+]
 const nonfederatedProperties = ['Organisation', 'Role', 'Headline', 'About']
 
 describe('Settings: Change personal information', { testIsolation: true }, () => {
 
 	before(() => {
+		// make sure the fediverse check does not do http requests
+		cy.runOccCommand('config:system:set has_internet_connection --value false')
 		// ensure we can set locale and language
 		cy.runOccCommand('config:system:delete force_language')
 		cy.runOccCommand('config:system:delete force_locale')
 	})
 
 	after(() => {
+		cy.runOccCommand('config:system:delete has_internet_connection')
+
 		cy.runOccCommand('config:system:set force_language --value en')
 		cy.runOccCommand('config:system:set force_locale --value en_US')
 	})
@@ -350,22 +358,21 @@ describe('Settings: Change personal information', { testIsolation: true }, () =>
 	})
 
 	// Check generic properties that allow any visibility and any value
-	genericProperties.forEach((property) => {
+	genericProperties.forEach(([property, value]) => {
 		it(`Can set ${property} and change its visibility`, () => {
-			const uniqueValue = `${property.toUpperCase()} ${property.toLowerCase()}`
 			cy.contains('label', property).scrollIntoView()
-			inputForLabel(property).type(uniqueValue)
+			inputForLabel(property).type(value)
 			handlePasswordConfirmation(user.password)
 
 			cy.wait('@submitSetting')
 			cy.reload()
-			inputForLabel(property).should('have.value', uniqueValue)
+			inputForLabel(property).should('have.value', value)
 
 			checkSettingsVisibility(property)
 
 			// check it is visible on the profile
 			cy.visit(`/u/${user.userId}`)
-			cy.contains(uniqueValue).should('be.visible')
+			cy.contains(value).should('be.visible')
 		})
 	})