From 629561833a95483232015c69f137fc9efe74c133 Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Wed, 14 Oct 2020 10:34:49 +0200 Subject: [PATCH] Write uid and gid from autoincrement index into accounts/groups --- accounts/pkg/indexer/indexer.go | 22 +++++++---- accounts/pkg/indexer/indexer_test.go | 28 +++++++------- accounts/pkg/service/v0/accounts.go | 46 +++++++++++++---------- accounts/pkg/service/v0/groups.go | 22 +++++++++-- accounts/pkg/service/v0/service.go | 55 ++++++++++++++++++++++++++-- 5 files changed, 125 insertions(+), 48 deletions(-) diff --git a/accounts/pkg/indexer/indexer.go b/accounts/pkg/indexer/indexer.go index 17b33a6de..89fd4b1a6 100644 --- a/accounts/pkg/indexer/indexer.go +++ b/accounts/pkg/indexer/indexer.go @@ -3,6 +3,8 @@ package indexer import ( "fmt" + "path" + "github.com/owncloud/ocis/accounts/pkg/config" "github.com/owncloud/ocis/accounts/pkg/indexer/errors" "github.com/owncloud/ocis/accounts/pkg/indexer/index" @@ -10,7 +12,6 @@ import ( _ "github.com/owncloud/ocis/accounts/pkg/indexer/index/disk" // to populate index "github.com/owncloud/ocis/accounts/pkg/indexer/option" "github.com/owncloud/ocis/accounts/pkg/indexer/registry" - "path" ) // Indexer is a facade to configure and query over multiple indices. @@ -19,6 +20,11 @@ type Indexer struct { indices typeMap } +// IdxAddResult represents the result of an Add call on an index +type IdxAddResult struct { + Field, Value string +} + // CreateIndexer creates a new Indexer. func CreateIndexer(cfg *config.Config) *Indexer { return &Indexer{ @@ -66,24 +72,24 @@ func (i Indexer) AddIndex(t interface{}, indexBy, pkName, entityDirName, indexTy } // Add a new entry to the indexer -func (i Indexer) Add(t interface{}) error { +func (i Indexer) Add(t interface{}) ([]IdxAddResult, error) { typeName := getTypeFQN(t) + var results []IdxAddResult if fields, ok := i.indices[typeName]; ok { for _, indices := range fields.IndicesByField { for _, idx := range indices { pkVal := valueOf(t, fields.PKFieldName) idxByVal := valueOf(t, idx.IndexBy()) - if idxByVal == "" { - continue - } - if _, err := idx.Add(pkVal, idxByVal); err != nil { - return err + value, err := idx.Add(pkVal, idxByVal) + if err != nil { + return []IdxAddResult{}, err } + results = append(results, IdxAddResult{Field: fields.PKFieldName, Value: value}) } } } - return nil + return results, nil } // FindBy finds a value on an index by field and value. diff --git a/accounts/pkg/indexer/indexer_test.go b/accounts/pkg/indexer/indexer_test.go index 8c6b49ad6..ecdb9c13f 100644 --- a/accounts/pkg/indexer/indexer_test.go +++ b/accounts/pkg/indexer/indexer_test.go @@ -24,7 +24,7 @@ func TestIndexer_AddWithUniqueIndex(t *testing.T) { assert.NoError(t, err) u := &User{ID: "abcdefg-123", UserName: "mikey", Email: "mikey@example.com"} - err = indexer.Add(u) + _, err = indexer.Add(u) assert.NoError(t, err) _ = os.RemoveAll(dataDir) @@ -47,7 +47,7 @@ func TestIndexer_AddWithUniqueIndexCS3(t *testing.T) { assert.NoError(t, err) u := &User{ID: "abcdefg-123", UserName: "mikey", Email: "mikey@example.com"} - err = indexer.Add(u) + _, err = indexer.Add(u) assert.NoError(t, err) _ = os.RemoveAll(dir) @@ -70,7 +70,7 @@ func TestIndexer_AddWithNonUniqueIndexCS3(t *testing.T) { assert.NoError(t, err) u := &User{ID: "abcdefg-123", UserName: "mikey", Email: "mikey@example.com"} - err = indexer.Add(u) + _, err = indexer.Add(u) assert.NoError(t, err) _ = os.RemoveAll(dataDir) @@ -90,7 +90,7 @@ func TestIndexer_FindByWithUniqueIndex(t *testing.T) { assert.NoError(t, err) u := &User{ID: "abcdefg-123", UserName: "mikey", Email: "mikey@example.com"} - err = indexer.Add(u) + _, err = indexer.Add(u) assert.NoError(t, err) res, err := indexer.FindBy(User{}, "UserName", "mikey") @@ -116,10 +116,10 @@ func TestIndexer_AddWithNonUniqueIndex(t *testing.T) { pet1 := Pet{ID: "goefe-789", Kind: "Hog", Color: "Green", Name: "Dicky"} pet2 := Pet{ID: "xadaf-189", Kind: "Hog", Color: "Green", Name: "Ricky"} - err = indexer.Add(pet1) + _, err = indexer.Add(pet1) assert.NoError(t, err) - err = indexer.Add(pet2) + _, err = indexer.Add(pet2) assert.NoError(t, err) res, err := indexer.FindBy(Pet{}, "Kind", "Hog") @@ -144,10 +144,10 @@ func TestIndexer_DeleteWithNonUniqueIndex(t *testing.T) { pet1 := Pet{ID: "goefe-789", Kind: "Hog", Color: "Green", Name: "Dicky"} pet2 := Pet{ID: "xadaf-189", Kind: "Hog", Color: "Green", Name: "Ricky"} - err = indexer.Add(pet1) + _, err = indexer.Add(pet1) assert.NoError(t, err) - err = indexer.Add(pet2) + _, err = indexer.Add(pet2) assert.NoError(t, err) err = indexer.Delete(pet2) @@ -172,10 +172,10 @@ func TestIndexer_SearchWithNonUniqueIndex(t *testing.T) { pet1 := Pet{ID: "goefe-789", Kind: "Hog", Color: "Green", Name: "Dicky"} pet2 := Pet{ID: "xadaf-189", Kind: "Hog", Color: "Green", Name: "Ricky"} - err = indexer.Add(pet1) + _, err = indexer.Add(pet1) assert.NoError(t, err) - err = indexer.Add(pet2) + _, err = indexer.Add(pet2) assert.NoError(t, err) res, err := indexer.FindByPartial(pet2, "Name", "*ky") @@ -204,10 +204,10 @@ func TestIndexer_UpdateWithUniqueIndex(t *testing.T) { user1 := &User{ID: "abcdefg-123", UserName: "mikey", Email: "mikey@example.com"} user2 := &User{ID: "hijklmn-456", UserName: "frank", Email: "frank@example.com"} - err = indexer.Add(user1) + _, err = indexer.Add(user1) assert.NoError(t, err) - err = indexer.Add(user2) + _, err = indexer.Add(user2) assert.NoError(t, err) err = indexer.Update(user1, &User{ @@ -259,10 +259,10 @@ func TestIndexer_UpdateWithNonUniqueIndex(t *testing.T) { pet1 := Pet{ID: "goefe-789", Kind: "Hog", Color: "Green", Name: "Dicky"} pet2 := Pet{ID: "xadaf-189", Kind: "Hog", Color: "Green", Name: "Ricky"} - err = indexer.Add(pet1) + _, err = indexer.Add(pet1) assert.NoError(t, err) - err = indexer.Add(pet2) + _, err = indexer.Add(pet2) assert.NoError(t, err) _ = os.RemoveAll(dataDir) diff --git a/accounts/pkg/service/v0/accounts.go b/accounts/pkg/service/v0/accounts.go index 7a0df76de..35ecca279 100644 --- a/accounts/pkg/service/v0/accounts.go +++ b/accounts/pkg/service/v0/accounts.go @@ -3,8 +3,10 @@ package service import ( "context" "fmt" + "path" "path/filepath" "regexp" + "strconv" "sync" "time" @@ -12,7 +14,6 @@ import ( "github.com/golang/protobuf/ptypes/empty" fieldmask_utils "github.com/mennanov/fieldmask-utils" merrors "github.com/micro/go-micro/v2/errors" - idxerrs "github.com/owncloud/ocis/accounts/pkg/indexer/errors" "github.com/owncloud/ocis/accounts/pkg/proto/v0" "github.com/owncloud/ocis/accounts/pkg/storage" "github.com/owncloud/ocis/ocis-pkg/roles" @@ -33,24 +34,6 @@ import ( // accLock mutually exclude readers from writers on account files var accLock sync.Mutex -func (s Service) indexAccount(id string) error { - a := &proto.Account{} - - if err := s.repo.LoadAccount(context.Background(), id, a); err != nil { - s.log.Error().Err(err).Str("account", id).Msg("could not load account") - return err - } - s.log.Debug().Interface("account", a).Msg("found account") - if err := s.index.Add(a); err != nil { - if idxerrs.IsAlreadyExistsErr(err) { - return nil - } - s.log.Error().Err(err).Interface("account", a).Msg("could not index account") - return err - } - return nil -} - // an auth request is currently hardcoded and has to match this regex // login eq \"teddy\" and password eq \"F&1!b90t111!\" var authQuery = regexp.MustCompile(`^login eq '(.*)' and password eq '(.*)'$`) // TODO how is ' escaped in the password? @@ -336,11 +319,34 @@ func (s Service) CreateAccount(ctx context.Context, in *proto.CreateAccountReque s.debugLogAccount(acc).Msg("could not persist new account") return merrors.InternalServerError(s.id, "could not persist new account: %v", err.Error()) } - if err = s.index.Add(acc); err != nil { + indexResults, err := s.index.Add(acc) + if err != nil { + // TODO: delete account when failed to add to indices return merrors.InternalServerError(s.id, "could not index new account: %v", err.Error()) } s.log.Debug().Interface("account", acc).Msg("account after indexing") + changed := false + for _, r := range indexResults { + if r.Field == "UidNumber" || r.Field == "GidNumber" { + id, err := strconv.ParseInt(path.Base(r.Value), 10, 0) + if err != nil { + return err + } + if r.Field == "UidNumber" { + acc.UidNumber = id + } else { + acc.GidNumber = id + } + changed = true + } + } + if changed { + if err := s.repo.WriteAccount(context.Background(), acc); err != nil { + return err + } + } + if acc.PasswordProfile != nil { acc.PasswordProfile.Password = "" } diff --git a/accounts/pkg/service/v0/groups.go b/accounts/pkg/service/v0/groups.go index e5ba000ba..ed4fec96c 100644 --- a/accounts/pkg/service/v0/groups.go +++ b/accounts/pkg/service/v0/groups.go @@ -2,12 +2,16 @@ package service import ( "context" + "path" + "path/filepath" + "strconv" + + "github.com/owncloud/ocis/accounts/pkg/storage" + "github.com/gofrs/uuid" "github.com/golang/protobuf/ptypes/empty" merrors "github.com/micro/go-micro/v2/errors" "github.com/owncloud/ocis/accounts/pkg/proto/v0" - "github.com/owncloud/ocis/accounts/pkg/storage" - "path/filepath" ) func (s Service) expandMembers(g *proto.Group) { @@ -133,10 +137,22 @@ func (s Service) CreateGroup(c context.Context, in *proto.CreateGroupRequest, ou return merrors.InternalServerError(s.id, "could not persist new group: %v", err.Error()) } - if err = s.index.Add(in.Group); err != nil { + indexResults, err := s.index.Add(in.Group) + if err != nil { return merrors.InternalServerError(s.id, "could not index new group: %v", err.Error()) } + for _, r := range indexResults { + if r.Field == "GidNumber" { + gid, err := strconv.ParseInt(path.Base(r.Value), 10, 0) + if err != nil { + return err + } + in.Group.GidNumber = gid + return s.repo.WriteGroup(context.Background(), in.Group) + } + } + return } diff --git a/accounts/pkg/service/v0/service.go b/accounts/pkg/service/v0/service.go index c1a9f2ad0..9f8757d94 100644 --- a/accounts/pkg/service/v0/service.go +++ b/accounts/pkg/service/v0/service.go @@ -5,7 +5,9 @@ import ( "errors" "github.com/owncloud/ocis/accounts/pkg/indexer" "github.com/owncloud/ocis/accounts/pkg/storage" + "path" "path/filepath" + "strconv" "strings" "time" @@ -71,7 +73,6 @@ func (s Service) buildIndex() (*indexer.Indexer, error) { idx := indexer.CreateIndexer(s.Config) // Accounts - if err := idx.AddIndex(&proto.Account{}, "DisplayName", "Id", "accounts", "non_unique"); err != nil { return nil, err } @@ -87,6 +88,13 @@ func (s Service) buildIndex() (*indexer.Indexer, error) { return nil, err } + if err := idx.AddIndex(&proto.Account{}, "UidNumber", "Id", "accounts", "autoincrement"); err != nil { + return nil, err + } + if err := idx.AddIndex(&proto.Account{}, "GidNumber", "Id", "accounts", "autoincrement"); err != nil { + return nil, err + } + // Groups if err := idx.AddIndex(&proto.Group{}, "OnPremisesSamAccountName", "Id", "groups", "unique"); err != nil { return nil, err @@ -96,6 +104,10 @@ func (s Service) buildIndex() (*indexer.Indexer, error) { return nil, err } + if err := idx.AddIndex(&proto.Group{}, "GidNumber", "Id", "groups", "autoincrement"); err != nil { + return nil, err + } + return idx, nil } @@ -215,13 +227,35 @@ func (s Service) createDefaultAccounts() (err error) { return err } - if err := s.indexAccount(accounts[i].Id); err != nil { + results, err := s.index.Add(&accounts[i]) + if err != nil { if idxerrs.IsAlreadyExistsErr(err) { continue } else { return err } } + + changed := false + for _, r := range results { + if r.Field == "UidNumber" || r.Field == "GidNumber" { + id, err := strconv.ParseInt(path.Base(r.Value), 10, 0) + if err != nil { + return err + } + if r.Field == "UidNumber" { + accounts[i].UidNumber = id + } else { + accounts[i].GidNumber = id + } + changed = true + } + } + if changed { + if err := s.repo.WriteAccount(context.Background(), &accounts[i]); err != nil { + return err + } + } } // set role for admin users and regular users @@ -281,13 +315,28 @@ func (s Service) createDefaultGroups() (err error) { return err } - if err := s.index.Add(&groups[i]); err != nil { + results, err := s.index.Add(&groups[i]) + if err != nil { if idxerrs.IsAlreadyExistsErr(err) { continue } else { return err } } + + for _, r := range results { + if r.Field == "GidNumber" { + gid, err := strconv.ParseInt(path.Base(r.Value), 10, 0) + if err != nil { + return err + } + groups[i].GidNumber = gid + if err := s.repo.WriteGroup(context.Background(), &groups[i]); err != nil { + return err + } + break + } + } } return nil }