keycloak-integration #750
keycloak-integration #750
Conversation
debojyoti-git
requested review from
junaid-shirur,
kalpadhwaryu,
shivamashtikar and
zereraz
as code owners
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds Keycloak SSO and multi-source authentication: dev/prod docker services and env template, setup script and docs, server Keycloak config/routes/middleware/client discovery/resolver, password auth endpoints and utilities, frontend login flow updates, DB user password support, and mounts Keycloak routes with a new global auth middleware. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant FE as Frontend (/auth)
participant BE as Server (/api/keycloak)
participant KC as Keycloak
rect rgb(245,248,255)
note over FE: Fetch /api/keycloak/config
U->>FE: Open Login
FE->>BE: GET /api/keycloak/config
BE-->>FE: baseUrl, realm, clientId, loginUrl
end
alt Keycloak SSO (auth code)
FE->>KC: Redirect to authorization endpoint
KC-->>FE: Redirect back with code,state
FE->>BE: POST /callback {code,state}
BE->>KC: Token exchange (code→tokens)
KC-->>BE: access_token, refresh_token
BE-->>FE: Set cookies + 200
FE->>FE: Redirect to "/"
else Email/Password (ROPC)
FE->>BE: POST /login {email,password}
BE->>KC: ROPC token request
KC-->>BE: Tokens
BE-->>FE: Set cookies + 200
FE->>FE: Redirect to "/"
else Passkey/WebAuthn
FE->>BE: Start WebAuthn flow
BE-->>FE: Challenge
FE->>BE: Verify response
BE-->>FE: Issue tokens
end
sequenceDiagram
autonumber
participant Client as Client (Browser/API)
participant App as Server (App)
participant MW as enhancedJwtMiddleware
participant KC as Keycloak
participant DB as Database
Client->>App: Request with Authorization/Cookies/x-api-key
App->>MW: Authenticate request
alt API Key present
MW-->>App: AuthContext { source: "api-key" }
else Token present
MW->>KC: Validate KC token (if enabled)
alt KC token valid
MW->>DB: Ensure/create user & workspace
DB-->>MW: User/workspace ids
MW-->>App: AuthContext { source: "keycloak", user: {...} }
else Fallback: validate XYNE JWT
MW-->>App: AuthContext { source: "xyne" }
end
end
App-->>Client: Response (includes authMethod in /me)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @debojyoti-git, 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 significantly upgrades the application's authentication system by integrating Keycloak. This change provides greater flexibility in how users can log in, supporting modern SSO flows alongside traditional methods. It also simplifies the setup process for development and production environments by incorporating Keycloak directly into the Docker infrastructure, ensuring a consistent and robust authentication experience.
Highlights
- Keycloak Integration: Introduces Keycloak as a robust identity and access management solution, enabling both Single Sign-On (SSO) and traditional email/password authentication flows.
- Unified Authentication Middleware: Implements an enhanced JWT middleware capable of validating tokens from existing Xyne authentication, new Keycloak JWTs, and API keys, providing a seamless authentication layer.
- Streamlined Environment Setup: Updates Docker Compose configurations for both development and production environments to include Keycloak services, along with a dedicated setup script and comprehensive documentation for quick initialization.
- Extended User Profile Information: Enhances the
/meAPI endpoint to return more detailed user information, including Keycloak-specific attributes like roles and preferred username, when authenticated via Keycloak. - Flexible User Management: Modifies the user database schema to optionally store passwords, accommodating users authenticated directly through Keycloak's email/password grant type, and includes auto-provisioning for new Keycloak users.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
frontend/cookies.txt
Outdated
| @@ -0,0 +1,5 @@ | |||
| # Netscape HTTP Cookie File | |||
server/cookies2.txt
Outdated
| @@ -0,0 +1,5 @@ | |||
| # Netscape HTTP Cookie File | |||
server/server.log
Outdated
| @@ -0,0 +1,151 @@ | |||
| warn: CPU lacks AVX support, strange crashes may occur. Reinstall Bun or use *-baseline build: | |||
There was a problem hiding this comment.
Code Review
This pull request introduces Keycloak integration for authentication, which is a significant and complex change. The implementation covers Docker configurations, a setup script, frontend login UI changes, and backend middleware and APIs. While the overall structure is good, there are several critical security vulnerabilities that must be addressed before merging. These include committing sensitive files, lack of JWT signature verification, plaintext password storage, and insecure production configurations. I have provided detailed comments and suggestions to fix these issues.
frontend/cookies.txt
Outdated
| # Netscape HTTP Cookie File | ||
| # https://curl.se/docs/http-cookies.html | ||
| # This file was generated by libcurl! Edit at your own risk. | ||
|
|
||
| localhost FALSE / FALSE 1756454920 refresh-token eyJhbGciOiJIUzUxMiIsInR5cCIgOiAiSldUIiwia2lkIiA6ICI5MDE5NDM4ZS01YTViLTQ5MmMtOGI2Ny02ZTAyZmVmZDE5MTEifQ.eyJleHAiOjE3NTY0NTQ5MjAsImlhdCI6MTc1NjQ1MzEyMCwianRpIjoiYjYwOTY2N2UtMTk5Mi03Njc2LTQwZDItYzFlOWUwOGExYmVkIiwiaXNzIjoiaHR0cDovL2xvY2FsaG9zdDo4MDgxL3JlYWxtcy94eW5lLXNoYXJlZCIsImF1ZCI6Imh0dHA6Ly9sb2NhbGhvc3Q6ODA4MS9yZWFsbXMveHluZS1zaGFyZWQiLCJzdWIiOiJkOWI1MDQ3Mi1iMTM2LTRlOTQtOGNiOC00NGIzYmQzMTRmYjAiLCJ0eXAiOiJSZWZyZXNoIiwiYXpwIjoib2EtYmFja2VuZCIsInNpZCI6IjRmNWY3ZDRkLTIwYmUtNDJhMC04Y2Q3LWUyMTU1N2IxOTg2ZiIsInNjb3BlIjoib3BlbmlkIHJvbGVzIGVtYWlsIGJhc2ljIGFjciB3ZWItb3JpZ2lucyBwcm9maWxlIn0.DmxPlQ0hSPsDf3GlHiSVrYdiTSoWIi_5nv57-S_k7_A9DfD3nSzM-wuJRHHnPwz_uRpouO6-5v3RZgw-rNkmVQ |
There was a problem hiding this comment.
This file, along with server/cookies2.txt, server/cookies3.txt, server/fresh_cookies.txt, server/test_cookies.txt, and server/server.log, contains sensitive information (authentication tokens, server logs). Committing such files to the repository is a critical security risk. These files should be removed from the repository's history, and the .gitignore file should be updated to prevent them from being committed in the future. Any leaked secrets should be considered compromised and rotated immediately.
| async function validateKeycloakToken( | ||
| token: string, | ||
| ): Promise<AuthContext | null> { | ||
| try { | ||
| // Decode JWT payload first | ||
| const parts = token.split(".") | ||
| if (parts.length !== 3) { | ||
| return null | ||
| } | ||
|
|
||
| const payload = JSON.parse(Buffer.from(parts[1], "base64").toString()) | ||
|
|
||
| // Check expiration | ||
| if (payload.exp < Date.now() / 1000) { | ||
| Logger.warn("Keycloak token expired") | ||
| return null | ||
| } | ||
|
|
||
| // Check if this is a Keycloak token by looking at issuer | ||
| if (!payload.iss || !payload.iss.includes(keycloakConfig.baseUrl)) { | ||
| return null | ||
| } | ||
|
|
||
| Logger.info("Processing Keycloak token for user:", payload.email) | ||
|
|
||
| // Extract roles from Keycloak token | ||
| const realmRoles = payload.realm_access?.roles || [] | ||
| const resourceRoles = payload.resource_access?.["oa-backend"]?.roles || [] | ||
| const allRoles = [...realmRoles, ...resourceRoles] | ||
|
|
||
| // Determine primary role (prioritize oa-backend manager, then admin, then user) | ||
| let primaryRole = UserRole.User | ||
| const oaBackendRoles = payload.resource_access?.["oa-backend"]?.roles || [] | ||
|
|
||
| if ( | ||
| oaBackendRoles.includes("manager") || | ||
| oaBackendRoles.includes("Manager") | ||
| ) { | ||
| primaryRole = UserRole.Admin // Map manager to admin for XYNE | ||
| } else if (allRoles.includes("admin") || allRoles.includes("Admin")) { | ||
| primaryRole = UserRole.Admin | ||
| } | ||
|
|
||
| // Auto-create user in database if they don't exist | ||
| const userEmail = payload.email | ||
| const userName = payload.name || payload.preferred_username || "User" | ||
|
|
||
| try { | ||
| // Check if user exists in database | ||
| const existingUsers = await getUserByEmail(db, userEmail) | ||
|
|
||
| if (!existingUsers || existingUsers.length === 0) { | ||
| // Create user with default values - XYNE doesn't have projects like DPIP | ||
| await createUser( | ||
| db, | ||
| 1, // Default workspace ID (juspay.in) | ||
| userEmail, | ||
| userName, | ||
| "", // No photo link from Keycloak by default | ||
| primaryRole, | ||
| "wkh5nwq7o0es10kpcgg8lu9o", // juspay.in workspace external ID | ||
| undefined, // No password for Keycloak users | ||
| ) | ||
| } | ||
| } catch (dbError) { | ||
| Logger.error("Error creating/checking user in database:", dbError) | ||
| // Continue with authentication even if DB operation fails | ||
| } | ||
|
|
||
| // Get the actual workspace external ID from the database | ||
| let workspaceExternalId = "wkh5nwq7o0es10kpcgg8lu9o" // Default to juspay.in workspace | ||
|
|
||
| try { | ||
| const existingUsers = await getUserByEmail(db, userEmail) | ||
| if (existingUsers && existingUsers.length > 0) { | ||
| workspaceExternalId = existingUsers[0].workspaceExternalId | ||
| } | ||
| } catch (error) { | ||
| Logger.error("Error getting workspace external ID:", error) | ||
| } | ||
|
|
||
| // Create auth context with role information | ||
| const authContext: AuthContext = { | ||
| sub: userEmail, | ||
| role: primaryRole, | ||
| workspaceId: workspaceExternalId, | ||
| source: "keycloak", | ||
| exp: payload.exp, | ||
| user: { | ||
| email: userEmail, | ||
| name: userName, | ||
| role: primaryRole, | ||
| workspace_external_id: workspaceExternalId, | ||
| keycloak_roles: allRoles, // Include all Keycloak roles for reference | ||
| }, | ||
| } | ||
|
|
||
| return authContext | ||
| } catch (error) { | ||
| Logger.error("Keycloak token validation failed:", error) | ||
| return null | ||
| } | ||
| } |
There was a problem hiding this comment.
The validateKeycloakToken function decodes the JWT payload but never verifies its signature against Keycloak's public keys. This is a critical security vulnerability, as it allows an attacker to forge tokens with any payload they want. You must verify the token signature using the public key from Keycloak's JWKS (JSON Web Key Set) endpoint (e.g., http://localhost:8081/realms/xyne-shared/protocol/openid-connect/certs). Libraries like jose or jsonwebtoken can handle JWKS-based verification.
| externalId: text("external_id").unique().notNull(), | ||
| // this will come handy for jwt token | ||
| workspaceExternalId: text("workspace_external_id").notNull(), | ||
| password: text("password"), |
There was a problem hiding this comment.
A password column has been added to the users table as plain text. Storing passwords in plaintext is a critical security vulnerability. If you need to store user passwords in your database, they must be hashed using a strong, salted hashing algorithm like Argon2 or bcrypt. Given this PR is about Keycloak integration, it's worth questioning if this application should be storing passwords at all, as Keycloak should be the source of truth for credentials. The password grant flow sends the password to Keycloak directly; it shouldn't be stored in the application's database.
There was a problem hiding this comment.
Should we keep email+password login? if yes, might keep hashed string in place of password
There was a problem hiding this comment.
kept it with hashing in db, nullable field
| - KEYCLOAK_ADMIN=${KEYCLOAK_ADMIN:-admin} | ||
| - KEYCLOAK_ADMIN_PASSWORD=${KEYCLOAK_ADMIN_PASSWORD:-admin} |
There was a problem hiding this comment.
The production Docker Compose file provides default credentials (admin/admin) for the Keycloak admin user. This is a significant security risk in a production environment, as it might be overlooked and deployed with these weak, well-known credentials. You should remove the default values to force the user to set secure credentials via environment variables.
- KEYCLOAK_ADMIN=${KEYCLOAK_ADMIN}
- KEYCLOAK_ADMIN_PASSWORD=${KEYCLOAK_ADMIN_PASSWORD}
| - KC_HOSTNAME_STRICT=false | ||
| - KC_HOSTNAME_STRICT_HTTPS=false | ||
| - KC_HTTP_ENABLED=true |
There was a problem hiding this comment.
The production configuration for Keycloak has KC_HOSTNAME_STRICT=false, KC_HOSTNAME_STRICT_HTTPS=false, and KC_HTTP_ENABLED=true. This is insecure for a production environment.
KC_HOSTNAME_STRICTshould betrueto prevent host header injection attacks.KC_HOSTNAME_STRICT_HTTPSshould betrueto ensure all communication is over HTTPS.KC_HTTP_ENABLEDshould befalseto disable insecure HTTP communication.
The setup should rely on a reverse proxy to handle TLS termination, and Keycloak should be configured securely behind it.
- KC_HOSTNAME_STRICT=true
- KC_HOSTNAME_STRICT_HTTPS=true
- KC_HTTP_ENABLED=false
| curl -s -X POST "${KEYCLOAK_URL}/admin/realms" \ | ||
| -H "Authorization: Bearer ${ADMIN_TOKEN}" \ | ||
| -H "Content-Type: application/json" \ | ||
| -d '{ | ||
| "realm": "'${REALM_NAME}'", | ||
| "enabled": true, | ||
| "displayName": "XYNE Shared Realm", | ||
| "registrationAllowed": true, | ||
| "loginWithEmailAllowed": true, | ||
| "duplicateEmailsAllowed": false, | ||
| "resetPasswordAllowed": true, | ||
| "editUsernameAllowed": true, | ||
| "bruteForceProtected": false | ||
| }' || echo "Realm might already exist" |
There was a problem hiding this comment.
The script uses || echo "Realm might already exist" to suppress errors from curl. This can hide other problems, such as network issues or an incorrect Keycloak URL, leading to a partially failed setup that is hard to debug. It's better to use curl -f which will cause the script to exit (due to set -e) if the HTTP request fails with a non-2xx status code. You can still handle the "already exists" case gracefully.
REALM_EXISTS=$(curl -s -o /dev/null -w "%{http_code}" -X GET "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}" -H "Authorization: Bearer ${ADMIN_TOKEN}")
if [ "$REALM_EXISTS" -ne 200 ]; then
echo "🌍 Creating realm: ${REALM_NAME}..."
curl -s -f -X POST "${KEYCLOAK_URL}/admin/realms" \
-H "Authorization: Bearer ${ADMIN_TOKEN}" \
-H "Content-Type: application/json" \
-d '{
"realm": "'${REALM_NAME}'",
"enabled": true,
"displayName": "XYNE Shared Realm",
"registrationAllowed": true,
"loginWithEmailAllowed": true,
"duplicateEmailsAllowed": false,
"resetPasswordAllowed": true,
"editUsernameAllowed": true,
"bruteForceProtected": false
}'
else
echo "✅ Realm ${REALM_NAME} already exists, skipping creation."
fi| redirect_uri: `${window.location.origin}/auth`, | ||
| response_type: "code", | ||
| scope: "openid email profile", | ||
| state: Math.random().toString(36).substring(2, 15), |
There was a problem hiding this comment.
The state parameter for the OAuth2 flow is generated using Math.random(). This is not cryptographically secure and does not fully protect against Cross-Site Request Forgery (CSRF) attacks. A better approach is to generate a cryptographically random string, store it in sessionStorage before redirecting, and then compare it with the state parameter returned by Keycloak in the callback.
| state: Math.random().toString(36).substring(2, 15), | |
| state: crypto.randomUUID(), |
| if (authMethod === "keycloak" && authContext) { | ||
| try { | ||
| // Get the original Keycloak token to extract additional fields (same logic as enhanced middleware) | ||
| const authHeader = c.req.header("Authorization") | ||
| const cookieToken = getCookie(c, "auth-token") || getCookie(c, "access-token") | ||
|
|
||
| let token = null | ||
| if (authHeader?.startsWith("Bearer ")) { | ||
| token = authHeader.substring(7) | ||
| } else if (cookieToken) { | ||
| token = cookieToken | ||
| } | ||
|
|
||
| if (token) { | ||
| // Decode JWT payload to extract Keycloak fields | ||
| const parts = token.split(".") | ||
| if (parts.length === 3) { | ||
| const payload = JSON.parse(Buffer.from(parts[1], "base64").toString()) | ||
|
|
||
| // Add Keycloak-specific fields to user object | ||
| if (response.user) { | ||
| response.user = { | ||
| ...response.user, | ||
| email_verified: payload.email_verified || false, | ||
| preferred_username: payload.preferred_username || payload.email, | ||
| given_name: payload.given_name || "", | ||
| family_name: payload.family_name || "", | ||
| realm_access: payload.realm_access || { roles: [] }, | ||
| resource_access: payload.resource_access || {}, | ||
| scope: payload.scope || "", | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } catch (error) { | ||
| Logger.error("Error extracting Keycloak token information:", error) | ||
| // Continue without Keycloak fields if extraction fails | ||
| } | ||
| } |
There was a problem hiding this comment.
This endpoint re-parses the JWT from the request to extract Keycloak-specific claims. This is inefficient because the token has already been processed in the enhancedJwtMiddleware. More importantly, it's insecure because it decodes the payload without verifying the signature, similar to the vulnerability in the middleware. The validated payload and user context should be passed from the middleware to the route handler via c.get().
| await createUser( | ||
| db, | ||
| 1, // Default workspace ID (juspay.in) | ||
| userEmail, | ||
| userName, | ||
| "", // No photo link from Keycloak by default | ||
| primaryRole, | ||
| "wkh5nwq7o0es10kpcgg8lu9o", // juspay.in workspace external ID | ||
| undefined, // No password for Keycloak users | ||
| ) |
There was a problem hiding this comment.
When a new user is auto-provisioned from a Keycloak token, the createUser function is called with a hardcoded workspace ID (1) and workspace external ID (wkh5nwq7o0es10kpcgg8lu9o). This makes the system inflexible and tightly coupled to a specific workspace. This logic should be revisited to support multiple workspaces, perhaps by mapping the user's domain or a group from the token to a workspace in your database.
| .post("/app-refresh-token", handleAppRefreshToken) // To refresh the access token for mobile app | ||
| .post("/refresh-token", getNewAccessRefreshToken) | ||
| .use("*", AuthMiddleware) | ||
| .use("*", enhancedJwtMiddleware()) |
There was a problem hiding this comment.
The old AuthMiddleware has been replaced with enhancedJwtMiddleware. This is a good change to support multiple authentication methods. However, please ensure that the new middleware has been thoroughly tested for all authentication paths (Xyne JWT, Keycloak JWT, API Key) to prevent any regressions.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 36
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
server/test_cookies.txt (1)
1-6: Remove PII-containing JWT from server/test_cookies.txt
Replace the real token with a non-sensitive placeholder (e.g. DUMMY_ACCESS_TOKEN):-server/test_cookies.txt:5 -localhost FALSE / FALSE 1756456696 access-token eyJhbGciOiJSUzI1NiIsInR5cCIgOiAiSldUIiwia2lkIiA6ICJhMm5idEV3SllqdFFRZGdleUxNZlZoS3k3cUs5SDd2bzhnNnV0X3dON1M4In0.eyJleHAiOjE3NTY0NTUxOTYsImlhdCI6MTc1NjQ1NDg5NiwianRpIjoib25ydHJvOmJhY2QwYmEyLWUwOGYtODBlNS01YWY2LWQwYzhkMzdhZjI3NyIsImlzcyI6Imh0dHA6Ly9sb2NhbGhvc3Q6MzAwMCIsImVtYWlsIjoiZGVi...VzAgQ +server/test_cookies.txt:5 +localhost FALSE / FALSE 1756456696 access-token DUMMY_ACCESS_TOKENOptionally generate tokens at test runtime instead of committing any secrets.
server/server.ts (1)
667-674: Use enhancedJwtMiddleware in AuthRedirect instead of AuthMiddleware
- In server/server.ts (around lines 280–295), replace the legacy
AuthMiddleware(c, next)call insideAuthRedirectwithenhancedJwtMiddleware()and redirect on a 401 status.- Suggested implementation:
const AuthRedirect = async (c: Context, next: Next) => { const bearer = c.req.header("Authorization") const token = bearer || getCookie(c, AccessTokenCookieName) || getCookie(c, "auth-token") if (!token) { Logger.warn("Redirected by server – No AuthToken") return c.redirect("/auth") } const guard = enhancedJwtMiddleware() const res = await guard(c, async () => {}) if (res?.status === 401) { Logger.warn("Redirected by server – Enhanced auth failed") return c.redirect("/auth") } return next() }- Verify: valid Keycloak token (cookie or Bearer) serves the SPA; missing/invalid token redirects to
/auth.
🧹 Nitpick comments (23)
frontend/package.json (2)
37-37: Move esbuild to devDependencies or remove if unused.Vite already brings esbuild as a build-time tool. Having it in dependencies bloats runtime installs. If you truly need it directly, move to devDependencies; otherwise remove.
- "esbuild": "^0.25.9",And, if needed, add under devDependencies:
"devDependencies": { + "esbuild": "^0.25.9",
47-47: Remove unused react-is dependency
react-isisn’t imported or required anywhere in the codebase—drop it fromfrontend/package.json.server/auth/keycloak/config.ts (1)
4-6: Avoid surfacing admin credentials in app config (limit blast radius).If these fields are only used by setup scripts, move them to a separate admin-only config and don’t export them broadly.
deployment/KEYCLOAK_SETUP.md (3)
9-9: Specify a language for the fenced block (markdownlint MD040).Use a neutral language like
textfor the ASCII diagram.-``` +```text
102-105: Avoid bare URL (markdownlint MD034) and add explicit “dev‑only” note for defaults.Wrap the URL and clarify these creds are for development only.
-- **Admin Console**: http://localhost:8081/admin/ +- **Admin Console**: <http://localhost:8081/admin/> - Username: `admin` - Password: `admin`
267-267: Spelling: “Infinispan”.Minor doc fix.
-- **Production cache**: Infinspan with Kubernetes clustering support +- **Production cache**: Infinispan with Kubernetes clustering supportdeployment/.env.prod.example (3)
12-16: Prefer explicit Keycloak hostname/proxy settings in prod.Add KC_HOSTNAME and KC_PROXY to avoid redirect/callback issues behind proxies/load balancers.
# Logging level (INFO, WARN, ERROR, DEBUG) KC_LOG_LEVEL=INFO +KC_HOSTNAME=change-me.example.com +KC_PROXY=edge
41-44: Strengthen secret management guidance and naming consistency.Call out that these must not be committed/checked into any real .env; recommend secret manager.
# Application secrets -USER_SECRET=your-jwt-secret-here -ACCESS_TOKEN_SECRET=your-access-token-secret-here -REFRESH_TOKEN_SECRET=your-refresh-token-secret-here +# Do NOT commit real values. Source from a secret manager in CI/CD. +USER_SECRET=your-jwt-secret-here +ACCESS_TOKEN_SECRET=your-access-token-secret-here +REFRESH_TOKEN_SECRET=your-refresh-token-secret-here
22-27: Minor: keep dotenv keys ordered; fix linter warnings.Reorder keys and add a trailing newline to satisfy dotenv-linter.
-POSTGRES_USER=xyne -POSTGRES_PASSWORD=your-secure-db-password-here +POSTGRES_PASSWORD=your-secure-db-password-here +POSTGRES_USER=xyneAlso add a blank line at the end of the file.
deployment/setup-keycloak.sh (1)
6-6: Harden shell options and prereq checks.Enable nounset/pipefail and ensure jq/curl exist before proceeding.
-set -e +set -euo pipefail +command -v curl >/dev/null || { echo "curl required"; exit 1; } +command -v jq >/dev/null || { echo "jq required"; exit 1; }deployment/docker-compose.dev.yml (2)
113-114: Pin Keycloak image version to avoid breaking changes.latest is risky in dev workflows too.
- image: quay.io/keycloak/keycloak:latest + image: quay.io/keycloak/keycloak:25.0.6
116-127: Avoid shipping default admin creds in compose.Source these from an env file for local overrides.
- - KEYCLOAK_ADMIN=admin - - KEYCLOAK_ADMIN_PASSWORD=admin + - KEYCLOAK_ADMIN=${KEYCLOAK_ADMIN:-admin} + - KEYCLOAK_ADMIN_PASSWORD=${KEYCLOAK_ADMIN_PASSWORD:-admin}deployment/docker-compose.prod.yml (2)
121-126: Enable hostname and proxy configuration for correct external URLs.Set KC_HOSTNAME and keep KC_PROXY=edge behind TLS-terminating proxy.
- KC_HOSTNAME_STRICT=false - KC_HOSTNAME_STRICT_HTTPS=false - KC_HTTP_ENABLED=true - KC_PROXY=edge + - KC_HOSTNAME=${KC_HOSTNAME:?external auth hostname}
114-114: Trim trailing spaces flagged by YAML linter.Clean up formatting to keep CI green.
Also applies to: 120-120, 126-126, 131-131
frontend/src/routes/auth.tsx (3)
122-128: Remove unused variable to fix TS6133.[result] is never read.
- const result = await response.json() - logger.info('Email/password authentication successful') + await response.json() + logger.info('Email/password authentication successful')
333-336: Remove unused tokens variable to fix TS6133.No need to store the response if cookies are set server-side.
- const tokens = await response.json() + await response.json()
320-353: Avoid using window.history in beforeLoad + redirect; pick one.Throwing redirect is sufficient; manipulating history can cause double navigation.
- window.history.replaceState({}, document.title, '/') - throw redirect({ to: "/" }) + throw redirect({ to: "/" })And similarly for error cases, rely on returned loader data without replaceState.
server/auth/keycloak/routes.ts (1)
53-68: Exposeenabledin public config; avoid surprises when disabled.Helps clients conditionally show SSO option.
return c.json({ + enabled: config.enabled, baseUrl: config.baseUrl, realm: config.defaultRealm, clientId: config.clientId, loginUrl: `${config.baseUrl}/realms/${config.defaultRealm}/protocol/openid-connect/auth`, // Don't expose sensitive config like clientSecret })server/api/auth.ts (1)
33-72: Prefer using authContext over re-parsing the token.You already have
authContextfrom middleware; extend from that to avoid duplicate parsing and drift.server/auth/keycloak/api.ts (2)
178-202: Avoid returning refresh_token to the frontend.Returning refresh tokens in JSON increases exfiltration risk. Prefer HttpOnly cookie only.
- refresh_token: tokens.refresh_token, + // refresh_token omitted intentionally (HttpOnly cookie set)
21-30: Move CORS origins to config.Hardcoded dev origins hinder deploys. Read from env/config and default to safe dev values.
server/auth/keycloak/middleware.ts (2)
134-140: Use base64url-safe decode if you keep manual parsing anywhere.- const payload = JSON.parse(Buffer.from(parts[1], "base64").toString()) + const b64 = parts[1].replace(/-/g, "+").replace(/_/g, "/") + const payload = JSON.parse(Buffer.from(b64, "base64").toString("utf8"))
41-49: Role value “Service” may bypass auth checks.If the system expects
UserRolevalues, set an allowed role (e.g.,UserRole.Admin) or introduce an explicitServicerole in the ACL.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (4)
frontend/bun.lockbis excluded by!**/bun.lockbfrontend/package-lock.jsonis excluded by!**/package-lock.jsonserver/bun.lockbis excluded by!**/bun.lockbserver/server.logis excluded by!**/*.log
📒 Files selected for processing (21)
deployment/.env.prod.example(1 hunks)deployment/KEYCLOAK_SETUP.md(1 hunks)deployment/docker-compose.dev.yml(1 hunks)deployment/docker-compose.prod.yml(1 hunks)deployment/setup-keycloak.sh(1 hunks)frontend/cookies.txt(1 hunks)frontend/package.json(3 hunks)frontend/src/routeTree.gen.ts(3 hunks)frontend/src/routes/auth.tsx(3 hunks)server/api/auth.ts(2 hunks)server/auth/keycloak/api.ts(1 hunks)server/auth/keycloak/config.ts(1 hunks)server/auth/keycloak/middleware.ts(1 hunks)server/auth/keycloak/routes.ts(1 hunks)server/cookies2.txt(1 hunks)server/cookies3.txt(1 hunks)server/db/schema/users.ts(3 hunks)server/db/user.ts(2 hunks)server/fresh_cookies.txt(1 hunks)server/server.ts(3 hunks)server/test_cookies.txt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
server/auth/keycloak/middleware.ts (4)
server/logger/index.ts (1)
getLogger(33-90)server/auth/keycloak/config.ts (1)
getKeycloakConfig(12-23)server/routes/vespa-proxy.ts (1)
validateApiKey(60-80)server/db/user.ts (2)
getUserByEmail(98-107)createUser(109-139)
server/auth/keycloak/routes.ts (2)
server/logger/index.ts (1)
getLogger(33-90)server/auth/keycloak/config.ts (1)
getKeycloakConfig(12-23)
server/auth/keycloak/api.ts (2)
server/logger/index.ts (1)
getLogger(33-90)server/auth/keycloak/config.ts (1)
getKeycloakConfig(12-23)
frontend/src/routes/auth.tsx (3)
server/auth/keycloak/config.ts (1)
KeycloakConfig(1-10)frontend/src/routes/_authenticated.tsx (1)
Route(5-18)frontend/src/api.ts (1)
api(4-4)
server/server.ts (1)
server/auth/keycloak/middleware.ts (1)
enhancedJwtMiddleware(27-94)
🪛 Shellcheck (0.10.0)
deployment/setup-keycloak.sh
[warning] 73-73: Quote this to prevent word splitting.
(SC2046)
🪛 dotenv-linter (3.3.0)
deployment/.env.prod.example
[warning] 24-24: [UnorderedKey] The POSTGRES_PASSWORD key should go before the POSTGRES_USER key
(UnorderedKey)
[warning] 35-35: [UnorderedKey] The KEYCLOAK_BASE_URL key should go before the KEYCLOAK_ENABLED key
(UnorderedKey)
[warning] 36-36: [UnorderedKey] The KEYCLOAK_DEFAULT_REALM key should go before the KEYCLOAK_ENABLED key
(UnorderedKey)
[warning] 37-37: [UnorderedKey] The KEYCLOAK_CLIENT_ID key should go before the KEYCLOAK_DEFAULT_REALM key
(UnorderedKey)
[warning] 38-38: [UnorderedKey] The KEYCLOAK_CLIENT_SECRET key should go before the KEYCLOAK_DEFAULT_REALM key
(UnorderedKey)
[warning] 42-42: [UnorderedKey] The ACCESS_TOKEN_SECRET key should go before the USER_SECRET key
(UnorderedKey)
[warning] 43-43: [UnorderedKey] The REFRESH_TOKEN_SECRET key should go before the USER_SECRET key
(UnorderedKey)
[warning] 52-52: [UnorderedKey] The GRAFANA_ENABLED key should go before the METRICS_ENABLED key
(UnorderedKey)
[warning] 94-94: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🪛 LanguageTool
frontend/cookies.txt
[grammar] ~5-~5: There might be a mistake here.
Context: ...bcurl! Edit at your own risk. localhost FALSE / FALSE 1756454920 refresh-token e...
(QB_NEW_EN)
[grammar] ~5-~5: There might be a mistake here.
Context: ... your own risk. localhost FALSE / FALSE 1756454920 refresh-token eyJhbGciOiJIUzU...
(QB_NEW_EN)
[grammar] ~5-~5: There might be a mistake here.
Context: ...isk. localhost FALSE / FALSE 1756454920 refresh-token eyJhbGciOiJIUzUxMiIsInR5cC...
(QB_NEW_EN)
[grammar] ~5-~5: There might be a mistake here.
Context: ...t FALSE / FALSE 1756454920 refresh-token eyJhbGciOiJIUzUxMiIsInR5cCIgOiAiSldUIiwi...
(QB_NEW_EN)
deployment/KEYCLOAK_SETUP.md
[grammar] ~28-~28: There might be a mistake here.
Context: ...ice | Container Name | Ports | Purpose | |---------|---------------|-------|-----...
(QB_NEW_EN)
[grammar] ~29-~29: There might be a mistake here.
Context: ...-----|---------------|-------|---------| | PostgreSQL | xyne-db | 5432 | Database...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ... | Database for both XYNE and Keycloak | | Keycloak | xyne-keycloak | 8081→8080 |...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...ak | 8081→8080 | Authentication server | | Vespa | vespa | 8080, 19071 | Search e...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ... | vespa | 8080, 19071 | Search engine | | Prometheus | xyne-prometheus | 9090 | ...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...prometheus | 9090 | Metrics collection | | Grafana | xyne-grafana | 3002→3000 | M...
(QB_NEW_EN)
[grammar] ~34-~34: There might be a mistake here.
Context: ...na | 3002→3000 | Monitoring dashboards | | Loki | loki | 3100 | Log aggregation |...
(QB_NEW_EN)
[grammar] ~92-~92: There might be a mistake here.
Context: ...es | Variable | Default | Description | |----------|---------|-------------| | `...
(QB_NEW_EN)
[grammar] ~93-~93: There might be a mistake here.
Context: ...n | |----------|---------|-------------| | KEYCLOAK_ENABLED | false | Enable ...
(QB_NEW_EN)
[grammar] ~94-~94: There might be a mistake here.
Context: ...alse| Enable Keycloak authentication | |KEYCLOAK_BASE_URL|http://localhos...
(QB_NEW_EN)
[grammar] ~95-~95: There might be a mistake here.
Context: .../localhost:8081| Keycloak server URL | |KEYCLOAK_DEFAULT_REALM|xyne-share...
(QB_NEW_EN)
[grammar] ~96-~96: There might be a mistake here.
Context: ...|xyne-shared| Default realm name | |KEYCLOAK_CLIENT_ID|oa-backend` | ...
(QB_NEW_EN)
[grammar] ~97-~97: There might be a mistake here.
Context: ...D|oa-backend| Client ID for XYNE | |KEYCLOAK_CLIENT_SECRET` | *(required)...
(QB_NEW_EN)
[grammar] ~102-~102: There might be a mistake here.
Context: ... Console**: http://localhost:8081/admin/ - Username: admin - Password: admin ...
(QB_NEW_EN)
[grammar] ~103-~103: There might be a mistake here.
Context: ...p://localhost:8081/admin/ - Username: admin - Password: admin - Realm: `xyne-sh...
(QB_NEW_EN)
[grammar] ~106-~106: There might be a mistake here.
Context: ...in - Password:admin - **Realm**:xyne-shared- **Client**:oa-backend` - Test User: ...
(QB_NEW_EN)
[grammar] ~107-~107: There might be a mistake here.
Context: ... Realm: xyne-shared - Client: oa-backend - Test User: `debojyoti.mandal@juspay.in...
(QB_NEW_EN)
[grammar] ~114-~114: There might be a mistake here.
Context: ...methods: 1. Google OAuth (existing) 2. Keycloak SSO (browser redirect) 3. **E...
(QB_NEW_EN)
[grammar] ~115-~115: There might be a mistake here.
Context: ...) 2. Keycloak SSO (browser redirect) 3. Email/Password (direct login via Keycl...
(QB_NEW_EN)
[grammar] ~136-~136: There might be a mistake here.
Context: ...All data is persisted in Docker volumes: - PostgreSQL data: ../server/xyne-data -...
(QB_NEW_EN)
[grammar] ~251-~251: There might be a mistake here.
Context: ... | Feature | Development | Production | |---------|-------------|------------| |...
(QB_NEW_EN)
[grammar] ~252-~252: There might be a mistake here.
Context: ...| |---------|-------------|------------| | Keycloak Mode | start-dev (hot r...
(QB_NEW_EN)
[grammar] ~253-~253: There might be a mistake here.
Context: ...| start --optimized (faster startup) | | Container Name | xyne-keycloak |...
(QB_NEW_EN)
[grammar] ~254-~254: There might be a mistake here.
Context: ...xyne-keycloak | xyne-keycloak-prod | | Resource Limits | None | 1GB RAM, ...
(QB_NEW_EN)
[grammar] ~255-~255: There might be a mistake here.
Context: ...rce Limits** | None | 1GB RAM, 0.5 CPU | | Cache Strategy | Simple | Kubernet...
(QB_NEW_EN)
[grammar] ~256-~256: There might be a mistake here.
Context: ...| Simple | Kubernetes clustering ready | | Health Checks | Basic | Extended w...
(QB_NEW_EN)
[grammar] ~257-~257: There might be a mistake here.
Context: ...ic | Extended with longer startup time | | Logging | DEBUG friendly | INFO/WA...
(QB_NEW_EN)
[grammar] ~264-~264: There might be a mistake here.
Context: ...: Faster startup and runtime performance - Resource limits: Memory and CPU constr...
(QB_NEW_EN)
[grammar] ~265-~265: There might be a mistake here.
Context: ... constraints for container orchestration - Extended health checks: 120s startup p...
(QB_NEW_EN)
[grammar] ~266-~266: There might be a mistake here.
Context: ...0s startup period for optimization phase - Production cache: Infinspan with Kuber...
(QB_NEW_EN)
[grammar] ~267-~267: Ensure spelling is correct
Context: ...imization phase - Production cache: Infinspan with Kubernetes clustering support - **...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~267-~267: There might be a mistake here.
Context: ...nspan with Kubernetes clustering support - Environment-based configuration: All s...
(QB_NEW_EN)
[grammar] ~274-~274: There might be a mistake here.
Context: ...production deployment: 1. Security: - Change default admin credentials via env...
(QB_NEW_EN)
[grammar] ~280-~280: There might be a mistake here.
Context: ...re proper CORS origins 2. Database: - Use separate PostgreSQL instance for Key...
(QB_NEW_EN)
[grammar] ~285-~285: There might be a mistake here.
Context: ...se clustering if needed 3. Scaling: - Configure Keycloak clustering with share...
(QB_NEW_EN)
[grammar] ~286-~286: There might be a mistake here.
Context: ...g**: - Configure Keycloak clustering with shared database - Use external load ...
(QB_NEW_EN)
[grammar] ~290-~290: There might be a mistake here.
Context: ... limits and requests 4. Monitoring: - Enable Keycloak metrics integration with...
(QB_NEW_EN)
[grammar] ~303-~303: There might be a mistake here.
Context: ...ence** - PostgreSQL backend for Keycloak - Survives container restarts - Easy backu...
(QB_NEW_EN)
[grammar] ~304-~304: There might be a mistake here.
Context: ...r Keycloak - Survives container restarts - Easy backup/restore ✅ **Team Collaborat...
(QB_NEW_EN)
[grammar] ~308-~308: There might be a mistake here.
Context: ...ion** - Version-controlled configuration - Consistent realm/client setup - Shared d...
(QB_NEW_EN)
[grammar] ~309-~309: There might be a mistake here.
Context: ...guration - Consistent realm/client setup - Shared development environment ✅ **Prod...
(QB_NEW_EN)
[grammar] ~313-~313: There might be a mistake here.
Context: ...y** - Similar architecture to production - Proper database integration - Service di...
(QB_NEW_EN)
[grammar] ~314-~314: There might be a mistake here.
Context: ...production - Proper database integration - Service discovery patterns
(QB_NEW_EN)
🪛 Gitleaks (8.27.2)
server/cookies3.txt
5-5: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
server/test_cookies.txt
5-5: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
server/fresh_cookies.txt
5-5: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
server/cookies2.txt
5-5: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🪛 YAMLlint (1.37.1)
deployment/docker-compose.prod.yml
[error] 114-114: trailing spaces
(trailing-spaces)
[error] 120-120: trailing spaces
(trailing-spaces)
[error] 126-126: trailing spaces
(trailing-spaces)
[error] 131-131: trailing spaces
(trailing-spaces)
🪛 markdownlint-cli2 (0.17.2)
deployment/KEYCLOAK_SETUP.md
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
102-102: Bare URL used
(MD034, no-bare-urls)
🪛 GitHub Actions: TypeScript Build Check
frontend/src/routes/auth.tsx
[error] 123-123: bunx tsc -b: TypeScript error TS6133: 'result' is declared but its value is never read.
[error] 334-334: bunx tsc -b: TypeScript error TS6133: 'tokens' is declared but its value is never read.
🔇 Additional comments (8)
deployment/docker-compose.dev.yml (1)
135-140: Healthcheck dependency on curl inside container.Confirm curl exists in the Keycloak image; if not, replace with wget or an exec-based Java health endpoint. Otherwise the container will be marked unhealthy regardless of readiness.
Would you like me to provide a curl-free healthcheck variant?
frontend/src/routes/auth.tsx (1)
101-140: Consider removing direct email/password flow or gate it to dev.ROPC against IdP is discouraged; prefer redirect to Keycloak login. If kept, ensure rate limiting and audit on server.
Would you like a follow-up PR to hide this behind a dev flag?
frontend/src/routeTree.gen.ts (1)
11-35: Alias refactor looks correct.Switching to
*RouteImportaliases androotRouteImportas parent is consistent and improves clarity in the generated file.server/server.ts (2)
99-101: Mount and middleware imports are sensible.Imports for
keycloakRoutesandenhancedJwtMiddlewareare correctly scoped for subsequent use.
1046-1048: Keycloak routes mount LGTM.Exposing
/api/keycloak(public health/config + OAuth flows) outside the/api/v1chain is appropriate.server/api/auth.ts (1)
8-8: Import looks good.server/auth/keycloak/middleware.ts (2)
96-121: No-op when Keycloak disabled is fine.Flow falls back to Xyne tokens cleanly.
258-286: API key validation approach is reasonable.Static allowlist + JWT “api_key” marker works.
| # Keycloak integration | ||
| KEYCLOAK_ENABLED=true | ||
| KEYCLOAK_BASE_URL=http://localhost:8081 | ||
| KEYCLOAK_DEFAULT_REALM=xyne-shared | ||
| KEYCLOAK_CLIENT_ID=oa-backend | ||
| KEYCLOAK_CLIENT_SECRET=your-client-secret-from-setup | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use HTTPS and production hostname for Keycloak base URL; avoid localhost defaults in prod templates.
Replace the localhost URL and make PORT optional guidance explicit to prevent misconfig in real deployments.
-KEYCLOAK_ENABLED=true
-KEYCLOAK_BASE_URL=http://localhost:8081
+KEYCLOAK_ENABLED=true
+# Use your public HTTPS hostname (behind a reverse proxy)
+# Example: https://auth.example.com
+KEYCLOAK_BASE_URL=https://change-me.example.com
KEYCLOAK_DEFAULT_REALM=xyne-shared
KEYCLOAK_CLIENT_ID=oa-backend
KEYCLOAK_CLIENT_SECRET=your-client-secret-from-setup📝 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.
| # Keycloak integration | |
| KEYCLOAK_ENABLED=true | |
| KEYCLOAK_BASE_URL=http://localhost:8081 | |
| KEYCLOAK_DEFAULT_REALM=xyne-shared | |
| KEYCLOAK_CLIENT_ID=oa-backend | |
| KEYCLOAK_CLIENT_SECRET=your-client-secret-from-setup | |
| # Keycloak integration | |
| KEYCLOAK_ENABLED=true | |
| # Use your public HTTPS hostname (behind a reverse proxy) | |
| # Example: https://auth.example.com | |
| KEYCLOAK_BASE_URL=https://change-me.example.com | |
| KEYCLOAK_DEFAULT_REALM=xyne-shared | |
| KEYCLOAK_CLIENT_ID=oa-backend | |
| KEYCLOAK_CLIENT_SECRET=your-client-secret-from-setup |
🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 35-35: [UnorderedKey] The KEYCLOAK_BASE_URL key should go before the KEYCLOAK_ENABLED key
(UnorderedKey)
[warning] 36-36: [UnorderedKey] The KEYCLOAK_DEFAULT_REALM key should go before the KEYCLOAK_ENABLED key
(UnorderedKey)
[warning] 37-37: [UnorderedKey] The KEYCLOAK_CLIENT_ID key should go before the KEYCLOAK_DEFAULT_REALM key
(UnorderedKey)
[warning] 38-38: [UnorderedKey] The KEYCLOAK_CLIENT_SECRET key should go before the KEYCLOAK_DEFAULT_REALM key
(UnorderedKey)
🤖 Prompt for AI Agents
In deployment/.env.prod.example around lines 33 to 39, the Keycloak base URL and
port guidance are insecurely set to localhost; update the template to use an
HTTPS production hostname and add a comment that the port is optional (and
should be omitted if using default 443). Replace KEYCLOAK_BASE_URL's localhost
value with a placeholder like https://auth.YOUR_DOMAIN and add an inline note to
set KEYCLOAK_CLIENT_SECRET and KEYCLOAK_DEFAULT_REALM for production; ensure the
example emphasizes using HTTPS and the real production host instead of
localhost.
| - KC_DB_URL=jdbc:postgresql://xyne-db:5432/keycloak | ||
| - KC_DB_USERNAME=xyne | ||
| - KC_DB_PASSWORD=xyne |
There was a problem hiding this comment.
Keycloak DB points to a non-existent database.
xyne-db initializes database "xyne", but KC_DB_URL targets "keycloak". This will fail to connect.
- - KC_DB_URL=jdbc:postgresql://xyne-db:5432/keycloak
+ - KC_DB_URL=jdbc:postgresql://xyne-db:5432/xyneAlternatively, create a separate "keycloak" DB at Postgres init.
Also applies to: 83-90
🤖 Prompt for AI Agents
In deployment/docker-compose.dev.yml around lines 119-121 (and similarly lines
83-90), Keycloak's KC_DB_URL points to database "keycloak" but the Postgres
service initializes a database named "xyne", causing connection failures; either
update KC_DB_URL to jdbc:postgresql://xyne-db:5432/xyne to match the initialized
DB or modify the Postgres initialization (docker-entrypoint-initdb.d SQL or
POSTGRES_DB env) to create a "keycloak" database (and ensure
KC_DB_USERNAME/KC_DB_PASSWORD have privileges for that DB); pick one approach
and make the corresponding change so Keycloak and Postgres use the same database
name.
| image: quay.io/keycloak/keycloak:latest | ||
| container_name: xyne-keycloak-prod | ||
| environment: | ||
| # Admin credentials - Change these in production! | ||
| - KEYCLOAK_ADMIN=${KEYCLOAK_ADMIN:-admin} | ||
| - KEYCLOAK_ADMIN_PASSWORD=${KEYCLOAK_ADMIN_PASSWORD:-admin} | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove default admin fallbacks in prod to prevent weak creds.
Force explicit values via .env; also pin image.
- image: quay.io/keycloak/keycloak:latest
+ image: quay.io/keycloak/keycloak:25.0.6
...
- - KEYCLOAK_ADMIN=${KEYCLOAK_ADMIN:-admin}
- - KEYCLOAK_ADMIN_PASSWORD=${KEYCLOAK_ADMIN_PASSWORD:-admin}
+ - KEYCLOAK_ADMIN=${KEYCLOAK_ADMIN:?set in .env}
+ - KEYCLOAK_ADMIN_PASSWORD=${KEYCLOAK_ADMIN_PASSWORD:?set in .env}Also applies to: 120-131
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 114-114: trailing spaces
(trailing-spaces)
🤖 Prompt for AI Agents
In deployment/docker-compose.prod.yml around lines 108-114 and also 120-131, the
compose file currently provides default fallback admin credentials and uses a
floating "latest" image tag; remove the fallback defaults so KEYCLOAK_ADMIN and
KEYCLOAK_ADMIN_PASSWORD are required (do not set ${...:-admin} defaults) and pin
the Keycloak image to a specific, approved release tag (replace
quay.io/keycloak/keycloak:latest with a fixed version). Ensure the environment
variables are documented in the .env for production and fail fast if they are
missing rather than using weak defaults.
| - KC_DB_URL=jdbc:postgresql://xyne-db:5432/keycloak | ||
| - KC_DB_USERNAME=${POSTGRES_USER:-xyne} | ||
| - KC_DB_PASSWORD=${POSTGRES_PASSWORD:-xyne} |
There was a problem hiding this comment.
Keycloak DB URL targets a DB that is never created.
xyne-db creates "xyne", not "keycloak". Fix URL or provision "keycloak" at init.
- - KC_DB_URL=jdbc:postgresql://xyne-db:5432/keycloak
+ - KC_DB_URL=jdbc:postgresql://xyne-db:5432/xyneAlso applies to: 78-85
🤖 Prompt for AI Agents
In deployment/docker-compose.prod.yml around lines 117-119 (and also lines
78-85), the KC_DB_URL points to a database named "keycloak" while the postgres
service only initializes a database named "xyne"; update the compose
configuration so the Keycloak DB target matches the created DB or provision a
"keycloak" database at init. Fix options: either change KC_DB_URL to
jdbc:postgresql://xyne-db:5432/xyne to point Keycloak at the existing "xyne" DB,
or adjust the postgres service environment (e.g., POSTGRES_DB or initialization
scripts) to create a "keycloak" database and users expected by Keycloak; ensure
KC_DB_USERNAME and KC_DB_PASSWORD align with the chosen DB provisioning.
| - KC_CACHE=ispn | ||
| - KC_CACHE_STACK=kubernetes | ||
| ports: |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Cache stack set to kubernetes in Docker Compose.
KC_CACHE_STACK=kubernetes requires KUBE_PING; not suitable for Compose. Use "local" (single node) or "tcp".
- - KC_CACHE=ispn
- - KC_CACHE_STACK=kubernetes
+ - KC_CACHE=ispn
+ - KC_CACHE_STACK=local📝 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.
| - KC_CACHE=ispn | |
| - KC_CACHE_STACK=kubernetes | |
| ports: | |
| - KC_CACHE=ispn | |
| - KC_CACHE_STACK=local | |
| ports: |
🤖 Prompt for AI Agents
In deployment/docker-compose.prod.yml around lines 133 to 135, KC_CACHE_STACK is
set to "kubernetes" which requires KUBE_PING and is not appropriate for Docker
Compose; change KC_CACHE_STACK to a Compose-friendly value such as "local" for
single-node or "tcp" for multi-node Compose setups and adjust any related
KC_CACHE transport/config (e.g., ensure KC_CACHE=ispn remains and update
Infinispan/KC configuration to use the selected stack) so the cache stack is
compatible with container-compose networking.
server/cookies2.txt
Outdated
| # https://curl.se/docs/http-cookies.html | ||
| # This file was generated by libcurl! Edit at your own risk. | ||
|
|
||
| localhost FALSE / FALSE 1756455227 refresh-token eyJhbGciOiJIUzUxMiIsInR5cCIgOiAiSldUIiwia2lkIiA6ICI5MDE5NDM4ZS01YTViLTQ5MmMtOGI2Ny02ZTAyZmVmZDE5MTEifQ.eyJleHAiOjE3NTY0NTUyMjcsImlhdCI6MTc1NjQ1MzQyNywianRpIjoiMWY3NzhlYWUtMDQ3Mi0xY2ZhLWE5MDMtYzVlOGQxYzFkM2YxIiwiaXNzIjoiaHR0cDovL2xvY2FsaG9zdDo4MDgxL3JlYWxtcy94eW5lLXNoYXJlZCIsImF1ZCI6Imh0dHA6Ly9sb2NhbGhvc3Q6ODA4MS9yZWFsbXMveHluZS1zaGFyZWQiLCJzdWIiOiJkOWI1MDQ3Mi1iMTM2LTRlOTQtOGNiOC00NGIzYmQzMTRmYjAiLCJ0eXAiOiJSZWZyZXNoIiwiYXpwIjoib2EtYmFja2VuZCIsInNpZCI6ImJkNjAxYzdlLTU2NjQtNDI2YS1hYzhiLTZiMGJhNzg5NzhhYyIsInNjb3BlIjoib3BlbmlkIHJvbGVzIGVtYWlsIGJhc2ljIGFjciB3ZWItb3JpZ2lucyBwcm9maWxlIn0.0PUrgw80WWnGEMctQGZQUQ82UMBtALCPpD9-lv-8pAQNbOG6Hbl_ievw4aCNXA-10yRfiebuoiXqJZXnXIG9dg |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refresh-token JWT checked in; remove and purge.
Same concern as other cookie files. Delete from repo, add ignore, and ensure any related secrets/keys are rotated if these were minted against a shared realm.
-# Netscape HTTP Cookie File
-# https://curl.se/docs/http-cookies.html
-# This file was generated by libcurl! Edit at your own risk.
-
-localhost FALSE / FALSE 1756455227 refresh-token <REDACTED>
-Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Gitleaks (8.27.2)
5-5: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🤖 Prompt for AI Agents
server/cookies2.txt lines 5-5: the refresh-token JWT was committed; delete the
file from the repo, remove the sensitive entry from git history (use a
history-rewrite tool like git filter-repo or BFG to purge the blob), add an
appropriate ignore rule to .gitignore to prevent re-adding, and ensure the
exposed token/keys are revoked or rotated (invalidate the token, rotate signing
keys/credentials for the affected realm) and document the remediation in the PR.
server/cookies3.txt
Outdated
| # https://curl.se/docs/http-cookies.html | ||
| # This file was generated by libcurl! Edit at your own risk. | ||
|
|
||
| localhost FALSE / FALSE 1756455453 access-token eyJhbGciOiJSUzI1NiIsInR5cCIgOiAiSldUIiwia2lkIiA6ICJhMm5idEV3SllqdFFRZGdleUxNZlZoS3k3cUs5SDd2bzhnNnV0X3dON1M4In0.eyJleHAiOjE3NTY0NTM5NTMsImlhdCI6MTc1NjQ1MzY1MywianRpIjoib25ydHJvOjZmNWQwOGY0LTgyOGYtNzk5Ny04ZjIxLTMzMzMxZGEyMThmMSIsImlzcyI6Imh0dHA6Ly9sb2NhbGhvc3Q6ODA4MS9yZWFsbXMveHluZS1zaGFyZWQiLCJhdWQiOiJhY2NvdW50Iiwic3ViIjoiZDliNTA0NzItYjEzNi00ZTk0LThjYjgtNDRiM2JkMzE0ZmIwIiwidHlwIjoiQmVhcmVyIiwiYXpwIjoib2EtYmFja2VuZCIsInNpZCI6IjBhNTdjM2IxLWZiMjMtNDQxZS05YWZkLWM3MzcyYTAzNmJkYSIsImFjciI6IjEiLCJhbGxvd2VkLW9yaWdpbnMiOlsiKiIsImh0dHA6Ly9sb2NhbGhvc3Q6MzAwMCIsImh0dHA6Ly9sb2NhbGhvc3Q6NTE3MyJdLCJyZWFsbV9hY2Nlc3MiOnsicm9sZXMiOlsib2ZmbGluZV9hY2Nlc3MiLCJ1bWFfYXV0aG9yaXphdGlvbiIsImRlZmF1bHQtcm9sZXMteHluZS1zaGFyZWQiXX0sInJlc291cmNlX2FjY2VzcyI6eyJhY2NvdW50Ijp7InJvbGVzIjpbIm1hbmFnZS1hY2NvdW50IiwibWFuYWdlLWFjY291bnQtbGlua3MiLCJ2aWV3LXByb2ZpbGUiXX19LCJzY29wZSI6Im9wZW5pZCBlbWFpbCBwcm9maWxlIiwiZW1haWxfdmVyaWZpZWQiOmZhbHNlLCJuYW1lIjoiRGVib2p5b3RpIE1hbmRhbCIsInByZWZlcnJlZF91c2VybmFtZSI6ImRlYm9qeW90aS5tYW5kYWxAanVzcGF5LmluIiwiZ2l2ZW5fbmFtZSI6IkRlYm9qeW90aSIsImZhbWlseV9uYW1lIjoiTWFuZGFsIiwiZW1haWwiOiJkZWJvanlvdGkubWFuZGFsQGp1c3BheS5pbiJ9.ZOvui27y5ffJZjUafhpmqKzbRjg2nXIEHa0TzK8DSZdYztHNMoAQbq95HMWPWuCCIrFgnT4_5sXtWzNexLm0WDwLpOAToaPrUnBjs2QyyYnMazDiIlI9I61CDMGa9wNaYbRoW_ojC3KXkJPU9MjBsx924PuiWZLYSGV3XxicC7W0zSzg0DPq64LCfdvF_XoQIJ1yDrdAw89aIbaL_N_yB0AFHrGN8xNjCeuAgYsuuwE6_g_InUfK_OGYebqswud87gxdDsYowpJIFQsJi7LxRoj61ty6tg9CcOeyTc2mP-vmnycnqr6UzJck0q0EH3dbVsKhCMpbNTTbxdPhSjW4lQ |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Access-token JWT committed; remove and replace with placeholders.
Same secret/PII exposure as the other cookie files. Delete, purge from history, and switch to non-sensitive fixtures.
-# Netscape HTTP Cookie File
-# https://curl.se/docs/http-cookies.html
-# This file was generated by libcurl! Edit at your own risk.
-
-localhost FALSE / FALSE 1756455453 access-token <REDACTED>
-Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Gitleaks (8.27.2)
5-5: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🤖 Prompt for AI Agents
In server/cookies3.txt around line 5, a real access-token JWT with sensitive
data was committed; remove the token and replace it with a non-sensitive
placeholder value (e.g. ACCESS_TOKEN_PLACEHOLDER) in this file, then purge the
secret from repository history (use git filter-repo or BFG) and force-push the
cleaned branch, rotate any exposed keys/secrets used by that token, and add this
file pattern to .gitignore or move to a sanitized test fixture to prevent future
commits.
| externalId: text("external_id").unique().notNull(), | ||
| // this will come handy for jwt token | ||
| workspaceExternalId: text("workspace_external_id").notNull(), | ||
| password: text("password"), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Do not store plaintext passwords; use a one‑way hash or remove the column (Keycloak makes this unnecessary).
Storing password as text is a high‑risk vulnerability. If local creds are truly needed, store only a hash (e.g., Argon2id) and never the plaintext. Given Keycloak handles auth, the safer path is to drop this column entirely.
If you must keep it, rename to hashed_password and ensure only hashed values are written. Also keep it omitted from public schema.
Apply:
- password: text("password"),
+ hashedPassword: text("hashed_password"),- password: true,
+ hashedPassword: true,Migration note: create a DB migration to (a) backfill or (b) drop the old password column and add hashed_password (nullable).
Also applies to: 71-72
🤖 Prompt for AI Agents
In server/db/schema/users.ts around line 40 (and also check lines 71-72), the
schema defines a plaintext `password` column which must be removed or replaced:
remove the plaintext `password` field from the public schema, and either drop
the column entirely or rename it to `hashed_password` and ensure only hashed
values are written (use Argon2id or similar) and that the column is omitted from
any public serializers; implement a DB migration that either backfills existing
passwords into a new nullable `hashed_password` column (if you can securely
re-hash) or drops the old `password` column and creates `hashed_password`
nullable, and update application code to write only hashed values and to never
expose the field in APIs.
| password?: string, | ||
| ) => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Plaintext password persistence — hash before storing or remove entirely.
Never write plaintext passwords. Either drop password handling (Keycloak owns auth) or store a one‑way hash in hashed_password.
- password?: string,
+ plaintextPassword?: string,- password,
+ hashedPassword: plaintextPassword
+ ? await hashPassword(plaintextPassword)
+ : null,Outside this hunk, add a helper (example):
// server/utils/password.ts
import argon2 from "argon2"
export async function hashPassword(pw: string) {
return argon2.hash(pw, { type: argon2.argon2id, memoryCost: 19456, timeCost: 2, parallelism: 1 })
}And import hashPassword here. Also update schema to use hashed_password (see users.ts comment) and add a migration.
Also applies to: 136-137
server/fresh_cookies.txt
Outdated
| # https://curl.se/docs/http-cookies.html | ||
| # This file was generated by libcurl! Edit at your own risk. | ||
|
|
||
| localhost FALSE / FALSE 1756456231 access-token eyJhbGciOiJSUzI1NiIsInR5cCIgOiAiSldUIiwia2lkIiA6ICJhMm5idEV3SllqdFFRZGdleUxNZlZoS3k3cUs5SDd2bzhnNnV0X3dON1M4In0.eyJleHAiOjE3NTY0NTQ3MzEsImlhdCI6MTc1NjQ1NDQzMSwianRpIjoib25ydHJvOmRkYjRiMzA2LTBlNDgtMDMxZi1kZGIyLTYxNTEwMTE3YzRmMCIsImlzcyI6Imh0dHA6Ly9sb2NhbGhvc3Q6ODA4MS9yZWFsbXMveHluZS1zaGFyZWQiLCJhdWQiOiJhY2NvdW50Iiwic3ViIjoiZDliNTA0NzItYjEzNi00ZTk0LThjYjgtNDRiM2JkMzE0ZmIwIiwidHlwIjoiQmVhcmVyIiwiYXpwIjoib2EtYmFja2VuZCIsInNpZCI6ImUwYmE2ODA1LTgwYmItNDZlOC1hOWU5LTFlZDdlZmM2MGNkMSIsImFjciI6IjEiLCJhbGxvd2VkLW9yaWdpbnMiOlsiKiIsImh0dHA6Ly9sb2NhbGhvc3Q6MzAwMCIsImh0dHA6Ly9sb2NhbGhvc3Q6NTE3MyJdLCJyZWFsbV9hY2Nlc3MiOnsicm9sZXMiOlsib2ZmbGluZV9hY2Nlc3MiLCJ1bWFfYXV0aG9yaXphdGlvbiIsImRlZmF1bHQtcm9sZXMteHluZS1zaGFyZWQiXX0sInJlc291cmNlX2FjY2VzcyI6eyJhY2NvdW50Ijp7InJvbGVzIjpbIm1hbmFnZS1hY2NvdW50IiwibWFuYWdlLWFjY291bnQtbGlua3MiLCJ2aWV3LXByb2ZpbGUiXX19LCJzY29wZSI6Im9wZW5pZCBlbWFpbCBwcm9maWxlIiwiZW1haWxfdmVyaWZpZWQiOmZhbHNlLCJuYW1lIjoiRGVib2p5b3RpIE1hbmRhbCIsInByZWZlcnJlZF91c2VybmFtZSI6ImRlYm9qeW90aS5tYW5kYWxAanVzcGF5LmluIiwiZ2l2ZW5fbmFtZSI6IkRlYm9qeW90aSIsImZhbWlseV9uYW1lIjoiTWFuZGFsIiwiZW1haWwiOiJkZWJvanlvdGkubWFuZGFsQGp1c3BheS5pbiJ9.ZoeZNw7fGu6Ums3EYxvJF4-X3IC_nBOKebZ_RPjNyb_T8Jomy50VBTKd3RfyhubkZqgeY6SZ1PHWODCGlC-8ocR4t7MRC5cjZsUYVnboxNo36DREb9g0Bx7Ai-ruIvR3vpDyF8tsVqd-4sjgVqtEN-T_CZk9EGI1NI39auZyTEooAAQDuj6pOV4hOEtll8EKYHc-qtj9pvmRouhbJ1ME42DqW4XqbdECfu2NNhsCUtfOnfoXKAXBt1PXNEfD_9WXP_2LsYPeluSqYMQ4aJ9AdhSJq2J8wMvYiFO7hPy_ft8E2GysSJlG-yALWLEB5fGTErxO41vxKPys_iexFnLZvw |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove committed JWT cookie; replace with non-sensitive fixture and purge from history.
A real-looking JWT with PII (name/email) is committed. Even if expired/local, this is a severe secret/PII leak and will be picked up by scanners. Delete the file, rewrite history to purge it, and rotate any associated credentials/keys.
Action:
- Remove this file from the repo and add an ignore rule for such artifacts.
- Use placeholder tokens in fixtures or generate short-lived test tokens at test runtime with a known test key.
- Consider a curl cookie example with placeholders instead of real values.
Apply this diff to delete contents (then remove the file in the PR):
-# Netscape HTTP Cookie File
-# https://curl.se/docs/http-cookies.html
-# This file was generated by libcurl! Edit at your own risk.
-
-localhost FALSE / FALSE 1756456231 access-token <REDACTED>
-Suggested repo hardening (add to .gitignore at repo root):
+server/cookies*.txt
+frontend/cookies*.txtIf needed, I can provide a minimal cookie-jar fixture with placeholders and a jose-based helper to mint test JWTs.
📝 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.
| localhost FALSE / FALSE 1756456231 access-token eyJhbGciOiJSUzI1NiIsInR5cCIgOiAiSldUIiwia2lkIiA6ICJhMm5idEV3SllqdFFRZGdleUxNZlZoS3k3cUs5SDd2bzhnNnV0X3dON1M4In0.eyJleHAiOjE3NTY0NTQ3MzEsImlhdCI6MTc1NjQ1NDQzMSwianRpIjoib25ydHJvOmRkYjRiMzA2LTBlNDgtMDMxZi1kZGIyLTYxNTEwMTE3YzRmMCIsImlzcyI6Imh0dHA6Ly9sb2NhbGhvc3Q6ODA4MS9yZWFsbXMveHluZS1zaGFyZWQiLCJhdWQiOiJhY2NvdW50Iiwic3ViIjoiZDliNTA0NzItYjEzNi00ZTk0LThjYjgtNDRiM2JkMzE0ZmIwIiwidHlwIjoiQmVhcmVyIiwiYXpwIjoib2EtYmFja2VuZCIsInNpZCI6ImUwYmE2ODA1LTgwYmItNDZlOC1hOWU5LTFlZDdlZmM2MGNkMSIsImFjciI6IjEiLCJhbGxvd2VkLW9yaWdpbnMiOlsiKiIsImh0dHA6Ly9sb2NhbGhvc3Q6MzAwMCIsImh0dHA6Ly9sb2NhbGhvc3Q6NTE3MyJdLCJyZWFsbV9hY2Nlc3MiOnsicm9sZXMiOlsib2ZmbGluZV9hY2Nlc3MiLCJ1bWFfYXV0aG9yaXphdGlvbiIsImRlZmF1bHQtcm9sZXMteHluZS1zaGFyZWQiXX0sInJlc291cmNlX2FjY2VzcyI6eyJhY2NvdW50Ijp7InJvbGVzIjpbIm1hbmFnZS1hY2NvdW50IiwibWFuYWdlLWFjY291bnQtbGlua3MiLCJ2aWV3LXByb2ZpbGUiXX19LCJzY29wZSI6Im9wZW5pZCBlbWFpbCBwcm9maWxlIiwiZW1haWxfdmVyaWZpZWQiOmZhbHNlLCJuYW1lIjoiRGVib2p5b3RpIE1hbmRhbCIsInByZWZlcnJlZF91c2VybmFtZSI6ImRlYm9qeW90aS5tYW5kYWxAanVzcGF5LmluIiwiZ2l2ZW5fbmFtZSI6IkRlYm9qeW90aSIsImZhbWlseV9uYW1lIjoiTWFuZGFsIiwiZW1haWwiOiJkZWJvanlvdGkubWFuZGFsQGp1c3BheS5pbiJ9.ZoeZNw7fGu6Ums3EYxvJF4-X3IC_nBOKebZ_RPjNyb_T8Jomy50VBTKd3RfyhubkZqgeY6SZ1PHWODCGlC-8ocR4t7MRC5cjZsUYVnboxNo36DREb9g0Bx7Ai-ruIvR3vpDyF8tsVqd-4sjgVqtEN-T_CZk9EGI1NI39auZyTEooAAQDuj6pOV4hOEtll8EKYHc-qtj9pvmRouhbJ1ME42DqW4XqbdECfu2NNhsCUtfOnfoXKAXBt1PXNEfD_9WXP_2LsYPeluSqYMQ4aJ9AdhSJq2J8wMvYiFO7hPy_ft8E2GysSJlG-yALWLEB5fGTErxO41vxKPys_iexFnLZvw |
🧰 Tools
🪛 Gitleaks (8.27.2)
5-5: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🤖 Prompt for AI Agents
server/fresh_cookies.txt lines 5-5: This file contains a committed real JWT with
PII and must be removed; delete the file from the PR, purge it from repository
history (use git filter-repo or BFG to remove all occurrences and then
force-push), add an entry to .gitignore to prevent committing cookie jars or
.txt token dumps, replace the committed artifact with a non-sensitive fixture
(use placeholder token strings or code that generates short-lived test JWTs at
runtime with a test key), and rotate any affected credentials/keys that may have
been exposed.
130c3c9 to
46e0076
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (19)
deployment/.env.prod.example (1)
33-38: Use HTTPS production hostname for Keycloak; avoid localhost in prod template.Also place BASE_URL before ENABLED to satisfy dotenv-linter order.
-# Keycloak integration -KEYCLOAK_ENABLED=true -KEYCLOAK_BASE_URL=http://localhost:8081 +# Keycloak integration +# Use your public HTTPS hostname (behind a reverse proxy). Omit port when using 443. +KEYCLOAK_BASE_URL=https://auth.change-me.example.com +KEYCLOAK_ENABLED=true KEYCLOAK_DEFAULT_REALM=xyne-shared KEYCLOAK_CLIENT_ID=oa-backend KEYCLOAK_CLIENT_SECRET=your-client-secret-from-setupdeployment/setup-keycloak.sh (5)
30-46: Don’t swallow errors on realm creation; check existence and use curl -f.-echo "🌍 Creating realm: ${REALM_NAME}..." -curl -s -X POST "${KEYCLOAK_URL}/admin/realms" \ +echo "🌍 Ensuring realm exists: ${REALM_NAME}..." +REALM_EXISTS=$(curl -s -o /dev/null -w "%{http_code}" -H "Authorization: Bearer ${ADMIN_TOKEN}" "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}") +if [ "$REALM_EXISTS" -ne 200 ]; then +curl -s -f -X POST "${KEYCLOAK_URL}/admin/realms" \ -H "Authorization: Bearer ${ADMIN_TOKEN}" \ -H "Content-Type: application/json" \ -d '{ "realm": "'${REALM_NAME}'", "enabled": true, "displayName": "XYNE Shared Realm", "registrationAllowed": true, "loginWithEmailAllowed": true, "duplicateEmailsAllowed": false, "resetPasswordAllowed": true, "editUsernameAllowed": true, "bruteForceProtected": false - }' || echo "Realm might already exist" + }' +else + echo "✅ Realm ${REALM_NAME} already exists, skipping creation." +fi
8-13: Parameterize URL/admin creds; echo context; allow CI fail-fast.-KEYCLOAK_URL="http://localhost:8081" -ADMIN_USER="admin" -ADMIN_PASS="admin" -REALM_NAME="xyne-shared" -CLIENT_ID="oa-backend" +: "${KEYCLOAK_URL:=http://localhost:8081}" +: "${ADMIN_USER:=admin}" +: "${ADMIN_PASS:=admin}" +: "${REALM_NAME:=xyne-shared}" +: "${CLIENT_ID:=oa-backend}" +export KEYCLOAK_URL ADMIN_USER ADMIN_PASS REALM_NAME CLIENT_ID +echo "Using KEYCLOAK_URL=${KEYCLOAK_URL} REALM=${REALM_NAME} CLIENT_ID=${CLIENT_ID}" + +# Optional CI guard +if [ "${CI:-false}" = "true" ]; then + : "${ADMIN_USER:?ADMIN_USER required in CI}" + : "${ADMIN_PASS:?ADMIN_PASS required in CI}" +fi
79-85: Fetch and print client secret after client creation.echo "📋 Client UUID: ${CLIENT_UUID}" +# Get and display client secret for confidential client +CLIENT_SECRET=$(curl -s -f -X GET "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}/clients/${CLIENT_UUID}/client-secret" \ + -H "Authorization: Bearer ${ADMIN_TOKEN}" | jq -r '.value') +echo "🔑 Client Secret: ${CLIENT_SECRET}"
86-105: Remove PII and weak password from test user; parameterize.-echo "👤 Creating test user: debojyoti.mandal@juspay.in..." +echo "👤 Creating test user: ${TEST_USERNAME:-test.user@example.com}..." curl -s -X POST "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}/users" \ -H "Authorization: Bearer ${ADMIN_TOKEN}" \ -H "Content-Type: application/json" \ -d '{ - "username": "debojyoti.mandal@juspay.in", - "email": "debojyoti.mandal@juspay.in", - "firstName": "Debojyoti", - "lastName": "Mandal", + "username": "'${TEST_USERNAME:-test.user@example.com}'", + "email": "'${TEST_USERNAME:-test.user@example.com}'", + "firstName": "Test", + "lastName": "User", "enabled": true, "emailVerified": false, "credentials": [{ "type": "password", - "value": "1", + "value": "'${TEST_PASSWORD:-devPassw0rd!}'", "temporary": false }] }' || echo "User might already exist"
110-120: Password grant needs client_secret for confidential client; use vars.-TEST_TOKEN=$(curl -s -X POST "${KEYCLOAK_URL}/realms/${REALM_NAME}/protocol/openid-connect/token" \ +TEST_TOKEN=$(curl -s -f -X POST "${KEYCLOAK_URL}/realms/${REALM_NAME}/protocol/openid-connect/token" \ -H "Content-Type: application/x-www-form-urlencoded" \ - -d "grant_type=password&client_id=${CLIENT_ID}&username=debojyoti.mandal@juspay.in&password=1&scope=openid email profile" | \ + -d "grant_type=password&client_id=${CLIENT_ID}&client_secret=${CLIENT_SECRET}&username=${TEST_USERNAME:-test.user@example.com}&password=${TEST_PASSWORD:-devPassw0rd!}&scope=openid email profile" | \ jq -r '.access_token' 2>/dev/null)server/auth/keycloak/middleware.ts (4)
33-36: Cookie name mismatch; include xyne-auth-token or standardize.- const cookieToken = getCookie(c, "auth-token") || getCookie(c, "access-token") + const cookieToken = + getCookie(c, "access-token") || + getCookie(c, "auth-token") || + getCookie(c, "xyne-auth-token")
150-166: Avoid hardcoded clientId ("oa-backend") when reading resource_access.- const resourceRoles = payload.resource_access?.["oa-backend"]?.roles || [] + const resourceRoles = + payload.resource_access?.[currentConfig.clientId]?.roles || [] @@ - const oaBackendRoles = payload.resource_access?.["oa-backend"]?.roles || [] + const oaBackendRoles = + payload.resource_access?.[currentConfig.clientId]?.roles || []
124-146: Critical: Keycloak JWT is never signature-verified. Use JWKS with jose.+import { createRemoteJWKSet, jwtVerify } from "jose" @@ -async function validateKeycloakToken( - token: string, -): Promise<AuthContext | null> { +async function validateKeycloakToken( + token: string, + currentConfig: ReturnType<typeof getKeycloakConfig>, +): Promise<AuthContext | null> { try { - // Decode JWT payload first - const parts = token.split(".") - if (parts.length !== 3) { - return null - } - - const payload = JSON.parse(Buffer.from(parts[1], "base64").toString()) + // Verify RS256 signature against Keycloak JWKS and extract payload + const issuer = `${currentConfig.baseUrl}/realms/${currentConfig.defaultRealm}` + const jwks = createRemoteJWKSet( + new URL(`${issuer}/protocol/openid-connect/certs`), + ) + const { payload } = await jwtVerify(token, jwks, { + issuer, + audience: currentConfig.clientId, + })
171-201: Hardcoded workspace IDs risk cross-tenant data leakage. Parameterize now; derive later.- await createUser( - db, - 1, // Default workspace ID (juspay.in) + await createUser( + db, + Number(process.env.KEYCLOAK_DEFAULT_WORKSPACE_ID ?? 1), userEmail, userName, "", // No photo link from Keycloak by default primaryRole, - "wkh5nwq7o0es10kpcgg8lu9o", // juspay.in workspace external ID + process.env.KEYCLOAK_DEFAULT_WORKSPACE_EXTERNAL_ID ?? + "wkh5nwq7o0es10kpcgg8lu9o", // default external ID undefined, // No password for Keycloak users ) @@ - let workspaceExternalId = "wkh5nwq7o0es10kpcgg8lu9o" // Default to juspay.in workspace + let workspaceExternalId = + process.env.KEYCLOAK_DEFAULT_WORKSPACE_EXTERNAL_ID ?? + "wkh5nwq7o0es10kpcgg8lu9o" // default external IDFollow-up: derive tenant/workspace from token claims (realm, groups, or a mapping) instead of defaults.
frontend/src/routes/auth.tsx (3)
41-48: Use loader data instead of route context for beforeLoad errors.useRouteContext() won’t carry loader return values. Read loader output via useLoaderData and simplify the effect.
- // Get route data for any errors from beforeLoad - const route = Route.useRouteContext() - - useEffect(() => { - if (route?.error) { - setError(route.error) - } - }, [route?.error]) + // Get loader data for any errors from beforeLoad + const data = Route.useLoaderData() + useEffect(() => { + if (data?.error) setError(data.error) + }, [data?.error])
23-28: Don’t construct OAuth URL on the client; use server-provided loginUrl.Prevents state/PKCE/redirect mismatches and centralizes security on backend. Also surface feature flag from config.
interface KeycloakConfig { - baseUrl: string - realm: string - clientId: string - loginUrl: string + enabled: boolean + loginUrl: string } ... - // Fetch Keycloak configuration + // Fetch Keycloak configuration const fetchKeycloakConfig = async () => { try { const response = await fetch(`${import.meta.env.VITE_API_BASE_URL}/api/keycloak/config`) if (response.ok) { const config = await response.json() setKeycloakConfig(config) - setKeycloakEnabled(true) + setKeycloakEnabled(!!config.enabled) logger.info("Keycloak configuration loaded:", config) } else { logger.warn("Keycloak configuration not available") } } catch (error) { logger.error("Failed to fetch Keycloak configuration:", error) } } ... - const handleKeycloakLogin = () => { + const handleKeycloakLogin = () => { if (!keycloakConfig) { logger.error("Keycloak configuration not available") return } - logger.info("User clicked login with Keycloak SSO") - - // Build Keycloak authorization URL - make sure it goes to user realm, not admin - const params = new URLSearchParams({ - client_id: keycloakConfig.clientId, - redirect_uri: `${window.location.origin}/auth`, - response_type: "code", - scope: "openid email profile", - state: Math.random().toString(36).substring(2, 15), - }) - - // Use the user realm URL, not admin realm - const userLoginUrl = `${keycloakConfig.baseUrl}/realms/${keycloakConfig.realm}/protocol/openid-connect/auth` - const authUrl = `${userLoginUrl}?${params.toString()}` - - logger.info("Redirecting to Keycloak:", authUrl) - window.location.href = authUrl + logger.info("Redirecting to Keycloak SSO") + window.location.href = keycloakConfig.loginUrl }Also applies to: 49-58, 76-99
70-74: Fix Google callback URL: missing /api prefix.Current path likely 404s; align with hc base /api/v1.
- const redirectUrl = `${import.meta.env.VITE_API_BASE_URL}/v1/auth/callback` + const redirectUrl = `${import.meta.env.VITE_API_BASE_URL}/api/v1/auth/callback`deployment/docker-compose.prod.yml (5)
108-110: Pin Keycloak image; avoid floating latest.Make the version explicit via env to ensure reproducible deploys.
- image: quay.io/keycloak/keycloak:latest + image: quay.io/keycloak/keycloak:${KEYCLOAK_VERSION:?pin explicit version}
111-114: Do not ship default admin creds in prod. Require explicit values.Prevents accidental weak credentials in production.
- - KEYCLOAK_ADMIN=${KEYCLOAK_ADMIN:-admin} - - KEYCLOAK_ADMIN_PASSWORD=${KEYCLOAK_ADMIN_PASSWORD:-admin} + - KEYCLOAK_ADMIN=${KEYCLOAK_ADMIN:?set in .env} + - KEYCLOAK_ADMIN_PASSWORD=${KEYCLOAK_ADMIN_PASSWORD:?set in .env}
117-119: KC_DB_URL points to a DB that isn’t provisioned.xyne-db creates “xyne”, not “keycloak”. Align URL or provision the DB at init.
- - KC_DB_URL=jdbc:postgresql://xyne-db:5432/keycloak + - KC_DB_URL=jdbc:postgresql://xyne-db:5432/xyneAlternatively, create a keycloak database via init scripts and keep the original URL.
121-126: Harden hostname and HTTPS settings; disable plain HTTP.Use strict hostname checks and HTTPS-only in production; rely on a reverse proxy for TLS.
- - KC_HOSTNAME_STRICT=false - - KC_HOSTNAME_STRICT_HTTPS=false - - KC_HTTP_ENABLED=true + - KC_HOSTNAME_STRICT=true + - KC_HOSTNAME_STRICT_HTTPS=true + - KC_HTTP_ENABLED=false
133-134: Use a Compose-suitable cache stack.“kubernetes” requires KUBE_PING; for Compose use “local” (single node) or “tcp”.
- - KC_CACHE=ispn - - KC_CACHE_STACK=kubernetes + - KC_CACHE=ispn + - KC_CACHE_STACK=localdeployment/KEYCLOAK_SETUP.md (1)
108-108: Replace real-looking email with a neutral placeholder.Remove PII from docs.
-- **Test User**: `debojyoti.mandal@juspay.in` / `*` +- **Example Test User**: `user@example.com` / `changeme`
🧹 Nitpick comments (14)
deployment/.env.prod.example (3)
41-44: Minor: reorder secrets to quiet dotenv-linter.-# Application secrets -USER_SECRET=your-jwt-secret-here -ACCESS_TOKEN_SECRET=your-access-token-secret-here -REFRESH_TOKEN_SECRET=your-refresh-token-secret-here +# Application secrets +ACCESS_TOKEN_SECRET=your-access-token-secret-here +REFRESH_TOKEN_SECRET=your-refresh-token-secret-here +USER_SECRET=your-jwt-secret-here
12-16: Clarify port guidance.Explicitly note KEYCLOAK_PORT is optional and typically omitted in prod TLS setups.
-# Keycloak port mapping (optional, defaults to 8081) +# Keycloak port mapping (optional for non-standard ports; omit for HTTPS :443) KEYCLOAK_PORT=8081
92-94: Add trailing newline.-# ./backup-restore.sh restore production-YYYYMMDD \ No newline at end of file +# ./backup-restore.sh restore production-YYYYMMDD +deployment/setup-keycloak.sh (2)
6-6: Harden shell settings.Fail on unset vars and pipeline errors for safer automation.
-set -e +set -euo pipefail
125-134: Avoid printing plaintext passwords; reference env instead.-echo " Test User: debojyoti.mandal@juspay.in / password: 1" +echo " Test User: ${TEST_USERNAME:-test.user@example.com} (password via TEST_PASSWORD env)"server/auth/keycloak/middleware.ts (4)
100-121: Pass config into validator; avoid stale global reads and log-safety.- if (currentKeycloakConfig.enabled) { + if (currentKeycloakConfig.enabled) { Logger.info("Attempting Keycloak token validation") - const keycloakResult = await validateKeycloakToken(token) + const keycloakResult = await validateKeycloakToken(token, currentKeycloakConfig)
142-145: Use the passed config for issuer check, not the module-global.- if (!payload.iss || !payload.iss.includes(keycloakConfig.baseUrl)) { + if (!payload.iss || !payload.iss.includes(currentConfig.baseUrl)) { return null }
258-266: Trim and normalize VALID_API_KEYS entries.- const validApiKeys = process.env.VALID_API_KEYS?.split(",") || [] + const validApiKeys = + process.env.VALID_API_KEYS?.split(",").map((s) => s.trim()).filter(Boolean) || []
206-219: Prefer sub from token for AuthContext.sub; keep email separately.- sub: userEmail, + sub: (payload.sub as string) || userEmail, role: primaryRole,deployment/docker-compose.prod.yml (2)
143-148: Optional: wait for DB readiness via healthcheck.Compose’s depends_on doesn’t wait for health. Consider adding condition to reduce flakiness.
- depends_on: - - xyne-db + depends_on: + xyne-db: + condition: service_startedIf you add a healthcheck to xyne-db, switch to condition: service_healthy.
112-131: Trailing spaces flagged by YAMLlint.Clean up to keep CI green.
deployment/KEYCLOAK_SETUP.md (3)
9-24: Add code-fence language for the ASCII diagram.Satisfies markdownlint MD040.
-``` +```text ┌─────────────────┐ ┌─────────────────┐ ┌─────────────────┐ │ XYNE Server │ │ Keycloak │ │ PostgreSQL │ ... -``` +```
102-105: Avoid bare URL; linkify the Admin Console.Fixes markdownlint MD034.
-- **Admin Console**: http://localhost:8081/admin/ +- **Admin Console**: <http://localhost:8081/admin/>
267-267: Fix typo: Infinispan.-- **Production cache**: Infinspan with Kubernetes clustering support +- **Production cache**: Infinispan with Kubernetes clustering support
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (15)
deployment/.env.prod.example(1 hunks)deployment/KEYCLOAK_SETUP.md(1 hunks)deployment/docker-compose.dev.yml(1 hunks)deployment/docker-compose.prod.yml(1 hunks)deployment/setup-keycloak.sh(1 hunks)frontend/package.json(3 hunks)frontend/src/routes/auth.tsx(3 hunks)server/api/auth.ts(2 hunks)server/auth/keycloak/api.ts(1 hunks)server/auth/keycloak/config.ts(1 hunks)server/auth/keycloak/middleware.ts(1 hunks)server/auth/keycloak/routes.ts(1 hunks)server/db/schema/users.ts(3 hunks)server/db/user.ts(2 hunks)server/server.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- server/auth/keycloak/config.ts
- server/db/user.ts
- frontend/package.json
- deployment/docker-compose.dev.yml
- server/db/schema/users.ts
- server/auth/keycloak/routes.ts
- server/auth/keycloak/api.ts
- server/api/auth.ts
- server/server.ts
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/src/routes/auth.tsx (3)
server/auth/keycloak/config.ts (1)
KeycloakConfig(1-10)frontend/src/routes/_authenticated.tsx (1)
Route(5-18)frontend/src/api.ts (1)
api(4-4)
server/auth/keycloak/middleware.ts (4)
server/logger/index.ts (1)
getLogger(33-90)server/auth/keycloak/config.ts (1)
getKeycloakConfig(12-23)server/routes/vespa-proxy.ts (1)
validateApiKey(60-80)server/db/user.ts (2)
getUserByEmail(98-107)createUser(109-139)
🪛 GitHub Actions: TypeScript Build Check
frontend/src/routes/auth.tsx
[error] 123-123: bunx tsc -b: TypeScript error TS6133: 'result' is declared but its value is never read.
[error] 334-334: bunx tsc -b: TypeScript error TS6133: 'tokens' is declared but its value is never read.
🪛 Shellcheck (0.10.0)
deployment/setup-keycloak.sh
[warning] 73-73: Quote this to prevent word splitting.
(SC2046)
🪛 dotenv-linter (3.3.0)
deployment/.env.prod.example
[warning] 24-24: [UnorderedKey] The POSTGRES_PASSWORD key should go before the POSTGRES_USER key
(UnorderedKey)
[warning] 35-35: [UnorderedKey] The KEYCLOAK_BASE_URL key should go before the KEYCLOAK_ENABLED key
(UnorderedKey)
[warning] 36-36: [UnorderedKey] The KEYCLOAK_DEFAULT_REALM key should go before the KEYCLOAK_ENABLED key
(UnorderedKey)
[warning] 37-37: [UnorderedKey] The KEYCLOAK_CLIENT_ID key should go before the KEYCLOAK_DEFAULT_REALM key
(UnorderedKey)
[warning] 38-38: [UnorderedKey] The KEYCLOAK_CLIENT_SECRET key should go before the KEYCLOAK_DEFAULT_REALM key
(UnorderedKey)
[warning] 42-42: [UnorderedKey] The ACCESS_TOKEN_SECRET key should go before the USER_SECRET key
(UnorderedKey)
[warning] 43-43: [UnorderedKey] The REFRESH_TOKEN_SECRET key should go before the USER_SECRET key
(UnorderedKey)
[warning] 52-52: [UnorderedKey] The GRAFANA_ENABLED key should go before the METRICS_ENABLED key
(UnorderedKey)
[warning] 94-94: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🪛 LanguageTool
deployment/KEYCLOAK_SETUP.md
[grammar] ~28-~28: There might be a mistake here.
Context: ...ice | Container Name | Ports | Purpose | |---------|---------------|-------|-----...
(QB_NEW_EN)
[grammar] ~29-~29: There might be a mistake here.
Context: ...-----|---------------|-------|---------| | PostgreSQL | xyne-db | 5432 | Database...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ... | Database for both XYNE and Keycloak | | Keycloak | xyne-keycloak | 8081→8080 |...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...ak | 8081→8080 | Authentication server | | Vespa | vespa | 8080, 19071 | Search e...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ... | vespa | 8080, 19071 | Search engine | | Prometheus | xyne-prometheus | 9090 | ...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...prometheus | 9090 | Metrics collection | | Grafana | xyne-grafana | 3002→3000 | M...
(QB_NEW_EN)
[grammar] ~34-~34: There might be a mistake here.
Context: ...na | 3002→3000 | Monitoring dashboards | | Loki | loki | 3100 | Log aggregation |...
(QB_NEW_EN)
[grammar] ~92-~92: There might be a mistake here.
Context: ...es | Variable | Default | Description | |----------|---------|-------------| | `...
(QB_NEW_EN)
[grammar] ~93-~93: There might be a mistake here.
Context: ...n | |----------|---------|-------------| | KEYCLOAK_ENABLED | false | Enable ...
(QB_NEW_EN)
[grammar] ~94-~94: There might be a mistake here.
Context: ...alse| Enable Keycloak authentication | |KEYCLOAK_BASE_URL|http://localhos...
(QB_NEW_EN)
[grammar] ~95-~95: There might be a mistake here.
Context: .../localhost:8081| Keycloak server URL | |KEYCLOAK_DEFAULT_REALM|xyne-share...
(QB_NEW_EN)
[grammar] ~96-~96: There might be a mistake here.
Context: ...|xyne-shared| Default realm name | |KEYCLOAK_CLIENT_ID|oa-backend` | ...
(QB_NEW_EN)
[grammar] ~97-~97: There might be a mistake here.
Context: ...D|oa-backend| Client ID for XYNE | |KEYCLOAK_CLIENT_SECRET` | *(required)...
(QB_NEW_EN)
[grammar] ~102-~102: There might be a mistake here.
Context: ... Console**: http://localhost:8081/admin/ - Username: admin - Password: admin ...
(QB_NEW_EN)
[grammar] ~103-~103: There might be a mistake here.
Context: ...p://localhost:8081/admin/ - Username: admin - Password: admin - Realm: `xyne-sh...
(QB_NEW_EN)
[grammar] ~106-~106: There might be a mistake here.
Context: ...in - Password:admin - **Realm**:xyne-shared- **Client**:oa-backend` - Test User: ...
(QB_NEW_EN)
[grammar] ~107-~107: There might be a mistake here.
Context: ... Realm: xyne-shared - Client: oa-backend - Test User: `debojyoti.mandal@juspay.in...
(QB_NEW_EN)
[grammar] ~114-~114: There might be a mistake here.
Context: ...methods: 1. Google OAuth (existing) 2. Keycloak SSO (browser redirect) 3. **E...
(QB_NEW_EN)
[grammar] ~115-~115: There might be a mistake here.
Context: ...) 2. Keycloak SSO (browser redirect) 3. Email/Password (direct login via Keycl...
(QB_NEW_EN)
[grammar] ~136-~136: There might be a mistake here.
Context: ...All data is persisted in Docker volumes: - PostgreSQL data: ../server/xyne-data -...
(QB_NEW_EN)
[grammar] ~251-~251: There might be a mistake here.
Context: ... | Feature | Development | Production | |---------|-------------|------------| |...
(QB_NEW_EN)
[grammar] ~252-~252: There might be a mistake here.
Context: ...| |---------|-------------|------------| | Keycloak Mode | start-dev (hot r...
(QB_NEW_EN)
[grammar] ~253-~253: There might be a mistake here.
Context: ...| start --optimized (faster startup) | | Container Name | xyne-keycloak |...
(QB_NEW_EN)
[grammar] ~254-~254: There might be a mistake here.
Context: ...xyne-keycloak | xyne-keycloak-prod | | Resource Limits | None | 1GB RAM, ...
(QB_NEW_EN)
[grammar] ~255-~255: There might be a mistake here.
Context: ...rce Limits** | None | 1GB RAM, 0.5 CPU | | Cache Strategy | Simple | Kubernet...
(QB_NEW_EN)
[grammar] ~256-~256: There might be a mistake here.
Context: ...| Simple | Kubernetes clustering ready | | Health Checks | Basic | Extended w...
(QB_NEW_EN)
[grammar] ~257-~257: There might be a mistake here.
Context: ...ic | Extended with longer startup time | | Logging | DEBUG friendly | INFO/WA...
(QB_NEW_EN)
[grammar] ~264-~264: There might be a mistake here.
Context: ...: Faster startup and runtime performance - Resource limits: Memory and CPU constr...
(QB_NEW_EN)
[grammar] ~265-~265: There might be a mistake here.
Context: ... constraints for container orchestration - Extended health checks: 120s startup p...
(QB_NEW_EN)
[grammar] ~266-~266: There might be a mistake here.
Context: ...0s startup period for optimization phase - Production cache: Infinspan with Kuber...
(QB_NEW_EN)
[grammar] ~267-~267: Ensure spelling is correct
Context: ...imization phase - Production cache: Infinspan with Kubernetes clustering support - **...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~267-~267: There might be a mistake here.
Context: ...nspan with Kubernetes clustering support - Environment-based configuration: All s...
(QB_NEW_EN)
[grammar] ~274-~274: There might be a mistake here.
Context: ...production deployment: 1. Security: - Change default admin credentials via env...
(QB_NEW_EN)
[grammar] ~280-~280: There might be a mistake here.
Context: ...re proper CORS origins 2. Database: - Use separate PostgreSQL instance for Key...
(QB_NEW_EN)
[grammar] ~285-~285: There might be a mistake here.
Context: ...se clustering if needed 3. Scaling: - Configure Keycloak clustering with share...
(QB_NEW_EN)
[grammar] ~286-~286: There might be a mistake here.
Context: ...g**: - Configure Keycloak clustering with shared database - Use external load ...
(QB_NEW_EN)
[grammar] ~290-~290: There might be a mistake here.
Context: ... limits and requests 4. Monitoring: - Enable Keycloak metrics integration with...
(QB_NEW_EN)
[grammar] ~303-~303: There might be a mistake here.
Context: ...ence** - PostgreSQL backend for Keycloak - Survives container restarts - Easy backu...
(QB_NEW_EN)
[grammar] ~304-~304: There might be a mistake here.
Context: ...r Keycloak - Survives container restarts - Easy backup/restore ✅ **Team Collaborat...
(QB_NEW_EN)
[grammar] ~308-~308: There might be a mistake here.
Context: ...ion** - Version-controlled configuration - Consistent realm/client setup - Shared d...
(QB_NEW_EN)
[grammar] ~309-~309: There might be a mistake here.
Context: ...guration - Consistent realm/client setup - Shared development environment ✅ **Prod...
(QB_NEW_EN)
[grammar] ~313-~313: There might be a mistake here.
Context: ...y** - Similar architecture to production - Proper database integration - Service di...
(QB_NEW_EN)
[grammar] ~314-~314: There might be a mistake here.
Context: ...production - Proper database integration - Service discovery patterns
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
deployment/KEYCLOAK_SETUP.md
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
102-102: Bare URL used
(MD034, no-bare-urls)
🪛 YAMLlint (1.37.1)
deployment/docker-compose.prod.yml
[error] 114-114: trailing spaces
(trailing-spaces)
[error] 120-120: trailing spaces
(trailing-spaces)
[error] 126-126: trailing spaces
(trailing-spaces)
[error] 131-131: trailing spaces
(trailing-spaces)
🔇 Additional comments (2)
frontend/src/routes/auth.tsx (1)
308-312: beforeLoad uses window; verify SSR safety.If SSR/prerendering is enabled, window is undefined in beforeLoad. Consider guarding with typeof window !== "undefined".
Do you render this route server-side at any point? If yes, I can patch in a safe guard.
deployment/KEYCLOAK_SETUP.md (1)
129-133: Doc–Compose mismatch: Keycloak DB provisioning.Docs state a separate “keycloak” DB, but prod compose points Keycloak at a DB that isn’t created by default. Either (a) change KC_DB_URL to xyne everywhere or (b) add an init step/script to create the “keycloak” DB and align examples accordingly.
I can update the compose/dev scripts and this doc to keep the DB story consistent—prefer (a) or (b)?
| # Email/Password Login | ||
| curl -X POST 'http://localhost:3000/api/keycloak/login' \ | ||
| -H 'Content-Type: application/json' \ | ||
| -d '{"email": "debojyoti.mandal@juspay.in", "password": "*"}' | ||
|
|
There was a problem hiding this comment.
Scrub PII from curl example.
Use placeholders.
- -d '{"email": "debojyoti.mandal@juspay.in", "password": "*"}'
+ -d '{"email": "user@example.com", "password": "changeme"}'📝 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.
| # Email/Password Login | |
| curl -X POST 'http://localhost:3000/api/keycloak/login' \ | |
| -H 'Content-Type: application/json' \ | |
| -d '{"email": "debojyoti.mandal@juspay.in", "password": "*"}' | |
| # Email/Password Login | |
| curl -X POST 'http://localhost:3000/api/keycloak/login' \ | |
| -H 'Content-Type: application/json' \ | |
| -d '{"email": "user@example.com", "password": "changeme"}' |
🤖 Prompt for AI Agents
In deployment/KEYCLOAK_SETUP.md around lines 161 to 165, the curl example
contains a real email address which exposes PII; replace the concrete email and
password with placeholders (e.g., user@example.com and your_password or {email}
and {password}) so the example uses non-identifiable values and update the JSON
payload in the curl command accordingly.
frontend/src/routes/auth.tsx
Outdated
| <Label htmlFor="email">Email</Label> | ||
| <Input | ||
| id="email" | ||
| type="email" | ||
| placeholder="debojyoti.mandal@juspay.in" | ||
| value={email} | ||
| onChange={(e) => setEmail(e.target.value)} | ||
| required | ||
| /> |
There was a problem hiding this comment.
Remove real-looking email from placeholder.
Avoid PII in UI and docs.
- placeholder="debojyoti.mandal@juspay.in"
+ placeholder="user@example.com"📝 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.
| <Label htmlFor="email">Email</Label> | |
| <Input | |
| id="email" | |
| type="email" | |
| placeholder="debojyoti.mandal@juspay.in" | |
| value={email} | |
| onChange={(e) => setEmail(e.target.value)} | |
| required | |
| /> | |
| <Label htmlFor="email">Email</Label> | |
| <Input | |
| id="email" | |
| type="email" | |
| placeholder="user@example.com" | |
| value={email} | |
| onChange={(e) => setEmail(e.target.value)} | |
| required | |
| /> |
🤖 Prompt for AI Agents
In frontend/src/routes/auth.tsx around lines 247 to 255 the input placeholder
uses a real-looking email (debojyoti.mandal@juspay.in) which may expose PII;
replace it with a non-identifying example such as "user@example.com" or
"email@example.com" (or a similarly generic placeholder) and ensure no other
placeholders in this file contain real-looking personal data.
| const Logger = getLogger(Subsystem.Server) | ||
| const { JwtPayloadKey } = config | ||
| const userSecret = process.env.USER_SECRET! | ||
| const keycloakConfig = getKeycloakConfig() | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fail fast if USER_SECRET is missing (prevents silent auth bypass).
-const userSecret = process.env.USER_SECRET!
+const userSecret = process.env.USER_SECRET
+if (!userSecret) {
+ throw new Error("USER_SECRET is not set")
+}📝 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.
| const Logger = getLogger(Subsystem.Server) | |
| const { JwtPayloadKey } = config | |
| const userSecret = process.env.USER_SECRET! | |
| const keycloakConfig = getKeycloakConfig() | |
| const Logger = getLogger(Subsystem.Server) | |
| const { JwtPayloadKey } = config | |
| const userSecret = process.env.USER_SECRET | |
| if (!userSecret) { | |
| throw new Error("USER_SECRET is not set") | |
| } | |
| const keycloakConfig = getKeycloakConfig() |
🤖 Prompt for AI Agents
In server/auth/keycloak/middleware.ts around lines 12 to 16, the code uses
process.env.USER_SECRET! which silently assumes USER_SECRET exists; change this
to a fail-fast check during module initialization: read process.env.USER_SECRET
into a const, and if it's undefined or empty throw a clear Error (or call
process.exit(1) after logging) so the application fails to start rather than
silently bypassing auth; remove the non-null assertion and ensure all downstream
code uses the validated variable.
| export interface AuthContext { | ||
| sub: string | ||
| role: string | ||
| workspaceId: string | ||
| source: "xyne" | "keycloak" | "api-key" | ||
| exp: number | ||
| user?: any | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
workspaceId vs workspaceExternalId type naming — verify callers.
You set AuthContext.workspaceId (string) to an external ID. Confirm no consumer expects a numeric DB ID.
Run to find consumers that assume a number:
Also applies to: 206-219
🏁 Script executed:
#!/bin/bash
rg -nP --type=ts -C2 '\bworkspaceId\s*:\s*number\b|get\("authContext"\).*workspaceId'Length of output: 12988
Rename and disambiguate workspace IDs in AuthContext
Change AuthContext to expose both the external-ID and the numeric DB ID (and deprecate the old string field) so callers aren’t broken:
export interface AuthContext {
sub: string
role: string
- workspaceId: string
+ /** internal numeric DB ID */
+ workspaceId: number
+ /** external UUID-style ID from auth provider */
+ workspaceExternalId: string
source: "xyne" | "keycloak" | "api-key"
exp: number
user?: any
}– Update middleware.ts to parse and set both values.
– Adjust all handlers/db calls that consume workspaceId (they currently expect number).
– Migrate any front-end types or clients that assumed string IDs.
📝 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.
| export interface AuthContext { | |
| sub: string | |
| role: string | |
| workspaceId: string | |
| source: "xyne" | "keycloak" | "api-key" | |
| exp: number | |
| user?: any | |
| } | |
| export interface AuthContext { | |
| sub: string | |
| role: string | |
| /** internal numeric DB ID */ | |
| workspaceId: number | |
| /** external UUID-style ID from auth provider */ | |
| workspaceExternalId: string | |
| source: "xyne" | "keycloak" | "api-key" | |
| exp: number | |
| user?: any | |
| } |
debojyoti-git
force-pushed
the
keycloak-integration
branch
from
46e0076 to
0bb92f8
Compare
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 (2)
server/db/user.ts (2)
146-154: Guard zero-row updates before parsing.
updatedUser[0]may be undefined if email not found;parsewill throw.- const updatedUser = await trx + const updatedUser = await trx .update(users) .set({ refreshToken, }) .where(eq(users.email, email)) .returning() - return selectUserSchema.parse(updatedUser[0]) + if (updatedUser.length === 0) { + throw new HTTPException(404, { message: "User not found" }) + } + return selectUserSchema.parse(updatedUser[0])
160-168: Same zero-row update risk here.Mirror the guard for delete path.
- const updatedUser = await trx + const updatedUser = await trx .update(users) .set({ refreshToken: "", }) .where(eq(users.email, email)) .returning() - return selectUserSchema.parse(updatedUser[0]) + if (updatedUser.length === 0) { + throw new HTTPException(404, { message: "User not found" }) + } + return selectUserSchema.parse(updatedUser[0])
♻️ Duplicate comments (21)
server/db/user.ts (2)
119-120: Stop accepting plaintext passwords; hash or remove.Storing/handling plaintext credentials is a critical security flaw. Either:
- Prefer: hash with argon2id and store in
hashedPassword.- Or (with Keycloak): drop local password entirely.
Option A — keep local login (hash):
- password?: string, + plaintextPassword?: string,Outside this hunk, add a helper and import:
// server/utils/password.ts import argon2 from "argon2" export async function hashPassword(pw: string) { return argon2.hash(pw, { type: argon2.argon2id, memoryCost: 19456, timeCost: 2, parallelism: 1 }) }// at top of this file import { hashPassword } from "@/utils/password"Also update schema/migration: replace
passwordcolumn withhashed_password(text, nullable), expose neither in public selects.Option B — SSO only (no local password):
- password?: string, + // no local password; Keycloak manages credentialsI can open a follow-up PR to wire argon2 + migration.
136-137: Never persist plaintext; write a hash or nothing.If you must support local auth, persist only a hash. Otherwise, remove the field.
Option A — keep local login (hash):
- password, + hashedPassword: plaintextPassword + ? await hashPassword(plaintextPassword) + : null,Option B — SSO only:
- password, + // no local password (Keycloak manages credentials)Note: ensure
userstable hashashed_passwordand public schemas exclude it.deployment/.env.prod.example (2)
8-11: Align admin env var names with server config (prevent misconfig).server/auth/keycloak/config.ts expects adminRealm/Username/Password. Rename and add missing admin realm.
-# Admin credentials (CHANGE THESE IN PRODUCTION!) -KEYCLOAK_ADMIN=admin -KEYCLOAK_ADMIN_PASSWORD=your-secure-admin-password-here +# Admin credentials (CHANGE THESE IN PRODUCTION!) +KEYCLOAK_ADMIN_REALM=master +KEYCLOAK_ADMIN_USERNAME=admin +KEYCLOAK_ADMIN_PASSWORD=your-secure-admin-password-here
33-39: Use HTTPS and production hostname for Keycloak base URL; avoid localhost in prod template.Make it explicit that a public HTTPS host behind a reverse proxy is required.
-KEYCLOAK_ENABLED=true -KEYCLOAK_BASE_URL=http://localhost:8081 +KEYCLOAK_ENABLED=true +# Use your public HTTPS hostname (behind a reverse proxy). Omit port if 443. +# Example: https://auth.example.com +KEYCLOAK_BASE_URL=https://change-me.example.com KEYCLOAK_DEFAULT_REALM=xyne-shared KEYCLOAK_CLIENT_ID=oa-backend KEYCLOAK_CLIENT_SECRET=your-client-secret-from-setupdeployment/KEYCLOAK_SETUP.md (2)
106-109: Scrub PII from “Test User”.Use neutral placeholders.
-- **Test User**: `debojyoti.mandal@juspay.in` / `*` +- **Example Test User**: `user@example.com` / `changeme`
160-165: Scrub PII in curl example.Use placeholders.
- -d '{"email": "debojyoti.mandal@juspay.in", "password": "*"}' + -d '{"email": "user@example.com", "password": "changeme"}'deployment/setup-keycloak.sh (5)
6-13: Parameterize and fail fast; avoid hard-coded defaults.Read from env with dev-safe fallbacks; echo effective config.
set -e - -KEYCLOAK_URL="http://localhost:8081" -ADMIN_USER="admin" -ADMIN_PASS="admin" -REALM_NAME="xyne-shared" -CLIENT_ID="oa-backend" +: "${KEYCLOAK_URL:=http://localhost:8081}" +: "${ADMIN_USER:=admin}" +: "${ADMIN_PASS:=admin}" +: "${REALM_NAME:=xyne-shared}" +: "${CLIENT_ID:=oa-backend}" +export KEYCLOAK_URL ADMIN_USER ADMIN_PASS REALM_NAME CLIENT_ID +echo "Using KEYCLOAK_URL=${KEYCLOAK_URL} REALM=${REALM_NAME} CLIENT_ID=${CLIENT_ID}"
30-47: Don’t mask HTTP errors; check existence explicitly.Use curl -f and 200 checks to avoid hiding network/misconfig issues.
-echo "🌍 Creating realm: ${REALM_NAME}..." -curl -s -X POST "${KEYCLOAK_URL}/admin/realms" \ +echo "🌍 Ensuring realm exists: ${REALM_NAME}..." +REALM_EXISTS=$(curl -s -o /dev/null -w "%{http_code}" -X GET "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}" -H "Authorization: Bearer ${ADMIN_TOKEN}") +if [ "$REALM_EXISTS" -ne 200 ]; then + curl -s -f -X POST "${KEYCLOAK_URL}/admin/realms" \ -H "Authorization: Bearer ${ADMIN_TOKEN}" \ -H "Content-Type: application/json" \ -d '{ "realm": "'${REALM_NAME}'", "enabled": true, "displayName": "XYNE Shared Realm", "registrationAllowed": true, "loginWithEmailAllowed": true, "duplicateEmailsAllowed": false, "resetPasswordAllowed": true, "editUsernameAllowed": true, "bruteForceProtected": false - }' || echo "Realm might already exist" + }' + echo "✅ Realm created" +else + echo "✅ Realm ${REALM_NAME} already exists" +fi
49-85: Make client confidential and fetch/print secret (doc promises it).Retrieve CLIENT_UUID and CLIENT_SECRET after create/verify.
echo "🔐 Creating client: ${CLIENT_ID}..." -curl -s -X POST "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}/clients" \ +curl -s -X POST "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}/clients" \ -H "Authorization: Bearer ${ADMIN_TOKEN}" \ -H "Content-Type: application/json" \ -d '{ "clientId": "'${CLIENT_ID}'", "enabled": true, "protocol": "openid-connect", "publicClient": false, "directAccessGrantsEnabled": true, "serviceAccountsEnabled": false, "standardFlowEnabled": true, "implicitFlowEnabled": false, "redirectUris": [ "http://localhost:3000/*", "http://localhost:5173/*" ], "webOrigins": [ "http://localhost:3000", "http://localhost:5173" ], "attributes": { - "access.token.lifespan": "300", - "client.secret.creation.time": "'$(date +%s)'" + "access.token.lifespan": "300", + "client.secret.creation.time": "'$(date +%s)'" } - }' || echo "Client might already exist" + }' || echo "Client might already exist" echo "✅ Client created/verified" # Get client UUID for further configuration CLIENT_UUID=$(curl -s -X GET "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}/clients?clientId=${CLIENT_ID}" \ -H "Authorization: Bearer ${ADMIN_TOKEN}" | \ jq -r '.[0].id') echo "📋 Client UUID: ${CLIENT_UUID}" + +# Get and display client secret (confidential clients) +CLIENT_SECRET=$(curl -s -X GET "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}/clients/${CLIENT_UUID}/client-secret" \ + -H "Authorization: Bearer ${ADMIN_TOKEN}" | jq -r '.value') +echo "🔑 Client Secret: ${CLIENT_SECRET}"
86-105: Remove real PII; use example user and stronger dev password/env.Avoid committing personal data; improve defaults.
-echo "👤 Creating test user: debojyoti.mandal@juspay.in..." +echo "👤 Creating test user: ${TEST_USERNAME:=test.user@example.com}..." ... - "username": "debojyoti.mandal@juspay.in", - "email": "debojyoti.mandal@juspay.in", - "firstName": "Debojyoti", - "lastName": "Mandal", + "username": "'${TEST_USERNAME}'", + "email": "'${TEST_USERNAME}'", + "firstName": "Test", + "lastName": "User", ... - "value": "1", + "value": "'${TEST_PASSWORD:=devPassw0rd!}'",
110-121: Password grant requires client_secret for confidential client.Include secret and env-driven test creds.
-TEST_TOKEN=$(curl -s -X POST "${KEYCLOAK_URL}/realms/${REALM_NAME}/protocol/openid-connect/token" \ +TEST_TOKEN=$(curl -s -X POST "${KEYCLOAK_URL}/realms/${REALM_NAME}/protocol/openid-connect/token" \ -H "Content-Type: application/x-www-form-urlencoded" \ - -d "grant_type=password&client_id=${CLIENT_ID}&username=debojyoti.mandal@juspay.in&password=1&scope=openid email profile" | \ + -d "grant_type=password&client_id=${CLIENT_ID}&client_secret=${CLIENT_SECRET}&username=${TEST_USERNAME}&password=${TEST_PASSWORD}&scope=openid email profile" | \ jq -r '.access_token' 2>/dev/null)frontend/src/routes/auth.tsx (6)
122-129: Remove unused body parse to fix TS6133 and a minor perf hit.- await response.json() // Parse response but don't store result logger.info('Email/password authentication successful')
251-255: Replace real-looking email in placeholder.- placeholder="debojyoti.mandal@juspay.in" + placeholder="user@example.com"
41-48: Use loader data, not route context, to surface beforeLoad errors.useRouteContext() won’t carry loader return values.
- const route = Route.useRouteContext() - - useEffect(() => { - if (route?.error) { - setError(route.error) - } - }, [route?.error]) + const data = Route.useLoaderData() + useEffect(() => { + if (data?.error) setError(data.error) + }, [data?.error])
70-74: Fix Google callback path (missing /api).Prevents 404s.
- const redirectUrl = `${import.meta.env.VITE_API_BASE_URL}/v1/auth/callback` + const redirectUrl = `${import.meta.env.VITE_API_BASE_URL}/api/v1/auth/callback`
82-99: Don’t assemble OAuth URL client-side; use server-provided loginUrl.Avoids CSRF/state/PKCE mistakes and keeps redirect_uri canonical.
- // Build Keycloak authorization URL - make sure it goes to user realm, not admin - const params = new URLSearchParams({ - client_id: keycloakConfig.clientId, - redirect_uri: `${window.location.origin}/auth`, - response_type: "code", - scope: "openid email profile", - state: Math.random().toString(36).substring(2, 15), - }) - // Use the user realm URL, not admin realm - const userLoginUrl = `${keycloakConfig.baseUrl}/realms/${keycloakConfig.realm}/protocol/openid-connect/auth` - const authUrl = `${userLoginUrl}?${params.toString()}` - - logger.info("Redirecting to Keycloak:", authUrl) - window.location.href = authUrl + logger.info("Redirecting to Keycloak SSO") + window.location.href = keycloakConfig.loginUrl
294-305: Bug: res.json() called twice.Read once and reuse to avoid runtime error.
if (res.ok) { - const userWorkspace = await res.json() + const userWorkspace = await res.json() // If User & Workspace exists, don't let user visit /auth if (userWorkspace?.user && userWorkspace?.workspace) { throw redirect({ to: "/" }) } - return await res.json() + return userWorkspace }deployment/docker-compose.prod.yml (4)
117-120: DB URL points to non-existent DB.Either create “keycloak” DB or point to existing “xyne”.
- - KC_DB_URL=jdbc:postgresql://xyne-db:5432/keycloak + - KC_DB_URL=jdbc:postgresql://xyne-db:5432/xyne # Or provision a 'keycloak' DB/user via init scripts and keep original URL.
108-114: Pin image and remove weak default admin creds.Force explicit values via .env; avoid “latest”.
- image: quay.io/keycloak/keycloak:latest + image: quay.io/keycloak/keycloak:25.0.6 ... - - KEYCLOAK_ADMIN=${KEYCLOAK_ADMIN:-admin} - - KEYCLOAK_ADMIN_PASSWORD=${KEYCLOAK_ADMIN_PASSWORD:-admin} + - KEYCLOAK_ADMIN=${KEYCLOAK_ADMIN:?set in .env} + - KEYCLOAK_ADMIN_PASSWORD=${KEYCLOAK_ADMIN_PASSWORD:?set in .env}
121-126: Harden hostname/TLS settings; disable plain HTTP.These should be secure in prod (behind reverse proxy).
- - KC_HOSTNAME_STRICT=false - - KC_HOSTNAME_STRICT_HTTPS=false - - KC_HTTP_ENABLED=true + - KC_HOSTNAME_STRICT=true + - KC_HOSTNAME_STRICT_HTTPS=true + - KC_HTTP_ENABLED=false - KC_PROXY=edge
133-135: Cache stack “kubernetes” is incorrect for Compose.Use local (single node) or tcp.
- - KC_CACHE=ispn - - KC_CACHE_STACK=kubernetes + - KC_CACHE=ispn + - KC_CACHE_STACK=local
🧹 Nitpick comments (13)
server/db/user.ts (4)
20-24: Rename parameter for accuracy: it's a workspaceExternalId.You compare against
users.workspaceExternalIdbut the param is namedworkspaceId. Rename for clarity and avoid future bugs.-export const getPublicUserAndWorkspaceByEmail = async ( - trx: TxnOrClient, - workspaceId: string, +export const getPublicUserAndWorkspaceByEmail = async ( + trx: TxnOrClient, + workspaceExternalId: string, email: string, ): Promise<PublicUserWorkspace> => { ... - eq(users.workspaceExternalId, workspaceId), // Filter by workspaceId + eq(users.workspaceExternalId, workspaceExternalId), // Filter by workspace external idRepeat similarly for
getUserAndWorkspaceByEmail.Also applies to: 33-36
90-94: Remove redundant and() wrapper.
and()with a single predicate is noise.- .where( - and( - eq(users.email, email), // Filter by user email - ), - ) + .where(eq(users.email, email)) // Filter by user email
103-106: Same nit: flatten where().- .where(and(eq(users.email, email))) + .where(eq(users.email, email))
1-19: Keycloak integration alignment: prefer provider-scoped identity fields.To avoid mixed auth states, add
authProvider(enum) andproviderUserId(e.g., Keycloaksub) and unique index(authProvider, providerUserId); makeI can propose a schema + migration that keeps local accounts separate from Keycloak-linked ones.
deployment/.env.prod.example (3)
22-27: Order and naming nits for dotenv consistency.Keeps templates tidy and passes dotenv-linter.
-POSTGRES_USER=xyne -POSTGRES_PASSWORD=your-secure-db-password-here +POSTGRES_PASSWORD=your-secure-db-password-here +POSTGRES_USER=xyne
40-44: Reorder secrets to satisfy linter; consider noting minimum length/rotation.Minor template polish.
-USER_SECRET=your-jwt-secret-here -ACCESS_TOKEN_SECRET=your-access-token-secret-here -REFRESH_TOKEN_SECRET=your-refresh-token-secret-here +ACCESS_TOKEN_SECRET=your-access-token-secret-here +REFRESH_TOKEN_SECRET=your-refresh-token-secret-here +USER_SECRET=your-jwt-secret-here
94-94: Add trailing newline.Fixes EndingBlankLine warning.
deployment/KEYCLOAK_SETUP.md (3)
100-105: Avoid bare URL; linkify admin console.Small docs polish.
-- **Admin Console**: http://localhost:8081/admin/ +- **Admin Console**: <http://localhost:8081/admin/>
9-24: Specify language for fenced diagram block.Satisfies markdownlint MD040.
-``` +```text ... -``` +```
264-268: Typo: Infinispan.Fix spelling in production cache notes.
-- **Production cache**: Infinspan with Kubernetes clustering support +- **Production cache**: Infinispan with Kubernetes clustering supportdeployment/setup-keycloak.sh (1)
125-136: Update summary to remove PII and show secret (dev only).Keep output consistent with script changes.
-echo " Test User: debojyoti.mandal@juspay.in / password: 1" +echo " Test User: ${TEST_USERNAME} / password: ${TEST_PASSWORD}" +echo " Client Secret: ${CLIENT_SECRET}"deployment/docker-compose.prod.yml (2)
135-137: Operational advice: Don’t expose Keycloak directly in prod.Prefer reverse proxy/TLS termination and internal network-only binding.
114-131: Trailing spaces in YAML keys/values.Clean up to satisfy linters.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (15)
deployment/.env.prod.example(1 hunks)deployment/KEYCLOAK_SETUP.md(1 hunks)deployment/docker-compose.dev.yml(1 hunks)deployment/docker-compose.prod.yml(1 hunks)deployment/setup-keycloak.sh(1 hunks)frontend/package.json(3 hunks)frontend/src/routes/auth.tsx(3 hunks)server/api/auth.ts(2 hunks)server/auth/keycloak/api.ts(1 hunks)server/auth/keycloak/config.ts(1 hunks)server/auth/keycloak/middleware.ts(1 hunks)server/auth/keycloak/routes.ts(1 hunks)server/db/schema/users.ts(3 hunks)server/db/user.ts(2 hunks)server/server.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- server/auth/keycloak/routes.ts
- frontend/package.json
- server/db/schema/users.ts
- server/auth/keycloak/config.ts
- server/api/auth.ts
- server/server.ts
- deployment/docker-compose.dev.yml
- server/auth/keycloak/middleware.ts
- server/auth/keycloak/api.ts
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/routes/auth.tsx (3)
server/auth/keycloak/config.ts (1)
KeycloakConfig(1-10)frontend/src/routes/_authenticated.tsx (1)
Route(5-18)frontend/src/api.ts (1)
api(4-4)
🪛 LanguageTool
deployment/KEYCLOAK_SETUP.md
[grammar] ~28-~28: There might be a mistake here.
Context: ...ice | Container Name | Ports | Purpose | |---------|---------------|-------|-----...
(QB_NEW_EN)
[grammar] ~29-~29: There might be a mistake here.
Context: ...-----|---------------|-------|---------| | PostgreSQL | xyne-db | 5432 | Database...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ... | Database for both XYNE and Keycloak | | Keycloak | xyne-keycloak | 8081→8080 |...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...ak | 8081→8080 | Authentication server | | Vespa | vespa | 8080, 19071 | Search e...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ... | vespa | 8080, 19071 | Search engine | | Prometheus | xyne-prometheus | 9090 | ...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...prometheus | 9090 | Metrics collection | | Grafana | xyne-grafana | 3002→3000 | M...
(QB_NEW_EN)
[grammar] ~34-~34: There might be a mistake here.
Context: ...na | 3002→3000 | Monitoring dashboards | | Loki | loki | 3100 | Log aggregation |...
(QB_NEW_EN)
[grammar] ~92-~92: There might be a mistake here.
Context: ...es | Variable | Default | Description | |----------|---------|-------------| | `...
(QB_NEW_EN)
[grammar] ~93-~93: There might be a mistake here.
Context: ...n | |----------|---------|-------------| | KEYCLOAK_ENABLED | false | Enable ...
(QB_NEW_EN)
[grammar] ~94-~94: There might be a mistake here.
Context: ...alse| Enable Keycloak authentication | |KEYCLOAK_BASE_URL|http://localhos...
(QB_NEW_EN)
[grammar] ~95-~95: There might be a mistake here.
Context: .../localhost:8081| Keycloak server URL | |KEYCLOAK_DEFAULT_REALM|xyne-share...
(QB_NEW_EN)
[grammar] ~96-~96: There might be a mistake here.
Context: ...|xyne-shared| Default realm name | |KEYCLOAK_CLIENT_ID|oa-backend` | ...
(QB_NEW_EN)
[grammar] ~97-~97: There might be a mistake here.
Context: ...D|oa-backend| Client ID for XYNE | |KEYCLOAK_CLIENT_SECRET` | *(required)...
(QB_NEW_EN)
[grammar] ~102-~102: There might be a mistake here.
Context: ... Console**: http://localhost:8081/admin/ - Username: admin - Password: admin ...
(QB_NEW_EN)
[grammar] ~103-~103: There might be a mistake here.
Context: ...p://localhost:8081/admin/ - Username: admin - Password: admin - Realm: `xyne-sh...
(QB_NEW_EN)
[grammar] ~106-~106: There might be a mistake here.
Context: ...in - Password:admin - **Realm**:xyne-shared- **Client**:oa-backend` - Test User: ...
(QB_NEW_EN)
[grammar] ~107-~107: There might be a mistake here.
Context: ... Realm: xyne-shared - Client: oa-backend - Test User: `debojyoti.mandal@juspay.in...
(QB_NEW_EN)
[grammar] ~114-~114: There might be a mistake here.
Context: ...methods: 1. Google OAuth (existing) 2. Keycloak SSO (browser redirect) 3. **E...
(QB_NEW_EN)
[grammar] ~115-~115: There might be a mistake here.
Context: ...) 2. Keycloak SSO (browser redirect) 3. Email/Password (direct login via Keycl...
(QB_NEW_EN)
[grammar] ~136-~136: There might be a mistake here.
Context: ...All data is persisted in Docker volumes: - PostgreSQL data: ../server/xyne-data -...
(QB_NEW_EN)
[grammar] ~251-~251: There might be a mistake here.
Context: ... | Feature | Development | Production | |---------|-------------|------------| |...
(QB_NEW_EN)
[grammar] ~252-~252: There might be a mistake here.
Context: ...| |---------|-------------|------------| | Keycloak Mode | start-dev (hot r...
(QB_NEW_EN)
[grammar] ~253-~253: There might be a mistake here.
Context: ...| start --optimized (faster startup) | | Container Name | xyne-keycloak |...
(QB_NEW_EN)
[grammar] ~254-~254: There might be a mistake here.
Context: ...xyne-keycloak | xyne-keycloak-prod | | Resource Limits | None | 1GB RAM, ...
(QB_NEW_EN)
[grammar] ~255-~255: There might be a mistake here.
Context: ...rce Limits** | None | 1GB RAM, 0.5 CPU | | Cache Strategy | Simple | Kubernet...
(QB_NEW_EN)
[grammar] ~256-~256: There might be a mistake here.
Context: ...| Simple | Kubernetes clustering ready | | Health Checks | Basic | Extended w...
(QB_NEW_EN)
[grammar] ~257-~257: There might be a mistake here.
Context: ...ic | Extended with longer startup time | | Logging | DEBUG friendly | INFO/WA...
(QB_NEW_EN)
[grammar] ~264-~264: There might be a mistake here.
Context: ...: Faster startup and runtime performance - Resource limits: Memory and CPU constr...
(QB_NEW_EN)
[grammar] ~265-~265: There might be a mistake here.
Context: ... constraints for container orchestration - Extended health checks: 120s startup p...
(QB_NEW_EN)
[grammar] ~266-~266: There might be a mistake here.
Context: ...0s startup period for optimization phase - Production cache: Infinspan with Kuber...
(QB_NEW_EN)
[grammar] ~267-~267: Ensure spelling is correct
Context: ...imization phase - Production cache: Infinspan with Kubernetes clustering support - **...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~267-~267: There might be a mistake here.
Context: ...nspan with Kubernetes clustering support - Environment-based configuration: All s...
(QB_NEW_EN)
[grammar] ~274-~274: There might be a mistake here.
Context: ...production deployment: 1. Security: - Change default admin credentials via env...
(QB_NEW_EN)
[grammar] ~280-~280: There might be a mistake here.
Context: ...re proper CORS origins 2. Database: - Use separate PostgreSQL instance for Key...
(QB_NEW_EN)
[grammar] ~285-~285: There might be a mistake here.
Context: ...se clustering if needed 3. Scaling: - Configure Keycloak clustering with share...
(QB_NEW_EN)
[grammar] ~286-~286: There might be a mistake here.
Context: ...g**: - Configure Keycloak clustering with shared database - Use external load ...
(QB_NEW_EN)
[grammar] ~290-~290: There might be a mistake here.
Context: ... limits and requests 4. Monitoring: - Enable Keycloak metrics integration with...
(QB_NEW_EN)
[grammar] ~303-~303: There might be a mistake here.
Context: ...ence** - PostgreSQL backend for Keycloak - Survives container restarts - Easy backu...
(QB_NEW_EN)
[grammar] ~304-~304: There might be a mistake here.
Context: ...r Keycloak - Survives container restarts - Easy backup/restore ✅ **Team Collaborat...
(QB_NEW_EN)
[grammar] ~308-~308: There might be a mistake here.
Context: ...ion** - Version-controlled configuration - Consistent realm/client setup - Shared d...
(QB_NEW_EN)
[grammar] ~309-~309: There might be a mistake here.
Context: ...guration - Consistent realm/client setup - Shared development environment ✅ **Prod...
(QB_NEW_EN)
[grammar] ~313-~313: There might be a mistake here.
Context: ...y** - Similar architecture to production - Proper database integration - Service di...
(QB_NEW_EN)
[grammar] ~314-~314: There might be a mistake here.
Context: ...production - Proper database integration - Service discovery patterns
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
deployment/KEYCLOAK_SETUP.md
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
102-102: Bare URL used
(MD034, no-bare-urls)
🪛 Shellcheck (0.10.0)
deployment/setup-keycloak.sh
[warning] 73-73: Quote this to prevent word splitting.
(SC2046)
🪛 dotenv-linter (3.3.0)
deployment/.env.prod.example
[warning] 24-24: [UnorderedKey] The POSTGRES_PASSWORD key should go before the POSTGRES_USER key
(UnorderedKey)
[warning] 35-35: [UnorderedKey] The KEYCLOAK_BASE_URL key should go before the KEYCLOAK_ENABLED key
(UnorderedKey)
[warning] 36-36: [UnorderedKey] The KEYCLOAK_DEFAULT_REALM key should go before the KEYCLOAK_ENABLED key
(UnorderedKey)
[warning] 37-37: [UnorderedKey] The KEYCLOAK_CLIENT_ID key should go before the KEYCLOAK_DEFAULT_REALM key
(UnorderedKey)
[warning] 38-38: [UnorderedKey] The KEYCLOAK_CLIENT_SECRET key should go before the KEYCLOAK_DEFAULT_REALM key
(UnorderedKey)
[warning] 42-42: [UnorderedKey] The ACCESS_TOKEN_SECRET key should go before the USER_SECRET key
(UnorderedKey)
[warning] 43-43: [UnorderedKey] The REFRESH_TOKEN_SECRET key should go before the USER_SECRET key
(UnorderedKey)
[warning] 52-52: [UnorderedKey] The GRAFANA_ENABLED key should go before the METRICS_ENABLED key
(UnorderedKey)
[warning] 94-94: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🪛 YAMLlint (1.37.1)
deployment/docker-compose.prod.yml
[error] 114-114: trailing spaces
(trailing-spaces)
[error] 120-120: trailing spaces
(trailing-spaces)
[error] 126-126: trailing spaces
(trailing-spaces)
[error] 131-131: trailing spaces
(trailing-spaces)
🔇 Additional comments (1)
server/db/user.ts (1)
119-137: No password exposure in API responses
All public user schemas omit thepasswordfield and no routes return raw user records—including no use ofselectUserSchemain HTTP handlers—so no secrets are leaked.
| ### Production Environment | ||
|
|
||
| #### 1. Set Environment Variables | ||
| ```bash | ||
| # Required for production | ||
| export KEYCLOAK_ADMIN=your-admin-username | ||
| export KEYCLOAK_ADMIN_PASSWORD=your-secure-password | ||
| export POSTGRES_USER=xyne | ||
| export POSTGRES_PASSWORD=your-db-password | ||
| export KEYCLOAK_PORT=8081 # Optional, defaults to 8081 | ||
| export KC_LOG_LEVEL=WARN # Optional, defaults to INFO | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Match env var names to backend config (avoid setup breakage).
Docs should use KEYCLOAK_ADMIN_USERNAME and KEYCLOAK_ADMIN_REALM.
- export KEYCLOAK_ADMIN=your-admin-username
- export KEYCLOAK_ADMIN_PASSWORD=your-secure-password
+ export KEYCLOAK_ADMIN_USERNAME=your-admin-username
+ export KEYCLOAK_ADMIN_PASSWORD=your-secure-password
+ export KEYCLOAK_ADMIN_REALM=master📝 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.
| ### Production Environment | |
| #### 1. Set Environment Variables | |
| ```bash | |
| # Required for production | |
| export KEYCLOAK_ADMIN=your-admin-username | |
| export KEYCLOAK_ADMIN_PASSWORD=your-secure-password | |
| export POSTGRES_USER=xyne | |
| export POSTGRES_PASSWORD=your-db-password | |
| export KEYCLOAK_PORT=8081 # Optional, defaults to 8081 | |
| export KC_LOG_LEVEL=WARN # Optional, defaults to INFO | |
| ``` | |
| ### Production Environment | |
| #### 1. Set Environment Variables | |
| # Required for production | |
| export KEYCLOAK_ADMIN_USERNAME=your-admin-username | |
| export KEYCLOAK_ADMIN_PASSWORD=your-secure-password | |
| export KEYCLOAK_ADMIN_REALM=master | |
| export POSTGRES_USER=xyne | |
| export POSTGRES_PASSWORD=your-db-password | |
| export KEYCLOAK_PORT=8081 # Optional, defaults to 8081 | |
| export KC_LOG_LEVEL=WARN # Optional, defaults to INFO |
🤖 Prompt for AI Agents
deployment/KEYCLOAK_SETUP.md lines 63-74: the documented environment variable
names do not match backend expectations and will break setup; update the docs to
use KEYCLOAK_ADMIN_USERNAME instead of KEYCLOAK_ADMIN and add
KEYCLOAK_ADMIN_REALM (with an example value) in the checklist, keep the password
variable as KEYCLOAK_ADMIN_PASSWORD, and ensure any mentions of these vars
elsewhere in the docs or compose files are updated to the new names so they map
exactly to the backend configuration.
0bb92f8 to
f1591c5
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (18)
deployment/.env.prod.example (2)
8-11: Align admin env var names with backend; add admin realm.Use KEYCLOAK_ADMIN_USERNAME and KEYCLOAK_ADMIN_REALM to match server config. This prevents broken setup.
-# Admin credentials (CHANGE THESE IN PRODUCTION!) -KEYCLOAK_ADMIN=admin -KEYCLOAK_ADMIN_PASSWORD=your-secure-admin-password-here +# Admin credentials (CHANGE THESE IN PRODUCTION!) +KEYCLOAK_ADMIN_REALM=master +KEYCLOAK_ADMIN_USERNAME=admin +KEYCLOAK_ADMIN_PASSWORD=your-secure-admin-password-here
33-38: Use HTTPS and real hostname in prod template; avoid localhost.Prod templates should not default to http://localhost.
-KEYCLOAK_ENABLED=true -KEYCLOAK_BASE_URL=http://localhost:8081 +KEYCLOAK_ENABLED=true +# Use your public HTTPS hostname (behind a reverse proxy). Omit port if 443. +KEYCLOAK_BASE_URL=https://change-me.example.com KEYCLOAK_DEFAULT_REALM=xyne-shared KEYCLOAK_CLIENT_ID=oa-backend KEYCLOAK_CLIENT_SECRET=your-client-secret-from-setupdeployment/KEYCLOAK_SETUP.md (3)
65-74: Fix admin env var names in production snippet; add admin realm.Matches backend expectations and previous guidance.
-# Required for production -export KEYCLOAK_ADMIN=your-admin-username -export KEYCLOAK_ADMIN_PASSWORD=your-secure-password +# Required for production +export KEYCLOAK_ADMIN_USERNAME=your-admin-username +export KEYCLOAK_ADMIN_PASSWORD=your-secure-password +export KEYCLOAK_ADMIN_REALM=master export POSTGRES_USER=xyne export POSTGRES_PASSWORD=your-db-password export KEYCLOAK_PORT=8081 # Optional, defaults to 8081 export KC_LOG_LEVEL=WARN # Optional, defaults to INFO
108-109: Remove real-looking PII in docs.Replace with neutral placeholders.
-- **Test User**: `debojyoti.mandal@juspay.in` / `*` +- **Example Test User**: `user@example.com` / `changeme`
161-165: Scrub PII from curl example.Use placeholders.
- -d '{"email": "debojyoti.mandal@juspay.in", "password": "*"}' + -d '{"email": "user@example.com", "password": "changeme"}'server/api/auth.ts (2)
27-31: Don’t hardcode agentWhiteList. Use configured value.Prevents misleading responses and respects config.
const response: any = { ...userAndWorkspace, - agentWhiteList: true, + agentWhiteList, authMethod, }
33-72: Stop re-parsing JWT; use validated claims from middleware.Current code decodes the token again (base64, no signature verification) and duplicates middleware logic. Rely on c.get("authContext") instead.
- // Add Keycloak-specific information if authenticated via Keycloak - if (authMethod === "keycloak" && authContext) { - try { - // Get the original Keycloak token to extract additional fields (same logic as enhanced middleware) - const authHeader = c.req.header("Authorization") - const cookieToken = getCookie(c, "auth-token") || getCookie(c, "access-token") - - let token = null - if (authHeader?.startsWith("Bearer ")) { - token = authHeader.substring(7) - } else if (cookieToken) { - token = cookieToken - } - - if (token) { - // Decode JWT payload to extract Keycloak fields - const parts = token.split(".") - if (parts.length === 3) { - const payload = JSON.parse(Buffer.from(parts[1], "base64").toString()) - - // Add Keycloak-specific fields to user object - if (response.user) { - response.user = { - ...response.user, - email_verified: payload.email_verified || false, - preferred_username: payload.preferred_username || payload.email, - given_name: payload.given_name || "", - family_name: payload.family_name || "", - realm_access: payload.realm_access || { roles: [] }, - resource_access: payload.resource_access || {}, - scope: payload.scope || "", - } - } - } - } - } catch (error) { - Logger.error("Error extracting Keycloak token information:", error) - // Continue without Keycloak fields if extraction fails - } - } + // Add Keycloak-specific information if authenticated via Keycloak + if (authMethod === "keycloak" && authContext && response.user) { + const kc: any = authContext + response.user = { + ...response.user, + email_verified: Boolean(kc.email_verified), + preferred_username: kc.preferred_username || kc.email, + given_name: kc.given_name || "", + family_name: kc.family_name || "", + realm_access: kc.realm_access || { roles: [] }, + resource_access: kc.resource_access || {}, + scope: kc.scope || "", + } + }If you decide to keep token parsing for fallback, at minimum use base64url-safe decoding and verify signature against Keycloak JWKS.
deployment/setup-keycloak.sh (6)
8-13: Parameterize and fail fast on env; avoid hard-coded creds/URL.Read from env with safe local defaults; print context; optionally fail in CI if unset.
Apply:
-KEYCLOAK_URL="http://localhost:8081" -ADMIN_USER="admin" -ADMIN_PASS="admin" -REALM_NAME="xyne-shared" -CLIENT_ID="oa-backend" +: "${KEYCLOAK_URL:=http://localhost:8081}" +: "${ADMIN_USER:=admin}" +: "${ADMIN_PASS:=admin}" +: "${REALM_NAME:=xyne-shared}" +: "${CLIENT_ID:=oa-backend}" +export KEYCLOAK_URL ADMIN_USER ADMIN_PASS REALM_NAME CLIENT_ID +echo "Using KEYCLOAK_URL=${KEYCLOAK_URL} REALM=${REALM_NAME} CLIENT_ID=${CLIENT_ID}" +# Optional CI guard +[ -n "${CI:-}" ] && { : "${KEYCLOAK_URL:?}"; : "${ADMIN_USER:?}"; : "${ADMIN_PASS:?}"; : "${REALM_NAME:?}"; : "${CLIENT_ID:?}"; }
30-46: Realm creation hides real errors; make idempotent and fail on HTTP errors.Use curl -f and existence check to avoid masking network/4xx/5xx issues.
Apply:
-echo "🌍 Creating realm: ${REALM_NAME}..." -curl -s -X POST "${KEYCLOAK_URL}/admin/realms" \ +echo "🌍 Ensuring realm: ${REALM_NAME}..." +REALM_CODE=$(curl -s -o /dev/null -w "%{http_code}" -X GET \ + "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}" -H "Authorization: Bearer ${ADMIN_TOKEN}") +if [ "$REALM_CODE" -ne 200 ]; then + curl -s -f -X POST "${KEYCLOAK_URL}/admin/realms" \ -H "Authorization: Bearer ${ADMIN_TOKEN}" \ -H "Content-Type: application/json" \ -d '{ "realm": "'${REALM_NAME}'", "enabled": true, "displayName": "XYNE Shared Realm", "registrationAllowed": true, "loginWithEmailAllowed": true, "duplicateEmailsAllowed": false, "resetPasswordAllowed": true, "editUsernameAllowed": true, "bruteForceProtected": false - }' || echo "Realm might already exist" + }' +else + echo "✅ Realm ${REALM_NAME} already exists" +fi
49-76: Confidential client without client_secret usage; align type vs. token flow.Either switch to publicClient=true for dev, or keep confidential and fetch/use secret.
Option A (public dev client):
- "publicClient": false, + "publicClient": true, "directAccessGrantsEnabled": true,Option B (confidential): keep publicClient=false and follow the secret retrieval + token changes in comments below (lines 79-85 and 110-115).
79-85: Guard for missing client UUID; exit on failure. Also fetch and display secret.Null/empty UUID will break subsequent steps; retrieving secret unblocks confidential flow and matches docs.
Apply:
CLIENT_UUID=$(curl -s -X GET "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}/clients?clientId=${CLIENT_ID}" \ -H "Authorization: Bearer ${ADMIN_TOKEN}" | \ jq -r '.[0].id') echo "📋 Client UUID: ${CLIENT_UUID}" +if [ -z "${CLIENT_UUID}" ] || [ "${CLIENT_UUID}" = "null" ]; then + echo "❌ Failed to resolve client UUID for ${CLIENT_ID}" + exit 1 +fi + +# If confidential, retrieve and show client secret +CLIENT_SECRET=$(curl -s -X GET "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}/clients/${CLIENT_UUID}/client-secret" \ + -H "Authorization: Bearer ${ADMIN_TOKEN}" | jq -r '.value' 2>/dev/null || true) +[ -n "${CLIENT_SECRET}" ] && echo "🔑 Client Secret: ${CLIENT_SECRET}"
86-105: Remove real PII and weak password; parameterize test user creds.Use example domain and env-driven password.
Apply:
-echo "👤 Creating test user: debojyoti.mandal@juspay.in..." +TEST_USERNAME=${TEST_USERNAME:-test.user@example.com} +TEST_PASSWORD=${TEST_PASSWORD:-devPassw0rd!} +echo "👤 Creating test user: ${TEST_USERNAME}..." ... - -d '{ - "username": "debojyoti.mandal@juspay.in", - "email": "debojyoti.mandal@juspay.in", - "firstName": "Debojyoti", - "lastName": "Mandal", + -d '{ + "username": "'${TEST_USERNAME}'", + "email": "'${TEST_USERNAME}'", + "firstName": "Test", + "lastName": "User", "enabled": true, "emailVerified": false, "credentials": [{ "type": "password", - "value": "1", + "value": "'${TEST_PASSWORD}'", "temporary": false }] }' || echo "User might already exist"
110-120: Password grant test missing client_secret for confidential client; parameterize test creds.Include client_secret when confidential, otherwise make client public.
Apply:
-TEST_TOKEN=$(curl -s -X POST "${KEYCLOAK_URL}/realms/${REALM_NAME}/protocol/openid-connect/token" \ +TEST_TOKEN=$(curl -s -X POST "${KEYCLOAK_URL}/realms/${REALM_NAME}/protocol/openid-connect/token" \ -H "Content-Type: application/x-www-form-urlencoded" \ - -d "grant_type=password&client_id=${CLIENT_ID}&username=debojyoti.mandal@juspay.in&password=1&scope=openid email profile" | \ + -d "grant_type=password&client_id=${CLIENT_ID}&client_secret=${CLIENT_SECRET:-}&username=${TEST_USERNAME}&password=${TEST_PASSWORD}&scope=openid email profile" | \ jq -r '.access_token' 2>/dev/null) if [ "$TEST_TOKEN" == "null" ] || [ -z "$TEST_TOKEN" ]; then - echo "⚠️ Password grant test failed - this might be expected if client secret is required" + echo "⚠️ Password grant test failed - verify client type and CLIENT_SECRET" else echo "✅ Password grant test successful" fideployment/docker-compose.prod.yml (5)
112-113: Remove default admin creds in prod.Force explicit values via environment; do not fall back to admin/admin.
Apply:
- - KEYCLOAK_ADMIN=${KEYCLOAK_ADMIN:-admin} - - KEYCLOAK_ADMIN_PASSWORD=${KEYCLOAK_ADMIN_PASSWORD:-admin} + - KEYCLOAK_ADMIN=${KEYCLOAK_ADMIN} + - KEYCLOAK_ADMIN_PASSWORD=${KEYCLOAK_ADMIN_PASSWORD}
116-120: DB URL points to non-existent database; align with provisioned DB or provision separate one.xyne-db initializes DB "xyne". Either switch KC_DB_URL to /xyne or create "keycloak" at init.
Apply:
- - KC_DB_URL=jdbc:postgresql://xyne-db:5432/keycloak - - KC_DB_USERNAME=${POSTGRES_USER:-xyne} - - KC_DB_PASSWORD=${POSTGRES_PASSWORD:-xyne} + - KC_DB_URL=jdbc:postgresql://xyne-db:5432/xyne + - KC_DB_USERNAME=${KC_DB_USERNAME} + - KC_DB_PASSWORD=${KC_DB_PASSWORD}Also document KC_DB_USERNAME/KC_DB_PASSWORD in .env.prod and provision least-privileged creds for Keycloak.
121-126: Harden hostname/HTTPS; disable plain HTTP in prod.Set strict host checks and HTTPS-only; rely on reverse proxy/TLS.
Apply:
- - KC_HOSTNAME_STRICT=false - - KC_HOSTNAME_STRICT_HTTPS=false - - KC_HTTP_ENABLED=true + - KC_HOSTNAME_STRICT=true + - KC_HOSTNAME_STRICT_HTTPS=true + - KC_HTTP_ENABLED=false - KC_PROXY=edge
133-134: Cache stack incompatible with Docker Compose.Use local (single-node) instead of kubernetes.
Apply:
- - KC_CACHE=ispn - - KC_CACHE_STACK=kubernetes + - KC_CACHE=ispn + - KC_CACHE_STACK=local
108-108: Pin Keycloak image; avoid floating latest.Use a vetted, explicit version.
Apply:
- image: quay.io/keycloak/keycloak:latest + image: quay.io/keycloak/keycloak:25.0.6
🧹 Nitpick comments (7)
deployment/.env.prod.example (1)
22-25: Satisfy dotenv-linter ordering (optional).Purely cosmetic, but keeps linters quiet.
-POSTGRES_USER=xyne -POSTGRES_PASSWORD=your-secure-db-password-here +POSTGRES_PASSWORD=your-secure-db-password-here +POSTGRES_USER=xynedeployment/KEYCLOAK_SETUP.md (3)
9-24: Specify code-fence language for the ASCII diagram.Silences MD040 and renders better.
-``` +```text ┌─────────────────┐ ┌─────────────────┐ ┌─────────────────┐ │ XYNE Server │ │ Keycloak │ │ PostgreSQL │ │ Port: 3000 │◄──►│ Port: 8081 │◄──►│ Port: 5432 │ └─────────────────┘ └─────────────────┘ └─────────────────┘ ...--- `102-105`: **Avoid bare URL; minor markdownlint fix.** Wrap the admin URL in angle brackets. ```diff -- **Admin Console**: http://localhost:8081/admin/ +- **Admin Console**: <http://localhost:8081/admin/>
263-268: Typo: Infinispan.Fix spelling in production cache bullet.
-- **Production cache**: Infinspan with Kubernetes clustering support +- **Production cache**: Infinispan with Kubernetes clustering supportserver/api/auth.ts (1)
91-97: Unify payload retrieval key.Use the same key middleware uses elsewhere (JwtPayloadKey) for consistency.
- const payload = c.get("jwtPayload") + const payload = c.get(JwtPayloadKey)deployment/setup-keycloak.sh (1)
125-133: Avoid printing real passwords; reflect new test user vars in summary.Keep dev-friendly but non-sensitive output.
Apply:
-echo " Test User: debojyoti.mandal@juspay.in / password: 1" +echo " Test User: ${TEST_USERNAME} / password: ${TEST_PASSWORD}" +[ -n "${CLIENT_SECRET:-}" ] && echo " Client Secret: ${CLIENT_SECRET}"deployment/docker-compose.prod.yml (1)
114-114: Remove trailing spaces flagged by linter.Clean up formatting to satisfy YAMLlint.
Apply:
- +(at the indicated lines)
Also applies to: 120-120, 126-126, 131-131
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (15)
deployment/.env.prod.example(1 hunks)deployment/KEYCLOAK_SETUP.md(1 hunks)deployment/docker-compose.dev.yml(1 hunks)deployment/docker-compose.prod.yml(1 hunks)deployment/setup-keycloak.sh(1 hunks)frontend/package.json(3 hunks)frontend/src/routes/auth.tsx(3 hunks)server/api/auth.ts(2 hunks)server/auth/keycloak/api.ts(1 hunks)server/auth/keycloak/config.ts(1 hunks)server/auth/keycloak/middleware.ts(1 hunks)server/auth/keycloak/routes.ts(1 hunks)server/db/schema/users.ts(3 hunks)server/db/user.ts(2 hunks)server/server.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- server/server.ts
- server/db/schema/users.ts
- server/auth/keycloak/config.ts
- server/auth/keycloak/routes.ts
- frontend/package.json
- deployment/docker-compose.dev.yml
- server/db/user.ts
- frontend/src/routes/auth.tsx
- server/auth/keycloak/api.ts
- server/auth/keycloak/middleware.ts
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
deployment/.env.prod.example
[warning] 24-24: [UnorderedKey] The POSTGRES_PASSWORD key should go before the POSTGRES_USER key
(UnorderedKey)
[warning] 35-35: [UnorderedKey] The KEYCLOAK_BASE_URL key should go before the KEYCLOAK_ENABLED key
(UnorderedKey)
[warning] 36-36: [UnorderedKey] The KEYCLOAK_DEFAULT_REALM key should go before the KEYCLOAK_ENABLED key
(UnorderedKey)
[warning] 37-37: [UnorderedKey] The KEYCLOAK_CLIENT_ID key should go before the KEYCLOAK_DEFAULT_REALM key
(UnorderedKey)
[warning] 38-38: [UnorderedKey] The KEYCLOAK_CLIENT_SECRET key should go before the KEYCLOAK_DEFAULT_REALM key
(UnorderedKey)
[warning] 42-42: [UnorderedKey] The ACCESS_TOKEN_SECRET key should go before the USER_SECRET key
(UnorderedKey)
[warning] 43-43: [UnorderedKey] The REFRESH_TOKEN_SECRET key should go before the USER_SECRET key
(UnorderedKey)
[warning] 52-52: [UnorderedKey] The GRAFANA_ENABLED key should go before the METRICS_ENABLED key
(UnorderedKey)
[warning] 94-94: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🪛 LanguageTool
deployment/KEYCLOAK_SETUP.md
[grammar] ~28-~28: There might be a mistake here.
Context: ...ice | Container Name | Ports | Purpose | |---------|---------------|-------|-----...
(QB_NEW_EN)
[grammar] ~29-~29: There might be a mistake here.
Context: ...-----|---------------|-------|---------| | PostgreSQL | xyne-db | 5432 | Database...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ... | Database for both XYNE and Keycloak | | Keycloak | xyne-keycloak | 8081→8080 |...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...ak | 8081→8080 | Authentication server | | Vespa | vespa | 8080, 19071 | Search e...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ... | vespa | 8080, 19071 | Search engine | | Prometheus | xyne-prometheus | 9090 | ...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...prometheus | 9090 | Metrics collection | | Grafana | xyne-grafana | 3002→3000 | M...
(QB_NEW_EN)
[grammar] ~34-~34: There might be a mistake here.
Context: ...na | 3002→3000 | Monitoring dashboards | | Loki | loki | 3100 | Log aggregation |...
(QB_NEW_EN)
[grammar] ~92-~92: There might be a mistake here.
Context: ...es | Variable | Default | Description | |----------|---------|-------------| | `...
(QB_NEW_EN)
[grammar] ~93-~93: There might be a mistake here.
Context: ...n | |----------|---------|-------------| | KEYCLOAK_ENABLED | false | Enable ...
(QB_NEW_EN)
[grammar] ~94-~94: There might be a mistake here.
Context: ...alse| Enable Keycloak authentication | |KEYCLOAK_BASE_URL|http://localhos...
(QB_NEW_EN)
[grammar] ~95-~95: There might be a mistake here.
Context: .../localhost:8081| Keycloak server URL | |KEYCLOAK_DEFAULT_REALM|xyne-share...
(QB_NEW_EN)
[grammar] ~96-~96: There might be a mistake here.
Context: ...|xyne-shared| Default realm name | |KEYCLOAK_CLIENT_ID|oa-backend` | ...
(QB_NEW_EN)
[grammar] ~97-~97: There might be a mistake here.
Context: ...D|oa-backend| Client ID for XYNE | |KEYCLOAK_CLIENT_SECRET` | *(required)...
(QB_NEW_EN)
[grammar] ~102-~102: There might be a mistake here.
Context: ... Console**: http://localhost:8081/admin/ - Username: admin - Password: admin ...
(QB_NEW_EN)
[grammar] ~103-~103: There might be a mistake here.
Context: ...p://localhost:8081/admin/ - Username: admin - Password: admin - Realm: `xyne-sh...
(QB_NEW_EN)
[grammar] ~106-~106: There might be a mistake here.
Context: ...in - Password:admin - **Realm**:xyne-shared- **Client**:oa-backend` - Test User: ...
(QB_NEW_EN)
[grammar] ~107-~107: There might be a mistake here.
Context: ... Realm: xyne-shared - Client: oa-backend - Test User: `debojyoti.mandal@juspay.in...
(QB_NEW_EN)
[grammar] ~114-~114: There might be a mistake here.
Context: ...methods: 1. Google OAuth (existing) 2. Keycloak SSO (browser redirect) 3. **E...
(QB_NEW_EN)
[grammar] ~115-~115: There might be a mistake here.
Context: ...) 2. Keycloak SSO (browser redirect) 3. Email/Password (direct login via Keycl...
(QB_NEW_EN)
[grammar] ~136-~136: There might be a mistake here.
Context: ...All data is persisted in Docker volumes: - PostgreSQL data: ../server/xyne-data -...
(QB_NEW_EN)
[grammar] ~251-~251: There might be a mistake here.
Context: ... | Feature | Development | Production | |---------|-------------|------------| |...
(QB_NEW_EN)
[grammar] ~252-~252: There might be a mistake here.
Context: ...| |---------|-------------|------------| | Keycloak Mode | start-dev (hot r...
(QB_NEW_EN)
[grammar] ~253-~253: There might be a mistake here.
Context: ...| start --optimized (faster startup) | | Container Name | xyne-keycloak |...
(QB_NEW_EN)
[grammar] ~254-~254: There might be a mistake here.
Context: ...xyne-keycloak | xyne-keycloak-prod | | Resource Limits | None | 1GB RAM, ...
(QB_NEW_EN)
[grammar] ~255-~255: There might be a mistake here.
Context: ...rce Limits** | None | 1GB RAM, 0.5 CPU | | Cache Strategy | Simple | Kubernet...
(QB_NEW_EN)
[grammar] ~256-~256: There might be a mistake here.
Context: ...| Simple | Kubernetes clustering ready | | Health Checks | Basic | Extended w...
(QB_NEW_EN)
[grammar] ~257-~257: There might be a mistake here.
Context: ...ic | Extended with longer startup time | | Logging | DEBUG friendly | INFO/WA...
(QB_NEW_EN)
[grammar] ~264-~264: There might be a mistake here.
Context: ...: Faster startup and runtime performance - Resource limits: Memory and CPU constr...
(QB_NEW_EN)
[grammar] ~265-~265: There might be a mistake here.
Context: ... constraints for container orchestration - Extended health checks: 120s startup p...
(QB_NEW_EN)
[grammar] ~266-~266: There might be a mistake here.
Context: ...0s startup period for optimization phase - Production cache: Infinspan with Kuber...
(QB_NEW_EN)
[grammar] ~267-~267: Ensure spelling is correct
Context: ...imization phase - Production cache: Infinspan with Kubernetes clustering support - **...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~267-~267: There might be a mistake here.
Context: ...nspan with Kubernetes clustering support - Environment-based configuration: All s...
(QB_NEW_EN)
[grammar] ~274-~274: There might be a mistake here.
Context: ...production deployment: 1. Security: - Change default admin credentials via env...
(QB_NEW_EN)
[grammar] ~280-~280: There might be a mistake here.
Context: ...re proper CORS origins 2. Database: - Use separate PostgreSQL instance for Key...
(QB_NEW_EN)
[grammar] ~285-~285: There might be a mistake here.
Context: ...se clustering if needed 3. Scaling: - Configure Keycloak clustering with share...
(QB_NEW_EN)
[grammar] ~286-~286: There might be a mistake here.
Context: ...g**: - Configure Keycloak clustering with shared database - Use external load ...
(QB_NEW_EN)
[grammar] ~290-~290: There might be a mistake here.
Context: ... limits and requests 4. Monitoring: - Enable Keycloak metrics integration with...
(QB_NEW_EN)
[grammar] ~303-~303: There might be a mistake here.
Context: ...ence** - PostgreSQL backend for Keycloak - Survives container restarts - Easy backu...
(QB_NEW_EN)
[grammar] ~304-~304: There might be a mistake here.
Context: ...r Keycloak - Survives container restarts - Easy backup/restore ✅ **Team Collaborat...
(QB_NEW_EN)
[grammar] ~308-~308: There might be a mistake here.
Context: ...ion** - Version-controlled configuration - Consistent realm/client setup - Shared d...
(QB_NEW_EN)
[grammar] ~309-~309: There might be a mistake here.
Context: ...guration - Consistent realm/client setup - Shared development environment ✅ **Prod...
(QB_NEW_EN)
[grammar] ~313-~313: There might be a mistake here.
Context: ...y** - Similar architecture to production - Proper database integration - Service di...
(QB_NEW_EN)
[grammar] ~314-~314: There might be a mistake here.
Context: ...production - Proper database integration - Service discovery patterns
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
deployment/KEYCLOAK_SETUP.md
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
102-102: Bare URL used
(MD034, no-bare-urls)
🪛 YAMLlint (1.37.1)
deployment/docker-compose.prod.yml
[error] 114-114: trailing spaces
(trailing-spaces)
[error] 120-120: trailing spaces
(trailing-spaces)
[error] 126-126: trailing spaces
(trailing-spaces)
[error] 131-131: trailing spaces
(trailing-spaces)
🪛 Shellcheck (0.10.0)
deployment/setup-keycloak.sh
[warning] 73-73: Quote this to prevent word splitting.
(SC2046)
🔇 Additional comments (1)
deployment/docker-compose.prod.yml (1)
143-148: Healthcheck relies on curl inside image; verify availability or change probe.Ensure curl exists in the Keycloak image version you pin; otherwise switch to wget or an exec-based script.
Suggested alternative if curl is absent:
healthcheck: test: ["CMD-SHELL", "wget -qO- http://localhost:8080/health/ready >/dev/null 2>&1"]
| set -e | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden shell settings and check prerequisites.
Use strict mode and verify required tools early.
Apply:
-set -e
+set -Eeuo pipefail
+IFS=$'\n\t'
+command -v curl >/dev/null || { echo "curl not found"; exit 1; }
+command -v jq >/dev/null || { echo "jq not found"; exit 1; }📝 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.
| set -e | |
| # fail on errors, unset vars, or pipeline failures | |
| set -Eeuo pipefail | |
| # split on newline/tab only | |
| IFS=$'\n\t' | |
| # verify required tools are installed early | |
| command -v curl >/dev/null || { echo "curl not found"; exit 1; } | |
| command -v jq >/dev/null || { echo "jq not found"; exit 1; } |
🤖 Prompt for AI Agents
In deployment/setup-keycloak.sh around lines 6 to 7, the script only has "set
-e" and needs stricter shell hardening plus early prerequisite checks; change
the shell settings to strict mode (e.g., set -euo pipefail and IFS=$'\n\t') and
add an early prerequisite check that iterates over required commands (such as
kubectl/oc, curl, jq, and any other project-specific tools) using command -v,
printing a clear error and exiting non-zero if any are missing so the script
fails fast with a helpful message.
| "attributes": { | ||
| "access.token.lifespan": "300", | ||
| "client.secret.creation.time": "'$(date +%s)'" | ||
| } | ||
| }' || echo "Client might already exist" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix command substitution quoting in JSON or drop unnecessary attribute.
The embedded $(date +%s) inside single-quoted JSON is fragile (SC2046). Safer to remove this attribute.
Apply:
- "attributes": {
- "access.token.lifespan": "300",
- "client.secret.creation.time": "'$(date +%s)'"
- }
+ "attributes": {
+ "access.token.lifespan": "300"
+ }📝 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.
| "attributes": { | |
| "access.token.lifespan": "300", | |
| "client.secret.creation.time": "'$(date +%s)'" | |
| } | |
| }' || echo "Client might already exist" | |
| "attributes": { | |
| "access.token.lifespan": "300" | |
| } | |
| }' || echo "Client might already exist" |
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 73-73: Quote this to prevent word splitting.
(SC2046)
🤖 Prompt for AI Agents
deployment/setup-keycloak.sh around lines 71-75: the JSON includes a fragile
command substitution "'$(date +%s)'" inside single quotes which breaks quoting
and triggers shellcheck warnings; remove the entire
"client.secret.creation.time" attribute from the JSON (or alternatively build
the JSON with proper double-quoting and escape sequences and inject the date
value before sending) — update the JSON payload so it no longer contains the
command substitution and ensure the line still ends with the existing
failure-safe "|| echo ..." handling.
f1591c5 to
fb8a913
Compare
There was a problem hiding this comment.
Actionable comments posted: 15
♻️ Duplicate comments (35)
deployment/.env.prod.example (2)
8-11: Align admin env names with backend config (prevents prod misconfig).Use KEYCLOAK_ADMIN_USERNAME and KEYCLOAK_ADMIN_REALM as read by server/auth/keycloak/config.ts.
-# Admin credentials (CHANGE THESE IN PRODUCTION!) -KEYCLOAK_ADMIN=admin -KEYCLOAK_ADMIN_PASSWORD=your-secure-admin-password-here +# Admin credentials (CHANGE THESE IN PRODUCTION!) +KEYCLOAK_ADMIN_REALM=master +KEYCLOAK_ADMIN_USERNAME=admin +KEYCLOAK_ADMIN_PASSWORD=your-secure-admin-password-here
33-39: Use HTTPS production hostname; avoid localhost defaults in prod templates.Replace with your public HTTPS host behind a reverse proxy. Also fix key order to satisfy dotenv-linter.
-# Keycloak integration -KEYCLOAK_ENABLED=true -KEYCLOAK_BASE_URL=http://localhost:8081 +# Keycloak integration +# Use your public HTTPS hostname (behind a reverse proxy) +# Example: https://auth.example.com +KEYCLOAK_BASE_URL=https://change-me.example.com +KEYCLOAK_ENABLED=true KEYCLOAK_DEFAULT_REALM=xyne-shared -KEYCLOAK_CLIENT_ID=oa-backend -KEYCLOAK_CLIENT_SECRET=your-client-secret-from-setup +KEYCLOAK_CLIENT_ID=oa-backend +KEYCLOAK_CLIENT_SECRET=your-client-secret-from-setupdeployment/KEYCLOAK_SETUP.md (3)
65-74: Fix production env var names to match server config.-# Required for production -export KEYCLOAK_ADMIN=your-admin-username -export KEYCLOAK_ADMIN_PASSWORD=your-secure-password +# Required for production +export KEYCLOAK_ADMIN_USERNAME=your-admin-username +export KEYCLOAK_ADMIN_PASSWORD=your-secure-password +export KEYCLOAK_ADMIN_REALM=master
108-109: Replace real-looking PII for Test User with placeholders.-- **Test User**: `debojyoti.mandal@juspay.in` / `*` +- **Test User**: `user@example.com` / `changeme`
160-165: Scrub PII in curl example; use placeholders.- -d '{"email": "debojyoti.mandal@juspay.in", "password": "*"}' + -d '{"email": "user@example.com", "password": "changeme"}'deployment/setup-keycloak.sh (8)
6-6: Harden shell and check prerequisites.-set -e +set -Eeuo pipefail +IFS=$'\n\t' +command -v curl >/dev/null || { echo "curl not found"; exit 1; } +command -v jq >/dev/null || { echo "jq not found"; exit 1; }
8-13: Parameterize URL/creds; support env overrides; print effective config.-KEYCLOAK_URL="http://localhost:8081" -ADMIN_USER="admin" -ADMIN_PASS="admin" -REALM_NAME="xyne-shared" -CLIENT_ID="oa-backend" +: "${KEYCLOAK_URL:=http://localhost:8081}" +: "${ADMIN_USER:=admin}" +: "${ADMIN_PASS:=admin}" +: "${REALM_NAME:=xyne-shared}" +: "${CLIENT_ID:=oa-backend}" +export KEYCLOAK_URL ADMIN_USER ADMIN_PASS REALM_NAME CLIENT_ID +echo "Using KEYCLOAK_URL=${KEYCLOAK_URL} REALM=${REALM_NAME} CLIENT_ID=${CLIENT_ID}"
31-46: Fail fast on realm creation errors; don't blanket-suppress curl failures.-echo "🌍 Creating realm: ${REALM_NAME}..." -curl -s -X POST "${KEYCLOAK_URL}/admin/realms" \ +echo "🌍 Creating realm: ${REALM_NAME}..." +REALM_STATUS=$(curl -s -o /dev/null -w "%{http_code}" -X GET "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}" \ + -H "Authorization: Bearer ${ADMIN_TOKEN}") +if [ "$REALM_STATUS" -ne 200 ]; then + curl -s -f -X POST "${KEYCLOAK_URL}/admin/realms" \ -H "Authorization: Bearer ${ADMIN_TOKEN}" \ -H "Content-Type: application/json" \ -d '{ "realm": "'${REALM_NAME}'", "enabled": true, "displayName": "XYNE Shared Realm", "registrationAllowed": true, "loginWithEmailAllowed": true, "duplicateEmailsAllowed": false, "resetPasswordAllowed": true, "editUsernameAllowed": true, "bruteForceProtected": false - }' || echo "Realm might already exist" + }' +else + echo "✅ Realm ${REALM_NAME} already exists, skipping creation." +fi
71-75: Fix fragile command substitution inside JSON; remove unused attribute.- "attributes": { - "access.token.lifespan": "300", - "client.secret.creation.time": "'$(date +%s)'" - } + "attributes": { + "access.token.lifespan": "300" + }
77-85: Retrieve and print client secret for confidential client (matches docs).echo "✅ Client created/verified" # Get client UUID for further configuration CLIENT_UUID=$(curl -s -X GET "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}/clients?clientId=${CLIENT_ID}" \ -H "Authorization: Bearer ${ADMIN_TOKEN}" | \ jq -r '.[0].id') -echo "📋 Client UUID: ${CLIENT_UUID}" +echo "📋 Client UUID: ${CLIENT_UUID}" + +# Get and display client secret +CLIENT_SECRET=$(curl -s -X GET "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}/clients/${CLIENT_UUID}/client-secret" \ + -H "Authorization: Bearer ${ADMIN_TOKEN}" | jq -r '.value') +echo "🔑 Client Secret: ${CLIENT_SECRET}"
86-105: Remove real PII; use example user and stronger dev password (or env).-echo "👤 Creating test user: debojyoti.mandal@juspay.in..." +echo "👤 Creating test user: ${TEST_USERNAME:-test.user@example.com}..." ... - -d '{ - "username": "debojyoti.mandal@juspay.in", - "email": "debojyoti.mandal@juspay.in", - "firstName": "Debojyoti", - "lastName": "Mandal", + -d '{ + "username": "'${TEST_USERNAME:-test.user@example.com}'", + "email": "'${TEST_USERNAME:-test.user@example.com}'", + "firstName": "Test", + "lastName": "User", "enabled": true, "emailVerified": false, "credentials": [{ "type": "password", - "value": "1", + "value": "'${TEST_PASSWORD:-devPassw0rd!}'", "temporary": false }] }' || echo "User might already exist"
110-120: Password grant must include client_secret for confidential clients; also parameterize test creds.-TEST_TOKEN=$(curl -s -X POST "${KEYCLOAK_URL}/realms/${REALM_NAME}/protocol/openid-connect/token" \ +TEST_TOKEN=$(curl -s -X POST "${KEYCLOAK_URL}/realms/${REALM_NAME}/protocol/openid-connect/token" \ -H "Content-Type: application/x-www-form-urlencoded" \ - -d "grant_type=password&client_id=${CLIENT_ID}&username=debojyoti.mandal@juspay.in&password=1&scope=openid email profile" | \ + -d "grant_type=password&client_id=${CLIENT_ID}&client_secret=${CLIENT_SECRET}&username=${TEST_USERNAME:-test.user@example.com}&password=${TEST_PASSWORD:-devPassw0rd!}&scope=openid email profile" | \ jq -r '.access_token' 2>/dev/null)
126-134: Avoid printing plaintext test password in summary; echo export line for secret instead.echo "📋 Configuration Summary:" echo " Realm: ${REALM_NAME}" echo " Client ID: ${CLIENT_ID}" echo " Base URL: ${KEYCLOAK_URL}" -echo " Test User: debojyoti.mandal@juspay.in / password: 1" +echo " Test User: ${TEST_USERNAME:-test.user@example.com}" +echo " (Set TEST_PASSWORD env var to override default dev password)" +echo " Export for server: KEYCLOAK_CLIENT_SECRET=${CLIENT_SECRET}"deployment/docker-compose.prod.yml (4)
108-113: Block: Stop shipping prod with :latest and admin/admin defaultsPin the Keycloak image and remove insecure fallback creds in prod. Force explicit env via .env and fail fast if missing.
Apply:
- image: quay.io/keycloak/keycloak:latest + image: quay.io/keycloak/keycloak:25.0.6 ... - - KEYCLOAK_ADMIN=${KEYCLOAK_ADMIN:-admin} - - KEYCLOAK_ADMIN_PASSWORD=${KEYCLOAK_ADMIN_PASSWORD:-admin} + - KEYCLOAK_ADMIN=${KEYCLOAK_ADMIN:?set in .env} + - KEYCLOAK_ADMIN_PASSWORD=${KEYCLOAK_ADMIN_PASSWORD:?set in .env}
117-119: Fix DB URL mismatch (service won’t boot otherwise)postgres creates DB "xyne" (Lines 82–85), but KC points to "keycloak". Align or provision the missing DB.
Choose one:
- - KC_DB_URL=jdbc:postgresql://xyne-db:5432/keycloak + - KC_DB_URL=jdbc:postgresql://xyne-db:5432/xyneOr create a "keycloak" DB/user in Postgres init scripts and keep current URL.
122-125: Harden hostname and HTTP settings for prodDo not run Keycloak over plain HTTP in prod; enable strict host checks and require HTTPS behind a proxy.
- - KC_HOSTNAME_STRICT=false - - KC_HOSTNAME_STRICT_HTTPS=false - - KC_HTTP_ENABLED=true + - KC_HOSTNAME_STRICT=true + - KC_HOSTNAME_STRICT_HTTPS=true + - KC_HTTP_ENABLED=false
133-134: Use a Compose-appropriate cache stack"kubernetes" stack requires KUBE_PING and isn’t suitable here. Use local (single node) or tcp.
- - KC_CACHE=ispn - - KC_CACHE_STACK=kubernetes + - KC_CACHE=ispn + - KC_CACHE_STACK=localfrontend/src/routes/auth.tsx (4)
125-131: Use crypto-safe state if you keep client-built URLsAt minimum, prefer crypto.randomUUID() and store/verify it via sessionStorage.
- state: Math.random().toString(36).substring(2, 15), + state: crypto.randomUUID(),And persist/verify across the redirect.
65-71: Read loader data correctly (don’t use Route context for loader output)Swap to useLoaderData to surface beforeLoad errors.
- const route = Route.useRouteContext() + const data = Route.useLoaderData() ... - useEffect(() => { - if (route?.error) { - setError(route.error) - } - }, [route?.error]) + useEffect(() => { + if (data?.error) setError(data.error) + }, [data?.error])
110-114: Fix Google callback base pathMissing /api prefix will 404 given server mounts under /api/v1.
- const redirectUrl = `${import.meta.env.VITE_API_BASE_URL}/v1/auth/callback` + const redirectUrl = `${import.meta.env.VITE_API_BASE_URL}/api/v1/auth/callback`
800-811: Bug: response body read twiceres.json() is consumed into userWorkspace and then called again.
if (res.ok) { - const userWorkspace = await res.json() + const userWorkspace = await res.json() // If User & Workspace exists, don't let user visit /auth if (userWorkspace?.user && userWorkspace?.workspace) { throw redirect({ to: "/" }) } - return await res.json() + return userWorkspace }server/api/auth.ts (2)
26-30: Don’t hardcode agentWhiteListReturn the configured value.
const response: any = { ...userAndWorkspace, - agentWhiteList: true, + agentWhiteList, authMethod, }
33-71: Stop re-decoding JWT here; rely on middleware context or, minimally, decode safelyThis re-parses an unverified token and duplicates middleware work. Prefer using fields from c.get("authContext")/c.get("user"). If you must decode, handle base64url and include the cookie name used by Keycloak callback.
- const authHeader = c.req.header("Authorization") - const cookieToken = getCookie(c, "auth-token") || getCookie(c, "access-token") + const authHeader = c.req.header("Authorization") + const cookieToken = + getCookie(c, "access-token") || + getCookie(c, "auth-token") || + getCookie(c, "xyne-auth-token") ... - const payload = JSON.parse(Buffer.from(parts[1], "base64").toString()) + let b64 = parts[1].replace(/-/g, "+").replace(/_/g, "/") + while (b64.length % 4) b64 += "=" + const payload = JSON.parse(Buffer.from(b64, "base64").toString("utf8"))Better: drop token parsing and extend response from authContext.user populated by the middleware. Want me to wire those fields through the middleware?
server/auth/keycloak/routes.ts (1)
16-50: Use well-known endpoint with timeout for health checksThe current health check uses an unreliable endpoint and lacks timeout handling, which could cause the health check to hang.
app.get("/health", async (c) => { try { const config = getKeycloakConfig() - const healthUrl = `${config.baseUrl}/realms/${config.defaultRealm}` + if (!config.enabled) { + return c.json({ status: "disabled", keycloak: "disabled" }) + } + + const controller = new AbortController() + const timeout = setTimeout(() => controller.abort(), 3000) + const healthUrl = `${config.baseUrl}/realms/${config.defaultRealm}/.well-known/openid-configuration` - const response = await fetch(healthUrl) + const response = await fetch(healthUrl, { + method: "GET", + signal: controller.signal + }) + clearTimeout(timeout) if (response.ok) { return c.json({ status: "healthy", keycloak: "connected", realm: config.defaultRealm, timestamp: new Date().toISOString(), }) } else { return c.json( { status: "unhealthy", keycloak: "disconnected", + httpStatus: response.status, error: "Could not reach Keycloak server", }, 503, ) } } catch (error) { Logger.error("Keycloak health check failed:", error) return c.json( { status: "unhealthy", keycloak: "error", - error: error instanceof Error ? error.message : "Unknown error", + error: error instanceof Error + ? (error.name === "AbortError" ? "timeout" : error.message) + : "Unknown error", }, 503, ) } })server/auth/keycloak/api.ts (5)
69-69: Invalid redirect_uri construction - missing protocolThe redirect_uri is built using only the host header which omits the protocol. Keycloak requires absolute URIs.
- redirect_uri: `${c.req.header("host")}/api/keycloak/callback`, + redirect_uri: `${new URL(c.req.url).origin}/api/keycloak/callback`,
91-94: Insecure cookie configuration with mismatched nameThe cookie uses
HttpOnly=falsewhich is insecure, and the namexyne-auth-tokendoesn't match what the middleware expects.- c.header( - "Set-Cookie", - `xyne-auth-token=${tokens.access_token}; Path=/; HttpOnly=false; SameSite=Strict`, - ) + const secure = new URL(c.req.url).protocol === "https:" ? "; Secure" : "" + c.header( + "Set-Cookie", + `access-token=${tokens.access_token}; Path=/; HttpOnly=true; SameSite=Lax${secure}`, + )
127-127: Hardcoded redirect_uri will break in other environmentsThe redirect URI is hardcoded to localhost:5173 which won't work in production.
- redirect_uri: "http://localhost:5173/auth", // Fixed redirect URI + redirect_uri: process.env.FRONTEND_URL + ? `${process.env.FRONTEND_URL}/auth` + : `${new URL(c.req.url).origin.replace(/:\d+$/, ':5173')}/auth`,
235-235: Refresh token retrieval doesn't match how it's setThe refresh endpoint only reads from header but login sets it as a cookie.
- const refreshToken = c.req.header("x-refresh-token") + const headerToken = c.req.header("x-refresh-token") + const cookieToken = getCookie(c, "refresh-token") + const refreshToken = headerToken || cookieToken
401-408: Logout clears wrong cookie namesThe logout endpoint clears
xyne-auth-tokenandxyne-refresh-tokenbut login setsaccess-tokenandrefresh-token.- c.header( - "Set-Cookie", - "xyne-auth-token=; Path=/; HttpOnly=false; SameSite=Strict; Max-Age=0", - ) - c.header( - "Set-Cookie", - "xyne-refresh-token=; Path=/; HttpOnly=false; SameSite=Strict; Max-Age=0", - ) + const secure = new URL(c.req.url).protocol === "https:" ? "; Secure" : "" + c.res.headers.append("Set-Cookie", + `access-token=; Path=/; HttpOnly=true; SameSite=Lax${secure}; Max-Age=0`) + c.res.headers.append("Set-Cookie", + `refresh-token=; Path=/; HttpOnly=true; SameSite=Lax${secure}; Max-Age=0`)server/auth/keycloak/middleware.ts (6)
16-16: USER_SECRET validation missing - fail fast on startupThe USER_SECRET is critical for JWT verification. Missing it will cause runtime failures.
-const userSecret = process.env.USER_SECRET! +const userSecret = process.env.USER_SECRET +if (!userSecret) { + throw new Error("USER_SECRET environment variable is required") +}
70-70: Cookie name inconsistency with Keycloak APIThe middleware reads "auth-token" and "access-token" but Keycloak API sets "xyne-auth-token".
- const cookieToken = getCookie(c, "auth-token") || getCookie(c, "access-token") + const cookieToken = + getCookie(c, "access-token") || + getCookie(c, "auth-token") || + getCookie(c, "xyne-auth-token")
159-181: Critical: JWT signature is never verified against Keycloak's public keysThe function only decodes the JWT and checks issuer/expiration but never verifies the RS256 signature. This allows anyone to forge tokens.
+import { createRemoteJWKSet, jwtVerify } from "jose" + +// Cache JWKS for performance +let jwksCache: any = null +let jwksCacheTime = 0 +const JWKS_CACHE_TTL = 3600000 // 1 hour + async function validateKeycloakToken( token: string, ): Promise<AuthContext | null> { try { - // Decode JWT payload first - const parts = token.split(".") - if (parts.length !== 3) { - return null - } - - const payload = JSON.parse(Buffer.from(parts[1], "base64").toString()) + // Get or refresh JWKS cache + if (!jwksCache || Date.now() - jwksCacheTime > JWKS_CACHE_TTL) { + const jwksUrl = `${keycloakConfig.baseUrl}/realms/${keycloakConfig.defaultRealm}/protocol/openid-connect/certs` + jwksCache = createRemoteJWKSet(new URL(jwksUrl)) + jwksCacheTime = Date.now() + } + + // Verify signature and get payload + const { payload } = await jwtVerify(token, jwksCache, { + issuer: `${keycloakConfig.baseUrl}/realms/${keycloakConfig.defaultRealm}`, + audience: keycloakConfig.clientId, + })
186-186: Hardcoded resource name "oa-backend" should be configurableThe resource access check uses a hardcoded client ID instead of configuration.
- const resourceRoles = payload.resource_access?.["oa-backend"]?.roles || [] + const resourceRoles = payload.resource_access?.[keycloakConfig.clientId || "oa-backend"]?.roles || []
52-59: Type confusion: workspaceId should be numeric but is set to stringThe AuthContext.workspaceId is typed as string but database functions expect numeric workspace IDs. This causes type mismatches throughout the codebase.
Based on the verification, many consumers expect numeric workspace IDs. Update the interface:
export interface AuthContext { sub: string role: string - workspaceId: string + workspaceId: number // Internal DB ID + workspaceExternalId: string // External UUID source: "xyne" | "keycloak" | "api-key" exp: number user?: any }Then update the middleware to set both values appropriately.
212-223: Auto-provisioning users with getDefaultWorkspaceConfig could lead to data isolation issuesWhile better than hardcoded values, auto-provisioning users into a default workspace still poses multi-tenancy risks.
Consider implementing proper workspace resolution:
- Extract organization/tenant from token claims
- Map email domain to workspace
- Require explicit workspace selection if ambiguous
- Only use defaults for development/testing
- const defaultWorkspace = await getDefaultWorkspaceConfig() + // Try to determine workspace from user's email domain + const domain = userEmail.split('@')[1] + const workspace = await getWorkspaceByDomain(db, domain) + + if (!workspace && process.env.NODE_ENV !== 'development') { + throw new Error('Unable to determine workspace for new user') + } + + const workspaceConfig = workspace || await getDefaultWorkspaceConfig()
🧹 Nitpick comments (16)
deployment/.env.prod.example (2)
22-27: Minor: reorder DB keys to appease dotenv-linter.-POSTGRES_USER=xyne -POSTGRES_PASSWORD=your-secure-db-password-here +POSTGRES_PASSWORD=your-secure-db-password-here +POSTGRES_USER=xyne
94-94: Add trailing newline.-# ./backup-restore.sh restore production-YYYYMMDD \ No newline at end of file +# ./backup-restore.sh restore production-YYYYMMDD +deployment/KEYCLOAK_SETUP.md (2)
9-24: Add language to fenced ASCII block (markdownlint).-``` +```text ...--- `102-105`: **Avoid bare URL (markdownlint).** ```diff -- **Admin Console**: http://localhost:8081/admin/ +- **Admin Console**: <http://localhost:8081/admin/>server/auth/keycloak/config.ts (2)
12-20: Minor: drop duplicate flow alias.
'token-refresh'duplicates'refresh-token'. Simplify to one.export type AuthFlow = | 'authorization-code' | 'password-grant' | 'client-credentials' - | 'refresh-token' - | 'token-refresh' + | 'refresh-token' | 'token-exchange' | 'admin-api'
239-258: Use structured logger instead of console for consistency.If pino is available, prefer it; fall back to console in tests.
deployment/docker-compose.prod.yml (1)
114-114: Trim trailing spaces flagged by YAMLlintClean trailing spaces to keep CI green.
- + ... - + ... - + ... - +Also applies to: 120-120, 126-126, 131-131
frontend/src/routes/auth.tsx (2)
149-165: Deduplicate IdP flows and avoid rebuilding OAuth URLsFactor the three handlers to reuse loginUrl + kc_idp_hint.
- const handleGoogleKeycloakLogin = () => { - ... - const authUrl = `${userLoginUrl}?${params.toString()}` - window.location.href = authUrl - } + const handleGoogleKeycloakLogin = () => { + if (!keycloakConfig?.loginUrl) return logger.error("loginUrl missing") + window.location.href = withIdpHint(keycloakConfig.loginUrl, "google") + }(Apply the same pattern for LinkedIn/Microsoft.)
Also applies to: 173-191, 201-217
371-377: Reduce sensitive/debug logs in Passkey flowAvoid logging raw challenge/credential objects in production.
- logger.info("Challenge received:", challenge) + logger.debug("Received challenge") ... - logger.info("Passkey credential created:", credential) + logger.debug("Passkey credential created")Also applies to: 409-417
server/utils/password.ts (1)
66-68: Lower verbosity of plaintext detection logUse debug to avoid noisy logs during bulk ops.
- Logger.info("Password is plaintext, hashing now") + Logger.debug("Password is plaintext, hashing now")server/auth/keycloak/routes.ts (1)
113-113: Avoid usinganytype for flow parameterThe
anytype bypasses TypeScript's type checking. Use the properAuthFlowtype instead.- const flow = c.req.param('flow') as any + const flow = c.req.param('flow') as AuthFlowserver/auth/keycloak/client-resolver.ts (1)
19-19: Avoid usinganytype for clientManagerUsing
anybypasses TypeScript's type safety. Import and use the proper type.+import type { IndustryKeycloakClientManager } from "./client-manager" + export class KeycloakClientResolver { - private clientManager: any + private clientManager: IndustryKeycloakClientManagerserver/auth/keycloak/client-manager.ts (4)
363-390: Potential admin token reuse opportunityThe
checkClientExistsmethod callsgetAdminToken()for each client validation, which could result in excessive token requests when validating multiple clients. Consider caching the admin token.Add admin token caching to reduce API calls:
+private adminTokenCache: { token: string; expiresAt: number } | null = null + +private async getCachedAdminToken(): Promise<string> { + if (this.adminTokenCache && Date.now() < this.adminTokenCache.expiresAt) { + return this.adminTokenCache.token + } + + const token = await this.getAdminToken() + // Cache for 50 seconds (tokens typically expire in 60s) + this.adminTokenCache = { + token, + expiresAt: Date.now() + 50000 + } + return token +} private async checkClientExists(client: KeycloakClient): Promise<boolean> { try { - const adminToken = await this.getAdminToken() + const adminToken = await this.getCachedAdminToken() const clientsUrl = `${this.config.baseUrl}/admin/realms/${this.config.defaultRealm}/clients?clientId=${encodeURIComponent(client.clientId)}`Also update the
discoverClientsmethod to usegetCachedAdminToken().
436-468: Performance concern: Sequential client validation in getClientStatusThe
getClientStatusmethod validates clients sequentially, which could be slow with many clients. Consider parallel validation.async getClientStatus(): Promise<{ total: number configured: number discovered: number builtin: number available: number strategy: ClientSelectionStrategy lastDiscovery?: Date }> { const allClients = await this.getAvailableClients() const configured = this.config.clients.length const builtin = this.getBuiltInClients().length const discovered = this.discoveryCache?.clients.length || 0 // Check availability of all clients - let available = 0 - for (const client of allClients) { - if (await this.validateClient(client)) { - available++ - } - } + const validationResults = await Promise.all( + allClients.map(client => this.validateClient(client)) + ) + const available = validationResults.filter(result => result).length return { total: allClients.length, configured, discovered, builtin, available, strategy: this.config.clientSelectionStrategy, lastDiscovery: this.discoveryCache?.lastUpdated } }
282-300: Unstable sort could cause unpredictable client selectionThe sorting algorithm could be unstable when clients have identical priority and environment match scores. Consider adding a stable tie-breaker.
private sortClientsByPreference(clients: KeycloakClient[], context: AuthContext): KeycloakClient[] { return clients.sort((a, b) => { // Higher priority first const priorityDiff = (b.priority || 0) - (a.priority || 0) if (priorityDiff !== 0) return priorityDiff // Environment match preference const aEnvMatch = this.getEnvironmentMatch(a, context.environment) const bEnvMatch = this.getEnvironmentMatch(b, context.environment) if (aEnvMatch !== bEnvMatch) return bEnvMatch - aEnvMatch // Prefer configured over discovered over built-in const aSource = this.getClientSource(a) const bSource = this.getClientSource(b) if (aSource !== bSource) return aSource - bSource - return 0 + // Use clientId as stable tie-breaker + return a.clientId.localeCompare(b.clientId) }) }
319-358: Consider adding cache invalidation methodThe validation cache has no way to be cleared except by waiting for TTL expiry. Consider adding a method to invalidate the cache when configurations change.
+/** + * Clear validation cache to force re-validation + */ +clearValidationCache(): void { + this.validationCache.clear() + Logger.info('Cleared client validation cache') +} + +/** + * Clear all caches + */ +clearAllCaches(): void { + this.discoveryCache = null + this.validationCache.clear() + this.adminTokenCache = null + Logger.info('Cleared all client manager caches') +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/public/assets/logo.svgis excluded by!**/*.svg
📒 Files selected for processing (20)
deployment/.env.prod.example(1 hunks)deployment/KEYCLOAK_SETUP.md(1 hunks)deployment/docker-compose.dev.yml(1 hunks)deployment/docker-compose.prod.yml(1 hunks)deployment/setup-keycloak.sh(1 hunks)frontend/package.json(1 hunks)frontend/src/routes/auth.tsx(3 hunks)server/api/auth.ts(1 hunks)server/api/password-auth.ts(1 hunks)server/auth/keycloak/api.ts(1 hunks)server/auth/keycloak/client-manager.ts(1 hunks)server/auth/keycloak/client-resolver.ts(1 hunks)server/auth/keycloak/config.ts(1 hunks)server/auth/keycloak/middleware.ts(1 hunks)server/auth/keycloak/routes.ts(1 hunks)server/db/schema/users.ts(3 hunks)server/db/user.ts(4 hunks)server/package.json(2 hunks)server/server.ts(3 hunks)server/utils/password.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- server/db/schema/users.ts
- frontend/package.json
- deployment/docker-compose.dev.yml
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-04T12:23:59.355Z
Learnt from: Aman-Asrani-Juspay
PR: xynehq/xyne#777
File: server/db/agent.ts:344-381
Timestamp: 2025-09-04T12:23:59.355Z
Learning: The team prefers to handle workspace scoping and security checks through higher-level backend functions rather than implementing them directly in database query functions like getAgentByExternalId.
Applied to files:
server/auth/keycloak/middleware.ts
📚 Learning: 2025-09-04T12:21:48.859Z
Learnt from: Aman-Asrani-Juspay
PR: xynehq/xyne#777
File: server/tests/test-executeAgent.ts:0-0
Timestamp: 2025-09-04T12:21:48.859Z
Learning: Test files in server/tests/ directory may contain hardcoded IDs, emails, and other values when they are basic test examples for development/testing purposes. This is acceptable and should not be flagged as an issue when the user clarifies it's just a basic test example.
Applied to files:
deployment/setup-keycloak.sh
🧬 Code graph analysis (11)
server/utils/password.ts (1)
server/logger/index.ts (1)
getLogger(33-90)
server/auth/keycloak/client-resolver.ts (3)
server/logger/index.ts (1)
getLogger(33-90)server/auth/keycloak/config.ts (2)
AuthFlow(12-19)KeycloakConfig(27-59)server/auth/keycloak/client-manager.ts (2)
getClientManager(474-476)AuthContext(7-13)
server/auth/keycloak/routes.ts (4)
server/logger/index.ts (1)
getLogger(33-90)server/auth/keycloak/config.ts (1)
getKeycloakConfig(154-193)server/auth/keycloak/client-resolver.ts (1)
getClientResolver(175-177)server/auth/keycloak/client-manager.ts (1)
getClientManager(474-476)
server/server.ts (1)
server/auth/keycloak/middleware.ts (1)
enhancedJwtMiddleware(62-129)
server/api/password-auth.ts (3)
server/logger/index.ts (1)
getLogger(33-90)server/utils/password.ts (1)
validatePasswordStrength(75-105)server/db/user.ts (4)
getUserByEmail(99-108)createUser(110-147)verifyUserPassword(200-235)updateUserPassword(244-255)
server/auth/keycloak/api.ts (6)
server/logger/index.ts (1)
getLogger(33-90)server/auth/keycloak/config.ts (1)
getKeycloakConfig(154-193)server/auth/keycloak/client-resolver.ts (1)
getClientResolver(175-177)server/db/schema/users.ts (1)
users(27-59)server/db/user.ts (2)
getUserByEmail(99-108)createUser(110-147)server/db/workspace.ts (2)
getWorkspaceByDomain(21-27)createWorkspace(73-91)
server/api/auth.ts (3)
server/logger/index.ts (1)
getLogger(33-90)server/db/schema/index.ts (1)
PublicUserWorkspace(23-26)server/db/user.ts (1)
getPublicUserAndWorkspaceByEmail(21-49)
server/auth/keycloak/middleware.ts (4)
server/logger/index.ts (1)
getLogger(33-90)server/auth/keycloak/config.ts (1)
getKeycloakConfig(154-193)server/db/schema/workspaces.ts (1)
workspaces(7-22)server/db/user.ts (2)
getUserByEmail(99-108)createUser(110-147)
server/auth/keycloak/client-manager.ts (2)
server/logger/index.ts (1)
getLogger(33-90)server/auth/keycloak/config.ts (4)
AuthFlow(12-19)KeycloakClient(1-10)KeycloakConfig(27-59)ClientSelectionStrategy(21-25)
frontend/src/routes/auth.tsx (3)
server/auth/keycloak/config.ts (1)
KeycloakConfig(27-59)frontend/src/routes/_authenticated.tsx (1)
Route(5-18)frontend/src/api.ts (1)
api(4-4)
server/db/user.ts (3)
server/utils/password.ts (2)
hashPassword(12-22)verifyPassword(30-42)server/types.ts (1)
TxnOrClient(301-301)server/db/schema/users.ts (1)
users(27-59)
🪛 markdownlint-cli2 (0.17.2)
deployment/KEYCLOAK_SETUP.md
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
102-102: Bare URL used
(MD034, no-bare-urls)
🪛 YAMLlint (1.37.1)
deployment/docker-compose.prod.yml
[error] 114-114: trailing spaces
(trailing-spaces)
[error] 120-120: trailing spaces
(trailing-spaces)
[error] 126-126: trailing spaces
(trailing-spaces)
[error] 131-131: trailing spaces
(trailing-spaces)
🪛 dotenv-linter (3.3.0)
deployment/.env.prod.example
[warning] 24-24: [UnorderedKey] The POSTGRES_PASSWORD key should go before the POSTGRES_USER key
(UnorderedKey)
[warning] 35-35: [UnorderedKey] The KEYCLOAK_BASE_URL key should go before the KEYCLOAK_ENABLED key
(UnorderedKey)
[warning] 36-36: [UnorderedKey] The KEYCLOAK_DEFAULT_REALM key should go before the KEYCLOAK_ENABLED key
(UnorderedKey)
[warning] 37-37: [UnorderedKey] The KEYCLOAK_CLIENT_ID key should go before the KEYCLOAK_DEFAULT_REALM key
(UnorderedKey)
[warning] 38-38: [UnorderedKey] The KEYCLOAK_CLIENT_SECRET key should go before the KEYCLOAK_DEFAULT_REALM key
(UnorderedKey)
[warning] 42-42: [UnorderedKey] The ACCESS_TOKEN_SECRET key should go before the USER_SECRET key
(UnorderedKey)
[warning] 43-43: [UnorderedKey] The REFRESH_TOKEN_SECRET key should go before the USER_SECRET key
(UnorderedKey)
[warning] 52-52: [UnorderedKey] The GRAFANA_ENABLED key should go before the METRICS_ENABLED key
(UnorderedKey)
[warning] 94-94: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🪛 Shellcheck (0.10.0)
deployment/setup-keycloak.sh
[warning] 73-73: Quote this to prevent word splitting.
(SC2046)
🔇 Additional comments (7)
server/package.json (1)
9-9: bcryptjs/@types present — verify Bun ESM/CJS import at runtime
- server/package.json contains bcryptjs@^3.0.2 and @types/bcryptjs@^3.0.0 (npm registry shows 3.0.2 / 3.0.0 available); server/utils/password.ts uses
import bcrypt from "bcryptjs"and callsbcrypt.hash/bcrypt.compare— API matches types.- Because package.json sets "type":"module" and dev uses Bun, verify in a Bun runtime that
import bcrypt from "bcryptjs"resolves; if it does not, switch to a CJS-safe import (e.g.import * as bcrypt from "bcryptjs"or use a dynamic/require shim) and re-runbun run --watch server.ts.server/server.ts (2)
731-731: Global auth middleware swap looks good; validate all pathsenhancedJwtMiddleware() now gates /api/v1. Verify Xyne JWT, Keycloak JWT, and API key flows across all routes.
Do you want a quick script to hit a few critical endpoints with each auth method?
1072-1077: Mount points OK; ensure CORS and rate-limits for public auth routes/api/keycloak and /api/password are unauthenticated entry points—confirm they have appropriate CORS and basic rate limiting at their own routers.
server/utils/password.ts (1)
12-21: LGTM: bcrypt-based hashing with sane roundsSolid defaults and error handling.
server/db/user.ts (1)
120-129: LGTM: optional password path with proper hashingcreateUser only stores hashed passwords; verify/update helpers look correct.
Also applies to: 144-145
server/auth/keycloak/client-manager.ts (2)
40-67: LGTM! Well-structured client discovery with proper deduplicationThe
getAvailableClientsmethod correctly implements the multi-source client strategy with proper deduplication logic. The hierarchical approach (configured → discovered → built-in) provides good fallback behavior.
72-110: Good implementation of intelligent client selectionThe
selectClientmethod properly filters compatible clients, sorts by preference, validates availability, and provides fallback logic. The logging provides good observability for debugging client selection issues.
| logger.info("User clicked login with Keycloak SSO") | ||
|
|
||
| // Build Keycloak authorization URL - make sure it goes to user realm, not admin | ||
| const params = new URLSearchParams({ | ||
| client_id: keycloakConfig.clientId, | ||
| redirect_uri: `${window.location.origin}/auth`, | ||
| response_type: "code", | ||
| scope: "openid email profile", | ||
| state: Math.random().toString(36).substring(2, 15), | ||
| }) | ||
|
|
||
| // Use the user realm URL, not admin realm | ||
| const userLoginUrl = `${keycloakConfig.baseUrl}/realms/${keycloakConfig.realm}/protocol/openid-connect/auth` | ||
| const authUrl = `${userLoginUrl}?${params.toString()}` | ||
|
|
||
| logger.info("Redirecting to Keycloak:", authUrl) | ||
| window.location.href = authUrl | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t hand-roll OAuth URL/state on the client
Use server-provided loginUrl; current Math.random state is not CSRF-safe, and redirect_uri can drift from backend.
- // Build Keycloak authorization URL - make sure it goes to user realm, not admin
- const params = new URLSearchParams({
- client_id: keycloakConfig.clientId,
- redirect_uri: `${window.location.origin}/auth`,
- response_type: "code",
- scope: "openid email profile",
- state: Math.random().toString(36).substring(2, 15),
- })
- // Use the user realm URL, not admin realm
- const userLoginUrl = `${keycloakConfig.baseUrl}/realms/${keycloakConfig.realm}/protocol/openid-connect/auth`
- const authUrl = `${userLoginUrl}?${params.toString()}`
-
- logger.info("Redirecting to Keycloak:", authUrl)
- window.location.href = authUrl
+ logger.info("Redirecting to Keycloak SSO")
+ if (!keycloakConfig.loginUrl) {
+ logger.error("loginUrl missing from Keycloak config")
+ return
+ }
+ window.location.href = keycloakConfig.loginUrlIf you must pass an IdP hint, append it to loginUrl safely:
// helper
function withIdpHint(loginUrl: string, hint: string) {
const u = new URL(loginUrl)
u.searchParams.set("kc_idp_hint", hint)
return u.toString()
}| import { UserRole } from "@/shared/types" | ||
|
|
||
| const Logger = getLogger(Subsystem.Server).child({ module: "password-auth" }) | ||
| const userSecret = process.env.USER_SECRET! |
There was a problem hiding this comment.
Missing critical USER_SECRET validation
The USER_SECRET environment variable is accessed with a non-null assertion which could cause runtime crashes if not set. Validate it during module initialization.
Apply this fix:
-const userSecret = process.env.USER_SECRET!
+const userSecret = process.env.USER_SECRET
+if (!userSecret) {
+ throw new Error("USER_SECRET environment variable is required for password authentication")
+}📝 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.
| const userSecret = process.env.USER_SECRET! | |
| const userSecret = process.env.USER_SECRET | |
| if (!userSecret) { | |
| throw new Error("USER_SECRET environment variable is required for password authentication") | |
| } |
🤖 Prompt for AI Agents
In server/api/password-auth.ts around line 14, the code uses a non-null
assertion on process.env.USER_SECRET which can crash at runtime if unset;
replace that with a safe initialization that reads process.env.USER_SECRET into
a constant, validate it during module initialization and throw a clear,
descriptive Error if it's missing (e.g., "USER_SECRET environment variable is
required"), so no non-null assertions remain and the app fails fast with a
helpful message.
| const defaultWorkspaceId = 3 // TODO: Make this configurable | ||
| const defaultWorkspaceExternalId = "goebza6pjn5xt2dgt5999val" // TODO: Make this configurable |
There was a problem hiding this comment.
Hardcoded workspace values are inappropriate for multi-tenant system
Using hardcoded workspace IDs (3 and "goebza6pjn5xt2dgt5999val") breaks multi-tenancy and could lead to data isolation violations. These values should be derived from the user's domain or organization context.
- // Get default workspace (simplified for now)
- const defaultWorkspaceId = 3 // TODO: Make this configurable
- const defaultWorkspaceExternalId = "goebza6pjn5xt2dgt5999val" // TODO: Make this configurable
+ // Derive workspace from email domain or use a proper onboarding flow
+ const domain = email.split('@')[1]
+ let workspace = await getWorkspaceByDomain(domain)
+
+ if (!workspace || workspace.length === 0) {
+ // Create new workspace for this domain if it doesn't exist
+ workspace = await createWorkspace(db, email, domain)
+ }
+
+ const defaultWorkspaceId = workspace[0].id
+ const defaultWorkspaceExternalId = workspace[0].externalIdCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In server/api/password-auth.ts around lines 70-71, remove the hardcoded
defaultWorkspaceId and defaultWorkspaceExternalId and instead resolve the
workspace dynamically from the incoming user's tenant/org context (e.g., derive
domain or org ID from the request or authenticated user), lookup the workspace
in the database or tenant service to get its numeric ID and externalId, and use
those values; fall back to a configurable system default only via app config
(not inline constants) or return an error if no workspace is found to avoid
silent misassignment.
| const accessCookie = `access-token=${accessToken}; Path=/; HttpOnly=true; SameSite=Strict; Max-Age=300` | ||
| const refreshCookie = `refresh-token=${refreshToken}; Path=/; HttpOnly=true; SameSite=Strict; Max-Age=2592000` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using Secure flag for cookies in production
The cookies should include the Secure flag when running in production to ensure they're only transmitted over HTTPS.
- const accessCookie = `access-token=${accessToken}; Path=/; HttpOnly=true; SameSite=Strict; Max-Age=300`
- const refreshCookie = `refresh-token=${refreshToken}; Path=/; HttpOnly=true; SameSite=Strict; Max-Age=2592000`
+ const isProduction = process.env.NODE_ENV === 'production'
+ const secureFlag = isProduction ? '; Secure' : ''
+ const accessCookie = `access-token=${accessToken}; Path=/; HttpOnly=true; SameSite=Strict${secureFlag}; Max-Age=300`
+ const refreshCookie = `refresh-token=${refreshToken}; Path=/; HttpOnly=true; SameSite=Strict${secureFlag}; Max-Age=2592000`📝 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.
| const accessCookie = `access-token=${accessToken}; Path=/; HttpOnly=true; SameSite=Strict; Max-Age=300` | |
| const refreshCookie = `refresh-token=${refreshToken}; Path=/; HttpOnly=true; SameSite=Strict; Max-Age=2592000` | |
| const isProduction = process.env.NODE_ENV === 'production' | |
| const secureFlag = isProduction ? '; Secure' : '' | |
| const accessCookie = `access-token=${accessToken}; Path=/; HttpOnly=true; SameSite=Strict${secureFlag}; Max-Age=300` | |
| const refreshCookie = `refresh-token=${refreshToken}; Path=/; HttpOnly=true; SameSite=Strict${secureFlag}; Max-Age=2592000` |
🤖 Prompt for AI Agents
In server/api/password-auth.ts around lines 108 to 109, the access and refresh
cookie strings are missing the Secure flag; update the cookie construction to
append "; Secure" when running in production (e.g. if process.env.NODE_ENV ===
"production" or request is secure) so cookies are only sent over HTTPS, keeping
all existing attributes (Path, HttpOnly, SameSite, Max-Age) and ensuring the
Secure token is included conditionally to avoid breaking local dev.
| const { currentPassword, newPassword } = c.req.valid("json") | ||
|
|
||
| // This would come from auth middleware in a complete implementation | ||
| const userEmail = c.req.header("x-user-email") // Temporary approach |
There was a problem hiding this comment.
Insecure authentication approach using header for user identification
Using a header (x-user-email) for user identification in the change-password endpoint is insecure. This should use proper JWT-based authentication.
The endpoint should:
- Extract and validate the JWT token from cookies or Authorization header
- Decode the token to get the authenticated user's email
- Verify the user is changing their own password
- // This would come from auth middleware in a complete implementation
- const userEmail = c.req.header("x-user-email") // Temporary approach
+ // Extract user from JWT token
+ const token = getCookie(c, "access-token") || c.req.header("Authorization")?.replace("Bearer ", "")
+ if (!token) {
+ return c.json({ error: "Authentication required" }, 401)
+ }
+
+ const payload = await verify(token, userSecret)
+ const userEmail = payload.sub📝 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.
| const userEmail = c.req.header("x-user-email") // Temporary approach | |
| // Extract user from JWT token | |
| const token = getCookie(c, "access-token") || c.req.header("Authorization")?.replace("Bearer ", "") | |
| if (!token) { | |
| return c.json({ error: "Authentication required" }, 401) | |
| } | |
| const payload = await verify(token, userSecret) | |
| const userEmail = payload.sub |
🤖 Prompt for AI Agents
In server/api/password-auth.ts around line 203, the endpoint currently reads the
user email from the x-user-email header which is insecure; replace this with
proper JWT authentication by extracting the token from the Authorization header
(Bearer) or from cookies, validating the token signature and expiry against your
auth secret/public key, decoding the token to obtain the authenticated user's
email/subject, and then use that decoded identity to ensure the requester is
changing their own password (compare decoded email/uid to the target account);
if validation fails or the identities don't match, return an
unauthorized/forbidden response and do not proceed with the password change.
| private inferSupportedFlows(client: any): AuthFlow[] { | ||
| const flows: AuthFlow[] = [] | ||
|
|
||
| if (client.standardFlowEnabled) flows.push('authorization-code') | ||
| if (client.directAccessGrantsEnabled) flows.push('password-grant') | ||
| if (client.serviceAccountsEnabled) flows.push('client-credentials') | ||
|
|
||
| // Always support refresh token if any flow is enabled | ||
| if (flows.length > 0) flows.push('refresh-token') | ||
|
|
||
| return flows | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Missing 'token-exchange' flow in inferSupportedFlows
The method doesn't infer support for the 'token-exchange' flow type defined in the AuthFlow type, which could lead to incomplete flow detection.
private inferSupportedFlows(client: any): AuthFlow[] {
const flows: AuthFlow[] = []
if (client.standardFlowEnabled) flows.push('authorization-code')
if (client.directAccessGrantsEnabled) flows.push('password-grant')
if (client.serviceAccountsEnabled) flows.push('client-credentials')
+
+ // Token exchange requires protocol mapper configuration
+ // This is a simplified check - actual validation may need more detail
+ if (client.protocolMappers?.some((mapper: any) =>
+ mapper.protocolMapper === 'oidc-token-exchange')) {
+ flows.push('token-exchange')
+ }
// Always support refresh token if any flow is enabled
if (flows.length > 0) flows.push('refresh-token')
return flows
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In server/auth/keycloak/client-manager.ts around lines 219 to 230, the
inferSupportedFlows function does not detect the 'token-exchange' flow; update
the function to detect token-exchange by checking the appropriate client flag
(e.g. client.tokenExchangeEnabled or client.attributes?.tokenExchangeEnabled or
client.attributes?.['token-exchange'] depending on how the client object encodes
it) and push 'token-exchange' into flows when set, keeping the existing logic
that adds 'refresh-token' if any flow is present.
| const clients = await response.json() | ||
| const clientExists = Array.isArray(clients) && clients.length > 0 && clients[0].enabled | ||
|
|
||
| Logger.debug(`Client ${client.clientId} validation: ${clientExists ? 'exists and enabled' : 'not found or disabled'}`) | ||
| return clientExists |
There was a problem hiding this comment.
Improve client existence validation logic
The current validation only checks if a client exists and is enabled, but doesn't verify if the client secret matches for confidential clients.
const clients = await response.json()
-const clientExists = Array.isArray(clients) && clients.length > 0 && clients[0].enabled
+const matchingClient = Array.isArray(clients) && clients.find(c => c.clientId === client.clientId)
+
+if (!matchingClient || !matchingClient.enabled) {
+ Logger.debug(`Client ${client.clientId} validation: not found or disabled`)
+ return false
+}
+
+// For confidential clients, verify the type matches
+if (client.type === 'confidential' && matchingClient.publicClient) {
+ Logger.warn(`Client ${client.clientId} type mismatch: expected confidential but found public`)
+ return false
+}
+
+const clientExists = true
Logger.debug(`Client ${client.clientId} validation: ${clientExists ? 'exists and enabled' : 'not found or disabled'}`)
return clientExistsCommittable suggestion skipped: line range outside the PR's diff.
| clientSecret: undefined, // Built-in clients don't need secrets | ||
| isPublic: false, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Built-in clients may require secrets in some Keycloak configurations
The comment states built-in clients don't need secrets, but this depends on Keycloak configuration. Some built-in clients like admin-cli can be configured as confidential.
🏁 Script executed:
#!/bin/bash
# Check if there are any references to client secrets for built-in clients
rg -n "admin-cli|account" --type yaml --type envLength of output: 27
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locating client-resolver files..."
rg -n --hidden "client-resolver.ts" -S || true
FILE="server/auth/keycloak/client-resolver.ts"
if [ -f "$FILE" ]; then
echo "Found $FILE — printing lines 1..240:"
sed -n '1,240p' "$FILE"
else
echo "$FILE not found, attempting to find any matching files and print their contents:"
rg -n --hidden "client-resolver" -S || true
for f in $(rg -n --hidden "client-resolver" -S --files | sed -n '1,20p' ); do
echo "---- $f ----"
sed -n '1,240p' "$f"
done
fi
echo
echo "Searching repository for related strings:"
rg -n --hidden "Built-in clients don't need secrets|Built-in client|Built in client|Built-in clients" -S || true
rg -n --hidden "clientSecret" -S || true
rg -n --hidden "isPublic" -S || true
rg -n --hidden "admin-cli|account-console|account|admin-console|realm-management" -S || trueLength of output: 60435
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== server/auth/keycloak/config.ts (first 300 lines) ==="
if [ -f server/auth/keycloak/config.ts ]; then
sed -n '1,300p' server/auth/keycloak/config.ts
else
echo "file not found: server/auth/keycloak/config.ts"
fi
echo
echo "=== server/auth/keycloak/client-manager.ts (first 500 lines) ==="
if [ -f server/auth/keycloak/client-manager.ts ]; then
sed -n '1,500p' server/auth/keycloak/client-manager.ts
else
echo "file not found: server/auth/keycloak/client-manager.ts"
fiLength of output: 23781
Don't assume built-in clients don't need secrets
server/auth/keycloak/client-resolver.ts returns built-in fallbacks with clientSecret: undefined (lines 137–138) but server/auth/keycloak/client-manager.ts registers those built-ins as type 'confidential' (getBuiltInClients). If your Keycloak realm requires secrets for these clients, token requests will omit client_secret and fail — either make built-in client secrets configurable (env/config) or align built-in client types/logic with Keycloak.
🤖 Prompt for AI Agents
In server/auth/keycloak/client-resolver.ts around lines 137–138, the built-in
fallback clients set clientSecret: undefined while getBuiltInClients registers
them as 'confidential', causing token requests to omit client_secret and fail if
the realm requires secrets; update the resolver to provide a configurable
clientSecret (read from env/config) for confidential built-ins, or change the
built-in client type/isPublic flag to true to match a public client; ensure
whatever approach you choose also updates getBuiltInClients and any registration
logic so the client type and presence/absence of clientSecret remain consistent
and token requests include client_secret when required.
| export const getKeycloakConfig = (): KeycloakConfig => { | ||
| const config: KeycloakConfig = { | ||
| enabled: process.env.KEYCLOAK_ENABLED === "true", | ||
| baseUrl: process.env.KEYCLOAK_BASE_URL || "http://localhost:8081", | ||
| adminRealm: process.env.KEYCLOAK_ADMIN_REALM || "master", | ||
| adminUsername: process.env.KEYCLOAK_ADMIN_USERNAME || "admin", | ||
| adminPassword: process.env.KEYCLOAK_ADMIN_PASSWORD || "admin", | ||
|
|
||
| // 🎯 REALM-CENTRIC: The core requirement - realm is mandatory | ||
| defaultRealm: process.env.KEYCLOAK_DEFAULT_REALM || "xyne-shared", | ||
|
|
||
| // 🏭 MULTI-CLIENT ARRAY: Clients are optional arrays | ||
| clients: parseClients(), | ||
| clientSelectionStrategy: (process.env.KEYCLOAK_CLIENT_STRATEGY as ClientSelectionStrategy) || 'hybrid', | ||
| environment: process.env.NODE_ENV || process.env.ENVIRONMENT || 'development', | ||
|
|
||
| // Built-in client preferences (per realm) | ||
| builtInClientPreferences: { | ||
| public: process.env.KEYCLOAK_BUILTIN_PUBLIC_CLIENT || 'account', | ||
| confidential: process.env.KEYCLOAK_BUILTIN_CONFIDENTIAL_CLIENT || 'admin-cli', | ||
| serviceAccount: process.env.KEYCLOAK_BUILTIN_SERVICE_CLIENT || 'admin-cli', | ||
| }, | ||
|
|
||
| // Auto-discovery settings | ||
| autoDiscovery: { | ||
| enabled: process.env.KEYCLOAK_AUTO_DISCOVERY !== 'false', // Default: true | ||
| cacheTtl: parseInt(process.env.KEYCLOAK_DISCOVERY_CACHE_TTL || '300'), // 5 minutes | ||
| retryInterval: parseInt(process.env.KEYCLOAK_DISCOVERY_RETRY_INTERVAL || '60'), // 1 minute | ||
| }, | ||
|
|
||
| // Legacy support (will be deprecated) | ||
| clientId: process.env.KEYCLOAK_CLIENT_ID, | ||
| clientSecret: process.env.KEYCLOAK_CLIENT_SECRET, | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden prod config: support alternate env name, validate HTTPS baseUrl, and sanitize numeric envs.
export const getKeycloakConfig = (): KeycloakConfig => {
const config: KeycloakConfig = {
enabled: process.env.KEYCLOAK_ENABLED === "true",
baseUrl: process.env.KEYCLOAK_BASE_URL || "http://localhost:8081",
@@
- clientSelectionStrategy: (process.env.KEYCLOAK_CLIENT_STRATEGY as ClientSelectionStrategy) || 'hybrid',
+ clientSelectionStrategy:
+ (process.env.KEYCLOAK_CLIENT_STRATEGY as ClientSelectionStrategy) ||
+ (process.env.KEYCLOAK_CLIENT_SELECTION_STRATEGY as ClientSelectionStrategy) ||
+ 'hybrid',
environment: process.env.NODE_ENV || process.env.ENVIRONMENT || 'development',
@@
- autoDiscovery: {
- enabled: process.env.KEYCLOAK_AUTO_DISCOVERY !== 'false', // Default: true
- cacheTtl: parseInt(process.env.KEYCLOAK_DISCOVERY_CACHE_TTL || '300'), // 5 minutes
- retryInterval: parseInt(process.env.KEYCLOAK_DISCOVERY_RETRY_INTERVAL || '60'), // 1 minute
- },
+ autoDiscovery: {
+ enabled: process.env.KEYCLOAK_AUTO_DISCOVERY !== 'false', // Default: true
+ cacheTtl: Number.parseInt(process.env.KEYCLOAK_DISCOVERY_CACHE_TTL || '300', 10) || 300,
+ retryInterval: Number.parseInt(process.env.KEYCLOAK_DISCOVERY_RETRY_INTERVAL || '60', 10) || 60,
+ },
@@
}
// Validate and log configuration
validateKeycloakConfig(config)
+ // Enforce HTTPS in non-dev
+ if (
+ config.enabled &&
+ config.environment !== 'development' &&
+ config.baseUrl.startsWith('http://')
+ ) {
+ throw new Error('KEYCLOAK_BASE_URL must be HTTPS in non-development environments')
+ }
return config
}📝 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.
| export const getKeycloakConfig = (): KeycloakConfig => { | |
| const config: KeycloakConfig = { | |
| enabled: process.env.KEYCLOAK_ENABLED === "true", | |
| baseUrl: process.env.KEYCLOAK_BASE_URL || "http://localhost:8081", | |
| adminRealm: process.env.KEYCLOAK_ADMIN_REALM || "master", | |
| adminUsername: process.env.KEYCLOAK_ADMIN_USERNAME || "admin", | |
| adminPassword: process.env.KEYCLOAK_ADMIN_PASSWORD || "admin", | |
| // 🎯 REALM-CENTRIC: The core requirement - realm is mandatory | |
| defaultRealm: process.env.KEYCLOAK_DEFAULT_REALM || "xyne-shared", | |
| // 🏭 MULTI-CLIENT ARRAY: Clients are optional arrays | |
| clients: parseClients(), | |
| clientSelectionStrategy: (process.env.KEYCLOAK_CLIENT_STRATEGY as ClientSelectionStrategy) || 'hybrid', | |
| environment: process.env.NODE_ENV || process.env.ENVIRONMENT || 'development', | |
| // Built-in client preferences (per realm) | |
| builtInClientPreferences: { | |
| public: process.env.KEYCLOAK_BUILTIN_PUBLIC_CLIENT || 'account', | |
| confidential: process.env.KEYCLOAK_BUILTIN_CONFIDENTIAL_CLIENT || 'admin-cli', | |
| serviceAccount: process.env.KEYCLOAK_BUILTIN_SERVICE_CLIENT || 'admin-cli', | |
| }, | |
| // Auto-discovery settings | |
| autoDiscovery: { | |
| enabled: process.env.KEYCLOAK_AUTO_DISCOVERY !== 'false', // Default: true | |
| cacheTtl: parseInt(process.env.KEYCLOAK_DISCOVERY_CACHE_TTL || '300'), // 5 minutes | |
| retryInterval: parseInt(process.env.KEYCLOAK_DISCOVERY_RETRY_INTERVAL || '60'), // 1 minute | |
| }, | |
| // Legacy support (will be deprecated) | |
| clientId: process.env.KEYCLOAK_CLIENT_ID, | |
| clientSecret: process.env.KEYCLOAK_CLIENT_SECRET, | |
| } | |
| export const getKeycloakConfig = (): KeycloakConfig => { | |
| const config: KeycloakConfig = { | |
| enabled: process.env.KEYCLOAK_ENABLED === "true", | |
| baseUrl: process.env.KEYCLOAK_BASE_URL || "http://localhost:8081", | |
| adminRealm: process.env.KEYCLOAK_ADMIN_REALM || "master", | |
| adminUsername: process.env.KEYCLOAK_ADMIN_USERNAME || "admin", | |
| adminPassword: process.env.KEYCLOAK_ADMIN_PASSWORD || "admin", | |
| // 🎯 REALM-CENTRIC: The core requirement - realm is mandatory | |
| defaultRealm: process.env.KEYCLOAK_DEFAULT_REALM || "xyne-shared", | |
| // 🏭 MULTI-CLIENT ARRAY: Clients are optional arrays | |
| clients: parseClients(), | |
| clientSelectionStrategy: | |
| (process.env.KEYCLOAK_CLIENT_STRATEGY as ClientSelectionStrategy) || | |
| (process.env.KEYCLOAK_CLIENT_SELECTION_STRATEGY as ClientSelectionStrategy) || | |
| 'hybrid', | |
| environment: process.env.NODE_ENV || process.env.ENVIRONMENT || 'development', | |
| // Built-in client preferences (per realm) | |
| builtInClientPreferences: { | |
| public: process.env.KEYCLOAK_BUILTIN_PUBLIC_CLIENT || 'account', | |
| confidential: process.env.KEYCLOAK_BUILTIN_CONFIDENTIAL_CLIENT || 'admin-cli', | |
| serviceAccount: process.env.KEYCLOAK_BUILTIN_SERVICE_CLIENT || 'admin-cli', | |
| }, | |
| // Auto-discovery settings | |
| autoDiscovery: { | |
| enabled: process.env.KEYCLOAK_AUTO_DISCOVERY !== 'false', // Default: true | |
| cacheTtl: Number.parseInt(process.env.KEYCLOAK_DISCOVERY_CACHE_TTL || '300', 10) || 300, | |
| retryInterval: Number.parseInt(process.env.KEYCLOAK_DISCOVERY_RETRY_INTERVAL || '60', 10) || 60, | |
| }, | |
| // Legacy support (will be deprecated) | |
| clientId: process.env.KEYCLOAK_CLIENT_ID, | |
| clientSecret: process.env.KEYCLOAK_CLIENT_SECRET, | |
| } | |
| // Validate and log configuration | |
| validateKeycloakConfig(config) | |
| // Enforce HTTPS in non-dev | |
| if ( | |
| config.enabled && | |
| config.environment !== 'development' && | |
| config.baseUrl.startsWith('http://') | |
| ) { | |
| throw new Error('KEYCLOAK_BASE_URL must be HTTPS in non-development environments') | |
| } | |
| return config | |
| } |
🤖 Prompt for AI Agents
In server/auth/keycloak/config.ts around lines 154-187, the config needs
hardening: accept alternate env names for key fields (e.g., KEYCLOAK_URL,
KC_BASE_URL for baseUrl; KEYCLOAK_ADMIN_USER for adminUsername;
KEYCLOAK_CLIENT_SECRET_ALT for clientSecret), validate that baseUrl uses HTTPS
when environment is production and throw/exit or log a hard error if not, and
sanitize numeric envs by using Number.parseInt with base 10, defaulting to safe
values and clamping negatives (for cacheTtl and retryInterval). Also tighten
boolean parsing for KEYCLOAK_ENABLED and ensure clientSelectionStrategy falls
back safely to the union type if the env value is invalid. Implement these
checks and fallbacks in the getKeycloakConfig function.
| export const validateKeycloakConfig = (config: KeycloakConfig): void => { | ||
| const errors: string[] = [] | ||
| const warnings: string[] = [] | ||
|
|
||
| // Core validation: Realm is mandatory | ||
| if (!config.defaultRealm) { | ||
| errors.push('🚨 KEYCLOAK_DEFAULT_REALM is mandatory') | ||
| } | ||
|
|
||
| if (!config.baseUrl) { | ||
| errors.push('🚨 KEYCLOAK_BASE_URL is mandatory') | ||
| } | ||
|
|
||
| // Client validation: Arrays are optional but should be valid if provided | ||
| if (config.clients.length === 0) { | ||
| warnings.push('⚠️ No clients configured - will use built-in clients only') | ||
| } | ||
|
|
||
| // Validate each client | ||
| config.clients.forEach((client, index) => { | ||
| if (!client.clientId) { | ||
| errors.push(`🚨 Client #${index + 1}: clientId is required`) | ||
| } | ||
|
|
||
| if (client.type === 'confidential' && !client.clientSecret) { | ||
| warnings.push(`⚠️ Client '${client.clientId}': confidential client without secret`) | ||
| } | ||
|
|
||
| if (!client.flows || client.flows.length === 0) { | ||
| warnings.push(`⚠️ Client '${client.clientId}': no flows specified`) | ||
| } | ||
| }) | ||
|
|
||
| // Strategy validation | ||
| const validStrategies: ClientSelectionStrategy[] = ['static', 'dynamic', 'hybrid', 'built-in-only'] | ||
| if (!validStrategies.includes(config.clientSelectionStrategy)) { | ||
| errors.push(`🚨 Invalid client strategy: ${config.clientSelectionStrategy}`) | ||
| } | ||
|
|
||
| // Discovery validation | ||
| if (config.autoDiscovery.enabled && (!config.adminUsername || !config.adminPassword)) { | ||
| warnings.push('⚠️ Auto-discovery enabled but admin credentials missing') | ||
| } | ||
|
|
||
| // Log results | ||
| if (errors.length > 0) { | ||
| console.error('❌ Keycloak Configuration Errors:') | ||
| errors.forEach(error => console.error(` ${error}`)) | ||
| throw new Error('Invalid Keycloak configuration') | ||
| } | ||
|
|
||
| if (warnings.length > 0) { | ||
| console.warn('⚠️ Keycloak Configuration Warnings:') | ||
| warnings.forEach(warning => console.warn(` ${warning}`)) | ||
| } | ||
|
|
||
| // Success log | ||
| console.log('✅ Keycloak Configuration Valid:') | ||
| console.log(` 📍 Realm: ${config.defaultRealm}`) | ||
| console.log(` 🏭 Strategy: ${config.clientSelectionStrategy}`) | ||
| console.log(` 🔧 Clients: ${config.clients.length} configured`) | ||
| console.log(` 🔍 Discovery: ${config.autoDiscovery.enabled ? 'enabled' : 'disabled'}`) | ||
| console.log(` 🌍 Environment: ${config.environment}`) | ||
| } No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Escalate confidential-without-secret to error in production; keep a warning in dev.
// Validate each client
config.clients.forEach((client, index) => {
@@
- if (client.type === 'confidential' && !client.clientSecret) {
- warnings.push(`⚠️ Client '${client.clientId}': confidential client without secret`)
- }
+ if (client.type === 'confidential' && !client.clientSecret) {
+ if (config.environment === 'development') {
+ warnings.push(`⚠️ Client '${client.clientId}': confidential client without secret`)
+ } else {
+ errors.push(`🚨 Client '${client.clientId}': confidential client requires KEYCLOAK_CLIENT*_SECRET in ${config.environment}`)
+ }
+ }📝 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.
| export const validateKeycloakConfig = (config: KeycloakConfig): void => { | |
| const errors: string[] = [] | |
| const warnings: string[] = [] | |
| // Core validation: Realm is mandatory | |
| if (!config.defaultRealm) { | |
| errors.push('🚨 KEYCLOAK_DEFAULT_REALM is mandatory') | |
| } | |
| if (!config.baseUrl) { | |
| errors.push('🚨 KEYCLOAK_BASE_URL is mandatory') | |
| } | |
| // Client validation: Arrays are optional but should be valid if provided | |
| if (config.clients.length === 0) { | |
| warnings.push('⚠️ No clients configured - will use built-in clients only') | |
| } | |
| // Validate each client | |
| config.clients.forEach((client, index) => { | |
| if (!client.clientId) { | |
| errors.push(`🚨 Client #${index + 1}: clientId is required`) | |
| } | |
| if (client.type === 'confidential' && !client.clientSecret) { | |
| warnings.push(`⚠️ Client '${client.clientId}': confidential client without secret`) | |
| } | |
| if (!client.flows || client.flows.length === 0) { | |
| warnings.push(`⚠️ Client '${client.clientId}': no flows specified`) | |
| } | |
| }) | |
| // Strategy validation | |
| const validStrategies: ClientSelectionStrategy[] = ['static', 'dynamic', 'hybrid', 'built-in-only'] | |
| if (!validStrategies.includes(config.clientSelectionStrategy)) { | |
| errors.push(`🚨 Invalid client strategy: ${config.clientSelectionStrategy}`) | |
| } | |
| // Discovery validation | |
| if (config.autoDiscovery.enabled && (!config.adminUsername || !config.adminPassword)) { | |
| warnings.push('⚠️ Auto-discovery enabled but admin credentials missing') | |
| } | |
| // Log results | |
| if (errors.length > 0) { | |
| console.error('❌ Keycloak Configuration Errors:') | |
| errors.forEach(error => console.error(` ${error}`)) | |
| throw new Error('Invalid Keycloak configuration') | |
| } | |
| if (warnings.length > 0) { | |
| console.warn('⚠️ Keycloak Configuration Warnings:') | |
| warnings.forEach(warning => console.warn(` ${warning}`)) | |
| } | |
| // Success log | |
| console.log('✅ Keycloak Configuration Valid:') | |
| console.log(` 📍 Realm: ${config.defaultRealm}`) | |
| console.log(` 🏭 Strategy: ${config.clientSelectionStrategy}`) | |
| console.log(` 🔧 Clients: ${config.clients.length} configured`) | |
| console.log(` 🔍 Discovery: ${config.autoDiscovery.enabled ? 'enabled' : 'disabled'}`) | |
| console.log(` 🌍 Environment: ${config.environment}`) | |
| } | |
| export const validateKeycloakConfig = (config: KeycloakConfig): void => { | |
| const errors: string[] = [] | |
| const warnings: string[] = [] | |
| // Core validation: Realm is mandatory | |
| if (!config.defaultRealm) { | |
| errors.push('🚨 KEYCLOAK_DEFAULT_REALM is mandatory') | |
| } | |
| if (!config.baseUrl) { | |
| errors.push('🚨 KEYCLOAK_BASE_URL is mandatory') | |
| } | |
| // Client validation: Arrays are optional but should be valid if provided | |
| if (config.clients.length === 0) { | |
| warnings.push('⚠️ No clients configured - will use built-in clients only') | |
| } | |
| // Validate each client | |
| config.clients.forEach((client, index) => { | |
| if (!client.clientId) { | |
| errors.push(`🚨 Client #${index + 1}: clientId is required`) | |
| } | |
| if (client.type === 'confidential' && !client.clientSecret) { | |
| if (config.environment === 'development') { | |
| warnings.push(`⚠️ Client '${client.clientId}': confidential client without secret`) | |
| } else { | |
| errors.push(`🚨 Client '${client.clientId}': confidential client requires KEYCLOAK_CLIENT*_SECRET in ${config.environment}`) | |
| } | |
| } | |
| if (!client.flows || client.flows.length === 0) { | |
| warnings.push(`⚠️ Client '${client.clientId}': no flows specified`) | |
| } | |
| }) | |
| // Strategy validation | |
| const validStrategies: ClientSelectionStrategy[] = ['static', 'dynamic', 'hybrid', 'built-in-only'] | |
| if (!validStrategies.includes(config.clientSelectionStrategy)) { | |
| errors.push(`🚨 Invalid client strategy: ${config.clientSelectionStrategy}`) | |
| } | |
| // Discovery validation | |
| if (config.autoDiscovery.enabled && (!config.adminUsername || !config.adminPassword)) { | |
| warnings.push('⚠️ Auto-discovery enabled but admin credentials missing') | |
| } | |
| // Log results | |
| if (errors.length > 0) { | |
| console.error('❌ Keycloak Configuration Errors:') | |
| errors.forEach(error => console.error(` ${error}`)) | |
| throw new Error('Invalid Keycloak configuration') | |
| } | |
| if (warnings.length > 0) { | |
| console.warn('⚠️ Keycloak Configuration Warnings:') | |
| warnings.forEach(warning => console.warn(` ${warning}`)) | |
| } | |
| // Success log | |
| console.log('✅ Keycloak Configuration Valid:') | |
| console.log(` 📍 Realm: ${config.defaultRealm}`) | |
| console.log(` 🏭 Strategy: ${config.clientSelectionStrategy}`) | |
| console.log(` 🔧 Clients: ${config.clients.length} configured`) | |
| console.log(` 🔍 Discovery: ${config.autoDiscovery.enabled ? 'enabled' : 'disabled'}`) | |
| console.log(` 🌍 Environment: ${config.environment}`) | |
| } |
🤖 Prompt for AI Agents
In server/auth/keycloak/config.ts around lines 195 to 258, the review asks that
confidential clients missing a secret be treated as an error in production but
remain a warning in development; change the current unconditional warnings.push
for confidential clients without clientSecret to branch on config.environment
(or a suitable env flag) so that if config.environment === 'production' you push
an errors.push with a clear message (include clientId or index fallback) and
otherwise push the existing warning; keep the existing error aggregation and
throw behavior unchanged so production misconfigs will cause the function to
throw.
e34e064 to
fb8a913
Compare
1 participant
server/.env
Keycloak Authentication Feature Toggles -
IS_KEYCLOAK_GOOGLE_LOGIN_ENABLED=true
IS_KEYCLOAK_MICROSOFT_LOGIN_ENABLED=true
IS_KEYCLOAK_LINKEDIN_LOGIN_ENABLED=true
IS_KEYCLOAK_EMAIL_PASSWORD_LOGIN_ENABLED=true
IS_KEYCLOAK_PASSKEY_LOGIN_ENABLED=true
IS_KEYCLOAK_SSO_LOGIN_ENABLED=false
Demo 1: https://juspay.slack.com/archives/C0980LU9NDR/p1757685616534829?thread_ts=1755871554.786219&cid=C0980LU9NDR
Demo 2: https://juspay.slack.com/archives/C0980LU9NDR/p1756232769734899?thread_ts=1755871554.786219&cid=C0980LU9NDR
Description
Testing
Additional Notes
Summary by CodeRabbit
New Features
Documentation
Chores