Skip to content

Commit

Permalink
Fix invalid page cursor gen due to RBAC exclusions
Browse files Browse the repository at this point in the history
  • Loading branch information
tjerman committed Feb 5, 2025
1 parent bf5e5b8 commit 691de53
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 22 deletions.
61 changes: 49 additions & 12 deletions server/compose/dalutils/records.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package dalutils
import (
"context"
"fmt"

"github.com/cortezaproject/corteza/server/compose/types"
"github.com/cortezaproject/corteza/server/pkg/dal"
"github.com/cortezaproject/corteza/server/pkg/filter"
Expand Down Expand Up @@ -147,10 +148,10 @@ func drainIterator(ctx context.Context, iter dal.Iterator, mod *types.Module, f
}

var (
ok bool
fetched uint
filtered uint
r *types.Record
ok bool
fetched uint
filtered uint
lastRecord *types.Record
)

// Get the requested number of record
Expand All @@ -161,11 +162,19 @@ func drainIterator(ctx context.Context, iter dal.Iterator, mod *types.Module, f
}

for f.Limit == 0 || uint(len(set)) < f.Limit {
var firstRecord *types.Record

// reset counters every drain
fetched = 0
filtered = 0

add := make(types.RecordSet, 0, 12)
err = WalkIterator(ctx, iter, mod, func(r *types.Record) error {
lastRecord = r
if firstRecord == nil {
firstRecord = r
}

// check fetched record
if f.Check != nil {
if ok, err = f.Check(r); err != nil {
Expand All @@ -177,7 +186,7 @@ func drainIterator(ctx context.Context, iter dal.Iterator, mod *types.Module, f
}

fetched++
set = append(set, r)
add = append(add, r)
return err
})

Expand All @@ -187,8 +196,15 @@ func drainIterator(ctx context.Context, iter dal.Iterator, mod *types.Module, f
return
}

// If it's reverse, we need to add extra fetches to the start
if f.PageCursor != nil && f.PageCursor.ROrder {
set = append(add, set...)
} else {
set = append(set, add...)
}

total := fetched + filtered
if total == 0 || f.Limit == 0 || (0 < f.Limit && total < f.Limit) {
if total == 0 || f.Limit == 0 {
// do not re-fetch if:
// 1) nothing was fetch in the previous run
// 2) there was no limit (everything was fetched)
Expand All @@ -198,11 +214,18 @@ func drainIterator(ctx context.Context, iter dal.Iterator, mod *types.Module, f

// Fetch more records
setLen := uint(len(set))
if setLen > 0 && setLen < f.Limit {
if total > 0 && setLen < f.Limit {
fetchMore := f.Limit - setLen
var crsrRec *types.Record

// request more items
if err = iter.More(fetchMore, r); err != nil {
if f.PageCursor == nil || !f.PageCursor.ROrder {
crsrRec = lastRecord
} else {
crsrRec = firstRecord
}

if err = iter.More(fetchMore, crsrRec); err != nil {
return
}
}
Expand Down Expand Up @@ -295,6 +318,18 @@ func generatePageNavigation(ctx context.Context, iter dal.Iterator, mod *types.M

return
}

recordChecker = func(i dal.Iterator) (ok bool, err error) {
rc := &types.Record{}
err = i.Scan(rc)
rc.SetModule(mod)

if err != nil {
return
}

return p.Check(rc)
}
)

if setLen == 0 {
Expand All @@ -314,13 +349,15 @@ func generatePageNavigation(ctx context.Context, iter dal.Iterator, mod *types.M
// if limit is not defined and set is empty
if p.Limit > 0 && len(set) > 0 {
// PrevPage
out.PrevPage, err = dal.PreLoadCursor(ctx, iter, 1, true, first)
if err != nil {
return
if p.PageCursor != nil {
out.PrevPage, err = dal.PreLoadCursor(ctx, iter, 100, true, first, recordChecker)
if err != nil {
return
}
}

// NextPage
out.NextPage, err = dal.PreLoadCursor(ctx, iter, 1, false, last)
out.NextPage, err = dal.PreLoadCursor(ctx, iter, 100, false, last, recordChecker)
if err != nil {
return
}
Expand Down
35 changes: 26 additions & 9 deletions server/pkg/dal/iterator.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,16 @@ func IteratorEncodeJSON(ctx context.Context, w io.Writer, iter Iterator, initTar

// PreLoadCursor into iterator and check it exist then return the cursor
// @todo this should be move to the Iterator
func PreLoadCursor(ctx context.Context, iter Iterator, limit uint, reverse bool, r ValueGetter) (out *filter.PagingCursor, err error) {
if reverse {
out, err = iter.BackCursor(r)
} else {
out, err = iter.ForwardCursor(r)
func PreLoadCursor(ctx context.Context, iter Iterator, limit uint, reverse bool, r ValueGetter, fx func(Iterator) (bool, error)) (out *filter.PagingCursor, err error) {
makeCursor := func() (*filter.PagingCursor, error) {
if reverse {
return iter.BackCursor(r)
} else {
return iter.ForwardCursor(r)
}
}

out, err = makeCursor()
if err != nil {
return
}
Expand All @@ -88,11 +91,25 @@ func PreLoadCursor(ctx context.Context, iter Iterator, limit uint, reverse bool,
return nil, nil
}

if !iter.Next(ctx) {
out = nil
}
for {
if !iter.Next(ctx) {
out = nil
return
}

return
ok, err := fx(iter)
if err != nil {
return nil, err
}

if ok {
return out, err

// // @todo Skip the things we don't have access to; could cause some edge cases so probably not
// // It adds some performance since we skip unneeded stuff but could some records change in the mean time?
// // return makeCursor()
}
}
}

// IteratorSorting return iterator sorting
Expand Down
8 changes: 7 additions & 1 deletion server/store/adapters/rdbms/dal/iterator.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,13 @@ func (i *iterator) More(max uint, last dal.ValueGetter) (err error) {

i.limit = max
if last != nil {
if i.cursor, err = i.collectCursorValues(last); err != nil {
if i.cursor == nil || !i.cursor.ROrder {
i.cursor, err = i.ForwardCursor(last)
} else {
i.cursor, err = i.BackCursor(last)
}

if err != nil {
return fmt.Errorf("could not collect cursor values: %w", err)
}
}
Expand Down

0 comments on commit 691de53

Please sign in to comment.