From d3b6f001febb7bbb804adae3f025225a55f1f702 Mon Sep 17 00:00:00 2001
From: zeripath <art27@cantab.net>
Date: Sun, 23 Feb 2020 20:46:17 +0000
Subject: [PATCH] Various fixes in login sources (#10428) (#10429)
Co-authored-by: guillep2k <18600385+guillep2k@users.noreply.github.com>
---
models/error.go | 15 +++++++++++++++
models/login_source.go | 27 +++++++++++++--------------
models/org_team_test.go | 2 +-
models/user.go | 9 +++++++++
modules/auth/pam/pam.go | 10 ++++++----
modules/auth/pam/pam_stub.go | 4 ++--
options/locale/locale_en-US.ini | 1 +
routers/admin/users.go | 3 +++
routers/api/v1/admin/org.go | 1 +
routers/api/v1/admin/user.go | 1 +
routers/api/v1/org/org.go | 1 +
routers/api/v1/repo/repo.go | 2 ++
routers/user/auth.go | 4 ++++
routers/user/auth_openid.go | 4 ++++
routers/user/setting/profile.go | 3 +++
15 files changed, 66 insertions(+), 21 deletions(-)
diff --git a/models/error.go b/models/error.go
index f0d5699aad..b9ebab9c6b 100644
--- a/models/error.go
+++ b/models/error.go
@@ -56,6 +56,21 @@ func (err ErrNamePatternNotAllowed) Error() string {
return fmt.Sprintf("name pattern is not allowed [pattern: %s]", err.Pattern)
}
+// ErrNameCharsNotAllowed represents a "character not allowed in name" error.
+type ErrNameCharsNotAllowed struct {
+ Name string
+}
+
+// IsErrNameCharsNotAllowed checks if an error is an ErrNameCharsNotAllowed.
+func IsErrNameCharsNotAllowed(err error) bool {
+ _, ok := err.(ErrNameCharsNotAllowed)
+ return ok
+}
+
+func (err ErrNameCharsNotAllowed) Error() string {
+ return fmt.Sprintf("User name is invalid [%s]: must be valid alpha or numeric or dash(-_) or dot characters", err.Name)
+}
+
// ErrSSHDisabled represents an "SSH disabled" error.
type ErrSSHDisabled struct {
}
diff --git a/models/login_source.go b/models/login_source.go
index f5dae860f8..2774d6f80d 100644
--- a/models/login_source.go
+++ b/models/login_source.go
@@ -12,7 +12,6 @@ import (
"fmt"
"net/smtp"
"net/textproto"
- "regexp"
"strings"
"code.gitea.io/gitea/modules/auth/ldap"
@@ -455,10 +454,6 @@ func composeFullName(firstname, surname, username string) string {
}
}
-var (
- alphaDashDotPattern = regexp.MustCompile(`[^\w-\.]`)
-)
-
// LoginViaLDAP queries if login/password is valid against the LDAP directory pool,
// and create a local user if success when enabled.
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
if len(sr.Username) == 0 {
sr.Username = login
}
- // Validate username make sure it satisfies requirement.
- if alphaDashDotPattern.MatchString(sr.Username) {
- return nil, fmt.Errorf("Invalid pattern for attribute 'username' [%s]: must be valid alpha or numeric or dash(-_) or dot characters", sr.Username)
- }
if len(sr.Mail) == 0 {
sr.Mail = fmt.Sprintf("%s@localhost", sr.Username)
@@ -666,7 +657,8 @@ func LoginViaSMTP(user *User, login, password string, sourceID int64, cfg *SMTPC
// LoginViaPAM queries if login/password is valid against the PAM,
// and create a local user if success when enabled.
func LoginViaPAM(user *User, login, password string, sourceID int64, cfg *PAMConfig) (*User, error) {
- if err := pam.Auth(cfg.ServiceName, login, password); err != nil {
+ pamLogin, err := pam.Auth(cfg.ServiceName, login, password)
+ if err != nil {
if strings.Contains(err.Error(), "Authentication failure") {
return nil, ErrUserNotExist{0, login, 0}
}
@@ -677,14 +669,21 @@ func LoginViaPAM(user *User, login, password string, sourceID int64, cfg *PAMCon
return user, nil
}
+ // Allow PAM sources with `@` in their name, like from Active Directory
+ username := pamLogin
+ idx := strings.Index(pamLogin, "@")
+ if idx > -1 {
+ username = pamLogin[:idx]
+ }
+
user = &User{
- LowerName: strings.ToLower(login),
- Name: login,
- Email: login,
+ LowerName: strings.ToLower(username),
+ Name: username,
+ Email: pamLogin,
Passwd: password,
LoginType: LoginPAM,
LoginSource: sourceID,
- LoginName: login,
+ LoginName: login, // This is what the user typed in
IsActive: true,
}
return user, CreateUser(user)
diff --git a/models/org_team_test.go b/models/org_team_test.go
index b7e2ef113d..0ff7b53b56 100644
--- a/models/org_team_test.go
+++ b/models/org_team_test.go
@@ -399,7 +399,7 @@ func TestIncludesAllRepositoriesTeams(t *testing.T) {
// Create org.
org := &User{
- Name: "All repo",
+ Name: "All_repo",
IsActive: true,
Type: UserTypeOrganization,
Visibility: structs.VisibleTypePublic,
diff --git a/models/user.go b/models/user.go
index c515bd222b..d8c0fcf47d 100644
--- a/models/user.go
+++ b/models/user.go
@@ -18,6 +18,7 @@ import (
"image/png"
"os"
"path/filepath"
+ "regexp"
"strconv"
"strings"
"time"
@@ -87,6 +88,9 @@ var (
// ErrUnsupportedLoginType login source is unknown error
ErrUnsupportedLoginType = errors.New("Login source is unknown")
+
+ // Characters prohibited in a user name (anything except A-Za-z0-9_.-)
+ alphaDashDotPattern = regexp.MustCompile(`[^\w-\.]`)
)
// User represents the object of individual and member of organization.
@@ -870,6 +874,11 @@ func isUsableName(names, patterns []string, name string) error {
// IsUsableUsername returns an error when a username is reserved
func IsUsableUsername(name string) error {
+ // Validate username make sure it satisfies requirement.
+ if alphaDashDotPattern.MatchString(name) {
+ // Note: usually this error is normally caught up earlier in the UI
+ return ErrNameCharsNotAllowed{Name: name}
+ }
return isUsableName(reservedUsernames, reservedUserPatterns, name)
}
diff --git a/modules/auth/pam/pam.go b/modules/auth/pam/pam.go
index 6f0f7240ae..ca299b08ba 100644
--- a/modules/auth/pam/pam.go
+++ b/modules/auth/pam/pam.go
@@ -13,7 +13,7 @@ import (
)
// Auth pam auth service
-func Auth(serviceName, userName, passwd string) error {
+func Auth(serviceName, userName, passwd string) (string, error) {
t, err := pam.StartFunc(serviceName, userName, func(s pam.Style, msg string) (string, error) {
switch s {
case pam.PromptEchoOff:
@@ -25,12 +25,14 @@ func Auth(serviceName, userName, passwd string) error {
})
if err != nil {
- return err
+ return "", err
}
if err = t.Authenticate(0); err != nil {
- return err
+ return "", err
}
- return nil
+ // PAM login names might suffer transformations in the PAM stack.
+ // We should take whatever the PAM stack returns for it.
+ return t.GetItem(pam.User)
}
diff --git a/modules/auth/pam/pam_stub.go b/modules/auth/pam/pam_stub.go
index ee2527dd89..604799ca97 100644
--- a/modules/auth/pam/pam_stub.go
+++ b/modules/auth/pam/pam_stub.go
@@ -11,6 +11,6 @@ import (
)
// Auth not supported lack of pam tag
-func Auth(serviceName, userName, passwd string) error {
- return errors.New("PAM not supported")
+func Auth(serviceName, userName, passwd string) (string, error) {
+ return "", errors.New("PAM not supported")
}
diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
index 5e4d945317..61d3371072 100644
--- a/options/locale/locale_en-US.ini
+++ b/options/locale/locale_en-US.ini
@@ -374,6 +374,7 @@ user_bio = Biography
form.name_reserved = The username '%s' is reserved.
form.name_pattern_not_allowed = The pattern '%s' is not allowed in a username.
+form.name_chars_not_allowed = User name '%s' contains invalid characters.
[settings]
profile = Profile
diff --git a/routers/admin/users.go b/routers/admin/users.go
index b5c7dbd383..5fbbdc83fb 100644
--- a/routers/admin/users.go
+++ b/routers/admin/users.go
@@ -121,6 +121,9 @@ func NewUserPost(ctx *context.Context, form auth.AdminCreateUserForm) {
case models.IsErrNamePatternNotAllowed(err):
ctx.Data["Err_UserName"] = true
ctx.RenderWithErr(ctx.Tr("user.form.name_pattern_not_allowed", err.(models.ErrNamePatternNotAllowed).Pattern), tplUserNew, &form)
+ case models.IsErrNameCharsNotAllowed(err):
+ ctx.Data["Err_UserName"] = true
+ ctx.RenderWithErr(ctx.Tr("user.form.name_chars_not_allowed", err.(models.ErrNameCharsNotAllowed).Name), tplUserNew, &form)
default:
ctx.ServerError("CreateUser", err)
}
diff --git a/routers/api/v1/admin/org.go b/routers/api/v1/admin/org.go
index 1db4e592ff..c75fb62d08 100644
--- a/routers/api/v1/admin/org.go
+++ b/routers/api/v1/admin/org.go
@@ -66,6 +66,7 @@ func CreateOrg(ctx *context.APIContext, form api.CreateOrgOption) {
if err := models.CreateOrganization(org, u); err != nil {
if models.IsErrUserAlreadyExist(err) ||
models.IsErrNameReserved(err) ||
+ models.IsErrNameCharsNotAllowed(err) ||
models.IsErrNamePatternNotAllowed(err) {
ctx.Error(http.StatusUnprocessableEntity, "", err)
} else {
diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go
index 6df8ad9393..f372c10cfd 100644
--- a/routers/api/v1/admin/user.go
+++ b/routers/api/v1/admin/user.go
@@ -90,6 +90,7 @@ func CreateUser(ctx *context.APIContext, form api.CreateUserOption) {
if models.IsErrUserAlreadyExist(err) ||
models.IsErrEmailAlreadyUsed(err) ||
models.IsErrNameReserved(err) ||
+ models.IsErrNameCharsNotAllowed(err) ||
models.IsErrNamePatternNotAllowed(err) {
ctx.Error(http.StatusUnprocessableEntity, "", err)
} else {
diff --git a/routers/api/v1/org/org.go b/routers/api/v1/org/org.go
index 67770e70aa..2bcd05179e 100644
--- a/routers/api/v1/org/org.go
+++ b/routers/api/v1/org/org.go
@@ -112,6 +112,7 @@ func Create(ctx *context.APIContext, form api.CreateOrgOption) {
if err := models.CreateOrganization(org, ctx.User); err != nil {
if models.IsErrUserAlreadyExist(err) ||
models.IsErrNameReserved(err) ||
+ models.IsErrNameCharsNotAllowed(err) ||
models.IsErrNamePatternNotAllowed(err) {
ctx.Error(http.StatusUnprocessableEntity, "", err)
} else {
diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go
index 8f34d8cca3..b7f9a539cb 100644
--- a/routers/api/v1/repo/repo.go
+++ b/routers/api/v1/repo/repo.go
@@ -511,6 +511,8 @@ func handleMigrateError(ctx *context.APIContext, repoOwner *models.User, remoteA
ctx.Error(http.StatusUnprocessableEntity, "", fmt.Sprintf("You have already reached your limit of %d repositories.", repoOwner.MaxCreationLimit()))
case models.IsErrNameReserved(err):
ctx.Error(http.StatusUnprocessableEntity, "", fmt.Sprintf("The username '%s' is reserved.", err.(models.ErrNameReserved).Name))
+ case models.IsErrNameCharsNotAllowed(err):
+ ctx.Error(http.StatusUnprocessableEntity, "", fmt.Sprintf("The username '%s' contains invalid characters.", err.(models.ErrNameCharsNotAllowed).Name))
case models.IsErrNamePatternNotAllowed(err):
ctx.Error(http.StatusUnprocessableEntity, "", fmt.Sprintf("The pattern '%s' is not allowed in a username.", err.(models.ErrNamePatternNotAllowed).Pattern))
default:
diff --git a/routers/user/auth.go b/routers/user/auth.go
index 6395836480..be0396cce1 100644
--- a/routers/user/auth.go
+++ b/routers/user/auth.go
@@ -928,6 +928,7 @@ func LinkAccountPostRegister(ctx *context.Context, cpt *captcha.Captcha, form au
LoginName: gothUser.(goth.User).UserID,
}
+ //nolint: dupl
if err := models.CreateUser(u); err != nil {
switch {
case models.IsErrUserAlreadyExist(err):
@@ -942,6 +943,9 @@ func LinkAccountPostRegister(ctx *context.Context, cpt *captcha.Captcha, form au
case models.IsErrNamePatternNotAllowed(err):
ctx.Data["Err_UserName"] = true
ctx.RenderWithErr(ctx.Tr("user.form.name_pattern_not_allowed", err.(models.ErrNamePatternNotAllowed).Pattern), tplLinkAccount, &form)
+ case models.IsErrNameCharsNotAllowed(err):
+ ctx.Data["Err_UserName"] = true
+ ctx.RenderWithErr(ctx.Tr("user.form.name_chars_not_allowed", err.(models.ErrNameCharsNotAllowed).Name), tplLinkAccount, &form)
default:
ctx.ServerError("CreateUser", err)
}
diff --git a/routers/user/auth_openid.go b/routers/user/auth_openid.go
index ccaea8264f..bd05538ad3 100644
--- a/routers/user/auth_openid.go
+++ b/routers/user/auth_openid.go
@@ -400,6 +400,7 @@ func RegisterOpenIDPost(ctx *context.Context, cpt *captcha.Captcha, form auth.Si
Passwd: password,
IsActive: !setting.Service.RegisterEmailConfirm,
}
+ //nolint: dupl
if err := models.CreateUser(u); err != nil {
switch {
case models.IsErrUserAlreadyExist(err):
@@ -414,6 +415,9 @@ func RegisterOpenIDPost(ctx *context.Context, cpt *captcha.Captcha, form auth.Si
case models.IsErrNamePatternNotAllowed(err):
ctx.Data["Err_UserName"] = true
ctx.RenderWithErr(ctx.Tr("user.form.name_pattern_not_allowed", err.(models.ErrNamePatternNotAllowed).Pattern), tplSignUpOID, &form)
+ case models.IsErrNameCharsNotAllowed(err):
+ ctx.Data["Err_UserName"] = true
+ ctx.RenderWithErr(ctx.Tr("user.form.name_chars_not_allowed", err.(models.ErrNameCharsNotAllowed).Name), tplSignUpOID, &form)
default:
ctx.ServerError("CreateUser", err)
}
diff --git a/routers/user/setting/profile.go b/routers/user/setting/profile.go
index 6db9fc7c6e..368dc23738 100644
--- a/routers/user/setting/profile.go
+++ b/routers/user/setting/profile.go
@@ -58,6 +58,9 @@ func handleUsernameChange(ctx *context.Context, newName string) {
case models.IsErrNamePatternNotAllowed(err):
ctx.Flash.Error(ctx.Tr("user.form.name_pattern_not_allowed", newName))
ctx.Redirect(setting.AppSubURL + "/user/settings")
+ case models.IsErrNameCharsNotAllowed(err):
+ ctx.Flash.Error(ctx.Tr("user.form.name_chars_not_allowed", newName))
+ ctx.Redirect(setting.AppSubURL + "/user/settings")
default:
ctx.ServerError("ChangeUserName", err)
}