search

flipboxfactory / saml-sp

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

Breaking Changes in v2.0 (Events) #35

Closed benjaminkohl closed 5 years ago

benjaminkohl commented 5 years ago

I am in the process of updating the plugin from v1.0.8 to v2.0.4 and the updating guide says there are breaking changes when using the Craft/Yii2 events and that the code will need to be refactored but it doesn't describe what has changed and I am not immediately seeing it.

We are using the UserLogin event and from what I can tell, the classes involved haven't changed and neither has the event parameter (at least in terms of how to get the user). What is it that I am not seeing that I need to change as part of this update?

dsmrt commented 5 years ago

The breaking changes are, we have switched from using the LightSaml lib to simplesamlphp. So any references to LightSaml should be removed/refactored. For instance, the Response object that is passed in the EVENT_AFTER_RESPONSE_TO_USER (thru the \flipbox\saml\sp\events\UserLogin event) is now SAML2\Response instead of \LightSaml\Model\Protocol\Response. So working with the object is going to work differently. For examples, take a look at this code here: \flipbox\saml\sp\services\login\User::transform or here.

Generally, all references to LightSaml need to be updated but if you don't have that problem, all should work as usual. I really want to make sure people are diligent about updating. Backup up the db (there are some migrations that run) and test login before pushing to production. If you need any help that you don't feel comfortable sharing here, ping me thru our contact us and we can share some code. I want to make sure this is smooth for you!

dsmrt commented 5 years ago

Also, if you use responseAttributeMap from within the config/saml-sp.php, the parameters have changed. They have changed due to switching to the simplesaml lib. If you can move away from that config and use the GUI in the CP, that'd be ideal. That way it's per IdP and not global.

benjaminkohl commented 5 years ago

Ah, thanks for the information. We weren't doing anything with that Response object in our module's event handler so I guess our code is unaffected by the change. I was thinking that maybe something about registering the event handler specifically had changed.

The site in question hasn't launched yet which is why we wanted to get the update out of the way now and I backed up the DB so we should be pretty safe. Thanks for the help!

dsmrt commented 5 years ago

Great! Let me know how all of that goes, or if you have any feedback.

benjaminkohl commented 5 years ago

It looks like everything went smoothly because the Okta login process still works. Thank you.