Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 58 additions & 7 deletions app/Domain/Calendar/Services/Calendar.php
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,16 @@ public function getIcalByHash(string $userHash, string $calHash): IcalCalendar
return $icalCalendar;
}

/**
* Internal: builds the calendar feed (personal events + ticket due/edit events) for a GIVEN
* user id. Deliberately NOT @api — it trusts the $userId param. The web caller passes the
* session id; RPC callers must use {@see getMyCalendar()}, which pins to the session user.
*
* @param int $userId The user whose calendar to build
* @param null|string|CarbonImmutable $from Optional start of the window
* @param null|string|CarbonImmutable $until Optional end of the window
* @return array<int, array<string, mixed>> FullCalendar-shaped event arrays
*/
public function getCalendar(int $userId, null|string|CarbonImmutable $from = null, null|string|CarbonImmutable $until = null): array
{
// Convert date parameters to Carbon instances if they're strings
Expand Down Expand Up @@ -617,7 +627,44 @@ public function getCalendar(int $userId, null|string|CarbonImmutable $from = nul
return $newValues;
}

public function getICalUrl()
/**
* Session-scoped calendar feed (personal events + ticket due/edit events) for the
* authenticated user. Mobile's calendar tab calls this; the optional from/until window lets
* it fetch just the visible month.
*
* getCalendar() itself is internal-only — it trusts an arbitrary $userId — so this wrapper is
* the API entry point and pins the read to the session user (no spoofable param).
*
* @param null|string|CarbonImmutable $from Optional ISO start of the window
* @param null|string|CarbonImmutable $until Optional ISO end of the window
* @return array<int, array<string, mixed>> FullCalendar-shaped event arrays
*
* @api
*/
#[RequiresPermission(CalendarPermissions::VIEW)]
public function getMyCalendar(null|string|CarbonImmutable $from = null, null|string|CarbonImmutable $until = null): array
{
$userId = $this->currentUserId() ?? 0;
if ($userId === 0) {
return [];
}

return $this->getCalendar($userId, $from, $until);
}
Comment on lines +644 to +653

/**
* The authenticated user's personal iCal subscription URL (hash-authenticated feed). Already
* session-scoped — it takes no userId. Mobile surfaces this so the user can subscribe their
* device calendar.
*
* @return string The full iCal feed URL
*
* @throws MissingParameterException When the user has no iCal feed configured (maps to RPC -32602)
*
* @api
*/
#[RequiresPermission(CalendarPermissions::VIEW)]
public function getICalUrl(): string
{
$userId = -1;
if (! empty(session('userdata.id'))) {
Expand All @@ -628,7 +675,7 @@ public function getICalUrl()
$icalHash = $this->settingsRepo->getSetting('usersettings.'.$userId.'.icalSecret');

if (empty($icalHash)) {
throw new \Exception('User has no ical hash');
throw new MissingParameterException('User has no iCal feed configured');
}

return BASE_URL.'/calendar/ical/'.$icalHash.'_'.$userHash;
Expand Down Expand Up @@ -675,13 +722,17 @@ public function getIcalByRequestToken(string $token, string $act = ''): IcalCale
}

/**
* Gets all events from external calendars for a user
* External-calendar events (from the user's subscribed iCal feeds) for the authenticated user.
* Already session-scoped — it keys off session('userdata.id') with no spoofable param. Mobile
* merges these into its calendar view.
*
* @param int $userId The user ID to get external calendar events for
* @param null|string|CarbonImmutable $from The start date to filter events by
* @param null|string|CarbonImmutable $until The end date to filter events by
* @return array Array of calendar events from all external calendars
* @param null|string|CarbonImmutable $from Optional ISO start of the window
* @param null|string|CarbonImmutable $until Optional ISO end of the window
* @return array<int, array<string, mixed>> Array of external calendar events
*
* @api
*/
#[RequiresPermission(CalendarPermissions::VIEW)]
public function getExternalCalendarEvents(null|string|CarbonImmutable $from = null, null|string|CarbonImmutable $until = null): array
{
$cacheKey = 'calendar.external.'.session('userdata.id');
Expand Down
3 changes: 2 additions & 1 deletion app/Domain/Notifications/Repositories/Notifications.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,11 @@ public function markNotificationRead(int $id, int $userId): bool
->update(['read' => 1]) > 0;
}

public function markNotificationUnread(int $id): bool
public function markNotificationUnread(int $id, int $userId): bool
{
return $this->db->table('zp_notifications')
->where('id', $id)
->where('userId', $userId)
->update(['read' => 0]) > 0;
}

Expand Down
62 changes: 51 additions & 11 deletions app/Domain/Notifications/Services/Notifications.php
Original file line number Diff line number Diff line change
Expand Up @@ -119,25 +119,28 @@ public function markNotificationRead($id, $userId): bool
}

/**
* Flip a previously-read notification back to unread. Powers the
* swipe-to-mark-unread inbox gesture on mobile — symmetric to
* markNotificationRead so users can re-surface something they
* tapped open accidentally or want to come back to.
* Flip a previously-read notification back to unread for the authenticated
* (session) user. Powers the swipe-to-mark-unread inbox gesture on mobile —
* symmetric to markRead() so users can re-surface something they tapped open
* by accident.
*
* Per the mobile-owns-explicit-RPC-params convention, userId is
* passed by the client. Scoping isn't critical here (the id is
* the primary key) but the param keeps audit logs consistent
* with the rest of the Notifications RPC surface.
* Session-scoped: the row is matched on (id, session user), so a caller
* cannot flip another user's notification unread by guessing its id. (The
* id is a global sequence, so an unscoped update would be an IDOR.)
*
* @param int $id The notification id to mark unread
* @return bool True on success
*
* @api
*/
public function markNotificationUnread(int $id, int $userId): bool
public function markNotificationUnread(int $id): bool
{
if ($id <= 0) {
$userId = (int) session('userdata.id');
if ($id <= 0 || $userId === 0) {
return false;
}

return $this->notificationsRepo->markNotificationUnread($id);
return $this->notificationsRepo->markNotificationUnread($id, $userId);
}

/**
Expand All @@ -162,6 +165,43 @@ public function getUnreadCount(): int
return $this->notificationsRepo->getUnreadCount($userId);
}

/**
* Inbox listing for the authenticated (session) user. Mobile's inbox tab
* calls this with {} and gets only its own notifications (newest first).
*
* Session-scoped on purpose: getAllNotifications() takes an arbitrary
* $userId and is deliberately NOT @api-exposed (it would let a caller read
* another user's inbox). This wrapper derives the user from the session,
* exactly like getUnreadCount()/markRead() do, so there is nothing to gate.
*
* (Mark-all-read is already covered by markRead('all').)
*
* Pagination is clamped and the repo's arbitrary column=>value filter
* passthrough is intentionally NOT exposed here — an @api caller gets a
* bounded page of its OWN rows only (no caller-supplied WHERE columns, no
* unbounded limit).
*
* @param int $showNewOnly 1 = only unread notifications, 0 = all
* @param int $limitStart Offset for paging (clamped to >= 0)
* @param int $limitEnd Page size (clamped to 1..100)
* @return array<int, array<string, mixed>> The session user's notifications, or [] when unauthenticated
*
* @api
*/
public function getInbox(int $showNewOnly = 0, int $limitStart = 0, int $limitEnd = 50): array
{
$userId = (int) session('userdata.id');
if ($userId === 0) {
return [];
}

// Clamp pagination so an @api caller cannot request an unbounded page.
$limitStart = max(0, $limitStart);
$limitEnd = min(max(1, $limitEnd), 100);

return $this->notificationsRepo->getAllNotifications($userId, (bool) $showNewOnly, $limitStart, $limitEnd) ?: [];
}

/**
* Register a mobile device's push token for the authenticated user.
* Mobile calls this on every login (idempotent). The push fields
Expand Down
22 changes: 11 additions & 11 deletions app/Domain/Widgets/Hxcontrollers/MyToDos.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,15 @@ public function saveSorting($params)
);

if ($result['successCount'] > 0 && $result['errorCount'] === 0) {
$this->tpl->setNotification($this->language->__('notifications.group_changes_applied'), 'success');
$this->tpl->setNotification($this->language->__('short_notifications.group_changes_applied'), 'success');
} elseif ($result['successCount'] > 0 && $result['errorCount'] > 0) {
$this->tpl->setNotification($this->language->__('notifications.group_changes_partial'), 'warning');
$this->tpl->setNotification($this->language->__('short_notifications.group_changes_partial'), 'warning');
} elseif ($result['errorCount'] > 0) {
$this->tpl->setNotification($this->language->__('notifications.group_changes_failed'), 'error');
$this->tpl->setNotification($this->language->__('short_notifications.group_changes_failed'), 'error');
}

if (! $result['sorted']) {
$this->tpl->setNotification($this->language->__('notifications.sorting_error'), 'error');
$this->tpl->setNotification($this->language->__('short_notifications.sorting_error'), 'error');
}
}

Expand Down Expand Up @@ -117,7 +117,7 @@ public function updateStatus()
if ($result) {
$this->tpl->setNotification($this->language->__('short_notifications.status_updated'), 'success');
} else {
$this->tpl->setNotification($this->language->__('notifications.status_update_error'), 'error');
$this->tpl->setNotification($this->language->__('short_notifications.status_update_error'), 'error');
}
}
}
Expand All @@ -133,9 +133,9 @@ public function updateMilestone()
$result = $this->ticketsService->patch($params['id'], ['milestoneid' => $params['milestoneId']]);

if ($result) {
$this->tpl->setNotification($this->language->__('notifications.milestone_updated'), 'success');
$this->tpl->setNotification($this->language->__('short_notifications.milestone_updated'), 'success');
} else {
$this->tpl->setNotification($this->language->__('notifications.milestone_update_error'), 'error');
$this->tpl->setNotification($this->language->__('short_notifications.milestone_update_error'), 'error');
}
}
}
Expand All @@ -151,9 +151,9 @@ public function updateDueDate()
$result = $this->ticketsService->patch($params['id'], ['dateToFinish' => $params['date']]);

if ($result) {
$this->tpl->setNotification($this->language->__('notifications.date_updated'), 'success');
$this->tpl->setNotification($this->language->__('short_notifications.date_updated'), 'success');
} else {
$this->tpl->setNotification($this->language->__('notifications.date_update_error'), 'error');
$this->tpl->setNotification($this->language->__('short_notifications.date_update_error'), 'error');
}
}
}
Expand All @@ -172,9 +172,9 @@ public function updateTitle($params)
$result = $this->ticketsService->patch($params['id'], ['headline' => $headline]);

if ($result) {
$this->tpl->setNotification($this->language->__('notifications.title_updated'), 'success');
$this->tpl->setNotification($this->language->__('short_notifications.title_updated'), 'success');
} else {
$this->tpl->setNotification($this->language->__('notifications.title_update_error'), 'error');
$this->tpl->setNotification($this->language->__('short_notifications.title_update_error'), 'error');
}

return $this->tpl->displayRaw("{$headline}");
Expand Down
9 changes: 9 additions & 0 deletions app/Language/en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,15 @@ short_notifications.user_updated = "User was updated"
short_notifications.sprint_updated = "Sprint was updated"
short_notifications.due_date_updated = "Due date was updated"
short_notifications.date_updated = "Date was changed"
short_notifications.date_update_error = "Date could not be changed"
short_notifications.status_update_error = "Status could not be updated"
short_notifications.milestone_update_error = "Milestone could not be updated"
short_notifications.title_updated = "Title was updated"
short_notifications.title_update_error = "Title could not be updated"
short_notifications.group_changes_applied = "Changes applied"
short_notifications.group_changes_partial = "Some changes could not be applied"
short_notifications.group_changes_failed = "Changes could not be applied"
short_notifications.sorting_error = "Could not save sort order"

short_notifications.remaining_hours_updated = "Remaining Hours updated!"
short_notifications.planned_hours_updated = "Planned Hours updated!"
Expand Down
64 changes: 64 additions & 0 deletions tests/Acceptance/API/BearerApiCest.php
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,32 @@ public function bearerAuthHonorsGatedReads(AcceptanceTester $I)

// 5) Entity-scoped comments resolve the project from (module, moduleId).
$this->assertRpcSucceeds($I, 'leantime.rpc.Comments.Comments.getComments', ['module' => 'ticket', 'moduleId' => 1]);

// 6) Session-scoped mobile endpoints: no userId param, the caller is resolved from the
// token. These previously 404'd on mobile because the userId-taking originals are
// deliberately NOT @api (IDOR guard); the session-scoped siblings are the fix.
// getInbox is the inbox list (getAllNotifications stays non-@api — it takes a $userId).
$inbox = $this->rpc($I, 'leantime.rpc.Notifications.Notifications.getInbox', new \stdClass);
Assert::assertArrayNotHasKey('error', $inbox, 'getInbox must be exposed + session-scoped: '.json_encode($inbox));
Assert::assertIsArray($inbox['result'] ?? null, 'getInbox should return an array: '.json_encode($inbox));

// getExternalCalendarEvents (subscribed iCal feeds) is already session-scoped; it only
// lacked the @api tag. No external calendars on a fresh install => an empty array.
$extEvents = $this->rpc($I, 'leantime.rpc.Calendar.Calendar.getExternalCalendarEvents', new \stdClass);
Assert::assertArrayNotHasKey('error', $extEvents, 'getExternalCalendarEvents must be exposed: '.json_encode($extEvents));
Assert::assertIsArray($extEvents['result'] ?? null, 'getExternalCalendarEvents should return an array: '.json_encode($extEvents));

// getICalUrl is now exposed too. It legitimately errors with -32602 when the user has no
// iCal feed configured yet, so a valid response is EITHER a URL string OR -32602 — but
// NEVER -32601 (method-not-found regression) or -32001 (permission regression).
$icalUrl = $this->rpc($I, 'leantime.rpc.Calendar.Calendar.getICalUrl', new \stdClass);
$icalErr = $icalUrl['error']['code'] ?? null;
Assert::assertNotSame(-32601, $icalErr, 'getICalUrl must be exposed via @api (not method-not-found): '.json_encode($icalUrl));
Assert::assertNotSame(-32001, $icalErr, 'getICalUrl must not be permission-denied for the owner: '.json_encode($icalUrl));
Assert::assertTrue(
array_key_exists('result', $icalUrl) || $icalErr === -32602,
'getICalUrl should return a URL or -32602 (no feed configured): '.json_encode($icalUrl)
);
}

#[Group('bearer-api')]
Expand Down Expand Up @@ -147,6 +173,44 @@ public function nonManagerBearerHonorsProjectScopedReads(AcceptanceTester $I)
$done = $this->rpc($I, 'leantime.rpc.Tickets.Tickets.markTicketDone', ['id' => $assignedTicketId]);
Assert::assertArrayNotHasKey('error', $done, 'markTicketDone must be exposed + authorized: '.json_encode($done));
Assert::assertTrue($done['result'] ?? false, 'markTicketDone should return true: '.json_encode($done));

// getMyCalendar is the session-scoped calendar feed (getCalendar itself trusts a userId and
// stays non-@api). Its calendar.view gate is project-scoped, but on an API call there is no
// session project, so it resolves capability-only against the effective role — which a
// non-manager editor holds (calendar.view is readonly+). This is the exact path that would
// -32001 if a project-scoped gate fell closed on a null project, so prove it resolves for a
// non-manager, not just the owner.
$cal = $this->rpc($I, 'leantime.rpc.Calendar.Calendar.getMyCalendar', new \stdClass);
Assert::assertArrayNotHasKey('error', $cal, 'getMyCalendar must resolve for a non-manager editor: '.json_encode($cal));
Assert::assertIsArray($cal['result'] ?? null, 'getMyCalendar should return an array: '.json_encode($cal));

// And the inbox list resolves for the editor too (session-scoped, ungated).
$editorInbox = $this->rpc($I, 'leantime.rpc.Notifications.Notifications.getInbox', new \stdClass);
Assert::assertArrayNotHasKey('error', $editorInbox, 'getInbox must resolve for a non-manager editor: '.json_encode($editorInbox));
Assert::assertIsArray($editorInbox['result'] ?? null, 'getInbox should return an array: '.json_encode($editorInbox));

// IDOR guard: markNotificationUnread is session-scoped (matches on (id, session user)).
// Seed a notification owned by the OWNER, read=1, then — as the editor — try to flip it
// unread by its id. With the previous unscoped where('id') update this would succeed; now
// it must NOT: the result is false and the row stays read.
$ownerId = (int) $I->grabFromDatabase('zp_user', 'id', ['username' => 'test@leantime.io']);
$ownerNotifId = (int) $I->haveInDatabase('zp_notifications', [
'userId' => $ownerId,
'read' => 1,
'type' => 'mention',
'module' => 'ticket',
'moduleId' => 1,
'message' => 'owner-only notification',
'datetime' => date('Y-m-d H:i:s'),
'url' => '',
'authorId' => $ownerId,
]);
// result === false proves the IDOR is closed: the session-scoped repo matched (id, editor)
// => 0 rows => update affected nothing. (A DB read-back is avoided here because `read` is a
// MySQL reserved word and Codeception's grabFromDatabase doesn't quote the column.)
$unread = $this->rpc($I, 'leantime.rpc.Notifications.Notifications.markNotificationUnread', ['id' => $ownerNotifId]);
Assert::assertArrayNotHasKey('error', $unread, 'markNotificationUnread should respond cleanly: '.json_encode($unread));
Assert::assertFalse($unread['result'] ?? true, 'editor must NOT mark the owner\'s notification unread (IDOR): '.json_encode($unread));
}

/** POST /api/jsonrpc with the test bearer + method, return the decoded body. */
Expand Down
Loading