Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #38690 +/- ##
==========================================
+ Coverage 66.08% 66.14% +0.06%
==========================================
Files 2414 2414
Lines 192657 192678 +21
Branches 8424 8427 +3
==========================================
+ Hits 127309 127454 +145
+ Misses 53788 53648 -140
- Partials 11560 11576 +16
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis PR implements automatic re-enrollment for the Android certificate app when the Fleet server returns HTTP 401 Unauthorized responses. When a deleted and re-added host's API key becomes invalid, the app now clears the key, re-enrolls to obtain a new one, and retries the failed request. Changes include new re-enrollment wrapper logic in ApiClient, test mode support in KeystoreManager, comprehensive integration tests, and dependency additions. Changes
Sequence DiagramsequenceDiagram
actor App as Android App
participant ApiClient
participant DataStore as Data Store<br/>(API Key)
participant FleetServer as Fleet Server
rect rgba(200, 150, 255, 0.5)
Note over App,FleetServer: Initial Request with Valid Key
App->>ApiClient: getOrbitConfig()
ApiClient->>DataStore: Retrieve API Key
DataStore-->>ApiClient: Return Valid Key (e.g., node_key_v1)
ApiClient->>FleetServer: GET /api/fleet/orbit/config<br/>Header: Authorization: node_key_v1
FleetServer-->>ApiClient: 200 OK + Config
ApiClient-->>App: Success(OrbitConfig)
end
rect rgba(255, 150, 150, 0.5)
Note over App,FleetServer: Host Deleted & Re-added<br/>Key Becomes Invalid
App->>ApiClient: getOrbitConfig()
ApiClient->>DataStore: Retrieve API Key
DataStore-->>ApiClient: Return Expired Key (e.g., node_key_v1)
ApiClient->>FleetServer: GET /api/fleet/orbit/config<br/>Header: Authorization: node_key_v1
FleetServer-->>ApiClient: 401 Unauthorized
ApiClient->>ApiClient: withReenrollOnUnauthorized detects 401
end
rect rgba(150, 200, 150, 0.5)
Note over App,FleetServer: Clear Key & Re-enroll
ApiClient->>DataStore: clearApiKey()
DataStore-->>ApiClient: Key cleared
ApiClient->>FleetServer: POST /api/fleet/orbit/enroll<br/>Body: enroll_secret
FleetServer-->>ApiClient: 200 OK + New Node Key (node_key_v2)
ApiClient->>DataStore: Store New Key (node_key_v2)
DataStore-->>ApiClient: Stored
end
rect rgba(150, 200, 150, 0.5)
Note over App,FleetServer: Retry Original Request
ApiClient->>FleetServer: GET /api/fleet/orbit/config<br/>Header: Authorization: node_key_v2
FleetServer-->>ApiClient: 200 OK + Config
ApiClient-->>App: Success(OrbitConfig)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@android/app/src/main/java/com/fleetdm/agent/ApiClient.kt`:
- Around line 88-92: The code currently auto-retries enrollment using the stored
enroll secret on 401 responses which conflicts with the one-time secret policy;
update the 401 handling to stop automatic re-enrollment and require manual
re-enroll (or confirm backend supports reuse). Locate the API client 401
handling logic and remove or disable the path that reads the stored secret and
retries enrollment; instead call clearApiKey() to remove the secret and return
an unrecoverable/auth-failure state or surface a user-visible error prompting
manual re-enrollment. Ensure any helper/utility that previously invoked the
retry path is updated to not call the enroll retry function and to propagate the
auth-failure up for manual intervention.
In `@android/gradle/libs.versions.toml`:
- Line 25: Update the okhttp version declared under the okhttp key by replacing
"4.12.0" with "5.3.2" (or the chosen recent 5.x release) in libs.versions.toml,
then run a full Gradle build to surface breakages and resolve them (adjust
artifact coordinates/imports and API changes where code references okhttp
classes). Search for the okhttp dependency key "okhttp" and all usages of the
old version "4.12.0" to ensure consistency, fix compilation/test failures, and
update any Gradle module dependency declarations that consume the version
constant.
🧹 Nitpick comments (2)
android/app/src/main/java/com/fleetdm/agent/KeystoreManager.kt (1)
19-42: Make test-mode API idempotent and test‑scoped.Regenerating the key on repeated calls can break decrypts if any encrypted values exist, and public visibility risks accidental production use. Consider making it idempotent and internal.
♻️ Proposed refactor
- fun enableTestMode() { - testMode = true - testKey = KeyGenerator.getInstance("AES").apply { - init(256) - }.generateKey() - } + internal fun enableTestMode() { + if (testMode && testKey != null) return + testMode = true + testKey = KeyGenerator.getInstance("AES").apply { + init(256) + }.generateKey() + } - fun disableTestMode() { + internal fun disableTestMode() { testMode = false testKey = null }android/app/build.gradle.kts (1)
279-279: MockWebServer version is well-aligned but outdated.The OkHttp and MockWebServer versions are correctly pinned to the same version (4.12.0), which maintains compatibility. However, this version is over 2 years old; the latest release is 5.3.2 (Nov 18, 2025). Consider upgrading to the current version if the project's Android API minimum is 5.0+ (API 21+), as OkHttp 5.x has new features and security improvements. If the project targets a lower API level, the current 4.12.0 is acceptable.
| private suspend fun clearApiKey() { | ||
| dataStore.edit { preferences -> | ||
| preferences.remove(API_KEY) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Get the ApiClient.kt file to examine the withReenrollOnUnauthorized wrapper at lines 179-210
cd android/app/src/main/java/com/fleetdm/agent
wc -l ApiClient.ktRepository: fleetdm/fleet
Length of output: 73
🏁 Script executed:
# View the withReenrollOnUnauthorized wrapper and surrounding context
cd android/app/src/main/java/com/fleetdm/agent
sed -n '175,215p' ApiClient.ktRepository: fleetdm/fleet
Length of output: 1751
🏁 Script executed:
# Also check the clearApiKey usage and how enrollment is triggered
cd android/app/src/main/java/com/fleetdm/agent
grep -n "clearApiKey\|enroll\|enrollSecret" ApiClient.kt | head -30Repository: fleetdm/fleet
Length of output: 2170
🏁 Script executed:
# Check AgentApplication.kt to understand the enrollment flow and secret handling
cd android/app/src/main/java/com/fleetdm/agent
sed -n '70,90p' AgentApplication.ktRepository: fleetdm/fleet
Length of output: 1042
Automatic re-enrollment contradicts one-time enroll secret policy.
This code auto-retries enrollment with the stored enroll secret when receiving a 401 (lines 202–210, 238–242, 264–271, 304–310). However, the stored secret is designed as a one-time credential with an intended manual unenroll/re-enroll workflow—automatic retry will fail and leave the device unrecovered. Either align the implementation with the one-time policy (remove auto-retry or require manual re-enroll) or confirm the backend now supports secret reuse and update the enrollment contract accordingly.
🤖 Prompt for AI Agents
In `@android/app/src/main/java/com/fleetdm/agent/ApiClient.kt` around lines 88 -
92, The code currently auto-retries enrollment using the stored enroll secret on
401 responses which conflicts with the one-time secret policy; update the 401
handling to stop automatic re-enrollment and require manual re-enroll (or
confirm backend supports reuse). Locate the API client 401 handling logic and
remove or disable the path that reads the stored secret and retries enrollment;
instead call clearApiKey() to remove the secret and return an
unrecoverable/auth-failure state or surface a user-visible error prompting
manual re-enrollment. Ensure any helper/utility that previously invoked the
retry path is updated to not call the enroll retry function and to propagate the
auth-failure up for manual intervention.
There was a problem hiding this comment.
I believe this is ok, we don't expect the enroll secret to change
android/gradle/libs.versions.toml
Outdated
| navigationCompose = "2.9.6" | ||
| json = "20231013" | ||
| mockk = "1.13.13" | ||
| okhttp = "4.12.0" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
latest OkHttp version release 2025 2026
💡 Result:
As of January 23, 2026, the latest OkHttp release on Maven Central is 5.3.2, released November 18, 2025. [1] (The OkHttp changelog also lists 5.3.2 — 2025-11-18.) [2]
Recent 2025 stable releases (changelog):
- 5.0.0 — 2025-07-03 [2]
- 5.1.0 — 2025-07-07 [1]
- 5.2.0 — 2025-10-07 [2]
- 5.3.0 — 2025-10-30 [2]
- 5.3.2 — 2025-11-18 [2]
No newer (2026-dated) OkHttp releases are shown in Maven Central’s directory listing up through 5.3.2. [1]
Sources
- Maven Central directory for
com.squareup.okhttp3:okhttp(shows latest version and timestamps) [1] - OkHttp changelog (lists version release dates, including 5.3.2) [2]
Consider upgrading OkHttp to 5.3.2 or a recent 5.x release.
Version 4.12.0 is significantly outdated. The latest OkHttp release is 5.3.2 (November 18, 2025), with multiple stable 5.x versions available since July 2025. Evaluate compatibility and breaking changes if upgrading.
Also applies to: 59-59
🤖 Prompt for AI Agents
In `@android/gradle/libs.versions.toml` at line 25, Update the okhttp version
declared under the okhttp key by replacing "4.12.0" with "5.3.2" (or the chosen
recent 5.x release) in libs.versions.toml, then run a full Gradle build to
surface breakages and resolve them (adjust artifact coordinates/imports and API
changes where code references okhttp classes). Search for the okhttp dependency
key "okhttp" and all usages of the old version "4.12.0" to ensure consistency,
fix compilation/test failures, and update any Gradle module dependency
declarations that consume the version constant.
dantecatalfamo
left a comment
There was a problem hiding this comment.
Cool way of re-executing blocks after a failure!
Related issue: Resolves #38035
When an Android host is deleted from Fleet and comes back, the app's stored node key becomes invalid. Previously, API calls would fail with 401 and never recover because the app assumed the enroll secret was
one-time.
This PR adds automatic re-enrollment on 401:
This change was AI assisted, so please be critical.
Changes file added for user-visible changes in
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Added/updated automated tests
QA'd all new/changed functionality manually
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.