Android certificates resend generate resent_certificate#49249
Conversation
ksykulev
changed the title
There was a problem hiding this comment.
Pull request overview
This PR extends the SCIM→host mapping / variable-triggered resend flow to surface resent_certificate activity details when certificate templates are automatically resent due to IdP/SCIM-driven host variable changes.
Changes:
- Updated SCIM host-mapping datastore APIs to return
[]ActivityTypeResentCertificatefor any triggered automatic resends. - Marked auto-resend
ActivityTypeResentCertificatedetails as automated (viaWasFromAutomation) so activities can be attributed to Fleet automation. - Wired some service entrypoints/tests to handle the updated datastore signatures (and added activity emission in
SetHostDeviceMapping).
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| server/service/osquery_utils/queries.go | Adjusts SCIM mapping calls for new return signature (currently discards returned activities). |
| server/service/osquery_utils/queries_test.go | Updates mocks to match new datastore method signatures. |
| server/service/orbit.go | Adjusts EUA enrollment SCIM mapping calls for new return signature (should emit returned activities). |
| server/service/hosts.go | Emits resent_certificate activities from returned SCIM mapping resend list (needs actor/error handling fixes). |
| server/service/hosts_test.go | Updates mocks for new signatures; should assert the new activity emission behavior. |
| server/mock/datastore_mock.go | Updates datastore mock function types and method signatures. |
| server/fleet/datastore.go | Updates datastore interface signatures and comments to return resent cert activities. |
| server/fleet/activities.go | Adds automation marker plumbing for ActivityTypeResentCertificate. |
| server/datastore/mysql/scim.go | Selects affected cert templates for resend and marks returned activities as automated. |
| server/datastore/mysql/scim_test.go | Updates SCIM datastore tests for signature changes / activity return values. |
| server/datastore/mysql/hosts.go | Returns resent cert activities from SCIM mapping set/delete helpers. |
| server/datastore/mysql/hosts_test.go | Updates tests for changed helper return signatures. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ds.SetOrUpdateHostSCIMUserMappingFunc = func(ctx context.Context, hostID uint, scimUserID uint) ([]fleet.ActivityTypeResentCertificate, error) { | ||
| return nil, nil | ||
| } |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughSCIM host mapping datastore methods now return resent-certificate activities alongside errors. Automatic profile resend logic marks returned activities as automated. Host device mapping creates Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
server/datastore/mysql/scim_test.go (1)
2637-2766: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the new
resentCertsreturn value.Every call to
ds.SetOrUpdateHostSCIMUserMappinghere discards the returned[]fleet.ActivityTypeResentCertificateand only checkshost_scim_userstate/errors. Since generating this slice correctly (and not duplicating/over-triggering it) is the actual behavior this PR introduces, consider adding a scenario withhost_certificate_templates/mdm_configuration_profile_variables/fleet_variablesfixtures that asserts on the returned certs — including re-calling with the samescimUserIDto confirm no spurious/duplicate resend is triggered when nothing changed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/datastore/mysql/scim_test.go` around lines 2637 - 2766, Extend testSetOrUpdateHostSCIMUserMapping to exercise the returned []fleet.ActivityTypeResentCertificate value, using host_certificate_templates, mdm_configuration_profile_variables, and fleet_variables fixtures that cause certificate resends. Capture and assert the returned certificates for an applicable mapping update, then call SetOrUpdateHostSCIMUserMapping again with the same scimUserID and verify no duplicate or spurious certificates are returned while preserving the existing mapping assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/datastore/mysql/hosts.go`:
- Around line 4934-4979: The deleteHostSCIMUserMapping function should skip
certificate selection and profile resend work when the DELETE affects no rows.
Capture the DELETE result, check RowsAffected(), and return an empty resend list
immediately when it is zero; preserve the existing error handling and resend
flow when a mapping was removed.
In `@server/service/orbit.go`:
- Around line 347-352: The SCIM mapping flows discard
ActivityTypeResentCertificate results, preventing audit activities. In
server/service/orbit.go lines 347-352, capture the returned certificates from
SetOrUpdateHostSCIMUserMapping and DeleteHostSCIMUserMapping and forward them to
svc.NewActivity, matching the existing hosts.go behavior. In
server/service/osquery_utils/queries.go lines 3185-3191, surface those
certificates to the caller or emit the corresponding activity there so the
Windows MDM path records them.
---
Nitpick comments:
In `@server/datastore/mysql/scim_test.go`:
- Around line 2637-2766: Extend testSetOrUpdateHostSCIMUserMapping to exercise
the returned []fleet.ActivityTypeResentCertificate value, using
host_certificate_templates, mdm_configuration_profile_variables, and
fleet_variables fixtures that cause certificate resends. Capture and assert the
returned certificates for an applicable mapping update, then call
SetOrUpdateHostSCIMUserMapping again with the same scimUserID and verify no
duplicate or spurious certificates are returned while preserving the existing
mapping assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 20b660e1-d61a-4994-8873-f4bdf352a089
📒 Files selected for processing (12)
server/datastore/mysql/hosts.goserver/datastore/mysql/hosts_test.goserver/datastore/mysql/scim.goserver/datastore/mysql/scim_test.goserver/fleet/activities.goserver/fleet/datastore.goserver/mock/datastore_mock.goserver/service/hosts.goserver/service/hosts_test.goserver/service/orbit.goserver/service/osquery_utils/queries.goserver/service/osquery_utils/queries_test.go
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #49249 +/- ##
==========================================
- Coverage 68.08% 68.07% -0.01%
==========================================
Files 3784 3784
Lines 239232 239271 +39
Branches 12423 12458 +35
==========================================
+ Hits 162885 162894 +9
- Misses 61571 61586 +15
- Partials 14776 14791 +15
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:
|
This
resent_certificateactivity is generated from the edit user flow in fleet server.Related issue: Resolves #49007
Testing
For unreleased bug fixes in a release candidate, one of:
Summary by CodeRabbit
New Features
Improvements