SAML-Toolkits / wordpress-saml

OneLogin SAML plugin for Wordpress
MIT License
65 stars 74 forks source link

Add check for SAMLResponse param to work around keycloak redirect #41

Closed starJammer closed 7 years ago

starJammer commented 7 years ago

I'm working with Keycloak as an IdP and was testing an integration with Wordpress.

The problem I ran into was during the logout flow.

Wordpress redirects to Keycloak to request a logout and on the redirect back to Wordpress Keycloak will neglect to include the ?saml_sls get parameter that triggers the plugin. (based on my rudimentary understanding after two days of looking at the plugin code)

I added this additional check for a SAMLResponse parameter as well. Now, the plugin correctly logs the user out of Wordpress when it receives the saml response from keycloak with SAMLResponse as the only get parameter. With the configuration I used, the plugin now correctly redirects the user back to keycloak to authenticate/login.

I understand this might not be desirable but I'm submitting the PR to gather additional comments and feedback.

pitbulk commented 7 years ago

I can't include that PR since SAMLResponse is also sent on SSO flow...

starJammer commented 7 years ago

@pitbulk , do you have any suggestions?

starJammer commented 7 years ago

@pitbulk , I did a quick search for SAMLResponse in the repo, https://github.com/onelogin/wordpress-saml/search?utf8=✓&q=SAMLResponse&type= , and I don't immediately see how this will alter the SSO flow. Additionally, it looks like the SSO flow code only uses POST, but this is checking the GET parameter. Do you have any suggestions or can you reconsider this PR?

pitbulk commented 7 years ago

It's true that the WP SAML plugin only defines ACS URL endpoint to work on the http-post binding, but a bad configured IdP could send SAMLResponse on the http-redirect parameter and then acs_sls will be executed, I'm not happy with the patch.

I wonder why Keycloak is not able to manage saml_sls, have you configured that endpoint in its config and Keycloak ignore the GET parameters?