Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

find rule with restricted field not working #95

Closed
romaincomtet opened this issue May 27, 2023 · 2 comments · Fixed by #110
Closed

find rule with restricted field not working #95

romaincomtet opened this issue May 27, 2023 · 2 comments · Fixed by #110

Comments

@romaincomtet
Copy link

Bug Explanation and Fix Proposal

Steps to Reproduce
Use the following rule: can('read', 'users', ['id', 'pseudo']);
my authorize hook looks like
const authorizeHook = authorize({
adapter: '@feathersjs/knex',
availableFields: Object.keys(userSchema.properties),
});

Expected Behavior

{
"total": 2,
"limit": 10,
"skip": 0,
"data": [
{
"id": "cefd6229-1e38-4dc4-8f18-0eae5abb8509",
"pseudo": "mattp"
},
{
"id": "490a4045-3c61-4878-a3d0-4984edc24354",
"pseudo": "mattp"
}
]
}

Actual Behavior

{
"total": 2,
"limit": 10,
"skip": 0,
"data": [
{}
]
}

Error Source

The issue arises in the getItemsIsArray function called in authorizeAfter when the options parameter to { from: "result" } and the method is "find". In this scenario, the returned items value is incorrect.

Actual

return in "items" = { "total": 2, "limit": 10, "skip": 0,"data": [[object],[object]]} and is array = false

Expected

expected: should return when method = "find" items = [[object],[object]] and is array = true

Proposed Fix

two potential fixes:

#1
Change options = { from: "result" } to options = { from: "automatic" }.
#2
Add the following line to the getItemsIsArray function:
itemOrItems = itemOrItems && context.method === "find" ? itemOrItems.data || itemOrItems : itemOrItems;

Code Samples
Here are the relevant code snippets for the proposed fixes:

Fix 1:

const authorizeAfter = async (context, options) => {
// ...
let { isArray, items } = feathersUtils.getItemsIsArray(context, { from: "automatic" });
// ...
};
Fix 2:
const getItemsIsArray = (context, options) => {
const {
from = "automatic"
} = options || {};
let itemOrItems;
if (from === "automatic") {
itemOrItems = context.type === "before" ? context.data : context.result;
itemOrItems = itemOrItems && context.method === "find" ? itemOrItems.data || itemOrItems : itemOrItems;
} else if (from === "data") {
itemOrItems = context.data;
} else if (from === "result") {
itemOrItems = context.result;
itemOrItems = itemOrItems && context.method === "find" ? itemOrItems.data || itemOrItems : itemOrItems; // here
}
const isArray = Array.isArray(itemOrItems);
return {
items: isArray ? itemOrItems : itemOrItems != null ? [itemOrItems] : [],
isArray
};
};

Module and Node.js Versions
Module versions: "^2.0.0"
Node.js version: v18.13.0

Servaker added a commit to Servaker/feathers-casl that referenced this issue Nov 13, 2023
In the 'authorizeAfter' function, an incorrect definition of the data
format when calling the 'feathersUtils.getItemsIsArray' function
resulted in the 'isArray' variable being incorrectly set to 'false',
which resulted in the 'getOrFind' variable being incorrectly set to
'get'. As a result, it affected incorrect data filtering when the user
requests data using the 'find' method with pagination enabled and
a restriction on allowed fields.

Fix fratzinger#95
@Servaker
Copy link
Contributor

Hello,

We cannot make corrections to the getItemsIsArray function because it belongs to a third party package feathersUtils.

I made changes to the authorizeAfter function and added a test script.

Please check PR and approve the changes.

fratzinger added a commit that referenced this issue Nov 13, 2023
* Fixes data filtering error when pagination is enabled

In the 'authorizeAfter' function, an incorrect definition of the data
format when calling the 'feathersUtils.getItemsIsArray' function
resulted in the 'isArray' variable being incorrectly set to 'false',
which resulted in the 'getOrFind' variable being incorrectly set to
'get'. As a result, it affected incorrect data filtering when the user
requests data using the 'find' method with pagination enabled and
a restriction on allowed fields.

* Fixes data filtering error when pagination is enabled

In the 'authorizeAfter' function, an incorrect definition of the data
format when calling the 'feathersUtils.getItemsIsArray' function
resulted in the 'isArray' variable being incorrectly set to 'false',
which resulted in the 'getOrFind' variable being incorrectly set to
'get'. As a result, it affected incorrect data filtering when the user
requests data using the 'find' method with pagination enabled and
a restriction on allowed fields.

Fix #95

* fix: getItemsIsArray result/automatic

---------

Co-authored-by: German Shapkov <[email protected]>
@fratzinger
Copy link
Owner

Thanks for the detailed bug report and sorry for the long delay!

Released as v2.1.1 https://github.com/fratzinger/feathers-casl/releases/tag/v2.1.1 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants