Skip to content

Fix sessionCreatedAt overwritten on token refresh#483

Merged
ar9708 merged 3 commits intomainfrom
copilot/fix-session-timestamp-refresh
Oct 25, 2025
Merged

Fix sessionCreatedAt overwritten on token refresh#483
ar9708 merged 3 commits intomainfrom
copilot/fix-session-timestamp-refresh

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Oct 25, 2025

Token refresh was overwriting sessionCreatedAt with the refresh timestamp instead of preserving the original login time. This caused the UI to show incorrect "session started" times after tokens were refreshed.

Changes

utils/auth.ts

  • Added optional isRefresh?: boolean parameter to parseAuthSessionData()
  • Conditionally omit sessionCreatedAt from returned object when isRefresh=true

api/axiosInstance.ts

  • Pass isRefresh: true when calling parseAuthSessionData() during token refresh (line 132)

utils/auth.test.ts (new)

  • Added unit tests validating sessionCreatedAt behavior for both login and refresh scenarios

Implementation

const res = {
  backend,
  basePath: "/api/v1",
  twoFactorEnabled: false,
  // Only set sessionCreatedAt on initial login, not during refresh
  ...(!isRefresh && { sessionCreatedAt: new Date().toISOString() }),
  sessionUpdatedAt: new Date().toISOString(),
  // ... rest of fields
};

When isRefresh=true, sessionCreatedAt is omitted from the partial object. The updateSession() merge preserves the existing value while updating other fields (sessionUpdatedAt, token timestamps).

