Feature/auth fixes and react sdk tests#242
Merged
Merged
Conversation
Contributor
|
Warning Review limit reached
More reviews will be available in 17 minutes and 15 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds soft-delete denial across auth endpoints and makes social exchange atomic via redis.getdel (tests updated); improves SDK error parsing and client init; introduces a React SDK (provider, hooks, UrAuth/Toast, tests) and a Vite+TS demo; updates dashboard styles, copy-key UX, and workspace/package versions. ChangesAuth Security & Social Token Exchange
SDK Error Handling & Docs
React SDK & Demo Application
Dashboard UI & Metadata Updates
Sequence Diagram(s)sequenceDiagram
participant Browser
participant UrProvider
participant AuthModule
participant Redis
Browser->>UrProvider: page load (location.search / location.hash)
UrProvider->>AuthModule: setToken(token) (if token in hash)
UrProvider->>AuthModule: socialExchange({token, rtCode}) (if rtCode present)
AuthModule->>Redis: getdel(exchangeKey) (atomic)
Redis-->>AuthModule: exchangePayload or null
alt payload exists and matches
AuthModule->>UrProvider: set session / issue refresh
UrProvider-->>Browser: render authenticated UI
else missing/invalid
AuthModule-->>UrProvider: error -> show redirect/error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ 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 |
Collaborator
Author
|
@coderabbitai |
Contributor
|
✅ Actions performedReview triggered.
|
Collaborator
Author
|
are u doing/? @coderabbitai |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
|
Collaborator
Author
|
@coderabbitai |
Contributor
🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check the current status of PR `#242` reviews and comments
gh pr view 242 --repo geturbackend/urBackend --json reviews,state,title,headRefName 2>&1Length of output: 750 🏁 Script executed: #!/bin/bash
# Check recent review comments on PR `#242`
gh api repos/geturbackend/urBackend/pulls/242/reviews 2>&1 | jq '[.[] | {id: .id, user: .user.login, state: .state, submitted_at: .submitted_at, body: .body[:200]}]' 2>&1Length of output: 555
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/public-api/src/controllers/userAuth.controller.js (1)
903-952:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize the exchange endpoint's error contract and stop leaking backend errors.
The success path uses
{ success, data, message }, but the error branches omitdata, and the catch block forwards rawerr.message. That breaks the controller contract and can expose Redis/internal failure details to clients. This endpoint should return the same envelope on every branch and map unexpected failures to a genericAppError/message.As per coding guidelines: "All API endpoints must return
{ success: bool, data: {}, message: "" }format. Use AppError class for errors—never raw throw and never expose MongoDB errors to client."🤖 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 `@apps/public-api/src/controllers/userAuth.controller.js` around lines 903 - 952, The exchangeSocialRefreshToken handler currently returns different response shapes and leaks err.message; update all branches (including the early 400/403 returns and the catch) to always return the envelope { success: bool, data: {}, message: "" } (e.g., data: {} for error paths) and replace any raw backend error text with a generic message or an AppError instance; specifically, in exchangeSocialRefreshToken adjust the responses for missing rtCode/token, invalid/expired rawExchange (from redis.getdel), malformed JSON, and invalid payload (parsedExchange) to include data: {} and a clear client-facing message, and in the catch block do not forward err.message—return status 500 with data: {} and a generic "Internal server error" (or an AppError) so Redis/internal errors are never exposed.sdks/urbackend-python/README.md (1)
148-158:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClarify client-side token extraction from URL fragment.
The example shows
parse_fragment(request).get("token")to extract the token from the URL fragment, but URL fragments are never sent to the server in HTTP requests—they're client-side only.The backend cannot directly parse
#token=...from an incoming request. This token must be extracted in the browser (using JavaScript likenew URLSearchParams(window.location.hash.slice(1))) and then passed to the backend via a separate request parameter, request body, or header.📝 Suggested documentation fix
-# 2. After the user returns to <siteUrl>/auth/callback?rtCode=...#token=... -# exchange BOTH the rtCode and the one-time token (backend requires both) -rt_code = request.GET.get("rtCode") -# Token must be parsed from the URL fragment (`#token`=...), not query string -token = parse_fragment(request).get("token") -session = client.auth.social_exchange(rt_code, token) +# 2. After the user returns to <siteUrl>/auth/callback?rtCode=...#token=... +# The rtCode is in the query string, but the token is in the URL fragment. +# URL fragments are client-side only and never sent to the server. +# +# Client-side (JavaScript): +# const params = new URLSearchParams(window.location.search); +# const rtCode = params.get("rtCode"); +# const token = new URLSearchParams(window.location.hash.slice(1)).get("token"); +# // Send both to your backend endpoint via POST or query params +# +# Backend (Python): +rt_code = request.GET.get("rtCode") # or from POST body +token = request.GET.get("token") # or from POST body (sent by client-side JS) +session = client.auth.social_exchange(rt_code, 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 `@sdks/urbackend-python/README.md` around lines 148 - 158, The README example incorrectly suggests extracting a one-time token from the URL fragment on the server using parse_fragment(request); fragments are never sent to the server. Update the docs to instruct developers to extract the token client-side (e.g., from window.location.hash) and then send it to the backend (as a POST body/query/header) so the backend endpoint can call client.auth.social_exchange(rt_code, token); mention parse_fragment only for client-side helpers or remove it and show that the server receives token via an explicit parameter before calling client.auth.social_exchange and then client.auth.refresh_token/session handling.
🧹 Nitpick comments (3)
apps/web-dashboard/src/index.css (1)
194-205: ⚡ Quick winDeduplicate
.glass-cardstyle blocks to avoid divergence.The same selector is defined twice with overlapping declarations and split hover behavior. Consolidating into a single block will prevent accidental drift.
Proposed consolidation
-.glass-card { - background: var(--color-glass-card-bg); - backdrop-filter: var(--glass-backdrop); - border: 1px solid var(--color-glass-card-border); - box-shadow: 0 4px 24px -8px rgba(0, 0, 0, 0.5); - transition: all 0.3s cubic-bezier(0.4, 0, 0.2, 1); -} - -.glass-card:hover { - border-color: rgba(255, 255, 255, 0.12); - box-shadow: 0 12px 32px -12px rgba(0, 0, 0, 0.6); -} @@ .glass-card { background: var(--color-glass-card-bg); backdrop-filter: var(--glass-backdrop); border: 1px solid var(--color-glass-card-border); box-shadow: 0 4px 24px -8px rgba(0, 0, 0, 0.5); transition: all 0.3s cubic-bezier(0.4, 0, 0.2, 1); } .glass-card:hover { border-color: rgba(255, 255, 255, 0.12); box-shadow: 0 12px 32px -12px rgba(0, 0, 0, 0.6); transform: translateY(-2px); }Also applies to: 222-234
🤖 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 `@apps/web-dashboard/src/index.css` around lines 194 - 205, The .glass-card styles are duplicated across multiple blocks, which can cause the base and hover declarations to drift apart. Consolidate the repeated .glass-card and .glass-card:hover rules into a single definition in index.css, keeping all shared properties together and preserving the hover state in one place so future changes only need to be made once.apps/public-api/src/__tests__/userAuth.social.test.js (1)
444-507: ⚡ Quick winAdd regression coverage for soft-deleted social users.
These updates cover the
getdelswap, but the new 403 branches infindOrCreateSocialUserstill aren't protected by tests for either provider-ID lookup or verified-email lookup. A couple of focused cases here would keep the auth-hardening change from silently regressing.Based on learnings: "When changing public auth behavior, verify changes in
userAuth.controller.js,userAuth.jsroutes,refreshToken.js, and corresponding test files."🤖 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 `@apps/public-api/src/__tests__/userAuth.social.test.js` around lines 444 - 507, Add targeted tests that assert findOrCreateSocialUser returns the 403 branches for soft-deleted social users by simulating both provider-id lookup and verified-email lookup paths: create two new test cases that mock the user lookup methods used by findOrCreateSocialUser (the provider ID lookup and the verifiedEmail lookup) to return a user object marked as soft-deleted, mock redis.getdel as needed, call controller.exchangeSocialRefreshToken (or directly call findOrCreateSocialUser if exported) with matching rtCode/token payloads, and assert the response status is 403 and the JSON matches the 'Invalid refresh token exchange payload' message; ensure mocks correspond to the same unique symbols in the diff (exchangeSocialRefreshToken and findOrCreateSocialUser) so the tests cover both provider-id and verified-email code paths to prevent regressions.sdks/urbackend-react/tests/context.test.tsx (1)
65-75: ⚡ Quick winAvoid reassigning
window.locationin this test.
window.locationis read-only/non-configurable in many JSDOM setups, sodelete window.locationmakes this test fragile.history.replaceState()can set the samepathname/search/hashvalues without patching browser globals.💡 Proposed fix
- const originalLocation = window.location; - // `@ts-ignore` - delete window.location; - window.location = { - ...originalLocation, - search: '?rtCode=test-rt-code', - hash: '`#token`=test-temp-token', - pathname: '/auth/callback', - replaceState: vi.fn(), - } as any; + const originalUrl = + window.location.pathname + window.location.search + window.location.hash; + window.history.replaceState( + {}, + '', + '/auth/callback?rtCode=test-rt-code#token=test-temp-token' + ); @@ - // Restore window.location - window.location = originalLocation; + window.history.replaceState({}, '', originalUrl);Also applies to: 102-103
🤖 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 `@sdks/urbackend-react/tests/context.test.tsx` around lines 65 - 75, Don't delete or reassign window.location; instead save originalLocation, use history.replaceState(null, '', '/auth/callback?rtCode=test-rt-code#token=test-temp-token') to set pathname/search/hash for the test (and revert with history.replaceState back to originalLocation.href afterwards), and remove the vi.fn replacement of window.location.replaceState; apply the same change to the other case that currently mutates window.location (the block using originalLocation and vi.fn).
🤖 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 `@apps/public-api/src/controllers/userAuth.controller.js`:
- Around line 26-34: The helper checkUserSoftDeleted only blocks
signup/login/public profile/social lookup; you must apply the same soft-delete
guard wherever code resolves an authenticated user: add checks in the
controllers/middleware used by refreshToken, me, updateProfile,
changePasswordUser, and resetPasswordUser so they call
checkUserSoftDeleted(user) (or inline equivalent) and return the deletion
message/error when a user.isDeleted is true; additionally, update the
refreshToken flow (refreshToken.js and its route in userAuth.js) to revoke the
entire session/token chain (invalidate refresh tokens and associated session
records) when a deleted user attempts refresh so existing sessions are
immediately terminated; update related tests to cover refresh attempts and
authorized mutations by soft-deleted users.
In `@apps/web-dashboard/src/pages/ProjectDetails.jsx`:
- Around line 96-103: The click handler currently calls
navigator.clipboard.writeText(newKey.key) without awaiting or handling errors,
then always shows toast.success; update the onClick handler used in the button
to await navigator.clipboard.writeText(newKey.key) inside a try/catch and call
toast.success(...) only on success and toast.error(...) in the catch with a
helpful message; keep the same button UI and title, but change the inline arrow
function to an async function that tries await
navigator.clipboard.writeText(newKey.key) and handles failures with toast.error.
In `@examples/react-sdk-demo/src/App.tsx`:
- Around line 7-8: The loading branch that checks isInitializing returns white
text on a white background making the app appear blank; update the JSX returned
when isInitializing (the inline-styled <div> currently showing "Loading
urBackend...") to use a readable combination—either set a contrasting background
(e.g., dark) or change the text color to a dark value and optionally add
accessible attributes (role="status", aria-live) so the loader is visible and
announced.
In `@examples/react-sdk-demo/src/main.tsx`:
- Around line 7-12: The app currently passes import.meta.env.VITE_UR_PUBLIC_KEY
and VITE_UR_BASE_URL directly into UrProvider which can produce cryptic runtime
errors if those env vars are missing; add an early check before calling
ReactDOM.createRoot/UrProvider to validate import.meta.env.VITE_UR_PUBLIC_KEY
and import.meta.env.VITE_UR_BASE_URL are present and non-empty, and if not, log
an explicit error (or throw) and stop initialization so the failure is fast and
actionable (update the code around ReactDOM.createRoot and the UrProvider
initialization to perform this guard).
- Line 1: The React import should be kept because <React.StrictMode> references
React; do not remove the line importing React in
examples/react-sdk-demo/src/main.tsx. Add a runtime guard in main.tsx to
validate import.meta.env.VITE_UR_PUBLIC_KEY before creating the UrProvider
(check for undefined/empty and show a clear error or prevent app bootstrap), and
add validation inside UrBackendClient (validate the apiKey in its constructor or
setter and throw a clear error if missing/empty) so UrProviderProps.apiKey:
string is enforced at runtime and missing envs fail fast with a helpful message;
reference the React import line, <React.StrictMode>, UrProviderProps,
UrBackendClient, and import.meta.env.VITE_UR_PUBLIC_KEY when making these
changes.
In `@sdks/urbackend-python/src/urbackend/auth.py`:
- Around line 405-410: The example in the auth callback docs incorrectly
attempts to read the fragment parameter on the server (request.GET.get("token"))
even though the fragment (token) is not sent to the server; update the example
in the docstring around rtCode/token so that it either (a) shows a client-side
fragment parser (e.g., parsing location.hash in JS and POSTing or redirecting
the token to the server callback) or (b) explains an explicit frontend handoff
step that reads the token from the fragment and forwards it to the server
endpoint that handles rtCode; ensure the text references the rtCode query param
and the token fragment and replace any server-side call like
request.GET.get("token") with the client-side handoff approach.
In `@sdks/urbackend-react/package.json`:
- Line 16: The package.json in sdks/urbackend-react currently pins the
dependency "`@urbackend/sdk`" to a local file path ("file:../urbackend-sdk");
replace that entry with the published semver version (e.g. "`@urbackend/sdk`":
"^0.4.2") so consumers can resolve the package outside this repo—update the
dependency string in sdks/urbackend-react/package.json and ensure package-lock
or yarn lock is updated accordingly.
In `@sdks/urbackend-react/src/components/Toast.tsx`:
- Around line 20-26: The nested exit timeout created inside the Toast component
(the inner setTimeout that calls onClose after 300ms) isn't being cleared on
unmount; change the effect so you store both timers (outer and inner) in
variables or refs (e.g., outerTimer and exitTimer) and clear both in the cleanup
function; specifically update the effect that calls setIsLeaving and
setTimeout(onClose, 300) to assign the inner timer to exitTimer and call
clearTimeout(outerTimer) and clearTimeout(exitTimer) in the return cleanup,
ensuring onClose cannot run after unmount.
In `@sdks/urbackend-react/src/components/UrAuth.tsx`:
- Around line 331-333: Replace the non-semantic clickable <span> elements used
for "Forgot password?" and the other clickable text in UrAuth.tsx with semantic
<button type="button"> elements so they are keyboard-accessible and focusable;
update the elements that call setMode('forgot') and clearError() (and the other
handlers using onClick) to use buttons, preserve the existing styles via the
styles.forgotLink class, and add an appropriate aria-label if the visible text
isn't fully descriptive to maintain accessibility.
- Around line 12-13: The default providers array in UrAuth.tsx currently
includes 'apple' even though the Apple flow is not implemented (the sign-in path
shows only an alert in the handler around the signIn/signInWithProvider code),
which ships a broken default; remove 'apple' from the default providers list
(providers = ['google', 'github']) and/or add a proper implementation for the
Apple flow where the current alert is shown (the handler around the
signInWithProvider / onClick for provider buttons). Update any related UI/prop
defaults that reference the same default (the block handling providers and the
provider button rendering around the current alert) so Apple is not advertised
or is wired to a real sign-in function. Ensure references to the provider names
(e.g., providers prop and the provider button renderer) are consistent after the
change.
In `@sdks/urbackend-react/src/context.tsx`:
- Around line 63-69: The catch for auth.socialExchange in the rtCode branch
currently swallows errors; change it so the error is propagated to fail the
callback flow (e.g., after logging rethrow the error or return a rejected
Promise) so the code does not continue with auth.me() using a short-lived
fragment token. Locate the rtCode handling where auth.socialExchange({ token,
rtCode }) is called and replace the silent catch with an error propagation
(throw err) or otherwise stop execution so the persistent refresh session
failure prevents the misleading “logged in until reload” state.
In `@sdks/urbackend-sdk/src/errors.ts`:
- Around line 57-78: The current 'error' branch in errors.ts (the block that
inspects errData.error and assigns to message) can assign unhelpful values like
"[]", "null" or JSON blobs and hide a useful errData.message; modify the 'error'
handling so you first derive the candidate string (same mapping logic for
string/array/object), then only set message = candidate if it's a non-empty
meaningful string (e.g. after String(...).trim() has length > 0 and is not the
literal "[]" or "null"); otherwise skip using errData.error and allow the
existing 'message' branch to run and provide the fallback. Ensure you keep the
same symbols (errData, message) and the existing mapping logic but gate
assignment on candidate validity.
In `@sdks/urbackend-sdk/src/modules/auth.ts`:
- Around line 504-513: The example for socialExchange is misleading because
auth.socialExchange returns a one-time refresh token payload (rtToken), not an
access token; update the example to read and use the returned refresh token
(e.g., response.rtToken or whatever key the API returns) and then call the SDK's
refresh flow (auth.refreshToken(...)) to obtain an access token; locate the
example block referencing auth.socialExchange and change the console.log and
subsequent usage to consume the refresh token and call auth.refreshToken with
that token.
---
Outside diff comments:
In `@apps/public-api/src/controllers/userAuth.controller.js`:
- Around line 903-952: The exchangeSocialRefreshToken handler currently returns
different response shapes and leaks err.message; update all branches (including
the early 400/403 returns and the catch) to always return the envelope {
success: bool, data: {}, message: "" } (e.g., data: {} for error paths) and
replace any raw backend error text with a generic message or an AppError
instance; specifically, in exchangeSocialRefreshToken adjust the responses for
missing rtCode/token, invalid/expired rawExchange (from redis.getdel), malformed
JSON, and invalid payload (parsedExchange) to include data: {} and a clear
client-facing message, and in the catch block do not forward err.message—return
status 500 with data: {} and a generic "Internal server error" (or an AppError)
so Redis/internal errors are never exposed.
In `@sdks/urbackend-python/README.md`:
- Around line 148-158: The README example incorrectly suggests extracting a
one-time token from the URL fragment on the server using
parse_fragment(request); fragments are never sent to the server. Update the docs
to instruct developers to extract the token client-side (e.g., from
window.location.hash) and then send it to the backend (as a POST
body/query/header) so the backend endpoint can call
client.auth.social_exchange(rt_code, token); mention parse_fragment only for
client-side helpers or remove it and show that the server receives token via an
explicit parameter before calling client.auth.social_exchange and then
client.auth.refresh_token/session handling.
---
Nitpick comments:
In `@apps/public-api/src/__tests__/userAuth.social.test.js`:
- Around line 444-507: Add targeted tests that assert findOrCreateSocialUser
returns the 403 branches for soft-deleted social users by simulating both
provider-id lookup and verified-email lookup paths: create two new test cases
that mock the user lookup methods used by findOrCreateSocialUser (the provider
ID lookup and the verifiedEmail lookup) to return a user object marked as
soft-deleted, mock redis.getdel as needed, call
controller.exchangeSocialRefreshToken (or directly call findOrCreateSocialUser
if exported) with matching rtCode/token payloads, and assert the response status
is 403 and the JSON matches the 'Invalid refresh token exchange payload'
message; ensure mocks correspond to the same unique symbols in the diff
(exchangeSocialRefreshToken and findOrCreateSocialUser) so the tests cover both
provider-id and verified-email code paths to prevent regressions.
In `@apps/web-dashboard/src/index.css`:
- Around line 194-205: The .glass-card styles are duplicated across multiple
blocks, which can cause the base and hover declarations to drift apart.
Consolidate the repeated .glass-card and .glass-card:hover rules into a single
definition in index.css, keeping all shared properties together and preserving
the hover state in one place so future changes only need to be made once.
In `@sdks/urbackend-react/tests/context.test.tsx`:
- Around line 65-75: Don't delete or reassign window.location; instead save
originalLocation, use history.replaceState(null, '',
'/auth/callback?rtCode=test-rt-code#token=test-temp-token') to set
pathname/search/hash for the test (and revert with history.replaceState back to
originalLocation.href afterwards), and remove the vi.fn replacement of
window.location.replaceState; apply the same change to the other case that
currently mutates window.location (the block using originalLocation and vi.fn).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 669f888a-928f-4a15-911f-58fdaf690aaf
⛔ Files ignored due to path filters (14)
examples/react-sdk-demo/package-lock.jsonis excluded by!**/package-lock.jsonexamples/react-sdk-demo/public/favicon.svgis excluded by!**/*.svgexamples/react-sdk-demo/public/icons.svgis excluded by!**/*.svgexamples/react-sdk-demo/src/assets/hero.pngis excluded by!**/*.pngexamples/react-sdk-demo/src/assets/react.svgis excluded by!**/*.svgexamples/react-sdk-demo/src/assets/vite.svgis excluded by!**/*.svgpackage-lock.jsonis excluded by!**/package-lock.jsonsdks/urbackend-react/dist/index.d.mtsis excluded by!**/dist/**sdks/urbackend-react/dist/index.d.tsis excluded by!**/dist/**sdks/urbackend-react/dist/index.jsis excluded by!**/dist/**sdks/urbackend-react/dist/index.js.mapis excluded by!**/dist/**,!**/*.mapsdks/urbackend-react/dist/index.mjsis excluded by!**/dist/**sdks/urbackend-react/dist/index.mjs.mapis excluded by!**/dist/**,!**/*.mapsdks/urbackend-sdk/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (42)
AGENTS.mdapps/consumer/package.jsonapps/dashboard-api/package.jsonapps/public-api/package.jsonapps/public-api/src/__tests__/userAuth.social.test.jsapps/public-api/src/controllers/userAuth.controller.jsapps/web-dashboard/package.jsonapps/web-dashboard/src/index.cssapps/web-dashboard/src/pages/ProjectDetails.jsxexamples/react-sdk-demo/.gitignoreexamples/react-sdk-demo/README.mdexamples/react-sdk-demo/eslint.config.jsexamples/react-sdk-demo/index.htmlexamples/react-sdk-demo/package.jsonexamples/react-sdk-demo/src/App.cssexamples/react-sdk-demo/src/App.tsxexamples/react-sdk-demo/src/index.cssexamples/react-sdk-demo/src/main.tsxexamples/react-sdk-demo/tsconfig.app.jsonexamples/react-sdk-demo/tsconfig.jsonexamples/react-sdk-demo/tsconfig.node.jsonexamples/react-sdk-demo/vite.config.tspackage.jsonpackages/common/package.jsonsdks/urbackend-python/README.mdsdks/urbackend-python/pyproject.tomlsdks/urbackend-python/src/urbackend/auth.pysdks/urbackend-react/package.jsonsdks/urbackend-react/src/components/Toast.tsxsdks/urbackend-react/src/components/UrAuth.tsxsdks/urbackend-react/src/context.tsxsdks/urbackend-react/src/hooks.tssdks/urbackend-react/src/index.tssdks/urbackend-react/tests/UrAuth.test.tsxsdks/urbackend-react/tests/context.test.tsxsdks/urbackend-react/tests/setupTests.tssdks/urbackend-react/tsconfig.jsonsdks/urbackend-react/tsup.config.tssdks/urbackend-react/vitest.config.tssdks/urbackend-sdk/package.jsonsdks/urbackend-sdk/src/errors.tssdks/urbackend-sdk/src/modules/auth.ts
Comment on lines
+7
to
+12
| ReactDOM.createRoot(document.getElementById('root')!).render( | ||
| <React.StrictMode> | ||
| <UrProvider | ||
| apiKey={import.meta.env.VITE_UR_PUBLIC_KEY} | ||
| baseUrl={import.meta.env.VITE_UR_BASE_URL} | ||
| > |
Contributor
There was a problem hiding this comment.
Fail fast when required env vars are missing.
If either Vite variable is unset, UrProvider is initialized with invalid config and the demo fails later with a much less actionable error.
🛡️ Proposed fix
+const apiKey = import.meta.env.VITE_UR_PUBLIC_KEY
+const baseUrl = import.meta.env.VITE_UR_BASE_URL
+
+if (!apiKey || !baseUrl) {
+ throw new Error('Missing VITE_UR_PUBLIC_KEY or VITE_UR_BASE_URL')
+}
+
ReactDOM.createRoot(document.getElementById('root')!).render(
<React.StrictMode>
<UrProvider
- apiKey={import.meta.env.VITE_UR_PUBLIC_KEY}
- baseUrl={import.meta.env.VITE_UR_BASE_URL}
+ apiKey={apiKey}
+ baseUrl={baseUrl}
>📝 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
| ReactDOM.createRoot(document.getElementById('root')!).render( | |
| <React.StrictMode> | |
| <UrProvider | |
| apiKey={import.meta.env.VITE_UR_PUBLIC_KEY} | |
| baseUrl={import.meta.env.VITE_UR_BASE_URL} | |
| > | |
| const apiKey = import.meta.env.VITE_UR_PUBLIC_KEY | |
| const baseUrl = import.meta.env.VITE_UR_BASE_URL | |
| if (!apiKey || !baseUrl) { | |
| throw new Error('Missing VITE_UR_PUBLIC_KEY or VITE_UR_BASE_URL') | |
| } | |
| ReactDOM.createRoot(document.getElementById('root')!).render( | |
| <React.StrictMode> | |
| <UrProvider | |
| apiKey={apiKey} | |
| baseUrl={baseUrl} | |
| > |
🤖 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 `@examples/react-sdk-demo/src/main.tsx` around lines 7 - 12, The app currently
passes import.meta.env.VITE_UR_PUBLIC_KEY and VITE_UR_BASE_URL directly into
UrProvider which can produce cryptic runtime errors if those env vars are
missing; add an early check before calling ReactDOM.createRoot/UrProvider to
validate import.meta.env.VITE_UR_PUBLIC_KEY and import.meta.env.VITE_UR_BASE_URL
are present and non-empty, and if not, log an explicit error (or throw) and stop
initialization so the failure is fast and actionable (update the code around
ReactDOM.createRoot and the UrProvider initialization to perform this guard).
- Add soft-delete guards to userAuth controllers and refresh flows - Update React SDK UrAuth semantics and Context error handling - Clarify Python SDK token fragment parsing example - Fix clipboard lint errors in Dashboard and consolidate CSS - Update React SDK dependencies and improve demo loading state
Collaborator
Author
|
all done?? |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
gh api repos/geturbackend/urBackend/pulls/242/reviews 2>&1 | jq '[.[] | {id: .id, user: .user.login, state: .state, submitted_at: .submitted_at, body: .body[:300]}]' 2>&1Length of output: 973
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdks/urbackend-react/tests/context.test.tsx (1)
65-81:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAlways restore mocked browser globals.
If this test fails before Line 110, the mocked
window.location/window.historyleak into later tests and can cause cascading failures. Wrap the override block intry/finallyso cleanup always runs.Proposed fix
const originalLocation = window.location; // `@ts-ignore` delete window.location; window.location = { ...originalLocation, search: '?rtCode=test-rt-code', hash: '`#token`=test-temp-token', pathname: '/auth/callback', } as any; const originalHistory = window.history; // `@ts-ignore` delete window.history; window.history = { ...originalHistory, replaceState: vi.fn(), } as any; - - const TestComponent = () => { - const { isInitializing } = useUrContext(); - if (isInitializing) return <div>Loading...</div>; - return <div>Ready</div>; - }; - - render( - <UrProvider apiKey="test-key" baseUrl="http://localhost:3000"> - <TestComponent /> - </UrProvider> - ); - - await waitFor(() => { - expect(screen.getByText('Ready')).toBeInTheDocument(); - }); - - const mockAuthInstance = vi.mocked(AuthModule).mock.results[0]?.value; - if (mockAuthInstance) { - expect(mockAuthInstance.setToken).toHaveBeenCalledWith('test-temp-token'); - expect(mockAuthInstance.socialExchange).toHaveBeenCalledWith({ token: 'test-temp-token', rtCode: 'test-rt-code' }); - } - - window.location = originalLocation; - window.history = originalHistory; + try { + const TestComponent = () => { + const { isInitializing } = useUrContext(); + if (isInitializing) return <div>Loading...</div>; + return <div>Ready</div>; + }; + + render( + <UrProvider apiKey="test-key" baseUrl="http://localhost:3000"> + <TestComponent /> + </UrProvider> + ); + + await waitFor(() => { + expect(screen.getByText('Ready')).toBeInTheDocument(); + }); + + const mockAuthInstance = vi.mocked(AuthModule).mock.results[0]?.value; + if (mockAuthInstance) { + expect(mockAuthInstance.setToken).toHaveBeenCalledWith('test-temp-token'); + expect(mockAuthInstance.socialExchange).toHaveBeenCalledWith({ token: 'test-temp-token', rtCode: 'test-rt-code' }); + } + } finally { + window.location = originalLocation; + window.history = originalHistory; + }Also applies to: 109-111
🤖 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 `@sdks/urbackend-react/tests/context.test.tsx` around lines 65 - 81, The test temporarily overwrites window.location and window.history (using originalLocation and originalHistory and setting history.replaceState to vi.fn()) but does not guarantee restoration on failure; wrap the override block in a try/finally so the finally always restores window.location = originalLocation and window.history = originalHistory (and any mutated properties) to prevent leaks into other tests.
🤖 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 `@apps/public-api/src/controllers/userAuth.controller.js`:
- Around line 1690-1705: The refresh-token user query (Model.findOne in the
refresh path) only projects {_id, isDeleted} but checkUserSoftDeleted(user)
requires user.deletedAt to compute the real deletion deadline; update the
projection in the Model.findOne call used in the refresh flow to include
deletedAt (e.g., { _id: 1, isDeleted: 1, deletedAt: 1 }) so checkUserSoftDeleted
returns the actual scheduled date and downstream logic (revokeSessionChain,
clearRefreshCookie, res.status responses) behaves correctly.
- Around line 1212-1213: The controller currently returns raw { error: ... }
when checkUserSoftDeleted(user) yields a message; replace these raw responses
with the standardized AppError flow and envelope: instead of
res.status(403).json({ error: deletedMsg }) call next(new AppError(deletedMsg,
403)) (or throw new AppError(deletedMsg, 403) if controller uses async error
handling), so the global error middleware returns the { success, data, message }
envelope; update every occurrence that checks checkUserSoftDeleted (including
the other spots you flagged) to use AppError and not direct res.json.
In `@sdks/urbackend-sdk/package.json`:
- Line 3: The package.json version was accidentally rolled back to 0.1.0; update
"version" in sdks/urbackend-sdk/package.json to the correct published/expected
semver (e.g., 0.4.1 or the current 0.4.x) and ensure the runtime User-Agent set
in sdks/urbackend-sdk/src/client.ts (which currently reports 0.4.1) matches that
package.json value so package.json and the User-Agent stay in sync before
publishing.
---
Outside diff comments:
In `@sdks/urbackend-react/tests/context.test.tsx`:
- Around line 65-81: The test temporarily overwrites window.location and
window.history (using originalLocation and originalHistory and setting
history.replaceState to vi.fn()) but does not guarantee restoration on failure;
wrap the override block in a try/finally so the finally always restores
window.location = originalLocation and window.history = originalHistory (and any
mutated properties) to prevent leaks into other tests.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e63fae2d-e211-4c2d-98ad-59d019dfafad
📒 Files selected for processing (16)
apps/public-api/src/__tests__/userAuth.refresh.test.jsapps/public-api/src/__tests__/userAuth.social.test.jsapps/public-api/src/controllers/userAuth.controller.jsapps/web-dashboard/src/index.cssapps/web-dashboard/src/pages/ProjectDetails.jsxexamples/react-sdk-demo/src/App.tsxexamples/react-sdk-demo/src/main.tsxsdks/urbackend-python/README.mdsdks/urbackend-python/src/urbackend/auth.pysdks/urbackend-react/package.jsonsdks/urbackend-react/src/components/Toast.tsxsdks/urbackend-react/src/components/UrAuth.tsxsdks/urbackend-react/src/context.tsxsdks/urbackend-react/tests/context.test.tsxsdks/urbackend-sdk/package.jsonsdks/urbackend-sdk/src/client.ts
💤 Files with no reviewable changes (1)
- apps/web-dashboard/src/index.css
✅ Files skipped from review due to trivial changes (1)
- sdks/urbackend-python/README.md
🚧 Files skipped from review as they are similar to previous changes (5)
- sdks/urbackend-react/package.json
- examples/react-sdk-demo/src/main.tsx
- sdks/urbackend-react/src/components/Toast.tsx
- examples/react-sdk-demo/src/App.tsx
- sdks/urbackend-react/src/context.tsx
Collaborator
Author
|
mergeable?? @coderabbitai |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check PR mergeability, checks, and review state
gh pr view 242 --repo geturbackend/urBackend --json mergeable,mergeStateStatus,reviewDecision,statusCheckRollup,reviews 2>&1 | jq '.'Length of output: 41529 🏁 Script executed: #!/bin/bash
# Check open/unresolved review comments
gh api repos/geturbackend/urBackend/pulls/242/reviews 2>&1 | jq '[.[] | {id: .id, user: .user.login, state: .state, submitted_at: .submitted_at}]'Length of output: 662 🏁 Script executed: #!/bin/bash
# Check review comments count
gh api repos/geturbackend/urBackend/pulls/242/comments 2>&1 | jq 'length'Length of output: 163
Technically GitHub will let you merge, but I'd recommend addressing the open comments first — especially these higher-priority ones:
Once those are resolved, the PR would be in good shape to merge. 🐇 |
Collaborator
Author
|
every single comment has been addressed in this last commit |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== 1. exchangeSocialRefreshToken error branches ==="
rg -n "err.message|catch.*err|socialExchange\|getdel" apps/public-api/src/controllers/userAuth.controller.js -A3 -B1 | head -80
echo ""
echo "=== 2. context.tsx socialExchange catch ==="
cat sdks/urbackend-react/src/context.tsx
echo ""
echo "=== 3. urbackend-react package.json ==="
cat sdks/urbackend-react/package.json
echo ""
echo "=== 4. refreshToken projection in controller ==="
rg -n "refreshToken|deletedAt|isDeleted" apps/public-api/src/controllers/userAuth.controller.js -A5 -B2 | head -100
echo ""
echo "=== 5. Python README fragment parsing ==="
cat sdks/urbackend-python/README.md
echo ""
echo "=== 6. Python auth.py docstring ==="
cat sdks/urbackend-python/src/urbackend/auth.py | head -80Length of output: 23465
4 out of 5 are confirmed fixed. The one remaining item ( The PR looks good to merge! 🚀 🐇 ／(•ᴗ•)＼ |
yash-pouranik
temporarily deployed
to
feature/auth-fixes-and-react-sdk-tests - urBackend-frankfrut PR #242
Render
Destroyed
Collaborator
Author
|
ok its fixed now |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== sdks/urbackend-react/package.json ==="
cat sdks/urbackend-react/package.jsonLength of output: 938
The PR is good to go! 🚀 🐇 ／(•ᴗ•)＼ If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
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
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
This PR addresses critical vulnerabilities in the authentication flows, patches a concurrency race condition in the OAuth exchange, stabilizes the
@urbackend/reactSDK by completing its social auth flow, and introduces comprehensive unit testing for the React components. Furthermore, all SDKs and repository documentation have been updated to reflect the latest state.🐛 Bug Fixes & Security Patches
403status. The endpoint now returns a generic success response for soft-deleted users, perfectly mimicking non-existent accounts to prevent enumeration attacks.userAuth.controller.js). Replaced the sequentialredis.get()andredis.del()logic with an atomicredis.getdel()operation to prevent replay attacks during concurrent requests.<UrProvider />wherertCodewas ignored. The provider now correctly extractsrtCodefrom query parameters andtokenfrom the URL fragment, executingauth.socialExchange({ token, rtCode })to ensure long-lived sessions are reliably established even in strict cross-site cookie environments.✨ Enhancements & Testing
@urbackend/reactSDK, specifically targetingcontext.test.tsx(Logic & Auth flows) andUrAuth.test.tsx(UI behavior). All tests are passing.README.md&auth.py) to correctly reflect that the social authtokenrelies in the URL fragment (#token=...), not the query string.@urbackend/sdk(auth.ts).AGENTS.mdwith the new SDK ecosystem, testing commands, and atomic Redis patterns.📦 Chore / Release
packages/*,apps/*, and@urbackend/sdk.@urbackend/reactversion tov0.1.0following standard urBackend naming conventions for new packages.urbackend) tov0.1.1.GETDELcommand. Please ensure the production Redis instance is running Version 6.2.0 or higher before deploying this PR. No database schema migrations are required.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Style
Tests
Chores