Skip to content

fix(server-communication-config): prevent set_communication_type from clearing acquired cookies#864

Merged
coroiu merged 2 commits intomainfrom
coroiu/fix-cookies-getting-cleared-bug
Mar 19, 2026
Merged

fix(server-communication-config): prevent set_communication_type from clearing acquired cookies#864
coroiu merged 2 commits intomainfrom
coroiu/fix-cookies-getting-cleared-bug

Conversation

@coroiu
Copy link
Contributor

@coroiu coroiu commented Mar 19, 2026

🎟️ Tracking

📔 Objective

During login, the serverConfig$ observable emits twice (once when activeUserId$ changes, once when auth status transitions from Locked to Unlocked). Each emission calls parseBootstrapConfig() which produces a ServerCommunicationConfig with cookieValue: undefined, and setCommunicationType() saved this directly to the repository, overwriting any previously acquired cookies. This caused SsoCookieMain to clear cookies and re-trigger acquisition on every config re-emission.

The fix introduces a SetCommunicationTypeRequest type (with BootstrapConfigRequest and SsoCookieVendorConfigRequest) that structurally excludes cookie_value. The set_communication_type method now:

  1. Accepts this request type, making it impossible for callers to accidentally pass cookie values
  2. Preserves any previously acquired cookie_value from the repository when saving

Cookies are only ever set through acquire_cookie, and this change makes that contract explicit at the type level.

🚨 Breaking Changes

set_communication_type now accepts SetCommunicationTypeRequest instead of ServerCommunicationConfig. Callers need to update to use the new request types (SetCommunicationTypeRequest, BootstrapConfigRequest, SsoCookieVendorConfigRequest), which have the same fields minus cookie_value.

Migration: Replace ServerCommunicationConfig / BootstrapConfig / SsoCookieVendorConfig with their *Request counterparts when calling setCommunicationType(). The cookie_value field is simply removed from the input.

… clearing acquired cookies

Introduce SetCommunicationTypeRequest (and associated BootstrapConfigRequest,
SsoCookieVendorConfigRequest) that exclude the cookie_value field. The
set_communication_type method now accepts this request type and preserves any
previously acquired cookies from the repository, preventing config re-emissions
during login auth state transitions from clobbering cookies.
@coroiu coroiu requested a review from a team as a code owner March 19, 2026 09:43
@coroiu coroiu requested a review from dereknance March 19, 2026 09:43
@github-actions
Copy link
Contributor

github-actions bot commented Mar 19, 2026

Logo
Checkmarx One – Scan Summary & Details2c34cb32-1ac1-452f-b0b5-22ad92326cdf

Great job! No new security vulnerabilities introduced in this pull request

@github-actions
Copy link
Contributor

github-actions bot commented Mar 19, 2026

🔍 SDK Breaking Change Detection Results

SDK Version: coroiu/fix-cookies-getting-cleared-bug (5969327)
Completed: 2026-03-19 10:21:20 UTC
Total Time: 245s

Client Status Details
typescript ✅ No breaking changes detected TypeScript compilation passed with new SDK version - View Details

Breaking change detection completed. View SDK workflow

@coroiu coroiu requested review from dani-garcia and removed request for dereknance March 19, 2026 09:53
@codecov
Copy link

codecov bot commented Mar 19, 2026

Codecov Report

❌ Patch coverage is 93.47826% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.76%. Comparing base (42edf30) to head (5969327).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...uniffi/src/platform/server_communication_config.rs 0.00% 4 Missing ⚠️
...den-server-communication-config/src/wasm/client.rs 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #864      +/-   ##
==========================================
+ Coverage   82.73%   82.76%   +0.02%     
==========================================
  Files         353      353              
  Lines       42134    42203      +69     
==========================================
+ Hits        34861    34928      +67     
- Misses       7273     7275       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sonarqubecloud
Copy link

@coroiu coroiu merged commit 7637fdf into main Mar 19, 2026
58 checks passed
@coroiu coroiu deleted the coroiu/fix-cookies-getting-cleared-bug branch March 19, 2026 10:40
bw-ghapp bot pushed a commit to bitwarden/sdk-swift that referenced this pull request Mar 19, 2026
…ication-config): prevent set_communication_type from clearing acquired cookies (bitwarden/sdk-internal#864)
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