From ffed7f8ebe3f48b853db90b24c1becb88e184130 Mon Sep 17 00:00:00 2001 From: Roberto Dip Date: Wed, 1 Feb 2023 12:50:22 -0300 Subject: [PATCH] return 422 status code if fleetdm.com returns any 4xx status for CSR (#9610) Related to https://github.com/fleetdm/fleet/issues/9588, we now handle 4xx responses from the fleetdm.com server and forward those to the client. At the time of this commit, the only 4xx response that wasn't already handled by the server is because of an invalid email domain, so we assume that, but we should look into establishing a pattern of error messages with the website instead. --- cmd/fleetctl/apple_mdm_test.go | 2 +- server/mdm/apple/cert.go | 15 ++++++++++- server/service/integration_mdm_test.go | 35 +++++++++++++------------- server/service/mdm.go | 32 +++++++++++++++++++++-- 4 files changed, 63 insertions(+), 21 deletions(-) diff --git a/cmd/fleetctl/apple_mdm_test.go b/cmd/fleetctl/apple_mdm_test.go index 4dfb429e95..960271a775 100644 --- a/cmd/fleetctl/apple_mdm_test.go +++ b/cmd/fleetctl/apple_mdm_test.go @@ -60,7 +60,7 @@ func TestGenerateMDMApple(t *testing.T) { "--email", "user@example.com", "--org", "Acme", }, - `POST /api/latest/fleet/mdm/apple/request_csr received status 502 Bad Gateway: FleetDM CSR request failed: api responded with 400: bad request`, + `POST /api/latest/fleet/mdm/apple/request_csr received status 422 Validation Failed: this email address is not valid: bad request`, ) }) diff --git a/server/mdm/apple/cert.go b/server/mdm/apple/cert.go index daef783939..aa300c4596 100644 --- a/server/mdm/apple/cert.go +++ b/server/mdm/apple/cert.go @@ -60,6 +60,19 @@ func GenerateAPNSCSRKey(email, org string) (*x509.CertificateRequest, *rsa.Priva return certReq, key, nil } +type FleetWebsiteError struct { + Status int + message string +} + +func (e FleetWebsiteError) Error() string { + if e.message != "" { + return e.message + } + + return "Unknown Error" +} + type getSignedAPNSCSRRequest struct { UnsignedCSRData []byte `json:"unsignedCsrData"` } @@ -98,7 +111,7 @@ func GetSignedAPNSCSR(client *http.Client, csr *x509.CertificateRequest) error { if resp.StatusCode != http.StatusOK { b, _ := ioutil.ReadAll(resp.Body) - return fmt.Errorf("api responded with %d: %s", resp.StatusCode, string(b)) + return FleetWebsiteError{Status: resp.StatusCode, message: string(b)} } return nil } diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go index e3f62954a8..8a4280c235 100644 --- a/server/service/integration_mdm_test.go +++ b/server/service/integration_mdm_test.go @@ -59,11 +59,11 @@ func TestIntegrationsMDM(t *testing.T) { type integrationMDMTestSuite struct { suite.Suite withServer - fleetCfg config.FleetConfig - fleetDMFailCSR atomic.Bool - pushProvider *mock.APNSPushProvider - depStorage nanodep_storage.AllStorage - depSchedule *schedule.Schedule + fleetCfg config.FleetConfig + fleetDMNextCSRStatus atomic.Value + pushProvider *mock.APNSPushProvider + depStorage nanodep_storage.AllStorage + depSchedule *schedule.Schedule } func (s *integrationMDMTestSuite) SetupSuite() { @@ -131,25 +131,20 @@ func (s *integrationMDMTestSuite) SetupSuite() { s.depSchedule = depSchedule fleetdmSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if s.fleetDMFailCSR.Swap(false) { - // fail this call - w.WriteHeader(http.StatusBadRequest) - _, _ = w.Write([]byte("bad request")) - return - } - w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte("ok")) + status := s.fleetDMNextCSRStatus.Swap(http.StatusOK) + w.WriteHeader(status.(int)) + _, _ = w.Write([]byte(fmt.Sprintf("status: %d", status))) })) s.T().Setenv("TEST_FLEETDM_API_URL", fleetdmSrv.URL) s.T().Cleanup(fleetdmSrv.Close) } -func (s *integrationMDMTestSuite) FailNextCSRRequest() { - s.fleetDMFailCSR.Store(true) +func (s *integrationMDMTestSuite) FailNextCSRRequestWith(status int) { + s.fleetDMNextCSRStatus.Store(status) } func (s *integrationMDMTestSuite) SucceedNextCSRRequest() { - s.fleetDMFailCSR.Store(false) + s.fleetDMNextCSRStatus.Store(http.StatusOK) } func (s *integrationMDMTestSuite) TearDownTest() { @@ -517,7 +512,13 @@ func (s *integrationMDMTestSuite) TestAppleMDMCSRRequest() { require.Equal(t, errResp.Errors[0].Name, "organization") // fleetdm CSR request failed - s.FailNextCSRRequest() + s.FailNextCSRRequestWith(http.StatusBadRequest) + errResp = validationErrResp{} + s.DoJSON("POST", "/api/latest/fleet/mdm/apple/request_csr", requestMDMAppleCSRRequest{EmailAddress: "a@b.c", Organization: "test"}, http.StatusUnprocessableEntity, &errResp) + require.Len(t, errResp.Errors, 1) + require.Contains(t, errResp.Errors[0].Reason, "this email address is not valid") + + s.FailNextCSRRequestWith(http.StatusInternalServerError) errResp = validationErrResp{} s.DoJSON("POST", "/api/latest/fleet/mdm/apple/request_csr", requestMDMAppleCSRRequest{EmailAddress: "a@b.c", Organization: "test"}, http.StatusBadGateway, &errResp) require.Len(t, errResp.Errors, 1) diff --git a/server/service/mdm.go b/server/service/mdm.go index faa2eb519b..5ec57f7162 100644 --- a/server/service/mdm.go +++ b/server/service/mdm.go @@ -148,8 +148,36 @@ func (svc *Service) RequestMDMAppleCSR(ctx context.Context, email, org string) ( // request the signed APNs CSR from fleetdm.com client := fleethttp.NewClient(fleethttp.WithTimeout(10 * time.Second)) if err := apple_mdm.GetSignedAPNSCSR(client, apnsCSR); err != nil { - err = fleet.NewUserMessageError(fmt.Errorf("FleetDM CSR request failed: %w", err), http.StatusBadGateway) - return nil, ctxerr.Wrap(ctx, err) + if ferr, ok := err.(apple_mdm.FleetWebsiteError); ok { + status := http.StatusBadGateway + if ferr.Status >= 400 && ferr.Status <= 499 { + // TODO: fleetdm.com returns a genereric "Bad + // Request" message, we should coordinate and + // stablish a response schema from which we can get + // the invalid field and use + // fleet.NewInvalidArgumentError instead + // + // For now, since we have already validated + // everything else, we assume that a 4xx + // response is an email with an invalid domain + return nil, ctxerr.Wrap( + ctx, + fleet.NewInvalidArgumentError( + "email_address", + fmt.Sprintf("this email address is not valid: %v", err), + ), + ) + } + return nil, ctxerr.Wrap( + ctx, + fleet.NewUserMessageError( + fmt.Errorf("FleetDM CSR request failed: %w", err), + status, + ), + ) + } + + return nil, ctxerr.Wrap(ctx, err, "get signed CSR") } // PEM-encode the cert and keys