Skip to content

feat(apps/chat): LTI semi-anonymous guest access#5083

Merged
rschlaefli merged 12 commits into
v3-aifrom
claude/condescending-swartz-993a42
May 10, 2026
Merged

feat(apps/chat): LTI semi-anonymous guest access#5083
rschlaefli merged 12 commits into
v3-aifrom
claude/condescending-swartz-993a42

Conversation

@rschlaefli
Copy link
Copy Markdown
Member

@rschlaefli rschlaefli commented May 4, 2026

Summary

Phase A of the LTI semi-anonymous chatbot work (replaces the stale claude/lti-chatbot-access-krtTu branch). Re-implements LTI guest access on top of current v3 with the 6 P0–P2 correctness bugs that the archive branch's own review feedback flagged, plus a few extras.

LTI-verified users can now use the tutor chatbot without a Klicker account. Guest persona is unlinkable from the LTI sub by design (HMAC-derived ssoId, no email). Guest tokens are useless against backend GraphQL (separate signing secret + host-only cookie).

Phase B (reasoning-effort tier gating: free efforts for everyone, paid efforts for credit-holders, optional guest premium pool) and Phase C (mode-switch UX + claim-history flow) follow in separate PRs. See plan for context.

What's in

  • ltiGuest.ts — per-course HMAC deriveGuestSsoId, race-safe findOrCreateGuestPersona (P2002 catch), separate-secret chat-guest JWT, LTI JWT verify with iss enforcement, pure resolveLtiAuthDecision resolver
  • /auth/lti entry route — verifies LTI JWT, validates chatbot ↔ course (cross-course access blocked), branches account/guest, sets host-only chat_participant_token
  • apiGuards returns authMode: 'account' | 'anonymous'; token order: guest first, then account
  • Middleware: dual-token check on top of existing CSP frame-ancestors; /auth/lti bypass; ?lti=1 on /noLogin redirect when guest cookie was the cause
  • Anonymous lock to fallback model in chat route runs after all existing model-resolution branches (fixes the P0 bypass when modelSelection=false)
  • Credits route filters availableModels AND recomputes automaticModelId from the filtered set (fixes P1 inconsistency)
  • Frontend: authMode in settingsStore, "Guest" sidebar badge, LTI-specific /noLogin copy
  • Lazy prod boot-time assertion: APP_CHAT_GUEST_SECRET and CHAT_GUEST_SEED required (fail-fast on first request, not at build time)
  • New env vars added to turbo.json globalEnv
  • @klicker-uzh/util declared as workspace dep (fixes implicit hoisted resolution that worked in Next.js but not in vitest)

Forward-compat invariants (Phase C will rely on these)

  • Guest rows are never deleted/modified when a real account exists for the same ltiSub — claim-history flow needs that data
  • deriveGuestSsoId exported and stable — Phase C recomputes it from any context
  • ParticipantAccount.type === 'lti_guest' is the canonical guest marker — indexed queries depend on the exact string
  • chat_participant_token-first apiGuards order — Phase C "switch to anonymous" only sets the cookie
  • No state outside cookies — Phase C "switch to account" only clears the cookie
  • /auth/lti decision factored into pure resolveLtiAuthDecision — Phase C UI page wraps it
  • ssoType (LTI1.1 / LTI1.3) recorded — cross-deployment migration remains possible

Out of scope

  • Embed-SDK generalization (EmbedIntegration, /auth/launch)
  • Anonymous entry from PWA without LTI
  • Persona-selection UX, sidebar mode-switch button, claim-history flow → Phase C
  • Reasoning-effort tier gating → Phase B
  • Guest cleanup / TTL job

Test plan

  • Unit tests (apps/chat/test/lti-guest.test.ts): 9/9 — derivation determinism + course scoping + sub scoping, verifyChatGuestToken rejects APP_SECRET-signed tokens, verifyLtiToken rejects wrong iss / scope / accepts valid LTI1.3
  • All chat tests still green (36/36, no regressions)
  • tsc --noEmit clean
  • next lint clean (pre-existing warnings only)
  • prettier --check clean
  • next build clean (route /auth/lti registered, middleware bundled)
  • Manual via agent-browser — needs Infisical secrets + DB seed + apps/chat + apps/lti running + minted test LTI JWT. Will run before merge.
    • Fresh launch, no account → guest, redirected, "Guest" badge, fallback model
    • Same launch repeated → same participantId, threads preserved
    • Different course, same sub → different participantId
    • Account user with valid participant_token → account mode, no badge
    • Account user, expired token → guest fallback (no /noLogin loop)
    • Cross-course chatbotId vs courseId → 403
    • chat_participant_token against backend GraphQL → unauthorized

Deploy notes

Set in stg + prd before merge:

  • APP_CHAT_GUEST_SECRET (random 32+ byte hex; must differ from APP_SECRET)
  • CHAT_GUEST_SEED (random 32+ byte hex)

Boot-time assertion in prod will throw 503 on first chatbot request if either is missing. Failure is loud + obvious.

Summary by CodeRabbit

  • New Features

    • LTI sign-in flow with guest SSO, guest-mode UI indicator, self‑heal redirect, and client-side guest token bootstrapping; authenticated client fetch helper for iframe fallback.
  • Bug Fixes

    • Anonymous/guest users now get a fallback model selection and clearer error responses; safer auth routing and middleware handling for mixed guest/account scenarios.
  • Documentation

    • Rollout plan and operational notes for CHIPS/LTI iframe auth.
  • Tests

    • New suites covering LTI guest flows and client auth helpers.

rschlaefli added 2 commits May 4, 2026 17:15
Enable LTI-verified users to access the tutor chatbot without a Klicker
account, via a separate guest persona with unlinkable HMAC-derived SSO id
and a chat-only token that backend GraphQL never accepts.

- Add `ltiGuest.ts` with deterministic per-course `deriveGuestSsoId`,
  race-safe `findOrCreateGuestPersona`, separate-secret chat-guest JWT
  sign/verify, LTI JWT verify with `iss` enforcement, and a pure
  `resolveLtiAuthDecision` resolver (Phase C will wrap with a UI page).
- Add `/auth/lti` entry route — verifies LTI JWT, validates chatbot ↔
  course, branches account-mode vs guest, sets host-only
  `chat_participant_token` cookie.
- Extend `apiGuards.getParticipantId` to return `authMode`. Token order:
  guest first, then account (Phase C "switch" sets only the guest cookie).
- Layer dual-token check on top of the existing CSP middleware; bypass
  `/auth/lti`; set `?lti=1` on `/noLogin` redirect when an invalid guest
  token was the cause.
- Lock anonymous users to fallback model in `chat/route.ts` after all
  existing model-resolution branches (avoids the bypass when
  `modelSelection=false`); filter `availableModels` + recompute
  `automaticModelId` from the filtered set in `credits/route.ts`.
- Frontend: add `authMode` to `settingsStore`, "Guest" badge in sidebar,
  LTI-specific copy in `/noLogin`.
