Add SSL validation and connection test to LDAP configuration methods

.changes/v3.0.0/714-features.md

@@ -1,8 +1,9 @@
 * Add trusted certificate management types `TrustedCertificate` and `types.TrustedCertificate`
   together with `VCDClient.AutoTrustCertificate`, `VCDClient.CreateTrustedCertificate`,
-  `VCDClient.GetAllTrustedCertificates`, `GetTrustedCertificateByName`,
-  `VCDClient.GetTrustedCertificateById`, `TrustedCertificate.Update`, `TrustedCertificate.Delete`
-  [GH-714]
+  `TmOrg.CreateTrustedCertificate`, `VCDClient.GetAllTrustedCertificates`, `TmOrg.GetAllTrustedCertificates`,
+  `VCDClient.GetTrustedCertificateByName`, `TmOrg.GetTrustedCertificateByName`,
+  `VCDClient.GetTrustedCertificateById`, `TmOrg.GetTrustedCertificateById`, `TrustedCertificate.Update`, `TrustedCertificate.Delete`
+  [GH-714, GH-746]
 * vCenter management types `VCenter` and `types.VSphereVirtualCenter` adds Create, Update and Delete
  methods: `VCDClient.CreateVcenter`, `VCDClient.GetAllVCenters`, `VCDClient.GetVCenterByName`,
  `VCDClient.GetVCenterById`, `VCenter.Update`, `VCenter.Delete`, `VCenter.RefreshVcenter`,

.changes/v3.0.0/732-features.md

@@ -1,3 +1,3 @@
-* Created new type `TmLdapSettings` to manage LDAP settings for the Tenant Manager [GH-732]
+* Created new type `TmLdapSettings` to manage LDAP settings for the Tenant Manager [GH-732, GH-746]
 * Added methods `VCDClient.TmLdapConfigure`, `TmOrg.LdapConfigure`, `VCDClient.TmLdapDisable`, `TmOrg.LdapDisable`,
-  `VCDClient.TmGetLdapConfiguration`, `TmOrg.GetLdapConfiguration` to configure LDAP in Tenant Manager and regular Organizations [GH-732]
+  `VCDClient.TmGetLdapConfiguration`, `TmOrg.GetLdapConfiguration` to configure LDAP in Tenant Manager and regular Organizations [GH-732, GH-746]

govcd/api.go

@@ -378,6 +378,10 @@ func checkResp(resp *http.Response, err error) (*http.Response, error) {
 	return checkRespWithErrType(types.BodyTypeXML, resp, err, &types.Error{})
 }
 
+func checkJsonResp(resp *http.Response, err error) (*http.Response, error) {
+	return checkRespWithErrType(types.BodyTypeJSON, resp, err, &types.Error{})
+}
+
 // checkRespWithErrType allows to specify custom error errType for checkResp unmarshaling
 // the error.
 func checkRespWithErrType(bodyType types.BodyType, resp *http.Response, err, errType error) (*http.Response, error) {

govcd/api_vcd_test.go

@@ -162,6 +162,16 @@ type TestConfig struct {
 		NsxtManagerUrl      string `yaml:"nsxtManagerUrl"`
 		NsxtEdgeCluster     string `yaml:"nsxtEdgeCluster"`
 		NsxtTier0Gateway    string `yaml:"nsxtTier0Gateway"`
+
+		Ldap struct {
+			Host                  string `yaml:"host"`
+			Port                  int    `yaml:"port"`
+			IsSsl                 bool   `yaml:"isSsl"`
+			Username              string `yaml:"username"`
+			Password              string `yaml:"password"`
+			BaseDistinguishedName string `yaml:"baseDistinguishedName"`
+			Type                  string `yaml:"type"`
+		} `yaml:"ldap,omitempty"`
 	} `yaml:"tm,omitempty"`
 	VCD struct {
 		Org         string `yaml:"org"`

govcd/nsxt_manager_openapi_test.go

@@ -27,7 +27,7 @@ func (vcd *TestVCD) Test_NsxtManagerOpenApi(check *C) {
 	// Certificate must be trusted before adding NSX-T Manager
 	url, err := url.Parse(cfg.Url)
 	check.Assert(err, IsNil)
-	trustedCert, err := vcd.client.AutoTrustCertificate(url)
+	trustedCert, err := vcd.client.AutoTrustHttpsCertificate(url, nil)
 	check.Assert(err, IsNil)
 	if trustedCert != nil {
 		AddToCleanupListOpenApi(trustedCert.TrustedCertificate.ID, check.TestName()+"trusted-cert", types.OpenApiPathVersion1_0_0+types.OpenApiEndpointTrustedCertificates+trustedCert.TrustedCertificate.ID)

govcd/org_oidc.go

@@ -12,7 +12,6 @@ import (
 	"io"
 	"net/http"
 	"net/url"
-	"strconv"
 	"strings"
 
 	"github.com/vmware/go-vcloud-director/v3/types/v56"
@@ -225,20 +224,13 @@ func oidcValidateConnection(client *Client, endpoint string) error {
 	if err != nil {
 		return err
 	}
-	isSecure := strings.ToLower(uri.Scheme) == "https"
-
-	rawPort := uri.Port()
-	if rawPort == "" {
-		rawPort = "80"
-		if isSecure {
-			rawPort = "443"
-		}
-	}
-	port, err := strconv.Atoi(rawPort)
+	port, err := getEndpointPort(uri)
 	if err != nil {
-		return err
+		return fmt.Errorf("error getting port number for host '%s': %s", uri.Hostname(), err)
 	}
 
+	isSecure := port == 443
+
 	result, err := client.TestConnection(types.TestConnection{
 		Host:   uri.Hostname(),
 		Port:   port,

govcd/sample_govcd_test_config.yaml

@@ -318,3 +318,12 @@ tm:
   nsxtManagerPassword: password for NSX-T Manager
   nsxtManagerUrl: https://HOST
   nsxtTier0Gateway: tier0gateway
+
+  ldap:
+    host: my.ldap.server.net
+    port: 636
+    isSsl: true
+    username: admin
+    password: StrongPassword
+    baseDistinguishedName: OU=demo,DC=foo,DC=bar,DC=test,DC=net
+    type: ACTIVE_DIRECTORY # ACTIVE_DIRECTORY or OPEN_LDAP

govcd/tm_common_test.go

@@ -48,7 +48,7 @@ func getOrCreateVCenter(vcd *TestVCD, check *C) (*VCenter, func()) {
 	// Certificate must be trusted before adding vCenter
 	url, err := url.Parse(vcCfg.Url)
 	check.Assert(err, IsNil)
-	trustedCert, err := vcd.client.AutoTrustCertificate(url)
+	trustedCert, err := vcd.client.AutoTrustHttpsCertificate(url, nil)
 	check.Assert(err, IsNil)
 	if trustedCert != nil {
 		printVerbose("# Certificate for vCenter is trusted %s\n", trustedCert.TrustedCertificate.ID)
@@ -136,7 +136,7 @@ func getOrCreateNsxtManager(vcd *TestVCD, check *C) (*NsxtManagerOpenApi, func()
 	// Certificate must be trusted before adding NSX-T Manager
 	url, err := url.Parse(nsxtCfg.Url)
 	check.Assert(err, IsNil)
-	trustedCert, err := vcd.client.AutoTrustCertificate(url)
+	trustedCert, err := vcd.client.AutoTrustHttpsCertificate(url, nil)
 	check.Assert(err, IsNil)
 	if trustedCert != nil {
 		printVerbose("# Certificate for NSX-T Manager is trusted %s\n", trustedCert.TrustedCertificate.ID)

govcd/tm_content_library_test.go

@@ -227,7 +227,7 @@ func (vcd *TestVCD) Test_ContentLibrarySubscribed(check *C) {
 	// Certificate must be trusted before adding subscribed content library
 	url, err := url.Parse(vcd.config.Tm.SubscriptionContentLibraryUrl)
 	check.Assert(err, IsNil)
-	trustedCert, err := vcd.client.AutoTrustCertificate(url)
+	trustedCert, err := vcd.client.AutoTrustHttpsCertificate(url, nil)
 	check.Assert(err, IsNil)
 	if trustedCert != nil {
 		AddToCleanupListOpenApi(trustedCert.TrustedCertificate.ID, check.TestName()+"trusted-cert", types.OpenApiPathVersion1_0_0+types.OpenApiEndpointTrustedCertificates+trustedCert.TrustedCertificate.ID)

govcd/tm_ldap.go

@@ -15,40 +15,140 @@ import (
 	"strings"
 )
 
-// TmLdapConfigure configures LDAP for the Tenant Manager "System" organization
-func (vcdClient *VCDClient) TmLdapConfigure(settings *types.TmLdapSettings) (*types.TmLdapSettings, error) {
+// TmLdapConfigure configures LDAP for the Tenant Manager "System" organization. If trustSslCertificate=true,
+// it will automatically trust the certificate of LDAP server, only if settings.IsSsl=true.
+// If settings.IsSsl=true and trustSslCertificate=false this method returns an error
+func (vcdClient *VCDClient) TmLdapConfigure(settings *types.TmLdapSettings, trustSslCertificate bool) (*types.TmLdapSettings, error) {
+	var trustedCert *TrustedCertificate
+
 	if !vcdClient.Client.IsTm() {
 		return nil, fmt.Errorf("this method is only supported in TM")
 	}
+	if settings == nil {
+		return nil, fmt.Errorf("LDAP settings cannot be nil")
+	}
+
+	// Always test connection, because this method always changes LDAP. There's no NONE mode like in
+	// the TmOrg receiver methods (that consume types.OrgLdapSettingsType)
+	connResult, err := ldapValidateConnection(&vcdClient.Client, settings.HostName, settings.Port, settings.IsSsl)
+	if err != nil {
+		return nil, err
+	}
+
+	if !trustSslCertificate && connResult.TargetProbe.SSLResult == types.UntrustedCertificate {
+		return nil, fmt.Errorf("cannot configure LDAP '%s:%d' over SSL without trusting the SSL certificate", settings.HostName, settings.Port)
+	}
+
+	if trustSslCertificate && connResult.TargetProbe.SSLResult == types.UntrustedCertificate {
+		// We retrieve existing LDAP configuration to check if the given input settings are for a fresh connection, or to override
+		// an existing one
+		existing, err := vcdClient.TmGetLdapConfiguration()
+		if err != nil {
+			return nil, fmt.Errorf("error retrieving existing LDAP configuration: %s", err)
+		}
+
+		// If SSL is configured and retrieved settings are empty, or the hostname changed, we need to trust the certificate
+		needsCheckCert := settings.IsSsl && (existing.HostName == "" || existing.HostName != settings.HostName)
+		if needsCheckCert {
+			trustedCert, err = trustCertificate(vcdClient, nil, settings.HostName, connResult.TargetProbe.CertificateChain)
+			if err != nil {
+				return nil, fmt.Errorf("could not trust certificate for %s, Connection result: '%s', SSL result: '%s': %s",
+					settings.HostName, connResult.TargetProbe.ConnectionResult, connResult.TargetProbe.SSLResult, err)
+			}
+		}
+	}
 
 	result, err := ldapExecuteRequest(vcdClient, "", http.MethodPut, settings)
 	if err != nil {
+		// An error happened, so we must clean the trusted certificate
+		if trustedCert != nil {
+			innerErr := trustedCert.Delete()
+			if innerErr != nil {
+				return nil, fmt.Errorf("%s - Also, cleanup of SSL certificate '%s' failed: %s", err, trustedCert.TrustedCertificate.ID, innerErr)
+			}
+		}
 		return nil, err
 	}
 	return result.(*types.TmLdapSettings), nil
 }
 
-// LdapConfigure configures LDAP for the given organization
-func (org *TmOrg) LdapConfigure(settings *types.OrgLdapSettingsType) (*types.OrgLdapSettingsType, error) {
+// LdapConfigure configures LDAP for the receiver Organization. If trustSslCertificate=true,
+// it will automatically trust the certificate of LDAP server, only if settings.IsSsl=true.
+// If settings.IsSsl=true and trustSslCertificate=false this method returns an error
+func (org *TmOrg) LdapConfigure(settings *types.OrgLdapSettingsType, trustSslCertificate bool) (*types.OrgLdapSettingsType, error) {
+	var trustedCert *TrustedCertificate
+
+	if settings == nil {
+		return nil, fmt.Errorf("LDAP settings cannot be nil")
+	}
+
+	// Validate the connection with LDAP only when types.LdapModeCustom mode is selected (as it's the only one that configures an LDAP server)
+	if settings.CustomOrgLdapSettings != nil && settings.OrgLdapMode == types.LdapModeCustom {
+		connResult, err := ldapValidateConnection(&org.vcdClient.Client, settings.CustomOrgLdapSettings.HostName, settings.CustomOrgLdapSettings.Port, settings.CustomOrgLdapSettings.IsSsl)
+		if err != nil {
+			return nil, err
+		}
+
+		if !trustSslCertificate && connResult.TargetProbe.SSLResult == types.UntrustedCertificate {
+			return nil, fmt.Errorf("cannot configure LDAP '%s:%d' over SSL without trusting the SSL certificate", settings.CustomOrgLdapSettings.HostName, settings.CustomOrgLdapSettings.Port)
+		}
+
+		if trustSslCertificate && connResult.TargetProbe.SSLResult == types.UntrustedCertificate {
+			// We retrieve existing LDAP configuration to check if the given input settings are for a fresh connection, or to override
+			// an existing one
+			existing, err := org.GetLdapConfiguration()
+			if err != nil {
+				return nil, fmt.Errorf("error retrieving existing LDAP configuration: %s", err)
+			}
+
+			// Pre-condition here: Settings are always LDAP=CUSTOM and CustomOrgLdapSettings is not nil.
+			// Just check if existing LDAP config is new (different from CUSTOM), or if the host changed, as we need to trust certificate
+			// in that case
+			needsCheckCert := settings.CustomOrgLdapSettings.IsSsl && (existing.OrgLdapMode != types.LdapModeCustom ||
+				(existing.CustomOrgLdapSettings != nil && existing.CustomOrgLdapSettings.HostName != settings.CustomOrgLdapSettings.HostName))
+			if needsCheckCert {
+				trustedCert, err = trustCertificate(org.vcdClient, &TenantContext{
+					OrgId:   org.TmOrg.ID,
+					OrgName: org.TmOrg.Name,
+				}, settings.CustomOrgLdapSettings.HostName, connResult.TargetProbe.CertificateChain)
+				if err != nil {
+					return nil, fmt.Errorf("could not trust certificate for %s, Connection result: '%s', SSL result: '%s': %s",
+						settings.CustomOrgLdapSettings.HostName, connResult.TargetProbe.ConnectionResult, connResult.TargetProbe.SSLResult, err)
+				}
+			}
+		}
+	}
+
 	result, err := ldapExecuteRequest(org.vcdClient, org.TmOrg.ID, http.MethodPut, settings)
-	if err != nil {
-		return nil, err
+	if err == nil {
+		return result.(*types.OrgLdapSettingsType), nil
 	}
-	return result.(*types.OrgLdapSettingsType), nil
+
+	// An error happened, so we must clean the trusted certificate
+	if trustedCert != nil {
+		innerErr := trustedCert.Delete()
+		if innerErr != nil {
+			return nil, fmt.Errorf("%s - Also, cleanup of SSL certificate '%s' failed: %s", err, trustedCert.TrustedCertificate.ID, innerErr)
+		}
+	}
+	return nil, err
 }
 
-// TmLdapDisable wraps LdapConfigure to disable LDAP configuration for the "System" organization
+// TmLdapDisable wraps LdapConfigure to disable LDAP configuration for the "System" organization.
+// Disabling LDAP does not remove any trusted certificate.
 func (vcdClient *VCDClient) TmLdapDisable() error {
 	if !vcdClient.Client.IsTm() {
 		return fmt.Errorf("this method is only supported in TM")
 	}
+	// It is a different endpoint, so we don't need to check certificates.
 	_, err := ldapExecuteRequest(vcdClient, "", http.MethodDelete, nil)
 	return err
 }
 
 // LdapDisable wraps LdapConfigure to disable LDAP configuration for the given organization
+// Disabling LDAP does not remove any trusted certificate.
 func (org *TmOrg) LdapDisable() error {
-	// For Orgs, deletion is PUT call with empty payload
+	// For Orgs, deletion is PUT call with empty payload. We don't need to check certificates.
 	_, err := ldapExecuteRequest(org.vcdClient, org.TmOrg.ID, http.MethodPut, &types.OrgLdapSettingsType{OrgLdapMode: types.LdapModeNone})
 	return err
 }
@@ -74,6 +174,30 @@ func (org *TmOrg) GetLdapConfiguration() (*types.OrgLdapSettingsType, error) {
 	return result.(*types.OrgLdapSettingsType), nil
 }
 
+// ldapValidateConnection executes a test probe against the given endpoint to validate that the client
+// can establish a connection.
+func ldapValidateConnection(client *Client, endpoint string, port int, isSecure bool) (*types.TestConnectionResult, error) {
+	uri, err := url.Parse(endpoint)
+	if err != nil {
+		return nil, err
+	}
+
+	result, err := client.TestConnection(types.TestConnection{
+		Host:                          endpoint,
+		HostnameVerificationAlgorithm: "LDAPS",
+		Port:                          port,
+		Secure:                        &isSecure,
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	if result.TargetProbe == nil || !result.TargetProbe.CanConnect || result.TargetProbe.ConnectionResult != "SUCCESS" {
+		return nil, fmt.Errorf("could not establish a connection to %s. Result: %s", uri.String(), result.TargetProbe.Result)
+	}
+	return result, nil
+}
+
 // ldapExecuteRequest executes a request to the LDAP endpoint with the given payload and HTTP method
 func ldapExecuteRequest(vcdClient *VCDClient, orgId, method string, payload interface{}) (interface{}, error) {
 	if method == http.MethodPut && payload == nil {
@@ -115,7 +239,7 @@ func ldapExecuteRequest(vcdClient *VCDClient, orgId, method string, payload inte
 
 	// Perform the HTTP call with the obtained endpoint and method
 	req := vcdClient.Client.newRequest(nil, nil, method, *endpoint, body, "", headers)
-	resp, err := checkResp(vcdClient.Client.Http.Do(req))
+	resp, err := checkJsonResp(vcdClient.Client.Http.Do(req))
 	if err != nil {
 		return nil, fmt.Errorf("error getting LDAP settings: %s", err)
 	}