diff --git a/compose/internal/service/access_control.go b/compose/internal/service/access_control.go index 3cfcaea6b..7bf33dbf6 100644 --- a/compose/internal/service/access_control.go +++ b/compose/internal/service/access_control.go @@ -163,7 +163,7 @@ func (svc accessControl) Grant(ctx context.Context, rr ...*permissions.Rule) err func (svc accessControl) FindRulesByRoleID(ctx context.Context, roleID uint64) (permissions.RuleSet, error) { if !svc.CanGrant(ctx) { - return nil, ErrNoPermissions + return nil, ErrNoGrantPermissions } return svc.permissions.FindRulesByRoleID(roleID), nil diff --git a/compose/internal/service/attachment.go b/compose/internal/service/attachment.go index dc764a250..4ad5b9051 100644 --- a/compose/internal/service/attachment.go +++ b/compose/internal/service/attachment.go @@ -165,7 +165,7 @@ func (svc attachment) CreatePageAttachment(namespaceID uint64, name string, size if p, err := svc.pageSvc.FindByID(namespaceID, pageID); err != nil { return nil, err } else if !svc.ac.CanUpdatePage(svc.ctx, p) { - return nil, errors.New("not allowed to add attachments to this page") + return nil, ErrNoUpdatePermissions.withStack() } att := &types.Attachment{ @@ -190,7 +190,7 @@ func (svc attachment) CreateRecordAttachment(namespaceID uint64, name string, si } else if _, err := svc.recordSvc.FindByID(namespaceID, recordID); err != nil { return nil, err } else if !svc.ac.CanUpdateRecord(svc.ctx, m) { - return nil, errors.New("not allowed to add attachments to this record") + return nil, ErrNoUpdatePermissions.withStack() } att := &types.Attachment{ diff --git a/compose/internal/service/chart.go b/compose/internal/service/chart.go index 980a4bc19..1e8a3f8c5 100644 --- a/compose/internal/service/chart.go +++ b/compose/internal/service/chart.go @@ -131,6 +131,10 @@ func (svc chart) Update(mod *types.Chart) (c *types.Chart, err error) { } func (svc chart) DeleteByID(namespaceID, chartID uint64) error { + if chartID == 0 { + return ErrInvalidID.withStack() + } + if namespaceID == 0 { return ErrNamespaceRequired.withStack() } diff --git a/compose/internal/service/error.go b/compose/internal/service/error.go index 7f4db35ff..d9190c46d 100644 --- a/compose/internal/service/error.go +++ b/compose/internal/service/error.go @@ -11,13 +11,14 @@ type ( const ( ErrInvalidID serviceError = "InvalidID" ErrStaleData serviceError = "StaleData" + ErrNoGrantPermissions serviceError = "NoGrantPermissions" ErrNoCreatePermissions serviceError = "NoCreatePermissions" ErrNoReadPermissions serviceError = "NoReadPermissions" ErrNoUpdatePermissions serviceError = "NoUpdatePermissions" ErrNoDeletePermissions serviceError = "NoDeletePermissions" ErrNamespaceRequired serviceError = "NamespaceRequired" + ErrModulePageExists serviceError = "ModulePageExists" ErrNotImplemented serviceError = "NotImplemented" - ErrNoPermissions serviceError = "NoPermissions" ) func (e serviceError) Error() string { diff --git a/compose/internal/service/module.go b/compose/internal/service/module.go index 623cad7af..ff21467dc 100644 --- a/compose/internal/service/module.go +++ b/compose/internal/service/module.go @@ -160,6 +160,10 @@ func (svc module) Update(mod *types.Module) (m *types.Module, err error) { } func (svc module) DeleteByID(namespaceID, moduleID uint64) error { + if moduleID == 0 { + return ErrInvalidID.withStack() + } + if namespaceID == 0 { return ErrNamespaceRequired.withStack() } diff --git a/compose/internal/service/namespace.go b/compose/internal/service/namespace.go index 423e528f4..a6a795b86 100644 --- a/compose/internal/service/namespace.go +++ b/compose/internal/service/namespace.go @@ -128,12 +128,16 @@ func (svc namespace) Update(mod *types.Namespace) (ns *types.Namespace, err erro return svc.namespaceRepo.Update(ns) } -func (svc namespace) DeleteByID(ID uint64) error { - if ns, err := svc.namespaceRepo.FindByID(ID); err != nil { +func (svc namespace) DeleteByID(namespaceID uint64) error { + if namespaceID == 0 { + return ErrInvalidID.withStack() + } + + if ns, err := svc.namespaceRepo.FindByID(namespaceID); err != nil { return err } else if !svc.ac.CanDeleteNamespace(svc.ctx, ns) { return ErrNoDeletePermissions.withStack() } - return svc.namespaceRepo.DeleteByID(ID) + return svc.namespaceRepo.DeleteByID(namespaceID) } diff --git a/compose/internal/service/page.go b/compose/internal/service/page.go index 856660808..47c9caa3d 100644 --- a/compose/internal/service/page.go +++ b/compose/internal/service/page.go @@ -51,10 +51,6 @@ type ( } ) -const ( - ErrModulePageExists serviceError = "ModulePageExists" -) - func Page() PageService { return (&page{ logger: DefaultLogger.Named("page"), @@ -249,14 +245,18 @@ func (svc page) checkModulePage(mod *types.Page) error { } func (svc page) DeleteByID(namespaceID, pageID uint64) error { + if pageID == 0 { + return ErrInvalidID.withStack() + } + if _, err := svc.loadNamespace(namespaceID); err != nil { return err } if p, err := svc.pageRepo.FindByID(namespaceID, pageID); err != nil { - return errors.Wrap(err, "could not delete page") + return err } else if !svc.ac.CanDeletePage(svc.ctx, p) { - return errors.New("not allowed to delete this page") + return ErrNoDeletePermissions.withStack() } return svc.pageRepo.DeleteByID(namespaceID, pageID) diff --git a/compose/internal/service/record.go b/compose/internal/service/record.go index 73c9b7837..4409ee8f6 100644 --- a/compose/internal/service/record.go +++ b/compose/internal/service/record.go @@ -236,6 +236,14 @@ func (svc record) Update(mod *types.Record) (r *types.Record, err error) { } func (svc record) DeleteByID(namespaceID, recordID uint64) (err error) { + if recordID == 0 { + return ErrInvalidID.withStack() + } + + if namespaceID == 0 { + return ErrNamespaceRequired + } + err = svc.db.Transaction(func() (err error) { var record *types.Record diff --git a/compose/internal/service/trigger.go b/compose/internal/service/trigger.go index 9c0c42112..debf90309 100644 --- a/compose/internal/service/trigger.go +++ b/compose/internal/service/trigger.go @@ -138,6 +138,10 @@ func (svc trigger) Update(mod *types.Trigger) (c *types.Trigger, err error) { } func (svc trigger) DeleteByID(namespaceID, triggerID uint64) error { + if triggerID == 0 { + return ErrInvalidID.withStack() + } + if namespaceID == 0 { return ErrNamespaceRequired.withStack() } diff --git a/messaging/internal/service/attachment.go b/messaging/internal/service/attachment.go index 5e094e4b8..db976f0be 100644 --- a/messaging/internal/service/attachment.go +++ b/messaging/internal/service/attachment.go @@ -123,7 +123,7 @@ func (svc attachment) Create(name string, size int64, fh io.ReadSeeker, channelI if ch, err := svc.channel.FindByID(channelId); err != nil { return nil, err } else if svc.ac.CanAttachMessage(svc.ctx, ch) { - return nil, errors.WithStack(ErrNoPermissions) + return nil, ErrNoPermissions.withStack() } att = &types.Attachment{ diff --git a/messaging/internal/service/channel.go b/messaging/internal/service/channel.go index a5a9d8642..bebab929a 100644 --- a/messaging/internal/service/channel.go +++ b/messaging/internal/service/channel.go @@ -111,7 +111,7 @@ func (svc *channel) FindByID(ID uint64) (ch *types.Channel, err error) { } if !svc.ac.CanReadChannel(svc.ctx, ch) { - return nil, errors.WithStack(ErrNoPermissions) + return nil, ErrNoPermissions.withStack() } return @@ -231,20 +231,20 @@ func (svc *channel) Create(in *types.Channel) (out *types.Channel, err error) { // Group already exists so let's just return it return nil } else if out != nil && !out.CanObserve { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } } if in.Type == types.ChannelTypePublic && !svc.ac.CanCreatePublicChannel(svc.ctx) { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } if in.Type == types.ChannelTypePrivate && !svc.ac.CanCreatePrivateChannel(svc.ctx) { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } if in.Type == types.ChannelTypeGroup && !svc.ac.CanCreateGroupChannel(svc.ctx) { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } // This is a fresh channel, just copy values @@ -335,6 +335,10 @@ func (svc *channel) checkGroupExistance(mm types.ChannelMemberSet) (out *types.C } func (svc *channel) Update(in *types.Channel) (ch *types.Channel, err error) { + if in.ID == 0 { + return nil, ErrInvalidID.withStack() + } + if len(in.Name) == 0 && in.Type != types.ChannelTypeGroup { return nil, errors.New("channel name not provided") } @@ -355,20 +359,20 @@ func (svc *channel) Update(in *types.Channel) (ch *types.Channel, err error) { } if !svc.ac.CanUpdateChannel(svc.ctx, ch) { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } if in.Type.IsValid() && ch.Type != in.Type { if in.Type == types.ChannelTypePublic && !svc.ac.CanCreatePublicChannel(svc.ctx) { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } if in.Type == types.ChannelTypePrivate && !svc.ac.CanCreatePrivateChannel(svc.ctx) { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } if in.Type == types.ChannelTypeGroup && !svc.ac.CanCreateGroupChannel(svc.ctx) { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } changed = true @@ -417,6 +421,10 @@ func (svc *channel) Update(in *types.Channel) (ch *types.Channel, err error) { } func (svc *channel) Delete(ID uint64) (ch *types.Channel, err error) { + if ID == 0 { + return nil, ErrInvalidID.withStack() + } + return ch, svc.db.Transaction(func() (err error) { var userID = repository.Identity(svc.ctx) @@ -425,7 +433,7 @@ func (svc *channel) Delete(ID uint64) (ch *types.Channel, err error) { } if !svc.ac.CanDeleteChannel(svc.ctx, ch) { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } if ch.DeletedAt != nil { @@ -451,6 +459,10 @@ func (svc *channel) Delete(ID uint64) (ch *types.Channel, err error) { } func (svc *channel) Undelete(ID uint64) (ch *types.Channel, err error) { + if ID == 0 { + return nil, ErrInvalidID.withStack() + } + return ch, svc.db.Transaction(func() (err error) { var userID = repository.Identity(svc.ctx) @@ -459,7 +471,7 @@ func (svc *channel) Undelete(ID uint64) (ch *types.Channel, err error) { } if !svc.ac.CanUndeleteChannel(svc.ctx, ch) { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } if ch.DeletedAt == nil { @@ -481,6 +493,10 @@ func (svc *channel) Undelete(ID uint64) (ch *types.Channel, err error) { } func (svc *channel) SetFlag(ID uint64, flag types.ChannelMembershipFlag) (ch *types.Channel, err error) { + if ID == 0 { + return nil, ErrInvalidID.withStack() + } + return ch, svc.db.Transaction(func() (err error) { var membership *types.ChannelMember var userID = repository.Identity(svc.ctx) @@ -510,6 +526,10 @@ func (svc *channel) SetFlag(ID uint64, flag types.ChannelMembershipFlag) (ch *ty } func (svc *channel) Archive(ID uint64) (ch *types.Channel, err error) { + if ID == 0 { + return nil, ErrInvalidID.withStack() + } + return ch, svc.db.Transaction(func() (err error) { var userID = repository.Identity(svc.ctx) @@ -518,7 +538,7 @@ func (svc *channel) Archive(ID uint64) (ch *types.Channel, err error) { } if !svc.ac.CanArchiveChannel(svc.ctx, ch) { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } if ch.ArchivedAt != nil { @@ -540,6 +560,10 @@ func (svc *channel) Archive(ID uint64) (ch *types.Channel, err error) { } func (svc *channel) Unarchive(ID uint64) (ch *types.Channel, err error) { + if ID == 0 { + return nil, ErrInvalidID.withStack() + } + return ch, svc.db.Transaction(func() (err error) { var userID = repository.Identity(svc.ctx) @@ -548,7 +572,7 @@ func (svc *channel) Unarchive(ID uint64) (ch *types.Channel, err error) { } if !svc.ac.CanUnarchiveChannel(svc.ctx, ch) { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } if ch.ArchivedAt == nil { @@ -570,6 +594,16 @@ func (svc *channel) Unarchive(ID uint64) (ch *types.Channel, err error) { } func (svc *channel) InviteUser(channelID uint64, memberIDs ...uint64) (out types.ChannelMemberSet, err error) { + if channelID == 0 { + return nil, ErrInvalidID.withStack() + } + + for _, memberID := range memberIDs { + if memberID == 0 { + return nil, ErrInvalidID.withStack() + } + } + var ( userID = repository.Identity(svc.ctx) ch *types.Channel @@ -587,7 +621,7 @@ func (svc *channel) InviteUser(channelID uint64, memberIDs ...uint64) (out types } if !svc.ac.CanManageChannelMembers(svc.ctx, ch) { - return nil, errors.WithStack(ErrNoPermissions) + return nil, ErrNoPermissions.withStack() } return out, svc.db.Transaction(func() (err error) { @@ -622,6 +656,16 @@ func (svc *channel) InviteUser(channelID uint64, memberIDs ...uint64) (out types } func (svc *channel) AddMember(channelID uint64, memberIDs ...uint64) (out types.ChannelMemberSet, err error) { + if channelID == 0 { + return nil, ErrInvalidID.withStack() + } + + for _, memberID := range memberIDs { + if memberID == 0 { + return nil, ErrInvalidID.withStack() + } + } + var ( userID = repository.Identity(svc.ctx) ch *types.Channel @@ -656,9 +700,9 @@ func (svc *channel) AddMember(channelID uint64, memberIDs ...uint64) (out types. } if memberID == userID && !svc.ac.CanJoinChannel(svc.ctx, ch) { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } else if memberID != userID && !svc.ac.CanManageChannelMembers(svc.ctx, ch) { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } if !exists { @@ -726,10 +770,10 @@ func (svc *channel) DeleteMember(channelID uint64, memberIDs ...uint64) (err err } if memberID == userID && !svc.ac.CanJoinChannel(svc.ctx, ch) { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } if !svc.ac.CanManageChannelMembers(svc.ctx, ch) { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } if userID == memberID { @@ -742,7 +786,7 @@ func (svc *channel) DeleteMember(channelID uint64, memberIDs ...uint64) (err err return err } - svc.event.Part(memberID, channelID) + _ = svc.event.Part(memberID, channelID) } return svc.flushSystemMessages() diff --git a/messaging/internal/service/command.go b/messaging/internal/service/command.go index d1a86fb77..4c0d4667b 100644 --- a/messaging/internal/service/command.go +++ b/messaging/internal/service/command.go @@ -80,5 +80,4 @@ func (svc command) Do(channelID uint64, command, input string) (*types.Message, } return webhookSvc.Do(webhooks[0], input) } - return nil, ErrUnknownCommand.new() } diff --git a/messaging/internal/service/error.go b/messaging/internal/service/error.go index 1f6b149b6..bd141ed5e 100644 --- a/messaging/internal/service/error.go +++ b/messaging/internal/service/error.go @@ -5,18 +5,22 @@ import ( ) type ( - readableError string + serviceError string ) -func (e readableError) Error() string { - return string(e) -} - -func (e readableError) new() error { - return errors.WithStack(e) -} - const ( - ErrNoPermissions readableError = "You don't have permissions for this operation" - ErrUnknownCommand readableError = "Unknown command" + ErrInvalidID serviceError = "InvalidID" + ErrNoPermissions serviceError = "NoPermissions" ) + +func (e serviceError) Error() string { + return e.String() +} + +func (e serviceError) String() string { + return "crust.messaging.service." + string(e) +} + +func (e serviceError) withStack() error { + return errors.WithStack(e) +} diff --git a/messaging/internal/service/message.go b/messaging/internal/service/message.go index 254db6cf1..1479769e8 100644 --- a/messaging/internal/service/message.go +++ b/messaging/internal/service/message.go @@ -163,7 +163,7 @@ func (svc message) channelAccessCheck(IDs ...uint64) error { if ch, err := svc.findChannelByID(ID); err != nil { return err } else if !svc.ac.CanReadChannel(svc.ctx, ch) { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } } } @@ -259,10 +259,10 @@ func (svc message) Create(in *types.Message) (message *types.Message, err error) } if in.ReplyTo > 0 && !svc.ac.CanReplyMessage(svc.ctx, ch) { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } if !svc.ac.CanSendMessage(svc.ctx, ch) { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } if message, err = svc.message.Create(in); err != nil { @@ -282,6 +282,10 @@ func (svc message) Create(in *types.Message) (message *types.Message, err error) } func (svc message) Update(in *types.Message) (message *types.Message, err error) { + if in.ID == 0 { + return nil, ErrInvalidID.withStack() + } + if in == nil { in = &types.Message{} } @@ -304,7 +308,7 @@ func (svc message) Update(in *types.Message) (message *types.Message, err error) if ch, err = svc.findChannelByID(in.ChannelID); err != nil { return err } else if !svc.ac.CanReadChannel(svc.ctx, ch) { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } message, err = svc.message.FindByID(in.ID) @@ -319,9 +323,9 @@ func (svc message) Update(in *types.Message) (message *types.Message, err error) } if message.UserID == currentUserID && !svc.ac.CanUpdateOwnMessages(svc.ctx, ch) { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } else if message.UserID != currentUserID && !svc.ac.CanUpdateMessages(svc.ctx, ch) { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } // Allow message content to be changed @@ -358,7 +362,7 @@ func (svc message) Delete(messageID uint64) error { if ch, err = svc.findChannelByID(deletedMsg.ChannelID); err != nil { return err } else if !svc.ac.CanReadChannel(svc.ctx, ch) { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } if deletedMsg.ReplyTo > 0 { @@ -371,10 +375,10 @@ func (svc message) Delete(messageID uint64) error { return } if original.UserID == currentUserID && !svc.ac.CanUpdateOwnMessages(svc.ctx, ch) { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } if !svc.ac.CanUpdateMessages(svc.ctx, ch) { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } // This is a reply to another message, decrease reply counter on the original, on struct and in the @@ -422,7 +426,7 @@ func (svc message) MarkAsRead(channelID, threadID, lastReadMessageID uint64) (co if ch, err = svc.findChannelByID(channelID); err != nil { return err } else if !svc.ac.CanReadChannel(svc.ctx, ch) { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } else if !ch.IsValid() { return errors.New("invalid channel") } @@ -531,10 +535,10 @@ func (svc message) flag(messageID uint64, flag string, remove bool) error { return } if !svc.ac.CanReadChannel(svc.ctx, ch) { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } if f.IsReaction() && !svc.ac.CanReactMessage(svc.ctx, ch) { - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } if remove { diff --git a/messaging/internal/service/webhook.go b/messaging/internal/service/webhook.go index 5b9404c92..c5f447fbc 100644 --- a/messaging/internal/service/webhook.go +++ b/messaging/internal/service/webhook.go @@ -100,7 +100,7 @@ func (svc webhook) Create(kind types.WebhookKind, channelID uint64, params types } if !svc.ac.CanCreateWebhook(svc.ctx) { - return nil, errors.WithStack(ErrNoPermissions) + return nil, ErrNoPermissions.withStack() } botUser := &systemTypes.User{ @@ -127,13 +127,17 @@ func (svc webhook) Create(kind types.WebhookKind, channelID uint64, params types } func (svc webhook) Update(webhookID uint64, kind types.WebhookKind, channelID uint64, params types.WebhookRequest) (*types.Webhook, error) { + if webhookID == 0 { + return nil, ErrInvalidID.withStack() + } + webhook, err := svc.Get(webhookID) if err != nil { return nil, err } if !svc.ac.CanManageOwnWebhooks(svc.ctx, webhook) || !svc.ac.CanManageWebhooks(svc.ctx) { - return nil, errors.WithStack(ErrNoPermissions) + return nil, ErrNoPermissions.withStack() } botUser, err := svc.users.FindByID(webhook.UserID) @@ -173,7 +177,7 @@ func (svc webhook) Delete(webhookID uint64) error { if webhook.OwnerUserID == userID && svc.ac.CanManageOwnWebhooks(svc.ctx, webhook) { return svc.webhook.Delete(webhookID) } - return errors.WithStack(ErrNoPermissions) + return ErrNoPermissions.withStack() } func (svc webhook) DeleteByToken(webhookID uint64, webhookToken string) error { diff --git a/system/internal/service/error.go b/system/internal/service/error.go index 437459435..64c7ceffc 100644 --- a/system/internal/service/error.go +++ b/system/internal/service/error.go @@ -5,23 +5,22 @@ import ( ) type ( - serviceError string - readableError string + serviceError string +) + +const ( + ErrInvalidID serviceError = "InvalidID" + ErrNoPermissions serviceError = "NoPermissions" ) func (e serviceError) Error() string { - return "crust.messaging.service." + string(e) + return e.String() } -func (e readableError) Error() string { - return string(e) +func (e serviceError) String() string { + return "crust.system.service." + string(e) } -func (e readableError) new() error { +func (e serviceError) withStack() error { return errors.WithStack(e) } - -const ( - ErrNoPermissions readableError = "You don't have permissions for this operation" - ErrAvatarOnlyHTTPS readableError = "Avatar URL only supports HTTPS" -) diff --git a/system/internal/service/role.go b/system/internal/service/role.go index f50e3b642..fbf3bdc81 100644 --- a/system/internal/service/role.go +++ b/system/internal/service/role.go @@ -77,14 +77,22 @@ func (svc role) log(fields ...zapcore.Field) *zap.Logger { return logger.AddRequestID(svc.ctx, svc.logger).With(fields...) } -func (svc role) FindByID(id uint64) (*types.Role, error) { - role, err := svc.role.FindByID(id) +func (svc role) FindByID(roleID uint64) (*types.Role, error) { + return svc.findByID(roleID) +} + +func (svc role) findByID(roleID uint64) (*types.Role, error) { + if roleID == 0 { + return nil, ErrInvalidID + } + + role, err := svc.role.FindByID(roleID) if err != nil { return nil, err } if !svc.ac.CanReadRole(svc.ctx, role) { - return nil, errors.New("Not allowed to read role") + return nil, ErrNoPermissions.withStack() } return role, nil } @@ -106,14 +114,18 @@ func (svc role) Find(filter *types.RoleFilter) ([]*types.Role, error) { func (svc role) Create(mod *types.Role) (*types.Role, error) { if !svc.ac.CanCreateRole(svc.ctx) { - return nil, errors.New("Not allowed to create role") + return nil, ErrNoPermissions.withStack() } return svc.role.Create(mod) } func (svc role) Update(mod *types.Role) (t *types.Role, err error) { + if mod.ID == 0 { + return nil, ErrInvalidID + } + if !svc.ac.CanUpdateRole(svc.ctx, mod) { - return nil, errors.New("Not allowed to update role") + return nil, ErrNoPermissions.withStack() } // @todo: make sure archived & deleted entries can not be edited @@ -135,60 +147,122 @@ func (svc role) Update(mod *types.Role) (t *types.Role, err error) { }) } -func (svc role) Delete(id uint64) error { - // @todo: make history unavailable - // @todo: notify users that role has been removed (remove from web UI) - - rl := &types.Role{ID: id} - if !svc.ac.CanDeleteRole(svc.ctx, rl) { - return errors.New("Not allowed to delete role") +func (svc role) Delete(roleID uint64) error { + role, err := svc.findByID(roleID) + if err != nil { + return err + } + + if !svc.ac.CanDeleteRole(svc.ctx, role) { + return ErrNoPermissions.withStack() } - return svc.role.DeleteByID(id) -} -func (svc role) Archive(id uint64) error { // @todo: make history unavailable // @todo: notify users that role has been removed (remove from web UI) - // @todo: permissions check if current user can remove role - return svc.role.ArchiveByID(id) + + return svc.role.DeleteByID(roleID) } -func (svc role) Unarchive(id uint64) error { - // @todo: permissions check if current user can unarchive role +func (svc role) Archive(roleID uint64) error { + role, err := svc.findByID(roleID) + if err != nil { + return err + } + + if !svc.ac.CanUpdateRole(svc.ctx, role) { + return ErrNoPermissions.withStack() + } + + // @todo: make history unavailable + // @todo: notify users that role has been removed (remove from web UI) + return svc.role.ArchiveByID(roleID) +} + +func (svc role) Unarchive(roleID uint64) error { + role, err := svc.findByID(roleID) + if err != nil { + return err + } + + if !svc.ac.CanUpdateRole(svc.ctx, role) { + return ErrNoPermissions.withStack() + } + // @todo: make history accessible // @todo: notify users that role has been unarchived - return svc.role.UnarchiveByID(id) + return svc.role.UnarchiveByID(roleID) } -func (svc role) Merge(id, targetroleID uint64) error { - // @todo: permission check if current user can merge role - return svc.role.MergeByID(id, targetroleID) +func (svc role) Merge(roleID, targetRoleID uint64) error { + role, err := svc.findByID(roleID) + if err != nil { + return err + } + + if targetRoleID == 0 { + return ErrInvalidID + } + + if !svc.ac.CanUpdateRole(svc.ctx, role) { + return ErrNoPermissions.withStack() + } + + return svc.role.MergeByID(roleID, targetRoleID) } -func (svc role) Move(id, targetOrganisationID uint64) error { - // @todo: permission check if current user can move role to another organisation - return svc.role.MoveByID(id, targetOrganisationID) +func (svc role) Move(roleID, targetOrganisationID uint64) error { + role, err := svc.findByID(roleID) + if err != nil { + return err + } + + if targetOrganisationID == 0 { + return ErrInvalidID + } + + if !svc.ac.CanUpdateRole(svc.ctx, role) { + return ErrNoPermissions.withStack() + } + + return svc.role.MoveByID(roleID, targetOrganisationID) } func (svc role) MemberList(roleID uint64) ([]*types.RoleMember, error) { - rl := &types.Role{ID: roleID} - if !svc.ac.CanManageRoleMembers(svc.ctx, rl) { - return nil, errors.New("Not allowed to manage role members") + _, err := svc.findByID(roleID) + if err != nil { + return nil, err } + return svc.role.MemberFindByRoleID(roleID) } func (svc role) MemberAdd(roleID, userID uint64) error { - rl := &types.Role{ID: roleID} - if !svc.ac.CanManageRoleMembers(svc.ctx, rl) { + role, err := svc.findByID(roleID) + if err != nil { + return err + } + + if userID == 0 { + return ErrInvalidID + } + + if !svc.ac.CanManageRoleMembers(svc.ctx, role) { return errors.New("Not allowed to manage role members") } return svc.role.MemberAddByID(roleID, userID) } func (svc role) MemberRemove(roleID, userID uint64) error { - rl := &types.Role{ID: roleID} - if !svc.ac.CanManageRoleMembers(svc.ctx, rl) { + role, err := svc.findByID(roleID) + if err != nil { + return err + } + + if userID == 0 { + return ErrInvalidID + } + + if !svc.ac.CanManageRoleMembers(svc.ctx, role) { return errors.New("Not allowed to manage role members") } return svc.role.MemberRemoveByID(roleID, userID) diff --git a/system/internal/service/user.go b/system/internal/service/user.go index 4141fa1ff..2db96c3c3 100644 --- a/system/internal/service/user.go +++ b/system/internal/service/user.go @@ -87,12 +87,16 @@ func (svc user) log(fields ...zapcore.Field) *zap.Logger { return logger.AddRequestID(svc.ctx, svc.logger).With(fields...) } -func (svc user) FindByID(id uint64) (*types.User, error) { - return svc.user.FindByID(id) +func (svc user) FindByID(ID uint64) (*types.User, error) { + if ID == 0 { + return nil, ErrInvalidID + } + + return svc.user.FindByID(ID) } -func (svc user) FindByIDs(ids ...uint64) (types.UserSet, error) { - return svc.user.FindByIDs(ids...) +func (svc user) FindByIDs(userIDs ...uint64) (types.UserSet, error) { + return svc.user.FindByIDs(userIDs...) } func (svc user) FindByEmail(email string) (*types.User, error) { @@ -109,7 +113,7 @@ func (svc user) Find(filter *types.UserFilter) (types.UserSet, error) { func (svc user) Create(input *types.User) (out *types.User, err error) { if !svc.ac.CanCreateUser(svc.ctx) { - return nil, ErrNoPermissions.new() + return nil, ErrNoPermissions.withStack() } return out, svc.db.Transaction(func() (err error) { @@ -124,12 +128,16 @@ func (svc user) CreateWithAvatar(input *types.User, avatar io.Reader) (out *type } func (svc user) Update(mod *types.User) (u *types.User, err error) { + if mod.ID == 0 { + return nil, ErrInvalidID + } + if u, err = svc.user.FindByID(mod.ID); err != nil { return } if mod.ID != internalAuth.GetIdentityFromContext(svc.ctx).Identity() && !svc.ac.CanUpdateUser(svc.ctx, u) { - return nil, ErrNoPermissions.new() + return nil, ErrNoPermissions.withStack() } // Assign changed values @@ -150,47 +158,59 @@ func (svc user) UpdateWithAvatar(mod *types.User, avatar io.Reader) (out *types. return svc.Create(mod) } -func (svc user) Delete(id uint64) (err error) { +func (svc user) Delete(ID uint64) (err error) { + if ID == 0 { + return ErrInvalidID + } + var u *types.User - if u, err = svc.user.FindByID(id); err != nil { + if u, err = svc.user.FindByID(ID); err != nil { return } if !svc.ac.CanDeleteUser(svc.ctx, u) { - return ErrNoPermissions.new() + return ErrNoPermissions.withStack() } return svc.db.Transaction(func() (err error) { - return svc.user.DeleteByID(id) + return svc.user.DeleteByID(ID) }) } -func (svc user) Suspend(id uint64) (err error) { +func (svc user) Suspend(ID uint64) (err error) { + if ID == 0 { + return ErrInvalidID + } + var u *types.User - if u, err = svc.user.FindByID(id); err != nil { + if u, err = svc.user.FindByID(ID); err != nil { return } if !svc.ac.CanSuspendUser(svc.ctx, u) { - return ErrNoPermissions.new() + return ErrNoPermissions.withStack() } return svc.db.Transaction(func() (err error) { - return svc.user.SuspendByID(id) + return svc.user.SuspendByID(ID) }) } -func (svc user) Unsuspend(id uint64) (err error) { +func (svc user) Unsuspend(ID uint64) (err error) { + if ID == 0 { + return ErrInvalidID + } + var u *types.User - if u, err = svc.user.FindByID(id); err != nil { + if u, err = svc.user.FindByID(ID); err != nil { return } if !svc.ac.CanUnsuspendUser(svc.ctx, u) { - return ErrNoPermissions.new() + return ErrNoPermissions.withStack() } return svc.db.Transaction(func() (err error) { - return svc.user.UnsuspendByID(id) + return svc.user.UnsuspendByID(ID) }) }