Description
Chris Cormack
2010-05-20 23:33:34 UTC
Is this still valid? Created attachment 9010 [details]
screenshot
When a user is restricted, the checkout box is hidden. This is still valid, you can't check out to a restricted user without removing the restriction.
Created attachment 28453 [details] [review] Add a button to temporarily override patron debarment. Add a button to temporarily override patron debarment. To test: 1) Go to patron check out page 2) If patron has been debarred (eg. due to having fees), there is no way to check out to that patron. 3) Apply patch 4) A debarred patron check out page now has a button for "Override debarment temporarily" - clicking on it should allow checking out to that patron. Patch tested with a sandbox, by Aleisha <aleishaamohia@hotmail.com> Created attachment 28753 [details] [review] Bug 643: Allow override of 'debarred' status Add a button to temporarily override patron debarment. To test: 1) Go to patron check out page 2) If patron has been debarred (eg. due to having fees), there is no way to check out to that patron. 3) Apply patch 4) A debarred patron check out page now has a button for "Override debarment temporarily" - clicking on it should allow checking out to that patron. Signed-off-by: Aleisha <aleishaamohia@hotmail.com> Bug 10863 introduced the lift_restriction permission. I think this button/action should be only displayed/possible if the librarian has this permission. Marked as Failed QA. Created attachment 30715 [details] [review] Bug 643: Allow override of 'debarred' status Add a button to temporarily override patron debarment. To test: 1) Go to patron check out page 2) If patron has been debarred (eg. due to having fees), there is no way to check out to that patron. 3) Apply patch 4) A debarred patron check out page now has a button for "Override debarment temporarily" - clicking on it should allow checking out to that patron. Signed-off-by: Aleisha <aleishaamohia@hotmail.com> Rebased because patch did not apply Created attachment 30716 [details] [review] Bug 643: Followup: Display override button only if issuing staff member has force_checkout permission To test: Apply first patch Apply this patch Log in as superlibrarian Go to Home > Patrons > Set permissions for issuing staff member Disable circulate - force_checkout Log in as issuing staff member Try to checkout to a debarred patron => should not be possible Log in again as superlibrarian Enable circulate - force_checkout for issuing staff member Log in as issuing staff_member Try to checkout to a debarred person => You should get the button 'Override debarment temporarily' Patch tested with a sandbox, by Aleisha <aleishaamohia@hotmail.com> Created attachment 30726 [details] [review] Bug 643: Allow override of 'debarred' status Add a button to temporarily override patron debarment. To test: 1) Go to patron check out page 2) If patron has been debarred (eg. due to having fees), there is no way to check out to that patron. 3) Apply patch 4) A debarred patron check out page now has a button for "Override debarment temporarily" - clicking on it should allow checking out to that patron. Signed-off-by: Aleisha <aleishaamohia@hotmail.com> Rebased because patch did not apply Signed-off-by: Aleisha <aleishaamohia@hotmail.com> Created attachment 30727 [details] [review] Bug 643: Followup: Display override button only if issuing staff member has force_checkout permission To test: Apply first patch Apply this patch Log in as superlibrarian Go to Home > Patrons > Set permissions for issuing staff member Disable circulate - force_checkout Log in as issuing staff member Try to checkout to a debarred patron => should not be possible Log in again as superlibrarian Enable circulate - force_checkout for issuing staff member Log in as issuing staff_member Try to checkout to a debarred person => You should get the button 'Override debarment temporarily' Signed-off-by: Aleisha <aleishaamohia@hotmail.com> The second patch does not fix correctly the problem. If I set forceallow=1 in the url, I can checkout an item without the force_checkout_permission. Marked as Failed QA. Created attachment 31564 [details] [review] Bug 643: Followup: Encrypt param 'forceallow' to prevent unauthorized override The 'forceallow' param uses now an encrypted param based on session, logged-in user and borrower (instead of 1) Sorry, but this doesn't solve the problem raised in comment #12. The issue is there is no equivalent CAN_user_force_checkout on the server-side, not whether someone can fake it or not. And also, even a lousy hacker can still read the randomish forceallow code from the page source and fake the url in this. You can use C4::Auth::haspermission to check if the user has this permission. Created attachment 31837 [details] [review] Bug 643: QA Followup - server-side verification of permissions Only allow $force_allow=1 if the logged in user has permissions. TEST PLAN --------- Attempt to intentionally override the checkout by passing an appropriately handcrafted URL. -- Regardless of the force_allow value, it should be not allowed for those lacking the force_checkout permission. NOTE: I didn't test this. I figured Marc Veron could do that. :) (Sorry, couldn't easily get git bz to work with the accent) Created attachment 31997 [details] [review] [SIGNED OFF] Bug 643: Allow override of 'debarred' status Add a button to temporarily override patron debarment. To test: 1) Go to patron check out page 2) If patron has been debarred (eg. due to having fees), there is no way to check out to that patron. 3) Apply patch 4) A debarred patron check out page now has a button for "Override debarment temporarily" - clicking on it should allow checking out to that patron. Signed-off-by: Aleisha <aleishaamohia@hotmail.com> Rebased because patch did not apply Signed-off-by: Aleisha <aleishaamohia@hotmail.com> Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 31998 [details] [review] [SIGNED OFF] Bug 643: Followup: Display override button only if issuing staff member has force_checkout permission To test: Apply first patch Apply this patch Log in as superlibrarian Go to Home > Patrons > Set permissions for issuing staff member Disable circulate - force_checkout Log in as issuing staff member Try to checkout to a debarred patron => should not be possible Log in again as superlibrarian Enable circulate - force_checkout for issuing staff member Log in as issuing staff_member Try to checkout to a debarred person => You should get the button 'Override debarment temporarily' Signed-off-by: Aleisha <aleishaamohia@hotmail.com> Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 31999 [details] [review] [SIGNED OFF] Bug 643: QA Followup - server-side verification of permissions Only allow $force_allow=1 if the logged in user has permissions. TEST PLAN --------- Attempt to intentionally override the checkout by passing an appropriately handcrafted URL. -- Regardless of the force_allow value, it should be not allowed for those lacking the force_checkout permission. NOTE: I didn't test this. I figured Marc Veron could do that. :) (Sorry, couldn't easily get git bz to work with the accent) Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 32120 [details] [review] [PASSED QA] Bug 643: Allow override of 'debarred' status Add a button to temporarily override patron debarment. To test: 1) Go to patron check out page 2) If patron has been debarred (eg. due to having fees), there is no way to check out to that patron. 3) Apply patch 4) A debarred patron check out page now has a button for "Override debarment temporarily" - clicking on it should allow checking out to that patron. Signed-off-by: Aleisha <aleishaamohia@hotmail.com> Rebased because patch did not apply Signed-off-by: Aleisha <aleishaamohia@hotmail.com> Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Tested with/without force_checkout and superlibrarian permission. Also checked crafting a URL is no longer working. No regressions found. Created attachment 32121 [details] [review] [PASSED QA] Bug 643: Followup: Display override button only if issuing staff member has force_checkout permission To test: Apply first patch Apply this patch Log in as superlibrarian Go to Home > Patrons > Set permissions for issuing staff member Disable circulate - force_checkout Log in as issuing staff member Try to checkout to a debarred patron => should not be possible Log in again as superlibrarian Enable circulate - force_checkout for issuing staff member Log in as issuing staff_member Try to checkout to a debarred person => You should get the button 'Override debarment temporarily' Signed-off-by: Aleisha <aleishaamohia@hotmail.com> Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Created attachment 32122 [details] [review] [PASSED QA] Bug 643: QA Followup - server-side verification of permissions Only allow $force_allow=1 if the logged in user has permissions. TEST PLAN --------- Attempt to intentionally override the checkout by passing an appropriately handcrafted URL. -- Regardless of the force_allow value, it should be not allowed for those lacking the force_checkout permission. NOTE: I didn't test this. I figured Marc Veron could do that. :) (Sorry, couldn't easily get git bz to work with the accent) Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Created attachment 32925 [details] [review] Bug 643 [Template follow-up] Allow override of 'debarred' status This minor followup makes two changes: - Replaced instances of "Debarment" with "Restriction" - Moved the override button into the block where the "View restrictions" button is, making the section slightly more compact. Created attachment 32946 [details] [review] [PASSED QA] Bug 643 [Template follow-up] Allow override of 'debarred' status This minor followup makes two changes: - Replaced instances of "Debarment" with "Restriction" - Moved the override button into the block where the "View restrictions" button is, making the section slightly more compact. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Some small template changes, no problems found. Patches pushed to master. Thanks Pasi, Marc, Mark and Owen! |