Various fixes in login sources (#10428) (#10429)

Co-authored-by: guillep2k <18600385+guillep2k@users.noreply.github.com>

diff --git a/models/error.go b/models/error.go
index f0d5699aad6b86a8ec26587bddac6f4ca68ae8d5..b9ebab9c6b93840158262b22e53e78b49253d103 100644 (file)
--- 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 f5dae860f8b49fd14fbbba9901fa7248809d6215..2774d6f80d986475ac75071613c48dd5b0142224 100644 (file)
--- 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 b7e2ef113d3d9ebedd7ccdb64303b5bfd1d78e11..0ff7b53b56053248f60cf149ec27d3d1963ad9ea 100644 (file)
--- 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 c515bd222b2b6c5c262837729b1213d73383a904..d8c0fcf47dc658359b6c24d164a0e7b6e6b2652f 100644 (file)
--- 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 6f0f7240ae2a38d427a9e936d25b37b800ad44ac..ca299b08ba0be2244130c54e7cf50058398d49bd 100644 (file)
--- 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 ee2527dd894ceaaa6e75f254f588f3c68cacd79b..604799ca973bceff1991c53a87949b341fd26cb9 100644 (file)
--- 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 5e4d945317e3ed24305ad27f10c41ea19d3d8259..61d3371072b7e0921ea29a10a839bf7bd8a1b275 100644 (file)
--- 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 b5c7dbd38346ba2c177ad98456e2f58b1716c1fe..5fbbdc83fb31988d36cfd73db91356c82fa720e3 100644 (file)
--- 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 1db4e592ff4963936151281250e2451fc47faf61..c75fb62d08f0f62aebcb1073153693b70ea41da0 100644 (file)
--- 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 6df8ad9393f6a2700698cbcd6bbd15a30980acfb..f372c10cfd127dd116c01feb311f65cc69ad7a08 100644 (file)
--- 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 67770e70aa62c175359ba406fb69ac90f4793718..2bcd05179e684599a1e351d1efff1ad12b62ba17 100644 (file)
--- 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 8f34d8cca3f789b8d0b7eee691a49b74abe53776..b7f9a539cb613fc680c896738dc382d22fa7d026 100644 (file)
--- 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 6395836480ab0e6b8e60f2f7ef4dd449ac2d56ba..be0396cce1d157135e03f1f9a7bfdb43500de56d 100644 (file)
--- 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 ccaea8264f087aec96482e6769a60a8c8a809e6b..bd05538ad3b239a51356147dfdcb160c936b021c 100644 (file)
--- 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 6db9fc7c6e7e5d1fac94ffc12ca052e267b049f1..368dc23738d6464f4cfd31d8179c3f58a53c55d6 100644 (file)
--- 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)
                        }