Skip to content

Commit e5b1fd3

Browse files
JeffMboyaDušan Borovčanin
JeffMboya
authored and
Dušan Borovčanin
committed
Address comments
Signed-off-by: JeffMboya <jangina.mboya@gmail.com> Address comments Signed-off-by: JeffMboya <jangina.mboya@gmail.com> Address comments Signed-off-by: JeffMboya <jangina.mboya@gmail.com> Address comments Signed-off-by: JeffMboya <jangina.mboya@gmail.com> Address comments Signed-off-by: JeffMboya <jangina.mboya@gmail.com> Refactor error Signed-off-by: JeffMboya <jangina.mboya@gmail.com> Fix CI Signed-off-by: JeffMboya <jangina.mboya@gmail.com> Update domains/service.go Co-authored-by: Arvindh <30824765+arvindh123@users.noreply.github.com> Fix CI Signed-off-by: JeffMboya <jangina.mboya@gmail.com> Refactor Signed-off-by: JeffMboya <jangina.mboya@gmail.com> Update error Signed-off-by: JeffMboya <jangina.mboya@gmail.com> Update error Signed-off-by: JeffMboya <jangina.mboya@gmail.com> UFix CI Signed-off-by: JeffMboya <jangina.mboya@gmail.com> Refactor Signed-off-by: JeffMboya <jangina.mboya@gmail.com> Refactor Signed-off-by: JeffMboya <jangina.mboya@gmail.com> Refactor Signed-off-by: JeffMboya <jangina.mboya@gmail.com> Fix CI Signed-off-by: JeffMboya <jangina.mboya@gmail.com> Refactor Signed-off-by: JeffMboya <jangina.mboya@gmail.com>
1 parent 9c06a89 commit e5b1fd3

File tree

4 files changed

+27
-51
lines changed

4 files changed

+27
-51
lines changed

