Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't expire iOS devices prematurely #25436

Merged
merged 5 commits into from
Jan 16, 2025

Conversation

dantecatalfamo
Copy link
Member

@dantecatalfamo dantecatalfamo commented Jan 14, 2025

#25406

The last_seen_times table is only updates when osquery hits one of its authenticated endpoints, meaning it isn't updated when devices without osquery, like iphones, are enrolled. I've left a comment on the original issue explaining how this happens. Originally, if there was no last_seen_time, the fallback value would be the created_at value on the hosts table, so ios devices would always get deleted once they were added X number of days ago.

In its place, I've added the detail_updated_at column on the hosts table as the fallback value, and only use created_at if that is also empty. detail_updated_at is updated every time a full detail refetch completes. In the case of ios/ipados, this is done using MDM. detail_updated_at is updated less frequently than last_seen_times, only once every hour or so instead of every 30 seconds, but since expiration policies are set on the scale of days instead of hours, this should be fine.

The way I've QA'd this is by adding an iOS device to my fleet instance, waited 24 hours, and set the expiration policy to 24 hours.

Big thanks to @gillespi314 for helping me navigate the MDM internals!

  • Changes file added for user-visible changes in changes/, orbit/changes/ or ee/fleetd-chrome/changes.
    See Changes files for more information.
  • Input data is properly validated, SELECT * is avoided, SQL injection is prevented (using placeholders for values in statements)
  • Added/updated automated tests
  • A detailed QA plan exists on the associated ticket (if it isn't there, work with the product group's QA engineer to add it)
  • Manual QA for all new/changed functionality

Copy link

codecov bot commented Jan 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.59%. Comparing base (d6eeaaa) to head (039b7a5).
Report is 35 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #25436      +/-   ##
==========================================
+ Coverage   63.57%   63.59%   +0.01%     
==========================================
  Files        1619     1619              
  Lines      154836   154992     +156     
  Branches     3994     3994              
==========================================
+ Hits        98436    98562     +126     
- Misses      48638    48660      +22     
- Partials     7762     7770       +8     
Flag Coverage Δ
backend 64.44% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dantecatalfamo dantecatalfamo changed the title Use detail_updated_at before created_at when no last seen Don't expire iOS devices prematurely Jan 15, 2025
@dantecatalfamo dantecatalfamo marked this pull request as ready for review January 15, 2025 16:49
@dantecatalfamo dantecatalfamo requested a review from a team as a code owner January 15, 2025 16:49
@dantecatalfamo dantecatalfamo marked this pull request as draft January 15, 2025 16:49
@dantecatalfamo dantecatalfamo marked this pull request as ready for review January 15, 2025 19:02
Copy link
Member

@lucasmrod lucasmrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Left two nit comments.

server/datastore/mysql/hosts.go Show resolved Hide resolved
changes/25406-premature-ios-deletion Outdated Show resolved Hide resolved
Co-authored-by: Lucas Manuel Rodriguez <lucas@fleetdm.com>
@dantecatalfamo dantecatalfamo merged commit 3a2a689 into main Jan 16, 2025
26 checks passed
@dantecatalfamo dantecatalfamo deleted the 25406-ios-deleted-non-expired-host branch January 16, 2025 15:13
dantecatalfamo added a commit that referenced this pull request Jan 17, 2025
#25406 

