Skip to content

Commit 09dbd85

Browse files
authored
Various fixes in login sources (#10428)
1 parent 542bd59 commit 09dbd85

File tree

15 files changed

+66
-21
lines changed

15 files changed

+66
-21
lines changed

models/error.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,21 @@ func (err ErrNamePatternNotAllowed) Error() string {
5757
return fmt.Sprintf("name pattern is not allowed [pattern: %s]", err.Pattern)
5858
}
5959

60+
// ErrNameCharsNotAllowed represents a "character not allowed in name" error.
61+
type ErrNameCharsNotAllowed struct {
62+
Name string
63+
}
64+
65+
// IsErrNameCharsNotAllowed checks if an error is an ErrNameCharsNotAllowed.
66+
func IsErrNameCharsNotAllowed(err error) bool {
67+
_, ok := err.(ErrNameCharsNotAllowed)
68+
return ok
69+
}
70+
71+
func (err ErrNameCharsNotAllowed) Error() string {
72+
return fmt.Sprintf("User name is invalid [%s]: must be valid alpha or numeric or dash(-_) or dot characters", err.Name)
73+
}
74+
6075
// ErrSSHDisabled represents an "SSH disabled" error.
6176
type ErrSSHDisabled struct {
6277
}

models/login_source.go

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"fmt"
1313
"net/smtp"
1414
"net/textproto"
15-
"regexp"
1615
"strings"
1716

1817
"code.gitea.io/gitea/modules/auth/ldap"
@@ -455,10 +454,6 @@ func composeFullName(firstname, surname, username string) string {
455454
}
456455
}
457456

