Skip to content

feat: add session duration management to user preferences#494

Merged
vasinl124 merged 6 commits intomainfrom
feat/user-session-settings
Mar 10, 2026
Merged

feat: add session duration management to user preferences#494
vasinl124 merged 6 commits intomainfrom
feat/user-session-settings

Conversation

@vasinl124
Copy link
Contributor

  • Introduced SessionDurationContent component for configuring session duration.
  • Updated B3DynamicModal to include session duration option.
  • Enhanced SettingsContent to navigate to session duration settings.
  • Implemented session duration utility functions for managing preferences.
  • Refactored authentication logic to utilize dynamic session duration from user preferences.

[LINEAR_ISSUE_ID_HERE]

Description

Write a description.

Test Plan

  • Locally
  • Unit Tests
  • Manually
  • CI/CD

Screenshots

For BE, include snippets, response payloads and/or curl commands to test endpoints

[FE] Before

[FE] After

[BE] Snippets/Response/Curl


automerge=false

- Introduced SessionDurationContent component for configuring session duration.
- Updated B3DynamicModal to include session duration option.
- Enhanced SettingsContent to navigate to session duration settings.
- Implemented session duration utility functions for managing preferences.
- Refactored authentication logic to utilize dynamic session duration from user preferences.
@vasinl124 vasinl124 requested a review from GioLogist March 10, 2026 21:51
@vasinl124 vasinl124 requested a review from a team as a code owner March 10, 2026 21:51
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a new feature allowing users to customize their session duration, enhancing both convenience and security. By providing options to control how long they remain signed in, users gain more flexibility over their account access. The changes involve new UI components, updated modal logic, and modifications to the core authentication process to respect these new user preferences.

Highlights

  • Session Duration Configuration Component: Introduced a new SessionDurationContent component for users to configure their session length.
  • Dynamic Modal Integration: Integrated the new session duration option into the B3DynamicModal for display.
  • Settings Navigation Update: Updated the SettingsContent to provide navigation to the session duration settings.
  • Session Duration Utility Functions: Developed utility functions to manage and retrieve user-defined session duration preferences.
  • Dynamic Authentication Logic: Modified the authentication process to dynamically apply the chosen session duration for user cookies.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • packages/sdk/src/app.shared.ts
    • Removed DEV_USER_GROUP constant.
    • Imported getSessionDurationDays utility.
    • Modified authenticate function to dynamically set cookie expiration based on user preferences or a default, replacing the fixed 30-day expiration for dev users.
  • packages/sdk/src/global-account/react/components/B3DynamicModal.tsx
    • Imported SessionDurationContent.
    • Added a new case for "sessionDuration" to render the SessionDurationContent component within the modal.
  • packages/sdk/src/global-account/react/components/ManageAccount/SessionDurationContent.tsx
    • Added a new React component to display and manage session duration options.
    • Implemented logic to read and update user preferences for session duration via API and local storage.
  • packages/sdk/src/global-account/react/components/ManageAccount/SettingsContent.tsx
    • Imported getSessionDurationDays.
    • Defined DURATION_LABELS for displaying session duration options.
    • Updated handleNavigate to support navigating to the "sessionDuration" modal type.
    • Added a new SettingsMenuItem to access the "Stay signed in" (session duration) settings.
  • packages/sdk/src/global-account/react/stores/useModalStore.ts
    • Defined a new interface SessionDurationModalProps for the session duration modal.
    • Extended the ModalContentType type to include SessionDurationModalProps.
  • packages/sdk/src/shared/utils/session-duration.ts
    • Added a new file containing utility functions for managing session duration.
    • Introduced SESSION_DURATION_OPTIONS and SessionDurationDays type.
    • Implemented getSessionDurationDays to retrieve session duration from user preferences or local storage with a fallback.
    • Implemented setSessionDurationDays to store the chosen session duration in local storage.
Activity
  • No human activity has been recorded on this pull request yet.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@claude
Copy link

claude bot commented Mar 10, 2026

Claude finished @vasinl124's task in 1m 27s —— View job


Code Review

  • Gathering context
  • Reading changed files
  • Analyzing code
  • Posting review

Overall the implementation is clean and well-structured. A few issues worth addressing:


Bug: Missing type="button" on buttons

SessionDurationContent.tsx:69 — The <button> elements inside the options list don't specify type="button". In a browser, buttons default to type="submit" when inside a <form>, which could cause unintended form submissions if this modal is ever rendered inside a form context (including third-party wrappers).

