Bug 37025

Summary: Add CLI tool to generate/delete api keys for a given patron on a Koha instance
Product: Koha Reporter: Paul Derscheid <paul.derscheid>
Component: Command-line UtilitiesAssignee: Paul Derscheid <paul.derscheid>
Status: Needs Signoff --- QA Contact: Testopia <testopia>
Severity: new feature    
Priority: P5 - low CC: blawlor, dcook, jake.deery, robin
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Bug 37025: Add CLI tool to generate/delete api keys for a given patron on a Koha instance
Bug 37025: (follow-up) Add missing docbook xml for koha-api-keys
Bug 37025: (follow-up) Wrap calls to koha modules in koha-shell

Description Paul Derscheid 2024-06-04 14:19:25 UTC
I bet this will not immediately go through QA. Please don't hesitate to nitpick. :)

Just as a disclaimer, I didn't want any flags, just appropriate positional arguments for the two contexts. That's why it might look unconventional.
Comment 1 Paul Derscheid 2024-06-04 14:33:54 UTC
Created attachment 167397 [details] [review]
Bug 37025: Add CLI tool to generate/delete api keys for a given patron on a Koha instance

To test:
1) Apply the patch.
2) Suggestion: `cp debian/scripts/koha-api-keys /usr/sbin/`.
3) Familiarize yourself with the tool by just running `koha-api-keys`.
4) Test that you can generate an api key with the generate op.
5) Test that you can delete an api key with the delete op.
6) Run `xsltproc /usr/share/xml/docbook/stylesheet/docbook-xsl-ns/manpages/docbook.xsl debian/docs/koha-api-keys.xml`.
7) Use man to verify that the manpage renders as expected (install man if not present, e.g. koha-testing-docker).
8) Run the syntax test for the docbook files `prove -v xt/verify-debian-docbook.t`.
9) Sign off.

Or leave a comment on what you thinks needs to be improved.
Comment 2 Brendan Lawlor 2024-06-05 20:19:17 UTC
This is cool.

I was able to create and delete API keys for a user.

At step #6 I tried running this command
`xsltproc /usr/share/xml/docbook/stylesheet/docbook-xsl-ns/manpages/docbook.xsl debian/docs/koha-api-keys.xml`

I got this error:
warning: failed to load external entity "debian/docs/koha-api-keys.xml"
unable to parse debian/docs/koha-api-keys.xml

I see xml files in debian/docs/ but none for koha-api-keys

I think it might be nice if the delete command would optionally take the client id as the first argument instead of prompting for it after.

Then the usage for delete could be one command with three arguments similar to create 
koha-api-keys delete [ client_id ] [ borrowernumber ] [ instancename ]
Comment 3 Paul Derscheid 2024-06-06 07:25:36 UTC
Hi Brendan, thanks for testing this.
By mistake I thought I had included that docbook file because I saw `koha-api-keys` in the diff. It is missing, though. You're absolutely correct.

The delete op doesn't need a borrowernumber, that's why I read the client id from input. In general many scripts are split up into enable|disable pairs to keep their interfaces simple. Might be also an idea for this one.

I mainly want the generation part because that I really need.
Comment 4 Paul Derscheid 2024-06-06 07:26:36 UTC
Created attachment 167524 [details] [review]
Bug 37025: (follow-up) Add missing docbook xml for koha-api-keys
Comment 5 Paul Derscheid 2024-06-06 09:29:59 UTC
BTW Brendan, reading from input also implies that you can just read the client id from STDIN like this:

`echo <client_id> | koha-api-keys delete <instance>`

So there's not really a need for another argument.
Comment 6 Paul Derscheid 2024-06-10 14:29:03 UTC
Here's another form I just learned about, just because I haven't seen it until a couple minutes ago:

`koha-api-keys delete kohadev <<< <client_id>`
Comment 7 Brendan Lawlor 2024-06-10 20:39:43 UTC
I tested this again, but I got stuck on verifying the manpage this time.

I'm using ktd, installed man with `sudo apt-get install man`

Step 6 seemed to work now with the xml file that you added.

`xsltproc /usr/share/xml/docbook/stylesheet/docbook-xsl-ns/manpages/docbook.xsl debian/docs/koha-api-keys.xml`
Note: Writing koha-api-keys.8

I tried the command `man koha-api-keys` but got
No manual entry for koha-api-keys

Step 8 tests passed
`prove -v xt/verify-debian-docbook.t`
All tests successful.
Comment 8 David Cook 2024-06-11 04:25:06 UTC
I'm intrigued. What kind of use cases would you have for this?
Comment 9 Paul Derscheid 2024-06-11 06:25:50 UTC
Mainly for remote testing. I have an additional API that requires client credentials for bearer token generation and if I want to quickly reproduce a bug, I always need to generate a pair via the UI.

That's why I wanted a solution which I can use to generate my client credentials, run my tests and invalidate the bearer token as well as the credentials all in one pipe.
Comment 10 Jake Deery 2024-09-04 08:55:16 UTC
Afraid I can't even get past the first hurdle:

kohadev-koha@kohadevbox:koha(bug_37025_so)$ sudo koha-api-keys generate default 51 kohadev
Unknown borrowernumber: '51'

Don't know if I'm being a complete fool, do feel free to correct me if that is so!

Jake.
Comment 11 Paul Derscheid 2024-09-04 08:56:41 UTC
Wow, that's weird!
Comment 12 Paul Derscheid 2024-09-04 09:03:06 UTC
Ah wait, don't use sudo :D
Comment 13 Jake Deery 2024-09-04 09:07:09 UTC
Ah, I see. In production, it would have to be run with sudo for proper access to sbin, so I don't think we can sign this off yet?

Jake.
Comment 14 Paul Derscheid 2024-09-04 09:09:16 UTC
Ah, you're totally right!

Also I think this should be wrapped in koha-shell when giving it a second look.
Comment 15 Paul Derscheid 2024-09-04 09:43:29 UTC
Created attachment 171014 [details] [review]
Bug 37025: (follow-up) Wrap calls to koha modules in koha-shell

This change should
- allow seamless usage when run as root (implied by /usr/sbin).
- give explicit control to the user instead of relying on the environment
Comment 16 Paul Derscheid 2024-09-04 09:44:01 UTC
Quoting on the shell is so annoying -_-
Comment 17 David Cook 2024-09-05 02:52:06 UTC
(In reply to Paul Derscheid from comment #16)
> Quoting on the shell is so annoying -_-

1000%
Comment 18 David Cook 2024-09-05 02:55:15 UTC
(In reply to Paul Derscheid from comment #16)
> Quoting on the shell is so annoying -_-

My hot tip is don't quote on the shell. 

Even though it's just a tiny bit of code, you're better off putting it in its own script file and calling that via koha-shell. 

I've had this same nightmare heaps of times with local devs, and it just wasn't worth trying to escape every scenario. 

I seem to recall koha-shell does some truly awful things with quoting...