feat: frontend rework with stable /setup fetching and initial js-utils/@kinde-oss/kinde-auth-react impl.#405
Conversation
…nd add type safety
…ved from `/setup`
…ctions and transforming client-side state
…hooks base on context
WalkthroughRemove the legacy browser client and its tests; add a client-side KindeProvider, provider-aware and providerless browser hooks, session-sync logic, frontend factories/types/constants/utils to translate React auth state, refactor /setup and token validation (20s threshold), update multiple import sources, and relocate public exports. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client as Client Component
participant KP as KindeProvider
participant USS as useSessionSync
participant API as /api/auth/setup
participant Store as clientStorage
Client->>KP: mount
KP->>USS: init session sync
USS->>API: fetchKindeState()
API-->>USS: setup payload (message, tokens, user, flags, org, env)
USS->>Store: store tokens (accessTokenEncoded, idTokenRaw)
USS-->>KP: { loading=false, config, getFetchedState }
KP-->>Client: render KindeReactProvider(children)
sequenceDiagram
autonumber
actor C as Component
participant UB as useKindeBrowserClient
participant P as useProvidedKindeAuth
participant PL as useProviderlessKindeAuth
participant USS as useSessionSync
participant SRV as refreshTokensServerAction
C->>UB: call useKindeBrowserClient()
alt KindeContext present
UB->>P: delegate -> transformReactAuthStateToNextState()
P-->>C: return KindeState + refreshData()
else no Provider
UB->>PL: delegate -> useSessionSync.getFetchedState()
PL-->>C: return KindeState + refreshData()
C->>PL: call refreshData()
PL->>SRV: dynamic import & invoke refreshTokensServerAction()
SRV-->>PL: success -> PL triggers refetch
end
sequenceDiagram
autonumber
participant Setup as setup handler
participant Sess as session manager
participant KC as kindeClient
Setup->>Sess: retrieve session tokens
alt tokens missing
Setup-->>Client: 200 NOT_LOGGED_IN + env
else tokens present
alt tokens expired (threshold 20s)
Setup->>KC: refreshTokens(session)
KC-->>Setup: updated tokens or error
alt refresh failure
Setup-->>Client: 500 REFRESH_FAILED + env
end
end
Setup->>Setup: decode tokens (access/id)
alt decode failure
Setup-->>Client: 500 ACCESS/ID_TOKEN_DECODE_FAILED + env
else success
Setup-->>Client: 200 OK + tokens, flags, user, org, permissions + env
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📚 Learning: 2025-10-13T23:54:25.463ZApplied to files:
📚 Learning: 2025-03-28T12:12:55.741ZApplied to files:
🧬 Code graph analysis (1)src/frontend/hooks/internal/use-session-sync.ts (4)
🔇 Additional comments (2)
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: 7
🧹 Nitpick comments (6)
src/session/getUserOrganizations.ts (1)
19-22: Inconsistent type assertion formatting.The type assertion for
idTokenOrgsnow uses multiline formatting, but the identical assertion foridTokenHasuraOrgson line 28 remains inline. For consistency, both should use the same format.Apply this diff to make line 28 consistent with the multiline format:
const idTokenHasuraOrgs = ((await kindeClient.getClaimValue( session, "x-hasura-organizations", "id_token", - )) as { id: string; name: string }[]) ?? []; + )) as { + id: string; + name: string; + }[]) ?? [];Alternatively, revert lines 19-22 to inline format to match line 28:
const idTokenOrgs = ((await kindeClient.getClaimValue( session, "organizations", "id_token", - )) as { - id: string; - name: string; - }[]) ?? []; + )) as { id: string; name: string }[]) ?? [];src/frontend/hooks/internal/use-fetched-kinde-state.ts (1)
12-49: Consider memoizing onSuccess callback.Since
onSuccessis included insetupState's dependencies (per the recommended fix above), callers should be advised to memoize the callback to prevent unnecessary re-fetches.Add a note in the JSDoc or inline comment:
type UseFetchedKindeStateProps = { + /** Callback invoked on successful state fetch. Should be memoized with useCallback to prevent re-fetches. */ onSuccess?: (state: FetchedKindeState) => void | Promise<void>; };src/frontend/KindeProvider.tsx (2)
17-17: Remove debug console.log.The console.log statement appears to be debug code left in production.
- console.log("[KindeProvider] Fetched state");
15-30: Memoize onSuccess callback.The
onSuccesscallback should be memoized withuseCallbackto prevent unnecessary re-execution ofsetupStateinuseFetchedKindeState, especially after the recommended fix to includeonSuccessin dependencies.+ const handleSuccess = useCallback(async (state: FetchedKindeState) => { + await Promise.all([ + store.clientStorage.setSessionItem( + StorageKeys.accessToken, + state.accessTokenEncoded, + ), + store.clientStorage.setSessionItem( + StorageKeys.idToken, + state.idTokenRaw, + ), + ]); + setConfig(state.env); + }, []); + const { loading } = useFetchedKindeState({ - onSuccess: async (state) => { - console.log("[KindeProvider] Fetched state"); - await Promise.all([ - store.clientStorage.setSessionItem( - StorageKeys.accessToken, - state.accessTokenEncoded, - ), - store.clientStorage.setSessionItem( - StorageKeys.idToken, - state.idTokenRaw, - ), - ]); - setConfig(state.env); - }, + onSuccess: handleSuccess, });src/frontend/hooks/public/use-providerless-kinde-auth.ts (1)
11-14: Consider extracting API path resolution.The API path resolution logic appears in multiple places (here and in other hooks). Consider extracting it to a utility function for consistency.
Create a utility function:
// src/frontend/utils.ts export const resolveApiPath = () => { return ( process.env.NEXT_PUBLIC_KINDE_AUTH_API_PATH || process.env.KINDE_AUTH_API_PATH || "/api/auth" ); };Then use it:
-export const useProviderlessKindeAuth = ( - apiPath = process.env.NEXT_PUBLIC_KINDE_AUTH_API_PATH || - process.env.KINDE_AUTH_API_PATH || - "/api/auth", -): KindeState => { +export const useProviderlessKindeAuth = ( + apiPath = resolveApiPath(), +): KindeState => {src/frontend/hooks/public/use-provided-kinde-auth.ts (1)
40-42: Consider implementing refreshData.The
refreshDatafunction is currently a no-op. Users might expect this to trigger a state refresh when using the provided auth pattern.Implement
refreshDatato re-transform state:return { ...clientState, - refreshData: async () => { - // noop for now - }, + refreshData: transformState, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package.jsonis excluded by!**/*.json
📒 Files selected for processing (20)
src/config/index.ts(2 hunks)src/frontend/KindeBrowserClient.ts(0 hunks)src/frontend/KindeProvider.tsx(1 hunks)src/frontend/OldAuthProvider.jsx(1 hunks)src/frontend/constants.ts(1 hunks)src/frontend/factories/feature-flag-factory.ts(1 hunks)src/frontend/factories/index.ts(1 hunks)src/frontend/factories/organization-factory.ts(1 hunks)src/frontend/factories/token-factory.ts(1 hunks)src/frontend/hooks/internal/use-fetched-kinde-state.ts(1 hunks)src/frontend/hooks/public/use-kinde-browser-client.ts(1 hunks)src/frontend/hooks/public/use-provided-kinde-auth.ts(1 hunks)src/frontend/hooks/public/use-providerless-kinde-auth.ts(1 hunks)src/frontend/index.js(1 hunks)src/frontend/store.ts(1 hunks)src/frontend/types.ts(1 hunks)src/frontend/utils.ts(1 hunks)src/handlers/setup.ts(1 hunks)src/session/getUserOrganizations.ts(1 hunks)tests/frontend/use-kinde-browser-client.test.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- src/frontend/KindeBrowserClient.ts
🧰 Additional context used
🧬 Code graph analysis (15)
src/frontend/hooks/internal/use-fetched-kinde-state.ts (4)
src/frontend/types.ts (2)
FetchedKindeState(19-31)KindeNextClientState(33-36)src/frontend/hooks/internal/use-sync-state.tsx (1)
useSyncState(7-26)src/frontend/constants.ts (1)
DefaultKindeNextClientState(3-16)src/frontend/utils.ts (1)
fetchKindeState(32-62)
src/frontend/constants.ts (1)
src/frontend/types.ts (1)
KindeNextClientState(33-36)
src/frontend/types.ts (1)
src/types.ts (7)
KindeFlagRaw(187-190)KindeAccessToken(14-32)KindeIdToken(96-142)KindeOrganization(217-221)KindePermissions(171-174)KindeUser(167-169)KindeOrganizations(223-229)
src/frontend/hooks/public/use-providerless-kinde-auth.ts (4)
src/types.ts (1)
KindeState(307-361)src/frontend/hooks/internal/use-fetched-kinde-state.ts (1)
useFetchedKindeState(12-49)src/frontend/utils.ts (1)
getRefreshTokensServerAction(4-13)src/frontend/factories/index.ts (1)
constructKindeClientState(47-55)
src/frontend/KindeProvider.tsx (2)
src/frontend/types.ts (1)
PublicKindeConfig(13-17)src/frontend/hooks/internal/use-fetched-kinde-state.ts (1)
useFetchedKindeState(12-49)
src/frontend/factories/token-factory.ts (1)
src/types.ts (2)
KindeAccessToken(14-32)KindeIdToken(96-142)
src/frontend/utils.ts (3)
src/session/refreshTokensServerAction.ts (1)
refreshTokensServerAction(6-9)src/frontend/types.ts (1)
FetchedKindeState(19-31)src/config/index.ts (2)
config(107-144)routes(150-167)
src/frontend/factories/index.ts (4)
src/frontend/factories/feature-flag-factory.ts (4)
getFlagFactory(6-36)getBooleanFlagFactory(38-49)getIntegerFlagFactory(64-75)getStringFlagFactory(51-62)src/frontend/factories/token-factory.ts (5)
getClaimFactory(3-14)getNextTypedAccessTokenFactory(16-20)getRawAccessTokenFactory(22-24)getNextTypedIdTokenFactory(26-28)getRawIdTokenFactory(30-32)src/frontend/factories/organization-factory.ts (1)
getOrganizationFactory(9-16)src/utils/generateUserObject.ts (1)
generateUserObject(5-68)
src/frontend/factories/feature-flag-factory.ts (4)
src/frontend/types.ts (1)
KindeFeatureFlags(12-12)src/types.ts (2)
KindeFlagTypeCode(192-192)KindeFlag(196-202)src/frontend/OldAuthProvider.jsx (2)
flagDataTypeMap(11-15)flagDataTypeMap(11-15)src/config/index.ts (1)
config(107-144)
src/frontend/factories/organization-factory.ts (2)
src/types.ts (3)
KindeIdToken(96-142)KindeAccessToken(14-32)KindeOrganization(217-221)src/utils/generateOrganizationObject.ts (1)
generateOrganizationObject(60-75)
src/handlers/setup.ts (8)
src/utils/getAccessToken.ts (1)
getAccessToken(7-41)src/utils/getIdToken.ts (1)
getIdToken(7-43)src/config/index.ts (1)
config(107-144)src/session/sessionManager.js (2)
sessionManager(22-40)sessionManager(22-40)src/utils/jwt/validation.ts (1)
isTokenExpired(7-15)src/session/kindeServerClient.ts (1)
kindeClient(7-10)src/types.ts (2)
KindeAccessToken(14-32)KindeIdToken(96-142)src/utils/generateUserObject.ts (1)
generateUserObject(5-68)
src/frontend/hooks/public/use-provided-kinde-auth.ts (4)
src/types.ts (1)
KindeState(307-361)src/frontend/types.ts (1)
KindeNextClientState(33-36)src/frontend/constants.ts (1)
DefaultKindeNextClientState(3-16)src/frontend/factories/index.ts (2)
transformReactAuthStateToNextState(57-111)constructKindeClientState(47-55)
src/frontend/hooks/public/use-kinde-browser-client.ts (3)
src/types.ts (1)
KindeState(307-361)src/frontend/hooks/public/use-provided-kinde-auth.ts (1)
useProvidedKindeAuth(17-44)src/frontend/hooks/public/use-providerless-kinde-auth.ts (1)
useProviderlessKindeAuth(11-36)
src/config/index.ts (1)
src/utils/removeTrailingSlash.js (1)
removeTrailingSlash(1-11)
tests/frontend/use-kinde-browser-client.test.tsx (2)
src/types.ts (6)
KindeAccessToken(14-32)KindeIdToken(96-142)KindeUser(167-169)KindeOrganization(217-221)KindePermissions(171-174)KindeOrganizations(223-229)src/frontend/hooks/public/use-kinde-browser-client.ts (1)
useKindeBrowserClient(8-19)
🔇 Additional comments (21)
tests/frontend/use-kinde-browser-client.test.tsx (18)
14-116: LGTM: Well-structured mock data factories.The mock data generators provide comprehensive coverage of all Kinde types (access token, id token, user, organization, permissions, user organizations, and composite setup response). The use of partial overrides makes tests flexible and maintainable.
129-185: LGTM: Thorough setup URL configuration tests.The test suite properly validates default, custom prop, and environment variable-based setup URL resolution, ensuring the hook correctly constructs the setup endpoint path.
187-210: LGTM: Initial state validation.Correctly verifies the hook initializes with loading state and null values for auth-related fields.
212-260: LGTM: Successful authentication response handling.Properly tests that the hook processes an OK response and populates all state fields correctly.
262-279: LGTM: NOT_LOGGED_IN response handling.Validates the hook correctly interprets an unauthenticated response.
281-328: LGTM: Error response handling.Comprehensive coverage of error scenarios including HTTP errors, missing error messages, and unknown status codes.
330-424: LGTM: Token getter tests.Thorough validation of all token accessors (getAccessToken, getAccessTokenRaw, getToken, getIdToken, getIdTokenRaw) in both authenticated and unauthenticated states.
426-458: LGTM: User getter tests.Validates getUser() returns the correct user object when authenticated and null when not.
460-513: LGTM: Organization getter tests.Tests getOrganization() and getUserOrganizations() for both authenticated and unauthenticated states.
515-586: LGTM: Permission getter tests.Comprehensive validation of getPermissions(), getPermission() for granted/denied permissions, and proper handling when permissions are null.
588-717: LGTM: Feature flag getFlag tests.Thorough coverage of getFlag() for string, boolean, and integer flags, including default value handling, missing flags, and type mismatch errors.
719-780: LGTM: getBooleanFlag tests.Validates boolean flag retrieval, default value handling, and error scenarios.
782-843: LGTM: getStringFlag tests.Tests string flag retrieval, default value handling, and type mismatch errors.
845-906: LGTM: getIntegerFlag tests.Validates integer flag retrieval, default value handling, and type mismatch errors.
908-987: LGTM: getClaim tests.Comprehensive coverage of getClaim() from both access token and id token, with proper null handling.
989-1031: Verify refreshData test expectations.The refreshData test acknowledges that dynamic import may fail in the test environment and only verifies the function can be called. Consider mocking the import more robustly to verify the full refresh flow.
Generate a more complete test by properly mocking the dynamic import:
it("should call refreshTokensServerAction and refetch state when available", async () => { const mockResponse = createMockSetupResponse(); const mockRefreshedResponse = createMockSetupResponse({ user: createMockUser({ given_name: "Updated" }), }); let callCount = 0; vi.spyOn(global, "fetch").mockImplementation(() => { callCount++; const response = callCount === 1 ? mockResponse : mockRefreshedResponse; return Promise.resolve({ json: () => Promise.resolve(response), ok: true, } as any); }); // Mock the refreshTokensServerAction at the module level const mockRefreshAction = vi.fn().mockResolvedValue(undefined); vi.doMock("../../src/session/refreshTokensServerAction.js", () => ({ refreshTokensServerAction: vi.fn(() => mockRefreshAction), })); const { result } = renderHook(() => useKindeBrowserClient()); await waitFor(() => { return expect(result.current.isLoading).toBe(false); }); expect(result.current.user?.given_name).toBe("Test"); await act(async () => { await result.current.refreshData(); }); await waitFor(() => { return expect(result.current.user?.given_name).toBe("Updated"); }); expect(mockRefreshAction).toHaveBeenCalled(); expect(callCount).toBe(2); // Initial fetch + refetch after refresh });
1033-1065: LGTM: isAuthenticated tests.Validates isAuthenticated flag for both authenticated and unauthenticated states.
1067-1134: LGTM: Edge case tests.Comprehensive edge case coverage including empty feature flags, empty permissions, and null flag values.
src/frontend/constants.ts (1)
1-16: LGTM: Well-defined default state.The DefaultKindeNextClientState provides a consistent baseline with appropriate initial values (isLoading: true, isAuthenticated: false, nulls for auth-related fields). This aligns well with the KindeNextClientState type.
src/frontend/hooks/public/use-providerless-kinde-auth.ts (1)
7-36: LGTM: Clean providerless auth implementation.The hook properly delegates to
useFetchedKindeStatefor state management and implements arefreshDatafunction that conditionally invokes the server action. The warning message for environments without App Router support is appropriate.src/frontend/index.js (1)
1-3: LGTM: Public API re-export updates.The updated exports correctly point to the new module locations for
useKindeAuth,useKindeBrowserClient, andKindeProvider.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/handlers/portal.ts (1)
4-10: Fix imports for storage and portal utilities
src/handlers/portal.ts (lines 4–10): @kinde-oss/kinde-auth-react/utils does not export generatePortalUrl, MemoryStorage, PortalPage, setActiveStorage, or StorageKeys; import these from @kinde/js-utils instead.
♻️ Duplicate comments (1)
src/session/sessionManager.js (1)
10-10: LGTM!The import source change is consistent with the migration strategy outlined in the PR objectives.
🧹 Nitpick comments (2)
src/frontend/KindeProvider.tsx (1)
19-19: Move side effect touseEffectto follow React best practices.Mutating
storageSettings.onRefreshHandlerduring render violates React's constraint that render functions should be pure. React may invoke render multiple times (especially in StrictMode or with concurrent features), potentially causing unexpected behavior.Apply this diff to move the side effect into a
useEffect:const { loading, config, refreshHandler } = useSessionSync(); - storageSettings.onRefreshHandler = refreshHandler; + useEffect(() => { + storageSettings.onRefreshHandler = refreshHandler; + }, [refreshHandler]); + if (loading && waitForInitialLoad) return null;If synchronous execution before paint is required, use
useLayoutEffectinstead:const { loading, config, refreshHandler } = useSessionSync(); - storageSettings.onRefreshHandler = refreshHandler; + useLayoutEffect(() => { + storageSettings.onRefreshHandler = refreshHandler; + }, [refreshHandler]); + if (loading && waitForInitialLoad) return null;src/handlers/setup.ts (1)
136-175: Consider removing redundant token fields and extracting env construction.The success response includes redundant fields:
accessTokenRaw(line 139) duplicatesaccessTokenEncoded(line 138)idTokenRaw(line 141) duplicatesidTokenEncoded(line 142)Additionally, the env object construction is repeated throughout the file (lines 28-32, 56-60, 82-86, 102-106, 170-174). Consider extracting it to a helper function:
const getEnvConfig = () => ({ clientId: config.clientID, issuerUrl: config.issuerURL, redirectUrl: config.redirectURL, });Then use
env: getEnvConfig()in all response objects.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package.jsonis excluded by!**/*.json
📒 Files selected for processing (10)
src/components/PortalLink.tsx(1 hunks)src/frontend/KindeProvider.tsx(1 hunks)src/frontend/hooks/internal/use-session-sync.ts(1 hunks)src/frontend/store.ts(1 hunks)src/handlers/portal.ts(1 hunks)src/handlers/setup.ts(1 hunks)src/session/getEntitlements.ts(1 hunks)src/session/index.ts(1 hunks)src/session/sessionManager.js(1 hunks)src/utils/cookies/getSplitSerializedCookies.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/frontend/store.ts
- src/frontend/hooks/internal/use-session-sync.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-03-28T12:12:55.741Z
Learnt from: DanielRivers
PR: kinde-oss/kinde-auth-nextjs#318
File: src/session/getFlag.js:23-23
Timestamp: 2025-03-28T12:12:55.741Z
Learning: The sessionManager function in the Kinde Auth NextJS package returns a Promise that must be properly awaited before using its result, especially when passing it to functions like kindeClient.getClaimValue().
Applied to files:
src/session/index.ts
📚 Learning: 2025-10-09T16:49:20.471Z
Learnt from: Yoshify
PR: kinde-oss/kinde-auth-nextjs#405
File: src/frontend/KindeProvider.tsx:17-34
Timestamp: 2025-10-09T16:49:20.471Z
Learning: In kinde-auth-nextjs, the KindeProvider from kinde-oss/kinde-auth-react can safely mount with empty/undefined clientId, domain, and redirectUri props and then receive real values on re-render. The provider's callbacks (login, register, logout) are defined with useCallback and depend on these props, so they recreate when the props update. The init() logic runs once and primarily validates tokens already in storage (pre-populated by useSessionSync), so it doesn't require valid config at initial mount. This pattern allows mounting the provider immediately to avoid UI flash while config is being fetched.
Applied to files:
src/frontend/KindeProvider.tsx
🧬 Code graph analysis (2)
src/handlers/setup.ts (9)
src/routerClients/RouterClient.js (1)
RouterClient(3-68)src/utils/getAccessToken.ts (1)
getAccessToken(7-41)src/utils/getIdToken.ts (1)
getIdToken(7-43)src/config/index.ts (1)
config(107-144)src/session/sessionManager.js (2)
sessionManager(22-40)sessionManager(22-40)src/utils/jwt/validation.ts (1)
isTokenExpired(7-15)src/session/kindeServerClient.ts (1)
kindeClient(7-10)src/types.ts (2)
KindeAccessToken(14-32)KindeIdToken(96-142)src/utils/generateUserObject.ts (1)
generateUserObject(5-68)
src/frontend/KindeProvider.tsx (2)
src/frontend/hooks/internal/use-session-sync.ts (1)
useSessionSync(27-123)src/config/index.ts (1)
config(107-144)
🔇 Additional comments (6)
src/session/index.ts (1)
33-33: LGTM! Import source change is consistent.The import source change for
getEntitlementsResponsefrom@kinde/js-utilsto@kinde-oss/kinde-auth-react/utilsis consistent with the changes insrc/session/getEntitlements.tsand aligns with the PR's module consolidation objective. The verification script in the previous file will confirm that this type is properly exported from the new source.src/session/getEntitlements.ts (1)
3-10: Verify utilities are exported from the new package source. Automated inspection couldn’t locate@kinde-oss/kinde-auth-react/utils; manually confirm thatMemoryStorage,StorageKeys,clearActiveStorage,getEntitlementsResponse,getEntitlementsUtils, andsetActiveStorageare correctly exported with unchanged signatures.src/utils/cookies/getSplitSerializedCookies.ts (1)
3-3: Import migration verified. @kinde-oss/kinde-auth-react@^5.8.0 is declared in package.json; no @kinde/js-utils imports remain; splitString signature and behavior match the previous implementation.src/handlers/setup.ts (2)
10-10: ConfirmisTokenExpiredsignature
Ensure that the React SDK’sisTokenExpiredcan be called with only{ threshold: 20 }(auto-reading the token)—if it instead requires an explicit token argument, pass the token or switch to the local utility.
17-189: Ensure token refresh coordination
Removing the queue exposes simultaneousrefreshTokenscalls each using the same refresh token; if your provider rotates or invalidates refresh tokens on use, the second request will fail. Confirm Kinde’s refreshTokens behavior under concurrent calls or reintroduce locking/queue around token refresh.src/components/PortalLink.tsx (1)
3-3: Verify installed package exports
- Confirm
@kinde-oss/kinde-auth-react@v5.6.0is installed and exposes a/utilssubpath in itspackage.jsonexports.- Ensure
GeneratePortalUrlParamsis exported from that subpath and its definition matches the original type.
Yoshify
force-pushed
the
feat/client-react
branch
from
c5ed115 to
c70dd8c
Compare
Yoshify
force-pushed
the
feat/client-react
branch
from
c70dd8c to
72caad8
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/*.yaml
📒 Files selected for processing (3)
src/authMiddleware/authMiddleware.ts(1 hunks)src/handlers/setup.ts(1 hunks)src/utils/jwt/validation.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: Yoshify
PR: kinde-oss/kinde-auth-nextjs#405
File: src/frontend/KindeProvider.tsx:17-34
Timestamp: 2025-10-09T16:49:20.471Z
Learning: In kinde-auth-nextjs, the KindeProvider from kinde-oss/kinde-auth-react can safely mount with empty/undefined clientId, domain, and redirectUri props and then receive real values on re-render. The provider's callbacks (login, register, logout) are defined with useCallback and depend on these props, so they recreate when the props update. The init() logic runs once and primarily validates tokens already in storage (pre-populated by useSessionSync), so it doesn't require valid config at initial mount. This pattern allows mounting the provider immediately to avoid UI flash while config is being fetched.
📚 Learning: 2024-11-21T17:55:06.897Z
Learnt from: DanielRivers
PR: kinde-oss/kinde-auth-nextjs#243
File: src/session/getIdToken.js:21-22
Timestamp: 2024-11-21T17:55:06.897Z
Learning: In the Kinde Auth Next.js library, the `getIdToken(req, res)` function in `src/utils/getIdToken.ts` performs token validation using `validateToken` from `kinde/jwt-validator`, ensuring proper token validation before decoding.
Applied to files:
src/authMiddleware/authMiddleware.ts
📚 Learning: 2024-11-21T09:58:35.193Z
Learnt from: DanielRivers
PR: kinde-oss/kinde-auth-nextjs#243
File: src/authMiddleware/authMiddleware.ts:46-49
Timestamp: 2024-11-21T09:58:35.193Z
Learning: In `src/authMiddleware/authMiddleware.ts`, the `validateToken` function from `kinde/jwt-validator` handles exceptions internally, so additional error handling is not necessary when calling it.
Applied to files:
src/utils/jwt/validation.ts
🧬 Code graph analysis (2)
src/authMiddleware/authMiddleware.ts (1)
src/utils/jwt/validation.ts (1)
isTokenExpired(7-20)
src/handlers/setup.ts (9)
src/routerClients/RouterClient.js (1)
RouterClient(3-68)src/utils/getAccessToken.ts (1)
getAccessToken(7-41)src/utils/getIdToken.ts (1)
getIdToken(7-43)src/config/index.ts (1)
config(107-144)src/session/sessionManager.js (2)
sessionManager(22-40)sessionManager(22-40)src/utils/jwt/validation.ts (1)
isTokenExpired(7-20)src/session/kindeServerClient.ts (1)
kindeClient(7-10)src/types.ts (2)
KindeAccessToken(14-32)KindeIdToken(96-142)src/utils/generateUserObject.ts (1)
generateUserObject(5-68)
🔇 Additional comments (1)
src/authMiddleware/authMiddleware.ts (1)
88-91: 20 s skew buffer avoids expiry edge cases.
Adding the 20-second grace period prevents last-second refresh races while matchingisTokenExpired’s threshold semantics.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/utils/jwt/validation.ts (2)
16-18: Error logging should respect debug mode.The error is logged unconditionally, while
validateTokenbelow checksconfig.isDebugModebefore logging. This inconsistency may clutter production logs or leak information.Apply this diff:
} catch (error) { + if (config.isDebugMode) { console.error("Error checking authentication:", error); + } return true; }
5-6: Consider removing or updating the outdated comment.The comment "currently assumes that the token is valid .. should we revalidate here as well?" may confuse readers now that validation logic (try-catch, exp checks) is in place.
src/handlers/setup.ts (1)
136-175: Comprehensive success response with proper env payload.The response structure correctly includes all required data (tokens, user info, permissions, organizations, feature flags) along with the environment configuration needed by the React provider.
Note: The response includes both
accessTokenRawandaccessTokenEncoded(which are identical), as well asidTokenRawandidTokenEncoded(also identical). This duplication may be intentional for backward compatibility or API contract reasons.If the duplication is not required for backward compatibility, consider consolidating in a future refactor:
return routerClient.json({ accessToken, accessTokenEncoded, - accessTokenRaw: accessTokenEncoded, idToken, - idTokenRaw: idTokenEncoded, idTokenEncoded,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/handlers/setup.ts(1 hunks)src/utils/jwt/validation.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/handlers/setup.ts (8)
src/routerClients/RouterClient.js (1)
RouterClient(3-68)src/utils/getAccessToken.ts (1)
getAccessToken(7-41)src/utils/getIdToken.ts (1)
getIdToken(7-43)src/config/index.ts (1)
config(107-144)src/session/sessionManager.js (2)
sessionManager(22-40)sessionManager(22-40)src/utils/jwt/validation.ts (1)
isTokenExpired(7-20)src/session/kindeServerClient.ts (1)
kindeClient(7-10)src/types.ts (2)
KindeAccessToken(14-32)KindeIdToken(96-142)
🔇 Additional comments (3)
src/utils/jwt/validation.ts (1)
8-19: Security fix confirmed: decode errors now treated as expired.The change at line 18 correctly addresses the critical security issue flagged in the previous review. Returning
trueon decode failures ensures malformed/forged tokens are treated as expired rather than valid.However, consider respecting debug mode when logging errors (see separate comment).
src/handlers/setup.ts (2)
17-36: LGTM! Proper handling of missing tokens.The initial token checks correctly identify when users are not logged in and return the required environment configuration for the React provider. The 200 status code is appropriate here since "not logged in" is a valid state, not an error.
176-193: Final error handler properly includes env payload.The previous review concern about the missing env payload in the
SETUP_FAILEDpath has been correctly addressed. The catch block now returns the environment configuration alongside the error details, ensuring the frontend can always bootstrap the provider even when unexpected failures occur.
Yoshify
force-pushed
the
feat/client-react
branch
from
c0f7dc5 to
8712e4b
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/frontend/hooks/internal/use-session-sync.ts(1 hunks)src/frontend/hooks/public/use-providerless-kinde-auth.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/frontend/hooks/public/use-providerless-kinde-auth.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/frontend/hooks/internal/use-session-sync.ts (4)
src/frontend/types.ts (3)
PublicKindeConfig(13-17)KindeNextClientState(33-36)FetchedKindeState(19-31)src/frontend/hooks/internal/use-sync-state.tsx (1)
useSyncState(7-26)src/frontend/constants.ts (1)
DefaultKindeNextClientState(3-16)src/frontend/utils.ts (1)
fetchKindeState(34-83)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/frontend/hooks/internal/use-session-sync.ts (2)
73-76: Good: log gated behind debug.This addresses prior feedback to log only in debug.
47-68: Fix stale closure and hook deps; break the cycle with a ref.updateTokensAndSetRefresh uses shouldAutoRefresh and refreshHandler but omits both from deps, risking stale closures. Add shouldAutoRefresh and read refreshHandler via a ref to avoid a cyclic dependency with refreshHandler.
- import { useCallback, useEffect, useState } from "react"; + import { useCallback, useEffect, useRef, useState } from "react"; @@ - const updateTokensAndSetRefresh = useCallback( + const refreshHandlerRef = useRef<(() => Promise<RefreshTokenResult>) | null>(null); + + const updateTokensAndSetRefresh = useCallback( async (kindeState: Omit<FetchedKindeState, "env">) => { @@ - if (shouldAutoRefresh) { - const expiry = await calculateExpirySeconds(); - setRefreshTimer(expiry, refreshHandler); - } + if (shouldAutoRefresh) { + const expiry = await calculateExpirySeconds(); + if (expiry && expiry > 0 && refreshHandlerRef.current) { + setRefreshTimer(expiry, refreshHandlerRef.current); + } + } @@ - [setFetchedState], + [setFetchedState, shouldAutoRefresh], ); @@ - const refreshHandler = useCallback(async (): Promise<RefreshTokenResult> => { + const refreshHandler = useCallback(async (): Promise<RefreshTokenResult> => { const setupResponse = await fetchKindeState(); @@ }, [handleError, updateTokensAndSetRefresh]); + + useEffect(() => { + refreshHandlerRef.current = refreshHandler; + }, [refreshHandler]);
🧹 Nitpick comments (3)
src/frontend/hooks/internal/use-session-sync.ts (3)
118-121: Mount-only effect: add brief justification comment.Rule suppression is fine; add a one-liner rationale to aid future readers.
useEffect(() => { setupState(); - // eslint-disable-next-line react-hooks/exhaustive-deps + // Runs once on mount to avoid duplicate /setup calls; setupState is memoized. + // eslint-disable-next-line react-hooks/exhaustive-deps }, []);
91-96: Type the refetch result to avoid leaking implicit shapes.setupState returns a token payload keyed by StorageKeys; give it an explicit return type so consumers don’t rely on implicit/any.
+type RefetchResult = + | { success: true; [StorageKeys.accessToken]: string | null; [StorageKeys.idToken]: string | null } + | { success: false; error: string }; - const setupState = useCallback(async () => { + const setupState = useCallback(async (): Promise<RefetchResult> => { @@ - return { + return { success: true, [StorageKeys.accessToken]: setupResponse.kindeState.accessTokenEncoded, [StorageKeys.idToken]: setupResponse.kindeState.idTokenRaw, };
98-116: Improve error detail on refresh failures.Propagate the actual server error when available to aid debugging (still safe for client logs).
- if (!setupResponse.success) { - await handleError("User is unauthenticated or refresh failed"); - return { - success: false, - error: "User is unauthenticated or refresh failed", - }; - } + if (!setupResponse.success) { + const msg = setupResponse.error || "User is unauthenticated or refresh failed"; + await handleError(msg); + return { success: false, error: msg }; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/frontend/hooks/internal/use-session-sync.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Yoshify
PR: kinde-oss/kinde-auth-nextjs#405
File: src/frontend/KindeProvider.tsx:17-34
Timestamp: 2025-10-09T16:49:20.471Z
Learning: In kinde-auth-nextjs, the KindeProvider from kinde-oss/kinde-auth-react can safely mount with empty/undefined clientId, domain, and redirectUri props and then receive real values on re-render. The provider's callbacks (login, register, logout) are defined with useCallback and depend on these props, so they recreate when the props update. The init() logic runs once and primarily validates tokens already in storage (pre-populated by useSessionSync), so it doesn't require valid config at initial mount. This pattern allows mounting the provider immediately to avoid UI flash while config is being fetched.
🧬 Code graph analysis (1)
src/frontend/hooks/internal/use-session-sync.ts (4)
src/frontend/types.ts (3)
PublicKindeConfig(13-17)KindeNextClientState(33-36)FetchedKindeState(19-31)src/frontend/hooks/internal/use-sync-state.tsx (1)
useSyncState(7-26)src/frontend/constants.ts (1)
DefaultKindeNextClientState(3-16)src/frontend/utils.ts (1)
fetchKindeState(34-83)
🔇 Additional comments (1)
src/frontend/hooks/internal/use-session-sync.ts (1)
35-45: Review comment is accurate but unfixable with current library constraints.The concern is valid: if a refresh timer fires after
handleErrordestroys the session, the callback will attempt to refresh a torn-down state, potentially causing cascading errors. However, @kinde-oss/kinde-auth-react/utils does not export a clearRefreshTimer or documented mechanism to cancel a scheduled refresh, and no local timer tracking exists in the codebase to manage this manually.The suggested fix requires either an API addition to the external library, local timer tracking/workaround, or accepting the minor inefficiency of a failed refresh attempt on a destroyed session. This is a limitation of the current implementation, not a correctable bug in the code under review.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/frontend/hooks/internal/use-session-sync.ts (1)
21-26: Apply the suggested fixes from the previous review.This function still has three unresolved issues from the previous review:
Inconsistent constant usage: Line 22 uses the string literal
"accessToken"instead ofStorageKeys.accessToken, which is inconsistent with lines 56-57 and 124-125 that use the constant.Missing type validation:
token.expis not validated as a number before the arithmetic operation. Ifexpis missing or invalid, the calculation will produce unexpected results.Unhandled negative expiry: The function can return negative values for expired tokens, which should not schedule a timer (the token should be refreshed immediately instead).
Apply the suggested diff from the previous review:
export const calculateExpirySeconds = async (): Promise<number | null> => { - const token = await getDecodedToken<JWTDecoded>("accessToken"); - if (!token) return null; - - return token.exp - Math.floor(Date.now() / 1000); + const token = await getDecodedToken<JWTDecoded>(StorageKeys.accessToken); + if (!token || typeof token.exp !== "number") return null; + const secondsRemaining = token.exp - Math.floor(Date.now() / 1000); + return Number.isFinite(secondsRemaining) ? Math.max(0, secondsRemaining) : null; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/frontend/hooks/internal/use-session-sync.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: Yoshify
PR: kinde-oss/kinde-auth-nextjs#405
File: src/frontend/KindeProvider.tsx:17-34
Timestamp: 2025-10-09T16:49:20.471Z
Learning: In kinde-auth-nextjs, the KindeProvider from kinde-oss/kinde-auth-react can safely mount with empty/undefined clientId, domain, and redirectUri props and then receive real values on re-render. The provider's callbacks (login, register, logout) are defined with useCallback and depend on these props, so they recreate when the props update. The init() logic runs once and primarily validates tokens already in storage (pre-populated by useSessionSync), so it doesn't require valid config at initial mount. This pattern allows mounting the provider immediately to avoid UI flash while config is being fetched.
📚 Learning: 2025-10-13T23:54:25.463Z
Learnt from: Yoshify
PR: kinde-oss/kinde-auth-nextjs#405
File: src/handlers/setup.ts:38-65
Timestamp: 2025-10-13T23:54:25.463Z
Learning: In the setup handler (`src/handlers/setup.ts`), it is intentional to check only the access token expiry (`isTokenExpired(accessTokenEncoded, 20)`) before attempting a token refresh, as confirmed by the maintainer. Checking the ID token expiry is not necessary in this context.
Applied to files:
src/frontend/hooks/internal/use-session-sync.ts
📚 Learning: 2025-03-28T12:12:55.741Z
Learnt from: DanielRivers
PR: kinde-oss/kinde-auth-nextjs#318
File: src/session/getFlag.js:23-23
Timestamp: 2025-03-28T12:12:55.741Z
Learning: The sessionManager function in the Kinde Auth NextJS package returns a Promise that must be properly awaited before using its result, especially when passing it to functions like kindeClient.getClaimValue().
Applied to files:
src/frontend/hooks/internal/use-session-sync.ts
🧬 Code graph analysis (1)
src/frontend/hooks/internal/use-session-sync.ts (4)
src/frontend/types.ts (3)
PublicKindeConfig(13-17)KindeNextClientState(33-36)FetchedKindeState(19-31)src/frontend/hooks/internal/use-sync-state.tsx (1)
useSyncState(7-26)src/frontend/constants.ts (1)
DefaultKindeNextClientState(3-16)src/frontend/utils.ts (1)
fetchKindeState(34-83)
🔇 Additional comments (2)
src/frontend/hooks/internal/use-session-sync.ts (2)
97-99: Good use of ref pattern to resolve circular dependency.The ref pattern correctly resolves the circular dependency issue flagged in previous reviews. By storing
refreshHandlerin a ref and reading it insideupdateTokensAndSetRefresh, you avoid includingrefreshHandlerin the dependency array while still accessing the latest version.
129-132: Appropriate use of eslint-disable for mount-only effect.The eslint-disable comment correctly addresses the exhaustive-deps rule for this mount-only effect. The
setupStatecallback is stable due to its own dependency array, so running it only on mount is intentional and safe.
2 participants
Complete refactor of the frontend structure that includes the following:
KindeProvidernow has a purpose and provides a stable context value for consumers to use. It wraps the provider fromkinde-auth-reactuseKindeBrowserClientnow routes between the 2 depending on whether a provider is available.useProvidedKindeAuthwrapsuseKindeAuthfrom the React SDK with some transformation done to ensure it adheres to the currentKindeStatetype contract and ensures there should be no breaking changes.useProviderlessKindeAuthis essentially a heavily trimmed down and cleaner version of the currentuseKindeBrowserClientThe setup handler has been updated to remove queueing and return public env vars - client ID, issuer URL and redirect URL. This is necessary for the React provider.
This should eliminate race conditions in refreshing, and all-in-all be a performance improvement.
Assuming you have a root layout with the provider, and a page (lets say Dashboard) that uses
useKindeBrowserClient(), and a nested component (lets say UserPanel) that also usesuseKindeBrowserClient(), the flow looks like this:->
KindeProviderfetches state and mounts-->
Dashboarddoesn't fetch, it consumes the provider state and has immediate access to it.--->
UserPaneldoesn't fetch, it consumes the provider state and has immediate access to it.If a user is not using the provider (which they should be, but for backwards compatibility we support that), it looks more like this:
->
Dashboardfetches state, content then loads in-->
UserPanelalso fetches state, content then loads inThe recommendation is to use the provider to eliminate redundancy, improve performance, and most of all prevent race conditions with refreshes. The further nested your chain of
useKindeBrowserClientbecomes, the worse your performance gets (and the higher the chance you'll encounter a race with the multiple/setupcalls)There is a tradeoff with using the Provider (as the React SDK doesn't currently support having values added to its store after its been mounted) - we can't mount the provider until the setup call has completed, so there'll be a flash (and likely affect LCP). A fallback option has been provided to theNo longer true 🥳KindeProviderto supply a user made loading state to be displayed in loading and error situations. We can revisit this after a future look at making the React SDK handle this.This also provides us with a fairly structured path to cutting over to a full React SDK + JS utils impl. in v3.