Fix user data masking on search
This commit is contained in:
@@ -15,7 +15,7 @@ func (s Store) convertUserFilter(f types.UserFilter) (query squirrel.SelectBuild
|
||||
query = filter.StateCondition(query, "usr.suspended_at", f.Suspended)
|
||||
|
||||
if len(f.UserID) > 0 {
|
||||
query = query.Where(squirrel.Eq{"usr.ID": f.UserID})
|
||||
query = query.Where(squirrel.Eq{"usr.id": f.UserID})
|
||||
}
|
||||
|
||||
if len(f.LabeledIDs) > 0 {
|
||||
@@ -38,11 +38,12 @@ func (s Store) convertUserFilter(f types.UserFilter) (query squirrel.SelectBuild
|
||||
query = query.Where(squirrel.Or{
|
||||
squirrel.Like{"usr.username": qs},
|
||||
squirrel.Like{"usr.handle": qs},
|
||||
})
|
||||
}
|
||||
|
||||
if f.Email != "" {
|
||||
query = query.Where(squirrel.Eq{"usr.email": f.Email})
|
||||
// do a lookup on a potentially masked fields
|
||||
// this should be filtered out in the check function
|
||||
squirrel.Like{"usr.email": qs},
|
||||
squirrel.Like{"usr.name": qs},
|
||||
})
|
||||
}
|
||||
|
||||
if f.Email != "" {
|
||||
|
||||
+25
-6
@@ -275,7 +275,7 @@ func (svc user) proc(u *types.User, err error) (*types.User, error) {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
svc.handlePrivateData(u)
|
||||
svc.handlePrivateData(svc.ctx, u)
|
||||
|
||||
return u, nil
|
||||
}
|
||||
@@ -289,12 +289,23 @@ func (svc user) Find(filter types.UserFilter) (uu types.UserSet, f types.UserFil
|
||||
)
|
||||
|
||||
// For each fetched item, store backend will check if it is valid or not
|
||||
filter.MaskedEmailsEnabled = svc.settings.Privacy.Mask.Email
|
||||
filter.MaskedNamesEnabled = svc.settings.Privacy.Mask.Name
|
||||
filter.Check = func(res *types.User) (bool, error) {
|
||||
if !svc.ac.CanReadUser(svc.ctx, res) {
|
||||
return false, nil
|
||||
}
|
||||
|
||||
svc.handlePrivateData(res)
|
||||
if svc.maskEmail(svc.ctx, res) && ((len(filter.Query) > 0 && strings.HasPrefix(res.Email, filter.Query)) || res.Email == filter.Email) {
|
||||
// user email matched but it will be masked later on, so exclude it to prevent data probing
|
||||
return false, nil
|
||||
}
|
||||
|
||||
if svc.maskName(svc.ctx, res) && (len(filter.Query) > 0 && strings.HasPrefix(res.Name, filter.Query)) {
|
||||
// user mail matched but it will be masked later on, so exclude it to prevent data probing
|
||||
return false, nil
|
||||
}
|
||||
|
||||
return true, nil
|
||||
}
|
||||
|
||||
@@ -338,7 +349,7 @@ func (svc user) Find(filter types.UserFilter) (uu types.UserSet, f types.UserFil
|
||||
}
|
||||
|
||||
return uu.Walk(func(u *types.User) error {
|
||||
svc.handlePrivateData(u)
|
||||
svc.handlePrivateData(svc.ctx, u)
|
||||
return nil
|
||||
})
|
||||
}()
|
||||
@@ -661,16 +672,24 @@ func (svc user) SetPassword(userID uint64, newPassword string) (err error) {
|
||||
}
|
||||
|
||||
// Masks (or leaves as-is) private data on user
|
||||
func (svc user) handlePrivateData(u *types.User) {
|
||||
if !svc.ac.CanUnmaskEmail(svc.ctx, u) {
|
||||
func (svc user) handlePrivateData(ctx context.Context, u *types.User) {
|
||||
if svc.maskEmail(ctx, u) {
|
||||
u.Email = maskPrivateDataEmail
|
||||
}
|
||||
|
||||
if !svc.ac.CanUnmaskName(svc.ctx, u) {
|
||||
if svc.maskName(ctx, u) {
|
||||
u.Name = maskPrivateDataName
|
||||
}
|
||||
}
|
||||
|
||||
func (svc user) maskEmail(ctx context.Context, u *types.User) bool {
|
||||
return svc.settings.Privacy.Mask.Email && !svc.ac.CanUnmaskEmail(ctx, u)
|
||||
}
|
||||
|
||||
func (svc user) maskName(ctx context.Context, u *types.User) bool {
|
||||
return svc.settings.Privacy.Mask.Name && !svc.ac.CanUnmaskName(ctx, u)
|
||||
}
|
||||
|
||||
// Preloader collects all ids of users, loads them and sets them back
|
||||
//
|
||||
//
|
||||
|
||||
@@ -0,0 +1,103 @@
|
||||
package service
|
||||
|
||||
import (
|
||||
"context"
|
||||
a "github.com/cortezaproject/corteza-server/pkg/auth"
|
||||
"github.com/cortezaproject/corteza-server/pkg/eventbus"
|
||||
"github.com/cortezaproject/corteza-server/pkg/rbac"
|
||||
"github.com/cortezaproject/corteza-server/store"
|
||||
"github.com/cortezaproject/corteza-server/store/sqlite3"
|
||||
"github.com/cortezaproject/corteza-server/system/types"
|
||||
"github.com/stretchr/testify/require"
|
||||
"go.uber.org/zap"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// Mock auth service with nil for current time, dummy provider validator and mock db
|
||||
func makeMockUserService() *user {
|
||||
var (
|
||||
ctx = context.Background()
|
||||
|
||||
mem, err = sqlite3.ConnectInMemory(ctx)
|
||||
|
||||
svc = &user{
|
||||
settings: &types.AppSettings{},
|
||||
ac: AccessControl(rbac.NewService(zap.NewNop(), mem)),
|
||||
eventbus: eventbus.New(),
|
||||
}
|
||||
)
|
||||
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
|
||||
if err = store.Upgrade(ctx, zap.NewNop(), mem); err != nil {
|
||||
panic(err)
|
||||
}
|
||||
|
||||
svc.store = mem
|
||||
|
||||
return svc
|
||||
}
|
||||
|
||||
func TestUser_ProtectedSearch(t *testing.T) {
|
||||
const testRoleID = 123
|
||||
|
||||
var (
|
||||
req = require.New(t)
|
||||
ctx = context.Background()
|
||||
|
||||
masked = &types.User{Email: "email.masked@us.er", Name: "Name MSKD", ID: nextID(), CreatedAt: *now()}
|
||||
unmasked = &types.User{Email: "email.unmasked@us.er", Name: "Name UNMSKD", ID: nextID(), CreatedAt: *now()}
|
||||
|
||||
set types.UserSet
|
||||
err error
|
||||
|
||||
testUser = &types.User{ID: 42}
|
||||
)
|
||||
|
||||
testUser.SetRoles([]uint64{testRoleID})
|
||||
ctx = a.SetIdentityToContext(ctx, testUser)
|
||||
|
||||
svc := makeMockUserService()
|
||||
|
||||
svc.ac.(*accessControl).permissions.Grant(ctx, svc.ac.(*accessControl).Whitelist(),
|
||||
rbac.AllowRule(testRoleID, (&types.User{}).RBACResource().AppendWildcard(), "read"),
|
||||
rbac.DenyRule(testRoleID, masked.RBACResource(), "unmask.email"),
|
||||
rbac.AllowRule(testRoleID, unmasked.RBACResource(), "unmask.email"),
|
||||
rbac.DenyRule(testRoleID, masked.RBACResource(), "unmask.name"),
|
||||
rbac.AllowRule(testRoleID, unmasked.RBACResource(), "unmask.name"),
|
||||
)
|
||||
req.NoError(store.CreateUser(ctx, svc.store, masked, unmasked))
|
||||
|
||||
t.Run("with disabled masking", func(t *testing.T) {
|
||||
// Masking disabled, expecting to fetch both users
|
||||
svc.settings.Privacy.Mask.Email = false
|
||||
svc.settings.Privacy.Mask.Name = false
|
||||
set, _, err = svc.With(ctx).Find(types.UserFilter{Query: "email"})
|
||||
req.NoError(err)
|
||||
req.Len(set, 2)
|
||||
req.NotEqual(set[0].Email, maskPrivateDataEmail)
|
||||
req.NotEqual(set[1].Email, maskPrivateDataEmail)
|
||||
})
|
||||
|
||||
t.Run("with enabled privacy", func(t *testing.T) {
|
||||
// Masking enabled, expecting to fetch only unmasked
|
||||
svc.settings.Privacy.Mask.Email = true
|
||||
svc.settings.Privacy.Mask.Name = true
|
||||
|
||||
set, _, err = svc.With(ctx).Find(types.UserFilter{Query: "email"})
|
||||
req.NoError(err)
|
||||
req.Len(set, 1)
|
||||
})
|
||||
|
||||
t.Run("email search with enabled privacy", func(t *testing.T) {
|
||||
// Masking enabled, expecting to fetch only unmasked
|
||||
svc.settings.Privacy.Mask.Email = true
|
||||
svc.settings.Privacy.Mask.Name = true
|
||||
|
||||
set, _, err = svc.With(ctx).Find(types.UserFilter{Email: "email.masked@us.er"})
|
||||
req.NoError(err)
|
||||
req.Len(set, 0)
|
||||
})
|
||||
}
|
||||
@@ -64,6 +64,9 @@ type (
|
||||
// Store then loads additional resources to satisfy the paging parameters
|
||||
Check func(*User) (bool, error) `json:"-"`
|
||||
|
||||
MaskedEmailsEnabled bool `json:"-"`
|
||||
MaskedNamesEnabled bool `json:"-"`
|
||||
|
||||
// Standard helpers for paging and sorting
|
||||
filter.Sorting
|
||||
filter.Paging
|
||||
|
||||
@@ -45,6 +45,13 @@ func TestUserRead(t *testing.T) {
|
||||
h := newHelper(t)
|
||||
h.clearUsers()
|
||||
|
||||
service.CurrentSettings.Privacy.Mask.Email = true
|
||||
service.CurrentSettings.Privacy.Mask.Name = true
|
||||
defer func() {
|
||||
service.CurrentSettings.Privacy.Mask.Email = false
|
||||
service.CurrentSettings.Privacy.Mask.Name = false
|
||||
}()
|
||||
|
||||
u := h.createUserWithEmail(h.randEmail())
|
||||
|
||||
h.apiInit().
|
||||
@@ -88,7 +95,6 @@ func TestUserListAll(t *testing.T) {
|
||||
Assert(helpers.AssertNoErrors).
|
||||
Assert(jsonpath.Present(`$.response.filter`)).
|
||||
Assert(jsonpath.Present(`$.response.set`)).
|
||||
Assert(jsonpath.Len(`$.response.set`, seedCount)).
|
||||
End()
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user