Bug 24018 - No need to die "Not logged in"
Summary: No need to die "Not logged in"
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 18403 20489
Blocks:
  Show dependency treegraph
 
Reported: 2019-11-12 15:03 UTC by Jonathan Druart
Modified: 2020-06-04 20:38 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00


Attachments
Bug 24018: Remove die "Not logged in" (20.17 KB, patch)
2019-11-12 15:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24018: Remove die "Not logged in" (20.12 KB, patch)
2020-01-10 08:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24018: Remove die "Not logged in" (20.17 KB, patch)
2020-01-10 16:20 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 24018: Remove die "Not logged in" (20.26 KB, patch)
2020-01-17 08:06 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2019-11-12 15:03:27 UTC
There is a wrong pattern in several controller files:

my $logged_in_user = Koha::Patrons->find( $loggedinuser ) or die "Not logged in";

This has been introduced by
  commit cee2cf9ff927504f9c24f6519462a7d702c5c27d
  Bug 18403: Add sub output_and_exit_if_error - unknown_patron & cannot_see_patron_infos

Which noted:
    Note that the 'or die "Not logged in";' part should not be needed, but... who trusts
    C4::Auth?
    I think it could be used as a safeguard later. I am willing to sed and remove  them
    if required.

Since we prevent DB user login it's no longer needed.
It is time to remove it, as we trust our code (don't you?) and it can be confusing.
Comment 1 Jonathan Druart 2019-11-12 15:22:02 UTC
Created attachment 95328 [details] [review]
Bug 24018: Remove die "Not logged in"
Comment 2 Michal Denar 2020-01-09 22:28:43 UTC
Jonathan, can you rebasi this patch on master?

Thank you.
Comment 3 Jonathan Druart 2020-01-10 08:32:26 UTC
Created attachment 97160 [details] [review]
Bug 24018: Remove die "Not logged in"
Comment 4 Michal Denar 2020-01-10 16:20:21 UTC
Created attachment 97232 [details] [review]
Bug 24018: Remove die "Not logged in"

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 5 Marcel de Rooy 2020-01-17 08:06:21 UTC
Created attachment 97476 [details] [review]
Bug 24018: Remove die "Not logged in"

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Marcel de Rooy 2020-01-17 08:09:12 UTC
Note: I checked a number of edits to see which call was preceding the find or die statement. In the very most cases it is a get_template_and_user. In members/mod_debarment.pl it was a checkauth call. I saw it twice. This should be fine especially since a flagsrequired is passed too.
Comment 7 Martin Renvoize 2020-01-20 14:04:30 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 8 Joy Nelson 2020-01-31 23:29:51 UTC
Does not apply to 19.11.x branch.  Please rebase if needed.