Bug 13342

Summary: Not logged in user can place a review/comment as a deleted patron
Product: Koha Reporter: Olli-Antti Kivilahti <olli-antti.kivilahti>
Component: OPACAssignee: Alexandre Noel <alexandre.noel>
Status: Pushed to oldoldstable --- QA Contact: Marcel de Rooy <m.de.rooy>
Severity: normal    
Priority: P5 - low CC: chris, david, dcook, didier.gautheron, fridolin.somers, hammat.wele, jonathan.druart, katrin.fischer, laurent.ducos, lisette, lucas, m.de.rooy, marie-luce.laflamme, martin.renvoize, nick, philippe.blouin, tomascohen, wainuiwitikapark
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.11.00,24.05.04,23.11.09,23.05.15
Circulation function:
Attachments: Bug 13342 - Not logged in user can place a review/comment as a deleted patron
Bug 13342: Not logged user can place a review/comment
Bug 13342: Perltidy
Bug 13342: Tidy
Bug 13342: Not logged user can place a review/comment
Bug 13342: Tidy
Bug 13342: Not logged user can place a review/comment

Description Olli-Antti Kivilahti 2014-11-26 07:09:38 UTC
REPLICATE:
1. Log in to OPAC.
2. Find a Biblio and open the comments-tab.
3. Wait for the session to timeout, or logout from another tab/window.
4. Modify/create a review/comment.
5. Save it.
6. Go to the staff client and moderate "Comments awaiting moderation".
7. Observe the review/comment as being created from "(deleted patron)"
Comment 1 Katrin Fischer 2019-02-07 09:11:31 UTC
This problem caused us some headache as it happened quite often in one of the libraries we support. I just confirmed that the problem still exists in 18.11 and therefore possibly master as well. 

Thx, Olli, for reporting!
Comment 2 Marie-Luce Laflamme 2024-06-11 19:46:48 UTC
This problem remains unsolved in the master and 23.05 versions. I have observed that Koha incorrectly saves a comment after a user logs out and immediately publishes the comment on the OPAC without prior approval.


1. Log in to OPAC.
2. Keep your account details in a tab
3. Open up a second tab and find a record
4. In the record comments section, click on the link: "Post your comments"
--> leave the comment form popup window open
5. Switch to the account details tab, and logout
6. Quickly return to the comment popup, add a message, and submit it.
--> Koha will save the comment, even if you are not logged in.
7. Check the record's comment while logged out.
--> Notice your comment in the record. 
--> This comment shouldn't be published before approval; Koha seems to skip a step here.
8. Go to the staff client and moderate "Comments awaiting moderation".
9. Check your comment
--> The comment was saved with "(deleted patron)" in the "from" field
Comment 3 Alexandre Noel 2024-06-28 18:23:49 UTC
Created attachment 168286 [details] [review]
Bug 13342 - Not logged in user can place a review/comment as a deleted patron

I find out the post comments page allows not logged in user, so I simply deleted the
relevant line.

1. Apply the patch
2. Log in to OPAC.
3. Find a Biblio and open the comments tab.
4. Open another tab/window, and log out from that page.
5. Return to the first page.
6. Click "Post your comments on this title."
 --> You should be redirected to the login page in a new window.

7. In the new window, log in again.
8. Write a comment but don't submit it.
9. In the seconf tab, log out from the account.
10. Return to the form post comments and click "Submit and close this
    window"
 --> You should be redirected to the login page.
Comment 4 David Nind 2024-07-07 20:22:32 UTC
Thanks for the patch Alexandre!

I have tested on main before and after the patch is applied (using koha-testing-docker).

If I follow the test plan, I now get this error page when I try to submit the comment (after logging out in another tab):

  Sorry, the requested page is not available
  Error 403
  This message can have the following reason(s):

  The form submission failed (Wrong CSRF token). Try to come back, refresh the 
  page, then try again.

  ... Standard page not found message

Maybe the CSRF patches (bugs 34478 and 36192) have solved this for 24.05 and later?

