feat: add subpath route support#60
Conversation
|
|
Add support for deploying the admin panel under a configurable URL subpath (e.g., /adminpanel) via the VITE_BASE_PATH environment variable. The app defaults to serving from root (/) when unset, preserving backward compatibility. Changes: - vite.config.ts: set Vite 'base' from VITE_BASE_PATH for correct asset prefixing - src/router.tsx: configure TanStack Router basepath from VITE_BASE_PATH - src/routes/__root.tsx: prefix favicon href with base path - server.ts: prefix static file routes with BASE_PATH, add /health endpoint, add redirect from BASE_PATH (no slash) to BASE_PATH/ - Dockerfile: accept VITE_BASE_PATH as build ARG, fix healthcheck to use /health - src/server/session.ts: scope session cookie path to VITE_BASE_PATH to prevent cookie conflicts with other apps on the same domain - .env.example: document VITE_BASE_PATH variable
openid-client v6 uses currentUrl.searchParams.size === 0 to distinguish initiation from callback. The redirect_uri param was not stripped by LibreChat's storeAndStripChallenge (only code_challenge is), causing Passport to misclassify the request as a callback and return 401. LibreChat already knows the admin panel callback URL via ADMIN_PANEL_URL env var, so the redirect_uri param was unnecessary.
Airamhh
force-pushed
the
feat/subpath-routing-support
branch
from
cbe5f83 to
64694cd
Compare
|
Hello! Sorry, I initially tested everything with a local account rather than using SSO When I tried it with SSO, I found an issue where the There’s also another fix needed in LibreChat, specifically in the |
|
Thank you for your contribution @Airamhh ! Will be able to take a look at this next week and give a proper review. |
|
Thanks! This was thought based on the actual implementation of the admin panel, so it can be a bit rough since you have to pass an argument to specify the subpath when building the image, which doesn’t allow for much customization on the image itself... I didn’t want to change the whole thing, rather just implement some kind of workaround so people can still use it under a subpath if they needed it But yeah, it depends on how you want to implement this feature and of course whether this could impact LibreChat App as well when implementing SSO logins See you and of course take all the time you need! |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 64694cd5dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| { | ||
| rel: 'icon', | ||
| href: '/favicon.ico', | ||
| href: `${import.meta.env.VITE_BASE_PATH || ''}/favicon.ico`, |
There was a problem hiding this comment.
When VITE_BASE_PATH is set to '/' (which is the default build arg in this change), this expression produces //favicon.ico instead of /favicon.ico. Browsers interpret //... as a scheme-relative absolute URL (host favicon.ico), so root deployments can request the icon from the wrong host and fail to load it. This regression is introduced by concatenating the raw env value without trimming the root-slash case.
Useful? React with 👍 / 👎.
| ...(BASE_PATH ? { [`${BASE_PATH}`]: () => Response.redirect(`${BASE_PATH}/`, 302) } : {}), | ||
| '/*': async (req) => { | ||
| const url = new URL(req.url); | ||
| const res = await withHttpMetrics(req, url.pathname, () => handler.fetch(req)); |
There was a problem hiding this comment.
In subpath deployments (for example VITE_BASE_PATH=/adminpanel), this passes /adminpanel/... into withHttpMetrics, but normalizeMetricsPath only maps unprefixed app routes like /login and /configuration. As a result, most application page hits are relabeled as unknown, which regresses per-route observability introduced by this feature. Strip BASE_PATH from url.pathname (or normalize it) before recording metrics.
Useful? React with 👍 / 👎.
- Normalize VITE_BASE_PATH before building favicon URL to avoid //favicon.ico - Strip BASE_PATH prefix from pathname before recording route metrics - Add required search prop to Link component to fix TS2741
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Reviewed by Cursor Bugbot for commit 143d976. Configure here.
| const codeChallenge = crypto.createHash('sha256').update(codeVerifier).digest('hex'); | ||
| authUrl.searchParams.set('code_challenge', codeChallenge); | ||
| if (requestOrigin) | ||
| authUrl.searchParams.set('redirect_uri', `${requestOrigin}/auth/openid/callback`); |
There was a problem hiding this comment.
SSO redirect_uri removed instead of made subpath-aware
Medium Severity
The openidLoginFn previously sent a redirect_uri parameter to LibreChat's OAuth endpoint so it knew where to redirect after authentication. This was removed entirely instead of being updated to include the base path. Without redirect_uri, LibreChat must determine the callback URL from its own config — which won't include the subpath (e.g., /adminpanel/auth/openid/callback). This means SSO is likely broken for subpath deployments, which is the core purpose of this PR. Even for non-subpath users, this is a behavioral regression since the callback URL is no longer explicitly communicated to the OAuth flow.
Reviewed by Cursor Bugbot for commit 143d976. Configure here.
|
Hello! I adressed the issues! Well, expect the one related to the redirect_uri one, depends on how you would prefer to deal with this approach since I think it's related to LibreChat as well |
4 participants
Summary
Hello there!
Hope you're doing great, this PR focuses on adding supath support to the application, I didn't know how you exactly wanted to implement it, hope you like it!
Closes #54
Change Type
Testing
--build-arg VITE_BASE_PATH=/adminpanel-e VITE_BASE_PATH=/adminpanel/healthreturns 200/adminpanel/loads the app (assets, routes, login all work)/) the app works as before at rootTest Configuration:
/adminpanel→http://localhost:4082Checklist
Note
Medium Risk
Introduces path-prefixing across build, routing, and session cookie configuration; misconfiguration of
VITE_BASE_PATHcould break asset loading, navigation, or auth/session behavior when deployed behind a reverse proxy.Overview
Adds support for serving the admin panel under a configurable subpath via
VITE_BASE_PATH, wiring it through Vite’s buildbase, TanStack Routerbasepath, static asset routing, favicon URLs, and session cookiepath.Updates the Bun server to prefix static routes with the base path, redirect
${BASE_PATH}→${BASE_PATH}/, adjust Prometheus metrics path normalization to ignore the prefix, and adds a dedicated/healthendpoint (also used by the DockerHEALTHCHECK). Documentation and.env.exampleare updated to describe the new variable and Docker build/run usage.Reviewed by Cursor Bugbot for commit 143d976. Bugbot is set up for automated code reviews on this repo. Configure here.