fix(runtime): pass auth environment variables to containers#280
Conversation
The web container's envsubst in 30-runtime-config.sh only substituted
SKILLHUB_WEB_API_BASE_URL and SKILLHUB_PUBLIC_BASE_URL, leaving auth-related
variables (authDirectEnabled, authSessionBootstrapEnabled, etc.) as literal
${...} strings in runtime-config.js. Additionally, compose.release.yml did not
pass SKILLHUB_WEB_AUTH_DIRECT_ENABLED or SKILLHUB_WEB_AUTH_DIRECT_PROVIDER to
the web container, nor SKILLHUB_AUTH_DIRECT_ENABLED to the server container.
This made it impossible to enable direct (username/password) authentication
for intranet deployments without OAuth2, even though the frontend template and
backend already supported it.
Changes:
- compose.release.yml: add SKILLHUB_AUTH_DIRECT_ENABLED to server env
- compose.release.yml: add auth direct and session bootstrap vars to web env
- 30-runtime-config.sh: expand envsubst to cover all runtime-config.js template variables
- .env.release.example: document the new auth configuration variables
All new variables default to false/empty, preserving existing GitHub OAuth behavior.
|
PR Review Helper seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
有一个阻塞点：compose.release.yml 里补了前端的 SKILLHUB_WEB_AUTH_SESSION_BOOTSTRAP_*，但没给 server 传 SKILLHUB_AUTH_SESSION_BOOTSTRAP_ENABLED。这样 release 环境里前端可能会展示或触发 session bootstrap，但后端仍然是关闭状态，会返回 403。 建议一起补上 server 侧开关；如果这次只想修 direct auth，那就先不要暴露 bootstrap 相关前端配置。 |
Per reviewer feedback: exposing SKILLHUB_WEB_AUTH_SESSION_BOOTSTRAP_* in the compose without matching SKILLHUB_AUTH_SESSION_BOOTSTRAP_ENABLED on the server would cause 403 errors when frontend attempts bootstrap. Keep this PR focused on direct auth only. Bootstrap variables are still handled in 30-runtime-config.sh with false defaults, so runtime-config.js will have authSessionBootstrapEnabled: "false" and frontend will not trigger bootstrap.
|
Thanks for the review! I've removed The 30-runtime-config.sh still handles these variables with |
|
没问题了，处理下冲突哈。 |
Resolve conflict in .env.release.example: place Direct Auth section after GitLab/OIDC blocks (added by main) and before SMTP, matching the logical ordering of auth provider configurations. Also improve the Direct Auth comment to explicitly document the dual-switch requirement (server + web), and add a why-comment to 30-runtime-config.sh explaining why session-bootstrap variables are defaulted here but not exposed in compose.release.yml.
Summary
envsubstin30-runtime-config.shonly substitutedSKILLHUB_WEB_API_BASE_URLandSKILLHUB_PUBLIC_BASE_URL, leaving auth-related variables (authDirectEnabled,authSessionBootstrapEnabled, etc.) as literal${...}strings inruntime-config.jscompose.release.ymldid not passSKILLHUB_WEB_AUTH_DIRECT_ENABLED/SKILLHUB_WEB_AUTH_DIRECT_PROVIDERto the web container, norSKILLHUB_AUTH_DIRECT_ENABLEDto the server containerChanges
SKILLHUB_AUTH_DIRECT_ENABLEDto server environmentenvsubstto cover allruntime-config.jstemplate variablesBackward Compatibility
All new variables default to
false/ empty string. Existing deployments that rely on GitHub OAuth are completely unaffected.