458-
var (
459-
alphaDashDotPattern = regexp.MustCompile(`[^\w-\.]`)
460-
)
461-
462457
// LoginViaLDAP queries if login/password is valid against the LDAP directory pool,
463458
// and create a local user if success when enabled.
464459
func LoginViaLDAP(user *User, login, password string, source *LoginSource) (*User, error) {
@@ -503,10 +498,6 @@ func LoginViaLDAP(user *User, login, password string, source *LoginSource) (*Use
503498
if len(sr.Username) == 0 {
504499
sr.Username = login
505500
}
506-
// Validate username make sure it satisfies requirement.
507-
if alphaDashDotPattern.MatchString(sr.Username) {
508-
return nil, fmt.Errorf("Invalid pattern for attribute 'username' [%s]: must be valid alpha or numeric or dash(-_) or dot characters", sr.Username)
509-
}
510501

511502
if len(sr.Mail) == 0 {
512503
sr.Mail = fmt.Sprintf("%s@localhost", sr.Username)
@@ -666,7 +657,8 @@ func LoginViaSMTP(user *User, login, password string, sourceID int64, cfg *SMTPC
666657
// LoginViaPAM queries if login/password is valid against the PAM,
667658
// and create a local user if success when enabled.
668659
func LoginViaPAM(user *User, login, password string, sourceID int64, cfg *PAMConfig) (*User, error) {
669-
if err := pam.Auth(cfg.ServiceName, login, password); err != nil {
660+
pamLogin, err := pam.Auth(cfg.ServiceName, login, password)
661+
if err != nil {
670662
if strings.Contains(err.Error(), "Authentication failure") {
671663
return nil, ErrUserNotExist{0, login, 0}
672664
}
@@ -677,14 +669,21 @@ func LoginViaPAM(user *User, login, password string, sourceID int64, cfg *PAMCon
677669
return user, nil
678670
}
679671

672+
// Allow PAM sources with `@` in their name, like from Active Directory
673+
username := pamLogin
674+
idx := strings.Index(pamLogin, "@")
675+
if idx > -1 {
676+
username = pamLogin[:idx]
677+
}
678+
680679
user = &User{
681-
LowerName: strings.ToLower(login),
682-
Name: login,
683-
Email: login,
680+
LowerName: strings.ToLower(username),
681+
Name: username,
682+
Email: pamLogin,
684683
Passwd: password,
685684
LoginType: LoginPAM,
686685
LoginSource: sourceID,
687-
LoginName: login,
686+
LoginName: login, // This is what the user typed in
688687
IsActive: true,
689688
}
690689
return user, CreateUser(user)

models/user.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"image/png"
1919
"os"
2020
"path/filepath"
21+
"regexp"
2122
"strconv"
2223
"strings"
2324
"time"
@@ -87,6 +88,9 @@ var (
8788

8889
// ErrUnsupportedLoginType login source is unknown error
8990
ErrUnsupportedLoginType = errors.New("Login source is unknown")
91+
92+
// Characters prohibited in a user name (anything except A-Za-z0-9_.-)
93+
alphaDashDotPattern = regexp.MustCompile(`[^\w-\.]`)
9094
)
9195

9296
// User represents the object of individual and member of organization.
@@ -906,6 +910,11 @@ func isUsableName(names, patterns []string, name string) error {
906910

907911
// IsUsableUsername returns an error when a username is reserved
908912
func IsUsableUsername(name string) error {
913+
// Validate username make sure it satisfies requirement.
914+
if alphaDashDotPattern.MatchString(name) {
915+
// Note: usually this error is normally caught up earlier in the UI
916+
return ErrNameCharsNotAllowed{Name: name}
917+
}
909918
return isUsableName(reservedUsernames, reservedUserPatterns, name)
910919
}
911920

modules/auth/pam/pam.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import (
1313
)
1414

1515
// Auth pam auth service
16-
func Auth(serviceName, userName, passwd string) error {
16+
func Auth(serviceName, userName, passwd string) (string, error) {
1717
t, err := pam.StartFunc(serviceName, userName, func(s pam.Style, msg string) (string, error) {
1818
switch s {
1919
case pam.PromptEchoOff:
@@ -25,12 +25,14 @@ func Auth(serviceName, userName, passwd string) error {
2525
})
2626

2727
if err != nil {
28-
return err
28+
return "", err
2929
}
3030

3131
if err = t.Authenticate(0); err != nil {
32-
return err
32+
return "", err
3333
}
3434

35-
return nil
35+
// PAM login names might suffer transformations in the PAM stack.
36+
// We should take whatever the PAM stack returns for it.
37+
return t.GetItem(pam.User)
3638
}

modules/auth/pam/pam_stub.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,6 @@ import (
1111
)
1212

1313
// Auth not supported lack of pam tag
14-
func Auth(serviceName, userName, passwd string) error {
15-
return errors.New("PAM not supported")
14+
func Auth(serviceName, userName, passwd string) (string, error) {
15+
return "", errors.New("PAM not supported")
1616
}

modules/repository/create_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ func TestIncludesAllRepositoriesTeams(t *testing.T) {
3535

3636
// Create org.
3737
org := &models.User{
38-
Name: "All repo",
38+
Name: "All_repo",
3939
IsActive: true,
4040
Type: models.UserTypeOrganization,
4141
Visibility: structs.VisibleTypePublic,

options/locale/locale_en-US.ini

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,7 @@ user_bio = Biography
379379
380380
form.name_reserved = The username '%s' is reserved.
381381
form.name_pattern_not_allowed = The pattern '%s' is not allowed in a username.
382+
form.name_chars_not_allowed = User name '%s' contains invalid characters.
382383
383384
[settings]
384385
profile = Profile

routers/admin/users.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,9 @@ func NewUserPost(ctx *context.Context, form auth.AdminCreateUserForm) {
124124
case models.IsErrNamePatternNotAllowed(err):
125125
ctx.Data["Err_UserName"] = true
126126
ctx.RenderWithErr(ctx.Tr("user.form.name_pattern_not_allowed", err.(models.ErrNamePatternNotAllowed).Pattern), tplUserNew, &form)
127+
case models.IsErrNameCharsNotAllowed(err):
128+
ctx.Data["Err_UserName"] = true
129+
ctx.RenderWithErr(ctx.Tr("user.form.name_chars_not_allowed", err.(models.ErrNameCharsNotAllowed).Name), tplUserNew, &form)
127130
default:
128131
ctx.ServerError("CreateUser", err)
129132
}

routers/api/v1/admin/org.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ func CreateOrg(ctx *context.APIContext, form api.CreateOrgOption) {
6767
if err := models.CreateOrganization(org, u); err != nil {
6868
if models.IsErrUserAlreadyExist(err) ||
6969
models.IsErrNameReserved(err) ||
70+
models.IsErrNameCharsNotAllowed(err) ||
7071
models.IsErrNamePatternNotAllowed(err) {
7172
ctx.Error(http.StatusUnprocessableEntity, "", err)
7273
} else {

routers/api/v1/admin/user.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ func CreateUser(ctx *context.APIContext, form api.CreateUserOption) {
9191
if models.IsErrUserAlreadyExist(err) ||
9292
models.IsErrEmailAlreadyUsed(err) ||
9393
models.IsErrNameReserved(err) ||
94+
models.IsErrNameCharsNotAllowed(err) ||
9495
models.IsErrNamePatternNotAllowed(err) {
9596
ctx.Error(http.StatusUnprocessableEntity, "", err)
9697
} else {

routers/api/v1/org/org.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ func Create(ctx *context.APIContext, form api.CreateOrgOption) {
179179
if err := models.CreateOrganization(org, ctx.User); err != nil {
180180
if models.IsErrUserAlreadyExist(err) ||
181181
models.IsErrNameReserved(err) ||
182+
models.IsErrNameCharsNotAllowed(err) ||
182183
models.IsErrNamePatternNotAllowed(err) {
183184
ctx.Error(http.StatusUnprocessableEntity, "", err)
184185
} else {

routers/api/v1/repo/migrate.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,8 @@ func handleMigrateError(ctx *context.APIContext, repoOwner *models.User, remoteA
199199
ctx.Error(http.StatusUnprocessableEntity, "", fmt.Sprintf("You have already reached your limit of %d repositories.", repoOwner.MaxCreationLimit()))
200200
case models.IsErrNameReserved(err):
201201
ctx.Error(http.StatusUnprocessableEntity, "", fmt.Sprintf("The username '%s' is reserved.", err.(models.ErrNameReserved).Name))
202+
case models.IsErrNameCharsNotAllowed(err):
203+
ctx.Error(http.StatusUnprocessableEntity, "", fmt.Sprintf("The username '%s' contains invalid characters.", err.(models.ErrNameCharsNotAllowed).Name))
202204
case models.IsErrNamePatternNotAllowed(err):
203205
ctx.Error(http.StatusUnprocessableEntity, "", fmt.Sprintf("The pattern '%s' is not allowed in a username.", err.(models.ErrNamePatternNotAllowed).Pattern))
204206
default:

routers/user/auth.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -928,6 +928,7 @@ func LinkAccountPostRegister(ctx *context.Context, cpt *captcha.Captcha, form au
928928
LoginName: gothUser.(goth.User).UserID,
929929
}
930930

931+
//nolint: dupl
931932
if err := models.CreateUser(u); err != nil {
932933
switch {
933934
case models.IsErrUserAlreadyExist(err):
@@ -942,6 +943,9 @@ func LinkAccountPostRegister(ctx *context.Context, cpt *captcha.Captcha, form au
942943
case models.IsErrNamePatternNotAllowed(err):
943944
ctx.Data["Err_UserName"] = true
944945
ctx.RenderWithErr(ctx.Tr("user.form.name_pattern_not_allowed", err.(models.ErrNamePatternNotAllowed).Pattern), tplLinkAccount, &form)
946+
case models.IsErrNameCharsNotAllowed(err):
947+
ctx.Data["Err_UserName"] = true
948+
ctx.RenderWithErr(ctx.Tr("user.form.name_chars_not_allowed", err.(models.ErrNameCharsNotAllowed).Name), tplLinkAccount, &form)
945949
default:
946950
ctx.ServerError("CreateUser", err)
947951
}

routers/user/auth_openid.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,7 @@ func RegisterOpenIDPost(ctx *context.Context, cpt *captcha.Captcha, form auth.Si
400400
Passwd: password,
401401
IsActive: !setting.Service.RegisterEmailConfirm,
402402
}
403+
//nolint: dupl
403404
if err := models.CreateUser(u); err != nil {
404405
switch {
405406
case models.IsErrUserAlreadyExist(err):
@@ -414,6 +415,9 @@ func RegisterOpenIDPost(ctx *context.Context, cpt *captcha.Captcha, form auth.Si
414415
case models.IsErrNamePatternNotAllowed(err):
415416
ctx.Data["Err_UserName"] = true
416417
ctx.RenderWithErr(ctx.Tr("user.form.name_pattern_not_allowed", err.(models.ErrNamePatternNotAllowed).Pattern), tplSignUpOID, &form)
418+
case models.IsErrNameCharsNotAllowed(err):
419+
ctx.Data["Err_UserName"] = true
420+
ctx.RenderWithErr(ctx.Tr("user.form.name_chars_not_allowed", err.(models.ErrNameCharsNotAllowed).Name), tplSignUpOID, &form)
417421
default:
418422
ctx.ServerError("CreateUser", err)
419423
}

routers/user/setting/profile.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ func handleUsernameChange(ctx *context.Context, newName string) {
5858
case models.IsErrNamePatternNotAllowed(err):
5959
ctx.Flash.Error(ctx.Tr("user.form.name_pattern_not_allowed", newName))
6060
ctx.Redirect(setting.AppSubURL + "/user/settings")
61+
case models.IsErrNameCharsNotAllowed(err):
62+
ctx.Flash.Error(ctx.Tr("user.form.name_chars_not_allowed", newName))
63+
ctx.Redirect(setting.AppSubURL + "/user/settings")
6164
default:
6265
ctx.ServerError("ChangeUserName", err)
6366
}

0 commit comments

Comments
 (0)