Skip to content

Enable axa-fr react-oidc service worker#619

Open
jnfrati wants to merge 1 commit intomainfrom
feat/oidc-service-worker
Open

Enable axa-fr react-oidc service worker#619
jnfrati wants to merge 1 commit intomainfrom
feat/oidc-service-worker

Conversation

@jnfrati
Copy link
Copy Markdown

@jnfrati jnfrati commented Apr 22, 2026

Issue ticket number and link

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Summary by CodeRabbit

Release Notes

  • Security Improvements

    • Strengthened authentication by moving token management to the service worker, preventing direct token exposure in page JavaScript.
    • Enabled service-worker-only mode for all OIDC operations.
  • Improvements

    • Updated OIDC configuration to support additional server endpoints for enhanced compatibility.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 22, 2026

📝 Walkthrough

Walkthrough

The changes migrate token handling from client-side JavaScript to a service worker. A new service worker script manages OIDC token injection, DPoP proof generation, and token state per OIDC configuration. Build scripts are updated to generate required service-worker assets, and API utilities are refactored to rely on the service worker for token injection.

Changes

Cohort / File(s) Summary
Build Scripts
package.json
Removed copy script and added oidc-gen script to prepare public OIDC assets during build. Updated build script to invoke npm run oidc-gen before next build.
Service Worker Implementation
public/OidcServiceWorker.js, public/OidcTrustedDomains.js.tmpl
New service worker handles OIDC token state management, token/DPoP injection into requests, Bearer and DPoP authorization, POST flow rewrites, and in-memory nonce handling. Updated trusted-domains template to include gRPC endpoint and removed auth0 configuration.
Application Configuration & Integration
src/auth/OIDCProvider.tsx, src/utils/api.tsx
Updated OIDC provider to enforce service-worker-only mode and specify service worker path. Refactored API utilities to use useOidcFetch for default token handling and added explicit idToken branch with manual Authorization header and expiration checks.

Sequence Diagram

sequenceDiagram
    participant Client as Client Application
    participant SW as Service Worker
    participant API as API Server
    participant OIDC as OIDC Server

    Client->>SW: Initialize with OIDC config
    OIDC->>SW: Token response (access_token, id_token)
    SW->>SW: Store tokens in memory
    
    Client->>API: Fetch request (no auth header)
    SW->>API: Intercept request
    SW->>SW: Generate DPoP proof (if required)
    SW->>SW: Retrieve stored access token
    SW->>API: Inject Authorization header
    API->>SW: Response
    SW->>Client: Response delivered
    
    Client->>SW: POST with token placeholder
    SW->>SW: Detect secured token in response
    SW->>SW: Replace placeholder with real token
    SW->>Client: Rewritten response
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Poem

🐰 A worker now guards all the tokens so dear,
Injecting with care as requests near,
DPoP proofs dance and permissions align,
No secrets exposed—just service worker's fine! 🔐✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is incomplete: neither documentation checkbox is selected, no explanation is provided for why documentation isn't needed, and no issue ticket link is included. Select one documentation option and provide justification. If documentation is not needed, explain why. Additionally, include the issue ticket number and link in the 'Issue ticket number and link' section.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Enable axa-fr react-oidc service worker' accurately describes the main change: integrating and enabling the axa-fr react-oidc service worker for OIDC operations.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/oidc-service-worker

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@jnfrati jnfrati requested a review from heisbrot April 24, 2026 08:08
@jnfrati jnfrati marked this pull request as ready for review April 24, 2026 08:08
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (2)
package.json (1)

13-13: Consider running oidc-gen in dev/turbo too.

next dev serves files from public/ as-is, and public/OidcServiceWorker.js does importScripts("OidcTrustedDomains.js") at registration time (see public/OidcServiceWorker.js line 504). If a developer hasn't manually generated the public/ assets, SW registration will fail in dev. Given that you already deleted public/OidcTrustedDomains.js after generation (the template is processed at runtime by envsubst in the container), local dev also needs a strategy (e.g., copytrusted pulling from public/local/OidcTrustedDomains.js) wired into dev. The existing copytrusted script looks like it was intended for this but isn't invoked anywhere.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@package.json` at line 13, The dev setup fails to register the service worker
when public/OidcTrustedDomains.js is missing; update package.json scripts so the
dev and turbo flows run the trusted-domains generation or fallback copy before
starting (e.g., ensure "dev" and any turbo pipeline script invoke "npm run
oidc-gen" or "npm run copytrusted" similar to "build" which runs oidc-gen), and
keep the copytrusted script wired in to populate
public/local/OidcTrustedDomains.js for local development so
public/OidcServiceWorker.js can importScripts("OidcTrustedDomains.js")
successfully; modify the "dev"/turbo script entries to run the generation/copy
step prior to starting Next.js.
public/OidcServiceWorker.js (1)

1-1: Avoid checking in the generated service worker — it will drift from the installed library version.

This file is produced by node ./node_modules/@axa-fr/oidc-client/bin/copy-service-worker-files.mjs public (the new oidc-gen script). According to the upstream library, the SW must always match the installed library version; the upstream guidance is to regenerate it on every install. Having both the checked-in copy and a regenerated one during build creates two problems:

  • npm install does not update it (no postinstall is defined), so dev builds (next dev) use whatever snapshot is in git; if you bump @axa-fr/react-oidc, page JS and SW can disagree until npm run build is run.
  • Reviewers lose the ability to tell “intentional fork of the SW” from “stale generated file” — right now nothing signals which it is.

Recommended: add public/OidcServiceWorker.js and public/OidcServiceWorker.js.map to .gitignore, add a postinstall (or a prepare) that runs oidc-gen, and keep only the .tmpl checked in. If you intentionally need to patch the SW, fork it into a separate file the build copies over the generated one, so the patch is explicit.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@public/OidcServiceWorker.js` at line 1, Remove the generated
public/OidcServiceWorker.js and public/OidcServiceWorker.js.map from version
control and add those filenames to .gitignore; add a postinstall (or prepare)
npm script that runs the upstream generator (node
./node_modules/@axa-fr/oidc-client/bin/copy-service-worker-files.mjs public or
your oidc-gen wrapper) so the service worker is regenerated on install; keep
only the source template (e.g., OidcServiceWorker.js.tmpl) checked in and, if
you need intentional local patches, create a clearly named forked file that the
build explicitly copies over the generated file instead of editing the generated
output.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@package.json`:
- Line 9: The "oidc-gen" npm script is non-portable and brittle: replace the
direct "./node_modules/@axa-fr/oidc-client/..." invocation and the plain "rm
public/OidcTrustedDomains.js" with a cross-platform, non-failing approach and
declare the missing package as a devDependency; specifically, add
"@axa-fr/oidc-client" to devDependencies (match the version used by
`@axa-fr/react-oidc`) and add "rimraf" to devDependencies, then update the
"oidc-gen" script to call the oidc client via npx (or node -e) and remove the
target file with rimraf (or a node unlink that ignores ENOENT) so removing
public/OidcTrustedDomains.js never fails on Windows or when the file is absent.

In `@public/OidcServiceWorker.js`:
- Line 504: The service worker importScripts call in OidcServiceWorker.js
(importScripts("OidcTrustedDomains.js")) will throw during next dev because
OidcTrustedDomains.js is only generated at container start; update the dev
workflow so the file always exists locally by wiring the existing copytrusted
step (or an equivalent that generates public/OidcTrustedDomains.js) into the
dev/turbo npm script pipeline (package.json) so running dev runs copytrusted
before starting the dev server; ensure oidc-gen no longer deletes the file
during local dev and document the dependency in package.json so
importScripts("OidcTrustedDomains.js") succeeds when registering the service
worker.
- Around line 492-502: The current code calls
trustedTypes.createPolicy("default", ...) which can conflict with other code
that also registers a "default" policy and cause the service worker to fail to
activate; change the policy registration to use a unique name (e.g.,
"oidc-sw-policy") and wrap the createPolicy call in a try/catch around the
trustedTypes.createPolicy("oidc-sw-policy", { createScriptURL: function(url) {
... } }) call so any "already defined" error is swallowed or logged instead of
throwing; keep the same URL whitelist check against scriptFilename inside
createScriptURL and only register when typeof trustedTypes !== "undefined" &&
typeof trustedTypes.createPolicy === "function".

In `@public/OidcTrustedDomains.js.tmpl`:
- Line 5: The template renders an empty NETBIRD_MGMT_GRPC_API_ENDPOINT into the
default allowlist which becomes the regex caret (^) and matches everything; fix
by preventing empty entries: update public/OidcTrustedDomains.js.tmpl to filter
out empty/blank env vars when building the default array (so
NETBIRD_MGMT_GRPC_API_ENDPOINT is omitted if unset) and/or add the same
required-check for NETBIRD_MGMT_GRPC_API_ENDPOINT in docker/init_react_envs.sh
(mirror the NETBIRD_MGMT_API_ENDPOINT guard); this ensures getDomains doesn't
produce a "" element that causes checkDomain and getCurrentDatabaseDomain to
compile /^/ and leak tokens.

In `@src/utils/api.tsx`:
- Around line 88-95: The polling loop in isIdTokenExpired captures the
render-time idToken and repeatedly calls isExpired(idToken), so it never sees a
refreshed token; update the loop to read the latest token each iteration (e.g.,
store the hook/getter result in a mutable ref or call the id token getter inside
the while) and call isExpired with that fresh value, or trigger a token refresh
and await a re-render before re-checking; ensure you keep the same
attempts/sleep logic but replace uses of the stale idToken with the up-to-date
value inside the loop so isExpired evaluates the current token each time.
- Around line 100-116: Change OIDCProvider.tsx so the service worker is not
enabled for idToken-only deployments: in the OIDCProvider component where you
set service_worker_only and service_worker_relative_url, check
config.tokenSource and if it equals "idtoken" do not set/omit those props (or
set them false/undefined). Update the code path that currently unconditionally
sets service_worker_only: true and service_worker_relative_url to skip/disable
the service worker when config.tokenSource === "idtoken" so fetches using the
idToken header are not intercepted and overwritten by the service worker.

---

Nitpick comments:
In `@package.json`:
- Line 13: The dev setup fails to register the service worker when
public/OidcTrustedDomains.js is missing; update package.json scripts so the dev
and turbo flows run the trusted-domains generation or fallback copy before
starting (e.g., ensure "dev" and any turbo pipeline script invoke "npm run
oidc-gen" or "npm run copytrusted" similar to "build" which runs oidc-gen), and
keep the copytrusted script wired in to populate
public/local/OidcTrustedDomains.js for local development so
public/OidcServiceWorker.js can importScripts("OidcTrustedDomains.js")
successfully; modify the "dev"/turbo script entries to run the generation/copy
step prior to starting Next.js.

In `@public/OidcServiceWorker.js`:
- Line 1: Remove the generated public/OidcServiceWorker.js and
public/OidcServiceWorker.js.map from version control and add those filenames to
.gitignore; add a postinstall (or prepare) npm script that runs the upstream
generator (node
./node_modules/@axa-fr/oidc-client/bin/copy-service-worker-files.mjs public or
your oidc-gen wrapper) so the service worker is regenerated on install; keep
only the source template (e.g., OidcServiceWorker.js.tmpl) checked in and, if
you need intentional local patches, create a clearly named forked file that the
build explicitly copies over the generated file instead of editing the generated
output.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d750d0df-c98a-4078-89f1-313bc104d75f

📥 Commits

Reviewing files that changed from the base of the PR and between e9ac1a1 and c6721a6.

📒 Files selected for processing (5)
  • package.json
  • public/OidcServiceWorker.js
  • public/OidcTrustedDomains.js.tmpl
  • src/auth/OIDCProvider.tsx
  • src/utils/api.tsx

Comment thread package.json
},
"scripts": {
"copy": "copyfiles -f ./node_modules/@axa-fr/react-oidc/dist/OidcServiceWorker.js ./public",
"oidc-gen": "node ./node_modules/@axa-fr/oidc-client/bin/copy-service-worker-files.mjs public && rm public/OidcTrustedDomains.js",
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm `@axa-fr/oidc-client` is not a direct dependency but is needed
rg -nP '"@axa-fr/oidc-client"' package.json || echo "NOT declared directly"
# Check what copy-service-worker-files.mjs emits and whether it always produces OidcTrustedDomains.js
fd -t f 'copy-service-worker-files' || true

Repository: netbirdio/dashboard

Length of output: 85


🏁 Script executed:

#!/bin/bash
# Check if `@axa-fr/react-oidc` is declared
rg -nP '"@axa-fr/react-oidc"' package.json

# Check if rimraf is already a dependency
rg -nP 'rimraf' package.json

# Check what scripts use oidc-gen
rg -nP 'oidc-gen' package.json

# Check the actual dependency section to understand the full picture
cat -n package.json | head -30

Repository: netbirdio/dashboard

Length of output: 1586


oidc-gen script has portability and reliability issues.

The script has three concerns:

  1. rm is unavailable on Windows, which breaks local dev builds for Windows contributors. Use rimraf or node -e instead.
  2. Without -f, rm fails if public/OidcTrustedDomains.js doesn't exist, halting the build pipeline. The file may not always be produced if upstream behavior changes.
  3. @axa-fr/oidc-client is invoked directly from ./node_modules/... but is undeclared. It's available only as a transitive dependency of @axa-fr/react-oidc. Changes to npm resolution, workspaces, or package managers could break the build. Add it explicitly to devDependencies or invoke via npx instead.
🛠️ Proposed fix
-    "oidc-gen": "node ./node_modules/@axa-fr/oidc-client/bin/copy-service-worker-files.mjs public && rm public/OidcTrustedDomains.js",
+    "oidc-gen": "node ./node_modules/@axa-fr/oidc-client/bin/copy-service-worker-files.mjs public && rimraf public/OidcTrustedDomains.js",

Add rimraf to devDependencies and explicitly declare @axa-fr/oidc-client in devDependencies (pinned to the version bundled with @axa-fr/react-oidc@7.26.3).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@package.json` at line 9, The "oidc-gen" npm script is non-portable and
brittle: replace the direct "./node_modules/@axa-fr/oidc-client/..." invocation
and the plain "rm public/OidcTrustedDomains.js" with a cross-platform,
non-failing approach and declare the missing package as a devDependency;
specifically, add "@axa-fr/oidc-client" to devDependencies (match the version
used by `@axa-fr/react-oidc`) and add "rimraf" to devDependencies, then update the
"oidc-gen" script to call the oidc client via npx (or node -e) and remove the
target file with rimraf (or a node unlink that ignores ENOENT) so removing
public/OidcTrustedDomains.js never fails on Windows or when the file is absent.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing the trusteddomains file should not be required.
E.g., when running locally, it would remove my local file when running the build :D