- Boot-time assertion: `APP_CHAT_GUEST_SECRET` and `CHAT_GUEST_SEED`
  required in production. Add both to `turbo.json` `globalEnv`.
- Phase A invariant: never delete or modify guest rows when a real
  account exists — Phase C claim flow will need that data.

Phase B (reasoning-effort tier gating) and Phase C (mode-switch UX +
claim history) follow in separate PRs.
Module-level assertion blew up `next build` in CI (Next evaluates route
modules at build time without prod secrets). Move the prod-required
checks into the lazy `getChatGuestSeed` / `getChatGuestSecret` helpers
so we still fail fast on the first request, without breaking builds.
Copilot AI review requested due to automatic review settings May 4, 2026 15:28
@dosubot dosubot Bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label May 4, 2026
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 4, 2026

📝 Walkthrough

Walkthrough

Adds LTI guest SSO and chat-guest token support, threads an authMode identity through server/middleware/APIs, adds client-side sessionStorage bootstrapping and authedFetch, updates UI to surface guest mode, and introduces shared util exports and tests.

Changes

LTI Guest Authentication + Client Auth Utilities

Layer / File(s) Summary
Auth primitives / types
apps/chat/src/lib/server/apiGuards.ts, apps/chat/src/stores/settingsStore.ts, packages/util/src/auth.ts, packages/util/src/clientAuth.ts
Introduce AuthMode type; getParticipantId/withChatbotAuth now return authMode; add bearer extraction, LTI probe cookie name, cookie security options, sessionStorage-backed token helpers, and client authed-fetch creator.
Core guest SSO & token logic
apps/chat/src/lib/server/ltiGuest.ts
New module: derive per-course guest SSO ID (HMAC), find-or-create guest persona with Prisma (race handling), sign/verify chat-guest JWTs, verify LTI JWTs, and resolve account-vs-guest decision.
Middleware & guards wiring
apps/chat/src/middleware.ts, apps/chat/src/lib/server/apiGuards.ts
Middleware and api guards prefer chat_participant_token guest JWT (with HMAC secret fallback rules), verify guest tokens in-middleware, add redirectToNoLogin helper and bypass for /auth/lti; withChatbotAuth threads authMode.
LTI route / server endpoints
apps/chat/src/app/auth/lti/route.ts, apps/chat/src/app/api/chatbots/[chatbotId]/credits/route.ts, apps/chat/src/app/api/chatbots/[chatbotId]/chat/route.ts
Add GET /auth/lti route validating query/JWT, resolving auth decision, issuing guest token and cookie (or redirect with ?_t= when cookies blocked); credits/chat APIs consume authMode, filter/override available models for anonymous users, and include authMode in responses.
Client bootstrapping & fetch wiring
apps/chat/src/lib/client/authedFetch.ts, apps/chat/src/hooks/useChatGuestTokenBootstrap.ts, apps/chat/src/lib/api/types.ts, apps/chat/src/hooks/useChatResponse.ts
Add authedFetch created from util helper; bootstrap ?_t= into sessionStorage and strip URL; switch client API and streaming chat calls to authedFetch.
Frontend UI & UX helpers
apps/chat/src/app/noLogin/page.tsx, apps/chat/src/components/NoLoginSelfHeal.tsx, apps/chat/src/components/app-sidebar.tsx, apps/chat/src/components/assistant.tsx
No-login page accepts lti param and shows LTI-specific message; NoLoginSelfHeal injects session token into redirect URL when cookies blocked; sidebar shows “Guest” pill when authMode==='anonymous'; assistant bootstraps guest token and uses authedFetch for disclaimers.
Packaging, build, config, docs
packages/util/package.json, packages/util/rollup.config.js, apps/chat/package.json, turbo.json, AGENTS.md, .gitignore, project/plans_wip/PLAN-chips-iframe-auth-rollout.md
Add exports and rollup inputs for util subpaths; add workspace dependency @klicker-uzh/util; add guest env vars to turbo.json; docs and gitignore updates.
Tests & scripts
apps/chat/test/lti-guest.test.ts, apps/chat/test/authedFetch.test.ts, packages/util/test/*, apps/chat/scripts/mint-lti-jwt.mjs
Add Vitest suites for ltiGuest, authedFetch, util auth helpers; add script to mint test LTI JWTs.

Sequence Diagram(s)

sequenceDiagram
    participant Browser
    participant AuthRoute
    participant DB
    participant Middleware
    participant API
    Browser->>AuthRoute: GET /auth/lti?jwt=...&courseId=...&chatbotId=...
    AuthRoute->>AuthRoute: validate query & verify LTI JWT
    AuthRoute->>DB: load course & chatbot
    AuthRoute->>AuthRoute: resolveLtiAuthDecision (account vs guest)
    alt account
        AuthRoute->>DB: upsert participation
        AuthRoute-->>Browser: redirect to /<chatbotId> (delete guest cookie)
    else guest
        AuthRoute->>DB: findOrCreateGuestPersona
        AuthRoute->>AuthRoute: signChatGuestToken
        AuthRoute-->>Browser: set chat_participant_token cookie or redirect with ?_t=token
    end
    Browser->>Middleware: subsequent requests (include cookie or ?_t)
    Middleware->>API: forward identity + authMode
    API->>Browser: responses include authMode (credits/chat)
    Browser->>Browser: show Guest badge when authMode === "anonymous"
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Suggested reviewers

  • sjschlapbach
  • jabbadizzleCode
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 24.39% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main feature: LTI semi-anonymous guest access for the chat app, which is the primary objective across multiple modified files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a2ca2cc913

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/chat/src/app/auth/lti/route.ts Outdated
Comment on lines +104 to +113
const participantTokenValid = await hasValidParticipantToken(req)

let decision
try {
decision = await resolveLtiAuthDecision({
ltiSub: ltiPayload.sub,
ltiScope: ltiPayload.scope,
courseId,
hasValidParticipantToken: participantTokenValid,
})
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Bind account-mode decision to the current token subject

The account branch is selected from a boolean (hasValidParticipantToken) and then redirects without updating cookies, so any valid participant_token in the browser is treated as sufficient for decision.mode === 'account'. In a shared/stale-session scenario, an LTI launch for user A can continue under user B's existing participant_token, which breaks identity binding and can expose the wrong participant's chat data if that token is still valid for the target chatbot/course.

Useful? React with 👍 / 👎.

Comment thread apps/chat/src/app/auth/lti/route.ts Outdated
Comment on lines +108 to +112
decision = await resolveLtiAuthDecision({
ltiSub: ltiPayload.sub,
ltiScope: ltiPayload.scope,
courseId,
hasValidParticipantToken: participantTokenValid,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Enforce LTI-course binding before creating participation

The route accepts courseId/chatbotId from query params and only checks that those two match each other, but the verified LTI JWT contains no course claim and is never bound to the requested course. A user with any valid LTI JWT can replay it against another known (courseId, chatbotId) pair and resolveLtiAuthDecision will upsert participation for that course, enabling unauthorized cross-course chatbot access.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a2ca2cc913

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/chat/src/app/auth/lti/route.ts Outdated
Comment on lines +104 to +113
const participantTokenValid = await hasValidParticipantToken(req)

let decision
try {
decision = await resolveLtiAuthDecision({
ltiSub: ltiPayload.sub,
ltiScope: ltiPayload.scope,
courseId,
hasValidParticipantToken: participantTokenValid,
})
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Bind account-mode decision to the current token subject

The account branch is selected from a boolean (hasValidParticipantToken) and then redirects without updating cookies, so any valid participant_token in the browser is treated as sufficient for decision.mode === 'account'. In a shared/stale-session scenario, an LTI launch for user A can continue under user B's existing participant_token, which breaks identity binding and can expose the wrong participant's chat data if that token is still valid for the target chatbot/course.

Useful? React with 👍 / 👎.

Comment thread apps/chat/src/app/auth/lti/route.ts Outdated
Comment on lines +108 to +112
decision = await resolveLtiAuthDecision({
ltiSub: ltiPayload.sub,
ltiScope: ltiPayload.scope,
courseId,
hasValidParticipantToken: participantTokenValid,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Enforce LTI-course binding before creating participation

The route accepts courseId/chatbotId from query params and only checks that those two match each other, but the verified LTI JWT contains no course claim and is never bound to the requested course. A user with any valid LTI JWT can replay it against another known (courseId, chatbotId) pair and resolveLtiAuthDecision will upsert participation for that course, enabling unauthorized cross-course chatbot access.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/chat/src/stores/settingsStore.ts (1)

194-246: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

loadCredits should reset fallback state on error paths, not only log.

At Line 197 and Line 245, the function exits after logging, which can leave stale authMode and model/credit state in the UI.

Suggested fix
       loadCredits: async (chatbotId: string) => {
         try {
           const response = await fetch(`/api/chatbots/${chatbotId}/credits`)
           if (!response.ok) {
             console.error('Failed to load credits:', response.statusText)
+            set((state) => ({
+              credits: { current: 0, total: 0 },
+              modelOptions: [],
+              selectedReasoningEffort: 'none',
+              authMode: 'account',
+              selectedModel: state.selectedModel,
+            }))
             return
           }
@@
         } catch (error) {
           console.error('Error loading credits:', error)
+          set((state) => ({
+            credits: { current: 0, total: 0 },
+            modelOptions: [],
+            selectedReasoningEffort: 'none',
+            authMode: 'account',
+            selectedModel: state.selectedModel,
+          }))
         }
       },

As per coding guidelines, "apps/chat/src/stores/**/*.ts: Zustand store async actions: must set fallback state in catch blocks, not just log, to prevent UI stuck in loading/broken state on network errors".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/chat/src/stores/settingsStore.ts` around lines 194 - 246, loadCredits
currently only logs on non-ok responses and in catch blocks, leaving stale UI
state; update both the non-ok branch and the catch block to call set(...) to
apply a safe fallback state: set credits to {current:0, total:0}, modelOptions
to [], selectedModel to undefined (or automaticModelId if available),
selectedReasoningEffort to
resolveReasoningEffortForModel(state.selectedReasoningEffort, undefined), and
authMode to a default (e.g., 'anonymous'); do this inside the loadCredits
function so the store is reset whenever fetch fails or an exception is thrown.
🧹 Nitpick comments (2)
apps/chat/src/components/app-sidebar.tsx (1)

