Hi. I am very new to the world of SAML and Drupal. This module seems straightforward enough and implementation seemed to go just fine. This module is up and running for manually registered users with no issues at all. So, if a user is added by an admin, they are able to log in using this module. My trouble starts when I give users the ability to create their own accounts without admin approval or intervention. When a new user follows the process of logging is through SAML, they make it to /saml/consume which gives the generic Drupal error screen if error reporting is disabled. In the logs we see the log in process stops with the following error:

Error: Call to a member function getAccountName() on bool in saml_sp_drupal_login__saml_authenticate()
	(line 204 of /modules/contrib/saml_sp/modules/saml_sp_drupal_login/saml_sp_drupal_login.module)

#0 /modules/contrib/saml_sp/src/Controller/SamlSPController.php(118):
	saml_sp_drupal_login__saml_authenticate(true, Object(OneLogin\Saml2\Response), Object(Drupal\saml_sp\Entity\Idp))
#1 [internal function]:
	Drupal\saml_sp\Controller\SamlSPController->consume()....

It keeps going for another 15 steps. Now, the user can refresh the error page and find all is well. They are taken to the front page and they are logged in. It seems the user is created but the $account variable is not stored or ready at line #204 for a new user and the boolean check fails.

This seems to have been an issue on the 2.x version but was patched. I see the 3.x version has that fix in place but still the same error. I've gone through all the settings and don't feel like I'm missing anything but who knows. Any help to get this running would be greatly appreciated.

The site running the module is on 8.7.10.

Comments

danheisel created an issue. See original summary.

jproctor’s picture

As a test, in /admin/config/people/accounts, I tried setting “Who can register accounts?” to Visitor (no approval, no email verification), and then in /admin/config/people/saml_sp/login I checked the box for “Allow users without an account to request an account?” (which is a little confusing in both title and description), and did not check any boxes for who to send the request to.

With that, I am able to replicate this problem.

Working on a fix now.

jproctor’s picture

Status: Active » Needs review
StatusFileSize
new9.1 KB

I believe this clears it up.

I want to try to improve the clarity in that settings form a little, too, so I will leave the patch here uncommitted, and aim to have a new release (at least -dev, maybe 3.3) next week.

@danheisel, if you’re also new to patching things with Composer, let us know and we’ll either give you a hand or publish that new release so it won’t matter. :)

jrglasgow’s picture

@jproctor,
Thanks for getting on this. Until now I haven't had a use case for this feature on D8, so it is not fully tested.

jproctor’s picture

StatusFileSize
new10.45 KB

A slightly better patch: mark the old saml_sp_drupal_login_get_uid() function as deprecated (even though it’s really unlikely anyone is using it).

danheisel’s picture

Fantastic! Thanks so much for the fast response. We'll give this a go tomorrow and report back.

jproctor’s picture

danheisel’s picture

Just a quick follow-up. Patching went just fine against 3.3 but now we're hitting a snag somewhere in a redirect to /user/login?destination=%2F. The error log shows the referrer as our SAML provider. There are three logs. One is the login attempt. The second is seen below.

RuntimeException: Failed to start the session because headers have already been sent by ".../vendor/symfony/http-foundation/Response.php" at line 377

The third is Invalid response, The status code of the Response was not Success, was Responder -> urn:oasis:names:tc:SAML:2.0:status:RequestDenied.

Once hitting the site and attempting a SSO, the user gets stuck in a loop that keeps repeating these same errors as the key in the URL keeps refreshing.

danheisel’s picture

Never mind. This was all on me. Patch #5 worked brilliantly. It was a network issue for us. Thanks so much for the fast response.

  • jrglasgow committed d0b9f51 on 8.x-3.x authored by jproctor
    Issue #3096220 by jproctor, danheisel: Newly Self-Registered Users and...
jrglasgow’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.