IDOR: GET contactpersonen returns any organisation's contacts plus their Nextcloud usernames and groups to any logged-in user

[rubenvdlinde]

The endpoint

SettingsController is not involved — this is ContactpersonenController::getContactpersonen(), routed as

['name' => 'contactpersonen#getContactpersonen', 'url' => '/api/contactpersonen/organisation/{organisationId}', 'verb' => 'GET']

/**
 * @NoAdminRequired
 * @NoCSRFRequired
 */
public function getContactpersonen(string $organisationId): JSONResponse
{
    if ($this->userSession->getUser() === null) {
        return new JSONResponse(['message' => 'Not authenticated'], Http::STATUS_UNAUTHORIZED);
    }
    …
    $searchParams = [
        'organisation' => $organisationId,   // <- straight from the caller
        '_limit'       => 100,
        '_schema'      => 'contactpersoon',
    ];
    $contactpersonen = $objectService->searchObjectsPaginated($searchParams);

The only guard is "is anybody logged in". $organisationId is a path parameter and is never checked against the caller's own organisation, so any authenticated user can read the contact persons of any organisation by changing the id in the URL.

What comes back

Per contact, the response carries the OpenRegister object plus enriched Nextcloud account data:

'user' => [
    'hasUser'  => …,
    'username' => $username,
    'groups'   => array_map(fn ($g) => $g->getGID(), $userGroups),
    'disabled' => ($user->isEnabled() === false),
]

Username, full group membership, and enabled/disabled state — for every contact of an arbitrary organisation. That is an account-enumeration surface, not just a data read.

The sibling seam says this is wrong

Every method in the same controller that touches another user's account data guards with isAdmin:

method	annotation	guard
convertToUser	(none = admin)	isAdmin
changePassword	@NoAdminRequired	isAdmin
updateUserGroups	@NoAdminRequired	isAdmin + group
getUserInfo	@NoAdminRequired	isAdmin
getBulkUserInfo	@NoAdminRequired	isAdmin
disableUser / enableUser	(none = admin)	isAdmin
getContactpersonen	@NoAdminRequired	authenticated only

getUserInfo and getBulkUserInfo return the same shape of user data and are admin-gated. getContactpersonen returns it for a caller-chosen organisation and is not.

Two more in the same class are authenticated-only and worth the same look: getContactPersonsWithUserDetailsForOrganization and getAvailableGroups — the latter returns the instance's group list, which SettingsController::getAllGroups() (admin-gated, see #456) refuses to non-admins.

A second, independent problem with the same call

The filter is a bare top-level 'organisation' => $organisationId. In OpenRegister, metadata filters must be nested under @self; a bare key is treated as an object-property filter. Depending on whether organisation is a real property on contactpersoon or only @self.organisation metadata, this query either scopes by property (pure IDOR) or does not scope at all. The comment above it — "Use a more generic search that doesn't require specific register/schema" — says the register and schema are deliberately unpinned too.

That is the shape opencatalogi#828 turned out to be: "a filter is present" is not "the filter scopes", and an empty/ineffective filter and a scoping filter are indistinguishable in the source.

Why no gate caught it

• gate-7 (no-admin-idor) accepts getUser() === null as an authorization guard and passes.
• gate-9 (semantic-auth) finds no mismatch: @NoAdminRequired with no admin check in the body is self-consistent.

Same blind spot as #456's finding — the defect is in the relationship between the annotation, the caller-supplied id, and what a sibling method already refuses.

Not fixed here, deliberately

The correct policy — whether an ambtenaar, an organisation admin, or only an instance admin may read another organisation's contacts — is a product decision, and the app has all three vocabularies in play (isUserInGroup('ambtenaar') in AangebodenGebruikController, getOrganizationAdminGroups() in SettingsService, groupManager->isAdmin() here). Guessing wrong breaks the legitimate surface, and #456 is the case study for why the obvious move is not always the right one. Filing with the evidence rather than shipping a guess.

Suggested shape when it is picked up: resolve the caller's own organisation, allow when it matches $organisationId, allow isAdmin and whatever elevated group the product intends, refuse otherwise — and pin the register/schema so the search cannot degrade to an unscoped query. The can-fail proof should assert on the ITEM (a username from another organisation absent from the body), not on the envelope.

[rubenvdlinde]

Addendum: the empty-string scope value

Confirmed against this method after the same shape was found in portaliq (#65) and opencatalogi (#828).

$organisationId is typed string, has no emptiness check anywhere in the method, and flows unchanged into the filter:

public function getContactpersonen(string $organisationId): JSONResponse
{
    …
    $searchParams = [
        'organisation' => $organisationId,   // '' is a perfectly valid value here
        '_limit'       => 100,
        '_schema'      => 'contactpersoon',
    ];

"" is the value that defeats every idiom this codebase uses to guard a scope:

• it is not null, so a ?? $default never fires;
• it is not absent, so isset() / empty(...) === false style checks that look for presence pass it through;
• any if ($scope !== '') guard elsewhere would silently disable itself.

So the failure ordering is: an empty scope value → a filter that is present but matches nothing meaningful → combined with the deliberately unpinned register and schema (_schema resolved by name, no register at all), a search whose only remaining constraint is a key that may not scope.

"A filter is present" is not "the filter scopes." An empty filter and a correct filter are indistinguishable in this source, and they produce opposite security outcomes.

Whatever guard is added for the IDOR must therefore also reject an empty $organisationId explicitly and pin the register and schema, rather than relying on the filter key alone.