// Current
<button
  key={days}
  onClick={() => handleSelect(days)}
  disabled={saving}
  ...>

// Fix
<button
  type="button"
  key={days}
  onClick={() => handleSelect(days)}
  disabled={saving}
  ...>

Fix this →


Dead prop: chain in SessionDurationContent

SessionDurationContent.tsx:14,34chain is declared in the interface and destructured but never used inside the component. It's passed from the modal store but not needed.

// chain is declared but unused
interface SessionDurationContentProps {
  partnerId: string;
  chain: Chain; // ← never referenced below
}

Fix this →


Code duplication: label maps defined twice

DURATION_LABELS in SettingsContent.tsx:13–19 is identical to LABELS in SessionDurationContent.tsx:18–24. These should be exported from session-duration.ts (where SESSION_DURATION_OPTIONS already lives) and imported in both places.

Fix this →


Potential UX stale state in SettingsContent subtitle

SettingsContent.tsx:35sessionDays is computed once at render:

const sessionDays = getSessionDurationDays(user?.preferences, partnerId);

If the user navigates to SessionDurationContent, changes their session duration, and then navigates back — whether the subtitle in SettingsContent reflects the new value depends entirely on whether the component unmounts/remounts on navigation. If the modal navigation keeps it mounted (e.g. via a stack), the subtitle will be stale. Worth verifying that the navigation stack causes a remount, or adding a reactive data source (e.g. Zustand slice / localStorage event listener).


Notable behavior change: cookie duration now applies to ALL users

app.shared.ts:35–39 — Previously, only dev users (userGroup 4) had their cookie extended to 30 days. Now ALL users have their cookie re-set to their preference (default: 7 days) on every authentication. This means:

  • Existing users signing in for the first time after this deploy will have their cookies reset to 7 days regardless of their current cookie state.
  • Users who previously had longer-lived sessions (from a different mechanism) will be downgraded to 7 days on next login.

This is likely intentional, but worth a callout in the PR description so reviewers/QA are aware of the prod impact.


Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces session duration management for user preferences, a valuable feature implemented with new UI components, utility functions, and authentication flow updates. However, a critical security vulnerability was identified: the authentication cookie is set without the Secure flag, which could expose it to interception over unencrypted connections. Additionally, the review suggests improvements for code safety, maintainability, and clarity, including addressing an unsafe type cast, handling potential errors when saving preferences, removing duplicated constants, and cleaning up an unused prop.

…S and remove hardcoded labels

- Modified B3DynamicModal to pass partnerId to SessionDurationContent.
- Updated SessionDurationContent to utilize SESSION_DURATION_LABELS for consistency.
- Removed hardcoded duration labels from SettingsContent, now using SESSION_DURATION_LABELS.
- Added SESSION_DURATION_LABELS to session-duration utility for better maintainability.
@claude
Copy link

claude bot commented Mar 10, 2026

Claude finished @vasinl124's task in 1m 44s —— View job


Code Review

  • Gathering context
  • Reading changed files
  • Analyzing code
  • Posting review

Overall the implementation is solid — the code is well-structured, the SESSION_DURATION_LABELS deduplication has already been addressed, and the chain prop cleanup was done correctly. A few issues worth addressing:


Bug: Missing type="button" on session duration option buttons

SessionDurationContent.tsx:63 — The <button> elements in the options list don't specify type="button". Without it, browsers default to type="submit", which can cause unintended form submissions if this modal is ever rendered inside a form context.

// Fix
<button
  type="button"
  key={days}
  onClick={() => handleSelect(days)}
  disabled={saving}
  ...>

Fix this →


Security: Cookie set without Secure or SameSite attributes

app.shared.ts:38 — The b3-auth cookie is set without Secure or sameSite attributes. The Secure flag ensures the cookie is only sent over HTTPS, preventing interception on downgraded connections. SameSite: "Lax" also provides basic CSRF protection.

// Current
Cookies.set(B3_AUTH_COOKIE_NAME, token, days > 0 ? { expires: days } : {});

// Recommended
Cookies.set(B3_AUTH_COOKIE_NAME, token, {
  ...(days > 0 ? { expires: days } : {}),
  secure: true,
  sameSite: "Lax",
});

Fix this →


Missing error logging in catch block

SessionDurationContent.tsx:47 — Errors are silently swallowed. The optimistic revert is correct, but without logging there's no visibility into why saves are failing in production.

} catch (error) {
  // Revert optimistic update so UI stays consistent with server state
  setSessionDays(previous);
  setSessionDurationDays(previous, partnerId);
  console.error("Failed to save session duration preference:", error);
}

