[PM-29885] Implement SSO cookie vending authentication flow by SaintPatrck · Pull Request #6522 · bitwarden/android

SaintPatrck · 2026-02-11T22:45:37Z

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-29885

📔 Objective

Add cookie interceptor support for enterprise SSO configurations that require ELB session cookies. The interceptor manages the full cookie lifecycle for API requests behind a load balancer:

Preemptive bootstrap: Detects when cookies are needed but not yet available and triggers acquisition before making a doomed request
Cookie attachment: Attaches stored cookies to outgoing API requests via the Cookie header
Redirect detection: Catches HTTP 302 responses indicating expired/missing cookies and triggers re-acquisition via the identity provider
Connection safety: Properly closes OkHttp responses before throwing to prevent connection pool exhaustion

Key components:

CookieInterceptor — OkHttp interceptor for request/response cookie handling
NetworkCookieManager / NetworkCookieManagerImpl — Bootstrap detection and cookie storage bridge
CookieRedirectException — Signal exception to trigger cookie acquisition flow
CookieProvider — Interface bridging the network and app layers

github-actions · 2026-02-11T22:52:09Z

Checkmarx One – Scan Summary & Details – 5d6104fd-d320-4492-ac4e-592a891e35c7

New Issues (4)

Checkmarx found the following issues in this Pull Request

#	Issue	Source File / Package	Checkmarx Insight
1	HttpOnly_Cookie_Flag_Not_Set	/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/manager/util/CookieConfigurationDataExtensionsTest.kt: 45	details The web application's `toNetworkCookieList should map multiple cookies correctly` method creates a cookie Cookie, at line 45 of /app/src/test/kot... Attack Vector
2	HttpOnly_Cookie_Flag_Not_Set	/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/manager/util/CookieConfigurationDataExtensionsTest.kt: 46	details The web application's `toNetworkCookieList should map multiple cookies correctly` method creates a cookie Cookie, at line 46 of /app/src/test/kot... Attack Vector
3	HttpOnly_Cookie_Flag_Not_Set	/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/manager/util/CookieConfigurationDataExtensionsTest.kt: 62	details The web application's `toNetworkCookie should map name and value correctly` method creates a cookie Cookie, at line 62 of /app/src/test/kotlin/co... Attack Vector
4	HttpOnly_Cookie_Flag_Not_Set	/app/src/test/kotlin/com/x8bit/bitwarden/data/platform/manager/util/CookieConfigurationDataExtensionsTest.kt: 31	details The web application's `toNetworkCookieList should map single cookie correctly` method creates a cookie Cookie, at line 31 of /app/src/test/kotlin... Attack Vector

...tlin/com/x8bit/bitwarden/data/platform/manager/util/CookieConfigurationDataExtensionsTest.kt

claude · 2026-02-11T22:53:37Z

Claude finished @SaintPatrck's task in 4m 45s —— View job

Re-Review Summary

PR: PM-29885 Implement SSO cookie vending authentication flow

Previous Findings Status

Finding	Status
⚠️ IMPORTANT -- HTTP 302 detection unreachable as application interceptor	Resolved in `6235626a7` -- moved to `addNetworkInterceptor()`
🟡 SUGGESTED -- Missing test assertion for cookie interceptor	Resolved in `d71ead8c2` -- assertions added to all retrofit client tests

Changes Since Last Review

Two new commits were pushed:

c421c14a7 -- Removed the location property from CookieRedirectException and simplified CookieInterceptor to treat all 302 responses as cookie re-acquisition triggers. This is correct behavior now that the interceptor runs as a network interceptor and sees raw 302s.
6235626a7 -- Changed addInterceptor(cookieInterceptor) to addNetworkInterceptor(cookieInterceptor) in RetrofitsImpl. This correctly addresses the previous finding: as a network interceptor, the CookieInterceptor now sees intermediate 302 responses before OkHttp's RetryAndFollowUpInterceptor follows them. Both the preemptive bootstrap path and the expired-cookie redirect detection path now work as designed.

Verification

Network interceptors are properly inherited through OkHttpClient.newBuilder(), so all authenticated, unauthenticated, and static retrofit clients include the cookie interceptor.
response.close() is correctly called before throwing CookieRedirectException to prevent connection pool exhaustion.
RetrofitsTest verifies isCookieInterceptorCalled across all six retrofit client configurations.
The authenticator module provides an appropriate no-op CookieProvider implementation.

Result

No new issues found. All previous findings have been properly addressed. The implementation is clean, well-tested, and follows established patterns.

Re-reviewed commit range: c421c14a7..6235626a7 | Previous review: 2026-02-17

