Conversation
cookbook/security/access_control.rst
Outdated
There was a problem hiding this comment.
rule and access_control do have the same semantic meaning in XML (actually access_control is the pluralised form of rule). In fact, nesting access-control and rule was a mistake we made in the documentation and only worked if we had only one rule.
The resulting array after processing the configuration would have been an array with rule as the key. This only worked because the extension just loops the array and doesn't deal with its keys. Though obviously this will break as soon as you have more than one rule.
There was a problem hiding this comment.
Actually, I am not sure if we should really use the rule element at all. Would it be more clear if we simply used multiple access-control elements?
3b08fb1 to
49e589c
Compare
|
@xabbuh first, thank you very much for this huge work! I have a question: do you still consider this PR as WIP? I'm asking because I've reviewed it and it looks ready to be merged. |
|
Yeah, there five or six files from the cookbook section I didn't look at yet. |
49e589c to
04fc6a6
Compare
|
This is ready to be reviewed. |
There was a problem hiding this comment.
In the examples above, we usually define the pattern as ^/api instead of /api/.* Is there any reason for using this other format?
There was a problem hiding this comment.
I don't know the reason for this (maybe just a matter of taste of the initial author of that file). I changed it for consistency.
|
@xabbuh I've reviewed this PR again and I love it. Thank you and congrats on this huge work. |
04fc6a6 to
2159bda
Compare
|
Thank you @xabbuh! 👍 |
2159bda to
34f7859
Compare
34f7859 to
9099cf2
Compare
There was a problem hiding this comment.
while this works, we always use attributes for non-array nodes (making this <acl connection="default"/>
As I promised @weaverryan I now found some time to review all the security-related code blocks. :)