I'm changing the bug description, because I didn't manage to have tests for the on/off switch without a real implemented route.
Created attachment 83613 [details] [review] Bug 22061: Add RESTPublicAPI syspref
Created attachment 83614 [details] [review] Bug 22061: Add a /public namespace that can be switched on/off This patch adds a check in Koha::REST::V1::Auth::under to catch all routes that begin with 'public' (inside /api/v1). If they match, and the RESTPublicAPI syspref is off, then an exception is thrown, rendering a 403 error to the consumer. Otherwise the routes are processed as usual. This is THE on/off switch for the public REST API. The target use case: people not wanting an OPAC or public interaction with the API besides privileged users. In order to test, the rest of the patches are needed because the only way to test a route is having it in the spec. To test: - Apply the patches - Run: $ kshell k$ prove t/db_dependent/api/v1/auth.t => SUCCESS: tests pass! - Sign off :-D
Leaving this as ASSIGNED until I solve some bugs found in the process.
Created attachment 83680 [details] [review] Bug 22061: Unit tests
Created attachment 83681 [details] [review] Bug 22061: OpenAPI spec
Created attachment 83682 [details] [review] Bug 22061: Public route to change password This patch implements a route to change patron's password for unprivileged users. It's intended for using in the OPAC (or however consumers find it useful). It is a starting point for the public API, and the bug also implements the on/off switch some community members asked for. To test: - Apply this patches - Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_password.t \ t/db_dependent/api/v1/auth.t => SUCCESS: Tests pass! i.e. - RESTPublicAPI is honoured - The /public/patrons/:patron_id/password endpoint works as expected - Use your favourite API testing tool to try the endpoint. - Sign off :-D
t/db_dependent/api/v1/patrons_password.t .. Could not load document from /home/vagrant/kohaclone/api/v1/swagger/paths/public_patrons.json: Can't open file "/home/vagrant/kohaclone/api/v1/swagger/paths/public_patrons.json": No such file or directory at /usr/share/perl5/JSON/Validator.pm line 154.
Created attachment 83755 [details] [review] Bug 22061: OpenAPI spec
Created attachment 83756 [details] [review] Bug 22061: Public route to change password This patch implements a route to change patron's password for unprivileged users. It's intended for using in the OPAC (or however consumers find it useful). It is a starting point for the public API, and the bug also implements the on/off switch some community members asked for. To test: - Apply this patches - Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_password.t \ t/db_dependent/api/v1/auth.t => SUCCESS: Tests pass! i.e. - RESTPublicAPI is honoured - The /public/patrons/:patron_id/password endpoint works as expected - Use your favourite API testing tool to try the endpoint. - Sign off :-D
Created attachment 83764 [details] [review] Bug 22061: Add RESTPublicAPI syspref Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 83765 [details] [review] Bug 22061: Add a /public namespace that can be switched on/off This patch adds a check in Koha::REST::V1::Auth::under to catch all routes that begin with 'public' (inside /api/v1). If they match, and the RESTPublicAPI syspref is off, then an exception is thrown, rendering a 403 error to the consumer. Otherwise the routes are processed as usual. This is THE on/off switch for the public REST API. The target use case: people not wanting an OPAC or public interaction with the API besides privileged users. In order to test, the rest of the patches are needed because the only way to test a route is having it in the spec. To test: - Apply the patches - Run: $ kshell k$ prove t/db_dependent/api/v1/auth.t => SUCCESS: tests pass! - Sign off :-D Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 83766 [details] [review] Bug 22061: Unit tests Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 83767 [details] [review] Bug 22061: OpenAPI spec Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 83768 [details] [review] Bug 22061: Public route to change password This patch implements a route to change patron's password for unprivileged users. It's intended for using in the OPAC (or however consumers find it useful). It is a starting point for the public API, and the bug also implements the on/off switch some community members asked for. To test: - Apply this patches - Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_password.t \ t/db_dependent/api/v1/auth.t => SUCCESS: Tests pass! i.e. - RESTPublicAPI is honoured - The /public/patrons/:patron_id/password endpoint works as expected - Use your favourite API testing tool to try the endpoint. - Sign off :-D Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
On the dev meeting it was requested that we change the 'password2' parameter into something that explains that it is a repeated password more explicitly. I'll do it on a follow-up, but wanted to mention that the inspiration for this implementation was Django's: https://django-rest-auth.readthedocs.io/en/latest/api_endpoints.html and the fact that most of the examples on the web are quite similar too.
Created attachment 84207 [details] [review] Bug 22061: (QA follow-up) Rename password_2 => password_repeated As voted when the RFC was discussed, the attribute gets renamed. The tests are adjusted accordingly. To test: - Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_password.t => SUCCESS: Tests pass! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 84288 [details] [review] Bug 22061: Add RESTPublicAPI syspref Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 84289 [details] [review] Bug 22061: Add RESTPublicAPI syspref Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 84290 [details] [review] Bug 22061: Add a /public namespace that can be switched on/off This patch adds a check in Koha::REST::V1::Auth::under to catch all routes that begin with 'public' (inside /api/v1). If they match, and the RESTPublicAPI syspref is off, then an exception is thrown, rendering a 403 error to the consumer. Otherwise the routes are processed as usual. This is THE on/off switch for the public REST API. The target use case: people not wanting an OPAC or public interaction with the API besides privileged users. In order to test, the rest of the patches are needed because the only way to test a route is having it in the spec. To test: - Apply the patches - Run: $ kshell k$ prove t/db_dependent/api/v1/auth.t => SUCCESS: tests pass! - Sign off :-D Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 84291 [details] [review] Bug 22061: Unit tests Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 84292 [details] [review] Bug 22061: OpenAPI spec Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 84293 [details] [review] Bug 22061: Public route to change password This patch implements a route to change patron's password for unprivileged users. It's intended for using in the OPAC (or however consumers find it useful). It is a starting point for the public API, and the bug also implements the on/off switch some community members asked for. To test: - Apply this patches - Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_password.t \ t/db_dependent/api/v1/auth.t => SUCCESS: Tests pass! i.e. - RESTPublicAPI is honoured - The /public/patrons/:patron_id/password endpoint works as expected - Use your favourite API testing tool to try the endpoint. - Sign off :-D Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 84294 [details] [review] Bug 22061: (QA follow-up) Rename password_2 => password_repeated As voted when the RFC was discussed, the attribute gets renamed. The tests are adjusted accordingly. To test: - Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_password.t => SUCCESS: Tests pass! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 84489 [details] [review] Bug 22061: (follow-up) set_password expects a hashref Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Awesome work all! Pushed to master for 19.05
New feature, will not be backported at this time to 18.11.x series.