<WIP> OIDC-UI for thunder UI SDK integration#1844
Open
zesu22 wants to merge 4 commits into
Open
Conversation
Signed-off-by: Zeeshan Mehboob <zeeshan.mehboob@infosys.com>
WalkthroughGreenfield eSignet OIDC React UI: project scaffolding, runtime-configurable theming, CSRF-protected API client, offline detection, SBI biometric integration, lazy routes/pages, utilities (encoding/hashing/parsing/redirect), and comprehensive Vitest tests. ChangeseSignet OIDC UI Application
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 41
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.agents/feature-doc/new_ui.md:
- Line 45: The markdown in .agents/feature-doc/new_ui.md has lint errors: add a
fenced code language after the opening ``` at the code block around line 45
(e.g., ```text) and ensure the code block is properly closed, and insert
required blank lines before the headings "Copy as-is (assets only)", "Copy and
adapt (CSS)" and "Rewrite in TypeScript" and before the table that begins with
"| Old file (JS) | New file (TS) | Notes |" so MD022/MD040/MD058 are satisfied;
locate the code block and headings/table by their text content and update the
fences and spacing accordingly.
In `@oidc-ui/.env`:
- Line 1: Add a POSIX trailing newline to the .env entry by ensuring the file
ends with a newline character after the line
"VITE_API_URL=https://localhost:8090" so the linter stops flagging the missing
newline; simply append a newline at the end of that file.
In `@oidc-ui/.env.production`:
- Line 1: The file ending for the environment variable VITE_API_URL is missing a
trailing newline; update the .env file that defines VITE_API_URL so the final
line ends with a newline character (i.e., add a single line break after the line
"VITE_API_URL=https://localhost:8090") to satisfy POSIX text file conventions
and the linter.
- Line 1: The production environment variable VITE_API_URL is incorrectly set to
a localhost address; change the VITE_API_URL value to the real production API
endpoint (replace "https://localhost:8090" with your production API domain/URL)
so the deployed app points to the correct backend, and ensure the scheme
(http/https) and port match the production service and any CI/CD or secrets
store is updated accordingly.
In `@oidc-ui/package.json`:
- Line 25: Move the "prop-types" package from dependencies to devDependencies in
package.json: remove the "prop-types": "^15.8.1" entry from the root
dependencies block and add the same entry under devDependencies, unless a
runtime dependency (e.g., `@asgardeo/react` or another package) explicitly lists
prop-types as a peerDependency — if so, leave it in dependencies; otherwise keep
it only in devDependencies to avoid shipping it in production bundles.
In `@oidc-ui/public/locales/en.json`:
- Around line 203-220: Fix the user-facing typos in the English locale: change
the value for esignet_details_heading from "wellknown APIs" to "well‑known
APIs", change wellknown_api from ".wellknown API" to ".well‑known API", rename
the key backgroud_image_alt to background_image_alt and correct its value to
"Sign in with eSignet", and search for any occurrences of the misspelled
"Authentiation" and replace with "Authentication" (update the corresponding
locale key/value(s) such as invalid_auth_factor to use proper casing and wording
like "Invalid authentication factors. Please try again...").
In `@oidc-ui/public/theme-config.js`:
- Around line 1-7: The script currently writes properties directly to DOM nodes
(using document.getElementsByTagName("body")[0].classList.add and
document.getElementById("icon"/"title"/"fonts-url") assignments) without
verifying those elements exist; update the code to guard each DOM lookup before
mutating it: check that document.getElementsByTagName("body")[0] is truthy
before calling classList.add, and check that document.getElementById("icon"),
document.getElementById("title"), and document.getElementById("fonts-url")
return non-null before setting href or innerHTML (also preserve the existing
DEFAULT_FONT_URL !== "" condition). Ensure you reference the same element IDs
("icon","title","fonts-url") and the body lookup so changes are localized.
- Line 5: The code sets document.getElementById("title").innerHTML from
window._env_.DEFAULT_TITLE which creates an XSS sink; update the assignment to
use textContent instead and add a null check: obtain the element via
document.getElementById("title"), ensure the element exists, and then set
element.textContent = window._env_.DEFAULT_TITLE (treating DEFAULT_TITLE as
potentially undefined/null); reference the existing call to
document.getElementById("title").innerHTML to locate the change and replace it
accordingly.
In `@oidc-ui/README.md`:
- Around line 124-151: The README.md fenced directory tree block lacks a
language tag and triggers MD040; update the fenced code block that begins with
"oidc-ui/" to include a language identifier (e.g., change ``` to ```text) so the
block becomes ```text ... ```; locate the block in the README.md around the
"oidc-ui/" listing and add the language specifier to satisfy the markdown lint
rule.
In `@oidc-ui/src/__tests__/pages/LoginPage.test.tsx`:
- Around line 5-10: The mock for react-i18next in the test uses a wrong
signature for t; update the vi.mock('react-i18next') implementation so
useTranslation returns t with an optional second parameter (e.g. t: (key:
string, options?: any) => key) instead of requiring a fallback, and keep i18n: {
language: 'en', changeLanguage: vi.fn() } unchanged; modify the t signature in
the mock inside LoginPage.test.tsx (the useTranslation mock) so it matches the
standard react-i18next API and existing single-argument calls.
In `@oidc-ui/src/__tests__/services/config.service.test.ts`:
- Around line 24-30: The test for fetchThemeConfig should assert the exact
request URL used; update the test that mocks axios.get to also verify axios.get
was called with CONFIG_URLS.THEME_CONFIG by adding an assertion referencing
axios.get and CONFIG_URLS.THEME_CONFIG alongside the existing call count and
result checks so the test fails if the wrong endpoint is used.
In `@oidc-ui/src/__tests__/services/lang-config.service.test.ts`:
- Around line 22-35: Tests for getLocaleConfiguration and getLangCodeMapping
only assert return values but don't verify the HTTP requests; add request-level
assertions to each test to ensure axios.get was called exactly once and with the
expected URL and options. In the getLocaleConfiguration test, assert
vi.mocked(axios.get) was called once and with the locale configuration endpoint
your service uses (the call invoked by getLocaleConfiguration) and any expected
headers or params; similarly, in the getLangCodeMapping test assert
vi.mocked(axios.get) was called once and with the same expected URL/options used
by getLangCodeMapping. Reference axios.get, getLocaleConfiguration,
getLangCodeMapping and mockLocaleConfig when adding these assertions so
duplicate requests or wrong endpoints fail the tests.
In `@oidc-ui/src/__tests__/utils/parsing.test.ts`:
- Around line 58-107: The tests mutate the global window._env_ and don't restore
it, causing cross-file leakage; fix by saving the original window._env_ before
tests (e.g., in a local variable in the describe scope) and restoring it in an
afterEach/afterAll teardown so other suites aren't affected; update the test
file (around the beforeEach that assigns window._env_ and the tests that call
getPollingConfig) to capture the original value and reassign window._env_ back
after each test.
In `@oidc-ui/src/__tests__/utils/redirect-on-error.test.ts`:
- Around line 33-36: The test currently asserts raw encoded substrings on the
url string (e.g., checking 'error_description=User+denied'), which is brittle;
update the assertions in redirect-on-error.test.ts to parse the redirected URL
(const parsed = new URL(url)) and assert query values via
parsed.searchParams.get('error') === 'access_denied',
parsed.searchParams.get('error_description') === 'User denied', and
parsed.searchParams.get('state') === 'abc', and assert the origin+pathname
(parsed.origin + parsed.pathname) contains the expected callback base instead of
matching encoded query fragments.
In `@oidc-ui/src/App.css`:
- Around line 15-25: The font-family declaration in App.css currently quotes
single-word family names which violates stylelint's font-family-name-quotes
rule; edit the font-family list in the declaration that includes 'Montserrat',
'Open Sans', and 'Helvetica Neue' to remove quotes only from single-word family
names (e.g., change 'Montserrat' to Montserrat) while keeping quotes for
multi-word names (e.g., 'Open Sans', 'Helvetica Neue'); apply the same fix to
the other occurrence referenced around lines 262-263 so both declarations
conform to the linter.
- Around line 1-5: Remove the legacy Tailwind v2/v3 directives from App.css:
keep the single v4 entry point `@import` "tailwindcss"; and delete the three lines
containing `@tailwind` base, `@tailwind` components, and `@tailwind` utilities so the
file uses only the Tailwind v4 import.
In `@oidc-ui/src/App.tsx`:
- Around line 74-80: The current onLanguageChanged useEffect only updates an
inner container via setDir, which can leave UI outside that subtree with
incorrect direction; after computing the direction (using
getLocaleConfiguration(), rtlLanguages, and i18n.language inside
onLanguageChanged), also apply it to the document root (e.g., set
document.documentElement.dir and/or document.body.dir to the computed 'rtl' or
'ltr') so the entire page respects the selected text direction; keep setDir for
the local wrapper but ensure the global document dir is updated as well.
- Around line 65-67: The catch block sets status to LoadingStates.ERROR but the
render only checks LoadingStates.LOADING, so update the App component render
logic to explicitly handle LoadingStates.ERROR (e.g., return an error/failure UI
or early return) instead of falling through to the normal shell; locate the
initialization catch that calls setStatus(LoadingStates.ERROR) and modify the
render branch that currently tests for LoadingStates.LOADING (and similar logic
around lines where LoadingStates is used, e.g., the JSX block covering lines
~91-111) to detect LoadingStates.ERROR and render an appropriate error
message/placeholder or stop rendering the main layout.
In `@oidc-ui/src/constants/public-assets.ts`:
- Around line 21-31: The CSS_IMAGE_VARIABLES mapping currently assigns
images/top_left_bg_logo.svg to both '--bottom-left-bg-logo-url' and
'--top-right-bg-logo-url' which looks like a copy/paste; inspect the assets and
decide whether these should reuse top_left_bg_logo.svg or reference distinct
files, and then update CSS_IMAGE_VARIABLES accordingly (either confirm and leave
as-is, or change the values for the keys '--bottom-left-bg-logo-url' and
'--top-right-bg-logo-url' to images/bottom_left_bg_logo.svg and
images/top_right_bg_logo.svg respectively) so the constant accurately reflects
the intended assets.
In `@oidc-ui/src/i18n.ts`:
- Around line 13-15: The code currently disables i18next interpolation escaping
globally by setting interpolation.escapeValue = false, which opens an XSS risk
when translation strings contain HTML like <strong>{{clientName}}</strong>;
revert this global setting so escaping is enabled by default (remove or set
interpolation.escapeValue = true in i18n.ts), then remove raw HTML from
translation strings or use safe, explicit mechanisms for deliberate HTML
injection (e.g., the i18next Trans component or localized strings that return
sanitized React nodes) and, if you must bypass escaping for a specific call,
disable escaping only for that call via the t(...) options or Trans props rather
than via the global interpolation.escapeValue flag.
In `@oidc-ui/src/main.tsx`:
- Line 11: The current definition of baseUrl (const baseUrl =
import.meta.env.VITE_API_URL || 'https://localhost:8090') silently falls back to
localhost; replace this with a strict check that fails fast when
import.meta.env.VITE_API_URL is missing: read import.meta.env.VITE_API_URL into
a variable (e.g., baseUrlRaw), validate it's set and non-empty, and if not throw
a clear Error (or call a startup-fatal logger) so the app won't silently point
auth traffic at localhost; update any usages to reference the validated baseUrl
variable instead.
In `@oidc-ui/src/pages/EsignetDetailsPage.tsx`:
- Around line 74-77: The onKeyDown handler for the detail row (the inline
function that checks e.key and calls openWellKnownEndpoint(detail.value)) must
call e.preventDefault() when the Space key is pressed to avoid browser
scrolling; update the handler so that when e.key === ' ' (or 'Space' if you
standardize) you first call e.preventDefault() and then call
openWellKnownEndpoint(detail.value) (you may also call preventDefault for Enter
if desired).
In `@oidc-ui/src/pages/LoginPage.tsx`:
- Around line 9-38: The current logic uses a non-reactive let searchParams
assigned inside one useEffect and referenced in another, so the validation
effect never re-runs; fix by consolidating into a single useEffect (or make
searchParams state via useState) that runs on mount, creates const searchParams
= new URLSearchParams(window.location.search), then performs the REQUIRED_PARAMS
check and calls setIsLoading(false) or navigate("/something-went-wrong", {
state: { code: 401 }, replace: true }) accordingly; update references to
searchParams, useEffect, REQUIRED_PARAMS, setIsLoading, and navigate in the same
effect so the validation runs reliably on mount.
In `@oidc-ui/src/pages/SomethingWrongPage.tsx`:
- Around line 18-26: Replace the non-semantic divs in SomethingWrongPage (the
JSX that renders the error header and detail) with semantic elements: change the
element that uses className "error-page-header" to an <h1> and the one with
"error-page-detail" to a <p>, keeping the existing className and t(...)
translation calls intact so markup and text remain the same but provide proper
document structure for screen readers.
- Line 16: Update the image alt text in the SomethingWrongPage component to a
natural-language phrase (e.g., "Something went wrong" or "An error occurred")
instead of the underscore-separated string; locate the img element with
alt="something_went_wrong" in SomethingWrongPage.tsx and replace the alt
attribute value with a readable sentence to improve screen-reader accessibility.
- Around line 8-9: The inferred type of statusCode is ambiguous because the
fallback is the string 'unknown'; explicitly annotate statusCode as a union
(e.g. number | 'unknown') and keep the current cast of location.state as {
code?: number } | null so the expression becomes clearly typed; locate the
declaration of statusCode and add the explicit type annotation to remove the
implicit number|string union.
In `@oidc-ui/src/routes/AppRouter.tsx`:
- Around line 102-110: The Detector component is currently a no-op (logs and
returns null); either remove it to save bundle size or implement offline
navigation: update the Detector's render prop in AppRouter.tsx to inspect the
detector's online status (the render callback passed to Detector) and call
react-router's useNavigate (or the router navigation method you use) to redirect
to the NETWORK_ERROR route when offline, and navigate back (or restore prior
route) when online; ensure you reference the existing pollingConfig for options
and import/use the NETWORK_ERROR route constant and useNavigate inside the
component so offline/online transitions perform the intended navigation rather
than just console.log.
- Line 109: Remove the stray debug logging in the Route render callback: delete
the console.log("render working") call inside the anonymous render function (the
render={() => { ... }} used in AppRouter) and keep the function returning null
so the route continues to render nothing when online.
- Line 60: The code references location.pathname in the AppRouter component when
building backgroundLogoDiv but never defines location; import and call the
useLocation hook from react-router-dom inside the AppRouter component (e.g.,
const location = useLocation()) and replace the undefined reference so
backgroundLogoDiv uses the hook-provided location; ensure the import for
useLocation is added to the module imports and that
checkRoute(location.pathname) is invoked after the hook call.
In `@oidc-ui/src/services/api.service.ts`:
- Around line 33-36: The current logic caches a CSRF token in sessionStorage
forever (csrfToken) which fails if the server rotates tokens; update the request
flow that uses getCsrfToken to handle token refresh on auth failures by
detecting a 403 response for POSTs, clearing
sessionStorage.getItem('csrfToken'), calling getCsrfToken() to obtain a fresh
token, storing it back in sessionStorage, and retrying the original request
once; ensure this retry logic lives near the code that reads/stores csrfToken
(the getCsrfToken usage) and only retries once to avoid loops.
- Around line 19-22: getCsrfToken currently calls axios.get(csrfEndpoint)
without sending cookies, so if the CSRF endpoint relies on auth cookies the
token will be missing; update getCsrfToken to send credentials (e.g., pass {
withCredentials: true } to the axios.get call) or, if safe from
circular-dependency issues, call the existing ApiService request helper instead
(use the same credentials/session behavior as other API calls) so that the CSRF
fetch includes cookies and returns a valid token.
- Around line 49-62: The response interceptor in setupResponseInterceptor
currently calls navigate(...) when a status >= 400 but does not return or
reject, causing an undefined return and breaking Axios' promise contract; update
the error handler in ApiService.interceptors.response.use (the second argument)
so that after calling navigate(SOMETHING_WENT_WRONG, { state: { code: status }
}) it still returns Promise.reject(error) (or rejects with a normalized Error)
in all cases (i.e., ensure both the navigate path and the non-status path
return/reject the promise) so callers receive a rejected promise.
- Around line 30-43: The request interceptor attached via
ApiService.interceptors.request.use currently awaits getCsrfToken() without
handling exceptions; wrap the await call in a try-catch inside the async config
handler (the function passed to ApiService.interceptors.request.use) so network
or 404 errors don't bubble and break all POST requests—on catch, log the error
(e.g., using console.error or a logger), optionally clear or leave
sessionStorage.getItem('csrfToken') unset, and either set a safe fallback header
value (e.g., omit X-XSRF-TOKEN or set it to an empty string) or explicitly
reject the request by returning Promise.reject(error) to ensure predictable
behavior; keep references to getCsrfToken, sessionStorage.getItem/setItem, and
the config.headers['X-XSRF-TOKEN'] update when implementing the fix.
In `@oidc-ui/src/types/index.ts`:
- Around line 1-12: The index signature `[key: string]: boolean` on ThemeConfig
makes the named properties (otp_info_icon, biometrics_info_icon, pin_info_icon,
username_info_icon, background_logo, footer, remove_language_indicator_pipe,
outline_toggle, outline_dropdown) effectively optional to TypeScript; remove the
index signature from ThemeConfig to ensure those named properties are required,
and if you still need arbitrary keys create a separate type (e.g., ThemeExtras =
Record<string, boolean>) and combine them where necessary (e.g., type FullTheme
= ThemeConfig & ThemeExtras) or alternatively make the known properties optional
explicitly or add runtime validation depending on intended design.
In `@oidc-ui/src/utils/encoding.ts`:
- Around line 4-12: The encodeBase64Url function currently spreads the
TextEncoder output which can overflow the call stack for large inputs; replace
the spread usage in String.fromCharCode(...new TextEncoder().encode(str)) with a
safe conversion such as Array.from(new TextEncoder().encode(str), byte =>
String.fromCharCode(byte)).join('') (or an equivalent chunking/loop approach) so
the byte array is converted to a string without using the spread operator,
preserving the subsequent btoa and URL-safe replacements.
- Around line 26-36: The decodeBase64Url function currently converts base64url
to base64 then uses atob + percent-encoding + decodeURIComponent to produce
UTF-8; replace the percent-encoding roundtrip with a TextDecoder like
decodeBase64 does: after converting to standard base64 (and adding any necessary
'=' padding), call atob to get binary, create a Uint8Array from binary char
codes, and decode it with new TextDecoder('utf-8'). Update decodeBase64Url to
mirror decodeBase64's byte->string decoding approach (keep the
base64url-to-base64 replacement and padding logic, but use TextDecoder instead
of Array.map + decodeURIComponent).
In `@oidc-ui/src/utils/hashing.ts`:
- Around line 5-18: In sha256Base64Url, avoid spreading the Uint8Array into
String.fromCharCode and instead convert the buffer to a normal array via
Array.from(hashArray) before calling String.fromCharCode to be consistent and
safe; also replace the padding removal logic using .split('=')[0] with
.replace(/=+$/,'') to clearly and correctly strip base64 padding.
In `@oidc-ui/src/utils/parsing.ts`:
- Around line 32-36: The DEV fallback for POLLING_URL is missing the
"/v1/esignet" prefix, causing url (const url) to resolve to "/actuator/health"
when import.meta.env.VITE_ESIGNET_API_URL is undefined; update the DEV branch
that builds the url (the expression using import.meta.env.DEV and
import.meta.env.VITE_ESIGNET_API_URL) to include the "/v1/esignet" prefix (e.g.,
default to "/v1/esignet" + "/actuator/health") so DEV and PROD use the same base
path, or alternatively ensure VITE_ESIGNET_API_URL is set to "/v1/esignet" in
dev env — change the code that constructs url (referenced by POLLING_URL and
VITE_ESIGNET_API_URL) accordingly.
In `@oidc-ui/src/utils/redirect-on-error.ts`:
- Around line 24-34: The redirectUri taken from oauthDetails is used directly in
window.location.replace, allowing open-redirect attacks; validate its origin
before using it by parsing redirectUri (e.g., new URL(redirectUri)) and
comparing redirectUrl.origin against a whitelist of allowedOrigins or against
the stored original OAuth request's redirect_uri, and if it does not match log
an error and return instead of calling window.location.replace; keep the
existing params construction (URLSearchParams, error, error_description, state)
but only perform the replace when origin validation of redirectUri passes.
In `@oidc-ui/src/vite-env.d.ts`:
- Around line 3-15: The EnvConfig interface declares a typo: rename the property
DEFAULT_FEVICON to DEFAULT_FAVICON in the EnvConfig type and update all runtime
references to the old name (search for DEFAULT_FEVICON, usage in config loaders,
environment mapping, and any code accessing process.env or import of EnvConfig)
so types and runtime keys remain consistent; ensure optional/required semantics
match the original and run type checks to catch remaining references.
In `@oidc-ui/tailwind.config.js`:
- Around line 1-9: The Tailwind config is still using v3-style properties (the
content array and CommonJS plugin require) and an unmaintained plugin; update
module.exports to remove the content array so v4 automatic content detection is
used, replace any custom content paths by adding `@source` directives in your CSS
sources, and replace the unreliable plugin referenced as
require('tailwindcss-dir') in the plugins array with a v4-compatible RTL/LTR
plugin (e.g., `@vltansky/tailwindcss-rtl-first`) or remove it if you adopt
CSS-based direction handling; also consolidate CSS usage so you’re consistently
using v4 syntax (clean up mixed `@import` / `@tailwind` directives) and run npx
`@tailwindcss/upgrade` to assist the migration.
🪄 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 UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c0063189-fe75-4050-8dc3-71f37cb4eaab
⛔ Files ignored due to path filters (57)
oidc-ui/package-lock.jsonis excluded by!**/package-lock.jsonoidc-ui/public/favicon.icois excluded by!**/*.icooidc-ui/public/favicon.svgis excluded by!**/*.svgoidc-ui/public/icons.svgis excluded by!**/*.svgoidc-ui/public/images/Sign in with Inji.pngis excluded by!**/*.pngoidc-ui/public/images/Sign in with Iris.pngis excluded by!**/*.pngoidc-ui/public/images/Sign in with face.pngis excluded by!**/*.pngoidc-ui/public/images/Sign in with fingerprint.pngis excluded by!**/*.pngoidc-ui/public/images/asterisk_icon.svgis excluded by!**/*.svgoidc-ui/public/images/bg_bottom_left.pngis excluded by!**/*.pngoidc-ui/public/images/bio_icon.svgis excluded by!**/*.svgoidc-ui/public/images/bottom_right_bg_logo.svgis excluded by!**/*.svgoidc-ui/public/images/brand_logo.pngis excluded by!**/*.pngoidc-ui/public/images/chevron_down.svgis excluded by!**/*.svgoidc-ui/public/images/cross_icon.svgis excluded by!**/*.svgoidc-ui/public/images/drop_down.pngis excluded by!**/*.pngoidc-ui/public/images/drop_down_icon.pngis excluded by!**/*.pngoidc-ui/public/images/edit_access_icon.pngis excluded by!**/*.pngoidc-ui/public/images/email_icon.svgis excluded by!**/*.svgoidc-ui/public/images/error_icon.svgis excluded by!**/*.svgoidc-ui/public/images/face_capture.pngis excluded by!**/*.pngoidc-ui/public/images/fingerprint_scan.pngis excluded by!**/*.pngoidc-ui/public/images/footer_logo.pngis excluded by!**/*.pngoidc-ui/public/images/identity_icon.pngis excluded by!**/*.pngoidc-ui/public/images/illustration_one.pngis excluded by!**/*.pngoidc-ui/public/images/info_icon.svgis excluded by!**/*.svgoidc-ui/public/images/iris_code.pngis excluded by!**/*.pngoidc-ui/public/images/kbi_icon.svgis excluded by!**/*.svgoidc-ui/public/images/language_icon.pngis excluded by!**/*.pngoidc-ui/public/images/left_arrow_icon.svgis excluded by!**/*.svgoidc-ui/public/images/logo.pngis excluded by!**/*.pngoidc-ui/public/images/mobile_icon.svgis excluded by!**/*.svgoidc-ui/public/images/no_internet.svgis excluded by!**/*.svgoidc-ui/public/images/nrc_id_icon.svgis excluded by!**/*.svgoidc-ui/public/images/otp_icon.svgis excluded by!**/*.svgoidc-ui/public/images/otp_image.pngis excluded by!**/*.pngoidc-ui/public/images/password_hide.svgis excluded by!**/*.svgoidc-ui/public/images/password_show.svgis excluded by!**/*.svgoidc-ui/public/images/photo_scan.pngis excluded by!**/*.pngoidc-ui/public/images/pwd_icon.svgis excluded by!**/*.svgoidc-ui/public/images/qr_code.pngis excluded by!**/*.pngoidc-ui/public/images/refresh_logo.pngis excluded by!**/*.pngoidc-ui/public/images/section-bg.pngis excluded by!**/*.pngoidc-ui/public/images/select_icon.pngis excluded by!**/*.pngoidc-ui/public/images/sign_in_with_kbi.pngis excluded by!**/*.pngoidc-ui/public/images/sign_in_with_otp.pngis excluded by!**/*.pngoidc-ui/public/images/submit_bg.pngis excluded by!**/*.pngoidc-ui/public/images/sync_alt_black.svgis excluded by!**/*.svgoidc-ui/public/images/toggle_off.pngis excluded by!**/*.pngoidc-ui/public/images/toggle_on.pngis excluded by!**/*.pngoidc-ui/public/images/top_left_bg_logo.svgis excluded by!**/*.svgoidc-ui/public/images/under_construction.svgis excluded by!**/*.svgoidc-ui/public/images/up_down_arrow_icon.svgis excluded by!**/*.svgoidc-ui/public/images/vid_icon.svgis excluded by!**/*.svgoidc-ui/public/images/wallet_icon.svgis excluded by!**/*.svgoidc-ui/public/images/warning_message_icon.svgis excluded by!**/*.svgoidc-ui/public/logo.pngis excluded by!**/*.png
📒 Files selected for processing (66)
.agents/feature-doc/new_ui.mdoidc-ui/.envoidc-ui/.env.productionoidc-ui/.gitignoreoidc-ui/README.mdoidc-ui/eslint.config.jsoidc-ui/index.htmloidc-ui/package.jsonoidc-ui/public/env-config.jsoidc-ui/public/locales/ar.jsonoidc-ui/public/locales/default.jsonoidc-ui/public/locales/en.jsonoidc-ui/public/locales/hi.jsonoidc-ui/public/locales/km.jsonoidc-ui/public/locales/kn.jsonoidc-ui/public/locales/ta.jsonoidc-ui/public/theme-config.jsoidc-ui/public/theme/config.jsonoidc-ui/public/theme/variables.cssoidc-ui/src/App.cssoidc-ui/src/App.tsxoidc-ui/src/__tests__/components/Footer.test.tsxoidc-ui/src/__tests__/components/LoadingIndicator.test.tsxoidc-ui/src/__tests__/components/NavHeader.test.tsxoidc-ui/src/__tests__/pages/LoginPage.test.tsxoidc-ui/src/__tests__/pages/NetworkErrorPage.test.tsxoidc-ui/src/__tests__/pages/PageNotFoundPage.test.tsxoidc-ui/src/__tests__/pages/SomethingWrongPage.test.tsxoidc-ui/src/__tests__/routes/AppRouter.test.tsxoidc-ui/src/__tests__/services/config.service.test.tsoidc-ui/src/__tests__/services/css-variable.service.test.tsoidc-ui/src/__tests__/services/lang-config.service.test.tsoidc-ui/src/__tests__/utils/encoding.test.tsoidc-ui/src/__tests__/utils/hashing.test.tsoidc-ui/src/__tests__/utils/parsing.test.tsoidc-ui/src/__tests__/utils/redirect-on-error.test.tsoidc-ui/src/components/Footer.tsxoidc-ui/src/components/LoadingIndicator.tsxoidc-ui/src/components/NavHeader.tsxoidc-ui/src/constants/public-assets.tsoidc-ui/src/constants/routes.tsoidc-ui/src/constants/states.tsoidc-ui/src/i18n.tsoidc-ui/src/main.tsxoidc-ui/src/pages/EsignetDetailsPage.tsxoidc-ui/src/pages/LoginPage.tsxoidc-ui/src/pages/NetworkErrorPage.tsxoidc-ui/src/pages/PageNotFoundPage.tsxoidc-ui/src/pages/SomethingWrongPage.tsxoidc-ui/src/routes/AppRouter.tsxoidc-ui/src/services/api.service.tsoidc-ui/src/services/config.service.tsoidc-ui/src/services/css-variable.service.tsoidc-ui/src/services/lang-config.service.tsoidc-ui/src/test-setup.tsoidc-ui/src/types/index.tsoidc-ui/src/utils/encoding.tsoidc-ui/src/utils/hashing.tsoidc-ui/src/utils/parsing.tsoidc-ui/src/utils/redirect-on-error.tsoidc-ui/src/vite-env.d.tsoidc-ui/tailwind.config.jsoidc-ui/tsconfig.app.jsonoidc-ui/tsconfig.jsonoidc-ui/tsconfig.node.jsonoidc-ui/vite.config.ts
|
|
||
| ## Project Structure | ||
|
|
||
| ``` |
There was a problem hiding this comment.
Fix markdownlint structure issues (MD040, MD022, MD058).
Line 45 needs a fenced code language, and headings/tables around Lines 92–104 need required blank lines.
Suggested fix
-```
+```text
oidc-ui/
├── index.html # Entry HTML
...
└── __tests__/ # Test files mirroring src structure@@
Copy as-is (assets only)
public/images/— all image assets (~60 files: logos, icons, illustrations)public/theme/config.json— feature toggle configpublic/theme/variables.css— 100+ CSS custom properties for themingpublic/locales/— i18n JSON language filespublic/env-config.js— runtimewindow._env_configuration
Copy and adapt (CSS)
src/App.css— carry over, then clean up unused classes (consent, biometric, KBI form styles are not needed in this phase). Ensure all class references use the CSS variables fromvariables.css.- Fix horizontal scroll overflow: add
overflow-x: hiddento.section-backgroundandmax-width: 100%; box-sizing: border-box;to.multipurpose-login-cardto prevent the card and absolute-positioned background logo elements from causing horizontal page overflow.
Rewrite in TypeScript
| Old file (JS) | New file (TS) | Notes |
|---|---|---|
src/helpers/utils.js |
src/utils/encoding.ts, hashing.ts, parsing.ts |
Split by concern. Replace crypto-js SHA-256 with SubtleCrypto.digest(). Replace Buffer usage with btoa/atob or TextEncoder. |
</details>
Also applies to: 92-104
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 45-45: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/feature-doc/new_ui.md at line 45, The markdown in
.agents/feature-doc/new_ui.md has lint errors: add a fenced code language after
the opening at the code block around line 45 (e.g.,text) and ensure the
code block is properly closed, and insert required blank lines before the
headings "Copy as-is (assets only)", "Copy and adapt (CSS)" and "Rewrite in
TypeScript" and before the table that begins with "| Old file (JS) | New file
(TS) | Notes |" so MD022/MD040/MD058 are satisfied; locate the code block and
headings/table by their text content and update the fences and spacing
accordingly.
</details>
<!-- fingerprinting:phantom:triton:hawk -->
<!-- d98c2f50 -->
<!-- This is an auto-generated comment by CodeRabbit -->
| @@ -0,0 +1 @@ | |||
| VITE_API_URL=https://localhost:8090 No newline at end of file | |||
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Add trailing newline.
The file is missing a trailing newline. While not a functional issue, it's a POSIX text file convention and the linter flags it.
📝 Proposed fix
VITE_API_URL=https://localhost:8090
+📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| VITE_API_URL=https://localhost:8090 | |
| VITE_API_URL=https://localhost:8090 | |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 1-1: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@oidc-ui/.env` at line 1, Add a POSIX trailing newline to the .env entry by
ensuring the file ends with a newline character after the line
"VITE_API_URL=https://localhost:8090" so the linter stops flagging the missing
newline; simply append a newline at the end of that file.
| @@ -0,0 +1 @@ | |||
| VITE_API_URL=https://localhost:8090 No newline at end of file | |||
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Add trailing newline.
The file is missing a trailing newline. While not a functional issue, it's a POSIX text file convention and the linter flags it.
📝 Proposed fix
VITE_API_URL=https://localhost:8090
+📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| VITE_API_URL=https://localhost:8090 | |
| VITE_API_URL=https://localhost:8090 | |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 1-1: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@oidc-ui/.env.production` at line 1, The file ending for the environment
variable VITE_API_URL is missing a trailing newline; update the .env file that
defines VITE_API_URL so the final line ends with a newline character (i.e., add
a single line break after the line "VITE_API_URL=https://localhost:8090") to
satisfy POSIX text file conventions and the linter.
Production API URL points to localhost.
The production environment file configures VITE_API_URL to https://localhost:8090, which will fail when the application is deployed to production. This should point to the actual production API endpoint.
⚠️ Recommended fix
Replace localhost with the production API domain:
-VITE_API_URL=https://localhost:8090
+VITE_API_URL=https://api.production-domain.com📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| VITE_API_URL=https://localhost:8090 | |
| VITE_API_URL=https://api.production-domain.com |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 1-1: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@oidc-ui/.env.production` at line 1, The production environment variable
VITE_API_URL is incorrectly set to a localhost address; change the VITE_API_URL
value to the real production API endpoint (replace "https://localhost:8090" with
your production API domain/URL) so the deployed app points to the correct
backend, and ensure the scheme (http/https) and port match the production
service and any CI/CD or secrets store is updated accordingly.
| "i18next-browser-languagedetector": "^8.2.1", | ||
| "i18next-http-backend": "^4.0.0", | ||
| "jose": "^6.2.3", | ||
| "prop-types": "^15.8.1", |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Consider moving prop-types to devDependencies.
React 19 removed propTypes support for function components, and prop-types is typically only needed during development. Unless @asgardeo/react or another dependency requires it as a peer dependency, moving it to devDependencies will reduce production bundle size.
📦 Proposed change
- "prop-types": "^15.8.1",
"react": "^19.2.5",And in devDependencies:
"postcss": "^8.5.14",
"prettier": "^3.8.3",
+ "prop-types": "^15.8.1",
"tailwindcss": "^4.3.0",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@oidc-ui/package.json` at line 25, Move the "prop-types" package from
dependencies to devDependencies in package.json: remove the "prop-types":
"^15.8.1" entry from the root dependencies block and add the same entry under
devDependencies, unless a runtime dependency (e.g., `@asgardeo/react` or another
package) explicitly lists prop-types as a peerDependency — if so, leave it in
dependencies; otherwise keep it only in devDependencies to avoid shipping it in
production bundles.
Comment on lines
+203
to
+220
| "esignet_details_heading": "wellknown APIs", | ||
| "internal_api": "Internal API", | ||
| "wellknown_api": ".wellknown API" | ||
| }, | ||
| "header": { | ||
| "login_heading": "Login using eSignet", | ||
| "verify_heading": "Verify using eSignet", | ||
| "link_heading": "Link using eSignet", | ||
| "login_subheading": "<strong>{{clientName}}</strong> is requesting authentication for login", | ||
| "verify_subheading": "<strong>{{clientName}}</strong> is requesting authentication for verification", | ||
| "link_subheading": "<strong>{{clientName}}</strong> is requesting authentication to link your account", | ||
| "login_linkName": "Preregister", | ||
| "more_ways_to_sign_in": "More Ways to Login", | ||
| "invalid_auth_factor": "Invalid Auth Factors. Please Try Again...", | ||
| "download_now": "Download Now", | ||
| "logo_alt": "eSignet", | ||
| "backgroud_image_alt": "Sign in with eSignet", | ||
| "language": "Language", |
There was a problem hiding this comment.
Fix user-facing typos in English locale strings.
Examples include "wellknown APIs", "backgroud_image_alt", and "Authentiation" (Line 503). These are directly visible and should be corrected before release.
Also applies to: 503-503
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@oidc-ui/public/locales/en.json` around lines 203 - 220, Fix the user-facing
typos in the English locale: change the value for esignet_details_heading from
"wellknown APIs" to "well‑known APIs", change wellknown_api from ".wellknown
API" to ".well‑known API", rename the key backgroud_image_alt to
background_image_alt and correct its value to "Sign in with eSignet", and search
for any occurrences of the misspelled "Authentiation" and replace with
"Authentication" (update the corresponding locale key/value(s) such as
invalid_auth_factor to use proper casing and wording like "Invalid
authentication factors. Please try again...").
Comment on lines
+5
to
+18
| export async function sha256Base64Url(value: unknown): Promise<string> { | ||
| const data = new TextEncoder().encode(JSON.stringify(value)); | ||
| const hashBuffer = await crypto.subtle.digest('SHA-256', data); | ||
| const hashArray = new Uint8Array(hashBuffer); | ||
|
|
||
| // Convert to base64 | ||
| const base64 = btoa(String.fromCharCode(...hashArray)); | ||
|
|
||
| // Convert to base64url | ||
| return base64 | ||
| .split('=')[0] | ||
| .replace(/\+/g, '-') | ||
| .replace(/\//g, '_'); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Use Array.from for consistency and clarity.
Line 11 uses String.fromCharCode(...hashArray) which spreads the array. While this won't overflow for SHA-256 (fixed 32 bytes), it's inconsistent with best practices. Also, line 15 uses .split('=')[0] to remove padding, which is less clear than .replace(/=+$/, '').
♻️ Optional improvements
// Convert to base64
- const base64 = btoa(String.fromCharCode(...hashArray));
+ const base64 = btoa(
+ Array.from(hashArray, (byte) => String.fromCharCode(byte)).join(''),
+ );
// Convert to base64url
return base64
- .split('=')[0]
+ .replace(/=+$/, '')
.replace(/\+/g, '-')
.replace(/\//g, '_');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export async function sha256Base64Url(value: unknown): Promise<string> { | |
| const data = new TextEncoder().encode(JSON.stringify(value)); | |
| const hashBuffer = await crypto.subtle.digest('SHA-256', data); | |
| const hashArray = new Uint8Array(hashBuffer); | |
| // Convert to base64 | |
| const base64 = btoa(String.fromCharCode(...hashArray)); | |
| // Convert to base64url | |
| return base64 | |
| .split('=')[0] | |
| .replace(/\+/g, '-') | |
| .replace(/\//g, '_'); | |
| } | |
| export async function sha256Base64Url(value: unknown): Promise<string> { | |
| const data = new TextEncoder().encode(JSON.stringify(value)); | |
| const hashBuffer = await crypto.subtle.digest('SHA-256', data); | |
| const hashArray = new Uint8Array(hashBuffer); | |
| // Convert to base64 | |
| const base64 = btoa( | |
| Array.from(hashArray, (byte) => String.fromCharCode(byte)).join(''), | |
| ); | |
| // Convert to base64url | |
| return base64 | |
| .replace(/=+$/, '') | |
| .replace(/\+/g, '-') | |
| .replace(/\//g, '_'); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@oidc-ui/src/utils/hashing.ts` around lines 5 - 18, In sha256Base64Url, avoid
spreading the Uint8Array into String.fromCharCode and instead convert the buffer
to a normal array via Array.from(hashArray) before calling String.fromCharCode
to be consistent and safe; also replace the padding removal logic using
.split('=')[0] with .replace(/=+$/,'') to clearly and correctly strip base64
padding.
Comment on lines
+32
to
+36
| const url = | ||
| POLLING_URL ?? | ||
| (import.meta.env.DEV | ||
| ? `${import.meta.env.VITE_ESIGNET_API_URL ?? ''}/actuator/health` | ||
| : `${window.origin}/v1/esignet/actuator/health`); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the value of VITE_ESIGNET_API_URL in environment files
rg -n 'VITE_ESIGNET_API_URL' --type=env -g '.env*'Repository: mosip/esignet
Length of output: 85
🏁 Script executed:
#!/bin/bash
# Find all environment files
fd -t f '\.env' . | head -20Repository: mosip/esignet
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Search for VITE_ESIGNET_API_URL across the entire codebase
rg -n 'VITE_ESIGNET_API_URL' . --max-count=20Repository: mosip/esignet
Length of output: 358
🏁 Script executed:
#!/bin/bash
# Get context around lines 32-36 in parsing.ts
cat -n oidc-ui/src/utils/parsing.ts | sed -n '25,45p'Repository: mosip/esignet
Length of output: 936
🏁 Script executed:
#!/bin/bash
# Get context around api.service.ts lines 6-7
cat -n oidc-ui/src/services/api.service.ts | sed -n '1,20p'Repository: mosip/esignet
Length of output: 823
🏁 Script executed:
#!/bin/bash
# Search for documentation or comments about VITE_ESIGNET_API_URL
rg -B3 -A3 'VITE_ESIGNET_API_URL' oidc-ui/ --max-count=30Repository: mosip/esignet
Length of output: 1064
🏁 Script executed:
#!/bin/bash
# Check for environment setup documentation
find oidc-ui -type f \( -name 'README*' -o -name '.env*' -o -name 'docker*' \) | head -20Repository: mosip/esignet
Length of output: 112
🏁 Script executed:
#!/bin/bash
# Check .env file
cat -n oidc-ui/.envRepository: mosip/esignet
Length of output: 99
🏁 Script executed:
#!/bin/bash
# Check .env.production file
cat -n oidc-ui/.env.productionRepository: mosip/esignet
Length of output: 99
🏁 Script executed:
#!/bin/bash
# Check README for environment configuration documentation
cat -n oidc-ui/README.md | head -100Repository: mosip/esignet
Length of output: 3903
DEV polling endpoint missing /v1/esignet prefix.
The polling URL has an inconsistency between DEV and PROD:
- DEV:
${VITE_ESIGNET_API_URL ?? ''}/actuator/health→ becomes/actuator/healthwhen undefined - PROD:
${window.origin}/v1/esignet/actuator/health
Since VITE_ESIGNET_API_URL is not defined in .env or .env.production, the DEV endpoint resolves to /actuator/health, missing the /v1/esignet prefix that PROD includes. The README documents the expected default as /v1/esignet/actuator/health. Either define VITE_ESIGNET_API_URL=/v1/esignet in the dev environment or hardcode the prefix in the DEV path to match PROD.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@oidc-ui/src/utils/parsing.ts` around lines 32 - 36, The DEV fallback for
POLLING_URL is missing the "/v1/esignet" prefix, causing url (const url) to
resolve to "/actuator/health" when import.meta.env.VITE_ESIGNET_API_URL is
undefined; update the DEV branch that builds the url (the expression using
import.meta.env.DEV and import.meta.env.VITE_ESIGNET_API_URL) to include the
"/v1/esignet" prefix (e.g., default to "/v1/esignet" + "/actuator/health") so
DEV and PROD use the same base path, or alternatively ensure
VITE_ESIGNET_API_URL is set to "/v1/esignet" in dev env — change the code that
constructs url (referenced by POLLING_URL and VITE_ESIGNET_API_URL) accordingly.
Comment on lines
+24
to
+34
|
|
||
| const redirectUri = oauthDetails.redirectUri; | ||
| if (!redirectUri) return; | ||
|
|
||
| const params = new URLSearchParams(); | ||
| if (errorDescription) params.set('error_description', errorDescription); | ||
| params.set('state', state); | ||
| params.set('error', errorCode); | ||
|
|
||
| window.onbeforeunload = null; | ||
| window.location.replace(`${redirectUri}?${params.toString()}`); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for redirect_uri validation patterns in the codebase
rg -n 'redirect.*[Uu]ri|redirect.*[Vv]alidat' --type=ts -C3Repository: mosip/esignet
Length of output: 3891
🏁 Script executed:
#!/bin/bash
# First, read the complete redirect-on-error.ts file
cat -n oidc-ui/src/utils/redirect-on-error.tsRepository: mosip/esignet
Length of output: 1334
🏁 Script executed:
#!/bin/bash
# Check the test file to understand expected behavior and security model
cat -n oidc-ui/src/__tests__/utils/redirect-on-error.test.tsRepository: mosip/esignet
Length of output: 2414
🏁 Script executed:
#!/bin/bash
# Search for where the hash/oauthDetails is set or populated
rg -n 'location\.hash|window\.location\.hash|oauthDetails' --type=ts --type=tsx -B2 -A2Repository: mosip/esignet
Length of output: 85
🏁 Script executed:
#!/bin/bash
# Search for any allowlist, whitelist, or origin validation patterns
rg -n 'allowlist|whitelist|allowOrigin|validOrigin|originCheck' --type=ts --type=tsx -iRepository: mosip/esignet
Length of output: 85
🏁 Script executed:
#!/bin/bash
# Search for server-side redirect_uri validation in the codebase
rg -n 'redirect_uri' --type=ts --type=tsx --type=js -B2 -A2 | head -100Repository: mosip/esignet
Length of output: 85
🏁 Script executed:
#!/bin/bash
# Find where redirectOnError is called
rg -n 'redirectOnError' --type=ts -B3 -A3Repository: mosip/esignet
Length of output: 3754
🏁 Script executed:
#!/bin/bash
# Search for OIDC initialization or flow setup
rg -n 'oauth|oidc|redirect.*callback' --type=ts -i | head -50Repository: mosip/esignet
Length of output: 714
🏁 Script executed:
#!/bin/bash
# Look for any server-side redirect_uri validation or whitelist
find . -type f \( -name '*.java' -o -name '*.py' -o -name '*.js' -o -name '*.ts' \) -exec grep -l 'redirect_uri\|redirectUri' {} \; | head -20Repository: mosip/esignet
Length of output: 201
🏁 Script executed:
#!/bin/bash
# Search for imports of redirectOnError more broadly
rg -n 'import.*redirectOnError|from.*redirect-on-error' -C2Repository: mosip/esignet
Length of output: 457
🏁 Script executed:
#!/bin/bash
# Look at the oidc-ui directory structure to find components using this
find oidc-ui/src -type f -name '*.ts' -o -name '*.tsx' | head -30Repository: mosip/esignet
Length of output: 1340
🏁 Script executed:
#!/bin/bash
# Search for any file that uses this function
rg -n 'redirectOnError\(' --type=tsRepository: mosip/esignet
Length of output: 519
🏁 Script executed:
#!/bin/bash
# Check the Java util file for redirect_uri handling
cat oidc-ui/src/utils/EsignetUtil.java 2>/dev/null || cat ui-test/src/main/java/utils/EsignetUtil.javaRepository: mosip/esignet
Length of output: 36268
🏁 Script executed:
#!/bin/bash
# Search for error handling that might call redirectOnError or use its pattern
rg -n 'error.*fatal|fatal.*error|window\.location|error.*handling' --type=ts -i | head -40Repository: mosip/esignet
Length of output: 1702
🏁 Script executed:
#!/bin/bash
# Check if there are any app-level error handlers or boundary components
find oidc-ui/src -name '*error*' -o -name '*boundary*' -o -name '*catch*' 2>/dev/nullRepository: mosip/esignet
Length of output: 150
Add validation to prevent open redirect attacks through arbitrary redirectUri values.
The redirectUri extracted from the URL hash (line 25) lacks validation, allowing attackers to craft malicious URLs redirecting users to phishing sites. Implement origin validation to enforce only trusted redirect destinations:
Suggested implementation
const allowedOrigins = [/* list of trusted origins */];
const redirectUrl = new URL(redirectUri);
if (!allowedOrigins.includes(redirectUrl.origin)) {
console.error('Invalid redirect URI origin');
return;
}Alternatively, verify against the original OAuth request's redirect_uri if stored.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@oidc-ui/src/utils/redirect-on-error.ts` around lines 24 - 34, The redirectUri
taken from oauthDetails is used directly in window.location.replace, allowing
open-redirect attacks; validate its origin before using it by parsing
redirectUri (e.g., new URL(redirectUri)) and comparing redirectUrl.origin
against a whitelist of allowedOrigins or against the stored original OAuth
request's redirect_uri, and if it does not match log an error and return instead
of calling window.location.replace; keep the existing params construction
(URLSearchParams, error, error_description, state) but only perform the replace
when origin validation of redirectUri passes.
Comment on lines
+3
to
+15
| interface EnvConfig { | ||
| DEFAULT_LANG: string; | ||
| DEFAULT_WELLKNOWN: string; | ||
| DEFAULT_THEME: string; | ||
| DEFAULT_FEVICON: string; | ||
| DEFAULT_TITLE: string; | ||
| DEFAULT_ID_PROVIDER_NAME: string; | ||
| DEFAULT_FONT_URL: string; | ||
| POLLING_URL?: string; | ||
| POLLING_INTERVAL?: string; | ||
| POLLING_TIMEOUT?: string; | ||
| POLLING_ENABLED?: string; | ||
| } |
There was a problem hiding this comment.
Fix typo: FEVICON should be FAVICON.
Line 7 has a typo: DEFAULT_FEVICON should be DEFAULT_FAVICON (favicon = favorite icon).
Since this is a type declaration, ensure that any runtime code using this property is updated simultaneously to maintain consistency.
📝 Suggested fix
DEFAULT_THEME: string;
- DEFAULT_FEVICON: string;
+ DEFAULT_FAVICON: string;
DEFAULT_TITLE: string;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| interface EnvConfig { | |
| DEFAULT_LANG: string; | |
| DEFAULT_WELLKNOWN: string; | |
| DEFAULT_THEME: string; | |
| DEFAULT_FEVICON: string; | |
| DEFAULT_TITLE: string; | |
| DEFAULT_ID_PROVIDER_NAME: string; | |
| DEFAULT_FONT_URL: string; | |
| POLLING_URL?: string; | |
| POLLING_INTERVAL?: string; | |
| POLLING_TIMEOUT?: string; | |
| POLLING_ENABLED?: string; | |
| } | |
| interface EnvConfig { | |
| DEFAULT_LANG: string; | |
| DEFAULT_WELLKNOWN: string; | |
| DEFAULT_THEME: string; | |
| DEFAULT_FAVICON: string; | |
| DEFAULT_TITLE: string; | |
| DEFAULT_ID_PROVIDER_NAME: string; | |
| DEFAULT_FONT_URL: string; | |
| POLLING_URL?: string; | |
| POLLING_INTERVAL?: string; | |
| POLLING_TIMEOUT?: string; | |
| POLLING_ENABLED?: string; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@oidc-ui/src/vite-env.d.ts` around lines 3 - 15, The EnvConfig interface
declares a typo: rename the property DEFAULT_FEVICON to DEFAULT_FAVICON in the
EnvConfig type and update all runtime references to the old name (search for
DEFAULT_FEVICON, usage in config loaders, environment mapping, and any code
accessing process.env or import of EnvConfig) so types and runtime keys remain
consistent; ensure optional/required semantics match the original and run type
checks to catch remaining references.
Comment on lines
+1
to
+9
| module.exports = { | ||
| content: [ | ||
| "./src/**/*.{js,jsx,ts,tsx}", | ||
| ], | ||
| theme: { | ||
| extend: {}, | ||
| }, | ||
| plugins: [require('tailwindcss-dir')()], | ||
| } No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check installed Tailwind version and look for v4 migration guidance
# Check package.json for tailwindcss version
echo "=== Installed Tailwind CSS version ==="
cat oidc-ui/package.json | jq '.dependencies.tailwindcss, .devDependencies.tailwindcss' 2>/dev/null || \
rg -A 1 '"tailwindcss"' oidc-ui/package.json
# Check if there's a CSS file with v4 `@import` or `@theme` directives
echo -e "\n=== Checking for v4 CSS syntax ==="
rg -n '@import "tailwindcss"|@theme|@source' oidc-ui/src/
# Check for v3-style `@tailwind` directives
echo -e "\n=== Checking for v3-style `@tailwind` directives ==="
rg -n '@tailwind (base|components|utilities)' oidc-ui/
# Verify tailwindcss-dir plugin compatibility
echo -e "\n=== Checking tailwindcss-dir plugin usage ==="
rg -n 'tailwindcss-dir' oidc-ui/Repository: mosip/esignet
Length of output: 487
🌐 Web query:
Tailwind CSS v4 backward compatibility v3 config content array
💡 Result:
Tailwind CSS v4 is not backward compatible with the v3 content array in tailwind.config.js. In v3, the content option specifies paths for Tailwind to scan for class names [1][2]. In v4, content detection is automatic using heuristics, eliminating the need for the content array, which should be removed from the config [3][4][5][6][7]. The content, corePlugins, safelist, and separator options from JS configs are unsupported in v4 [8][9]. For custom paths not auto-detected (e.g., external libs), use @source directives in CSS: @source "./custom/path//*.js"; [3][6][7]. For safelisting classes (v4.1+), use @source inline("class-name"); or brace expansion like @source inline("{hover:,}bg-red-{50..950}"); [10][11][12][7]. Legacy JS configs can be loaded via @config "path/to/tailwind.config.js"; but unsupported options like content are ignored [9].
Citations:
- 1: https://v3.tailwindcss.com/docs/content-configuration
- 2: https://v3.tailwindcss.com/docs/configuration
- 3: https://mintlify.com/tailwindlabs/tailwindcss/upgrading/v3-to-v4
- 4: https://dev.to/whoffagents/tailwind-css-v4-what-actually-changed-and-how-to-migrate-without-breaking-everything-4f6h
- 5: https://dev.to/malahim_haseeb_981126d794/tailwind-css-v4-what-actually-changed-and-what-it-means-for-your-nextjs-project-472f
- 6: https://tailwindcss.com/blog/tailwindcss-v4
- 7: https://tailwindcss.com/docs/content-configuration
- 8: https://tailwindcss.com/docs/upgrade-guide
- 9: Version 4 (v4) Upgrade Guide does not mention safelist tailwindlabs/tailwindcss.com#2015
- 10: https://blakejones.dev/blog/how-to-make-a-safelist-in-tailwind-v4.1%2B
- 11: Safelist in V4 tailwindlabs/tailwindcss#15291
- 12: https://stackoverflow.com/questions/79323991/how-is-it-possible-to-specify-a-safelist-in-tailwindcss-v4-is-it-possible-to-li
🌐 Web query:
Tailwind CSS v4 breaking changes v3 config still supported
💡 Result:
Tailwind CSS v4 introduces several breaking changes from v3, primarily shifting to a CSS-first configuration model using directives like @theme, @utility, and @variant, replacing the traditional tailwind.config.js file[1][2][3]. Key breaking changes include: removal of @tailwind directives (replaced by @import "tailwindcss;")[1], renamed utilities (e.g., shadow-sm to shadow-xs, ring default width from 3px to 1px, border color to currentColor)[1][4], updated scales for shadows/radius/blur, changes to container/prefix/important modifiers, and browser targeting modern versions (Safari 16.4+, Chrome 111+, Firefox 128+)[1]. Preflight base styles also changed (e.g., dialog margins removed)[1]. v3 config files (tailwind.config.js) are still supported via the @config directive for backward compatibility during migration: @import "tailwindcss;"; @config "./tailwind.config.js;"[1][5][6][3]. However, not all v3 config options are fully supported (e.g., corePlugins, safelist, separator ignored or limited)[1][6][3], and CSS-defined values take precedence[3]. This allows incremental migration without immediate full rewrite, but the recommended long-term approach is migrating to CSS config[7][8]. Use npx @tailwindcss/upgrade for automated migration handling most changes[9].
Citations:
- 1: https://tailwindcss.com/docs/upgrade-guide
- 2: https://tailwindcss.com/blog/tailwindcss-v4
- 3: https://tailwindcss.com/docs/functions-and-directives
- 4: https://thesyntaxdiaries.com/tailwind-css-v4-migration-breaking-changes
- 5: Support loading config files via
@configtailwindlabs/tailwindcss#14239 - 6: https://v3.tailwindcss.com/docs/v4-beta
- 7: https://ui.spectrumhq.in/blog/tailwind-css-v4-migration-guide
- 8: Upgrading to Tailwind v4: Missing Defaults, Broken Dark Mode, and Config Issues tailwindlabs/tailwindcss#16517
- 9: https://mintlify.com/tailwindlabs/tailwindcss/upgrading/v3-to-v4
🌐 Web query:
tailwindcss-dir plugin Tailwind v4 compatibility
💡 Result:
Tailwindcss-dir (Ron Melkhior’s “Tailwind Direction” plugin) is not Tailwind CSS v4-specific, and there is no evidence it explicitly supports Tailwind v4. What we can verify from the sources: 1) The npm package “tailwindcss-dir” last released v4.0.0 in 2019, and its GitHub repo shows the latest release was v4.0.0 (2019-09-08) with the repo’s last push around 2020-10-28; this suggests it’s effectively unmaintained rather than actively updated for Tailwind v4 changes. [1][2] 2) The plugin’s README says it “requires Tailwind >= v1.0.1” and describes adding custom “ltr” and “rtl” variants via the Tailwind config plugins section. [1][2] 3) Tailwind CSS v4 has architectural/processing changes (e.g., it uses modern browser-targeted tooling and requires using its v4 integration pieces like @tailwindcss/postcss when using PostCSS), so third-party plugins that depend on older Tailwind internals may break even if they technically “load.” [3][4] Practical conclusion for “Tailwind v4 compatibility”: - Likely: it may work only if Tailwind v4 still supports the plugin mechanism used by tailwindcss-dir (adding variants). But there’s no authoritative statement in the sources confirming Tailwind v4 compatibility. - If you need RTL/LTR in Tailwind v4 reliably, prefer a plugin with explicit “works with Tailwind v4” messaging. For example, “@vltansky/tailwindcss-rtl-first” explicitly claims compatibility with Tailwind v4. [5] If you want a quick validation checklist for your project: - Ensure you’re using Tailwind v4 correctly for your build (for PostCSS setups, Tailwind v4 moved to the separate @tailwindcss/postcss package). [6] - Add the plugin in Tailwind’s plugins list and confirm the generated utilities include the ltr:/rtl: variants (build and inspect the output CSS). - If it fails, you’ll typically need a newer direction plugin or to use Tailwind v4’s supported approaches rather than ltr:/rtl: variants from an older plugin (since there’s no confirmed v4 support here). [1][3][4] Sources: [1][2][3][4][6][5]
Citations:
- 1: https://registry.npmjs.org/tailwindcss-dir
- 2: https://github.com/RonMelkhior/tailwindcss-dir
- 3: https://tailwindcss.com/docs/compatibility
- 4: https://tailwindcss.com/blog/tailwindcss-v4
- 5: https://github.com/vltansky/tailwindcss-rtl-first
- 6: https://tailkits.com/blog/tailwind-rtl-not-working/
Migrate Tailwind CSS configuration to v4 format.
The current configuration uses Tailwind v3 syntax (content array, theme.extend, CommonJS require() for plugins) with Tailwind v4.3.0 installed. While Tailwind v4 maintains backward compatibility for legacy JS configs via the @config directive, this approach is transitional and the v3 content array is unsupported and will be ignored in v4.
More critically, the tailwindcss-dir plugin (last maintained in 2019) has no explicit Tailwind v4 compatibility confirmation and the repository appears unmaintained. This plugin's functionality with v4 is uncertain and may cause issues.
Recommended actions:
- Remove the
contentarray and use v4's automatic content detection (content paths auto-detected via heuristics) - For custom paths not auto-detected, use
@sourcedirectives in CSS:@source "./custom/path/*.js"; - Verify or replace
tailwindcss-dirwith a v4-compatible RTL/LTR solution (e.g.,@vltansky/tailwindcss-rtl-firstexplicitly supports v4) - Consolidate CSS syntax (currently mixing v4
@import "tailwindcss"with v3@tailwinddirectives; the v3 directives can remain during migration but should be cleaned up) - Use
npx@tailwindcss/upgrade`` for automated migration assistance
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@oidc-ui/tailwind.config.js` around lines 1 - 9, The Tailwind config is still
using v3-style properties (the content array and CommonJS plugin require) and an
unmaintained plugin; update module.exports to remove the content array so v4
automatic content detection is used, replace any custom content paths by adding
`@source` directives in your CSS sources, and replace the unreliable plugin
referenced as require('tailwindcss-dir') in the plugins array with a
v4-compatible RTL/LTR plugin (e.g., `@vltansky/tailwindcss-rtl-first`) or remove
it if you adopt CSS-based direction handling; also consolidate CSS usage so
you’re consistently using v4 syntax (clean up mixed `@import` / `@tailwind`
directives) and run npx `@tailwindcss/upgrade` to assist the migration.
15 tasks
Contributor
There was a problem hiding this comment.
Remove this feature-doc
anushasunkada
changed the title
Signed-off-by: Zeeshan Mehboob <zeeshan.mehboob@infosys.com>
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (9)
oidc-ui/public/theme-config.js (1)
2-4:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winGuard empty theme tokens before
classList.add.
DEFAULT_THEMEdefaults to an empty string, andclassList.add('')throws aSyntaxErrorper HTML spec, which breaks bootstrap at load time.Suggested fix
if (body) { - body.classList.add(window._env_.DEFAULT_THEME); + if (window._env_.DEFAULT_THEME) { + body.classList.add(window._env_.DEFAULT_THEME); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@oidc-ui/public/theme-config.js` around lines 2 - 4, The code calls body.classList.add(window._env_.DEFAULT_THEME) which can pass an empty string and throw a SyntaxError; update the body/theme application logic (the block that references body, window._env_.DEFAULT_THEME, and classList.add) to first verify body exists and that DEFAULT_THEME is a non-empty trimmed string (e.g., typeof check and .trim() length > 0) before calling classList.add so you never call classList.add with an empty token.oidc-ui/src/__tests__/services/config.service.test.ts (1)
24-30:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAssert the exact request URL.
The test verifies call count and returned data but doesn't check which URL
axios.getwas invoked with. Add an assertion to confirm the correct endpoint is used.✅ Suggested assertion
it('fetches theme config from the correct URL', async () => { vi.mocked(axios.get).mockResolvedValue({ data: mockConfig }); const result = await fetchThemeConfig(); expect(axios.get).toHaveBeenCalledTimes(1); + expect(axios.get).toHaveBeenCalledWith(expect.stringContaining('/theme/config.json')); expect(result).toEqual(mockConfig); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@oidc-ui/src/__tests__/services/config.service.test.ts` around lines 24 - 30, Add an assertion that axios.get was called with the expected endpoint when testing fetchThemeConfig: after invoking fetchThemeConfig() and before/alongside the existing expectations, assert that vi.mocked(axios.get) (or axios.get) was called with the exact URL string used by fetchThemeConfig (and any expected options) so the test confirms the correct request URL; locate the call inside the test for fetchThemeConfig in oidc-ui/src/__tests__/services/config.service.test.ts and update that test to include the URL assertion referencing axios.get and fetchThemeConfig.oidc-ui/src/__tests__/utils/parsing.test.ts (1)
57-107:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRestore
window._env_after each test to prevent cross-test leakage.This suite mutates
window._env_inbeforeEachbut never restores the original state. Subsequent test files may inherit modified environment values.🧹 Suggested teardown
describe('parsing utilities', () => { + const originalEnv = { ...window._env_ }; + + afterEach(() => { + window._env_ = { ...originalEnv }; + }); + describe('parsePositiveInt', () => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@oidc-ui/src/__tests__/utils/parsing.test.ts` around lines 57 - 107, The tests mutate the global window._env_ and don't restore it, causing cross-test leakage; modify the test suite around getPollingConfig to capture the original window._env_ (e.g., const originalEnv = window._env_) in beforeEach or at top, then add an afterEach that restores window._env_ = originalEnv (or deletes it if it was undefined) so each test leaves the global environment as it was; reference window._env_ and the getPollingConfig tests to locate where to add the save/restore teardown.oidc-ui/src/__tests__/utils/redirect-on-error.test.ts (1)
33-37:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winParse the redirect URL instead of matching encoded substrings.
Asserting raw encoded query fragments like
'error_description=User+denied'is brittle; valid%20space encoding would fail. Parse the URL and checksearchParamsvalues.🔧 Suggested robust assertions
const url = (window.location.replace as ReturnType<typeof vi.fn>).mock.calls[0][0] as string; -expect(url).toContain('http://localhost:3000/callback?'); -expect(url).toContain('error=access_denied'); -expect(url).toContain('error_description=User+denied'); -expect(url).toContain('state=abc'); +const parsed = new URL(url); +expect(parsed.origin + parsed.pathname).toBe('http://localhost:3000/callback'); +expect(parsed.searchParams.get('error')).toBe('access_denied'); +expect(parsed.searchParams.get('error_description')).toBe('User denied'); +expect(parsed.searchParams.get('state')).toBe('abc');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@oidc-ui/src/__tests__/utils/redirect-on-error.test.ts` around lines 33 - 37, The test currently asserts encoded query substrings from the mocked window.location.replace call which is brittle; instead, parse the redirected URL string into a URL object and assert on its components and searchParams: locate the URL via (window.location.replace as ReturnType<typeof vi.fn>).mock.calls[0][0], construct a URL from that string, assert the origin+pathname contains the callback path and use url.searchParams.get('error') === 'access_denied', url.searchParams.get('error_description') === 'User denied', and url.searchParams.get('state') === 'abc' to avoid relying on specific percent-encoding.oidc-ui/src/services/api.service.ts (3)
30-43:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing error handling for CSRF token fetch.
If
getCsrfToken()throws (network error, 404, etc.), the promise rejection is unhandled and will propagate to the caller, potentially breaking all POST requests. Consider adding try-catch and a fallback strategy (retry, log, or reject the request explicitly).🛡️ Proposed fix to handle CSRF fetch errors
ApiService.interceptors.request.use( async (config) => { if (config.method?.toLowerCase() === 'post') { let csrfToken = sessionStorage.getItem('csrfToken'); if (!csrfToken) { - csrfToken = await getCsrfToken(); - if (csrfToken) sessionStorage.setItem('csrfToken', csrfToken); + try { + csrfToken = await getCsrfToken(); + if (csrfToken) sessionStorage.setItem('csrfToken', csrfToken); + } catch (error) { + console.error('Failed to fetch CSRF token:', error); + return Promise.reject(new Error('CSRF token unavailable')); + } } config.headers['X-XSRF-TOKEN'] = csrfToken; } return config; }, (error) => Promise.reject(error as Error), );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@oidc-ui/src/services/api.service.ts` around lines 30 - 43, Wrap the await getCsrfToken() call inside a try-catch in the ApiService.interceptors.request handler: in the try block await getCsrfToken(), store it into sessionStorage and set config.headers['X-XSRF-TOKEN'] only if a token was returned; in the catch block log the error (console.error or your app logger) including the caught error and explicitly abort the request by returning Promise.reject(new Error('Failed to fetch CSRF token')) (or another clear error) so POST requests don't proceed with an undefined token. Ensure you reference ApiService.interceptors.request, getCsrfToken, sessionStorage and the X-XSRF-TOKEN header when making these changes.
49-62:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winResponse interceptor has broken control flow for HTTP errors.
When
status >= 400(lines 55-56), the code callsnavigate()but doesn't return or reject the promise, causing the interceptor to implicitly returnundefined. This breaks the Axios contract and may cause unexpected behavior in callers expecting a rejected promise.🐛 Proposed fix to reject promise after navigation
export function setupResponseInterceptor(navigate: NavigateFunction): void { ApiService.interceptors.response.use( (response) => response, (error) => { const status = (error as { response?: { status?: number } })?.response ?.status; if (status && status >= 400) { navigate(SOMETHING_WENT_WRONG, { state: { code: status } }); - } else { - return Promise.reject(error instanceof Error ? error : new Error(String(error))); } + return Promise.reject(error instanceof Error ? error : new Error(String(error))); }, ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@oidc-ui/src/services/api.service.ts` around lines 49 - 62, The response interceptor in setupResponseInterceptor currently calls navigate(SOMETHING_WENT_WRONG, ...) when status >= 400 but does not return or reject, causing an implicit undefined return and breaking Axios' promise contract; update the error branch in ApiService.interceptors.response.use so that after calling navigate(...) you explicitly return Promise.reject(error instanceof Error ? error : new Error(String(error))) (or throw the same) to ensure callers receive a rejected promise.
19-22:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCSRF token fetch should use credentials.
The
getCsrfToken()function uses rawaxios.getwithoutwithCredentials: true. If the CSRF endpoint requires authentication cookies (which is typical), this call may fail or return an invalid token.🔒 Proposed fix to add credentials
async function getCsrfToken(): Promise<string> { - const response = await axios.get<{ token: string }>(csrfEndpoint); + const response = await axios.get<{ token: string }>(csrfEndpoint, { + withCredentials: true, + }); return response.data.token; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@oidc-ui/src/services/api.service.ts` around lines 19 - 22, The getCsrfToken() function calls axios.get without sending cookies, so include credentials when fetching the CSRF token; modify getCsrfToken to call axios.get(csrfEndpoint, { withCredentials: true }) (or use the app's configured axios instance that already sets withCredentials) so the authentication cookies are sent and response.data.token is valid.oidc-ui/src/main.tsx (1)
11-19:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail fast when
VITE_API_URLis missing (don’t silently fallback to localhost).This still allows production builds to continue with a localhost auth endpoint, which is unsafe and hard to detect.
💡 Proposed fix
const baseUrlRaw = import.meta.env.VITE_API_URL; -if (!baseUrlRaw) { +const baseUrl = + baseUrlRaw ?? (import.meta.env.DEV ? 'https://localhost:8088' : undefined); + +if (!baseUrl) { console.error( - 'VITE_API_URL environment variable is not set. ' + - 'Add it to your .env file (e.g. VITE_API_URL=https://your-api-host:8090).', + 'VITE_API_URL must be set in non-development environments.', ); + throw new Error('Missing VITE_API_URL'); } -const baseUrl = baseUrlRaw || `https://localhost:8088`;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@oidc-ui/src/main.tsx` around lines 11 - 19, The current code silently falls back to localhost when VITE_API_URL is missing; change main.tsx so that if baseUrlRaw (import.meta.env.VITE_API_URL) is falsy the app throws an explicit Error (or otherwise aborts startup) instead of using a default, and remove the fallback assignment to baseUrl = `https://localhost:8088`; keep the explicit console.error message if desired but ensure the process fails fast by throwing an Error referencing baseUrlRaw/baseUrl so production builds cannot continue with a localhost endpoint.oidc-ui/src/pages/LoginPage.tsx (1)
9-38:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix non-reactive
searchParamsflow; validation currently won’t run reliably.This mutable local variable pattern breaks effect reactivity and can leave the page stuck in loading. Also,
.has()accepts empty values.💡 Proposed fix
export default function LoginPage() { const [isLoading, setIsLoading] = useState(true); - let searchParams: URLSearchParams | null = null; const navigate = useNavigate(); useEffect(() => { - // setting search params in useEffect to ensure it runs - // only on client side and after the component mounts - searchParams = new URLSearchParams(window.location.search); - }, []); - - useEffect(() => { - // If searchParams is not set yet, we cannot validate. - // Wait for the next effect run. - if (!searchParams) return; - - // Check if all required params are present - const allParamsPresent = REQUIRED_PARAMS.every((p: string) => - searchParams?.has(p), - ); + const searchParams = new URLSearchParams(window.location.search); + const allParamsPresent = REQUIRED_PARAMS.every((p) => { + const value = searchParams.get(p); + return value !== null && value.trim() !== ""; + }); if (allParamsPresent) { setIsLoading(false); return; } - // If we have search params but are missing - // required ones, show page not found navigate("/something-went-wrong", { state: { code: 401 }, replace: true, }); - }, [searchParams]); + }, [navigate]);#!/bin/bash set -euo pipefail # Verify the current non-reactive local variable + effect dependency pattern. rg -n "let searchParams|useEffect\\(|\\[searchParams\\]" oidc-ui/src/pages/LoginPage.tsx -C2🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@oidc-ui/src/pages/LoginPage.tsx` around lines 9 - 38, The current use of a mutable local variable searchParams with two useEffect hooks is non-reactive and can leave the page stuck; replace this pattern by deriving params from React Router or component state (e.g., useLocation() or useSearchParams()) so re-renders trigger validation, and validate required parameters by checking their values (use URLSearchParams.get(p) and ensure non-empty) instead of .has(); update the effect that currently references searchParams to depend on the reactive params value and keep logic that calls setIsLoading(false) or navigate("/something-went-wrong", ...) intact, referencing REQUIRED_PARAMS, setIsLoading, and navigate.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@oidc-ui/src/__tests__/utils/parsing.test.ts`:
- Around line 30-32: The test shows parsePositiveInt preserves decimals but its
name and usage for timing (POLLING_INTERVAL, POLLING_TIMEOUT) require integer
milliseconds; change parsePositiveInt to coerce decimal inputs to an integer
(e.g., use Math.floor or Math.round) and update/remove the decimal test
expecting 3.14; specifically, modify the implementation of parsePositiveInt to
return a truncated/rounded integer for numeric inputs and adjust the test
(remove or change the "handles decimal numbers" case) so parsePositiveInt(3.14,
10) yields the integer value, ensuring all uses
(POLLING_INTERVAL/POLLING_TIMEOUT) receive integer milliseconds.
In `@oidc-ui/src/App.tsx`:
- Around line 16-19: The retry function currently inspects error.status but
axios errors embed HTTP status as error.response.status; update the retry logic
inside the retry: (failureCount, error) => { ... } block to extract status from
(error as any)?.response?.status (falling back to (error as any)?.status if
desired) and use that status to short-circuit retries for 4xx responses,
otherwise continue to retry while failureCount < 3; adjust any type annotations
if needed to satisfy TypeScript.
In `@oidc-ui/src/components/Footer.tsx`:
- Line 14: In the Footer component where the config fetch promise currently ends
with .catch(() => {}), stop silently swallowing errors: update that catch to
accept the error (e) and surface it (e.g., console.error("Footer config fetch
failed:", e)) and/or update component state to reflect the failure so the UI can
show a fallback; locate the promise chain in the Footer component and replace
.catch(() => {}) with a handler that logs the error and/or sets an error state.
In `@oidc-ui/src/pages/NetworkErrorPage.tsx`:
- Around line 8-9: The code in NetworkErrorPage reads location.state -> path and
calls window.location.replace(path) directly, which allows open redirects;
validate and restrict the redirect target before calling
window.location.replace. Update the logic around the path variable (the
extracted location.state as { path?: string } | null) to only allow same-origin
relative paths (e.g., ensure path is a string that begins with "/" but not "//",
does not include a scheme like "http:" or "https:", or alternatively parse with
new URL(path, window.location.origin) and verify its origin equals
window.location.origin), and fall back to "/" if validation fails, then call
window.location.replace(validatedPath).
In `@oidc-ui/src/pages/PageNotFoundPage.tsx`:
- Line 12: Update the image alt text in the PageNotFoundPage component: replace
the machine-style alt="page_not_found" with a natural-language, readable string
(e.g., "Page not found" or "Illustration: page not found") on the <img> element
in PageNotFoundPage so assistive technologies receive a clear, human-friendly
description.
In `@oidc-ui/src/routes/AppRouter.tsx`:
- Around line 109-112: The Detector render callback is performing navigation
side effects during render (render={({ online }) => { if (!online &&
location.pathname !== NETWORK_ERROR) navigate(...) }}), so move that logic into
a useEffect that depends on online, location.pathname and navigate: create a
useEffect that checks if online is false and location.pathname !== NETWORK_ERROR
then calls navigate(NETWORK_ERROR, { state: { path: location.pathname } }); and
update the Detector render prop to only return null (or the presentational
output) with no calls to navigate.
- Line 63: In the JSX div inside AppRouter.tsx that contains the long className
(the element around "flex justify-center ..."), replace the invalid Tailwind
variant token "mb:mt-0" with the correct responsive variant "md:mt-0" so the
top-margin reset is applied at the medium breakpoint; update the className
string accordingly and run a quick build/check to confirm Tailwind recognizes
the corrected utility.
In `@oidc-ui/src/services/api.service.ts`:
- Line 38: The CSRF header is being set unconditionally; modify the logic in
api.service.ts so that after retrieving the token (via getCsrfToken() /
sessionStorage lookup) you only assign config.headers['X-XSRF-TOKEN'] =
csrfToken when csrfToken is truthy (non-empty/non-null). Update the code path
that builds the request config (the place where config.headers is set) to guard
the assignment with a simple existence check to avoid sending falsy tokens.
In `@oidc-ui/src/utils/redirect-on-error.ts`:
- Around line 28-44: The same-origin check on redirectUrl (comparing
redirectUrl.origin to window.location.origin) is incorrect for OAuth2/OIDC and
breaks error handoff; instead validate redirectUri by parsing it into
redirectUrl, ensuring it uses an allowed scheme (e.g., https:) and then check
its origin against an allowlist of registered/allowed redirect URIs (e.g.,
oauthConfig.allowedRedirectUris or allowedRedirectUris) or verify the
oauthDetails fragment is signed/validated by the server and trust that boundary;
keep the URL parseability and scheme checks (new URL(redirectUri) and
redirectUrl.protocol === 'https:') and replace the window.origin equality with
an allowlist membership test or server-signature validation using identifiers
like redirectUri, redirectUrl, and oauthDetails.
- Around line 46-52: The current redirect builds the target by
string-concatenating `${redirectUri}?${params.toString()}` which breaks when
redirectUri already has query params; instead construct a URL object (use the
existing parsed redirectUrl if available or new URL(redirectUri)), then populate
its searchParams with error_description, state and error (using
url.searchParams.set for the same keys you added to params), and finally call
window.location.replace(url.toString()) after clearing window.onbeforeunload;
update the code around redirectUri/params/window.location.replace to use the URL
API so parameters are appended safely.
---
Duplicate comments:
In `@oidc-ui/public/theme-config.js`:
- Around line 2-4: The code calls body.classList.add(window._env_.DEFAULT_THEME)
which can pass an empty string and throw a SyntaxError; update the body/theme
application logic (the block that references body, window._env_.DEFAULT_THEME,
and classList.add) to first verify body exists and that DEFAULT_THEME is a
non-empty trimmed string (e.g., typeof check and .trim() length > 0) before
calling classList.add so you never call classList.add with an empty token.
In `@oidc-ui/src/__tests__/services/config.service.test.ts`:
- Around line 24-30: Add an assertion that axios.get was called with the
expected endpoint when testing fetchThemeConfig: after invoking
fetchThemeConfig() and before/alongside the existing expectations, assert that
vi.mocked(axios.get) (or axios.get) was called with the exact URL string used by
fetchThemeConfig (and any expected options) so the test confirms the correct
request URL; locate the call inside the test for fetchThemeConfig in
oidc-ui/src/__tests__/services/config.service.test.ts and update that test to
include the URL assertion referencing axios.get and fetchThemeConfig.
In `@oidc-ui/src/__tests__/utils/parsing.test.ts`:
- Around line 57-107: The tests mutate the global window._env_ and don't restore
it, causing cross-test leakage; modify the test suite around getPollingConfig to
capture the original window._env_ (e.g., const originalEnv = window._env_) in
beforeEach or at top, then add an afterEach that restores window._env_ =
originalEnv (or deletes it if it was undefined) so each test leaves the global
environment as it was; reference window._env_ and the getPollingConfig tests to
locate where to add the save/restore teardown.
In `@oidc-ui/src/__tests__/utils/redirect-on-error.test.ts`:
- Around line 33-37: The test currently asserts encoded query substrings from
the mocked window.location.replace call which is brittle; instead, parse the
redirected URL string into a URL object and assert on its components and
searchParams: locate the URL via (window.location.replace as ReturnType<typeof
vi.fn>).mock.calls[0][0], construct a URL from that string, assert the
origin+pathname contains the callback path and use url.searchParams.get('error')
=== 'access_denied', url.searchParams.get('error_description') === 'User
denied', and url.searchParams.get('state') === 'abc' to avoid relying on
specific percent-encoding.
In `@oidc-ui/src/main.tsx`:
- Around line 11-19: The current code silently falls back to localhost when
VITE_API_URL is missing; change main.tsx so that if baseUrlRaw
(import.meta.env.VITE_API_URL) is falsy the app throws an explicit Error (or
otherwise aborts startup) instead of using a default, and remove the fallback
assignment to baseUrl = `https://localhost:8088`; keep the explicit
console.error message if desired but ensure the process fails fast by throwing
an Error referencing baseUrlRaw/baseUrl so production builds cannot continue
with a localhost endpoint.
In `@oidc-ui/src/pages/LoginPage.tsx`:
- Around line 9-38: The current use of a mutable local variable searchParams
with two useEffect hooks is non-reactive and can leave the page stuck; replace
this pattern by deriving params from React Router or component state (e.g.,
useLocation() or useSearchParams()) so re-renders trigger validation, and
validate required parameters by checking their values (use
URLSearchParams.get(p) and ensure non-empty) instead of .has(); update the
effect that currently references searchParams to depend on the reactive params
value and keep logic that calls setIsLoading(false) or
navigate("/something-went-wrong", ...) intact, referencing REQUIRED_PARAMS,
setIsLoading, and navigate.
In `@oidc-ui/src/services/api.service.ts`:
- Around line 30-43: Wrap the await getCsrfToken() call inside a try-catch in
the ApiService.interceptors.request handler: in the try block await
getCsrfToken(), store it into sessionStorage and set
config.headers['X-XSRF-TOKEN'] only if a token was returned; in the catch block
log the error (console.error or your app logger) including the caught error and
explicitly abort the request by returning Promise.reject(new Error('Failed to
fetch CSRF token')) (or another clear error) so POST requests don't proceed with
an undefined token. Ensure you reference ApiService.interceptors.request,
getCsrfToken, sessionStorage and the X-XSRF-TOKEN header when making these
changes.
- Around line 49-62: The response interceptor in setupResponseInterceptor
currently calls navigate(SOMETHING_WENT_WRONG, ...) when status >= 400 but does
not return or reject, causing an implicit undefined return and breaking Axios'
promise contract; update the error branch in
ApiService.interceptors.response.use so that after calling navigate(...) you
explicitly return Promise.reject(error instanceof Error ? error : new
Error(String(error))) (or throw the same) to ensure callers receive a rejected
promise.
- Around line 19-22: The getCsrfToken() function calls axios.get without sending
cookies, so include credentials when fetching the CSRF token; modify
getCsrfToken to call axios.get(csrfEndpoint, { withCredentials: true }) (or use
the app's configured axios instance that already sets withCredentials) so the
authentication cookies are sent and response.data.token is valid.
🪄 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 UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 97bcfaaa-c8f7-4f1d-8a9d-d3b4a600ee93
⛔ Files ignored due to path filters (57)
oidc-ui/package-lock.jsonis excluded by!**/package-lock.jsonoidc-ui/public/favicon.icois excluded by!**/*.icooidc-ui/public/favicon.svgis excluded by!**/*.svgoidc-ui/public/icons.svgis excluded by!**/*.svgoidc-ui/public/images/Sign in with Inji.pngis excluded by!**/*.pngoidc-ui/public/images/Sign in with Iris.pngis excluded by!**/*.pngoidc-ui/public/images/Sign in with face.pngis excluded by!**/*.pngoidc-ui/public/images/Sign in with fingerprint.pngis excluded by!**/*.pngoidc-ui/public/images/asterisk_icon.svgis excluded by!**/*.svgoidc-ui/public/images/bg_bottom_left.pngis excluded by!**/*.pngoidc-ui/public/images/bio_icon.svgis excluded by!**/*.svgoidc-ui/public/images/bottom_right_bg_logo.svgis excluded by!**/*.svgoidc-ui/public/images/brand_logo.pngis excluded by!**/*.pngoidc-ui/public/images/chevron_down.svgis excluded by!**/*.svgoidc-ui/public/images/cross_icon.svgis excluded by!**/*.svgoidc-ui/public/images/drop_down.pngis excluded by!**/*.pngoidc-ui/public/images/drop_down_icon.pngis excluded by!**/*.pngoidc-ui/public/images/edit_access_icon.pngis excluded by!**/*.pngoidc-ui/public/images/email_icon.svgis excluded by!**/*.svgoidc-ui/public/images/error_icon.svgis excluded by!**/*.svgoidc-ui/public/images/face_capture.pngis excluded by!**/*.pngoidc-ui/public/images/fingerprint_scan.pngis excluded by!**/*.pngoidc-ui/public/images/footer_logo.pngis excluded by!**/*.pngoidc-ui/public/images/identity_icon.pngis excluded by!**/*.pngoidc-ui/public/images/illustration_one.pngis excluded by!**/*.pngoidc-ui/public/images/info_icon.svgis excluded by!**/*.svgoidc-ui/public/images/iris_code.pngis excluded by!**/*.pngoidc-ui/public/images/kbi_icon.svgis excluded by!**/*.svgoidc-ui/public/images/language_icon.pngis excluded by!**/*.pngoidc-ui/public/images/left_arrow_icon.svgis excluded by!**/*.svgoidc-ui/public/images/logo.pngis excluded by!**/*.pngoidc-ui/public/images/mobile_icon.svgis excluded by!**/*.svgoidc-ui/public/images/no_internet.svgis excluded by!**/*.svgoidc-ui/public/images/nrc_id_icon.svgis excluded by!**/*.svgoidc-ui/public/images/otp_icon.svgis excluded by!**/*.svgoidc-ui/public/images/otp_image.pngis excluded by!**/*.pngoidc-ui/public/images/password_hide.svgis excluded by!**/*.svgoidc-ui/public/images/password_show.svgis excluded by!**/*.svgoidc-ui/public/images/photo_scan.pngis excluded by!**/*.pngoidc-ui/public/images/pwd_icon.svgis excluded by!**/*.svgoidc-ui/public/images/qr_code.pngis excluded by!**/*.pngoidc-ui/public/images/refresh_logo.pngis excluded by!**/*.pngoidc-ui/public/images/section-bg.pngis excluded by!**/*.pngoidc-ui/public/images/select_icon.pngis excluded by!**/*.pngoidc-ui/public/images/sign_in_with_kbi.pngis excluded by!**/*.pngoidc-ui/public/images/sign_in_with_otp.pngis excluded by!**/*.pngoidc-ui/public/images/submit_bg.pngis excluded by!**/*.pngoidc-ui/public/images/sync_alt_black.svgis excluded by!**/*.svgoidc-ui/public/images/toggle_off.pngis excluded by!**/*.pngoidc-ui/public/images/toggle_on.pngis excluded by!**/*.pngoidc-ui/public/images/top_left_bg_logo.svgis excluded by!**/*.svgoidc-ui/public/images/under_construction.svgis excluded by!**/*.svgoidc-ui/public/images/up_down_arrow_icon.svgis excluded by!**/*.svgoidc-ui/public/images/vid_icon.svgis excluded by!**/*.svgoidc-ui/public/images/wallet_icon.svgis excluded by!**/*.svgoidc-ui/public/images/warning_message_icon.svgis excluded by!**/*.svgoidc-ui/public/logo.pngis excluded by!**/*.png
📒 Files selected for processing (55)
oidc-ui/.envoidc-ui/.env.productionoidc-ui/.gitignoreoidc-ui/README.mdoidc-ui/eslint.config.jsoidc-ui/index.htmloidc-ui/package.jsonoidc-ui/public/env-config.jsoidc-ui/public/theme-config.jsoidc-ui/public/theme/config.jsonoidc-ui/public/theme/variables.cssoidc-ui/src/App.cssoidc-ui/src/App.tsxoidc-ui/src/__tests__/components/Footer.test.tsxoidc-ui/src/__tests__/components/LoadingIndicator.test.tsxoidc-ui/src/__tests__/components/NavHeader.test.tsxoidc-ui/src/__tests__/pages/LoginPage.test.tsxoidc-ui/src/__tests__/pages/NetworkErrorPage.test.tsxoidc-ui/src/__tests__/pages/PageNotFoundPage.test.tsxoidc-ui/src/__tests__/pages/SomethingWrongPage.test.tsxoidc-ui/src/__tests__/routes/AppRouter.test.tsxoidc-ui/src/__tests__/services/config.service.test.tsoidc-ui/src/__tests__/services/css-variable.service.test.tsoidc-ui/src/__tests__/utils/encoding.test.tsoidc-ui/src/__tests__/utils/hashing.test.tsoidc-ui/src/__tests__/utils/parsing.test.tsoidc-ui/src/__tests__/utils/redirect-on-error.test.tsoidc-ui/src/components/Footer.tsxoidc-ui/src/components/LoadingIndicator.tsxoidc-ui/src/components/NavHeader.tsxoidc-ui/src/constants/public-assets.tsoidc-ui/src/constants/routes.tsoidc-ui/src/constants/states.tsoidc-ui/src/main.tsxoidc-ui/src/pages/EsignetDetailsPage.tsxoidc-ui/src/pages/LoginPage.tsxoidc-ui/src/pages/NetworkErrorPage.tsxoidc-ui/src/pages/PageNotFoundPage.tsxoidc-ui/src/pages/SomethingWrongPage.tsxoidc-ui/src/routes/AppRouter.tsxoidc-ui/src/services/api.service.tsoidc-ui/src/services/config.service.tsoidc-ui/src/services/css-variable.service.tsoidc-ui/src/test-setup.tsoidc-ui/src/types/index.tsoidc-ui/src/utils/encoding.tsoidc-ui/src/utils/hashing.tsoidc-ui/src/utils/parsing.tsoidc-ui/src/utils/redirect-on-error.tsoidc-ui/src/vite-env.d.tsoidc-ui/tailwind.config.jsoidc-ui/tsconfig.app.jsonoidc-ui/tsconfig.jsonoidc-ui/tsconfig.node.jsonoidc-ui/vite.config.ts
Comment on lines
+30
to
+32
| it('handles decimal numbers', () => { | ||
| expect(parsePositiveInt(3.14, 10)).toBe(3.14); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check parsePositiveInt implementation to determine if it should handle decimals
rg -nP -A10 'function parsePositiveInt|const parsePositiveInt|export.*parsePositiveInt' --type=tsRepository: mosip/esignet
Length of output: 667
🏁 Script executed:
cat -n oidc-ui/src/__tests__/utils/parsing.test.ts | head -50Repository: mosip/esignet
Length of output: 1958
🏁 Script executed:
rg -n 'parsePositiveInt' --type=ts --type=tsxRepository: mosip/esignet
Length of output: 85
🏁 Script executed:
fd -e ts -e tsx -path '*parsing*' | grep -E '(test|spec)'Repository: mosip/esignet
Length of output: 226
🏁 Script executed:
rg -n 'parsePositiveInt' --type=tsRepository: mosip/esignet
Length of output: 1221
🏁 Script executed:
rg -n 'POLLING_INTERVAL|POLLING_TIMEOUT' --type=ts -B2 -A2Repository: mosip/esignet
Length of output: 1974
Rename function or fix test expectation for decimal handling.
The function name parsePositiveInt implies integer parsing, but the implementation accepts and preserves decimal values unchanged (as shown by the test expecting parsePositiveInt(3.14, 10) to return 3.14). Since the function is used exclusively for timing configurations (POLLING_INTERVAL, POLLING_TIMEOUT), which should be millisecond integers, either:
- Rename to
parsePositiveNumberto reflect actual behavior, or - Truncate/round to integer in the implementation and remove the decimal test case at lines 30-32.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@oidc-ui/src/__tests__/utils/parsing.test.ts` around lines 30 - 32, The test
shows parsePositiveInt preserves decimals but its name and usage for timing
(POLLING_INTERVAL, POLLING_TIMEOUT) require integer milliseconds; change
parsePositiveInt to coerce decimal inputs to an integer (e.g., use Math.floor or
Math.round) and update/remove the decimal test expecting 3.14; specifically,
modify the implementation of parsePositiveInt to return a truncated/rounded
integer for numeric inputs and adjust the test (remove or change the "handles
decimal numbers" case) so parsePositiveInt(3.14, 10) yields the integer value,
ensuring all uses (POLLING_INTERVAL/POLLING_TIMEOUT) receive integer
milliseconds.
Comment on lines
+16
to
+19
| retry: (failureCount, error) => { | ||
| const status = (error as { status?: number })?.status; | ||
| if (status && status >= 400 && status < 500) return false; | ||
| return failureCount < 3; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify retry logic and inspect service error-shape usage.
rg -n "retry:\\s*\\(" oidc-ui/src/App.tsx -C3
rg -n "response\\.status|\\.status|throw|Promise\\.reject|Axios" oidc-ui/src/services/api.service.ts -C3Repository: mosip/esignet
Length of output: 1077
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check what ApiService is used in queries and how it's called
rg -n "ApiService|useQuery|useMutation" oidc-ui/src/App.tsx -C5
# Check if ApiService is imported and used elsewhere in App.tsx
rg -n "import.*ApiService\|from.*api" oidc-ui/src/App.tsx
# Check the full QueryClient setup to understand the error flow
sed -n '13,30p' oidc-ui/src/App.tsxRepository: mosip/esignet
Length of output: 39
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# First, check the file exists and its general structure
echo "=== Checking App.tsx structure ==="
head -50 oidc-ui/src/App.tsx
echo -e "\n=== Checking api.service.ts ==="
wc -l oidc-ui/src/services/api.service.tsRepository: mosip/esignet
Length of output: 1682
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the complete api.service.ts file
cat -n oidc-ui/src/services/api.service.ts
# Search for where ApiService or queries are used
echo -e "\n=== Checking for ApiService usage ==="
rg "ApiService" oidc-ui/src/ -lRepository: mosip/esignet
Length of output: 2467
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find where setupResponseInterceptor is called
echo "=== setupResponseInterceptor usage ==="
rg "setupResponseInterceptor" oidc-ui/src/ -C3
# Find where ApiService is used (imported in other files)
echo -e "\n=== Searching for ApiService imports ==="
rg "import.*ApiService\|from.*api.service" oidc-ui/src/ -l
# Check AppRouter to see what queries are defined
echo -e "\n=== Checking AppRouter ==="
wc -l oidc-ui/src/routes/AppRouter.tsx
head -100 oidc-ui/src/routes/AppRouter.tsxRepository: mosip/esignet
Length of output: 697
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for all ApiService references (not just imports)
echo "=== All ApiService references ==="
rg "ApiService" oidc-ui/src/ -C2
# Search for setupResponseInterceptor calls
echo -e "\n=== setupResponseInterceptor calls ==="
rg "setupResponseInterceptor" oidc-ui/src/ --type ts --type tsx -C2Repository: mosip/esignet
Length of output: 1157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all .ts and .tsx files
echo "=== TypeScript files in src ==="
find oidc-ui/src -type f \( -name "*.ts" -o -name "*.tsx" \) | head -20
# Search for axios usage more broadly
echo -e "\n=== axios usage ==="
rg "axios\|useQuery\|useMutation" oidc-ui/src -t js -t json
# Look for HTTP requests in hooks or services
echo -e "\n=== Looking for fetch or HTTP calls ==="
rg "fetch|axios\\.get|axios\\.post|\\.get\(|\\.post\(" oidc-ui/src --type jsRepository: mosip/esignet
Length of output: 1036
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get complete file listing
echo "=== All TypeScript files ==="
find oidc-ui/src -type f \( -name "*.ts" -o -name "*.tsx" \) | sort
# Look at pages directory which likely has queries
echo -e "\n=== Pages directory ==="
ls -la oidc-ui/src/pages/ 2>/dev/null || echo "Pages directory not found"
# Look for any HTTP request patterns
echo -e "\n=== Searching for fetch or HTTP patterns ==="
rg "fetch|request|get\(|post\(" oidc-ui/src --max-count=20Repository: mosip/esignet
Length of output: 4416
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read config.service.ts
cat -n oidc-ui/src/services/config.service.tsRepository: mosip/esignet
Length of output: 509
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify axios error shape by checking axios documentation and how it behaves
# Also check if there are any other error sources that might use error.status
echo "=== Checking for other error sources ==="
rg "throw|reject|Error" oidc-ui/src/services --max-count=10
# Look at main.tsx and test setup to see if there's anything special
echo -e "\n=== Checking main.tsx ==="
cat oidc-ui/src/main.tsx
# Check if setupResponseInterceptor is EVER called anywhere
echo -e "\n=== Complete search for setupResponseInterceptor ==="
rg -r "setupResponseInterceptor" oidc-ui/Repository: mosip/esignet
Length of output: 1516
Fix retry policy to handle axios error shape.
The retry policy checks error.status, but axios errors have the shape { response: { status } }. This means 4xx failures won't be caught and will be retried instead of failing fast.
💡 Proposed fix
retry: (failureCount, error) => {
- const status = (error as { status?: number })?.status;
+ const status = (error as { response?: { status?: number } })?.response?.status;
if (status && status >= 400 && status < 500) return false;
return failureCount < 3;
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| retry: (failureCount, error) => { | |
| const status = (error as { status?: number })?.status; | |
| if (status && status >= 400 && status < 500) return false; | |
| return failureCount < 3; | |
| retry: (failureCount, error) => { | |
| const status = (error as { response?: { status?: number } })?.response?.status; | |
| if (status && status >= 400 && status < 500) return false; | |
| return failureCount < 3; | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@oidc-ui/src/App.tsx` around lines 16 - 19, The retry function currently
inspects error.status but axios errors embed HTTP status as
error.response.status; update the retry logic inside the retry: (failureCount,
error) => { ... } block to extract status from (error as any)?.response?.status
(falling back to (error as any)?.status if desired) and use that status to
short-circuit retries for 4xx responses, otherwise continue to retry while
failureCount < 3; adjust any type annotations if needed to satisfy TypeScript.
| .then((cfg) => { | ||
| if (mounted) setConfig(cfg); | ||
| }) | ||
| .catch(() => {}); |
There was a problem hiding this comment.
Don’t silently swallow config-fetch errors.
An empty catch hides production config failures and makes troubleshooting harder.
💡 Proposed fix
- .catch(() => {});
+ .catch((error) => {
+ console.error("Failed to fetch footer config:", error);
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| .catch(() => {}); | |
| .catch((error) => { | |
| console.error("Failed to fetch footer config:", error); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@oidc-ui/src/components/Footer.tsx` at line 14, In the Footer component where
the config fetch promise currently ends with .catch(() => {}), stop silently
swallowing errors: update that catch to accept the error (e) and surface it
(e.g., console.error("Footer config fetch failed:", e)) and/or update component
state to reflect the failure so the UI can show a fallback; locate the promise
chain in the Footer component and replace .catch(() => {}) with a handler that
logs the error and/or sets an error state.
Comment on lines
+8
to
+9
| const path = (location.state as { path?: string } | null)?.path ?? '/'; | ||
| window.location.replace(path); |
There was a problem hiding this comment.
Validate redirect target before window.location.replace.
On Line 8/Line 9, path is used as-is. This can enable open redirects if state is ever populated with an absolute or protocol-relative URL. Restrict it to safe same-origin relative paths.
🔒 Suggested fix
const tryAgain = () => {
window.onbeforeunload = null;
const path = (location.state as { path?: string } | null)?.path ?? '/';
- window.location.replace(path);
+ const safePath = path.startsWith('/') && !path.startsWith('//') ? path : '/';
+ window.location.replace(safePath);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const path = (location.state as { path?: string } | null)?.path ?? '/'; | |
| window.location.replace(path); | |
| const tryAgain = () => { | |
| window.onbeforeunload = null; | |
| const path = (location.state as { path?: string } | null)?.path ?? '/'; | |
| const safePath = path.startsWith('/') && !path.startsWith('//') ? path : '/'; | |
| window.location.replace(safePath); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@oidc-ui/src/pages/NetworkErrorPage.tsx` around lines 8 - 9, The code in
NetworkErrorPage reads location.state -> path and calls
window.location.replace(path) directly, which allows open redirects; validate
and restrict the redirect target before calling window.location.replace. Update
the logic around the path variable (the extracted location.state as { path?:
string } | null) to only allow same-origin relative paths (e.g., ensure path is
a string that begins with "/" but not "//", does not include a scheme like
"http:" or "https:", or alternatively parse with new URL(path,
window.location.origin) and verify its origin equals window.location.origin),
and fall back to "/" if validation fails, then call
window.location.replace(validatedPath).
| <img | ||
| className="mx-auto my-0" | ||
| src={IMAGES.UNDER_CONSTRUCTION} | ||
| alt="page_not_found" |
There was a problem hiding this comment.
Use readable alt text for assistive technologies.
On Line 12, replace underscore-form text with natural language.
♿ Suggested fix
- alt="page_not_found"
+ alt="Page not found"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| alt="page_not_found" | |
| alt="Page not found" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@oidc-ui/src/pages/PageNotFoundPage.tsx` at line 12, Update the image alt text
in the PageNotFoundPage component: replace the machine-style
alt="page_not_found" with a natural-language, readable string (e.g., "Page not
found" or "Illustration: page not found") on the <img> element in
PageNotFoundPage so assistive technologies receive a clear, human-friendly
description.
| // we can safely access its properties, adding null checks where appropriate. | ||
| const backgroundLogoDiv = checkRoute(location.pathname) ? ( | ||
| config && config["background_logo"] ? ( | ||
| <div className="flex justify-center m-10 lg:mt-20 mb:mt-0 lg:w-1/2 md:w-1/2 md:block sm:w-1/2 sm:block hidden w-5/6 mt-20 mb-10 md:mb-0"> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify invalid utility token occurrence.
rg -n "\\bmb:mt-0\\b" oidc-ui/src/routes/AppRouter.tsx -C1Repository: mosip/esignet
Length of output: 267
Fix invalid Tailwind variant token mb:mt-0.
mb: is not a valid Tailwind variant—should be md: to match the responsive breakpoint pattern used elsewhere in the className. Without this fix, the mt-0 utility will be ignored.
- <div className="flex justify-center m-10 lg:mt-20 mb:mt-0 lg:w-1/2 md:w-1/2 md:block sm:w-1/2 sm:block hidden w-5/6 mt-20 mb-10 md:mb-0">
+ <div className="flex justify-center m-10 lg:mt-20 md:mt-0 lg:w-1/2 md:w-1/2 md:block sm:w-1/2 sm:block hidden w-5/6 mt-20 mb-10 md:mb-0">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <div className="flex justify-center m-10 lg:mt-20 mb:mt-0 lg:w-1/2 md:w-1/2 md:block sm:w-1/2 sm:block hidden w-5/6 mt-20 mb-10 md:mb-0"> | |
| <div className="flex justify-center m-10 lg:mt-20 md:mt-0 lg:w-1/2 md:w-1/2 md:block sm:w-1/2 sm:block hidden w-5/6 mt-20 mb-10 md:mb-0"> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@oidc-ui/src/routes/AppRouter.tsx` at line 63, In the JSX div inside
AppRouter.tsx that contains the long className (the element around "flex
justify-center ..."), replace the invalid Tailwind variant token "mb:mt-0" with
the correct responsive variant "md:mt-0" so the top-margin reset is applied at
the medium breakpoint; update the className string accordingly and run a quick
build/check to confirm Tailwind recognizes the corrected utility.
Comment on lines
+109
to
+112
| render={({ online }) => { | ||
| if (!online && location.pathname !== NETWORK_ERROR) { | ||
| navigate(NETWORK_ERROR, { state: { path: location.pathname } }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify navigate is executed inside render callback.
rg -n "render\\s*=\\s*\\(\\{\\s*online\\s*\\}\\)\\s*=>|navigate\\(NETWORK_ERROR" oidc-ui/src/routes/AppRouter.tsx -C3Repository: mosip/esignet
Length of output: 359
🏁 Script executed:
#!/bin/bash
# Get more context around lines 109-112 to understand the component structure
head -150 oidc-ui/src/routes/AppRouter.tsx | tail -60Repository: mosip/esignet
Length of output: 1857
🏁 Script executed:
#!/bin/bash
# Search for Detector component definition and useEffect usage in the file
rg -n "Detector|useEffect|render=" oidc-ui/src/routes/AppRouter.tsxRepository: mosip/esignet
Length of output: 265
🏁 Script executed:
#!/bin/bash
# Check what navigate and location are imported from
head -30 oidc-ui/src/routes/AppRouter.tsxRepository: mosip/esignet
Length of output: 1291
Move offline navigation into a useEffect hook.
Calling navigate within the Detector render callback executes side effects during the render phase, which can cause unstable updates, multiple renders, or unintended re-navigation loops. Move the navigation logic to a useEffect that responds to the online status instead.
Example:
useEffect(() => {
if (!online && location.pathname !== NETWORK_ERROR) {
navigate(NETWORK_ERROR, { state: { path: location.pathname } });
}
}, [online, location.pathname, navigate]);Then update the Detector render prop to simply return null without side effects.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@oidc-ui/src/routes/AppRouter.tsx` around lines 109 - 112, The Detector render
callback is performing navigation side effects during render (render={({ online
}) => { if (!online && location.pathname !== NETWORK_ERROR) navigate(...) }}),
so move that logic into a useEffect that depends on online, location.pathname
and navigate: create a useEffect that checks if online is false and
location.pathname !== NETWORK_ERROR then calls navigate(NETWORK_ERROR, { state:
{ path: location.pathname } }); and update the Detector render prop to only
return null (or the presentational output) with no calls to navigate.
| csrfToken = await getCsrfToken(); | ||
| if (csrfToken) sessionStorage.setItem('csrfToken', csrfToken); | ||
| } | ||
| config.headers['X-XSRF-TOKEN'] = csrfToken; |
There was a problem hiding this comment.
CSRF header set even when token is falsy.
Line 38 sets X-XSRF-TOKEN unconditionally, but if getCsrfToken() returns an empty string or the sessionStorage retrieval returns null, the header will be set to a falsy value. This may cause POST requests to fail with an invalid CSRF token.
🛡️ Proposed fix to only set header when token exists
let csrfToken = sessionStorage.getItem('csrfToken');
if (!csrfToken) {
csrfToken = await getCsrfToken();
if (csrfToken) sessionStorage.setItem('csrfToken', csrfToken);
}
- config.headers['X-XSRF-TOKEN'] = csrfToken;
+ if (csrfToken) {
+ config.headers['X-XSRF-TOKEN'] = csrfToken;
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| config.headers['X-XSRF-TOKEN'] = csrfToken; | |
| let csrfToken = sessionStorage.getItem('csrfToken'); | |
| if (!csrfToken) { | |
| csrfToken = await getCsrfToken(); | |
| if (csrfToken) sessionStorage.setItem('csrfToken', csrfToken); | |
| } | |
| if (csrfToken) { | |
| config.headers['X-XSRF-TOKEN'] = csrfToken; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@oidc-ui/src/services/api.service.ts` at line 38, The CSRF header is being set
unconditionally; modify the logic in api.service.ts so that after retrieving the
token (via getCsrfToken() / sessionStorage lookup) you only assign
config.headers['X-XSRF-TOKEN'] = csrfToken when csrfToken is truthy
(non-empty/non-null). Update the code path that builds the request config (the
place where config.headers is set) to guard the assignment with a simple
existence check to avoid sending falsy tokens.
Comment on lines
+28
to
+44
| // Validate redirect URI origin to prevent open-redirect attacks | ||
| let redirectUrl: URL; | ||
| try { | ||
| redirectUrl = new URL(redirectUri); | ||
| } catch { | ||
| console.error('Invalid redirect URI:', redirectUri); | ||
| return; | ||
| } | ||
|
|
||
| if (redirectUrl.origin !== window.location.origin) { | ||
| console.error( | ||
| 'Redirect URI origin mismatch: expected %s, got %s', | ||
| window.location.origin, | ||
| redirectUrl.origin, | ||
| ); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Same-origin check breaks the OAuth2 error-handoff contract.
In OAuth2/OIDC, redirect_uri points back to the Relying Party (client), which by design runs on a different origin than this OP (eSignet) UI. Requiring redirectUrl.origin === window.location.origin will cause this branch to log "origin mismatch" and silently return for essentially every real-world deployment, so fatal errors will never be delivered back to the RP.
This addresses the previous open-redirect concern in the wrong way. The correct mitigation is to validate redirectUri against the registered/allowlisted redirect URIs for the client (typically supplied by the authorization server / configuration), not against window.location.origin. If the oauthDetails fragment is produced and signed/validated by the server during the authorize step, you can also document that trust boundary and skip client-side origin equality checks, while keeping URL parseability and scheme (https: / allowed) checks.
Suggested direction:
🛡️ Replace same-origin equality with an allowlist / scheme check
- if (redirectUrl.origin !== window.location.origin) {
- console.error(
- 'Redirect URI origin mismatch: expected %s, got %s',
- window.location.origin,
- redirectUrl.origin,
- );
- return;
- }
+ // Validate against the client's registered redirect URIs (from config/auth server).
+ // Same-origin is incorrect here because the RP lives on a different origin than the OP UI.
+ const allowedRedirectUris: string[] = getAllowedRedirectUris(/* clientId from oauthDetails */);
+ const isAllowed = allowedRedirectUris.some(
+ (uri) => uri === redirectUri || new URL(uri).origin === redirectUrl.origin,
+ );
+ if (!isAllowed || !['https:', 'http:'].includes(redirectUrl.protocol)) {
+ console.error('Redirect URI not in allowlist:', redirectUri);
+ return;
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@oidc-ui/src/utils/redirect-on-error.ts` around lines 28 - 44, The same-origin
check on redirectUrl (comparing redirectUrl.origin to window.location.origin) is
incorrect for OAuth2/OIDC and breaks error handoff; instead validate redirectUri
by parsing it into redirectUrl, ensuring it uses an allowed scheme (e.g.,
https:) and then check its origin against an allowlist of registered/allowed
redirect URIs (e.g., oauthConfig.allowedRedirectUris or allowedRedirectUris) or
verify the oauthDetails fragment is signed/validated by the server and trust
that boundary; keep the URL parseability and scheme checks (new URL(redirectUri)
and redirectUrl.protocol === 'https:') and replace the window.origin equality
with an allowlist membership test or server-signature validation using
identifiers like redirectUri, redirectUrl, and oauthDetails.
Comment on lines
+46
to
+52
| const params = new URLSearchParams(); | ||
| if (errorDescription) params.set('error_description', errorDescription); | ||
| params.set('state', state); | ||
| params.set('error', errorCode); | ||
|
|
||
| window.onbeforeunload = null; | ||
| window.location.replace(`${redirectUri}?${params.toString()}`); |
There was a problem hiding this comment.
Malformed URL when redirectUri already contains a query string.
`${redirectUri}?${params.toString()}` unconditionally inserts a ?. If the RP registers a callback like https://rp.example.com/cb?tenant=acme, the redirect becomes https://rp.example.com/cb?tenant=acme?error=..., which is not a valid query separator and many RPs will fail to parse state/error. Use the URL API (you already parsed redirectUrl above) to append parameters safely.
🔧 Append params via URL.searchParams
- const params = new URLSearchParams();
- if (errorDescription) params.set('error_description', errorDescription);
- params.set('state', state);
- params.set('error', errorCode);
-
- window.onbeforeunload = null;
- window.location.replace(`${redirectUri}?${params.toString()}`);
+ redirectUrl.searchParams.set('error', errorCode);
+ if (errorDescription) {
+ redirectUrl.searchParams.set('error_description', errorDescription);
+ }
+ redirectUrl.searchParams.set('state', state);
+
+ window.onbeforeunload = null;
+ window.location.replace(redirectUrl.toString());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@oidc-ui/src/utils/redirect-on-error.ts` around lines 46 - 52, The current
redirect builds the target by string-concatenating
`${redirectUri}?${params.toString()}` which breaks when redirectUri already has
query params; instead construct a URL object (use the existing parsed
redirectUrl if available or new URL(redirectUri)), then populate its
searchParams with error_description, state and error (using url.searchParams.set
for the same keys you added to params), and finally call
window.location.replace(url.toString()) after clearing window.onbeforeunload;
update the code around redirectUri/params/window.location.replace to use the URL
API so parameters are appended safely.
Signed-off-by: Zeeshan Mehboob <zeeshan.mehboob@infosys.com>
Signed-off-by: Zeeshan Mehboob <zeeshan.mehboob@infosys.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
oidc-ui/src/main.tsx (1)
12-20:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail fast when
VITE_API_URLis missing outside development.Line 19 still unconditionally falls back to localhost, which can silently misroute auth calls in deployed environments.
Suggested minimal fix
const baseUrlRaw = import.meta.env.VITE_API_URL; -if (!baseUrlRaw) { +if (!baseUrlRaw && !import.meta.env.DEV) { + throw new Error("VITE_API_URL is required in non-development environments."); +} +if (!baseUrlRaw && import.meta.env.DEV) { console.error( "VITE_API_URL environment variable is not set. " + "Add it to your .env file (e.g. VITE_API_URL=https://your-api-host:8090).", ); } -const baseUrl = baseUrlRaw || `https://localhost:8088`; +const baseUrl = baseUrlRaw || "https://localhost:8088";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@oidc-ui/src/main.tsx` around lines 12 - 20, The code currently falls back to localhost which can misroute auth in non-development; update the startup logic that reads import.meta.env.VITE_API_URL (baseUrlRaw) and import.meta.env.MODE so that if MODE !== 'development' and baseUrlRaw is falsy you throw an error (or otherwise abort) instead of silently using the localhost fallback. Keep the existing localhost fallback only when MODE === 'development'; ensure the single source values referenced are baseUrlRaw, baseUrl, and import.meta.env.MODE so you can remove the unconditional fallback and fail fast in production.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@oidc-ui/src/components/SbiComponent.tsx`:
- Around line 90-99: When validateBiometricResponse(biometricResponse) returns a
non-null errorCode, clear the stored biometric value so a previous successful
payload isn't left in the form: call setValue with an empty string (or null per
app convention) and call context.onInputChange(component.id, sameEmptyValue)
before returning; update the early-return branch that checks errorCode to
perform these clears (references: validateBiometricResponse, biometricResponse,
setValue, context.onInputChange, component.id).
- Around line 53-74: The effect currently returns immediately after init because
firstRender.current is true, so propChange never gets registered and
authenticateBiometricResponse is never called; fix by ensuring propChange is
invoked on mount as well — e.g., after calling init (inside the same useEffect)
also call propChange({ onCapture: (response) =>
authenticateBiometricResponse(response) }) or move the propChange registration
out of the early-return branch so it runs both on the initial render and
subsequent renders; update the useEffect (which references firstRender, init,
propChange, authenticateBiometricResponse) accordingly and keep any necessary
cleanup but remove the premature return that prevents propChange registration.
---
Duplicate comments:
In `@oidc-ui/src/main.tsx`:
- Around line 12-20: The code currently falls back to localhost which can
misroute auth in non-development; update the startup logic that reads
import.meta.env.VITE_API_URL (baseUrlRaw) and import.meta.env.MODE so that if
MODE !== 'development' and baseUrlRaw is falsy you throw an error (or otherwise
abort) instead of silently using the localhost fallback. Keep the existing
localhost fallback only when MODE === 'development'; ensure the single source
values referenced are baseUrlRaw, baseUrl, and import.meta.env.MODE so you can
remove the unconditional fallback and fail fast in production.
🪄 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 UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1ebbe438-28de-404d-82b7-075f80b8b0e5
📒 Files selected for processing (7)
oidc-ui/package.jsonoidc-ui/src/components/SbiComponent.tsxoidc-ui/src/components/SbiCustomRenderer.tsxoidc-ui/src/constants/states.tsoidc-ui/src/main.tsxoidc-ui/src/utils/encoding.tsoidc-ui/src/vite-env.d.ts
Comment on lines
+53
to
+74
| const firstRender = useRef(true); | ||
| const [, setValue] = useState(""); | ||
|
|
||
| useEffect(() => { | ||
| if (firstRender.current) { | ||
| firstRender.current = false; | ||
| init({ | ||
| container: document.getElementById(SBI_CONTAINER_ID), | ||
| buttonLabel: "scan_and_verify", | ||
| transactionId: "transactionId", | ||
| sbiEnv: DEFAULT_SBI_ENV, | ||
| langCode: "en", | ||
| disable: false, | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| propChange({ | ||
| onCapture: (response: BiometricResponse | null) => | ||
| authenticateBiometricResponse(response), | ||
| }); | ||
| }, []); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f SbiComponent.tsxRepository: mosip/esignet
Length of output: 97
🏁 Script executed:
cat -n oidc-ui/src/components/SbiComponent.tsxRepository: mosip/esignet
Length of output: 4466
propChange callback registration is unreachable in the mount effect.
The effect runs once (empty dependency array), and on that single run, firstRender.current is true, so the condition at line 57 executes init() and returns at line 67. Lines 70–73 are never reached, meaning the biometric capture callback is never wired, and authenticateBiometricResponse is never invoked when a capture occurs.
Suggested fix
-export default function Sbi({ component, context }: SbiProps) {
- const firstRender = useRef(true);
+export default function Sbi({ component, context }: SbiProps) {
const [, setValue] = useState("");
useEffect(() => {
- if (firstRender.current) {
- firstRender.current = false;
- init({
- container: document.getElementById(SBI_CONTAINER_ID),
- buttonLabel: "scan_and_verify",
- transactionId: "transactionId",
- sbiEnv: DEFAULT_SBI_ENV,
- langCode: "en",
- disable: false,
- });
- return;
- }
-
+ init({
+ container: document.getElementById(SBI_CONTAINER_ID),
+ buttonLabel: "scan_and_verify",
+ transactionId: "transactionId",
+ sbiEnv: DEFAULT_SBI_ENV,
+ langCode: "en",
+ disable: false,
+ });
propChange({
- onCapture: (response: BiometricResponse | null) =>
- authenticateBiometricResponse(response),
+ onCapture: (response: BiometricResponse | null) => {
+ void authenticateBiometricResponse(response);
+ },
});
}, []);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const firstRender = useRef(true); | |
| const [, setValue] = useState(""); | |
| useEffect(() => { | |
| if (firstRender.current) { | |
| firstRender.current = false; | |
| init({ | |
| container: document.getElementById(SBI_CONTAINER_ID), | |
| buttonLabel: "scan_and_verify", | |
| transactionId: "transactionId", | |
| sbiEnv: DEFAULT_SBI_ENV, | |
| langCode: "en", | |
| disable: false, | |
| }); | |
| return; | |
| } | |
| propChange({ | |
| onCapture: (response: BiometricResponse | null) => | |
| authenticateBiometricResponse(response), | |
| }); | |
| }, []); | |
| const [, setValue] = useState(""); | |
| useEffect(() => { | |
| init({ | |
| container: document.getElementById(SBI_CONTAINER_ID), | |
| buttonLabel: "scan_and_verify", | |
| transactionId: "transactionId", | |
| sbiEnv: DEFAULT_SBI_ENV, | |
| langCode: "en", | |
| disable: false, | |
| }); | |
| propChange({ | |
| onCapture: (response: BiometricResponse | null) => { | |
| void authenticateBiometricResponse(response); | |
| }, | |
| }); | |
| }, []); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@oidc-ui/src/components/SbiComponent.tsx` around lines 53 - 74, The effect
currently returns immediately after init because firstRender.current is true, so
propChange never gets registered and authenticateBiometricResponse is never
called; fix by ensuring propChange is invoked on mount as well — e.g., after
calling init (inside the same useEffect) also call propChange({ onCapture:
(response) => authenticateBiometricResponse(response) }) or move the propChange
registration out of the early-return branch so it runs both on the initial
render and subsequent renders; update the useEffect (which references
firstRender, init, propChange, authenticateBiometricResponse) accordingly and
keep any necessary cleanup but remove the premature return that prevents
propChange registration.
Comment on lines
+90
to
+99
| const { errorCode } = validateBiometricResponse(biometricResponse); | ||
|
|
||
| if (errorCode !== null) return; | ||
|
|
||
| const encoded = encodeBase64( | ||
| JSON.stringify(biometricResponse?.biometrics ?? []), | ||
| ); | ||
| setValue(encoded); | ||
| context.onInputChange(component.id, encoded); | ||
| }; |
There was a problem hiding this comment.
Clear form value on invalid capture to avoid stale biometric submission.
When validation fails at Line 92, the previous successful payload remains in context and can be submitted unintentionally.
Suggested fix
const { errorCode } = validateBiometricResponse(biometricResponse);
- if (errorCode !== null) return;
+ if (errorCode !== null) {
+ setValue("");
+ context.onInputChange(component.id, "");
+ return;
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@oidc-ui/src/components/SbiComponent.tsx` around lines 90 - 99, When
validateBiometricResponse(biometricResponse) returns a non-null errorCode, clear
the stored biometric value so a previous successful payload isn't left in the
form: call setValue with an empty string (or null per app convention) and call
context.onInputChange(component.id, sameEmptyValue) before returning; update the
early-return branch that checks errorCode to perform these clears (references:
validateBiometricResponse, biometricResponse, setValue, context.onInputChange,
component.id).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Tests