From f6732fd14e51e4ba32be98cd49d7ad856da8fad5 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Mon, 28 Sep 2020 16:19:53 +0200 Subject: [PATCH 01/28] Extract storage layer + cs3impl --- accounts/pkg/service/v0/accounts.go | 66 ++----------- accounts/pkg/service/v0/groups.go | 69 +++---------- accounts/pkg/service/v0/service.go | 2 + accounts/pkg/storage/cs3.go | 144 ++++++++++++++++++++++++++++ accounts/pkg/storage/cs3_test.go | 18 ++++ accounts/pkg/storage/disk.go | 133 +++++++++++++++++++++++++ accounts/pkg/storage/repo.go | 13 +++ 7 files changed, 333 insertions(+), 112 deletions(-) create mode 100644 accounts/pkg/storage/cs3.go create mode 100644 accounts/pkg/storage/cs3_test.go create mode 100644 accounts/pkg/storage/disk.go create mode 100644 accounts/pkg/storage/repo.go diff --git a/accounts/pkg/service/v0/accounts.go b/accounts/pkg/service/v0/accounts.go index 3eb06ce9ac..c3040ecc23 100644 --- a/accounts/pkg/service/v0/accounts.go +++ b/accounts/pkg/service/v0/accounts.go @@ -2,9 +2,7 @@ package service import ( "context" - "encoding/json" "fmt" - "io/ioutil" "os" "path/filepath" "regexp" @@ -63,7 +61,7 @@ func (s Service) indexAccount(id string) error { a := &proto.BleveAccount{ BleveType: "account", } - if err := s.loadAccount(id, &a.Account); err != nil { + if err := s.repo.LoadAccount(context.Background(), id, &a.Account); err != nil { s.log.Error().Err(err).Str("account", id).Msg("could not load account") return err } @@ -79,37 +77,6 @@ func (s Service) indexAccount(id string) error { // 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? -func (s Service) loadAccount(id string, a *proto.Account) (err error) { - path := filepath.Join(s.Config.Server.AccountsDataPath, "accounts", id) - - var data []byte - if data, err = ioutil.ReadFile(path); err != nil { - return merrors.NotFound(s.id, "could not read account: %v", err.Error()) - } - - if err = json.Unmarshal(data, a); err != nil { - return merrors.InternalServerError(s.id, "could not unmarshal account: %v", err.Error()) - } - return -} - -func (s Service) writeAccount(a *proto.Account) (err error) { - // leave only the group id - s.deflateMemberOf(a) - - var bytes []byte - if bytes, err = json.Marshal(a); err != nil { - return merrors.InternalServerError(s.id, "could not marshal account: %v", err.Error()) - } - - path := filepath.Join(s.Config.Server.AccountsDataPath, "accounts", a.Id) - - if err = ioutil.WriteFile(path, bytes, 0600); err != nil { - return merrors.InternalServerError(s.id, "could not write account: %v", err.Error()) - } - return -} - func (s Service) expandMemberOf(a *proto.Account) { if a == nil { return @@ -118,7 +85,7 @@ func (s Service) expandMemberOf(a *proto.Account) { for i := range a.MemberOf { g := &proto.Group{} // TODO resolve by name, when a create or update is issued they may not have an id? fall back to searching the group id in the index? - if err := s.loadGroup(a.MemberOf[i].Id, g); err == nil { + if err := s.repo.LoadGroup(context.Background(), a.MemberOf[i].Id, g); err == nil { g.Members = nil // always hide members when expanding expanded = append(expanded, g) } else { @@ -129,23 +96,6 @@ func (s Service) expandMemberOf(a *proto.Account) { a.MemberOf = expanded } -// deflateMemberOf replaces the groups of a user with an instance that only contains the id -func (s Service) deflateMemberOf(a *proto.Account) { - if a == nil { - return - } - deflated := []*proto.Group{} - for i := range a.MemberOf { - if a.MemberOf[i].Id != "" { - deflated = append(deflated, &proto.Group{Id: a.MemberOf[i].Id}) - } else { - // TODO fetch and use an id when group only has a name but no id - s.log.Error().Str("id", a.Id).Interface("group", a.MemberOf[i]).Msg("resolving groups by name is not implemented yet") - } - } - a.MemberOf = deflated -} - func (s Service) passwordIsValid(hash string, pwd string) (ok bool) { defer func() { if r := recover(); r != nil { @@ -233,7 +183,7 @@ func (s Service) ListAccounts(ctx context.Context, in *proto.ListAccountsRequest for _, hit := range searchResult.Hits { a := &proto.Account{} - if err = s.loadAccount(hit.ID, a); err != nil { + if err = s.repo.LoadAccount(ctx, hit.ID, a); err != nil { s.log.Error().Err(err).Str("account", hit.ID).Msg("could not load account, skipping") continue } @@ -281,7 +231,7 @@ func (s Service) GetAccount(ctx context.Context, in *proto.GetAccountRequest, ou return merrors.InternalServerError(s.id, "could not clean up account id: %v", err.Error()) } - if err = s.loadAccount(id, out); err != nil { + if err = s.repo.LoadAccount(ctx, id, out); err != nil { s.log.Error().Err(err).Str("id", id).Msg("could not load account") return } @@ -346,7 +296,7 @@ func (s Service) CreateAccount(ctx context.Context, in *proto.CreateAccountReque // TODO groups should be ignored during create, use groups.AddMember? return error? // write and index account - note: don't do anything else in between! - if err = s.writeAccount(acc); err != nil { + if err = s.repo.WriteAccount(ctx, acc); err != nil { s.log.Error().Err(err).Str("id", id).Msg("could not persist new account") s.debugLogAccount(acc).Msg("could not persist new account") return @@ -407,7 +357,7 @@ func (s Service) UpdateAccount(ctx context.Context, in *proto.UpdateAccountReque path := filepath.Join(s.Config.Server.AccountsDataPath, "accounts", id) - if err = s.loadAccount(id, out); err != nil { + if err = s.repo.LoadAccount(ctx, id, out); err != nil { s.log.Error().Err(err).Str("id", id).Msg("could not load account") return } @@ -461,7 +411,7 @@ func (s Service) UpdateAccount(ctx context.Context, in *proto.UpdateAccountReque out.ExternalUserStateChangeDateTime = tsnow } - if err = s.writeAccount(out); err != nil { + if err = s.repo.WriteAccount(ctx, out); err != nil { s.log.Error().Err(err).Str("id", out.Id).Msg("could not persist updated account") return } @@ -513,7 +463,7 @@ func (s Service) DeleteAccount(ctx context.Context, in *proto.DeleteAccountReque path := filepath.Join(s.Config.Server.AccountsDataPath, "accounts", id) a := &proto.Account{} - if err = s.loadAccount(id, a); err != nil { + if err = s.repo.LoadAccount(ctx, id, a); err != nil { s.log.Error().Err(err).Str("id", id).Msg("could not load account") return } diff --git a/accounts/pkg/service/v0/groups.go b/accounts/pkg/service/v0/groups.go index 19727ad468..a2b346197d 100644 --- a/accounts/pkg/service/v0/groups.go +++ b/accounts/pkg/service/v0/groups.go @@ -2,8 +2,6 @@ package service import ( "context" - "encoding/json" - "io/ioutil" "os" "path/filepath" "sync" @@ -46,7 +44,7 @@ func (s Service) indexGroup(id string) error { g := &proto.BleveGroup{ BleveType: "group", } - if err := s.loadGroup(id, &g.Group); err != nil { + if err := s.repo.LoadGroup(context.Background(), id, &g.Group); err != nil { s.log.Error().Err(err).Str("group", id).Msg("could not load group") return err } @@ -58,43 +56,6 @@ func (s Service) indexGroup(id string) error { return nil } -func (s Service) loadGroup(id string, g *proto.Group) (err error) { - path := filepath.Join(s.Config.Server.AccountsDataPath, "groups", id) - - groupLock.Lock() - defer groupLock.Unlock() - var data []byte - if data, err = ioutil.ReadFile(path); err != nil { - return merrors.NotFound(s.id, "could not read group: %v", err.Error()) - } - - if err = json.Unmarshal(data, g); err != nil { - return merrors.InternalServerError(s.id, "could not unmarshal group: %v", err.Error()) - } - - return -} - -func (s Service) writeGroup(g *proto.Group) (err error) { - - // leave only the member id - s.deflateMembers(g) - - var bytes []byte - if bytes, err = json.Marshal(g); err != nil { - return merrors.InternalServerError(s.id, "could not marshal group: %v", err.Error()) - } - - path := filepath.Join(s.Config.Server.AccountsDataPath, "groups", g.Id) - - groupLock.Lock() - defer groupLock.Unlock() - if err = ioutil.WriteFile(path, bytes, 0600); err != nil { - return merrors.InternalServerError(s.id, "could not write group: %v", err.Error()) - } - return -} - func (s Service) expandMembers(g *proto.Group) { if g == nil { return @@ -103,7 +64,7 @@ func (s Service) expandMembers(g *proto.Group) { for i := range g.Members { // TODO resolve by name, when a create or update is issued they may not have an id? fall back to searching the group id in the index? a := &proto.Account{} - if err := s.loadAccount(g.Members[i].Id, a); err == nil { + if err := s.repo.LoadAccount(context.Background(), g.Members[i].Id, a); err == nil { expanded = append(expanded, a) } else { // log errors but continue execution for now @@ -173,7 +134,7 @@ func (s Service) ListGroups(c context.Context, in *proto.ListGroupsRequest, out for _, hit := range searchResult.Hits { g := &proto.Group{} - if err = s.loadGroup(hit.ID, g); err != nil { + if err = s.repo.LoadGroup(c, hit.ID, g); err != nil { s.log.Error().Err(err).Str("group", hit.ID).Msg("could not load group, skipping") continue } @@ -196,7 +157,7 @@ func (s Service) GetGroup(c context.Context, in *proto.GetGroupRequest, out *pro return merrors.InternalServerError(s.id, "could not clean up group id: %v", err.Error()) } - if err = s.loadGroup(id, out); err != nil { + if err = s.repo.LoadGroup(c, id, out); err != nil { s.log.Error().Err(err).Str("id", id).Msg("could not load group") return } @@ -226,7 +187,7 @@ func (s Service) CreateGroup(c context.Context, in *proto.CreateGroupRequest, ou // extract member id s.deflateMembers(in.Group) - if err = s.writeGroup(in.Group); err != nil { + if err = s.repo.WriteGroup(c, in.Group); err != nil { s.log.Error().Err(err).Interface("group", in.Group).Msg("could not persist new group") return } @@ -252,7 +213,7 @@ func (s Service) DeleteGroup(c context.Context, in *proto.DeleteGroupRequest, ou path := filepath.Join(s.Config.Server.AccountsDataPath, "groups", id) g := &proto.Group{} - if err = s.loadGroup(id, g); err != nil { + if err = s.repo.LoadGroup(c, id, g); err != nil { s.log.Error().Err(err).Str("id", id).Msg("could not load account") return } @@ -297,13 +258,13 @@ func (s Service) AddMember(c context.Context, in *proto.AddMemberRequest, out *p // load structs a := &proto.Account{} - if err = s.loadAccount(accountID, a); err != nil { + if err = s.repo.LoadAccount(c, accountID, a); err != nil { s.log.Error().Err(err).Str("id", accountID).Msg("could not load account") return } g := &proto.Group{} - if err = s.loadGroup(groupID, g); err != nil { + if err = s.repo.LoadGroup(c, groupID, g); err != nil { s.log.Error().Err(err).Str("id", groupID).Msg("could not load group") return } @@ -331,11 +292,11 @@ func (s Service) AddMember(c context.Context, in *proto.AddMemberRequest, out *p a.MemberOf = append(a.MemberOf, g) } - if err = s.writeAccount(a); err != nil { + if err = s.repo.WriteAccount(c, a); err != nil { s.log.Error().Err(err).Interface("account", a).Msg("could not persist account") return } - if err = s.writeGroup(g); err != nil { + if err = s.repo.WriteGroup(c, g); err != nil { s.log.Error().Err(err).Interface("group", g).Msg("could not persist group") return } @@ -362,13 +323,13 @@ func (s Service) RemoveMember(c context.Context, in *proto.RemoveMemberRequest, // load structs a := &proto.Account{} - if err = s.loadAccount(accountID, a); err != nil { + if err = s.repo.LoadAccount(c, accountID, a); err != nil { s.log.Error().Err(err).Str("id", accountID).Msg("could not load account") return } g := &proto.Group{} - if err = s.loadGroup(groupID, g); err != nil { + if err = s.repo.LoadGroup(c, groupID, g); err != nil { s.log.Error().Err(err).Str("id", groupID).Msg("could not load group") return } @@ -391,11 +352,11 @@ func (s Service) RemoveMember(c context.Context, in *proto.RemoveMemberRequest, } a.MemberOf = newGroups - if err = s.writeAccount(a); err != nil { + if err = s.repo.WriteAccount(c, a); err != nil { s.log.Error().Err(err).Interface("account", a).Msg("could not persist account") return } - if err = s.writeGroup(g); err != nil { + if err = s.repo.WriteGroup(c, g); err != nil { s.log.Error().Err(err).Interface("group", g).Msg("could not persist group") return } @@ -416,7 +377,7 @@ func (s Service) ListMembers(c context.Context, in *proto.ListMembersRequest, ou } g := &proto.Group{} - if err = s.loadGroup(groupID, g); err != nil { + if err = s.repo.LoadGroup(c, groupID, g); err != nil { s.log.Error().Err(err).Str("id", groupID).Msg("could not load group") return } diff --git a/accounts/pkg/service/v0/service.go b/accounts/pkg/service/v0/service.go index 38181cb9fb..6b50bf3266 100644 --- a/accounts/pkg/service/v0/service.go +++ b/accounts/pkg/service/v0/service.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "github.com/owncloud/ocis/accounts/pkg/storage" "io/ioutil" "os" "path/filepath" @@ -405,6 +406,7 @@ type Service struct { index bleve.Index RoleService settings.RoleService RoleManager *roles.Manager + repo storage.DiskRepo } func cleanupID(id string) (string, error) { diff --git a/accounts/pkg/storage/cs3.go b/accounts/pkg/storage/cs3.go new file mode 100644 index 0000000000..71b1ec4789 --- /dev/null +++ b/accounts/pkg/storage/cs3.go @@ -0,0 +1,144 @@ +package storage + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + v1beta11 "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" + provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + "github.com/cs3org/reva/pkg/rgrpc/todo/pool" + "github.com/cs3org/reva/pkg/token" + "github.com/cs3org/reva/pkg/token/manager/jwt" + merrors "github.com/micro/go-micro/v2/errors" + "github.com/owncloud/ocis/accounts/pkg/proto/v0" + "google.golang.org/grpc/metadata" + "net/http" +) + +type CS3Repo struct { + serviceID string + dataPath string + rootPath string + + tm token.Manager + storageClient provider.ProviderAPIClient +} + +func NewCS3Repo(secret string) (Repo, error) { + tokenManager, err := jwt.New(map[string]interface{}{ + "secret": "Pive-Fumkiu4", + }) + + if err != nil { + return nil, err + } + + client, err := pool.GetStorageProviderServiceClient("localhost:9185") + if err != nil { + return nil, err + } + + return CS3Repo{tm: tokenManager, storageClient: client}, nil + +} + +func (r CS3Repo) WriteAccount(ctx context.Context, a *proto.Account) (err error) { + t, err := r.authenticate(ctx) + if err != nil { + return err + } + + ctx = metadata.AppendToOutgoingContext(ctx, token.TokenHeader, t) + if err := r.makeRootDirIfNotExist(ctx); err != nil { + return err + } + + var by []byte + if by, err = json.Marshal(a); err != nil { + return merrors.InternalServerError(r.serviceID, "could not marshal account: %v", err.Error()) + } + + ureq, err := http.NewRequest("PUT", fmt.Sprintf("http://localhost:9187/data/accounts/%s", a.Id), bytes.NewReader(by)) + if err != nil { + return err + } + + ureq.Header.Add("x-access-token", t) + cl := http.Client{ + Transport: http.DefaultTransport, + } + + if _, err := cl.Do(ureq); err != nil { + return err + } + + return nil +} + +func (r CS3Repo) LoadAccount(ctx context.Context, id string, a *proto.Account) (err error) { + t, err := r.authenticate(ctx) + if err != nil { + return err + } + + ctx = metadata.AppendToOutgoingContext(ctx, token.TokenHeader, t) + + ureq, err := http.NewRequest("GET", fmt.Sprintf("http://localhost:9187/data/accounts/%s", id), nil) + if err != nil { + return err + } + + ureq.Header.Add("x-access-token", t) + cl := http.Client{ + Transport: http.DefaultTransport, + } + + if _, err = cl.Do(ureq); err != nil { + return err + } + + return nil +} + +func (r CS3Repo) WriteGroup(ctx context.Context, g *proto.Group) (err error) { + panic("implement me") +} + +func (r CS3Repo) LoadGroup(ctx context.Context, id string, g *proto.Group) (err error) { + panic("implement me") +} + +func (r CS3Repo) authenticate(ctx context.Context) (token string, err error) { + return r.tm.MintToken(ctx, &user.User{ + Id: &user.UserId{}, + Groups: []string{}, + }) +} + +func (r CS3Repo) makeRootDirIfNotExist(ctx context.Context) error { + var rootPathRef = &provider.Reference{ + Spec: &provider.Reference_Path{Path: "/meta/accounts"}, + } + + resp, err := r.storageClient.Stat(ctx, &provider.StatRequest{ + Ref: rootPathRef, + }) + + if err != nil { + return err + } + + if resp.Status.Code == v1beta11.Code_CODE_NOT_FOUND { + _, err := r.storageClient.CreateContainer(ctx, &provider.CreateContainerRequest{ + Ref: rootPathRef, + }) + + if err != nil { + return err + } + } + + return nil +} diff --git a/accounts/pkg/storage/cs3_test.go b/accounts/pkg/storage/cs3_test.go new file mode 100644 index 0000000000..1a55c80996 --- /dev/null +++ b/accounts/pkg/storage/cs3_test.go @@ -0,0 +1,18 @@ +package storage + +/* +func TestFoo(t *testing.T) { + r, err := NewCS3Repo("fooo") + assert.NoError(t, err) + + err = r.WriteAccount(context.Background(), &proto.Account{ + Id: "fefef-egegweg-gegeg", + AccountEnabled: true, + DisplayName: "Mike Jones", + Mail: "mike@example.com", + }) + + assert.NoError(t, err) +} + +*/ diff --git a/accounts/pkg/storage/disk.go b/accounts/pkg/storage/disk.go new file mode 100644 index 0000000000..f70fe6ce79 --- /dev/null +++ b/accounts/pkg/storage/disk.go @@ -0,0 +1,133 @@ +package storage + +import ( + "context" + "encoding/json" + merrors "github.com/micro/go-micro/v2/errors" + "github.com/owncloud/ocis/accounts/pkg/proto/v0" + "github.com/rs/zerolog" + "io/ioutil" + "path/filepath" + "sync" +) + +var groupLock sync.Mutex + +type DiskRepo struct { + serviceID string + dataPath string + log zerolog.Logger +} + +func New(serviceID string, dataPath string, log zerolog.Logger) DiskRepo { + return DiskRepo{ + serviceID: serviceID, + dataPath: dataPath, + log: log.With().Str("id", serviceID).Logger(), + } +} + +// WriteAccount to the storage +func (r DiskRepo) WriteAccount(ctx context.Context, a *proto.Account) (err error) { + // leave only the group id + r.deflateMemberOf(a) + + var bytes []byte + if bytes, err = json.Marshal(a); err != nil { + return merrors.InternalServerError(r.serviceID, "could not marshal account: %v", err.Error()) + } + + path := filepath.Join(r.dataPath, "accounts", a.Id) + + if err = ioutil.WriteFile(path, bytes, 0600); err != nil { + return merrors.InternalServerError(r.serviceID, "could not write account: %v", err.Error()) + } + return +} + +// LoadAccount from the storage +func (r DiskRepo) LoadAccount(ctx context.Context, id string, a *proto.Account) (err error) { + path := filepath.Join(r.dataPath, "accounts", id) + + var data []byte + if data, err = ioutil.ReadFile(path); err != nil { + return merrors.NotFound(r.serviceID, "could not read account: %v", err.Error()) + } + + if err = json.Unmarshal(data, a); err != nil { + return merrors.InternalServerError(r.serviceID, "could not unmarshal account: %v", err.Error()) + } + return +} + +// WriteGroup persists a given group to the storage +func (r DiskRepo) WriteGroup(ctx context.Context, g *proto.Group) (err error) { + // leave only the member id + r.deflateMembers(g) + + var bytes []byte + if bytes, err = json.Marshal(g); err != nil { + return merrors.InternalServerError(r.serviceID, "could not marshal group: %v", err.Error()) + } + + path := filepath.Join(r.dataPath, "groups", g.Id) + + groupLock.Lock() + defer groupLock.Unlock() + if err = ioutil.WriteFile(path, bytes, 0600); err != nil { + return merrors.InternalServerError(r.serviceID, "could not write group: %v", err.Error()) + } + return +} + +// LoadGroup from the storage +func (r DiskRepo) LoadGroup(ctx context.Context, id string, g *proto.Group) (err error) { + path := filepath.Join(r.dataPath, "groups", id) + + groupLock.Lock() + defer groupLock.Unlock() + var data []byte + if data, err = ioutil.ReadFile(path); err != nil { + return merrors.NotFound(r.serviceID, "could not read group: %v", err.Error()) + } + + if err = json.Unmarshal(data, g); err != nil { + return merrors.InternalServerError(r.serviceID, "could not unmarshal group: %v", err.Error()) + } + + return +} + +// deflateMemberOf replaces the groups of a user with an instance that only contains the id +func (r DiskRepo) deflateMemberOf(a *proto.Account) { + if a == nil { + return + } + deflated := []*proto.Group{} + for i := range a.MemberOf { + if a.MemberOf[i].Id != "" { + deflated = append(deflated, &proto.Group{Id: a.MemberOf[i].Id}) + } else { + // TODO fetch and use an id when group only has a name but no id + r.log.Error().Str("id", a.Id).Interface("group", a.MemberOf[i]).Msg("resolving groups by name is not implemented yet") + } + } + a.MemberOf = deflated +} + +// deflateMembers replaces the users of a group with an instance that only contains the id +func (r DiskRepo) deflateMembers(g *proto.Group) { + if g == nil { + return + } + deflated := []*proto.Account{} + for i := range g.Members { + if g.Members[i].Id != "" { + deflated = append(deflated, &proto.Account{Id: g.Members[i].Id}) + } else { + // TODO fetch and use an id when group only has a name but no id + r.log.Error().Str("id", g.Id).Interface("account", g.Members[i]).Msg("resolving members by name is not implemented yet") + } + } + g.Members = deflated +} diff --git a/accounts/pkg/storage/repo.go b/accounts/pkg/storage/repo.go new file mode 100644 index 0000000000..bb060eee75 --- /dev/null +++ b/accounts/pkg/storage/repo.go @@ -0,0 +1,13 @@ +package storage + +import ( + "context" + "github.com/owncloud/ocis/accounts/pkg/proto/v0" +) + +type Repo interface { + WriteAccount(ctx context.Context, a *proto.Account) (err error) + LoadAccount(ctx context.Context, id string, a *proto.Account) (err error) + WriteGroup(ctx context.Context, g *proto.Group) (err error) + LoadGroup(ctx context.Context, id string, g *proto.Group) (err error) +} From 901ada3ffcd1e8317809f532610650f6aaeeab86 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Tue, 29 Sep 2020 11:19:22 +0200 Subject: [PATCH 02/28] Basic implementation of CS3 [WIP] --- accounts/pkg/service/v0/groups.go | 6 +- accounts/pkg/storage/cs3.go | 135 +++++++++++++++++++++++++++++- accounts/pkg/storage/disk.go | 21 +++++ accounts/pkg/storage/repo.go | 2 + 4 files changed, 158 insertions(+), 6 deletions(-) diff --git a/accounts/pkg/service/v0/groups.go b/accounts/pkg/service/v0/groups.go index a2b346197d..c7a102d01e 100644 --- a/accounts/pkg/service/v0/groups.go +++ b/accounts/pkg/service/v0/groups.go @@ -228,9 +228,9 @@ func (s Service) DeleteGroup(c context.Context, in *proto.DeleteGroupRequest, ou s.log.Error().Err(err).Str("groupid", id).Str("accountid", g.Members[i].Id).Msg("could not remove account memberof, skipping") } } - if err = os.Remove(path); err != nil { - s.log.Error().Err(err).Str("id", id).Str("path", path).Msg("could not remove group") - return merrors.InternalServerError(s.id, "could not remove group: %v", err.Error()) + + if err = s.repo.DeleteGroup(c, id); err != nil { + return err } if err = s.index.Delete(id); err != nil { diff --git a/accounts/pkg/storage/cs3.go b/accounts/pkg/storage/cs3.go index 71b1ec4789..12f7a55321 100644 --- a/accounts/pkg/storage/cs3.go +++ b/accounts/pkg/storage/cs3.go @@ -14,6 +14,7 @@ import ( merrors "github.com/micro/go-micro/v2/errors" "github.com/owncloud/ocis/accounts/pkg/proto/v0" "google.golang.org/grpc/metadata" + "io/ioutil" "net/http" ) @@ -95,19 +96,147 @@ func (r CS3Repo) LoadAccount(ctx context.Context, id string, a *proto.Account) ( Transport: http.DefaultTransport, } - if _, err = cl.Do(ureq); err != nil { + resp, err := cl.Do(ureq) + if err != nil { + return err + } + + b, err := ioutil.ReadAll(resp.Body) + if err != nil { + return err + } + defer resp.Body.Close() + + if err := json.Unmarshal(b, &a); err != nil { return err } return nil } +func (r CS3Repo) DeleteAccount(ctx context.Context, id string) (err error) { + t, err := r.authenticate(ctx) + if err != nil { + return err + } + + ctx = metadata.AppendToOutgoingContext(ctx, token.TokenHeader, t) + + ureq, err := http.NewRequest("DELETE", fmt.Sprintf("http://localhost:9187/data/accounts/%s", id), nil) + if err != nil { + return err + } + + ureq.Header.Add("x-access-token", t) + cl := http.Client{ + Transport: http.DefaultTransport, + } + + resp, err := cl.Do(ureq) + if err != nil { + return err + } + + defer resp.Body.Close() + + return nil +} + +func (r CS3Repo) DeleteGroup(ctx context.Context, id string) (err error) { + t, err := r.authenticate(ctx) + if err != nil { + return err + } + + ctx = metadata.AppendToOutgoingContext(ctx, token.TokenHeader, t) + + ureq, err := http.NewRequest("DELETE", fmt.Sprintf("http://localhost:9187/data/groups/%s", id), nil) + if err != nil { + return err + } + + ureq.Header.Add("x-access-token", t) + cl := http.Client{ + Transport: http.DefaultTransport, + } + + resp, err := cl.Do(ureq) + if err != nil { + return err + } + + defer resp.Body.Close() + + return nil +} + func (r CS3Repo) WriteGroup(ctx context.Context, g *proto.Group) (err error) { - panic("implement me") + t, err := r.authenticate(ctx) + if err != nil { + return err + } + + ctx = metadata.AppendToOutgoingContext(ctx, token.TokenHeader, t) + if err := r.makeRootDirIfNotExist(ctx); err != nil { + return err + } + + var by []byte + if by, err = json.Marshal(g); err != nil { + return merrors.InternalServerError(r.serviceID, "could not marshal account: %v", err.Error()) + } + + ureq, err := http.NewRequest("PUT", fmt.Sprintf("http://localhost:9187/data/groups/%s", g.Id), bytes.NewReader(by)) + if err != nil { + return err + } + + ureq.Header.Add("x-access-token", t) + cl := http.Client{ + Transport: http.DefaultTransport, + } + + if _, err := cl.Do(ureq); err != nil { + return err + } + + return nil } func (r CS3Repo) LoadGroup(ctx context.Context, id string, g *proto.Group) (err error) { - panic("implement me") + t, err := r.authenticate(ctx) + if err != nil { + return err + } + + ctx = metadata.AppendToOutgoingContext(ctx, token.TokenHeader, t) + + ureq, err := http.NewRequest("GET", fmt.Sprintf("http://localhost:9187/data/groups/%s", id), nil) + if err != nil { + return err + } + + ureq.Header.Add("x-access-token", t) + cl := http.Client{ + Transport: http.DefaultTransport, + } + + resp, err := cl.Do(ureq) + if err != nil { + return err + } + + b, err := ioutil.ReadAll(resp.Body) + if err != nil { + return err + } + defer resp.Body.Close() + + if err := json.Unmarshal(b, &g); err != nil { + return err + } + + return nil } func (r CS3Repo) authenticate(ctx context.Context) (token string, err error) { diff --git a/accounts/pkg/storage/disk.go b/accounts/pkg/storage/disk.go index f70fe6ce79..a9ca4b15b5 100644 --- a/accounts/pkg/storage/disk.go +++ b/accounts/pkg/storage/disk.go @@ -7,6 +7,7 @@ import ( "github.com/owncloud/ocis/accounts/pkg/proto/v0" "github.com/rs/zerolog" "io/ioutil" + "os" "path/filepath" "sync" ) @@ -115,6 +116,26 @@ func (r DiskRepo) deflateMemberOf(a *proto.Account) { a.MemberOf = deflated } +func (r DiskRepo) DeleteAccount(ctx context.Context, id string) (err error) { + path := filepath.Join(r.dataPath, "accounts", id) + if err = os.Remove(path); err != nil { + r.log.Error().Err(err).Str("id", id).Str("path", path).Msg("could not remove account") + return merrors.InternalServerError(r.serviceID, "could not remove account: %v", err.Error()) + } + + return nil +} + +func (r DiskRepo) DeleteGroup(ctx context.Context, id string) (err error) { + path := filepath.Join(r.dataPath, "groups", id) + if err = os.Remove(path); err != nil { + r.log.Error().Err(err).Str("id", id).Str("path", path).Msg("could not remove group") + return merrors.InternalServerError(r.serviceID, "could not remove group: %v", err.Error()) + } + + return nil +} + // deflateMembers replaces the users of a group with an instance that only contains the id func (r DiskRepo) deflateMembers(g *proto.Group) { if g == nil { diff --git a/accounts/pkg/storage/repo.go b/accounts/pkg/storage/repo.go index bb060eee75..bac46dc639 100644 --- a/accounts/pkg/storage/repo.go +++ b/accounts/pkg/storage/repo.go @@ -8,6 +8,8 @@ import ( type Repo interface { WriteAccount(ctx context.Context, a *proto.Account) (err error) LoadAccount(ctx context.Context, id string, a *proto.Account) (err error) + DeleteAccount(ctx context.Context, id string) (err error) WriteGroup(ctx context.Context, g *proto.Group) (err error) LoadGroup(ctx context.Context, id string, g *proto.Group) (err error) + DeleteGroup(ctx context.Context, id string) (err error) } From 4428b771bc173098cf5c6aec401015e9f16a82bc Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Tue, 29 Sep 2020 12:58:12 +0200 Subject: [PATCH 03/28] - Implemented delete + some test --- accounts/pkg/storage/cs3.go | 20 ++++---------- accounts/pkg/storage/cs3_test.go | 47 ++++++++++++++++++++++++++++++-- 2 files changed, 50 insertions(+), 17 deletions(-) diff --git a/accounts/pkg/storage/cs3.go b/accounts/pkg/storage/cs3.go index 12f7a55321..47466ac4cd 100644 --- a/accounts/pkg/storage/cs3.go +++ b/accounts/pkg/storage/cs3.go @@ -122,23 +122,16 @@ func (r CS3Repo) DeleteAccount(ctx context.Context, id string) (err error) { ctx = metadata.AppendToOutgoingContext(ctx, token.TokenHeader, t) - ureq, err := http.NewRequest("DELETE", fmt.Sprintf("http://localhost:9187/data/accounts/%s", id), nil) + _, err = r.storageClient.Delete(ctx, &provider.DeleteRequest{ + Ref: &provider.Reference{ + Spec: &provider.Reference_Path{Path: fmt.Sprintf("/meta/accounts/%s", id)}, + }, + }) + if err != nil { return err } - ureq.Header.Add("x-access-token", t) - cl := http.Client{ - Transport: http.DefaultTransport, - } - - resp, err := cl.Do(ureq) - if err != nil { - return err - } - - defer resp.Body.Close() - return nil } @@ -149,7 +142,6 @@ func (r CS3Repo) DeleteGroup(ctx context.Context, id string) (err error) { } ctx = metadata.AppendToOutgoingContext(ctx, token.TokenHeader, t) - ureq, err := http.NewRequest("DELETE", fmt.Sprintf("http://localhost:9187/data/groups/%s", id), nil) if err != nil { return err diff --git a/accounts/pkg/storage/cs3_test.go b/accounts/pkg/storage/cs3_test.go index 1a55c80996..a99efecbd4 100644 --- a/accounts/pkg/storage/cs3_test.go +++ b/accounts/pkg/storage/cs3_test.go @@ -1,7 +1,13 @@ package storage -/* -func TestFoo(t *testing.T) { +import ( + "context" + "github.com/owncloud/ocis/accounts/pkg/proto/v0" + "github.com/stretchr/testify/assert" + "testing" +) + +func TestCS3Repo_WriteAccount(t *testing.T) { r, err := NewCS3Repo("fooo") assert.NoError(t, err) @@ -15,4 +21,39 @@ func TestFoo(t *testing.T) { assert.NoError(t, err) } -*/ +func TestCS3Repo_LoadAccount(t *testing.T) { + r, err := NewCS3Repo("fooo") + assert.NoError(t, err) + + err = r.WriteAccount(context.Background(), &proto.Account{ + Id: "fefef-egegweg-gegeg", + AccountEnabled: true, + DisplayName: "Mike Jones", + Mail: "mike@example.com", + }) + + acc := &proto.Account{} + err = r.LoadAccount(context.Background(), "fefef-egegweg-gegeg", acc) + + assert.NoError(t, err) + assert.Equal(t, "fefef-egegweg-gegeg", acc.Id) + assert.Equal(t, "Mike Jones", acc.DisplayName) + assert.Equal(t, "mike@example.com", acc.Mail) +} + +func TestCS3Repo_DeleteAccount(t *testing.T) { + r, err := NewCS3Repo("fooo") + assert.NoError(t, err) + + err = r.WriteAccount(context.Background(), &proto.Account{ + Id: "delete-me-id", + AccountEnabled: true, + DisplayName: "Mike Jones", + Mail: "mike@example.com", + }) + + err = r.DeleteAccount(context.Background(), "delete-me-id") + + assert.NoError(t, err) + +} From 5f37acbb0032f66397b47ba75731763905bcf78b Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Tue, 29 Sep 2020 14:40:50 +0200 Subject: [PATCH 04/28] Add config for repo implementations --- accounts/go.mod | 3 + accounts/pkg/config/config.go | 16 +++++ accounts/pkg/flagset/flagset.go | 28 ++++++++ accounts/pkg/service/v0/service.go | 19 +++++- accounts/pkg/storage/cs3.go | 100 ++++++++++++++++------------- accounts/pkg/storage/cs3_test.go | 13 +++- accounts/pkg/storage/disk.go | 71 ++++++++++---------- accounts/pkg/storage/repo.go | 5 ++ 8 files changed, 172 insertions(+), 83 deletions(-) diff --git a/accounts/go.mod b/accounts/go.mod index 81a05293ca..dc13f404b0 100644 --- a/accounts/go.mod +++ b/accounts/go.mod @@ -5,6 +5,8 @@ go 1.13 require ( github.com/CiscoM31/godata v0.0.0-20191007193734-c2c4ebb1b415 github.com/blevesearch/bleve v1.0.9 + github.com/cs3org/go-cs3apis v0.0.0-20200730121022-c4f3d4f7ddfd + github.com/cs3org/reva v1.1.0 github.com/cznic/b v0.0.0-20181122101859-a26611c4d92d // indirect github.com/cznic/mathutil v0.0.0-20181122101859-297441e03548 // indirect github.com/cznic/strutil v0.0.0-20181122101858-275e90344537 // indirect @@ -32,6 +34,7 @@ require ( github.com/tredoe/osutil v1.0.5 golang.org/x/net v0.0.0-20200822124328-c89045814202 google.golang.org/genproto v0.0.0-20200624020401-64a14ca9d1ad + google.golang.org/grpc v1.31.0 google.golang.org/protobuf v1.25.0 ) diff --git a/accounts/pkg/config/config.go b/accounts/pkg/config/config.go index fea59089e9..7237030450 100644 --- a/accounts/pkg/config/config.go +++ b/accounts/pkg/config/config.go @@ -61,6 +61,21 @@ type Log struct { Color bool } +type Repo struct { + Disk Disk + CS3 CS3 +} + +type Disk struct { + Path string +} + +type CS3 struct { + ProviderAddr string + DriverURL string + DataPrefix string +} + // Config merges all Account config parameters. type Config struct { LDAP LDAP @@ -70,6 +85,7 @@ type Config struct { Asset Asset Log Log TokenManager TokenManager + Repo Repo } // New returns a new config. diff --git a/accounts/pkg/flagset/flagset.go b/accounts/pkg/flagset/flagset.go index 4e0b41212b..98da16cc67 100644 --- a/accounts/pkg/flagset/flagset.go +++ b/accounts/pkg/flagset/flagset.go @@ -99,6 +99,34 @@ func ServerWithConfig(cfg *config.Config) []cli.Flag { EnvVars: []string{"ACCOUNTS_JWT_SECRET"}, Destination: &cfg.TokenManager.JWTSecret, }, + &cli.StringFlag{ + Name: "storage-disk-path", + Value: "", + Usage: "Path on the local disk, e.g. /var/tmp/ocis-accounts", + EnvVars: []string{"ACCOUNTS_STORAGE_CS3_PROVIDER_ADDR"}, + Destination: &cfg.Repo.CS3.ProviderAddr, + }, + &cli.StringFlag{ + Name: "storage-cs3-provider-addr", + Value: "0.0.0.0:9215", + Usage: "bind address for the metadata storage provider", + EnvVars: []string{"ACCOUNTS_STORAGE_CS3_PROVIDER_ADDR"}, + Destination: &cfg.Repo.CS3.ProviderAddr, + }, + &cli.StringFlag{ + Name: "storage-cs3-driver-url", + Value: "http://localhost:9216", + Usage: "http endpoint of the metadata storage, without trailing slash", + EnvVars: []string{"ACCOUNTS_STORAGE_CS3_DRIVER_URL"}, + Destination: &cfg.Repo.CS3.DriverURL, + }, + &cli.StringFlag{ + Name: "storage-cs3-data-prefix", + Value: "data", + Usage: "path prefix for the http endpoint of the metadata storage, without leading slash", + EnvVars: []string{"ACCOUNTS_STORAGE_CS3_DATA_PREFIX"}, + Destination: &cfg.Repo.CS3.DataPrefix, + }, } } diff --git a/accounts/pkg/service/v0/service.go b/accounts/pkg/service/v0/service.go index 6b50bf3266..eb2aa566e4 100644 --- a/accounts/pkg/service/v0/service.go +++ b/accounts/pkg/service/v0/service.go @@ -51,12 +51,14 @@ func New(opts ...Option) (s *Service, err error) { roleManager = &m } + serviceID := cfg.GRPC.Namespace + "." + cfg.Server.Name s = &Service{ - id: cfg.GRPC.Namespace + "." + cfg.Server.Name, + id: serviceID, log: logger, Config: cfg, RoleService: roleService, RoleManager: roleManager, + repo: createMetadataStorage(serviceID, cfg, logger), } // build an index @@ -398,6 +400,19 @@ func assignRoleToUser(accountID, roleID string, rs settings.RoleService, logger return true } +func createMetadataStorage(serviceID string, cfg *config.Config, logger log.Logger) storage.Repo { + // for now we detect the used storage implementation based on which storage is configured + // the config with defaults needs to be checked last + if cfg.Repo.Disk.Path != "" { + return storage.NewDiskRepo(serviceID, cfg, logger) + } + repo, err := storage.NewCS3Repo(serviceID, cfg) + if err != nil { + logger.Fatal().Err(err).Msg("cs3 storage was configured but failed to start") + } + return repo +} + // Service implements the AccountsServiceHandler interface type Service struct { id string @@ -406,7 +421,7 @@ type Service struct { index bleve.Index RoleService settings.RoleService RoleManager *roles.Manager - repo storage.DiskRepo + repo storage.Repo } func cleanupID(id string) (string, error) { diff --git a/accounts/pkg/storage/cs3.go b/accounts/pkg/storage/cs3.go index 47466ac4cd..39865a39a8 100644 --- a/accounts/pkg/storage/cs3.go +++ b/accounts/pkg/storage/cs3.go @@ -12,37 +12,41 @@ import ( "github.com/cs3org/reva/pkg/token" "github.com/cs3org/reva/pkg/token/manager/jwt" merrors "github.com/micro/go-micro/v2/errors" + "github.com/owncloud/ocis/accounts/pkg/config" "github.com/owncloud/ocis/accounts/pkg/proto/v0" "google.golang.org/grpc/metadata" "io/ioutil" "net/http" + "path" ) type CS3Repo struct { - serviceID string - dataPath string - rootPath string - + serviceID string + cfg *config.Config tm token.Manager storageClient provider.ProviderAPIClient } -func NewCS3Repo(secret string) (Repo, error) { +func NewCS3Repo(serviceID string, cfg *config.Config) (Repo, error) { tokenManager, err := jwt.New(map[string]interface{}{ - "secret": "Pive-Fumkiu4", + "secret": cfg.TokenManager.JWTSecret, }) if err != nil { return nil, err } - client, err := pool.GetStorageProviderServiceClient("localhost:9185") + client, err := pool.GetStorageProviderServiceClient(cfg.Repo.CS3.ProviderAddr) if err != nil { return nil, err } - return CS3Repo{tm: tokenManager, storageClient: client}, nil - + return CS3Repo{ + serviceID: serviceID, + cfg: cfg, + tm: tokenManager, + storageClient: client, + }, nil } func (r CS3Repo) WriteAccount(ctx context.Context, a *proto.Account) (err error) { @@ -52,7 +56,7 @@ func (r CS3Repo) WriteAccount(ctx context.Context, a *proto.Account) (err error) } ctx = metadata.AppendToOutgoingContext(ctx, token.TokenHeader, t) - if err := r.makeRootDirIfNotExist(ctx); err != nil { + if err := r.makeRootDirIfNotExist(ctx, accountsFolder); err != nil { return err } @@ -61,7 +65,7 @@ func (r CS3Repo) WriteAccount(ctx context.Context, a *proto.Account) (err error) return merrors.InternalServerError(r.serviceID, "could not marshal account: %v", err.Error()) } - ureq, err := http.NewRequest("PUT", fmt.Sprintf("http://localhost:9187/data/accounts/%s", a.Id), bytes.NewReader(by)) + ureq, err := http.NewRequest("PUT", r.accountUrl(a.Id), bytes.NewReader(by)) if err != nil { return err } @@ -86,7 +90,7 @@ func (r CS3Repo) LoadAccount(ctx context.Context, id string, a *proto.Account) ( ctx = metadata.AppendToOutgoingContext(ctx, token.TokenHeader, t) - ureq, err := http.NewRequest("GET", fmt.Sprintf("http://localhost:9187/data/accounts/%s", id), nil) + ureq, err := http.NewRequest("GET", r.accountUrl(id), nil) if err != nil { return err } @@ -135,33 +139,6 @@ func (r CS3Repo) DeleteAccount(ctx context.Context, id string) (err error) { return nil } -func (r CS3Repo) DeleteGroup(ctx context.Context, id string) (err error) { - t, err := r.authenticate(ctx) - if err != nil { - return err - } - - ctx = metadata.AppendToOutgoingContext(ctx, token.TokenHeader, t) - ureq, err := http.NewRequest("DELETE", fmt.Sprintf("http://localhost:9187/data/groups/%s", id), nil) - if err != nil { - return err - } - - ureq.Header.Add("x-access-token", t) - cl := http.Client{ - Transport: http.DefaultTransport, - } - - resp, err := cl.Do(ureq) - if err != nil { - return err - } - - defer resp.Body.Close() - - return nil -} - func (r CS3Repo) WriteGroup(ctx context.Context, g *proto.Group) (err error) { t, err := r.authenticate(ctx) if err != nil { @@ -169,7 +146,7 @@ func (r CS3Repo) WriteGroup(ctx context.Context, g *proto.Group) (err error) { } ctx = metadata.AppendToOutgoingContext(ctx, token.TokenHeader, t) - if err := r.makeRootDirIfNotExist(ctx); err != nil { + if err := r.makeRootDirIfNotExist(ctx, groupsFolder); err != nil { return err } @@ -178,7 +155,7 @@ func (r CS3Repo) WriteGroup(ctx context.Context, g *proto.Group) (err error) { return merrors.InternalServerError(r.serviceID, "could not marshal account: %v", err.Error()) } - ureq, err := http.NewRequest("PUT", fmt.Sprintf("http://localhost:9187/data/groups/%s", g.Id), bytes.NewReader(by)) + ureq, err := http.NewRequest("PUT", r.groupUrl(g.Id), bytes.NewReader(by)) if err != nil { return err } @@ -203,7 +180,7 @@ func (r CS3Repo) LoadGroup(ctx context.Context, id string, g *proto.Group) (err ctx = metadata.AppendToOutgoingContext(ctx, token.TokenHeader, t) - ureq, err := http.NewRequest("GET", fmt.Sprintf("http://localhost:9187/data/groups/%s", id), nil) + ureq, err := http.NewRequest("GET", r.groupUrl(id), nil) if err != nil { return err } @@ -231,6 +208,33 @@ func (r CS3Repo) LoadGroup(ctx context.Context, id string, g *proto.Group) (err return nil } +func (r CS3Repo) DeleteGroup(ctx context.Context, id string) (err error) { + t, err := r.authenticate(ctx) + if err != nil { + return err + } + + ctx = metadata.AppendToOutgoingContext(ctx, token.TokenHeader, t) + ureq, err := http.NewRequest("DELETE", r.groupUrl(id), nil) + if err != nil { + return err + } + + ureq.Header.Add("x-access-token", t) + cl := http.Client{ + Transport: http.DefaultTransport, + } + + resp, err := cl.Do(ureq) + if err != nil { + return err + } + + defer resp.Body.Close() + + return nil +} + func (r CS3Repo) authenticate(ctx context.Context) (token string, err error) { return r.tm.MintToken(ctx, &user.User{ Id: &user.UserId{}, @@ -238,9 +242,17 @@ func (r CS3Repo) authenticate(ctx context.Context) (token string, err error) { }) } -func (r CS3Repo) makeRootDirIfNotExist(ctx context.Context) error { +func (r CS3Repo) accountUrl(id string) string { + return path.Join(r.cfg.Repo.CS3.DriverURL, r.cfg.Repo.CS3.DataPrefix, accountsFolder, id) +} + +func (r CS3Repo) groupUrl(id string) string { + return path.Join(r.cfg.Repo.CS3.DriverURL, r.cfg.Repo.CS3.DataPrefix, groupsFolder, id) +} + +func (r CS3Repo) makeRootDirIfNotExist(ctx context.Context, folder string) error { var rootPathRef = &provider.Reference{ - Spec: &provider.Reference_Path{Path: "/meta/accounts"}, + Spec: &provider.Reference_Path{Path: fmt.Sprintf("/meta/%v", folder)}, } resp, err := r.storageClient.Stat(ctx, &provider.StatRequest{ diff --git a/accounts/pkg/storage/cs3_test.go b/accounts/pkg/storage/cs3_test.go index a99efecbd4..67ee881759 100644 --- a/accounts/pkg/storage/cs3_test.go +++ b/accounts/pkg/storage/cs3_test.go @@ -2,13 +2,20 @@ package storage import ( "context" + "github.com/owncloud/ocis/accounts/pkg/config" "github.com/owncloud/ocis/accounts/pkg/proto/v0" "github.com/stretchr/testify/assert" "testing" ) +var cfg = &config.Config{ + TokenManager: config.TokenManager{ + JWTSecret: "Pive-Fumkiu4", + }, +} + func TestCS3Repo_WriteAccount(t *testing.T) { - r, err := NewCS3Repo("fooo") + r, err := NewCS3Repo(cfg) assert.NoError(t, err) err = r.WriteAccount(context.Background(), &proto.Account{ @@ -22,7 +29,7 @@ func TestCS3Repo_WriteAccount(t *testing.T) { } func TestCS3Repo_LoadAccount(t *testing.T) { - r, err := NewCS3Repo("fooo") + r, err := NewCS3Repo(cfg) assert.NoError(t, err) err = r.WriteAccount(context.Background(), &proto.Account{ @@ -42,7 +49,7 @@ func TestCS3Repo_LoadAccount(t *testing.T) { } func TestCS3Repo_DeleteAccount(t *testing.T) { - r, err := NewCS3Repo("fooo") + r, err := NewCS3Repo(cfg) assert.NoError(t, err) err = r.WriteAccount(context.Background(), &proto.Account{ diff --git a/accounts/pkg/storage/disk.go b/accounts/pkg/storage/disk.go index a9ca4b15b5..dfb0a56275 100644 --- a/accounts/pkg/storage/disk.go +++ b/accounts/pkg/storage/disk.go @@ -3,28 +3,30 @@ package storage import ( "context" "encoding/json" - merrors "github.com/micro/go-micro/v2/errors" - "github.com/owncloud/ocis/accounts/pkg/proto/v0" - "github.com/rs/zerolog" + "github.com/owncloud/ocis/accounts/pkg/config" "io/ioutil" "os" "path/filepath" "sync" + + merrors "github.com/micro/go-micro/v2/errors" + "github.com/owncloud/ocis/accounts/pkg/proto/v0" + olog "github.com/owncloud/ocis/ocis-pkg/log" ) var groupLock sync.Mutex type DiskRepo struct { serviceID string - dataPath string - log zerolog.Logger + cfg *config.Config + log olog.Logger } -func New(serviceID string, dataPath string, log zerolog.Logger) DiskRepo { +func NewDiskRepo(serviceID string, cfg *config.Config, log olog.Logger) DiskRepo { return DiskRepo{ serviceID: serviceID, - dataPath: dataPath, - log: log.With().Str("id", serviceID).Logger(), + cfg: cfg, + log: log, } } @@ -38,7 +40,7 @@ func (r DiskRepo) WriteAccount(ctx context.Context, a *proto.Account) (err error return merrors.InternalServerError(r.serviceID, "could not marshal account: %v", err.Error()) } - path := filepath.Join(r.dataPath, "accounts", a.Id) + path := filepath.Join(r.cfg.Repo.Disk.Path, accountsFolder, a.Id) if err = ioutil.WriteFile(path, bytes, 0600); err != nil { return merrors.InternalServerError(r.serviceID, "could not write account: %v", err.Error()) @@ -48,7 +50,7 @@ func (r DiskRepo) WriteAccount(ctx context.Context, a *proto.Account) (err error // LoadAccount from the storage func (r DiskRepo) LoadAccount(ctx context.Context, id string, a *proto.Account) (err error) { - path := filepath.Join(r.dataPath, "accounts", id) + path := filepath.Join(r.cfg.Repo.Disk.Path, accountsFolder, id) var data []byte if data, err = ioutil.ReadFile(path); err != nil { @@ -61,6 +63,17 @@ func (r DiskRepo) LoadAccount(ctx context.Context, id string, a *proto.Account) return } +// DeleteAccount from the storage +func (r DiskRepo) DeleteAccount(ctx context.Context, id string) (err error) { + path := filepath.Join(r.cfg.Repo.Disk.Path, accountsFolder, id) + if err = os.Remove(path); err != nil { + r.log.Error().Err(err).Str("id", id).Str("path", path).Msg("could not remove account") + return merrors.InternalServerError(r.serviceID, "could not remove account: %v", err.Error()) + } + + return nil +} + // WriteGroup persists a given group to the storage func (r DiskRepo) WriteGroup(ctx context.Context, g *proto.Group) (err error) { // leave only the member id @@ -71,7 +84,7 @@ func (r DiskRepo) WriteGroup(ctx context.Context, g *proto.Group) (err error) { return merrors.InternalServerError(r.serviceID, "could not marshal group: %v", err.Error()) } - path := filepath.Join(r.dataPath, "groups", g.Id) + path := filepath.Join(r.cfg.Repo.Disk.Path, groupsFolder, g.Id) groupLock.Lock() defer groupLock.Unlock() @@ -83,7 +96,7 @@ func (r DiskRepo) WriteGroup(ctx context.Context, g *proto.Group) (err error) { // LoadGroup from the storage func (r DiskRepo) LoadGroup(ctx context.Context, id string, g *proto.Group) (err error) { - path := filepath.Join(r.dataPath, "groups", id) + path := filepath.Join(r.cfg.Repo.Disk.Path, groupsFolder, id) groupLock.Lock() defer groupLock.Unlock() @@ -99,12 +112,22 @@ func (r DiskRepo) LoadGroup(ctx context.Context, id string, g *proto.Group) (err return } +func (r DiskRepo) DeleteGroup(ctx context.Context, id string) (err error) { + path := filepath.Join(r.cfg.Repo.Disk.Path, groupsFolder, id) + if err = os.Remove(path); err != nil { + r.log.Error().Err(err).Str("id", id).Str("path", path).Msg("could not remove group") + return merrors.InternalServerError(r.serviceID, "could not remove group: %v", err.Error()) + } + + return nil +} + // deflateMemberOf replaces the groups of a user with an instance that only contains the id func (r DiskRepo) deflateMemberOf(a *proto.Account) { if a == nil { return } - deflated := []*proto.Group{} + var deflated []*proto.Group for i := range a.MemberOf { if a.MemberOf[i].Id != "" { deflated = append(deflated, &proto.Group{Id: a.MemberOf[i].Id}) @@ -116,32 +139,12 @@ func (r DiskRepo) deflateMemberOf(a *proto.Account) { a.MemberOf = deflated } -func (r DiskRepo) DeleteAccount(ctx context.Context, id string) (err error) { - path := filepath.Join(r.dataPath, "accounts", id) - if err = os.Remove(path); err != nil { - r.log.Error().Err(err).Str("id", id).Str("path", path).Msg("could not remove account") - return merrors.InternalServerError(r.serviceID, "could not remove account: %v", err.Error()) - } - - return nil -} - -func (r DiskRepo) DeleteGroup(ctx context.Context, id string) (err error) { - path := filepath.Join(r.dataPath, "groups", id) - if err = os.Remove(path); err != nil { - r.log.Error().Err(err).Str("id", id).Str("path", path).Msg("could not remove group") - return merrors.InternalServerError(r.serviceID, "could not remove group: %v", err.Error()) - } - - return nil -} - // deflateMembers replaces the users of a group with an instance that only contains the id func (r DiskRepo) deflateMembers(g *proto.Group) { if g == nil { return } - deflated := []*proto.Account{} + var deflated []*proto.Account for i := range g.Members { if g.Members[i].Id != "" { deflated = append(deflated, &proto.Account{Id: g.Members[i].Id}) diff --git a/accounts/pkg/storage/repo.go b/accounts/pkg/storage/repo.go index bac46dc639..12f9327ee5 100644 --- a/accounts/pkg/storage/repo.go +++ b/accounts/pkg/storage/repo.go @@ -5,6 +5,11 @@ import ( "github.com/owncloud/ocis/accounts/pkg/proto/v0" ) +const ( + accountsFolder = "accounts" + groupsFolder = "groups" +) + type Repo interface { WriteAccount(ctx context.Context, a *proto.Account) (err error) LoadAccount(ctx context.Context, id string, a *proto.Account) (err error) From d108f00f5c72d0f6439cee71b72f210ee6240595 Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Tue, 29 Sep 2020 14:42:06 +0200 Subject: [PATCH 05/28] Comment tests for now --- accounts/pkg/storage/cs3_test.go | 119 ++++++++++++++----------------- 1 file changed, 55 insertions(+), 64 deletions(-) diff --git a/accounts/pkg/storage/cs3_test.go b/accounts/pkg/storage/cs3_test.go index 67ee881759..627cc56962 100644 --- a/accounts/pkg/storage/cs3_test.go +++ b/accounts/pkg/storage/cs3_test.go @@ -1,66 +1,57 @@ package storage -import ( - "context" - "github.com/owncloud/ocis/accounts/pkg/config" - "github.com/owncloud/ocis/accounts/pkg/proto/v0" - "github.com/stretchr/testify/assert" - "testing" -) - -var cfg = &config.Config{ - TokenManager: config.TokenManager{ - JWTSecret: "Pive-Fumkiu4", - }, -} - -func TestCS3Repo_WriteAccount(t *testing.T) { - r, err := NewCS3Repo(cfg) - assert.NoError(t, err) - - err = r.WriteAccount(context.Background(), &proto.Account{ - Id: "fefef-egegweg-gegeg", - AccountEnabled: true, - DisplayName: "Mike Jones", - Mail: "mike@example.com", - }) - - assert.NoError(t, err) -} - -func TestCS3Repo_LoadAccount(t *testing.T) { - r, err := NewCS3Repo(cfg) - assert.NoError(t, err) - - err = r.WriteAccount(context.Background(), &proto.Account{ - Id: "fefef-egegweg-gegeg", - AccountEnabled: true, - DisplayName: "Mike Jones", - Mail: "mike@example.com", - }) - - acc := &proto.Account{} - err = r.LoadAccount(context.Background(), "fefef-egegweg-gegeg", acc) - - assert.NoError(t, err) - assert.Equal(t, "fefef-egegweg-gegeg", acc.Id) - assert.Equal(t, "Mike Jones", acc.DisplayName) - assert.Equal(t, "mike@example.com", acc.Mail) -} - -func TestCS3Repo_DeleteAccount(t *testing.T) { - r, err := NewCS3Repo(cfg) - assert.NoError(t, err) - - err = r.WriteAccount(context.Background(), &proto.Account{ - Id: "delete-me-id", - AccountEnabled: true, - DisplayName: "Mike Jones", - Mail: "mike@example.com", - }) - - err = r.DeleteAccount(context.Background(), "delete-me-id") - - assert.NoError(t, err) - -} +//var cfg = &config.Config{ +// TokenManager: config.TokenManager{ +// JWTSecret: "Pive-Fumkiu4", +// }, +//} +// +//func TestCS3Repo_WriteAccount(t *testing.T) { +// r, err := NewCS3Repo("hello", cfg) +// assert.NoError(t, err) +// +// err = r.WriteAccount(context.Background(), &proto.Account{ +// Id: "fefef-egegweg-gegeg", +// AccountEnabled: true, +// DisplayName: "Mike Jones", +// Mail: "mike@example.com", +// }) +// +// assert.NoError(t, err) +//} +// +//func TestCS3Repo_LoadAccount(t *testing.T) { +// r, err := NewCS3Repo("hello", cfg) +// assert.NoError(t, err) +// +// err = r.WriteAccount(context.Background(), &proto.Account{ +// Id: "fefef-egegweg-gegeg", +// AccountEnabled: true, +// DisplayName: "Mike Jones", +// Mail: "mike@example.com", +// }) +// +// acc := &proto.Account{} +// err = r.LoadAccount(context.Background(), "fefef-egegweg-gegeg", acc) +// +// assert.NoError(t, err) +// assert.Equal(t, "fefef-egegweg-gegeg", acc.Id) +// assert.Equal(t, "Mike Jones", acc.DisplayName) +// assert.Equal(t, "mike@example.com", acc.Mail) +//} +// +//func TestCS3Repo_DeleteAccount(t *testing.T) { +// r, err := NewCS3Repo("hello", cfg) +// assert.NoError(t, err) +// +// err = r.WriteAccount(context.Background(), &proto.Account{ +// Id: "delete-me-id", +// AccountEnabled: true, +// DisplayName: "Mike Jones", +// Mail: "mike@example.com", +// }) +// +// err = r.DeleteAccount(context.Background(), "delete-me-id") +// +// assert.NoError(t, err) +//} From 4c17d56ae0a3b4a39850728e8bf3694771e032bb Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Tue, 29 Sep 2020 14:49:59 +0200 Subject: [PATCH 06/28] Fix config --- accounts/pkg/flagset/flagset.go | 4 ++-- accounts/pkg/storage/cs3.go | 25 ++++++++++++++++++++----- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/accounts/pkg/flagset/flagset.go b/accounts/pkg/flagset/flagset.go index 98da16cc67..7956d58ff0 100644 --- a/accounts/pkg/flagset/flagset.go +++ b/accounts/pkg/flagset/flagset.go @@ -103,8 +103,8 @@ func ServerWithConfig(cfg *config.Config) []cli.Flag { Name: "storage-disk-path", Value: "", Usage: "Path on the local disk, e.g. /var/tmp/ocis-accounts", - EnvVars: []string{"ACCOUNTS_STORAGE_CS3_PROVIDER_ADDR"}, - Destination: &cfg.Repo.CS3.ProviderAddr, + EnvVars: []string{"ACCOUNTS_STORAGE_DISK_PATH"}, + Destination: &cfg.Repo.Disk.Path, }, &cli.StringFlag{ Name: "storage-cs3-provider-addr", diff --git a/accounts/pkg/storage/cs3.go b/accounts/pkg/storage/cs3.go index 39865a39a8..27ade503ba 100644 --- a/accounts/pkg/storage/cs3.go +++ b/accounts/pkg/storage/cs3.go @@ -5,6 +5,11 @@ import ( "context" "encoding/json" "fmt" + "io/ioutil" + "net/http" + "path" + "strings" + user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" v1beta11 "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" @@ -15,9 +20,6 @@ import ( "github.com/owncloud/ocis/accounts/pkg/config" "github.com/owncloud/ocis/accounts/pkg/proto/v0" "google.golang.org/grpc/metadata" - "io/ioutil" - "net/http" - "path" ) type CS3Repo struct { @@ -243,11 +245,11 @@ func (r CS3Repo) authenticate(ctx context.Context) (token string, err error) { } func (r CS3Repo) accountUrl(id string) string { - return path.Join(r.cfg.Repo.CS3.DriverURL, r.cfg.Repo.CS3.DataPrefix, accountsFolder, id) + return singleJoiningSlash(r.cfg.Repo.CS3.DriverURL, path.Join(r.cfg.Repo.CS3.DataPrefix, accountsFolder, id)) } func (r CS3Repo) groupUrl(id string) string { - return path.Join(r.cfg.Repo.CS3.DriverURL, r.cfg.Repo.CS3.DataPrefix, groupsFolder, id) + return singleJoiningSlash(r.cfg.Repo.CS3.DriverURL, path.Join(r.cfg.Repo.CS3.DataPrefix, groupsFolder, id)) } func (r CS3Repo) makeRootDirIfNotExist(ctx context.Context, folder string) error { @@ -275,3 +277,16 @@ func (r CS3Repo) makeRootDirIfNotExist(ctx context.Context, folder string) error return nil } + +// TODO: this is copied from proxy. Find a better solution or move it to ocis-pkg +func singleJoiningSlash(a, b string) string { + aslash := strings.HasSuffix(a, "/") + bslash := strings.HasPrefix(b, "/") + switch { + case aslash && bslash: + return a + b[1:] + case !aslash && !bslash: + return a + "/" + b + } + return a + b +} From 60efd6371b5c60da375088a5dbba54ba828e459b Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Tue, 29 Sep 2020 15:06:32 +0200 Subject: [PATCH 07/28] Fix DeleteGroup implementation and staticchecks --- accounts/pkg/config/config.go | 3 ++ accounts/pkg/service/v0/groups.go | 4 --- accounts/pkg/storage/cs3.go | 56 ++++++++++++------------------- accounts/pkg/storage/disk.go | 13 ++++--- accounts/pkg/storage/repo.go | 1 + 5 files changed, 34 insertions(+), 43 deletions(-) diff --git a/accounts/pkg/config/config.go b/accounts/pkg/config/config.go index 7237030450..956c12cea6 100644 --- a/accounts/pkg/config/config.go +++ b/accounts/pkg/config/config.go @@ -61,15 +61,18 @@ type Log struct { Color bool } +// Repo defines which storage implementation is to be used. type Repo struct { Disk Disk CS3 CS3 } +// Disk is the local disk implementation of the storage. type Disk struct { Path string } +// CS3 is the cs3 implementation of the storage. type CS3 struct { ProviderAddr string DriverURL string diff --git a/accounts/pkg/service/v0/groups.go b/accounts/pkg/service/v0/groups.go index c7a102d01e..94660c8fc8 100644 --- a/accounts/pkg/service/v0/groups.go +++ b/accounts/pkg/service/v0/groups.go @@ -4,7 +4,6 @@ import ( "context" "os" "path/filepath" - "sync" "github.com/CiscoM31/godata" "github.com/blevesearch/bleve" @@ -15,9 +14,6 @@ import ( "github.com/owncloud/ocis/accounts/pkg/provider" ) -// accLock mutually exclude readers from writers on group files -var groupLock sync.Mutex - func (s Service) indexGroups(path string) (err error) { var f *os.File if f, err = os.Open(path); err != nil { diff --git a/accounts/pkg/storage/cs3.go b/accounts/pkg/storage/cs3.go index 27ade503ba..0545813c76 100644 --- a/accounts/pkg/storage/cs3.go +++ b/accounts/pkg/storage/cs3.go @@ -22,6 +22,7 @@ import ( "google.golang.org/grpc/metadata" ) +// CS3Repo provides a cs3 implementation of the Repo interface type CS3Repo struct { serviceID string cfg *config.Config @@ -29,6 +30,7 @@ type CS3Repo struct { storageClient provider.ProviderAPIClient } +// NewCS3Repo creates a new cs3 repo func NewCS3Repo(serviceID string, cfg *config.Config) (Repo, error) { tokenManager, err := jwt.New(map[string]interface{}{ "secret": cfg.TokenManager.JWTSecret, @@ -51,6 +53,7 @@ func NewCS3Repo(serviceID string, cfg *config.Config) (Repo, error) { }, nil } +// WriteAccount writes an account via cs3 and modifies the provided account (e.g. with a generated id). func (r CS3Repo) WriteAccount(ctx context.Context, a *proto.Account) (err error) { t, err := r.authenticate(ctx) if err != nil { @@ -67,7 +70,7 @@ func (r CS3Repo) WriteAccount(ctx context.Context, a *proto.Account) (err error) return merrors.InternalServerError(r.serviceID, "could not marshal account: %v", err.Error()) } - ureq, err := http.NewRequest("PUT", r.accountUrl(a.Id), bytes.NewReader(by)) + ureq, err := http.NewRequest("PUT", r.accountURL(a.Id), bytes.NewReader(by)) if err != nil { return err } @@ -84,15 +87,14 @@ func (r CS3Repo) WriteAccount(ctx context.Context, a *proto.Account) (err error) return nil } +// LoadAccount loads an account via cs3 by id and writes it to the provided account func (r CS3Repo) LoadAccount(ctx context.Context, id string, a *proto.Account) (err error) { t, err := r.authenticate(ctx) if err != nil { return err } - ctx = metadata.AppendToOutgoingContext(ctx, token.TokenHeader, t) - - ureq, err := http.NewRequest("GET", r.accountUrl(id), nil) + ureq, err := http.NewRequest("GET", r.accountURL(id), nil) if err != nil { return err } @@ -120,6 +122,7 @@ func (r CS3Repo) LoadAccount(ctx context.Context, id string, a *proto.Account) ( return nil } +// DeleteAccount deletes an account via cs3 by id func (r CS3Repo) DeleteAccount(ctx context.Context, id string) (err error) { t, err := r.authenticate(ctx) if err != nil { @@ -130,17 +133,13 @@ func (r CS3Repo) DeleteAccount(ctx context.Context, id string) (err error) { _, err = r.storageClient.Delete(ctx, &provider.DeleteRequest{ Ref: &provider.Reference{ - Spec: &provider.Reference_Path{Path: fmt.Sprintf("/meta/accounts/%s", id)}, + Spec: &provider.Reference_Path{Path: fmt.Sprintf("/meta/%s/%s", accountsFolder, id)}, }, }) - - if err != nil { - return err - } - - return nil + return err } +// WriteGroup writes a group via cs3 and modifies the provided group (e.g. with a generated id). func (r CS3Repo) WriteGroup(ctx context.Context, g *proto.Group) (err error) { t, err := r.authenticate(ctx) if err != nil { @@ -157,7 +156,7 @@ func (r CS3Repo) WriteGroup(ctx context.Context, g *proto.Group) (err error) { return merrors.InternalServerError(r.serviceID, "could not marshal account: %v", err.Error()) } - ureq, err := http.NewRequest("PUT", r.groupUrl(g.Id), bytes.NewReader(by)) + ureq, err := http.NewRequest("PUT", r.groupURL(g.Id), bytes.NewReader(by)) if err != nil { return err } @@ -174,15 +173,14 @@ func (r CS3Repo) WriteGroup(ctx context.Context, g *proto.Group) (err error) { return nil } +// LoadGroup loads a group via cs3 by id and writes it to the provided group func (r CS3Repo) LoadGroup(ctx context.Context, id string, g *proto.Group) (err error) { t, err := r.authenticate(ctx) if err != nil { return err } - ctx = metadata.AppendToOutgoingContext(ctx, token.TokenHeader, t) - - ureq, err := http.NewRequest("GET", r.groupUrl(id), nil) + ureq, err := http.NewRequest("GET", r.groupURL(id), nil) if err != nil { return err } @@ -210,6 +208,7 @@ func (r CS3Repo) LoadGroup(ctx context.Context, id string, g *proto.Group) (err return nil } +// DeleteGroup deletes a group via cs3 by id func (r CS3Repo) DeleteGroup(ctx context.Context, id string) (err error) { t, err := r.authenticate(ctx) if err != nil { @@ -217,24 +216,13 @@ func (r CS3Repo) DeleteGroup(ctx context.Context, id string) (err error) { } ctx = metadata.AppendToOutgoingContext(ctx, token.TokenHeader, t) - ureq, err := http.NewRequest("DELETE", r.groupUrl(id), nil) - if err != nil { - return err - } - ureq.Header.Add("x-access-token", t) - cl := http.Client{ - Transport: http.DefaultTransport, - } - - resp, err := cl.Do(ureq) - if err != nil { - return err - } - - defer resp.Body.Close() - - return nil + _, err = r.storageClient.Delete(ctx, &provider.DeleteRequest{ + Ref: &provider.Reference{ + Spec: &provider.Reference_Path{Path: fmt.Sprintf("/meta/%s/%s", groupsFolder, id)}, + }, + }) + return err } func (r CS3Repo) authenticate(ctx context.Context) (token string, err error) { @@ -244,11 +232,11 @@ func (r CS3Repo) authenticate(ctx context.Context) (token string, err error) { }) } -func (r CS3Repo) accountUrl(id string) string { +func (r CS3Repo) accountURL(id string) string { return singleJoiningSlash(r.cfg.Repo.CS3.DriverURL, path.Join(r.cfg.Repo.CS3.DataPrefix, accountsFolder, id)) } -func (r CS3Repo) groupUrl(id string) string { +func (r CS3Repo) groupURL(id string) string { return singleJoiningSlash(r.cfg.Repo.CS3.DriverURL, path.Join(r.cfg.Repo.CS3.DataPrefix, groupsFolder, id)) } diff --git a/accounts/pkg/storage/disk.go b/accounts/pkg/storage/disk.go index dfb0a56275..1c7f21e203 100644 --- a/accounts/pkg/storage/disk.go +++ b/accounts/pkg/storage/disk.go @@ -16,12 +16,14 @@ import ( var groupLock sync.Mutex +// DiskRepo provides a local filesystem implementation of the Repo interface type DiskRepo struct { serviceID string cfg *config.Config log olog.Logger } +// NewDiskRepo creates a new disk repo func NewDiskRepo(serviceID string, cfg *config.Config, log olog.Logger) DiskRepo { return DiskRepo{ serviceID: serviceID, @@ -30,7 +32,7 @@ func NewDiskRepo(serviceID string, cfg *config.Config, log olog.Logger) DiskRepo } } -// WriteAccount to the storage +// WriteAccount to the local filesystem func (r DiskRepo) WriteAccount(ctx context.Context, a *proto.Account) (err error) { // leave only the group id r.deflateMemberOf(a) @@ -48,7 +50,7 @@ func (r DiskRepo) WriteAccount(ctx context.Context, a *proto.Account) (err error return } -// LoadAccount from the storage +// LoadAccount from the local filesystem func (r DiskRepo) LoadAccount(ctx context.Context, id string, a *proto.Account) (err error) { path := filepath.Join(r.cfg.Repo.Disk.Path, accountsFolder, id) @@ -63,7 +65,7 @@ func (r DiskRepo) LoadAccount(ctx context.Context, id string, a *proto.Account) return } -// DeleteAccount from the storage +// DeleteAccount from the local filesystem func (r DiskRepo) DeleteAccount(ctx context.Context, id string) (err error) { path := filepath.Join(r.cfg.Repo.Disk.Path, accountsFolder, id) if err = os.Remove(path); err != nil { @@ -74,7 +76,7 @@ func (r DiskRepo) DeleteAccount(ctx context.Context, id string) (err error) { return nil } -// WriteGroup persists a given group to the storage +// WriteGroup to the local filesystem func (r DiskRepo) WriteGroup(ctx context.Context, g *proto.Group) (err error) { // leave only the member id r.deflateMembers(g) @@ -94,7 +96,7 @@ func (r DiskRepo) WriteGroup(ctx context.Context, g *proto.Group) (err error) { return } -// LoadGroup from the storage +// LoadGroup from the local filesystem func (r DiskRepo) LoadGroup(ctx context.Context, id string, g *proto.Group) (err error) { path := filepath.Join(r.cfg.Repo.Disk.Path, groupsFolder, id) @@ -112,6 +114,7 @@ func (r DiskRepo) LoadGroup(ctx context.Context, id string, g *proto.Group) (err return } +// DeleteGroup from the local filesystem func (r DiskRepo) DeleteGroup(ctx context.Context, id string) (err error) { path := filepath.Join(r.cfg.Repo.Disk.Path, groupsFolder, id) if err = os.Remove(path); err != nil { diff --git a/accounts/pkg/storage/repo.go b/accounts/pkg/storage/repo.go index 12f9327ee5..27f278876a 100644 --- a/accounts/pkg/storage/repo.go +++ b/accounts/pkg/storage/repo.go @@ -10,6 +10,7 @@ const ( groupsFolder = "groups" ) +// Repo defines the storage operations type Repo interface { WriteAccount(ctx context.Context, a *proto.Account) (err error) LoadAccount(ctx context.Context, id string, a *proto.Account) (err error) From b1b1ecfc540fcfa980bb26a073e1b69a5bd05b5c Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Tue, 29 Sep 2020 15:10:28 +0200 Subject: [PATCH 08/28] Fix copy/paste typo in log message --- accounts/pkg/storage/cs3.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/accounts/pkg/storage/cs3.go b/accounts/pkg/storage/cs3.go index 0545813c76..87835f98cd 100644 --- a/accounts/pkg/storage/cs3.go +++ b/accounts/pkg/storage/cs3.go @@ -153,7 +153,7 @@ func (r CS3Repo) WriteGroup(ctx context.Context, g *proto.Group) (err error) { var by []byte if by, err = json.Marshal(g); err != nil { - return merrors.InternalServerError(r.serviceID, "could not marshal account: %v", err.Error()) + return merrors.InternalServerError(r.serviceID, "could not marshal group: %v", err.Error()) } ureq, err := http.NewRequest("PUT", r.groupURL(g.Id), bytes.NewReader(by)) From 355c042fa2753d29f26e22e503380e5122c3cb70 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Tue, 29 Sep 2020 16:04:35 +0200 Subject: [PATCH 09/28] load default accounts / groups using the service repository --- accounts/pkg/service/v0/accounts.go | 22 -- accounts/pkg/service/v0/groups.go | 23 -- accounts/pkg/service/v0/service.go | 402 ++++++++++++---------------- 3 files changed, 176 insertions(+), 271 deletions(-) diff --git a/accounts/pkg/service/v0/accounts.go b/accounts/pkg/service/v0/accounts.go index c3040ecc23..07e887096a 100644 --- a/accounts/pkg/service/v0/accounts.go +++ b/accounts/pkg/service/v0/accounts.go @@ -35,28 +35,6 @@ import ( // accLock mutually exclude readers from writers on account files var accLock sync.Mutex -func (s Service) indexAccounts(path string) (err error) { - var f *os.File - if f, err = os.Open(path); err != nil { - s.log.Error().Err(err).Str("dir", path).Msg("could not open accounts folder") - return - } - list, err := f.Readdir(-1) - f.Close() - if err != nil { - s.log.Error().Err(err).Str("dir", path).Msg("could not list accounts folder") - return - } - for _, file := range list { - err = s.indexAccount(file.Name()) - if err != nil { - s.log.Error().Err(err).Str("file", file.Name()).Msg("could not index account") - } - } - - return -} - func (s Service) indexAccount(id string) error { a := &proto.BleveAccount{ BleveType: "account", diff --git a/accounts/pkg/service/v0/groups.go b/accounts/pkg/service/v0/groups.go index 94660c8fc8..5003018dd7 100644 --- a/accounts/pkg/service/v0/groups.go +++ b/accounts/pkg/service/v0/groups.go @@ -2,7 +2,6 @@ package service import ( "context" - "os" "path/filepath" "github.com/CiscoM31/godata" @@ -14,28 +13,6 @@ import ( "github.com/owncloud/ocis/accounts/pkg/provider" ) -func (s Service) indexGroups(path string) (err error) { - var f *os.File - if f, err = os.Open(path); err != nil { - s.log.Error().Err(err).Str("dir", path).Msg("could not open groups folder") - return - } - list, err := f.Readdir(-1) - f.Close() - if err != nil { - s.log.Error().Err(err).Str("dir", path).Msg("could not list groups folder") - return - } - for _, file := range list { - err = s.indexGroup(file.Name()) - if err != nil { - s.log.Error().Err(err).Str("file", file.Name()).Msg("could not index account") - } - } - - return -} - func (s Service) indexGroup(id string) error { g := &proto.BleveGroup{ BleveType: "group", diff --git a/accounts/pkg/service/v0/service.go b/accounts/pkg/service/v0/service.go index eb2aa566e4..21c8690e39 100644 --- a/accounts/pkg/service/v0/service.go +++ b/accounts/pkg/service/v0/service.go @@ -2,11 +2,8 @@ package service import ( "context" - "encoding/json" "errors" - "fmt" "github.com/owncloud/ocis/accounts/pkg/storage" - "io/ioutil" "os" "path/filepath" "strings" @@ -61,31 +58,18 @@ func New(opts ...Option) (s *Service, err error) { repo: createMetadataStorage(serviceID, cfg, logger), } - // build an index if s.index, err = s.buildIndex(); err != nil { return nil, err } - // create default accounts - accountsDir := filepath.Join(cfg.Server.AccountsDataPath, "accounts") - if err = s.createDefaultAccounts(accountsDir); err != nil { - return nil, err - } - if err = s.indexAccounts(accountsDir); err != nil { + if err = s.createDefaultAccounts(); err != nil { return nil, err } - // create default groups - groupsDir := filepath.Join(cfg.Server.AccountsDataPath, "groups") - if err = s.createDefaultGroups(groupsDir); err != nil { + if err = s.createDefaultGroups(); err != nil { return nil, err } - if err = s.indexGroups(groupsDir); err != nil { - return nil, err - } - // TODO watch folders for new records - return } @@ -170,220 +154,186 @@ func (s Service) buildIndex() (index bleve.Index, err error) { return } -func (s Service) createDefaultAccounts(accountsDir string) (err error) { - // check if accounts exist - var fi os.FileInfo - if fi, err = os.Stat(accountsDir); err != nil { - if os.IsNotExist(err) { - // create accounts directory - if err = os.MkdirAll(accountsDir, 0700); err != nil { - return - } - // create default accounts - accounts := []proto.Account{ - { - Id: "4c510ada-c86b-4815-8820-42cdf82c3d51", - PreferredName: "einstein", - OnPremisesSamAccountName: "einstein", - Mail: "einstein@example.org", - DisplayName: "Albert Einstein", - UidNumber: 20000, - GidNumber: 30000, - PasswordProfile: &proto.PasswordProfile{ - Password: "$6$rounds=35210$sa1u5Pmfo4cr23Vw$RJNGElaDB1D3xorWkfTEGm2Ko.o2QL3E0cimKx23MNxVWVFSkUUeRoC7FqC4RzYDNQBD6cKzovTEaDD.8TDkD.", - }, - AccountEnabled: true, - MemberOf: []*proto.Group{ - {Id: "509a9dcd-bb37-4f4f-a01a-19dca27d9cfa"}, // users - {Id: "6040aa17-9c64-4fef-9bd0-77234d71bad0"}, // sailing-lovers - {Id: "dd58e5ec-842e-498b-8800-61f2ec6f911f"}, // violin-haters - {Id: "262982c1-2362-4afa-bfdf-8cbfef64a06e"}, // physics-lovers - }, - }, - { - Id: "f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c", - PreferredName: "marie", - OnPremisesSamAccountName: "marie", - Mail: "marie@example.org", - DisplayName: "Marie Curie", - UidNumber: 20001, - GidNumber: 30000, - PasswordProfile: &proto.PasswordProfile{ - Password: "$6$rounds=81434$sa1u5Pmfo4cr23Vw$W78cyL884GmuvDpxYPvSRBVzEj02T5QhTTcI8Dv4IKvMooDFGv4bwaWMkH9HfJ0wgpEBW7Lp.4Cad0xE/MYSg1", - }, - AccountEnabled: true, - MemberOf: []*proto.Group{ - {Id: "509a9dcd-bb37-4f4f-a01a-19dca27d9cfa"}, // users - {Id: "7b87fd49-286e-4a5f-bafd-c535d5dd997a"}, // radium-lovers - {Id: "cedc21aa-4072-4614-8676-fa9165f598ff"}, // polonium-lovers - {Id: "262982c1-2362-4afa-bfdf-8cbfef64a06e"}, // physics-lovers - }, - }, - { - Id: "932b4540-8d16-481e-8ef4-588e4b6b151c", - PreferredName: "richard", - OnPremisesSamAccountName: "richard", - Mail: "richard@example.org", - DisplayName: "Richard Feynman", - UidNumber: 20002, - GidNumber: 30000, - PasswordProfile: &proto.PasswordProfile{ - Password: "$6$rounds=5524$sa1u5Pmfo4cr23Vw$58bQVL/JeUlwM0RY21YKAFMvKvwKLLysGllYXox.vwKT5dHMwdzJjCxwTDMnB2o2pwexC8o/iOXyP2zrhALS40", - }, - AccountEnabled: true, - MemberOf: []*proto.Group{ - {Id: "509a9dcd-bb37-4f4f-a01a-19dca27d9cfa"}, // users - {Id: "a1726108-01f8-4c30-88df-2b1a9d1cba1a"}, // quantum-lovers - {Id: "167cbee2-0518-455a-bfb2-031fe0621e5d"}, // philosophy-haters - {Id: "262982c1-2362-4afa-bfdf-8cbfef64a06e"}, // physics-lovers - }, - }, - // admin user(s) - { - Id: "058bff95-6708-4fe5-91e4-9ea3d377588b", - PreferredName: "moss", - OnPremisesSamAccountName: "moss", - Mail: "moss@example.org", - DisplayName: "Maurice Moss", - UidNumber: 20003, - GidNumber: 30000, - PasswordProfile: &proto.PasswordProfile{ - Password: "$6$rounds=47068$lhw6odzXW0LTk/ao$GgxS.pIgP8jawLJBAiyNor2FrWzrULF95PwspRkli2W3VF.4HEwTYlQfRXbNQBMjNCEcEYlgZo3a.kRz2k2N0/", - }, - AccountEnabled: true, - MemberOf: []*proto.Group{ - {Id: "509a9dcd-bb37-4f4f-a01a-19dca27d9cfa"}, // users - }, - }, - // technical users for kopano and reva - { - Id: "820ba2a1-3f54-4538-80a4-2d73007e30bf", - PreferredName: "konnectd", - OnPremisesSamAccountName: "konnectd", - Mail: "idp@example.org", - DisplayName: "Kopano Konnectd", - UidNumber: 10000, - GidNumber: 15000, - PasswordProfile: &proto.PasswordProfile{ - Password: "$6$rounds=9746$sa1u5Pmfo4cr23Vw$2hnwpkTvUkWX0v6mh8Aw1pbzEXa9EUJzmrey4g2W/8arwWCwhteqU//3aWnA3S0d5T21fOKYteoqlsN1IbTcN.", - }, - AccountEnabled: true, - MemberOf: []*proto.Group{ - {Id: "34f38767-c937-4eb6-b847-1c175829a2a0"}, // sysusers - }, - }, - { - Id: "bc596f3c-c955-4328-80a0-60d018b4ad57", - PreferredName: "reva", - OnPremisesSamAccountName: "reva", - Mail: "storage@example.org", - DisplayName: "Reva Inter Operability Platform", - UidNumber: 10001, - GidNumber: 15000, - PasswordProfile: &proto.PasswordProfile{ - Password: "$6$rounds=91087$sa1u5Pmfo4cr23Vw$wPC3BbMTbP/ytlo0p.f99zJifyO70AUCdKIK9hkhwutBKGCirLmZs/MsWAG6xHjVvmnmHN5NoON7FUGv5pPaN.", - }, - AccountEnabled: true, - MemberOf: []*proto.Group{ - {Id: "34f38767-c937-4eb6-b847-1c175829a2a0"}, // sysusers - }, - }, - } - for i := range accounts { - // create account on disk - var bytes []byte - if bytes, err = json.Marshal(&accounts[i]); err != nil { - s.log.Error().Err(err).Interface("account", &accounts[i]).Msg("could not marshal default account") - return - } - path := filepath.Join(accountsDir, accounts[i].Id) - if err = ioutil.WriteFile(path, bytes, 0600); err != nil { - accounts[i].PasswordProfile.Password = "***REMOVED***" - s.log.Error().Err(err).Str("path", path).Interface("account", &accounts[i]).Msg("could not persist default account") - return - } - } - - // set role for admin users and regular users - assignRoleToUser("058bff95-6708-4fe5-91e4-9ea3d377588b", settings_svc.BundleUUIDRoleAdmin, s.RoleService, s.log) - for _, accountID := range []string{ - "058bff95-6708-4fe5-91e4-9ea3d377588b", //moss - } { - assignRoleToUser(accountID, settings_svc.BundleUUIDRoleAdmin, s.RoleService, s.log) - } - for _, accountID := range []string{ - "4c510ada-c86b-4815-8820-42cdf82c3d51", //einstein - "f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c", //marie - "932b4540-8d16-481e-8ef4-588e4b6b151c", //richard - } { - assignRoleToUser(accountID, settings_svc.BundleUUIDRoleUser, s.RoleService, s.log) - } +func (s Service) createDefaultAccounts() (err error) { + accounts := []proto.Account{ + { + Id: "4c510ada-c86b-4815-8820-42cdf82c3d51", + PreferredName: "einstein", + OnPremisesSamAccountName: "einstein", + Mail: "einstein@example.org", + DisplayName: "Albert Einstein", + UidNumber: 20000, + GidNumber: 30000, + PasswordProfile: &proto.PasswordProfile{ + Password: "$6$rounds=35210$sa1u5Pmfo4cr23Vw$RJNGElaDB1D3xorWkfTEGm2Ko.o2QL3E0cimKx23MNxVWVFSkUUeRoC7FqC4RzYDNQBD6cKzovTEaDD.8TDkD.", + }, + AccountEnabled: true, + MemberOf: []*proto.Group{ + {Id: "509a9dcd-bb37-4f4f-a01a-19dca27d9cfa"}, // users + {Id: "6040aa17-9c64-4fef-9bd0-77234d71bad0"}, // sailing-lovers + {Id: "dd58e5ec-842e-498b-8800-61f2ec6f911f"}, // violin-haters + {Id: "262982c1-2362-4afa-bfdf-8cbfef64a06e"}, // physics-lovers + }, + }, + { + Id: "f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c", + PreferredName: "marie", + OnPremisesSamAccountName: "marie", + Mail: "marie@example.org", + DisplayName: "Marie Curie", + UidNumber: 20001, + GidNumber: 30000, + PasswordProfile: &proto.PasswordProfile{ + Password: "$6$rounds=81434$sa1u5Pmfo4cr23Vw$W78cyL884GmuvDpxYPvSRBVzEj02T5QhTTcI8Dv4IKvMooDFGv4bwaWMkH9HfJ0wgpEBW7Lp.4Cad0xE/MYSg1", + }, + AccountEnabled: true, + MemberOf: []*proto.Group{ + {Id: "509a9dcd-bb37-4f4f-a01a-19dca27d9cfa"}, // users + {Id: "7b87fd49-286e-4a5f-bafd-c535d5dd997a"}, // radium-lovers + {Id: "cedc21aa-4072-4614-8676-fa9165f598ff"}, // polonium-lovers + {Id: "262982c1-2362-4afa-bfdf-8cbfef64a06e"}, // physics-lovers + }, + }, + { + Id: "932b4540-8d16-481e-8ef4-588e4b6b151c", + PreferredName: "richard", + OnPremisesSamAccountName: "richard", + Mail: "richard@example.org", + DisplayName: "Richard Feynman", + UidNumber: 20002, + GidNumber: 30000, + PasswordProfile: &proto.PasswordProfile{ + Password: "$6$rounds=5524$sa1u5Pmfo4cr23Vw$58bQVL/JeUlwM0RY21YKAFMvKvwKLLysGllYXox.vwKT5dHMwdzJjCxwTDMnB2o2pwexC8o/iOXyP2zrhALS40", + }, + AccountEnabled: true, + MemberOf: []*proto.Group{ + {Id: "509a9dcd-bb37-4f4f-a01a-19dca27d9cfa"}, // users + {Id: "a1726108-01f8-4c30-88df-2b1a9d1cba1a"}, // quantum-lovers + {Id: "167cbee2-0518-455a-bfb2-031fe0621e5d"}, // philosophy-haters + {Id: "262982c1-2362-4afa-bfdf-8cbfef64a06e"}, // physics-lovers + }, + }, + // admin user(s) + { + Id: "058bff95-6708-4fe5-91e4-9ea3d377588b", + PreferredName: "moss", + OnPremisesSamAccountName: "moss", + Mail: "moss@example.org", + DisplayName: "Maurice Moss", + UidNumber: 20003, + GidNumber: 30000, + PasswordProfile: &proto.PasswordProfile{ + Password: "$6$rounds=47068$lhw6odzXW0LTk/ao$GgxS.pIgP8jawLJBAiyNor2FrWzrULF95PwspRkli2W3VF.4HEwTYlQfRXbNQBMjNCEcEYlgZo3a.kRz2k2N0/", + }, + AccountEnabled: true, + MemberOf: []*proto.Group{ + {Id: "509a9dcd-bb37-4f4f-a01a-19dca27d9cfa"}, // users + }, + }, + // technical users for kopano and reva + { + Id: "820ba2a1-3f54-4538-80a4-2d73007e30bf", + PreferredName: "konnectd", + OnPremisesSamAccountName: "konnectd", + Mail: "idp@example.org", + DisplayName: "Kopano Konnectd", + UidNumber: 10000, + GidNumber: 15000, + PasswordProfile: &proto.PasswordProfile{ + Password: "$6$rounds=9746$sa1u5Pmfo4cr23Vw$2hnwpkTvUkWX0v6mh8Aw1pbzEXa9EUJzmrey4g2W/8arwWCwhteqU//3aWnA3S0d5T21fOKYteoqlsN1IbTcN.", + }, + AccountEnabled: true, + MemberOf: []*proto.Group{ + {Id: "34f38767-c937-4eb6-b847-1c175829a2a0"}, // sysusers + }, + }, + { + Id: "bc596f3c-c955-4328-80a0-60d018b4ad57", + PreferredName: "reva", + OnPremisesSamAccountName: "reva", + Mail: "storage@example.org", + DisplayName: "Reva Inter Operability Platform", + UidNumber: 10001, + GidNumber: 15000, + PasswordProfile: &proto.PasswordProfile{ + Password: "$6$rounds=91087$sa1u5Pmfo4cr23Vw$wPC3BbMTbP/ytlo0p.f99zJifyO70AUCdKIK9hkhwutBKGCirLmZs/MsWAG6xHjVvmnmHN5NoON7FUGv5pPaN.", + }, + AccountEnabled: true, + MemberOf: []*proto.Group{ + {Id: "34f38767-c937-4eb6-b847-1c175829a2a0"}, // sysusers + }, + }, + } + for i := range accounts { + if err := s.repo.WriteAccount(context.Background(), &accounts[i]); err != nil { + return err } - } else if !fi.IsDir() { - return fmt.Errorf("%s is not a directory", accountsDir) + + if err := s.indexAccount(accounts[i].Id); err != nil { + return err + } + } + + // set role for admin users and regular users + assignRoleToUser("058bff95-6708-4fe5-91e4-9ea3d377588b", settings_svc.BundleUUIDRoleAdmin, s.RoleService, s.log) + for _, accountID := range []string{ + "058bff95-6708-4fe5-91e4-9ea3d377588b", //moss + } { + assignRoleToUser(accountID, settings_svc.BundleUUIDRoleAdmin, s.RoleService, s.log) + } + for _, accountID := range []string{ + "4c510ada-c86b-4815-8820-42cdf82c3d51", //einstein + "f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c", //marie + "932b4540-8d16-481e-8ef4-588e4b6b151c", //richard + } { + assignRoleToUser(accountID, settings_svc.BundleUUIDRoleUser, s.RoleService, s.log) } return nil } -func (s Service) createDefaultGroups(groupsDir string) (err error) { - // check if groups exist - var fi os.FileInfo - if fi, err = os.Stat(groupsDir); err != nil { - if os.IsNotExist(err) { - // create accounts directory - if err = os.MkdirAll(groupsDir, 0700); err != nil { - return - } - // create default accounts - groups := []proto.Group{ - {Id: "34f38767-c937-4eb6-b847-1c175829a2a0", GidNumber: 15000, OnPremisesSamAccountName: "sysusers", DisplayName: "Technical users", Description: "A group for technical users. They should not show up in sharing dialogs.", Members: []*proto.Account{ - {Id: "820ba2a1-3f54-4538-80a4-2d73007e30bf"}, // konnectd - {Id: "bc596f3c-c955-4328-80a0-60d018b4ad57"}, // reva - }}, - {Id: "509a9dcd-bb37-4f4f-a01a-19dca27d9cfa", GidNumber: 30000, OnPremisesSamAccountName: "users", DisplayName: "Users", Description: "A group every normal user belongs to.", Members: []*proto.Account{ - {Id: "4c510ada-c86b-4815-8820-42cdf82c3d51"}, // einstein - {Id: "f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c"}, // marie - {Id: "932b4540-8d16-481e-8ef4-588e4b6b151c"}, // feynman - }}, - {Id: "6040aa17-9c64-4fef-9bd0-77234d71bad0", GidNumber: 30001, OnPremisesSamAccountName: "sailing-lovers", DisplayName: "Sailing lovers", Members: []*proto.Account{ - {Id: "4c510ada-c86b-4815-8820-42cdf82c3d51"}, // einstein - }}, - {Id: "dd58e5ec-842e-498b-8800-61f2ec6f911f", GidNumber: 30002, OnPremisesSamAccountName: "violin-haters", DisplayName: "Violin haters", Members: []*proto.Account{ - {Id: "4c510ada-c86b-4815-8820-42cdf82c3d51"}, // einstein - }}, - {Id: "7b87fd49-286e-4a5f-bafd-c535d5dd997a", GidNumber: 30003, OnPremisesSamAccountName: "radium-lovers", DisplayName: "Radium lovers", Members: []*proto.Account{ - {Id: "f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c"}, // marie - }}, - {Id: "cedc21aa-4072-4614-8676-fa9165f598ff", GidNumber: 30004, OnPremisesSamAccountName: "polonium-lovers", DisplayName: "Polonium lovers", Members: []*proto.Account{ - {Id: "f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c"}, // marie - }}, - {Id: "a1726108-01f8-4c30-88df-2b1a9d1cba1a", GidNumber: 30005, OnPremisesSamAccountName: "quantum-lovers", DisplayName: "Quantum lovers", Members: []*proto.Account{ - {Id: "932b4540-8d16-481e-8ef4-588e4b6b151c"}, // feynman - }}, - {Id: "167cbee2-0518-455a-bfb2-031fe0621e5d", GidNumber: 30006, OnPremisesSamAccountName: "philosophy-haters", DisplayName: "Philosophy haters", Members: []*proto.Account{ - {Id: "932b4540-8d16-481e-8ef4-588e4b6b151c"}, // feynman - }}, - {Id: "262982c1-2362-4afa-bfdf-8cbfef64a06e", GidNumber: 30007, OnPremisesSamAccountName: "physics-lovers", DisplayName: "Physics lovers", Members: []*proto.Account{ - {Id: "4c510ada-c86b-4815-8820-42cdf82c3d51"}, // einstein - {Id: "f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c"}, // marie - {Id: "932b4540-8d16-481e-8ef4-588e4b6b151c"}, // feynman - }}, - } - for i := range groups { - var bytes []byte - if bytes, err = json.Marshal(&groups[i]); err != nil { - s.log.Error().Err(err).Interface("group", &groups[i]).Msg("could not marshal default group") - return - } - path := filepath.Join(groupsDir, groups[i].Id) - if err = ioutil.WriteFile(path, bytes, 0600); err != nil { - s.log.Error().Err(err).Str("path", path).Interface("group", &groups[i]).Msg("could not persist default group") - return - } - } +func (s Service) createDefaultGroups() (err error) { + groups := []proto.Group{ + {Id: "34f38767-c937-4eb6-b847-1c175829a2a0", GidNumber: 15000, OnPremisesSamAccountName: "sysusers", DisplayName: "Technical users", Description: "A group for technical users. They should not show up in sharing dialogs.", Members: []*proto.Account{ + {Id: "820ba2a1-3f54-4538-80a4-2d73007e30bf"}, // konnectd + {Id: "bc596f3c-c955-4328-80a0-60d018b4ad57"}, // reva + }}, + {Id: "509a9dcd-bb37-4f4f-a01a-19dca27d9cfa", GidNumber: 30000, OnPremisesSamAccountName: "users", DisplayName: "Users", Description: "A group every normal user belongs to.", Members: []*proto.Account{ + {Id: "4c510ada-c86b-4815-8820-42cdf82c3d51"}, // einstein + {Id: "f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c"}, // marie + {Id: "932b4540-8d16-481e-8ef4-588e4b6b151c"}, // feynman + }}, + {Id: "6040aa17-9c64-4fef-9bd0-77234d71bad0", GidNumber: 30001, OnPremisesSamAccountName: "sailing-lovers", DisplayName: "Sailing lovers", Members: []*proto.Account{ + {Id: "4c510ada-c86b-4815-8820-42cdf82c3d51"}, // einstein + }}, + {Id: "dd58e5ec-842e-498b-8800-61f2ec6f911f", GidNumber: 30002, OnPremisesSamAccountName: "violin-haters", DisplayName: "Violin haters", Members: []*proto.Account{ + {Id: "4c510ada-c86b-4815-8820-42cdf82c3d51"}, // einstein + }}, + {Id: "7b87fd49-286e-4a5f-bafd-c535d5dd997a", GidNumber: 30003, OnPremisesSamAccountName: "radium-lovers", DisplayName: "Radium lovers", Members: []*proto.Account{ + {Id: "f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c"}, // marie + }}, + {Id: "cedc21aa-4072-4614-8676-fa9165f598ff", GidNumber: 30004, OnPremisesSamAccountName: "polonium-lovers", DisplayName: "Polonium lovers", Members: []*proto.Account{ + {Id: "f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c"}, // marie + }}, + {Id: "a1726108-01f8-4c30-88df-2b1a9d1cba1a", GidNumber: 30005, OnPremisesSamAccountName: "quantum-lovers", DisplayName: "Quantum lovers", Members: []*proto.Account{ + {Id: "932b4540-8d16-481e-8ef4-588e4b6b151c"}, // feynman + }}, + {Id: "167cbee2-0518-455a-bfb2-031fe0621e5d", GidNumber: 30006, OnPremisesSamAccountName: "philosophy-haters", DisplayName: "Philosophy haters", Members: []*proto.Account{ + {Id: "932b4540-8d16-481e-8ef4-588e4b6b151c"}, // feynman + }}, + {Id: "262982c1-2362-4afa-bfdf-8cbfef64a06e", GidNumber: 30007, OnPremisesSamAccountName: "physics-lovers", DisplayName: "Physics lovers", Members: []*proto.Account{ + {Id: "4c510ada-c86b-4815-8820-42cdf82c3d51"}, // einstein + {Id: "f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c"}, // marie + {Id: "932b4540-8d16-481e-8ef4-588e4b6b151c"}, // feynman + }}, + } + for i := range groups { + if err := s.repo.WriteGroup(context.Background(), &groups[i]); err != nil { + return err + } + + if err := s.indexGroup(groups[i].Id); err != nil { + return err } - } else if !fi.IsDir() { - return fmt.Errorf("%s is not a directory", groupsDir) } return nil } From 5168f8e2c0c9547bda7512ecc4db3e21cdc35ae9 Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Tue, 29 Sep 2020 16:30:10 +0200 Subject: [PATCH 10/28] Create data subfolders on disk repo start --- accounts/pkg/storage/disk.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/accounts/pkg/storage/disk.go b/accounts/pkg/storage/disk.go index 1c7f21e203..09e037d5e7 100644 --- a/accounts/pkg/storage/disk.go +++ b/accounts/pkg/storage/disk.go @@ -25,6 +25,19 @@ type DiskRepo struct { // NewDiskRepo creates a new disk repo func NewDiskRepo(serviceID string, cfg *config.Config, log olog.Logger) DiskRepo { + paths := []string{ + filepath.Join(cfg.Repo.Disk.Path, accountsFolder), + filepath.Join(cfg.Repo.Disk.Path, groupsFolder), + } + for i := range paths { + if _, err := os.Stat(paths[i]); err != nil { + if os.IsNotExist(err) { + if err = os.MkdirAll(paths[i], 0700); err != nil { + log.Fatal().Err(err).Msgf("could not create data folder %v", paths[i]) + } + } + } + } return DiskRepo{ serviceID: serviceID, cfg: cfg, From 2ffff93bca4534c24a5431c7e4f04012f59a9395 Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Wed, 30 Sep 2020 09:08:36 +0200 Subject: [PATCH 11/28] Add changelog --- accounts/changelog/unreleased/configurable-storage.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 accounts/changelog/unreleased/configurable-storage.md diff --git a/accounts/changelog/unreleased/configurable-storage.md b/accounts/changelog/unreleased/configurable-storage.md new file mode 100644 index 0000000000..021368ea5b --- /dev/null +++ b/accounts/changelog/unreleased/configurable-storage.md @@ -0,0 +1,7 @@ +Change: Choose disk or cs3 storage for accounts and groups + +The accounts service now has an abstraction layer for the storage. In addition to the local disk implementation +we implemented a cs3 storage, which is the new default for the accounts service. + +https://github.com/owncloud/ocis/pull/623 + From a02579b4ad27d9816075cd787cc903c1b5aade1b Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Wed, 30 Sep 2020 10:08:06 +0200 Subject: [PATCH 12/28] Start reva storage metadata by default in ocis --- ocis/pkg/command/revastoragemetadata.go | 51 +++++++++++++++++++++++++ ocis/pkg/command/server.go | 1 + 2 files changed, 52 insertions(+) create mode 100644 ocis/pkg/command/revastoragemetadata.go diff --git a/ocis/pkg/command/revastoragemetadata.go b/ocis/pkg/command/revastoragemetadata.go new file mode 100644 index 0000000000..18d80abb2f --- /dev/null +++ b/ocis/pkg/command/revastoragemetadata.go @@ -0,0 +1,51 @@ +// +build !simple + +package command + +import ( + "github.com/micro/cli/v2" + "github.com/owncloud/ocis/ocis-reva/pkg/command" + svcconfig "github.com/owncloud/ocis/ocis-reva/pkg/config" + "github.com/owncloud/ocis/ocis-reva/pkg/flagset" + "github.com/owncloud/ocis/ocis/pkg/config" + "github.com/owncloud/ocis/ocis/pkg/register" +) + +// RevaStorageMetadataCommand is the entrypoint for the reva-storage-metadata command. +func RevaStorageMetadataCommand(cfg *config.Config) *cli.Command { + return &cli.Command{ + Name: "reva-storage-metadata", + Usage: "Start reva storage service for metadata mount", + Category: "Extensions", + Flags: flagset.StorageMetadata(cfg.Reva), + Action: func(c *cli.Context) error { + revaStorageMetadataCommand := command.StorageMetadata(configureRevaStorageMetadata(cfg)) + + if err := revaStorageMetadataCommand.Before(c); err != nil { + return err + } + + return cli.HandleAction(revaStorageMetadataCommand.Action, c) + }, + } +} + +func configureRevaStorageMetadata(cfg *config.Config) *svcconfig.Config { + cfg.Reva.Log.Level = cfg.Log.Level + cfg.Reva.Log.Pretty = cfg.Log.Pretty + cfg.Reva.Log.Color = cfg.Log.Color + + if cfg.Tracing.Enabled { + cfg.Reva.Tracing.Enabled = cfg.Tracing.Enabled + cfg.Reva.Tracing.Type = cfg.Tracing.Type + cfg.Reva.Tracing.Endpoint = cfg.Tracing.Endpoint + cfg.Reva.Tracing.Collector = cfg.Tracing.Collector + cfg.Reva.Tracing.Service = cfg.Tracing.Service + } + + return cfg.Reva +} + +func init() { + register.AddCommand(RevaStorageMetadataCommand) +} diff --git a/ocis/pkg/command/server.go b/ocis/pkg/command/server.go index 6d22e71f91..b9a53beeb3 100644 --- a/ocis/pkg/command/server.go +++ b/ocis/pkg/command/server.go @@ -57,6 +57,7 @@ func Server(cfg *config.Config) *cli.Command { "reva-storage-oc", "reva-storage-oc-data", "reva-storage-public-link", + "reva-storage-metadata", }, runtime.MicroServices...)...) }, } From b01fc62b21e51b3822bf851ec847821483a92f96 Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Wed, 30 Sep 2020 10:16:53 +0200 Subject: [PATCH 13/28] Fix order of extensions on startup --- ocis/pkg/command/server.go | 16 ++++++++-------- ocis/pkg/runtime/runtime.go | 1 + 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/ocis/pkg/command/server.go b/ocis/pkg/command/server.go index b9a53beeb3..a3305f1277 100644 --- a/ocis/pkg/command/server.go +++ b/ocis/pkg/command/server.go @@ -35,16 +35,12 @@ func Server(cfg *config.Config) *cli.Command { r := runtime.New() // TODO temporary service startup selection. Should go away and the runtime should take care of it. return r.Start(append([]string{ - "accounts", - "settings", - "konnectd", "proxy", - "ocs", - "phoenix", - "glauth", - "webdav", "store", - "thumbnails", + "settings", + "phoenix", + "ocs", + "webdav", "reva-frontend", "reva-gateway", "reva-users", @@ -58,6 +54,10 @@ func Server(cfg *config.Config) *cli.Command { "reva-storage-oc-data", "reva-storage-public-link", "reva-storage-metadata", + "accounts", + "glauth", + "konnectd", + "thumbnails", }, runtime.MicroServices...)...) }, } diff --git a/ocis/pkg/runtime/runtime.go b/ocis/pkg/runtime/runtime.go index 7ea0a83393..75a407086d 100644 --- a/ocis/pkg/runtime/runtime.go +++ b/ocis/pkg/runtime/runtime.go @@ -50,6 +50,7 @@ var ( "reva-storage-oc", "reva-storage-oc-data", "reva-storage-public-link", + "reva-storage-metadata", "accounts", "glauth", "konnectd", From d316acad372266741af2068eda908d23b18ceb85 Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Wed, 30 Sep 2020 10:34:38 +0200 Subject: [PATCH 14/28] Reuse persisted bleve index --- accounts/pkg/service/v0/service.go | 157 +++++++++++++++-------------- 1 file changed, 82 insertions(+), 75 deletions(-) diff --git a/accounts/pkg/service/v0/service.go b/accounts/pkg/service/v0/service.go index 21c8690e39..b5283c7c2c 100644 --- a/accounts/pkg/service/v0/service.go +++ b/accounts/pkg/service/v0/service.go @@ -74,82 +74,89 @@ func New(opts ...Option) (s *Service, err error) { } func (s Service) buildIndex() (index bleve.Index, err error) { - indexMapping := bleve.NewIndexMapping() - // keep all symbols in terms to allow exact maching, eg. emails - indexMapping.DefaultAnalyzer = keyword.Name - // TODO don't bother to store fields as we will load the account from disk - - // Reusable mapping for text - standardTextFieldMapping := bleve.NewTextFieldMapping() - standardTextFieldMapping.Analyzer = standard.Name - standardTextFieldMapping.Store = false - - // Reusable mapping for text, uses english stop word removal - simpleTextFieldMapping := bleve.NewTextFieldMapping() - simpleTextFieldMapping.Analyzer = simple.Name - simpleTextFieldMapping.Store = false - - // Reusable mapping for keyword text - keywordFieldMapping := bleve.NewTextFieldMapping() - keywordFieldMapping.Analyzer = keyword.Name - keywordFieldMapping.Store = false - - // Reusable mapping for lowercase text - err = indexMapping.AddCustomAnalyzer("lowercase", - map[string]interface{}{ - "type": custom.Name, - "tokenizer": unicode.Name, - "token_filters": []string{ - lowercase.Name, - }, - }) - if err != nil { - return - } - lowercaseTextFieldMapping := bleve.NewTextFieldMapping() - lowercaseTextFieldMapping.Analyzer = "lowercase" - lowercaseTextFieldMapping.Store = true - - // accounts - accountMapping := bleve.NewDocumentMapping() - indexMapping.AddDocumentMapping("account", accountMapping) - - // Text - accountMapping.AddFieldMappingsAt("display_name", standardTextFieldMapping) - accountMapping.AddFieldMappingsAt("description", standardTextFieldMapping) - - // Lowercase - accountMapping.AddFieldMappingsAt("on_premises_sam_account_name", lowercaseTextFieldMapping) - accountMapping.AddFieldMappingsAt("preferred_name", lowercaseTextFieldMapping) - - // Keywords - accountMapping.AddFieldMappingsAt("mail", keywordFieldMapping) - - // groups - groupMapping := bleve.NewDocumentMapping() - indexMapping.AddDocumentMapping("group", groupMapping) - - // Text - groupMapping.AddFieldMappingsAt("display_name", standardTextFieldMapping) - groupMapping.AddFieldMappingsAt("description", standardTextFieldMapping) - - // Lowercase - groupMapping.AddFieldMappingsAt("on_premises_sam_account_name", lowercaseTextFieldMapping) - - // Tell blevesearch how to determine the type of the structs that are indexed. - // The referenced field needs to match the struct field exactly and it must be public. - // See pkg/proto/v0/bleve.go how we wrap the generated Account and Group to add a - // BleveType property which is indexed as `bleve_type` so we can also distinguish the - // documents in the index by querying for that property. - indexMapping.TypeField = "BleveType" - indexDir := filepath.Join(s.Config.Server.AccountsDataPath, "index.bleve") - // for now recreate index on every start - if err = os.RemoveAll(indexDir); err != nil { - return - } - if index, err = bleve.New(indexDir, indexMapping); err != nil { - return nil, err + if index, err = bleve.Open(indexDir); err != nil { + if err != bleve.ErrorIndexPathDoesNotExist { + s.log.Error().Err(err).Msg("failed to read index") + return + } + + indexMapping := bleve.NewIndexMapping() + // keep all symbols in terms to allow exact maching, eg. emails + indexMapping.DefaultAnalyzer = keyword.Name + // TODO don't bother to store fields as we will load the account from disk + + // Reusable mapping for text + standardTextFieldMapping := bleve.NewTextFieldMapping() + standardTextFieldMapping.Analyzer = standard.Name + standardTextFieldMapping.Store = false + + // Reusable mapping for text, uses english stop word removal + simpleTextFieldMapping := bleve.NewTextFieldMapping() + simpleTextFieldMapping.Analyzer = simple.Name + simpleTextFieldMapping.Store = false + + // Reusable mapping for keyword text + keywordFieldMapping := bleve.NewTextFieldMapping() + keywordFieldMapping.Analyzer = keyword.Name + keywordFieldMapping.Store = false + + // Reusable mapping for lowercase text + err = indexMapping.AddCustomAnalyzer("lowercase", + map[string]interface{}{ + "type": custom.Name, + "tokenizer": unicode.Name, + "token_filters": []string{ + lowercase.Name, + }, + }) + if err != nil { + return + } + lowercaseTextFieldMapping := bleve.NewTextFieldMapping() + lowercaseTextFieldMapping.Analyzer = "lowercase" + lowercaseTextFieldMapping.Store = true + + // accounts + accountMapping := bleve.NewDocumentMapping() + indexMapping.AddDocumentMapping("account", accountMapping) + + // Text + accountMapping.AddFieldMappingsAt("display_name", standardTextFieldMapping) + accountMapping.AddFieldMappingsAt("description", standardTextFieldMapping) + + // Lowercase + accountMapping.AddFieldMappingsAt("on_premises_sam_account_name", lowercaseTextFieldMapping) + accountMapping.AddFieldMappingsAt("preferred_name", lowercaseTextFieldMapping) + + // Keywords + accountMapping.AddFieldMappingsAt("mail", keywordFieldMapping) + + // groups + groupMapping := bleve.NewDocumentMapping() + indexMapping.AddDocumentMapping("group", groupMapping) + + // Text + groupMapping.AddFieldMappingsAt("display_name", standardTextFieldMapping) + groupMapping.AddFieldMappingsAt("description", standardTextFieldMapping) + + // Lowercase + groupMapping.AddFieldMappingsAt("on_premises_sam_account_name", lowercaseTextFieldMapping) + + // Tell blevesearch how to determine the type of the structs that are indexed. + // The referenced field needs to match the struct field exactly and it must be public. + // See pkg/proto/v0/bleve.go how we wrap the generated Account and Group to add a + // BleveType property which is indexed as `bleve_type` so we can also distinguish the + // documents in the index by querying for that property. + indexMapping.TypeField = "BleveType" + + // for now recreate index on every start + if err = os.RemoveAll(indexDir); err != nil { + return + } + if index, err = bleve.New(indexDir, indexMapping); err != nil { + return nil, err + } } return } From f5a51cd754c112531a6a764e861401a47db02333 Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Wed, 30 Sep 2020 10:53:02 +0200 Subject: [PATCH 15/28] Fix runtime startup order issues --- ocis/pkg/command/run.go | 10 ++------ ocis/pkg/command/server.go | 27 +-------------------- ocis/pkg/runtime/command.go | 19 --------------- ocis/pkg/runtime/runtime.go | 48 ++++++++++++++++++++----------------- 4 files changed, 29 insertions(+), 75 deletions(-) delete mode 100644 ocis/pkg/runtime/command.go diff --git a/ocis/pkg/command/run.go b/ocis/pkg/command/run.go index 51f62d1c99..47aa12dcaa 100644 --- a/ocis/pkg/command/run.go +++ b/ocis/pkg/command/run.go @@ -2,6 +2,7 @@ package command import ( "fmt" + "github.com/owncloud/ocis/ocis/pkg/runtime" "log" "net" "net/rpc" @@ -11,7 +12,6 @@ import ( "github.com/owncloud/ocis/ocis/pkg/config" "github.com/owncloud/ocis/ocis/pkg/register" - "github.com/refs/pman/pkg/process" ) // RunCommand is the entrypoint for the run command. @@ -40,13 +40,7 @@ func RunCommand(cfg *config.Config) *cli.Command { log.Fatal("dialing:", err) } - proc := process.NewProcEntry(os.Args[2], os.Environ(), []string{os.Args[2]}...) - var res int - - if err := client.Call("Service.Start", proc, &res); err != nil { - log.Fatal(err) - } - + res := runtime.RunService(client, os.Args[2]) fmt.Println(res) return nil }, diff --git a/ocis/pkg/command/server.go b/ocis/pkg/command/server.go index a3305f1277..20927e3f94 100644 --- a/ocis/pkg/command/server.go +++ b/ocis/pkg/command/server.go @@ -33,32 +33,7 @@ func Server(cfg *config.Config) *cli.Command { } r := runtime.New() - // TODO temporary service startup selection. Should go away and the runtime should take care of it. - return r.Start(append([]string{ - "proxy", - "store", - "settings", - "phoenix", - "ocs", - "webdav", - "reva-frontend", - "reva-gateway", - "reva-users", - "reva-auth-basic", - "reva-auth-bearer", - "reva-storage-home", - "reva-storage-home-data", - "reva-storage-eos", - "reva-storage-eos-data", - "reva-storage-oc", - "reva-storage-oc-data", - "reva-storage-public-link", - "reva-storage-metadata", - "accounts", - "glauth", - "konnectd", - "thumbnails", - }, runtime.MicroServices...)...) + return r.Start() }, } } diff --git a/ocis/pkg/runtime/command.go b/ocis/pkg/runtime/command.go deleted file mode 100644 index 0125f048f4..0000000000 --- a/ocis/pkg/runtime/command.go +++ /dev/null @@ -1,19 +0,0 @@ -package runtime - -import ( - "github.com/micro/cli/v2" -) - -// Command adds micro runtime commands to the cli app -func Command(app *cli.App) *cli.Command { - command := cli.Command{ - Name: "micro", - Description: "starts the go-micro runtime services", - Category: "Micro", - Action: func(c *cli.Context) error { - runtime := New() - return runtime.Start() - }, - } - return &command -} diff --git a/ocis/pkg/runtime/runtime.go b/ocis/pkg/runtime/runtime.go index 75a407086d..8cd8d0332d 100644 --- a/ocis/pkg/runtime/runtime.go +++ b/ocis/pkg/runtime/runtime.go @@ -51,7 +51,6 @@ var ( "reva-storage-oc-data", "reva-storage-public-link", "reva-storage-metadata", - "accounts", "glauth", "konnectd", "thumbnails", @@ -59,6 +58,7 @@ var ( // There seem to be a race condition when reva-sharing needs to read the sharing.json file and the parent folder is not present. dependants = []string{ + "accounts", "reva-sharing", } @@ -75,13 +75,13 @@ func New() Runtime { } // Start rpc runtime -func (r *Runtime) Start(services ...string) error { - go r.Launch(services) +func (r *Runtime) Start() error { + go r.Launch() return service.Start() } // Launch ocis default ocis extensions. -func (r *Runtime) Launch(services []string) { +func (r *Runtime) Launch() { var client *rpc.Client var err error var try int @@ -101,32 +101,36 @@ func (r *Runtime) Launch(services []string) { } OUT: - for _, v := range services { - args := process.NewProcEntry(v, os.Environ(), []string{v}...) - var reply int - - if err := client.Call("Service.Start", args, &reply); err != nil { - golog.Fatal(err) - } + for _, v := range MicroServices { + RunService(client, v) } - // TODO(refs) this should disappear and tackled at the runtime (pman) level. - // see https://github.com/cs3org/reva/issues/795 for race condition. - // dependants might not be needed on a ocis_simple build, therefore - // it should not be started under these circumstances. - if len(services) >= len(Extensions) { // it will not run for ocis_simple builds. + for _, v := range Extensions { + RunService(client, v) + } + + if len(dependants) > 0 { + // TODO(refs) this should disappear and tackled at the runtime (pman) level. + // see https://github.com/cs3org/reva/issues/795 for race condition. + // dependants might not be needed on a ocis_simple build, therefore + // it should not be started under these circumstances. time.Sleep(2 * time.Second) for _, v := range dependants { - args := process.NewProcEntry(v, os.Environ(), []string{v}...) - var reply int - - if err := client.Call("Service.Start", args, &reply); err != nil { - golog.Fatal(err) - } + RunService(client, v) } } } +// RunService sends a Service.Start command with the given service name to pman +func RunService(client *rpc.Client, service string) (reply int) { + args := process.NewProcEntry(service, os.Environ(), []string{service}...) + + if err := client.Call("Service.Start", args, &reply); err != nil { + golog.Fatal(err) + } + return +} + // AddMicroPlatform adds the micro subcommands to the cli app func AddMicroPlatform(app *cli.App) { setDefaults() From e674b5ce4405d1466dd461f61de18a2df60dbb3e Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Wed, 30 Sep 2020 14:36:27 +0200 Subject: [PATCH 16/28] Move micro-framework error handling out of storage --- .../pkg/proto/v0/accounts.pb.micro_test.go | 30 +-------- accounts/pkg/service/v0/accounts.go | 36 ++++++++--- .../service/v0/accounts_permission_test.go | 1 + accounts/pkg/service/v0/groups.go | 59 ++++++++++++------ accounts/pkg/storage/cs3.go | 39 ++++++++++-- accounts/pkg/storage/cs3_test.go | 17 +++++ accounts/pkg/storage/disk.go | 62 ++++++++++--------- accounts/pkg/storage/errors.go | 19 ++++++ 8 files changed, 173 insertions(+), 90 deletions(-) create mode 100644 accounts/pkg/storage/errors.go diff --git a/accounts/pkg/proto/v0/accounts.pb.micro_test.go b/accounts/pkg/proto/v0/accounts.pb.micro_test.go index 80fd0319d0..223cfd4397 100644 --- a/accounts/pkg/proto/v0/accounts.pb.micro_test.go +++ b/accounts/pkg/proto/v0/accounts.pb.micro_test.go @@ -163,6 +163,7 @@ func init() { cfg := config.New() cfg.Server.AccountsDataPath = dataPath + cfg.Repo.Disk.Path = dataPath var hdlr *svc.Service var err error @@ -754,7 +755,6 @@ func TestGetGroupInvalidID(t *testing.T) { assert.IsType(t, &proto.Group{}, resp) assert.Empty(t, resp) assert.Error(t, err) - assert.Equal(t, "{\"id\":\".\",\"code\":404,\"detail\":\"could not read group: open accounts-store/groups/42: no such file or directory\",\"status\":\"Not Found\"}", err.Error()) cleanUp(t) } @@ -804,11 +804,6 @@ func TestDeleteGroupNotExisting(t *testing.T) { assert.IsType(t, &empty.Empty{}, res) assert.Empty(t, res) assert.Error(t, err) - assert.Equal( - t, - fmt.Sprintf("{\"id\":\".\",\"code\":404,\"detail\":\"could not read group: open accounts-store/groups/%v: no such file or directory\",\"status\":\"Not Found\"}", id), - err.Error(), - ) } cleanUp(t) } @@ -826,17 +821,12 @@ func TestDeleteGroupInvalidId(t *testing.T) { client := service.Client() cl := proto.NewGroupsService("com.owncloud.api.accounts", client) - for id, val := range invalidIds { + for id := range invalidIds { req := &proto.DeleteGroupRequest{Id: id} res, err := cl.DeleteGroup(context.Background(), req) assert.IsType(t, &empty.Empty{}, res) assert.Empty(t, res) assert.Error(t, err) - assert.Equal( - t, - fmt.Sprintf("{\"id\":\".\",\"code\":500,\"detail\":\"could not clean up group id: invalid id %v\",\"status\":\"Internal Server Error\"}", val), - err.Error(), - ) } cleanUp(t) } @@ -859,11 +849,7 @@ func TestUpdateGroup(t *testing.T) { assert.IsType(t, &proto.Group{}, res) assert.Empty(t, res) assert.Error(t, err) - assert.Equal( - t, - "{\"id\":\".\",\"code\":500,\"detail\":\"not implemented\",\"status\":\"Internal Server Error\"}", - err.Error(), - ) + cleanUp(t) } @@ -953,11 +939,6 @@ func TestAddMemberNonExisting(t *testing.T) { assert.IsType(t, &proto.Group{}, res) assert.Empty(t, res) assert.Error(t, err) - assert.Equal( - t, - fmt.Sprintf("{\"id\":\".\",\"code\":404,\"detail\":\"could not read account: open accounts-store/accounts/%v: no such file or directory\",\"status\":\"Not Found\"}", id), - err.Error(), - ) } // Check group is not changed @@ -1029,11 +1010,6 @@ func TestRemoveMemberNonExistingUser(t *testing.T) { assert.IsType(t, &proto.Group{}, res) assert.Empty(t, res) assert.Error(t, err) - assert.Equal( - t, - fmt.Sprintf("{\"id\":\".\",\"code\":404,\"detail\":\"could not read account: open accounts-store/accounts/%v: no such file or directory\",\"status\":\"Not Found\"}", id), - err.Error(), - ) } // Check group is not changed diff --git a/accounts/pkg/service/v0/accounts.go b/accounts/pkg/service/v0/accounts.go index 07e887096a..3f85b868db 100644 --- a/accounts/pkg/service/v0/accounts.go +++ b/accounts/pkg/service/v0/accounts.go @@ -3,7 +3,7 @@ package service import ( "context" "fmt" - "os" + "github.com/owncloud/ocis/accounts/pkg/storage" "path/filepath" "regexp" "sync" @@ -210,8 +210,12 @@ func (s Service) GetAccount(ctx context.Context, in *proto.GetAccountRequest, ou } 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()) + } + s.log.Error().Err(err).Str("id", id).Msg("could not load account") - return + return merrors.InternalServerError(s.id, "could not load account: %v", err.Error()) } s.debugLogAccount(out).Msg("found account") @@ -277,7 +281,7 @@ func (s Service) CreateAccount(ctx context.Context, in *proto.CreateAccountReque if err = s.repo.WriteAccount(ctx, acc); err != nil { s.log.Error().Err(err).Str("id", id).Msg("could not persist new account") s.debugLogAccount(acc).Msg("could not persist new account") - return + return merrors.InternalServerError(s.id, "could not persist new account: %v", err.Error()) } if err = s.indexAccount(acc.Id); err != nil { return merrors.InternalServerError(s.id, "could not index new account: %v", err.Error()) @@ -336,8 +340,13 @@ func (s Service) UpdateAccount(ctx context.Context, in *proto.UpdateAccountReque 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()) + } + s.log.Error().Err(err).Str("id", id).Msg("could not load account") - return + return merrors.InternalServerError(s.id, "could not load account: %v", err.Error()) + } t := time.Now() @@ -391,7 +400,7 @@ func (s Service) UpdateAccount(ctx context.Context, in *proto.UpdateAccountReque if err = s.repo.WriteAccount(ctx, out); err != nil { s.log.Error().Err(err).Str("id", out.Id).Msg("could not persist updated account") - return + return merrors.InternalServerError(s.id, "could not persist updated account: %v", err.Error()) } if err = s.indexAccount(id); err != nil { @@ -438,12 +447,15 @@ func (s Service) DeleteAccount(ctx context.Context, in *proto.DeleteAccountReque if id, err = cleanupID(in.Id); err != nil { return merrors.InternalServerError(s.id, "could not clean up account id: %v", err.Error()) } - path := filepath.Join(s.Config.Server.AccountsDataPath, "accounts", id) a := &proto.Account{} if err = s.repo.LoadAccount(ctx, id, a); err != nil { + if storage.IsNotFoundErr(err) { + return merrors.NotFound(s.id, "account not found: %v", err.Error()) + } + s.log.Error().Err(err).Str("id", id).Msg("could not load account") - return + return merrors.InternalServerError(s.id, "could not load account: %v", err.Error()) } // delete member relationship in groups @@ -457,13 +469,17 @@ func (s Service) DeleteAccount(ctx context.Context, in *proto.DeleteAccountReque } } - if err = os.Remove(path); err != nil { - s.log.Error().Err(err).Str("id", id).Str("path", path).Msg("could not remove account") + if err = s.repo.DeleteAccount(ctx, id); err != nil { + if storage.IsNotFoundErr(err) { + return merrors.NotFound(s.id, "account not found: %v", err.Error()) + } + + s.log.Error().Err(err).Str("id", id).Str("accountId", id).Msg("could not remove account") return merrors.InternalServerError(s.id, "could not remove account: %v", err.Error()) } if err = s.index.Delete(id); err != nil { - s.log.Error().Err(err).Str("id", id).Str("path", path).Msg("could not remove account from index") + s.log.Error().Err(err).Str("id", id).Str("accountId", id).Msg("could not remove account from index") return merrors.InternalServerError(s.id, "could not remove account from index: %v", err.Error()) } diff --git a/accounts/pkg/service/v0/accounts_permission_test.go b/accounts/pkg/service/v0/accounts_permission_test.go index 3e3f2eaacd..10e05bcc5c 100644 --- a/accounts/pkg/service/v0/accounts_permission_test.go +++ b/accounts/pkg/service/v0/accounts_permission_test.go @@ -34,6 +34,7 @@ func init() { cfg := config.New() cfg.Server.Name = "accounts" cfg.Server.AccountsDataPath = dataPath + cfg.Repo.Disk.Path = dataPath logger := olog.NewLogger(olog.Color(true), olog.Pretty(true)) roleServiceMock = buildRoleServiceMock() roleManager := roles.NewManager( diff --git a/accounts/pkg/service/v0/groups.go b/accounts/pkg/service/v0/groups.go index 5003018dd7..357ba55504 100644 --- a/accounts/pkg/service/v0/groups.go +++ b/accounts/pkg/service/v0/groups.go @@ -2,6 +2,7 @@ package service import ( "context" + "github.com/owncloud/ocis/accounts/pkg/storage" "path/filepath" "github.com/CiscoM31/godata" @@ -131,8 +132,12 @@ func (s Service) GetGroup(c context.Context, in *proto.GetGroupRequest, out *pro } if err = s.repo.LoadGroup(c, id, out); err != nil { + if storage.IsNotFoundErr(err) { + return merrors.NotFound(s.id, "group not found: %v", err.Error()) + } + s.log.Error().Err(err).Str("id", id).Msg("could not load group") - return + return merrors.InternalServerError(s.id, "could not load group: %v", err.Error()) } s.log.Debug().Interface("group", out).Msg("found group") @@ -162,7 +167,7 @@ func (s Service) CreateGroup(c context.Context, in *proto.CreateGroupRequest, ou if err = s.repo.WriteGroup(c, in.Group); err != nil { s.log.Error().Err(err).Interface("group", in.Group).Msg("could not persist new group") - return + return merrors.InternalServerError(s.id, "could not persist new group: %v", err.Error()) } if err = s.indexGroup(id); err != nil { @@ -187,8 +192,11 @@ func (s Service) DeleteGroup(c context.Context, in *proto.DeleteGroupRequest, ou g := &proto.Group{} if err = s.repo.LoadGroup(c, id, g); err != nil { - s.log.Error().Err(err).Str("id", id).Msg("could not load account") - return + if storage.IsNotFoundErr(err) { + return merrors.NotFound(s.id, "group not found: %v", err.Error()) + } + s.log.Error().Err(err).Str("id", id).Msg("could not load group") + return merrors.InternalServerError(s.id, "could not load group: %v", err.Error()) } // delete memberof relationship in users @@ -203,7 +211,11 @@ func (s Service) DeleteGroup(c context.Context, in *proto.DeleteGroupRequest, ou } if err = s.repo.DeleteGroup(c, id); err != nil { - return err + if storage.IsNotFoundErr(err) { + return merrors.NotFound(s.id, "group not found: %v", err.Error()) + } + + return merrors.InternalServerError(s.id, "could not load group: %v", err.Error()) } if err = s.index.Delete(id); err != nil { @@ -217,7 +229,6 @@ func (s Service) DeleteGroup(c context.Context, in *proto.DeleteGroupRequest, ou // AddMember implements the GroupsServiceHandler interface func (s Service) AddMember(c context.Context, in *proto.AddMemberRequest, out *proto.Group) (err error) { - // cleanup ids var groupID string if groupID, err = cleanupID(in.GroupId); err != nil { @@ -232,14 +243,20 @@ func (s Service) AddMember(c context.Context, in *proto.AddMemberRequest, out *p // load structs a := &proto.Account{} if err = s.repo.LoadAccount(c, accountID, a); err != nil { + if storage.IsNotFoundErr(err) { + return merrors.NotFound(s.id, "group not found: %v", err.Error()) + } s.log.Error().Err(err).Str("id", accountID).Msg("could not load account") - return + return merrors.InternalServerError(s.id, "could not load group: %v", err.Error()) } g := &proto.Group{} if err = s.repo.LoadGroup(c, groupID, g); err != nil { + if storage.IsNotFoundErr(err) { + return merrors.NotFound(s.id, "could not load group: %v", err.Error()) + } s.log.Error().Err(err).Str("id", groupID).Msg("could not load group") - return + return merrors.InternalServerError(s.id, "could not load group: %v", err.Error()) } // check if we need to add the account to the group @@ -267,11 +284,11 @@ func (s Service) AddMember(c context.Context, in *proto.AddMemberRequest, out *p if err = s.repo.WriteAccount(c, a); err != nil { s.log.Error().Err(err).Interface("account", a).Msg("could not persist account") - return + return merrors.InternalServerError(s.id, "could not persist updated account: %v", err.Error()) } if err = s.repo.WriteGroup(c, g); err != nil { s.log.Error().Err(err).Interface("group", g).Msg("could not persist group") - return + return merrors.InternalServerError(s.id, "could not persist group: %v", err.Error()) } // FIXME update index! // TODO rollback changes when only one of them failed? @@ -297,14 +314,20 @@ func (s Service) RemoveMember(c context.Context, in *proto.RemoveMemberRequest, // load structs a := &proto.Account{} if err = s.repo.LoadAccount(c, accountID, a); err != nil { + if storage.IsNotFoundErr(err) { + return merrors.NotFound(s.id, "could not load account: %v", err.Error()) + } s.log.Error().Err(err).Str("id", accountID).Msg("could not load account") - return + return merrors.InternalServerError(s.id, "could not load account: %v", err.Error()) } g := &proto.Group{} if err = s.repo.LoadGroup(c, groupID, g); err != nil { + if storage.IsNotFoundErr(err) { + return merrors.NotFound(s.id, "could not load group: %v", err.Error()) + } s.log.Error().Err(err).Str("id", groupID).Msg("could not load group") - return + return merrors.InternalServerError(s.id, "could not load group: %v", err.Error()) } //remove the account from the group if it exists @@ -327,11 +350,11 @@ func (s Service) RemoveMember(c context.Context, in *proto.RemoveMemberRequest, if err = s.repo.WriteAccount(c, a); err != nil { s.log.Error().Err(err).Interface("account", a).Msg("could not persist account") - return + return merrors.InternalServerError(s.id, "could not persist account: %v", err.Error()) } if err = s.repo.WriteGroup(c, g); err != nil { s.log.Error().Err(err).Interface("group", g).Msg("could not persist group") - return + return merrors.InternalServerError(s.id, "could not persist group: %v", err.Error()) } // FIXME update index! // TODO rollback changes when only one of them failed? @@ -342,7 +365,6 @@ func (s Service) RemoveMember(c context.Context, in *proto.RemoveMemberRequest, // ListMembers implements the GroupsServiceHandler interface func (s Service) ListMembers(c context.Context, in *proto.ListMembersRequest, out *proto.ListMembersResponse) (err error) { - // cleanup ids var groupID string if groupID, err = cleanupID(in.Id); err != nil { @@ -351,15 +373,16 @@ func (s Service) ListMembers(c context.Context, in *proto.ListMembersRequest, ou g := &proto.Group{} if err = s.repo.LoadGroup(c, groupID, g); err != nil { + if storage.IsNotFoundErr(err) { + return merrors.NotFound(s.id, "group not found: %v", err.Error()) + } s.log.Error().Err(err).Str("id", groupID).Msg("could not load group") - return + return merrors.InternalServerError(s.id, "could not load group: %v", err.Error()) } // TODO only expand accounts if requested // if in.FieldMask ... s.expandMembers(g) - out.Members = g.Members - return } diff --git a/accounts/pkg/storage/cs3.go b/accounts/pkg/storage/cs3.go index 87835f98cd..6fdb4bc729 100644 --- a/accounts/pkg/storage/cs3.go +++ b/accounts/pkg/storage/cs3.go @@ -16,7 +16,6 @@ import ( "github.com/cs3org/reva/pkg/rgrpc/todo/pool" "github.com/cs3org/reva/pkg/token" "github.com/cs3org/reva/pkg/token/manager/jwt" - merrors "github.com/micro/go-micro/v2/errors" "github.com/owncloud/ocis/accounts/pkg/config" "github.com/owncloud/ocis/accounts/pkg/proto/v0" "google.golang.org/grpc/metadata" @@ -67,7 +66,7 @@ func (r CS3Repo) WriteAccount(ctx context.Context, a *proto.Account) (err error) var by []byte if by, err = json.Marshal(a); err != nil { - return merrors.InternalServerError(r.serviceID, "could not marshal account: %v", err.Error()) + return err } ureq, err := http.NewRequest("PUT", r.accountURL(a.Id), bytes.NewReader(by)) @@ -109,6 +108,10 @@ func (r CS3Repo) LoadAccount(ctx context.Context, id string, a *proto.Account) ( return err } + if resp.StatusCode == http.StatusNotFound { + return ¬FoundErr{"account", id} + } + b, err := ioutil.ReadAll(resp.Body) if err != nil { return err @@ -131,12 +134,22 @@ func (r CS3Repo) DeleteAccount(ctx context.Context, id string) (err error) { ctx = metadata.AppendToOutgoingContext(ctx, token.TokenHeader, t) - _, err = r.storageClient.Delete(ctx, &provider.DeleteRequest{ + resp, err := r.storageClient.Delete(ctx, &provider.DeleteRequest{ Ref: &provider.Reference{ Spec: &provider.Reference_Path{Path: fmt.Sprintf("/meta/%s/%s", accountsFolder, id)}, }, }) - return err + + if err != nil { + return err + } + + // TODO Handle other error codes? + if resp.Status.Code == v1beta11.Code_CODE_NOT_FOUND { + return ¬FoundErr{"account", id} + } + + return nil } // WriteGroup writes a group via cs3 and modifies the provided group (e.g. with a generated id). @@ -153,7 +166,7 @@ func (r CS3Repo) WriteGroup(ctx context.Context, g *proto.Group) (err error) { var by []byte if by, err = json.Marshal(g); err != nil { - return merrors.InternalServerError(r.serviceID, "could not marshal group: %v", err.Error()) + return err } ureq, err := http.NewRequest("PUT", r.groupURL(g.Id), bytes.NewReader(by)) @@ -195,6 +208,10 @@ func (r CS3Repo) LoadGroup(ctx context.Context, id string, g *proto.Group) (err return err } + if resp.StatusCode == http.StatusNotFound { + return ¬FoundErr{"group", id} + } + b, err := ioutil.ReadAll(resp.Body) if err != nil { return err @@ -217,11 +234,21 @@ func (r CS3Repo) DeleteGroup(ctx context.Context, id string) (err error) { ctx = metadata.AppendToOutgoingContext(ctx, token.TokenHeader, t) - _, err = r.storageClient.Delete(ctx, &provider.DeleteRequest{ + resp, err := r.storageClient.Delete(ctx, &provider.DeleteRequest{ Ref: &provider.Reference{ Spec: &provider.Reference_Path{Path: fmt.Sprintf("/meta/%s/%s", groupsFolder, id)}, }, }) + + if err != nil { + return err + } + + // TODO Handle other error codes? + if resp.Status.Code == v1beta11.Code_CODE_NOT_FOUND { + return ¬FoundErr{"group", id} + } + return err } diff --git a/accounts/pkg/storage/cs3_test.go b/accounts/pkg/storage/cs3_test.go index 627cc56962..af576acfbc 100644 --- a/accounts/pkg/storage/cs3_test.go +++ b/accounts/pkg/storage/cs3_test.go @@ -1,9 +1,26 @@ package storage +// Uncomment to test locally, requires started metadata-storage for now + +//import ( +// "context" +// "github.com/owncloud/ocis/accounts/pkg/config" +// "github.com/owncloud/ocis/accounts/pkg/proto/v0" +// "github.com/stretchr/testify/assert" +// "testing" +//) +// //var cfg = &config.Config{ // TokenManager: config.TokenManager{ // JWTSecret: "Pive-Fumkiu4", // }, +// Repo: config.Repo{ +// CS3: config.CS3{ +// ProviderAddr: "0.0.0.0:9215", +// DriverURL: "http://localhost:9216", +// DataPrefix: "data", +// }, +// }, //} // //func TestCS3Repo_WriteAccount(t *testing.T) { diff --git a/accounts/pkg/storage/disk.go b/accounts/pkg/storage/disk.go index 09e037d5e7..fdab3200dc 100644 --- a/accounts/pkg/storage/disk.go +++ b/accounts/pkg/storage/disk.go @@ -9,7 +9,6 @@ import ( "path/filepath" "sync" - merrors "github.com/micro/go-micro/v2/errors" "github.com/owncloud/ocis/accounts/pkg/proto/v0" olog "github.com/owncloud/ocis/ocis-pkg/log" ) @@ -52,41 +51,40 @@ func (r DiskRepo) WriteAccount(ctx context.Context, a *proto.Account) (err error var bytes []byte if bytes, err = json.Marshal(a); err != nil { - return merrors.InternalServerError(r.serviceID, "could not marshal account: %v", err.Error()) + return err } path := filepath.Join(r.cfg.Repo.Disk.Path, accountsFolder, a.Id) - - if err = ioutil.WriteFile(path, bytes, 0600); err != nil { - return merrors.InternalServerError(r.serviceID, "could not write account: %v", err.Error()) - } - return + return ioutil.WriteFile(path, bytes, 0600) } // LoadAccount from the local filesystem func (r DiskRepo) LoadAccount(ctx context.Context, id string, a *proto.Account) (err error) { path := filepath.Join(r.cfg.Repo.Disk.Path, accountsFolder, id) - var data []byte if data, err = ioutil.ReadFile(path); err != nil { - return merrors.NotFound(r.serviceID, "could not read account: %v", err.Error()) + if os.IsNotExist(err) { + err = ¬FoundErr{"account", id} + } + return } - if err = json.Unmarshal(data, a); err != nil { - return merrors.InternalServerError(r.serviceID, "could not unmarshal account: %v", err.Error()) - } - return + return json.Unmarshal(data, a) } // DeleteAccount from the local filesystem func (r DiskRepo) DeleteAccount(ctx context.Context, id string) (err error) { path := filepath.Join(r.cfg.Repo.Disk.Path, accountsFolder, id) if err = os.Remove(path); err != nil { - r.log.Error().Err(err).Str("id", id).Str("path", path).Msg("could not remove account") - return merrors.InternalServerError(r.serviceID, "could not remove account: %v", err.Error()) + if os.IsNotExist(err) { + err = ¬FoundErr{"account", id} + } } - return nil + //r.log.Error().Err(err).Str("id", id).Str("path", path).Msg("could not remove account") + //return merrors.InternalServerError(r.serviceID, "could not remove account: %v", err.Error()) + + return } // WriteGroup to the local filesystem @@ -96,17 +94,19 @@ func (r DiskRepo) WriteGroup(ctx context.Context, g *proto.Group) (err error) { var bytes []byte if bytes, err = json.Marshal(g); err != nil { - return merrors.InternalServerError(r.serviceID, "could not marshal group: %v", err.Error()) + return err } path := filepath.Join(r.cfg.Repo.Disk.Path, groupsFolder, g.Id) groupLock.Lock() defer groupLock.Unlock() - if err = ioutil.WriteFile(path, bytes, 0600); err != nil { - return merrors.InternalServerError(r.serviceID, "could not write group: %v", err.Error()) - } - return + + return ioutil.WriteFile(path, bytes, 0600) + + //return merrors.InternalServerError(r.serviceID, "could not marshal group: %v", err.Error()) + + //return merrors.InternalServerError(r.serviceID, "could not write group: %v", err.Error()) } // LoadGroup from the local filesystem @@ -117,25 +117,29 @@ func (r DiskRepo) LoadGroup(ctx context.Context, id string, g *proto.Group) (err defer groupLock.Unlock() var data []byte if data, err = ioutil.ReadFile(path); err != nil { - return merrors.NotFound(r.serviceID, "could not read group: %v", err.Error()) + if os.IsNotExist(err) { + err = ¬FoundErr{"group", id} + } + + return } - if err = json.Unmarshal(data, g); err != nil { - return merrors.InternalServerError(r.serviceID, "could not unmarshal group: %v", err.Error()) - } - - return + return json.Unmarshal(data, g) } // DeleteGroup from the local filesystem func (r DiskRepo) DeleteGroup(ctx context.Context, id string) (err error) { path := filepath.Join(r.cfg.Repo.Disk.Path, groupsFolder, id) if err = os.Remove(path); err != nil { - r.log.Error().Err(err).Str("id", id).Str("path", path).Msg("could not remove group") - return merrors.InternalServerError(r.serviceID, "could not remove group: %v", err.Error()) + if os.IsNotExist(err) { + err = ¬FoundErr{"account", id} + } } return nil + + //r.log.Error().Err(err).Str("id", id).Str("path", path).Msg("could not remove group") + //return merrors.InternalServerError(r.serviceID, "could not remove group: %v", err.Error()) } // deflateMemberOf replaces the groups of a user with an instance that only contains the id diff --git a/accounts/pkg/storage/errors.go b/accounts/pkg/storage/errors.go new file mode 100644 index 0000000000..3dfa306e56 --- /dev/null +++ b/accounts/pkg/storage/errors.go @@ -0,0 +1,19 @@ +package storage + +import ( + "fmt" +) + +type notFoundErr struct { + typ, id string +} + +func (e *notFoundErr) Error() string { + return fmt.Sprintf("%s with id %s not found", e.typ, e.id) +} + +// IsNotFoundErr can be returned by repo Load and Delete operations +func IsNotFoundErr(e error) bool { + _, ok := e.(*notFoundErr) + return ok +} From 403ccc662e86b1a3fb440f4409193b869fac7265 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Wed, 30 Sep 2020 14:44:35 +0200 Subject: [PATCH 17/28] Remove commented code --- accounts/pkg/storage/disk.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/accounts/pkg/storage/disk.go b/accounts/pkg/storage/disk.go index fdab3200dc..2b0265324d 100644 --- a/accounts/pkg/storage/disk.go +++ b/accounts/pkg/storage/disk.go @@ -81,9 +81,6 @@ func (r DiskRepo) DeleteAccount(ctx context.Context, id string) (err error) { } } - //r.log.Error().Err(err).Str("id", id).Str("path", path).Msg("could not remove account") - //return merrors.InternalServerError(r.serviceID, "could not remove account: %v", err.Error()) - return } @@ -103,10 +100,6 @@ func (r DiskRepo) WriteGroup(ctx context.Context, g *proto.Group) (err error) { defer groupLock.Unlock() return ioutil.WriteFile(path, bytes, 0600) - - //return merrors.InternalServerError(r.serviceID, "could not marshal group: %v", err.Error()) - - //return merrors.InternalServerError(r.serviceID, "could not write group: %v", err.Error()) } // LoadGroup from the local filesystem From 9c64f6a0b510668279bce7f92749876d98e215b6 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Wed, 30 Sep 2020 15:38:52 +0200 Subject: [PATCH 18/28] Create data-provider client --- accounts/pkg/storage/cs3.go | 120 +++++++++++++++--------------------- 1 file changed, 51 insertions(+), 69 deletions(-) diff --git a/accounts/pkg/storage/cs3.go b/accounts/pkg/storage/cs3.go index 6fdb4bc729..bb5c346cdd 100644 --- a/accounts/pkg/storage/cs3.go +++ b/accounts/pkg/storage/cs3.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "fmt" + "io" "io/ioutil" "net/http" "path" @@ -23,10 +24,11 @@ import ( // CS3Repo provides a cs3 implementation of the Repo interface type CS3Repo struct { - serviceID string - cfg *config.Config - tm token.Manager - storageClient provider.ProviderAPIClient + serviceID string + cfg *config.Config + tm token.Manager + storageProvider provider.ProviderAPIClient + dataProvider dataProviderClient // Used to create and download data via http, bypassing reva upload protocol } // NewCS3Repo creates a new cs3 repo @@ -45,10 +47,15 @@ func NewCS3Repo(serviceID string, cfg *config.Config) (Repo, error) { } return CS3Repo{ - serviceID: serviceID, - cfg: cfg, - tm: tokenManager, - storageClient: client, + serviceID: serviceID, + cfg: cfg, + tm: tokenManager, + storageProvider: client, + dataProvider: dataProviderClient{ + client: http.Client{ + Transport: http.DefaultTransport, + }, + }, }, nil } @@ -69,21 +76,8 @@ func (r CS3Repo) WriteAccount(ctx context.Context, a *proto.Account) (err error) return err } - ureq, err := http.NewRequest("PUT", r.accountURL(a.Id), bytes.NewReader(by)) - if err != nil { - return err - } - - ureq.Header.Add("x-access-token", t) - cl := http.Client{ - Transport: http.DefaultTransport, - } - - if _, err := cl.Do(ureq); err != nil { - return err - } - - return nil + _, err = r.dataProvider.put(r.accountURL(a.Id), bytes.NewReader(by), t) + return err } // LoadAccount loads an account via cs3 by id and writes it to the provided account @@ -93,17 +87,7 @@ func (r CS3Repo) LoadAccount(ctx context.Context, id string, a *proto.Account) ( return err } - ureq, err := http.NewRequest("GET", r.accountURL(id), nil) - if err != nil { - return err - } - - ureq.Header.Add("x-access-token", t) - cl := http.Client{ - Transport: http.DefaultTransport, - } - - resp, err := cl.Do(ureq) + resp, err := r.dataProvider.get(r.accountURL(id), t) if err != nil { return err } @@ -134,7 +118,7 @@ func (r CS3Repo) DeleteAccount(ctx context.Context, id string) (err error) { ctx = metadata.AppendToOutgoingContext(ctx, token.TokenHeader, t) - resp, err := r.storageClient.Delete(ctx, &provider.DeleteRequest{ + resp, err := r.storageProvider.Delete(ctx, &provider.DeleteRequest{ Ref: &provider.Reference{ Spec: &provider.Reference_Path{Path: fmt.Sprintf("/meta/%s/%s", accountsFolder, id)}, }, @@ -169,21 +153,8 @@ func (r CS3Repo) WriteGroup(ctx context.Context, g *proto.Group) (err error) { return err } - ureq, err := http.NewRequest("PUT", r.groupURL(g.Id), bytes.NewReader(by)) - if err != nil { - return err - } - - ureq.Header.Add("x-access-token", t) - cl := http.Client{ - Transport: http.DefaultTransport, - } - - if _, err := cl.Do(ureq); err != nil { - return err - } - - return nil + _, err = r.dataProvider.put(r.groupURL(g.Id), bytes.NewReader(by), t) + return err } // LoadGroup loads a group via cs3 by id and writes it to the provided group @@ -193,17 +164,7 @@ func (r CS3Repo) LoadGroup(ctx context.Context, id string, g *proto.Group) (err return err } - ureq, err := http.NewRequest("GET", r.groupURL(id), nil) - if err != nil { - return err - } - - ureq.Header.Add("x-access-token", t) - cl := http.Client{ - Transport: http.DefaultTransport, - } - - resp, err := cl.Do(ureq) + resp, err := r.dataProvider.get(r.groupURL(id), t) if err != nil { return err } @@ -216,13 +177,10 @@ func (r CS3Repo) LoadGroup(ctx context.Context, id string, g *proto.Group) (err if err != nil { return err } + defer resp.Body.Close() - if err := json.Unmarshal(b, &g); err != nil { - return err - } - - return nil + return json.Unmarshal(b, &g) } // DeleteGroup deletes a group via cs3 by id @@ -234,7 +192,7 @@ func (r CS3Repo) DeleteGroup(ctx context.Context, id string) (err error) { ctx = metadata.AppendToOutgoingContext(ctx, token.TokenHeader, t) - resp, err := r.storageClient.Delete(ctx, &provider.DeleteRequest{ + resp, err := r.storageProvider.Delete(ctx, &provider.DeleteRequest{ Ref: &provider.Reference{ Spec: &provider.Reference_Path{Path: fmt.Sprintf("/meta/%s/%s", groupsFolder, id)}, }, @@ -272,7 +230,7 @@ func (r CS3Repo) makeRootDirIfNotExist(ctx context.Context, folder string) error Spec: &provider.Reference_Path{Path: fmt.Sprintf("/meta/%v", folder)}, } - resp, err := r.storageClient.Stat(ctx, &provider.StatRequest{ + resp, err := r.storageProvider.Stat(ctx, &provider.StatRequest{ Ref: rootPathRef, }) @@ -281,7 +239,7 @@ func (r CS3Repo) makeRootDirIfNotExist(ctx context.Context, folder string) error } if resp.Status.Code == v1beta11.Code_CODE_NOT_FOUND { - _, err := r.storageClient.CreateContainer(ctx, &provider.CreateContainerRequest{ + _, err := r.storageProvider.CreateContainer(ctx, &provider.CreateContainerRequest{ Ref: rootPathRef, }) @@ -305,3 +263,27 @@ func singleJoiningSlash(a, b string) string { } return a + b } + +type dataProviderClient struct { + client http.Client +} + +func (d dataProviderClient) put(url string, body io.Reader, token string) (*http.Response, error) { + req, err := http.NewRequest(http.MethodPut, url, body) + if err != nil { + return nil, err + } + + req.Header.Add("x-access-token", token) + return d.client.Do(req) +} + +func (d dataProviderClient) get(url string, token string) (*http.Response, error) { + req, err := http.NewRequest(http.MethodGet, url, nil) + if err != nil { + return nil, err + } + + req.Header.Add("x-access-token", token) + return d.client.Do(req) +} From 60a3f3cc6e4396d2f46fb6608db30b61fdf7390b Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Thu, 1 Oct 2020 08:37:09 +0200 Subject: [PATCH 19/28] Fix config variable naming --- accounts/pkg/config/config.go | 2 +- accounts/pkg/flagset/flagset.go | 10 +++++----- accounts/pkg/service/v0/accounts.go | 2 +- accounts/pkg/storage/cs3.go | 4 ++-- accounts/pkg/storage/cs3_test.go | 2 +- accounts/pkg/storage/disk.go | 2 +- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/accounts/pkg/config/config.go b/accounts/pkg/config/config.go index 956c12cea6..2b8fcfb7cd 100644 --- a/accounts/pkg/config/config.go +++ b/accounts/pkg/config/config.go @@ -75,7 +75,7 @@ type Disk struct { // CS3 is the cs3 implementation of the storage. type CS3 struct { ProviderAddr string - DriverURL string + DataURL string DataPrefix string } diff --git a/accounts/pkg/flagset/flagset.go b/accounts/pkg/flagset/flagset.go index 7956d58ff0..885a079229 100644 --- a/accounts/pkg/flagset/flagset.go +++ b/accounts/pkg/flagset/flagset.go @@ -108,17 +108,17 @@ func ServerWithConfig(cfg *config.Config) []cli.Flag { }, &cli.StringFlag{ Name: "storage-cs3-provider-addr", - Value: "0.0.0.0:9215", + Value: "localhost:9215", Usage: "bind address for the metadata storage provider", EnvVars: []string{"ACCOUNTS_STORAGE_CS3_PROVIDER_ADDR"}, Destination: &cfg.Repo.CS3.ProviderAddr, }, &cli.StringFlag{ - Name: "storage-cs3-driver-url", + Name: "storage-cs3-data-url", Value: "http://localhost:9216", - Usage: "http endpoint of the metadata storage, without trailing slash", - EnvVars: []string{"ACCOUNTS_STORAGE_CS3_DRIVER_URL"}, - Destination: &cfg.Repo.CS3.DriverURL, + Usage: "http endpoint of the metadata storage", + EnvVars: []string{"ACCOUNTS_STORAGE_CS3_DATA_URL"}, + Destination: &cfg.Repo.CS3.DataURL, }, &cli.StringFlag{ Name: "storage-cs3-data-prefix", diff --git a/accounts/pkg/service/v0/accounts.go b/accounts/pkg/service/v0/accounts.go index 3f85b868db..f8a4579e79 100644 --- a/accounts/pkg/service/v0/accounts.go +++ b/accounts/pkg/service/v0/accounts.go @@ -3,7 +3,6 @@ package service import ( "context" "fmt" - "github.com/owncloud/ocis/accounts/pkg/storage" "path/filepath" "regexp" "sync" @@ -17,6 +16,7 @@ import ( merrors "github.com/micro/go-micro/v2/errors" "github.com/owncloud/ocis/accounts/pkg/proto/v0" "github.com/owncloud/ocis/accounts/pkg/provider" + "github.com/owncloud/ocis/accounts/pkg/storage" "github.com/owncloud/ocis/ocis-pkg/roles" settings "github.com/owncloud/ocis/settings/pkg/proto/v0" settings_svc "github.com/owncloud/ocis/settings/pkg/service/v0" diff --git a/accounts/pkg/storage/cs3.go b/accounts/pkg/storage/cs3.go index bb5c346cdd..12609a9f2b 100644 --- a/accounts/pkg/storage/cs3.go +++ b/accounts/pkg/storage/cs3.go @@ -218,11 +218,11 @@ func (r CS3Repo) authenticate(ctx context.Context) (token string, err error) { } func (r CS3Repo) accountURL(id string) string { - return singleJoiningSlash(r.cfg.Repo.CS3.DriverURL, path.Join(r.cfg.Repo.CS3.DataPrefix, accountsFolder, id)) + return singleJoiningSlash(r.cfg.Repo.CS3.DataURL, path.Join(r.cfg.Repo.CS3.DataPrefix, accountsFolder, id)) } func (r CS3Repo) groupURL(id string) string { - return singleJoiningSlash(r.cfg.Repo.CS3.DriverURL, path.Join(r.cfg.Repo.CS3.DataPrefix, groupsFolder, id)) + return singleJoiningSlash(r.cfg.Repo.CS3.DataURL, path.Join(r.cfg.Repo.CS3.DataPrefix, groupsFolder, id)) } func (r CS3Repo) makeRootDirIfNotExist(ctx context.Context, folder string) error { diff --git a/accounts/pkg/storage/cs3_test.go b/accounts/pkg/storage/cs3_test.go index af576acfbc..d788dc7ac3 100644 --- a/accounts/pkg/storage/cs3_test.go +++ b/accounts/pkg/storage/cs3_test.go @@ -17,7 +17,7 @@ package storage // Repo: config.Repo{ // CS3: config.CS3{ // ProviderAddr: "0.0.0.0:9215", -// DriverURL: "http://localhost:9216", +// DataURL: "http://localhost:9216", // DataPrefix: "data", // }, // }, diff --git a/accounts/pkg/storage/disk.go b/accounts/pkg/storage/disk.go index 2b0265324d..df3288936c 100644 --- a/accounts/pkg/storage/disk.go +++ b/accounts/pkg/storage/disk.go @@ -3,12 +3,12 @@ package storage import ( "context" "encoding/json" - "github.com/owncloud/ocis/accounts/pkg/config" "io/ioutil" "os" "path/filepath" "sync" + "github.com/owncloud/ocis/accounts/pkg/config" "github.com/owncloud/ocis/accounts/pkg/proto/v0" olog "github.com/owncloud/ocis/ocis-pkg/log" ) From 673d7cf50c25d795ed4a020499f76f3413fba5ca Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Thu, 1 Oct 2020 09:35:33 +0200 Subject: [PATCH 20/28] Hardcoded service user for auth requests --- accounts/pkg/config/config.go | 9 +++++++++ accounts/pkg/flagset/flagset.go | 28 ++++++++++++++++++++++++++++ accounts/pkg/service/v0/accounts.go | 18 ++++++++++++++++++ 3 files changed, 55 insertions(+) diff --git a/accounts/pkg/config/config.go b/accounts/pkg/config/config.go index 2b8fcfb7cd..d2df73c18b 100644 --- a/accounts/pkg/config/config.go +++ b/accounts/pkg/config/config.go @@ -79,6 +79,14 @@ type CS3 struct { DataPrefix string } +// ServiceUser defines the user required for EOS +type ServiceUser struct { + Username string + Password string + UID int64 + GID int64 +} + // Config merges all Account config parameters. type Config struct { LDAP LDAP @@ -89,6 +97,7 @@ type Config struct { Log Log TokenManager TokenManager Repo Repo + ServiceUser ServiceUser } // New returns a new config. diff --git a/accounts/pkg/flagset/flagset.go b/accounts/pkg/flagset/flagset.go index 885a079229..6b659592a0 100644 --- a/accounts/pkg/flagset/flagset.go +++ b/accounts/pkg/flagset/flagset.go @@ -127,6 +127,34 @@ func ServerWithConfig(cfg *config.Config) []cli.Flag { EnvVars: []string{"ACCOUNTS_STORAGE_CS3_DATA_PREFIX"}, Destination: &cfg.Repo.CS3.DataPrefix, }, + &cli.StringFlag{ + Name: "service-user-username", + Value: "", + Usage: "username of the internal service user (required on EOS)", + EnvVars: []string{"ACCOUNTS_SERVICE_USER_USERNAME"}, + Destination: &cfg.ServiceUser.Username, + }, + &cli.StringFlag{ + Name: "service-user-password", + Value: "", + Usage: "password of the internal service user (required on EOS)", + EnvVars: []string{"ACCOUNTS_SERVICE_USER_PASSWORD"}, + Destination: &cfg.ServiceUser.Password, + }, + &cli.Int64Flag{ + Name: "service-user-uid", + Value: 0, + Usage: "uid of the internal service user (required on EOS)", + EnvVars: []string{"ACCOUNTS_SERVICE_USER_UID"}, + Destination: &cfg.ServiceUser.UID, + }, + &cli.Int64Flag{ + Name: "service-user-gid", + Value: 0, + Usage: "gid of the internal service user (required on EOS)", + EnvVars: []string{"ACCOUNTS_SERVICE_USER_GID"}, + Destination: &cfg.ServiceUser.GID, + }, } } diff --git a/accounts/pkg/service/v0/accounts.go b/accounts/pkg/service/v0/accounts.go index f8a4579e79..5ff29c5193 100644 --- a/accounts/pkg/service/v0/accounts.go +++ b/accounts/pkg/service/v0/accounts.go @@ -5,6 +5,7 @@ import ( "fmt" "path/filepath" "regexp" + "strings" "sync" "time" @@ -120,6 +121,23 @@ func (s Service) ListAccounts(ctx context.Context, in *proto.ListAccountsRequest if password == "" { return merrors.Unauthorized(s.id, "password must not be empty") } + + // hardcoded check against service user + if s.Config.ServiceUser.Username != "" && + strings.EqualFold(match[1], s.Config.ServiceUser.Username) && + match[2] == s.Config.ServiceUser.Password { + out.Accounts = []*proto.Account{ + { + Id: "95cb8724-03b2-11eb-a0a6-c33ef8ef53ad", + AccountEnabled: true, + PreferredName: s.Config.ServiceUser.Username, + DisplayName: s.Config.ServiceUser.Username, + UidNumber: s.Config.ServiceUser.UID, + GidNumber: s.Config.ServiceUser.GID, + }, + } + return nil + } } // only search for accounts From 862329cce0abbf9e3716f9cbca0a66705ff533b5 Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Thu, 1 Oct 2020 11:35:24 +0200 Subject: [PATCH 21/28] Fix service user usage for eos --- accounts/pkg/config/config.go | 2 +- accounts/pkg/flagset/flagset.go | 14 ++++---- accounts/pkg/service/v0/accounts.go | 56 +++++++++++++++++++---------- accounts/pkg/storage/cs3.go | 8 +++-- 4 files changed, 51 insertions(+), 29 deletions(-) diff --git a/accounts/pkg/config/config.go b/accounts/pkg/config/config.go index d2df73c18b..6240b06b1b 100644 --- a/accounts/pkg/config/config.go +++ b/accounts/pkg/config/config.go @@ -81,8 +81,8 @@ type CS3 struct { // ServiceUser defines the user required for EOS type ServiceUser struct { + UUID string Username string - Password string UID int64 GID int64 } diff --git a/accounts/pkg/flagset/flagset.go b/accounts/pkg/flagset/flagset.go index 6b659592a0..e236426368 100644 --- a/accounts/pkg/flagset/flagset.go +++ b/accounts/pkg/flagset/flagset.go @@ -127,6 +127,13 @@ func ServerWithConfig(cfg *config.Config) []cli.Flag { EnvVars: []string{"ACCOUNTS_STORAGE_CS3_DATA_PREFIX"}, Destination: &cfg.Repo.CS3.DataPrefix, }, + &cli.StringFlag{ + Name: "service-user-uuid", + Value: "95cb8724-03b2-11eb-a0a6-c33ef8ef53ad", + Usage: "uuid of the internal service user (required on EOS)", + EnvVars: []string{"ACCOUNTS_SERVICE_USER_UUID"}, + Destination: &cfg.ServiceUser.UUID, + }, &cli.StringFlag{ Name: "service-user-username", Value: "", @@ -134,13 +141,6 @@ func ServerWithConfig(cfg *config.Config) []cli.Flag { EnvVars: []string{"ACCOUNTS_SERVICE_USER_USERNAME"}, Destination: &cfg.ServiceUser.Username, }, - &cli.StringFlag{ - Name: "service-user-password", - Value: "", - Usage: "password of the internal service user (required on EOS)", - EnvVars: []string{"ACCOUNTS_SERVICE_USER_PASSWORD"}, - Destination: &cfg.ServiceUser.Password, - }, &cli.Int64Flag{ Name: "service-user-uid", Value: 0, diff --git a/accounts/pkg/service/v0/accounts.go b/accounts/pkg/service/v0/accounts.go index 5ff29c5193..acdd1c855b 100644 --- a/accounts/pkg/service/v0/accounts.go +++ b/accounts/pkg/service/v0/accounts.go @@ -5,7 +5,6 @@ import ( "fmt" "path/filepath" "regexp" - "strings" "sync" "time" @@ -102,6 +101,36 @@ func (s Service) hasAccountManagementPermissions(ctx context.Context) bool { return s.RoleManager.FindPermissionByID(ctx, roleIDs, AccountManagementPermissionID) != nil } +// serviceUserToIndex temporarily adds a service user to the index, which is supposed to be removed before the lock on the handler function is released +func (s Service) serviceUserToIndex() (teardownServiceUser func()) { + if s.Config.ServiceUser.Username != "" && s.Config.ServiceUser.UUID != "" { + err := s.index.Index(s.Config.ServiceUser.UUID, &proto.BleveAccount{ + BleveType: "account", + Account: s.getInMemoryServiceUser(), + }) + if err != nil { + s.log.Logger.Err(err).Msg("service user was configured but failed to be added to the index") + } else { + return func() { + _ = s.index.Delete(s.Config.ServiceUser.UUID) + } + } + } + return func() {} +} + +func (s Service) getInMemoryServiceUser() proto.Account { + return proto.Account{ + AccountEnabled: true, + Id: s.Config.ServiceUser.UUID, + PreferredName: s.Config.ServiceUser.Username, + OnPremisesSamAccountName: s.Config.ServiceUser.Username, + DisplayName: s.Config.ServiceUser.Username, + UidNumber: s.Config.ServiceUser.UID, + GidNumber: s.Config.ServiceUser.GID, + } +} + // ListAccounts implements the AccountsServiceHandler interface // the query contains account properties func (s Service) ListAccounts(ctx context.Context, in *proto.ListAccountsRequest, out *proto.ListAccountsResponse) (err error) { @@ -113,6 +142,9 @@ func (s Service) ListAccounts(ctx context.Context, in *proto.ListAccountsRequest defer accLock.Unlock() var password string + teardownServiceUser := s.serviceUserToIndex() + defer teardownServiceUser() + // check if this looks like an auth request match := authQuery.FindStringSubmatch(in.Query) if len(match) == 3 { @@ -121,23 +153,6 @@ func (s Service) ListAccounts(ctx context.Context, in *proto.ListAccountsRequest if password == "" { return merrors.Unauthorized(s.id, "password must not be empty") } - - // hardcoded check against service user - if s.Config.ServiceUser.Username != "" && - strings.EqualFold(match[1], s.Config.ServiceUser.Username) && - match[2] == s.Config.ServiceUser.Password { - out.Accounts = []*proto.Account{ - { - Id: "95cb8724-03b2-11eb-a0a6-c33ef8ef53ad", - AccountEnabled: true, - PreferredName: s.Config.ServiceUser.Username, - DisplayName: s.Config.ServiceUser.Username, - UidNumber: s.Config.ServiceUser.UID, - GidNumber: s.Config.ServiceUser.GID, - }, - } - return nil - } } // only search for accounts @@ -179,7 +194,10 @@ func (s Service) ListAccounts(ctx context.Context, in *proto.ListAccountsRequest for _, hit := range searchResult.Hits { a := &proto.Account{} - if err = s.repo.LoadAccount(ctx, hit.ID, a); err != nil { + if hit.ID == s.Config.ServiceUser.UUID { + acc := s.getInMemoryServiceUser() + a = &acc + } else if err = s.repo.LoadAccount(ctx, hit.ID, a); err != nil { s.log.Error().Err(err).Str("account", hit.ID).Msg("could not load account, skipping") continue } diff --git a/accounts/pkg/storage/cs3.go b/accounts/pkg/storage/cs3.go index 12609a9f2b..b1b51fe2a5 100644 --- a/accounts/pkg/storage/cs3.go +++ b/accounts/pkg/storage/cs3.go @@ -211,10 +211,14 @@ func (r CS3Repo) DeleteGroup(ctx context.Context, id string) (err error) { } func (r CS3Repo) authenticate(ctx context.Context) (token string, err error) { - return r.tm.MintToken(ctx, &user.User{ + u := &user.User{ Id: &user.UserId{}, Groups: []string{}, - }) + } + if r.cfg.ServiceUser.Username != "" { + u.Id.OpaqueId = r.cfg.ServiceUser.UUID + } + return r.tm.MintToken(ctx, u) } func (r CS3Repo) accountURL(id string) string { From 14bd4c83f68a8694260d5c6048b1843cf0ca0099 Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Mon, 5 Oct 2020 11:51:23 +0200 Subject: [PATCH 22/28] Get rid of unused serviceID and redundant log lines --- accounts/pkg/service/v0/groups.go | 7 +------ accounts/pkg/service/v0/service.go | 11 +++++------ accounts/pkg/storage/cs3.go | 4 +--- accounts/pkg/storage/disk.go | 4 +--- 4 files changed, 8 insertions(+), 18 deletions(-) diff --git a/accounts/pkg/service/v0/groups.go b/accounts/pkg/service/v0/groups.go index 357ba55504..d6d8302d09 100644 --- a/accounts/pkg/service/v0/groups.go +++ b/accounts/pkg/service/v0/groups.go @@ -195,7 +195,6 @@ func (s Service) DeleteGroup(c context.Context, in *proto.DeleteGroupRequest, ou if storage.IsNotFoundErr(err) { return merrors.NotFound(s.id, "group not found: %v", err.Error()) } - s.log.Error().Err(err).Str("id", id).Msg("could not load group") return merrors.InternalServerError(s.id, "could not load group: %v", err.Error()) } @@ -246,7 +245,6 @@ func (s Service) AddMember(c context.Context, in *proto.AddMemberRequest, out *p if storage.IsNotFoundErr(err) { return merrors.NotFound(s.id, "group not found: %v", err.Error()) } - s.log.Error().Err(err).Str("id", accountID).Msg("could not load account") return merrors.InternalServerError(s.id, "could not load group: %v", err.Error()) } @@ -255,7 +253,6 @@ func (s Service) AddMember(c context.Context, in *proto.AddMemberRequest, out *p if storage.IsNotFoundErr(err) { return merrors.NotFound(s.id, "could not load group: %v", err.Error()) } - s.log.Error().Err(err).Str("id", groupID).Msg("could not load group") return merrors.InternalServerError(s.id, "could not load group: %v", err.Error()) } @@ -283,11 +280,9 @@ func (s Service) AddMember(c context.Context, in *proto.AddMemberRequest, out *p } if err = s.repo.WriteAccount(c, a); err != nil { - s.log.Error().Err(err).Interface("account", a).Msg("could not persist account") - return merrors.InternalServerError(s.id, "could not persist updated account: %v", err.Error()) + return merrors.InternalServerError(s.id, "could not persist account: %v", err.Error()) } if err = s.repo.WriteGroup(c, g); err != nil { - s.log.Error().Err(err).Interface("group", g).Msg("could not persist group") return merrors.InternalServerError(s.id, "could not persist group: %v", err.Error()) } // FIXME update index! diff --git a/accounts/pkg/service/v0/service.go b/accounts/pkg/service/v0/service.go index b5283c7c2c..772210253e 100644 --- a/accounts/pkg/service/v0/service.go +++ b/accounts/pkg/service/v0/service.go @@ -48,14 +48,13 @@ func New(opts ...Option) (s *Service, err error) { roleManager = &m } - serviceID := cfg.GRPC.Namespace + "." + cfg.Server.Name s = &Service{ - id: serviceID, + id: cfg.GRPC.Namespace + "." + cfg.Server.Name, log: logger, Config: cfg, RoleService: roleService, RoleManager: roleManager, - repo: createMetadataStorage(serviceID, cfg, logger), + repo: createMetadataStorage(cfg, logger), } if s.index, err = s.buildIndex(); err != nil { @@ -357,13 +356,13 @@ func assignRoleToUser(accountID, roleID string, rs settings.RoleService, logger return true } -func createMetadataStorage(serviceID string, cfg *config.Config, logger log.Logger) storage.Repo { +func createMetadataStorage(cfg *config.Config, logger log.Logger) storage.Repo { // for now we detect the used storage implementation based on which storage is configured // the config with defaults needs to be checked last if cfg.Repo.Disk.Path != "" { - return storage.NewDiskRepo(serviceID, cfg, logger) + return storage.NewDiskRepo(cfg, logger) } - repo, err := storage.NewCS3Repo(serviceID, cfg) + repo, err := storage.NewCS3Repo(cfg) if err != nil { logger.Fatal().Err(err).Msg("cs3 storage was configured but failed to start") } diff --git a/accounts/pkg/storage/cs3.go b/accounts/pkg/storage/cs3.go index b1b51fe2a5..edf23ba0cd 100644 --- a/accounts/pkg/storage/cs3.go +++ b/accounts/pkg/storage/cs3.go @@ -24,7 +24,6 @@ import ( // CS3Repo provides a cs3 implementation of the Repo interface type CS3Repo struct { - serviceID string cfg *config.Config tm token.Manager storageProvider provider.ProviderAPIClient @@ -32,7 +31,7 @@ type CS3Repo struct { } // NewCS3Repo creates a new cs3 repo -func NewCS3Repo(serviceID string, cfg *config.Config) (Repo, error) { +func NewCS3Repo(cfg *config.Config) (Repo, error) { tokenManager, err := jwt.New(map[string]interface{}{ "secret": cfg.TokenManager.JWTSecret, }) @@ -47,7 +46,6 @@ func NewCS3Repo(serviceID string, cfg *config.Config) (Repo, error) { } return CS3Repo{ - serviceID: serviceID, cfg: cfg, tm: tokenManager, storageProvider: client, diff --git a/accounts/pkg/storage/disk.go b/accounts/pkg/storage/disk.go index df3288936c..3490424a6d 100644 --- a/accounts/pkg/storage/disk.go +++ b/accounts/pkg/storage/disk.go @@ -17,13 +17,12 @@ var groupLock sync.Mutex // DiskRepo provides a local filesystem implementation of the Repo interface type DiskRepo struct { - serviceID string cfg *config.Config log olog.Logger } // NewDiskRepo creates a new disk repo -func NewDiskRepo(serviceID string, cfg *config.Config, log olog.Logger) DiskRepo { +func NewDiskRepo(cfg *config.Config, log olog.Logger) DiskRepo { paths := []string{ filepath.Join(cfg.Repo.Disk.Path, accountsFolder), filepath.Join(cfg.Repo.Disk.Path, groupsFolder), @@ -38,7 +37,6 @@ func NewDiskRepo(serviceID string, cfg *config.Config, log olog.Logger) DiskRepo } } return DiskRepo{ - serviceID: serviceID, cfg: cfg, log: log, } From 675906ca25a9df1de6b00de843f12da00728a239 Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Mon, 5 Oct 2020 11:53:03 +0200 Subject: [PATCH 23/28] Get rid of named return and obsolete build tag --- ocis/pkg/command/revastoragemetadata.go | 2 -- ocis/pkg/runtime/runtime.go | 5 +++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/ocis/pkg/command/revastoragemetadata.go b/ocis/pkg/command/revastoragemetadata.go index 18d80abb2f..61c7af1125 100644 --- a/ocis/pkg/command/revastoragemetadata.go +++ b/ocis/pkg/command/revastoragemetadata.go @@ -1,5 +1,3 @@ -// +build !simple - package command import ( diff --git a/ocis/pkg/runtime/runtime.go b/ocis/pkg/runtime/runtime.go index 8cd8d0332d..0733b8ee0e 100644 --- a/ocis/pkg/runtime/runtime.go +++ b/ocis/pkg/runtime/runtime.go @@ -122,13 +122,14 @@ OUT: } // RunService sends a Service.Start command with the given service name to pman -func RunService(client *rpc.Client, service string) (reply int) { +func RunService(client *rpc.Client, service string) int { args := process.NewProcEntry(service, os.Environ(), []string{service}...) + var reply int if err := client.Call("Service.Start", args, &reply); err != nil { golog.Fatal(err) } - return + return reply } // AddMicroPlatform adds the micro subcommands to the cli app From b759691db8bf6faea7ecac89131fe20f3c638ce4 Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Mon, 5 Oct 2020 11:57:50 +0200 Subject: [PATCH 24/28] Build paths with path.Join --- accounts/pkg/storage/cs3.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/accounts/pkg/storage/cs3.go b/accounts/pkg/storage/cs3.go index edf23ba0cd..f642e31530 100644 --- a/accounts/pkg/storage/cs3.go +++ b/accounts/pkg/storage/cs3.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "encoding/json" - "fmt" "io" "io/ioutil" "net/http" @@ -118,7 +117,7 @@ func (r CS3Repo) DeleteAccount(ctx context.Context, id string) (err error) { resp, err := r.storageProvider.Delete(ctx, &provider.DeleteRequest{ Ref: &provider.Reference{ - Spec: &provider.Reference_Path{Path: fmt.Sprintf("/meta/%s/%s", accountsFolder, id)}, + Spec: &provider.Reference_Path{Path: path.Join("/meta", accountsFolder, id)}, }, }) @@ -192,7 +191,7 @@ func (r CS3Repo) DeleteGroup(ctx context.Context, id string) (err error) { resp, err := r.storageProvider.Delete(ctx, &provider.DeleteRequest{ Ref: &provider.Reference{ - Spec: &provider.Reference_Path{Path: fmt.Sprintf("/meta/%s/%s", groupsFolder, id)}, + Spec: &provider.Reference_Path{Path: path.Join("/meta", groupsFolder, id)}, }, }) @@ -229,7 +228,7 @@ func (r CS3Repo) groupURL(id string) string { func (r CS3Repo) makeRootDirIfNotExist(ctx context.Context, folder string) error { var rootPathRef = &provider.Reference{ - Spec: &provider.Reference_Path{Path: fmt.Sprintf("/meta/%v", folder)}, + Spec: &provider.Reference_Path{Path: path.Join("/meta", folder)}, } resp, err := r.storageProvider.Stat(ctx, &provider.StatRequest{ From 05e314ed1fce5c69db544262d377cf8357d8d8ce Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Mon, 5 Oct 2020 12:17:07 +0200 Subject: [PATCH 25/28] Replace ioutil.ReadAll with a buffered decoder --- accounts/pkg/storage/cs3.go | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/accounts/pkg/storage/cs3.go b/accounts/pkg/storage/cs3.go index f642e31530..c930cba2ba 100644 --- a/accounts/pkg/storage/cs3.go +++ b/accounts/pkg/storage/cs3.go @@ -5,7 +5,6 @@ import ( "context" "encoding/json" "io" - "io/ioutil" "net/http" "path" "strings" @@ -93,17 +92,13 @@ func (r CS3Repo) LoadAccount(ctx context.Context, id string, a *proto.Account) ( return ¬FoundErr{"account", id} } - b, err := ioutil.ReadAll(resp.Body) - if err != nil { - return err - } defer resp.Body.Close() - - if err := json.Unmarshal(b, &a); err != nil { + dec := json.NewDecoder(resp.Body) + var b []byte + if err = dec.Decode(&b); err != nil { return err } - - return nil + return json.Unmarshal(b, &a) } // DeleteAccount deletes an account via cs3 by id @@ -170,13 +165,12 @@ func (r CS3Repo) LoadGroup(ctx context.Context, id string, g *proto.Group) (err return ¬FoundErr{"group", id} } - b, err := ioutil.ReadAll(resp.Body) - if err != nil { + defer resp.Body.Close() + dec := json.NewDecoder(resp.Body) + var b []byte + if err = dec.Decode(&b); err != nil { return err } - - defer resp.Body.Close() - return json.Unmarshal(b, &g) } From e145c3be77d4ad0bd4a6ab44a95e1f1248189ae3 Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Mon, 5 Oct 2020 12:22:30 +0200 Subject: [PATCH 26/28] Handle error on response.Body.Close() --- accounts/pkg/storage/cs3.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/accounts/pkg/storage/cs3.go b/accounts/pkg/storage/cs3.go index c930cba2ba..d8b1c7fc97 100644 --- a/accounts/pkg/storage/cs3.go +++ b/accounts/pkg/storage/cs3.go @@ -92,12 +92,14 @@ func (r CS3Repo) LoadAccount(ctx context.Context, id string, a *proto.Account) ( return ¬FoundErr{"account", id} } - defer resp.Body.Close() dec := json.NewDecoder(resp.Body) var b []byte if err = dec.Decode(&b); err != nil { return err } + if err = resp.Body.Close(); err != nil { + return err + } return json.Unmarshal(b, &a) } @@ -165,12 +167,14 @@ func (r CS3Repo) LoadGroup(ctx context.Context, id string, g *proto.Group) (err return ¬FoundErr{"group", id} } - defer resp.Body.Close() dec := json.NewDecoder(resp.Body) var b []byte if err = dec.Decode(&b); err != nil { return err } + if err = resp.Body.Close(); err != nil { + return err + } return json.Unmarshal(b, &g) } From b49126e234d6ffdade5f3f9b3fa560c38fe4c112 Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Mon, 5 Oct 2020 14:59:27 +0200 Subject: [PATCH 27/28] Revert "Replace ioutil.ReadAll with a buffered decoder" This reverts commit 05e314ed1fce5c69db544262d377cf8357d8d8ce. --- accounts/pkg/storage/cs3.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/accounts/pkg/storage/cs3.go b/accounts/pkg/storage/cs3.go index d8b1c7fc97..5fce0839c0 100644 --- a/accounts/pkg/storage/cs3.go +++ b/accounts/pkg/storage/cs3.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "io" + "io/ioutil" "net/http" "path" "strings" @@ -92,9 +93,8 @@ func (r CS3Repo) LoadAccount(ctx context.Context, id string, a *proto.Account) ( return ¬FoundErr{"account", id} } - dec := json.NewDecoder(resp.Body) - var b []byte - if err = dec.Decode(&b); err != nil { + b, err := ioutil.ReadAll(resp.Body) + if err != nil { return err } if err = resp.Body.Close(); err != nil { @@ -167,9 +167,8 @@ func (r CS3Repo) LoadGroup(ctx context.Context, id string, g *proto.Group) (err return ¬FoundErr{"group", id} } - dec := json.NewDecoder(resp.Body) - var b []byte - if err = dec.Decode(&b); err != nil { + b, err := ioutil.ReadAll(resp.Body) + if err != nil { return err } if err = resp.Body.Close(); err != nil { From 3081dd1a7b04b6b79868fbcbe2e3fe4cd4df5d5f Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Mon, 5 Oct 2020 15:22:12 +0200 Subject: [PATCH 28/28] Fix OCS test infrastructure --- ocs/go.mod | 1 + ocs/go.sum | 1 + ocs/pkg/server/http/svc_test.go | 5 +++++ 3 files changed, 7 insertions(+) diff --git a/ocs/go.mod b/ocs/go.mod index 720d59a48b..f9e941ae41 100644 --- a/ocs/go.mod +++ b/ocs/go.mod @@ -15,6 +15,7 @@ require ( github.com/micro/cli/v2 v2.1.2 github.com/micro/go-micro/v2 v2.9.1 github.com/oklog/run v1.1.0 + github.com/olekukonko/tablewriter v0.0.4 github.com/openzipkin/zipkin-go v0.2.2 github.com/owncloud/ocis/accounts v0.0.0-20200918125107-fcca9faa81c8 github.com/owncloud/ocis/ocis-pkg v0.0.0-20200918114005-1a0ddd2190ee diff --git a/ocs/go.sum b/ocs/go.sum index 77b0bb99dd..9a7f7dac78 100644 --- a/ocs/go.sum +++ b/ocs/go.sum @@ -1927,6 +1927,7 @@ golang.org/x/tools v0.0.0-20200204074204-1cc6d1ef6c74/go.mod h1:TB2adYChydJhpapK golang.org/x/tools v0.0.0-20200207183749-b753a1ba74fa/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= golang.org/x/tools v0.0.0-20200212150539-ea181f53ac56/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= golang.org/x/tools v0.0.0-20200224181240-023911ca70b2/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= +golang.org/x/tools v0.0.0-20200331025713-a30bf2db82d4 h1:kDtqNkeBrZb8B+atrj50B5XLHpzXXqcCdZPP/ApQ5NY= golang.org/x/tools v0.0.0-20200331025713-a30bf2db82d4/go.mod h1:Sl4aGygMT6LrqrWclx+PTx3U+LnKx/seiNR+3G19Ar8= golang.org/x/tools v0.0.0-20200421042724-cfa8b22178d2/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20200427214658-4697a2867c88/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= diff --git a/ocs/pkg/server/http/svc_test.go b/ocs/pkg/server/http/svc_test.go index b18dd98ce7..5dc429038a 100644 --- a/ocs/pkg/server/http/svc_test.go +++ b/ocs/pkg/server/http/svc_test.go @@ -249,6 +249,11 @@ func init() { Server: accountsCfg.Server{ AccountsDataPath: dataPath, }, + Repo: accountsCfg.Repo{ + Disk: accountsCfg.Disk{ + Path: dataPath, + }, + }, Log: accountsCfg.Log{ Level: "info", Pretty: true,