Session cookie being set, breaking through Varnish cache.

+1 for this. Having it start a session on every pageload makes reverse-proxy caching next to impossible. Would love to see this as a configurable option.

We were experiencing this same issue. I have applied the patch and it seems to have solved the issue.

It appears like the cookie is not destroyed until after the user has logged out AND closed the browser - if they just log out they will not be served cached pages.

There are several problems with this approach.

1. Logout won't work because the logout URLs are not captured.
2. Custom login URLs are not captured and will not work.
3. This module provides a block. If it is turned on with this patch applied every page the block displays on becomes an infinite redirect loop back to itself.

I have attached a patch that takes a slightly different approach. I only load the init function when it is needed. Calling it from the login, logout and block display functions. So warning if you turn on the block it will break caching for any page it shows up on.

Finally, there are two patches. One for the current D6 version of the module and one for the Current D7 version of the module. Hopefully this helps.

I also had this issue, and I used the original suggestion of a patch in the init hook to fix it.

Can anybody explain me why all this activity needs to be done *every* page view in hook_init()? Isn't it sufficient to run this code only on the /saml_login page callback?

I'm aware that my patch above will prevent auto-login-on-every-page behavior, but for high performance sites this is probably necessary.

Ah, that's why! ;)

Here's a patch that disables the login code with a separate setting in the admin form: "Try to validate the SAML session on each page view."

It changes the _simplesamlphp_auth_isEnabled() behavior by checking that variable and the current path.

I'm sorry but I'm pretty sure you are misunderstanding both my patch and how the module works.

First, with my patch hook_init() is not called on every page load (unless you turn on the block provided by this module). You can see this with both my comment above the init function and by the _(underscore) in front of _simplesamlphp_auth_init(). That underscore both makes the method private and prevents hook_init() from calling it.
Second, your patch is effectively the same as disabling the block provided by this module as that is where the auto login functionality lives. Thus it is also true that your patch breaks the block functionality unless your new checkbox is checked.
Finally, as I commented hard coding URLs is a bad idea. First, your single hard coded URL breaks the ability to detect the standard login page and take action when the module is configured to block it. Second, your patch breaks all custom login URLs. Third, as with the original patch your patch breaks the logout link unless your new checkbox is checked.

In short, please don't use your patch just disable the block and you will get the functionality or your patch without the nasty side effects. (Assuming my patch is installed.)

Hiding new patch as I believe it makes things worse.

Yeah, you're right ... I thought I could fix it in a single callback, instead of everywhere, but I missed the block (don't use it) and the logout page.

I do believe your _simplesamlphp_auth_init() should be called after the _simplesamlphp_auth_isEnabled() check.

But that's just a minor issue, which this module is filled with ;)

The previous Drupal 7 patch works for existing users, but will throw a database exception if a new user tries to log in via SAML. This patch is the same as the one by antgiant, minus one call to _simplesamlphp_auth_init() that was causing a duplicate call to user_save(), causing the db error.

I wrote a new SimpleSAMLphp implementation that I'd like to sandbox in a couple of weeks. It's only around 150 lines long and supports ErrorURL, cleans up cookies, uses the Drupal API everywhere, Varnish-tested, and lots of other fixes ...

We had to get our DigiD login ready for an audit, and it would require more than a couple of patches to bend this module to our needs.

My module is quite heavily geared towards DigiD, I'll have a look if I can make it more generic and publish it as a leaner version of the simplesamlphp_auth module.

@MrHaroldA - I have considered making a simplified saml SP module for Drupal using sporto https://code.google.com/p/sporto/ as the SAML SP implementation. I've also considered making it an installation choice in this module (i.e., Basic single IdP environment or Advanced and multiple IdP environments). I'd be interested to see your simplified module.

@geekwisdom / @all: the code will be available as part of a distribribution for Dutch government sites, but if you can't wait that long: please use my contact form to email me, and I'll mail you the DigiD-feature in advance ;)

Neither the patch in #5 or the patch in #12 work. Both produce fatals at various points in the login flow.

I've created a 7.x-3.x branch of this module with significant refactoring. It includes patches from the following issues:

#2190487: User_login form should have a federated link
#2213283: When simplesamlphp_auth_activate is 0 watchdog messages flood the syslog/dblog
#2221679: Destruction of session causes fatal error
#2020009: Session cookie being set, breaking through Varnish cache.
#1824194: hook_user_insert issues when user adding a new user is logged in via sso
#2205901: Allow SimpleSAML authentication on a per-user basis

https://github.com/grasmash/simplesamlphp_auth/tree/7.x-3.x

Given that so much work has been done (and it will not cleanly merge with 7.x-2.x) I propose that a new 7.x-3.x-dev branch be added to the module.

I have attached a reroll of patch #12.

That patch is missing 2 calls to _simplesamlphp_auth_init(); one in hook_user_insert(), the other in simplesamlphp_auth_is_authenticated(). This results in the following error:

PHP Fatal error:  Call to a member function isAuthenticated() on a non-object

I have also added a gate to _simplesamlphp_auth_init() to ensure it is not accidentally executed multiple times per page load.

Edit: @grasmash - this patch applies cleanly to your 3.x branch on github.

Lets focus on implementing this against 3.x, it was set up based on Matts fork. Could you reroll the patch?

Also here is related issue https://www.drupal.org/node/2377727 - session cookie still exists after user logs out.

Here is a straight reroll of #18 against 7.x-3.x-dev. I've not tested it and no idea if it's the right approach.

We are using #18 with 7.x-2.0-alpha2 in prod and it works as advertised for us.