From c16e4bf53f683aba2e31033872749cfd0bf881c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Tue, 6 Oct 2026 16:29:51 +0200 Subject: [PATCH] feat: update profile in buspro without the teamer's password --- src/BusProNet/ApiClient.php | 18 ++- .../Teamer/Profile/IndexController.php | 48 ++++++- tests/BusProNet/ApiClientTest.php | 129 ++++++++++++++++++ 3 files changed, 187 insertions(+), 8 deletions(-) create mode 100644 tests/BusProNet/ApiClientTest.php diff --git a/src/BusProNet/ApiClient.php b/src/BusProNet/ApiClient.php index 753520f..05d02cd 100644 --- a/src/BusProNet/ApiClient.php +++ b/src/BusProNet/ApiClient.php @@ -68,15 +68,28 @@ class ApiClient } /** + * Writes the teamer's profile to their BusPro address without their password. + * + * "login" switches the password check off, so the two ids are the whole authorisation of + * this write: an address can hold several persons, and only "idadresseperson" names the + * person, while "email" is ignored. The ids must therefore only ever come from a verified + * source - MyE&P userinfo or a BusPro answer - never from a form or a request parameter. + * Element order as verified on the BusPro test system, see + * .ddev/plans/myep-team-oauth-only-login.md. + * * @throws ApiClientException * @throws ResponseParserException */ - public function updateProfile(User $user, string $password): NotificationResponse|ProfileUpdateResponse + public function updateProfile(User $user): NotificationResponse|ProfileUpdateResponse { if (null === $teamer = $user->getTeamer()) { throw new ApiClientException('Invalid argument'); } + if (null === $user->getBusProAddressId() || null === $user->getBusProPersonId()) { + throw new ApiClientException('User has no BusPro address and person id'); + } + $data = [ 'anfrage' => [ 'user' => $this->config['bpn_username'], @@ -84,8 +97,9 @@ class ApiClient 'satz' => ['@typ' => static::TYPE_CUSTOMER_DATA], 'art' => 'Adressdaten_Ändern', 'email' => $user->getEmail(), - 'passwort' => $password, + 'login' => 'J', 'idadresse' => $user->getBusProAddressId(), + 'idadresseperson' => $user->getBusProPersonId(), 'adressdaten' => $teamer->toPayload(), ], ]; diff --git a/src/Controller/Teamer/Profile/IndexController.php b/src/Controller/Teamer/Profile/IndexController.php index ddcef51..c750628 100644 --- a/src/Controller/Teamer/Profile/IndexController.php +++ b/src/Controller/Teamer/Profile/IndexController.php @@ -4,6 +4,9 @@ namespace App\Controller\Teamer\Profile; use App\BusProNet\ApiClient; use App\BusProNet\ApiClientException; +use App\BusProNet\Model\NotificationResponse; +use App\BusProNet\Model\ProfileUpdateResponse; +use App\BusProNet\ResponseParserException; use App\Entity\Teamer; use App\Entity\Upload; use App\Entity\User; @@ -59,12 +62,7 @@ class IndexController extends AbstractController $form->handleRequest($request); if ($form->isSubmitted() && $form->isValid()) { - try { - $bpnPassword = $request->getSession()->get('bpn_password'); - $this->apiClient->updateProfile($user, $bpnPassword); - $this->entityManager->flush(); - } catch (ApiClientException $e) { - } + $this->updateBusProProfile($user); // Saving the full profile form is a stronger statement than ticking the // confirmation box, so it clears the verification check too. @@ -88,6 +86,44 @@ class IndexController extends AbstractController ]); } + /** + * The local save does not depend on BusPro: a failed write is logged, and the teamer's data + * is kept here either way. + */ + private function updateBusProProfile(User $user): void + { + $context = [ + 'user_id' => $user->getId(), + 'bus_pro_address_id' => $user->getBusProAddressId(), + 'bus_pro_person_id' => $user->getBusProPersonId(), + ]; + + if (null === $user->getBusProAddressId() || null === $user->getBusProPersonId()) { + $this->logger->warning('Skip BusPro profile update: user has no BusPro IDs', $context); + + return; + } + + try { + $response = $this->apiClient->updateProfile($user); + } catch (ApiClientException|ResponseParserException $e) { + $this->logger->error('BusPro profile update failed', $context + [ + 'error_message' => $e->getMessage(), + ]); + + return; + } + + if ($response instanceof NotificationResponse && false === $response->isSuccessful()) { + $this->logger->error('BusPro rejected profile update', $context + [ + 'bpn_code' => $response->getCode(), + 'bpn_message' => $response->getMessage(), + ]); + } elseif ($response instanceof ProfileUpdateResponse && false === $response->isUpdated()) { + $this->logger->error('BusPro did not update profile', $context); + } + } + private function updatePhoto(User $user, UploadSessionDto $uploadSession): void { $teamer = $user->getTeamer(); diff --git a/tests/BusProNet/ApiClientTest.php b/tests/BusProNet/ApiClientTest.php new file mode 100644 index 0000000..67c4d71 --- /dev/null +++ b/tests/BusProNet/ApiClientTest.php @@ -0,0 +1,129 @@ +capturePayload(fn (ApiClient $client) => $client->updateProfile($this->user(addressId: 141747, personId: 224526))); + + $this->assertSame('Adressdaten_Ändern', $payload['art']); + $this->assertSame('J', $payload['login']); + $this->assertSame(141747, $payload['idadresse']); + $this->assertSame(224526, $payload['idadresseperson']); + $this->assertArrayNotHasKey('passwort', $payload); + $this->assertSame( + ['user', 'key', 'satz', 'art', 'email', 'login', 'idadresse', 'idadresseperson', 'adressdaten'], + array_keys($payload), + ); + } + + /** + * The ids are the whole authorisation of the write, so a user missing one is refused before + * anything is sent. + */ + public function testUpdateProfileRefusesAUserWithoutBothIds(): void + { + $serializer = $this->createMock(SerializerInterface::class); + $serializer->expects($this->never())->method('serialize'); + + $this->expectException(ApiClientException::class); + + $this->client($serializer)->updateProfile($this->user(addressId: 141747, personId: null)); + } + + /** + * @param callable(ApiClient): mixed $request + */ + private function capturePayload(callable $request): array + { + $captured = null; + + $serializer = $this->createMock(SerializerInterface::class); + $serializer + ->expects($this->once()) + ->method('serialize') + ->willReturnCallback(function (mixed $data) use (&$captured): never { + $captured = $data; + + throw new \LogicException('Request captured'); + }) + ; + + try { + $request($this->client($serializer)); + } catch (\LogicException) { + } + + $this->assertIsArray($captured); + + return $captured['anfrage']; + } + + private function client(SerializerInterface $serializer): ApiClient + { + return new ApiClient( + $serializer, + $this->createStub(ResponseParser::class), + new NullLogger(), + $this->createStub(FilesystemOperator::class), + [ + 'bpn_username' => 'api', + 'bpn_password' => 'secret', + 'bpn_api_ip' => '127.0.0.1', + 'bpn_api_port' => 1, + ], + ); + } + + private function user(?int $addressId, ?int $personId): User + { + $teamer = new Teamer(); + $teamer + ->setFirstName('Anna') + ->setLastName('Muster') + ->setGender('F') + ->setAddress(new Address()) + ->setCommunication(new Communication()) + ; + + $user = new User(); + $user + ->setEmail('anna@example.org') + ->setTeamer($teamer) + ; + + if (null !== $addressId) { + $user->setBusProAddressId($addressId); + } + + if (null !== $personId) { + $user->setBusProPersonId($personId); + } + + return $user; + } +}