feat(auth)!: stop persisting ID token; persist decoded profile instead

[bmanquen]

• Decode ID token once at sign-in to derive userInfo, then discard it

  • Persist the decoded profile (Zod-validated on read) so it survives reloads
  • Clear stored profile on sign-out and on unrecoverable session expiry
  • core: drop idToken from saveAuthData; add saveUserInfo/storedUserInfo and getStoredUserInfo()
  • hooks: remove AuthenticationState.idToken; provider rehydrates from stored profile
  • ui: update authenticated-user test helper

  BREAKING CHANGE: the ID token is no longer exposed or persisted.

  • Removed AuthenticationState.idToken (use userInfo for profile data)
  • Removed the YouVersionPlatformConfiguration.idToken getter
  • saveAuthData(accessToken, refreshToken, expiryDate) no longer accepts an idToken arg

Greptile Summary

This PR removes the ID token from the persistence layer: it is decoded once at sign-in to produce a YouVersionUserInfoJSON profile, which is then saved to localStorage (Zod-validated on read-back via storedUserInfo), while the token itself is discarded. The provider and useYVAuth are updated to rehydrate user state from the stored profile rather than from an idToken getter.

• handleAuthCallback now calls saveUserInfo after saveAuthData; extractSignInResult still populates idToken on the in-memory SignInWithYouVersionResult (available to processCallback callers), which is an incomplete removal of the exposure path described in the changeset.
• YouVersionUserInfoJSONSchema has all four fields as optional, so an empty {} object passes validation and would produce a non-null YouVersionUserInfo — consider requiring at least id to be a non-empty string to prevent a tampered entry from appearing as a valid session.
• handleAuthCallback catch block removes PKCE/OAuth temporary keys but not the auth tokens or user profile saved earlier in the same try block; a failure after saveUserInfo leaves the user partially authenticated on the next page load.

Confidence Score: 4/5

The change is safe to merge; the core rehydration flow (callback → saveUserInfo → storedUserInfo → setUserInfo) is correct, test coverage is solid, and sign-out/expiry cleanup is handled consistently.

The three findings are all non-blocking: the idToken is still reachable through the in-memory result object, the Zod schema is overly permissive on the profile shape, and the catch block leaves stale auth data in a failure scenario that requires an extremely unlikely runtime error. None of these affect the happy path or common error paths.

packages/core/src/Users.ts (extractSignInResult idToken field, catch block cleanup) and packages/core/src/schemas/user-info.ts (all-optional schema) are the areas worth a second look before merging.

Important Files Changed

Filename	Overview
packages/core/src/Users.ts	Core auth logic: handleAuthCallback now saves the decoded profile via saveUserInfo; extractSignInResult still populates idToken on the in-memory result; catch block doesn't clean up persisted auth data or userInfo on error
packages/core/src/YouVersionPlatformConfiguration.ts	Adds saveUserInfo/storedUserInfo; storedUserInfo getter correctly applies Zod safeParse to prevent blindly trusting stored data; all schema fields are optional so an empty {} also passes
packages/core/src/schemas/user-info.ts	New Zod schema for the persisted profile; all four fields are optional, so {} passes validation and would produce a non-null but empty YouVersionUserInfo on read-back
packages/hooks/src/context/YouVersionAuthProvider.tsx	Provider simplified: reads stored profile via getStoredUserInfo() after callback and after refresh; early-return mounted guards and loading state transitions are correct
packages/ui/src/test/utils.ts	Test helper now calls saveUserInfo and mocks getStoredUserInfo; refreshTokenIfNeeded correctly changed to resolve true so getStoredUserInfo is reached

Comments Outside Diff (2)

1. packages/core/src/Users.ts, line 192-201 (link)

   extractSignInResult still attaches idToken to the result — partial removal of the exposure path

   handleAuthCallback no longer persists the ID token, but it still returns a SignInWithYouVersionResult whose idToken field is set (via extractSignInResult). processCallback() in useYVAuth returns this object directly, so any consumer calling result.idToken can still read the raw token from memory. If the intent is to fully remove the exposure vector, idToken should be omitted from the result constructed inside extractSignInResult.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: packages/core/src/Users.ts
   Line: 192-201
   
   Comment:
   **`extractSignInResult` still attaches `idToken` to the result — partial removal of the exposure path**
   
   `handleAuthCallback` no longer persists the ID token, but it still returns a `SignInWithYouVersionResult` whose `idToken` field is set (via `extractSignInResult`). `processCallback()` in `useYVAuth` returns this object directly, so any consumer calling `result.idToken` can still read the raw token from memory. If the intent is to fully remove the exposure vector, `idToken` should be omitted from the result constructed inside `extractSignInResult`.
   
   
   
   How can I resolve this? If you propose a fix, please make it concise.

   

