From 776a6143a222d96ecd82ab200533962aec0e8a8f Mon Sep 17 00:00:00 2001
From: Victor Lyuboslavsky <victor.lyuboslavsky@gmail.com>
Date: Fri, 3 Nov 2023 09:33:25 -0500
Subject: [PATCH] Invalid SSO metadata now generates 400 error instead of 500
 (#14903)

/fleet/sso endpoint now returns 400 status code (as opposed to 500) when
SSO Metadata URL returns invalid data or SSO Metadata is invalid

#12559

# 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/` or
`orbit/changes/`.
See [Changes
files](https://fleetdm.com/docs/contributing/committing-changes#changes-files)
for more information.
- [x] Added/updated tests
- [x] Manual QA for all new/changed functionality
---
 changes/12559-sso-metadata-url         |  1 +
 server/service/integration_sso_test.go | 55 ++++++++++++++++++++++++++
 server/service/sessions.go             |  2 +-
 3 files changed, 57 insertions(+), 1 deletion(-)
 create mode 100644 changes/12559-sso-metadata-url

diff --git a/changes/12559-sso-metadata-url b/changes/12559-sso-metadata-url
new file mode 100644
index 0000000000..a2debaf1d5
--- /dev/null
+++ b/changes/12559-sso-metadata-url
@@ -0,0 +1 @@
+/fleet/sso endpoint now returns 400 status code (as opposed to 500) when SSO Metadata URL returns invalid data or when SSO Metadata provided by user is invalid.
\ No newline at end of file
diff --git a/server/service/integration_sso_test.go b/server/service/integration_sso_test.go
index ade53abc18..ced7ad685b 100644
--- a/server/service/integration_sso_test.go
+++ b/server/service/integration_sso_test.go
@@ -83,6 +83,61 @@ func (s *integrationSSOTestSuite) TestGetSSOSettings() {
 	assert.True(t, strings.HasPrefix(authReq.ID, "id"), authReq.ID)
 }
 
+func (s *integrationSSOTestSuite) TestSSOInvalidMetadataURL() {
+	t := s.T()
+
+	badMetadataUrl := "https://www.fleetdm.com"
+	acResp := appConfigResponse{}
+	s.DoJSON(
+		"PATCH", "/api/latest/fleet/config", json.RawMessage(
+			`{
+		"sso_settings": {
+			"enable_sso": true,
+			"entity_id": "https://localhost:8080",
+			"issuer_uri": "http://localhost:8080/simplesaml/saml2/idp/SSOService.php",
+			"idp_name": "SimpleSAML",
+			"metadata_url": "`+badMetadataUrl+`",
+			"enable_jit_provisioning": false
+		}
+	}`,
+		), http.StatusOK, &acResp,
+	)
+	require.NotNil(t, acResp)
+
+	var resIni initiateSSOResponse
+	expectedStatus := http.StatusBadRequest
+	t.Logf("Expecting 400 %v status when bad SSO metadata_url is set: %v", expectedStatus, badMetadataUrl)
+	s.DoJSON("POST", "/api/v1/fleet/sso", map[string]string{}, expectedStatus, &resIni)
+}
+
+func (s *integrationSSOTestSuite) TestSSOInvalidMetadata() {
+	t := s.T()
+
+	badMetadata := "<EntityDescriptor>foo</EntityDescriptor>"
+	acResp := appConfigResponse{}
+	s.DoJSON(
+		"PATCH", "/api/latest/fleet/config", json.RawMessage(
+			`{
+		"sso_settings": {
+			"enable_sso": true,
+			"entity_id": "https://localhost:8080",
+			"issuer_uri": "http://localhost:8080/simplesaml/saml2/idp/SSOService.php",
+			"idp_name": "SimpleSAML",
+			"metadata": "`+badMetadata+`",
+			"metadata_url": "",
+			"enable_jit_provisioning": false
+		}
+	}`,
+		), http.StatusOK, &acResp,
+	)
+	require.NotNil(t, acResp)
+
+	var resIni initiateSSOResponse
+	expectedStatus := http.StatusBadRequest
+	t.Logf("Expecting %v status when bad SSO metadata is provided: %v", expectedStatus, badMetadata)
+	s.DoJSON("POST", "/api/v1/fleet/sso", map[string]string{}, expectedStatus, &resIni)
+}
+
 func (s *integrationSSOTestSuite) TestSSOValidation() {
 	acResp := appConfigResponse{}
 	// Test we are validating metadata_url
diff --git a/server/service/sessions.go b/server/service/sessions.go
index 84f6308d26..da46ef3041 100644
--- a/server/service/sessions.go
+++ b/server/service/sessions.go
@@ -303,7 +303,7 @@ func (svc *Service) InitiateSSO(ctx context.Context, redirectURL string) (string
 
 	metadata, err := sso.GetMetadata(&appConfig.SSOSettings.SSOProviderSettings)
 	if err != nil {
-		return "", ctxerr.Wrap(ctx, err, "InitiateSSO getting metadata")
+		return "", ctxerr.Wrap(ctx, badRequestErr("Could not get SSO Metadata. Check your SSO settings.", err))
 	}
 
 	serverURL := appConfig.ServerSettings.ServerURL