[PEGASUS-935] Redirect to default unauthorised zendesk url when SSO is disabled #159
Conversation
yoshdog
changed the title
| @@ -27,13 +27,13 @@ public function loginAction() | |||
| { | |||
| $return_url = Mage::helper('core')->urlDecode($this->getRequest()->getParam('return_url', "")); | |||
There was a problem hiding this comment.
Not sure if this is meant to be decoding. Usually you would be taking the raw value and when adding it back to the URL below you would encode it.
Currently you could add additional query parameters to the resulting URL going to Zendesk due to lack of encoding and the manual query building. Probably benign but doesn’t seem intentional.
I would recommend http_build_query below.
There was a problem hiding this comment.
thanks @driskell
is your comment in relation to lines 79 - 81?
https://github.com/zendesk/magento_extension/pull/159/files#diff-d8d07556aac1f9945f929f9079d3f0afR79-R81
There was a problem hiding this comment.
merging this as is... happy to look into this in a followup PR.
There was a problem hiding this comment.
Yes the return URL could contain ampersand. In which case rather than being elements of the return url they would become additional query parameters sent to the JWT endpoint. So could cause undesired behaviour.
For example If url is: http://hello/something?query&something=more
The resulting url in the redirect becomes:
https://zendeskdomain/access/jwt?jwt=accesstoken&return_to=http://hello/something?query&something=more
So not only not encoded but the final part won’t be part of the return_to and will be stripped when the return happens.
Can’t see if causing an issue except maybe breaking some return URLs. But I don’t know what parameters that endpoint accepts.
…gate-reported-sso-open-redirect-vulnerabili
yoshdog
deleted the
PEGASUS-935-investigate-reported-sso-open-redirect-vulnerabili
branch
When SSO is disabled, we should redirect to the default Zendesk unauthorised URL instead of the defined redirect url thats passed from the user.