sevenhill/QSfera
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

prevent duplicate accounts and validate account update

Browse Source
This commit is contained in:
Benedikt Kulmann
2020-10-15 22:34:00 +02:00
parent f53e022041
commit 5f6b36c1db
3 changed files with 109 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files
accounts/pkg/indexer/indexer.go
+14 -2
View File
@@ -191,11 +191,23 @@ func (i Indexer) Update(from, to interface{}) error {
oldV := valueOf(from, fName)
newV := valueOf(to, fName)
pkVal := valueOf(from, fields.PKFieldName)
for _, index := range indices {
for _, idx := range indices {
if oldV == newV {
continue
}
if err := index.Update(pkVal, oldV, newV); err != nil {
if oldV == "" {
if _, err := idx.Add(pkVal, newV); err != nil {
return err
}
continue
}
if newV == "" {
if err := idx.Remove(pkVal, oldV); err != nil {
return err
}
continue
}
if err := idx.Update(pkVal, oldV, newV); err != nil {
return err
}
}
accounts/pkg/proto/v0/accounts.pb.micro_test.go
+31 -24
View File
@@ -74,11 +74,11 @@ func getAccount(user string) *proto.Account {
IsResourceAccount: true,
CreationType: "",
DisplayName: "User One",
PreferredName: "user1",
OnPremisesSamAccountName: "user1",
PreferredName: user,
OnPremisesSamAccountName: user,
UidNumber: 20009,
GidNumber: 30000,
Mail: "user1@example.com",
Mail: fmt.Sprintf("%s@example.com", user),
Identities: []*proto.Identities{nil},
PasswordProfile: &proto.PasswordProfile{Password: "heysdjfsdlk"},
MemberOf: []*proto.Group{
@@ -92,11 +92,11 @@ func getAccount(user string) *proto.Account {
IsResourceAccount: true,
CreationType: "",
DisplayName: "User Two",
PreferredName: "user2",
OnPremisesSamAccountName: "user2",
PreferredName: user,
OnPremisesSamAccountName: user,
UidNumber: 20010,
GidNumber: 30000,
Mail: "user2@example.com",
Mail: fmt.Sprintf("%s@example.com", user),
Identities: []*proto.Identities{nil},
PasswordProfile: &proto.PasswordProfile{Password: "hello123"},
MemberOf: []*proto.Group{
@@ -392,7 +392,7 @@ func deleteGroup(t *testing.T, id string) (*empty.Empty, error) {
// createTmpDir creates a temporary dir for tests data.
func createTmpDir() string {
name, err := ioutil.TempDir("/var/tmp", "ocis-accounts-store-*")
name, err := ioutil.TempDir("/var/tmp", "ocis-accounts-store-")
if err != nil {
panic(err)
}
@@ -468,30 +468,33 @@ func TestCreateAccountInvalidUserName(t *testing.T) {
func TestUpdateAccount(t *testing.T) {
tests := []struct {
name string
userAccount *proto.Account
name string
userAccount *proto.Account
expectedErrOnUpdate error
}{
{
"Update user (demonstration of updatable fields)",
&proto.Account{
DisplayName: "Alice Hansen",
PreferredName: "Wonderful Alice",
PreferredName: "Wonderful-Alice",
OnPremisesSamAccountName: "Alice",
UidNumber: 20010,
GidNumber: 30001,
Mail: "alice@example.com",
},
nil,
},
{
"Update user with unicode data",
&proto.Account{
DisplayName: "एलिस हेन्सेन",
PreferredName: "अद्भुत एलिस",
PreferredName: "अद्भुत-एलिस",
OnPremisesSamAccountName: "एलिस",
UidNumber: 20010,
GidNumber: 30001,
Mail: "एलिस@उदाहरण.com",
},
merrors.BadRequest(".", "preferred_name 'अद्भुत-एलिस' must be at least the local part of an email"),
},
{
"Update user with empty data values",
@@ -503,19 +506,19 @@ func TestUpdateAccount(t *testing.T) {
GidNumber: 0,
Mail: "",
},
merrors.BadRequest(".", "preferred_name '' must be at least the local part of an email"),
},
{
"Update user with strange data",
&proto.Account{
DisplayName: "12345",
PreferredName: "12345",
PreferredName: "a12345",
OnPremisesSamAccountName: "54321",
UidNumber: 1000,
GidNumber: 1000,
// No email validation
// https://github.com/owncloud/ocis/accounts/issues/77
Mail: "1.2@3.c_@",
Mail: "1.2@3.c_@",
},
merrors.BadRequest(".", "mail '1.2@3.c_@' must be a valid email"),
},
}
@@ -533,18 +536,22 @@ func TestUpdateAccount(t *testing.T) {
}
t.Run(tt.name, func(t *testing.T) {
_, _ = createAccount(t, "user1")
tt.userAccount.Id = "f9149a32-2b8e-4f04-9e8d-937d81712b9a"
acc, err := createAccount(t, "user1")
assert.NoError(t, err)
tt.userAccount.Id = acc.Id
tt.userAccount.AccountEnabled = false
tt.userAccount.IsResourceAccount = false
resp, err := updateAccount(t, tt.userAccount, updateMask)
assert.NoError(t, err)
assert.IsType(t, &proto.Account{}, resp)
assertAccountsSame(t, tt.userAccount, resp)
assertUserExists(t, tt.userAccount)
_, _ = deleteAccount(t, "f9149a32-2b8e-4f04-9e8d-937d81712b9a")
if tt.expectedErrOnUpdate != nil {
assert.Error(t, err)
assert.Equal(t, tt.expectedErrOnUpdate, err)
} else {
assert.NoError(t, err)
assert.IsType(t, &proto.Account{}, resp)
assertAccountsSame(t, tt.userAccount, resp)
assertUserExists(t, tt.userAccount)
}
cleanUp(t)
})
}
accounts/pkg/service/v0/accounts.go
+64 -11
View File
@@ -4,7 +4,6 @@ import (
"context"
"fmt"
"path"
"path/filepath"
"regexp"
"strconv"
"sync"
@@ -284,17 +283,22 @@ func (s Service) CreateAccount(ctx context.Context, in *proto.CreateAccountReque
if acc.Id == "" {
acc.Id = uuid.Must(uuid.NewV4()).String()
}
if !s.isValidUsername(acc.PreferredName) {
return merrors.BadRequest(s.id, "preferred_name '%s' must be at least the local part of an email", acc.PreferredName)
}
if !s.isValidEmail(acc.Mail) {
return merrors.BadRequest(s.id, "mail '%s' must be a valid email", acc.Mail)
if err = validateAccount(s.id, *acc); err != nil {
return err
}
if id, err = cleanupID(acc.Id); err != nil {
return merrors.InternalServerError(s.id, "could not clean up account id: %v", err.Error())
}
exists, err := s.accountExists(ctx, acc.PreferredName, acc.Mail, acc.Id)
if err != nil {
return merrors.InternalServerError(s.id, "could not check if account exists: %v", err.Error())
}
if exists {
return merrors.BadRequest(s.id, "account already exists")
}
if acc.PasswordProfile != nil {
if acc.PasswordProfile.Password != "" {
// encrypt password
@@ -414,8 +418,6 @@ func (s Service) UpdateAccount(ctx context.Context, in *proto.UpdateAccountReque
return merrors.InternalServerError(s.id, "could not clean up account id: %v", err.Error())
}
path := filepath.Join(s.Config.Server.AccountsDataPath, "accounts", id)
if err = s.repo.LoadAccount(ctx, id, out); err != nil {
if storage.IsNotFoundErr(err) {
return merrors.NotFound(s.id, "account not found: %v", err.Error())
@@ -437,6 +439,10 @@ func (s Service) UpdateAccount(ctx context.Context, in *proto.UpdateAccountReque
return merrors.BadRequest(s.id, "%s", err)
}
if err = validateAccount(s.id, *in.Account); err != nil {
return err
}
if err := fieldmask_utils.StructToStruct(validMask, in.Account, out); err != nil {
return merrors.InternalServerError(s.id, "%s", err)
}
@@ -488,7 +494,7 @@ func (s Service) UpdateAccount(ctx context.Context, in *proto.UpdateAccountReque
}
if err = s.index.Update(old, out); err != nil {
s.log.Error().Err(err).Str("id", id).Str("path", path).Msg("could not index new account")
s.log.Error().Err(err).Str("id", id).Msg("could not index new account")
return merrors.InternalServerError(s.id, "could not index updated account: %v", err.Error())
}
@@ -571,12 +577,22 @@ func (s Service) DeleteAccount(ctx context.Context, in *proto.DeleteAccountReque
return
}
func validateAccount(serviceID string, a proto.Account) error {
if !isValidUsername(a.PreferredName) {
return merrors.BadRequest(serviceID, "preferred_name '%s' must be at least the local part of an email", a.PreferredName)
}
if !isValidEmail(a.Mail) {
return merrors.BadRequest(serviceID, "mail '%s' must be a valid email", a.Mail)
}
return nil
}
// We want to allow email addresses as usernames so they show up when using them in ACLs on storages that allow intergration with our glauth LDAP service
// so we are adding a few restrictions from https://stackoverflow.com/questions/6949667/what-are-the-real-rules-for-linux-usernames-on-centos-6-and-rhel-6
// names should not start with numbers
var usernameRegex = regexp.MustCompile("^[a-zA-Z_][a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]*(@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*)*$")
func (s Service) isValidUsername(e string) bool {
func isValidUsername(e string) bool {
if len(e) < 1 && len(e) > 254 {
return false
}
@@ -586,7 +602,7 @@ func (s Service) isValidUsername(e string) bool {
// regex from https://www.w3.org/TR/2016/REC-html51-20161101/sec-forms.html#valid-e-mail-address
var emailRegex = regexp.MustCompile("^[a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$")
func (s Service) isValidEmail(e string) bool {
func isValidEmail(e string) bool {
if len(e) < 3 && len(e) > 254 {
return false
}
@@ -673,3 +689,40 @@ func unique(strSlice []string) []string {
}
return list
}
func (s Service) accountExists(ctx context.Context, username, mail, id string) (exists bool, err error) {
var ids []string
ids, err = s.index.FindBy(&proto.Account{}, "preferred_name", username)
if err != nil {
return false, err
}
if len(ids) > 0 {
return true, nil
}
ids, err = s.index.FindBy(&proto.Account{}, "on_premises_sam_account_name", username)
if err != nil {
return false, err
}
if len(ids) > 0 {
return true, nil
}
ids, err = s.index.FindBy(&proto.Account{}, "mail", mail)
if err != nil {
return false, err
}
if len(ids) > 0 {
return true, nil
}
a := &proto.Account{}
err = s.repo.LoadAccount(ctx, id, a)
if err == nil {
return true, nil
}
if !storage.IsNotFoundErr(err) {
return true, err
}
return false, nil
}