Problem/Motivation
Discovered in #2930028-42: Comprehensive JSON API integration test coverage phase 1: for every entity type, individual resources only.
This is a WTF for developers. Consequently, it also muddles the test coverage that must ensure proper authorization is granted .
This was fine in an initial development phase, but now it seems like this is security through obscurity?
Proposed resolution
Stop requiring the access content permission.
Remaining tasks
Discuss
User interface changes
None.
API changes
None. (Well, no longer requires the access content permission.)
Data model changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| 2940336-10-follow_up.patch | 2.2 KB | wim leers | |
| #2 | 2940336-2.patch | 2.4 KB | wim leers |
Comments
Comment #2
wim leersComment #3
gabesulliceLGTM
Comment #4
wim leersI think this either needs:
I did some
gitarcheology, and it looks like this was introduced by @e0ipso in commita6dd110cin August 2016, without a d.o issue. Because there's a fair amount of placeholder code/TODOs in that commit, I think it's 99% likely that theaccess contentpermission being required was just a "let's get the basics working" thing.I think that qualifies as a very strong indication.
Comment #5
e0ipsoThank you @Wim Leers. That is 100% true. 😛
Comment #6
e0ipsoI wanted to commit, but it seems the patch does not apply after pulling latest changes.
Comment #8
e0ipsoComment #9
wim leersGreat, thanks Mateu! :D