search

flipboxfactory / saml-sp

SAML Service Provider (SP) Plugin for Craft CMS
https://saml-sp.flipboxfactory.com/
Other
19 stars 5 forks source link

SP Login controller loads disabled IdP's by EntityId #175

Closed 7ochem closed 2 years ago

7ochem commented 2 years ago

I have 4 IdP's configured with the same EntityId, but the metadata is different (different applications in Azure AD). This is because we have 4 environments: dev, testing, acceptance and production. I have all providers on all environments, but I only enable the one specific for that environment on each environment.

SAML-Identity-Providers-disabled

Upon login, I was send back to Craft CMS and got the error: Unable to validate Signature coming from the SimpleSamlPHP Assertion validator. I have been debugging the whole call stack and finally compared the signing key that was used with the ones in my database and found that the IdP with the lowest database ID (the first it encountered with the same Entity ID) was being used, despite the fact that I've set it to disabled...

I expect the plugin to only query IdP's that are enabled.

https://github.com/flipboxfactory/saml-sp/blob/935ee918b21a7c679347ebd30e364d546ab685db/src/controllers/LoginController.php#L79-L84 Either the login controller should call (a new method) ->findByEntityIdAndEnabled() or something along that line. Or the ->findByEntityId() method should add "enabled => 1" to the query, but that might have side effects on other places where it is being used.

I'm on flipboxfactory/saml-sp v 2.7.4

dsmrt commented 2 years ago

👋 @7ochem

Good catch here ...

I added this in 2.7.5 which looks like this: https://github.com/flipboxfactory/saml-sp/blob/b874186a9cd15a9a68830a19ec91b1e5fbbc7d65/src/controllers/LoginController.php#L80-L84

Let me know if you have any issues with this.

7ochem commented 2 years ago

Wow, thank you for this ultra fast response! 🚀

Your fixed has worked. I can now have several IdP's on disabled and it takes the enabled one correctly. Thanks! 👍🏻