Duplicate Entry Exception <userId>

[thkus]

I think we have an issue similar to the one fixed in #73.

Scenario: The username on the IdP changes and the user receives an exception similar to "E-Mail is already in use". We then manually change the username to the one matching the IdP but the user subsequently receives another Exception. We can see in the Logs, that it is the same issue that was mentioned in #73

SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '3-12474' for key 'saml_sp_provider_identity_providerId_userId_unq_idx'
The SQL being executed was: INSERT INTO `saml_sp_provider_identity` (`providerId`, `nameId`, `userId`, `enabled`, `sessionId`, `lastLoginDate`, `dateCreated`, `dateUpdated`, `uid`) VALUES (3, '<nameId>', 12474, 1, '<sessionId>', '2020-08-06 08:12:23', '2020-08-06 08:12:23', '2020-08-06 08:12:23', '<uid>')
Error Info: Array
(
    [0] => 23000
    [1] => 1062
    [2] => Duplicate entry '3-12474' for key 'saml_sp_provider_identity_providerId_userId_unq_idx'
)

We are on version 2.1.12 and didn't have problems before with that. I'm not so keen on updating the plugin now to the latest version as we have some traffic on the site and i don't want to risk downtime. Do you have any explanation as to what might be causing this?

Our saml-sp config looks like this

    '*' => [
        'syncGroups' => false,
        'enableCpLoginButtons' => false,
        'mergeLocalUsers' => true,

        'responseAttributeMap' => [
            'Email' => 'email',
            'FirstName' => 'firstName',
            'LastName' => 'lastName',
        ]
    ],

Thanks a lot in advance!

[dsmrt]

The unique constraint on that table was too broad by only looking at the provider id and the craft user id. I added the nameid to the constraint to help. So if the username or nameid changes but the user stays the same, you will get that error on that version of the plugin.

Definitely test the upgrade before you run it on production but most of the updates are relatively small. There have been minor updates/ breaking changes to plugin events for be aware there.

Hope this helps!

[thkus]

Thanks for the quick reply! Ok, i see. Just to be clear: This issue should have occured always? Because we have change usernames before and everything was working fine.

I will see if i can update the plugin then. Thanks!

[dsmrt]

I think this always should have been an issue. I was surprised when this came up in issue #73 that we hadn't seen it before but I imagine the scenario is pretty rare.

[thkus]

Hmm, i dug a little deeper and the initial problem seems to be an exception during User save: 2020-09-01 13:35:11 [-][-][-][error][yii\base\UserException] yii\base\UserException: User save failed: {"email":["E-Mail \"jon.doe@example.com\" is already taken."]}

My findings are:

1. In Craft CMS username and e-mail are matching
2. The SAML Response contains the same E-Mail
3. In general the e-mail is automatically set as the username (could this be an issue, since we didn't declare a specific mapping for that?)

Our attribute Mapping is as posted above

'responseAttributeMap' => [
    'Email' => 'email',
    'FirstName' => 'firstName',
    'LastName' => 'lastName',
]

Are any other properties taken into account besides the one defined in the mapping?

[dsmrt]

This user already exists in Craft right? This is an error I pull back from Craft when I try to save the user.

Couple things here:

Username is will be set from the NameID attribute (NameID is the unique id for a user in SAML terms). See https://github.com/flipboxfactory/saml-sp/blob/2.4.1/src/services/login/User.php#L45

Related to #73, you can reassign another attribute to set the username using nameIdAttributeOverride in the config/saml-sp.php if that will help.

Also, quick note, I deprecated the responseAttributeMap setting. That mapping should be set in the backend now. https://github.com/flipboxfactory/saml-sp/blob/2.4.1/src/models/Settings.php#L176

[thkus]

Thanks for the explanation! This is helping a lot. So the problem seems to be, that the NameId is changing due to an action taken in the IdP. This happens for every user, which has been logged in once to Craft and tries to login to Craft after that.

Unfortunately it seems we can not use nameIdAttributeOverride, because that would change the behavior for all IdP's (around 20-25) and the issue is not persistent. I guess creating a separate field mapping for each IdP (via the Admin panel) won't help us either, right?

[dsmrt]

Ok ... I think I understand your situation. Correct me if I'm wrong.

Since you have multiple IdPs connected to this this Craft site, nameIdAttributeOverride won't suffice because it is a system-wide setting and not per provider. It seems like what you need is one of the following enhancements: 1) a saml specific before user save event, so you can edit the user as needed to update the username (and know that the plugin is saving the user). You may be able to deduce this now with a normal user before save but it could be tricky or a bit hacky. 2) a provider specific setting for the name id override.

Generally speaking, both of these would be good enhancements.

[thkus]

Hey, apologies for not getting back to you at all!

Yes both suggestions sound indeed like a good solution / enhancement.

[dsmrt]

I added both 1 and 2 from my comment above. For the NameID override, one the IdP provider configure tab, you can write-in an alternative attribute name like so:

[dsmrt]

Let me know how this looks. Closing for now.

[dsmrt]

Quick note... looks like it forgot to push the tag for this! 🤦

I just pushed one up now! (sorry about that!)