BUG: Upgrading to 2.0 - Assigned Service Provider Key Pair No Longer Works during SLO

[jeffsikes]

I recently went through the upgrade process for the SAML plugin on our site, and this issue occurred during both upgrades (on our Test environment and Production). There is a workaround, however.

Our SAML configuration requires all requests to be signed, including the SLO requests.

After the upgrade, the SLO requests were no longer being signed, so our SAML implementation denied the requests. SSO processes continued to be signed and worked as expected.

After a bit of fiddling in our Test environment, I found that if I went to "My Provider (SP)" and clicked the "Generate key pair" button from there, then the SLO requests were signed again. After updating my SAML provider with the new certificate value, the SLO process worked as expected.

A note here - if I went directly to the "Keychain" section and generated a new Key Pair from there, and then assigned that new Key Pair back on the My Provider (SP) page, it did NOT work. Still wasn't signing the requests. The only way it works is if you click the "Generate Key Pair" button on the "My Provider (SP)" page directly.

[dsmrt]

Hey @jeffsikes 👋

Sorry about the issue!

Can you help me with these? 1) What plugin version did you upgrade from? (helpful, but no worries if you can't figure it out) 2) What plugin version did you upgrade to? 3) What database (mysql/postgres) are you using? Version would be helpful as well if you have it.

I'll see if I can replicate some of the issues you are seeing.

[jeffsikes]

Hi there! Sure, we happen to have an instance running right now that is pre-upgrade.

• CraftCMS Pro - Version 3.3.4.1, upgraded to 3.5.15.1
• SAML SSO Service Provider - Version 1.0.8, upgraded to 2.5.2

mysql is a different version from QA to Production, but happened in both instances:

• MySQL in Testing Environment - Version 5.7.32
• MySQL in Production Environment - Version 5.7.22

[dsmrt]

I did some testing last night, and i wasn’t able to replicate the upgrade issue. There is a migration with latest release that changes the look up for those key pairs so what you are saying makes me think that there’s an edge case issue that i’m not aware of, or the upgrade failed on your side to a degree.

Can you search the logs for any upgrade errors and see if anything sticks out? Let me know if you can think of anything else that may contribute to that edge case scenario.

[dsmrt]

To expand on that migration mention above, there was a change there to the unique constraints on the link table (saml_provider_keychain_link). The current structure of that table should look something like this:

CREATE TABLE `saml_provider_keychain_link` (
  `id` int(11) NOT NULL AUTO_INCREMENT,
  `providerId` int(11) NOT NULL,
  `keyChainId` int(11) NOT NULL,
  `dateUpdated` datetime NOT NULL,
  `dateCreated` datetime NOT NULL,
  `uid` char(36) NOT NULL DEFAULT '0',
  `providerUid` varchar(40) DEFAULT NULL,
  PRIMARY KEY (`id`),
  UNIQUE KEY `saml_provider_keychain_link_providerUid_keyChainId_unq_fk` (`providerUid`,`keyChainId`),
  KEY `saml_provider_keychain_link_keyChainId_fk` (`keyChainId`),
  CONSTRAINT `saml_provider_keychain_link_keyChainId_fk` FOREIGN KEY (`keyChainId`) REFERENCES `keychain` (`id`) ON DELETE CASCADE
) ENGINE=InnoDB AUTO_INCREMENT=2 DEFAULT CHARSET=utf8;

If the table doesn't look like this, I'd assume the migration failed.

[jeffsikes]

Hey there.

I'm not seeing any exceptions in any of the logs being stored at /storage/logs at the time of the upgrade related to SAML.

The table structure is exactly the same as you mentioned above.

I'm ok without a resolution, as I think you're right - it's a unique migration issue that probably only affects a few scenarios. And there is a workaround.

I just wanted my explanation to be out there to assist anyone else that gets caught in this situation moving forward. Hopefully no one!

[dsmrt]

Of course! I always want you and others to let me know when something like this happens. I'll be keeping a close eye on this and see if anyone else sees an issue like this. It's always a little nerve-racking adding migrations so I avoid those at all costs but things do pop up.

Thanks for sharing the issue. At least if someone else has the same thing happen they can come here for more help. I'll leave this open for a little bit and close it in a month or two, if nothing else comes from it.

Thanks!

[dsmrt]

Haven't heard anything from this in a while. I'm going to close this for now but let me know if have any issues.