Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v2.0: Settings params deprecations #699

Merged

Conversation

johnnyshields
Copy link
Collaborator

Deprecate RubySaml#Settings params and alias them to the new ones. All these deprecations were declared in CHANGELOG in earlier versions already. This does not affect any existing behavior.

  • issuer --> sp_entity_id
  • idp_sso_target_url --> idp_sso_service_url
  • idp_slo_target_url --> idp_slo_service_url
  • assertion_consumer_logout_service_url --> single_logout_service_url
  • assertion_consumer_logout_service_binding --> single_logout_service_binding

…d alias them to the new ones. All these deprecations were declared in CHANGELOG in earlier versions already.

- issuer --> sp_entity_id
- idp_sso_target_url --> idp_sso_service_url
- idp_slo_target_url --> idp_slo_service_url
- assertion_consumer_logout_service_url --> single_logout_service_url
- assertion_consumer_logout_service_binding --> single_logout_service_binding
@johnnyshields
Copy link
Collaborator Author

@pitbulk ready for review and merge.

@pitbulk
Copy link
Collaborator

pitbulk commented Jul 9, 2024

Merging this one, but we need to be careful when we move to 2.1

Maintaining clean code is nice, but it's not cool if you break several projects while doing this.

Suppose you execute a simple search on Github. In that case, you will find that tons of projects still use settings.issuer, so imagine all those projects using ruby-saml in real production environments that added SAML support years ago (when this parameter was in use). I'm pretty sure that is possible that the developer that made the integration could not be anymore in the company. Nobody going to be aware of the change that is required or will not notice the need for the change until the integration breaks.

@pitbulk pitbulk merged commit 3229214 into SAML-Toolkits:v2.x Jul 9, 2024
16 of 17 checks passed
@johnnyshields johnnyshields deleted the settings-params-deprecations branch July 9, 2024 17:35
@johnnyshields
Copy link
Collaborator Author

johnnyshields commented Jul 9, 2024

we need to be careful when we move to 2.1

In version 2.1.0 I am thinking we should keep the methods but raise a hard RuntimeError Settings#issuer parameter has been removed in RubySaml v2.1.0 and migrated to Settings#sp_entity_id. Please downgrade to RubySaml v2.0.0 and fix all deprecation warnings before upgrading to v2.1.0.

It is very likely users will see this error at application boot time, or in their tests. If they don't see it, they are doing something massively wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants