Labels API: `count` field takes into the count hosts that user don't have permissions to read

[mna]

Fleet version: v4.48.3

Web browser and operating system: N/A

---

💥  Actual behavior

Labels API:

The count field is inconsistent. In the "Get", "Create" and "Modify" label it returns the count of all hosts in the label, while in "List labels" it returns the count of hosts that the user has the right to see.

🛠️ To fix

count for each user should reflect a number of hosts that the user has permission to see. If user has permission to see only a subset of label members, the returned count key should reflect that.

[noahtalerman]

the field is always shown as null in the examples and there is no further documentation about what it should contain.

Up to now, this field has never been set in the code, so it was always returned as null.

@marko-lisica and @mna if the API is behaving as documented is this a bug?

[mna]

@noahtalerman @marko-lisica if we documented that field as present, we surely did not intend it to always be null, but it looks like we never documented what it should return.

[marko-lisica]

Hey @mna and @noahtalerman,

I see 2 issues here:

1. count in most cases doesn't reflect correct numbers based on the role that's calling API. Not sure if this was designed but skipped so we consider it a bug, or we didn't design this. I think we should solve this in any way.
2. Empty host_ids key. Seems that we had that reported before and we didn't prioritize that, but we didn't consider it a bug since it was documented in the docs (at least it was in the example response). I think it's confusing and we should probably remove it as we have ways to get members of the label via GET /api/v1/fleet/labels/:id/hosts.

I think we probably need to file 2 feature requests for this. @noahtalerman Regarding the count field, I guess we don't return count of all members for each role intentionally?

[marko-lisica]

During the design review today, we decided to solve issue with count not taking user rights into the permission. I'll change description of this bug to cover that.

Regarding host_ids, we decided to remove that from all endpoints, since it's always null and not used by UI. I'll file a feature request for this.

[marko-lisica]

@mna Saving old issue description here:

💥  Actual behavior

The "Create", "Modify", "Get" and "List" labels endpoints all show a host_ids field as part of the response payload in the documentation (e.g. https://fleetdm.com/docs/rest-api/rest-api#default-response60), but the field is always shown as null in the examples and there is no further documentation about what it should contain.

Up to now, this field has never been set in the code, so it was always returned as null.

We should clarify what this field should contain, and document it accordingly. Some things to keep in mind:

1. There is also a count field which counts the hosts that are members of the label. This returns the count for both manual and dynamic label memberships. Unlike host_ids, this count field is currently filled.
2. The count is also inconsistent, as in "Get", "Create" and "Modify" label it returns the count of all hosts in the label, while in "List labels" it returns the count of hosts that the user has the right to see (keep in mind that while only global admin/maintainers can write labels, anyone can read them, so their "visible hosts" can be a subset of those that are members of the label)
3. For dynamic labels, if we decided that host_ids should be returned for that type of labels, the number of host IDs could be very large (would be ALL host IDs for the All Hosts label). Encoded in JSON as part of the response, in a 100K hosts deployment this would be an additional ~800KB just for that label, resulting in quite big response payloads for the "List labels" endpoint.
4. Unlike for "List hosts in labels" which is a paginated API endpoint, the host_ids field cannot be paginated - it is a subfield of a top-level object in the response.
5. If we decide to return it only for manual labels, should it take into account what hosts the user can see? If so, it may return a subset of the hosts that make up the manual label. If not, at worse we "leak" a host ID, which is probably not really a "leak" (our IDs are not unpredictable).
6. We could decide to remove the field altogether, and rely on "List hosts in a label" endpoint to view host membership with all the correct options, pagination, user-based restrictions. But there's still the count field inconsistency to resolve (point 2.).

All that to say, this requires product input. Based on all this and the fact it was not filled up until now without any issues that I know of, I'd suggest 6.

🕯️ More info (optional)

Note that while implementing the "Add manual labels to API and UI" story, we noticed that and thought we would need that information for the frontend, so we added the "host_ids" only for manual labels and only for the "Create", "Modify" and "Get" labels endpoints (not the "List" one which was more involved and we were pressed by time). It turned out we didn't need this data after all (the UI used the "List hosts in label" endpoint instead).

I think it would still be fine to remove it if we opted to go with 6, it was never clearly documented regarding its content and was never filled before.

[marko-lisica]

Feature request to remove host_ids: https://github.com/fleetdm/fleet/issues/18515

[PezHub]

Confirmed the counts match the hosts the user has permissions to see based on team membership.

Example: User with permission to 2 Teams with 3 hosts enrolled shows a count of 3

{
      "created_at": "2024-05-02T01:06:18Z",
      "updated_at": "2024-05-02T01:06:18Z",
      "id": 7,
      "name": "All Hosts",
      "description": "All hosts which have enrolled in Fleet",
      "query": "select 1;",
      "platform": "",
      "label_type": "builtin",
      "label_membership_type": "dynamic",
      "host_count": 3,
      "display_text": "All Hosts",
      "count": 3,
      "host_ids": null
    },

Global Admin results show all 11 hosts that are enrolled on my local instance

{
      "created_at": "2024-05-02T01:06:18Z",
      "updated_at": "2024-05-02T01:06:18Z",
      "id": 7,
      "name": "All Hosts",
      "description": "All hosts which have enrolled in Fleet",
      "query": "select 1;",
      "platform": "",
      "label_type": "builtin",
      "label_membership_type": "dynamic",
      "host_count": 11,
      "display_text": "All Hosts",
      "count": 11,
      "host_ids": null
    },

NOTE: the "hosts_id" is still null but based on the comments above that was pulled out of this issue and will be addressed at a later time.

QA Approved!

[fleet-release]

Labels API vision clear, Count reflects users' sphere, Cloud city draws near.