Maybe we can move the public/local/OidcTrustedDomains.js directly in public, then we can also remove the copytrusted from the package.json

The docker build should take care of replacing the OidcTrustedDomains with the actual prod values (need to verify or adjust if not)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the local OidcTrustedDomains.js content
fd -t f 'OidcTrustedDomains' --exec cat {}

# Check for Dockerfile and entrypoint scripts
fd -t f 'Dockerfile' | head -5
fd -t f 'entrypoint' | head -5
fd -t f 'docker-entrypoint' | head -5

Length of output: 374


🏁 Script executed:

#!/bin/bash
# Check docker-related files for OidcTrustedDomains handling
fd -t f 'Dockerfile' --exec grep -l 'OidcTrustedDomains' {} 2>/dev/null || echo "No Dockerfile matches"
fd -t f '*.sh' --exec grep -l 'OidcTrustedDomains' {} 2>/dev/null || echo "No shell script matches"

# Check .gitignore to see if public/OidcTrustedDomains.js is ignored
cat .gitignore 2>/dev/null || echo "No .gitignore found"

Length of output: 652


🏁 Script executed:

#!/bin/bash
# Check docker/Dockerfile content
cat docker/Dockerfile

# Check for any entrypoint or startup scripts in docker/ directory
ls docker/ 2>/dev/null || echo "No docker directory"
fd -t f . docker/ | head -20

