Bug 20489

Summary: Prevent DB user login
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: AuthenticationAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: enhancement    
Priority: P5 - low CC: chris, dpavlin, katrin.fischer, kyle, m.de.rooy, martin.renvoize, mtompset, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18538
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9164
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17218
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
It is no longer possible to use the database user (defined in koha-conf.xml) to login into Koha. You should first create a superlibrarian patron and use it for logging in. See the script misc/devel/create_superlibrarian.pl
Version(s) released in:
Bug Depends on: 20580    
Bug Blocks: 20877, 21746, 24018, 36019    
Attachments: Bug 20489: Prevent DB user login
Bug 20489: Remove warnings from the interface
Bug 20489: Remove warnings from the interface
Bug 20489: Prevent DB user login
Bug 20489: Remove warnings from the interface
Bug 20489: Prevent DB user login
Bug 20489: Remove warnings from the interface
Bug 20489: Prevent DB user login
Bug 20489: Remove warnings from the interface

Description Jonathan Druart 2018-03-28 17:29:53 UTC
Since we have the onboarding tool that create a superlibrarian patron as well as a misc script (misc/devel/create_superlibrarian.pl), we could reject login with the DB user.
Comment 1 Jonathan Druart 2018-03-28 17:44:48 UTC
Created attachment 73400 [details] [review]
Bug 20489: Prevent DB user login
Comment 2 Jonathan Druart 2018-03-28 17:45:46 UTC
Something like that, would you agree guys?
Comment 3 Jonathan Druart 2018-03-28 17:49:36 UTC
Created attachment 73402 [details] [review]
Bug 20489: Remove warnings from the interface
Comment 4 Marcel de Rooy 2018-03-29 07:36:21 UTC
(In reply to Jonathan Druart from comment #2)
> Something like that, would you agree guys?

Sounds good. Second patch contains other stuff too.
Comment 5 Kyle M Hall 2018-03-29 11:04:11 UTC
(In reply to Marcel de Rooy from comment #4)
> (In reply to Jonathan Druart from comment #2)
> > Something like that, would you agree guys?
> 
> Sounds good. Second patch contains other stuff too.

Agreed!
Comment 6 Jonathan Druart 2018-03-29 14:00:10 UTC
Created attachment 73425 [details] [review]
Bug 20489: Remove warnings from the interface
Comment 7 Jonathan Druart 2018-03-29 14:00:58 UTC
(In reply to Marcel de Rooy from comment #4)
> Second patch contains other stuff too.

Indeed, fixed! I was testing the installer part.
Comment 8 Jonathan Druart 2018-03-29 14:01:44 UTC
No test plan provided, use your knowledge and imagination to know if these patches are relevant and enough.
Comment 9 Mark Tompsett 2018-04-01 15:35:24 UTC
Tested:
Staff
OPAC
Self-checkout
And fresh install

Only the fresh install allowed the system log in, as desired.
Comment 10 Mark Tompsett 2018-04-01 15:49:18 UTC
Attempted steps to go to urls which should trigger modified code in both logged in and logged out states. Failures and successes as expected.
Comment 11 Mark Tompsett 2018-04-01 15:50:30 UTC
Comment on attachment 73400 [details] [review]
Bug 20489: Prevent DB user login

Review of attachment 73400 [details] [review]:
-----------------------------------------------------------------

::: acqui/neworderempty.pl
@@ +203,5 @@
>          my $order_patron = Koha::Patrons->find( $order_user_id );
>          push @order_users, $order_patron if $order_patron;
>      }
> +    if ( $data->{subscriptionid} ) {
> +        $template->param( subscriptionid => $data->{subscriptionid} );

This tweak is the one I don't understand.
Comment 12 Jonathan Druart 2018-04-02 14:35:30 UTC
Created attachment 73528 [details] [review]
Bug 20489: Prevent DB user login
Comment 13 Jonathan Druart 2018-04-02 14:35:33 UTC
Created attachment 73529 [details] [review]
Bug 20489: Remove warnings from the interface
Comment 14 Jonathan Druart 2018-04-02 14:36:13 UTC
(In reply to M. Tompsett from comment #11)
> This tweak is the one I don't understand.

It was not related to this patch.
Comment 15 Mark Tompsett 2018-04-03 04:05:56 UTC
Created attachment 73543 [details] [review]
Bug 20489: Prevent DB user login

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 16 Mark Tompsett 2018-04-03 04:05:59 UTC
Created attachment 73544 [details] [review]
Bug 20489: Remove warnings from the interface

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 17 Mark Tompsett 2018-04-03 04:06:44 UTC
See comment #9 and comment #10 for how I tested.
Comment 18 Marcel de Rooy 2018-04-13 09:12:17 UTC
QA: Looking here
Comment 19 Marcel de Rooy 2018-04-13 09:53:56 UTC
Is the term Basic installation complete? from onboarding still valid?
Since you cannot login anymore, you will have to go further..
Comment 20 Marcel de Rooy 2018-04-13 10:04:44 UTC
die "A patron with cardnumber '42' already exists" ???
From the new script misc/devel/create_superlibrarian.pl
Comment 21 Marcel de Rooy 2018-04-13 10:06:06 UTC
Just another note on that script: We should really remove the default koha/koha. Force people to enter something.
Comment 22 Marcel de Rooy 2018-04-13 10:07:13 UTC
Created attachment 74150 [details] [review]
Bug 20489: Prevent DB user login

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 23 Marcel de Rooy 2018-04-13 10:07:24 UTC
Created attachment 74151 [details] [review]
Bug 20489: Remove warnings from the interface

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 24 Marcel de Rooy 2018-04-13 10:08:38 UTC
In the release notes it would be good to refer to the Forgot password feature (hopefully enabled) or the script create_superlibrarian
Comment 25 Jonathan Druart 2018-04-13 13:36:50 UTC
(In reply to Marcel de Rooy from comment #21)
> Just another note on that script: We should really remove the default
> koha/koha. Force people to enter something.

I think that a blocker for this one.
Comment 26 Jonathan Druart 2018-04-20 15:49:40 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 27 Katrin Fischer 2018-04-22 08:49:21 UTC
I am not sure yet if this is the cause, but I cannot log in the OPAC with my staff user right now, which is not even superlibrarian?
Comment 28 Katrin Fischer 2018-04-22 08:58:07 UTC
Ok, reset_all fixed it, still baffled.
Comment 29 Katrin Fischer 2018-04-22 09:02:31 UTC
For other confused comments see bug 20632.
Comment 30 Katrin Fischer 2018-04-22 09:59:29 UTC
I am not sure about this one. 

Won't it remove the possibility to fix a forgotten superlibrarian password?
I know there is the script, but it's often hard to get IT run something for you, especially in smaller libraries.

Have updates been tested? I don't see that in comment#9.

What happens with libraries that didn't add a superlibrarian user in the past (pre-onboarding)?

>In the release notes it would be good to refer to the Forgot password feature >(hopefully enabled) or the script create_superlibrarian

The reset password feature won't ever be used by all libraries. For example libraries using LDAP, Shibboleth or CAS won't ever use it.

Also, why is there no proper test plan or nice description on the commit messages? (looking at you, Jonathan, this time! ;) )
Comment 31 Jonathan Druart 2018-04-23 14:50:41 UTC
(In reply to Katrin Fischer from comment #30)
> I am not sure about this one. 
> 
> Won't it remove the possibility to fix a forgotten superlibrarian password?
> I know there is the script, but it's often hard to get IT run something for
> you, especially in smaller libraries.

What is sure is that it put too much work on dev side, see related bug reports.
We discussed that several times and I understood there was a general agreement on removing this "feature".
It's a long standing problem and we will need to re-educate the users/sysadmin to use other tools, like the script to create real user accounts.
I think we will certainly have to provide more tools to replace this workaround.

> Have updates been tested? I don't see that in comment#9.

Marcel and I did.

> What happens with libraries that didn't add a superlibrarian user in the
> past (pre-onboarding)?

They will have to create a superlibrarian user.

> >In the release notes it would be good to refer to the Forgot password feature >(hopefully enabled) or the script create_superlibrarian
> 
> The reset password feature won't ever be used by all libraries. For example
> libraries using LDAP, Shibboleth or CAS won't ever use it.

I do not understand what it means.

> Also, why is there no proper test plan or nice description on the commit
> messages? (looking at you, Jonathan, this time! ;) )

I told it on comment 8, the idea was to let testers think about what I forgot.
Comment 32 Katrin Fischer 2018-04-23 14:54:25 UTC
If you use LDAP, you don't store patron's passwords, so you won't turn on the password reset feature. And you will turn off the password change option in the OPAC, because it will give people wrong ideas :)

Could the superlibrarian script be extended a bit (separate bug) to allow for changing passwords from command line?
Comment 33 Katrin Fischer 2019-02-04 22:38:32 UTC
*** Bug 11590 has been marked as a duplicate of this bug. ***