SAML: Validate SSO URL matches the one in entity descriptor if both a…

…re given
gravitational · Jan 31, 2025 · 1d81afc · 1d81afc
1 parent 5494b72
commit 1d81afc
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 1 deletion.
diff --git a/lib/services/saml.go b/lib/services/saml.go
@@ -102,7 +102,16 @@ 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)
+			ssoUrl := md.IDPSSODescriptor.SingleSignOnServices[0].Location
+			if sc.GetSSO() == "" {
+				sc.SetSSO(ssoUrl)
+			} else if sc.GetSSO() != ssoUrl {
+				//TODO(kopiczko) test?
+				//TODO(kopiczko) test?
+				//TODO(kopiczko) test?
+				//TODO(kopiczko) test?
+				return trace.BadParameter("connector has set SSO url %q, but it does not match the one found in IDP metadata %q", sc.GetSSO(), ssoUrl)
+			}
 		}
 	}
 

diff --git 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) {
+	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"></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
+		expectedErr   string
+	}{
+		{
+			name: "happy path",
+			connectorDesc: types.SAMLConnectorSpecV2{
+				AssertionConsumerService: "https://example.com/acs",
+				EntityDescriptor:         entityDescriptor,
+				AttributesToRoles: []types.AttributeMapping{
+					{Name: "groups", Value: "Everyone", Roles: []string{"existing-test-role"}},
+				},
+			},
+			expectedErr: "",
+		},
+		{
+			name: "set SSO URL and entity descriptior do not match",
+			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"}},
+				},
+			},
+			expectedErr: `connector has set SSO url "https://i.do.not.match.example.com/sso", but it does not match the one found in IDP metadata "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)
+		if tc.expectedErr == "" {
+			require.NoError(t, err)
+		} else {
+			require.Error(t, err)
+			require.ErrorContains(t, err, tc.expectedErr)
+		}
+	}
+}