From feb75daec02092f6a394c2f5bb28c5ff858e5ccd Mon Sep 17 00:00:00 2001 From: Tobias Gross-Vogt Date: Wed, 11 Sep 2024 10:30:10 +0200 Subject: [PATCH] Remove reference to UserSession to get the current user identifier and use PersonProvider::getCurrentPerson directly instead --- CHANGELOG.md | 5 ++++ composer.json | 2 +- composer.lock | 18 +++++++------- src/Service/TuitionFeeService.php | 21 +++++++---------- tests/TuitionFeeServiceTest.php | 39 ++++++++++++++++++++++--------- 5 files changed, 51 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3740531..a616026 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## v0.2.5 + +* Remove reference to UserSession to get the current user identifier and use +PersonProvider::getCurrentPerson directly instead + ## v0.2.4 * Fix compatibility with the latest CAMPUSonline release, resulting in authentication diff --git a/composer.json b/composer.json index 0248b04..7fcc15d 100644 --- a/composer.json +++ b/composer.json @@ -7,7 +7,7 @@ "php": ">=8.1", "ext-json": "*", "api-platform/core": "^2.7.11 || ^3.2", - "dbp/relay-base-person-bundle": "^0.2.27", + "dbp/relay-base-person-bundle": "^0.2.33", "dbp/relay-core-bundle": "^0.1.148", "dbp/relay-mono-bundle": "^0.4.10", "guzzlehttp/guzzle": "^7.4", diff --git a/composer.lock b/composer.lock index ca28a1e..c623fbc 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "c58a74a921c7a03cadc1473a72fa4346", + "content-hash": "1bced0cf84135a8b74e73ee56830f656", "packages": [ { "name": "api-platform/core", @@ -196,21 +196,21 @@ }, { "name": "dbp/relay-base-person-bundle", - "version": "v0.2.31", + "version": "v0.2.33", "source": { "type": "git", "url": "https://github.com/digital-blueprint/relay-base-person-bundle.git", - "reference": "4f5812af9997609ef7cf65408be347f8eeffaeb5" + "reference": "9f00b3bc3c8bb75a66aca0fe448d8424ec49cc5f" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/digital-blueprint/relay-base-person-bundle/zipball/4f5812af9997609ef7cf65408be347f8eeffaeb5", - "reference": "4f5812af9997609ef7cf65408be347f8eeffaeb5", + "url": "https://api.github.com/repos/digital-blueprint/relay-base-person-bundle/zipball/9f00b3bc3c8bb75a66aca0fe448d8424ec49cc5f", + "reference": "9f00b3bc3c8bb75a66aca0fe448d8424ec49cc5f", "shasum": "" }, "require": { "api-platform/core": "^2.7.11 || ^3.2", - "dbp/relay-core-bundle": "^0.1.171", + "dbp/relay-core-bundle": "^0.1.181", "ext-json": "*", "php": ">=8.1", "symfony/config": "^5.4 || ^6.4", @@ -244,9 +244,9 @@ ], "support": { "issues": "https://github.com/digital-blueprint/relay-base-person-bundle/issues", - "source": "https://github.com/digital-blueprint/relay-base-person-bundle/tree/v0.2.31" + "source": "https://github.com/digital-blueprint/relay-base-person-bundle/tree/v0.2.33" }, - "time": "2024-07-10T14:12:10+00:00" + "time": "2024-09-11T08:27:02+00:00" }, { "name": "dbp/relay-core-bundle", @@ -11325,5 +11325,5 @@ "platform-overrides": { "php": "8.1" }, - "plugin-api-version": "2.6.0" + "plugin-api-version": "2.2.0" } diff --git a/src/Service/TuitionFeeService.php b/src/Service/TuitionFeeService.php index 5b7d46e..f37ec05 100644 --- a/src/Service/TuitionFeeService.php +++ b/src/Service/TuitionFeeService.php @@ -5,7 +5,6 @@ namespace Dbp\Relay\MonoConnectorCampusonlineBundle\Service; use Dbp\Relay\BasePersonBundle\API\PersonProviderInterface; -use Dbp\Relay\CoreBundle\API\UserSessionInterface; use Dbp\Relay\CoreBundle\Exception\ApiError; use Dbp\Relay\CoreBundle\Rest\Options; use Dbp\Relay\MonoBundle\ApiPlatform\Payment; @@ -28,7 +27,6 @@ class TuitionFeeService implements BackendServiceInterface, LoggerAwareInterface public const PERSON_TITLE_LOCAL_DATA_ATTRIBUTE = 'title'; - private UserSessionInterface $userSession; private TranslatorInterface $translator; private LoggerInterface $auditLogger; private PersonProviderInterface $personProvider; @@ -42,12 +40,10 @@ class TuitionFeeService implements BackendServiceInterface, LoggerAwareInterface public function __construct( TranslatorInterface $translator, - UserSessionInterface $userSession, PersonProviderInterface $personProvider, ConfigurationService $config, ) { $this->translator = $translator; - $this->userSession = $userSession; $this->logger = new NullLogger(); $this->auditLogger = new NullLogger(); $this->personProvider = $personProvider; @@ -113,19 +109,18 @@ public function updateData(PaymentPersistence $paymentPersistence): bool || $payment->getDataUpdatedAt() <= $updateExpiration ) { $this->auditLogger->debug('CO: Updating the payment data', $this->getLoggingContext($payment)); - $userIdentifier = $this->userSession->getUserIdentifier(); - if ($userIdentifier === null) { - throw new ApiError(Response::HTTP_UNAUTHORIZED, 'No user identifier!'); - } $personProviderOptions = []; - $person = $this->personProvider->getPerson($userIdentifier, + $currentPerson = $this->personProvider->getCurrentPerson( Options::requestLocalDataAttributes($personProviderOptions, [self::PERSON_TITLE_LOCAL_DATA_ATTRIBUTE])); + if ($currentPerson === null) { + throw new ApiError(Response::HTTP_FORBIDDEN, 'Forbidden'); + } - $payment->setLocalIdentifier($person->getIdentifier()); - $payment->setGivenName($person->getGivenName()); - $payment->setFamilyName($person->getFamilyName()); - $payment->setHonorificSuffix($person->getLocalDataValue(self::PERSON_TITLE_LOCAL_DATA_ATTRIBUTE)); + $payment->setLocalIdentifier($currentPerson->getIdentifier()); + $payment->setGivenName($currentPerson->getGivenName()); + $payment->setFamilyName($currentPerson->getFamilyName()); + $payment->setHonorificSuffix($currentPerson->getLocalDataValue(self::PERSON_TITLE_LOCAL_DATA_ATTRIBUTE)); $api = $this->getApiByType($payment->getType(), $payment); $obfuscatedId = $payment->getLocalIdentifier(); diff --git a/tests/TuitionFeeServiceTest.php b/tests/TuitionFeeServiceTest.php index e934920..a927266 100644 --- a/tests/TuitionFeeServiceTest.php +++ b/tests/TuitionFeeServiceTest.php @@ -4,10 +4,8 @@ namespace Dbp\Relay\MonoConnectorCampusonlineBundle\Tests; -use Dbp\Relay\BasePersonBundle\Entity\Person; use Dbp\Relay\BasePersonBundle\Service\DummyPersonProvider; use Dbp\Relay\CoreBundle\Exception\ApiError; -use Dbp\Relay\CoreBundle\TestUtils\TestUserSession; use Dbp\Relay\MonoBundle\ApiPlatform\Payment; use Dbp\Relay\MonoBundle\Persistence\PaymentPersistence; use Dbp\Relay\MonoBundle\Persistence\PaymentStatus; @@ -25,14 +23,13 @@ class TuitionFeeServiceTest extends KernelTestCase { private TuitionFeeService $tuitionFeeService; + private ?DummyPersonProvider $personProvider = null; + protected function setUp(): void { - $dummyPersonProvider = new DummyPersonProvider(); - $person = new Person(); - $person->setIdentifier('testuser'); - $person->setGivenName('John'); - $person->setFamilyName('Doe'); - $dummyPersonProvider->setCurrentPerson($person); + $this->personProvider = new DummyPersonProvider(); + $this->personProvider->addPerson('testuser', 'John', 'Doe', ['title' => 'Dr.']); + $this->personProvider->setCurrentPersonIdentifier('testuser'); $config = new ConfigurationService(); $config->setConfig([ 'payment_types' => [ @@ -44,8 +41,7 @@ protected function setUp(): void ], ]); $translator = self::getContainer()->get(TranslatorInterface::class); - $this->tuitionFeeService = new TuitionFeeService($translator, - new TestUserSession('testuser'), $dummyPersonProvider, $config); + $this->tuitionFeeService = new TuitionFeeService($translator, $this->personProvider, $config); } private function getAuthResponses(): array @@ -141,7 +137,28 @@ public function testUpdateData(): void $this->assertSame($paymentPersistence->getCurrency(), 'EUR'); $this->assertSame($paymentPersistence->getGivenName(), 'John'); $this->assertSame($paymentPersistence->getFamilyName(), 'Doe'); - $this->assertSame($paymentPersistence->getHonorificSuffix(), 'title'); + $this->assertSame($paymentPersistence->getHonorificSuffix(), 'Dr.'); + } + + public function testUpdateDataNoCurrentPerson(): void + { + $this->mockResponses([ + new Response(200, ['Content-Type' => 'application/json'], '{"amount":300,"semesterKey":"2022S"}'), + ]); + + $paymentPersistence = new PaymentPersistence(); + $paymentPersistence->setIdentifier('test_payment_persistence'); + $paymentPersistence->setType('test_payment_type'); + $paymentPersistence->setData('22S'); + + $this->personProvider->setCurrentPersonIdentifier(null); + + try { + $this->tuitionFeeService->updateData($paymentPersistence); + $this->fail('Expected an ApiError'); + } catch (ApiError $apiError) { + $this->assertSame($apiError->getStatusCode(), HttpResponse::HTTP_FORBIDDEN); + } } public function testUpdateDataAmountToSmall(): void