diff --git a/components/Group/Controller/Group.php b/components/Group/Controller/Group.php index c687b4eeaa..6cd0999d57 100644 --- a/components/Group/Controller/Group.php +++ b/components/Group/Controller/Group.php @@ -99,7 +99,7 @@ class Group extends Controller return [ '_template' => 'group/settings.html.twig', 'group' => $group_actor, - 'personal_info_form' => ActorForms::personalInfo($request, $actor, $local_group)->createView(), + 'personal_info_form' => ActorForms::personalInfo(request: $request, scope: $actor, target: $group_actor)->createView(), 'open_details_query' => $this->string('open'), ]; } else { diff --git a/components/Group/Entity/LocalGroup.php b/components/Group/Entity/LocalGroup.php index c54d5b384d..0620c453bd 100644 --- a/components/Group/Entity/LocalGroup.php +++ b/components/Group/Entity/LocalGroup.php @@ -117,7 +117,7 @@ class LocalGroup extends Entity public function getActor() { - return DB::findOneBy('actor', ['id' => $this->actor_id]); + return DB::findOneBy(Actor::class, ['id' => $this->actor_id]); } public static function getByNickname(string $nickname): ?self diff --git a/components/Person/Controller/PersonSettings.php b/components/Person/Controller/PersonSettings.php index 1765c16a4c..94e853bcd1 100644 --- a/components/Person/Controller/PersonSettings.php +++ b/components/Person/Controller/PersonSettings.php @@ -94,7 +94,7 @@ class PersonSettings extends Controller $user = Common::ensureLoggedIn(); $actor = $user->getActor(); - $personal_form = ActorForms::personalInfo($request, $actor, $user); + $personal_form = ActorForms::personalInfo(request: $request, scope: $actor, target: $actor); $email_form = self::email($request); $password_form = self::password($request); $notifications_form_array = self::notifications($request); diff --git a/src/Core/Form.php b/src/Core/Form.php index 42b052dbad..1c9b835c5e 100644 --- a/src/Core/Form.php +++ b/src/Core/Form.php @@ -136,12 +136,21 @@ abstract class Form } /** - * Handle the full life cycle of a form. Creates it with @see + * Handle the full life cycle of a form. Creates it with @param array $form_definition + * @param Request $request + * @param object|null $target + * @param array $extra_args If specified, is used as $target->set($data[$key], $extra_args[$key]) + * @param callable|null $after_step + * @param array $create_args + * @param SymfForm|null $testing_only_form + * @return mixed + * @throws RedirectException + * @throws ServerException + * @see * self::create and inserts the submitted values into the database * - * @throws ServerException */ - public static function handle(array $form_definition, Request $request, ?object $target, array $extra_args = [], ?callable $extra_step = null, array $create_args = [], ?SymfForm $testing_only_form = null): mixed + public static function handle(array $form_definition, Request $request, ?object $target, array $extra_args = [], ?callable $before_step = null, ?callable $after_step = null, array $create_args = [], ?SymfForm $testing_only_form = null): mixed { $form = $testing_only_form ?? self::create($form_definition, $target, ...$create_args); @@ -162,6 +171,13 @@ abstract class Form } unset($data['translation_domain'], $data['save']); + + if (isset($before_step)) { + // @codeCoverageIgnoreStart + $before_step($data, $extra_args); + // @codeCoverageIgnoreEnd + } + foreach ($data as $key => $val) { $method = 'set' . ucfirst(Formatting::snakeCaseToCamelCase($key)); if (method_exists($target, $method)) { @@ -175,9 +191,9 @@ abstract class Form } } - if (isset($extra_step)) { + if (isset($after_step)) { // @codeCoverageIgnoreStart - $extra_step($data, $extra_args); + $after_step($data, $extra_args); // @codeCoverageIgnoreEnd } diff --git a/src/Entity/Actor.php b/src/Entity/Actor.php index e9545d1b1a..70763a7cb6 100644 --- a/src/Entity/Actor.php +++ b/src/Entity/Actor.php @@ -32,10 +32,12 @@ use App\Core\Router\Router; use App\Util\Exception\BugFoundException; use App\Util\Exception\NicknameException; use App\Util\Exception\NotFoundException; +use App\Util\Exception\NotImplementedException; use App\Util\Formatting; use App\Util\Nickname; use Component\Avatar\Avatar; use Component\Group\Entity\GroupMember; +use Component\Group\Entity\LocalGroup; use Component\Language\Entity\ActorLanguage; use Component\Language\Entity\Language; use Component\Subscription\Entity\ActorSubscription; @@ -264,13 +266,24 @@ class Actor extends Entity ]; } - public function getLocalUser(): ?LocalUser + /** + * Note: You will receive a Local from Database, it's a persisted instance + * + * @return LocalUser|LocalGroup Returns the local, if actor is local, null otherwise + */ + public function getLocal(): LocalUser|LocalGroup|null { if ($this->getIsLocal()) { - return DB::findOneBy('local_user', ['id' => $this->getId()]); - } else { - throw new NotFoundException('This is a remote actor.'); + switch ($this->getType()) { + case Actor::PERSON: + return DB::findOneBy(LocalUser::class, ['id' => $this->getId()]); + case Actor::GROUP: + return DB::findOneBy(LocalGroup::class, ['actor_id' => $this->getId()]); + case Actor::BOT: + throw new NotImplementedException('We don\'t exactly have a local application abstraction yet...'); + } } + return null; } public function getAvatarUrl(string $size = 'medium') diff --git a/src/Util/Form/ActorForms.php b/src/Util/Form/ActorForms.php index 91e19b2ad0..f327c81f1f 100644 --- a/src/Util/Form/ActorForms.php +++ b/src/Util/Form/ActorForms.php @@ -59,7 +59,10 @@ class ActorForms /** * Actor personal information panel * - * @throws \App\Util\Exception\NicknameException + * @param Request $request + * @param Actor $scope The perpetrator of the change + * @param Actor $target The victim of changes + * @return mixed * @throws ClientException * @throws NicknameEmptyException * @throws NicknameInvalidException @@ -67,9 +70,15 @@ class ActorForms * @throws NicknameTakenException * @throws NicknameTooLongException * @throws ServerException + * @throws \App\Util\Exception\NicknameException */ - public static function personalInfo(Request $request, Actor $target, LocalUser|LocalGroup $user): mixed + public static function personalInfo(Request $request, Actor $scope, Actor $target): mixed { + // Is $target in $scope's sight? + if (!$scope->canModerate($target)) { + throw new ClientException(_m('You do not have permissions to change :nickname\'s settings', [':nickname' => $target->getNickname()])); + } + // Defining the various form fields $form_definition = [ ['nickname', TextType::class, ['label' => _m('Nickname'), 'required' => true, 'help' => _m('1-64 lowercase letters or numbers, no punctuation or spaces.')]], @@ -82,14 +91,26 @@ class ActorForms ]; // Setting nickname normalised and setting actor cache - $extra_step = static function ($data, $extra_args) use ($user, $target) { - if (!strcmp($user->getNickname(), $data['nickname'])) { - $data['nickname'] = Nickname::normalize($data['nickname'], check_already_used: false, which: Nickname::CHECK_LOCAL_GROUP, check_is_allowed: true); + $before_step = static function ($data, $extra_args) use ($target) { + if ($target->getNickname() !== $data['nickname']) { + // We must only check if is both already used and allowed if the actor is local + $check_is_allowed = $target->getIsLocal(); + if ($target->isGroup()) { + $data['nickname'] = Nickname::normalize($data['nickname'], check_already_used: $check_is_allowed, which: Nickname::CHECK_LOCAL_GROUP, check_is_allowed: $check_is_allowed); + } else { + $data['nickname'] = Nickname::normalize($data['nickname'], check_already_used: $check_is_allowed, which: Nickname::CHECK_LOCAL_USER, check_is_allowed: $check_is_allowed); + } + + // We will set $target actor's nickname in the form::handle, + // but if it is local, we must update the local reference as well + if (!is_null($local = $target->getLocal())) { + $local->setNickname($data['nickname']); + } } - if (!strcmp($target->getFullname(), $data['full_name'])) { + if ($target->getFullname() !== $data['full_name']) { $data['full_name'] = trim($data['full_name']); if (mb_strlen($data['full_name']) > 64) { - throw new ClientException('Fullname cannot be more than 64 character long.'); + throw new ClientException(_m('Full name cannot be more than 64 character long.')); } } @@ -104,7 +125,7 @@ class ActorForms $request, target: $target, extra_args: [], - extra_step: $extra_step, + before_step: $before_step, ); } } diff --git a/tests/Core/FormTest.php b/tests/Core/FormTest.php index 9e72632640..fdd93cc052 100644 --- a/tests/Core/FormTest.php +++ b/tests/Core/FormTest.php @@ -115,12 +115,12 @@ class FormTest extends GNUsocialTestCase $mock_form->method('isSubmitted')->willReturn(true); $mock_form->method('isValid')->willReturn(true); $mock_form->method('getData')->willReturn($data); - $ret = Form::handle(form_definition: [/* not normal usage */], request: $mock_request, target: null, extra_args: [], extra_step: null, create_args: [], testing_only_form: $mock_form); + $ret = Form::handle(form_definition: [/* not normal usage */], request: $mock_request, target: null, extra_args: [], before_step: null, after_step: null, create_args: [], testing_only_form: $mock_form); static::assertSame($data, $ret); $actor = Actor::create(['nickname' => 'form_testing_new_user', 'is_local' => false]); DB::persist($actor); - $ret = Form::handle(form_definition: [/* not normal usage */], request: $mock_request, target: $actor, extra_args: [], extra_step: null, create_args: [], testing_only_form: $mock_form); + $ret = Form::handle(form_definition: [/* not normal usage */], request: $mock_request, target: $actor, extra_args: [], before_step: null, after_step: null, create_args: [], testing_only_form: $mock_form); static::assertSame($mock_form, $ret); static::assertSame($data['fullname'], $actor->getFullname()); static::assertSame($data['homepage'], $actor->getHomepage());