Conversation
🔍 Code Review Summary❗ Attention Required: This push has potential issues. 🚨 Overview
🚨 Critical Issuessecurity (1 issues)Details1. Hardcoded application server key in service worker.📁 File: apps/pwa/src/sw.ts 💡 Solution: Current Code: applicationServerKey: 'BLA70jg5Wgi6XD6BAElOfW7YXcQ3l3iFRzyPj5AV5ZuSr_uTugv-9hbgXwfPhuw_JfbDAqn-Fl5nKSvnQpjFV8g',Suggested Code: applicationServerKey: env.VAPID_PUBLIC_KEY,
Useful Commands
|
![kaizen-bot[bot]](https://avatars.githubusercontent.com/in/480502?s=60&v=4){kind=small}
| // Proceed with subscription if not already subscribed | ||
| const registration = await navigator.serviceWorker.ready; | ||
| const subscription = await registration.pushManager.subscribe({ | ||
| userVisibleOnly: true, |
There was a problem hiding this comment.
Comment: Hardcoded application server key in service worker.
Solution: Store sensitive keys in environment variables and access them securely.
Potential Fix:
applicationServerKey: env.VAPID_PUBLIC_KEY,
There was a problem hiding this comment.
This needs to be trpc routes.
| export function addSubscription(subscription: PushSubscription) { | ||
| pushSubscriptions.add(subscription); | ||
| } | ||
|
|
||
| export function removeSubscription(subscription: PushSubscription) { | ||
| pushSubscriptions.delete(subscription); | ||
| } | ||
|
|
There was a problem hiding this comment.
If multiple instances of backend are running in production, this will lead to same notification being sent to user multiple times.
| // Schedule a notification every 5 seconds | ||
| export function scheduleFrequentNotification() { | ||
| scheduleJob('*/5 * * * * *', async () => { | ||
| console.log('Sending notification'); | ||
| await sendNotificationToAll('Frequent Update', 'Here is your notification every 5 seconds!'); | ||
| }); | ||
| } No newline at end of file |
There was a problem hiding this comment.
I understand you needed this for dev, but then have guardrails to run only in isDev
| const registration = await navigator.serviceWorker.ready; | ||
| const subscription = await registration.pushManager.subscribe({ | ||
| userVisibleOnly: true, | ||
| applicationServerKey: 'BLA70jg5Wgi6XD6BAElOfW7YXcQ3l3iFRzyPj5AV5ZuSr_uTugv-9hbgXwfPhuw_JfbDAqn-Fl5nKSvnQpjFV8g', |
There was a problem hiding this comment.
Seperate SW config from vite config. & add comments with examples on impact/importance for specific fields in config.
2 participants
Comprehensive Update on Push Notifications and Service Worker Enhancements
Implement push notifications and service worker updates for the PWA application.
web-pushlibrary.vite-plugin-pwafor managing service worker and PWA manifest.node-scheduleandweb-push.antd,react-toastify, andworkbox-core.node-scheduleandweb-push.Users will receive push notifications, enhancing engagement, while the application will automatically update, improving overall user experience.
Original Description
# Add Push Notification Functionality**
Implement push notification capabilities in the API server and PWA client.
node-scheduleandweb-pushdependencies to handle scheduling and sending push notifications.**
Users will be able to subscribe to push notifications and receive real-time updates from the application.
Original Description