Length of output: 1035


🏁 Script executed:

#!/bin/bash
# Check docker scripts for template substitution / envsubst
fd -t f . docker/ --exec grep -l 'envsubst\|OidcTrustedDomains\|tmpl' {} 2>/dev/null
# Also check any shell scripts anywhere
rg -nP 'envsubst|OidcTrustedDomains|\.tmpl' --type sh -l 2>/dev/null || true
# Check if there's a start.sh or entrypoint.sh
fd -t f -e sh | head -20

Length of output: 98


🏁 Script executed:

#!/bin/bash
cat docker/init_react_envs.sh

Length of output: 2945


@heisbrot Great point! I've verified that docker/init_react_envs.sh already handles the OidcTrustedDomains.js replacement at container startup via envsubst on the .tmpl file, so Docker prod is covered ✓.

Here's the full picture and the recommended approach to implement the suggestion:

How it works today:

  1. copy-service-worker-files.mjs always produces public/OidcTrustedDomains.js (a generic version) — that's why rm was added.
  2. Docker: init_react_envs.sh regenerates OidcTrustedDomains.js from OidcTrustedDomains.js.tmpl at runtime — no change needed there ✓.
  3. oidc-gen is only called in npm run build, not in next dev — so local devs running next dev are already unaffected by the rm.

Proposed implementation of heisbrot's suggestion:

  1. Move public/local/OidcTrustedDomains.jspublic/OidcTrustedDomains.js (committed with localhost values for local dev).
  2. Remove the && rm public/OidcTrustedDomains.js from oidc-gen — Docker handles it at runtime anyway.
  3. Add public/OidcTrustedDomains.js to .gitignore — this prevents git noise when someone runs npm run build locally (which would overwrite it via copy-service-worker-files.mjs) and avoids accidentally committing local dev values.
  4. Remove the copytrusted script — no longer needed.

The updated diff would look like:

# package.json
-    "oidc-gen": "node ./node_modules/@axa-fr/oidc-client/bin/copy-service-worker-files.mjs public && rm public/OidcTrustedDomains.js",
+    "oidc-gen": "node ./node_modules/@axa-fr/oidc-client/bin/copy-service-worker-files.mjs public",
-    "copytrusted": "copyfiles -f ./public/local/OidcTrustedDomains.js ./public",
# .gitignore
+/public/OidcTrustedDomains.js

