Fix addFleetMaintainedAppEndpoint to accept fleet_id param (#41805)

<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**Related issue:** Resolves #41771

# Details

Solves two issues in 4.82:

1. The `fleet_id` param in `POST /software/fleet_maintained_apps` wasn't
being read, causing all FMAs using that param to be added to fleet ID 0
(unassigned aka No Team)
2. We were logging deprecation warnings for body params even if the
topic was turned off, meaning Fleet would generate deprecation warnings
in certain cases that users wouldn't be able to fix.

# Checklist for submitter

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

- [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/guides/committing-changes.md#changes-files)
for more information.

## Testing

- [X] Added/updated automated tests
Added unit tests for the decoder since it's got one-off logic in it
- [X] QA'd all new/changed functionality manually
  - [X] Added an FMA to a fleet successfully using `fleet_id`
- [X] Added an FMA to a fleet successfully using `team_id` and saw
deprecation warning
  - [X] Added an FMA to "Unassigned" successfully using `fleet_id=0`
  - [X] Added an FMA to "Unassigned" successfully using `team_id=0`
- [X] Added an FMA to "Unassigned" successfully with no `fleet_id` or
`team_id` param (this seems like a bug but it's existing behavior)

---------

Co-authored-by: Ian Littman <iansltx@gmail.com>

changes/41771-fix-fma-endpoint

@@ -0,0 +1,2 @@
+- Fixed an issue where the "add Fleet-maintained app" endpoint incorrectly added software to the Unassigned fleet.
+- Muted deprecation warnings for body params when the "deprecated-field-names" topic is not enabled.

server/platform/endpointer/endpoint_utils.go

@@ -620,25 +620,6 @@ func MakeDecoder(
 				v = reflect.ValueOf(req)
 			}
 
-			// Log deprecation warnings when deprecated field names are used.
-			if rewriter != nil {
-				if deprecated := rewriter.UsedDeprecatedKeys(); len(deprecated) > 0 {
-					newNames := make([]string, len(deprecated))
-					for i, old := range deprecated {
-						for _, rule := range aliasRules {
-							if rule.OldKey == old {
-								newNames[i] = rule.NewKey
-								break
-							}
-						}
-					}
-					logging.WithLevel(ctx, slog.LevelWarn)
-					logging.WithExtras(ctx,
-						"deprecated_fields", fmt.Sprintf("%v", deprecated),
-						"deprecation_warning", fmt.Sprintf("use the updated field names (%s) instead", newNames),
-					)
-				}
-			}
 		}
 
 		fields := allFields(v)
@@ -710,16 +691,25 @@ func MakeDecoder(
 				return nil, err
 			}
 
-			// Log deprecation warnings when deprecated field names are used
-			// (bodyDecoder path).
-			if rewriter != nil {
-				if deprecated := rewriter.UsedDeprecatedKeys(); len(deprecated) > 0 {
-					logging.WithLevel(ctx, slog.LevelWarn)
-					logging.WithExtras(ctx,
-						"deprecated_fields", fmt.Sprintf("%v", deprecated),
-						"deprecation_warning", "use the updated field names instead",
-					)
+		}
+
+		// Log deprecation warnings when deprecated field names are used.
+		if rewriter != nil && platform_logging.TopicEnabled(platform_logging.DeprecatedFieldTopic) {
+			if deprecated := rewriter.UsedDeprecatedKeys(); len(deprecated) > 0 {
+				newNames := make([]string, len(deprecated))
+				for i, old := range deprecated {
+					for _, rule := range aliasRules {
+						if rule.OldKey == old {
+							newNames[i] = rule.NewKey
+							break
+						}
+					}
 				}
+				logging.WithLevel(ctx, slog.LevelWarn)
+				logging.WithExtras(ctx,
+					"deprecated_fields", fmt.Sprintf("%v", deprecated),
+					"deprecation_warning", fmt.Sprintf("use the updated field names (%s) instead", newNames),
+				)
 			}
 		}
 

server/service/maintained_apps.go

@@ -6,12 +6,18 @@ import (
 	"errors"
 	"net/http"
 
+	"github.com/fleetdm/fleet/v4/server/contexts/logging"
 	"github.com/fleetdm/fleet/v4/server/fleet"
-	"github.com/fleetdm/fleet/v4/server/mdm/maintainedapps"
+	maintained_apps "github.com/fleetdm/fleet/v4/server/mdm/maintainedapps"
+	platform_logging "github.com/fleetdm/fleet/v4/server/platform/logging"
 )
 
 type addFleetMaintainedAppRequest struct {
-	TeamID            *uint    `json:"team_id" renameto:"fleet_id"`
+	TeamID *uint `json:"team_id"`
+	// Note that we're adding an explicit FleetID field rather than using `renameto`.
+	// The POST /software/fleet_maintained_apps endpoint has a custom decoder
+	// and in this special case it's easier to handle the aliasing manually.
+	FleetID           *uint    `json:"fleet_id"`
 	AppID             uint     `json:"fleet_maintained_app_id"`
 	InstallScript     string   `json:"install_script"`
 	PreInstallQuery   string   `json:"pre_install_query"`
@@ -39,6 +45,23 @@ func (addFleetMaintainedAppRequest) DecodeRequest(ctx context.Context, r *http.R
 		}
 	}
 
+	// Resolve fleet_id → team_id aliasing. The struct has both fields so
+	// json.Decode populates whichever the caller sent; we normalize here.
+	if req.FleetID != nil {
+		if req.TeamID != nil {
+			return nil, &fleet.BadRequestError{
+				Message: `Specify only one of "team_id" or "fleet_id"`,
+			}
+		}
+		req.TeamID = req.FleetID
+		req.FleetID = nil
+	} else if req.TeamID != nil && platform_logging.TopicEnabled(platform_logging.DeprecatedFieldTopic) {
+		// Add a deprecation warning.
+		logging.WithExtras(ctx,
+			"deprecated_fields", "[team_id]",
+			"deprecation_warning", "use the updated field names (fleet_id) instead",
+		)
+	}
 	// Check if scripts are base64 encoded
 	if isScriptsEncoded(r) {
 		var err error

server/service/maintained_apps_test.go

@@ -0,0 +1,73 @@
+package service
+
+import (
+	"bytes"
+	"context"
+	"io"
+	"net/http"
+	"testing"
+
+	"github.com/fleetdm/fleet/v4/server/ptr"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestAddFleetMaintainedAppDecodeRequest(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name       string
+		body       string
+		wantTeamID *uint
+		wantErr    string
+	}{
+		{
+			name:       "fleet_id accepted",
+			body:       `{"fleet_id": 42, "fleet_maintained_app_id": 1}`,
+			wantTeamID: ptr.Uint(42),
+		},
+		{
+			name:       "team_id still accepted",
+			body:       `{"team_id": 7, "fleet_maintained_app_id": 1}`,
+			wantTeamID: ptr.Uint(7),
+		},
+		{
+			name:       "neither provided",
+			body:       `{"fleet_maintained_app_id": 1}`,
+			wantTeamID: nil,
+		},
+		{
+			name:    "both provided is an error",
+			body:    `{"team_id": 1, "fleet_id": 2, "fleet_maintained_app_id": 1}`,
+			wantErr: `Specify only one of "team_id" or "fleet_id"`,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			r, err := http.NewRequestWithContext(context.Background(), http.MethodPost, "/", io.NopCloser(bytes.NewBufferString(tt.body)))
+			require.NoError(t, err)
+
+			result, err := addFleetMaintainedAppRequest{}.DecodeRequest(context.Background(), r)
+
+			if tt.wantErr != "" {
+				require.Error(t, err)
+				assert.Contains(t, err.Error(), tt.wantErr)
+				return
+			}
+
+			require.NoError(t, err)
+			req := result.(*addFleetMaintainedAppRequest)
+			if tt.wantTeamID == nil {
+				assert.Nil(t, req.TeamID)
+			} else {
+				require.NotNil(t, req.TeamID)
+				assert.Equal(t, *tt.wantTeamID, *req.TeamID)
+			}
+			// FleetID should always be nil after normalization
+			assert.Nil(t, req.FleetID)
+		})
+	}
+}