Merge pull request #50699 from nextcloud/backport/50678/stable30

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

apps/provisioning_api/lib/Controller/UsersController.php

@@ -889,7 +889,7 @@ public function editUserMultiValue(
 	 */
 	#[PasswordConfirmationRequired]
 	#[NoAdminRequired]
-	#[UserRateLimit(limit: 50, period: 60)]
+	#[UserRateLimit(limit: 50, period: 600)]
 	public function editUser(string $userId, string $key, string $value): DataResponse {
 		$currentLoggedInUser = $this->userSession->getUser();
 

apps/settings/lib/Controller/UsersController.php

@@ -31,6 +31,7 @@
 use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
 use OCP\AppFramework\Http\Attribute\OpenAPI;
 use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired;
+use OCP\AppFramework\Http\Attribute\UserRateLimit;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\AppFramework\Http\TemplateResponse;
@@ -312,6 +313,7 @@ protected function canAdminChangeUserPasswords(): bool {
 	 */
 	#[NoAdminRequired]
 	#[PasswordConfirmationRequired]
+	#[UserRateLimit(limit: 5, period: 60)]
 	public function setUserSettings(?string $avatarScope = null,
 		?string $displayname = null,
 		?string $displaynameScope = null,

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

@@ -4,30 +4,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

@@ -4,30 +4,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

@@ -110,12 +110,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({
@@ -188,11 +188,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
@@ -201,3 +201,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

@@ -8,7 +8,7 @@ import { generateOcsUrl } from '@nextcloud/router'
 import { confirmPassword } from '@nextcloud/password-confirmation'
 import axios from '@nextcloud/axios'
 
-import { ACCOUNT_PROPERTY_ENUM, SCOPE_SUFFIX } from '../../constants/AccountPropertyConstants.js'
+import { ACCOUNT_PROPERTY_ENUM, SCOPE_SUFFIX } from '../../constants/AccountPropertyConstants.ts'
 
 import '@nextcloud/password-confirmation/dist/style.css'
 

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

@@ -8,7 +8,7 @@ import { generateOcsUrl } from '@nextcloud/router'
 import { confirmPassword } from '@nextcloud/password-confirmation'
 import axios from '@nextcloud/axios'
 
-import { SCOPE_SUFFIX } from '../../constants/AccountPropertyConstants.js'
+import { SCOPE_SUFFIX } from '../../constants/AccountPropertyConstants.ts'
 
 import '@nextcloud/password-confirmation/dist/style.css'
 

apps/settings/src/utils/validate.js

@@ -9,7 +9,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

@@ -12,6 +12,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;
@@ -50,8 +51,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);
@@ -62,6 +66,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":"[email protected]","scope":"v2-federated","verified":"1","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":"@[email protected]","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":""},"birthdate":{"name":"birthdate","value":"","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":"[email protected]","scope":"v2-published","verified":"0","verificationData":""},{"name":"additional_mail","value":"[email protected]","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":"[email protected]","scope":"v2-federated","verified":"1","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":"[email protected]","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":""},"birthdate":{"name":"birthdate","value":"","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":"[email protected]","scope":"v2-published","verified":"0","verificationData":""},{"name":"additional_mail","value":"[email protected]","scope":"v2-local","verified":"0","verificationData":""}]}

build/integration/features/bootstrap/BasicStructure.php

@@ -120,7 +120,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

@@ -13,9 +13,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

@@ -4,6 +4,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"
@@ -570,7 +573,7 @@ Feature: provisioning
 		And group "new-group" does not exist
 
 	Scenario: Delete a group with special characters
-	    Given As an "admin"
+		Given As an "admin"
 		And group "España" exists
 		When sending "DELETE" to "/cloud/groups/España"
 		Then the OCS status code should be "100"
@@ -600,6 +603,7 @@ Feature: provisioning
 			| settings |
 			| sharebymail |
 			| systemtags |
+			| testing |
 			| theming |
 			| twofactor_backupcodes |
 			| updatenotification |
@@ -625,6 +629,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"
@@ -638,13 +643,15 @@ Feature: provisioning
 		Then the OCS status code should be "998"
 		And the HTTP status code should be "200"
 
-	Scenario: disable an app
-		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"
+  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

@@ -2483,6 +2483,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>