From fc9c0bbd5d4b4edaef470f79ac92003eda855595 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Krzysztof=20Skrz=C4=99tnicki?=
 <krzysztof.skrzetnicki@goteleport.com>
Date: Wed, 29 Jan 2025 09:32:29 +0100
Subject: [PATCH] fix panic in CheckSAMLEntityDescriptor

---
 lib/fixtures/fixtures.go  | 13 +++++++++++++
 lib/services/saml.go      |  4 ++++
 lib/services/saml_test.go | 14 +++++++++-----
 3 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/lib/fixtures/fixtures.go b/lib/fixtures/fixtures.go
index c329baa143cb3..3c92e534c7dc6 100644
--- a/lib/fixtures/fixtures.go
+++ b/lib/fixtures/fixtures.go
@@ -183,6 +183,19 @@ spec:
       pHM7WKwFyW1dvEDax3BGj9/cbKvpvcwR
     </ds:X509Certificate></ds:X509Data></ds:KeyInfo></md:KeyDescriptor><md:NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress</md:NameIDFormat><md:NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified</md:NameIDFormat><md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="https://dev-813354.oktapreview.com/app/gravitationaldev813354_teleportsaml_1/exkafftca6RqPVgyZ0h7/sso/saml"/><md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="https://dev-813354.oktapreview.com/app/gravitationaldev813354_teleportsaml_1/exkafftca6RqPVgyZ0h7/sso/saml"/></md:IDPSSODescriptor></md:EntityDescriptor>`
 
+const SAMLConnectorMissingIDPSSODescriptor = `kind: saml
+version: v2
+metadata:
+  name: OktaSAML
+  namespace: default
+spec:
+  acs: https://localhost:3080/v1/webapi/saml/acs
+  sso: https://dev-813354.oktapreview.com/app/gravitationaldev813354_teleportsaml_1/exkafftca6RqPVgyZ0h7/sso/saml
+  attributes_to_roles:
+    - {name: "groups", value: "Everyone", roles: ["admin"]}
+  entity_descriptor: |
+    <?xml version="1.0" encoding="UTF-8"?><EntityDescriptor xmlns="urn:oasis:names:tc:SAML:2.0:metadata" validUntil="0001-01-01T00:00:00Z" entityID="dummy"></EntityDescriptor>`
+
 const (
 	TLSCACertPEM = apifixtures.TLSCACertPEM
 	TLSCAKeyPEM  = apifixtures.TLSCAKeyPEM
diff --git a/lib/services/saml.go b/lib/services/saml.go
index c4bec91d2310d..da444418de4f3 100644
--- a/lib/services/saml.go
+++ b/lib/services/saml.go
@@ -167,6 +167,10 @@ func CheckSAMLEntityDescriptor(entityDescriptor string) ([]*x509.Certificate, er
 		return nil, trace.Wrap(err, "failed to parse entity_descriptor")
 	}
 
+	if metadata.IDPSSODescriptor == nil {
+		return nil, nil
+	}
+
 	var roots []*x509.Certificate
 
 	for _, kd := range metadata.IDPSSODescriptor.KeyDescriptors {
diff --git a/lib/services/saml_test.go b/lib/services/saml_test.go
index ab060dd218d9f..adebce529cf03 100644
--- a/lib/services/saml_test.go
+++ b/lib/services/saml_test.go
@@ -58,12 +58,16 @@ func TestParseFromMetadata(t *testing.T) {
 func TestCheckSAMLEntityDescriptor(t *testing.T) {
 	t.Parallel()
 
-	for name, input := range map[string]string{
-		"without certificate padding": fixtures.SAMLOktaConnectorV2,
-		"with certificate padding":    fixtures.SAMLOktaConnectorV2WithPadding,
+	for name, tt := range map[string]struct {
+		resource  string
+		wantCerts int
+	}{
+		"without certificate padding": {resource: fixtures.SAMLOktaConnectorV2, wantCerts: 1},
+		"with certificate padding":    {resource: fixtures.SAMLOktaConnectorV2WithPadding, wantCerts: 1},
+		"missing IDPSSODescriptor":    {resource: fixtures.SAMLConnectorMissingIDPSSODescriptor, wantCerts: 0},
 	} {
 		t.Run(name, func(t *testing.T) {
-			decoder := kyaml.NewYAMLOrJSONDecoder(strings.NewReader(input), defaults.LookaheadBufSize)
+			decoder := kyaml.NewYAMLOrJSONDecoder(strings.NewReader(tt.resource), defaults.LookaheadBufSize)
 			var raw UnknownResource
 			err := decoder.Decode(&raw)
 			require.NoError(t, err)
@@ -74,7 +78,7 @@ func TestCheckSAMLEntityDescriptor(t *testing.T) {
 			ed := oc.GetEntityDescriptor()
 			certs, err := CheckSAMLEntityDescriptor(ed)
 			require.NoError(t, err)
-			require.Len(t, certs, 1)
+			require.Len(t, certs, tt.wantCerts)
 		})
 	}
 }