Description
Mason James
2014-10-24 17:54:04 UTC
Created attachment 32682 [details] [review] Bug 13143 - Add a tool to show a Koha's password Created attachment 32685 [details] [review] Bug 13143 - Add a tool to show a Koha's password to test... 1/ run without any args, and get some help Displays a Koha instance's password. Usage: /usr/sbin/koha-passwd instancename1 instancename2... Missing instance name... 2/ run with a bad instance, get an error Unknown instance xxxxx 3/ run with a correct instance, get a password foofoo69 4/ run with many instances, get many passwords foofoo69 foofoo68 foofoo67 Created attachment 32686 [details] [review] Bug 13143 - Add a tool to show a Koha's password to test... 1/ run without any args, and get some help # koha-passwd Displays a Koha instance's password. Usage: /usr/sbin/koha-passwd instancename1 instancename2... Missing instance name... 2/ run with a bad instance, get an error # koha-passwd xxxxx Unknown instance xxxxx 3/ run with a correct instance, get a password # koha-passwd koha1 foofoo69 4/ run with many instances, get many passwords # koha-passwd koha1 koha2 koha3 foofoo69 foofoo68 foofoo67 Created attachment 32687 [details] [review] Bug 13143 - Add a tool to show a Koha's password to test... 1/ run without any args, and get some help # koha-passwd Displays a Koha instance's password. Usage: /usr/sbin/koha-passwd instancename1 instancename2... Missing instance name... 2/ run with a bad instance, get an error # koha-passwd xxxxx Unknown instance xxxxx 3/ run with a correct instance, get a password # koha-passwd koha1 foofoo69 4/ run with many instances, get many passwords # koha-passwd koha1 koha2 koha3 foofoo69 foofoo68 foofoo67 This is good and useful, though some suggestions: it needs a manpage. if it's at a terminal (you can detect this, somehow) it should perhaps wait for a keypress and clear the screen unless you provide some command line arg. The second one is just a thought, and isn't required. Hi Mason, The idea of this is great, it'll be a great addition to the suite of scripts for making koha administration easier. One thing I noticed, we now have common functions for debian scripts in the debian/scripts/koha-functions.sh for things like "get_instance" - it would probably be good to use that function instead of duplicating it in your script. Cheers, Liz (In reply to Mason James from comment #4) > Created attachment 32687 [details] [review] [review] > Bug 13143 - Add a tool to show a Koha's password > > to test... > # koha-passwd > # koha-passwd xxxxx > # koha-passwd koha1 > # koha-passwd koha1 koha2 koha3 Try running as non-root user, it will fail. Created attachment 33019 [details] [review] Bug 13143 - Add a tool to show a Koha's password to test... 1/ run without any args, and get some help # koha-passwd Displays a Koha instance's password. Usage: /usr/sbin/koha-passwd instancename1 instancename2... Missing instance name... 2/ run with a bad instance, get an error # koha-passwd xxxxx Unknown instance xxxxx 3/ run with a correct instance, get a password # koha-passwd koha1 foofoo69 4/ run with many instances, get many passwords # koha-passwd koha1 koha2 koha3 foofoo69 foofoo68 foofoo67 Signed-off-by: Mason James <mtj@kohaaloha.com> (In reply to M. Tompsett from comment #7) > (In reply to Mason James from comment #4) > > Created attachment 32687 [details] [review] [review] [review] > > Bug 13143 - Add a tool to show a Koha's password > > > > to test... > > # koha-passwd > > # koha-passwd xxxxx > > # koha-passwd koha1 > > # koha-passwd koha1 koha2 koha3 > > Try running as non-root user, it will fail. all /usr/sbin/koha-* tools fail when executed by a non-root user this is normal behaviour, not a bug (In reply to Mason James from comment #9) > (In reply to M. Tompsett from comment #7) > > (In reply to Mason James from comment #4) > > > Created attachment 32687 [details] [review] [review] [review] [review] > > > Bug 13143 - Add a tool to show a Koha's password > > > > > > to test... > > > # koha-passwd > > > # koha-passwd xxxxx > > > # koha-passwd koha1 > > > # koha-passwd koha1 koha2 koha3 > > > > Try running as non-root user, it will fail. > > > all /usr/sbin/koha-* tools fail when executed by a non-root user > > this is normal behaviour, not a bug oop, sorry - thats incorrect ^ (In reply to M. Tompsett from comment #7) > (In reply to Mason James from comment #4) > > Created attachment 32687 [details] [review] [review] [review] > > Bug 13143 - Add a tool to show a Koha's password > > > > to test... > > # koha-passwd > > # koha-passwd xxxxx > > # koha-passwd koha1 > > # koha-passwd koha1 koha2 koha3 > > Try running as non-root user, it will fail. if you run it as a valid koha user, and ask for your instance's password - it does correctly return your passwd for everything else, it is expected to fail root@xen1:~# koha-shell -l fka fka-koha@xen1:~$ /usr/sbin/koha-passwd fka narfnarf69 (In reply to Liz Rea from comment #6) > Hi Mason, > > The idea of this is great, it'll be a great addition to the suite of scripts > for making koha administration easier. One thing I noticed, we now have > common functions for debian scripts in the debian/scripts/koha-functions.sh > for things like "get_instance" - it would probably be good to use that > function instead of duplicating it in your script. > > Cheers, > Liz thanks, i attached a new patch that fixes this problem Created attachment 33020 [details] [review] Bug 13143 - follow up - usability improvements * makes sure output is not on the line with the prompt * adds ability to clear screen after password display Same test plan as for original bug. Created attachment 33962 [details] [review] Bug 13143: Followup - Correct the pause message 'read anykey' actually requires enter under sh. This patch changes the string from: 'Press any key to clear the screen...' to 'Press enter to clear the screen...' TEST PLAN --------- 1) Apply the first two patches 2) Trigger the script. 2) Press 'q' or anything other than enter when prompted. --- You could type a whole sentence! Oops! 3) Press enter. 4) Apply this patch 5) Trigger the script. 6) Be happy that you aren't told any key will work. 7) run koha qa tools Created attachment 33963 [details] [review] Bug 13143 - Add a tool to show a Koha's password to test... 1/ run without any args, and get some help # koha-passwd Displays a Koha instance's password. Usage: /usr/sbin/koha-passwd instancename1 instancename2... Missing instance name... 2/ run with a bad instance, get an error # koha-passwd xxxxx Unknown instance xxxxx 3/ run with a correct instance, get a password # koha-passwd koha1 foofoo69 4/ run with many instances, get many passwords # koha-passwd koha1 koha2 koha3 foofoo69 foofoo68 foofoo67 Signed-off-by: Mason James <mtj@kohaaloha.com> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Created attachment 33964 [details] [review] Bug 13143 - follow up - usability improvements * makes sure output is not on the line with the prompt * adds ability to clear screen after password display Same test plan as for original bug. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Created attachment 36435 [details] [review] Bug 13143: Followup - Correct the pause message 'read anykey' actually requires enter under sh. This patch changes the string from: 'Press any key to clear the screen...' to 'Press enter to clear the screen...' TEST PLAN --------- 1) Apply the first two patches 2) Trigger the script. 2) Press 'q' or anything other than enter when prompted. --- You could type a whole sentence! Oops! 3) Press enter. 4) Apply this patch 5) Trigger the script. 6) Be happy that you aren't told any key will work. 7) run koha qa tools Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net> http://bugs.koha-community.org/show_bug.cgi?id=13141 My issue is that the pausing thing means it can't be used within a script. It shouldn't say "password for x is y", it should just output the password, and it should only block if it's outputting to a terminal, otherwise it'll block forever. A test for this is here: http://stackoverflow.com/questions/911168/how-to-detect-if-my-shell-script-is-running-through-a-pipe running koha-passwd instance | cat should output the password and then finish without issuing a clear or anything. This makes it easy for people who need the password as part of some other shell script. Additionally, it'd probably be good (although I wouldn't consider it a blocker) to make a function in koha-functions.sh to fetch the password and use that, that way other scripts in the koha-* set can use that (if you're really keen you can change them to use it, but certainly don't consider than necessary.) Created attachment 40817 [details] [review] Bug 13143 - Detect if the password is in a pipe 1. cd kohaclone/debian/scripts 2. sudo koha-passwd instance | cat Output should be password only 3. sudo koha-passwd instance Output should be "Password for $instance is: $password" Does this get at what you were wanting, Robin? Note that you'll need the whole set of patches to test this. :) Created attachment 46924 [details] [review] Bug 13143 - Add a tool to show a Koha's password to test... 1/ run without any args, and get some help # koha-passwd Displays a Koha instance's password. Usage: /usr/sbin/koha-passwd instancename1 instancename2... Missing instance name... 2/ run with a bad instance, get an error # koha-passwd xxxxx Unknown instance xxxxx 3/ run with a correct instance, get a password # koha-passwd koha1 foofoo69 4/ run with many instances, get many passwords # koha-passwd koha1 koha2 koha3 foofoo69 foofoo68 foofoo67 Signed-off-by: Mason James <mtj@kohaaloha.com> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Created attachment 46925 [details] [review] Bug 13143 - follow up - usability improvements * makes sure output is not on the line with the prompt * adds ability to clear screen after password display Same test plan as for original bug. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Created attachment 46926 [details] [review] Bug 13143: Followup - Correct the pause message 'read anykey' actually requires enter under sh. This patch changes the string from: 'Press any key to clear the screen...' to 'Press enter to clear the screen...' TEST PLAN --------- 1) Apply the first two patches 2) Trigger the script. 2) Press 'q' or anything other than enter when prompted. --- You could type a whole sentence! Oops! 3) Press enter. 4) Apply this patch 5) Trigger the script. 6) Be happy that you aren't told any key will work. 7) run koha qa tools Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net> http://bugs.koha-community.org/show_bug.cgi?id=13141 Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Created attachment 46927 [details] [review] Bug 13143 - Detect if the password is in a pipe 1. cd kohaclone/debian/scripts 2. sudo koha-passwd instance | cat Output should be password only 3. sudo koha-passwd instance Output should be "Password for $instance is: $password" Does this get at what you were wanting, Robin? NOTE: $ cd ~ $ sudo mkdir /etc/koha $ sudo mkdir /etc/koha/sites $ sudo mkdir /etc/koha/sites/library $ sudo ln -s ~/koha-dev/etc/koha-conf.xml /etc/koha/sites/library/koha-conf.xml $ sudo mkdir /usr/share/koha $ sudo ln -s ~/kohaclone/debian/scripts /usr/share/koha/bin This will fake out your system enough to get koha-passwd to run in your git system. Since you are using your git system, no need for the sudo's in steps 2 & 3. And no need to roll a custom build to test it. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Also corrected a rebase issue caused by the release of plack while I was at it. Created attachment 46985 [details] [review] Bug 13143 - Add a tool to show a Koha's password to test... 1/ run without any args, and get some help # koha-passwd Displays a Koha instance's password. Usage: /usr/sbin/koha-passwd instancename1 instancename2... Missing instance name... 2/ run with a bad instance, get an error # koha-passwd xxxxx Unknown instance xxxxx 3/ run with a correct instance, get a password # koha-passwd koha1 foofoo69 4/ run with many instances, get many passwords # koha-passwd koha1 koha2 koha3 foofoo69 foofoo68 foofoo67 Signed-off-by: Mason James <mtj@kohaaloha.com> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 46986 [details] [review] Bug 13143 - follow up - usability improvements * makes sure output is not on the line with the prompt * adds ability to clear screen after password display Same test plan as for original bug. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 46987 [details] [review] Bug 13143: Followup - Correct the pause message 'read anykey' actually requires enter under sh. This patch changes the string from: 'Press any key to clear the screen...' to 'Press enter to clear the screen...' TEST PLAN --------- 1) Apply the first two patches 2) Trigger the script. 2) Press 'q' or anything other than enter when prompted. --- You could type a whole sentence! Oops! 3) Press enter. 4) Apply this patch 5) Trigger the script. 6) Be happy that you aren't told any key will work. 7) run koha qa tools Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net> http://bugs.koha-community.org/show_bug.cgi?id=13141 Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 46988 [details] [review] Bug 13143 - Detect if the password is in a pipe 1. cd kohaclone/debian/scripts 2. sudo koha-passwd instance | cat Output should be password only 3. sudo koha-passwd instance Output should be "Password for $instance is: $password" Does this get at what you were wanting, Robin? NOTE: $ cd ~ $ sudo mkdir /etc/koha $ sudo mkdir /etc/koha/sites $ sudo mkdir /etc/koha/sites/library $ sudo ln -s ~/koha-dev/etc/koha-conf.xml /etc/koha/sites/library/koha-conf.xml $ sudo mkdir /usr/share/koha $ sudo ln -s ~/kohaclone/debian/scripts /usr/share/koha/bin This will fake out your system enough to get koha-passwd to run in your git system. Since you are using your git system, no need for the sudo's in steps 2 & 3. And no need to roll a custom build to test it. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Pushed to Master - Should be in the May 2016 release. Thanks! |