[Bug 42719] New: OAuth/OIDC login crashes with 500 when no CGISESSID cookie exists (IdP-initiated flow)
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42719 Bug ID: 42719 Summary: OAuth/OIDC login crashes with 500 when no CGISESSID cookie exists (IdP-initiated flow) Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Authentication Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: martin.renvoize@openfifth.co.uk CC: dpavlin@rot13.org When a user initiates login from the IdP (e.g. clicking an app tile in Okta), they are redirected directly to Koha's /api/v1/oauth/login/{provider}/{interface} endpoint without ever visiting a Koha page first. Since no Koha page was visited, no session exists and no CGISESSID cookie is present. The code at Koha::REST::V1::OAuth::Client line 99 calls: $c->req->cookie('CGISESSID')->value This crashes with: Can't call method "value" on an undefined value The fix is to create a session on-the-fly when the initial OAuth request has no existing session cookie. This supports IdP-initiated SSO flows while preserving CSRF protection (the state parameter is still bound to the session via the browser's cookie jar). The same guard is needed on the callback branch's check_csrf call â if the cookie is missing on callback, redirect with an error instead of crashing. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42719 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | Status|NEW |ASSIGNED CC| |tomascohen@gmail.com -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42719 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> 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=42719 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42719 --- Comment #1 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 199694 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199694&action=edit Bug 42719: Add regression tests Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42719 --- Comment #2 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 199695 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199695&action=edit Bug 42719: Handle missing CGISESSID in OAuth login (IdP-initiated flow) When a user initiates login from the IdP (e.g. Okta app tile), they hit the OAuth endpoint without an existing session cookie. The code crashed calling ->value on an undefined cookie object. This patch: - Creates a session on-the-fly for the initial request when no cookie exists (IdP-initiated flow) - On callback, redirects with wrong_csrf_token error instead of crashing when the cookie is missing CSRF protection is preserved: the state token is bound to the session created in the initial request, and the browser returns the cookie on callback for validation. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42719 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jacob.omara@openfifth.co.uk | |, | |pedro.amorim@openfifth.co.u | |k -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42719 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=40736 --- Comment #3 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Bug 40736 makes the condition display a nicer message. This one goes further by implementing this missing flow. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42719 --- Comment #4 from David Cook <dcook@prosentient.com.au> --- (In reply to Tomás Cohen Arazi (tcohen) from comment #3)
Bug 40736 makes the condition display a nicer message. This one goes further by implementing this missing flow.
Sounds good to me! I can't think of any reason why you wouldn't want to do that. I'll make a note to return to this. Happy to QA if someone else signs off. -- On a side note, what's the scenario where the IdP initiates the SSO? The scenarios I'm thinking of are: 1. User is using Koha and needs auth so they click "SSO Login" 2. User is using third-party system (either something like VuFind/EDS or an app portal like Okta/Microsoft provides) where they're authenticated and they click a link to Koha. Typically, the user would need to click "SSO Login" to bounce back and forth with the IdP, although a local customization could allow for an automatic redirect. Is the idea of putting in the API endpoint into the IdP app portal so that it directly goes to the SSO step and then bounces them back to the main page? That would be one way to avoid the issue of them needing to click the "SSO Login" button manually or applying a local customization to force SSO... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42719 David Cook <dcook@prosentient.com.au> 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=42719 --- Comment #5 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #4)
(In reply to Tomás Cohen Arazi (tcohen) from comment #3)
Bug 40736 makes the condition display a nicer message. This one goes further by implementing this missing flow.
Sounds good to me! I can't think of any reason why you wouldn't want to do that. I'll make a note to return to this. Happy to QA if someone else signs off.
Hmm back on https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40736#c9 I said that this idea of creating a session where it's missing would be a CSRF vulnerability. I was probably thinking that a phisher could cause someone to do a GET to http://localhost:8080/api/v1/public/oauth/login/test/opac which would then yield a state change (ie a login) if they were already authenticated with the IdP. At that point then, they could try to exploit an XSS attack or further CSRF... I'm not sure how viable that it, but I think that's probably what my thought process was. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42719 --- Comment #6 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #5)
I'm not sure how viable that is, but I think that's probably what my thought process was.
I've got a proof of concept brewing in my mind... I might try it out quickly after lunch... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42719 --- Comment #7 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #6)
(In reply to David Cook from comment #5)
I'm not sure how viable that is, but I think that's probably what my thought process was.
I've got a proof of concept brewing in my mind... I might try it out quickly after lunch...
This might need to wait as I have other more urgent things to try to prevent... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42719 Michaela Sieber <michaela.sieber@kit.edu> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |clemens.tubach@kit.edu, | |lukasz.koszyk@kit.edu, | |michaela.sieber@kit.edu, | |raphael.straub@kit.edu -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42719 --- Comment #8 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #5)
I was probably thinking that a phisher could cause someone to do a GET to http://localhost:8080/api/v1/public/oauth/login/test/opac which would then yield a state change (ie a login) if they were already authenticated with the IdP. At that point then, they could try to exploit an XSS attack or further CSRF...
I'm not sure how viable that it, but I think that's probably what my thought process was.
The more I think about it... it shouldn't be doable under most circumstances. Browsers should prevent that type of cross origin request, although there are ways of allowing it... I notice things like outlook.com will redirect you to https://www.microsoft.com/en-gb/microsoft-365/outlook/email-and-calendar-software-microsoft-outlook?deeplink=%2Fmail%2F&sdf=0 You have to click the "Sign in" button before it'll take you to the IdP SSO signin. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42719 --- Comment #9 from David Cook <dcook@prosentient.com.au> --- If we look at OIDC, it does outline some information about "Initiating Login from a Third Party": https://openid.net/specs/openid-connect-core-1_0.html#ThirdPartyInitiatedLog... Okta itself refers you to the OIDC docs: https://help.okta.com/en-us/content/topics/apps/apps_app_integration_wizard_... It looks like X-FRAME-OPTIONS is what blocks the iframe clickjacking in modern browsers... And my testing for putting it into something like an <img> actually only failed because of using localhost for testing... For Okta, wouldn't it make sense to create a "initiate_login_uri" that takes the "iss" parameter, checks it against the Koha database, and then prompts the user saying "Issuer <iss> is initiating login. Would you like to proceed?" and then go off that? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42719 --- Comment #10 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #9)
If we look at OIDC, it does outline some information about "Initiating Login from a Third Party":
https://openid.net/specs/openid-connect-core-1_0. html#ThirdPartyInitiatedLogin
Okta itself refers you to the OIDC docs: https://help.okta.com/en-us/content/topics/apps/ apps_app_integration_wizard_oidc.htm
It looks like X-FRAME-OPTIONS is what blocks the iframe clickjacking in modern browsers...
And my testing for putting it into something like an <img> actually only failed because of using localhost for testing...
For Okta, wouldn't it make sense to create a "initiate_login_uri" that takes the "iss" parameter, checks it against the Koha database, and then prompts the user saying "Issuer <iss> is initiating login. Would you like to proceed?" and then go off that?
I think that this is the way to go. That way, you provide a workable option using third-party initiated login according to spec, plus you're defeating login CSRF. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42719 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #199694|0 |1 is obsolete| | Attachment #199695|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42719 --- Comment #11 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203772 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203772&action=edit Bug 42719: Add regression tests Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42719 --- Comment #12 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203773 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203773&action=edit Bug 42719: Handle missing CGISESSID in OAuth login (IdP-initiated flow) When a user initiates login from the IdP (e.g. Okta app tile), they hit the OAuth endpoint without an existing session cookie. The code crashed calling ->value on an undefined cookie object. This patch: - Creates a session on-the-fly for the initial request when no cookie exists (IdP-initiated flow) - On callback, redirects with wrong_csrf_token error instead of crashing when the cookie is missing CSRF protection is preserved: the state token is bound to the session created in the initial request, and the browser returns the cookie on callback for validation. Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42719 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |43334 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|martin.renvoize@openfifth.c |dcook@prosentient.com.au |o.uk | --- Comment #13 from David Cook <dcook@prosentient.com.au> --- I think I'm happy for us to go with this for now and then implement the standardized version on bug 43334 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43334 [Bug 43334] Implement OIDC Third-Party Initiated Login endpoint -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42719 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #203772|0 |1 is obsolete| | Attachment #203773|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42719 --- Comment #14 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203800 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203800&action=edit Bug 42719: Add regression tests Add tests for the OAuth/OIDC login endpoint covering: - IdP-initiated flow (no CGISESSID cookie on initial request) - Callback without CGISESSID cookie (session lost) - Normal SP-initiated flow (cookie present) Tests create a real identity provider in the database and assert: - Initial request without cookie redirects to the IdP authorize URL - A CGISESSID cookie is set for the new session - Callback without cookie returns wrong_csrf_token error (not 500) - Normal flow with cookie redirects to IdP as expected Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42719 --- Comment #15 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 203801 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203801&action=edit Bug 42719: Handle missing CGISESSID in OAuth login (IdP-initiated flow) When a user initiates login from the IdP (e.g. Okta app tile), they hit the OAuth endpoint without an existing session cookie. The code crashed calling ->value on an undefined cookie object. This patch: - Removes the premature 'No user session found' guard that blocked IdP-initiated flows entirely - Creates a session on-the-fly for the initial request when no cookie exists (IdP-initiated flow) - On callback, redirects with wrong_csrf_token error instead of crashing when the cookie is missing CSRF protection is preserved: the state token is bound to the session created in the initial request, and the browser returns the cookie on callback for validation. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org