[PM-29144] Add server communication config to /api/config
#6892
Conversation
|
Claude finished @dereknance's task in 1m 21s —— View job Code Review: PM-29144 Add server communication config to
|
| Severity | Finding | Location |
|---|---|---|
Missing unit tests for CommunicationSettings.Build() logic |
ConfigResponseModel.cs |
Analysis
What this PR does well:
- Clean integration with existing
ConfigResponseModelstructure - Proper null safety with null-conditional operators
- Case-insensitive bootstrap type matching
- Graceful handling of unknown/unconfigured bootstrap types (returns
null) - Configuration exposed is non-sensitive (URLs, cookie names/domains for client bootstrapping)
Security Assessment:
The configuration values exposed (IdpLoginUrl, CookieName, CookieDomain) are appropriate for an unauthenticated endpoint as they contain no secrets and are required for clients to properly bootstrap their connection.
Reviewed by Claude via Bitwarden Code Review
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
New Issues (4)Checkmarx found the following issues in this Pull Request
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6892 +/- ##
==========================================
+ Coverage 56.07% 56.10% +0.02%
==========================================
Files 1968 1968
Lines 86927 87027 +100
Branches 7742 7753 +11
==========================================
+ Hits 48744 48825 +81
- Misses 36383 36392 +9
- Partials 1800 1810 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
dereknance
force-pushed
the
pm-29144-communication-in-config-api
branch
from
8c8e2b0 to
d0398e4
Compare
|
While writing up tests for QA, I realized the shape of the config response was wrong. So now the response contains |
🎟️ Tracking
PM-29144
📔 Objective
This PR adds the new config from #6880 to the
/api/configendpoint so clients may learn if the server requires extra work to establish a connection.endpoint payload comparison
Without
ssoCookieVendorThe
communicationproperty is present, but null.{ "version": "2026.1.0", "gitHash": "f9c0cec8", "server": null, "environment": { "cloudRegion": null, "vault": "https://localhost:8080", "api": "http://localhost:4000", "identity": "http://localhost:33656", "notifications": "http://localhost:61840", "sso": "http://localhost:51822" }, "featureStates": { "web-push": false }, "push": { "pushTechnology": 0, "vapidPublicKey": null }, "communication": null, "settings": { "disableUserRegistration": false }, "object": "config" }With
ssoCookieVendorThe
communicationproperty is present and includes the additional required properties for that type.{ "version": "2026.1.0", "gitHash": "f9c0cec8", "server": null, "environment": { "cloudRegion": null, "vault": "https://localhost:8080", "api": "http://localhost:4000", "identity": "http://localhost:33656", "notifications": "http://localhost:61840", "sso": "http://localhost:51822" }, "featureStates": { "web-push": false }, "push": { "pushTechnology": 0, "vapidPublicKey": null }, "communication": { "type": "ssoCookieVendor", "idpLoginUrl": "", "cookieName": "", "cookieDomain": "" }, "settings": { "disableUserRegistration": false }, "object": "config" }⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes