Skip to content

Commit e29e163

Browse files
zeripathzeripath
authored
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 <[email protected]>
1 parent f1a810e commit e29e163

File tree

15 files changed

+161
-77
lines changed

15 files changed

+161
-77
lines changed

‎docs/content/doc/features/authentication.en-us.md

+6-4
Original file line numberDiff line numberDiff line change
@@ -201,16 +201,18 @@ configure this, set the fields below:
201201
with multiple domains.
202202
- Example: `gitea.io,mydomain.com,mydomain2.com`
203203

204-
- Enable TLS Encryption
204+
- Force SMTPS
205205

206-
- Enable TLS encryption on authentication.
206+
- SMTPS will be used by default for connections to port 465, if you wish to use SMTPS
207+
for other ports. Set this value.
208+
- Otherwise if the server provides the `STARTTLS` extension this will be used.
207209

208210
- Skip TLS Verify
209211

210212
- Disable TLS verify on authentication.
211213

212-
- This authentication is activate
213-
- Enable or disable this auth.
214+
- This Authentication Source is Activated
215+
- Enable or disable this authentication source.
214216

215217
## FreeIPA
216218

‎options/locale/locale_en-US.ini

+5-1
Original file line numberDiff line numberDiff line change
@@ -2427,8 +2427,12 @@ auths.smtphost = SMTP Host
24272427
auths.smtpport = SMTP Port
24282428
auths.allowed_domains = Allowed Domains
24292429
auths.allowed_domains_helper = Leave empty to allow all domains. Separate multiple domains with a comma (',').
2430-
auths.enable_tls = Enable TLS Encryption
24312430
auths.skip_tls_verify = Skip TLS Verify
2431+
auths.force_smtps = Force SMTPS
2432+
auths.force_smtps_helper = By default SMTPS will be used for port 465, set this to use smtps on other ports, otherwise STARTTLS is used if supported.
2433+
auths.helo_hostname = HELO Hostname
2434+
auths.helo_hostname_helper = Hostname sent with HELO. Leave blank to send current hostname.
2435+
auths.disable_helo = Disable HELO
24322436
auths.pam_service_name = PAM Service Name
24332437
auths.pam_email_domain = PAM Email Domain (optional)
24342438
auths.oauth2_provider = OAuth2 Provider

‎routers/web/admin/auths.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,10 @@ func parseSMTPConfig(form forms.AuthenticationForm) *smtp.Source {
154154
Host: form.SMTPHost,
155155
Port: form.SMTPPort,
156156
AllowedDomains: form.AllowedDomains,
157-
TLS: form.TLS,
157+
ForceSMTPS: form.ForceSMTPS,
158158
SkipVerify: form.SkipVerify,
159+
HeloHostname: form.HeloHostname,
160+
DisableHelo: form.DisableHelo,
159161
}
160162
}
161163

‎services/auth/source/ldap/source_search.go

