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.

CommentFileSizeAuthor
2940336-10-follow_up.patch2.2 KBwim leers
#2 2940336-2.patch2.4 KBwim leers

Comments

Wim Leers created an issue. See original summary.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Active » Needs review
StatusFileSize
new 2940336-2.patch2.4 KB
gabesullice’s picture

gabesullice
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

LGTM

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

I think this either needs:

  • sign-off from @e0ipso or
  • very strong indications that this was not an intentional decision

I did some git archeology, and it looks like this was introduced by @e0ipso in commit a6dd110c in 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 the access content permission being required was just a "let's get the basics working" thing.
I think that qualifies as a very strong indication.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented

I think it's 99% likely that the access content permission being required was just a "let's get the basics working" thing.

Thank you @Wim Leers. That is 100% true. 😛

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented
Issue tags: +Needs reroll

I wanted to commit, but it seems the patch does not apply after pulling latest changes.

  • e0ipso committed d0b577f on 8.x-1.x authored by Wim Leers 
    Issue #2940336 by Wim Leers, e0ipso, gabesullice: JSON API incorrectly...
e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented
Status: Reviewed & tested by the community » Fixed
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Great, thanks Mateu! :D

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.