-
Notifications
You must be signed in to change notification settings - Fork 60
[ES-2765] [ES-2766] [ES-2767] identity verification flow from signup itself #842
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
Signed-off-by: sajid.mannikeri <[email protected]>
Signed-off-by: Sajid Mannikeri <[email protected]>
Signed-off-by: Zeeshan Mehboob <[email protected]>
Signed-off-by: Sajid Mannikeri <[email protected]>
Signed-off-by: Zeeshan Mehboob <[email protected]>
Signed-off-by: Sajid Mannikeri <[email protected]>
Signed-off-by: Zeeshan Mehboob <[email protected]>
Signed-off-by: Zeeshan Mehboob <[email protected]>
WalkthroughThis PR adds RP/OAuth configuration to backend properties, exposes RP config to the frontend, and implements an identity-verification flow: state generation/storage, authorization URL construction using rp.config, centralized dismissal callbacks, auto-redirect countdowns, and multiple component prop changes to use the new flow. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Browser
participant UI as Signup UI
participant Settings as Settings Service
participant Storage as localStorage
participant Auth as OAuth Authorization Endpoint
User->>UI: Click "Verify Identity"
UI->>Settings: Read `rp.config`
Settings-->>UI: rp.config (client_id, auth endpoint, redirect_uri_verification, scopes, acr_values, expiry)
UI->>Storage: generateState(stateObj) -> stores base64 JSON under key
Storage-->>UI: return encoded stateKey
UI->>Auth: Navigate to authorization_endpoint?state=stateKey&client_id=...
Auth-->>User: Perform identity verification (external flow)
Auth->>UI: Redirect to redirect_uri_verification with state param
UI->>Storage: getStateData(state) -> decode & validate expiry
alt state valid
UI->>UI: handleDismiss() -> navigate to ROOT_ROUTE or continue flow
else state missing/expired
UI->>UI: handleDismiss({ key, error })
end
UI-->>User: show result (countdown/redirect or error)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
signup-ui/src/pages/SignUpPage/PhoneStatus/PhoneStatus.tsx (1)
53-60: Guard against missingrp.configbefore dereference.Line 56 can throw if
response.configsorrp.configare absent; optional chaining chain breaks at.configs["rp.config"].redirect_uri_signin. Add complete optional chaining and a guard before redirect, consistent with the pattern used inAccountRegistrationStatusLayout.tsx(line 29).🛠️ Suggested guard
- window.location.href = getSignInRedirectURLV2( - settings?.response.configs["rp.config"].redirect_uri_signin, - fromSignInHash, - search, - "/signup" - ); + const redirectUri = + settings?.response?.configs?.["rp.config"]?.redirect_uri_signin; + if (!redirectUri) return; + window.location.href = getSignInRedirectURLV2( + redirectUri, + fromSignInHash, + search, + "/signup" + );signup-ui/src/pages/EkycVerificationPage/VerificationScreen/VerificationScreen.tsx (1)
104-130: Missing dependency in useEffect.The
hashCodevariable is used within theuseEffectcallback (line 120) but is not included in the dependency array (line 130). This can cause stale closure issues where the callback uses an outdatedhashCodevalue.Suggested fix
- }, [webSocketError]); + }, [webSocketError, hashCode, handleDismiss, t]);signup-ui/src/components/session-alert.tsx (1)
77-84: Guard against missing rp.config redirect URI.
If settings aren’t loaded orrp.configis absent,getSignInRedirectURLV2receivesundefined, which can lead to malformed redirects. Consider a fallback to the legacy key or short-circuit with a safe default.🔧 Suggested fix
- window.location.href = getSignInRedirectURLV2( - settings?.response.configs["rp.config"].redirect_uri_signin, + const redirectUri = + settings?.response.configs["rp.config"]?.redirect_uri_signin ?? + settings?.response.configs["signin.redirect-url"]; + if (!redirectUri) return; + window.location.href = getSignInRedirectURLV2( + redirectUri, fromSignInHash, search, "/signup" );signup-ui/src/pages/EkycVerificationPage/IdentityVerificationStatus/IdentityVerificationStatus.tsx (1)
88-99: Same issue: side effect in render body.This
handleDismisscall has the same re-render risk. Consolidate both error handling cases into a singleuseEffectthat handles all error scenarios.Consolidated fix
+ useEffect(() => { + if (isIdentityVerificationStatusError) { + handleDismiss({ + key: hashCode?.state || "", + error: "ekyc_failed", + }); + return; + } + + if ( + identityVerificationStatus?.errors && + identityVerificationStatus.errors.length > 0 && + !retriableErrorCodes.includes( + identityVerificationStatus.errors[0].errorCode + ) + ) { + handleDismiss({ + key: hashCode?.state || "", + error: identityVerificationStatus.errors[0].errorCode, + }); + } + }, [ + isIdentityVerificationStatusError, + identityVerificationStatus?.errors, + retriableErrorCodes, + handleDismiss, + hashCode?.state, + ]);Then remove both inline
ifblocks that callhandleDismiss.
🤖 Fix all issues with AI agents
In
`@signup-ui/src/pages/EkycVerificationPage/IdentityVerificationStatus/components/IdentityVerificationStatusLayout.tsx`:
- Around line 64-74: The countdown is currently shown whenever status ===
"success" even if autoRedirect is false; update the
IdentityVerificationStatusLayout so the <Trans> countdown block is only rendered
when both status === "success" and autoRedirect are true (use the existing
autoRedirect prop/variable), keeping the same Trans props and values
(CountDownSpan and countdownDisplay) so the timer is only visible when the
auto-redirect behavior is enabled.
In
`@signup-ui/src/pages/EkycVerificationPage/IdentityVerificationStatus/IdentityVerificationStatus.tsx`:
- Around line 79-84: The code calls handleDismiss directly during render when
isIdentityVerificationStatusError is true, which can cause infinite re-renders;
move that call into a useEffect within the IdentityVerificationStatus component
so it runs after render: create a useEffect that watches
isIdentityVerificationStatusError, hashCode?.state (and handleDismiss if it's
not stable), and inside the effect call handleDismiss({ key: hashCode?.state ||
"", error: "ekyc_failed" }) only when isIdentityVerificationStatusError is true;
ensure the dependency array contains the referenced symbols to avoid stale
closures.
In `@signup-ui/src/pages/LandingPage/LandingPage.tsx`:
- Around line 41-53: The handler handleVerifyIdentity must guard against
settings.response.configs["rp.config"] being undefined before calling
generateState and navigate; update handleVerifyIdentity to check rpConfig (and
required fields like redirect_uri_verification, expiry_time, scope, acr_values)
and either early-return (or show an error) when missing, or disable the verify
button until settings are ready; keep resetEkycVerificationStore and navigate to
EKYC_VERIFICATION + fromSignInHash only after rpConfig validation so
generateState is called with defined values.
In `@signup-ui/src/pages/ResetPasswordPage/UserInfo/UserInfo.tsx`:
- Around line 213-215: The access to
settings?.response.configs["rp.config"].redirect_uri_signin can throw if
configs["rp.config"] is undefined; update the call to getSignInRedirectURLV2 to
use optional chaining on the intermediate object (e.g.,
settings?.response.configs["rp.config"]?.redirect_uri_signin) so the value
safely resolves to undefined when missing, keeping the existing fromSignInHash
argument unchanged and ensuring getSignInRedirectURLV2 receives a possibly
undefined redirect URI instead of throwing.
In
`@signup-ui/src/pages/SignUpPage/AccountRegistrationStatus/components/AccountRegistrationStatusLayout.tsx`:
- Around line 29-38: The handler currently calls getSignInRedirectURLV2 with
rpConfig?.redirect_uri_signin which may be undefined and produce a malformed
URL; update handleAction in AccountRegistrationStatusLayout to first guard that
rpConfig and rpConfig.redirect_uri_signin are present (e.g., return early or
show an error) before calling getSignInRedirectURLV2, or alternatively update
getSignInRedirectURLV2 to explicitly handle undefined redirectUrl and return a
safe fallback; target the symbols rpConfig, handleAction, and
getSignInRedirectURLV2 when making the change.
In
`@signup-ui/src/pages/SignUpPage/AccountSetup/components/UploadFileErrorModal.tsx`:
- Around line 19-23: The redirect call uses
settings?.response.configs["rp.config"].redirect_uri_signin which can be
undefined and will throw; update the code in UploadFileErrorModal (around the
getSignInRedirectURLV2 invocation) to guard access with optional chaining (e.g.,
settings?.response?.configs?.["rp.config"]?.redirect_uri_signin) and provide a
safe fallback value or disable the redirect button when missing; ensure
getSignInRedirectURLV2 is only called when a non-empty redirect_uri_signin is
available (or pass a default URL) and keep fromSignInHash and search parameters
unchanged.
In `@signup-ui/src/utils/identityVerificationUtil.ts`:
- Around line 32-49: The function removeEncodedToken currently returns a
URL-encoded string which causes double-encoding downstream; update
removeEncodedToken to return raw, space-delimited ACR tokens instead.
Specifically, change the empty-acr branch to return the unencoded token list
with spaces (e.g. "mosip:idp:acr:generated-code mosip:idp:acr:password ..."),
decode the incoming acr (keep decodeURIComponent(acr)), split and filter as you
already do, and replace the final encodeURIComponent(...).replace(...) chain
with simply return filtered.join(" ") so the query builder/URLSearchParams can
perform encoding once. Ensure you only modify removeEncodedToken and preserve
token matching logic.
- Around line 7-10: The randomKey function uses only a timestamp which is
predictable; update the implementation of randomKey to use cryptographically
secure randomness by incorporating crypto.randomUUID() (e.g.,
`${prefix}-${crypto.randomUUID()}` or combine UUID + timestamp) instead of new
Date().getTime(), keeping the function signature randomKey(prefix: string):
string and ensuring crypto.randomUUID() is used for the state generation to make
the OAuth state parameter unpredictable.
- Around line 58-105: The code uses btoa/atob which break on non-ASCII
characters; update generateState and getStateData to use UTF-8–safe base64
helpers instead: implement encodeBase64Utf8(str) and decodeBase64Utf8(b64)
(which UTF-8 encode/decode before base64 operations) and replace all btoa/atob
calls so stateKey, stateData (JSON) and the returned encoded stateKey are passed
through these helpers; ensure getStateData still wraps decoding in try/catch and
removes expired items as before.
♻️ Duplicate comments (4)
signup-ui/src/pages/SignUpPage/Phone/Phone.tsx (1)
174-179: Samerp.configdereference guard as noted in PhoneStatus.tsx.signup-ui/src/pages/SignUpPage/SignUpPopover.tsx (1)
35-42: Samerp.configdereference guard as noted in PhoneStatus.tsx.signup-ui/src/pages/ResetPasswordPage/ResetPasswordConfirmation/components/ResetPasswordConfirmationLayout.tsx (1)
26-33: Samerp.configdereference guard as noted in PhoneStatus.tsx.signup-ui/src/pages/ResetPasswordPage/ResetPasswordPopover.tsx (1)
51-56: Samerp.configdereference guard as noted in PhoneStatus.tsx.
🧹 Nitpick comments (7)
signup-ui/src/pages/EkycVerificationPage/IdentityVerificationStatus/components/IdentityVerificationStatusLayout.tsx (1)
29-37: Synthetic event cast is acceptable but worth noting.The timer's
onExpirecallback creates an empty object cast asReact.MouseEvent<HTMLButtonElement>to invokeonBtnClick. This works for the current use case where the handler likely doesn't inspect event properties, but could cause issues if the handler is later modified to access event details liketargetorpreventDefault().An alternative approach would be to separate the redirect logic into its own function that both the button and timer can call.
signup-ui/src/pages/EkycVerificationPage/EkycVerificationPopover.tsx (1)
21-25: Consider extracting the inline type to a shared interface.The
{ handleDismiss: (args: { key: string; error: string }) => void }type is used across multiple components in this PR (LoadingScreen, IdentityVerificationStatus, etc.). Consider extracting it totypes.tsfor consistency and reusability.Example shared type
// In types.ts export interface HandleDismissProps { handleDismiss: (args: { key: string; error: string }) => void; }signup-ui/src/pages/SignUpPage/AccountRegistrationStatus/components/AccountRegistrationStatusLayout.tsx (1)
41-52: Duplicated logic withLandingPage.tsx.The
handleVerifyIdentityfunction is nearly identical to the one inLandingPage.tsx. Consider extracting this into a shared hook or utility to avoid duplication.Example custom hook
// useVerifyIdentity.ts export const useVerifyIdentity = () => { const navigate = useNavigate(); const { i18n } = useTranslation(); const { data: settings } = useSettings(); const resetEkycVerificationStore = useEkycVerificationStore.getState().reset; const handleVerifyIdentity = (fromSignInHash: string) => { const rpConfig = settings?.response?.configs["rp.config"]; if (!rpConfig) return; resetEkycVerificationStore(); const state = generateState({ redirectUrl: rpConfig.redirect_uri_verification, expiryTime: rpConfig.expiry_time, scope: rpConfig.scope, acrValues: rpConfig.acr_values, uiLocales: i18n.language, }); navigate(`${EKYC_VERIFICATION}${fromSignInHash}?state=${state}`); }; return { handleVerifyIdentity, isReady: !!settings?.response?.configs["rp.config"] }; };signup-ui/src/pages/EkycVerificationPage/IdentityVerificationStatus/IdentityVerificationStatusFailed.tsx (1)
12-14: Unused props fromDefaultEkyVerificationProp.The component only destructures
handleDismissbutDefaultEkyVerificationPropalso includessettingsandcancelPopup. Consider either:
- Destructuring only what's needed if the interface is intentionally broad
- Using a narrower type if these props are truly unused
Option: Destructure explicitly to clarify usage
export const IdentityVerificationStatusFailed = ({ handleDismiss, -}: DefaultEkyVerificationProp) => { +}: Pick<DefaultEkyVerificationProp, 'handleDismiss'>) => {signup-ui/src/utils/identityVerificationUtil.ts (1)
16-21: Prefer snapshot iteration when deleting localStorage keys.Removing items while iterating a
Storageobject can skip keys in some browsers.♻️ Proposed refactor
const clearLocalStorageKey = (keyPrefix: string) => { - for (let key in localStorage) { - if (key.startsWith(keyPrefix)) { - localStorage.removeItem(key); - } - } + const keysToRemove: string[] = []; + for (let i = 0; i < localStorage.length; i++) { + const key = localStorage.key(i); + if (key && key.startsWith(keyPrefix)) keysToRemove.push(key); + } + keysToRemove.forEach((key) => localStorage.removeItem(key)); };signup-ui/src/pages/EkycVerificationPage/useEkycVerificationStore.tsx (1)
23-34: ReuseinitialStatefor initialization and reset.The store currently duplicates defaults. Spreading
initialStateinto the initializer avoids drift and a shared reference during reset.♻️ Proposed refactor
export const useEkycVerificationStore = create<EkycVerificationStore>()( devtools((set, get) => ({ - step: EkycVerificationStep.VerificationSteps, - criticalError: null, - kycProvider: null, - kycProviderDetail: null, - kycProvidersList: null, - hashCode: null, - isNoBackground: false, - errorBannerMessage: null, - slotId: null, - providerListStatus: false, + ...initialState, @@ - reset: () => set(() => initialState) + reset: () => set(() => ({ ...initialState })) })) );Also applies to: 57-122
signup-ui/src/pages/EkycVerificationPage/EkycVerificationPage.tsx (1)
154-178: Avoid falling through tonavigateToLandingPage()after a redirect.Once
authorizeUser()is invoked, returning early prevents an unnecessary SPA navigation and avoids double invocation whenstateandid_token_hintcoexist.💡 Optional refactor
if (stateData) { authorizeUser({ state: stateValue, redirect_uri: stateData.redirectUrl, scope: stateData.scope, acr_values: stateData.acrValues, claims: stateData.claims, ui_locales: stateData.uiLocales, }); + return; } } // Authorize user if id_token_hint is present in query params if (searchParams.has("id_token_hint")) { authorizeUser({ state: stateValue, id_token_hint: searchParams.get("id_token_hint") ?? "", }); + return; } navigateToLandingPage();
...cVerificationPage/IdentityVerificationStatus/components/IdentityVerificationStatusLayout.tsx
Show resolved
Hide resolved
...-ui/src/pages/EkycVerificationPage/IdentityVerificationStatus/IdentityVerificationStatus.tsx
Show resolved
Hide resolved
| const handleVerifyIdentity = (e: any) => { | ||
| e.preventDefault(); | ||
| resetEkycVerificationStore(); | ||
| const rpConfig = settings?.response?.configs["rp.config"]; | ||
| const state = generateState({ | ||
| redirectUrl: rpConfig?.redirect_uri_verification, | ||
| expiryTime: rpConfig?.expiry_time, | ||
| scope: rpConfig?.scope, | ||
| acrValues: rpConfig?.acr_values, | ||
| uiLocales: i18n.language, | ||
| }); | ||
| navigate(`${EKYC_VERIFICATION}${fromSignInHash}?state=${state}`); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard against undefined rpConfig before navigating.
If settings hasn't loaded yet (or fails to load), rpConfig will be undefined, causing generateState to receive undefined values for critical parameters like redirectUrl. Consider disabling the button or adding a guard.
Suggested guard
const handleVerifyIdentity = (e: any) => {
e.preventDefault();
+ if (!settings?.response?.configs["rp.config"]) {
+ return;
+ }
resetEkycVerificationStore();
const rpConfig = settings?.response?.configs["rp.config"];Or disable the button when settings aren't ready:
<Button
className="h-[52px] w-[250px] border-[2px] border-primary bg-white text-primary hover:text-primary/80 md:mt-3 md:w-full"
id="verify-identity-button"
name="verify-identity-button"
variant="outline"
onClick={handleVerifyIdentity}
+ disabled={!settings?.response?.configs["rp.config"]}
>📝 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.
| const handleVerifyIdentity = (e: any) => { | |
| e.preventDefault(); | |
| resetEkycVerificationStore(); | |
| const rpConfig = settings?.response?.configs["rp.config"]; | |
| const state = generateState({ | |
| redirectUrl: rpConfig?.redirect_uri_verification, | |
| expiryTime: rpConfig?.expiry_time, | |
| scope: rpConfig?.scope, | |
| acrValues: rpConfig?.acr_values, | |
| uiLocales: i18n.language, | |
| }); | |
| navigate(`${EKYC_VERIFICATION}${fromSignInHash}?state=${state}`); | |
| }; | |
| const handleVerifyIdentity = (e: any) => { | |
| e.preventDefault(); | |
| if (!settings?.response?.configs["rp.config"]) { | |
| return; | |
| } | |
| resetEkycVerificationStore(); | |
| const rpConfig = settings?.response?.configs["rp.config"]; | |
| const state = generateState({ | |
| redirectUrl: rpConfig?.redirect_uri_verification, | |
| expiryTime: rpConfig?.expiry_time, | |
| scope: rpConfig?.scope, | |
| acrValues: rpConfig?.acr_values, | |
| uiLocales: i18n.language, | |
| }); | |
| navigate(`${EKYC_VERIFICATION}${fromSignInHash}?state=${state}`); | |
| }; |
🤖 Prompt for AI Agents
In `@signup-ui/src/pages/LandingPage/LandingPage.tsx` around lines 41 - 53, The
handler handleVerifyIdentity must guard against
settings.response.configs["rp.config"] being undefined before calling
generateState and navigate; update handleVerifyIdentity to check rpConfig (and
required fields like redirect_uri_verification, expiry_time, scope, acr_values)
and either early-return (or show an error) when missing, or disable the verify
button until settings are ready; keep resetEkycVerificationStore and navigate to
EKYC_VERIFICATION + fromSignInHash only after rpConfig validation so
generateState is called with defined values.
...rc/pages/SignUpPage/AccountRegistrationStatus/components/AccountRegistrationStatusLayout.tsx
Show resolved
Hide resolved
signup-ui/src/pages/SignUpPage/AccountSetup/components/UploadFileErrorModal.tsx
Show resolved
Hide resolved
| const removeEncodedToken = (acr: string, tokenToRemove: string): string => { | ||
| if (acr === "") { | ||
| return "mosip:idp:acr:generated-code%20mosip:idp:acr:password%20mosip:idp:acr:linked-wallet%20mosip:idp:acr:knowledge"; | ||
| } | ||
| // 1) Decode to operate on plain text | ||
| const decoded = decodeURIComponent(acr); // %20 -> ' ' | ||
|
|
||
| // 2) Split on whitespace (handles multiple spaces just in case) | ||
| const tokens = decoded.trim().split(/\s+/); | ||
|
|
||
| // 3) Filter out the target token (exact match) | ||
| const filtered = tokens.filter((t) => t !== tokenToRemove); | ||
|
|
||
| // 4) Re-encode; keep ':' readable, optionally also keep '-' safe | ||
| // encodeURIComponent encodes ':', so we bring it back for readability. | ||
| return encodeURIComponent(filtered.join(" ")) | ||
| .replace(/%3A/g, ":") | ||
| .replace(/%2D/g, "-"); // optional: keep dashes readable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
acr_values will be double-encoded downstream.
This helper returns a URL-encoded string, and later URLSearchParams encodes again, turning %20 into %2520. That can cause the IdP to receive an invalid ACR list. Return raw space-delimited tokens and let the query builder encode once.
🐛 Proposed fix
- if (acr === "") {
- return "mosip:idp:acr:generated-code%20mosip:idp:acr:password%20mosip:idp:acr:linked-wallet%20mosip:idp:acr:knowledge";
- }
+ if (acr === "") {
+ return "mosip:idp:acr:generated-code mosip:idp:acr:password mosip:idp:acr:linked-wallet mosip:idp:acr:knowledge";
+ }
@@
- return encodeURIComponent(filtered.join(" "))
- .replace(/%3A/g, ":")
- .replace(/%2D/g, "-"); // optional: keep dashes readable
+ return filtered.join(" ");
};📝 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.
| const removeEncodedToken = (acr: string, tokenToRemove: string): string => { | |
| if (acr === "") { | |
| return "mosip:idp:acr:generated-code%20mosip:idp:acr:password%20mosip:idp:acr:linked-wallet%20mosip:idp:acr:knowledge"; | |
| } | |
| // 1) Decode to operate on plain text | |
| const decoded = decodeURIComponent(acr); // %20 -> ' ' | |
| // 2) Split on whitespace (handles multiple spaces just in case) | |
| const tokens = decoded.trim().split(/\s+/); | |
| // 3) Filter out the target token (exact match) | |
| const filtered = tokens.filter((t) => t !== tokenToRemove); | |
| // 4) Re-encode; keep ':' readable, optionally also keep '-' safe | |
| // encodeURIComponent encodes ':', so we bring it back for readability. | |
| return encodeURIComponent(filtered.join(" ")) | |
| .replace(/%3A/g, ":") | |
| .replace(/%2D/g, "-"); // optional: keep dashes readable | |
| const removeEncodedToken = (acr: string, tokenToRemove: string): string => { | |
| if (acr === "") { | |
| return "mosip:idp:acr:generated-code mosip:idp:acr:password mosip:idp:acr:linked-wallet mosip:idp:acr:knowledge"; | |
| } | |
| // 1) Decode to operate on plain text | |
| const decoded = decodeURIComponent(acr); // %20 -> ' ' | |
| // 2) Split on whitespace (handles multiple spaces just in case) | |
| const tokens = decoded.trim().split(/\s+/); | |
| // 3) Filter out the target token (exact match) | |
| const filtered = tokens.filter((t) => t !== tokenToRemove); | |
| // 4) Return raw space-delimited tokens; let query builder handle encoding | |
| return filtered.join(" "); | |
| }; |
🤖 Prompt for AI Agents
In `@signup-ui/src/utils/identityVerificationUtil.ts` around lines 32 - 49, The
function removeEncodedToken currently returns a URL-encoded string which causes
double-encoding downstream; update removeEncodedToken to return raw,
space-delimited ACR tokens instead. Specifically, change the empty-acr branch to
return the unencoded token list with spaces (e.g. "mosip:idp:acr:generated-code
mosip:idp:acr:password ..."), decode the incoming acr (keep
decodeURIComponent(acr)), split and filter as you already do, and replace the
final encodeURIComponent(...).replace(...) chain with simply return
filtered.join(" ") so the query builder/URLSearchParams can perform encoding
once. Ensure you only modify removeEncodedToken and preserve token matching
logic.
Signed-off-by: Zeeshan Mehboob <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@signup-ui/src/pages/LandingPage/LandingPage.tsx`:
- Around line 45-52: The generated state (from generateState) is being appended
raw to the query string when calling
navigate(`${EKYC_VERIFICATION}${fromSignInHash}?state=${state}`), which can
break parsing because Base64 contains characters like +,/ and =; update the code
to URL-encode the state (e.g., via encodeURIComponent or equivalent) before
building the URL so getStateData can reliably decode it. Locate the navigate
call that uses EKYC_VERIFICATION and fromSignInHash and replace the raw state
with the encoded state returned from encoding the value returned by
generateState.
♻️ Duplicate comments (2)
signup-ui/src/pages/LandingPage/LandingPage.tsx (1)
41-45: Guard against missingrp.config, not justsettings.
settingsbeing defined doesn’t guaranteesettings.response.configs["rp.config"]exists; the handler can still proceed with undefined fields.✅ Suggested guard
const handleVerifyIdentity = (e: any) => { e.preventDefault(); + if (!settings?.response?.configs["rp.config"]) return; resetEkycVerificationStore(); const rpConfig = settings?.response?.configs["rp.config"];- disabled={settings !== undefined ? false : true} + disabled={!settings?.response?.configs["rp.config"]}Also applies to: 86-93
signup-ui/src/utils/identityVerificationUtil.ts (1)
45-62:removeEncodedTokenstill returns URL‑encoded ACR values.This will be encoded again by query builders, producing
%2520and breaking IdP parsing. Return a raw space‑delimited list and let the URL builder encode once.🔧 Proposed fix
- if (acr === "") { - return "mosip:idp:acr:generated-code%20mosip:idp:acr:password%20mosip:idp:acr:linked-wallet%20mosip:idp:acr:knowledge"; - } + if (acr === "") { + return "mosip:idp:acr:generated-code mosip:idp:acr:password mosip:idp:acr:linked-wallet mosip:idp:acr:knowledge"; + } @@ - return encodeURIComponent(filtered.join(" ")) - .replace(/%3A/g, ":") - .replace(/%2D/g, "-"); // optional: keep dashes readable + return filtered.join(" "); };
| const state = generateState({ | ||
| redirectUrl: rpConfig?.redirect_uri_verification, | ||
| expiryTime: rpConfig?.expiry_time, | ||
| scope: rpConfig?.scope, | ||
| acrValues: rpConfig?.acr_values, | ||
| uiLocales: i18n.language, | ||
| }); | ||
| navigate(`${EKYC_VERIFICATION}${fromSignInHash}?state=${state}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
URL‑encode the generated state before appending to the query string.
Base64 can include +, /, = which can be transformed by query parsing (e.g., + → space), breaking getStateData.
🔧 Proposed fix
- navigate(`${EKYC_VERIFICATION}${fromSignInHash}?state=${state}`);
+ navigate(
+ `${EKYC_VERIFICATION}${fromSignInHash}?state=${encodeURIComponent(state)}`
+ );📝 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.
| const state = generateState({ | |
| redirectUrl: rpConfig?.redirect_uri_verification, | |
| expiryTime: rpConfig?.expiry_time, | |
| scope: rpConfig?.scope, | |
| acrValues: rpConfig?.acr_values, | |
| uiLocales: i18n.language, | |
| }); | |
| navigate(`${EKYC_VERIFICATION}${fromSignInHash}?state=${state}`); | |
| const state = generateState({ | |
| redirectUrl: rpConfig?.redirect_uri_verification, | |
| expiryTime: rpConfig?.expiry_time, | |
| scope: rpConfig?.scope, | |
| acrValues: rpConfig?.acr_values, | |
| uiLocales: i18n.language, | |
| }); | |
| navigate( | |
| `${EKYC_VERIFICATION}${fromSignInHash}?state=${encodeURIComponent(state)}` | |
| ); |
🤖 Prompt for AI Agents
In `@signup-ui/src/pages/LandingPage/LandingPage.tsx` around lines 45 - 52, The
generated state (from generateState) is being appended raw to the query string
when calling navigate(`${EKYC_VERIFICATION}${fromSignInHash}?state=${state}`),
which can break parsing because Base64 contains characters like +,/ and =;
update the code to URL-encode the state (e.g., via encodeURIComponent or
equivalent) before building the URL so getStateData can reliably decode it.
Locate the navigate call that uses EKYC_VERIFICATION and fromSignInHash and
replace the raw state with the encoded state returned from encoding the value
returned by generateState.
gk-XL7
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zesu22 plz check my comment and also i see there are few major and critical issues raised by coderabbit.
| ...(queryParam.claims && { claims: queryParam.claims }), | ||
| ...(queryParam.id_token_hint && { | ||
| id_token_hint: queryParam.id_token_hint, | ||
| acr_values: "mosip:idp:acr:id-token", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zesu22 i see 2 acr_values being used, can we use at one place?
The user can initiate the verification flow either from the landing page or after completing registration. A button to start the verification process will be available on both pages.
After the verification is successfully completed, a popup modal will appear. It will remain visible for a configurable duration (e.g., 10 seconds) before redirecting the user back to the page from which the flow was initiated—either the eSignet consent page or the signup landing page.
Summary by CodeRabbit
New Features
Behavior
Localization
✏️ Tip: You can customize this high-level summary in your review settings.