Skip to content

Commit

Permalink
Add check if user is authenticated before requesting user identifier,…
Browse files Browse the repository at this point in the history
… to prevent internal exception on

request of current person
  • Loading branch information
tobiasgv committed Sep 11, 2024
1 parent 602c125 commit 8e88e5d
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 27 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Unreleased

# v0.5.7

* Add check if user is authenticated before requesting user identifier, to prevent internal exception on
request of current person

# v0.5.6

* Fixes in case the provider is called in an unauthenticated context like health checks
Expand Down
4 changes: 2 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
"php": ">=8.1",
"ext-json": "*",
"api-platform/core": "^2.7.11 || ^3.2",
"dbp/relay-base-person-bundle": "^0.2.31",
"dbp/relay-core-bundle": "^0.1.180",
"dbp/relay-base-person-bundle": "^0.2.33",
"dbp/relay-core-bundle": "^0.1.181",
"dbp/relay-core-connector-ldap-bundle": "^0.2.5",
"psr/container": "^1.1 || ^2",
"psr/log": "^1.1.4 || ^2.0 || ^3.0",
Expand Down
18 changes: 9 additions & 9 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 10 additions & 14 deletions src/Service/LDAPApi.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class LDAPApi implements LoggerAwareInterface
private const FAMILY_NAME_ATTRIBUTE_KEY = 'familyName';
private const LOCAL_DATA_BASE_PATH = 'localData.';

private ?CacheItemPoolInterface $personCache;
private ?CacheItemPoolInterface $currentPersonCache;
private ?Person $currentPerson = null;
private AttributeMapper $attributeMapper;
private LocalDataEventDispatcher $eventDispatcher;
Expand Down Expand Up @@ -81,7 +81,7 @@ public function assertAttributesExist()

public function setPersonCache(?CacheItemPoolInterface $cachePool): void
{
$this->personCache = $cachePool;
$this->currentPersonCache = $cachePool;
}

/*
Expand Down Expand Up @@ -214,21 +214,16 @@ private function getPersonEntry(string $identifier): LdapEntryInterface
/**
* @thorws ApiError
*/
public function getPerson(string $id, array $options = []): Person
public function getPerson(string $identifier, array $options = []): Person
{
$this->eventDispatcher->onNewOperation($options);

$currentIdentifier = null;
if ($this->userSession->isAuthenticated() && !$this->userSession->isServiceAccount()) {
$currentIdentifier = $this->userSession->getUserIdentifier();
}

if ($currentIdentifier !== null && $currentIdentifier === $id) {
// fast path
// fast path if requested person is the currently logged-in user
if ($this->userSession->isAuthenticated() && $this->userSession->getUserIdentifier() === $identifier) {
$person = $this->getCurrentPersonCached(true);
assert($person !== null);
} else {
$personEntry = $this->getPersonEntry($id);
$personEntry = $this->getPersonEntry($identifier);
$person = $this->personFromLdapEntry($personEntry);
// this should never happen (since we have searched by identifier):
if ($person === null) {
Expand Down Expand Up @@ -283,8 +278,9 @@ private function personFromLdapEntry(LdapEntryInterface $ldapEntry): ?Person
*/
private function getCurrentPersonCached(bool $checkLocalDataAttributes): ?Person
{
$currentIdentifier = $this->userSession->getUserIdentifier();
if ($currentIdentifier === null || $this->userSession->isServiceAccount()) {
if (!$this->userSession->isAuthenticated()
|| $this->userSession->isServiceAccount()
|| ($currentIdentifier = $this->userSession->getUserIdentifier()) === null) {
return null;
}

Expand All @@ -296,7 +292,7 @@ private function getCurrentPersonCached(bool $checkLocalDataAttributes): ?Person
$this->currentPerson = null;
}

$cache = $this->personCache;
$cache = $this->currentPersonCache;
$cacheKey = $this->userSession->getSessionCacheKey().'-'.$currentIdentifier;
// make sure the cache is longer than the session, so just double it.
$cacheTTL = $this->userSession->getSessionTTL() * 2;
Expand Down
4 changes: 2 additions & 2 deletions src/Service/LDAPPersonProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ public function getPersons(int $currentPageNumber, int $maxNumItemsPerPage, arra
*
* @throws ApiError
*/
public function getPerson(string $id, array $options = []): Person
public function getPerson(string $identifier, array $options = []): Person
{
return $this->ldapApi->getPerson($id, $options);
return $this->ldapApi->getPerson($identifier, $options);
}

/**
Expand Down

0 comments on commit 8e88e5d

Please sign in to comment.