Skip to content

fix(webservice): judge admin user operations by the API session - #1810

Open
ROB-ernst wants to merge 1 commit into
LibreBooking:developfrom
ROB-ernst:fix/api-session-admin-user-operations
Open

ROB-ernst wants to merge 1 commit into
LibreBooking:developfrom
ROB-ernst:fix/api-session-admin-user-operations

Conversation

@ROB-ernst

Copy link
Copy Markdown

Fixes #1736

Problem

ManageUsersService checks the acting user with ServiceLocator::GetServer()->GetUserSession(), i.e. the PHP
browser session. A REST request authenticated by X-Booked-SessionToken / X-Booked-UserId has no browser session,
so the check sees a NullUserSession and an application administrator is denied:

  • POST /Web/Services/Users/ → 400 "Only application administrators may create users"
    (log: AddUser denied. UserId=0 is not an application administrator)
  • DELETE /Web/Services/Users/{id} → 200 "The item was deleted", but nothing is deleted
    (DeleteUser returns early on the same check)
  • POST /Web/Services/Users/{id} (update), password and attribute changes are affected the same way

The admin-only route check in WebServiceSecurity passes, so this is only the second check inside the service.
Reproduced on 7.0.0 (Docker image librebooking/librebooking:7.0.0-3.5.1); the code is unchanged on develop.

A related point: ServiceLocator::GetUserSession() already knows the API server, but it returns the browser session
first. A REST call that also carries a browser cookie of another user would therefore be judged by that user.

Change

  • ServiceLocator::GetUserSession() returns the authenticated API session first during a web service request, then
    the browser session, else null. Outside the API (no API server registered) nothing changes.
  • ManageUsersService uses it (via a private CurrentUser() that falls back to NullUserSession) for all six admin
    checks.
  • Nothing is written to the PHP session for API calls (setting the browser session from the API session would also
    have worked, but would put an admin session into $_SESSION).
  • ServiceLocator::SetApiServer() accepts null; TestBase::teardown() resets it so tests stay independent.

Tests

  • ManageUsersServiceTest: user creation and deletion succeed for an API admin without a browser session; an API
    non-admin is denied even when the browser session is an admin.
  • New ServiceLocatorTest: precedence of API and browser session, fallback when the API request is not
    authenticated, null without any session.
  • The four new tests fail without the change and pass with it. Full suite: the same set of tests fails before and
    after the change in my environment (missing PHP extensions in the container); phpstan and php-cs-fixer report
    nothing for the changed files.
  • Checked on a 7.0.0 instance with the two files patched: creating a user, booking for that user, and deleting the
    booking and the user through the API all work.

🤖 Generated with Claude Code

https://claude.ai/code/session_013tFERcCHKhG6St25mzFRYq

ManageUsersService checked the browser session
(ServiceLocator::GetServer()->GetUserSession()). A REST request has
none, so an application administrator authenticated by API token was
denied ("Only application administrators may create users", log:
"AddUser denied. UserId=0"), and a browser cookie sent along with an
API request would decide instead of the API token.

ServiceLocator::GetUserSession() now returns the authenticated API
session first during a web service request, and ManageUsersService
uses it for all its admin checks. No PHP session is written for API
calls.

Fixes LibreBooking#1736

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ROB-ernst
ROB-ernst force-pushed the fix/api-session-admin-user-operations branch from a288044 to 4c12f53 Compare October 18, 2026 14:15

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] REST admin user operations use browser session instead of API session

1 participant