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

Team ID filter should not be required in "List Fleet-maintained apps" endpoint #24509

Closed
9 tasks done
rachaelshaw opened this issue Dec 6, 2024 · 5 comments
Closed
9 tasks done
Assignees
Labels
~backend Backend-related issue. bug Something isn't working as documented #g-endpoint-ops Endpoint ops product group :release Ready to write code. Scheduled in a release. See "Making changes" in handbook. ~released bug This bug was found in a stable release.
Milestone

Comments

@rachaelshaw
Copy link
Member

rachaelshaw commented Dec 6, 2024

Fleet version: 4.60.1


💥  Actual behavior

API users are only able to get a list of Fleet-maintained apps if they have a team ID. This makes it so the endpoint can't be used in situations where users may just want to get a complete list of what's available.

🧑‍💻  Steps to reproduce

Attempt to access the GET /fleet_maintained_apps endpoint in the API (the frontend doesn't do this) without team_id set.

🛠️ To fix

Display all available FMAs (regardless of whether they've been added as installers anywhere) when calling the FMA list endpoint with no team param.

Test Plan

No special instance setup required. The associated PR's auth tests may be helpful here.

New functionality:

  • GET /api/v1/fleet/software/fleet_maintained_apps works with no query params
  • Result of the above doesn't change after adding a FMA to a team
  • Pagination works
  • Auth succeeds for both global and team-limited maintainers and admins
  • Auth fails for onservers/observer+

Regression testing:

  • Fleet Maintained Apps list view works for global admins and maintainers
  • Fleet Maintained Apps list view works for admins and maintainers on the same team
  • GET /api/v1/fleet/software/fleet_maintained_apps with team specified fails auth for a team-specific admin/maintainer on the wrong team
  • The above fails auth for an observer/observer+, regardless of global/same team/different team
@rachaelshaw rachaelshaw added bug Something isn't working as documented :product Product Design department (shows up on 🦢 Drafting board) labels Dec 6, 2024
@iansltx
Copy link
Member

iansltx commented Dec 9, 2024

Looked at this endpoint while fixing #23305. We need the team here because the list query filters out apps for which an installer already exists for that team, which is what the UI for adding FMAs needs.

The endpoint could be tweaked to skip the "where installer doesn't exist" part of the DB query if team ID isn't specified, though this would require tweaks to auth so team-specific maintainers/owners could hit the endpoint with either no team query or a team query matching a team they're part of.

Either way, this seems like a FR rather than a bug?

@iansltx
Copy link
Member

iansltx commented Dec 9, 2024

Per design review, this will stay as a bug. Fix is to, when the team ID query parameter is not supplied, drop the "available" filter while maintaining existing functionality when the team ID is supplied. Doing the fix that way ensures the fix respects BC and as a result is backend-only.

@noahtalerman
Copy link
Member

noahtalerman commented Dec 9, 2024

Hey @rachaelshaw, I think @eashaw and @mikermcneil are trying to ship Fleet-maintained apps on the website ASAP. Can you please check if this bug is blocking them?

If this is blocking, can you please add a P2, tag Luke (per the priority label process), and include a deploying a patch to dogfood as part of this issue? That way Eric can hit the updated endpoint in dogfood ASAP.

@iansltx iansltx added #g-endpoint-ops Endpoint ops product group :release Ready to write code. Scheduled in a release. See "Making changes" in handbook. labels Dec 9, 2024
@rachaelshaw rachaelshaw added :incoming New issue in triage process. and removed :product Product Design department (shows up on 🦢 Drafting board) :incoming New issue in triage process. labels Dec 9, 2024
@iansltx iansltx self-assigned this Dec 9, 2024
@iansltx iansltx added the ~backend Backend-related issue. label Dec 9, 2024
@iansltx iansltx added this to the 4.62.0-tentative milestone Dec 9, 2024
@iansltx iansltx modified the milestones: 4.62.0-tentative, 4.61.0 Dec 10, 2024
iansltx added a commit that referenced this issue Dec 12, 2024
…g a team ID (#24595)

For #24509

# Checklist for submitter

If some of the following don't apply, delete the relevant line.

<!-- Note that API documentation changes are now addressed by the
product design team. -->

- [x] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.
See [Changes
files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/Committing-Changes.md#changes-files)
for more information.
- [x] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented (using placeholders for values in statements)
- [x] Added/updated tests
- [x] Manual QA for all new/changed functionality
iansltx added a commit that referenced this issue Dec 12, 2024
…g a team ID (#24595)

For #24509

# Checklist for submitter

If some of the following don't apply, delete the relevant line.

<!-- Note that API documentation changes are now addressed by the
product design team. -->

- [x] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.
See [Changes
files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/Committing-Changes.md#changes-files)
for more information.
- [x] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented (using placeholders for values in statements)
- [x] Added/updated tests
- [x] Manual QA for all new/changed functionality
iansltx added a commit that referenced this issue Dec 12, 2024
…hout requiring a team ID (#24693)

For #24509, merged into `main` in #24595.
@jmwatts
Copy link
Member

jmwatts commented Dec 12, 2024

QA Notes:

Verified team_id is not longer required. Tested all steps outlined in "New functionality" and "Regression testing" above.

@lukeheath lukeheath added the ~released bug This bug was found in a stable release. label Dec 13, 2024
@fleet-release
Copy link
Contributor

Apps list now unfurls,
No team ID, still reveals,
Ease like clouds that swirl.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
~backend Backend-related issue. bug Something isn't working as documented #g-endpoint-ops Endpoint ops product group :release Ready to write code. Scheduled in a release. See "Making changes" in handbook. ~released bug This bug was found in a stable release.
Development

No branches or pull requests

6 participants