Fix this →


Stale subtitle in SettingsContent after navigating back

SettingsContent.tsx:27sessionDays is computed once at render:

const sessionDays = getSessionDurationDays(user?.preferences, partnerId);

If the user opens SessionDurationContent, changes their preference, and navigates back without the SettingsContent component unmounting (e.g. a modal stack keeps it mounted), the subtitle will show the stale value. Verify that the navigation stack unmounts SettingsContent on navigate-away, or add a Zustand slice / storage event listener to make it reactive.


Behavioral change callout: cookie now reset for ALL users on every login

app.shared.ts:35–39 — Previously, only dev users (userGroup 4) had their cookie extended. This PR changes the behavior so every user's cookie is re-set on each authentication. Existing users signing in for the first time after this deploy will have their cookie reset to 7 days (the default) regardless of any previously longer-lived session. This is likely intentional, but QA should test that existing sessions are not disrupted unexpectedly on first post-deploy login.


…Content

- Added type="button" to buttons in SessionDurationContent and SettingsContent for better accessibility and to prevent unintended form submissions.
- Updated handleBack prop in ModalHeader for consistency in SessionDurationContent.
…anagement

- Updated cookie settings in authenticate function to include secure and sameSite attributes for improved security.
- Refined error handling in SessionDurationContent to log errors when saving session duration preferences fails, ensuring better debugging and user feedback.
@claude
Copy link

claude bot commented Mar 10, 2026

Claude finished @vasinl124's task in 2m 8s —— View job


Code Review

  • Gathering context
  • Reading changed files
  • Analyzing code
  • Posting review

The previous review iterations caught several bugs (missing type="button", removed unused chain prop, deduplicated labels, added console.error, added secure/sameSite to cookie). Those are all fixed. A few remaining issues worth addressing:


Storage adapter doesn't apply cookie security attributes

app.shared.ts:62 — The clientOptions.storage.setItem (used by the authentication client to write the initial cookie) doesn't apply secure or sameSite:

setItem: (key: string, value: string) => {
  Cookies.set(key, value); // ← no secure/sameSite
},

So on every app.authenticate() call, the cookie is first written insecurely by the storage adapter, then immediately re-written securely by the authenticate wrapper. The window is tiny, but it's architecturally inconsistent — and if clientOptions.storage.setItem is ever called outside of authenticate(), the security flags won't be applied.

Fix this →


sessionDays initialized once via useState — won't reflect external preference changes

SessionDurationContent.tsx:28–30 — The initial value is computed once via lazy useState init:

const [sessionDays, setSessionDays] = useState<SessionDurationDays>(() =>
  getSessionDurationDays(user?.preferences, partnerId),
);

If user.preferences changes after mount (e.g. hydrated from another tab, or another part of the app updates the store), the displayed selection won't update. This is a minor concern since preferences are unlikely to change mid-session, but it's worth being aware of. Deriving the value directly from user?.preferences (without local state) and only managing optimistic UI during the save would be more robust.


Blank SVG icon on the "Stay signed in" settings row

SettingsContent.tsx:99–105 — The new SettingsMenuItem for session duration uses the same blank rounded rectangle SVG as the other items (no inner icon). The Linked Accounts and Notifications items presumably have icons baked in. This looks like a placeholder — if intentional it's fine, but worth checking against the design.


Behavioral change note (for QA)

app.shared.ts:35–39 — Previously, only dev users (userGroup 4) had their cookie extended. Now every user has their cookie re-set on every login, defaulting to 7 days if no preference is set. Users who had longer-lived cookies from an earlier mechanism will be downgraded to 7 days on their next login. Make sure QA covers this transition scenario.


Overall the implementation is solid and the previous review feedback was well addressed. The secure/sameSite gap in the storage adapter is the only concrete correctness issue remaining.

GioLogist
GioLogist previously approved these changes Mar 10, 2026
…gement

- Enhanced readability by formatting object properties in the authenticate function and SessionDurationContent component.
- Ensured consistent code style for better maintainability.
@claude
Copy link

claude bot commented Mar 10, 2026

Claude finished @vasinl124's task in 2m 10s —— View job


Code Review

  • Gathering context
  • Reading changed files
  • Analyzing code
  • Posting review

The previous review iterations caught several issues (missing type="button", unused chain prop, label deduplication, console.error, secure/sameSite), all of which have been addressed. The implementation is solid overall. Two remaining issues worth fixing, plus a behavioral callout:


Security: Storage adapter sets cookie without security flags

app.shared.ts:65–67 — The clientOptions.storage.setItem adapter writes the initial auth cookie without secure or sameSite:

setItem: (key: string, value: string) => {
  Cookies.set(key, value); // ← no secure/sameSite
},

On every app.authenticate() call, the cookie is first written insecurely by this adapter, then immediately re-written with the correct flags at line 38–42. The window is brief, but architecturally inconsistent — and if storage.setItem is ever called outside of authenticate(), the security attributes won't be applied.

// Fix
setItem: (key: string, value: string) => {
  Cookies.set(key, value, { secure: true, sameSite: "Lax" });
},

Fix this →


Privacy: localStorage preference persists across user sessions on shared devices

session-duration.ts:58–63setSessionDurationDays writes to localStorage keyed only by partnerId, not by user identity. When a user logs out and a different user logs in on the same device, getSessionDurationDays will fall back to the previous user's localStorage preference if the new user has no user.preferences set. This means a new user could silently inherit a 30-day session from a previous user's choice.

Consider clearing the localStorage key on logout, or keying it by partnerId + userId so preferences don't bleed across accounts.

Fix this →


Behavioral change note (for QA)

app.shared.ts:35–43 — Previously, only dev users (userGroup 4) had their cookie extended to 30 days. Now every user's cookie is re-set on every login, defaulting to 7 days if no preference exists. Users who had longer-lived sessions from an earlier mechanism will see their cookie reset to 7 days on next login. QA should verify this transition doesn't unexpectedly sign out users who have existing long-lived sessions.


@vasinl124 vasinl124 enabled auto-merge March 10, 2026 22:02
- Updated the way user preferences are accessed in SessionDurationContent to ensure proper type handling and prevent potential runtime errors.
- Enhanced code clarity by refining the object destructuring for partnerId preferences.
@claude
Copy link

claude bot commented Mar 10, 2026

Claude finished @vasinl124's task in 1m 46s —— View job


Code Review

  • Gathering context
  • Reading changed files
  • Analyzing code
  • Posting review

Previous review iterations addressed most issues (missing type="button", unused chain prop, duplicate label maps, console.error in catch, secure/sameSite on the auth cookie). One concrete correctness issue remains, plus a couple worth noting.


Bug: Storage adapter cookie written without security attributes

app.shared.ts:65–67 — The clientOptions.storage.setItem used by the Feathers authentication client writes the cookie without secure or sameSite:

setItem: (key: string, value: string) => {
  Cookies.set(key, value); // ← no secure/sameSite
},

The authenticate() wrapper immediately re-sets the cookie with proper attributes (lines 38–43), so the window is narrow on login. But any token-refresh flow or direct use of this storage adapter (e.g. app.authentication.setAccessToken) will write the cookie without security flags. This is architecturally inconsistent and could result in a Secure-less auth cookie in token-refresh scenarios.

// Fix
setItem: (key: string, value: string) => {
  Cookies.set(key, value, { secure: true, sameSite: "Lax" });
},

Fix this →


UX: No visual feedback while saving

SessionDurationContent.tsx:67–76saving disables buttons but there's no spinner, opacity change, or any visual indicator that a save is in progress. When a user taps an option it'll silently appear selected with no indication of network activity, then either stick or silently revert on failure.

At minimum, a visual cue (e.g. opacity-50 on the container, or a small spinner) would prevent double-taps and clarify that the selection is being persisted.


Note: sessionDays subtitle in SettingsContent depends on navigation behavior

SettingsContent.tsx:27sessionDays is derived from user?.preferences on render, which is correct if SettingsContent unmounts when navigating into SessionDurationContent, or if setUser(updated) on a successful save propagates back to the useAuthentication hook shared by both components. If the modal navigation stack keeps SettingsContent mounted without re-rendering it, the subtitle will show a stale value after navigating back. Worth verifying this path works end-to-end in manual testing.


Informational: Cookie reset behavior change for all users

app.shared.ts:35–43 — Previously only dev users (userGroup 4) got their cookie extended. Now every user has their cookie re-set on each login, defaulting to 7 days if no preference is saved. Users signing in for the first time after this deploy will see their cookie reset to 7 days regardless of any prior session length. Make sure QA covers this transition scenario.


Overall the implementation is clean and the previous review feedback was well-addressed. The storage adapter security gap is the only concrete correctness issue remaining.

@vasinl124 vasinl124 merged commit fc00b0a into main Mar 10, 2026
8 checks passed
@vasinl124 vasinl124 deleted the feat/user-session-settings branch March 10, 2026 22:18
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.

2 participants