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

[stable30] fix(AccountProperty): better validation of twitter and fediverse handles
This commit is contained in:
Andy Scherzinger 2025-02-07 13:47:30 +01:00 committed by GitHub
commit ea612c5e70
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
23 changed files with 661 additions and 201 deletions

View file

@ -889,7 +889,7 @@ class UsersController extends AUserData {
*/
#[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();

View file

@ -31,6 +31,7 @@ use OCP\AppFramework\Http\Attribute\NoAdminRequired;
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 @@ class UsersController extends Controller {
*/
#[NoAdminRequired]
#[PasswordConfirmationRequired]
#[UserRateLimit(limit: 5, period: 60)]
public function setUserSettings(?string $avatarScope = null,
?string $displayname = null,
?string $displaynameScope = null,

View file

@ -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 { t } from '@nextcloud/l10n'
import { ref } from 'vue'
import { NAME_READABLE_ENUM } from '../../constants/AccountPropertyConstants.js'
import AccountPropertySection from './shared/AccountPropertySection.vue'
import { NAME_READABLE_ENUM } from '../../constants/AccountPropertyConstants.js'
const { fediverse } = loadState<AccountProperties>('settings', 'personalInfoParameters', {})
const { fediverse } = loadState('settings', 'personalInfoParameters', {})
const value = ref({ ...fediverse })
const readable = NAME_READABLE_ENUM[fediverse.name]
export default {
name: 'FediverseSection',
/**
* Validate a fediverse handle
* @param text The potential fediverse handle
*/
function onValidate(text: string): boolean {
const result = text.match(/^@?([^@/]+)@([^@/]+)$/)
if (result === null) {
return false
}
components: {
AccountPropertySection,
},
data() {
return {
fediverse: { ...fediverse, readable: NAME_READABLE_ENUM[fediverse.name] },
}
},
try {
return URL.parse(`https://${result[2]}/`) !== null
} catch {
return false
}
}
</script>

View file

@ -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<AccountProperties>('settings', 'personalInfoParameters', {})
const { twitter } = loadState('settings', 'personalInfoParameters', {})
const value = ref({ ...twitter })
const readable = NAME_READABLE_ENUM[twitter.name]
export default {
name: 'TwitterSection',
components: {
AccountPropertySection,
},
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>

View file

@ -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>

View file

@ -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'

View file

@ -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'

View file

@ -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

View file

@ -12,6 +12,7 @@ use OCA\Settings\UserMigration\AccountMigrator;
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 @@ class AccountMigratorTest extends TestCase {
$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) {

View file

@ -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":"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":"@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":""},"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":"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":"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":"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":""},"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":"steve@example.com","scope":"v2-published","verified":"0","verificationData":""},{"name":"additional_mail","value":"steve@earth.world","scope":"v2-local","verified":"0","verificationData":""}]}

View file

@ -120,7 +120,11 @@ trait BasicStructure {
* @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;
}
/**

View file

@ -13,9 +13,16 @@ require __DIR__ . '/../../vendor/autoload.php';
* 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');
}
}

View file

@ -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"

View file

@ -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>

View file

@ -98,18 +98,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')
})
@ -333,22 +341,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')
})
})

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

View file

@ -23,6 +23,7 @@ use OCP\Cache\CappedMemoryCache;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\Defaults;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Http\Client\IClientService;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IL10N;
@ -92,50 +93,11 @@ class AccountManager implements IAccountManager {
private IURLGenerator $urlGenerator,
private ICrypto $crypto,
private IPhoneNumberUtil $phoneNumberUtil,
private IClientService $clientService,
) {
$this->internalCache = new CappedMemoryCache();
}
/**
* @return string Provided phone number in E.164 format when it was a valid number
* @throws InvalidArgumentException When the phone number was invalid or no default region is set and the number doesn't start with a country code
*/
protected function parsePhoneNumber(string $input): string {
$defaultRegion = $this->config->getSystemValueString('default_phone_region', '');
if ($defaultRegion === '') {
// When no default region is set, only +49… numbers are valid
if (!str_starts_with($input, '+')) {
throw new InvalidArgumentException(self::PROPERTY_PHONE);
}
$defaultRegion = 'EN';
}
$phoneNumber = $this->phoneNumberUtil->convertToStandardFormat($input, $defaultRegion);
if ($phoneNumber !== null) {
return $phoneNumber;
}
throw new InvalidArgumentException(self::PROPERTY_PHONE);
}
/**
* @throws InvalidArgumentException When the website did not have http(s) as protocol or the host name was empty
*/
protected function parseWebsite(string $input): string {
$parts = parse_url($input);
if (!isset($parts['scheme']) || ($parts['scheme'] !== 'https' && $parts['scheme'] !== 'http')) {
throw new InvalidArgumentException(self::PROPERTY_WEBSITE);
}
if (!isset($parts['host']) || $parts['host'] === '') {
throw new InvalidArgumentException(self::PROPERTY_WEBSITE);
}
return $input;
}
/**
* @param IAccountProperty[] $properties
*/
@ -174,42 +136,6 @@ class AccountManager implements IAccountManager {
}
}
protected function sanitizePhoneNumberValue(IAccountProperty $property, bool $throwOnData = false): void {
if ($property->getName() !== self::PROPERTY_PHONE) {
if ($throwOnData) {
throw new InvalidArgumentException(sprintf('sanitizePhoneNumberValue can only sanitize phone numbers, %s given', $property->getName()));
}
return;
}
if ($property->getValue() === '') {
return;
}
try {
$property->setValue($this->parsePhoneNumber($property->getValue()));
} catch (InvalidArgumentException $e) {
if ($throwOnData) {
throw $e;
}
$property->setValue('');
}
}
protected function sanitizeWebsite(IAccountProperty $property, bool $throwOnData = false): void {
if ($property->getName() !== self::PROPERTY_WEBSITE) {
if ($throwOnData) {
throw new InvalidArgumentException(sprintf('sanitizeWebsite can only sanitize web domains, %s given', $property->getName()));
}
}
try {
$property->setValue($this->parseWebsite($property->getValue()));
} catch (InvalidArgumentException $e) {
if ($throwOnData) {
throw $e;
}
$property->setValue('');
}
}
protected function updateUser(IUser $user, array $data, ?array $oldUserData, bool $throwOnData = false): array {
if ($oldUserData === null) {
$oldUserData = $this->getUser($user, false);
@ -728,18 +654,139 @@ class AccountManager implements IAccountManager {
return $account;
}
/**
* Converts value (phone number) in E.164 format when it was a valid number
* @throws InvalidArgumentException When the phone number was invalid or no default region is set and the number doesn't start with a country code
*/
protected function sanitizePropertyPhoneNumber(IAccountProperty $property): void {
$defaultRegion = $this->config->getSystemValueString('default_phone_region', '');
if ($defaultRegion === '') {
// When no default region is set, only +49… numbers are valid
if (!str_starts_with($property->getValue(), '+')) {
throw new InvalidArgumentException(self::PROPERTY_PHONE);
}
$defaultRegion = 'EN';
}
$phoneNumber = $this->phoneNumberUtil->convertToStandardFormat($property->getValue(), $defaultRegion);
if ($phoneNumber === null) {
throw new InvalidArgumentException(self::PROPERTY_PHONE);
}
$property->setValue($phoneNumber);
}
/**
* @throws InvalidArgumentException When the website did not have http(s) as protocol or the host name was empty
*/
private function sanitizePropertyWebsite(IAccountProperty $property): void {
$parts = parse_url($property->getValue());
if (!isset($parts['scheme']) || ($parts['scheme'] !== 'https' && $parts['scheme'] !== 'http')) {
throw new InvalidArgumentException(self::PROPERTY_WEBSITE);
}
if (!isset($parts['host']) || $parts['host'] === '') {
throw new InvalidArgumentException(self::PROPERTY_WEBSITE);
}
}
/**
* @throws InvalidArgumentException If the property value is not a valid user handle according to X's rules
*/
private function sanitizePropertyTwitter(IAccountProperty $property): void {
if ($property->getName() === self::PROPERTY_TWITTER) {
$matches = [];
// twitter handles only contain alpha numeric characters and the underscore and must not be longer than 15 characters
if (preg_match('/^@?([a-zA-Z0-9_]{2,15})$/', $property->getValue(), $matches) !== 1) {
throw new InvalidArgumentException(self::PROPERTY_TWITTER);
}
// drop the leading @ if any to make it the valid handle
$property->setValue($matches[1]);
}
}
/**
* @throws InvalidArgumentException If the property value is not a valid fediverse handle (username@instance where instance is a valid domain)
*/
private function sanitizePropertyFediverse(IAccountProperty $property): void {
if ($property->getName() === self::PROPERTY_FEDIVERSE) {
$matches = [];
if (preg_match('/^@?([^@\s\/\\\]+)@([^\s\/\\\]+)$/', trim($property->getValue()), $matches) !== 1) {
throw new InvalidArgumentException(self::PROPERTY_FEDIVERSE);
}
[, $username, $instance] = $matches;
$validated = filter_var($instance, FILTER_VALIDATE_DOMAIN, FILTER_FLAG_HOSTNAME);
if ($validated !== $instance) {
throw new InvalidArgumentException(self::PROPERTY_FEDIVERSE);
}
if ($this->config->getSystemValueBool('has_internet_connection', true)) {
$client = $this->clientService->newClient();
try {
// try the public account lookup API of mastodon
$response = $client->get("https://{$instance}/api/v1/accounts/lookup?acct={$username}@{$instance}");
// should be a json response with account information
$data = $response->getBody();
if (is_resource($data)) {
$data = stream_get_contents($data);
}
$decoded = json_decode($data, true);
// ensure the username is the same the user passed
// in this case we can assume this is a valid fediverse server and account
if (!is_array($decoded) || ($decoded['username'] ?? '') !== $username) {
throw new InvalidArgumentException();
}
} catch (InvalidArgumentException) {
throw new InvalidArgumentException(self::PROPERTY_FEDIVERSE);
} catch (\Exception $error) {
$this->logger->error('Could not verify fediverse account', ['exception' => $error, 'instance' => $instance]);
throw new InvalidArgumentException(self::PROPERTY_FEDIVERSE);
}
}
$property->setValue("$username@$instance");
}
}
public function updateAccount(IAccount $account): void {
$this->testValueLengths(iterator_to_array($account->getAllProperties()), true);
try {
$property = $account->getProperty(self::PROPERTY_PHONE);
$this->sanitizePhoneNumberValue($property);
if ($property->getValue() !== '') {
$this->sanitizePropertyPhoneNumber($property);
}
} catch (PropertyDoesNotExistException $e) {
// valid case, nothing to do
}
try {
$property = $account->getProperty(self::PROPERTY_WEBSITE);
$this->sanitizeWebsite($property);
if ($property->getValue() !== '') {
$this->sanitizePropertyWebsite($property);
}
} catch (PropertyDoesNotExistException $e) {
// valid case, nothing to do
}
try {
$property = $account->getProperty(self::PROPERTY_TWITTER);
if ($property->getValue() !== '') {
$this->sanitizePropertyTwitter($property);
}
} catch (PropertyDoesNotExistException $e) {
// valid case, nothing to do
}
try {
$property = $account->getProperty(self::PROPERTY_FEDIVERSE);
if ($property->getValue() !== '') {
$this->sanitizePropertyFediverse($property);
}
} catch (PropertyDoesNotExistException $e) {
// valid case, nothing to do
}

View file

@ -10,6 +10,7 @@ declare(strict_types=1);
namespace OC\Profile\Actions;
use OCP\Accounts\IAccountManager;
use OCP\Accounts\PropertyDoesNotExistException;
use OCP\IURLGenerator;
use OCP\IUser;
use OCP\L10N\IFactory;
@ -27,8 +28,13 @@ class FediverseAction implements ILinkAction {
}
public function preload(IUser $targetUser): void {
$account = $this->accountManager->getAccount($targetUser);
$this->value = $account->getProperty(IAccountManager::PROPERTY_FEDIVERSE)->getValue();
try {
$account = $this->accountManager->getAccount($targetUser);
$this->value = $account->getProperty(IAccountManager::PROPERTY_FEDIVERSE)->getValue();
} catch (PropertyDoesNotExistException) {
// `getTarget` will return null to skip this action
$this->value = '';
}
}
public function getAppId(): string {
@ -57,11 +63,18 @@ class FediverseAction implements ILinkAction {
}
public function getTarget(): ?string {
if (empty($this->value)) {
if ($this->value === '') {
return null;
}
$handle = $this->value[0] === '@' ? substr($this->value, 1) : $this->value;
[$username, $instance] = [...explode('@', $handle, 2), ''];
if (($username === '') || ($instance === '')) {
return null;
} elseif (str_contains($username, '/') || str_contains($instance, '/')) {
return null;
}
$username = $this->value[0] === '@' ? substr($this->value, 1) : $this->value;
[$username, $instance] = explode('@', $username);
return 'https://' . $instance . '/@' . $username;
}
}

View file

@ -17,6 +17,9 @@ use OCP\Accounts\UserUpdatedEvent;
use OCP\BackgroundJob\IJobList;
use OCP\Defaults;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Http\Client\IClient;
use OCP\Http\Client\IClientService;
use OCP\Http\Client\IResponse;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IPhoneNumberUtil;
@ -37,45 +40,31 @@ use Test\TestCase;
* @package Test\Accounts
*/
class AccountManagerTest extends TestCase {
/** @var IVerificationToken|MockObject */
protected $verificationToken;
/** @var IMailer|MockObject */
protected $mailer;
/** @var ICrypto|MockObject */
protected $crypto;
/** @var IURLGenerator|MockObject */
protected $urlGenerator;
/** @var Defaults|MockObject */
protected $defaults;
/** @var IFactory|MockObject */
protected $l10nFactory;
/** @var IDBConnection */
private $connection;
/** @var IConfig|MockObject */
private $config;
/** @var IEventDispatcher|MockObject */
private $eventDispatcher;
/** @var IJobList|MockObject */
private $jobList;
/** @var IPhoneNumberUtil */
private $phoneNumberUtil;
/** accounts table name */
private string $table = 'accounts';
/** @var LoggerInterface|MockObject */
private $logger;
private AccountManager $accountManager;
private IDBConnection $connection;
private IPhoneNumberUtil $phoneNumberUtil;
protected IVerificationToken&MockObject $verificationToken;
protected IMailer&MockObject $mailer;
protected ICrypto&MockObject $crypto;
protected IURLGenerator&MockObject $urlGenerator;
protected Defaults&MockObject $defaults;
protected IFactory&MockObject $l10nFactory;
protected IConfig&MockObject $config;
protected IEventDispatcher&MockObject $eventDispatcher;
protected IJobList&MockObject $jobList;
private LoggerInterface&MockObject $logger;
private IClientService&MockObject $clientService;
protected function setUp(): void {
parent::setUp();
$this->connection = \OCP\Server::get(IDBConnection::class);
$this->phoneNumberUtil = new PhoneNumberUtil();
$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
$this->connection = \OC::$server->get(IDBConnection::class);
$this->config = $this->createMock(IConfig::class);
$this->jobList = $this->createMock(IJobList::class);
$this->logger = $this->createMock(LoggerInterface::class);
@ -85,7 +74,7 @@ class AccountManagerTest extends TestCase {
$this->l10nFactory = $this->createMock(IFactory::class);
$this->urlGenerator = $this->createMock(IURLGenerator::class);
$this->crypto = $this->createMock(ICrypto::class);
$this->phoneNumberUtil = new PhoneNumberUtil();
$this->clientService = $this->createMock(IClientService::class);
$this->accountManager = new AccountManager(
$this->connection,
@ -100,6 +89,7 @@ class AccountManagerTest extends TestCase {
$this->urlGenerator,
$this->crypto,
$this->phoneNumberUtil,
$this->clientService,
);
}
@ -465,6 +455,7 @@ class AccountManagerTest extends TestCase {
$this->urlGenerator,
$this->crypto,
$this->phoneNumberUtil,
$this->clientService,
])
->onlyMethods($mockedMethods)
->getMock();
@ -678,7 +669,7 @@ class AccountManagerTest extends TestCase {
$this->assertEquals($expected, $accountManager->getAccount($user));
}
public function dataParsePhoneNumber(): array {
public static function dataParsePhoneNumber(): array {
return [
['0711 / 25 24 28-90', 'DE', '+4971125242890'],
['0711 / 25 24 28-90', '', null],
@ -689,39 +680,198 @@ class AccountManagerTest extends TestCase {
/**
* @dataProvider dataParsePhoneNumber
*/
public function testParsePhoneNumber(string $phoneInput, string $defaultRegion, ?string $phoneNumber): void {
public function testSanitizePhoneNumberOnUpdateAccount(string $phoneInput, string $defaultRegion, ?string $phoneNumber): void {
$this->config->method('getSystemValueString')
->willReturn($defaultRegion);
$user = $this->createMock(IUser::class);
$account = new Account($user);
$account->setProperty(IAccountManager::PROPERTY_PHONE, $phoneInput, IAccountManager::SCOPE_LOCAL, IAccountManager::NOT_VERIFIED);
$manager = $this->getInstance(['getUser', 'updateUser']);
$manager->method('getUser')
->with($user, false)
->willReturn([]);
$manager->expects($phoneNumber === null ? self::never() : self::once())
->method('updateUser');
if ($phoneNumber === null) {
$this->expectException(\InvalidArgumentException::class);
self::invokePrivate($this->accountManager, 'parsePhoneNumber', [$phoneInput]);
} else {
self::assertEquals($phoneNumber, self::invokePrivate($this->accountManager, 'parsePhoneNumber', [$phoneInput]));
}
$manager->updateAccount($account);
if ($phoneNumber !== null) {
self::assertEquals($phoneNumber, $account->getProperty(IAccountManager::PROPERTY_PHONE)->getValue());
}
}
public function dataParseWebsite(): array {
public static function dataSanitizeOnUpdate(): array {
return [
['https://nextcloud.com', 'https://nextcloud.com'],
['http://nextcloud.com', 'http://nextcloud.com'],
['ftp://nextcloud.com', null],
['//nextcloud.com/', null],
['https:///?query', null],
[IAccountManager::PROPERTY_WEBSITE, 'https://nextcloud.com', 'https://nextcloud.com'],
[IAccountManager::PROPERTY_WEBSITE, 'http://nextcloud.com', 'http://nextcloud.com'],
[IAccountManager::PROPERTY_WEBSITE, 'ftp://nextcloud.com', null],
[IAccountManager::PROPERTY_WEBSITE, '//nextcloud.com/', null],
[IAccountManager::PROPERTY_WEBSITE, 'https:///?query', null],
[IAccountManager::PROPERTY_TWITTER, '@nextcloud', 'nextcloud'],
[IAccountManager::PROPERTY_TWITTER, '_nextcloud', '_nextcloud'],
[IAccountManager::PROPERTY_TWITTER, 'FooB4r', 'FooB4r'],
[IAccountManager::PROPERTY_TWITTER, 'X', null],
[IAccountManager::PROPERTY_TWITTER, 'next.cloud', null],
[IAccountManager::PROPERTY_TWITTER, 'ab/cd.zip', null],
[IAccountManager::PROPERTY_TWITTER, 'tooLongForTwitterAndX', null],
[IAccountManager::PROPERTY_FEDIVERSE, 'nextcloud@mastodon.social', 'nextcloud@mastodon.social'],
[IAccountManager::PROPERTY_FEDIVERSE, '@nextcloud@mastodon.xyz', 'nextcloud@mastodon.xyz'],
[IAccountManager::PROPERTY_FEDIVERSE, 'l33t.h4x0r@sub.localhost.local', 'l33t.h4x0r@sub.localhost.local'],
[IAccountManager::PROPERTY_FEDIVERSE, 'invalid/name@mastodon.social', null],
[IAccountManager::PROPERTY_FEDIVERSE, 'name@evil.host/malware.exe', null],
[IAccountManager::PROPERTY_FEDIVERSE, '@is-it-a-host-or-name', null],
[IAccountManager::PROPERTY_FEDIVERSE, 'only-a-name', null],
];
}
/**
* @dataProvider dataParseWebsite
* @param string $websiteInput
* @param string|null $websiteOutput
* @dataProvider dataSanitizeOnUpdate
*/
public function testParseWebsite(string $websiteInput, ?string $websiteOutput): void {
if ($websiteOutput === null) {
public function testSanitizingOnUpdateAccount(string $property, string $input, ?string $output): void {
if ($property === IAccountManager::PROPERTY_FEDIVERSE) {
// We do not test the server response here we do this in the `testSanitizingFediverseServer`
$this->config
->method('getSystemValueBool')
->with('has_internet_connection', true)
->willReturn(false);
}
$user = $this->createMock(IUser::class);
$account = new Account($user);
$account->setProperty($property, $input, IAccountManager::SCOPE_LOCAL, IAccountManager::NOT_VERIFIED);
$manager = $this->getInstance(['getUser', 'updateUser']);
$manager->method('getUser')
->with($user, false)
->willReturn([]);
$manager->expects($output === null ? self::never() : self::once())
->method('updateUser');
if ($output === null) {
$this->expectException(\InvalidArgumentException::class);
self::invokePrivate($this->accountManager, 'parseWebsite', [$websiteInput]);
$this->expectExceptionMessage($property);
}
$manager->updateAccount($account);
if ($output !== null) {
self::assertEquals($output, $account->getProperty($property)->getValue());
}
}
public static function dataSanitizeFediverseServer(): array {
return [
'no internet' => [
'@foo@example.com',
'foo@example.com',
false,
null,
],
'no internet - no at' => [
'foo@example.com',
'foo@example.com',
false,
null,
],
'valid response' => [
'@foo@example.com',
'foo@example.com',
true,
json_encode(['username' => 'foo']),
],
'valid response - no at' => [
'foo@example.com',
'foo@example.com',
true,
json_encode(['username' => 'foo']),
],
// failures
'invalid response' => [
'@foo@example.com',
null,
true,
json_encode(['not found']),
],
'no response' => [
'@foo@example.com',
null,
true,
null,
],
'wrong user' => [
'@foo@example.com',
null,
true,
json_encode(['username' => 'foo@other.example.com']),
],
];
}
/**
* @dataProvider dataSanitizeFediverseServer
*/
public function testSanitizingFediverseServer(string $input, ?string $output, bool $hasInternet, ?string $serverResponse): void {
$this->config->expects(self::once())
->method('getSystemValueBool')
->with('has_internet_connection', true)
->willReturn($hasInternet);
if ($hasInternet) {
$client = $this->createMock(IClient::class);
if ($serverResponse !== null) {
$response = $this->createMock(IResponse::class);
$response->method('getBody')
->willReturn($serverResponse);
$client->expects(self::once())
->method('get')
->with('https://example.com/api/v1/accounts/lookup?acct=foo@example.com')
->willReturn($response);
} else {
$client->expects(self::once())
->method('get')
->with('https://example.com/api/v1/accounts/lookup?acct=foo@example.com')
->willThrowException(new \Exception('404'));
}
$this->clientService
->expects(self::once())
->method('newClient')
->willReturn($client);
} else {
self::assertEquals($websiteOutput, self::invokePrivate($this->accountManager, 'parseWebsite', [$websiteInput]));
$this->clientService
->expects(self::never())
->method('newClient');
}
$user = $this->createMock(IUser::class);
$account = new Account($user);
$account->setProperty(IAccountManager::PROPERTY_FEDIVERSE, $input, IAccountManager::SCOPE_LOCAL, IAccountManager::NOT_VERIFIED);
$manager = $this->getInstance(['getUser', 'updateUser']);
$manager->method('getUser')
->with($user, false)
->willReturn([]);
$manager->expects($output === null ? self::never() : self::once())
->method('updateUser');
if ($output === null) {
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage(IAccountManager::PROPERTY_FEDIVERSE);
}
$manager->updateAccount($account);
if ($output !== null) {
self::assertEquals($output, $account->getProperty(IAccountManager::PROPERTY_FEDIVERSE)->getValue());
}
}

View file

@ -0,0 +1,186 @@
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace Test\Remote;
use OC\Profile\Actions\FediverseAction;
use OCP\Accounts\IAccount;
use OCP\Accounts\IAccountManager;
use OCP\Accounts\IAccountProperty;
use OCP\Accounts\PropertyDoesNotExistException;
use OCP\IL10N;
use OCP\IURLGenerator;
use OCP\IUser;
use OCP\L10N\IFactory;
use PHPUnit\Framework\MockObject\MockObject;
use Test\TestCase;
class FediverseActionTest extends TestCase {
private FediverseAction $action;
private IAccountManager&MockObject $accountManager;
private IL10N&MockObject $l10n;
private IURLGenerator&MockObject $urlGenerator;
protected function setUp(): void {
parent::setUp();
$this->accountManager = $this->createMock(IAccountManager::class);
$this->l10n = $this->createMock(IL10N::class);
$this->urlGenerator = $this->createMock(IURLGenerator::class);
$factory = $this->createMock(IFactory::class);
$factory->method('get')
->with('lib')
->willReturn($this->l10n);
$this->action = new FediverseAction(
$this->accountManager,
$factory,
$this->urlGenerator,
);
}
public function testGetActionId(): void {
self::assertEquals(
IAccountManager::PROPERTY_FEDIVERSE,
$this->action->getId(),
);
}
public function testGetAppId(): void {
self::assertEquals(
'core',
$this->action->getAppId(),
);
}
public function testGetDisplayId(): void {
$this->l10n->expects(self::once())
->method('t')
->with('Fediverse')
->willReturn('Translated fediverse');
self::assertEquals(
'Translated fediverse',
$this->action->getDisplayId(),
);
}
public function testGetPriority(): void {
self::assertEquals(
50,
$this->action->getPriority(),
);
}
public function testGetIcon(): void {
$this->urlGenerator->expects(self::once())
->method('getAbsoluteURL')
->with('the-image-path')
->willReturn('resolved-image-path');
$this->urlGenerator->expects(self::once())
->method('imagePath')
->with('core', 'actions/mastodon.svg')
->willReturn('the-image-path');
self::assertEquals(
'resolved-image-path',
$this->action->getIcon(),
);
}
public static function dataGetTitle(): array {
return [
['the-user@example.com'],
['@the-user@example.com'],
];
}
/** @dataProvider dataGetTitle */
public function testGetTitle(string $value): void {
$property = $this->createMock(IAccountProperty::class);
$property->method('getValue')
->willReturn($value);
$user = $this->createMock(IUser::class);
$account = $this->createMock(IAccount::class);
$account->method('getProperty')
->with(IAccountManager::PROPERTY_FEDIVERSE)
->willReturn($property);
$this->accountManager->method('getAccount')
->with($user)
->willReturn($account);
$this->l10n->expects(self::once())
->method('t')
->with(self::anything(), ['@the-user@example.com'])
->willReturn('Translated title');
// Preload and test
$this->action->preload($user);
self::assertEquals(
'Translated title',
$this->action->getTitle(),
);
}
public static function dataGetTarget(): array {
return [
['', null],
[null, null],
['user@example.com', 'https://example.com/@user'],
['@user@example.com', 'https://example.com/@user'],
['@user@social.example.com', 'https://social.example.com/@user'],
// invalid stuff
['@user', null],
['@example.com', null],
['@@example.com', null],
// evil stuff
['user@example.com/evil.exe', null],
['@user@example.com/evil.exe', null],
['user/evil.exe@example.com', null],
['@user/evil.exe@example.com', null],
];
}
/** @dataProvider dataGetTarget */
public function testGetTarget(?string $value, ?string $expected): void {
$user = $this->createMock(IUser::class);
$account = $this->createMock(IAccount::class);
$this->accountManager->method('getAccount')
->with($user)
->willReturn($account);
if ($value === null) {
// Property does not exist so throw
$account->method('getProperty')
->with(IAccountManager::PROPERTY_FEDIVERSE)
->willThrowException(new PropertyDoesNotExistException(IAccountManager::PROPERTY_FEDIVERSE));
} else {
$property = $this->createMock(IAccountProperty::class);
$property->method('getValue')
->willReturn($value);
$account->method('getProperty')
->willReturn($property);
}
// Preload and test
$this->action->preload($user);
self::assertEquals(
$expected,
$this->action->getTarget(),
);
}
}