From b2221743449faaf9188b1e72943b870c41766c3c Mon Sep 17 00:00:00 2001 From: Denis Arh Date: Wed, 9 Oct 2019 20:44:56 +0200 Subject: [PATCH 1/2] Add basic user data masking functionality --- system/service/access_control.go | 8 ++++++ system/service/user.go | 46 ++++++++++++++++++++++++++++---- 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/system/service/access_control.go b/system/service/access_control.go index 403540817..ec42c8da1 100644 --- a/system/service/access_control.go +++ b/system/service/access_control.go @@ -129,6 +129,14 @@ func (svc accessControl) CanDeleteUser(ctx context.Context, u *types.User) bool return svc.can(ctx, u, "delete") } +func (svc accessControl) CanUnmaskEmail(ctx context.Context, u *types.User) bool { + return svc.can(ctx, u, "unmask.email") +} + +func (svc accessControl) CanUnmaskName(ctx context.Context, u *types.User) bool { + return svc.can(ctx, u, "unmask.name") +} + func (svc accessControl) CanReadAnyAutomationScript(ctx context.Context) bool { return svc.can(ctx, types.AutomationScriptPermissionResource.AppendWildcard(), "read") } diff --git a/system/service/user.go b/system/service/user.go index 06833fbb4..94acb8c05 100644 --- a/system/service/user.go +++ b/system/service/user.go @@ -21,6 +21,9 @@ const ( ErrUserUsernameNotUnique = serviceError("UserUsernameNotUnique") ErrUserEmailNotUnique = serviceError("UserEmailNotUnique") ErrUserLocked = serviceError("UserLocked") + + maskPrivateDataEmail = "####.#######@######.###" + maskPrivateDataName = "##### ##########" ) type ( @@ -36,6 +39,11 @@ type ( ac userAccessController user repository.UserRepository credentials repository.CredentialsRepository + + // @todo wire this with settings (privacy.mask.email) + privacyMaskEmail bool + // @todo wire this with settings (privacy.mask.name) + privacyMaskName bool } userAuth interface { @@ -50,6 +58,8 @@ type ( CanDeleteUser(context.Context, *types.User) bool CanSuspendUser(context.Context, *types.User) bool CanUnsuspendUser(context.Context, *types.User) bool + CanUnmaskEmail(context.Context, *types.User) bool + CanUnmaskName(context.Context, *types.User) bool } UserService interface { @@ -101,6 +111,12 @@ func (svc user) With(ctx context.Context) UserService { user: repository.User(ctx, db), credentials: repository.Credentials(ctx, db), + + // @todo wire this with settings (privacy.mask.email) + privacyMaskEmail: true, + + // @todo wire this with settings (privacy.mask.name) + privacyMaskName: true, } } @@ -129,11 +145,15 @@ func (svc user) proc(u *types.User, err error) (*types.User, error) { return nil, err } + svc.handlePrivateData(u) + return u, nil } func (svc user) FindByIDs(userIDs ...uint64) (types.UserSet, error) { - return svc.procSet(svc.user.FindByIDs(userIDs...)) + uu, err := svc.user.FindByIDs(userIDs...) + uu, _, err = svc.procSet(uu, types.UserFilter{}, err) + return uu, err } func (svc user) Find(f types.UserFilter) (types.UserSet, types.UserFilter, error) { @@ -143,15 +163,20 @@ func (svc user) Find(f types.UserFilter) (types.UserSet, types.UserFilter, error } } - return svc.user.Find(f) + return svc.procSet(svc.user.Find(f)) } -func (svc user) procSet(u types.UserSet, err error) (types.UserSet, error) { +func (svc user) procSet(u types.UserSet, f types.UserFilter, err error) (types.UserSet, types.UserFilter, error) { if err != nil { - return nil, err + return nil, f, err } - return u, nil + _ = u.Walk(func(u *types.User) error { + svc.handlePrivateData(u) + return nil + }) + + return u, f, nil } func (svc user) Create(input *types.User) (out *types.User, err error) { @@ -323,3 +348,14 @@ func (svc user) SetPassword(userID uint64, newPassword string) (err error) { return nil }) } + +// Masks (or leaves as-is) private data on user +func (svc user) handlePrivateData(u *types.User) { + if svc.privacyMaskEmail && !svc.ac.CanUnmaskEmail(svc.ctx, u) { + u.Email = maskPrivateDataEmail + } + + if svc.privacyMaskName && !svc.ac.CanUnmaskEmail(svc.ctx, u) { + u.Name = maskPrivateDataName + } +} From a66f75c46d573adf7676f1d320dd5b0f9f9a8ad8 Mon Sep 17 00:00:00 2001 From: Denis Arh Date: Thu, 10 Oct 2019 10:02:23 +0200 Subject: [PATCH 2/2] Refactor user repo, enable user data masking --- pkg/permissions/filter.go | 4 +++ pkg/rh/access_check.go | 51 ++++++++++++++++++++++++++++++++ system/repository/user.go | 36 +++++++++++++++------- system/rest/user.go | 5 ++-- system/service/access_control.go | 12 ++++++++ system/service/user.go | 36 ++++++++++++++++++++-- system/types/user.go | 18 ++++++++--- 7 files changed, 144 insertions(+), 18 deletions(-) create mode 100644 pkg/rh/access_check.go diff --git a/pkg/permissions/filter.go b/pkg/permissions/filter.go index 0dc701eb9..75ebfa217 100644 --- a/pkg/permissions/filter.go +++ b/pkg/permissions/filter.go @@ -43,6 +43,10 @@ func (ac AccessCheck) HasOperation() bool { return ac.operation != "" } +func (ac AccessCheck) HasResource() bool { + return ac.resource != "" +} + // ToSql converts access check to SQL (with args) that will help with filtering // // Satisfies squirrel.Sqlizer interface diff --git a/pkg/rh/access_check.go b/pkg/rh/access_check.go new file mode 100644 index 000000000..fd9620a71 --- /dev/null +++ b/pkg/rh/access_check.go @@ -0,0 +1,51 @@ +package rh + +import ( + "fmt" + + "gopkg.in/Masterminds/squirrel.v1" + + "github.com/cortezaproject/corteza-server/pkg/permissions" +) + +type ( + expAccessCheck struct { + ac permissions.AccessCheck + expr squirrel.Sqlizer + } +) + +// ExpAccessCheck wraps expression into access control check +// +// If access-check is enabled and user can not perform an operation +// condition will fail +func ExpAccessCheck(ac permissions.AccessCheck, expr squirrel.Sqlizer) expAccessCheck { + if ac.HasOperation() && !ac.HasResource() { + // This is not runtime error + // developer should make sure access control has resource! + panic("can not wrap expression with access check without resource") + } + + return expAccessCheck{ + ac: ac, + expr: expr, + } +} + +func (eac expAccessCheck) ToSql() (string, []interface{}, error) { + if !eac.ac.HasOperation() { + return eac.expr.ToSql() + } + + acSql, acArgs, err := eac.ac.ToSql() + if err != nil { + return "", nil, err + } + + sql, args, err := eac.expr.ToSql() + if err != nil { + return "", nil, err + } + + return fmt.Sprintf(`IF(%s, %s, false)`, acSql, sql), append(acArgs, args...), nil +} diff --git a/system/repository/user.go b/system/repository/user.go index c7f7807ed..fab0f66cc 100644 --- a/system/repository/user.go +++ b/system/repository/user.go @@ -8,6 +8,7 @@ import ( "github.com/titpetric/factory" "gopkg.in/Masterminds/squirrel.v1" + "github.com/cortezaproject/corteza-server/pkg/rh" "github.com/cortezaproject/corteza-server/system/types" ) @@ -127,37 +128,46 @@ func (r user) Find(filter types.UserFilter) (set types.UserSet, f types.UserFilt f = filter q := r.queryNoFilter() + f.AccessCheckEmail.BindToEnv(types.UserPermissionResource, "sys") + f.AccessCheckName.BindToEnv(types.UserPermissionResource, "sys") + if !f.IncDeleted { - q = q.Where("u.deleted_at IS NULL") + q = q.Where(squirrel.Eq{"u.deleted_at": nil}) } if !f.IncSuspended { - q = q.Where("u.suspended_at IS NULL") + q = q.Where(squirrel.Eq{"u.suspended_at": nil}) } if f.Query != "" { qs := f.Query + "%" - q = q.Where("u.username LIKE ? OR u.email LIKE ? OR u.name LIKE ?", qs, qs, qs) + q = q.Where(squirrel.Or{ + squirrel.Like{"u.username": qs}, + squirrel.Like{"u.handle": qs}, + rh.ExpAccessCheck(f.AccessCheckEmail, squirrel.Like{"u.email": qs}), + rh.ExpAccessCheck(f.AccessCheckName, squirrel.Like{"u.name": qs}), + }) } if f.Email != "" { - q = q.Where("u.email = ?", f.Email) + q = q.Where(rh.ExpAccessCheck(f.AccessCheckEmail, squirrel.Eq{"u.email": f.Email})) } if f.Username != "" { - q = q.Where("u.username = ?", f.Username) + q = q.Where(squirrel.Eq{"u.username": f.Username}) } if f.Handle != "" { - q = q.Where("u.handle = ?", f.Handle) + q = q.Where(squirrel.Eq{"u.handle": f.Handle}) } if f.Kind != "" { - q = q.Where("u.kind = ?", f.Kind) + q = q.Where(squirrel.Eq{"u.kind": f.Kind}) } - if f.Email != "" { - q = q.Where("u.email = ?", f.Email) + if f.AccessCheck.HasOperation() { + // Filter users based on what current user can read + q = q.Where(f.AccessCheck.BindToEnv(types.UserPermissionResource, "sys")) } // @todo add support for more sophisticated sorting through ql @@ -180,7 +190,13 @@ func (r user) Find(filter types.UserFilter) (set types.UserSet, f types.UserFilt q = q.OrderBy("id") } - return set, f, r.fetchPaged(&set, q, f.Page, f.PerPage) + db := r.db() + + if f.Count, err = rh.Count(db, q); err != nil { + return + } + + return set, f, rh.FetchPaged(db, q, f.Page, f.PerPage, &set) } func (r user) Total() (count uint) { diff --git a/system/rest/user.go b/system/rest/user.go index e2cdd74d7..ca8b5df72 100644 --- a/system/rest/user.go +++ b/system/rest/user.go @@ -7,6 +7,7 @@ import ( "github.com/titpetric/factory/resputil" "github.com/cortezaproject/corteza-server/pkg/payload" + "github.com/cortezaproject/corteza-server/pkg/rh" "github.com/cortezaproject/corteza-server/system/rest/request" "github.com/cortezaproject/corteza-server/system/service" "github.com/cortezaproject/corteza-server/system/types" @@ -42,8 +43,8 @@ func (ctrl User) List(ctx context.Context, r *request.UserList) (interface{}, er Kind: r.Kind, IncSuspended: r.IncSuspended, IncDeleted: r.IncDeleted, - Page: r.Page, - PerPage: r.PerPage, + + PageFilter: rh.Paging(r.Page, r.PerPage), } set, filter, err := ctrl.user.With(ctx).Find(f) diff --git a/system/service/access_control.go b/system/service/access_control.go index ec42c8da1..05ea15ee9 100644 --- a/system/service/access_control.go +++ b/system/service/access_control.go @@ -113,6 +113,18 @@ func (svc accessControl) CanDeleteApplication(ctx context.Context, app *types.Ap return svc.can(ctx, app, "delete") } +func (svc accessControl) CanReadAnyUser(ctx context.Context) bool { + return svc.can(ctx, types.UserPermissionResource.AppendWildcard(), "read") +} + +func (svc accessControl) CanUnmaskEmailOnAnyUser(ctx context.Context) bool { + return svc.can(ctx, types.UserPermissionResource.AppendWildcard(), "unmask.email") +} + +func (svc accessControl) CanUnmaskNameOnAnyUser(ctx context.Context) bool { + return svc.can(ctx, types.UserPermissionResource.AppendWildcard(), "unmask.name") +} + func (svc accessControl) CanUpdateUser(ctx context.Context, u *types.User) bool { return svc.can(ctx, u, "update") } diff --git a/system/service/user.go b/system/service/user.go index 94acb8c05..e389b4147 100644 --- a/system/service/user.go +++ b/system/service/user.go @@ -11,6 +11,7 @@ import ( internalAuth "github.com/cortezaproject/corteza-server/pkg/auth" "github.com/cortezaproject/corteza-server/pkg/logger" + "github.com/cortezaproject/corteza-server/pkg/permissions" "github.com/cortezaproject/corteza-server/system/repository" "github.com/cortezaproject/corteza-server/system/types" ) @@ -53,6 +54,9 @@ type ( userAccessController interface { CanAccess(context.Context) bool + CanReadAnyUser(context.Context) bool + CanUnmaskEmailOnAnyUser(context.Context) bool + CanUnmaskNameOnAnyUser(context.Context) bool CanCreateUser(context.Context) bool CanUpdateUser(context.Context, *types.User) bool CanDeleteUser(context.Context, *types.User) bool @@ -113,10 +117,12 @@ func (svc user) With(ctx context.Context) UserService { credentials: repository.Credentials(ctx, db), // @todo wire this with settings (privacy.mask.email) - privacyMaskEmail: true, + // new default value will be true! + privacyMaskEmail: false, // @todo wire this with settings (privacy.mask.name) - privacyMaskName: true, + // new default value will be true! + privacyMaskName: false, } } @@ -158,11 +164,37 @@ func (svc user) FindByIDs(userIDs ...uint64) (types.UserSet, error) { func (svc user) Find(f types.UserFilter) (types.UserSet, types.UserFilter, error) { if f.IncDeleted || f.IncSuspended { + // If list with deleted or suspended users is requested + // user must have access permissions to system (ie: is admin) if !svc.ac.CanAccess(svc.ctx) { return nil, f, ErrNoPermissions.withStack() } } + if svc.privacyMaskEmail { + // Prepare filter for email unmasking check + f.AccessCheckEmail = permissions.InitAccessCheckFilter( + "unmask.email", + internalAuth.GetIdentityFromContext(svc.ctx).Roles(), + svc.ac.CanUnmaskEmailOnAnyUser(svc.ctx), + ) + } + + if svc.privacyMaskName { + // Prepare filter for name unmasking check + f.AccessCheckName = permissions.InitAccessCheckFilter( + "unmask.name", + internalAuth.GetIdentityFromContext(svc.ctx).Roles(), + svc.ac.CanUnmaskNameOnAnyUser(svc.ctx), + ) + } + + f.AccessCheck = permissions.InitAccessCheckFilter( + "read", + internalAuth.GetIdentityFromContext(svc.ctx).Roles(), + svc.ac.CanReadAnyUser(svc.ctx), + ) + return svc.procSet(svc.user.Find(f)) } diff --git a/system/types/user.go b/system/types/user.go index 86300b8a6..bbb216950 100644 --- a/system/types/user.go +++ b/system/types/user.go @@ -8,6 +8,7 @@ import ( "github.com/pkg/errors" "github.com/cortezaproject/corteza-server/pkg/permissions" + "github.com/cortezaproject/corteza-server/pkg/rh" ) type ( @@ -51,10 +52,19 @@ type ( IncDeleted bool `json:"incDeleted"` IncSuspended bool `json:"incSuspended"` - Page uint `json:"page"` - PerPage uint `json:"perPage"` - Sort string `json:"sort"` - Count uint `json:"count"` + Sort string `json:"sort"` + + // Can we use email for searching or is it supposed to be masked for the current user? + AccessCheckEmail permissions.AccessCheck `json:"-"` + + // Can we use name for searching or is it supposed to be masked for the current user? + AccessCheckName permissions.AccessCheck `json:"-"` + + // Access check for user + permissions.AccessCheck `json:"-"` + + // Standard paging fields & helpers + rh.PageFilter } UserKind string