The `last_seen_times` table is only updates when osquery hits one of its
authenticated endpoints, meaning it isn't updated when devices without
osquery, like iphones, are enrolled. I've left a
[comment](#25406 (comment))
on the original issue explaining how this happens. Originally, if there
was no `last_seen_time`, the fallback value would be the `created_at`
value on the `hosts` table, so ios devices would always get deleted once
they were added X number of days ago.

In its place, I've added the `detail_updated_at` column on the `hosts`
table as the fallback value, and only use `created_at` if that is also
empty. `detail_updated_at` is updated every time a full detail refetch
completes. In the case of ios/ipados, [this is done using
MDM](https://github.com/fleetdm/fleet/blob/cd5c0e8aed10664458f597b5d9600dd20bf3fdac/server/service/apple_mdm.go#L3101).
`detail_updated_at` is updated less frequently than `last_seen_times`,
only once every hour or so instead of every 30 seconds, but since
expiration policies are set on the scale of days instead of hours, this
should be fine.

The way I've QA'd this is by adding an iOS device to my fleet instance,
waited 24 hours, and set the expiration policy to 24 hours.
lukeheath pushed a commit that referenced this pull request Jan 17, 2025
#25406 

The `last_seen_times` table is only updates when osquery hits one of its
authenticated endpoints, meaning it isn't updated when devices without
osquery, like iphones, are enrolled. I've left a
[comment](#25406 (comment))
on the original issue explaining how this happens. Originally, if there
was no `last_seen_time`, the fallback value would be the `created_at`
value on the `hosts` table, so ios devices would always get deleted once
they were added X number of days ago.

In its place, I've added the `detail_updated_at` column on the `hosts`
table as the fallback value, and only use `created_at` if that is also
empty. `detail_updated_at` is updated every time a full detail refetch
completes. In the case of ios/ipados, [this is done using
MDM](https://github.com/fleetdm/fleet/blob/cd5c0e8aed10664458f597b5d9600dd20bf3fdac/server/service/apple_mdm.go#L3101).
`detail_updated_at` is updated less frequently than `last_seen_times`,
only once every hour or so instead of every 30 seconds, but since
expiration policies are set on the scale of days instead of hours, this
should be fine.

The way I've QA'd this is by adding an iOS device to my fleet instance,
waited 24 hours, and set the expiration policy to 24 hours.
lukeheath pushed a commit that referenced this pull request Jan 17, 2025
#25406 

The `last_seen_times` table is only updates when osquery hits one of its
authenticated endpoints, meaning it isn't updated when devices without
osquery, like iphones, are enrolled. I've left a
[comment](#25406 (comment))
on the original issue explaining how this happens. Originally, if there
was no `last_seen_time`, the fallback value would be the `created_at`
value on the `hosts` table, so ios devices would always get deleted once
they were added X number of days ago.

In its place, I've added the `detail_updated_at` column on the `hosts`
table as the fallback value, and only use `created_at` if that is also
empty. `detail_updated_at` is updated every time a full detail refetch
completes. In the case of ios/ipados, [this is done using
MDM](https://github.com/fleetdm/fleet/blob/cd5c0e8aed10664458f597b5d9600dd20bf3fdac/server/service/apple_mdm.go#L3101).
`detail_updated_at` is updated less frequently than `last_seen_times`,
only once every hour or so instead of every 30 seconds, but since
expiration policies are set on the scale of days instead of hours, this
should be fine.

The way I've QA'd this is by adding an iOS device to my fleet instance,
waited 24 hours, and set the expiration policy to 24 hours.
lukeheath pushed a commit that referenced this pull request Jan 17, 2025
#25406 

The `last_seen_times` table is only updates when osquery hits one of its
authenticated endpoints, meaning it isn't updated when devices without
osquery, like iphones, are enrolled. I've left a
[comment](#25406 (comment))
on the original issue explaining how this happens. Originally, if there
was no `last_seen_time`, the fallback value would be the `created_at`
value on the `hosts` table, so ios devices would always get deleted once
they were added X number of days ago.

In its place, I've added the `detail_updated_at` column on the `hosts`
table as the fallback value, and only use `created_at` if that is also
empty. `detail_updated_at` is updated every time a full detail refetch
completes. In the case of ios/ipados, [this is done using
MDM](https://github.com/fleetdm/fleet/blob/cd5c0e8aed10664458f597b5d9600dd20bf3fdac/server/service/apple_mdm.go#L3101).
`detail_updated_at` is updated less frequently than `last_seen_times`,
only once every hour or so instead of every 30 seconds, but since
expiration policies are set on the scale of days instead of hours, this
should be fine.

The way I've QA'd this is by adding an iOS device to my fleet instance,
waited 24 hours, and set the expiration policy to 24 hours.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants