Validate existing session before redirecting to RelayState

Your ratio of useful contributions vs. amount of posts on drupal.org is remarkably high :-)

Thank you for opening this issue. Getting my thoughts organized around what to do when another user logs in, was still on my list. (Two thing I've now answered for myself, 1. "what does Core do" and 2. do we want to do the same for this module, in #2992183: Check behavior if user tries to log in multiple times.)

I agree this is useful and should be implemented in an option (with default value FALSE) like you have done.

My doubt is around how to implement it. It feels to me like this is logically part of the code in SamlController/Service::acs() - that is: it feels like the code around an if (!$validateSession) ... which we would need to add there is quite small, and saves us from needing to have a separate event subscriber with duplicate code doing Auth:processResponse() and redirection.

I do appreciate that you've added a complete self contained patch with unit tests, and regret that the SamlAuth service doesn't have tests yet... but it seems like the resulting code will be easier to understand if this function is not split into a separate subscriber.

My question to you is: what is the reason that this is implemented as a separate event subscriber? Is it just the ugliness of the current SamlService code, or is there another reason I'm not seeing?

(Whether the separate route subscriber will still be needed... I'm not 100% sure yet. I hope it will logically follow from a rewrite, that we can just drop access controls from the saml_controller_acs route.)

Looking at this again (now that I have time),

1)
I realized that there was already existing code modifying the ACS route (reacting to the "Access denied" for logged-in users), inside an event subscriber. If it was moved into SamlService::acs(), #3129800: Processes relay state in AccessDeniedSubscriber event would not have needed to be opened - and would not have needed to duplicate code.

So I moved that code into SamlService::acs() - which already opened up the /saml/acs route to logged-in users.

2)
I did the same with this issue's functionality. Now SamlService::acs() is even longer and uglier than it was before, but IMHO there still is an advantage to having all the logic in one place, not spread out over different event subscribers. We can have better user facing messages, and we only need to decode the SAML response once.

I hope that works for you too. I did change the configuration name "saml_validate_existing_session" to "logout_different_user".