Bug 20568 implements a way to define client_id/secret API keys for patrons. We should use those instead of the hardcoded ones in koha-conf.xml
Created attachment 74473 [details] [review] Bug 20612: Unit tests This patch makes the oauth.t tests leverage on the new Koha::ApiKey(s) classes. It adds tests for expired tokens too. To test: - Apply this patch - Run: $ kshell k$ prove t/db_dependent/api/v1/acquisitions_orders.t => FAIL: Tests should fail without the rest of the patches. Sponsored-by: ByWater Solutions
Created attachment 74474 [details] [review] Bug 20612: Make OAuth2 use patron's client_id/secret pairs This patch wires the OAuth related code so it leverages on the new Koha::ApiKey(s) classes and tools introduced by bug 20568 instead of the hardcoded entries in koha-conf.xml originally proposed by bug 20402. To test revisit the test plan for bug 20402, and verify that it works. But create API key pairs instead of writing them down in koha-conf.xml. Also: - Run: $ prove t/db_dependent/api/v1/oauth.t => SUCCESS: Tests pass! - Sign off :-D Sponsored-by: ByWater Solutions
Created attachment 74475 [details] [review] Bug 20612: koha-conf.xml cleanup
Created attachment 74483 [details] [review] Bug 20612: Unit tests This patch makes the oauth.t tests leverage on the new Koha::ApiKey(s) classes. It adds tests for expired tokens too. To test: - Apply this patch - Run: $ kshell k$ prove t/db_dependent/api/v1/acquisitions_orders.t => FAIL: Tests should fail without the rest of the patches. Sponsored-by: ByWater Solutions
Created attachment 74484 [details] [review] Bug 20612: Make OAuth2 use patron's client_id/secret pairs This patch wires the OAuth related code so it leverages on the new Koha::ApiKey(s) classes and tools introduced by bug 20568 instead of the hardcoded entries in koha-conf.xml originally proposed by bug 20402. To test revisit the test plan for bug 20402, and verify that it works. But create API key pairs instead of writing them down in koha-conf.xml. Also: - Run: $ prove t/db_dependent/api/v1/oauth.t => SUCCESS: Tests pass! - Sign off :-D Sponsored-by: ByWater Solutions
Created attachment 74485 [details] [review] Bug 20612: koha-conf.xml cleanup
Created attachment 74497 [details] [review] Bug 20612: Unit tests This patch makes the oauth.t tests leverage on the new Koha::ApiKey(s) classes. It adds tests for expired tokens too. To test: - Apply this patch - Run: $ kshell k$ prove t/db_dependent/api/v1/oauth.t => FAIL: Tests should fail without the rest of the patches. Sponsored-by: ByWater Solutions Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 74498 [details] [review] Bug 20612: Make OAuth2 use patron's client_id/secret pairs This patch wires the OAuth related code so it leverages on the new Koha::ApiKey(s) classes and tools introduced by bug 20568 instead of the hardcoded entries in koha-conf.xml originally proposed by bug 20402. To test revisit the test plan for bug 20402, and verify that it works. But create API key pairs instead of writing them down in koha-conf.xml. Also: - Run: $ prove t/db_dependent/api/v1/oauth.t => SUCCESS: Tests pass! - Sign off :-D Sponsored-by: ByWater Solutions Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 74499 [details] [review] Bug 20612: koha-conf.xml cleanup Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 74521 [details] [review] Bug 20612: Unit tests This patch makes the oauth.t tests leverage on the new Koha::ApiKey(s) classes. It adds tests for expired tokens too. To test: - Apply this patch - Run: $ kshell k$ prove t/db_dependent/api/v1/oauth.t => FAIL: Tests should fail without the rest of the patches. Sponsored-by: ByWater Solutions Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>
Created attachment 74522 [details] [review] Bug 20612: Make OAuth2 use patron's client_id/secret pairs This patch wires the OAuth related code so it leverages on the new Koha::ApiKey(s) classes and tools introduced by bug 20568 instead of the hardcoded entries in koha-conf.xml originally proposed by bug 20402. To test revisit the test plan for bug 20402, and verify that it works. But create API key pairs instead of writing them down in koha-conf.xml. Also: - Run: $ prove t/db_dependent/api/v1/oauth.t => SUCCESS: Tests pass! - Sign off :-D Sponsored-by: ByWater Solutions Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>
Created attachment 74523 [details] [review] Bug 20612: koha-conf.xml cleanup Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>
Created attachment 74531 [details] [review] Bug 20612: Unit tests This patch makes the oauth.t tests leverage on the new Koha::ApiKey(s) classes. It adds tests for expired tokens too. To test: - Apply this patch - Run: $ kshell k$ prove t/db_dependent/api/v1/oauth.t => FAIL: Tests should fail without the rest of the patches. Sponsored-by: ByWater Solutions Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 74532 [details] [review] Bug 20612: Unit tests This patch makes the oauth.t tests leverage on the new Koha::ApiKey(s) classes. It adds tests for expired tokens too. To test: - Apply this patch - Run: $ kshell k$ prove t/db_dependent/api/v1/oauth.t => FAIL: Tests should fail without the rest of the patches. Sponsored-by: ByWater Solutions Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 74533 [details] [review] Bug 20612: Make OAuth2 use patron's client_id/secret pairs This patch wires the OAuth related code so it leverages on the new Koha::ApiKey(s) classes and tools introduced by bug 20568 instead of the hardcoded entries in koha-conf.xml originally proposed by bug 20402. To test revisit the test plan for bug 20402, and verify that it works. But create API key pairs instead of writing them down in koha-conf.xml. Also: - Run: $ prove t/db_dependent/api/v1/oauth.t => SUCCESS: Tests pass! - Sign off :-D Sponsored-by: ByWater Solutions Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 74534 [details] [review] Bug 20612: koha-conf.xml cleanup Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 74550 [details] [review] Bug 20612: Unit tests This patch makes the oauth.t tests leverage on the new Koha::ApiKey(s) classes. It adds tests for expired tokens too. To test: - Apply this patch - Run: $ kshell k$ prove t/db_dependent/api/v1/oauth.t => FAIL: Tests should fail without the rest of the patches. Sponsored-by: ByWater Solutions Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 74551 [details] [review] Bug 20612: Make OAuth2 use patron's client_id/secret pairs This patch wires the OAuth related code so it leverages on the new Koha::ApiKey(s) classes and tools introduced by bug 20568 instead of the hardcoded entries in koha-conf.xml originally proposed by bug 20402. To test revisit the test plan for bug 20402, and verify that it works. But create API key pairs instead of writing them down in koha-conf.xml. Also: - Run: $ prove t/db_dependent/api/v1/oauth.t => SUCCESS: Tests pass! - Sign off :-D Sponsored-by: ByWater Solutions Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 74552 [details] [review] Bug 20612: koha-conf.xml cleanup Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Please write a wiki page about these changes.
Could we have a global switch to turn on/off the whole REST API stuffs?
Should we really remove a deleted key from the DB table? I would flag it as inactive instead, it would permit to keep track of the different requests and associate it to a user. Disclaimer: This may be a very stupid idea.
(In reply to Jonathan Druart from comment #22) > Should we really remove a deleted key from the DB table? > I would flag it as inactive instead, it would permit to keep track of the > different requests and associate it to a user. > Disclaimer: This may be a very stupid idea. I thought about it too. That would mean an extra column: deleted TINYINT (1), and it makes sense.
(In reply to Jonathan Druart from comment #21) > Could we have a global switch to turn on/off the whole REST API stuffs? s/REST API/OAuth server
Created attachment 75024 [details] [review] Bug 20612: (QA follow-up) Fix typo in method POD As Martin correctly highlighted, the method name is not correctly spelled in POD. This patch fixes it. To test: - Look carefully => FAIL: Method name is _verify_client_cb but POD says _verify_client_db - Apply this patch - Look carefully => SUCCESS: Notice the POD is fixed! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 75034 [details] [review] Bug 20612: Unit tests This patch makes the oauth.t tests leverage on the new Koha::ApiKey(s) classes. It adds tests for expired tokens too. To test: - Apply this patch - Run: $ kshell k$ prove t/db_dependent/api/v1/oauth.t => FAIL: Tests should fail without the rest of the patches. Sponsored-by: ByWater Solutions Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 75035 [details] [review] Bug 20612: Make OAuth2 use patron's client_id/secret pairs This patch wires the OAuth related code so it leverages on the new Koha::ApiKey(s) classes and tools introduced by bug 20568 instead of the hardcoded entries in koha-conf.xml originally proposed by bug 20402. To test revisit the test plan for bug 20402, and verify that it works. But create API key pairs instead of writing them down in koha-conf.xml. Also: - Run: $ prove t/db_dependent/api/v1/oauth.t => SUCCESS: Tests pass! - Sign off :-D Sponsored-by: ByWater Solutions Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 75036 [details] [review] Bug 20612: koha-conf.xml cleanup Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 75037 [details] [review] Bug 20612: (QA follow-up) Fix typo in method POD As Martin correctly highlighted, the method name is not correctly spelled in POD. This patch fixes it. To test: - Look carefully => FAIL: Method name is _verify_client_cb but POD says _verify_client_db - Apply this patch - Look carefully => SUCCESS: Notice the POD is fixed! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(In reply to Jonathan Druart from comment #20) > Please write a wiki page about these changes. See https://wiki.koha-community.org/wiki/APIs_and_protocols_supported_by_Koha#OAuth2
(In reply to Tomás Cohen Arazi from comment #23) > (In reply to Jonathan Druart from comment #22) > > Should we really remove a deleted key from the DB table? > > I would flag it as inactive instead, it would permit to keep track of the > > different requests and associate it to a user. > > Disclaimer: This may be a very stupid idea. > > I thought about it too. That would mean an extra column: deleted TINYINT > (1), and it makes sense. See bug 20733.
Please take care of bug 20734 before the end of the week.
(In reply to Jonathan Druart from comment #30) > (In reply to Jonathan Druart from comment #20) > > Please write a wiki page about these changes. > > See > https://wiki.koha-community.org/wiki/ > APIs_and_protocols_supported_by_Koha#OAuth2 Done.
(In reply to Jonathan Druart from comment #32) > Please take care of bug 20734 before the end of the week. Done!
Pushed to master for 18.05, thanks to everybody involved!