feat: update profile in buspro without the teamer's password

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(),
             ],
         ];

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();

tests/BusProNet/ApiClientTest.php

@@ -0,0 +1,129 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\BusProNet;
+
+use App\BusProNet\ApiClient;
+use App\BusProNet\ApiClientException;
+use App\BusProNet\ResponseParser;
+use App\Entity\Embeddable\Address;
+use App\Entity\Embeddable\Communication;
+use App\Entity\Teamer;
+use App\Entity\User;
+use League\Flysystem\FilesystemOperator;
+use PHPUnit\Framework\TestCase;
+use Psr\Log\NullLogger;
+use Symfony\Component\Serializer\SerializerInterface;
+
+/**
+ * Pins the request payloads. The serializer is the last step before the socket, so the payload is
+ * captured there and the request aborted - no connection is ever opened.
+ */
+class ApiClientTest extends TestCase
+{
+    /**
+     * Without the password: "login" switches BusPro's password check off and the two ids identify
+     * the person. The element order is the one verified on the BusPro test system.
+     */
+    public function testUpdateProfileIdentifiesThePersonByIdInsteadOfPassword(): void
+    {
+        $payload = $this->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('[email protected]')
+            ->setTeamer($teamer)
+        ;
+
+        if (null !== $addressId) {
+            $user->setBusProAddressId($addressId);
+        }
+
+        if (null !== $personId) {
+            $user->setBusProPersonId($personId);
+        }
+
+        return $user;
+    }
+}