Bug 34755

I was about to submit the same fix as bug 34163. Closing this one!

*** This bug has been marked as a duplicate of bug 34163 ***

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.

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.

Created attachment 161783 [details] [review]
Bug 34755: Unit tests

Created attachment 161784 [details] [review]
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

Excited to see patches on this one!

We tested OPAC and staff and had no error messages.

Thank you so much!

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>

I'm really interested in this one, and probably will be taking a look at it more over the next week or so.

I can't reproduce this problem in master. Can you double-check your steps for reproducing the problem?

It's working perfectly for me.

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.

(In reply to Maryse Simard from comment #10)
> 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.

If you can give me an exact procedure that can reliably reproduce the problem, that would be great. 

It might be that you're bumping into a different bug and this patch is papering over that problem as well. 

Overall, this might not be relevant following some other work brewing in the background. 

But yeah if you can provide more info to reliably reproduce it.. that would be great.

Hi David,

we tested once again in Master with koha testing docker and we still have this error in Firefox and Chrome. 
We can reproduce it by following the test plan. It is important, that at least one login / session already exists before you test the OIDC authentication as Oliver wrote in comment 2.

After installing Maryse's Patch the error does not occur any more  (still in Master ktd)

 --> We need this Patch in Master and stable versions :-)

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.

(In reply to Jonathan Druart from comment #13)
> David, isn't the same problem we encountered on 34478? Should not we
> backport "Make Koha::Token use session id not userenv id"?

That's what I'm wondering. I haven't been able to reproduce the issue as described above, but backporting that change might fix it for them. 

I am going to take another look to see if I can reproduce this problem...

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...

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).

(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.

Comment on attachment 161783 [details] [review]
Bug 34755: Unit tests

Review of attachment 161783 [details] [review]:
-----------------------------------------------------------------

::: t/db_dependent/api/v1/idp.t
@@ +309,5 @@
> +    $tx->req->cookies( { name => 'CGISESSID', value => $session_id } );
> +    $t->request_ok($tx);
> +
> +    my $error = URI->new($t->tx->res->headers->location)->query_param("auth_error");
> +    isnt($error, "wrong_csrf_token", "CRSF token is recognized even when another user is logged in.");

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

(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.

(In reply to Jonathan Druart from comment #13)
> David, isn't the same problem we encountered on 34478? Should not we
> backport "Make Koha::Token use session id not userenv id"?

Yep, 100%. We need to backport it.

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.

(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.

Thanks for testing, Olivier. I'll take a look at this a little bit later today.

(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.

(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...

Created attachment 162710 [details] [review]
Bug 34755: Backport Koha::Token change from bug 34478

This change includes the Koha::Token changes which uses
Koha::Session for generating and checking CSRF tokens.

0. Apply the patch and koha-plack --restart kohadev
1. Setup Keycloak OIDC SSO according to "Testing SSO"
wiki guide
2. In a regular window go to http://localhost:8080
3. In a private window go to http://localhost:8080 and click
the SSO "Log in with..." button, but don't log into Keycloak
4. In the regular window, login locally, and navigate to 5-6 pages
5. In the private window, log into Keycloak
6. Note that you are redirected back to Koha and logged in
successfully (no wrong_csrf_token error).

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.

Olivier, please test this (with bug 36098) as soon as you can.

Neither this bug (34755) nor 36098 applies on a fresh master. Both have conflicts.

(In reply to Olivier Hubert from comment #28)
> Neither this bug (34755) nor 36098 applies on a fresh master. Both have
> conflicts.

Master already has this change. This bug is only for backport. You need 23.11.x to test this.

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!

(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?

Created attachment 162748 [details] [review]
Bug 34755: Backport Koha::Token change from bug 34478

This change includes the Koha::Token changes which uses
Koha::Session for generating and checking CSRF tokens.

0. Apply the patch and koha-plack --restart kohadev
1. Setup Keycloak OIDC SSO according to "Testing SSO"
wiki guide
2. In a regular window go to http://localhost:8080
3. In a private window go to http://localhost:8080 and click
the SSO "Log in with..." button, but don't log into Keycloak
4. In the regular window, login locally, and navigate to 5-6 pages
5. In the private window, log into Keycloak
6. Note that you are redirected back to Koha and logged in
successfully (no wrong_csrf_token error).

Signed-off-by: Olivier Hubert <olivier.hubert@inlibro.com>

Applied fine on top of 36098.

QA script happy.

Created attachment 162977 [details] [review]
Bug 34755: Backport Koha::Token change from bug 34478

This change includes the Koha::Token changes which uses
Koha::Session for generating and checking CSRF tokens.

0. Apply the patch and koha-plack --restart kohadev
1. Setup Keycloak OIDC SSO according to "Testing SSO"
wiki guide
2. In a regular window go to http://localhost:8080
3. In a private window go to http://localhost:8080 and click
the SSO "Log in with..." button, but don't log into Keycloak
4. In the regular window, login locally, and navigate to 5-6 pages
5. In the private window, log into Keycloak
6. Note that you are redirected back to Koha and logged in
successfully (no wrong_csrf_token error).

Signed-off-by: Olivier Hubert <olivier.hubert@inlibro.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Today I managed to test this properly.

Looks good and works as expected.

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?

(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.

(In reply to Tomás Cohen Arazi from comment #37)
> (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.

Thanks for clarifying!

Pushed to 23.11.x for 23.11.04

Note that we'll need this for 23.05 and 22.11 as well since they implement bug 33675

Backported to 23.05.x for upcoming 23.05.10.

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/

(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...

(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).

(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=355282c6ed83f89f18ed0b7609667772f579b991;hp=d34089a9087a372dfcd0d50cb9d2c3e483daeb3f;hb=c9375e746b3f57e8c1101fce32d48138dc9c030b;hpb=f859c30704f868b11e0b5bbfb8185d4966ba0606

Will look tomorrow.

Yeah it'll be something like that.

Sorry for not being more useful ﷐[U+1F62D]﷑

(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!

Frédéric can we get this into 22.11? I know there are folk out there having this issue in prod still.

Backported to 22.11.x for 22.11.16