18-18: ⚡ Quick win

Use path alias for the new store import.

Line 18 adds a relative import, but this repo expects @/~ aliases for TS/JS imports.

Suggested fix
-import { useSettingsStore } from '../stores/settingsStore'
+import { useSettingsStore } from '@/src/stores/settingsStore'

As per coding guidelines, "**/*.{ts,tsx,js,jsx}: Import paths: use @ and ~ path aliases for imports".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/chat/src/components/app-sidebar.tsx` at line 18, Replace the relative
import of the settings store with the project's path alias: in app-sidebar.tsx
update the import that brings in useSettingsStore (currently from
'../stores/settingsStore') to use the configured alias (e.g.,
'@/stores/settingsStore' or '~/stores/settingsStore') so the module resolution
follows the repo's TypeScript/JS import guidelines.
apps/chat/src/lib/server/apiGuards.ts (1)

6-6: ⚡ Quick win

Use project alias import instead of relative path.

Switch this local import to the configured alias style for consistency with repo conventions.

Suggested patch
-import { type AuthMode, verifyChatGuestToken } from './ltiGuest'
+import { type AuthMode, verifyChatGuestToken } from '@/src/lib/server/ltiGuest'

As per coding guidelines, "**/*.{ts,tsx,js,jsx}: Import paths: use @ and ~ path aliases for imports".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/chat/src/lib/server/apiGuards.ts` at line 6, Replace the relative import
of AuthMode and verifyChatGuestToken from './ltiGuest' with the project's
configured path-alias import (using the @ or ~ alias style) so imports follow
repo conventions; update the import statement that currently references AuthMode
and verifyChatGuestToken to use the alias-based module path instead of the
'./ltiGuest' relative path.
🤖 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/chat/src/app/api/chatbots/`[chatbotId]/chat/route.ts:
- Around line 920-933: Anonymous requests should not call getAutomaticModelId or
rely on CreditsService.getUserCredits; instead, when authMode === 'anonymous'
directly select a fallback model: find an entry in modelRegistry whose id is in
chatbot.allowedModelIds and whose fallback flag is true, set selectedModel to
that id and selectedModelConfig to that entry, and only if no such fallback
exists return the 503 error. Remove or short-circuit the
CreditsService.getUserCredits/getAutomaticModelId path for the anonymous branch
so anonymous users always use a fallback model when available (references:
authMode, CreditsService.getUserCredits, getAutomaticModelId, selectedModel,
selectedModelConfig, modelRegistry, chatbot.allowedModelIds).

In `@apps/chat/src/app/api/chatbots/`[chatbotId]/credits/route.ts:
- Around line 47-50: When authMode === 'anonymous' you currently build
allowedIdsForAuto from availableModels but later the automatic model-selection
path can still pick a global model; change the logic so allowedIdsForAuto is set
to availableModels.map(...) (or [] if none) and then update the automatic
model-selection branch to first check allowedIdsForAuto.length > 0 before
selecting any global/default model; if allowedIdsForAuto is empty, do not fall
back to the global registry—return an explicit no-selection result (or a
400/appropriate error) so anonymous requests cannot get a model id outside the
filtered availableModels. Ensure this touches the allowedIdsForAuto assignment
and the automatic selection code that currently falls back to the global
registry.

In `@apps/chat/src/app/auth/lti/route.ts`:
- Around line 132-135: When decision.mode === 'account' in the LTI route
handler, clear the guest participant cookie before returning
NextResponse.redirect(chatbotUrl) so a stale chat_participant_token cannot later
force authMode:'anonymous'; update the branch around the decision.mode ===
'account' check in route.ts to remove or invalidate the 'chat_participant_token'
cookie (use the same response object you return from NextResponse.redirect) so
the cookie is cleared for the browser and subsequent guard logic cannot
misclassify the user.

In `@apps/chat/src/lib/server/ltiGuest.ts`:
- Around line 204-210: The verifyLtiToken function currently reads issuer from
process.env.APP_ORIGIN_LTI without validating it; add a guard before calling
verifyJWT to throw a clear error if process.env.APP_ORIGIN_LTI is missing
(similar to the existing APP_SECRET check) so issuer enforcement cannot be
bypassed—update verifyLtiToken to read and validate APP_ORIGIN_LTI, throw an
Error if falsy, then pass that value as the issuer to verifyJWT.

In `@apps/chat/src/middleware.ts`:
- Around line 20-29: getChatGuestSecretForMiddleware currently falls back to
deriving a guest secret from APP_SECRET, which conflicts with the signer in
apps/chat/src/lib/server/ltiGuest.ts and weakens trust boundaries; change
getChatGuestSecretForMiddleware so that if APP_CHAT_GUEST_SECRET is not set and
the runtime is production (e.g., process.env.NODE_ENV === 'production' or your
platform production flag), it returns null instead of deriving from APP_SECRET,
while keeping the existing derivation behavior only for non-production
environments and still honoring cachedDerivedSecret when appropriate.

---

Outside diff comments:
In `@apps/chat/src/stores/settingsStore.ts`:
- Around line 194-246: loadCredits currently only logs on non-ok responses and
in catch blocks, leaving stale UI state; update both the non-ok branch and the
catch block to call set(...) to apply a safe fallback state: set credits to
{current:0, total:0}, modelOptions to [], selectedModel to undefined (or
automaticModelId if available), selectedReasoningEffort to
resolveReasoningEffortForModel(state.selectedReasoningEffort, undefined), and
authMode to a default (e.g., 'anonymous'); do this inside the loadCredits
function so the store is reset whenever fetch fails or an exception is thrown.

---

Nitpick comments:
In `@apps/chat/src/components/app-sidebar.tsx`:
- Line 18: Replace the relative import of the settings store with the project's
path alias: in app-sidebar.tsx update the import that brings in useSettingsStore
(currently from '../stores/settingsStore') to use the configured alias (e.g.,
'@/stores/settingsStore' or '~/stores/settingsStore') so the module resolution
follows the repo's TypeScript/JS import guidelines.

In `@apps/chat/src/lib/server/apiGuards.ts`:
- Line 6: Replace the relative import of AuthMode and verifyChatGuestToken from
'./ltiGuest' with the project's configured path-alias import (using the @ or ~
alias style) so imports follow repo conventions; update the import statement
that currently references AuthMode and verifyChatGuestToken to use the
alias-based module path instead of the './ltiGuest' relative path.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3141f060-5293-48cf-96c6-4d5649ca7851

📥 Commits

Reviewing files that changed from the base of the PR and between ee4dbd5 and a2ca2cc.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (12)
  • apps/chat/package.json
  • apps/chat/src/app/api/chatbots/[chatbotId]/chat/route.ts
  • apps/chat/src/app/api/chatbots/[chatbotId]/credits/route.ts
  • apps/chat/src/app/auth/lti/route.ts
  • apps/chat/src/app/noLogin/page.tsx
  • apps/chat/src/components/app-sidebar.tsx
  • apps/chat/src/lib/server/apiGuards.ts
  • apps/chat/src/lib/server/ltiGuest.ts
  • apps/chat/src/middleware.ts
  • apps/chat/src/stores/settingsStore.ts
  • apps/chat/test/lti-guest.test.ts
  • turbo.json

Comment thread apps/chat/src/app/api/chatbots/[chatbotId]/chat/route.ts
Comment thread apps/chat/src/app/api/chatbots/[chatbotId]/credits/route.ts Outdated
Comment thread apps/chat/src/app/auth/lti/route.ts
Comment thread apps/chat/src/lib/server/ltiGuest.ts Outdated
Comment on lines +20 to +29
async function getChatGuestSecretForMiddleware(): Promise<string | null> {
if (process.env.APP_CHAT_GUEST_SECRET) {
return process.env.APP_CHAT_GUEST_SECRET
}

if (cachedDerivedSecret) return cachedDerivedSecret

const appSecret = process.env.APP_SECRET
if (!appSecret) return null

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fail closed in production when APP_CHAT_GUEST_SECRET is missing.

This middleware still derives a guest secret from APP_SECRET in production, but the signer path (apps/chat/src/lib/server/ltiGuest.ts:37-51) explicitly forbids that. The mismatch weakens guest-token trust boundaries.

Suggested patch
 async function getChatGuestSecretForMiddleware(): Promise<string | null> {
   if (process.env.APP_CHAT_GUEST_SECRET) {
     return process.env.APP_CHAT_GUEST_SECRET
   }
+  if (process.env.NODE_ENV === 'production') {
+    return null
+  }

   if (cachedDerivedSecret) return cachedDerivedSecret

   const appSecret = process.env.APP_SECRET
   if (!appSecret) return null
📝 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.

Suggested change
async function getChatGuestSecretForMiddleware(): Promise<string | null> {
if (process.env.APP_CHAT_GUEST_SECRET) {
return process.env.APP_CHAT_GUEST_SECRET
}
if (cachedDerivedSecret) return cachedDerivedSecret
const appSecret = process.env.APP_SECRET
if (!appSecret) return null
async function getChatGuestSecretForMiddleware(): Promise<string | null> {
if (process.env.APP_CHAT_GUEST_SECRET) {
return process.env.APP_CHAT_GUEST_SECRET
}
if (process.env.NODE_ENV === 'production') {
return null
}
if (cachedDerivedSecret) return cachedDerivedSecret
const appSecret = process.env.APP_SECRET
if (!appSecret) return null
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/chat/src/middleware.ts` around lines 20 - 29,
getChatGuestSecretForMiddleware currently falls back to deriving a guest secret
from APP_SECRET, which conflicts with the signer in
apps/chat/src/lib/server/ltiGuest.ts and weakens trust boundaries; change
getChatGuestSecretForMiddleware so that if APP_CHAT_GUEST_SECRET is not set and
the runtime is production (e.g., process.env.NODE_ENV === 'production' or your
platform production flag), it returns null instead of deriving from APP_SECRET,
while keeping the existing derivation behavior only for non-production
environments and still honoring cachedDerivedSecret when appropriate.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Implements Phase A of “semi-anonymous” LTI guest access for the chat app, allowing LTI-verified users to access the tutor chatbot without a Klicker account, while keeping guest identity unlinkable from the LTI sub and isolating guest auth from backend GraphQL.

