From dfe5b728c24c2bff24f26bbe2c8baf57f8ed6ea7 Mon Sep 17 00:00:00 2001 From: Jahziel Villasana-Espinoza Date: Mon, 3 Jun 2024 17:23:54 -0400 Subject: [PATCH 1/5] feat: return better errors when private key not configured --- .../components/content/ApplePushCertSetup.tsx | 11 ++++++++-- server/service/apple_mdm.go | 10 +++++++++ server/service/integration_mdm_test.go | 15 +++++++++++++ server/service/mdm.go | 21 +++++++++++++++++-- 4 files changed, 53 insertions(+), 4 deletions(-) diff --git a/frontend/pages/admin/IntegrationsPage/cards/MdmSettings/MacOSMdmPage/components/content/ApplePushCertSetup.tsx b/frontend/pages/admin/IntegrationsPage/cards/MdmSettings/MacOSMdmPage/components/content/ApplePushCertSetup.tsx index 8eb0ea88d5..5cf157b47b 100644 --- a/frontend/pages/admin/IntegrationsPage/cards/MdmSettings/MacOSMdmPage/components/content/ApplePushCertSetup.tsx +++ b/frontend/pages/admin/IntegrationsPage/cards/MdmSettings/MacOSMdmPage/components/content/ApplePushCertSetup.tsx @@ -7,6 +7,7 @@ import mdmAppleApi from "services/entities/mdm_apple"; import CustomLink from "components/CustomLink"; import FileUploader from "components/FileUploader"; import DownloadCSR from "../../../../../../components/DownloadFileButtons/DownloadCSR"; +import { ms } from "date-fns/locale"; interface IApplePushCertSetupProps { baseClass: string; @@ -32,7 +33,10 @@ const ApplePushCertSetup = ({ onSetupSuccess(); } catch (e) { const msg = getErrorReason(e); - if (msg.toLowerCase().includes("invalid certificate")) { + if ( + msg.toLowerCase().includes("invalid certificate") || + msg.toLowerCase().includes("required private key") + ) { renderFlash("error", msg); } else { renderFlash("error", "Couldn’t connect. Please try again."); @@ -46,7 +50,10 @@ const ApplePushCertSetup = ({ const onDownloadError = useCallback( (e: unknown) => { const msg = getErrorReason(e); - if (msg.toLowerCase().includes("email address")) { + if ( + msg.toLowerCase().includes("email address") || + msg.toLowerCase().includes("required private key") + ) { renderFlash("error", msg); } else { renderFlash("error", "Something’s gone wrong. Please try again."); diff --git a/server/service/apple_mdm.go b/server/service/apple_mdm.go index 21a3e9227b..0de70d9f3a 100644 --- a/server/service/apple_mdm.go +++ b/server/service/apple_mdm.go @@ -3578,6 +3578,16 @@ func (svc *Service) GenerateABMKeyPair(ctx context.Context) (*fleet.MDMAppleDEPK if err := svc.authz.Authorize(ctx, &fleet.AppleBM{}, fleet.ActionWrite); err != nil { return nil, err } + + privateKey := svc.config.Server.PrivateKey + if testSetEmptyPrivateKey { + privateKey = "" + } + + if len(privateKey) == 0 { + return nil, ctxerr.New(ctx, "Couldn't download public key. Missing required private key. Learn how to configure the private key here: https://fleetdm.com/learn-more-about/fleet-server-private-key") + } + var publicKeyPEM, privateKeyPEM []byte assets, err := svc.ds.GetAllMDMConfigAssetsByName(ctx, []fleet.MDMAssetName{ fleet.MDMAssetABMCert, diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go index 126b5160fa..294e0f37cf 100644 --- a/server/service/integration_mdm_test.go +++ b/server/service/integration_mdm_test.go @@ -929,6 +929,14 @@ func (s *integrationMDMTestSuite) TestGetMDMCSR() { t := s.T() ctx := context.Background() + // Validate errors if no private key is set + testSetEmptyPrivateKey = true + s.uploadAPNSCert([]byte("-----BEGIN CERTIFICATE-----\nZm9vCg==\n-----END CERTIFICATE-----"), http.StatusInternalServerError, "Couldn't upload APNs certificate. Missing required private key. Learn how to configure the private key here: https://fleetdm.com/learn-more-about/fleet-server-private-key") + + r := s.Do("GET", "/api/latest/fleet/mdm/apple/request_csr", getMDMAppleCSRRequest{}, http.StatusInternalServerError) + require.Contains(t, extractServerErrorText(r.Body), "Couldn't download signed CSR. Missing required private key. Learn how to configure the private key here: https://fleetdm.com/learn-more-about/fleet-server-private-key") + testSetEmptyPrivateKey = false + // ensure we leave everything in a clean state for other tests t.Cleanup(s.appleCoreCertsSetup) @@ -8667,6 +8675,13 @@ func (s *integrationMDMTestSuite) TestABMAssetManagement() { // ensure enable ABM again for other tests t.Cleanup(s.enableABM) + // Validate error when server private key not set + testSetEmptyPrivateKey = true + + r := s.Do("GET", "/api/latest/fleet/mdm/apple/abm_public_key", generateABMKeyPairResponse{}, http.StatusInternalServerError) + require.Contains(t, extractServerErrorText(r.Body), "Couldn't download public key. Missing required private key. Learn how to configure the private key here: https://fleetdm.com/learn-more-about/fleet-server-private-key") + testSetEmptyPrivateKey = false + // grab the current public key var abmResp generateABMKeyPairResponse s.DoJSON("GET", "/api/latest/fleet/mdm/apple/abm_public_key", nil, http.StatusOK, &abmResp) diff --git a/server/service/mdm.go b/server/service/mdm.go index b631f6818d..069a5ecf1d 100644 --- a/server/service/mdm.go +++ b/server/service/mdm.go @@ -2120,6 +2120,9 @@ func (svc *Service) ResendHostMDMProfile(ctx context.Context, hostID uint, profi // GET /mdm/apple/request_csr //////////////////////////////////////////////////////////////////////////////// +// Used for overriding the env var value in testing +var testSetEmptyPrivateKey bool + type getMDMAppleCSRRequest struct{} type getMDMAppleCSRResponse struct { @@ -2143,8 +2146,13 @@ func (svc *Service) GetMDMAppleCSR(ctx context.Context) ([]byte, error) { return nil, err } - if len(svc.config.Server.PrivateKey) == 0 { - return nil, ctxerr.New(ctx, "no private key configured") + privateKey := svc.config.Server.PrivateKey + if testSetEmptyPrivateKey { + privateKey = "" + } + + if len(privateKey) == 0 { + return nil, ctxerr.New(ctx, "Couldn't download signed CSR. Missing required private key. Learn how to configure the private key here: https://fleetdm.com/learn-more-about/fleet-server-private-key") } vc, ok := viewer.FromContext(ctx) @@ -2298,6 +2306,15 @@ func (svc *Service) UploadMDMAppleAPNSCert(ctx context.Context, cert io.ReadSeek return err } + privateKey := svc.config.Server.PrivateKey + if testSetEmptyPrivateKey { + privateKey = "" + } + + if len(privateKey) == 0 { + return ctxerr.New(ctx, "Couldn't upload APNs certificate. Missing required private key. Learn how to configure the private key here: https://fleetdm.com/learn-more-about/fleet-server-private-key") + } + if cert == nil { return ctxerr.Wrap(ctx, fleet.NewInvalidArgumentError("certificate", "Invalid certificate. Please provide a valid certificate from Apple Push Certificate Portal.")) } From a7e16b0915467447adcf9e2218f8d08877826900 Mon Sep 17 00:00:00 2001 From: Jahziel Villasana-Espinoza Date: Mon, 3 Jun 2024 17:25:39 -0400 Subject: [PATCH 2/5] chore: changes file --- changes/19464-private-key-errors | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/19464-private-key-errors diff --git a/changes/19464-private-key-errors b/changes/19464-private-key-errors new file mode 100644 index 0000000000..ddd0fd2f65 --- /dev/null +++ b/changes/19464-private-key-errors @@ -0,0 +1 @@ +- Adds clearer error messages when attempting to set up Apple MDM without a server private key configured. \ No newline at end of file From dc7639c07bd682f4eb56c34e09d6b2b59cea81eb Mon Sep 17 00:00:00 2001 From: Jahziel Villasana-Espinoza Date: Mon, 3 Jun 2024 17:28:23 -0400 Subject: [PATCH 3/5] fix: remove extra import --- .../MacOSMdmPage/components/content/ApplePushCertSetup.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/frontend/pages/admin/IntegrationsPage/cards/MdmSettings/MacOSMdmPage/components/content/ApplePushCertSetup.tsx b/frontend/pages/admin/IntegrationsPage/cards/MdmSettings/MacOSMdmPage/components/content/ApplePushCertSetup.tsx index 5cf157b47b..272c74f75c 100644 --- a/frontend/pages/admin/IntegrationsPage/cards/MdmSettings/MacOSMdmPage/components/content/ApplePushCertSetup.tsx +++ b/frontend/pages/admin/IntegrationsPage/cards/MdmSettings/MacOSMdmPage/components/content/ApplePushCertSetup.tsx @@ -7,7 +7,6 @@ import mdmAppleApi from "services/entities/mdm_apple"; import CustomLink from "components/CustomLink"; import FileUploader from "components/FileUploader"; import DownloadCSR from "../../../../../../components/DownloadFileButtons/DownloadCSR"; -import { ms } from "date-fns/locale"; interface IApplePushCertSetupProps { baseClass: string; From 3dc65d74326683b12d686ab232e80306f467dd61 Mon Sep 17 00:00:00 2001 From: Jahziel Villasana-Espinoza Date: Mon, 3 Jun 2024 17:39:56 -0400 Subject: [PATCH 4/5] feat: add UI support for ABM error message --- .../DownloadFileButtons/DownloadABMKey.tsx | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/frontend/pages/admin/components/DownloadFileButtons/DownloadABMKey.tsx b/frontend/pages/admin/components/DownloadFileButtons/DownloadABMKey.tsx index 70df97ea3e..891ea45a58 100644 --- a/frontend/pages/admin/components/DownloadFileButtons/DownloadABMKey.tsx +++ b/frontend/pages/admin/components/DownloadFileButtons/DownloadABMKey.tsx @@ -1,6 +1,14 @@ -import React, { FormEvent, useCallback, useMemo, useState } from "react"; +import React, { + FormEvent, + useCallback, + useMemo, + useState, + useContext, +} from "react"; import mdmAppleBusinessManagerApi from "services/entities/mdm_apple_bm"; +import { NotificationContext } from "context/notification"; +import { getErrorReason } from "interfaces/errors"; import Icon from "components/Icon"; import Button from "components/buttons/Button"; @@ -23,6 +31,7 @@ const useDownloadABMKey = ({ onError, }: Omit) => { const [downloadState, setDownloadState] = useState(undefined); + const { renderFlash } = useContext(NotificationContext); const handleDownload = useCallback( async (evt: FormEvent) => { @@ -34,6 +43,8 @@ const useDownloadABMKey = ({ setDownloadState("success"); onSuccess && onSuccess(); } catch (e) { + const msg = getErrorReason(e); + renderFlash("error", msg); setDownloadState("error"); onError && onError(e); } From 945eb778a66b85d68066b7c912978be4aef60e2d Mon Sep 17 00:00:00 2001 From: Jahziel Villasana-Espinoza Date: Tue, 4 Jun 2024 09:45:53 -0400 Subject: [PATCH 5/5] fix: ensure test cleanup --- server/service/integration_mdm_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go index 294e0f37cf..aeca01bc33 100644 --- a/server/service/integration_mdm_test.go +++ b/server/service/integration_mdm_test.go @@ -931,6 +931,7 @@ func (s *integrationMDMTestSuite) TestGetMDMCSR() { // Validate errors if no private key is set testSetEmptyPrivateKey = true + t.Cleanup(func() { testSetEmptyPrivateKey = false }) s.uploadAPNSCert([]byte("-----BEGIN CERTIFICATE-----\nZm9vCg==\n-----END CERTIFICATE-----"), http.StatusInternalServerError, "Couldn't upload APNs certificate. Missing required private key. Learn how to configure the private key here: https://fleetdm.com/learn-more-about/fleet-server-private-key") r := s.Do("GET", "/api/latest/fleet/mdm/apple/request_csr", getMDMAppleCSRRequest{}, http.StatusInternalServerError) @@ -8677,6 +8678,7 @@ func (s *integrationMDMTestSuite) TestABMAssetManagement() { // Validate error when server private key not set testSetEmptyPrivateKey = true + t.Cleanup(func() { testSetEmptyPrivateKey = false }) r := s.Do("GET", "/api/latest/fleet/mdm/apple/abm_public_key", generateABMKeyPairResponse{}, http.StatusInternalServerError) require.Contains(t, extractServerErrorText(r.Body), "Couldn't download public key. Missing required private key. Learn how to configure the private key here: https://fleetdm.com/learn-more-about/fleet-server-private-key")