-
Notifications
You must be signed in to change notification settings - Fork 1.2k
IsOwner Guide Update for V4 Fix for #652 #674
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
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/strapijs/documentation/DP1Apd2LsKUm4H3TnojFUJnV1vHV |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of controllers, this type of action should use route policies and route middlewares
68018e1 to
3c365df
Compare
|
Hi @derrickmehaffy, thank you a lot for your feedback. I updated the code using policies and middlewares. If you can give me any feedback on the code or the way its written, I can do another update and rewording. |
Yes I'll try and provide some feedback hopefully later today if I get some time or on monday if my schedule fills up |
|
Hi @nextrapi i try your solution and it's works perfectly, thanks a lot. But i have a little problem, when i try to make post request from my app i have an error: But from postman it works fine. Maybe you can tell me what i'm doing wrong? My axios call: |
Hi @Spasio, thank you but I think your axios config is wrong. Try this: var axios = require('axios');
axios({
method: 'post',
url: 'http://localhost:1337/api/articles',
headers: {
'Authorization': 'Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6MSwiaWF0IjoxNjQzNDM1OTg2LCJleHAiOjE2NDYwMjc5ODZ9.byIvqOTfx-zNjN85v3OCirBAubaKt3YqOZ5mNI0y370',
'Content-Type': 'application/json'
},
data : JSON.stringify({"data":{"title":"Test"}})
})I am however going to correct it so it returns the correct error! |
Thanks for fast reply, your axios config solved my problem. Now everything working great! |
|
This pull request has been mentioned on Strapi Community Forum. There might be relevant details there: https://forum.strapi.io/t/isowner-policy-in-strapi-4-0/14824/5 |
} else {
ctx.request.query.filters = { ...ctx.request.query.filters, [field]: userId };
return true;
}You can't do this, setting stuff in the policy ctx won't travel down the chain. You can't set something in a policy only return true/false. I think this policy could be simplified quite a bit. I also don't understand the purpose of the middleware |
|
Hey everyone, at the moment my code in isOwner.js policy looks like this: As far as I have understood, this works for update, findOne and delete controllers (everything that works on one article). However, what would be the approach if I would want to filter the find controller in a policy? Or am I just able to filter in the controller, e.g. article.js: Filtering the ctx object in the policy file is not possible as @derrickmehaffy mentioned. My approach works I am just not sure if it is the most sensible approach (I doubt that :D) |
|
@Convly and/or @petersg83 I'd like to have your review on the suggested changes, from a technical point of view, before I can have my technical writing review ready :-) |
|
@nextrapi I am very grateful for this documentation PR. |
|
Hello @derrickmehaffy , @petersg83 and @Convly . I'm following up on this PR to have your technical feedback and see how we can go forward with it 😎 |
| }, | ||
| update: { | ||
| policies: [IsOwner], | ||
| middlewares: [SetOwner], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might be wrong, but this SetOwner won't do much since we can only update entities that belong to the current user.
If this is just to demonstrate where you can put the middleware, maybe we could add a comment next to it to explain things?
|
Hello everyone! I'm closing this PR as the recommended way to add an "is-owner policy" equivalent in Strapi v4 is now documented: https://docs.strapi.io/dev-docs/backend-customization/middlewares#restricting-content-access-with-an-is-owner-policy |
What does it do?
Updates the IsOwner guide for Strapi v4
Why is it needed?
The current guide is deprecated.
Important to note that I think this guide is useful and popular enough that it should be added to the sidebar but I haven't done that yet.
Related issue(s)/PR(s)
Fix for #652