Bug 5974 - Bogus auth check for "StaffMember" role
Summary: Bogus auth check for "StaffMember" role
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: 3.6
Hardware: All All
: PATCH-Sent (DO NOT USE) normal (vote)
Assignee: Owen Leonard
QA Contact: Bugs List
URL:
Keywords:
: 7269 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-03-28 17:32 UTC by Owen Leonard
Modified: 2013-12-05 19:59 UTC (History)
8 users (show)

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


Attachments
Proposed fix (7.13 KB, patch)
2011-03-28 18:11 UTC, Owen Leonard
Details | Diff | Splinter Review
Fix for Bug 5974 - Bogus auth check for "StaffMember" role (6.49 KB, patch)
2011-07-08 02:57 UTC, Chris Cormack
Details | Diff | Splinter Review
Fix for Bug 5974 - Bogus auth check for "StaffMember" role (6.54 KB, patch)
2011-11-04 03:14 UTC, Chris Cormack
Details | Diff | Splinter Review
follow up for trailing " (1.36 KB, patch)
2011-11-25 13:03 UTC, Paul Poulain
Details | Diff | Splinter Review
Bug 5974: Fix broken toolbar on paton checkout tab (2.48 KB, patch)
2011-11-27 14:32 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 5974: Fix broken toolbar on paton checkout tab (2.83 KB, patch)
2011-11-27 15:14 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 5974: Fix broken toolbar on paton checkout tab (2.83 KB, patch)
2011-11-27 15:14 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 5974: Fix broken toolbar on paton checkout tab (2.88 KB, patch)
2011-11-30 07:30 UTC, Chris Cormack
Details | Diff | Splinter Review
[3.6.x] Patch - includes all 3 previous patches for master (7.66 KB, patch)
2011-12-03 18:47 UTC, Katrin Fischer
Details | Diff | Splinter Review
Fix for Bug 5974 - Bogus auth check for "StaffMember" role (7.65 KB, patch)
2011-12-06 22:46 UTC, Chris Cormack
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2011-03-28 17:32:30 UTC
In circ-toolbar.inc:

<!-- TMPL_IF NAME="StaffMember" -->
  <!-- TMPL_IF NAME="CAN_user_staffaccess" -->
    new YAHOO.widget.Button("changepassword");
  <!-- /TMPL_IF-->
<!-- TMPL_ELSE-->
    new YAHOO.widget.Button("changepassword");
<!-- /TMPL_IF -->

Besides being pointless, this refers to a "StaffMember" variable which is only defined in one place, moremember.pl, based only on the patron category. A remnant  of a time before granular permissions?
Comment 1 Owen Leonard 2011-03-28 18:11:10 UTC Comment hidden (obsolete)
Comment 2 MJ Ray (software.coop) 2011-04-02 11:41:53 UTC
This bug is mentioned in:
Fix for Bug 5974 - Bogus auth check for	&quot;StaffMember&quot; role http://lists.koha-community.org/pipermail/koha-patches/2011-March/014287.html
Comment 3 Chris Cormack 2011-07-08 02:57:15 UTC Comment hidden (obsolete)
Comment 4 Chris Cormack 2011-11-04 03:14:19 UTC Comment hidden (obsolete)
Comment 5 Ian Walls 2011-11-19 15:56:37 UTC
Template only change, simplifies template logic and removes check for bogus value.  Marking as Passed QA
Comment 6 Paul Poulain 2011-11-24 13:12:40 UTC
patch pushed, please test
Comment 7 Paul Poulain 2011-11-25 13:03:21 UTC Comment hidden (obsolete)
Comment 8 Katrin Fischer 2011-11-27 14:27:49 UTC
*** Bug 7269 has been marked as a duplicate of this bug. ***
Comment 9 Katrin Fischer 2011-11-27 14:32:53 UTC Comment hidden (obsolete)
Comment 10 Katrin Fischer 2011-11-27 15:14:12 UTC Comment hidden (obsolete)
Comment 11 Katrin Fischer 2011-11-27 15:14:34 UTC Comment hidden (obsolete)
Comment 12 Chris Cormack 2011-11-30 07:30:13 UTC Comment hidden (obsolete)
Comment 13 Ian Walls 2011-12-01 17:22:30 UTC
Template only change, easy to read, marking as Passed QA
Comment 14 Paul Poulain 2011-12-03 06:37:41 UTC
follow-up pushed, please test
Comment 15 Katrin Fischer 2011-12-03 06:41:33 UTC
Tested succesfully on master, waiting to close until the patch has made it into 3.6.
Comment 16 Chris Nighswonger 2011-12-03 12:21:55 UTC
This patch does not apply cleanly to the 3.6.x branch. If it should, please rebase over that branch and attache a corrected patch to this bug report.

cnighs@erasmus:~/koha.3.2.test (3.6.x)$ git cherry-pick -x -s 9cecb7a9d30dfe
Automatic cherry-pick failed.  After resolving the conflicts,
mark the corrected paths with 'git add <paths>' or 'git rm <paths>'
and commit the result with:

        git commit -c 9cecb7a9d30dfeb2c39789b7493d20734647b9b6

cnighs@erasmus:~/koha.3.2.test (3.6.x *+)$ git status
# On branch 3.6.x
# Your branch is ahead of 'origin/3.6.x' by 9 commits.
#
# Unmerged paths:
#   (use "git reset HEAD <file>..." to unstage)
#   (use "git add/rm <file>..." as appropriate to mark resolution)
#
#       both modified:      koha-tmpl/intranet-tmpl/prog/en/includes/circ-toolbar.inc
Comment 17 Katrin Fischer 2011-12-03 18:47:40 UTC Comment hidden (obsolete)
Comment 18 Chris Cormack 2011-12-06 22:46:40 UTC
Created attachment 6625 [details] [review]
Fix for Bug 5974 - Bogus auth check for "StaffMember" role

Also removing some YAHOO.widget.Button declarations which
are redundant.

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

follow-up: trailing " that makes T::T fail

http://bugs.koha-community.org/show_bug.cgi?id=5974

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 5974: Fix broken toolbar on paton checkout tab

Fixed a couple of errors:
- [% CAN_user_staffaccess %] should be [% IF ( CAN_user_staffaccess ) %]
- added missing [% END %]
- added missing line new YAHOO.widget.Button("duplicate");

To test:
1) Open patron account
2) Go through tabs, especially 'Check out'
3) Check toolbars are correct and buttons work

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Comment 19 Chris Cormack 2011-12-06 22:47:18 UTC
This has already been pushed, for master, the patch here is for 3.6.x and applies cleanly there
Comment 20 Chris Nighswonger 2011-12-07 02:03:06 UTC
Patch pushed to 3.6.x.
Comment 21 Liz Rea 2012-04-03 15:15:50 UTC
This makes it so that the "set password" button is now hidden from users that do not have can_user_staffaccess - I don't think this makes sense - a user with edit borrowers permission can change the password from "edit patron," so why, for the button, do we need staffaccess?

I think we perhaps assigned the wrong permission level to this button?