Skip to content

Commit

Permalink
Auth: Normalize Username (trim space, lowercase) #1103 #1309 #1546 #1610
Browse files Browse the repository at this point in the history
  • Loading branch information
lastzero committed Nov 12, 2021
1 parent 666a235 commit a354a17
Show file tree
Hide file tree
Showing 11 changed files with 76 additions and 36 deletions.
4 changes: 2 additions & 2 deletions internal/commands/passwd.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func passwdAction(ctx *cli.Context) error {

user := entity.Admin

log.Infof("please enter a new password for %s (at least 6 characters)\n", txt.Quote(user.UserName))
log.Infof("please enter a new password for %s (at least 6 characters)\n", txt.Quote(user.Username()))

newPassword := getPassword("New Password: ")

Expand All @@ -57,7 +57,7 @@ func passwdAction(ctx *cli.Context) error {
return err
}

log.Infof("changed password for %s\n", txt.Quote(user.UserName))
log.Infof("changed password for %s\n", txt.Quote(user.Username()))

conf.Shutdown()

Expand Down
6 changes: 3 additions & 3 deletions internal/commands/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ func usersListAction(ctx *cli.Context) error {
fmt.Printf("%-4s %-16s %-16s %-16s\n", "ID", "LOGIN", "NAME", "EMAIL")

for _, user := range users {
fmt.Printf("%-4d %-16s %-16s %-16s", user.ID, user.UserName, user.FullName, user.PrimaryEmail)
fmt.Printf("%-4d %-16s %-16s %-16s", user.ID, user.Username(), user.FullName, user.PrimaryEmail)
fmt.Printf("\n")
}

Expand Down Expand Up @@ -242,7 +242,7 @@ func usersUpdateAction(ctx *cli.Context) error {
if err != nil {
return err
}
fmt.Printf("password successfully changed: %v\n", u.UserName)
fmt.Printf("password successfully changed: %v\n", u.Username())
}

if ctx.IsSet("fullname") {
Expand All @@ -261,7 +261,7 @@ func usersUpdateAction(ctx *cli.Context) error {
return err
}

fmt.Printf("user successfully updated: %v\n", u.UserName)
fmt.Printf("user successfully updated: %v\n", u.Username())

return nil
})
Expand Down
35 changes: 21 additions & 14 deletions internal/entity/cell.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,34 +79,41 @@ func (m *Cell) Refresh(api string) (err error) {
return nil
}

place := &Place{}
cellTable := Cell{}.TableName()
placeTable := Place{}.TableName()

place := Place{}

// Find existing place by label.
if err := UnscopedDb().Where("place_label = ?", l.Label()).First(&place).Error; err != nil {
log.Tracef("places: %s for cell %s", err, m.ID)
place = &Place{ID: m.ID}
place = Place{ID: m.ID}
} else {
log.Tracef("places: found matching place %s for cell %s", place.ID, m.ID)
}

// Update place.
if res := UnscopedDb().Model(place).Updates(Values{
"PlaceLabel": l.Label(),
"PlaceCity": l.City(),
"PlaceDistrict": l.District(),
"PlaceState": l.State(),
"PlaceCountry": l.CountryCode(),
"PlaceKeywords": l.KeywordString(),
}); res.Error == nil && res.RowsAffected == 1 {
if place.ID == "" {
// Do nothing.
} else if res := UnscopedDb().Table(placeTable).Where("id = ?", place.ID).UpdateColumns(Values{
"place_label": l.Label(),
"place_city": l.City(),
"place_district": l.District(),
"place_state": l.State(),
"place_country": l.CountryCode(),
"place_keywords": l.KeywordString(),
}); res.Error != nil {
log.Tracef("places: %s for cell %s", err, m.ID)
} else if res.RowsAffected > 0 {
// Update cell place id, name, and category.
log.Tracef("places: updating place, name, and category for cell %s", m.ID)
m.PlaceID = place.ID
err = UnscopedDb().Model(m).Updates(Values{"PlaceID": m.PlaceID, "CellName": l.Name(), "CellCategory": l.Category()}).Error

err = UnscopedDb().Table(cellTable).Where("id = ?", m.ID).
UpdateColumns(Values{"cell_name": l.Name(), "cell_category": l.Category(), "place_id": place.ID}).Error
} else {
// Update cell name and category.
log.Tracef("places: updating name and category for cell %s", m.ID)
err = UnscopedDb().Model(m).Updates(Values{"CellName": l.Name(), "CellCategory": l.Category()}).Error
err = UnscopedDb().Table(cellTable).Where("id = ?", m.ID).
UpdateColumns(Values{"cell_name": l.Name(), "cell_category": l.Category()}).Error
}

log.Debugf("places: refreshed cell %s [%s]", txt.Quote(m.ID), time.Since(start))
Expand Down
32 changes: 23 additions & 9 deletions internal/entity/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,8 @@ func FirstOrCreateUser(m *User) *User {

// FindUserByName returns an existing user or nil if not found.
func FindUserByName(userName string) *User {
userName = txt.NormalizeUsername(userName)

if userName == "" {
return nil
}
Expand Down Expand Up @@ -211,8 +213,8 @@ func (m *User) Deleted() bool {

// String returns an identifier that can be used in logs.
func (m *User) String() string {
if m.UserName != "" {
return m.UserName
if n := m.Username(); n != "" {
return n
}

if m.FullName != "" {
Expand All @@ -222,9 +224,14 @@ func (m *User) String() string {
return m.UserUID
}

// Username returns the normalized username.
func (m *User) Username() string {
return txt.NormalizeUsername(m.UserName)
}

// Registered tests if the user is registered e.g. has a username.
func (m *User) Registered() bool {
return m.UserName != "" && rnd.IsPPID(m.UserUID, 'u')
return m.Username() != "" && rnd.IsPPID(m.UserUID, 'u')
}

// Admin returns true if the user is an admin with user name.
Expand All @@ -249,7 +256,7 @@ func (m *User) SetPassword(password string) error {
}

if len(password) < 4 {
return fmt.Errorf("new password for %s must be at least 4 characters", txt.Quote(m.UserName))
return fmt.Errorf("new password for %s must be at least 4 characters", txt.Quote(m.Username()))
}

pw := NewPassword(m.UserUID, password)
Expand Down Expand Up @@ -342,30 +349,37 @@ func (m *User) Role() acl.Role {

// Validate Makes sure username and email are unique and meet requirements. Returns error if any property is invalid
func (m *User) Validate() error {
if m.UserName == "" {
if m.Username() == "" {
return errors.New("username must not be empty")
}
if len(m.UserName) < 4 {

if len(m.Username()) < 4 {
return errors.New("username must be at least 4 characters")
}

var err error
var resultName = User{}
if err = Db().Where("user_name = ? AND id <> ?", m.UserName, m.ID).First(&resultName).Error; err == nil {

if err = Db().Where("user_name = ? AND id <> ?", m.Username(), m.ID).First(&resultName).Error; err == nil {
return errors.New("username already exists")
} else if err != gorm.ErrRecordNotFound {
return err
}

// stop here if no email is provided
if m.PrimaryEmail == "" {
return nil
}

// validate email address
if a, err := mail.ParseAddress(m.PrimaryEmail); err != nil {
return err
} else {
m.PrimaryEmail = a.Address // make sure email address will be used without name
}

var resultMail = User{}

if err = Db().Where("primary_email = ? AND id <> ?", m.PrimaryEmail, m.ID).First(&resultMail).Error; err == nil {
return errors.New("email already exists")
} else if err != gorm.ErrRecordNotFound {
Expand All @@ -384,7 +398,7 @@ func CreateWithPassword(uc form.UserCreate) error {
RoleAdmin: true,
}
if len(uc.Password) < 4 {
return fmt.Errorf("new password for %s must be at least 4 characters", txt.Quote(u.UserName))
return fmt.Errorf("new password for %s must be at least 4 characters", txt.Quote(u.Username()))
}
err := u.Validate()
if err != nil {
Expand All @@ -398,7 +412,7 @@ func CreateWithPassword(uc form.UserCreate) error {
if err := tx.Create(&pw).Error; err != nil {
return err
}
log.Infof("created user %v with uid %v", txt.Quote(u.UserName), txt.Quote(u.UserUID))
log.Infof("created user %v with uid %v", txt.Quote(u.Username()), txt.Quote(u.UserUID))
return nil
})
}
2 changes: 2 additions & 0 deletions internal/entity/user_fixtures_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@ func TestUserMap_Get(t *testing.T) {
t.Run("get existing user", func(t *testing.T) {
r := UserFixtures.Get("alice")
assert.Equal(t, "alice", r.UserName)
assert.Equal(t, "alice", r.Username())
assert.IsType(t, User{}, r)
})
t.Run("get not existing user", func(t *testing.T) {
r := UserFixtures.Get("monstera")
assert.Equal(t, "", r.UserName)
assert.Equal(t, "", r.Username())
assert.IsType(t, User{}, r)
})
}
Expand Down
16 changes: 10 additions & 6 deletions internal/entity/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ func TestFindUserByName(t *testing.T) {
assert.Equal(t, 1, m.ID)
assert.NotEmpty(t, m.UserUID)
assert.Equal(t, "admin", m.UserName)
assert.Equal(t, "admin", m.Username())
m.UserName = "Admin "
assert.Equal(t, "admin", m.Username())
assert.Equal(t, "Admin ", m.UserName)
assert.Equal(t, "Admin", m.FullName)
assert.True(t, m.RoleAdmin)
assert.False(t, m.RoleGuest)
Expand Down Expand Up @@ -203,7 +207,7 @@ func TestFindUserByUID(t *testing.T) {

assert.Equal(t, 5, m.ID)
assert.Equal(t, "uqxetse3cy5eo9z2", m.UserUID)
assert.Equal(t, "alice", m.UserName)
assert.Equal(t, "alice", m.Username())
assert.Equal(t, "Alice", m.FullName)
assert.Equal(t, "alice@example.com", m.PrimaryEmail)
assert.True(t, m.RoleAdmin)
Expand Down Expand Up @@ -251,17 +255,17 @@ func TestFindUserByUID(t *testing.T) {
}

func TestUser_String(t *testing.T) {
t.Run("return UID", func(t *testing.T) {
t.Run("UID", func(t *testing.T) {
p := User{UserUID: "abc123", UserName: "", FullName: ""}
assert.Equal(t, "abc123", p.String())
})
t.Run("return display name", func(t *testing.T) {
t.Run("DisplayName", func(t *testing.T) {
p := User{UserUID: "abc123", UserName: "", FullName: "Test"}
assert.Equal(t, "Test", p.String())
})
t.Run("return user name", func(t *testing.T) {
p := User{UserUID: "abc123", UserName: "Super-User", FullName: "Test"}
assert.Equal(t, "Super-User", p.String())
t.Run("UserName", func(t *testing.T) {
p := User{UserUID: "abc123", UserName: "Super-User ", FullName: "Test"}
assert.Equal(t, "super-user", p.String())
})
}

Expand Down
7 changes: 7 additions & 0 deletions internal/form/user.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
package form

import "github.com/photoprism/photoprism/pkg/txt"

// UserCreate represents a User with a new password.
type UserCreate struct {
UserName string `json:"username"`
FullName string `json:"fullname"`
Email string `json:"email"`
Password string `json:"password"`
}

// Username returns the normalized username in lowercase and without whitespace padding.
func (f UserCreate) Username() string {
return txt.NormalizeUsername(f.UserName)
}
2 changes: 1 addition & 1 deletion internal/photoprism/places.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func (w *Places) Start() (updated []string, err error) {
}

// Short break.
time.Sleep(100 * time.Millisecond)
time.Sleep(25 * time.Millisecond)
}

return updated, err
Expand Down
2 changes: 1 addition & 1 deletion internal/query/users_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ func TestRegisteredUsers(t *testing.T) {
users := RegisteredUsers()

for _, user := range users {
t.Logf("user: %v, %s, %s, %s", user.ID, user.UserUID, user.UserName, user.FullName)
t.Logf("user: %v, %s, %s, %s", user.ID, user.UserUID, user.Username(), user.FullName)
}

t.Logf("user count: %v", len(users))
Expand Down
1 change: 1 addition & 0 deletions pkg/txt/clip.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const (
Ellipsis = "…"
ClipCountryCode = 2
ClipKeyword = 40
ClipUsername = 64
ClipSlug = 80
ClipCategory = 100
ClipPlace = 128
Expand Down
5 changes: 5 additions & 0 deletions pkg/txt/normalize.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,8 @@ func NormalizeQuery(s string) string {
s = strings.ReplaceAll(s, "%", "*")
return strings.Trim(s, "+&|_-=!@$%^(){}\\<>,.;: ")
}

// NormalizeUsername returns the normalized username (lowercase, whitespace trimmed).
func NormalizeUsername(s string) string {
return strings.ToLower(Clip(s, ClipUsername))
}

0 comments on commit a354a17

Please sign in to comment.