fixed context passing in role resolver with isOwner
#3211
Conversation
|
Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test." |
|
Can one of the admins verify this patch? |
3 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
@slnode ok to test |
| @@ -218,13 +218,16 @@ module.exports = function(Role) { | |||
| * @param {Boolean} isOwner True if the user is an owner. | |||
| * @promise | |||
| */ | |||
| Role.isOwner = function isOwner(modelClass, modelId, userId, principalType, callback) { | |||
| if (!callback && typeof principalType === 'function') { | |||
| Role.isOwner = function isOwner(modelClass, modelId, userId, principalType, accessToken, callback) { | |||
There was a problem hiding this comment.
You should pass full options object, as it may contain additional information beyond accessToken, for example the current transaction to use when talking to an SQL datastore.
|
I had a quick look at the tests, but unfortunately it was a bit over my head to figure out how to set up a test environment with a specific set of ACL rules and an additional |
I'll see. I'll try to find some time this or next week to provide you with an example showing how to write such test. |
|
There is a test project in #3209 that reproduces the issue
…--
Benjamin Schuster-Böckler
[email protected]
http://twitter.com/evershelf
snap. search. share.
On 22 Feb 2017, at 13:22, Miroslav Bajtoš ***@***.***> wrote:
I had a quick look at the tests, but unfortunately it was a bit over my head to figure out how to set up a test environment with a specific set of ACL rules and an additional access hook
I'll see. I'll try to find some time this or next week to provide you with an example showing how to write such test.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Hello, I have the test ready. I wanted to commit it directly into this pull request, but unfortunately committing into other people's I have opened a new pull request which contains the added test where I also made some small improvements in the contributed fix - see #3230. |
Description
With the move to explicit context passing of an
optionsobject, internal calls tofindetc need to be updated to pass the necessary information along, too. In this case,isOwnercallsfindByIdon a Model, but does not pass an access token inoptions.Related issues
Checklist
guide