fix: gate settings UI to configured admin DIDs#21
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 42 minutes and 15 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR implements client-side admin access control by introducing a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 3
🧹 Nitpick comments (2)
client/src/app/page.tsx (1)
31-32: Consider centralizing the admin-access check.
isAuthenticated && isAdminDID(session?.did, env.ADMIN_DIDS)is now repeated across pages/components. A smalluseAdminAccess()hook would reduce drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/page.tsx` around lines 31 - 32, Extract the repeated check into a new hook called useAdminAccess that encapsulates the logic currently written as isAuthenticated && isAdminDID(session?.did, env.ADMIN_DIDS): create useAdminAccess() to call useAuth(), compute and return { isAuthenticated, session, hasAdminAccess } or just hasAdminAccess (depending on usage), and replace direct checks in components/pages (e.g., where useAuth() and isAdminDID are used in client/src/app/page.tsx) with calls to useAdminAccess() to centralize admin-access logic and avoid duplication.client/src/lib/env.ts (1)
89-107: Split public client env from server-only env.Client components import
envand accessADMIN_DIDS(and may accessNEXT_PUBLIC_HYPERINDEX_URLin the future). While theenvobject also contains server-only entries (COOKIE_SECRET,ATPROTO_JWK_PRIVATE,HYPERINDEX_URL), Next.js does not inline non-NEXT_PUBLIC_prefixed values into the browser bundle—accessing them client-side returnsundefined. However, explicitly separating public and server-only config into a dedicatedpublicEnvexport prevents accidental boundary drift and makes intent clear.Proposed refactor:
- Create
publicEnvexport with onlyNEXT_PUBLIC_HYPERINDEX_URLandADMIN_DIDS- Remove these from the main
envobject- Update client components to import
publicEnvinstead🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/lib/env.ts` around lines 89 - 107, Split public and server-only configuration by creating a new export publicEnv containing only NEXT_PUBLIC_HYPERINDEX_URL and ADMIN_DIDS, remove those two keys from the existing env object, and keep COOKIE_SECRET, ATPROTO_JWK_PRIVATE and HYPERINDEX_URL in env so server-only values remain server-only; update any client code to import publicEnv instead of env (look for usages of NEXT_PUBLIC_HYPERINDEX_URL and ADMIN_DIDS to change imports) and ensure publicEnv is the only object used in client components while env stays for server code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/.env.example`:
- Around line 10-12: Replace the concrete DID value used for
NEXT_PUBLIC_ADMIN_DIDS with a generic placeholder so examples don't expose a
real admin identity; locate NEXT_PUBLIC_ADMIN_DIDS in the .env.example and
change its value to a clearly generic token such as YOUR_ADMIN_DID_HERE or a
comma-separated list of placeholders (e.g., DID_1_PLACEHOLDER,DID_2_PLACEHOLDER)
so consumers must intentionally replace it with real admin DIDs.
In `@docs/ENV_VARS.md`:
- Line 84: Update the table row for NEXT_PUBLIC_ADMIN_DIDS to explicitly state
it is client-side UI gating only and does not grant server authorization;
mention the corresponding backend env var ADMIN_DIDS must be used for all
server-side authorization checks and that NEXT_PUBLIC_ADMIN_DIDS should be kept
in sync only for UI purposes, not as a substitute for backend validation.
Reference NEXT_PUBLIC_ADMIN_DIDS and ADMIN_DIDS in the sentence so readers
clearly see both variables and add a brief caveat like "Not for server
authorization — always validate admin access against ADMIN_DIDS on the backend."
In `@README.md`:
- Around line 243-244: Update the README note for NEXT_PUBLIC_ADMIN_DIDS to
explicitly state it is client-side only and not an authorization source of
truth: mention that NEXT_PUBLIC_ADMIN_DIDS is exposed in the client bundle and
used solely for UI gating, and that backend ADMIN_DIDS remains the authoritative
authorization source (server-side enforcement must still check ADMIN_DIDS).
---
Nitpick comments:
In `@client/src/app/page.tsx`:
- Around line 31-32: Extract the repeated check into a new hook called
useAdminAccess that encapsulates the logic currently written as isAuthenticated
&& isAdminDID(session?.did, env.ADMIN_DIDS): create useAdminAccess() to call
useAuth(), compute and return { isAuthenticated, session, hasAdminAccess } or
just hasAdminAccess (depending on usage), and replace direct checks in
components/pages (e.g., where useAuth() and isAdminDID are used in
client/src/app/page.tsx) with calls to useAdminAccess() to centralize
admin-access logic and avoid duplication.
In `@client/src/lib/env.ts`:
- Around line 89-107: Split public and server-only configuration by creating a
new export publicEnv containing only NEXT_PUBLIC_HYPERINDEX_URL and ADMIN_DIDS,
remove those two keys from the existing env object, and keep COOKIE_SECRET,
ATPROTO_JWK_PRIVATE and HYPERINDEX_URL in env so server-only values remain
server-only; update any client code to import publicEnv instead of env (look for
usages of NEXT_PUBLIC_HYPERINDEX_URL and ADMIN_DIDS to change imports) and
ensure publicEnv is the only object used in client components while env stays
for server code.
🪄 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: 1f9a92a9-5a68-4280-bfd4-6607a05fb954
📒 Files selected for processing (8)
README.mdclient/.env.exampleclient/src/app/page.tsxclient/src/app/settings/page.tsxclient/src/components/layout/Header.tsxclient/src/lib/env.test.tsclient/src/lib/env.tsdocs/ENV_VARS.md
1 participant
Summary
NEXT_PUBLIC_ADMIN_DIDSTesting
Summary by CodeRabbit
New Features
Documentation