Improve SMTP authentication and Fix user creation bugs (#16612)

* Improve SMTP authentication, Fix user creation bugs and add LDAP cert/key options

This PR has two parts:

Improvements for SMTP authentication:

* Default to use SMTPS if port is 465, and allow setting of force SMTPS.
* Always use STARTTLS if available
* Provide CRAM-MD5 mechanism
* Add options for HELO hostname disabling
* Add options for providing certificates and keys
* Handle application specific password response as a failed user login
instead of as a 500.

Close #16104

Fix creation of new users:

* A bug was introduced when allowing users to change usernames which
prevents the creation of external users.
* The LoginSource refactor also broke this page.

Close #16104

Signed-off-by: Andrew Thornton <art27@cantab.net>

6 files changed, 68 insertions, 33 deletions

diff --git a/templates/admin/auth/edit.tmpl b/templates/admin/auth/edit.tmpl
index 2b499c7c76..109186a178 100644
--- a/templates/admin/auth/edit.tmpl
+++ b/templates/admin/auth/edit.tmpl
@@ -44,6 +44,12 @@
 						<label for="port">{{.i18n.Tr "admin.auths.port"}}</label>
 						<input id="port" name="port" value="{{$cfg.Port}}"  placeholder="e.g. 636" required>
 					</div>
+					<div class="has-tls inline field {{if not .HasTLS}}hide{{end}}">
+						<div class="ui checkbox">
+							<label><strong>{{.i18n.Tr "admin.auths.skip_tls_verify"}}</strong></label>
+							<input name="skip_verify" type="checkbox" {{if .Source.SkipVerify}}checked{{end}}>
+						</div>
+					</div>
 					{{if .Source.IsLDAP}}
 						<div class="field">
 							<label for="bind_dn">{{.i18n.Tr "admin.auths.bind_dn"}}</label>
@@ -174,6 +180,30 @@
 						<input id="smtp_port" name="smtp_port" value="{{$cfg.Port}}" required>
 					</div>
 					<div class="field">
+						<div class="ui checkbox">
+							<label for="force_smtps"><strong>{{.i18n.Tr "admin.auths.force_smtps"}}</strong></label>
+							<input id="force_smtps" name="force_smtps" type="checkbox" {{if $cfg.ForceSMTPS}}checked{{end}}>
+						</div>
+						<p class="help">{{.i18n.Tr "admin.auths.force_smtps_helper"}}</p>
+					</div>
+					<div class="has-tls inline field {{if not .HasTLS}}hide{{end}}">
+						<div class="ui checkbox">
+							<label><strong>{{.i18n.Tr "admin.auths.skip_tls_verify"}}</strong></label>
+							<input name="skip_verify" type="checkbox" {{if .Source.SkipVerify}}checked{{end}}>
+						</div>
+					</div>
+					<div class="field">
+						<label for="helo_hostname">{{.i18n.Tr "admin.auths.helo_hostname"}}</label>
+						<input id="helo_hostname" name="helo_hostname" value="{{$cfg.HeloHostname}}">
+						<p class="help">{{.i18n.Tr "admin.auths.helo_hostname_helper"}}</p>
+					</div>
+					<div class="inline field">
+						<div class="ui checkbox">
+							<label for="disable_helo"><strong>{{.i18n.Tr "admin.auths.disable_helo"}}</strong></label>
+							<input id="disable_helo" name="disable_helo" type="checkbox" {{if $cfg.DisableHelo}}checked{{end}}>
+						</div>
+					</div>
+					<div class="field">
 						<label for="allowed_domains">{{.i18n.Tr "admin.auths.allowed_domains"}}</label>
 						<input id="allowed_domains" name="allowed_domains" value="{{$cfg.AllowedDomains}}">
 						<p class="help">{{.i18n.Tr "admin.auths.allowed_domains_helper"}}</p>
@@ -308,26 +338,13 @@
 						<p class="help">{{.i18n.Tr "admin.auths.sspi_default_language_helper"}}</p>
 					</div>
 				{{end}}
-
-				<div class="inline field {{if not .Source.IsSMTP}}hide{{end}}">
-					<div class="ui checkbox">
-						<label><strong>{{.i18n.Tr "admin.auths.enable_tls"}}</strong></label>
-						<input name="tls" type="checkbox" {{if .Source.UseTLS}}checked{{end}}>
-					</div>
-				</div>
-				<div class="has-tls inline field {{if not .HasTLS}}hide{{end}}">
-					<div class="ui checkbox">
-						<label><strong>{{.i18n.Tr "admin.auths.skip_tls_verify"}}</strong></label>
-						<input name="skip_verify" type="checkbox" {{if .Source.SkipVerify}}checked{{end}}>
-					</div>
-				</div>
 				{{if .Source.IsLDAP}}
-				<div class="inline field">
-					<div class="ui checkbox">
-						<label><strong>{{.i18n.Tr "admin.auths.syncenabled"}}</strong></label>
-						<input name="is_sync_enabled" type="checkbox" {{if .Source.IsSyncEnabled}}checked{{end}}>
+					<div class="inline field">
+						<div class="ui checkbox">
+							<label><strong>{{.i18n.Tr "admin.auths.syncenabled"}}</strong></label>
+							<input name="is_sync_enabled" type="checkbox" {{if .Source.IsSyncEnabled}}checked{{end}}>
+						</div>
 					</div>
-				</div>
 				{{end}}
 				<div class="inline field">
 					<div class="ui checkbox">
diff --git a/templates/admin/auth/new.tmpl b/templates/admin/auth/new.tmpl
index 6addc50d09..ba1f145a4a 100644
--- a/templates/admin/auth/new.tmpl
+++ b/templates/admin/auth/new.tmpl
@@ -54,18 +54,6 @@
 						<input name="attributes_in_bind" type="checkbox" {{if .attributes_in_bind}}checked{{end}}>
 					</div>
 				</div>
-				<div class="smtp inline field {{if not (eq .type 3)}}hide{{end}}">
-					<div class="ui checkbox">
-						<label><strong>{{.i18n.Tr "admin.auths.enable_tls"}}</strong></label>
-						<input name="tls" type="checkbox" {{if .tls}}checked{{end}}>
-					</div>
-				</div>
-				<div class="has-tls inline field {{if not .HasTLS}}hide{{end}}">
-					<div class="ui checkbox">
-						<label><strong>{{.i18n.Tr "admin.auths.skip_tls_verify"}}</strong></label>
-						<input name="skip_verify" type="checkbox" {{if .skip_verify}}checked{{end}}>
-					</div>
-				</div>
 				<div class="ldap inline field {{if not (eq .type 2)}}hide{{end}}">
 					<div class="ui checkbox">
 						<label><strong>{{.i18n.Tr "admin.auths.syncenabled"}}</strong></label>
diff --git a/templates/admin/auth/source/ldap.tmpl b/templates/admin/auth/source/ldap.tmpl
index 0b7ad7a4da..295e001cf4 100644
--- a/templates/admin/auth/source/ldap.tmpl
+++ b/templates/admin/auth/source/ldap.tmpl
@@ -20,6 +20,12 @@
 		<label for="port">{{.i18n.Tr "admin.auths.port"}}</label>
 		<input id="port" name="port" value="{{.port}}"  placeholder="e.g. 636">
 	</div>
+	<div class="has-tls inline field {{if not .HasTLS}}hide{{end}}">
+		<div class="ui checkbox">
+			<label><strong>{{.i18n.Tr "admin.auths.skip_tls_verify"}}</strong></label>
+			<input name="skip_verify" type="checkbox" {{if .skip_verify}}checked{{end}}>
+		</div>
+	</div>
 	<div class="ldap field {{if not (eq .type 2)}}hide{{end}}">
 		<label for="bind_dn">{{.i18n.Tr "admin.auths.bind_dn"}}</label>
 		<input id="bind_dn" name="bind_dn" value="{{.bind_dn}}" placeholder="e.g. cn=Search,dc=mydomain,dc=com">
diff --git a/templates/admin/auth/source/smtp.tmpl b/templates/admin/auth/source/smtp.tmpl
index 670c4b3b50..b0f643b8ca 100644
--- a/templates/admin/auth/source/smtp.tmpl
+++ b/templates/admin/auth/source/smtp.tmpl
@@ -20,6 +20,30 @@
 		<label for="smtp_port">{{.i18n.Tr "admin.auths.smtpport"}}</label>
 		<input id="smtp_port" name="smtp_port" value="{{.smtp_port}}">
 	</div>
+	<div class="inline field">
+		<div class="ui checkbox">
+			<label for="force_smtps"><strong>{{.i18n.Tr "admin.auths.force_smtps"}}</strong></label>
+			<input id="force_smtps" name="force_smtps" type="checkbox" {{if .force_smtps}}checked{{end}}>
+			<p class="help">{{.i18n.Tr "admin.auths.force_smtps_helper"}}</p>
+		</div>
+	</div>
+	<div class="inline field">
+		<div class="ui checkbox">
+			<label><strong>{{.i18n.Tr "admin.auths.skip_tls_verify"}}</strong></label>
+			<input name="skip_verify" type="checkbox" {{if .skip_verify}}checked{{end}}>
+		</div>
+	</div>
+	<div class="field">
+		<label for="helo_hostname">{{.i18n.Tr "admin.auths.helo_hostname"}}</label>
+		<input id="helo_hostname" name="helo_hostname" value="{{.helo_hostname}}">
+		<p class="help">{{.i18n.Tr "admin.auths.helo_hostname_helper"}}</p>
+	</div>
+	<div class="inline field">
+		<div class="ui checkbox">
+			<label for="disable_helo"><strong>{{.i18n.Tr "admin.auths.disable_helo"}}</strong></label>
+			<input id="disable_helo" name="disable_helo" type="checkbox" {{if .disable_helo}}checked{{end}}>
+		</div>
+	</div>
 	<div class="field">
 		<label for="allowed_domains">{{.i18n.Tr "admin.auths.allowed_domains"}}</label>
 		<input id="allowed_domains" name="allowed_domains" value="{{.allowed_domains}}">
diff --git a/templates/admin/user/edit.tmpl b/templates/admin/user/edit.tmpl
index 5e5bc75c96..60cd8ad523 100644
--- a/templates/admin/user/edit.tmpl
+++ b/templates/admin/user/edit.tmpl
@@ -17,13 +17,13 @@
 				<div class="inline required field {{if .Err_LoginType}}error{{end}}">
 					<label>{{.i18n.Tr "admin.users.auth_source"}}</label>
 					<div class="ui selection type dropdown">
-						<input type="hidden" id="login_type" name="login_type" value="{{.LoginSource.Type}}-{{.LoginSource.ID}}" required>
+						<input type="hidden" id="login_type" name="login_type" value="{{.LoginSource.Type.Int}}-{{.LoginSource.ID}}" required>
 						<div class="text">{{.i18n.Tr "admin.users.local"}}</div>
 						{{svg "octicon-triangle-down" 14 "dropdown icon"}}
 						<div class="menu">
 							<div class="item" data-value="0-0">{{.i18n.Tr "admin.users.local"}}</div>
 							{{range .Sources}}
-								<div class="item" data-value="{{.Type}}-{{.ID}}">{{.Name}}</div>
+								<div class="item" data-value="{{.Type.Int}}-{{.ID}}">{{.Name}}</div>
 							{{end}}
 						</div>
 					</div>
diff --git a/templates/admin/user/new.tmpl b/templates/admin/user/new.tmpl
index a433c5a7cc..d454d1cd98 100644
--- a/templates/admin/user/new.tmpl
+++ b/templates/admin/user/new.tmpl
@@ -19,7 +19,7 @@
 						<div class="menu">
 							<div class="item" data-value="0-0">{{.i18n.Tr "admin.users.local"}}</div>
 							{{range .Sources}}
-								<div class="item" data-value="{{.Type}}-{{.ID}}">{{.Name}}</div>
+								<div class="item" data-value="{{.Type.Int}}-{{.ID}}">{{.Name}}</div>
 							{{end}}
 						</div>
 					</div>