‎api/http/common.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,8 @@ func EncodeError(_ context.Context, err error, w http.ResponseWriter) {
233233
errors.Contains(err, apiutil.ErrMissingRoleMembers),
234234
errors.Contains(err, apiutil.ErrMissingDescription),
235235
errors.Contains(err, apiutil.ErrMissingEntityID),
236-
errors.Contains(err, apiutil.ErrInvalidRouteFormat):
236+
errors.Contains(err, apiutil.ErrInvalidRouteFormat),
237+
errors.Contains(err, svcerr.ErrRetainOneMember):
237238
err = unwrap(err)
238239
w.WriteHeader(http.StatusBadRequest)
239240

‎domains/service.go‎

Lines changed: 11 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ package domains
55

66
import (
77
"context"
8-
"fmt"
98
"time"
109

1110
"github.com/absmach/supermq"
@@ -317,7 +316,7 @@ func (svc *service) RejectInvitation(ctx context.Context, session authn.Session,
317316
func (svc *service) DeleteInvitation(ctx context.Context, session authn.Session, inviteeUserID, domainID string) error {
318317
if session.UserID == inviteeUserID {
319318
if err := svc.repo.DeleteInvitation(ctx, inviteeUserID, domainID); err != nil {
320-
return errors.Wrap(svcerr.ErrRemoveEntity, err)
319+
return err
321320
}
322321
return nil
323322
}
@@ -353,34 +352,24 @@ func (svc *service) RemoveEntityMembers(ctx context.Context, session authn.Sessi
353352
}
354353

355354
func (svc *service) RoleRemoveMembers(ctx context.Context, session authn.Session, entityID, roleID string, members []string) error {
356-
if len(members) == 0 {
357-
return svcerr.ErrMalformedEntity
358-
}
359-
360355
ro, err := svc.repo.RetrieveEntityRole(ctx, entityID, roleID)
361356
if err != nil {
362-
return errors.Wrap(svcerr.ErrViewEntity, fmt.Errorf("failed to retrieve role '%s' for entity '%s': %w", roleID, entityID, err))
363-
}
364-
365-
isBuiltInRole := false
366-
if _, ok := svc.ProvisionManageService.BuiltInRoles[roles.BuiltInRoleName(ro.Name)]; ok {
367-
isBuiltInRole = true
357+
return errors.Wrap(svcerr.ErrViewEntity, err)
368358
}
369359

370-
if isBuiltInRole {
371-
membersPage, listErr := svc.repo.RoleListMembers(ctx, ro.ID, 0, 0)
372-
if listErr != nil {
373-
return errors.Wrap(svcerr.ErrViewEntity, fmt.Errorf("failed to list members for built-in role '%s' to check count: %w", ro.Name, listErr))
360+
if _, err := svc.ProvisionManageService.BuiltInRoleActions(roles.BuiltInRoleName(ro.Name)); err == nil {
361+
membersPage, err := svc.repo.RoleListMembers(ctx, ro.ID, 0, 0)
362+
if err != nil {
363+
return errors.Wrap(svcerr.ErrViewEntity, err)
374364
}
375-
376365
if membersPage.Total <= uint64(len(members)) {
377-
return errors.Wrap(svcerr.ErrRemoveEntity, fmt.Errorf("built-in role '%s' must retain at least one member. Attempting to remove %d member(s) from %d would leave too few.", ro.Name, len(members), membersPage.Total))
366+
return svcerr.ErrRetainOneMember
378367
}
379368
}
380369

381370
for _, memberID := range members {
382371
if err := svc.repo.DeleteInvitation(ctx, memberID, entityID); err != nil && err != repoerr.ErrNotFound {
383-
return errors.Wrap(svcerr.ErrRemoveEntity, fmt.Errorf("failed to delete invitation for member '%s' in domain '%s' from role '%s': %w", memberID, entityID, ro.Name, err))
372+
return svcerr.ErrRetainOneMember
384373
}
385374
}
386375

@@ -390,37 +379,12 @@ func (svc *service) RoleRemoveMembers(ctx context.Context, session authn.Session
390379
func (svc *service) RoleRemoveAllMembers(ctx context.Context, session authn.Session, entityID, roleID string) error {
391380
ro, err := svc.repo.RetrieveEntityRole(ctx, entityID, roleID)
392381
if err != nil {
393-
return errors.Wrap(svcerr.ErrViewEntity, fmt.Errorf("failed to retrieve role '%s' for entity '%s': %w", roleID, entityID, err))
382+
return errors.Wrap(svcerr.ErrViewEntity, err)
394383
}
395384

396-
isBuiltInRole := false
397-
if _, ok := svc.ProvisionManageService.BuiltInRoles[roles.BuiltInRoleName(ro.Name)]; ok {
398-
isBuiltInRole = true
385+
if _, err := svc.ProvisionManageService.BuiltInRoleActions(roles.BuiltInRoleName(ro.Name)); err == nil {
386+
return svcerr.ErrRetainOneMember
399387
}
400388

401-
if isBuiltInRole {
402-
membersPage, listErr := svc.repo.RoleListMembers(ctx, ro.ID, 0, 0)
403-
if listErr != nil {
404-
return errors.Wrap(svcerr.ErrViewEntity, fmt.Errorf("failed to list members for built-in role '%s' to check count: %w", ro.Name, listErr))
405-
}
406-
407-
if membersPage.Total > 0 {
408-
return errors.Wrap(svcerr.ErrRemoveEntity, fmt.Errorf("built-in role '%s' must retain at least one member and cannot have all members removed", ro.Name))
409-
}
410-
}
411-
412-
currentMembersPage, listErr := svc.repo.RoleListMembers(ctx, ro.ID, 0, 0)
413-
if listErr != nil {
414-
return errors.Wrap(svcerr.ErrViewEntity, fmt.Errorf("failed to list members of role '%s' for invitation cleanup: %w", ro.Name, listErr))
415-
}
416-
417-
if len(currentMembersPage.Members) > 0 {
418-
for _, member := range currentMembersPage.Members {
419-
memberID := string(member)
420-
if err := svc.repo.DeleteInvitation(ctx, memberID, entityID); err != nil && err != repoerr.ErrNotFound {
421-
return errors.Wrap(svcerr.ErrRemoveEntity, fmt.Errorf("failed to delete invitation for member '%s' in domain '%s' from role '%s': %w", memberID, entityID, ro.Name, err))
422-
}
423-
}
424-
}
425389
return svc.ProvisionManageService.RoleRemoveAllMembers(ctx, session, entityID, roleID)
426390
}

‎pkg/errors/service/types.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,4 +87,7 @@ var (
8787

8888
// ErrUnauthorizedPAT indicates failure occurred while authorizing PAT.
8989
ErrUnauthorizedPAT = errors.New("failed to authorize PAT")
90+
91+
// ErrRetainOneMember indicates that at least one owner must be retained in the entity.
92+
ErrRetainOneMember = errors.New("must retain at least one member")
9093
)

‎pkg/roles/provisionmanage.go‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ type ProvisionManageService struct {
3434
sidProvider supermq.IDProvider
3535
policy policies.Service
3636
actions []Action
37-
BuiltInRoles map[BuiltInRoleName][]Action
37+
builtInRoles map[BuiltInRoleName][]Action
3838
}
3939

4040
func NewProvisionManageService(entityType string, repo Repository, policy policies.Service, sidProvider supermq.IDProvider, actions []Action, builtInRoles map[BuiltInRoleName][]Action) (ProvisionManageService, error) {
@@ -44,11 +44,19 @@ func NewProvisionManageService(entityType string, repo Repository, policy polici
4444
sidProvider: sidProvider,
4545
policy: policy,
4646
actions: actions,
47-
BuiltInRoles: builtInRoles,
47+
builtInRoles: builtInRoles,
4848
}
4949
return rm, nil
5050
}
5151

52+
func (pms ProvisionManageService) BuiltInRoleActions(name BuiltInRoleName) ([]Action, error) {
53+
actions, ok := pms.builtInRoles[name]
54+
if !ok {
55+
return nil, errors.Wrap(svcerr.ErrNotFound, fmt.Errorf("role %s not found", name))
56+
}
57+
return actions, nil
58+
}
59+
5260
func toRolesActions(actions []string) []Action {
5361
roActions := []Action{}
5462
for _, action := range actions {
@@ -143,7 +151,7 @@ func (r ProvisionManageService) AddNewEntitiesRoles(ctx context.Context, domainI
143151

144152
for _, entityID := range entityIDs {
145153
for defaultRole, defaultRoleMembers := range newBuiltInRoleMembers {
146-
actions, ok := r.BuiltInRoles[defaultRole]
154+
actions, ok := r.builtInRoles[defaultRole]
147155
if !ok {
148156
return []RoleProvision{}, fmt.Errorf("default role %s not found in in-built roles", defaultRole)
149157
}

0 commit comments

Comments
 (0)