Changes:

  • Adds server-side guest persona derivation/creation and LTI JWT verification, plus a dedicated guest JWT (chat_participant_token) and /auth/lti entry route.
  • Extends auth plumbing to return authMode (account vs anonymous) and updates UI to reflect guest mode.
  • Applies guest restrictions to model selection (fallback-only) and fixes model/credits consistency issues.

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
turbo.json Adds new env vars (APP_CHAT_GUEST_SECRET, CHAT_GUEST_SEED) to Turbo global env passthrough.
pnpm-lock.yaml Lockfile updates and workspace dep resolution changes.
apps/chat/package.json Declares @klicker-uzh/util as a workspace dependency for chat.
apps/chat/test/lti-guest.test.ts Adds unit tests for guest SSO derivation and token verification logic.
apps/chat/src/lib/server/ltiGuest.ts Introduces core LTI guest logic: SSO derivation, persona create/find, guest token + LTI token verification, auth decision resolver.
apps/chat/src/app/auth/lti/route.ts New /auth/lti route to verify LTI JWT, enforce chatbot↔course mapping, and set guest cookie when needed.
apps/chat/src/lib/server/apiGuards.ts Adds guest-cookie-first auth and returns authMode to callers.
apps/chat/src/middleware.ts Adds dual-cookie auth check (guest then account), /auth/lti bypass, and LTI-aware redirect-to-/noLogin.
apps/chat/src/app/api/chatbots/[chatbotId]/credits/route.ts Filters available models for guests and returns authMode with credits payload.
apps/chat/src/app/api/chatbots/[chatbotId]/chat/route.ts Enforces fallback-only model usage for anonymous guests as a final override.
apps/chat/src/stores/settingsStore.ts Stores authMode in settings state based on credits response.
apps/chat/src/components/app-sidebar.tsx Shows a “Guest” badge in the sidebar when in anonymous mode.
apps/chat/src/app/noLogin/page.tsx Adds LTI-specific messaging (?lti=1) for invalid/expired LTI sessions.
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +106 to +114
const randomSuffix = randomBytes(8).toString('hex')
const randomPassword = randomBytes(16).toString('hex')

