Undefined value: $user_allowed_default_login

Thanks @jnicola. I ran into this issue as well, but also a related bug. If a user leaves the settings for both the roles list and the user account field empty, then the module should assume that the local account can log in.

I've updated the patch in a way that seems to improve the intention of the settings. As it stands, upgrading our existing site, which didn't specify any roles that should be allowed to log in (since its a new setting), meant that NO local accounts could log in given the current logic. This patch fixes that issue.

Sorry, silly syntax error in that last patch. Here's another shot.

Patch in #5 solved the local login issue for me. Thanks!

RTBC

@jnicola, the problem is, without the patch, I'm unable to upgrade simplesamlphp_auth. My local accounts are no longer able to log in because I haven't updated this setting. Furthermore, creating new roles in the system should not require that I come to simplesamlphp_auth and add that role's ability to log in. The system should assume that an admin who hasn't configured the role based settings isn't using this feature.

Perhaps I've confused the issue by adding my patch to yours, which is a separate issue. We happened to upgrade to php7 and upgrade simplesamlphp_auth at the same time. Given that my patch touched the same code as yours, I thought it the best place to add it.

To try to be clear, the $allowed_default_login_roles functionality that was added introduces a bug, because any site using an older version of simplesamlphp_auth loses the ability for Drupal accounts to log in. In our case, we need to have both SSO and Drupal accounts, because we have both internal and external users, the latter who can't use SSO.

Furthermore, given that the code was already checking to see if $allowed_default_login_users was being used, it is important to assume that an admin who hasn't added any roles to $user_allowed_default_login_roles wants all roles to be able to log in. That's not a security issue, it keeps backwards compatibility with users like us.

I'd argue that, perhaps, simplesamlphp_auth_allowdefaultlogin should default to FALSE, but again, I think that would break backwards compatibility since not all admins would have set that variable.

I second a-fro,

Even on a clean install it was not working the way I expected it to. With local login enabled and no options selected from the roles or users I was not able to log in with a local account. My expectation is that with local login enabled and no option set in the roles or users options that I should be able to log in with ALL of the local accounts. This is much preferred as it will prevent issues and maintenance later as roles or users are added.

I think the patch in #5 does a good job in checking the available configuration and does not add a security hole.

#5 works fine

#5 works great. I would say if this not fixed then maybe only the variable warning should be fixed and the description for the field "Which users should be allowed to login with local accounts?" should not say "If left blank, all local accounts can login."