Existing call sites in SignIn/Otp screens continue to work without modification (default isRefresh=undefined sets timestamp on initial login).

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • eu-assets.i.posthog.com
    • Triggering command: /usr/local/bin/node /home/REDACTED/work/web-client/web-client/OwnTube.tv/node_modules/jest-worker/build/workers/processChild.js (dns block)
  • eu.i.posthog.com
    • Triggering command: /usr/local/bin/node /home/REDACTED/work/web-client/web-client/OwnTube.tv/node_modules/jest-worker/build/workers/processChild.js (dns block)
  • peertube2.cpy.re
    • Triggering command: /usr/local/bin/node /home/REDACTED/work/web-client/web-client/OwnTube.tv/node_modules/jest-worker/build/workers/processChild.js (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt

Bug Report: Session timestamp incorrectly updates during token refresh

Problem: When users authenticate and then tokens are refreshed (either proactively or reactively), the session start timestamp (sessionCreatedAt) is incorrectly updated to show the latest token refresh time instead of preserving the original login time.

Root Cause: The parseAuthSessionData() function in OwnTube.tv/utils/auth.ts is used for both initial login AND token refresh, but it always sets sessionCreatedAt to the current time using new Date().toISOString().

Current Behavior:

  • User logs in at 2025-10-25 11:00:00
  • Tokens refresh at 2025-10-25 13:00:00
  • Session info shows: sessionCreatedAt: "2025-10-25T13:00:00.000Z" (WRONG - should be 11:00:00)
  • User sees "session startad 2025-10-25 13:00" in UI instead of original login time

Expected Behavior:

  • sessionCreatedAt should remain "2025-10-25T11:00:00.000Z" (original login time)
  • sessionUpdatedAt should update to "2025-10-25T13:00:00.000Z" (current behavior is correct)
  • accessTokenIssuedAt and refreshTokenIssuedAt should update to "2025-10-25T13:00:00.000Z" (current behavior is correct)

Solution Required:

Modify OwnTube.tv/utils/auth.ts to accept an optional parameter indicating whether this is an initial login or a token refresh:

  1. Add a new parameter isRefresh?: boolean to parseAuthSessionData() function
  2. When isRefresh is true, do not set sessionCreatedAt (omit it from returned object so it's not overwritten by updateSession())
  3. When isRefresh is false or undefined (initial login), set sessionCreatedAt as currently done
  4. Update the call site in OwnTube.tv/api/axiosInstance.ts:132 to pass isRefresh: true:
    const parsed = parseAuthSessionData(refreshed, backend, true);
  5. Existing call sites in login screens don't need changes (they'll use default isRefresh = false or undefined)

Files to Modify:

  1. OwnTube.tv/utils/auth.ts - Update function signature and logic:

    export const parseAuthSessionData = (
      loginResponse: UserLogin & { expires_in: number; refresh_token_expires_in: number },
      backend: string,
      isRefresh?: boolean,  // ADD THIS PARAMETER
    ): Partial<AuthSession> => {
      const { getConfigByBackend } = useInstanceConfigStore.getState();
      const instanceConfig = getConfigByBackend(backend);
    
      const res = {
        backend,
        basePath: "/api/v1",
        twoFactorEnabled: false,
        // Only set sessionCreatedAt on initial login, not during refresh
        ...(!isRefresh && { sessionCreatedAt: new Date().toISOString() }),
        sessionUpdatedAt: new Date().toISOString(),
        sessionExpired: false,
        tokenType: loginResponse.token_type,
        accessToken: loginResponse.access_token,
        accessTokenIssuedAt: new Date().toISOString(),
        accessTokenExpiresIn:
          instanceConfig?.customizations?.loginAccessTokenExpirationOverride ?? loginResponse.expires_in,
        refreshToken: loginResponse.refresh_token,
        refreshTokenIssuedAt: new Date().toISOString(),
        refreshTokenExpiresIn:
          instanceConfig?.customizations?.loginRefreshTokenExpirationOverride ?? loginResponse.refresh_token_expires_in,
      };
    
      return res;
    };
  2. OwnTube.tv/api/axiosInstance.ts - Update call site at line 132:

    const parsed = parseAuthSessionData(refreshed, backend, true);  // Pass true for isRefresh

Testing:

  1. Sign in to the application
  2. Note the "session startad" timestamp in Settings or Client Info JSON
  3. Wait for token refresh to occur (or trigger it by waiting past TOKEN_REFRESH_LIFETIME_PERCENTAGE of token lifetime)
  4. Check the session timestamp again - it should remain unchanged from initial login
  5. Verify sessionUpdatedAt, accessTokenIssuedAt, and refreshTokenIssuedAt DO update to reflect the refresh

TypeScript Compatibility: The change is backwards compatible - existing call sites will use the default value and continue to work correctly.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI self-assigned this Oct 25, 2025
Copilot AI and others added 2 commits October 25, 2025 12:01
Co-authored-by: mblomdahl <786326+mblomdahl@users.noreply.github.com>
- Add optional isRefresh parameter to parseAuthSessionData()
- When isRefresh=true, omit sessionCreatedAt to preserve original login time
- Update axiosInstance.ts to pass isRefresh=true during token refresh
- Add comprehensive unit tests for the fix

Co-authored-by: mblomdahl <786326+mblomdahl@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix session timestamp update during token refresh Fix sessionCreatedAt overwritten on token refresh Oct 25, 2025
Copilot AI requested a review from mblomdahl October 25, 2025 12:08
Copy link
Copy Markdown
Contributor

@mblomdahl mblomdahl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@claude Please review if the implementation here aligns with your latest comment on #451 here: #451 (comment)

@claude
Copy link
Copy Markdown

claude bot commented Oct 25, 2025

Claude finished @Copilot's task —— View job


Code Review: Session Timestamp Fix

I've reviewed the implementation in this PR. While I cannot access my specific comment from issue #451 due to tool restrictions, I can thoroughly analyze the implementation against the problem statement and best practices.


Summary

The implementation correctly solves the problem - sessionCreatedAt is now preserved during token refresh by conditionally omitting it from the parsed data.


Detailed Review

OwnTube.tv/utils/auth.ts (Lines 4-32)

What was changed:

  • Added optional isRefresh?: boolean parameter to parseAuthSessionData()
  • Line 17: Conditionally includes sessionCreatedAt only when isRefresh is falsy

Review:

...(!isRefresh && { sessionCreatedAt: new Date().toISOString() }),

This is the correct approach:

  • Uses spread operator with conditional object to cleanly omit the field
  • When isRefresh=true, sessionCreatedAt is not included in the returned partial object
  • The updateSession() merge (in the store) will preserve the existing sessionCreatedAt value
  • Backwards compatible: existing callers without the parameter default to undefined (falsy), setting timestamp on initial login

Potential concern: The implementation relies on updateSession() doing a shallow merge. If updateSession() were to replace the entire session object, this wouldn't work. However, based on typical Zustand store patterns, this is likely correct.


OwnTube.tv/api/axiosInstance.ts (Line 132)

What was changed:

const parsed = parseAuthSessionData(refreshed, backend, true);

Review:

  • Correctly passes isRefresh: true during token refresh
  • This is the only call site that should pass true (the others in SignIn/OTP screens should use default behavior)
  • Implementation matches the stated requirement

OwnTube.tv/utils/auth.test.ts (New file, 76 lines)

What was tested:

  1. sessionCreatedAt is set when isRefresh is not provided (default behavior)
  2. sessionCreatedAt is set when isRefresh is explicitly false
  3. sessionCreatedAt is NOT set when isRefresh is true ✨ (key test case)
  4. sessionUpdatedAt is always set regardless of isRefresh
  5. Token fields are always set regardless of isRefresh
  6. Backend/basePath fields are correctly set

Review:

  • Excellent test coverage of the new behavior
  • Tests the critical scenarios (initial login vs. refresh)
  • Line 42-46: The key assertion validates sessionCreatedAt is undefined during refresh
  • Mocks are properly configured
  • Tests follow project conventions (Jest + React Testing Library patterns)

Architecture Verification

The fix relies on this flow:

  1. Initial login (SignIn/OTP screens):

    • Calls parseAuthSessionData(response, backend) ← no 3rd parameter
    • Returns object with sessionCreatedAt: "2025-10-25T11:00:00.000Z"
    • Store saves this timestamp
  2. Token refresh (axiosInstance interceptor):

    • Calls parseAuthSessionData(response, backend, true)isRefresh=true
    • Returns object without sessionCreatedAt field (property omitted)
    • Store merge preserves existing sessionCreatedAt: "2025-10-25T11:00:00.000Z"
    • Updates other fields: sessionUpdatedAt, accessToken, accessTokenIssuedAt, etc.

Critical assumption: The Zustand updateSession() implementation must perform a shallow merge (not a full replacement). This is standard behavior for Zustand update functions.


Recommendations

1. ✅ No issues found - implementation is correct

The code correctly solves the problem as described. The approach is:

  • Clean and maintainable
  • Backwards compatible
  • Well-tested
  • Type-safe (TypeScript properly handles Partial<AuthSession>)

2. 📝 Optional enhancement: Verify store behavior

If you want to be extra defensive, you could add an integration test that verifies the full flow:

  1. Mock a login response
  2. Call updateSession() with the result
  3. Mock a refresh response
  4. Call updateSession() with isRefresh=true result
  5. Assert that sessionCreatedAt remains unchanged from step 2

However, this may be overkill if the store implementation is already tested elsewhere.


Conclusion

Approval recommendation: ✅ APPROVE

The implementation correctly fixes the bug where sessionCreatedAt was being overwritten during token refresh. The solution is:

  • Architecturally sound
  • Well-tested
  • Type-safe
  • Backwards compatible

The PR is ready to merge from a code quality perspective.


| View PR

@mblomdahl mblomdahl marked this pull request as ready for review October 25, 2025 17:01
@mblomdahl mblomdahl requested review from ar9708 and okaziya October 25, 2025 17:01
@mblomdahl mblomdahl changed the title Fix sessionCreatedAt overwritten on token refresh Fix sessionCreatedAt overwritten on token refresh Oct 25, 2025
@ar9708 ar9708 merged commit e864ed5 into main Oct 25, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants