From a08d94e8b3682394ea3bc759928fe2887a2c913b Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Wed, 21 Oct 2020 12:12:23 +0200 Subject: [PATCH] fix staticcheck + linter --- .../pkg/proto/v0/accounts.pb.micro_test.go | 14 ++-- accounts/pkg/service/v0/accounts.go | 76 +++++++++---------- accounts/pkg/service/v0/groups.go | 32 ++++---- ocis/go.sum | 2 + 4 files changed, 61 insertions(+), 63 deletions(-) diff --git a/accounts/pkg/proto/v0/accounts.pb.micro_test.go b/accounts/pkg/proto/v0/accounts.pb.micro_test.go index 461e9279b8..f3e09a515e 100644 --- a/accounts/pkg/proto/v0/accounts.pb.micro_test.go +++ b/accounts/pkg/proto/v0/accounts.pb.micro_test.go @@ -667,7 +667,7 @@ func TestListAccountsWithFilterQuery(t *testing.T) { }, { name: "ListAccounts with exact match on mail", - query: "mail eq 'user1@example.org'", + query: "mail eq 'user1@example.com'", expectedIDs: []string{user1.Id}, }, //{ @@ -682,32 +682,32 @@ func TestListAccountsWithFilterQuery(t *testing.T) { //}, //{ // name: "ListAccounts with exact match on preferred_name AND mail", - // query: "preferred_name eq 'user1' and mail eq 'user1@example.org'", + // query: "preferred_name eq 'user1' and mail eq 'user1@example.com'", // expectedIDs: []string{user1.Id}, //}, //{ // name: "ListAccounts without match on preferred_name AND mail", - // query: "preferred_name eq 'user1' and mail eq 'wololo@example.org'", + // query: "preferred_name eq 'user1' and mail eq 'wololo@example.com'", // expectedIDs: []string{}, //}, //{ // name: "ListAccounts with exact match on preferred_name OR mail, preferred_name exists, mail exists", - // query: "preferred_name eq 'user1' or mail eq 'user1@example.org'", + // query: "preferred_name eq 'user1' or mail eq 'user1@example.com'", // expectedIDs: []string{user1.Id}, //}, //{ // name: "ListAccounts with exact match on preferred_name OR mail, preferred_name exists, mail does not exist", - // query: "preferred_name eq 'user1' or mail eq 'wololo@example.org'", + // query: "preferred_name eq 'user1' or mail eq 'wololo@example.com'", // expectedIDs: []string{user1.Id}, //}, //{ // name: "ListAccounts with exact match on preferred_name OR mail, preferred_name does not exists, mail exists", - // query: "preferred_name eq 'wololo' or mail eq 'user1@example.org'", + // query: "preferred_name eq 'wololo' or mail eq 'user1@example.com'", // expectedIDs: []string{user1.Id}, //}, //{ // name: "ListAccounts without match on preferred_name OR mail, preferred_name and mail do not exist", - // query: "preferred_name eq 'wololo' or mail eq 'wololo@example.org'", + // query: "preferred_name eq 'wololo' or mail eq 'wololo@example.com'", // expectedIDs: []string{}, //}, //{ diff --git a/accounts/pkg/service/v0/accounts.go b/accounts/pkg/service/v0/accounts.go index bca89e9765..b4b3c3023b 100644 --- a/accounts/pkg/service/v0/accounts.go +++ b/accounts/pkg/service/v0/accounts.go @@ -304,22 +304,24 @@ func (s Service) CreateAccount(ctx context.Context, in *proto.CreateAccountReque accLock.Lock() defer accLock.Unlock() var id string - var acc = in.Account - if acc == nil { - return merrors.BadRequest(s.id, "account missing") + + if in.Account == nil { + return merrors.InternalServerError(s.id, "invalid account: empty", nil) } - if acc.Id == "" { - acc.Id = uuid.Must(uuid.NewV4()).String() + *out = *in.Account + + if out.Id == "" { + out.Id = uuid.Must(uuid.NewV4()).String() } - if err = validateAccount(s.id, acc); err != nil { + if err = validateAccount(s.id, out); err != nil { return err } - if id, err = cleanupID(acc.Id); err != nil { + if id, err = cleanupID(out.Id); err != nil { return merrors.InternalServerError(s.id, "could not clean up account id: %v", err.Error()) } - exists, err := s.accountExists(ctx, acc.PreferredName, acc.Mail, acc.Id) + exists, err := s.accountExists(ctx, out.PreferredName, out.Mail, out.Id) if err != nil { return merrors.InternalServerError(s.id, "could not check if account exists: %v", err.Error()) } @@ -327,17 +329,17 @@ func (s Service) CreateAccount(ctx context.Context, in *proto.CreateAccountReque return merrors.BadRequest(s.id, "account already exists") } - if acc.PasswordProfile != nil { - if acc.PasswordProfile.Password != "" { + if out.PasswordProfile != nil { + if out.PasswordProfile.Password != "" { // encrypt password c := crypt.New(crypt.SHA512) - if acc.PasswordProfile.Password, err = c.Generate([]byte(acc.PasswordProfile.Password), nil); err != nil { + if out.PasswordProfile.Password, err = c.Generate([]byte(out.PasswordProfile.Password), nil); err != nil { s.log.Error().Err(err).Str("id", id).Msg("could not hash password") return merrors.InternalServerError(s.id, "could not hash password: %v", err.Error()) } } - if err := passwordPoliciesValid(acc.PasswordProfile.PasswordPolicies); err != nil { + if err := passwordPoliciesValid(out.PasswordProfile.PasswordPolicies); err != nil { return merrors.BadRequest(s.id, "%s", err) } } @@ -346,33 +348,33 @@ 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.repo.WriteAccount(ctx, acc); err != nil { + if err = s.repo.WriteAccount(ctx, out); 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") + s.debugLogAccount(out).Msg("could not persist new account") return merrors.InternalServerError(s.id, "could not persist new account: %v", err.Error()) } - indexResults, err := s.index.Add(acc) + indexResults, err := s.index.Add(out) if err != nil { - s.rollbackCreateAccount(ctx, acc) + s.rollbackCreateAccount(ctx, out) return merrors.BadRequest(s.id, "Account already exists %v", err.Error()) } - s.log.Debug().Interface("account", acc).Msg("account after indexing") + s.log.Debug().Interface("account", out).Msg("account after indexing") for _, r := range indexResults { if r.Field == "UidNumber" { id, err := strconv.Atoi(path.Base(r.Value)) if err != nil { - s.rollbackCreateAccount(ctx, acc) + s.rollbackCreateAccount(ctx, out) return err } - acc.UidNumber = int64(id) + out.UidNumber = int64(id) break } } - if in.Account.GidNumber == 0 { - in.Account.GidNumber = userDefaultGID + if out.GidNumber == 0 { + out.GidNumber = userDefaultGID } r := proto.ListGroupsResponse{} @@ -383,27 +385,25 @@ func (s Service) CreateAccount(ctx context.Context, in *proto.CreateAccountReque } for _, group := range r.Groups { - if group.GidNumber == in.Account.GidNumber { - in.Account.MemberOf = append(in.Account.MemberOf, group) + if group.GidNumber == out.GidNumber { + out.MemberOf = append(out.MemberOf, group) } } //acc.MemberOf = append(acc.MemberOf, &group) - if err := s.repo.WriteAccount(context.Background(), acc); err != nil { + if err := s.repo.WriteAccount(context.Background(), out); err != nil { return err } - if acc.PasswordProfile != nil { - acc.PasswordProfile.Password = "" + if out.PasswordProfile != nil { + out.PasswordProfile.Password = "" } - *out = *acc - // TODO: assign user role to all new users for now, as create Account request does not have any role field if s.RoleService == nil { return merrors.InternalServerError(s.id, "could not assign role to account: roleService not configured") } if _, err = s.RoleService.AssignRoleToUser(ctx, &settings.AssignRoleToUserRequest{ - AccountUuid: acc.Id, + AccountUuid: out.Id, RoleId: settings_svc.BundleUUIDRoleUser, }); err != nil { return merrors.InternalServerError(s.id, "could not assign role to account: %v", err.Error()) @@ -468,18 +468,18 @@ func (s Service) UpdateAccount(ctx context.Context, in *proto.UpdateAccountReque } if _, exists := validMask.Filter("PreferredName"); exists { - if err = validateAccountPreferredName(s.id, *in.Account); err != nil { + if err = validateAccountPreferredName(s.id, in.Account); err != nil { return err } } if _, exists := validMask.Filter("OnPremisesSamAccountName"); exists { - if err = validateAccountOnPremisesSamAccountName(s.id, *in.Account); err != nil { + if err = validateAccountOnPremisesSamAccountName(s.id, in.Account); err != nil { return err } } if _, exists := validMask.Filter("Mail"); exists { if in.Account.Mail != "" { - if err = validateAccountEmail(s.id, *in.Account); err != nil { + if err = validateAccountEmail(s.id, in.Account); err != nil { return err } } @@ -620,33 +620,33 @@ func (s Service) DeleteAccount(ctx context.Context, in *proto.DeleteAccountReque } func validateAccount(serviceID string, a *proto.Account) error { - if err := validateAccountPreferredName(serviceID, *a); err != nil { + if err := validateAccountPreferredName(serviceID, a); err != nil { return err } - if err := validateAccountOnPremisesSamAccountName(serviceID, *a); err != nil { + if err := validateAccountOnPremisesSamAccountName(serviceID, a); err != nil { return err } - if err := validateAccountEmail(serviceID, *a); err != nil { + if err := validateAccountEmail(serviceID, a); err != nil { return err } return nil } -func validateAccountPreferredName(serviceID string, a proto.Account) error { +func validateAccountPreferredName(serviceID string, a *proto.Account) error { if !isValidUsername(a.PreferredName) { return merrors.BadRequest(serviceID, "preferred_name '%s' must be at least the local part of an email", a.PreferredName) } return nil } -func validateAccountOnPremisesSamAccountName(serviceID string, a proto.Account) error { +func validateAccountOnPremisesSamAccountName(serviceID string, a *proto.Account) error { if !isValidUsername(a.OnPremisesSamAccountName) { return merrors.BadRequest(serviceID, "on_premises_sam_account_name '%s' must be at least the local part of an email", a.OnPremisesSamAccountName) } return nil } -func validateAccountEmail(serviceID string, a proto.Account) error { +func validateAccountEmail(serviceID string, a *proto.Account) error { if !isValidEmail(a.Mail) { return merrors.BadRequest(serviceID, "mail '%s' must be a valid email", a.Mail) } diff --git a/accounts/pkg/service/v0/groups.go b/accounts/pkg/service/v0/groups.go index cfa0e26cec..9968b3a7a0 100644 --- a/accounts/pkg/service/v0/groups.go +++ b/accounts/pkg/service/v0/groups.go @@ -110,31 +110,29 @@ func (s Service) GetGroup(c context.Context, in *proto.GetGroupRequest, out *pro // CreateGroup implements the GroupsServiceHandler interface func (s Service) CreateGroup(c context.Context, in *proto.CreateGroupRequest, out *proto.Group) (err error) { - var _ string if in.Group == nil { - return merrors.BadRequest(s.id, "account missing") + return merrors.InternalServerError(s.id, "invalid group: empty", nil) } - if in.Group.Id == "" { - in.Group.Id = uuid.Must(uuid.NewV4()).String() + *out = *in.Group + + if out.Id == "" { + out.Id = uuid.Must(uuid.NewV4()).String() } - _ = in.Group.Id - - if _, err = cleanupID(in.Group.Id); err != nil { + if _, err = cleanupID(out.Id); err != nil { return merrors.InternalServerError(s.id, "could not clean up account id: %v", err.Error()) } - // extract member id - s.deflateMembers(in.Group) + s.deflateMembers(out) - 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") + if err = s.repo.WriteGroup(c, out); err != nil { + s.log.Error().Err(err).Interface("group", out).Msg("could not persist new group") return merrors.InternalServerError(s.id, "could not persist new group: %v", err.Error()) } - indexResults, err := s.index.Add(in.Group) + indexResults, err := s.index.Add(out) if err != nil { - s.rollbackCreateGroup(c, in.Group) + s.rollbackCreateGroup(c, out) return merrors.InternalServerError(s.id, "could not index new group: %v", err.Error()) } @@ -142,16 +140,14 @@ func (s Service) CreateGroup(c context.Context, in *proto.CreateGroupRequest, ou if r.Field == "GidNumber" { gid, err := strconv.Atoi(path.Base(r.Value)) if err != nil { - s.rollbackCreateGroup(c, in.Group) + s.rollbackCreateGroup(c, out) return err } - in.Group.GidNumber = int64(gid) - return s.repo.WriteGroup(context.Background(), in.Group) + out.GidNumber = int64(gid) + return s.repo.WriteGroup(context.Background(), out) } } - *out = *in.Group - return } diff --git a/ocis/go.sum b/ocis/go.sum index f2bcfb242b..38a3e868a1 100644 --- a/ocis/go.sum +++ b/ocis/go.sum @@ -277,6 +277,8 @@ github.com/cs3org/reva v1.2.2-0.20200924071957-e6676516e61e h1:khITGSnfDXtByQsLe github.com/cs3org/reva v1.2.2-0.20200924071957-e6676516e61e/go.mod h1:DOV5SjpOBKN+aWfOHLdA4KiLQkpyC786PQaXEdRAZ0M= github.com/cs3org/reva v1.3.1-0.20201020152305-05ab93ed7a66 h1:RI/prn0/25zv1UCXB8FPp3VY+oIRz7kKGHT2wultvUw= github.com/cs3org/reva v1.3.1-0.20201020152305-05ab93ed7a66/go.mod h1:rTJhfVoZggB5iSPH5oWqQSO+W1iTQIxNmaX/ueS9GAU= +github.com/cs3org/reva v1.3.1-0.20201021065855-dc400f81ecbc h1:8y6N/bscmzdXd4yAv6bIJiqggter8iEmb4cYMhuZkE8= +github.com/cs3org/reva v1.3.1-0.20201021065855-dc400f81ecbc/go.mod h1:rTJhfVoZggB5iSPH5oWqQSO+W1iTQIxNmaX/ueS9GAU= github.com/cznic/b v0.0.0-20181122101859-a26611c4d92d h1:SwD98825d6bdB+pEuTxWOXiSjBrHdOl/UVp75eI7JT8= github.com/cznic/b v0.0.0-20181122101859-a26611c4d92d/go.mod h1:URriBxXwVq5ijiJ12C7iIZqlA69nTlI+LgI6/pwftG8= github.com/cznic/mathutil v0.0.0-20181122101859-297441e03548 h1:iwZdTE0PVqJCos1vaoKsclOGD3ADKpshg3SRtYBbwso=