[vvshk]

No description provided.

[claude]

Claude finished @vvshk's task in 2m 9s —— View job

---

PR #486 review complete

• Read repo rules (AGENTS.md)
• Review the diff (6 files)
• Post inline comments and a summary

Main concern: kiosk security. A kioskAdmin token can check in or check out any bookingid (comment). It can also read or generate WiFi codes for any card number (comment). Nothing proves the guest is at the kiosk, and the kiosk bypass skips authorizeRoles.

Behavior change to confirm: gate entry no longer checks in pending checkin stays, and gate exit no longer checks out overdue flat stays.

Nit: an unused catch (err) in middleware/AdminAuth.js (comment).

The full summary is in this comment. I did not run lint or tests, because AGENTS.md says they are unavailable in GitHub Actions reviews.

[claude]

Security: a kiosk token can check in or check out any booking, not just the scanned guest's.

This allowlist entry lets a kioskAdmin call PUT /stay/checkin/:bookingid, /checkout/:bookingid, /flat_checkin/:bookingid and /flat_checkout/:bookingid. Those handlers only look up the booking by bookingid. Nothing ties the request to the card scanned at the kiosk. On top of that, req.kioskRoute = true skips every authorizeRoles check.

Kiosks are unattended, shared devices, and their token can be pulled from the client. Anyone who has it can:

1. Loop over card numbers with GET /stay/kiosk/bookings/:cardno to collect names and bookingids.
2. Call PUT /stay/checkout/:bookingid on any checkedin stay. An early checkout runs handleEarlyCheckout: it cancels the booking, refunds the transaction as credits and creates a new one.

Suggested fixes:

• Add kiosk-specific checkin and checkout handlers, or a guard on these routes, that require the scanned cardno and reject the request when booking.cardno !== cardno.
• Rate-limit the /stay/kiosk/* routes, and log req.user.username on each kiosk action so it can be audited.

[claude]

Security: WiFi passwords can be read for any card number.

fetchTempCodesForAdmin returns the WiFi passwords for whatever :cardno is in the URL. Any office, room or super admin can call it, and so can a kiosk-only token, because req.kioskRoute skips authorizeRoles. The kiosk has no proof that the guest is actually at the device. Anyone with the kiosk token can loop over card numbers and collect the passwords of every checked-in guest. generateTempCodeForAdmin has the same problem, and it also uses up the guest's single code (MAX_WIFI_PASS_LIMIT = 1).

If card numbers are sequential or easy to guess, this is easy to exploit. Consider:

• Requiring a second factor from the guest, such as the last 4 digits of their mobile number or a scanned QR or NFC payload, rather than the bare card number.
• Rate-limiting these routes per kiosk account.

[claude]

Nit: err is never used. Also, ApiError treats its third argument as data, and when data is set it skips Error.captureStackTrace. If these strings are only for the client, that's fine. Otherwise, consider logging the real reason (expired or malformed) with req.log before throwing.

Suggested change

	} catch (err) {
	} catch {

[claude]

Review summary

This PR adds self-service kiosk endpoints (booking lookup with a checkout preview, and WiFi code generation and lookup), a kioskAdmin role with an endpoint allowlist, and an overdue_checkouts report. It also stops gate entry and exit from changing room and flat booking status. The code is well structured, and the preview credit matches what cancelTransaction actually credits.

Main concern: kiosk security (see the inline comments). A kioskAdmin token can run checkin and checkout on any bookingid, and can read or generate WiFi codes for any card number. It needs no proof that the guest is at the kiosk, and req.kioskRoute skips authorizeRoles. On a shared, unattended device this lets someone loop over card numbers to collect names, booking IDs and WiFi passwords, and to trigger early checkouts that refund credits. Please tie the actions to the scanned card and add rate limiting before this reaches main.

Behavior change to confirm: removing the hooks from gateManagement.controller.js changes two things:

• Gate entry no longer moves pending checkin stays to checkedin.
• Gate exit no longer checks out overdue flat stays.

Stays now stay in those states until staff or the kiosk acts. The new overdue report covers part of this. The shared business docs in aashray-docs also need updating.

Minor:

• fetchKioskBookingsByCard finds transactions by bookingid only. flatCheckout also filters by category: TYPE_FLAT, so the preview could pick a different row than the real checkout does.
• The overdue_checkouts rows set id: null. Make sure the UI hides the revoke action on that tab.

I did not run lint or tests: per AGENTS.md, they are unavailable in GitHub Actions reviews.

🤖 Generated with Claude Code