[Bug 22227] New: Make GET /cities staff only

classic Classic list List threaded Threaded
18 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bug 22227] New: Make GET /cities staff only

bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22227

            Bug ID: 22227
           Summary: Make GET /cities staff only
 Change sponsored?: ---
           Product: Koha
           Version: master
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P5 - low
         Component: REST api
          Assignee: [hidden email]
          Reporter: [hidden email]
  Target Milestone: ---

Now that we voted to have a /public namespace for unprivileged access
endpoints, the existing /cities endpoint needs to be adapted.

--
You are receiving this mail because:
You are the assignee for the bug.
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22227] Make GET /cities staff only

bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22227

Tomás Cohen Arazi <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |Needs Signoff

--
You are receiving this mail because:
You are the assignee for the bug.
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22227] Make GET /cities staff only

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22227

--- Comment #1 from Tomás Cohen Arazi <[hidden email]> ---
Created attachment 84507
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=84507&action=edit
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

--
You are receiving this mail because:
You are watching all bug changes.
You are the assignee for the bug.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22227] Make GET /cities staff only

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22227

Tomás Cohen Arazi <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |22218
         Depends on|                            |22061


Referenced Bugs:

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22061
[Bug 22061] Public route to change password
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22218
[Bug 22218] Make endpoints not in /public require privileged credentials
[OMNIBUS]
--
You are receiving this mail because:
You are watching all bug changes.
You are the assignee for the bug.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22227] Make GET /cities staff only

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22227

Jon Knight <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[hidden email]

--- Comment #2 from Jon Knight <[hidden email]> ---
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$

--
You are receiving this mail because:
You are the assignee for the bug.
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22227] Make GET /cities staff only

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22227

--- Comment #3 from Tomás Cohen Arazi <[hidden email]> ---
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

--
You are receiving this mail because:
You are the assignee for the bug.
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22227] Make GET /cities staff only

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22227
Bug 22227 depends on bug 22061, which changed state.

Bug 22061 Summary: Public route to change password
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22061

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Pushed to Master            |RESOLVED
         Resolution|---                         |FIXED

--
You are receiving this mail because:
You are watching all bug changes.
You are the assignee for the bug.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22227] Make GET /cities staff only

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22227

Katrin Fischer <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[hidden email]
           Assignee|[hidden email]-commun |[hidden email]
                   |ity.org                     |

--
You are receiving this mail because:
You are watching all bug changes.
You are the assignee for the bug.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22227] Make GET /cities staff only

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22227

Martin Renvoize <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #84507|0                           |1
        is obsolete|                            |

--- Comment #4 from Martin Renvoize <[hidden email]> ---
Created attachment 85060
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85060&action=edit
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 <[hidden email]>

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22227] Make GET /cities staff only

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22227

Martin Renvoize <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Needs Signoff               |Signed Off
                 CC|                            |martin.renvoize@ptfs-europe
                   |                            |.com

--- Comment #5 from Martin Renvoize <[hidden email]> ---
Solid change, passes tests... going straight for PQA on this one.

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22227] Make GET /cities staff only

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22227

Martin Renvoize <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Signed Off                  |Passed QA

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22227] Make GET /cities staff only

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22227

Martin Renvoize <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |release-notes-needed

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22227] Make GET /cities staff only

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22227

--- Comment #6 from Martin Renvoize <[hidden email]> ---
Another api breaking change, can we get a note for the release notes.

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22227] Make GET /cities staff only

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22227

--- Comment #7 from Jon Knight <[hidden email]> ---
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).

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22227] Make GET /cities staff only

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22227

--- Comment #8 from Martin Renvoize <[hidden email]> ---
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

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22227] Make GET /cities staff only

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22227

--- Comment #9 from Martin Renvoize <[hidden email]> ---
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?

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22227] Make GET /cities staff only

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22227

--- Comment #10 from Jon Knight <[hidden email]> ---
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. ;-)

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22227] Make GET /cities staff only

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22227

Nick Clemens <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[hidden email]
             Status|Passed QA                   |Pushed to Master

--- Comment #11 from Nick Clemens <[hidden email]> ---
Awesome work all!

Pushed to master for 19.05

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/