Note on .gitignore: If public/OidcTrustedDomains.js is committed (not ignored), every developer who runs npm run build locally will have it overwritten by copy-service-worker-files.mjs, creating a dirty working tree. Adding it to .gitignore is the cleaner path — developers would need to create their local copy once (e.g., by copying from public/local/). Alternatively, you could document this in a README/CONTRIBUTING guide.

Comment on lines +492 to +502
if (typeof trustedTypes !== "undefined" && typeof trustedTypes.createPolicy === "function") {
trustedTypes.createPolicy("default", {
createScriptURL: function(url) {
if (url === scriptFilename) {
return url;
} else {
throw new Error("Untrusted script URL blocked: " + url);
}
}
});
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Heads-up on the default Trusted Types policy.

trustedTypes.createPolicy("default", …) creates the browser-wide default policy for this worker's scope. If the host application (or any third party it runs — analytics, error monitoring, Next.js in strict CSP mode) also installs a default policy, whichever one runs second throws at registration and the SW fails to activate. This is worth knowing about if you plan to tighten the app's CSP with Trusted Types. No change required today; flagging for awareness.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@public/OidcServiceWorker.js` around lines 492 - 502, The current code calls
trustedTypes.createPolicy("default", ...) which can conflict with other code
that also registers a "default" policy and cause the service worker to fail to
activate; change the policy registration to use a unique name (e.g.,
"oidc-sw-policy") and wrap the createPolicy call in a try/catch around the
trustedTypes.createPolicy("oidc-sw-policy", { createScriptURL: function(url) {
... } }) call so any "already defined" error is swallowed or logged instead of
throwing; keep the same URL whitelist check against scriptFilename inside
createScriptURL and only register when typeof trustedTypes !== "undefined" &&
typeof trustedTypes.createPolicy === "function".

});
}
const _self = self;
_self.importScripts(scriptFilename);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

importScripts("OidcTrustedDomains.js") will fail in local dev.

The build flow generates public/OidcTrustedDomains.js at container start via envsubst (see docker/init_react_envs.sh). For next dev, no such step runs, and the new oidc-gen script explicitly deletes public/OidcTrustedDomains.js after copying. That means next dev will attempt to register a SW that synchronously fails at importScripts, leaving developers without a working login flow locally. Please wire copytrusted (or an equivalent) into dev/turbo so a local OidcTrustedDomains.js is always present — see also the package.json comment on this.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@public/OidcServiceWorker.js` at line 504, The service worker importScripts
call in OidcServiceWorker.js (importScripts("OidcTrustedDomains.js")) will throw
during next dev because OidcTrustedDomains.js is only generated at container
start; update the dev workflow so the file always exists locally by wiring the
existing copytrusted step (or an equivalent that generates
public/OidcTrustedDomains.js) into the dev/turbo npm script pipeline
(package.json) so running dev runs copytrusted before starting the dev server;
ensure oidc-gen no longer deletes the file during local dev and document the
dependency in package.json so importScripts("OidcTrustedDomains.js") succeeds
when registering the service worker.

default:["$NETBIRD_MGMT_API_ENDPOINT"],
auth0:[]
}; No newline at end of file
default:["$NETBIRD_MGMT_API_ENDPOINT", "$NETBIRD_MGMT_GRPC_API_ENDPOINT"],
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm the gRPC endpoint is not enforced at env-init time
rg -nP -C3 'NETBIRD_MGMT_GRPC_API_ENDPOINT' docker/
# Sanity-check the SW regex construction
rg -nP -C2 'new RegExp\(`\^\$\{' public/OidcServiceWorker.js

Repository: netbirdio/dashboard

Length of output: 1951


🏁 Script executed:

#!/bin/bash
# Check if NETBIRD_MGMT_API_ENDPOINT is enforced as required
rg -nP 'NETBIRD_MGMT_API_ENDPOINT' docker/init_react_envs.sh | head -20

# Examine the actual template file
cat public/OidcTrustedDomains.js.tmpl

# Check if SW filters empty domains before using in regex
rg -nP -B10 'new RegExp\(`\^\$\{' public/OidcServiceWorker.js | head -40

Repository: netbirdio/dashboard

Length of output: 1859


🏁 Script executed:

#!/bin/bash
# Check getDomains function to see if it filters empty domains
rg -nP -A15 'function getDomains' public/OidcServiceWorker.js

# Also check how domainsToSendTokens is used
rg -nP -B5 -A10 'domainsToSendTokens' public/OidcServiceWorker.js

Repository: netbirdio/dashboard

