https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |33675 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33675 [Bug 33675] Add CSRF protection to OAuth/OIDC authentication -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 Olivier Hubert <olivier.hubert@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |REOPENED Resolution|DUPLICATE |--- CC| |olivier.hubert@inlibro.com --- Comment #2 from Olivier Hubert <olivier.hubert@inlibro.com> --- I am reopening this bug as we noticed this exact problem happening with our only instance using SSO (and because Jonathan mentioned I should reopen the bug instead of creating a new one). I have reproduced the bug using Koha Testing Docker. Steps to reproduce, assuming you're using Koha Testing Docker, properly configured with Keycloak, although this might work using other configurations as well: 1. Open the OPAC 2. Log in with a regular Koha user, not using SSO. 3. In a private browser window, open the OPAC 4. log in using SSO with a different user account. 5. Notice that you get the error message "There was an error authenticating to external identity provider wrong_csrf_token" 6. Click on the "Log in with [...]" button, to log in again using SSO. 7. Notice that the user is immediately logged in. I have figured out part of the problem. It seems that when first loading the OPAC, the userenv variable is loaded with the latest session data instead of no data at all (e.g. anonymous). By adding a warning on line 224 of Koha/Token.pm, I can get the id on the first OPAC visit, as well as the id when returning from the SSO authentication. If I first login with a user named "koha", at step 2 of my steps to reproduce, I get something like this in plack-api-error.log: [2024/01/12 19:49:08] [WARN] The id for generating the CSRF token is: koha_16b598a250e8b2a9de9b3d94ec89d2c9 at /kohadevbox/koha/Koha/Token.pm line 224. [2024/01/12 19:49:13] [WARN] The id for generating the CSRF token is: anonymous_16b598a250e8b2a9de9b3d94ec89d2c9 at /kohadevbox/koha/Koha/Token.pm line 224. As you can see, the session id is the same, so the session is successfully saved and reused. But it seems the anonymisation happens after the userenv variable is used, and therefore the last used session username is used for generating the CSRF token instead of "anonymous". Note that getting "anonymous" on returning from authentication (step 5) is not guaranteed. On a very busy instance, I was able to get the last used session username instead. Also note that you can prevent the issue by reloading the OPAC page between step 3 and 4. If no one else has a more recent session, then the SSO login will work on the first try. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 --- Comment #3 from Lukas Koszyk <lukasz.koszyk@kit.edu> --- I confirm that we have the same problem (in version 23.05.04). I also checked the userenv variable when generating and checking the CSRF token, in the logs I see the userenv of another user (not "anonymous"). The solution could be to destroy the hash for the active user and build a new hash for environment variables in initial OIDC request before generating the CSRF token (or even in callback by checking the token). Or it might be necessary to delete the entire session and initiate a new one when logging in using OIDC. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 Maryse Simard <maryse.simard@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|REOPENED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 --- Comment #5 from Maryse Simard <maryse.simard@inlibro.com> --- Created attachment 161784 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161784&action=edit Bug 34755: Resets the userenv when calling OAuth api endpoints When using OIDC authentication, the error "wrong_csrf_token" can happen if another user's userenv is loaded during login. This is because the wrong userenv is used to compare the tokens. This patch explicitly resets userenv when using OAuth endpoints of the Koha api. Steps to reproduce: 0. On a Koha with SSO configured 1. Open the OPAC 2. Log in with a regular Koha user, not using SSO. 3. In a private browser window, open the OPAC 4. log in using SSO with a different user account. 5. Notice that you get the error message "There was an error authenticating to external identity provider wrong_csrf_token" 6. Click on the "Log in with [...]" button again. 7. Notice that the user is immediately logged in. To test: - Apply the tests patch - prove t/db_dependent/api/v1/idp.t => FAIL: wrong_csrf_token - Apply patch - prove t/db_dependent/api/v1/idp.t => SUCCESS - Run other OAuth endpoints tests to make sure nothing broke: prove t/db_dependent/api/v1/auth_authenticate_api_request.t prove t/db_dependent/api/v1/oauth.t -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 --- Comment #6 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Excited to see patches on this one! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 --- Comment #8 from David Cook <dcook@prosentient.com.au> --- I'm really interested in this one, and probably will be taking a look at it more over the next week or so. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 --- Comment #10 from Maryse Simard <maryse.simard@inlibro.com> --- I still reproduce this in current master when using the exact steps given here. I admit it is a bit tricky to test because it seems the error doesn't occur every time but, while I can sometimes login correctly, the "wrong_csrf_token" still happens fairly often for me. I've experienced during testing that by visiting a few pages across Koha during the procedure (particularly during step 4, just before posting the SSO login form) with the user logged in directly to Koha, I can more easily reproduce the error. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |philippe.blouin@inlibro.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 --- Comment #13 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- David, isn't the same problem we encountered on 34478? Should not we backport "Make Koha::Token use session id not userenv id"? Olivier, bug 34478 is going to be pushed very soon. It will be interesting to let us know if it fixes this problem. In this case we will submit some patches here for stable branches. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #15 from David Cook <dcook@prosentient.com.au> --- Once again, I cannot reproduce this problem. After authenticating in Keycloak, I'm redirected to the Koha endpoint at http://localhost:8080/api/v1/public/oauth/login/Keycloak/opac which is a GET request. My request cookie has a CGISESSID value of 8435418b15fc0280af09f72a5f6c31cd. This is an anonymous session. My response cookie has a CGISESSID value of 1a11450a805d152b0dfcfcc91da3c341. This is an authenticated session. http://localhost:8080/api/v1/public/oauth/login/Keycloak/opac redirects as a GET request to opac-user.pl. Anti-CSRF shouldn't be involved here... -- I'm looking at Koha/REST/V1/OAuth/Client.pm and it looks OK to me. Personally, I would use a random value for a "state" parameter and save it to the session. Using a CSRF token in this case is problematic for a number of reasons I won't get into here. But the thing is... if you're getting wrong_csrf_token here it's because you've lost your session (well more specifically your userenv since master doesn't yet have the CSRF session fix Jonathan mentions above) between the time the "state" parameter was generated and the time the "state" parameter was checked. That means the test plan given in Comment 2 can't possibly work. Which takes me to Comment 10... there was something about navigating around Koha before posting the SSO login form. Now I have no idea why a person would do that, because it makes no sense at all. If you're clicking "Log in with Keycloak", then I'd just complete the login. But anyway... Recently I did notice that there is somewhere in Koha.. I think on the OPAC.. where the cookie isn't returned when it should be and that did cause me some grief. Let me try... -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 --- Comment #17 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #16)

I was trying my best to break my sessions, and I did manage to reproduce the problem, but I can't recall exactly how.

So I'm going to keep trying again (more carefully).

Ok so one way to do it would be to do the following: 1. Go to http://localhost:8080 2. Click "Log in with Keycloak" but don't log in 3. In a new tab, go to http://localhost:8080, and do a local login 4. Log out of the local login 5. Go back to original tab and complete the Keycloak login You'll get a wrong_csrf_token because the CGISESSID cookie value has changed. An anonymous user becoming an authenticated user will keep the same session ID, but an authenticated user becoming an anonymous user will lose their session ID. You should be getting the wrong_csrf_token warning in this case. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 --- Comment #19 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #18)

Ok this is interesting. Why would another user be logged in?

Ahhhhh I know what the problem is now. So I'm going to keep this as Failed QA because the attached patch is not the solution. As I mentioned on Comment 11, you've actually bumped into a different bug which isn't directly related to the OpenID Connect Authentication. It is due to a problem with C4::Context->userenv, but I won't get into the reasons why. Leave it to me and Jonathan. We'll take care of it. Thanks for your patience on this one. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |36098 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36098 [Bug 36098] Create Koha::Session module -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 --- Comment #21 from Olivier Hubert <olivier.hubert@inlibro.com> --- Following a discussion on IRC with Jonathan, I waited until the commits for 34478 were pushed to master. I then updated everything on my KTD and made sure the commits were there before I started testing. When using SSO to login, I now get the "wrong_csrf_token" error message all the time. This happens using both the OPAC and Intranet. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 --- Comment #23 from David Cook <dcook@prosentient.com.au> --- (In reply to Olivier Hubert from comment #21)

Following a discussion on IRC with Jonathan, I waited until the commits for 34478 were pushed to master. I then updated everything on my KTD and made sure the commits were there before I started testing.

When using SSO to login, I now get the "wrong_csrf_token" error message all the time. This happens using both the OPAC and Intranet.

It looks like 34478 accidentally breaks the generation of the state parameter. I am going to fix that in a different bug report I think, so that we can keep this one for the userenv related issue. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 --- Comment #24 from David Cook <dcook@prosentient.com.au> --- (In reply to Olivier Hubert from comment #21)

Following a discussion on IRC with Jonathan, I waited until the commits for 34478 were pushed to master. I then updated everything on my KTD and made sure the commits were there before I started testing.

When using SSO to login, I now get the "wrong_csrf_token" error message all the time. This happens using both the OPAC and Intranet.

Ok, so I've fixed the breakage caused by 34478 with a patch on bug 36219. It's worth noting that bug 34478 + bug 36219 resolves the issue reported here in bug 34755, but only for the master branch. -- For stable branches like 23.05... we'll want to backport bug 36098 and bring over the changes from git hash d75f1df99d32ab615365ffb87b975d9a53c219f7 I'll look at that now... -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 --- Comment #26 from David Cook <dcook@prosentient.com.au> --- I've just tested this on 23.11.03. Bug 36098 patches plus this patch should fix the problem. I'll comment on bug 36098 asking for folk to backport the Koha::Session change. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 Olivier Hubert <olivier.hubert@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply --- Comment #28 from Olivier Hubert <olivier.hubert@inlibro.com> --- Neither this bug (34755) nor 36098 applies on a fresh master. Both have conflicts. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 --- Comment #31 from David Cook <dcook@prosentient.com.au> --- (In reply to Olivier Hubert from comment #30)

This looks like it's fixed, in 23.11.x. With the patches from this bug and 36098, I was not able to reproduce the issue. SSO works as it should.

Thanks!

Great! Want to sign this one off? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 --- Comment #33 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Applied fine on top of 36098. QA script happy. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |tomascohen@gmail.com |y.org | Status|Signed Off |Passed QA --- Comment #35 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Today I managed to test this properly. Looks good and works as expected. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 --- Comment #37 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Katrin Fischer from comment #36)

Hi Tomas, thanks for testing! Am I correct, that the patches on bug 36098 and here are supposed to go in 23.11 directly now?

Bug 36098 is already in master. So on the maintainers queue. And this one is marked for 23.11 (not master). They should all get backported as much as possible. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |23.11.04 released in| | Status|Passed QA |Pushed to stable CC| |fridolin.somers@biblibre.co | |m --- Comment #39 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.11.x for 23.11.04 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable CC| |lucas@bywatersolutions.com Version(s)|23.11.04 |23.11.04,23.05.10 released in| | --- Comment #41 from Lucas Gass <lucas@bywatersolutions.com> --- Backported to 23.05.x for upcoming 23.05.10. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 --- Comment #43 from David Cook <dcook@prosentient.com.au> --- (In reply to Lucas Gass from comment #42)

This is causing some tests to fail in 23.05.x:

https://jenkins.koha-community.org/view/23.05/job/Koha_23.05_D10/ lastCompletedBuild/testReport/ https://jenkins.koha-community.org/view/23.05/job/Koha_23.05_D11/ lastCompletedBuild/testReport/ https://jenkins.koha-community.org/view/23.05/job/Koha_23.05_D12/ lastCompletedBuild/testReport/

Ah right. I think we removed some tests in master. I'll take a look... -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 --- Comment #45 from Lucas Gass <lucas@bywatersolutions.com> --- (In reply to David Cook from comment #44)

(In reply to David Cook from comment #43)

...

Ah right. I think we removed some tests in master. I'll take a look...

Ran out of time to look at this today, but the answer is in master (via bug 34478 most likely).

Ah, thanks! I think 23.05 needs something like this: https://git.koha-community.org/gitweb/?p=koha.git;a=blobdiff;f=t/Token.t;h=3... Will look tomorrow. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 --- Comment #47 from David Cook <dcook@prosentient.com.au> --- (In reply to Lucas Gass from comment #45)

https://git.koha-community.org/gitweb/?p=koha.git;a=blobdiff;f=t/Token.t; h=355282c6ed83f89f18ed0b7609667772f579b991; hp=d34089a9087a372dfcd0d50cb9d2c3e483daeb3f; hb=c9375e746b3f57e8c1101fce32d48138dc9c030b; hpb=f859c30704f868b11e0b5bbfb8185d4966ba0606

Will look tomorrow.

I went to look at this, and noticed that you've already fixed them. Thanks, Lucas! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 --- Comment #49 from Frédéric Demians <f.demians@tamil.fr> --- Backported to 22.11.x for 22.11.16 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED CC| |david@davidnind.com Resolution|--- |FIXED --- Comment #50 from David Nind <david@davidnind.com> --- Not covered in the manual. No update to the manual required. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34755 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=43037 -- You are receiving this mail because: You are watching all bug changes.