Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shift role initialization from accounts to settings #1696

Merged
merged 14 commits into from
Feb 24, 2021
Merged

Shift role initialization from accounts to settings #1696

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
2004942
shift role initialization from accounts to settings
refs Feb 22, 2021
5d98dba
Merge branch 'master' into initialization-responsibility
refs Feb 23, 2021
f280e98
no race condition
refs Feb 23, 2021
97271d5
add circuit breakers
refs Feb 23, 2021
7609b11
fix empty branch
refs Feb 23, 2021
a361c12
defensive code around accounts init
refs Feb 23, 2021
78eabd2
rework settings tests
refs Feb 23, 2021
9a507a4
add readme
refs Feb 23, 2021
9342b5a
go around circuit breaker
refs Feb 23, 2021
89fc088
Merge branch 'master' into initialization-responsibility
refs Feb 23, 2021
71d63a6
update dependencies
refs Feb 23, 2021
8596afc
bring back dependants
refs Feb 23, 2021
9cb417b
Merge branch 'master' into initialization-responsibility
refs Feb 23, 2021
7b4ed39
add circuit breakers to the changelog
refs Feb 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions accounts/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ require (
contrib.go.opencensus.io/exporter/ocagent v0.6.0
contrib.go.opencensus.io/exporter/zipkin v0.1.1
github.com/asim/go-micro/plugins/client/grpc/v3 v3.0.0-20210217182006-0f0ace1a44a9
github.com/asim/go-micro/plugins/registry/kubernetes/v3 v3.0.0-20210217182006-0f0ace1a44a9 // indirect
github.com/asim/go-micro/plugins/registry/nats/v3 v3.0.0-20210217182006-0f0ace1a44a9 // indirect
github.com/asim/go-micro/v3 v3.5.1-0.20210217182006-0f0ace1a44a9
github.com/cs3org/go-cs3apis v0.0.0-20210209082852-35ace33082f5
github.com/cs3org/reva v1.6.1-0.20210223065028-53f39499762e
Expand Down
3 changes: 0 additions & 3 deletions accounts/pkg/command/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,6 @@ func Server(cfg *config.Config) *cli.Command {
)

gr.Add(func() error {
logger.Info().Str("service", server.Name()).Msg("Reporting settings bundles to settings service")
svc.RegisterSettingsBundles(&logger)
svc.RegisterPermissions(&logger)
return server.Run()
}, func(_ error) {
logger.Info().
Expand Down
56 changes: 29 additions & 27 deletions accounts/pkg/proto/v0/accounts.pb.micro_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"path/filepath"
"testing"

mgrpcc "github.com/asim/go-micro/plugins/client/grpc/v3"
refs marked this conversation as resolved.
Show resolved Hide resolved

"github.com/asim/go-micro/v3/client"
merrors "github.com/asim/go-micro/v3/errors"
"github.com/golang/protobuf/ptypes/empty"
Expand All @@ -24,7 +26,7 @@ import (
"google.golang.org/protobuf/types/known/timestamppb"
)

var service = grpc.Service{}
var service = grpc.NewService()

var dataPath = createTmpDir()

Expand Down Expand Up @@ -313,7 +315,7 @@ func assertGroupHasMember(t *testing.T, grp *proto.Group, memberId string) {
}

func createAccount(t *testing.T, user string) (*proto.Account, error) {
client := service.Client()
client := mgrpcc.NewClient()
cl := proto.NewAccountsService("com.owncloud.api.accounts", client)

account := getAccount(user)
Expand All @@ -326,7 +328,7 @@ func createAccount(t *testing.T, user string) (*proto.Account, error) {
}

func createGroup(t *testing.T, group *proto.Group) (*proto.Group, error) {
client := service.Client()
client := mgrpcc.NewClient()
cl := proto.NewGroupsService("com.owncloud.api.accounts", client)

request := &proto.CreateGroupRequest{Group: group}
Expand All @@ -338,7 +340,7 @@ func createGroup(t *testing.T, group *proto.Group) (*proto.Group, error) {
}

func updateAccount(t *testing.T, account *proto.Account, updateArray []string) (*proto.Account, error) {
client := service.Client()
client := mgrpcc.NewClient()
cl := proto.NewAccountsService("com.owncloud.api.accounts", client)

updateMask := &field_mask.FieldMask{
Expand All @@ -352,7 +354,7 @@ func updateAccount(t *testing.T, account *proto.Account, updateArray []string) (

func listAccounts(t *testing.T) (*proto.ListAccountsResponse, error) {
request := &proto.ListAccountsRequest{}
client := service.Client()
client := mgrpcc.NewClient()
cl := proto.NewAccountsService("com.owncloud.api.accounts", client)

response, err := cl.ListAccounts(context.Background(), request)
Expand All @@ -361,7 +363,7 @@ func listAccounts(t *testing.T) (*proto.ListAccountsResponse, error) {

func listGroups(t *testing.T) *proto.ListGroupsResponse {
request := &proto.ListGroupsRequest{}
client := service.Client()
client := mgrpcc.NewClient()
cl := proto.NewGroupsService("com.owncloud.api.accounts", client)

response, err := cl.ListGroups(context.Background(), request)
Expand All @@ -370,7 +372,7 @@ func listGroups(t *testing.T) *proto.ListGroupsResponse {
}

func deleteAccount(t *testing.T, id string) (*empty.Empty, error) {
client := service.Client()
client := mgrpcc.NewClient()
cl := proto.NewAccountsService("com.owncloud.api.accounts", client)

req := &proto.DeleteAccountRequest{Id: id}
Expand All @@ -379,7 +381,7 @@ func deleteAccount(t *testing.T, id string) (*empty.Empty, error) {
}

func deleteGroup(t *testing.T, id string) (*empty.Empty, error) {
client := service.Client()
client := mgrpcc.NewClient()
cl := proto.NewGroupsService("com.owncloud.api.accounts", client)

req := &proto.DeleteGroupRequest{Id: id}
Expand Down Expand Up @@ -767,7 +769,7 @@ func TestDeleteAccount(t *testing.T) {

req := &proto.DeleteAccountRequest{Id: getAccount("user1").Id}

client := service.Client()
client := mgrpcc.NewClient()
cl := proto.NewAccountsService("com.owncloud.api.accounts", client)

resp, err := cl.DeleteAccount(context.Background(), req)
Expand All @@ -785,7 +787,7 @@ func TestDeleteAccount(t *testing.T) {
func TestListGroups(t *testing.T) {
req := &proto.ListGroupsRequest{}

client := service.Client()
client := mgrpcc.NewClient()
cl := proto.NewGroupsService("com.owncloud.api.accounts", client)

resp, err := cl.ListGroups(context.Background(), req)
Expand All @@ -812,7 +814,7 @@ func TestListGroups(t *testing.T) {
}

func TestGetGroups(t *testing.T) {
client := service.Client()
client := mgrpcc.NewClient()
cl := proto.NewGroupsService("com.owncloud.api.accounts", client)

groups := []string{
Expand Down Expand Up @@ -860,7 +862,7 @@ func TestCreateGroup(t *testing.T) {
}

func TestGetGroupInvalidID(t *testing.T) {
client := service.Client()
client := mgrpcc.NewClient()
cl := proto.NewGroupsService("com.owncloud.api.accounts", client)

req := &proto.GetGroupRequest{Id: "42"}
Expand All @@ -880,7 +882,7 @@ func TestDeleteGroup(t *testing.T) {
createGroup(t, grp2)
createGroup(t, grp3)

client := service.Client()
client := mgrpcc.NewClient()
cl := proto.NewGroupsService("com.owncloud.api.accounts", client)

req := &proto.DeleteGroupRequest{Id: grp1.Id}
Expand Down Expand Up @@ -909,7 +911,7 @@ func TestDeleteGroupNotExisting(t *testing.T) {
" ",
}

client := service.Client()
client := mgrpcc.NewClient()
cl := proto.NewGroupsService("com.owncloud.api.accounts", client)

for _, id := range invalidIds {
Expand All @@ -932,7 +934,7 @@ func TestDeleteGroupInvalidId(t *testing.T) {
"": ".",
}

client := service.Client()
client := mgrpcc.NewClient()
cl := proto.NewGroupsService("com.owncloud.api.accounts", client)

for id := range invalidIds {
Expand All @@ -949,7 +951,7 @@ func TestUpdateGroup(t *testing.T) {
grp1 := getTestGroups("grp1")
createGroup(t, grp1)

client := service.Client()
client := mgrpcc.NewClient()
cl := proto.NewGroupsService("com.owncloud.api.accounts", client)

updateGrp := &proto.Group{
Expand Down Expand Up @@ -978,7 +980,7 @@ func TestAddMember(t *testing.T) {
createGroup(t, grp1)
createAccount(t, account.PreferredName)

client := service.Client()
client := mgrpcc.NewClient()
cl := proto.NewGroupsService("com.owncloud.api.accounts", client)

req := &proto.AddMemberRequest{GroupId: grp1.Id, AccountId: account.Id}
Expand Down Expand Up @@ -1010,7 +1012,7 @@ func TestAddMemberAlreadyInGroup(t *testing.T) {

addMemberToGroup(t, grp1.Id, account.Id)

client := service.Client()
client := mgrpcc.NewClient()
cl := proto.NewGroupsService("com.owncloud.api.accounts", client)

req := &proto.AddMemberRequest{GroupId: grp1.Id, AccountId: account.Id}
Expand All @@ -1035,7 +1037,7 @@ func TestAddMemberNonExisting(t *testing.T) {

createGroup(t, grp1)

client := service.Client()
client := mgrpcc.NewClient()
cl := proto.NewGroupsService("com.owncloud.api.accounts", client)

invalidIds := []string{
Expand Down Expand Up @@ -1063,7 +1065,7 @@ func TestAddMemberNonExisting(t *testing.T) {
}

func addMemberToGroup(t *testing.T, groupId, memberId string) (*proto.Group, error) {
client := service.Client()
client := mgrpcc.NewClient()
cl := proto.NewGroupsService("com.owncloud.api.accounts", client)

req := &proto.AddMemberRequest{GroupId: groupId, AccountId: memberId}
Expand All @@ -1083,7 +1085,7 @@ func TestRemoveMember(t *testing.T) {

addMemberToGroup(t, grp1.Id, account.Id)

client := service.Client()
client := mgrpcc.NewClient()
cl := proto.NewGroupsService("com.owncloud.api.accounts", client)

req := &proto.RemoveMemberRequest{GroupId: grp1.Id, AccountId: account.Id}
Expand All @@ -1106,7 +1108,7 @@ func TestRemoveMemberNonExistingUser(t *testing.T) {

createGroup(t, grp1)

client := service.Client()
client := mgrpcc.NewClient()
cl := proto.NewGroupsService("com.owncloud.api.accounts", client)

invalidIds := []string{
Expand Down Expand Up @@ -1140,7 +1142,7 @@ func TestRemoveMemberNotInGroup(t *testing.T) {
createGroup(t, grp1)
createAccount(t, account.PreferredName)

client := service.Client()
client := mgrpcc.NewClient()
cl := proto.NewGroupsService("com.owncloud.api.accounts", client)

req := &proto.RemoveMemberRequest{GroupId: grp1.Id, AccountId: account.Id}
Expand Down Expand Up @@ -1177,7 +1179,7 @@ func TestListMembers(t *testing.T) {
"physics-lovers",
}

client := service.Client()
client := mgrpcc.NewClient()
cl := proto.NewGroupsService("com.owncloud.api.accounts", client)

for _, group := range groups {
Expand Down Expand Up @@ -1209,7 +1211,7 @@ func TestListMembers(t *testing.T) {
func TestListMembersEmptyGroup(t *testing.T) {
group := &proto.Group{Id: "5d58e5ec-842e-498b-8800-61f2ec6f911c", GidNumber: 60000, OnPremisesSamAccountName: "quantum-group", DisplayName: "Quantum Group", Members: []*proto.Account{}}

client := service.Client()
client := mgrpcc.NewClient()
cl := proto.NewGroupsService("com.owncloud.api.accounts", client)

request := &proto.CreateGroupRequest{Group: group}
Expand All @@ -1231,7 +1233,7 @@ func TestListMembersEmptyGroup(t *testing.T) {
func TestAccountUpdateMask(t *testing.T) {
createAccount(t, "user1")
user1 := getAccount("user1")
client := service.Client()
client := mgrpcc.NewClient()
req := &proto.UpdateAccountRequest{
// We only want to update the display-name, rest should be ignored
UpdateMask: &field_mask.FieldMask{Paths: []string{"DisplayName"}},
Expand All @@ -1254,7 +1256,7 @@ func TestAccountUpdateMask(t *testing.T) {
func TestAccountUpdateReadOnlyField(t *testing.T) {
createAccount(t, "user1")
user1 := getAccount("user1")
client := service.Client()
client := mgrpcc.NewClient()
req := &proto.UpdateAccountRequest{
// We only want to update the display-name, rest should be ignored
UpdateMask: &field_mask.FieldMask{Paths: []string{"CreatedDateTime"}},
Expand Down
60 changes: 27 additions & 33 deletions accounts/pkg/service/v0/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ import (
"github.com/owncloud/ocis/accounts/pkg/config"
"github.com/owncloud/ocis/accounts/pkg/proto/v0"
"github.com/owncloud/ocis/ocis-pkg/log"
oreg "github.com/owncloud/ocis/ocis-pkg/registry"
"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"
)

// userDefaultGID is the default integer representing the "users" group.
Expand Down Expand Up @@ -57,6 +57,31 @@ func New(opts ...Option) (s *Service, err error) {
repo: createMetadataStorage(cfg, logger),
}

retries := 20
var current int
r := oreg.GetRegistry()
if cfg.Repo.Disk.Path == "" {
for {
if current >= retries {
panic("metadata service failed to start.")
}
s, err := r.GetService("com.owncloud.storage.metadata")
if err != nil {
logger.Error().Err(err).Msg("error getting metadata service from service registry")
}
if len(s) > 0 {
break
}
logger.Info().Msg("accounts blocked waiting for metadata service to be up and running...")
time.Sleep(2 * time.Second)
current++
}
}

// we want to wait anyway. If it depends on a reva service it could be the case that the entry on the registry
// happens prior to the reva service being up and running
time.Sleep(500 * time.Millisecond)

if s.index, err = s.buildIndex(); err != nil {
return nil, err
}
Expand All @@ -68,7 +93,6 @@ func New(opts ...Option) (s *Service, err error) {
if err = s.createDefaultGroups(); err != nil {
return nil, err
}
// TODO watch folders for new records
return
}

Expand Down Expand Up @@ -267,6 +291,7 @@ func (s Service) createDefaultAccounts() (err error) {
},
},
}
// this only deals with the metadata service.
for i := range accounts {
a := &proto.Account{}
err := s.repo.LoadAccount(context.Background(), accounts[i].Id, a)
Expand All @@ -287,7 +312,6 @@ func (s Service) createDefaultAccounts() (err error) {
}
}

// TODO: can be removed again as soon as we respect the predefined UIDs and GIDs from the account. Then no autoincrement is happening, therefore we don't need to update accounts.
changed := false
for _, r := range results {
if r.Field == "UidNumber" || r.Field == "GidNumber" {
Expand All @@ -309,24 +333,6 @@ func (s Service) createDefaultAccounts() (err error) {
}
}
}

// 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
"ddc2004c-0977-11eb-9d3f-a793888cd0f8", //admin
"820ba2a1-3f54-4538-80a4-2d73007e30bf", //idp
"bc596f3c-c955-4328-80a0-60d018b4ad57", //reva
} {
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
}

Expand Down Expand Up @@ -403,18 +409,6 @@ func (s Service) createDefaultGroups() (err error) {
return nil
}

func assignRoleToUser(accountID, roleID string, rs settings.RoleService, logger log.Logger) (ok bool) {
_, err := rs.AssignRoleToUser(context.Background(), &settings.AssignRoleToUserRequest{
AccountUuid: accountID,
RoleId: roleID,
})
if err != nil {
logger.Error().Err(err).Str("accountID", accountID).Str("roleID", roleID).Msg("could not set role for account")
return false
}
return true
}

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
Expand Down
Loading