| Summary: | Make GET /cities staff only | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Tomás Cohen Arazi (tcohen) <tomascohen> |
| Component: | REST API | Assignee: | Tomás Cohen Arazi (tcohen) <tomascohen> |
| Status: | CLOSED FIXED | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | P5 - low | CC: | J.P.Knight, martin.renvoize, nick |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
| Bug Depends on: | 22061 | ||
| Bug Blocks: | 22218 | ||
| Attachments: |
Bug 22227: Make GET /cities staff only
Bug 22227: Make GET /cities staff only |
||
|
Description
Tomás Cohen Arazi (tcohen)
2019-01-29 14:41:05 UTC
Created attachment 84507 [details] [review] Bug 22227: Make GET /cities staff only This patch removes the possibility to access the city objects without privileged access (minimum permissions == catalogue). It does so by adding the required permissions to the spec. The tests are adjusted. To test: - Apply this patch - Run: $ kshell k$ prove t/db_dependent/api/v1/cities.t => SUCCESS: Tests pass! - Sign off :-D Do we need to do anything other than what is in the test plan on a kohadevbox? For example have some particular data loaded in our database? I've just tried to apply the patch with the following result: vagrant@kohadevbox:kohaclone(master)$ git checkout -b bug22227 Switched to a new branch 'bug22227' vagrant@kohadevbox:kohaclone(bug22227)$ git bz apply 22227 Bug 22227 - Make GET /cities staff only 84507 - Bug 22227: Make GET /cities staff only Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 22227: Make GET /cities staff only vagrant@kohadevbox:kohaclone(bug22227)$ kshell kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove t/db_dependent/api/v1/cities.t t/db_dependent/api/v1/cities.t .. # Failed test '200 OK' # at t/db_dependent/api/v1/cities.t line 87. # got: '400' # expected: '200' # Failed test 'exact match for JSON Pointer ""' # at t/db_dependent/api/v1/cities.t line 87. # Structures begin differing at: # $got->[0]{state} = Does not exist # $expected->[0]{state} = 'O5v1BdpdGC68BA87LFM5W_sSkKGJLmnkPL4rz_34K8hlXzvLUtZsoeuJ8x6tv8zKdJ' # Failed test '200 OK' # at t/db_dependent/api/v1/cities.t line 96. # got: '400' # expected: '200' # Failed test 'exact match for JSON Pointer ""' # at t/db_dependent/api/v1/cities.t line 96. # Structures begin differing at: # $got->[0]{city_id} = Does not exist # $expected->[0]{city_id} = '12' # Looks like you failed 4 tests of 18. t/db_dependent/api/v1/cities.t .. 1/5 # Failed test 'list() tests' # at t/db_dependent/api/v1/cities.t line 109. # Failed test '403 Forbidden' # at t/db_dependent/api/v1/cities.t line 162. # got: '200' # expected: '403' # Looks like you failed 1 test of 17. # Failed test 'add() tests' # at t/db_dependent/api/v1/cities.t line 226. # Failed test '403 Forbidden' # at t/db_dependent/api/v1/cities.t line 246. # got: '400' # expected: '403' # Looks like you failed 1 test of 15. # Failed test 'update() tests' # at t/db_dependent/api/v1/cities.t line 324. # Failed test '403 Forbidden' # at t/db_dependent/api/v1/cities.t line 343. # got: '200' # expected: '403' # Failed test '200 OK' # at t/db_dependent/api/v1/cities.t line 349. # got: '404' # expected: '200' # Failed test 'exact match for content' # at t/db_dependent/api/v1/cities.t line 349. # got: '{"error":"Object not found"}' # expected: '""' # Looks like you failed 3 tests of 7. # Failed test 'delete() tests' # at t/db_dependent/api/v1/cities.t line 360. # Looks like you failed 4 tests of 5. t/db_dependent/api/v1/cities.t .. Dubious, test returned 4 (wstat 1024, 0x400) Failed 4/5 subtests Test Summary Report ------------------- t/db_dependent/api/v1/cities.t (Wstat: 1024 Tests: 5 Failed: 4) Failed tests: 1, 3-5 Non-zero exit status: 4 Files=1, Tests=5, 3 wallclock secs ( 0.02 usr 0.00 sys + 2.37 cusr 0.24 csys = 2.63 CPU) Result: FAIL kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ Can you retry?: $ git fetch $ git checkout origin/master -b bug22227 $ kshell k$ prove t/db_dependent/api/v1/cities.pl In my case: $ kshell k$ prove t/db_dependent/api/v1/cities.t t/db_dependent/api/v1/cities.t .. ok All tests successful. Files=1, Tests=5, 4 wallclock secs ( 0.03 usr 0.01 sys + 3.21 cusr 0.37 csys = 3.62 CPU) Result: PASS Created attachment 85060 [details] [review] Bug 22227: Make GET /cities staff only This patch removes the possibility to access the city objects without privileged access (minimum permissions == catalogue). It does so by adding the required permissions to the spec. The tests are adjusted. To test: - Apply this patch - Run: $ kshell k$ prove t/db_dependent/api/v1/cities.t => SUCCESS: Tests pass! - Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Solid change, passes tests... going straight for PQA on this one. Another api breaking change, can we get a note for the release notes. Weird - it still fails on my kohadevbox: vagrant@kohadevbox:kohaclone((66f3a72...))$ git fetch vagrant@kohadevbox:kohaclone((66f3a72...))$ git checkout origin/master -b bug22227 Branch bug22227 set up to track remote branch master from origin. Switched to a new branch 'bug22227' vagrant@kohadevbox:kohaclone(bug22227)$ git bz apply 22227 Bug 22227 - Make GET /cities staff only 85060 - Bug 22227: Make GET /cities staff only Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 22227: Make GET /cities staff only vagrant@kohadevbox:kohaclone(bug22227)$ kshell kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove t/db_dependent/api/v1/cities.t t/db_dependent/api/v1/cities.t .. # Failed test '200 OK' # at t/db_dependent/api/v1/cities.t line 87. # got: '400' # expected: '200' # Failed test 'exact match for JSON Pointer ""' # at t/db_dependent/api/v1/cities.t line 87. # Structures begin differing at: # $got->[0]{state} = Does not exist # $expected->[0]{state} = 'VsCDNM9zaCm6qJBgAu2IdTVEi5Vxnw7unyoPbnQ1hgf8Yyfx02jJPwsjX6wzcG2zFN0o1OkAFt7zq_cGwi9piH' # Failed test '200 OK' # at t/db_dependent/api/v1/cities.t line 96. # got: '400' # expected: '200' # Failed test 'exact match for JSON Pointer ""' # at t/db_dependent/api/v1/cities.t line 96. # Structures begin differing at: # $got->[0]{postal_code} = Does not exist # $expected->[0]{postal_code} = 'JAmMlEZoTch' # Looks like you failed 4 tests of 18. t/db_dependent/api/v1/cities.t .. 1/5 # Failed test 'list() tests' # at t/db_dependent/api/v1/cities.t line 109. # Failed test '403 Forbidden' # at t/db_dependent/api/v1/cities.t line 162. # got: '200' # expected: '403' # Looks like you failed 1 test of 17. # Failed test 'add() tests' # at t/db_dependent/api/v1/cities.t line 226. # Failed test '403 Forbidden' # at t/db_dependent/api/v1/cities.t line 246. # got: '400' # expected: '403' # Looks like you failed 1 test of 15. # Failed test 'update() tests' # at t/db_dependent/api/v1/cities.t line 324. # Failed test '403 Forbidden' # at t/db_dependent/api/v1/cities.t line 343. # got: '200' # expected: '403' # Failed test '200 OK' # at t/db_dependent/api/v1/cities.t line 349. # got: '404' # expected: '200' # Failed test 'exact match for content' # at t/db_dependent/api/v1/cities.t line 349. # got: '{"error":"Object not found"}' # expected: '""' # Looks like you failed 3 tests of 7. # Failed test 'delete() tests' # at t/db_dependent/api/v1/cities.t line 360. # Looks like you failed 4 tests of 5. t/db_dependent/api/v1/cities.t .. Dubious, test returned 4 (wstat 1024, 0x400) Failed 4/5 subtests Test Summary Report ------------------- t/db_dependent/api/v1/cities.t (Wstat: 1024 Tests: 5 Failed: 4) Failed tests: 1, 3-5 Non-zero exit status: 4 Files=1, Tests=5, 3 wallclock secs ( 0.02 usr 0.01 sys + 2.36 cusr 0.17 csys = 2.56 CPU) Result: FAIL kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ But if it works on the Martin and Tomas's machines, I think I'll have to put it down to something weird in my database it doesn't like (probably from testing other patches). How odd.. I'll give it another test run through koha-testing-docker just in case I missed something. Thanks for pointing it out Jon Yup, passes perfectly on a re-check in koha-testing-docker.. I wonder if it is data related, though the test looks like it shouldn't rely on (nor be affected by) any existing data. Clutching at straws have you recently run the dbi updates and dbic schema update script? Perhaps there's a missing dbic schema class file change crept into the code somewhere? Yes, I ran dbic update at the start of the week (when working on another bug) to make sure my database schema was up to date. I will have different data in the database to a "clean" kohadevbox though as I've used it for testing over time. Ho hum, don't worry about it if it works for both of you. ;-) Awesome work all! Pushed to master for 19.05 Change of behaviour, won't be backported to 18.11.x series |