fix: unblock camera for proctored skill tests (Permissions-Policy)#395
fix: unblock camera for proctored skill tests (Permissions-Policy)#395KRISHNA-RATHI-32 wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRefines camera access error messages (distinguishing policy blocks, permission denial, no device, device-in-use), enables camera/microphone via Permissions-Policy, and adds a repository guard to the deploy workflow. ChangesCamera Access Error Handling
CI Workflow Guard
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
client/src/module/student/skill-verification/SkillTestPage.tsx (1)
496-496:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReplace arbitrary text sizes with standard scale classes.
The code uses
text-[11px](line 496) andtext-[10px](lines 745, 752, 759, 766), which violates the coding guideline: "Do not use arbitrary bracket sizes liketext-[17px], use standard scale classes instead."Use
text-xs(0.75rem / 12px) instead for design system consistency. As per coding guidelines, TailwindCSS v4 canonical classes should be used without arbitrary bracket values.📏 Proposed fix: use standard text-xs class
- <p className="text-[11px] text-gray-500 dark:text-gray-400"> + <p className="text-xs text-gray-500 dark:text-gray-400"> {item.label} </p>- <span className="px-2 py-1 bg-red-50 dark:bg-red-900/30 text-red-600 dark:text-red-400 text-[10px] font-semibold rounded-md flex items-center gap-1"> + <span className="px-2 py-1 bg-red-50 dark:bg-red-900/30 text-red-600 dark:text-red-400 text-xs font-semibold rounded-md flex items-center gap-1">Apply the same change to all other
text-[10px]occurrences on lines 752, 759, and 766.Also applies to: 745-745, 752-752, 759-759, 766-766
🤖 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 `@client/src/module/student/skill-verification/SkillTestPage.tsx` at line 496, Replace the arbitrary Tailwind text size utilities in the SkillTestPage component with the standard scale: change all occurrences of text-[11px] and text-[10px] to text-xs to follow the design system; specifically update the paragraph element using text-[11px] and the other small text spans/paragraphs (the occurrences noted in the diff) inside SkillTestPage.tsx so they use text-xs consistently across the component.
🤖 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 `@client/src/module/student/skill-verification/SkillTestPage.tsx`:
- Around line 361-378: Extract the duplicated getUserMedia error-to-message
logic into a shared helper function (e.g., getCameraErrorMessage) and replace
the inline handling in SkillTestPage (where setCameraError is called) and in
useFaceDetection with calls to that helper; specifically, move the err.name
checks for "NotAllowedError", "NotFoundError", and "NotReadableError" plus the
policy keyword inspection into getCameraErrorMessage(err): string, return the
mapped user-facing string, and update the SkillTestPage catch block and the
corresponding catch in useFaceDetection to call
setCameraError(getCameraErrorMessage(err)) (or use the returned string) instead
of duplicating the logic.
---
Outside diff comments:
In `@client/src/module/student/skill-verification/SkillTestPage.tsx`:
- Line 496: Replace the arbitrary Tailwind text size utilities in the
SkillTestPage component with the standard scale: change all occurrences of
text-[11px] and text-[10px] to text-xs to follow the design system; specifically
update the paragraph element using text-[11px] and the other small text
spans/paragraphs (the occurrences noted in the diff) inside SkillTestPage.tsx so
they use text-xs consistently across the component.
🪄 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: 23518da7-85d8-4caf-8bfc-9497a79cc1f9
📒 Files selected for processing (3)
client/src/hooks/useFaceDetection.tsclient/src/module/student/skill-verification/SkillTestPage.tsxclient/vercel.json
| } catch (err: any) { | ||
| let msg: string; | ||
| if (err?.name === "NotAllowedError") { | ||
| const isPolicy = | ||
| err?.message?.toLowerCase().includes("permissions policy") || | ||
| err?.message?.toLowerCase().includes("feature policy"); | ||
| msg = isPolicy | ||
| ? "Camera is blocked by a site security policy. This is a known issue — please contact support." | ||
| : "Camera permission denied. Please allow camera access in your browser settings and try again."; | ||
| } else if (err?.name === "NotFoundError") { | ||
| msg = "No camera detected. Please connect a camera and try again."; | ||
| } else if (err?.name === "NotReadableError") { | ||
| msg = "Camera is in use by another app. Please close it and try again."; | ||
| } else { | ||
| msg = "Camera not available. Please check your device and try again."; | ||
| } | ||
| setCameraError(msg); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Extract duplicated error handling to a shared utility.
The getUserMedia error handling logic here is nearly identical to the code in useFaceDetection.ts (lines 67-81). Both map err.name to user-facing messages and inspect the error message for policy-related keywords. This duplication creates a maintenance burden—if the error detection logic needs updating, it must be changed in both locations.
As per coding guidelines, apply the DRY principle and extract this to a shared helper.
♻️ Suggested refactor: extract to shared utility
Create a new utility file, e.g., client/src/utils/cameraErrors.ts:
export function getCameraErrorMessage(err: any): string {
if (err?.name === "NotAllowedError") {
const isPolicy =
err?.message?.toLowerCase().includes("permissions policy") ||
err?.message?.toLowerCase().includes("feature policy");
return isPolicy
? "Camera blocked by site security policy. Please contact support."
: "Camera permission denied. Please allow camera access in your browser settings and try again.";
} else if (err?.name === "NotFoundError") {
return "No camera found. Please connect a camera and try again.";
} else if (err?.name === "NotReadableError") {
return "Camera is in use by another application. Please close it and try again.";
} else {
return "Camera not available. Please check your device and try again.";
}
}Then replace both occurrences with:
} catch (err: any) {
- let msg: string;
- if (err?.name === "NotAllowedError") {
- const isPolicy =
- err?.message?.toLowerCase().includes("permissions policy") ||
- err?.message?.toLowerCase().includes("feature policy");
- msg = isPolicy
- ? "Camera blocked by site security policy. Please contact support."
- : "Camera permission denied. Please allow camera access in your browser settings and try again.";
- } else if (err?.name === "NotFoundError") {
- msg = "No camera found. Please connect a camera and try again.";
- } else if (err?.name === "NotReadableError") {
- msg = "Camera is in use by another application. Please close it and try again.";
- } else {
- msg = "Camera not available. Please check your device and try again.";
- }
+ const msg = getCameraErrorMessage(err);
setCameraError(msg);🤖 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 `@client/src/module/student/skill-verification/SkillTestPage.tsx` around lines
361 - 378, Extract the duplicated getUserMedia error-to-message logic into a
shared helper function (e.g., getCameraErrorMessage) and replace the inline
handling in SkillTestPage (where setCameraError is called) and in
useFaceDetection with calls to that helper; specifically, move the err.name
checks for "NotAllowedError", "NotFoundError", and "NotReadableError" plus the
policy keyword inspection into getCameraErrorMessage(err): string, return the
mapped user-facing string, and update the SkillTestPage catch block and the
corresponding catch in useFaceDetection to call
setCameraError(getCameraErrorMessage(err)) (or use the returned string) instead
of duplicating the logic.
KRISHNA-RATHI-32
force-pushed
the
fix/camera-permissions-policy
branch
from
634b04d to
442fbe4
Compare
1 participant
Problem
Camera access fails on proctored skill tests with:
Users see "Camera access denied" but it's not a user permission issue — the browser never even shows a permission prompt.
Root Cause
client/vercel.jsonline 27 sets:camera=()blocks camera access at the document level for all origins, including self. This overrides any user permission and causesgetUserMedia()to fail immediately.Changes
client/vercel.jsoncamera=()→camera=(self),microphone=()→microphone=(self)client/src/hooks/useFaceDetection.tsclient/src/module/student/skill-verification/SkillTestPage.tsxSecurity Note
camera=(self)allows camera only for the site's own origin — third-party embeds still cannot access it.geolocation=()andinterest-cohort=()remain fully blocked.Testing
After deploy, verify:
/test/:testId→ click "Enable Camera & Continue" → camera prompt should appearPermissions-Policy: camera=(self), microphone=(self), ...Summary by CodeRabbit
Bug Fixes
Chores