try {
const created = await prisma.participant.create({
data: {
username: `guest_${randomSuffix}`,
password: randomPassword,
email: null,
Comment thread apps/chat/src/lib/server/ltiGuest.ts Outdated
Comment on lines +208 to +209
const payload = await verifyJWT(token, appSecret, {
issuer: process.env.APP_ORIGIN_LTI,
Comment on lines +249 to +268
const realAccount = await prisma.participantAccount.findFirst({
where: { ssoId: ltiSub, NOT: { type: GUEST_ACCOUNT_TYPE } },
select: { participantId: true },
})

if (realAccount && hasValidParticipantToken) {
await prisma.participation.upsert({
where: {
courseId_participantId: {
courseId,
participantId: realAccount.participantId,
},
},
create: {
course: { connect: { id: courseId } },
participant: { connect: { id: realAccount.participantId } },
},
update: {},
})
return { mode: 'account', participantId: realAccount.participantId }
Comment on lines +27 to +31
const appSecret = process.env.APP_SECRET
if (!appSecret) return null

const encoder = new TextEncoder()
const cryptoKey = await crypto.subtle.importKey(
Comment thread apps/chat/src/middleware.ts Outdated
Comment on lines 115 to 118
await jwtVerify(
participantToken || '',
participantToken,
new TextEncoder().encode(process.env.APP_SECRET || '')
)
Comment thread apps/chat/src/app/auth/lti/route.ts Outdated
response.cookies.set('chat_participant_token', chatGuestToken, {
httpOnly: true,
secure: isProduction,
sameSite: 'lax',
Comment on lines +924 to +927
selectedModel = getAutomaticModelId(
userCredits,
chatbot.allowedModelIds as string[]
)
@cypress
Copy link
Copy Markdown

cypress Bot commented May 4, 2026

klicker-uzh    Run #6740

Run Properties:  status check passed Passed #6740  •  git commit 6711277f42 ℹ️: Merge 2dfd42b2b1ec3d3351fdc55b892d11d4b6acf9c7 into b38895a57a9f7e59deea9f706f62...
Project klicker-uzh
Branch Review claude/condescending-swartz-993a42
Run status status check passed Passed #6740
Run duration 11m 15s
Commit git commit 6711277f42 ℹ️: Merge 2dfd42b2b1ec3d3351fdc55b892d11d4b6acf9c7 into b38895a57a9f7e59deea9f706f62...
Committer Roland Schläfli
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 1
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 758
View all changes introduced in this branch ↗︎

rschlaefli added 2 commits May 9, 2026 21:07
Combine LTI guest access (Phase A, #5083) with student practice MCP from
v3-ai (#5090). Resolved apps/chat/package.json (keep both
@klicker-uzh/types and @klicker-uzh/util deps), chat/route.ts (combine
authMode + chatbot:authChatbot destructuring of withChatbotAuth result),
and regenerated pnpm-lock.yaml.
- Bind account-mode to participant_token subject (F2). resolveLtiAuthDecision
  now takes the verified `sub` from `participant_token`, not a boolean. The
  account branch only fires when `realAccount.participantId` matches the
  cookie's sub, preventing cross-participant cookie reuse on shared browsers.
- Clear `chat_participant_token` on account branch (F5). Without this, the
  guest-first middleware order kept forcing `authMode='anonymous'` after the
  account redirect when a stale guest cookie was present.
- Pick a fallback directly for anonymous users in chat/route.ts (F3).
  Previously called `getAutomaticModelId(credits, chatbot.allowedModelIds)`,
  which returns the *primary* (non-fallback) when credits>0; this then re-
  tripped the `!fallback` check and returned 503 for anonymous users with
  positive credits.
- Return null `automaticModelId` for anonymous when no fallbacks remain (F4).
  Previously fell through to a global-registry id that contradicted the empty
  filtered `availableModels` list.
- Throw in production when `APP_ORIGIN_LTI` is unset (F9). jose silently skips
  the `iss` check when issuer is `undefined`, which would let any
  APP_SECRET-signed token be accepted.
- Mirror the server-side guest-secret rule in middleware (F7). The
  middleware's APP_SECRET-derived fallback is forbidden in production, where
  the signer (`getChatGuestSecret`) refuses the same fallback for trust-
  domain separation. Asymmetric trust would otherwise let the middleware
  accept tokens the signer cannot produce.
- Fail closed on missing `APP_SECRET` in middleware (F12). The previous
  `|| ''` empty-key fallback was not a meaningful gate.

Add a vitest case for the production APP_ORIGIN_LTI guard in verifyLtiToken.

Defers F1 (LTI JWT lacks course-context binding — requires apps/lti changes,
tracked for Phase A.5) and F13 (cookie SameSite=lax for LMS iframe
embedding — platform-wide convention is Lax; deployment-specific change).

Defers F8 (random plaintext password on guest persona — high-entropy random
bytes, no exploit; cosmetic inconsistency only).
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 10, 2026

Confidence Score: 4/5

Safe to merge with awareness: the CHIPS-fallback URL token handoff exposes a 14-day credential to server logs, and the bootstrap hook's useEffect timing can cause a blank-model first render for CHIPS-unsupported browser users.

The core auth paths (LTI JWT verification with issuer enforcement, separate signing secret, guest-first token ordering, anonymous model lock placement) are well-designed and thoroughly unit-tested. Two non-blocking concerns remain: the 14-day ?_t= JWT is captured in server/CDN access logs before the client strips it, and the useEffect-only bootstrap of sessionStorage can race against initial API calls on CHIPS-fallback first loads, leaving guest users briefly with an empty model list until router.replace triggers a re-render.

apps/chat/src/hooks/useChatGuestTokenBootstrap.ts (useEffect timing on first CHIPS-fallback load) and apps/chat/src/app/auth/lti/route.ts (14-day JWT in redirect URL logged server-side)

Important Files Changed

Filename Overview
apps/chat/src/lib/server/ltiGuest.ts New module — HMAC-based guest SSO ID derivation, race-safe find-or-create persona, separate-secret chat-guest JWT, LTI JWT verifier with iss enforcement, and pure auth-decision resolver. Logic is sound; previously-noted P2002 race concern (username vs ssoId) is addressed via isUniqueViolationOn helper.
apps/chat/src/app/auth/lti/route.ts New LTI entry route — verifies JWT, validates chatbot/course binding, branches account vs guest, sets/clears chat_participant_token. The 14-day JWT in the ?_t= fallback URL is logged by every upstream proxy before client strips it.
apps/chat/src/lib/server/apiGuards.ts Adds dual-token auth (chat_participant_token first, then participant_token) with Authorization header fallback for CHIPS-unsupported path; authMode propagated to callers. Cookie/header ordering and fall-through logic are correct.
apps/chat/src/middleware.ts Extended to verify chat-guest token before participant_token; adds /auth/lti bypass and ?_t= query fallback; replaces empty-string APP_SECRET fallback with fail-closed guard. Web Crypto HMAC derivation for edge runtime is correct.
apps/chat/src/hooks/useChatGuestTokenBootstrap.ts Bootstraps ?_t= token into sessionStorage and strips URL via router.replace; relies entirely on useEffect timing, creating a potential race where API calls fire before the token is stored on first CHIPS-fallback load.
apps/chat/src/app/api/chatbots/[chatbotId]/credits/route.ts Anonymous users restricted to fallback models; automaticModelId computed from filtered availableModels for anonymous path, fixing the previously noted inconsistency where getAutomaticModelId could return a non-guest model.
apps/chat/src/app/api/chatbots/[chatbotId]/chat/route.ts Anonymous model lock now runs after all account-mode model resolution branches, fixing the bypass where !modelSelection could overwrite the guest fallback assignment.
packages/util/src/auth.ts Adds extractBearerToken, cookieSecurityOptions, cookiesAvailableViaLtiProbe, and LTI_PROBE_COOKIE_NAME; centralises cookie security logic that was previously inlined in each app.
packages/util/src/clientAuth.ts New client-auth utilities: getStoredAuthToken, createAuthedFetch, bootstrapTokenFromUrl — all use a sessionStorage abstraction safe in SSR/edge contexts. Well-tested.
apps/frontend-pwa/src/lib/getParticipantToken.ts Refactored to use cookiesAvailableViaLtiProbe and cookieSecurityOptions; adds appendCookieAttribute to patch the nookies-generated Set-Cookie header with Partitioned when CHIPS is active (workaround for nookies not supporting Partitioned).
apps/chat/src/stores/settingsStore.ts authMode added to store, populated from credits response; getCreditsFallbackState added to both catch and non-ok response paths; authedFetch used for all API calls.
apps/chat/src/components/NoLoginSelfHeal.tsx Client-only self-heal: reads sessionStorage token and reattaches ?_t= when the user lands on /noLogin with a valid stored token; includes correct same-origin validation before redirect.

Sequence Diagram

sequenceDiagram
    participant LMS
    participant LTI as apps/lti
    participant Chat as apps/chat /auth/lti
    participant MW as Middleware
    participant API as /api/chatbots/*/credits|chat
    participant DB as Prisma DB

    LMS->>LTI: LTI 1.3 launch
    LTI->>LMS: "redirect to /auth/lti?jwt=&courseId=&chatbotId="

    LMS->>Chat: "GET /auth/lti?jwt="
    Chat->>Chat: "verifyLtiToken (APP_SECRET, iss=APP_ORIGIN_LTI)"
    Chat->>DB: findUnique course + chatbot (cross-course check)
    Chat->>DB: findFirst realAccount (ltiSub, NOT lti_guest)

    alt Real account + matching participant_token
        Chat->>Chat: "mode = account, clear chat_participant_token"
        Chat-->>LMS: "302 to /{chatbotId} (participant_token cookie)"
    else Guest path
        Chat->>DB: findOrCreateGuestPersona (HMAC ssoId, P2002-safe)
        Chat->>Chat: signChatGuestToken (APP_CHAT_GUEST_SECRET)
        alt Cookies available (CHIPS probe)
            Chat-->>LMS: "302 to /{chatbotId} (chat_participant_token cookie)"
        else CHIPS-unsupported browser
            Chat-->>LMS: "302 to /{chatbotId}?_t=JWT"
        end
    end

    LMS->>MW: "GET /{chatbotId}"
    MW->>MW: verifyChatGuestToken OR jwtVerify participant_token
    MW-->>LMS: 200 pass through

    Note over LMS: useChatGuestTokenBootstrap stores _t to sessionStorage, strips URL

    LMS->>API: GET /credits (cookie or Authorization: Bearer)
    API->>API: getParticipantId, authMode: anonymous
    API->>DB: getUserCredits
    API->>API: filter availableModels to fallback only
    API-->>LMS: availableModels, automaticModelId, authMode: anonymous

    LMS->>API: POST /chat (cookie or Bearer)
    API->>API: withChatbotAuth, authMode: anonymous
    Note over API: anonymous lock: selectedModel to fallback after all account branches
    API-->>LMS: streamed response
Loading

Fix All in Codex Fix All in Claude Code

Reviews (5): Last reviewed commit: "docs(auth): plan LTI course binding foll..." | Re-trigger Greptile

Comment thread apps/chat/src/app/auth/lti/route.ts Outdated
Comment thread apps/chat/src/lib/server/ltiGuest.ts
total: 0.0,
},
modelSelectionEnabled: false,
authMode: 'account' as AuthMode,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 authMode initialises to 'account' before loadCredits resolves

authMode is intentionally excluded from partialize so it is never persisted, but it defaults to 'account' on every mount. Any component that renders before loadCredits completes — including app-sidebar.tsx's "Guest" badge — will briefly show the account state. For most users the flicker is cosmetic, but on slower connections the badge could appear/disappear noticeably. Initialising to null (or a third state like 'loading') and gating badge rendering on a non-null check would avoid the flash.

Lets `chat_participant_token` survive third-party iframe contexts on
Chrome/Edge/Firefox/Safari (LTI launch into LMS frame). Two-prong:

1. CHIPS (Partitioned cookies). Set `Partitioned; Secure; SameSite=None`
   on `chat_participant_token` in production. Browser keeps the cookie
   keyed by top-level site, so per-LMS partitioning is enforced at the
   browser layer in addition to the per-(ltiSub, courseId) HMAC. Modern
   browsers: Chrome 114+, Edge 114+, Firefox 141+, Safari 26.2+.
2. sessionStorage fallback for browsers without CHIPS (older Safari,
   pre-141 Firefox, locked-down Brave). `/auth/lti` probes whether the
   `lti-token` cookie set by `apps/lti` survived the iframe. If not,
   the redirect target gets `?_t=<chatGuestToken>` appended. A client
   bootstrap hook stuffs the token into sessionStorage and removes the
   query via `router.replace`. An `authedFetch` wrapper attaches
   `Authorization: Bearer <token>` to API calls when sessionStorage
   carries a token; cookie-friendly browsers no-op through native fetch.

Server side:
- `apiGuards.getParticipantId` accepts `Authorization: Bearer` in
  addition to the cookie. Also fixes the `APP_SECRET || ''` empty-key
  fallback the middleware patch already addressed.
- `middleware.ts` accepts `?_t=` query and `Authorization: Bearer` for
  both guest and account token paths.

UX:
- `/noLogin` self-heals on full reloads. If middleware redirected the
  user but sessionStorage still has a valid token, a small client
  component reconstructs the redirect target with `?_t=<token>` and
  navigates back. Server-rendered fallback markup stays for the no-
  sessionStorage path.

Approach mirrors PWA's `getParticipantToken` /
`useParticipantToken` pattern (cookies-or-sessionStorage), validated
since OpenOLAT does not implement the LTI 1.3 Platform Storage spec
(`lti_storage_target` / `lti.put_data`), which would have been the
fully cookieless route. The Storage Access API is also unreliable for
LMS-only users (no prior first-party interaction).

Adds vitest coverage for `authedFetch` header attachment + pass-through
behavior. Production rollout: chat first; PWA `participant_token`
gets the same `Partitioned` flag in a follow-up PR once chat CHIPS is
validated in real browsers.
@dosubot dosubot Bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels May 10, 2026
Comment thread apps/chat/src/app/auth/lti/route.ts
Three-tier plan covering chat (already shipped on this branch), PWA
CHIPS migration, and consolidation of the duplicated bearer-token /
lti-token-probe / cookie-security helpers into @klicker-uzh/util.

Documents the OpenOLAT constraint (no LTI Platform Storage spec
support per OpenOLAT/OpenOLAT source search) that forces the
cookies-plus-sessionStorage path, the browser support matrix, the
fallback handoff via short-lived URL query parameter, and the
reverse-proxy / CHIPS attribute considerations for production
rollout. Sequenced as three PRs so PR-A is mechanical refactor,
PR-B introduces Tier 2 helpers alongside PWA migration, PR-C
defers verifyLtiToken consolidation until a third LTI consumer
forces the API shape.
Comment thread apps/chat/src/app/api/chatbots/[chatbotId]/credits/route.ts
Comment thread apps/chat/src/middleware.ts Outdated
Comment thread packages/util/src/auth.ts Fixed
@rschlaefli rschlaefli changed the title feat(apps/chat): LTI semi-anonymous guest access (Phase A) feat(apps/chat): LTI semi-anonymous guest access May 10, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/chat/src/stores/settingsStore.ts (1)

195-250: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

loadCredits catch/!response.ok paths leave stale state.

loadModeOptions resets to defaults on failure, but loadCredits only logs in both the !response.ok branch (Lines 200-203) and the catch (Lines 247-249). After a transient failure the store keeps the previous credits, modelOptions, and—now relevantly—the previous authMode. If a session transitions and loadCredits fails, the UI can keep showing 'anonymous' (or the inverse) from a prior load, which also drives the "Guest" sidebar pill.

♻️ Suggested fix to set explicit fallback state
       loadCredits: async (chatbotId: string) => {
         try {
           const response = await authedFetch(
             `/api/chatbots/${chatbotId}/credits`
           )
           if (!response.ok) {
             console.error('Failed to load credits:', response.statusText)
+            set({
+              credits: { current: 0, total: 0 },
+              authMode: 'account',
+            })
             return
           }
           ...
         } catch (error) {
           console.error('Error loading credits:', error)
+          set({
+            credits: { current: 0, total: 0 },
+            authMode: 'account',
+          })
         }
       },

As per coding guidelines: "Async actions in Zustand stores must set fallback state in catch blocks, not just log".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/chat/src/stores/settingsStore.ts` around lines 195 - 250, The
loadCredits function currently only logs errors on !response.ok and in its
catch, which can leave stale store values (credits, modelOptions, authMode,
selectedModel, selectedReasoningEffort) from a previous session; update
loadCredits (the async function and its error branches) to call set(...) with
explicit safe fallback state when the fetch fails or an exception is
thrown—e.g., reset credits to {current:0,total:0}, modelOptions to [],
selectedModel to undefined or automaticModelId fallback, selectedReasoningEffort
to a resolved default via resolveReasoningEffortForModel, and authMode to a
deterministic value ('anonymous' or 'account') so the UI cannot retain stale
authMode/guest state after failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/chat/scripts/mint-lti-jwt.mjs`:
- Around line 6-14: Add explicit guards before calling signJWT to validate
process.env.APP_SECRET and process.env.APP_ORIGIN_LTI: check each variable and
if missing throw or log a clear error (including which variable is missing) and
exit non‑zero so the script fails fast; then call signJWT as before (the call
where sub, email, scope are passed) once both env vars are present. Ensure error
messages reference APP_SECRET and APP_ORIGIN_LTI and that the signJWT invocation
remains unchanged except that it now uses the validated env values.

In `@apps/frontend-pwa/src/lib/useParticipantToken.ts`:
- Around line 47-55: The router.push call is using the Pages Router API
incorrectly by passing an object as the second argument; update the call to pass
a UrlObject as the first argument so the redirect pathname and query are
preserved. Replace the current router.push(...) invocation that uses redirectTo,
PARTICIPANT_QUERY_KEY and participantToken so the first argument is an object
with pathname: redirectTo and query: { ...router.query, [PARTICIPANT_QUERY_KEY]:
participantToken } (omit the second object argument), ensuring router.push
receives the UrlObject and optional as/options parameters correctly.

---

Outside diff comments:
In `@apps/chat/src/stores/settingsStore.ts`:
- Around line 195-250: The loadCredits function currently only logs errors on
!response.ok and in its catch, which can leave stale store values (credits,
modelOptions, authMode, selectedModel, selectedReasoningEffort) from a previous
session; update loadCredits (the async function and its error branches) to call
set(...) with explicit safe fallback state when the fetch fails or an exception
is thrown—e.g., reset credits to {current:0,total:0}, modelOptions to [],
selectedModel to undefined or automaticModelId fallback, selectedReasoningEffort
to a resolved default via resolveReasoningEffortForModel, and authMode to a
deterministic value ('anonymous' or 'account') so the UI cannot retain stale
authMode/guest state after failures.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bbffc420-a226-4af4-beed-f81c1ef048d1

📥 Commits

Reviewing files that changed from the base of the PR and between b38895a and b65219f.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (33)
  • .gitignore
  • AGENTS.md
  • apps/chat/package.json
  • apps/chat/scripts/mint-lti-jwt.mjs
  • apps/chat/src/app/api/chatbots/[chatbotId]/chat/route.ts
  • apps/chat/src/app/api/chatbots/[chatbotId]/credits/route.ts
  • apps/chat/src/app/auth/lti/route.ts
  • apps/chat/src/app/noLogin/page.tsx
  • apps/chat/src/components/NoLoginSelfHeal.tsx
  • apps/chat/src/components/app-sidebar.tsx
  • apps/chat/src/components/assistant.tsx
  • apps/chat/src/hooks/useChatGuestTokenBootstrap.ts
  • apps/chat/src/hooks/useChatResponse.ts
  • apps/chat/src/lib/api/types.ts
  • apps/chat/src/lib/client/authedFetch.ts
  • apps/chat/src/lib/server/apiGuards.ts
  • apps/chat/src/lib/server/ltiGuest.ts
  • apps/chat/src/middleware.ts
  • apps/chat/src/stores/settingsStore.ts
  • apps/chat/test/authedFetch.test.ts
  • apps/chat/test/lti-guest.test.ts
  • apps/frontend-pwa/src/lib/apollo.ts
  • apps/frontend-pwa/src/lib/getParticipantToken.ts
  • apps/frontend-pwa/src/lib/useParticipantToken.ts
  • packages/util/package.json
  • packages/util/rollup.config.js
  • packages/util/src/auth.ts
  • packages/util/src/clientAuth.ts
  • packages/util/src/index.ts
  • packages/util/test/auth.test.ts
  • packages/util/test/clientAuth.test.ts
  • project/plans_wip/PLAN-chips-iframe-auth-rollout.md
  • turbo.json

Comment thread apps/chat/scripts/mint-lti-jwt.mjs
Comment thread apps/frontend-pwa/src/lib/useParticipantToken.ts Outdated
@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
apps/chat/test/lti-guest.test.ts (1)

79-81: ⚡ Quick win

Tighten negative-path JWT assertions to avoid false-positive passes.

Using .rejects.toBeDefined() is too broad and passes on any rejection value. Prefer .rejects.toThrow() so these security-path tests explicitly assert that the expected Error is thrown.

Suggested diff
-    await expect(
-      mod.verifyChatGuestToken(tokenSignedWithAppSecret)
-    ).rejects.toBeDefined()
+    await expect(
+      mod.verifyChatGuestToken(tokenSignedWithAppSecret)
+    ).rejects.toThrow()
@@
-    await expect(mod.verifyLtiToken(ltiJwt)).rejects.toBeDefined()
+    await expect(mod.verifyLtiToken(ltiJwt)).rejects.toThrow()
@@
-    await expect(mod.verifyLtiToken(ltiJwt)).rejects.toBeDefined()
+    await expect(mod.verifyLtiToken(ltiJwt)).rejects.toThrow()

Also applies to: 114-115, 129-130

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/chat/test/lti-guest.test.ts` around lines 79 - 81, The tests currently
use broad assertions like await
expect(mod.verifyChatGuestToken(tokenSignedWithAppSecret)).rejects.toBeDefined(),
which can false-pass on any rejection; update these negative-path JWT assertions
to assert the specific error by using .rejects.toThrow() (or
.rejects.toThrowErrorMatchingInlineSnapshot()/message when appropriate) for the
verifyChatGuestToken calls (and the other similar assertions in this file) so
the tests explicitly assert that an Error is thrown rather than any rejection
value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@apps/chat/test/lti-guest.test.ts`:
- Around line 79-81: The tests currently use broad assertions like await
expect(mod.verifyChatGuestToken(tokenSignedWithAppSecret)).rejects.toBeDefined(),
which can false-pass on any rejection; update these negative-path JWT assertions
to assert the specific error by using .rejects.toThrow() (or
.rejects.toThrowErrorMatchingInlineSnapshot()/message when appropriate) for the
verifyChatGuestToken calls (and the other similar assertions in this file) so
the tests explicitly assert that an Error is thrown rather than any rejection
value.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 58371ec7-ee57-4b81-a111-8acb3d67e14b

📥 Commits

Reviewing files that changed from the base of the PR and between b65219f and 2dfd42b.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (14)
  • apps/chat/package.json
  • apps/chat/scripts/mint-lti-jwt.mjs
  • apps/chat/src/app/api/chatbots/[chatbotId]/chat/route.ts
  • apps/chat/src/app/api/chatbots/[chatbotId]/credits/route.ts
  • apps/chat/src/app/auth/lti/route.ts
  • apps/chat/src/components/app-sidebar.tsx
  • apps/chat/src/lib/server/apiGuards.ts
  • apps/chat/src/lib/server/ltiGuest.ts
  • apps/chat/src/middleware.ts
  • apps/chat/src/stores/settingsStore.ts
  • apps/chat/test/lti-guest.test.ts
  • apps/frontend-pwa/src/lib/useParticipantToken.ts
  • packages/util/src/auth.ts
  • project/plans_future/PLAN-lti-course-binding.md
✅ Files skipped from review due to trivial changes (1)
  • apps/chat/package.json
🚧 Files skipped from review as they are similar to previous changes (9)
  • apps/chat/scripts/mint-lti-jwt.mjs
  • apps/chat/src/components/app-sidebar.tsx
  • apps/chat/src/stores/settingsStore.ts
  • apps/frontend-pwa/src/lib/useParticipantToken.ts
  • apps/chat/src/middleware.ts
  • packages/util/src/auth.ts
  • apps/chat/src/app/auth/lti/route.ts
  • apps/chat/src/lib/server/ltiGuest.ts
  • apps/chat/src/lib/server/apiGuards.ts

@rschlaefli rschlaefli merged commit 2e3caa2 into v3-ai May 10, 2026
24 of 26 checks passed
@rschlaefli rschlaefli deleted the claude/condescending-swartz-993a42 branch May 10, 2026 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature size:XXL This PR changes 1000+ lines, ignoring generated files.

Development

Successfully merging this pull request may close these issues.

3 participants