From 989b132c6ab0b6f1c2d58480359802b6aafe2776 Mon Sep 17 00:00:00 2001 From: simoleo89 Date: Tue, 19 May 2026 19:45:19 +0200 Subject: [PATCH] fix(hooks): useHasPermission must distinguish ALLOWED from ROOM_OWNER MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The permission map shipped over the wire carries both PermissionSetting.ALLOWED (value 1) and PermissionSetting.ROOM_OWNER (value 2). Server-side, `Habbo.hasPermission(key)` calls `Rank.hasPermission(key, isRoomOwner=false)`, whose implementation at Rank.java:120 is: setting == ALLOWED || (setting == ROOM_OWNER && isRoomOwner) So a permission whose rank value is ROOM_OWNER is only granted when the caller is the active room owner — Habbo.hasPermission(key) with the default `false` therefore returns false for ROOM_OWNER entries. The previous useHasPermission implementation (`> 0`) treated ROOM_OWNER as unconditionally true, which would let a UI gate light up even when the server would refuse the action. Real example from the default seed: `acc_closedice_room` is ROOM_OWNER for rank_1..6 and ALLOWED only for rank_7 — under `> 0` the predicate was true for every rank, diverging from the server behaviour. Tighten useHasPermission to `=== 1` (ALLOWED only). For the genuine "this is a ROOM_OWNER permission, combine with room session" scenarios, code reaches for usePermissionValue(key) and checks `=== 2 && roomSession.isRoomOwner` explicitly. None of the 11 migrated consumers are affected by the tightening: the keys they use (acc_supporttool / acc_anyroomowner / acc_catalogfurni / acc_calendar_force / acc_staff_pick / acc_ambassador) are all ALLOWED-only in the default seed. Test refresh: - useHasPermission('acc_supporttool') (value 1) stays true. - useHasPermission('acc_anyroomowner') with value 2 in the mock flips from true to false — the new contract. - Other cases unchanged. Verification: yarn typecheck clean, yarn lint:hooks clean, yarn test 214/214. --- .../session/useSessionSnapshots.test.tsx | 19 +++++++--- src/hooks/session/useSessionSnapshots.ts | 35 +++++++++++++------ 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/src/hooks/session/useSessionSnapshots.test.tsx b/src/hooks/session/useSessionSnapshots.test.tsx index 1905fd8..bf61ad2 100644 --- a/src/hooks/session/useSessionSnapshots.test.tsx +++ b/src/hooks/session/useSessionSnapshots.test.tsx @@ -214,16 +214,25 @@ describe('useHasPermission + usePermissionValue + useUserPermissions', () => }); }); - it('useHasPermission returns true for any non-zero value, false for absent/zero', () => + it('useHasPermission returns true only for ALLOWED (value 1), false for ROOM_OWNER/absent/zero', () => { permissionsSnapshot = new Map([ - [ 'acc_supporttool', 1 ], - [ 'acc_anyroomowner', 2 ], - [ 'acc_closedice_room', 0 ] + [ 'acc_supporttool', 1 ], // ALLOWED + [ 'acc_anyroomowner', 2 ], // ROOM_OWNER — requires room ownership at call time + [ 'acc_closedice_room', 0 ] // DISALLOWED (shouldn't reach the client, but defensive) ]); + // ALLOWED → true. Matches Habbo.hasPermission(key) which calls + // Rank.hasPermission(key, false) → only ALLOWED short-circuits. expect(renderHook(() => useHasPermission('acc_supporttool')).result.current).toBe(true); - expect(renderHook(() => useHasPermission('acc_anyroomowner')).result.current).toBe(true); + + // ROOM_OWNER → false. The server-side check requires the + // caller to pass isRoomOwner=true, which the client doesn't + // have ambiently. Code that needs to combine this with the + // active room session should call usePermissionValue(key) and + // check === 2 alongside roomSession.isRoomOwner. + expect(renderHook(() => useHasPermission('acc_anyroomowner')).result.current).toBe(false); + expect(renderHook(() => useHasPermission('acc_closedice_room')).result.current).toBe(false); expect(renderHook(() => useHasPermission('acc_unknown_key')).result.current).toBe(false); }); diff --git a/src/hooks/session/useSessionSnapshots.ts b/src/hooks/session/useSessionSnapshots.ts index b512403..8305a64 100644 --- a/src/hooks/session/useSessionSnapshots.ts +++ b/src/hooks/session/useSessionSnapshots.ts @@ -192,24 +192,39 @@ export const useUserPermissions = (): ReadonlyMap => /** * Reactive predicate: does the current user have the named - * permission (ALLOWED or ROOM_OWNER)? `key` must match a row in - * `permission_definitions.permission_key` (e.g. `'acc_supporttool'`, - * `'acc_anyroomowner'`, `'acc_catalogfurni'`). Prefer this over any - * rank-based gate — it survives rank renumbering and adding new - * ranks without touching the React code. + * permission **unconditionally** (ALLOWED only)? `key` must match a + * row in `permission_definitions.permission_key` (e.g. + * `'acc_supporttool'`, `'acc_anyroomowner'`, `'acc_catalogfurni'`). + * + * Mirrors the server-side semantics of `Habbo.hasPermission(key)` + * (PermissionsManager → `Rank.hasPermission(key, isRoomOwner=false)` + * which returns `setting == ALLOWED` — `setting == ROOM_OWNER` + * requires the call site to pass `isRoomOwner=true`, which the + * client doesn't have ambiently). So this hook returns `true` + * only for ALLOWED (value 1) and `false` for ROOM_OWNER (value 2) + * — the latter has to be re-checked against the active room + * ownership via `usePermissionValue(key) === 2 && roomSession.isRoomOwner`. + * + * Prefer this over any rank-based gate — it survives rank + * renumbering and adding new ranks without touching the React code. */ export const useHasPermission = (key: string): boolean => { const permissions = useUserPermissions(); - return useMemo(() => (permissions.get(key) ?? 0) > 0, [ permissions, key ]); + return useMemo(() => permissions.get(key) === 1, [ permissions, key ]); }; /** - * Reactive raw permission value (1 = ALLOWED, 2 = ROOM_OWNER, 0 if - * absent). Useful for the handful of permissions whose - * `permission_definitions.max_value > 1` (e.g. - * `acc_closedice_room`) where the precise value matters. + * Reactive raw permission value: + * 0 = DISALLOWED (also returned for absent keys) + * 1 = ALLOWED + * 2 = ROOM_OWNER (granted only when the caller is the active + * room owner — combine with `roomSession.isRoomOwner`) + * + * Use this when the gate needs to distinguish ROOM_OWNER from + * plain ALLOWED, or for the handful of permissions whose + * `permission_definitions.max_value > 1`. */ export const usePermissionValue = (key: string): number => {