I also tested on one of the demo instances (https://koha-community.org/demo/) for Koha 23.11 and could replicate the issue. So the issue still exists.

I'm not sure of the best way forward here:
1. Fix the CSRF issue (or maybe this is doing what it is supposed to do?)
2. Make this patch for 23.11 and before?
3. A combination of these?

As I'm not a developer, and I don't really understand the CSRF issue, I'm probably not going to be of any help in solving the actual issue. But happy to test the patches!


Note: The patch needs to follow the commit message guidelines - see https://wiki.koha-community.org/wiki/Commit_messages Post here or on the Koha Community Chat - Development channel if you need help with that (https://chat.koha-community.org/).
Comment 5 Alexandre Noel 2024-08-01 16:33:45 UTC
Hi, thank you for your reply and advice!

I don't really understand why it's giving you an error 403, because I tried again on main (v24.06) and can't seem to get this error with or without the patch. I don't use the KTD, so maybe it would work without using it?

Anyway, as long as the user can't post the comment, it's fine because that's what we want to do.

Also, thank you for letting me know that there is a chat for the community. You can reach me at @alexandre.nl
Comment 6 Hammat wele 2024-09-03 20:36:27 UTC
Created attachment 171000 [details] [review]
Bug 13342: Not logged user can place a review/comment

We are able to comment a notice even when no user is connected

To test

1. Log in to OPAC.
2. Find a Biblio and open the comments tab.
3. Open another tab/window, and log out from the account.
4. Return to the first tab.
5. Click "Post your comments on this title."
 --> We are able to post a comment
6. Apply the patch
7. Repeat step 1, 2, 3, 4, 5
 --> You should be redirected to the login page.
Comment 7 Hammat wele 2024-09-03 20:36:31 UTC
Created attachment 171001 [details] [review]
Bug 13342: Perltidy
Comment 8 David Cook 2024-09-04 06:51:52 UTC
Failing this as the patch doesn't apply. But also... that first patch looks bizarre. It's changed a lot of whitespace unnecessarily.
Comment 9 David Cook 2024-09-04 06:54:38 UTC
(In reply to Alexandre Noel from comment #5)
> I don't really understand why it's giving you an error 403, because I tried
> again on main (v24.06) and can't seem to get this error with or without the
> patch. I don't use the KTD, so maybe it would work without using it?
> 
> Anyway, as long as the user can't post the comment, it's fine because that's
> what we want to do.

The 403 is a CSRF error. While the user is POSTing the form with a CSRF token, that token is tied to the user's session. If their session is no longer valid, then the CSRF token validation fails. 

So 24.05+ won't have this issue.
Comment 10 David Cook 2024-09-04 06:59:53 UTC
Changing the version to 23.11 as that should be the last version with the bug
Comment 11 Jonathan Druart 2024-09-04 07:17:17 UTC
You really shouldn't embed indentation changes that are not within the block of code your patch modifies. Especially for this kind of bugs, that will need to be backported. Please keep the changes as short as possible.
Comment 12 Hammat wele 2024-09-04 12:33:27 UTC
(In reply to David Cook from comment #8)
> Failing this as the patch doesn't apply. But also... that first patch looks
> bizarre. It's changed a lot of whitespace unnecessarily.

My bad, i forgot to make the first patch obsolete, should apply now
Comment 13 David Cook 2024-09-04 23:42:01 UTC
(In reply to Jonathan Druart from comment #11)
> You really shouldn't embed indentation changes that are not within the block
> of code your patch modifies. Especially for this kind of bugs, that will
> need to be backported. Please keep the changes as short as possible.

Ditto

That second patch should be obsoleted too.
Comment 14 Hammat wele 2024-09-05 12:51:20 UTC
(In reply to David Cook from comment #13)
> (In reply to Jonathan Druart from comment #11)
> > You really shouldn't embed indentation changes that are not within the block
> > of code your patch modifies. Especially for this kind of bugs, that will
> > need to be backported. Please keep the changes as short as possible.
> 
> Ditto
> 
> That second patch should be obsoleted too.

I made it obsolete but QA testing shows some tidiness.


 WARN	opac/opac-review.pl
   OK	  critic
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  pod
   OK	  spelling
   WARN	  tidiness
		The file is less tidy than before (bad/messy lines before: 27, now: 30)
   OK	  valid


Processing additional checks OK!
Comment 15 David Cook 2024-09-05 23:46:06 UTC
(In reply to Hammat wele from comment #14)
> I made it obsolete but QA testing shows some tidiness.

No worries. We can take care of that.
Comment 16 David Cook 2024-09-06 00:30:43 UTC
Created attachment 171109 [details] [review]
Bug 13342: Tidy
Comment 17 David Cook 2024-09-06 00:35:56 UTC
You can test this in ktd "main" by commenting out the following line in /etc/koha/sites/kohadev/plack.psgi and restarting Plack:

enable "+Koha::Middleware::CSRF";
Comment 18 David Cook 2024-09-06 00:36:25 UTC
Created attachment 171110 [details] [review]
Bug 13342: Not logged user can place a review/comment

We are able to comment a notice even when no user is connected

To test

1. Log in to OPAC.
2. Find a Biblio and open the comments tab.
3. Open another tab/window, and log out from the account.
4. Return to the first tab.
5. Click "Post your comments on this title."
 --> We are able to post a comment
6. Apply the patch
7. Repeat step 1, 2, 3, 4, 5
 --> You should be redirected to the login page.

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 19 David Cook 2024-09-06 00:36:28 UTC
Created attachment 171111 [details] [review]
Bug 13342: Tidy

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 20 Marcel de Rooy 2024-09-06 07:21:10 UTC
Created attachment 171118 [details] [review]
Bug 13342: Not logged user can place a review/comment

We are able to comment a notice even when no user is connected

To test

1. Log in to OPAC.
2. Find a Biblio and open the comments tab.
3. Open another tab/window, and log out from the account.
4. Return to the first tab.
5. Click "Post your comments on this title."
 --> We are able to post a comment
6. Apply the patch
7. Repeat step 1, 2, 3, 4, 5
 --> You should be redirected to the login page.

Signed-off-by: David Cook <dcook@prosentient.com.au>

Bug 13342: Tidy

Signed-off-by: David Cook <dcook@prosentient.com.au>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 21 Marcel de Rooy 2024-09-06 07:21:36 UTC
Just within 10 years ;)
Comment 22 Wainui Witika-Park 2024-09-11 03:57:12 UTC
Backported to 23.05.x-security
Comment 23 Fridolin Somers 2024-10-01 13:09:46 UTC
Backported to 23.11.x-security
Comment 24 Lucas Gass (lukeg) 2024-10-09 14:36:00 UTC
Backported to 24.05.x for 24.05.04
Comment 25 Katrin Fischer 2024-10-18 10:17:08 UTC
Pushed for 24.11!

Well done everyone, thank you!