notify: a member could list their phones but not register one
`personal` is matched against what remains after the permission's own prefix is stripped, so the entries had to be the sidecar's real paths. they were written '/devices' and '/subscriptions'; the sidecar serves '/_officer/devices'. nothing matched, so read level permitted the GET (a safe method, allowed anyway) and refused the POST and the DELETE — a member could see their registered devices and neither add nor remove one. that is the exact opposite of the comment sitting above it, which says device registration is the caller's own. invisible so far because the owner short-circuits every check and no member has ever been granted notify. found while writing the app-side spec: the mobile client posts to /api/notify/_officer/devices, which is what made the mismatch visible. the regression test pins the real path and fails against the old value — checked, not assumed.
This commit is contained in:
@@ -283,3 +283,18 @@ describe('kinds', () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
test('notify: a read-level member can register and deregister their own device', () => {
|
||||
// Regression. `personal` was ['/devices'], but the sidecar serves '/_officer/devices' — and `personal`
|
||||
// is matched against the path AFTER the permission's own prefix is stripped, so it matched nothing.
|
||||
// A member could GET their devices and neither add nor remove one, which is the opposite of the intent.
|
||||
const notify = permissionForApiPath('/api/notify/_officer/devices');
|
||||
expect(notify?.key).toBe('notify');
|
||||
|
||||
expect(isRequestAllowedAtLevel(notify!, 'read', 'POST', '/api/notify/_officer/devices')).toBe(true);
|
||||
expect(isRequestAllowedAtLevel(notify!, 'read', 'DELETE', '/api/notify/_officer/devices/tok')).toBe(true);
|
||||
expect(isRequestAllowedAtLevel(notify!, 'read', 'GET', '/api/notify/_officer/devices')).toBe(true);
|
||||
|
||||
// Still scoped: read level does not become write level for the rest of the surface.
|
||||
expect(isRequestAllowedAtLevel(notify!, 'read', 'POST', '/api/notify/_officer/notify')).toBe(false);
|
||||
});
|
||||
|
||||
@@ -169,7 +169,12 @@ const CORE_REGISTRY: Permission[] = [
|
||||
kind: 'app',
|
||||
api: ['/notify'],
|
||||
// Device registration is the caller's own — a phone subscribing to its own push channel.
|
||||
personal: ['/devices', '/subscriptions'],
|
||||
//
|
||||
// The `_officer/` segment is NOT decoration: these are the sidecar's paths, and `personal` is matched
|
||||
// against what remains after the permission's own prefix is stripped. Written as `/devices` it matched
|
||||
// nothing, so a read-level member could list their phones but not register or delete one — the exact
|
||||
// opposite of the line above. Invisible in testing because the owner bypasses every check.
|
||||
personal: ['/_officer/devices', '/_officer/subscriptions'],
|
||||
},
|
||||
// `vpn` (POST /api/vpn/enroll) was here until 2026-08-14 — the one member-grantable piece of headscale,
|
||||
// minting a pre-auth key bound to the caller. Deleted because it had no caller anywhere: the standalone
|
||||
|
||||
Reference in New Issue
Block a user