+15-12
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ package ldap
88
import (
99
"crypto/tls"
1010
"fmt"
11+
"net"
12+
"strconv"
1113
"strings"
1214

1315
"code.gitea.io/gitea/modules/log"
@@ -103,26 +105,27 @@ func (ls *Source) findUserDN(l *ldap.Conn, name string) (string, bool) {
103105
return userDN, true
104106
}
105107

106-
func dial(ls *Source) (*ldap.Conn, error) {
107-
log.Trace("Dialing LDAP with security protocol (%v) without verifying: %v", ls.SecurityProtocol, ls.SkipVerify)
108+
func dial(source *Source) (*ldap.Conn, error) {
109+
log.Trace("Dialing LDAP with security protocol (%v) without verifying: %v", source.SecurityProtocol, source.SkipVerify)
108110

109-
tlsCfg := &tls.Config{
110-
ServerName: ls.Host,
111-
InsecureSkipVerify: ls.SkipVerify,
111+
tlsConfig := &tls.Config{
112+
ServerName: source.Host,
113+
InsecureSkipVerify: source.SkipVerify,
112114
}
113-
if ls.SecurityProtocol == SecurityProtocolLDAPS {
114-
return ldap.DialTLS("tcp", fmt.Sprintf("%s:%d", ls.Host, ls.Port), tlsCfg)
115+
116+
if source.SecurityProtocol == SecurityProtocolLDAPS {
117+
return ldap.DialTLS("tcp", net.JoinHostPort(source.Host, strconv.Itoa(source.Port)), tlsConfig)
115118
}
116119

117-
conn, err := ldap.Dial("tcp", fmt.Sprintf("%s:%d", ls.Host, ls.Port))
120+
conn, err := ldap.Dial("tcp", net.JoinHostPort(source.Host, strconv.Itoa(source.Port)))
118121
if err != nil {
119-
return nil, fmt.Errorf("Dial: %v", err)
122+
return nil, fmt.Errorf("error during Dial: %v", err)
120123
}
121124

122-
if ls.SecurityProtocol == SecurityProtocolStartTLS {
123-
if err = conn.StartTLS(tlsCfg); err != nil {
125+
if source.SecurityProtocol == SecurityProtocolStartTLS {
126+
if err = conn.StartTLS(tlsConfig); err != nil {
124127
conn.Close()
125-
return nil, fmt.Errorf("StartTLS: %v", err)
128+
return nil, fmt.Errorf("error during StartTLS: %v", err)
126129
}
127130
}
128131

‎services/auth/source/smtp/auth.go

+43-19
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@ package smtp
66

77
import (
88
"crypto/tls"
9-
"errors"
109
"fmt"
10+
"net"
1111
"net/smtp"
12+
"os"
13+
"strconv"
1214

1315
"code.gitea.io/gitea/models"
1416
)
@@ -42,40 +44,62 @@ func (auth *loginAuthenticator) Next(fromServer []byte, more bool) ([]byte, erro
4244

4345
// SMTP authentication type names.
4446
const (
45-
PlainAuthentication = "PLAIN"
46-
LoginAuthentication = "LOGIN"
47+
PlainAuthentication = "PLAIN"
48+
LoginAuthentication = "LOGIN"
49+
CRAMMD5Authentication = "CRAM-MD5"
4750
)
4851

4952
// Authenticators contains available SMTP authentication type names.
50-
var Authenticators = []string{PlainAuthentication, LoginAuthentication}
53+
var Authenticators = []string{PlainAuthentication, LoginAuthentication, CRAMMD5Authentication}
5154

5255
// Authenticate performs an SMTP authentication.
5356
func Authenticate(a smtp.Auth, source *Source) error {
54-
c, err := smtp.Dial(fmt.Sprintf("%s:%d", source.Host, source.Port))
57+
tlsConfig := &tls.Config{
58+
InsecureSkipVerify: source.SkipVerify,
59+
ServerName: source.Host,
60+
}
61+
62+
conn, err := net.Dial("tcp", net.JoinHostPort(source.Host, strconv.Itoa(source.Port)))
5563
if err != nil {
5664
return err
5765
}
58-
defer c.Close()
66+
defer conn.Close()
5967

60-
if err = c.Hello("gogs"); err != nil {
61-
return err
68+
if source.UseTLS() {
69+
conn = tls.Client(conn, tlsConfig)
70+
}
71+
72+
client, err := smtp.NewClient(conn, source.Host)
73+
if err != nil {
74+
return fmt.Errorf("failed to create NewClient: %w", err)
6275
}
76+
defer client.Close()
6377

64-
if source.TLS {
65-
if ok, _ := c.Extension("STARTTLS"); ok {
66-
if err = c.StartTLS(&tls.Config{
67-
InsecureSkipVerify: source.SkipVerify,
68-
ServerName: source.Host,
69-
}); err != nil {
70-
return err
78+
if !source.DisableHelo {
79+
hostname := source.HeloHostname
80+
if len(hostname) == 0 {
81+
hostname, err = os.Hostname()
82+
if err != nil {
83+
return fmt.Errorf("failed to find Hostname: %w", err)
7184
}
72-
} else {
73-
return errors.New("SMTP server unsupports TLS")
85+
}
86+
87+
if err = client.Hello(hostname); err != nil {
88+
return fmt.Errorf("failed to send Helo: %w", err)
89+
}
90+
}
91+
92+
// If not using SMTPS, always use STARTTLS if available
93+
hasStartTLS, _ := client.Extension("STARTTLS")
94+
if !source.UseTLS() && hasStartTLS {
95+
if err = client.StartTLS(tlsConfig); err != nil {
96+
return fmt.Errorf("failed to start StartTLS: %v", err)
7497
}
7598
}
7699

77-
if ok, _ := c.Extension("AUTH"); ok {
78-
return c.Auth(a)
100+
if ok, _ := client.Extension("AUTH"); ok {
101+
return client.Auth(a)
79102
}
103+
80104
return models.ErrUnsupportedLoginType
81105
}

‎services/auth/source/smtp/source.go

+4-2
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@ type Source struct {
2222
Host string
2323
Port int
2424
AllowedDomains string `xorm:"TEXT"`
25-
TLS bool
25+
ForceSMTPS bool
2626
SkipVerify bool
27+
HeloHostname string
28+
DisableHelo bool
2729

2830
// reference to the loginSource
2931
loginSource *models.LoginSource
@@ -51,7 +53,7 @@ func (source *Source) HasTLS() bool {
5153

5254
// UseTLS returns if TLS is set
5355
func (source *Source) UseTLS() bool {
54-
return source.TLS
56+
return source.ForceSMTPS || source.Port == 465
5557
}
5658

5759
// SetLoginSource sets the related LoginSource

‎services/auth/source/smtp/source_authenticate.go

+11-4
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,15 @@ func (source *Source) Authenticate(user *models.User, login, password string) (*
2828
}
2929

3030
var auth smtp.Auth
31-
if source.Auth == PlainAuthentication {
31+
switch source.Auth {
32+
case PlainAuthentication:
3233
auth = smtp.PlainAuth("", login, password, source.Host)
33-
} else if source.Auth == LoginAuthentication {
34+
case LoginAuthentication:
3435
auth = &loginAuthenticator{login, password}
35-
} else {
36-
return nil, errors.New("Unsupported SMTP auth type")
36+
case CRAMMD5Authentication:
37+
auth = smtp.CRAMMD5Auth(login, password)
38+
default:
39+
return nil, errors.New("unsupported SMTP auth type")
3740
}
3841

3942
if err := Authenticate(auth, source); err != nil {
@@ -44,6 +47,10 @@ func (source *Source) Authenticate(user *models.User, login, password string) (*
4447
strings.Contains(err.Error(), "Username and Password not accepted") {
4548
return nil, models.ErrUserNotExist{Name: login}
4649
}
50+
if (ok && tperr.Code == 534) ||
51+
strings.Contains(err.Error(), "Application-specific password required") {
52+
return nil, models.ErrUserNotExist{Name: login}
53+
}
4754
return nil, err
4855
}
4956

‎services/forms/auth_form.go

+3
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ type AuthenticationForm struct {
5050
SecurityProtocol int `binding:"Range(0,2)"`
5151
TLS bool
5252
SkipVerify bool
53+
HeloHostname string
54+
DisableHelo bool
55+
ForceSMTPS bool
5356
PAMServiceName string
5457
PAMEmailDomain string
5558
Oauth2Provider string

‎templates/admin/auth/edit.tmpl

+35-18
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@
4444
<label for="port">{{.i18n.Tr "admin.auths.port"}}</label>
4545
<input id="port" name="port" value="{{$cfg.Port}}" placeholder="e.g. 636" required>
4646
</div>
47+
<div class="has-tls inline field {{if not .HasTLS}}hide{{end}}">
48+
<div class="ui checkbox">
49+
<label><strong>{{.i18n.Tr "admin.auths.skip_tls_verify"}}</strong></label>
50+
<input name="skip_verify" type="checkbox" {{if .Source.SkipVerify}}checked{{end}}>
51+
</div>
52+
</div>
4753
{{if .Source.IsLDAP}}
4854
<div class="field">
4955
<label for="bind_dn">{{.i18n.Tr "admin.auths.bind_dn"}}</label>
@@ -173,6 +179,30 @@
173179
<label for="smtp_port">{{.i18n.Tr "admin.auths.smtpport"}}</label>
174180
<input id="smtp_port" name="smtp_port" value="{{$cfg.Port}}" required>
175181
</div>
182+
<div class="field">
183+
<div class="ui checkbox">
184+
<label for="force_smtps"><strong>{{.i18n.Tr "admin.auths.force_smtps"}}</strong></label>
185+
<input id="force_smtps" name="force_smtps" type="checkbox" {{if $cfg.ForceSMTPS}}checked{{end}}>
186+
</div>
187+
<p class="help">{{.i18n.Tr "admin.auths.force_smtps_helper"}}</p>
188+
</div>
189+
<div class="has-tls inline field {{if not .HasTLS}}hide{{end}}">
190+
<div class="ui checkbox">
191+
<label><strong>{{.i18n.Tr "admin.auths.skip_tls_verify"}}</strong></label>
192+
<input name="skip_verify" type="checkbox" {{if .Source.SkipVerify}}checked{{end}}>
193+
</div>
194+
</div>
195+
<div class="field">
196+
<label for="helo_hostname">{{.i18n.Tr "admin.auths.helo_hostname"}}</label>
197+
<input id="helo_hostname" name="helo_hostname" value="{{$cfg.HeloHostname}}">
198+
<p class="help">{{.i18n.Tr "admin.auths.helo_hostname_helper"}}</p>
199+
</div>
200+
<div class="inline field">
201+
<div class="ui checkbox">
202+
<label for="disable_helo"><strong>{{.i18n.Tr "admin.auths.disable_helo"}}</strong></label>
203+
<input id="disable_helo" name="disable_helo" type="checkbox" {{if $cfg.DisableHelo}}checked{{end}}>
204+
</div>
205+
</div>
176206
<div class="field">
177207
<label for="allowed_domains">{{.i18n.Tr "admin.auths.allowed_domains"}}</label>
178208
<input id="allowed_domains" name="allowed_domains" value="{{$cfg.AllowedDomains}}">
@@ -308,26 +338,13 @@
308338
<p class="help">{{.i18n.Tr "admin.auths.sspi_default_language_helper"}}</p>
309339
</div>
310340
{{end}}
311-
312-
<div class="inline field {{if not .Source.IsSMTP}}hide{{end}}">
313-
<div class="ui checkbox">
314-
<label><strong>{{.i18n.Tr "admin.auths.enable_tls"}}</strong></label>
315-
<input name="tls" type="checkbox" {{if .Source.UseTLS}}checked{{end}}>
316-
</div>
317-
</div>
318-
<div class="has-tls inline field {{if not .HasTLS}}hide{{end}}">
319-
<div class="ui checkbox">
320-
<label><strong>{{.i18n.Tr "admin.auths.skip_tls_verify"}}</strong></label>
321-
<input name="skip_verify" type="checkbox" {{if .Source.SkipVerify}}checked{{end}}>
322-
</div>
323-
</div>
324341
{{if .Source.IsLDAP}}
325-
<div class="inline field">
326-
<div class="ui checkbox">
327-
<label><strong>{{.i18n.Tr "admin.auths.syncenabled"}}</strong></label>
328-
<input name="is_sync_enabled" type="checkbox" {{if .Source.IsSyncEnabled}}checked{{end}}>
342+
<div class="inline field">
343+
<div class="ui checkbox">
344+
<label><strong>{{.i18n.Tr "admin.auths.syncenabled"}}</strong></label>
345+
<input name="is_sync_enabled" type="checkbox" {{if .Source.IsSyncEnabled}}checked{{end}}>
346+
</div>
329347
</div>
330-
</div>
331348
{{end}}
332349
<div class="inline field">
333350
<div class="ui checkbox">

‎templates/admin/auth/new.tmpl

-12
Original file line numberDiff line numberDiff line change
@@ -54,18 +54,6 @@
5454
<input name="attributes_in_bind" type="checkbox" {{if .attributes_in_bind}}checked{{end}}>
5555
</div>
5656
</div>
57-
<div class="smtp inline field {{if not (eq .type 3)}}hide{{end}}">
58-
<div class="ui checkbox">
59-
<label><strong>{{.i18n.Tr "admin.auths.enable_tls"}}</strong></label>
60-
<input name="tls" type="checkbox" {{if .tls}}checked{{end}}>
61-
</div>
62-
</div>
63-
<div class="has-tls inline field {{if not .HasTLS}}hide{{end}}">
64-
<div class="ui checkbox">
65-
<label><strong>{{.i18n.Tr "admin.auths.skip_tls_verify"}}</strong></label>
66-
<input name="skip_verify" type="checkbox" {{if .skip_verify}}checked{{end}}>
67-
</div>
68-
</div>
6957
<div class="ldap inline field {{if not (eq .type 2)}}hide{{end}}">
7058
<div class="ui checkbox">
7159
<label><strong>{{.i18n.Tr "admin.auths.syncenabled"}}</strong></label>

‎templates/admin/auth/source/ldap.tmpl

+6
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,12 @@
2020
<label for="port">{{.i18n.Tr "admin.auths.port"}}</label>
2121
<input id="port" name="port" value="{{.port}}" placeholder="e.g. 636">
2222
</div>
23+
<div class="has-tls inline field {{if not .HasTLS}}hide{{end}}">
24+
<div class="ui checkbox">
25+
<label><strong>{{.i18n.Tr "admin.auths.skip_tls_verify"}}</strong></label>
26+
<input name="skip_verify" type="checkbox" {{if .skip_verify}}checked{{end}}>
27+
</div>
28+
</div>
2329
<div class="ldap field {{if not (eq .type 2)}}hide{{end}}">
2430
<label for="bind_dn">{{.i18n.Tr "admin.auths.bind_dn"}}</label>
2531
<input id="bind_dn" name="bind_dn" value="{{.bind_dn}}" placeholder="e.g. cn=Search,dc=mydomain,dc=com">

‎templates/admin/auth/source/smtp.tmpl

+24
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,30 @@
2020
<label for="smtp_port">{{.i18n.Tr "admin.auths.smtpport"}}</label>
2121
<input id="smtp_port" name="smtp_port" value="{{.smtp_port}}">
2222
</div>
23+
<div class="inline field">
24+
<div class="ui checkbox">
25+
<label for="force_smtps"><strong>{{.i18n.Tr "admin.auths.force_smtps"}}</strong></label>
26+
<input id="force_smtps" name="force_smtps" type="checkbox" {{if .force_smtps}}checked{{end}}>
27+
<p class="help">{{.i18n.Tr "admin.auths.force_smtps_helper"}}</p>
28+
</div>
29+
</div>
30+
<div class="inline field">
31+
<div class="ui checkbox">
32+
<label><strong>{{.i18n.Tr "admin.auths.skip_tls_verify"}}</strong></label>
33+
<input name="skip_verify" type="checkbox" {{if .skip_verify}}checked{{end}}>
34+
</div>
35+
</div>
36+
<div class="field">
37+
<label for="helo_hostname">{{.i18n.Tr "admin.auths.helo_hostname"}}</label>
38+
<input id="helo_hostname" name="helo_hostname" value="{{.helo_hostname}}">
39+
<p class="help">{{.i18n.Tr "admin.auths.helo_hostname_helper"}}</p>
40+
</div>
41+
<div class="inline field">
42+
<div class="ui checkbox">
43+
<label for="disable_helo"><strong>{{.i18n.Tr "admin.auths.disable_helo"}}</strong></label>
44+
<input id="disable_helo" name="disable_helo" type="checkbox" {{if .disable_helo}}checked{{end}}>
45+
</div>
46+
</div>
2347
<div class="field">
2448
<label for="allowed_domains">{{.i18n.Tr "admin.auths.allowed_domains"}}</label>
2549
<input id="allowed_domains" name="allowed_domains" value="{{.allowed_domains}}">

0 commit comments

Comments
 (0)