feat: address all remaining TODOs#9
Conversation
Auth: - Replace 32-bit simpleHash with SHA-256 via Web Crypto API (crypto.subtle) - Auto-upgrade legacy mvp_ hashes to sha256_ transparently on next login - New registrations and demo seed use SHA-256 from the start Viewer count: - New viewerHeartbeats schema table with compound index - pingViewer public mutation: upsert heartbeat every 30s from public page - getViewerCount presenter query: count heartbeats within 90s window - Session page shows live viewer count next to status badge - Hourly cron cleans up stale heartbeats (convex/crons.ts) Multiple sessions: - Dashboard warns when multiple live sessions run for the same handout Export: - New /dashboard/handout/[id]/print page: all blocks with reveal badges - Auto-opens print dialog on load; print CSS hides UI chrome - "Exportieren" button added to handout editor header PowerPoint fullscreen workaround: - Direct slide number input in the add-in (always visible when connected) - Allows quick recovery when auto-sync is suspended in fullscreen mode Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
❌ Deploy Preview for handoutmarc failed.
|
📝 WalkthroughWalkthroughThe changes implement four previously pending features: PowerPoint fullscreen workaround via direct slide-number input, migration to SHA-256 authentication with automatic legacy hash migration, real-time viewer count tracking with 30-second heartbeat polling, and a dedicated print route. Additionally, the dashboard now warns about multiple concurrent live sessions for the same handout. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Public Viewer<br/>(h/[token])
participant Server as Convex Server<br/>(viewers.ts)
participant DB as ViewerHeartbeats<br/>Table
Client->>Client: Every 30s: getViewerId()
Client->>Server: pingViewer(publicToken, viewerId)
Server->>Server: Find active session by publicToken
Server->>DB: Upsert heartbeat<br/>(sessionId, viewerId, now)
DB-->>Server: Success
Presenter->>Server: getViewerCount(token, sessionId)
Server->>DB: Query heartbeats for sessionId<br/>where lastSeenAt > now - 90s
DB-->>Server: Count of active viewers
Server-->>Presenter: Display viewer badge
Note over Server,DB: Hourly scheduled cron
Server->>DB: cleanupHeartbeats()<br/>Delete rows older than 24h
DB-->>Server: { deleted: N }
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR addresses all remaining TODOs by upgrading password hashing to SHA-256, adding real-time viewer counting via heartbeats, adding a multi-live-session warning, adding a print/export page for handouts, and adding a direct slide-number input in the PowerPoint add-in for fullscreen recovery. Key changes:
Issues found:
Confidence Score: 4/5Safe to merge with the static-salt security concern tracked as a follow-up; no changes break existing functionality. All five features work correctly end-to-end. The static-salt issue in sha256Hash is a meaningful security regression over a proper bcrypt/argon2 approach, but is still a significant improvement over the legacy 32-bit hash and the app is explicitly MVP-scoped. The cleanup full-table-scan is a real scalability risk but only triggers in a background cron and won't affect users until the table is very large. The useEffect dependency is a cosmetic issue. No data-loss or runtime errors are introduced. convex/_utils.ts (static salt), convex/viewers.ts (full table scan in cleanupHeartbeats) Important Files Changed
Sequence DiagramsequenceDiagram
participant Viewer as Viewer Browser
participant Public as /h/[token]
participant Convex as Convex DB
participant Session as Session Page
participant Cron as Hourly Cron
Viewer->>Public: Open handout page
Public->>Convex: pingViewer({publicToken, viewerId})
Convex->>Convex: Upsert viewerHeartbeats row
loop Every 30s
Public->>Convex: pingViewer({publicToken, viewerId})
Convex->>Convex: Update lastSeenAt
end
Session->>Convex: getViewerCount({token, sessionId})
Convex->>Convex: Filter heartbeats where lastSeenAt > now-90s
Convex-->>Session: viewerCount
Cron->>Convex: cleanupHeartbeats()
Convex->>Convex: Delete rows where lastSeenAt < now-24h
Reviews (1): Last reviewed commit: "feat: address all remaining TODOs" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| export async function sha256Hash(password: string): Promise<string> { | ||
| const salted = `slide-handout:${password}`; | ||
| const data = new TextEncoder().encode(salted); | ||
| const buffer = await crypto.subtle.digest("SHA-256", data); | ||
| const hex = Array.from(new Uint8Array(buffer)) | ||
| .map((b) => b.toString(16).padStart(2, "0")) | ||
| .join(""); | ||
| return `sha256_${hex}`; |
There was a problem hiding this comment.
sha256Hash prepends the fixed string "slide-handout:" to every password before hashing. Because this prefix is constant and publicly visible in source code, all users who share the same password produce identical hashes. An attacker who obtains the database can build a single rainbow table targeting this known prefix and crack every matching account in one pass.
A secure implementation requires a random per-user salt stored alongside the hash. At minimum, the schema would need a passwordSalt field and the hash/verify logic updated:
// Generate on register/upgrade:
const saltBytes = crypto.getRandomValues(new Uint8Array(16));
const salt = Array.from(saltBytes).map(b => b.toString(16).padStart(2, '0')).join('');
const hash = await sha256Hash(password, salt);
// Store both `hash` and `salt` in the presenters row.
export async function sha256Hash(password: string, salt: string): Promise<string> {
const data = new TextEncoder().encode(`slide-handout:${salt}:${password}`);
const buffer = await crypto.subtle.digest('SHA-256', data);
const hex = Array.from(new Uint8Array(buffer)).map(b => b.toString(16).padStart(2, '0')).join('');
return `sha256_${hex}`;
}Alternatively, crypto.subtle.importKey + crypto.subtle.deriveBits (PBKDF2) is available in Convex's V8 isolate and provides iteration-count hardening on top of salting.
| export const cleanupHeartbeats = internalMutation({ | ||
| args: {}, | ||
| handler: async (ctx) => { | ||
| const cutoff = Date.now() - 24 * 60 * 60_000; // älter als 24h | ||
| const old = await ctx.db.query("viewerHeartbeats").collect(); | ||
| let deleted = 0; | ||
| for (const h of old) { | ||
| if (h.lastSeenAt < cutoff) { | ||
| await ctx.db.delete(h._id); | ||
| deleted++; | ||
| } | ||
| } | ||
| return { deleted }; | ||
| }, | ||
| }); |
There was a problem hiding this comment.
cleanupHeartbeats will not scale
cleanupHeartbeats calls .collect() on the entire viewerHeartbeats table and then filters by lastSeenAt in JavaScript. Convex imposes a per-query document limit (currently 16 384 documents), so this cron job will throw once the table grows large.
The fix is to add a by_lastSeenAt index to the schema and filter at the database level:
// In schema.ts, add to viewerHeartbeats:
.index("by_lastSeenAt", ["lastSeenAt"])
// In cleanupHeartbeats:
const old = await ctx.db
.query("viewerHeartbeats")
.withIndex("by_lastSeenAt", (q) => q.lt("lastSeenAt", cutoff))
.collect();This ensures only expired rows are fetched, regardless of total table size.
| useEffect(() => { | ||
| if (data) { | ||
| const timer = setTimeout(() => window.print(), 300); | ||
| return () => clearTimeout(timer); | ||
| } | ||
| }, [!!data]); |
There was a problem hiding this comment.
[!!data] dependency in useEffect
The dependency array [!!data] passes a derived boolean expression rather than the reactive value data itself. ESLint's react-hooks/exhaustive-deps rule flags derived values in dependency arrays because React's equality check operates on the coerced type, and the indirection obscures what the effect truly depends on.
The idiomatic pattern that achieves the same intent and satisfies the linter:
| useEffect(() => { | |
| if (data) { | |
| const timer = setTimeout(() => window.print(), 300); | |
| return () => clearTimeout(timer); | |
| } | |
| }, [!!data]); | |
| }, [data]); |
The if (data) guard inside the callback already prevents the print dialog from opening when data is still undefined, so the behaviour is identical.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
convex/_utils.ts (1)
22-34: Improvement over legacy hash; consider per-user salt and key stretching for production.The SHA-256 implementation is a significant security improvement over the 32-bit
simpleHash. However, for production hardening, consider:
Per-user salt: The static salt
"slide-handout:"means identical passwords produce identical hashes. A unique random salt per user (stored alongside the hash) would prevent rainbow table attacks.Key stretching: SHA-256 is fast by design, making it vulnerable to brute-force. Libraries like
bcryptorargon2are preferred for password hashing as they're intentionally slow.For an MVP with the migration path in place, this is acceptable—but flag for future hardening before production scale.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/_utils.ts` around lines 22 - 34, The sha256Hash function currently uses a static salt ("slide-handout:") and no key stretching; change it to accept and use a per-user salt and prepare for a slow KDF: update sha256Hash(password: string, salt?: string) to require/provision a unique random salt (store alongside the hash) and derive the stored value as e.g. "sha256_<salt_hex>_<hash_hex>" or migrate to a proper KDF by replacing sha256Hash with a wrapper that delegates to a slow algorithm (bcrypt/argon2) when available; ensure code paths referencing sha256Hash are updated to supply/handle the salt and that salt generation (secure random) and storage schema are implemented where accounts are created.convex/viewers.ts (1)
72-78: Add time-based indexes before this becomes a hot path.Lines 73-78 materialize every heartbeat for the session and filter in memory, while Lines 89-96 scan the whole table hourly for cleanup. That makes both paths grow with historical traffic instead of the 90-second active window. A
lastSeenAtindex (orsessionId,lastSeenAt) would let both operations range-scan only recent/stale rows.Also applies to: 88-96
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/viewers.ts` around lines 72 - 78, The viewerHeartbeats query currently materializes all rows for a session and filters by lastSeenAt in memory, and the hourly cleanup also scans the whole table; add a time-based index (e.g., "by_session_lastSeen" or "by_lastSeen") on lastSeenAt or on (sessionId,lastSeenAt), then change the active-count query using viewerHeartbeats.withIndex("by_session_lastSeen", q => q.eq("sessionId", args.sessionId).gt("lastSeenAt", cutoff)) (or the equivalent range predicate) so only recent rows are scanned, and update the cleanup routine to range-scan stale rows via the new index instead of iterating the whole table.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/app/dashboard/handout/`[id]/print/page.tsx:
- Around line 94-96: The reveal badge is being hidden in printed/PDF output
because the span rendering revealLabel(block.revealRule) includes the "no-print"
class; remove that class (or adjust print CSS to override .no-print to display
inline in `@media` print) so the revealLabel is included in exports. Locate the
span that renders revealLabel(block.revealRule) in page.tsx, remove "no-print"
from its className (or add a print-specific rule that forces .no-print to be
visible) and verify printed/PDF output shows the badge.
- Around line 18-21: The current useQuery call with
api.handouts.getHandoutWithBlocks passes "skip" when token is absent so data
never resolves and the page stays in the loading branch; change the logic to
explicitly handle the skipped-query path: first check the auth store hydration
flag from useAuthStore (e.g., isHydrated or similar) and if not hydrated show
nothing/placeholder, then if hydrated and token is missing render an
auth-required state or redirect instead of the loading spinner, and only call
useQuery(api.handouts.getHandoutWithBlocks, { token, handoutId }) when token is
present; update the component branches that render Lädt... (the loading lines)
to distinguish between real loading (data.status/loading) and the
skipped/no-token case so you don't show a permanent spinner.
In `@apps/web/src/app/h/`[token]/page.tsx:
- Around line 29-38: The heartbeat effect starts while sessionInfo is undefined
(loading) and never reruns if the query resolves to null, so stop/avoid starting
pings until sessionInfo is resolved and ensure the effect reacts to a transition
from loading→null; modify the useEffect guarding logic (the effect around
getViewerId/pingViewer/ping) to only start the interval when sessionInfo !==
undefined && sessionInfo !== null && sessionInfo.status !== "ended", and include
sessionInfo (not just sessionInfo?.status) in the dependency array so the effect
will re-run and clear the interval when the query resolves to null; reference
the useEffect, sessionInfo, publicToken, getViewerId, and pingViewer identifiers
when making the change.
In `@convex/auth.ts`:
- Around line 30-33: Replace the use of the fast sha256Hash for storing
passwords with a proper password KDF (e.g., bcrypt or Argon2) when creating
presenter records and in the legacy-upgrade path: stop calling sha256Hash to
produce passwordHash in the registration flow (the assignment that goes into
ctx.db.insert("presenters")), instead generate a per-user salted KDF hash with a
secure cost/work factor and store that; likewise locate the legacy upgrade logic
that checks existing SHA-256 hashes (the branch handling oldPassword / legacy
verification) and update it to verify the old SHA-256 value, then re-hash the
password with the chosen KDF and replace the stored passwordHash. Also ensure
any authentication/verify code uses the KDF's verify function rather than raw
SHA-256 comparisons and preserve a migration flag or scheme identifier so you
can recognize legacy hashes.
In `@convex/viewers.ts`:
- Around line 30-57: The handler currently writes the raw args.viewerId into the
indexed viewerHeartbeats table (functions: handler, viewerHeartbeats
query/insert/patch), which allows unbounded/high-cardinality values; before any
DB read/write, validate and bound viewerId (e.g., enforce allowed charset and
max length) or replace it with a fixed-size fingerprint (hash) and use that
bounded identifier in all queries/patches/inserts so the index only ever stores
a constrained, predictable value; ensure the same transformation is used for the
existing lookup (withIndex("by_session_viewer")) and for insert/patch
operations.
- Around line 64-79: getViewerCount currently calls requirePresenter(ctx,
args.token) but never verifies that args.sessionId belongs to that presenter;
fix by loading the presentation session and checking ownership before returning
the count: after requirePresenter, fetch the session via
ctx.db.get(args.sessionId) (presentationSessions), ensure the session exists and
its presenter identifier (e.g., session.presenterId or session.presenterToken)
matches the authenticated presenter identity returned by requirePresenter (or
modify requirePresenter to return the presenter id), and throw an unauthorized
error if it does not; only then query viewerHeartbeats and return the active
count.
In `@README.md`:
- Line 244: The README contains a mismatched-quote string `„Add-in
verbinden"-Panel`; replace the ASCII closing quote with the matching German
closing quote so the text reads `„Add-in verbinden“-Panel` (or remove the quotes
entirely) to ensure consistent quotation marks and correct rendering.
---
Nitpick comments:
In `@convex/_utils.ts`:
- Around line 22-34: The sha256Hash function currently uses a static salt
("slide-handout:") and no key stretching; change it to accept and use a per-user
salt and prepare for a slow KDF: update sha256Hash(password: string, salt?:
string) to require/provision a unique random salt (store alongside the hash) and
derive the stored value as e.g. "sha256_<salt_hex>_<hash_hex>" or migrate to a
proper KDF by replacing sha256Hash with a wrapper that delegates to a slow
algorithm (bcrypt/argon2) when available; ensure code paths referencing
sha256Hash are updated to supply/handle the salt and that salt generation
(secure random) and storage schema are implemented where accounts are created.
In `@convex/viewers.ts`:
- Around line 72-78: The viewerHeartbeats query currently materializes all rows
for a session and filters by lastSeenAt in memory, and the hourly cleanup also
scans the whole table; add a time-based index (e.g., "by_session_lastSeen" or
"by_lastSeen") on lastSeenAt or on (sessionId,lastSeenAt), then change the
active-count query using viewerHeartbeats.withIndex("by_session_lastSeen", q =>
q.eq("sessionId", args.sessionId).gt("lastSeenAt", cutoff)) (or the equivalent
range predicate) so only recent rows are scanned, and update the cleanup routine
to range-scan stale rows via the new index instead of iterating the whole table.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3e9959e9-16cb-4365-ab37-a6df4f6f3915
📒 Files selected for processing (13)
README.mdapps/powerpoint-addin/src/App.tsxapps/web/src/app/dashboard/handout/[id]/page.tsxapps/web/src/app/dashboard/handout/[id]/print/page.tsxapps/web/src/app/dashboard/page.tsxapps/web/src/app/dashboard/session/[id]/page.tsxapps/web/src/app/h/[token]/page.tsxconvex/_utils.tsconvex/auth.tsconvex/crons.tsconvex/init.tsconvex/schema.tsconvex/viewers.ts
| const data = useQuery( | ||
| api.handouts.getHandoutWithBlocks, | ||
| token ? { token, handoutId: handoutId as Id<"handouts"> } : "skip" | ||
| ); |
There was a problem hiding this comment.
Handle the skipped-query path separately from the loading state.
Lines 18-21 pass "skip" when token is missing. In that state data never resolves, so Lines 40-41 keep rendering Lädt... forever instead of redirecting or showing an auth-required state. If useAuthStore() rehydrates asynchronously, gate this on its hydration flag so you don't trade the permanent spinner for a login flash.
Also applies to: 40-41
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/dashboard/handout/`[id]/print/page.tsx around lines 18 - 21,
The current useQuery call with api.handouts.getHandoutWithBlocks passes "skip"
when token is absent so data never resolves and the page stays in the loading
branch; change the logic to explicitly handle the skipped-query path: first
check the auth store hydration flag from useAuthStore (e.g., isHydrated or
similar) and if not hydrated show nothing/placeholder, then if hydrated and
token is missing render an auth-required state or redirect instead of the
loading spinner, and only call useQuery(api.handouts.getHandoutWithBlocks, {
token, handoutId }) when token is present; update the component branches that
render Lädt... (the loading lines) to distinguish between real loading
(data.status/loading) and the skipped/no-token case so you don't show a
permanent spinner.
| <span className="no-print text-xs bg-blue-50 text-blue-700 border border-blue-200 rounded px-1.5 py-0.5"> | ||
| {revealLabel(block.revealRule)} | ||
| </span> |
There was a problem hiding this comment.
Reveal badges are excluded from the actual print output.
Line 94 adds no-print, and Lines 49-50 hide that class in print media. That means the preview shows the reveal label, but the exported PDF / printed page drops it entirely.
Suggested fix
- <span className="no-print text-xs bg-blue-50 text-blue-700 border border-blue-200 rounded px-1.5 py-0.5">
+ <span className="text-xs bg-blue-50 text-blue-700 border border-blue-200 rounded px-1.5 py-0.5">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <span className="no-print text-xs bg-blue-50 text-blue-700 border border-blue-200 rounded px-1.5 py-0.5"> | |
| {revealLabel(block.revealRule)} | |
| </span> | |
| <span className="text-xs bg-blue-50 text-blue-700 border border-blue-200 rounded px-1.5 py-0.5"> | |
| {revealLabel(block.revealRule)} | |
| </span> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/dashboard/handout/`[id]/print/page.tsx around lines 94 - 96,
The reveal badge is being hidden in printed/PDF output because the span
rendering revealLabel(block.revealRule) includes the "no-print" class; remove
that class (or adjust print CSS to override .no-print to display inline in
`@media` print) so the revealLabel is included in exports. Locate the span that
renders revealLabel(block.revealRule) in page.tsx, remove "no-print" from its
className (or add a print-specific rule that forces .no-print to be visible) and
verify printed/PDF output shows the badge.
| // Viewer-Heartbeat: beim Laden und danach alle 30s | ||
| useEffect(() => { | ||
| if (sessionInfo?.status === "ended") return; | ||
| const viewerId = getViewerId(); | ||
| const ping = () => pingViewer({ publicToken, viewerId }).catch(() => {}); | ||
| ping(); | ||
| const interval = setInterval(ping, 30_000); | ||
| return () => clearInterval(interval); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [publicToken, sessionInfo?.status, pingViewer]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/web/src/app/h/[token]/page.tsx | head -60Repository: EinfachMxrc/handout
Length of output: 2648
🏁 Script executed:
rg "sessionInfo" apps/web/src/app/h/[token]/page.tsx -B 2 -A 2Repository: EinfachMxrc/handout
Length of output: 2458
🏁 Script executed:
rg "useQuery\|sessionInfo" apps/web/src -A 3 | head -100Repository: EinfachMxrc/handout
Length of output: 45
🏁 Script executed:
fd "schema\|type" apps/web/src --type f | grep -i session | head -10Repository: EinfachMxrc/handout
Length of output: 45
Distinguish loading from not found before starting the heartbeat.
The interval starts while sessionInfo is undefined (loading). If the query later resolves to null (session not found), the dependency on line 38 remains undefined because optional chaining collapses both cases, so the effect never reruns and the page keeps pinging an invalid publicToken every 30 seconds.
Suggested fix
+ const heartbeatState =
+ sessionInfo === undefined ? "loading" : sessionInfo === null ? "missing" : sessionInfo.status;
+
// Viewer-Heartbeat: beim Laden und danach alle 30s
useEffect(() => {
- if (sessionInfo?.status === "ended") return;
+ if (heartbeatState !== "draft" && heartbeatState !== "live") return;
const viewerId = getViewerId();
const ping = () => pingViewer({ publicToken, viewerId }).catch(() => {});
ping();
const interval = setInterval(ping, 30_000);
return () => clearInterval(interval);
- }, [publicToken, sessionInfo?.status, pingViewer]);
+ }, [publicToken, heartbeatState, pingViewer]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Viewer-Heartbeat: beim Laden und danach alle 30s | |
| useEffect(() => { | |
| if (sessionInfo?.status === "ended") return; | |
| const viewerId = getViewerId(); | |
| const ping = () => pingViewer({ publicToken, viewerId }).catch(() => {}); | |
| ping(); | |
| const interval = setInterval(ping, 30_000); | |
| return () => clearInterval(interval); | |
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| }, [publicToken, sessionInfo?.status, pingViewer]); | |
| const heartbeatState = | |
| sessionInfo === undefined ? "loading" : sessionInfo === null ? "missing" : sessionInfo.status; | |
| // Viewer-Heartbeat: beim Laden und danach alle 30s | |
| useEffect(() => { | |
| if (heartbeatState !== "draft" && heartbeatState !== "live") return; | |
| const viewerId = getViewerId(); | |
| const ping = () => pingViewer({ publicToken, viewerId }).catch(() => {}); | |
| ping(); | |
| const interval = setInterval(ping, 30_000); | |
| return () => clearInterval(interval); | |
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| }, [publicToken, heartbeatState, pingViewer]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/h/`[token]/page.tsx around lines 29 - 38, The heartbeat
effect starts while sessionInfo is undefined (loading) and never reruns if the
query resolves to null, so stop/avoid starting pings until sessionInfo is
resolved and ensure the effect reacts to a transition from loading→null; modify
the useEffect guarding logic (the effect around getViewerId/pingViewer/ping) to
only start the interval when sessionInfo !== undefined && sessionInfo !== null
&& sessionInfo.status !== "ended", and include sessionInfo (not just
sessionInfo?.status) in the dependency array so the effect will re-run and clear
the interval when the query resolves to null; reference the useEffect,
sessionInfo, publicToken, getViewerId, and pingViewer identifiers when making
the change.
| const passwordHash = await sha256Hash(args.password); | ||
| const presenterId = await ctx.db.insert("presenters", { | ||
| email: args.email, | ||
| passwordHash, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "auth.ts" convex/ && fd -t f "_utils.ts" convex/Repository: EinfachMxrc/handout
Length of output: 94
🏁 Script executed:
cat -n convex/auth.ts | head -90Repository: EinfachMxrc/handout
Length of output: 3172
🏁 Script executed:
cat -n convex/_utils.tsRepository: EinfachMxrc/handout
Length of output: 2904
Replace SHA-256 with a proper password KDF (bcrypt, Argon2, or PBKDF2).
The sha256Hash function uses a fast cryptographic hash with only a static string prefix as salt. SHA-256 is not a Key Derivation Function and provides insufficient protection:
- No work factor or iteration count to slow brute-force attacks
- Static prefix "slide-handout:" offers no per-user randomness
- Identical passwords hash identically across accounts, leaking information
- Offline dictionary attacks and precomputed rainbow tables are feasible
Apply this fix to both the registration path (line 30) and the legacy upgrade path (lines 68-80). Use bcrypt, Argon2, or PBKDF2 with appropriate cost/iteration parameters to make password cracking computationally expensive.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@convex/auth.ts` around lines 30 - 33, Replace the use of the fast sha256Hash
for storing passwords with a proper password KDF (e.g., bcrypt or Argon2) when
creating presenter records and in the legacy-upgrade path: stop calling
sha256Hash to produce passwordHash in the registration flow (the assignment that
goes into ctx.db.insert("presenters")), instead generate a per-user salted KDF
hash with a secure cost/work factor and store that; likewise locate the legacy
upgrade logic that checks existing SHA-256 hashes (the branch handling
oldPassword / legacy verification) and update it to verify the old SHA-256
value, then re-hash the password with the chosen KDF and replace the stored
passwordHash. Also ensure any authentication/verify code uses the KDF's verify
function rather than raw SHA-256 comparisons and preserve a migration flag or
scheme identifier so you can recognize legacy hashes.
| args: { | ||
| publicToken: v.string(), | ||
| viewerId: v.string(), | ||
| }, | ||
| handler: async (ctx, args) => { | ||
| const session = await ctx.db | ||
| .query("presentationSessions") | ||
| .withIndex("by_public_token", (q) => q.eq("publicToken", args.publicToken)) | ||
| .first(); | ||
|
|
||
| if (!session || session.status === "ended") return; | ||
|
|
||
| const existing = await ctx.db | ||
| .query("viewerHeartbeats") | ||
| .withIndex("by_session_viewer", (q) => | ||
| q.eq("sessionId", session._id).eq("viewerId", args.viewerId) | ||
| ) | ||
| .first(); | ||
|
|
||
| if (existing) { | ||
| await ctx.db.patch(existing._id, { lastSeenAt: Date.now() }); | ||
| } else { | ||
| await ctx.db.insert("viewerHeartbeats", { | ||
| sessionId: session._id, | ||
| viewerId: args.viewerId, | ||
| lastSeenAt: Date.now(), | ||
| }); | ||
| } |
There was a problem hiding this comment.
Bound viewerId before writing it into an indexed public table.
Lines 31-55 accept an anonymous viewerId as an unrestricted string and persist it into the indexed viewerHeartbeats table. A client can spam oversized or high-cardinality IDs here and turn this public mutation into an avoidable storage/write-amplification vector.
Suggested fix
handler: async (ctx, args) => {
+ if (args.viewerId.length === 0 || args.viewerId.length > 64) {
+ throw new Error("Ungültige Zuschauer-ID");
+ }
+
const session = await ctx.db🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@convex/viewers.ts` around lines 30 - 57, The handler currently writes the raw
args.viewerId into the indexed viewerHeartbeats table (functions: handler,
viewerHeartbeats query/insert/patch), which allows unbounded/high-cardinality
values; before any DB read/write, validate and bound viewerId (e.g., enforce
allowed charset and max length) or replace it with a fixed-size fingerprint
(hash) and use that bounded identifier in all queries/patches/inserts so the
index only ever stores a constrained, predictable value; ensure the same
transformation is used for the existing lookup (withIndex("by_session_viewer"))
and for insert/patch operations.
| export const getViewerCount = query({ | ||
| args: { | ||
| token: v.string(), | ||
| sessionId: v.id("presentationSessions"), | ||
| }, | ||
| handler: async (ctx, args) => { | ||
| await requirePresenter(ctx, args.token); | ||
|
|
||
| const cutoff = Date.now() - ACTIVE_WINDOW_MS; | ||
| const heartbeats = await ctx.db | ||
| .query("viewerHeartbeats") | ||
| .withIndex("by_session", (q) => q.eq("sessionId", args.sessionId)) | ||
| .collect(); | ||
|
|
||
| return heartbeats.filter((h) => h.lastSeenAt > cutoff).length; | ||
| }, |
There was a problem hiding this comment.
Authorize the requested session before returning its viewer count.
Line 70 validates the caller's token, but this query never loads args.sessionId or checks that the session belongs to that presenter. Any authenticated presenter who gets another session ID can read that session's viewer count.
Suggested fix
handler: async (ctx, args) => {
- await requirePresenter(ctx, args.token);
+ const presenter = await requirePresenter(ctx, args.token);
+ const session = await ctx.db.get(args.sessionId);
+ if (!session || session.presenterId !== presenter._id) {
+ throw new Error("Nicht autorisiert");
+ }
const cutoff = Date.now() - ACTIVE_WINDOW_MS;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const getViewerCount = query({ | |
| args: { | |
| token: v.string(), | |
| sessionId: v.id("presentationSessions"), | |
| }, | |
| handler: async (ctx, args) => { | |
| await requirePresenter(ctx, args.token); | |
| const cutoff = Date.now() - ACTIVE_WINDOW_MS; | |
| const heartbeats = await ctx.db | |
| .query("viewerHeartbeats") | |
| .withIndex("by_session", (q) => q.eq("sessionId", args.sessionId)) | |
| .collect(); | |
| return heartbeats.filter((h) => h.lastSeenAt > cutoff).length; | |
| }, | |
| export const getViewerCount = query({ | |
| args: { | |
| token: v.string(), | |
| sessionId: v.id("presentationSessions"), | |
| }, | |
| handler: async (ctx, args) => { | |
| const presenter = await requirePresenter(ctx, args.token); | |
| const session = await ctx.db.get(args.sessionId); | |
| if (!session || session.presenterId !== presenter._id) { | |
| throw new Error("Nicht autorisiert"); | |
| } | |
| const cutoff = Date.now() - ACTIVE_WINDOW_MS; | |
| const heartbeats = await ctx.db | |
| .query("viewerHeartbeats") | |
| .withIndex("by_session", (q) => q.eq("sessionId", args.sessionId)) | |
| .collect(); | |
| return heartbeats.filter((h) => h.lastSeenAt > cutoff).length; | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@convex/viewers.ts` around lines 64 - 79, getViewerCount currently calls
requirePresenter(ctx, args.token) but never verifies that args.sessionId belongs
to that presenter; fix by loading the presentation session and checking
ownership before returning the count: after requirePresenter, fetch the session
via ctx.db.get(args.sessionId) (presentationSessions), ensure the session exists
and its presenter identifier (e.g., session.presenterId or
session.presenterToken) matches the authenticated presenter identity returned by
requirePresenter (or modify requirePresenter to return the presenter id), and
throw an unauthorized error if it does not; only then query viewerHeartbeats and
return the active count.
| - [ ] **Auth:** MVP nutzt einfaches Hash-System – für Produktion: Clerk/Auth0/Convex Auth | ||
| - [x] **PowerPoint Fullscreen-Sync:** Direkteingabe der Foliennummer im Add-in als Workaround; Add-in wechselt automatisch in Hybrid/Manuell-Modus (Office.js-Limitation bleibt bestehen) | ||
| - [x] **Auth:** SHA-256 via Web Crypto API; Bestehende Legacy-Hashes werden beim nächsten Login automatisch migriert | ||
| - [x] **Add-in Token-Übergabe:** „Add-in verbinden"-Panel in der Session-Seite – Token, Session-ID und Convex-URL mit einem Klick kopierbar (kein DevTools mehr) |
There was a problem hiding this comment.
Fix the mismatched quotes in the panel name.
„Add-in verbinden"-Panel mixes German opening quotes with an ASCII closing quote. Use „Add-in verbinden“-Panel (or drop the quotes) so the README renders cleanly.
🧰 Tools
🪛 LanguageTool
[typographical] ~244-~244: Zeichen ohne sein Gegenstück: ‚“‘ scheint zu fehlen
Context: ...griert - [x] Add-in Token-Übergabe: „Add-in verbinden"-Panel in der Session-S...
(DE_UNPAIRED_QUOTES)
[typographical] ~244-~244: Zeichen ohne sein Gegenstück: ‚"‘ scheint zu fehlen
Context: ...d-in Token-Übergabe:** „Add-in verbinden"-Panel in der Session-Seite – Token, Ses...
(DE_UNPAIRED_QUOTES)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` at line 244, The README contains a mismatched-quote string
`„Add-in verbinden"-Panel`; replace the ASCII closing quote with the matching
German closing quote so the text reads `„Add-in verbinden“-Panel` (or remove the
quotes entirely) to ensure consistent quotation marks and correct rendering.
- print/page.tsx: remove no-print class from reveal badge so labels appear in PDF/print output - h/[token]/page.tsx: guard heartbeat effect against null sessionInfo (loading→null transition no longer starts unnecessary pings) - convex/_utils.ts: add pbkdf2Hash (PBKDF2-SHA256, 100k iter, random salt) and verifyPassword helper supporting all hash prefixes - convex/auth.ts: use pbkdf2Hash for new accounts; auto-upgrade legacy sha256_ and mvp_ hashes to pbkdf2_ on successful login - convex/init.ts: seed demo account with pbkdf2Hash - convex/viewers.ts: hash viewerId to fixed-size fingerprint before storing in index; add session ownership check in getViewerCount Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Added by_lastSeenAt index to viewerHeartbeats table so the hourly cron can range-scan only stale rows instead of collecting the entire table (which would hit Convex's 16 384-document query limit at scale). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 participant
Summary
Alle verbleibenden TODOs aus der README-Liste adressiert.
Changes
Auth: SHA-256 (Security)
simpleHash(32-bit, unsicher) ersetzt durchsha256Hashviacrypto.subtlemvp_…) automatisch zu SHA-256 beim nächsten LoginZuschauer-Count (Echtzeit)
viewerHeartbeats-Tabelleconvex/crons.ts)Mehrere aktive Sessions
Export / Drucken
/dashboard/handout/[id]/print: alle Blöcke mit Reveal-BadgesPowerPoint Fullscreen-Workaround
Test plan
sha256_migriertsha256_-Hash🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes