Skip to content

fix(api): the admin operations need an admin with the session cookie too - #1068

Merged
azarouski merged 1 commit into
developfrom
fix/api-admin-cookie
Oct 28, 2026
Merged

azarouski merged 1 commit into
developfrom
fix/api-admin-cookie

Conversation

@azarouski

Copy link
Copy Markdown
Contributor

Problem

lib/units/api/helpers/securityHandlers.ts checks the admin tag of an operation only for an access token (Authorization: Bearer). The other branch, the session cookie of the app, sets req.user without any check, and the controllers do not check the privilege either (only metrics). So any signed-in user can call the 26 admin operations from the browser, e.g.:

  • GET /api/v1/users/<admin email>/accessTokens → the ids of the tokens of the admin, which are the bearer secrets (STF_TOKEN of mcloud-grid too);
  • DELETE /users/{email}, DELETE /devices, PUT /devices/{serial} (status), the origin groups of the devices.

This is the upstream code (DeviceFarmer/stf).

Changes

  • forbidsUser() (fork lines): the same 403 for a user on an admin operation with the cookie as with a token; an operation without tags is not admin.
  • ZEBRUNNER.md: REST API.

The UI of a user calls none of these operations (GET /users, GET /users/{email}, GET /users/alertMessage are not admin; the origin groups are managed by admins).

Testing

  • test/units/zebrunner/api-admin.mts (7): every operation of api_v1.yaml tagged admin (found there, so a new one is covered): 403 for a user with a session cookie and with an access token, an admin passes with both; a user passes every other operation with both. Without the fix the cookie test fails.
  • npm run lint, npm run test:unit: 498 passing.

The security handler of the API checked the `admin` tag of an operation for an
access token only: any signed-in user could call the 26 admin operations from
the browser with the session cookie, e.g. read the access tokens of the admin
(GET /users/{email}/accessTokens returns their ids, the bearer secrets,
STF_TOKEN of the grid too), remove users and devices or change their status.

Both ways of the authentication now refuse a user (403) for them.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

❌ STF CI report

5 failing  ·  6 test tiers (6 pass / 0 fail / 0 skip)  ·  10 Android versions (10 with a usable device in STF)  ·  6 iOS versions (1 passing)

Test tiers

Tier Result
✅ Build (npm + webpack) deps cache miss
✅ Lint (eslint + jsonlint + translations + workflow shell) lib/, res/ and root JS + JSON, translation catalogs, and every workflow run: block
✅ Unit tests (mocha) lib/util, lib/wire and lib/db specs
✅ Component tests (vitest + React) 104 specs, 0 failed, 0 errored
✅ Integration + device-less E2E (stf local + RethinkDB + Playwright + SeaweedFS) stf local success, fake devices success, web UI suite success, storage success
✅ Docker Compose + AVD + Playwright (Android 16, API 36) 21 Playwright tests, image success, Compose/AVD success, teardown success

Android device matrix

Android API Image Boot In STF Usable SIM IDs Screen Touch Shell UI E2E Teardown
✅ 12 31 ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅
✅ 12L 32 ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅
✅ 13 33 ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅
✅ 14 34 ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅
✅ 15 35 ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅
✅ 16 36 ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅
✅ 16.1 36.1 ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅
✅ 17 37.0 ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅
✅ 17 37.1 ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅
✅ 17 37.2 ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅

Image = a system image is published for that API and arch. Boot = emulator reached sys.boot_completed and answers adb. In STF = the device registered with the STF provider. Usable = STF offered it as Use and handed over control. SIM IDs = imei, imsi, iccid and phoneNumber landed on the device document, which from API 23 up needs an STFService build whose agent answers --telephony (a leg whose emulator reports no usable SIM records a pass, because there is nothing to read). Screen = minicap frames reached the browser canvas and kept changing. Touch = a browser tap and swipe showed up in getevent on the device, at the coordinates they were aimed at: the tap within 5% of the centre of the touch axes, the swipe descending monotonically from 0.75 to 0.25 (the emulator runs with adb root, which minitouch needs to open /dev/input). Shell = the dashboard shell widget ran a command on it. UI E2E = the rest of the Playwright web UI suite. Teardown = the emulator action shut the AVD down and exited on its own.

iOS simulator matrix

iOS Runtime Xcode Simulator WDA Session Appium kept Size Orientation Home Touch Screenshot Settings Clipboard URL Stream Stable
❌ 15 15.5 16.4 ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ❌ ➖ ✅ ✅
❌ 16 16.4 16.4 ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ❌ ❌ ➖ ✅ ✅
❌ 17 17.5 16.4 ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ❌ ✅ ✅ ✅
❌ 18 18.6 16.4 ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ❌ ✅ ✅ ✅
✅ 26 26.5 26.6 ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅
❌ 27 27.1 27.1 ✅ ❌ ➖ ➖ ➖ ➖ ➖ ➖ ➖ ➖ ➖ ➖ ➖ ➖

WebDriverAgent on an iPhone simulator against the WDA client of the ios-device unit (test/zebrunner-ios). A red column: Simulator, the simulator did not boot · WDA, WebDriverAgent did not start · Session, no WDA session · Appium kept, the session of an Appium test was not kept · Size, wrong screen size · Orientation, no orientation or battery · Home, the home button did not work · Touch, tap or swipe failed · Screenshot, no screenshot · Settings, Settings did not open or the app was lost · Clipboard, the pasteboard was not written or read · URL, Safari did not open the address · Stream, no MJPEG frame · Stable, the unit would have stopped

Failures

  • iOS 15: the pasteboard was not written or read: expected 'com.facebook.WebDriverAgentRunner.xct…' to equal 'com.apple.springboard'
  • iOS 16: Settings did not open or the app was lost: expected 'Error: ESOCKETTIMEDOUT' to equal 'Settings has no Wi-Fi item'
  • iOS 17: the pasteboard was not written or read: expected 'com.facebook.WebDriverAgentRunner.xct…' to equal 'com.apple.springboard'
  • iOS 18: the pasteboard was not written or read: expected 'com.facebook.WebDriverAgentRunner.xct…' to equal 'com.apple.springboard'
  • iOS 27: WebDriverAgent did not start

Logs, screenshots, logcat and Playwright traces are attached to the run as artifacts · full run · commit bd8c5d1

@azarouski
azarouski merged commit d17c524 into develop Oct 28, 2026
23 of 29 checks passed
@azarouski
azarouski deleted the fix/api-admin-cookie branch October 28, 2026 12:55
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.

1 participant