Conversation
…orted - admin/stats: parallelize 4 independent stat queries (users, sessions, api-keys, oauth-clients) - admin/users: parallelize data fetch and count query - admin/irc-accounts: parallelize data fetch and count query - admin/xmpp-accounts: parallelize data fetch and count query - admin/mailcow-accounts: parallelize data fetch and count query - routing/permissions: use .toSorted() instead of .sort() to avoid mutating source arrays Applied Vercel React Best Practices (async-parallel, js-tosorted-immutable).
Prosody 13's mod_http_admin_api depends on mod_tokenauth and only accepts Bearer token authentication. Portal was sending HTTP Basic auth (username + password), which always returned 401 Unauthorized. Changes: - Add PROSODY_REST_TOKEN env var for Bearer token auth - Update createAuthHeader() to send Bearer instead of Basic - Update validateXmppConfig/isXmppConfigured to check for token - Keep legacy PROSODY_REST_USERNAME/PASSWORD vars for backward compat Discovered by testing Portal against atl.chat's Prosody 13.0.4 instance.
Two bugs prevented Portal from communicating with Atheme JSONRPC: 1. **id must be a string**: Atheme's jsonrpclib.c validates MOWGLI_JSON_TAG(id) == MOWGLI_JSON_TAG_STRING. Portal sent numeric id (1), causing Atheme to silently drop the request without sending any HTTP response (connection hangs until timeout). Fixed: id: 1 → id: "1" 2. **Empty params rejected**: Atheme's jsonrpcmethod_command checks if (*param == 0) and returns fault_badparams. Portal sent "" as the accountname for unauthenticated commands. Fixed: "" → "." (consistent with the cookie placeholder). Verified end-to-end against live Atheme 7.3.0-rc2: - NickServ REGISTER: ✅ "portaluser is now registered" - atheme.login: ✅ returns authcookie - atheme.ison: ✅ returns online status
Patch to apply to the atl.chat repo (cursor/development-environment-setup-635d branch). Adds mod_http_oauth2 to Prosody for Bearer token generation needed by Portal. Apply with: cd atl.chat git checkout cursor/development-environment-setup-635d git am < patches/atl-chat-mod-http-oauth2.patch
|
Cursor Agent can help with this pull request. Just |
Reviewer's GuideRefactors several admin API routes to parallelize database queries, updates IRC Atheme and XMPP Prosody integrations to satisfy their current auth/protocol requirements, introduces Bearer token-based XMPP auth configuration, and adds Cursor Cloud dev-environment guidance plus an atl.chat Prosody OAuth2 patch, along with corresponding test and doc updates. Sequence diagram for XMPP account management using Prosody Bearer tokensequenceDiagram
actor Admin
participant Portal_Admin_UI
participant Portal_API as Portal_Admin_API
participant Xmpp_Client as Prosody_REST_Client
participant Prosody as Prosody_mod_http_admin_api
Admin->>Portal_Admin_UI: Manage XMPP accounts
Portal_Admin_UI->>Portal_API: GET /api/admin/xmpp-accounts
Portal_API->>Xmpp_Client: listAccounts()
Xmpp_Client->>Xmpp_Client: createAuthHeader()
Xmpp_Client->>Xmpp_Client: read PROSODY_REST_TOKEN from xmppConfig
Xmpp_Client-->>Prosody: HTTP request with Authorization: Bearer token
Prosody-->>Xmpp_Client: 200 OK JSON
Xmpp_Client-->>Portal_API: Parsed account list
Portal_API-->>Portal_Admin_UI: Render accounts
Admin->>Portal_Admin_UI: Create XMPP account
Portal_Admin_UI->>Portal_API: POST /api/admin/xmpp-accounts
Portal_API->>Xmpp_Client: createAccount(jid, password)
Xmpp_Client->>Xmpp_Client: createAuthHeader()
Xmpp_Client-->>Prosody: HTTP POST /rest/user with Bearer token
Prosody-->>Xmpp_Client: 201 Created
Xmpp_Client-->>Portal_API: Success
Portal_API-->>Portal_Admin_UI: Operation successful
Sequence diagram for IRC Atheme JSON-RPC command with string id and placeholder accountsequenceDiagram
actor Admin
participant Portal_Admin_UI
participant Portal_API as Portal_Admin_API
participant Atheme_Client
participant Atheme as Atheme_JSON_RPC
Admin->>Portal_Admin_UI: Create IRC account
Portal_Admin_UI->>Portal_API: POST /api/admin/irc-accounts
Portal_API->>Atheme_Client: registerNick(nick, password, email, sourceIp)
Atheme_Client->>Atheme_Client: athemeCommand(params)
Atheme_Client->>Atheme_Client: Build JsonRpcRequest
Note over Atheme_Client: jsonrpc: 2.0<br/>method: atheme.command<br/>params: [".", ".", sourceIp, "NickServ", "REGISTER", ...]<br/>id: "1"
Atheme_Client-->>Atheme: HTTP POST /jsonrpc
Atheme-->>Atheme_Client: JsonRpcSuccess with id "1"
Atheme_Client-->>Portal_API: Success
Portal_API-->>Portal_Admin_UI: Operation successful
Class diagram for updated XMPP config/auth and IRC Atheme JSON-RPC typesclassDiagram
class XmppEnvKeys {
+string XMPP_DOMAIN
+string PROSODY_REST_URL
+string PROSODY_REST_TOKEN
+string PROSODY_REST_USERNAME
+string PROSODY_REST_PASSWORD
}
class ProsodyConfig {
+string restUrl
+string token
+string username
+string password
}
class XmppConfigModule {
+ProsodyConfig prosody
+void validateXmppConfig()
+boolean isXmppConfigured()
}
class ProsodyAuthClient {
+string createAuthHeader()
}
class JsonRpcRequest {
+string jsonrpc
+string method
+string[] params
+string id
}
class JsonRpcSuccess {
+string jsonrpc
+string result
+string id
}
class JsonRpcObjectSuccess~T~ {
+string jsonrpc
+T result
+string id
}
class JsonRpcError {
+string jsonrpc
+int code
+string message
+string id
}
class AthemeClientModule {
+Promise~T~ athemeRpc(method, params)
+Promise~string~ athemeCommand(params)
+Promise~void~ registerNick(nick, password, email, sourceIp)
+Promise~void~ dropNick(nick, sourceIp)
}
class ProsodyAccountNotFoundError {
+string message
}
XmppEnvKeys <.. XmppConfigModule
XmppConfigModule --> ProsodyConfig
ProsodyConfig <.. ProsodyAuthClient
ProsodyAuthClient <.. XmppConfigModule
AthemeClientModule --> JsonRpcRequest
AthemeClientModule --> JsonRpcSuccess
AthemeClientModule --> JsonRpcObjectSuccess
AthemeClientModule --> JsonRpcError
ProsodyAccountNotFoundError <|-- Error
Flow diagram for parallelized admin API database queries with Promise.allgraph TD
A[Admin requests admin endpoint<br/>GET /api/admin/users or stats] --> B[requireAdminOrStaff]
B --> C[Build query conditions]
C --> D[Start parallel queries with Promise.all]
subgraph Parallel_Users[Example: /api/admin/users]
D --> E[Query users list<br/>select from user with filters]
D --> F[Query users count<br/>select count with same filters]
end
subgraph Parallel_Stats[Example: /api/admin/stats]
D --> G[Select user stats]
D --> H[Select session stats]
D --> I[Select API key stats]
D --> J[Select OAuth client stats]
end
E --> K[Await Promise.all]
F --> K
G --> K
H --> K
I --> K
J --> K
K --> L[Construct JSON response<br/>users, pagination, stats]
L --> M[Send HTTP response]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughThis PR implements Bearer token authentication for Prosody XMPP services via OAuth2 configuration, parallelizes admin API database queries for performance improvement, updates IRC Atheme JSON-RPC message IDs from numeric to string type, and adds corresponding environment variable and test updates. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses critical integration challenges with Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
This PR successfully addresses critical integration issues with IRC (Atheme) and XMPP (Prosody) services. The changes implement necessary protocol fixes and performance improvements.
Key Changes Verified:
- IRC integration fixes (JSON-RPC string IDs and "." placeholders) correctly address Atheme API requirements
- XMPP Bearer token authentication properly replaces deprecated Basic auth for Prosody 13+
- Database query parallelization with Promise.all() improves admin API performance without introducing race conditions
- Array immutability improvements with toSorted() follow best practices
Testing: Unit tests updated to reflect the integration changes. Manual testing recommended to verify end-to-end functionality with live Atheme and Prosody services.
No blocking issues identified. The implementation is clean and well-documented.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new
Promise.allusages in the admin routes (e.g.,stats,users,irc-accounts, etc.) rely on nested array destructuring like[[userStats], [sessionStats], ...], which is a bit opaque at a glance; consider extracting small helper functions or intermediate variables so the shape of each query result is clearer. - Both
validateXmppConfigandcreateAuthHeadernow enforce the presence ofPROSODY_REST_TOKENwith slightly different error messages; it might be worth centralizing this validation in one place to avoid divergent behavior or messages if the auth requirements change again.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `Promise.all` usages in the admin routes (e.g., `stats`, `users`, `irc-accounts`, etc.) rely on nested array destructuring like `[[userStats], [sessionStats], ...]`, which is a bit opaque at a glance; consider extracting small helper functions or intermediate variables so the shape of each query result is clearer.
- Both `validateXmppConfig` and `createAuthHeader` now enforce the presence of `PROSODY_REST_TOKEN` with slightly different error messages; it might be worth centralizing this validation in one place to avoid divergent behavior or messages if the auth requirements change again.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
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)
src/features/integrations/lib/xmpp/client.ts (1)
19-28:⚠️ Potential issue | 🟡 MinorStale documentation: update comment block to reflect Bearer token auth.
The header comment still references HTTP Basic authentication with username/password, but the implementation now uses Bearer token authentication. This inconsistency could confuse future maintainers.
📝 Proposed fix
// ============================================================================ // Prosody REST API Client // ============================================================================ // Client for interacting with Prosody's mod_http_admin_api module -// Uses HTTP Basic authentication with PROSODY_REST_USERNAME:PROSODY_REST_PASSWORD +// Uses Bearer token authentication via mod_tokenauth (PROSODY_REST_TOKEN) // // Documentation: https://modules.prosody.im/mod_http_admin_api.html // Endpoint: PUT {PROSODY_REST_URL}/admin_api/users/{username} // Body: { "password": "..." }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/integrations/lib/xmpp/client.ts` around lines 19 - 28, Update the top comment block in src/features/integrations/lib/xmpp/client.ts (the "Prosody REST API Client" header) to replace references to HTTP Basic auth and PROSODY_REST_USERNAME:PROSODY_REST_PASSWORD with Bearer token authentication (e.g., mention using a PROSODY_REST_TOKEN or Bearer token in the Authorization header), and update the brief usage/endpoint notes to reflect that the client sends Authorization: Bearer <token> for PUT {PROSODY_REST_URL}/admin_api/users/{username}; keep the existing documentation link and endpoint/body example but correct the auth description so it matches the implementation in this module.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENTS.md`:
- Line 134: Update the heading "Cursor Cloud specific instructions" to use a
hyphenated compound modifier by changing it to "Cursor Cloud-specific
instructions" so the title follows correct compound adjective formatting; locate
the heading text in AGENTS.md and replace the existing unhyphenated phrase.
In `@patches/atl-chat-mod-http-oauth2.patch`:
- Around line 56-69: The config enables the "authorization_code" grant but
disables PKCE (oauth2_require_code_challenge = false) and also provides a
hardcoded fallback for oauth2_registration_key; update this by (1) ensuring
oauth2_require_code_challenge is true when "authorization_code" is present in
allowed_oauth2_grant_types (or remove "authorization_code" if PKCE cannot be
enforced), and (2) removing the hardcoded fallback for oauth2_registration_key
so it must come from the environment (fail fast if
Lua.os.getenv("PROSODY_OAUTH2_REGISTRATION_KEY") is nil/empty) to force a
secure, randomly-generated secret in production.
---
Outside diff comments:
In `@src/features/integrations/lib/xmpp/client.ts`:
- Around line 19-28: Update the top comment block in
src/features/integrations/lib/xmpp/client.ts (the "Prosody REST API Client"
header) to replace references to HTTP Basic auth and
PROSODY_REST_USERNAME:PROSODY_REST_PASSWORD with Bearer token authentication
(e.g., mention using a PROSODY_REST_TOKEN or Bearer token in the Authorization
header), and update the brief usage/endpoint notes to reflect that the client
sends Authorization: Bearer <token> for PUT
{PROSODY_REST_URL}/admin_api/users/{username}; keep the existing documentation
link and endpoint/body example but correct the auth description so it matches
the implementation in this module.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
AGENTS.mdpatches/atl-chat-mod-http-oauth2.patchsrc/app/api/admin/irc-accounts/route.tssrc/app/api/admin/mailcow-accounts/route.tssrc/app/api/admin/stats/route.tssrc/app/api/admin/users/route.tssrc/app/api/admin/xmpp-accounts/route.tssrc/features/integrations/lib/irc/atheme/client.tssrc/features/integrations/lib/xmpp/client.tssrc/features/integrations/lib/xmpp/config.tssrc/features/integrations/lib/xmpp/keys.tssrc/features/routing/lib/permissions.tstests/lib/integrations/irc/atheme/client.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Seer Code Review
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Build
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/sentry.mdc)
**/*.{js,jsx,ts,tsx}: UseSentry.captureException(error)to capture exceptions and log errors in Sentry, particularly in try-catch blocks or areas where exceptions are expected
UseSentry.startSpan()function to create spans for meaningful actions within applications like button clicks, API calls, and function calls
Files:
src/app/api/admin/mailcow-accounts/route.tssrc/app/api/admin/users/route.tssrc/features/integrations/lib/xmpp/keys.tssrc/app/api/admin/stats/route.tssrc/features/integrations/lib/xmpp/client.tssrc/app/api/admin/xmpp-accounts/route.tssrc/features/routing/lib/permissions.tstests/lib/integrations/irc/atheme/client.test.tssrc/app/api/admin/irc-accounts/route.tssrc/features/integrations/lib/xmpp/config.tssrc/features/integrations/lib/irc/atheme/client.ts
**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/sentry.mdc)
When creating custom spans for API calls with
Sentry.startSpan(), ensure thenameandopproperties are meaningful (e.g.,op: "http.client"with descriptive names likeGET /api/users/${userId}), and attach attributes based on relevant request information and metrics
Files:
src/app/api/admin/mailcow-accounts/route.tssrc/app/api/admin/users/route.tssrc/features/integrations/lib/xmpp/keys.tssrc/app/api/admin/stats/route.tssrc/features/integrations/lib/xmpp/client.tssrc/app/api/admin/xmpp-accounts/route.tssrc/features/routing/lib/permissions.tstests/lib/integrations/irc/atheme/client.test.tssrc/app/api/admin/irc-accounts/route.tssrc/features/integrations/lib/xmpp/config.tssrc/features/integrations/lib/irc/atheme/client.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/sentry.mdc)
**/*.{js,ts,jsx,tsx}: Import Sentry usingimport * as Sentry from "@sentry/nextjs"when using logs in NextJS projects
Reference the Sentry logger usingconst { logger } = Sentrywhen using logging functionality
Uselogger.fmtas a template literal function to bring variables into structured logs, providing better log formatting and variable interpolation
Files:
src/app/api/admin/mailcow-accounts/route.tssrc/app/api/admin/users/route.tssrc/features/integrations/lib/xmpp/keys.tssrc/app/api/admin/stats/route.tssrc/features/integrations/lib/xmpp/client.tssrc/app/api/admin/xmpp-accounts/route.tssrc/features/routing/lib/permissions.tstests/lib/integrations/irc/atheme/client.test.tssrc/app/api/admin/irc-accounts/route.tssrc/features/integrations/lib/xmpp/config.tssrc/features/integrations/lib/irc/atheme/client.ts
**/*.{css,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/tailwind-v4.mdc)
Use container query support with
@container,@sm:,@md:for container-based breakpoints and@max-md:for max-width queries
Files:
src/app/api/admin/mailcow-accounts/route.tssrc/app/api/admin/users/route.tssrc/features/integrations/lib/xmpp/keys.tssrc/app/api/admin/stats/route.tssrc/features/integrations/lib/xmpp/client.tssrc/app/api/admin/xmpp-accounts/route.tssrc/features/routing/lib/permissions.tstests/lib/integrations/irc/atheme/client.test.tssrc/app/api/admin/irc-accounts/route.tssrc/features/integrations/lib/xmpp/config.tssrc/features/integrations/lib/irc/atheme/client.ts
**/*.{ts,tsx,js,jsx,html}
📄 CodeRabbit inference engine (.cursor/rules/tailwind-v4.mdc)
**/*.{ts,tsx,js,jsx,html}: Use 3D transform utilities:transform-3d,rotate-x-*,rotate-y-*,rotate-z-*,scale-z-*,translate-z-*,perspective-*, andperspective-origin-*
Use linear gradient angles withbg-linear-45syntax and gradient interpolation likebg-linear-to-r/oklchorbg-linear-to-r/srgb
Use conic and radial gradients withbg-conicandbg-radial-[at_25%_25%]utilities
Useinset-shadow-*andinset-ring-*utilities instead of deprecated shadow opacity utilities
Usefield-sizing-contentutility for auto-resizing textareas
Usescheme-lightandscheme-darkutilities forcolor-schemeproperty
Usefont-stretch-*utilities for variable font configuration
Chain variants together for composable variants (e.g.,group-has-data-potato:opacity-100)
Usestartingvariant for@starting-styletransitions
Usenot-*variant for:not()pseudo-class (e.g.,not-first:mb-4)
Useinertvariant for styling elements with theinertattribute
Usenth-*variants:nth-3:,nth-last-5:,nth-of-type-4:,nth-last-of-type-6:for targeting specific elements
Usein-*variant as a simpler alternative togroup-*without addinggroupclass
Useopenvariant to support:popover-openpseudo-class
Use**variant for targeting all descendants
Replace deprecatedbg-opacity-*utilities with color values using slash notation (e.g.,bg-black/50)
Replace deprecatedtext-opacity-*utilities with color values using slash notation (e.g.,text-black/50)
Replace deprecatedborder-opacity-*,divide-opacity-*and similar opacity utilities with color slash notation
Useshadow-xsinstead ofshadow-smandshadow-sminstead ofshadow
Usedrop-shadow-xsinstead ofdrop-shadow-smanddrop-shadow-sminstead ofdrop-shadow
Useblur-xsinstead ofblur-smandblur-sminstead ofblur
Userounded-xsinstead ofrounded-smandrounded-sminstead ofrounded
Useoutline-hiddeninstead ofoutline-nonefor...
Files:
src/app/api/admin/mailcow-accounts/route.tssrc/app/api/admin/users/route.tssrc/features/integrations/lib/xmpp/keys.tssrc/app/api/admin/stats/route.tssrc/features/integrations/lib/xmpp/client.tssrc/app/api/admin/xmpp-accounts/route.tssrc/features/routing/lib/permissions.tstests/lib/integrations/irc/atheme/client.test.tssrc/app/api/admin/irc-accounts/route.tssrc/features/integrations/lib/xmpp/config.tssrc/features/integrations/lib/irc/atheme/client.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/tanstack-query.mdc)
**/*.{ts,tsx}: Create QueryClient using getQueryClient() function that automatically handles server/client isolation (new instance per server request, singleton on client)
Always use the centralized query key factory from src/lib/api/query-keys.ts instead of creating keys manually
Include all variables used in queryFn in the query key to ensure proper cache management and query invalidation
**/*.{ts,tsx}: Use explicit types for function parameters and return values when they enhance clarity
Preferunknownoveranywhen the type is genuinely unknown
Use const assertions (as const) for immutable values and literal types
Leverage TypeScript's type narrowing instead of type assertions
**/*.{ts,tsx}: Use TypeScript strict mode with explicit types; preferunknownoverany
Use functional components and hooks in React code
Use selective imports over barrel exports for performance
Follow Next.js App Router conventions
Always await promises in async functions
Use semantic HTML and ARIA attributes for accessibility
Import authentication operations from@/authmodule using barrel exports
Import database operations from@/dbmodule using barrel exports
Import app configuration from@/configor@/config/appusing barrel exports
Import UI components using direct imports from@/components/ui/*(e.g.,@/components/ui/button)
Import custom hooks using direct imports from@/hooks/*(e.g.,@/hooks/use-permissions)
Centralize major types insrc/types/directory: use@/types/auth,@/types/api,@/types/routes,@/types/email,@/types/commonfor imports
Import major constants from@/lib/utils/constants.ts:USER_ROLES,PERMISSIONS,HTTP_STATUS,API_ERROR_CODES,QUERY_CACHE,RATE_LIMIT,INTEGRATION_STATUSES,PAGINATION,VALIDATION_PATTERNS,MOBILE_BREAKPOINT,DATE_FORMATS
Use TanStack Query for all server state management and data fetching
Optimize images with next/image component instead of HTML img tags
Validate all input...
Files:
src/app/api/admin/mailcow-accounts/route.tssrc/app/api/admin/users/route.tssrc/features/integrations/lib/xmpp/keys.tssrc/app/api/admin/stats/route.tssrc/features/integrations/lib/xmpp/client.tssrc/app/api/admin/xmpp-accounts/route.tssrc/features/routing/lib/permissions.tstests/lib/integrations/irc/atheme/client.test.tssrc/app/api/admin/irc-accounts/route.tssrc/features/integrations/lib/xmpp/config.tssrc/features/integrations/lib/irc/atheme/client.ts
{src/app/**,src/components/**}/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/tanstack-query.mdc)
{src/app/**,src/components/**}/*.{ts,tsx}: Use useUsers(), useUser(), useSessions(), useAdminStats(), useUpdateUser(), useDeleteUser(), useDeleteSession() hooks from src/hooks/use-admin.ts for standard queries
Use corresponding Suspense hooks (useUsersSuspense(), useUserSuspense(), etc.) from src/hooks/use-admin-suspense.ts when wrapping components in Suspense and Error Boundaries
Files:
src/app/api/admin/mailcow-accounts/route.tssrc/app/api/admin/users/route.tssrc/app/api/admin/stats/route.tssrc/app/api/admin/xmpp-accounts/route.tssrc/app/api/admin/irc-accounts/route.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{ts,tsx,js,jsx}: Use meaningful variable names instead of magic numbers - extract constants with descriptive names
Use arrow functions for callbacks and short functions
Preferfor...ofloops over.forEach()and indexedforloops
Use optional chaining (?.) and nullish coalescing (??) for safer property access
Prefer template literals over string concatenation
Use destructuring for object and array assignments
Useconstby default,letonly when reassignment is needed, nevervar
Alwaysawaitpromises in async functions - don't forget to use the return value
Useasync/awaitsyntax instead of promise chains for better readability
Handle errors appropriately in async code with try-catch blocks
Don't use async functions as Promise executors
Use function components over class components
Call hooks at the top level only, never conditionally
Specify all dependencies in hook dependency arrays correctly
Use thekeyprop for elements in iterables (prefer unique IDs over array indices)
Nest children between opening and closing tags instead of passing as props in React components
Don't define components inside other components
Use semantic HTML and ARIA attributes for accessibility: provide meaningful alt text for images, use proper heading hierarchy, add labels for form inputs, include keyboard event handlers alongside mouse events, and use semantic elements instead of divs with roles
Removeconsole.log,debugger, andalertstatements from production code
ThrowErrorobjects with descriptive messages, not strings or other values
Usetry-catchblocks meaningfully - don't catch errors just to rethrow them
Prefer early returns over nested conditionals for error cases
Keep functions focused and under reasonable cognitive complexity limits
Extract complex conditions into well-named boolean variables
Use early returns to reduce nesting
Prefer simple conditionals over nested ternary operators
Group related code together and separate concerns
Add `...
Files:
src/app/api/admin/mailcow-accounts/route.tssrc/app/api/admin/users/route.tssrc/features/integrations/lib/xmpp/keys.tssrc/app/api/admin/stats/route.tssrc/features/integrations/lib/xmpp/client.tssrc/app/api/admin/xmpp-accounts/route.tssrc/features/routing/lib/permissions.tstests/lib/integrations/irc/atheme/client.test.tssrc/app/api/admin/irc-accounts/route.tssrc/features/integrations/lib/xmpp/config.tssrc/features/integrations/lib/irc/atheme/client.ts
**/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/app/**/*.{ts,tsx}: Implement proper error boundaries in Next.js App Router pages and layouts
Use Suspense for loading states in Next.js App Router components
Files:
src/app/api/admin/mailcow-accounts/route.tssrc/app/api/admin/users/route.tssrc/app/api/admin/stats/route.tssrc/app/api/admin/xmpp-accounts/route.tssrc/app/api/admin/irc-accounts/route.ts
**/lib/*/keys.ts
📄 CodeRabbit inference engine (AGENTS.md)
Define module environment variables using t3-env
createEnvwith Zod schemas in module-levelkeys.tsfiles
Files:
src/features/integrations/lib/xmpp/keys.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{test,spec}.{ts,tsx,js,jsx}: Write assertions insideit()ortest()blocks
Avoid done callbacks in async tests - use async/await instead
Don't use.onlyor.skipin committed code
Keep test suites reasonably flat - avoid excessivedescribenesting
Files:
tests/lib/integrations/irc/atheme/client.test.ts
🧠 Learnings (20)
📚 Learning: 2025-12-18T18:18:05.202Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/anti-slop.mdc:0-0
Timestamp: 2025-12-18T18:18:05.202Z
Learning: Applies to **/*route*.{ts,tsx,js,jsx} : Clean, non-duplicated REST routes in backend code
Applied to files:
src/app/api/admin/mailcow-accounts/route.tssrc/app/api/admin/users/route.tssrc/app/api/admin/stats/route.tssrc/features/routing/lib/permissions.ts
📚 Learning: 2026-02-01T01:00:48.727Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-01T01:00:48.727Z
Learning: Portal is a centralized hub and identity management system for ATL community that provisions and manages access to all ATL services through a single identity
Applied to files:
AGENTS.md
📚 Learning: 2026-01-15T06:16:09.034Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/tanstack-query.mdc:0-0
Timestamp: 2026-01-15T06:16:09.034Z
Learning: Applies to src/app/**/*.{tsx} : Prefetch only critical data that users will definitely see, using Promise.all() for parallel prefetching instead of sequential await
Applied to files:
src/app/api/admin/users/route.tssrc/app/api/admin/stats/route.tssrc/app/api/admin/xmpp-accounts/route.tssrc/app/api/admin/irc-accounts/route.ts
📚 Learning: 2026-01-15T06:16:09.034Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/tanstack-query.mdc:0-0
Timestamp: 2026-01-15T06:16:09.034Z
Learning: Applies to {src/app/**,src/components/**}/*.{ts,tsx} : Use useUsers(), useUser(), useSessions(), useAdminStats(), useUpdateUser(), useDeleteUser(), useDeleteSession() hooks from src/hooks/use-admin.ts for standard queries
Applied to files:
src/app/api/admin/users/route.tssrc/app/api/admin/stats/route.ts
📚 Learning: 2026-01-15T06:16:30.014Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2026-01-15T06:16:30.014Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `async/await` syntax instead of promise chains for better readability
Applied to files:
src/app/api/admin/users/route.ts
📚 Learning: 2026-02-01T01:00:48.727Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-01T01:00:48.727Z
Learning: Applies to **/*.{ts,tsx} : Import authentication operations from `@/auth` module using barrel exports
Applied to files:
src/app/api/admin/users/route.tssrc/features/integrations/lib/xmpp/client.ts
📚 Learning: 2026-01-15T06:16:09.034Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/tanstack-query.mdc:0-0
Timestamp: 2026-01-15T06:16:09.034Z
Learning: Applies to {src/app/**,src/components/**}/*.{tsx} : Use useSuspenseQueries for parallel queries to avoid waterfalls and improve performance in Server Components and Client Components
Applied to files:
src/app/api/admin/users/route.ts
📚 Learning: 2026-02-01T01:00:48.728Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-01T01:00:48.728Z
Learning: Applies to **/lib/*/keys.ts : Define module environment variables using t3-env `createEnv` with Zod schemas in module-level `keys.ts` files
Applied to files:
src/features/integrations/lib/xmpp/keys.ts
📚 Learning: 2026-02-01T01:00:48.728Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-01T01:00:48.728Z
Learning: Applies to **/*.{ts,tsx} : Never expose API keys in client code; keep sensitive credentials server-side only
Applied to files:
src/features/integrations/lib/xmpp/keys.ts
📚 Learning: 2026-02-01T01:00:48.728Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-01T01:00:48.728Z
Learning: Applies to src/env.ts : Centralize all module environment variables in main `src/env.ts` using `extends` to combine module `keys()` functions
Applied to files:
src/features/integrations/lib/xmpp/keys.ts
📚 Learning: 2026-01-15T06:16:09.034Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/tanstack-query.mdc:0-0
Timestamp: 2026-01-15T06:16:09.034Z
Learning: Applies to **/*.{ts,tsx} : Always use the centralized query key factory from src/lib/api/query-keys.ts instead of creating keys manually
Applied to files:
src/features/integrations/lib/xmpp/keys.ts
📚 Learning: 2025-12-18T18:18:05.202Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/anti-slop.mdc:0-0
Timestamp: 2025-12-18T18:18:05.202Z
Learning: Applies to **/config*.{ts,tsx,js,jsx} : ENV must be validated at startup in backend
Applied to files:
src/features/integrations/lib/xmpp/keys.tssrc/features/integrations/lib/xmpp/config.ts
📚 Learning: 2025-12-18T18:18:05.202Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/anti-slop.mdc:0-0
Timestamp: 2025-12-18T18:18:05.202Z
Learning: Applies to **/backend/**/*.{ts,tsx,js,jsx} : Use a single HTTP client and single DB client (no parallel stacks) in backend
Applied to files:
src/app/api/admin/stats/route.ts
📚 Learning: 2026-01-15T06:16:09.034Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/tanstack-query.mdc:0-0
Timestamp: 2026-01-15T06:16:09.034Z
Learning: Applies to src/lib/api/server-queries.ts : Server-side query functions in src/lib/api/server-queries.ts must use direct database access via ORM instead of HTTP requests
Applied to files:
src/app/api/admin/stats/route.ts
📚 Learning: 2026-01-15T06:16:09.034Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/tanstack-query.mdc:0-0
Timestamp: 2026-01-15T06:16:09.034Z
Learning: Applies to src/app/**/*.{tsx} : In Server Components, create a QueryClient per request using getServerQueryClient(), prefetch data in parallel with Promise.all(), and dehydrate using dehydrate(queryClient)
Applied to files:
src/app/api/admin/stats/route.tssrc/app/api/admin/irc-accounts/route.ts
📚 Learning: 2026-02-01T01:00:48.727Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-01T01:00:48.727Z
Learning: Applies to **/*.{ts,tsx} : Use TanStack Query for all server state management and data fetching
Applied to files:
src/app/api/admin/stats/route.ts
📚 Learning: 2026-02-01T01:00:48.728Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-01T01:00:48.728Z
Learning: Applies to **/*.{ts,tsx} : Use BetterAuth for all authentication operations
Applied to files:
src/features/integrations/lib/xmpp/client.tssrc/features/integrations/lib/xmpp/config.ts
📚 Learning: 2026-02-01T01:00:48.728Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-01T01:00:48.728Z
Learning: Applies to **/*.{ts,tsx} : Implement proper role-based access control (RBAC) using the permissions module
Applied to files:
src/features/routing/lib/permissions.ts
📚 Learning: 2026-01-15T06:15:48.416Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/tailwind-v4.mdc:0-0
Timestamp: 2026-01-15T06:15:48.416Z
Learning: Applies to **/*.{ts,tsx,js,jsx,html} : Stacked variants now apply left-to-right (not right-to-left) - update variant order when migrating from Tailwind v3
Applied to files:
src/features/routing/lib/permissions.ts
📚 Learning: 2026-01-15T06:15:18.973Z
Learnt from: CR
Repo: allthingslinux/portal PR: 0
File: .cursor/rules/sentry.mdc:0-0
Timestamp: 2026-01-15T06:15:18.973Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use `Sentry.captureException(error)` to capture exceptions and log errors in Sentry, particularly in try-catch blocks or areas where exceptions are expected
Applied to files:
src/features/integrations/lib/xmpp/config.ts
🧬 Code graph analysis (7)
src/app/api/admin/mailcow-accounts/route.ts (1)
src/shared/db/schema/mailcow.ts (1)
mailcowAccount(21-50)
src/features/integrations/lib/xmpp/keys.ts (1)
src/features/auth/lib/keys.ts (1)
keys(4-14)
src/app/api/admin/stats/route.ts (3)
src/shared/db/schema/auth.ts (1)
session(28-46)src/shared/db/schema/api-keys.ts (1)
apikey(12-46)src/shared/db/schema/oauth.ts (1)
oauthClient(5-36)
src/features/integrations/lib/xmpp/client.ts (1)
src/features/integrations/lib/xmpp/config.ts (1)
xmppConfig(15-34)
src/app/api/admin/xmpp-accounts/route.ts (1)
src/shared/db/schema/xmpp.ts (1)
xmppAccount(22-54)
src/app/api/admin/irc-accounts/route.ts (1)
src/shared/db/schema/irc.ts (1)
ircAccount(23-52)
src/features/integrations/lib/xmpp/config.ts (2)
src/env.ts (1)
env(13-28)src/app/error-content.tsx (1)
Error(24-90)
🪛 LanguageTool
AGENTS.md
[grammar] ~134-~134: Use a hyphen to join words.
Context: ...able agent skills index ## Cursor Cloud specific instructions ### Services | S...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (16)
src/features/integrations/lib/xmpp/keys.ts (1)
4-31: LGTM!The environment variable configuration follows the established t3-env pattern (consistent with
src/features/auth/lib/keys.ts). The newPROSODY_REST_TOKENis correctly:
- Defined as optional for backward compatibility
- Placed in the
serverschema only (not exposed to client)- Documented with clear guidance on token generation
src/features/integrations/lib/xmpp/config.ts (3)
25-33: LGTM!The configuration correctly introduces token-based authentication with clear documentation distinguishing the new Bearer token approach from the legacy username/password fields. The
as constassertion ensures type safety.
41-59: LGTM!The validation logic correctly requires
PROSODY_REST_TOKENand uses Sentry error capture with appropriate tags. The empty catch block is appropriate here since Sentry may not be initialized during early configuration validation.
84-86: LGTM!The
isXmppConfigured()check properly validates both required fields (tokenandrestUrl) for the new authentication scheme.src/features/integrations/lib/xmpp/client.ts (1)
33-41: LGTM!The Bearer token authentication implementation is correct:
- Extracts token from config
- Validates presence with a descriptive error message
- Returns properly formatted
BearerheaderThe redundant validation here provides defense-in-depth alongside
validateXmppConfig().src/features/routing/lib/permissions.ts (2)
75-82: Good immutability improvement in navigation sorting.Switching both sort paths to
toSortedavoids in-place mutation and keeps route config/group arrays safer to reuse.
76-82: No action needed. The project explicitly requires Node >=22.18.0 inpackage.json, which natively supports ES2023 APIs includingArray.prototype.toSorted(). TypeScript is configured withtarget: "ESNext"and includes esnext library definitions, ensuring proper type support. No transpilation or polyfill strategy is necessary.Likely an incorrect or invalid review comment.
AGENTS.md (1)
5-5: Scope update is clear and consistent.Line 5 reads well and keeps the project overview concise.
src/features/integrations/lib/irc/atheme/client.ts (1)
6-33: JSON-RPC typing and placeholder changes are internally consistent.The
idstring migration and"."placeholder updates are applied coherently across types, payload construction, and NickServ command calls.Also applies to: 63-63, 116-117, 140-140, 162-162
tests/lib/integrations/irc/atheme/client.test.ts (1)
52-52: Test updates correctly track the new Atheme payload contract.The moved expectations (
id: "1"and"."placeholder) match the implementation changes.Also applies to: 68-68, 76-76, 93-93, 131-131
patches/atl-chat-mod-http-oauth2.patch (1)
33-33: Module enablement changes align with the Bearer-token integration goal.Adding
http_oauth2and enabling required modules is a solid step for the Portal ↔ Prosody flow.Also applies to: 42-42, 98-98
src/app/api/admin/mailcow-accounts/route.ts (1)
39-56: Parallel query refactor is clean and improves latency.The
Promise.allchange removes the waterfall while preserving filtering, pagination, and count semantics.src/app/api/admin/users/route.ts (1)
40-52: Good performance refactor with no API shape drift.The parallel fetch/count pattern is applied clearly and keeps the response contract stable.
src/app/api/admin/xmpp-accounts/route.ts (1)
41-58: Looks good — this removes a DB waterfall cleanly.The query/count parallelization is straightforward and preserves existing filtering and pagination behavior.
src/app/api/admin/irc-accounts/route.ts (1)
46-63: Nice consistency win with the other admin routes.This
Promise.allconversion is clean and should reduce response latency under load.src/app/api/admin/stats/route.ts (1)
16-44: LGTM! Correct parallelization of independent database queries.The refactoring to
Promise.all()properly eliminates the sequential waterfall. The nested array destructuring[[userStats], ...]is safe here because aggregate queries withoutGROUP BYalways return exactly one row. This aligns with the project's guidance to usePromise.all()for parallel prefetching.
| - [Code Standards](.agents/code-standards.md) — Rules beyond what Biome enforces | ||
| - [Project Skills](.agents/skills.md) — Available agent skills index | ||
|
|
||
| ## Cursor Cloud specific instructions |
There was a problem hiding this comment.
Hyphenate the compound modifier in the heading.
Use Cursor Cloud-specific instructions on Line 134.
🧰 Tools
🪛 LanguageTool
[grammar] ~134-~134: Use a hyphen to join words.
Context: ...able agent skills index ## Cursor Cloud specific instructions ### Services | S...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AGENTS.md` at line 134, Update the heading "Cursor Cloud specific
instructions" to use a hyphenated compound modifier by changing it to "Cursor
Cloud-specific instructions" so the title follows correct compound adjective
formatting; locate the heading text in AGENTS.md and replace the existing
unhyphenated phrase.
| +allowed_oauth2_grant_types = { | ||
| + "authorization_code", | ||
| + "device_code", | ||
| + "password", -- Resource Owner Password Grant (Portal provisioning) | ||
| +} | ||
| +allowed_oauth2_response_types = { | ||
| + "code", | ||
| +} | ||
| +oauth2_access_token_ttl = 86400 -- 24 hours | ||
| +oauth2_refresh_token_ttl = 2592000 -- 30 days | ||
| +oauth2_require_code_challenge = false -- Portal uses password grant, not PKCE | ||
| +-- Dynamic client registration (enables Portal to register as OAuth2 client) | ||
| +oauth2_registration_key = Lua.os.getenv("PROSODY_OAUTH2_REGISTRATION_KEY") or "dev-oauth2-registration-key" | ||
| + |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Prosody mod_http_oauth2 oauth2_require_code_challenge authorization_code production settings
💡 Result:
For a production OAuth2 Authorization Code setup with Prosody’s mod_http_oauth2, the “safe default” posture is basically: Authorization Code + PKCE (S256), HTTPS-only, and disable legacy flows.
1) Keep PKCE required (recommended)
mod_http_oauth2 requires a code challenge by default (oauth2_require_code_challenge = true), and its default allowed method is S256 (with plain commented out). For production, keep those defaults (i.e., don’t turn PKCE off, don’t enable plain). [1]
2) Allow only the Authorization Code flow (and other flows only if you truly need them)
The docs show defaults that enable authorization_code and device_code, while implicit and password grants are disabled by default. In production, keep implicit and password disabled, and only enable additional grants if you have a specific, reviewed need. [1]
Example (tightened to just auth code):
allowed_oauth2_grant_types = {
"authorization_code";
-- "device_code"; -- only if you need device flow
-- "password"; -- keep disabled
}
allowed_oauth2_response_types = {
"code";
-- "token"; -- keep disabled (implicit flow)
}
-- Keep PKCE required (default is true)
-- oauth2_require_code_challenge = true
allowed_oauth2_code_challenge_methods = {
"S256";
-- "plain"; -- keep disabled
}[1]
3) Serve the OAuth2 endpoints over HTTPS (not plain HTTP)
Prosody’s HTTP server provides separate http and https services; in production you generally want OAuth2 endpoints reachable via HTTPS. Ensure TLS is configured and avoid exposing cleartext HTTP externally. [2]
4) Be cautious about “production” use at all (current module limitations)
mod_http_oauth2 is explicitly marked alpha and the docs recommend it for “experimental and test deployments only”. It also notes current scope limitations that can make it unsuitable for untrusted clients (clients may effectively get “all or nothing” access). Factor that into any production rollout and threat model. [1]
Sources: Prosody module documentation for mod_http_oauth2 (PKCE requirement, supported flows, grant/response type settings, alpha/deployment notes) [1]; Prosody HTTP server documentation (HTTP/HTTPS services and port/interface behavior) [2]; general OIDC client best-practice note that PKCE is recommended for Authorization Code flows [3].
🌐 Web query:
Prosody mod_http_oauth2 registration key security best practices
💡 Result:
Security best practices for mod_http_oauth2 dynamic client registration key (oauth2_registration_key) in Prosody:
- Treat
oauth2_registration_keylike a high-value signing secret (it’s the JWT/HMAC key used to enable Dynamic Client Registration; default algorithm HS256). Generate it with strong randomness (e.g., 32+ random bytes, base64/hex encoded), and don’t reuse it elsewhere. [1] - Lock down where the secret lives: keep Prosody config (and any included secret files) readable only by root and the Prosody service user/group; avoid leaking it via backups, support bundles, or logs. Prosody’s own TLS key guidance applies here too (tight file permissions, minimize who can read secrets). [2]
- Consider setting a finite registration token lifetime instead of the default “unlimited”: configure
oauth2_registration_ttlso registrations don’t rely on indefinitely-valid signed artifacts. [1] - Limit exposure/abuse of the registration endpoint (
/oauth2/register): put it behind a reverse proxy/WAF rate limit, and if your deployment allows it, restrict by source IP/VPN/allowlist to reduce automated client-registration abuse. [3] - Prefer HTTPS-only access to the OAuth2 HTTP service (don’t publish plain HTTP for these endpoints; ensure TLS is correctly configured). Prosody’s HTTP service exposes both
httpandhttpsendpoints depending on config—verify you’re serving OAuth2 overhttps. [4]
Sources: Prosody community module docs for mod_http_oauth2 registration settings [1], Prosody guidance on protecting key material with strict permissions [2], general DCR hardening controls (rate limits/allowlists) [3], and Prosody HTTP service exposure details [4]. [1][2][3][4]
🌐 Web query:
OAuth2 PKCE code challenge authorization_code grant security requirements
💡 Result:
OAuth 2.0 Authorization Code Grant with PKCE — security requirements (client + server)
Client (app) requirements
- Generate a high-entropy
code_verifierper authorization request using only unreserved characters (A-Z a-z 0-9 - . _ ~) and length 43–128. [1] - Derive and send a
code_challengein the authorization request (thecode_challengeparameter is REQUIRED in PKCE). [1] - Use
code_challenge_method=S256whenever capable:S256:BASE64URL-ENCODE(SHA256(ASCII(code_verifier)))plainis only for constrained/legacy cases and should not be used in new implementations; if the client can doS256, it MUST use it. [1]
- Do not allow downgrade to
plainafter attemptingS256(prevents downgrade/MITM scenarios). [2] - Send the original
code_verifierto the token endpoint when exchanging the authorization code (Authorization Code → Tokens). [2] - Use standard Authorization Code protections:
- Use and validate
statewhen used in the request (CSRF protection pattern). [3] - Ensure the exact
redirect_urihandling matches what’s registered/expected; authorization servers must not redirect to invalid/mismatching redirect URIs. [3]
- Use and validate
Authorization Server (AS) requirements
- Support PKCE (Best Current Practice: AS MUST support PKCE). [4]
- If an authorization request includes a
code_challenge, the AS MUST enforce correctcode_verifierusage at the token endpoint (i.e., verify the verifier against the stored/bound challenge). [4] - Bind the
code_challenge(+ method) to the issued authorization code, and at the token endpoint recompute + compare before issuing tokens. [2] - Mitigate PKCE downgrade attacks: a token request containing
code_verifiermust be accepted only if the original authorization request containedcode_challenge. [4] - Advertise PKCE capabilities (recommended): publish
code_challenge_methods_supportedin AS metadata (RFC 8414 metadata element referenced by BCP). [4]
Sources: RFC 7636 (PKCE) [1][2], RFC 6749 (OAuth 2.0) [3], OAuth 2.0 Security Best Current Practice (IETF draft) [4].
Harden OAuth2 defaults before shipping.
Line 66 disables PKCE (oauth2_require_code_challenge = false) while authorization_code is enabled, and Line 68 falls back to a static dev registration key. Prosody's mod_http_oauth2 documentation explicitly recommends keeping PKCE enabled in production; disabling it weakens the authorization code flow. Additionally, the registration key should never fall back to a known static value—it must be a randomly-generated secret managed via environment variable with no hardcoded default.
Suggested hardening
-oauth2_require_code_challenge = false -- Portal uses password grant, not PKCE
+oauth2_require_code_challenge = true -- Keep PKCE protection for authorization_code clients
-- Dynamic client registration (enables Portal to register as OAuth2 client)
-oauth2_registration_key = Lua.os.getenv("PROSODY_OAUTH2_REGISTRATION_KEY") or "dev-oauth2-registration-key"
+oauth2_registration_key = Lua.os.getenv("PROSODY_OAUTH2_REGISTRATION_KEY")
+if not oauth2_registration_key then
+ error("PROSODY_OAUTH2_REGISTRATION_KEY must be set")
+end📝 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.
| +allowed_oauth2_grant_types = { | |
| + "authorization_code", | |
| + "device_code", | |
| + "password", -- Resource Owner Password Grant (Portal provisioning) | |
| +} | |
| +allowed_oauth2_response_types = { | |
| + "code", | |
| +} | |
| +oauth2_access_token_ttl = 86400 -- 24 hours | |
| +oauth2_refresh_token_ttl = 2592000 -- 30 days | |
| +oauth2_require_code_challenge = false -- Portal uses password grant, not PKCE | |
| +-- Dynamic client registration (enables Portal to register as OAuth2 client) | |
| +oauth2_registration_key = Lua.os.getenv("PROSODY_OAUTH2_REGISTRATION_KEY") or "dev-oauth2-registration-key" | |
| + | |
| allowed_oauth2_grant_types = { | |
| "authorization_code", | |
| "device_code", | |
| "password", -- Resource Owner Password Grant (Portal provisioning) | |
| } | |
| allowed_oauth2_response_types = { | |
| "code", | |
| } | |
| oauth2_access_token_ttl = 86400 -- 24 hours | |
| oauth2_refresh_token_ttl = 2592000 -- 30 days | |
| oauth2_require_code_challenge = true -- Keep PKCE protection for authorization_code clients | |
| -- Dynamic client registration (enables Portal to register as OAuth2 client) | |
| oauth2_registration_key = Lua.os.getenv("PROSODY_OAUTH2_REGISTRATION_KEY") | |
| if not oauth2_registration_key then | |
| error("PROSODY_OAUTH2_REGISTRATION_KEY must be set") | |
| end |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@patches/atl-chat-mod-http-oauth2.patch` around lines 56 - 69, The config
enables the "authorization_code" grant but disables PKCE
(oauth2_require_code_challenge = false) and also provides a hardcoded fallback
for oauth2_registration_key; update this by (1) ensuring
oauth2_require_code_challenge is true when "authorization_code" is present in
allowed_oauth2_grant_types (or remove "authorization_code" if PKCE cannot be
enforced), and (2) removing the hardcoded fallback for oauth2_registration_key
so it must come from the environment (fail fast if
Lua.os.getenv("PROSODY_OAUTH2_REGISTRATION_KEY") is nil/empty) to force a
secure, randomly-generated secret in production.
There was a problem hiding this comment.
Code Review
This pull request introduces significant improvements across several areas, addressing critical integration issues with IRC and XMPP services, enhancing performance, and improving code maintainability. The changes to the Atheme JSON-RPC client correctly handle string IDs and placeholder account names, resolving communication problems. The XMPP integration now correctly uses Bearer token authentication, aligning with modern security practices. Performance has been boosted by parallelizing database queries in multiple admin API routes using Promise.all(), effectively eliminating network waterfalls. Additionally, the use of .toSorted() instead of .sort() ensures immutability in array operations, which is a good practice for maintainability. The documentation for Cursor Cloud specific instructions has also been updated, and relevant tests have been adjusted to reflect these changes. Overall, this is a well-executed and impactful set of changes.
…h2 integration The patch for adding OAuth2 support via mod_http_oauth2 is removed. This reverts the previous addition of OAuth2 token generation for Bearer token authentication in the Portal's mod_http_admin_api. The removal is necessary due to unforeseen issues or changes in requirements that make the OAuth2 integration no longer needed or viable at this time.
Description
This PR addresses critical integration issues with
atl.chat's IRC and XMPP services and includes general codebase cleanup based on best practices. The changes enable Portal to correctly communicate with Atheme (IRC) and Prosody (XMPP) for account management.Changes
mod_http_admin_api.idas a string ("1") instead of an integer (1).""to"."in Atheme JSON-RPC commands to satisfy Atheme's validation."."account names.Promise.all()in 5 admin API routes (stats,users,irc-accounts,xmpp-accounts,mailcow-accounts) to eliminate network waterfalls..sort()with.toSorted()inrouting/permissions.tsto ensure immutability of arrays.atl.chatPatch: Addedpatches/atl-chat-mod-http-oauth2.patchto the repository, which configures Prosody in theatl.chatcodebase to enablemod_http_oauth2for Bearer token generation.Type of Change
Testing
Test Steps
To verify the integration fixes, ensure the
atl.chatservices are running (with the provided patch applied toatl.chat's Prosody config) and Portal is configured with the correctPROSODY_REST_TOKEN.atl.chatpatch:atl.chatProsody service:integration_test_findings_final.logartifact to register an OAuth2 client and obtain a Bearer token from Prosody.PROSODY_REST_TOKEN=<your_generated_token>in Portal's.envfile.atl.chatservices.Checklist
Related Issues
Closes #
Screenshots (if applicable)
N/A
Additional Notes
The
atl.chatpatch (patches/atl-chat-mod-http-oauth2.patch) is included in this PR to provide the necessary configuration for Prosody's OAuth2 module, which enables Bearer token authentication required by Portal's XMPP integration. This patch needs to be applied to theatl.chatrepository manually.Summary by Sourcery
Update admin APIs and integrations to improve performance and fix IRC/XMPP integration issues for the atl.chat environment.
Bug Fixes:
Enhancements:
Documentation:
Tests:
Chores: