[PM-31954] Add server communication models to ConfigResponseJson #6500
Conversation
github-actions
bot
added
app:password-manager
|
Great job! No new security vulnerabilities introduced in this pull request |
SaintPatrck
force-pushed
the
PM-31954/config-response-communication-prop
branch
2 times, most recently
from
429ef05 to
fe4bbb7
Compare
| @Serializable | ||
| data class CommunicationJson( | ||
| @SerialName("bootstrap") | ||
| val bootstrap: BootstrapJson?, |
There was a problem hiding this comment.
Is this really nullable?
If they send the CommunicationJson, I would expect this property to be present, right?
There was a problem hiding this comment.
Correct. Didn't mean to make this one nullable. Thanks.
| val cookieName: String?, | ||
|
|
||
| @SerialName("cookieDomain") | ||
| val cookieDomain: String?, |
There was a problem hiding this comment.
Is everything on this model really nullable?
There was a problem hiding this comment.
Based on the server PR, these are all nullable.
There was a problem hiding this comment.
So the BootstrapJson could be present but completely empty?
SaintPatrck
force-pushed
the
PM-31954/config-response-communication-prop
branch
from
fe4bbb7 to
60da5ff
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6500 +/- ##
=======================================
Coverage 86.36% 86.36%
=======================================
Files 767 767
Lines 56035 56045 +10
Branches 8151 8152 +1
=======================================
+ Hits 48395 48405 +10
Misses 4800 4800
Partials 2840 2840 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
SaintPatrck
force-pushed
the
PM-31954/config-response-communication-prop
branch
from
60da5ff to
82a4331
Compare
SaintPatrck
added
ai-review
|
Claude finished @SaintPatrck's task in 1m 56s —— View job Code Review Complete
Status: Review completed and posted to GitHub. No blocking issues found. The PR correctly adds new The existing discussion about nullable properties has been resolved - |
Introduce new data models to the `network` module for handling server communication configuration received in the `/config` endpoint response.
Specifically, adds `CommunicationJson` and its nested `BootstrapJson` class to `ConfigResponseJson` to support server-driven cookie acquisition flows.
### Specific Changes:
- **`ConfigResponseJson.kt`**
- Added a new nullable property `communication: CommunicationJson?` to `ConfigResponseJson`.
- Defined the new serializable data class `CommunicationJson`, which contains a non-nullable `bootstrap: BootstrapJson` property.
- Defined the nested serializable data class `BootstrapJson`, which includes properties for SSO cookie vendor authentication:
- `type: String`
- `idpLoginUrl: String?`
- `cookieName: String?`
- `cookieDomain: String?`
SaintPatrck
force-pushed
the
PM-31954/config-response-communication-prop
branch
from
82a4331 to
c892ba1
Compare
3 participants
🎟️ Tracking
PM-31954
Relates to bitwarden/server#6892
📔 Objective
PM-27124 Add server communication models to ConfigResponseJson
Introduce new data models to the
networkmodule for handling server communication configuration received in the/configendpoint response.Specifically, adds
CommunicationJsonand its nestedBootstrapJsonclass toConfigResponseJsonto support server-driven cookie acquisition flows.Specific Changes:
ConfigResponseJson.ktcommunication: CommunicationJson?toConfigResponseJson.CommunicationJson, which contains a nullablebootstrap: BootstrapJson?property.BootstrapJson, which includes properties for SSO cookie vendor authentication:type: StringidpLoginUrl: String?cookieName: String?cookieDomain: String?⏰ 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