remove protobuf fields for storing Jamf username, password, `clie…

…nt_id` and `client_secret` This PR builds on top of #45255 and removes the protobuf fields that could potencially be used to insecurely store Jamf secrets Signed-off-by: Tiago Silva <[email protected]>
gravitational · Aug 28, 2024 · 922d75d · 922d75d
1 parent 164ec45
commit 922d75d
Show file tree

Hide file tree

Showing 9 changed files with 254 additions and 565 deletions.
diff --git a/api/proto/teleport/legacy/types/types.proto b/api/proto/teleport/legacy/types/types.proto
@@ -7072,6 +7072,8 @@ message ServerInfoSpecV1 {
 
 // JamfSpecV1 is the base configuration for the Jamf MDM service.
 message JamfSpecV1 {
+  reserved 5, 6, 8, 9;
+  reserved "username", "password", "client_id", "client_secret";
   option (gogoproto.equal) = true;
   // Enabled toggles the service on or off.
   bool enabled = 1 [(gogoproto.jsontag) = "enabled,omitempty"];
@@ -7090,29 +7092,9 @@ message JamfSpecV1 {
   // Example: "https://yourtenant.jamfcloud.com/api".
   // Required.
   string api_endpoint = 4 [(gogoproto.jsontag) = "api_endpoint,omitempty"];
-  // Jamf API username.
-  // Username and password are used to acquire short-lived Jamf Pro API tokens.
-  // See https://developer.jamf.com/jamf-pro/docs/jamf-pro-api-overview.
-  // Prefer using client_id and client_secret.
-  // Either username+password or client_id+client_secret are required.
-  string username = 5 [(gogoproto.jsontag) = "username,omitempty"];
-  // Jamf API password.
-  // Username and password are used to acquire short-lived Jamf Pro API tokens.
-  // See https://developer.jamf.com/jamf-pro/docs/jamf-pro-api-overview.
-  // Prefer using client_id and client_secret.
-  // Either username+password or client_id+client_secret are required.
-  string password = 6 [(gogoproto.jsontag) = "password,omitempty"];
   // Inventory sync entries.
   // If empty a default sync configuration is used.
   repeated JamfInventoryEntry inventory = 7 [(gogoproto.jsontag) = "inventory,omitempty"];
-  // Jamf API client ID.
-  // See https://developer.jamf.com/jamf-pro/docs/client-credentials.
-  // Either username+password or client_id+client_secret are required.
-  string client_id = 8 [(gogoproto.jsontag) = "client_id,omitempty"];
-  // Jamf API client secret.
-  // See https://developer.jamf.com/jamf-pro/docs/client-credentials.
-  // Either username+password or client_id+client_secret are required.
-  string client_secret = 9 [(gogoproto.jsontag) = "client_secret,omitempty"];
 }
 
 // JamfInventoryEntry is an inventory sync entry for [JamfSpecV1].

diff --git a/api/types/jamf.go b/api/types/jamf.go
@@ -44,13 +44,6 @@ func ValidateJamfSpecV1(s *JamfSpecV1) error {
 		return trace.BadParameter("spec required")
 	}
 
-	// Jamf can handle both credential sets being present, so we let it pass.
-	hasUserPass := s.Username != "" && s.Password != ""
-	hasAPICreds := s.ClientId != "" && s.ClientSecret != ""
-	if !hasUserPass && !hasAPICreds {
-		return trace.BadParameter("either username+password or clientID+clientSecret must be provided")
-	}
-
 	switch u, err := url.Parse(s.ApiEndpoint); {
 	case err != nil:
 		return trace.BadParameter("invalid API endpoint: %v", err)

diff --git a/api/types/jamf_test.go b/api/types/jamf_test.go
@@ -29,8 +29,6 @@ func TestValidateJamfSpecV1(t *testing.T) {
 	validSpec := &types.JamfSpecV1{
 		Enabled:     true,
 		ApiEndpoint: "https://yourtenant.jamfcloud.com",
-		Username:    "llama",
-		Password:    "supersecret!!1!",
 	}
 	validEntry := &types.JamfInventoryEntry{
 		FilterRsql:        "", // no filters
@@ -59,8 +57,6 @@ func TestValidateJamfSpecV1(t *testing.T) {
 			spec: &types.JamfSpecV1{
 				Enabled:     true,
 				ApiEndpoint: "https://yourtenant.jamfcloud.com",
-				Username:    "llama",
-				Password:    "supersecret!!1!",
 				Inventory: []*types.JamfInventoryEntry{
 					{
 						FilterRsql:        `general.remoteManagement.managed==true and general.platform=="Mac"`,
@@ -76,36 +72,6 @@ func TestValidateJamfSpecV1(t *testing.T) {
 				},
 			},
 		},
-		{
-			name: "all fields",
-			spec: &types.JamfSpecV1{
-				Enabled:     true,
-				Name:        "jamf2",
-				SyncDelay:   types.Duration(2 * time.Minute),
-				ApiEndpoint: "https://yourtenant.jamfcloud.com",
-				Username:    "llama",
-				Password:    "supersecret!!1!",
-				Inventory: []*types.JamfInventoryEntry{
-					{
-						FilterRsql:        `general.remoteManagement.managed==true and general.platform=="Mac"`,
-						SyncPeriodPartial: types.Duration(4 * time.Hour),
-						SyncPeriodFull:    types.Duration(48 * time.Hour),
-						OnMissing:         "DELETE",
-					},
-				},
-				ClientId:     "llama-UUID", // "duplicate" creds are fine
-				ClientSecret: "supersecretsecret!!1!",
-			},
-		},
-		{
-			name: "using API credentials",
-			spec: modify(func(spec *types.JamfSpecV1) {
-				spec.Username = ""
-				spec.Password = ""
-				spec.ClientId = "llama-UUID"
-				spec.ClientSecret = "supersecretsecret!11!"
-			}),
-		},
 		{
 			name:    "nil spec",
 			spec:    nil,
@@ -125,38 +91,6 @@ func TestValidateJamfSpecV1(t *testing.T) {
 			}),
 			wantErr: "missing hostname",
 		},
-		{
-			name: "username empty",
-			spec: modify(func(spec *types.JamfSpecV1) {
-				spec.Username = ""
-			}),
-			wantErr: "username",
-		},
-		{
-			name: "password empty",
-			spec: modify(func(spec *types.JamfSpecV1) {
-				spec.Password = ""
-			}),
-			wantErr: "password",
-		},
-		{
-			name: "client_id empty",
-			spec: modify(func(spec *types.JamfSpecV1) {
-				spec.Username = ""
-				spec.Password = ""
-				spec.ClientSecret = "supersecretsecret!!1!"
-			}),
-			wantErr: "clientID",
-		},
-		{
-			name: "client_secret empty",
-			spec: modify(func(spec *types.JamfSpecV1) {
-				spec.Username = ""
-				spec.Password = ""
-				spec.ClientId = "llama-UUID"
-			}),
-			wantErr: "clientSecret",
-		},
 		{
 			name: "inventory nil entry",
 			spec: modify(func(spec *types.JamfSpecV1) {