[PM-33981] feat: Add device management models and API layer#2870
[PM-33981] feat: Add device management models and API layer#2870andrebispo5 wants to merge 3 commits into
Conversation
- Promote DeviceType from Int typealias to a proper enum in BitwardenKit with a safe Decodable fallback to .unknownBrowser for unknown values; update DefaultHeadersRequestHandler and IdentityTokenRequestModel to use .rawValue where an Int is required - Add DeviceTypeCategory and DeviceType display extensions (category, platform, displayName, pendingRequestMatchKey) in BitwardenShared Core - Add DeviceActivityStatus enum mapping server dates to activity buckets using calendar-day comparison - Add DeviceListItem domain model, DeviceResponse/DevicesListResponse API models, CurrentDeviceRequest/DevicesListRequest, and DeviceAPIService with getCurrentDevice(appId:) and getDevices() methods - Add deviceAPIService as a proper init parameter to ServiceContainer (consistent with all other services); update withMocks() default to MockDeviceAPIService() - Add tests for all new types and request/response models; add JSON fixtures CurrentDevice.json and DevicesList.json
github-actions
Bot
added
app:password-manager
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the device management data and API layer: the Code Review DetailsNo new findings. The All existing review threads on this PR appear addressed in the current code (the |
andrebispo5
added
the
innovation
There was a problem hiding this comment.
Pull request overview
Adds the device-management data/API layer (no UI), including new device request/response models, domain mapping helpers, and a promoted DeviceAPIService dependency in ServiceContainer.
Changes:
- Introduces device API models/requests (
DeviceResponse,DevicesListResponse,CurrentDeviceRequest,DevicesListRequest) and extendsDeviceAPIServicewithgetCurrentDevice(appId:)andgetDevices(). - Promotes
DeviceTypeinBitwardenKitfrom anInttypealias to a realenumwith safe decoding fallback; updates dependent request/header building to userawValue. - Adds device display/mapping utilities (
DeviceTypeCategory,DeviceActivityStatus,DeviceListItem) plus fixtures and unit tests.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| BitwardenShared/Core/Platform/Services/TestHelpers/ServiceContainer+Mocks.swift | Adds injectable deviceAPIService to mocks container defaults. |
| BitwardenShared/Core/Platform/Services/ServiceContainer.swift | Promotes deviceAPIService to an initializer dependency and stores it instead of computing from apiService. |
| BitwardenShared/Core/Auth/Services/API/Device/Requests/DevicesListRequestTests.swift | Adds Swift Testing coverage for DevicesListRequest defaults. |
| BitwardenShared/Core/Auth/Services/API/Device/Requests/DevicesListRequest.swift | Adds GET /devices request model. |
| BitwardenShared/Core/Auth/Services/API/Device/Requests/CurrentDeviceRequestTests.swift | Adds Swift Testing coverage for CurrentDeviceRequest defaults/path interpolation. |
| BitwardenShared/Core/Auth/Services/API/Device/Requests/CurrentDeviceRequest.swift | Adds GET /devices/identifier/{appId} request model. |
| BitwardenShared/Core/Auth/Services/API/Device/Fixtures/DevicesList.json | Adds JSON fixture for devices list response decoding/tests. |
| BitwardenShared/Core/Auth/Services/API/Device/Fixtures/CurrentDevice.json | Adds JSON fixture for current device response decoding/tests. |
| BitwardenShared/Core/Auth/Services/API/Device/Fixtures/APITestData+Device.swift | Registers device fixtures for API tests. |
| BitwardenShared/Core/Auth/Services/API/Device/DeviceAPIServiceTests.swift | Adds unit tests for new DeviceAPIService endpoints. |
| BitwardenShared/Core/Auth/Services/API/Device/DeviceAPIService.swift | Extends protocol + APIService conformance with getCurrentDevice/getDevices. |
| BitwardenShared/Core/Auth/Models/Response/Fixtures/DeviceResponse+Fixtures.swift | Adds test fixture builder for DeviceResponse. |
| BitwardenShared/Core/Auth/Models/Response/DevicesListResponseTests.swift | Adds decoding test for DevicesListResponse. |
| BitwardenShared/Core/Auth/Models/Response/DevicesListResponse.swift | Adds response wrapper model for list endpoint. |
| BitwardenShared/Core/Auth/Models/Response/DeviceResponseTests.swift | Adds decoding test for DeviceResponse. |
| BitwardenShared/Core/Auth/Models/Response/DeviceResponse.swift | Adds device API response model. |
| BitwardenShared/Core/Auth/Models/Request/IdentityTokenRequestModel.swift | Updates device type form encoding to use DeviceType.rawValue. |
| BitwardenShared/Core/Auth/Models/Enum/DeviceType.swift | Adds device type categorization + display utilities on BitwardenKit.DeviceType. |
| BitwardenShared/Core/Auth/Models/Enum/DeviceActivityStatusTests.swift | Adds tests for activity bucketing and localization strings. |
| BitwardenShared/Core/Auth/Models/Enum/DeviceActivityStatus.swift | Adds calendar-day-based bucketing for “last active” timestamps. |
| BitwardenShared/Core/Auth/Models/Domain/Fixtures/DeviceListItem+Fixtures.swift | Adds test fixture builder for DeviceListItem. |
| BitwardenShared/Core/Auth/Models/Domain/DeviceListItem.swift | Adds UI-friendly domain model + mapping init from DeviceResponse. |
| BitwardenResources/Localizations/en.lproj/Localizable.strings | Adds localization keys for device category/status/display formatting. |
| BitwardenKit/Core/Platform/Utilities/Constants.swift | Removes DeviceType typealias and updates constants to use new enum. |
| BitwardenKit/Core/Platform/Services/API/Handlers/DefaultHeadersRequestHandler.swift | Updates device-type header value to use rawValue. |
| BitwardenKit/Core/Platform/Models/Enum/DeviceType.swift | Introduces DeviceType: Int enum with safe decoding fallback. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2870 +/- ##
==========================================
- Coverage 81.27% 79.00% -2.28%
==========================================
Files 1027 1157 +130
Lines 66109 73641 +7532
==========================================
+ Hits 53733 58183 +4450
- Misses 12376 15458 +3082 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…or backward compat Tests that configure device API responses via MockHTTPClient (e.g. LoginProcessorTests) continue to work unchanged. Callers that need precise mock control pass MockDeviceAPIService explicitly.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| /// Initializes a `DeviceListItem` with all properties. | ||
| /// | ||
| /// - Parameters: | ||
| /// - activityStatus: The activity status of the device. | ||
| /// - deviceType: The type of the device. | ||
| /// - displayName: The display name of the device. | ||
| /// - firstLogin: The date of the first login on this device. | ||
| /// - id: The server-assigned UUID for this device record. | ||
| /// - identifier: The client-generated UUID identifying the app installation. | ||
| /// - isCurrentSession: Whether this is the current session's device. | ||
| /// - isTrusted: Whether the device is trusted. | ||
| /// - lastActivityDate: The date of the last activity on this device. | ||
| /// - pendingRequest: The most recent pending login request for this device. | ||
| /// | ||
| init( | ||
| activityStatus: DeviceActivityStatus, | ||
| deviceType: DeviceType, | ||
| displayName: String, | ||
| firstLogin: Date, | ||
| id: String, | ||
| identifier: String, | ||
| isCurrentSession: Bool, | ||
| isTrusted: Bool, | ||
| lastActivityDate: Date?, | ||
| pendingRequest: LoginRequest?, | ||
| ) { | ||
| self.activityStatus = activityStatus | ||
| self.deviceType = deviceType | ||
| self.displayName = displayName | ||
| self.firstLogin = firstLogin | ||
| self.id = id | ||
| self.identifier = identifier | ||
| self.isCurrentSession = isCurrentSession | ||
| self.isTrusted = isTrusted | ||
| self.lastActivityDate = lastActivityDate | ||
| self.pendingRequest = pendingRequest | ||
| } | ||
|
|
||
| /// Initializes a `DeviceListItem` from a `DeviceResponse`. | ||
| /// | ||
| /// - Parameters: | ||
| /// - device: The device response from the API. | ||
| /// - timeProvider: The time provider to use for calculating the activity status. | ||
| /// | ||
| init( | ||
| device: DeviceResponse, | ||
| timeProvider: TimeProvider, | ||
| ) { | ||
| activityStatus = DeviceActivityStatus(from: device.lastActivityDate, timeProvider: timeProvider) | ||
| deviceType = device.type | ||
| displayName = device.type.displayName | ||
| firstLogin = device.creationDate | ||
| id = device.id | ||
| identifier = device.identifier | ||
| isCurrentSession = false | ||
| isTrusted = device.isTrusted | ||
| lastActivityDate = device.lastActivityDate | ||
| pendingRequest = nil | ||
| } |
There was a problem hiding this comment.
🎨 If you move the custom initializer init(device:timeProvider) into an extension on this type, then you'll get the top initializer synthesized for free.
| case 15 ... 30: | ||
| self = .thisMonth |
There was a problem hiding this comment.
🤔 Shouldn't "this month" only be the case if the month components are the same? If it's July 15, and the device was last active June 20 (25 days ago), this would return .thisMonth, but that's actually last month.
| var deviceAPIService: DeviceAPIService { | ||
| apiService | ||
| } |
There was a problem hiding this comment.
❓ I'm curious why change this? Do we need to store a copy of the APIService as DeviceAPIService in the service container instead of just using this computed variable?
There was a problem hiding this comment.
It's needed as a stored property, not revertible to a computed passthrough — checked the git history on this branch and it was a deliberate change. Earlier revisions had a test-only mutable
deviceAPIServiceOverridevar with a computeddeviceAPIServiceproperty (deviceAPIServiceOverride ?? apiService), which is the anti-pattern this commit removed. Promoting it to a properinitparameter letsServiceContainer.withMocks(deviceAPIService:)inject a realMockDeviceAPIServicethrough the standard DI path (consistent withauthService,configService, and most other services in the container), instead of a mutable side-door override. Reverting to a computed{ apiService }passthrough would mean bringing that mutable override var back to keep it testable — happy to do that if you'd prefer it, but I think the stored-property version is the better outcome here.
There was a problem hiding this comment.
⛏️ Should this be named DeviceTypeCategory.swift to match the enum?
| public init(from decoder: any Decoder) throws { | ||
| let container = try decoder.singleValueContainer() | ||
| let rawValue = try container.decode(Int.self) | ||
| self = DeviceType(rawValue: rawValue) ?? .unknownBrowser | ||
| } |
There was a problem hiding this comment.
🎨 Instead of implementing a custom initializer to provide a default unknown value, you can use the DefaultValue property wrapper.
DeviceType would need to conform to DefaultValueProvider:
// MARK: - DefaultValueProvider
extension DeviceType: DefaultValueProvider {
public static var defaultValue: DeviceType { .unknownBrowser }
}
And then in DeviceResponse, the type can be wrapped with the property wrapper:
@DefaultValue var type: DeviceType
- Use the DefaultValue property wrapper for DeviceResponse.type instead of a hand-rolled Decodable init, matching the CollectionType/ CipherRepromptType pattern; add Sendable conformance to DefaultValue so it composes with Sendable response models - Fix DeviceActivityStatus to classify "this month" by actual calendar month instead of a fixed 15-30 day range, which mislabeled dates from a previous calendar month (e.g. June 20 relative to a July 15 "now") - Move DeviceListItem's DeviceResponse-based init into an extension so the compiler synthesizes the memberwise init instead of hand-writing it - Rename BitwardenShared DeviceType.swift to DeviceTypeCategory.swift to match the DeviceTypeCategory enum it defines - Add missing coverage: DeviceType/DeviceResponse unknown-value decode fallback, and DeviceTypeCategory/DeviceType category-platform- displayName-pendingRequestMatchKey mappings
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-33981
📔 Objective
Adds the complete data and API layer for the device management feature — no UI changes in this PR.
DeviceTypefrom anInttypealias to a properenum DeviceType: Intin BitwardenKit, with a safeDecodablefallback to.unknownBrowserfor unknown server valuesDeviceTypeCategoryandDeviceTypedisplay extensions (category,platform,displayName,pendingRequestMatchKey) in BitwardenShared CoreDeviceActivityStatusenum mapping server timestamps to activity buckets using calendar-day comparisonDeviceListItemdomain model,DeviceResponse/DevicesListResponseAPI models,CurrentDeviceRequest/DevicesListRequest, andDeviceAPIServicewithgetCurrentDevice(appId:)andgetDevices()endpointsdeviceAPIServiceto a properServiceContainerinit parameter (consistent with all other services); updateswithMocks()default toMockDeviceAPIService()📸 Screenshots
No UI changes in this PR.