https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite We should offer a system preference so that libraries who'd like to increase their security can set the SameSite attribute of the session cookie to Strict, rather than the default Lax.
Created attachment 148577 [details] [review] Bug 33259: Add SameSiteSessionCookie system preference Sponsored-by: Toi Ohomai Institute of Technology, New Zealand
Created attachment 148578 [details] [review] Bug 33259: Optionally set SameSite attribute of session cookie to Strict Note: The below test plan is assuming you are running the Mozilla Firefox browser. Test plan: 1. Apply patches 2. Upgrade database sudo koha-upgrade-schema <instance> 3. Confirm the new system preference 'SameSiteSessionCookie' is 'Lax' 4. Go to a staff client and OPAC page, right click, choose 'Inspect', go to the 'Storage' tab, click on 'Cookies', click on 'CGISESSID' 5. Confirm 'SameSite' = 'Lax' 6. Change the 'SameSiteSessionCookie' to 'Strict' 7. Open a new private browser window. Go to a staff client and OPAC page, right click, choose 'Inspect, go to the 'Storage' tab, click on 'Cookies', click on 'CGISESSID' 8. Repeat step 4 and confirm 'SameSite' = 'Strict' 9. Go to: Cataloguing > New record 10. Right click, click 'Inspect', go to the 'Console' tab, confirm there are no JavaScript errors Sponsored-by: Toi Ohomai Institute of Technology, New Zealand
Created attachment 148579 [details] [review] Bug 33259: Add SameSiteSessionCookie system preference Sponsored-by: Toi Ohomai Institute of Technology, New Zealand
Created attachment 148580 [details] [review] Bug 33259: Optionally set SameSite attribute of session cookie to Strict Note: The below test plan is assuming you are running the Mozilla Firefox browser. Test plan: 1. Apply patches 2. Upgrade database sudo koha-upgrade-schema <instance> 3. Confirm the new system preference 'SameSiteSessionCookie' is 'Lax' 4. Go to a staff client and OPAC page, right click, choose 'Inspect', go to the 'Storage' tab, click on 'Cookies', click on 'CGISESSID' 5. Confirm 'SameSite' = 'Lax' 6. Change the 'SameSiteSessionCookie' to 'Strict' 7. Open a new private browser window. Go to a staff client and OPAC page, right click, choose 'Inspect, go to the 'Storage' tab, click on 'Cookies', click on 'CGISESSID' 8. Repeat step 4 and confirm 'SameSite' = 'Strict' 9. Go to: Cataloguing > New record 10. Right click, click 'Inspect', go to the 'Console' tab, confirm there are no JavaScript errors Sponsored-by: Toi Ohomai Institute of Technology, New Zealand
Ready for testing. Note: I have not attached unit tests because no unit test files currently check the cookie values.
Created attachment 148589 [details] [review] Bug 33259: Add SameSiteSessionCookie system preference Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Created attachment 148590 [details] [review] Bug 33259: Optionally set SameSite attribute of session cookie to Strict Note: The below test plan is assuming you are running the Mozilla Firefox browser. Test plan: 1. Apply patches 2. Upgrade database sudo koha-upgrade-schema <instance> 3. Confirm the new system preference 'SameSiteSessionCookie' is 'Lax' 4. Go to a staff client and OPAC page, right click, choose 'Inspect', go to the 'Storage' tab, click on 'Cookies', click on 'CGISESSID' 5. Confirm 'SameSite' = 'Lax' 6. Change the 'SameSiteSessionCookie' to 'Strict' 7. Open a new private browser window. Go to a staff client and OPAC page, right click, choose 'Inspect, go to the 'Storage' tab, click on 'Cookies', click on 'CGISESSID' 8. Repeat step 4 and confirm 'SameSite' = 'Strict' 9. Go to: Cataloguing > New record 10. Right click, click 'Inspect', go to the 'Console' tab, confirm there are no JavaScript errors Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Works as described, thanks for the patch.
Created attachment 150259 [details] [review] Bug 33259: (follow-up) Use different speechmarks to prevent errors To test: Confirm the checkouts table for a patron loads successfully
Alex, The QA tool complains about quite a few missing filters within Cookies.set. I think we can add the html filter in those cases, right? Also, should none be option along with lax and strict?
Created attachment 150557 [details] [review] Bug 33259: Add SameSiteSessionCookie system preference Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Created attachment 150558 [details] [review] Bug 33259: Optionally set SameSite attribute of session cookie to Strict Note: The below test plan is assuming you are running the Mozilla Firefox browser. Test plan: 1. Apply patches 2. Upgrade database sudo koha-upgrade-schema <instance> 3. Confirm the new system preference 'SameSiteSessionCookie' is 'Lax' 4. Go to a staff client and OPAC page, right click, choose 'Inspect', go to the 'Storage' tab, click on 'Cookies', click on 'CGISESSID' 5. Confirm 'SameSite' = 'Lax' 6. Change the 'SameSiteSessionCookie' to 'Strict' 7. Open a new private browser window. Go to a staff client and OPAC page, right click, choose 'Inspect, go to the 'Storage' tab, click on 'Cookies', click on 'CGISESSID' 8. Repeat step 4 and confirm 'SameSite' = 'Strict' 9. Go to: Cataloguing > New record 10. Right click, click 'Inspect', go to the 'Console' tab, confirm there are no JavaScript errors 11. Change 'SameSiteSessionCookie' to 'None' 12. Open a new private browser window, repeat step 4, and confirm the 'SameSite' = 'None' 13. Go to: Cataloguing > New record 14. Right click, click 'Inspect', go to the 'Console' tab, confirm there are no JavaScript errors. Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Created attachment 150559 [details] [review] Bug 33259: (follow-up) Use different speechmarks to prevent errors To test: Confirm the checkouts table for a patron loads successfully
Created attachment 150560 [details] [review] Bug 33259: (follow-up) Add HTML filters to Cookies.set Test plan: Run QA test tools and confirm the no complaints about filters
(In reply to Lucas Gass from comment #10) > Alex, > > The QA tool complains about quite a few missing filters within Cookies.set. > I think we can add the html filter in those cases, right? > > Also, should none be option along with lax and strict? Hi Lucas, Thanks for your comments. Yes, adding the HTML filters does fix up the QA test tool failures, so I have added that as a followup patch. Currently all Koha cookies have 'samesite' = 'Lax' which is why I made it the default syspref value. However, according to https://web.dev/samesite-cookies-explained/ there are three possible options for the 'samesite' attribute: - Strict - Lax - None - means you intentionally want the cookie sent in a third-party context. So I think yes we should probably add 'none' as a value along with lax and strict. I have amended the first patch 'Add SameSiteSessionCookie system preference' to add the 'none' option. Thanks, Alex
Created attachment 154142 [details] [review] Bug 33259: Add SameSiteSessionCookie system preference Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Created attachment 154143 [details] [review] Bug 33259: Optionally set SameSite attribute of session cookie to Strict Note: The below test plan is assuming you are running the Mozilla Firefox browser. Test plan: 1. Apply patches 2. Upgrade database sudo koha-upgrade-schema <instance> 3. Confirm the new system preference 'SameSiteSessionCookie' is 'Lax' 4. Go to a staff client and OPAC page, right click, choose 'Inspect', go to the 'Storage' tab, click on 'Cookies', click on 'CGISESSID' 5. Confirm 'SameSite' = 'Lax' 6. Change the 'SameSiteSessionCookie' to 'Strict' 7. Open a new private browser window. Go to a staff client and OPAC page, right click, choose 'Inspect, go to the 'Storage' tab, click on 'Cookies', click on 'CGISESSID' 8. Repeat step 4 and confirm 'SameSite' = 'Strict' 9. Go to: Cataloguing > New record 10. Right click, click 'Inspect', go to the 'Console' tab, confirm there are no JavaScript errors 11. Change 'SameSiteSessionCookie' to 'None' 12. Open a new private browser window, repeat step 4, and confirm the 'SameSite' = 'None' 13. Go to: Cataloguing > New record 14. Right click, click 'Inspect', go to the 'Console' tab, confirm there are no JavaScript errors. Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Created attachment 154144 [details] [review] Bug 33259: (follow-up) Use different speechmarks to prevent errors To test: Confirm the checkouts table for a patron loads successfully
Created attachment 154145 [details] [review] Bug 33259: (follow-up) Add HTML filters to Cookies.set Test plan: Run QA test tools and confirm the no complaints about filters
Rebased against master, applies cleanly now
It might be worth adding that "None" has security implications and is not recommended? Overall, I think only "Lax" and "Strict" makes sense for Koha with "Strict" being preferable overall, although it would need some testing, especially with identity providers...
(In reply to David Cook from comment #21) > It might be worth adding that "None" has security implications and is not > recommended? > > Overall, I think only "Lax" and "Strict" makes sense for Koha with "Strict" > being preferable overall, although it would need some testing, especially > with identity providers... That's a great idea to add that None has security implications and isn't recommended, thanks for suggesting that David, Will add that to the syspref description in the yaml file.
Another thing about "None" is that I think it can only be set with the Secure attribute set as well. Not sure what happens when you try to send a cookie with "None" that isn't secure... I imagine it would default back to Lax but don't know for sure.
Maybe need a rebase I can't apply the patch.
(In reply to Matthias Le Gac from comment #24) > Maybe need a rebase I can't apply the patch. Merge conflicts due to Bug 35651.
Created attachment 160786 [details] [review] Bug 33259: (follow-up) perltidy Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 160787 [details] [review] Bug 33259: Add SameSiteSessionCookie system preference Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 160788 [details] [review] Bug 33259: Optionally set SameSite attribute of session cookie to Strict Note: The below test plan is assuming you are running the Mozilla Firefox browser. Test plan: 1. Apply patches 2. Upgrade database sudo koha-upgrade-schema <instance> 3. Confirm the new system preference 'SameSiteSessionCookie' is 'Lax' 4. Go to a staff client and OPAC page, right click, choose 'Inspect', go to the 'Storage' tab, click on 'Cookies', click on 'CGISESSID' 5. Confirm 'SameSite' = 'Lax' 6. Change the 'SameSiteSessionCookie' to 'Strict' 7. Open a new private browser window. Go to a staff client and OPAC page, right click, choose 'Inspect, go to the 'Storage' tab, click on 'Cookies', click on 'CGISESSID' 8. Repeat step 4 and confirm 'SameSite' = 'Strict' 9. Go to: Cataloguing > New record 10. Right click, click 'Inspect', go to the 'Console' tab, confirm there are no JavaScript errors 11. Change 'SameSiteSessionCookie' to 'None' 12. Open a new private browser window, repeat step 4, and confirm the 'SameSite' = 'None' 13. Go to: Cataloguing > New record 14. Right click, click 'Inspect', go to the 'Console' tab, confirm there are no JavaScript errors. Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 160789 [details] [review] Bug 33259: (follow-up) Use different speechmarks to prevent errors To test: Confirm the checkouts table for a patron loads successfully Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 160790 [details] [review] Bug 33259: (follow-up) Add HTML filters to Cookies.set Test plan: Run QA test tools and confirm the no complaints about filters Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 160791 [details] [review] Bug 33259: (follow-up) perltidy Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 160792 [details] [review] Bug 33259: Add SameSiteSessionCookie system preference Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Created attachment 160793 [details] [review] Bug 33259: Optionally set SameSite attribute of session cookie to Strict Note: The below test plan is assuming you are running the Mozilla Firefox browser. Test plan: 1. Apply patches 2. Upgrade database sudo koha-upgrade-schema <instance> 3. Confirm the new system preference 'SameSiteSessionCookie' is 'Lax' 4. Go to a staff client and OPAC page, right click, choose 'Inspect', go to the 'Storage' tab, click on 'Cookies', click on 'CGISESSID' 5. Confirm 'SameSite' = 'Lax' 6. Change the 'SameSiteSessionCookie' to 'Strict' 7. Open a new private browser window. Go to a staff client and OPAC page, right click, choose 'Inspect, go to the 'Storage' tab, click on 'Cookies', click on 'CGISESSID' 8. Repeat step 4 and confirm 'SameSite' = 'Strict' 9. Go to: Cataloguing > New record 10. Right click, click 'Inspect', go to the 'Console' tab, confirm there are no JavaScript errors 11. Change 'SameSiteSessionCookie' to 'None' 12. Open a new private browser window, repeat step 4, and confirm the 'SameSite' = 'None' 13. Go to: Cataloguing > New record 14. Right click, click 'Inspect', go to the 'Console' tab, confirm there are no JavaScript errors. Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Oops Lucas and I were working on this at the same time. I'll obselete what I did and put his back.
(In reply to Aleisha Amohia from comment #34) > Oops Lucas and I were working on this at the same time. I'll obselete what I > did and put his back. Aleisha, I'm sorry. Since my bug caused the merge conflict I'd figure I would try the rebase. You're more than welcome to obsolete my rebase.
Created attachment 162181 [details] [review] Bug 33259: Add SameSiteSessionCookie system preference Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 162182 [details] [review] Bug 33259: Optionally set SameSite attribute of session cookie to Strict Note: The below test plan is assuming you are running the Mozilla Firefox browser. Test plan: 1. Apply patches 2. Upgrade database sudo koha-upgrade-schema <instance> 3. Confirm the new system preference 'SameSiteSessionCookie' is 'Lax' 4. Go to a staff client and OPAC page, right click, choose 'Inspect', go to the 'Storage' tab, click on 'Cookies', click on 'CGISESSID' 5. Confirm 'SameSite' = 'Lax' 6. Change the 'SameSiteSessionCookie' to 'Strict' 7. Open a new private browser window. Go to a staff client and OPAC page, right click, choose 'Inspect, go to the 'Storage' tab, click on 'Cookies', click on 'CGISESSID' 8. Repeat step 4 and confirm 'SameSite' = 'Strict' 9. Go to: Cataloguing > New record 10. Right click, click 'Inspect', go to the 'Console' tab, confirm there are no JavaScript errors 11. Change 'SameSiteSessionCookie' to 'None' 12. Open a new private browser window, repeat step 4, and confirm the 'SameSite' = 'None' 13. Go to: Cataloguing > New record 14. Right click, click 'Inspect', go to the 'Console' tab, confirm there are no JavaScript errors. Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 162183 [details] [review] Bug 33259: (follow-up) Use different speechmarks to prevent errors To test: Confirm the checkouts table for a patron loads successfully Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 162184 [details] [review] Bug 33259: (follow-up) Add HTML filters to Cookies.set Test plan: Run QA test tools and confirm the no complaints about filters Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 162185 [details] [review] Bug 33259: (follow-up) perltidy Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
No regressions found, this is a real improvement. My one slight query though is the default, I'd be tempted to set it to Strict on new installs by default. It's great allowing people to downgrade their security a little as needed, but I think we should default to highest security level.
Created attachment 162186 [details] [review] Bug 33259: (QA follow-up) Default 'Strict' for new installs
(In reply to Martin Renvoize from comment #41) > No regressions found, this is a real improvement. > > My one slight query though is the default, I'd be tempted to set it to > Strict on new installs by default. It's great allowing people to downgrade > their security a little as needed, but I think we should default to highest > security level. +1
Comment on attachment 162186 [details] [review] Bug 33259: (QA follow-up) Default 'Strict' for new installs Review of attachment 162186 [details] [review]: ----------------------------------------------------------------- ::: installer/data/mysql/mandatory/sysprefs.sql @@ +668,4 @@ > ('RoutingListAddReserves','0','','If ON the patrons on routing lists are automatically added to holds on the issue.','YesNo'), > ('RoutingListNote','To change this note edit <a href=\"/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=RoutingListNote#jumped\">RoutingListNote</a> system preference.','70|10','Define a note to be shown on all routing lists','Textarea'), > ('RoutingSerials','1',NULL,'If ON, serials routing is enabled','YesNo'), > +('SameSiteSessionCookie', 'Strict', 'None|Lax|Strict', 'If Strict, then the SameSite attribute of the session cookie will be set to Strict, rather than the default Lax. None means you want the cookie sent iun a third-party context.', 'Choice'), There's a little typo here. "iun" instead of "in".
Rebasing this one as opac/sco-sco-main.pl wasn't applying anymore.
Technically, a lot of this was probably overkill. The key cookie that needed Strict is the CGISESSID cookie, because that's where there's security implications. However, it doesn't hurt doing it like we have here.
Created attachment 163066 [details] [review] Bug 33259: Add SameSiteSessionCookie system preference Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 163067 [details] [review] Bug 33259: Optionally set SameSite attribute of session cookie to Strict Note: The below test plan is assuming you are running the Mozilla Firefox browser. Test plan: 1. Apply patches 2. Upgrade database sudo koha-upgrade-schema <instance> 3. Confirm the new system preference 'SameSiteSessionCookie' is 'Lax' 4. Go to a staff client and OPAC page, right click, choose 'Inspect', go to the 'Storage' tab, click on 'Cookies', click on 'CGISESSID' 5. Confirm 'SameSite' = 'Lax' 6. Change the 'SameSiteSessionCookie' to 'Strict' 7. Open a new private browser window. Go to a staff client and OPAC page, right click, choose 'Inspect, go to the 'Storage' tab, click on 'Cookies', click on 'CGISESSID' 8. Repeat step 4 and confirm 'SameSite' = 'Strict' 9. Go to: Cataloguing > New record 10. Right click, click 'Inspect', go to the 'Console' tab, confirm there are no JavaScript errors 11. Change 'SameSiteSessionCookie' to 'None' 12. Open a new private browser window, repeat step 4, and confirm the 'SameSite' = 'None' 13. Go to: Cataloguing > New record 14. Right click, click 'Inspect', go to the 'Console' tab, confirm there are no JavaScript errors. Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 163068 [details] [review] Bug 33259: (follow-up) Use different speechmarks to prevent errors To test: Confirm the checkouts table for a patron loads successfully Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 163069 [details] [review] Bug 33259: (follow-up) Add HTML filters to Cookies.set Test plan: Run QA test tools and confirm the no complaints about filters Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 163070 [details] [review] Bug 33259: (follow-up) perltidy Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 163071 [details] [review] Bug 33259: (QA follow-up) Default 'Strict' for new installs
Actually, I'm momentarily going to move this to "In Discussion". I think using a Strict SameSite attribute would break the SSO implementation especially in terms of CSRF. You'd have an anonymous session, then you redirect to the IdP, and then you're redirected back. Since you're being redirected back via an external site, your browser shouldn't send the Strict CGISESSID cookie, which means your CSRF validation will fail. I'll test that in a minute. We might need to always use a "Lax" SameSite attribute for anonymous sessions. I'll try to think of any other problem scenarios...
Going to mark this as Failed QA in any case as it doesn't take into account the CGISESSID cookie created by the OIDC in Koha/REST/V1/OAuth/Client.pm. -- Also, as I feared, with SameSite of "Strict", OIDC SSO is broken. -- Interestingly, when Keycloak POSTs a 302 to Koha it doesn't work, but there is a scenario with Keycloak where a Strict cookie is still sent after a redirect. If I log into Keycloak first, and then I go to Koha and click "Log in with Keycloak", I'm redirected from Koha to Keycloak and redirected from Keycloak to Koha. In this situation where it's two GET 302s in a row, the Strict cookie is still sent, and I'm logged into Koha using Keycloak.
(In reply to David Cook from comment #54) > Interestingly, when Keycloak POSTs a 302 to Koha it doesn't work, but there > is a scenario with Keycloak where a Strict cookie is still sent after a > redirect. > > If I log into Keycloak first, and then I go to Koha and click "Log in with > Keycloak", I'm redirected from Koha to Keycloak and redirected from Keycloak > to Koha. In this situation where it's two GET 302s in a row, the Strict > cookie is still sent, and I'm logged into Koha using Keycloak. I'm not describing this right. With Strict, it's failing because Keycloak is doing a GET to Koha and that's not allowed. With Lax, it's succeeding because it's allowed for Keycloak to do a GET to Koha. And I think in the scenario where it goes Koha -> Keycloak -> Koha will all 302s, I think the browser is considering that to be user-initiated from the Koha side, and that's why it's allowing it to send the cookie. Because we have a POST to Keycloak with the login, that's why it won't let us login with Keycloak when Strict is in use. I think that brings me back to anonymous CGISESSID cookies needing to be Lax, even if an authenticated CGISESSID cookie is Strict.
Oh good. Chrome and Firefox appear to work differently. In Chrome, if you login to Keycloak first, and then do that Koha->Keycloak->Koha redirect you're fine. In Firefox, if you login to Keycloak first, and then do that Koha->Keycloak->Koha redirect, you still will get an error. Of course, you should only be clicking "Login with Keycloak" from an anonymous session, which takes me back to that thing about the anonymous session not being Strict even when authenticated cookies are Strict.
I hate to say it now, but I think we should change tack, and just focus on the SameSiteSessionCookie being for the CGISESSID cookie and only for authenticated contexts. (Focusing only on the CGISESSID cookie is mostly just to help in terms of testability, although I think the best practice is to only set Strict for sensitive cookies.) That should provide security for authenticated Koha users while also allowing usability for things like SSO.
(In reply to David Cook from comment #56) > Oh good. Chrome and Firefox appear to work differently. > > In Chrome, if you login to Keycloak first, and then do that > Koha->Keycloak->Koha redirect you're fine. > > In Firefox, if you login to Keycloak first, and then do that > Koha->Keycloak->Koha redirect, you still will get an error. I'm working on integrating another system with OIDC (testing with Keycloak), and I'm running into the same problem. My app session cookie is set to Strict and since it's going "302 App to Keycloak" -> "200 Keycloak to Keycloak" -> "302 Keycloak to App" Chrome prevents the original session cookie from being sent to the App. But after a successful Keycloak to Keycloak login, then it's "302 App to Keycloak" -> "302 Keycloak to App", and then Chrome will send the session cookie to the App. Firefox will prevent the cookie from being sent in both scenarios. -- In my App, I don't think that I have the ability to use different SameSite attributes for anonymous sessions vs authenticated sessions. It's governed by config for a plugin to the MVC framework. I might have a little play, but I'm thinking I might just use a different cookie for managing the whole SSO interaction. I'll let you know how I go...
(In reply to David Cook from comment #58) > In my App, I don't think that I have the ability to use different SameSite > attributes for anonymous sessions vs authenticated sessions. It's governed > by config for a plugin to the MVC framework. I might have a little play, but > I'm thinking I might just use a different cookie for managing the whole SSO > interaction. > > I'll let you know how I go... I ended up saving the app sessionid in a "sso cookie" with a very short expiry and a SameSite of Lax. So when the SSO redirected back to the App, I used the "sso cookie" to get the sessionid, looked up the older anonymous session using the sessionid, and then I was able to access the relevant state data. I used that to validate the "state" value, fetch the redirect_uri I used for the auth request, etc. It seemed like the least amount of change to get the desired result while still maintaining security. -- At some point with Koha it would be nice to be able to redirect back to the point where the SSO was initiated, but that's difficult with Plack since the request uri gets shortened to "/opac" and "/intranet", although I guess we can always have rewrites for those.