Task #626: stop notifications after logout + tighten meeting reminder
- /auth/logout: delete all push_subscriptions rows for the user before destroying the session. Best-effort with logger.warn on failure so an operator can correlate any leaked-ring report to a real DB error. - home.tsx handleLogout: call pushSub.disable() before the logout mutation, raced against a 1.5s timeout so a stalled service worker cannot block sign-out. - executive-meeting-scheduler: switch the eligibility filter from denylist (ne cancelled + ne completed) to whitelist (eq status='scheduled'). Postponed / rescheduled / future statuses can no longer trigger false 5-minute reminders. - docker-compose.yml: pin TZ=Asia/Riyadh on postgres, api, and web services so the scheduler's naive date/time math matches the operator's wall clock instead of UTC. Gitea push failed with TLS error during this session — code committed locally, needs manual `./scripts/publish-to-gitea.sh --push` retry when the desktop-11cj93j tunnel recovers.
This commit is contained in:
Riyadh
@@ -1,4 +1,4 @@
|
||||
import { and, eq, inArray, ne, sql } from "drizzle-orm";
|
||||
import { and, eq, inArray, sql } from "drizzle-orm";
|
||||
import { db } from "@workspace/db";
|
||||
import {
|
||||
executiveMeetingAlertStateTable,
|
||||
@@ -92,8 +92,13 @@ async function tickOnce(): Promise<void> {
|
||||
.where(
|
||||
and(
|
||||
inArray(executiveMeetingsTable.meetingDate, [todayKey, tomorrowKey]),
|
||||
ne(executiveMeetingsTable.status, "cancelled"),
|
||||
ne(executiveMeetingsTable.status, "completed"),
|
||||
// #626: only ring for meetings whose status is still the
|
||||
// default "scheduled". Previously we used a denylist
|
||||
// (ne cancelled + ne completed), which let postponed /
|
||||
// rescheduled / in_progress and any future status sneak past
|
||||
// and trigger a false 5-minute push reminder. Whitelist is
|
||||
// safer: any status other than "scheduled" is silenced.
|
||||
eq(executiveMeetingsTable.status, "scheduled"),
|
||||
),
|
||||
);
|
||||
|
||||
|
||||
@@ -12,9 +12,11 @@ import {
|
||||
groupsTable,
|
||||
userGroupsTable,
|
||||
auditLogsTable,
|
||||
pushSubscriptionsTable,
|
||||
} from "@workspace/db";
|
||||
import { requireAuth, requireAdmin, getUserRoles } from "../middlewares/auth";
|
||||
import { saveSession, destroySession } from "../lib/session";
|
||||
import { logger } from "../lib/logger";
|
||||
import {
|
||||
loginLimiters,
|
||||
registerLimiters,
|
||||
@@ -354,6 +356,36 @@ router.post(
|
||||
);
|
||||
|
||||
router.post("/auth/logout", async (req, res): Promise<void> => {
|
||||
// #626: drop every Web Push subscription this user owns before
|
||||
// tearing down the session. Otherwise the scheduler keeps firing
|
||||
// pushes to the iPad after sign-out (the rows live independently of
|
||||
// the session table, keyed by user_id). The client also tries to
|
||||
// unsubscribe its own browser endpoint, but that fails closed if the
|
||||
// SW or network hiccups — this server-side wipe is the authoritative
|
||||
// stop. Pruning every endpoint (not just the caller's) is the
|
||||
// correct behaviour for a single-user-per-account product: signing
|
||||
// out on any device implies "I don't want notifications anywhere
|
||||
// until I sign back in".
|
||||
const userId = req.session.userId;
|
||||
if (typeof userId === "number" && Number.isInteger(userId) && userId > 0) {
|
||||
try {
|
||||
await db
|
||||
.delete(pushSubscriptionsTable)
|
||||
.where(eq(pushSubscriptionsTable.userId, userId));
|
||||
} catch (err) {
|
||||
// Best-effort: a failed delete must not block sign-out. Log so
|
||||
// an operator investigating "I logged out but still got a push"
|
||||
// can correlate the leaked row to a real DB failure instead of
|
||||
// chasing a silent regression. The scheduler also prunes
|
||||
// 404/410 endpoints on send, so a leaked row eventually
|
||||
// self-cleans the next time it's used (worst case: one stale
|
||||
// ring until the OS-level subscription expires).
|
||||
logger.warn(
|
||||
{ err, userId },
|
||||
"Failed to delete push_subscriptions on logout — endpoint may continue receiving pushes until next 404/410 prune",
|
||||
);
|
||||
}
|
||||
}
|
||||
await destroySession(req);
|
||||
res.json({ success: true });
|
||||
});
|
||||
|
||||
@@ -18,6 +18,7 @@ import {
|
||||
} from "@workspace/api-client-react";
|
||||
import { useQueryClient } from "@tanstack/react-query";
|
||||
import { useAuth } from "@/contexts/AuthContext";
|
||||
import { usePushSubscription } from "@/hooks/use-push-subscription";
|
||||
import { useReceivedNotes } from "@/lib/notes-api";
|
||||
import { useIncomingNotePopup } from "@/contexts/IncomingNotePopupContext";
|
||||
import {
|
||||
@@ -491,6 +492,7 @@ export default function HomePage() {
|
||||
const notesPulse = notePopupQueueLength > 0;
|
||||
|
||||
const logout = useLogout();
|
||||
const pushSub = usePushSubscription();
|
||||
|
||||
const openApp = (app: App) => {
|
||||
// Fire-and-forget: keepalive ensures the request survives any
|
||||
@@ -521,7 +523,31 @@ export default function HomePage() {
|
||||
? (user?.displayNameAr ?? user?.username ?? "")
|
||||
: (user?.displayNameEn ?? user?.username ?? "");
|
||||
|
||||
const handleLogout = () => {
|
||||
const handleLogout = async () => {
|
||||
// #626: tear down this browser's Web Push subscription BEFORE
|
||||
// hitting /auth/logout, so the local pushManager endpoint and the
|
||||
// server row both go away. The server-side logout also deletes
|
||||
// every push_subscriptions row for this user as a safety net, but
|
||||
// doing the browser unsubscribe first means the OS-level
|
||||
// subscription is gone too — otherwise the SW would keep an
|
||||
// orphan endpoint that the next signed-in user could inherit on
|
||||
// resubscribe.
|
||||
// Race the browser unsubscribe against a 1.5s timeout. `disable()`
|
||||
// awaits `navigator.serviceWorker.ready`, which can hang forever
|
||||
// if the SW never activates (rare but observed on first iOS PWA
|
||||
// launch). A stalled unsubscribe must not block the user from
|
||||
// signing out — the server-side delete in /auth/logout is the
|
||||
// authoritative cleanup, so a missed browser unsubscribe just
|
||||
// leaves an orphan pushManager endpoint that the OS will GC on
|
||||
// its own schedule.
|
||||
try {
|
||||
await Promise.race([
|
||||
pushSub.disable(),
|
||||
new Promise<void>((resolve) => setTimeout(resolve, 1500)),
|
||||
]);
|
||||
} catch {
|
||||
// Best-effort: a failed unsubscribe must not block sign-out.
|
||||
}
|
||||
logout.mutate(undefined, {
|
||||
onSuccess: () => {
|
||||
queryClient.clear();
|
||||
|
||||
@@ -8,6 +8,10 @@ services:
|
||||
POSTGRES_USER: ${POSTGRES_USER:-tx}
|
||||
POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-tx_dev_password}
|
||||
POSTGRES_DB: ${POSTGRES_DB:-tx}
|
||||
# #626: pin container TZ so PG's `now()` (used by audit logs
|
||||
# only — meeting_date/start_time are stored as date/time WITHOUT
|
||||
# timezone) matches the operator's wall clock instead of UTC.
|
||||
TZ: Asia/Riyadh
|
||||
volumes:
|
||||
- postgres_data:/var/lib/postgresql/data
|
||||
healthcheck:
|
||||
@@ -63,6 +67,13 @@ services:
|
||||
environment:
|
||||
NODE_ENV: production
|
||||
PORT: 8080
|
||||
# #626: pin container TZ to Riyadh so JS `new Date()` calls in
|
||||
# the meeting scheduler interpret the DB's naive date+time
|
||||
# columns against the operator's local clock. Without this the
|
||||
# container runs in UTC and the 5-minute upcoming-meeting window
|
||||
# is offset by 3 hours, causing reminders to land at the wrong
|
||||
# wall-clock time.
|
||||
TZ: Asia/Riyadh
|
||||
DATABASE_URL: postgres://${POSTGRES_USER:-tx}:${POSTGRES_PASSWORD:-tx_dev_password}@postgres:5432/${POSTGRES_DB:-tx}
|
||||
SESSION_SECRET: ${SESSION_SECRET:?SESSION_SECRET is required — copy .env.docker.example to .env and edit it}
|
||||
LOCAL_STORAGE_SIGNING_SECRET: ${LOCAL_STORAGE_SIGNING_SECRET:-${SESSION_SECRET}}
|
||||
@@ -98,6 +109,12 @@ services:
|
||||
restart: unless-stopped
|
||||
depends_on:
|
||||
- api
|
||||
environment:
|
||||
# #626: pin container TZ for consistency with the api service.
|
||||
# The web image only serves static assets so this is cosmetic
|
||||
# (affects access-log timestamps only), but keeping it aligned
|
||||
# avoids confusion when reading logs side-by-side.
|
||||
TZ: Asia/Riyadh
|
||||
# No host port binding — Caddy is the single public edge.
|
||||
expose:
|
||||
- "80"
|
||||
|
||||
Reference in New Issue
Block a user