fix: require authentication for GET /api/newsletter stats endpoint (#351)#353
fix: require authentication for GET /api/newsletter stats endpoint (#351)#353Siddh2024 wants to merge 5 commits into
Conversation
|
@Siddh2024 is attempting to deploy a commit to the Darshan Rajput's projects Team on Vercel. A member of the Team first needs to authorize it. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb1b80716e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const { userId } = await auth(); | ||
|
|
||
| if (!userId) { | ||
| return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); | ||
| } |
There was a problem hiding this comment.
For any regular signed-in Clerk user, this check only verifies that auth() returned a userId, so GET /api/newsletter still returns recentSubscribers with subscriber emails/names/sources. Since this handler is marked admin-only and sign-up is public in the app, the sensitive subscriber list remains exposed to all authenticated users unless you also validate an admin role/metadata or move it behind an admin-only route.
Useful? React with 👍 / 👎.
Darshan3690
added
level:advanced
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@Siddh2024 You're right — the current implementation only checks whether the request is authenticated ( Could you please update the implementation to enforce admin authorization (e.g., validate admin role/metadata or use our existing admin access helper) before returning newsletter stats? Once that's addressed, I'll re-review and merge. |
|
@Darshan3690 I've added the admin authorization check as requested. The GET handler now:
This ensures only users with the admin role can access newsletter subscriber data. The update is pushed to the ix/issue-351-newsletter-auth branch. |
|
resolve conflicts. @Siddh2024 |
|
|
|
||
| // GET /api/newsletter/stats - Get newsletter statistics (admin only) | ||
| export async function GET(request: NextRequest) { | ||
| export async function GET(_request: NextRequest) { |
|
|
||
| // GET /api/newsletter/stats - Get newsletter statistics (admin only) | ||
| export async function GET(request: NextRequest) { | ||
| export async function GET(_request: NextRequest) { |
|
@Siddh2024 check copilot comment |
Description
Fixes #351
The \GET /api/newsletter\ endpoint was exposing subscriber data (emails, names, dates, sources) without any authentication. Any visitor could enumerate the full subscriber list.
Changes
Security Impact