2. packages/core/src/Users.ts, line 148-154 (link)

   Catch block doesn't clean up saveAuthData / saveUserInfo on error

   If an exception is raised after saveAuthData and saveUserInfo have already written to localStorage (e.g. window.history.replaceState throwing in a restrictive browser environment), the catch block removes the PKCE/OAuth temporary keys but leaves the persisted tokens and user profile intact. On the next page load the user would be treated as authenticated, while the provider also exposes the error that was thrown. Adding YouVersionPlatformConfiguration.clearAuthTokens() at the top of the catch block would make the failure path consistent with sign-out semantics.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: packages/core/src/Users.ts
   Line: 148-154
   
   Comment:
   **Catch block doesn't clean up `saveAuthData` / `saveUserInfo` on error**
   
   If an exception is raised after `saveAuthData` and `saveUserInfo` have already written to localStorage (e.g. `window.history.replaceState` throwing in a restrictive browser environment), the catch block removes the PKCE/OAuth temporary keys but leaves the persisted tokens and user profile intact. On the next page load the user would be treated as authenticated, while the provider also exposes the error that was thrown. Adding `YouVersionPlatformConfiguration.clearAuthTokens()` at the top of the catch block would make the failure path consistent with sign-out semantics.
   
   How can I resolve this? If you propose a fix, please make it concise.

   

Prompt To Fix All With AI

Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
packages/core/src/schemas/user-info.ts:11-16
**All schema fields optional — empty object passes validation**

Because every field is `optional`, `{}` is a fully valid `YouVersionUserInfoJSON`. If the `userInfo` key in localStorage is tampered with and set to `{}`, `storedUserInfo` will return `{}`, `getStoredUserInfo()` will return a non-null `YouVersionUserInfo({})`, and the provider will call `setUserInfo(non-null)` — making `isAuthenticated` true with an empty profile. This only fires alongside a valid refresh token, so the practical risk is low, but adding `z.string().min(1)` for the `id` field (the canonical unique identifier) would let the schema reject an empty stub and give you a meaningful invariant to lean on.

### Issue 2 of 3
packages/core/src/Users.ts:192-201
**`extractSignInResult` still attaches `idToken` to the result — partial removal of the exposure path**

`handleAuthCallback` no longer persists the ID token, but it still returns a `SignInWithYouVersionResult` whose `idToken` field is set (via `extractSignInResult`). `processCallback()` in `useYVAuth` returns this object directly, so any consumer calling `result.idToken` can still read the raw token from memory. If the intent is to fully remove the exposure vector, `idToken` should be omitted from the result constructed inside `extractSignInResult`.

```suggestion
    const resultData = {
      accessToken: tokens.access_token,
      expiresIn: tokens.expires_in,
      refreshToken: tokens.refresh_token,
      // idToken is intentionally omitted — decoded once above and discarded.
      yvpUserId: idClaims.sub as string,
      name: idClaims.name as string,
      profilePicture: idClaims.profile_picture as string,
      email: idClaims.email as string,
    };
```

### Issue 3 of 3
packages/core/src/Users.ts:148-154
**Catch block doesn't clean up `saveAuthData` / `saveUserInfo` on error**

If an exception is raised after `saveAuthData` and `saveUserInfo` have already written to localStorage (e.g. `window.history.replaceState` throwing in a restrictive browser environment), the catch block removes the PKCE/OAuth temporary keys but leaves the persisted tokens and user profile intact. On the next page load the user would be treated as authenticated, while the provider also exposes the error that was thrown. Adding `YouVersionPlatformConfiguration.clearAuthTokens()` at the top of the catch block would make the failure path consistent with sign-out semantics.

_{Reviews (1): Last reviewed commit: "feat(auth)!: stop persisting ID token; p..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

[changeset-bot]

🦋 Changeset detected

Latest commit: abcf0b6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages

Name	Type
@youversion/platform-core	Major
@youversion/platform-react-hooks	Major
@youversion/platform-react-ui	Major
vite-react	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[greptile-apps]

All schema fields optional — empty object passes validation

Because every field is optional, {} is a fully valid YouVersionUserInfoJSON. If the userInfo key in localStorage is tampered with and set to {}, storedUserInfo will return {}, getStoredUserInfo() will return a non-null YouVersionUserInfo({}), and the provider will call setUserInfo(non-null) — making isAuthenticated true with an empty profile. This only fires alongside a valid refresh token, so the practical risk is low, but adding z.string().min(1) for the id field (the canonical unique identifier) would let the schema reject an empty stub and give you a meaningful invariant to lean on.

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/core/src/schemas/user-info.ts
Line: 11-16

Comment:
**All schema fields optional — empty object passes validation**

Because every field is `optional`, `{}` is a fully valid `YouVersionUserInfoJSON`. If the `userInfo` key in localStorage is tampered with and set to `{}`, `storedUserInfo` will return `{}`, `getStoredUserInfo()` will return a non-null `YouVersionUserInfo({})`, and the provider will call `setUserInfo(non-null)` — making `isAuthenticated` true with an empty profile. This only fires alongside a valid refresh token, so the practical risk is low, but adding `z.string().min(1)` for the `id` field (the canonical unique identifier) would let the schema reject an empty stub and give you a meaningful invariant to lean on.

How can I resolve this? If you propose a fix, please make it concise.