DeforaNetworks/corteza
3
0
Fork 0
Code Activity

Add proper support and access control for record owner

Browse Source
This commit is contained in:
Denis Arh
2022-05-26 20:56:33 +02:00
parent 35469c4749
commit 5c317cdbdf
5 changed files with 217 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
compose/record.cue
+1
View File
@@ -42,6 +42,7 @@ record: schema.#Resource & {
"read": {}
"update": {}
"delete": {}
"owner.manage": {}
}
}
compose/service/access_control.gen.go
Generated
+16 -3
View File
@@ -184,6 +184,11 @@ func (svc accessControl) List() (out []map[string]string) {
"any": types.RecordRbacResource(0, 0, 0),
"op": "delete",
},
{
"type": types.RecordResourceType,
"any": types.RecordRbacResource(0, 0, 0),
"op": "owner.manage",
},
{
"type": types.ComponentResourceType,
"any": types.ComponentRbacResource(),
@@ -469,6 +474,13 @@ func (svc accessControl) CanDeleteRecord(ctx context.Context, r *types.Record) b
return svc.can(ctx, "delete", r)
}
// CanManageOwnerOnRecord checks if current user can owner.manage
//
// This function is auto-generated
func (svc accessControl) CanManageOwnerOnRecord(ctx context.Context, r *types.Record) bool {
return svc.can(ctx, "owner.manage", r)
}
// CanGrant checks if current user can manage compose permissions
//
// This function is auto-generated
@@ -586,9 +598,10 @@ func rbacResourceOperations(r string) map[string]bool {
}
case types.RecordResourceType:
return map[string]bool{
"read": true,
"update": true,
"delete": true,
"read": true,
"update": true,
"delete": true,
"owner.manage": true,
}
case types.ComponentResourceType:
return map[string]bool{
compose/service/record.go
+83 -21
View File
@@ -4,6 +4,7 @@ import (
"context"
"encoding/json"
"fmt"
"github.com/cortezaproject/corteza-server/pkg/locale"
"regexp"
"sort"
"strconv"
@@ -70,6 +71,10 @@ type (
CanUpdateRecordValueOnModuleField(context.Context, *types.ModuleField) bool
}
recordManageOwnerAccessController interface {
CanManageOwnerOnRecord(context.Context, *types.Record) bool
}
recordAccessController interface {
CanCreateRecordOnModule(context.Context, *types.Module) bool
CanSearchRecordsOnModule(context.Context, *types.Module) bool
@@ -79,6 +84,7 @@ type (
CanUpdateRecord(context.Context, *types.Record) bool
CanDeleteRecord(context.Context, *types.Record) bool
recordManageOwnerAccessController
recordValueAccessController
}
@@ -617,29 +623,68 @@ func RecordValueSanitization(m *types.Module, vv types.RecordValueSet) (err erro
return
}
func RecordUpdateOwner(invokerID uint64, r, old *types.Record) *types.Record {
if old == nil {
if r.OwnedBy == 0 {
// If od owner is not set, make current user
// the owner of the record
r.OwnedBy = invokerID
func SetRecordOwner(ctx context.Context, ac recordManageOwnerAccessController, s store.Storer, old, upd *types.Record, invoker uint64) *types.RecordValueErrorSet {
if upd == nil {
// no-op
return nil
}
var (
curOwner uint64
updOwner = upd.OwnedBy
)
if old != nil {
curOwner = old.OwnedBy
}
updOwner = CalcRecordOwner(curOwner, updOwner, invoker)
var (
mkError = func(kind, tkey string) *types.RecordValueErrorSet {
return &types.RecordValueErrorSet{Set: []types.RecordValueError{{
Kind: kind,
Meta: map[string]interface{}{"field": "", "value": updOwner},
Message: locale.Global().T(ctx, "compose", tkey),
}}}
}
} else {
if r.OwnedBy == 0 {
if old.OwnedBy > 0 {
// Owner not set/send in the payload
//
// Fallback to old owner (if set)
r.OwnedBy = old.OwnedBy
} else {
// If od owner is not set, make current user
// the owner of the record
r.OwnedBy = invokerID
}
)
if (old != nil && curOwner != updOwner) || (old == nil && updOwner != invoker) {
// check if ownership can be changed when:
// a) updating (old != nil) and ownership changed
// b) creating (old == nil) and ownership id not set to the invoking user
if !ac.CanManageOwnerOnRecord(ctx, upd) {
return mkError("accessDenied", "record.errors.ownershipChangeDenied")
}
}
return r
if _, err := store.LookupUserByID(ctx, s, updOwner); err != nil {
if errors.IsNotFound(err) {
return mkError("invalidValue", "record.errors.invalidOwner")
} else {
return mkError("internal", "record.errors.store")
}
}
upd.OwnedBy = updOwner
return nil
}
func CalcRecordOwner(current, new, invoker uint64) uint64 {
if invoker == 0 {
// invoker is, for some reason 0,
// use current owner as invoker
invoker = current
}
if new == 0 {
// if new owner is not set, use invoker
return invoker
}
// keep owner unchanged
return new
}
func RecordValueUpdateOpCheck(ctx context.Context, ac recordValueAccessController, m *types.Module, vv types.RecordValueSet) *types.RecordValueErrorSet {
@@ -873,7 +918,22 @@ func (svc record) procCreate(ctx context.Context, invokerID uint64, m *types.Mod
new.DeletedAt = nil
new.DeletedBy = 0
new = RecordUpdateOwner(invokerID, new, nil)
if new.OwnedBy == 0 {
new.OwnedBy = invokerID
}
if new.OwnedBy != invokerID {
// we're creating record here and this check goes against current
// RBAC implementation logic on record creation
if !svc.ac.CanManageOwnerOnRecord(ctx, new) {
// not permitted to change the owner
}
}
if err := SetRecordOwner(ctx, svc.ac, svc.store, nil, new, invokerID); err != nil {
return err
}
if rve = RecordValueUpdateOpCheck(ctx, svc.ac, m, new.Values); !rve.IsValid() {
return
@@ -924,7 +984,9 @@ func (svc record) procUpdate(ctx context.Context, invokerID uint64, m *types.Mod
upd.DeletedAt = old.DeletedAt
upd.DeletedBy = old.DeletedBy
upd = RecordUpdateOwner(invokerID, upd, old)
if err := SetRecordOwner(ctx, svc.ac, svc.store, old, upd, invokerID); err != nil {
return err
}
upd.Values = old.Values.Merge(m.Fields, upd.Values, func(f *types.ModuleField) bool {
return svc.ac.CanUpdateRecordValueOnModuleField(ctx, m.Fields.FindByName(f.Name))
compose/service/record_test.go
+115
View File
@@ -3,6 +3,7 @@ package service
import (
"context"
"fmt"
"github.com/davecgh/go-spew/spew"
"testing"
"github.com/cortezaproject/corteza-server/compose/service/values"
@@ -856,3 +857,117 @@ func TestRecord_contextualRolesAccessControl(t *testing.T) {
hits, _, err = svc.Find(ctx, f)
req.Len(hits, 9)
}
func TestSetRecordOwner(t *testing.T) {
var (
req = require.New(t)
// uncomment to enable sql conn debugging
//ctx = logger.ContextWithValue(context.Background(), logger.MakeDebugLogger())
ctx = context.Background()
s, err = sqlite.ConnectInMemoryWithDebug(ctx)
)
req.NoError(err)
req.NoError(store.Upgrade(ctx, zap.NewNop(), s))
req.NoError(store.TruncateComposeNamespaces(ctx, s))
req.NoError(store.TruncateComposeModules(ctx, s))
req.NoError(store.TruncateComposeRecords(ctx, s, nil))
req.NoError(store.TruncateRbacRules(ctx, s))
var (
rbacService = rbac.NewService(
//zap.NewNop(),
logger.MakeDebugLogger(),
nil,
)
ac = &accessControl{rbac: rbacService}
invoker = &sysTypes.User{ID: 1001}
originalOwner = &sysTypes.User{ID: 2001}
alternativeOwner = &sysTypes.User{ID: 2002}
role = &sysTypes.Role{Name: "role-with-ownership-change-permission", ID: 3000}
old, upd *types.Record
rvse *types.RecordValueErrorSet
)
rbacService.UpdateRoles(
rbac.CommonRole.Make(role.ID, role.Handle),
)
req.NoError(rbacService.Grant(ctx, rbac.AllowRule(role.ID, types.RecordRbacResource(0, 0, 0), "owner.manage")))
spew.Dump(rbacService.Rules())
req.NoError(store.CreateUser(ctx, s, invoker, originalOwner, alternativeOwner))
t.Run("invalid input", func(t *testing.T) {
old, upd = nil, nil
require.Nil(t, SetRecordOwner(ctx, ac, s, old, upd, invoker.ID))
})
t.Run("new record owner is invoker", func(t *testing.T) {
old, upd = nil, &types.Record{ID: 1, NamespaceID: 2, ModuleID: 3}
// this must work, invoker can always set owner to self on a new record
if rvse = SetRecordOwner(ctx, ac, s, old, upd, invoker.ID); !rvse.IsValid() {
t.Fatalf("errors: %v", rvse.Set)
}
req.Equal(upd.OwnedBy, invoker.ID)
})
t.Run("deny setting to alternative owner on create", func(t *testing.T) {
old, upd = nil, &types.Record{ID: 1, NamespaceID: 2, ModuleID: 3, OwnedBy: alternativeOwner.ID}
// this must work, invoker can always set owner to self on a new record
if rvse = SetRecordOwner(ctx, ac, s, old, upd, invoker.ID); rvse.IsValid() {
t.Fatalf("expecting error")
}
})
t.Run("deny setting to alternative owner on update", func(t *testing.T) {
old = &types.Record{ID: 1, NamespaceID: 2, ModuleID: 3, OwnedBy: originalOwner.ID}
upd = &types.Record{ID: 1, NamespaceID: 2, ModuleID: 3, OwnedBy: alternativeOwner.ID}
// this must work, invoker can always set owner to self on a new record
if rvse = SetRecordOwner(ctx, ac, s, old, upd, invoker.ID); rvse.IsValid() {
t.Fatalf("expecting error")
}
})
t.Run("allow setting to alternative owner on create", func(t *testing.T) {
ctx = auth.SetIdentityToContext(ctx, auth.Authenticated(invoker.ID, role.ID))
old, upd = nil, &types.Record{ID: 1, NamespaceID: 2, ModuleID: 3, OwnedBy: alternativeOwner.ID}
if rvse = SetRecordOwner(ctx, ac, s, old, upd, invoker.ID); !rvse.IsValid() {
t.Fatalf("errors: %v", rvse.Set)
}
req.Equal(upd.OwnedBy, alternativeOwner.ID)
})
t.Run("allow setting to alternative owner on update", func(t *testing.T) {
ctx = auth.SetIdentityToContext(ctx, auth.Authenticated(invoker.ID, role.ID))
old = &types.Record{ID: 1, NamespaceID: 2, ModuleID: 3, OwnedBy: originalOwner.ID}
upd = &types.Record{ID: 1, NamespaceID: 2, ModuleID: 3, OwnedBy: alternativeOwner.ID}
if rvse = SetRecordOwner(ctx, ac, s, old, upd, invoker.ID); !rvse.IsValid() {
t.Fatalf("errors: %v", rvse.Set)
}
req.Equal(upd.OwnedBy, alternativeOwner.ID)
})
t.Run("allow setting to new owner to zerp", func(t *testing.T) {
ctx = auth.SetIdentityToContext(ctx, auth.Authenticated(invoker.ID, role.ID))
old = &types.Record{ID: 1, NamespaceID: 2, ModuleID: 3, OwnedBy: originalOwner.ID}
upd = &types.Record{ID: 1, NamespaceID: 2, ModuleID: 3, OwnedBy: 0}
if rvse = SetRecordOwner(ctx, ac, s, old, upd, invoker.ID); !rvse.IsValid() {
t.Fatalf("errors: %v", rvse.Set)
}
req.Equal(upd.OwnedBy, invoker.ID)
})
}
pkg/envoy/store/compose_record_marshal.go
+2 -1
View File
@@ -327,7 +327,8 @@ func (n *composeRecord) Encode(ctx context.Context, pl *payload) (err error) {
rec.OwnedBy = ux[r.Us.OwnedBy.Ref]
}
}
service.RecordUpdateOwner(pl.invokerID, rec, old)
rec.OwnedBy = service.CalcRecordOwner(old.OwnedBy, rec.OwnedBy, pl.invokerID)
rvs := make(composeTypes.RecordValueSet, 0, len(r.Values))
for k, v := range r.Values {