feat(auth): Integrate @naap/plugin-server-sdk for authentication with NaaP#131
Merged
Conversation
…update dependencies - Added @naap/plugin-server-sdk as a dependency across multiple plugins for unified authentication. - Implemented createAuthMiddleware in server setups for capacity-planner, developer-api, marketplace, and publisher plugins, enhancing security with token validation. - Updated auth middleware to remove deprecated token handling and streamline user ID extraction from requests. - Adjusted package-lock.json to reflect new dependencies and their versions.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
size/M
📝 WalkthroughWalkthroughConsolidates authentication to the centralized auth service by updating the plugin-server-sdk middleware (making Changes
Sequence DiagramsequenceDiagram
participant Client
participant AuthMiddleware
participant AuthService
participant ExpressHandler
Client->>AuthMiddleware: Request (e.g. POST /.../commit)
AuthMiddleware->>AuthMiddleware: Is path in publicPaths?
alt path not public
AuthMiddleware->>AuthService: Validate token (/api/v1/auth/me)
AuthService-->>AuthMiddleware: Authenticated user
AuthMiddleware->>AuthMiddleware: Attach user -> req.user
AuthMiddleware->>ExpressHandler: next() (with req.user)
else path is public
AuthMiddleware->>ExpressHandler: next() (no auth)
end
ExpressHandler->>ExpressHandler: Use req.user.id for authorization/handlers
ExpressHandler-->>Client: Response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Suggested labels
Suggested reviewers
🚥 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 unit tests (beta)
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 (3)
packages/plugin-server-sdk/src/middleware/auth.ts (1)
202-207:⚠️ Potential issue | 🟠 MajorReturn 503 on auth service outage (not 401).
A failure to reach the auth service is an upstream availability issue, not an authentication failure. The client-side code explicitly clears all stored tokens when it receives a 401 response—returning 401 forAUTH_SERVICE_ERRORwill cause valid tokens to be dropped and prevent retry/backoff logic from engaging. Use 503 (or 504) instead.🔧 Proposed change
- return res.status(401).json({ + return res.status(503).json({ success: false, error: { code: 'AUTH_SERVICE_ERROR', message: 'Unable to validate authorization token' }, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-server-sdk/src/middleware/auth.ts` around lines 202 - 207, The catch block handling failures to reach the auth service (inside middleware/auth.ts where authServiceUrl is used) currently returns res.status(401) which causes clients to clear tokens; change the response status to 503 (or 504) for the AUTH_SERVICE_ERROR path so upstream availability is signaled instead of an auth failure—update the res.status(...) call in the catch block that logs console.error(`[auth] Failed to reach auth service at ${authServiceUrl}:`, err) to return res.status(503). Ensure the JSON body remains { success: false, error: { code: 'AUTH_SERVICE_ERROR', message: 'Unable to validate authorization token' } } so clients can detect the service outage.plugins/community/backend/src/server.ts (1)
221-226:⚠️ Potential issue | 🟠 MajorAdd community read endpoints to
publicRoutesto prevent breaking anonymous access.Restricting
publicRoutesto only['/healthz']makes/community/posts,/tags,/badges,/leaderboard,/stats, and/searchauth-required by the SDK middleware. These endpoints have no auth enforcement in their handlers and are called publicly in the documentation and client code. Either add read endpoints topublicRoutes(following the daydream-video example pattern) or update all clients to provide auth tokens.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/community/backend/src/server.ts` around lines 221 - 226, The createPluginServer call currently sets publicRoutes to only ['/healthz'], which forces SDK auth on public community endpoints; update the publicRoutes array in the createPluginServer invocation (symbol: createPluginServer, property: publicRoutes) to include the read-only community routes ['/community/posts','/community/tags','/community/badges','/community/leaderboard','/community/stats','/community/search'] (or the exact route paths used by handlers like the posts/tags/badges/leaderboard/stats/search handlers) so those endpoints remain accessible anonymously following the daydream-video pattern; ensure route strings match the handler paths exposed by this plugin.plugins/developer-api/backend/src/server.ts (1)
389-395:⚠️ Potential issue | 🟡 MinorBug: In-memory usage fallback returns unscoped data.
The in-memory fallback returns counts for all keys in
inMemoryApiKeysrather than filtering by the authenticateduserId. This is inconsistent with the Prisma path (lines 370-386) which correctly scopes by user.Proposed fix
// Fallback + const userKeys = inMemoryApiKeys.filter(k => k.userId === userId); res.json({ - totalKeys: inMemoryApiKeys.length, - activeKeys: inMemoryApiKeys.filter(k => k.status === 'active').length, + totalKeys: userKeys.length, + activeKeys: userKeys.filter(k => k.status === 'active').length, totalRequests: 0, totalCost: '0.0000', });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/developer-api/backend/src/server.ts` around lines 389 - 395, The in-memory fallback currently aggregates across inMemoryApiKeys for all users; update the block that returns res.json to first filter inMemoryApiKeys by the authenticated userId (e.g., const userKeys = inMemoryApiKeys.filter(k => k.userId === userId)) and then compute totalKeys, activeKeys, totalRequests and totalCost from that userKeys array so the response is scoped the same way as the Prisma branch (referencing inMemoryApiKeys, userId, and the existing res.json payload).
🧹 Nitpick comments (1)
plugins/capacity-planner/backend/src/server.ts (1)
407-416: Consider using authenticated user for comment author.The
/commentsendpoint (lines 407-446) still acceptsauthorfrom the request body, allowing clients to impersonate any author name. Since authentication is now enforced, consider usingreq.userfor the author identity similar to how the commit endpoint was updated.Suggested approach
app.post('/api/v1/capacity-planner/requests/:id/comments', async (req, res) => { try { - const { author, text } = req.body; + const user = (req as any).user; + if (!user?.id) { + return res.status(401).json({ success: false, error: 'Authentication required' }); + } + const author = req.body.author || user.email || user.id; + const { text } = req.body;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/capacity-planner/backend/src/server.ts` around lines 407 - 416, The comments endpoint app.post('/api/v1/capacity-planner/requests/:id/comments') is trusting author from req.body which allows impersonation; change it to use the authenticated user from req.user (like the commit endpoint does) when creating the prisma.capacityRequestComment record — remove or ignore the body author, derive author identity from req.user (e.g., req.user.name or req.user.email or req.user.id depending on your auth shape), validate req.user exists and fall back to a safe default like 'Anonymous' only if absolutely necessary, and pass that value into prisma.capacityRequestComment.create instead of the incoming author field.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/plugin-publisher/backend/src/server.ts`:
- Around line 97-99: The auth middleware registration using createAuthMiddleware
currently only lists '/healthz' as a public path, causing authenticated blocking
of plugin bundle requests; update the publicPaths passed to createAuthMiddleware
in the app.use call (the createAuthMiddleware(...) invocation) to include
'/static' (or a prefix match like '/static/*' if supported) so requests to
/static/* bypass authentication and reach the static file handler and CORS
logic.
---
Outside diff comments:
In `@packages/plugin-server-sdk/src/middleware/auth.ts`:
- Around line 202-207: The catch block handling failures to reach the auth
service (inside middleware/auth.ts where authServiceUrl is used) currently
returns res.status(401) which causes clients to clear tokens; change the
response status to 503 (or 504) for the AUTH_SERVICE_ERROR path so upstream
availability is signaled instead of an auth failure—update the res.status(...)
call in the catch block that logs console.error(`[auth] Failed to reach auth
service at ${authServiceUrl}:`, err) to return res.status(503). Ensure the JSON
body remains { success: false, error: { code: 'AUTH_SERVICE_ERROR', message:
'Unable to validate authorization token' } } so clients can detect the service
outage.
In `@plugins/community/backend/src/server.ts`:
- Around line 221-226: The createPluginServer call currently sets publicRoutes
to only ['/healthz'], which forces SDK auth on public community endpoints;
update the publicRoutes array in the createPluginServer invocation (symbol:
createPluginServer, property: publicRoutes) to include the read-only community
routes
['/community/posts','/community/tags','/community/badges','/community/leaderboard','/community/stats','/community/search']
(or the exact route paths used by handlers like the
posts/tags/badges/leaderboard/stats/search handlers) so those endpoints remain
accessible anonymously following the daydream-video pattern; ensure route
strings match the handler paths exposed by this plugin.
In `@plugins/developer-api/backend/src/server.ts`:
- Around line 389-395: The in-memory fallback currently aggregates across
inMemoryApiKeys for all users; update the block that returns res.json to first
filter inMemoryApiKeys by the authenticated userId (e.g., const userKeys =
inMemoryApiKeys.filter(k => k.userId === userId)) and then compute totalKeys,
activeKeys, totalRequests and totalCost from that userKeys array so the response
is scoped the same way as the Prisma branch (referencing inMemoryApiKeys,
userId, and the existing res.json payload).
---
Nitpick comments:
In `@plugins/capacity-planner/backend/src/server.ts`:
- Around line 407-416: The comments endpoint
app.post('/api/v1/capacity-planner/requests/:id/comments') is trusting author
from req.body which allows impersonation; change it to use the authenticated
user from req.user (like the commit endpoint does) when creating the
prisma.capacityRequestComment record — remove or ignore the body author, derive
author identity from req.user (e.g., req.user.name or req.user.email or
req.user.id depending on your auth shape), validate req.user exists and fall
back to a safe default like 'Anonymous' only if absolutely necessary, and pass
that value into prisma.capacityRequestComment.create instead of the incoming
author field.
…resources - Updated the createAuthMiddleware configuration to allow access to the "/static" path in addition to the existing "/healthz" path, enhancing the flexibility of the authentication setup.
Contributor
Author
|
@seanhanca this was a requirement I began adding into developer-api when I realized it was a broader pattern issue across all plugins. Let me know if you're ok with merging |
seanhanca
approved these changes
Feb 20, 2026
Contributor
seanhanca
left a comment
seanhanca
left a comment
There was a problem hiding this comment.
thanks for spotting this. it is great find
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes a security limitation where plugins trusted the NaaP platform itself rather than the identity of the authenticated user.
Previously, plugins relied on per-plugin shared secrets configured via .env to authenticate requests from NaaP. While this established platform trust, it did not allow plugins to verify which user initiated a request, preventing per-user authorization, auditing, and policy enforcement.
This change forwards the authenticated user bearer token to plugins and introduces validation using the @naap/plugin-server-sdk auth middleware. Plugins can now assert user identity directly from the token claims instead of relying solely on shared secrets.
This shifts the trust model from platform-level trust to user-scoped identity verification while remaining compatible with existing plugin authentication mechanisms.
Changes
Type
Plugin(s) Affected
Checklist
npm run lint)npm run build)Breaking Changes
None
Screenshots / Recordings
Summary by CodeRabbit
New Features
Bug Fixes