Length of output: 1092


🏁 Script executed:

#!/bin/bash
# Find getDomains function definition
rg -nP 'function getDomains|const getDomains|getDomains\s*=' public/OidcServiceWorker.js

# Show the full function
rg -nP -A20 'const getDomains\s*=' public/OidcServiceWorker.js

Repository: netbirdio/dashboard

Length of output: 1028


🏁 Script executed:

#!/bin/bash
# Quick test: does ^.test(anything) return true?
node <<'EOF'
const regex = new RegExp("^");
console.log("Test 'anything':", regex.test("anything"));
console.log("Test 'https://example.com':", regex.test("https://example.com"));
console.log("Test empty string:", regex.test(""));
EOF

Repository: netbirdio/dashboard

Length of output: 142


Empty $NETBIRD_MGMT_GRPC_API_ENDPOINT makes the allowlist match every URL (token-leak risk).

The service worker evaluates trusted domains via new RegExp(\^${domain2}`)(seecheckDomainandgetCurrentDatabaseDomaininpublic/OidcServiceWorker.js). If NETBIRD_MGMT_GRPC_API_ENDPOINTis unset at container start,envsubstrenders the entry as"", the regex becomes ^`, and every URL matches — so the SW will attach the access token to any request made by page code that falls through to a trusted-domain check.

docker/init_react_envs.sh enforces NETBIRD_MGMT_API_ENDPOINT is set (line 43–44), but NETBIRD_MGMT_GRPC_API_ENDPOINT is assigned with no guard (line 58). The getDomains function does not filter empty strings before they reach the regex construction in line 263. Please either:

  • Require the gRPC endpoint in init_react_envs.sh (mirror the API endpoint check), and/or
  • Filter out empty strings when building the default array in the template so an unset var simply drops out.
🛡️ Template-side defense-in-depth
-const trustedDomains = {
-  default:["$NETBIRD_MGMT_API_ENDPOINT", "$NETBIRD_MGMT_GRPC_API_ENDPOINT"],
-};
+const trustedDomains = {
+  default: [
+    "$NETBIRD_MGMT_API_ENDPOINT",
+    "$NETBIRD_MGMT_GRPC_API_ENDPOINT",
+  ].filter((d) => d && d.length > 0),
+};
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@public/OidcTrustedDomains.js.tmpl` at line 5, The template renders an empty
NETBIRD_MGMT_GRPC_API_ENDPOINT into the default allowlist which becomes the
regex caret (^) and matches everything; fix by preventing empty entries: update
public/OidcTrustedDomains.js.tmpl to filter out empty/blank env vars when
building the default array (so NETBIRD_MGMT_GRPC_API_ENDPOINT is omitted if
unset) and/or add the same required-check for NETBIRD_MGMT_GRPC_API_ENDPOINT in
docker/init_react_envs.sh (mirror the NETBIRD_MGMT_API_ENDPOINT guard); this
ensures getDomains doesn't produce a "" element that causes checkDomain and
getCurrentDatabaseDomain to compile /^/ and leak tokens.

Comment thread src/utils/api.tsx
Comment on lines +88 to 95
const isIdTokenExpired = async () => {
let attempts = 4;
while (isExpired(token) && attempts > 0) {
while (isExpired(idToken) && attempts > 0) {
await sleep(500);
attempts = attempts - 1;
}
return isExpired(token);
return isExpired(idToken);
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Polling loop uses a stale idToken.

idToken is captured from the useOidcIdToken() hook at render time. The while (isExpired(idToken)) loop keeps re-checking the same value for 4 × 500 ms — if the library refreshes the token during the wait, this closure never sees the new value. A re-render is required to pick up the fresh token, but nativeFetch has already committed to its captured copy.

Pull the current token from a ref (or re-invoke the getter) inside the loop, or trigger a refresh explicitly and re-render before re-checking.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/api.tsx` around lines 88 - 95, The polling loop in isIdTokenExpired
captures the render-time idToken and repeatedly calls isExpired(idToken), so it
never sees a refreshed token; update the loop to read the latest token each
iteration (e.g., store the hook/getter result in a mutable ref or call the id
token getter inside the while) and call isExpired with that fresh value, or
trigger a token refresh and await a re-render before re-checking; ensure you
keep the same attempts/sleep logic but replace uses of the stale idToken with
the up-to-date value inside the loop so isExpired evaluates the current token
each time.

Comment thread src/utils/api.tsx
Comment on lines +100 to 116
if (useIdToken) {
const tokenExpired = await isIdTokenExpired();
if (tokenExpired) {
return handleErrors({
code: 401,
message: "token expired",
} as ErrorResponse);
}
return fetch(input, {
...init,
headers: {
"Content-Type": "application/json",
Accept: "application/json",
Authorization: `Bearer ${idToken}`,
},
});
}
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm no bypass path is wired in the SW fetch handler for non-placeholder Authorization headers
rg -nP -C3 'oidc-vanilla|bypass|skip' public/OidcServiceWorker.js
# Confirm tokenSource=idToken is a supported deployment configuration
rg -nP --type=ts --type=tsx -C2 '\btokenSource\b'
rg -nP -C2 'NETBIRD_TOKEN_SOURCE'

Repository: netbirdio/dashboard

Length of output: 2447


🏁 Script executed:

# Get detailed context around handleFetch and Authorization handling
rg -nP -A30 'const handleFetch = \(event\)' public/OidcServiceWorker.js

# Search for where Authorization header is actually modified
rg -nP -C5 'authorization|Authorization' public/OidcServiceWorker.js | head -80

# Get the actual api.tsx code at lines 100-116
sed -n '95,120p' src/utils/api.tsx

Repository: netbirdio/dashboard

Length of output: 3607


🏁 Script executed:

# Find where useIdToken is set/initialized
rg -nP -B5 -A5 'useIdToken\s*=' src/utils/api.tsx

# Check if there's conditional logic that sets useIdToken based on tokenSource
rg -nP -C5 'useIdToken' src/utils/api.tsx

# Trace config usage for tokenSource
rg -nP -C3 'config\.tokenSource' src/

Repository: netbirdio/dashboard

Length of output: 2087


🏁 Script executed:

# Check OIDCProvider.tsx for conditional service worker setup based on tokenSource
rg -nP -C10 'service_worker|OIDCProvider' src/ | head -100

# Search for where OIDCProvider is instantiated and how config is passed
rg -nP -B5 -A5 'OIDCProvider' src/

# Check if there's any bypass logic for idToken in handleFetch
rg -nP -C10 'currentDatabaseForRequestAccessToken\s*\?' public/OidcServiceWorker.js

Repository: netbirdio/dashboard

Length of output: 9626


idToken code path is broken when the service worker is active.

When config.tokenSource === "idtoken" (a supported deployment configuration), the code at lines 100–116 calls fetch() with a plain Authorization: Bearer ${idToken} header. However, OIDCProvider.tsx unconditionally enables the service worker (service_worker_only: true at line 63, regardless of tokenSource). The service worker's handleFetch function (in public/OidcServiceWorker.js, lines 544–609) intercepts every fetch to trusted domains. When no placeholder is present in the Authorization header, it defaults to key "default" (line 564) and finds the database entry "default#tabId=default" via endsWith("default") match (line 573). If this entry has cached tokens, line 608 unconditionally rewrites the Authorization header to Bearer ${currentDatabaseForRequestAccessToken.tokens.access_token}, overwriting the supplied idToken with the stored access_token.

Result: When the service worker is enabled, config.tokenSource === "idtoken" deployments receive the access_token instead of the idToken, making the idToken code path dead.

Fix: Conditionally skip service_worker_relative_url and service_worker_only in OIDCProvider.tsx when config.tokenSource === "idtoken", or add a general authorization bypass header in the service worker's handleFetch for idToken deployments (the "oidc-vanilla" header currently only works in keepAliveAsync, not in the fetch handler).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/api.tsx` around lines 100 - 116, Change OIDCProvider.tsx so the
service worker is not enabled for idToken-only deployments: in the OIDCProvider
component where you set service_worker_only and service_worker_relative_url,
check config.tokenSource and if it equals "idtoken" do not set/omit those props
(or set them false/undefined). Update the code path that currently
unconditionally sets service_worker_only: true and service_worker_relative_url
to skip/disable the service worker when config.tokenSource === "idtoken" so
fetches using the idToken header are not intercepted and overwritten by the
service worker.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems valid, can you verify / test this?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# 1. Get OIDCProvider.tsx full content
cat src/auth/OIDCProvider.tsx

echo "=== SEPARATOR ==="

# 2. Check OidcServiceWorker.js handleFetch logic around Authorization header rewriting
sed -n '540,620p' public/OidcServiceWorker.js

echo "=== SEPARATOR2 ==="

# 3. Check if there's any condition that skips Authorization header rewriting
rg -nP -C5 'access_token|authorization' public/OidcServiceWorker.js | grep -i 'authorization' | head -30

echo "=== SEPARATOR3 ==="

# 4. Check config.ts for tokenSource
rg -nP -C5 'tokenSource' src/utils/config.ts

echo "=== SEPARATOR4 ==="

# 5. Get exact lines around the handleFetch Authorization injection in OidcServiceWorker
rg -nP -B5 -A20 'currentDatabaseForRequest.*tokens.*access_token' public/OidcServiceWorker.js | head -80

Length of output: 11549


@heisbrot — confirmed. The bug is real and reproducible by tracing the exact code paths:

Trace of the broken flow:

  1. OIDCProvider.tsx sets service_worker_only: true and service_worker_relative_url: "/OidcServiceWorker.js" unconditionally, regardless of config.tokenSource.

  2. When config.tokenSource === "idtoken", nativeFetch in api.tsx correctly calls native fetch() with:

    Authorization: Bearer <idToken>
    
  3. The service worker intercepts all fetch events (not just oidcFetch-originated ones). In handleFetch (line 562–608 of OidcServiceWorker.js):

    • It reads the Authorization header → "Bearer <idToken>"
    • Since the value does not contain "ACCESS_TOKEN_SECURED_BY_OIDC_SERVICE_WORKER_", key remains "default"
    • It finds the "default" database entry (which always has tokens.access_token because service_worker_only: true means the SW manages all token storage)
    • Line 608 unconditionally overwrites the header: authorization: \Bearer ${access_token}``
  4. Result: The idToken supplied by api.tsx is silently replaced with access_token by the service worker before the request hits the network.

Fix: In OIDCProvider.tsx, conditionally disable the service worker when tokenSource === "idtoken":

useEffect(() => {
  const useIdToken = config.tokenSource?.toLowerCase() === "idtoken";
  setProviderConfig({
    authority: config.authority,
    client_id: config.clientId,
    redirect_uri: window.location.origin + config.redirectURI,
    refresh_time_before_tokens_expiration_in_second: 30,
    silent_redirect_uri: window.location.origin + config.silentRedirectURI,
    scope: config.scopesSupported,
    // Disable the service worker for idToken deployments: the SW only injects
    // access tokens, so it would silently overwrite any manually-set idToken header.
    service_worker_relative_url: useIdToken ? undefined : "/OidcServiceWorker.js",
    service_worker_only: !useIdToken,
    authority_configuration: config.auth0Auth ? auth0AuthorityConfig : undefined,
    extras: buildExtras(),
    ...(config.clientSecret
      ? { token_request_extras: { client_secret: config.clientSecret } }
      : null),
  });
  setMounted(true);
}, []);

Without service_worker_only: true, @axa-fr/react-oidc falls back to in-memory token storage, so useOidcIdToken() in api.tsx still correctly returns the idToken — no other code changes are needed.

Would you like me to plan and apply this fix?


🧠 Learnings used
Learnt from: heisbrot
Repo: netbirdio/dashboard PR: 552
File: src/modules/reverse-proxy/events/ReverseProxyEventsLocationIpCell.tsx:48-54
Timestamp: 2026-02-12T14:11:47.370Z
Learning: In components that use CopyToClipboardText, the message prop should be the notification shown after copying, while the children content is what gets copied to the clipboard. For example: <CopyToClipboardText message="Copied!">{valueToCopy}</CopyToClipboardText> copies valueToCopy and shows 'Copied!' as the notification. When reviewing, ensure this separation is preserved and that tests cover both the copied content and the notification message.

Comment thread src/auth/OIDCProvider.tsx
//service_worker_relative_url: "/OidcServiceWorker.js",
service_worker_only: false,
service_worker_relative_url: "/OidcServiceWorker.js",
service_worker_only: true,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can also leave service_worker_only at false, this looks to me like a fallback when service worker fails. Can you verify that?

Also an ENV would be good to disable it.

Comment thread src/utils/api.tsx
Comment on lines +100 to 116
if (useIdToken) {
const tokenExpired = await isIdTokenExpired();
if (tokenExpired) {
return handleErrors({
code: 401,
message: "token expired",
} as ErrorResponse);
}
return fetch(input, {
...init,
headers: {
"Content-Type": "application/json",
Accept: "application/json",
Authorization: `Bearer ${idToken}`,
},
});
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems valid, can you verify / test this?

Comment thread package.json
},
"scripts": {
"copy": "copyfiles -f ./node_modules/@axa-fr/react-oidc/dist/OidcServiceWorker.js ./public",
"oidc-gen": "node ./node_modules/@axa-fr/oidc-client/bin/copy-service-worker-files.mjs public && rm public/OidcTrustedDomains.js",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing the trusteddomains file should not be required.
E.g., when running locally, it would remove my local file when running the build :D

Maybe we can move the public/local/OidcTrustedDomains.js directly in public, then we can also remove the copytrusted from the package.json

The docker build should take care of replacing the OidcTrustedDomains with the actual prod values (need to verify or adjust if not)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants