search

simplesamlphp / simplesamlphp-module-adfs

This module adds support for WS-Federation
GNU Lesser General Public License v2.1
5 stars 5 forks source link

ADFS Post response should exit #4

Closed pradtke closed 4 years ago

pradtke commented 4 years ago

When we've been testing v1.0.2 module we get a weird behavior where the ADFS module post a response AND shows an about "Code should never be reached". I believe this is because the response code below

https://github.com/simplesamlphp/simplesamlphp-module-adfs/blob/10fd1c9045c7e3e1aeda62b0d081e127a06deb76/lib/IdP/ADFS.php#L201-L210

is different from HTTP::submitPOSTData which calls exit(0) after running.

The ADFS response does not call exit() which results in code in prp.php to continue to run.

Adding exit() into ADFS class works fine in our test environment. I was looking to make a PR but I see master has moved to Symfony controller, which I'm not familiar with. I think this issue wouldn't exist in master since the controller is returning a streamed response.

Should we make a v1.0.3 that has a fix for this issue, and the cherry-picked mis-named file commit (https://github.com/simplesamlphp/simplesamlphp-module-adfs/commit/a2973512bcbb515141b7a9bfee3b1cffb0e7aead) and any other issues we uncover with our testing?

tvdijen commented 4 years ago

HI Patrick! Thanks for reporting this! It is indeed a bug and for some reason the exit fell off when rewriting the method to use Twig-templates. I've tagged a new v1.0.3 release to solve this (and branched of into a release-1.0-branch to receive your PRs, should you find any other bugs)

pradtke commented 4 years ago

Thanks @tvdijen!