[PM-39840] fix: Use dedicated OkHttpClient for fill-assist to prevent load-balancer prompt#7131
Conversation
…tor and PermissionInterceptor
github-actions
Bot
added
app:password-manager
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the fix that routes Code Review DetailsNo blocking findings.
Minor note (not blocking): the PR description states the fix excludes |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7131 +/- ##
==========================================
+ Coverage 86.19% 86.76% +0.57%
==========================================
Files 1032 868 -164
Lines 67112 63939 -3173
Branches 9443 9577 +134
==========================================
- Hits 57848 55479 -2369
+ Misses 6001 5017 -984
- Partials 3263 3443 +180
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
|
||
| // Fill-assist might use HTTP 302 redirects for content delivery. | ||
| // CookieInterceptor treats all 302s as auth redirects. | ||
| private val fillAssistOkHttpClient: OkHttpClient = OkHttpClient.Builder() |
There was a problem hiding this comment.
Can we make this more generic? I can envision a future API that also need to avoid the Cookie Interceptor.
Something that makes it more reusable and not explicitly tied to the FillAssist feature.
| ) | ||
| .build() | ||
|
|
||
| private fun createFillAssistRetrofit( |
There was a problem hiding this comment.
Can we rename this function to createExternalRetrofit to match the OkHttpClient too.
| // For requests to external (non-Bitwarden) URLs. CookieInterceptor must be excluded because | ||
| // it treats all 302s as Bitwarden load-balancer auth redirects, which is only correct for | ||
| // Bitwarden's own infrastructure. | ||
| // Bitwarden own infrastructure. |
There was a problem hiding this comment.
This should have an s, right?
2 participants
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-39840
📔 Objective
The fill-assist service fetches targeting rules from
https://github.com/bitwarden/map-the-web/releases/latest/download/manifest.json. GitHub's CDN responds with an HTTP 302 redirect for content delivery.The
CookieInterceptor, present on the shared unauthenticatedOkHttpClient, treats any 302 response as a Bitwarden load-balancer auth redirect. It callscookieProvider.acquireCookies(hostname)as a side effect before throwingCookieRedirectException, which navigates the app to the "Sync with Browser" load-balancer screen — even though the 302 was a legitimate CDN redirect unrelated to Bitwarden authentication.This affected users on self-hosted environments without a load balancer (USQA, USDEV, and similar), blocking sign-in and vault unlock.
Fix: Introduce a dedicated
fillAssistOkHttpClientinRetrofitsImplthat keeps headers and SSL certificate validation but excludesCookieInterceptorandPermissionInterceptor.fillAssistRetrofitnow uses this client so OkHttp's default redirect handling processes the CDN 302 correctly.