Fix dev proxy auth header forwarding in federated modules#7065
Conversation
Each federated module's webpack dev server captured the kubeconfig token at startup and set it as static proxy headers, overwriting any auth headers forwarded by the host backend proxy (e.g. dev impersonation). Switch all modules to an onProxyReq callback that dynamically forwards incoming authorization headers when present and falls back to the startup token for standalone dev mode. Fixes: RHOAIENG-56705 Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Mike Turley <mike.turley@alum.cs.umass.edu>
…rst) The notebooks/upstream/ directory mirrors kubeflow/notebooks and should be fixed upstream first, similar to model-registry. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Mike Turley <mike.turley@alum.cs.umass.edu>
📝 WalkthroughWalkthroughSix webpack configuration files across packages (automl, autorag, eval-hub, gen-ai, maas, mlflow) were refactored to change proxy authentication handling. The changes move Kubernetes/OpenShift token retrieval from synchronous execution during config initialization into a startup capture phase via new helper functions ( Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Security & Implementation ConcernsCWE-522 (Insufficiently Protected Credentials): The CWE-347 (Improper Verification of Cryptographic Signature): No validation of the incoming Token Leakage via Proxy Logs: Forwarding raw Inconsistent Token Extraction: The gen-ai package uses Missing Authorization Header Validation: No check for whether 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/gen-ai/frontend/config/webpack.dev.js (1)
91-101:⚠️ Potential issue | 🟠 MajorAdd
getProxyHeaders()support for internal auth mode.The proxy config at lines 91–101 lacks the
headersproperty used by maas, eval-hub, automl, autorag, and other packages to handleAUTH_METHOD === 'internal'. Without it, dev mode fails to forward the requiredkubeflow-useridheader when internal auth is configured.Add
getProxyHeaders()function and includeheaders: getProxyHeaders()in the proxy config:Example (from maas webpack.dev.js)
const getProxyHeaders = () => { if (AUTH_METHOD === 'internal') { return { 'kubeflow-userid': 'user@example.com', }; } return {}; }; // In proxy config: proxy: [ { context: ['/api', '/gen-ai/api'], target: { ... }, changeOrigin: true, headers: getProxyHeaders(), onProxyReq, }, ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/gen-ai/frontend/config/webpack.dev.js` around lines 91 - 101, The proxy config is missing headers needed for AUTH_METHOD === 'internal'; add a getProxyHeaders() helper that returns { 'kubeflow-userid': 'user@example.com' } when AUTH_METHOD === 'internal' (and {} otherwise), then include headers: getProxyHeaders() in the proxy object alongside context, target, changeOrigin and onProxyReq so the dev server forwards the internal auth header; reference AUTH_METHOD, getProxyHeaders(), and the proxy config entry that uses PROXY_HOST/PROXY_PORT/PROXY_PROTOCOL and onProxyReq.
🧹 Nitpick comments (2)
packages/maas/frontend/config/webpack.dev.js (1)
31-48: Significant code duplication across packages.This
getKubeconfigToken()implementation is nearly identical across 5 package webpack configs. Extract to a shared utility (e.g.,@coderabbit/shared/webpack-utils) to reduce maintenance burden and drift risk.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/maas/frontend/config/webpack.dev.js` around lines 31 - 48, The getKubeconfigToken() function is duplicated across multiple webpack configs; extract it into a shared utility module (e.g., export a function from `@coderabbit/shared/webpack-utils` named getKubeconfigToken) and replace the local implementations in packages/maas/frontend/config/webpack.dev.js (and the other 4 configs) with an import from that new module; ensure the shared function preserves the same behavior (uses execSync, console.info on username, console.error on failure, and returns the token or empty string) and update imports in each webpack config to reference the new shared symbol.packages/automl/frontend/config/webpack.dev.js (1)
107-114: Minor inconsistency: target format differs from other packages.This uses a template string
${PROXY_PROTOCOL}://${PROXY_HOST}:${PROXY_PORT}while other packages (maas, mlflow, eval-hub, autorag) use an object{ host, protocol, port }. Both work, but object format is more consistent across the codebase.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/automl/frontend/config/webpack.dev.js` around lines 107 - 114, The proxy target in the webpack dev config uses a template string; replace it with the object-form target used elsewhere for consistency: change the proxy entry in the proxy array (the object with context ['/api','/automl/api'], changeOrigin, headers: getProxyHeaders(), onProxyReq) to supply target as { protocol: PROXY_PROTOCOL, host: PROXY_HOST, port: PROXY_PORT } instead of `${PROXY_PROTOCOL}://${PROXY_HOST}:${PROXY_PORT}` so the proxy configuration aligns with other packages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/automl/frontend/config/webpack.dev.js`:
- Around line 70-74: The current code blindly strips a "Bearer " prefix from
incomingAuth which mangles non-Bearer schemes; update the logic around
incomingAuth and proxyReq.setHeader so you only strip the "Bearer" prefix when
the header actually matches /^Bearer\s+/i (e.g., extract token with a regex
capture and fall back to the original header otherwise), then set
'Authorization' and 'x-forwarded-access-token' using the correctly extracted
values (refer to the variables incomingAuth, token and calls to
proxyReq.setHeader).
In `@packages/autorag/frontend/config/webpack.dev.js`:
- Around line 70-74: The current code always strips a "Bearer " prefix via
replace which can mangle non-Bearer Authorization values; update the extraction
so you only pull the token when the header actually matches the Bearer scheme:
check incomingAuth against /^Bearer\s+(.+)$/i (via match) and if it matches set
x-forwarded-access-token to the captured token, otherwise forward the original
incomingAuth (or omit the x-forwarded-access-token) while still setting the
Authorization header via proxyReq.setHeader; update the logic around
incomingAuth, proxyReq.setHeader('Authorization', ...), and
proxyReq.setHeader('x-forwarded-access-token', ...) accordingly.
In `@packages/eval-hub/frontend/config/webpack.dev.js`:
- Around line 71-75: The Authorization header handling currently strips "Bearer"
unconditionally using replace(/^Bearer\s+/i, ''), which can mangle non-Bearer
tokens; change the logic in the block that reads incomingAuth and sets headers
(variables/methods: incomingAuth, proxyReq.setHeader) so you only remove the
Bearer prefix when the header actually starts with it (use
/^Bearer\s+/i.test(incomingAuth)); if it matches, set x-forwarded-access-token
to the header value with the prefix removed, otherwise set
x-forwarded-access-token to the raw incomingAuth value.
In `@packages/gen-ai/frontend/config/webpack.dev.js`:
- Around line 46-49: The x-forwarded-access-token currently strips "Bearer "
only via replace and accidentally leaves the full header for non-Bearer schemes;
update the logic around incomingAuth in the webpack dev proxy handler (where
proxyReq.setHeader is called) to detect a Bearer scheme first (use
/^Bearer\s+/i.test(incomingAuth)) and only then remove the prefix, otherwise
pass the original incomingAuth through as the token, and then set
'x-forwarded-access-token' to that token.
In `@packages/maas/frontend/config/webpack.dev.js`:
- Around line 71-74: The code sets x-forwarded-access-token using
incomingAuth.replace which incorrectly forwards non-Bearer credentials; update
the block that handles incomingAuth so it only extracts and forwards a token
when incomingAuth is a Bearer token (use /^Bearer\s+/i.test(incomingAuth) and
then token = incomingAuth.replace(/^Bearer\s+/i, '') before calling
proxyReq.setHeader('x-forwarded-access-token', token)), and do not set
x-forwarded-access-token for other schemes (e.g., Basic) — continue to set the
original Authorization header via proxyReq.setHeader('Authorization',
incomingAuth') as before.
In `@packages/mlflow/frontend/config/webpack.dev.js`:
- Around line 58-62: The code currently always derives a token via replace;
instead only extract and forward an access token when the Authorization header
is a Bearer token: check incomingAuth against the /^Bearer\s+(.+)$/i regex, if
it matches capture the token and call
proxyReq.setHeader('x-forwarded-access-token', token); continue to set the
original Authorization header (proxyReq.setHeader('Authorization',
incomingAuth)) but do not set x-forwarded-access-token for non-Bearer schemes;
refer to the incomingAuth variable, the token capture, and the
proxyReq.setHeader calls to locate where to apply this change.
---
Outside diff comments:
In `@packages/gen-ai/frontend/config/webpack.dev.js`:
- Around line 91-101: The proxy config is missing headers needed for AUTH_METHOD
=== 'internal'; add a getProxyHeaders() helper that returns { 'kubeflow-userid':
'user@example.com' } when AUTH_METHOD === 'internal' (and {} otherwise), then
include headers: getProxyHeaders() in the proxy object alongside context,
target, changeOrigin and onProxyReq so the dev server forwards the internal auth
header; reference AUTH_METHOD, getProxyHeaders(), and the proxy config entry
that uses PROXY_HOST/PROXY_PORT/PROXY_PROTOCOL and onProxyReq.
---
Nitpick comments:
In `@packages/automl/frontend/config/webpack.dev.js`:
- Around line 107-114: The proxy target in the webpack dev config uses a
template string; replace it with the object-form target used elsewhere for
consistency: change the proxy entry in the proxy array (the object with context
['/api','/automl/api'], changeOrigin, headers: getProxyHeaders(), onProxyReq) to
supply target as { protocol: PROXY_PROTOCOL, host: PROXY_HOST, port: PROXY_PORT
} instead of `${PROXY_PROTOCOL}://${PROXY_HOST}:${PROXY_PORT}` so the proxy
configuration aligns with other packages.
In `@packages/maas/frontend/config/webpack.dev.js`:
- Around line 31-48: The getKubeconfigToken() function is duplicated across
multiple webpack configs; extract it into a shared utility module (e.g., export
a function from `@coderabbit/shared/webpack-utils` named getKubeconfigToken) and
replace the local implementations in
packages/maas/frontend/config/webpack.dev.js (and the other 4 configs) with an
import from that new module; ensure the shared function preserves the same
behavior (uses execSync, console.info on username, console.error on failure, and
returns the token or empty string) and update imports in each webpack config to
reference the new shared symbol.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: fdf5e434-48cd-40d2-954a-1f81ac7e0b82
📒 Files selected for processing (6)
packages/automl/frontend/config/webpack.dev.jspackages/autorag/frontend/config/webpack.dev.jspackages/eval-hub/frontend/config/webpack.dev.jspackages/gen-ai/frontend/config/webpack.dev.jspackages/maas/frontend/config/webpack.dev.jspackages/mlflow/frontend/config/webpack.dev.js
|
This PR is stale because it has been open 21 days with no activity. Remove stale label or comment or this will be closed in 7 days. |
github-actions
Bot
added
the
Stale
lucferbux
removed
the
Stale
|
/retest-required |
lucferbux
left a comment
lucferbux
left a comment
There was a problem hiding this comment.
/lgtm
/approve
After reviewing, this is doing more than it should but it does no harm, we can approve it and if anyone wants to tweak it later they can.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lucferbux The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest-required |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7065 +/- ##
==========================================
+ Coverage 64.93% 64.97% +0.03%
==========================================
Files 2355 2349 -6
Lines 74532 74477 -55
Branches 18280 18267 -13
==========================================
- Hits 48400 48389 -11
+ Misses 26132 26088 -44 see 8 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
openshift-merge-bot
Bot
merged commit d73dabc
into
opendatahub-io:main
2 participants
https://issues.redhat.com/browse/RHOAIENG-56705
Description
Each federated module's webpack dev server captures the kubeconfig/oc token at startup as static proxy headers. This overwrites any dynamically forwarded auth headers from the host backend proxy, breaking dev impersonation (
DEV_IMPERSONATE_USER/DEV_IMPERSONATE_PASSWORD).This PR replaces the static
headersapproach with anonProxyReqcallback that dynamically forwards theAuthorizationheader from each incoming request when present, falling back to the startup-captured token for standalone dev mode.Modules fixed:
oc whoami)Not included:
packages/notebooks/upstream/— mirrors upstream kubeflow/notebooks; fix submitted upstream in fix: forward incoming auth headers in webpack dev proxy kubeflow/notebooks#1013packages/model-registry/upstream/— already fixed upstream in fix(ui): forward incoming auth headers in webpack dev proxy kubeflow/hub#2544How Has This Been Tested?
Tested manually in the model-registry module (upstream fix) against an ODH cluster:
DEV_IMPERSONATE_USERandDEV_IMPERSONATE_PASSWORDsetonProxyReq, the incoming auth header from the host proxy is forwarded correctly, and impersonation works as expectedThe same pattern is applied consistently to all other modules.
Test Impact
This is a dev-only webpack config change (not shipped in production builds). No automated tests are applicable — webpack dev server proxy configuration is validated through manual dev workflow testing.
Request review criteria:
Self checklist (all need to be checked):
After the PR is posted & before it merges:
main