From 37ef1dce1f20c16a7de0288fb93d87fbf9bdd6a9 Mon Sep 17 00:00:00 2001
From: Pawel Kopiczko <pawel.kopiczko@goteleport.com>
Date: Tue, 11 Feb 2025 14:34:56 +0000
Subject: [PATCH] SAML: Validate SSO URL matches the one in entity descriptor
 if both are given (#51968)

---
 lib/services/saml.go      | 12 ++++++++-
 lib/services/saml_test.go | 57 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+), 1 deletion(-)

diff --git a/lib/services/saml.go b/lib/services/saml.go
index e6f3a83c49d23..b78c30c76204d 100644
--- a/lib/services/saml.go
+++ b/lib/services/saml.go
@@ -98,7 +98,17 @@ func ValidateSAMLConnector(sc types.SAMLConnector, rg RoleGetter) error {
 
 		sc.SetIssuer(md.EntityID)
 		if md.IDPSSODescriptor != nil && len(md.IDPSSODescriptor.SingleSignOnServices) > 0 {
-			sc.SetSSO(md.IDPSSODescriptor.SingleSignOnServices[0].Location)
+			metadataSsoUrl := md.IDPSSODescriptor.SingleSignOnServices[0].Location
+			if sc.GetSSO() != "" && sc.GetSSO() != metadataSsoUrl {
+				log.WithFields(log.Fields{
+					"connector_name":       sc.GetName(),
+					"connector_sso_url":    sc.GetSSO(),
+					"idp_metadata_sso_url": metadataSsoUrl,
+				}).Warn(
+					"Connector has set SSO URL, but it does not match the one found in IDP metadata. Overwriting with the IDP metadata SSO URL.",
+				)
+			}
+			sc.SetSSO(metadataSsoUrl)
 		}
 	}
 
diff --git a/lib/services/saml_test.go b/lib/services/saml_test.go
index a90eb7d5ab526..e4e9031d6524a 100644
--- a/lib/services/saml_test.go
+++ b/lib/services/saml_test.go
@@ -254,3 +254,60 @@ func (rs roleSet) GetRole(ctx context.Context, name string) (types.Role, error)
 	}
 	return nil, trace.NotFound("unknown role: %s", name)
 }
+
+func Test_ValidateSAMLConnector(t *testing.T) {
+	t.Parallel()
+
+	const entityDescriptor = `
+		<EntityDescriptor xmlns="urn:oasis:names:tc:SAML:2.0:metadata" validUntil="2024-07-04T11:40:17.481Z" cacheDuration="PT48H" entityID="teleport.example.com/metadata">
+			<IDPSSODescriptor xmlns="urn:oasis:names:tc:SAML:2.0:metadata" protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol">
+				<SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="https://first.in.the.descriptor.example.com/sso/saml"></SingleSignOnService>
+				<SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="https://second.in.the.descriptor.example.com/sso/saml"></SingleSignOnService>
+			</IDPSSODescriptor>
+		</EntityDescriptor>`
+
+	// Create a roleSet with <nil> role values as ValidateSAMLConnector only checks if the role
+	// in the connector role mapping exists.
+	var existingRoles roleSet = map[string]types.Role{
+		"existing-test-role": nil,
+	}
+
+	testCases := []struct {
+		name           string
+		connectorDesc  types.SAMLConnectorSpecV2
+		expectedSsoUrl string
+	}{
+		{
+			name: "matching set SSO and entity descriptor URLs",
+			connectorDesc: types.SAMLConnectorSpecV2{
+				AssertionConsumerService: "https://example.com/acs",
+				EntityDescriptor:         entityDescriptor,
+				AttributesToRoles: []types.AttributeMapping{
+					{Name: "groups", Value: "Everyone", Roles: []string{"existing-test-role"}},
+				},
+			},
+			expectedSsoUrl: "https://first.in.the.descriptor.example.com/sso/saml",
+		},
+		{
+			name: "if set SSO URL and entity descriptor do not match, overwrite with the one from the descriptor",
+			connectorDesc: types.SAMLConnectorSpecV2{
+				AssertionConsumerService: "https://example.com/acs",
+				SSO:                      "https://i.do.not.match.example.com/sso",
+				EntityDescriptor:         entityDescriptor,
+				AttributesToRoles: []types.AttributeMapping{
+					{Name: "groups", Value: "Everyone", Roles: []string{"existing-test-role"}},
+				},
+			},
+			expectedSsoUrl: "https://first.in.the.descriptor.example.com/sso/saml",
+		},
+	}
+	for _, tc := range testCases {
+		connector, err := types.NewSAMLConnector("test-connector", tc.connectorDesc)
+		require.NoError(t, err)
+
+		err = ValidateSAMLConnector(connector, existingRoles)
+		require.NoError(t, err)
+
+		require.Equal(t, tc.expectedSsoUrl, connector.GetSSO())
+	}
+}