Skip to content

Enable axa-fr react-oidc service worker #619

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jnfrati wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Enable axa-fr react-oidc service worker #619

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
"node": ">=20.9.0"
},
"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 •
edited
Loading

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

@heisbrot heisbrot Apr 24, 2026

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

@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.

🧩 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.js → public/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.

"copytrusted": "copyfiles -f ./public/local/OidcTrustedDomains.js ./public",
"dev": "next dev -p 3000",
"turbo": "next dev -p 3000 --turbo",
"build": "next build",
"build": "npm run oidc-gen && next build",
"start": "next start",
"lint": "next lint",
"cypress:open": "cypress open"
Expand Down
Loading
Loading