codecov · 2026-02-11T23:23:11Z

Codecov Report

❌ Patch coverage is 93.75000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.35%. Comparing base (4a68c23) to head (6235626).
⚠️ Report is 8 commits behind head on main.

Files with missing lines	Patch %	Lines
...atform/manager/network/NetworkCookieManagerImpl.kt	84.61%	0 Missing and 4 partials ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6522   +/-   ##
=======================================
  Coverage   86.34%   86.35%           
=======================================
  Files         791      795    +4     
  Lines       56717    56781   +64     
  Branches     8213     8229   +16     
=======================================
+ Hits        48971    49031   +60     
  Misses       4891     4891           
- Partials     2855     2859    +4

☔ 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.

network/src/test/kotlin/com/bitwarden/network/retrofit/RetrofitsTest.kt

...rc/main/kotlin/com/x8bit/bitwarden/data/platform/manager/network/NetworkCookieManagerImpl.kt

david-livefront · 2026-02-12T22:55:38Z

network/src/main/kotlin/com/bitwarden/network/interceptor/CookieInterceptor.kt

+        // Return the response if it is not a redirect or does not contain
+        // a Location header.
+        val location = response.header(HEADER_LOCATION)
+        if (response.code != HTTP_REDIRECT || location == null) {


Should this be an &&?

Hm. Now that I think about it, we shouldn't need this location check. We should technically already have the IdP login URL saved. Could have swore I saw that in the spec somewhere. 🤔

Let me double check with the other teams and make sure we actually need to check and capture this.

Thanks for calling attention to this.

network/src/main/kotlin/com/bitwarden/network/BitwardenServiceClientImpl.kt

Add cookie interceptor that manages ELB session cookies for enterprise SSO configurations. The interceptor attaches stored cookies to API requests, preempts requests when cookie bootstrap is needed, and detects HTTP 302 redirects to trigger cookie re-acquisition via an identity provider. Key components: - CookieInterceptor for OkHttp request/response cookie handling - NetworkCookieManager for bootstrap detection and cookie storage - CookieRedirectException to signal cookie acquisition flow - CookieProvider interface bridging network and app layers Co-Authored-By: Claude <noreply@anthropic.com>

This commit updates the `RetrofitsTest` to ensure that the `CookieInterceptor` is consistently called across all Retrofit client configurations. Previously, the tests for `api`, `identity`, `events`, and `notifications` clients did not verify the presence of the cookie interceptor. This change adds an assertion (`assertTrue(isCookieInterceptorCalled)`) to each relevant test case to confirm that the interceptor is included in the chain.

This commit refactors the `CookieRedirectException` to simplify its interface and updates the `CookieInterceptor` accordingly. The `location` property has been removed from `CookieRedirectException`. This property was redundant as it was not being used by any of the callers, and its removal simplifies the exception's constructor and message. The `CookieInterceptor` is updated to no longer extract or pass the `Location` header value when throwing a `CookieRedirectException`, aligning with the exception's new, cleaner interface.

claude · 2026-02-17T13:55:12Z

network/src/main/kotlin/com/bitwarden/network/interceptor/CookieInterceptor.kt

+        if (response.code != HTTP_REDIRECT) {
+            return response
+        }
+
+        // Close the response body to release the connection back to the pool
+        // before throwing to cancel the current request.
+        response.close()
+        cookieProvider.acquireCookies(hostname)
+        throw CookieRedirectException(
+            hostname = hostname,
+        )
+    }


⚠️ IMPORTANT -- Correctness: HTTP 302 detection is unreachable as an application interceptor

This interceptor is registered via addInterceptor() in RetrofitsImpl, making it an OkHttp application interceptor. OkHttp's internal RetryAndFollowUpInterceptor runs after application interceptors in the chain and automatically follows 302 redirects (the default followRedirects is true and is never set to false in this codebase). As a result, chain.proceed(request) returns the final response after all redirects have been followed -- the application interceptor never sees the intermediate 302 status code.

This means:

The response.code != HTTP_REDIRECT check on line 66 would only see the final response (e.g., 200 HTML from the IdP, or a parsing error), not the 302.

The response.close() connection safety and CookieRedirectException throw on lines 72-76 are unreachable under normal operation.

The expired cookie scenario (cookies were attached but the server rejects them with a redirect) won't trigger the clean re-acquisition flow as designed.

The preemptive bootstrap path (lines 58-61) works correctly because it runs before chain.proceed().

To make the 302 detection work, consider one of:

Register this as a network interceptor via addNetworkInterceptor(), which sees raw responses before redirect following.

Set followRedirects(false) on the OkHttpClient and handle redirects manually.

Detect the redirect condition from the final response characteristics (e.g., unexpected content type or domain change) rather than relying on the 302 status code.

Updated to use addNetworkInterceptor for cookie interceptor.

This commit updates the construction of the base `OkHttpClient` to use `addNetworkInterceptor` for the `cookieInterceptor` instead of `addInterceptor`. This change ensures that the `cookieInterceptor` is added as a network interceptor, which is the correct type for interceptors that need to operate on the final request just before it's transmitted over the network and observe the response as it comes back. This is particularly important for handling cookies, as they are part of the HTTP headers that are added and processed closer to the network layer.

david-livefront · 2026-02-17T17:55:44Z

network/src/main/kotlin/com/bitwarden/network/retrofit/RetrofitsImpl.kt


    private val baseOkHttpClient: OkHttpClient = OkHttpClient.Builder()
        .addInterceptor(headersInterceptor)
+        .addNetworkInterceptor(cookieInterceptor)


github-actions bot added app:password-manager Bitwarden Password Manager app context app:authenticator Bitwarden Authenticator app context labels Feb 11, 2026

SaintPatrck added the ai-review Request a Claude code review label Feb 11, 2026

github-advanced-security bot found potential problems Feb 11, 2026

View reviewed changes

SaintPatrck marked this pull request as ready for review February 11, 2026 22:59

SaintPatrck requested review from a team and david-livefront as code owners February 11, 2026 22:59

SaintPatrck force-pushed the cookie-vending/p9-t9_cookie-interceptor branch from 5087f55 to 0dc2bc5 Compare February 12, 2026 01:03

claude bot reviewed Feb 12, 2026

View reviewed changes

network/src/test/kotlin/com/bitwarden/network/retrofit/RetrofitsTest.kt Show resolved Hide resolved

david-livefront reviewed Feb 12, 2026

View reviewed changes

...rc/main/kotlin/com/x8bit/bitwarden/data/platform/manager/network/NetworkCookieManagerImpl.kt Outdated Show resolved Hide resolved

SaintPatrck requested a review from david-livefront February 12, 2026 21:45

david-livefront reviewed Feb 12, 2026

View reviewed changes

david-livefront reviewed Feb 13, 2026

View reviewed changes

network/src/main/kotlin/com/bitwarden/network/BitwardenServiceClientImpl.kt Outdated Show resolved Hide resolved

SaintPatrck and others added 3 commits February 17, 2026 08:38

Make bootstrapType the subject of when

9e7350e

SaintPatrck force-pushed the cookie-vending/p9-t9_cookie-interceptor branch from 4307955 to afec3d5 Compare February 17, 2026 13:42

SaintPatrck force-pushed the cookie-vending/p9-t9_cookie-interceptor branch from afec3d5 to c421c14 Compare February 17, 2026 13:45

SaintPatrck requested a review from david-livefront February 17, 2026 13:46

claude bot reviewed Feb 17, 2026

View reviewed changes

david-livefront reviewed Feb 17, 2026

View reviewed changes

david-livefront approved these changes Feb 17, 2026

View reviewed changes

SaintPatrck added this pull request to the merge queue Feb 17, 2026

Merged via the queue into main with commit 436ae93 Feb 17, 2026
28 checks passed

SaintPatrck deleted the cookie-vending/p9-t9_cookie-interceptor branch February 17, 2026 19:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PM-29885] Implement SSO cookie vending authentication flow#6522

[PM-29885] Implement SSO cookie vending authentication flow#6522
SaintPatrck merged 5 commits intomainfrom
cookie-vending/p9-t9_cookie-interceptor

SaintPatrck commented Feb 11, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Feb 11, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

claude bot commented Feb 11, 2026 •

edited

Loading

Uh oh!

codecov bot commented Feb 11, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

david-livefront Feb 12, 2026

Uh oh!

SaintPatrck Feb 13, 2026

Uh oh!

Uh oh!

claude bot Feb 17, 2026

Uh oh!

SaintPatrck Feb 17, 2026

Uh oh!

david-livefront Feb 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

SaintPatrck commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎟️ Tracking

📔 Objective

Uh oh!

github-actions bot commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

claude bot commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Re-Review Summary

Previous Findings Status

Changes Since Last Review

Verification

Result

Uh oh!

codecov bot commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

david-livefront Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

SaintPatrck Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

claude bot Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

SaintPatrck Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

david-livefront Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

SaintPatrck commented Feb 11, 2026 •

edited

Loading

github-actions bot commented Feb 11, 2026 •

edited

Loading

claude bot commented Feb 11, 2026 •

edited

Loading

codecov bot commented Feb 11, 2026 •

edited

Loading