- Issue created by @brockfanning
- π³π±Netherlands roderik Amsterdam,NL / Budapest,HU
I would like to have the module give a more descriptive error than this, in every case. But from reading the code, I don't see how this happens.
Since the code is a maze, here's a simplified call tree:
1) SamlService::acs() calls ExternalAuth::register().
$unique_id is from the "Unique ID attribute". I don't know what this is in your case, but it is always nonempty at this point, even if the 'name' (from the SAML attribute) would not be.
2) ExternalAuth::register() does its thing and calls $account->save(). samlauth_user_presave() calls UserSyncEventSubscriber::onUserSync() just before the user is actually saved.
At this point, $account should have a nonempty name (even if it is bogus - it might contain the unique ID in theory), and it should still be isNew().
At the end of this function, the name and the email should be overwritten with the value from your user name / email attribute. (getAttributeByConfig() -> setUsername() and setEmail().)
From just reading the code, I cannot see a path that has emptied out the name and not thrown an error, at the end of this function. This is where to debug / log to watchdog / whatever.
If the username is still nonempty at the end of UserSyncEventSubscriber::onUserSync(), it's out of our hands. You have e.g. another hook_user/entity_presave that is messing things up.
(Linking other issue that might have nothing to do with it, just for a personal reminder to re-check this when I get to doing that one.)
- πΊπΈUnited States brockfanning
Thank you for the quick response @roderik! That's helpful. We did do a little debugging at this point in the code:
https://git.drupalcode.org/project/samlauth/-/blob/8.x-3.x/src/SamlServi...
We confirmed that $account_data['name'] was indeed null/empty at that point.
So, this points to some hook_user or entity_presave somewhere else in our custom or contrib code, is that right?
- π³π±Netherlands roderik Amsterdam,NL / Budapest,HU
No, this points to
1) your "User name attribute" configuration being incorrect (or your SAML data disappearing, but I don't think so). Because
$this->getAttributeByConfig('user_name_attribute')
should be getting the value from the attribute named "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress", but apparently that does not exist or is empty.(If needed, you can turn on some debugging in configuration - User - SAML authentication - SAML , to get your full IDP response including the attriubutes logged to watchdog.)
2) UserSyncEventSubscriber::onUserSync() not doing its job of giving you a proper error message.
(From reading the code, I was expecting the email to be empty and this being turned into an exception - because you're using the same attribute for name and email. I'll doublecheck this sometime when I address the linked issue - I should really make automated tests for this code.)
- π³π±Netherlands roderik Amsterdam,NL / Budapest,HU
@brockfanning I am mixing things up. For completeness: in my reasoning, things are going on:
1) I was expecting the account name not to be empty at the start of UserSyncEventSubscriber::onUserSync() , but filled with the $unique_id. So I still don't know where your database error about a NULL name is coming from.
Maybe the cause of this is some other hook, or maybe I am just not reading ExternalAuth code right.
But that's not of direct interest to you, at first.
2) The first interesting thing for you is that $account_data['name'] is empty at that earlier point, and that warrants checking, per my previous comment.