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

Parse SLO endpoint as array from SP metadata #184

Open
wants to merge 6 commits into
base: v1.0.0-saml_lib
Choose a base branch
from

Conversation

Zogoo
Copy link
Collaborator

@Zogoo Zogoo commented Jul 16, 2022

The current method for accessing the SLO (Single Logout) endpoint is user-friendly. Because the Service Provider (SP) metadata contains multiple SLO endpoints with different bindings, the Identity Provider (IdP) should respond based on the specified binding.
Therefore, I believe it would be better to enable access using keywords such as "binding."

Current way.

(ruby) sp_config.single_logout_services
{"HTTP-Redirect"=>"http://secfense.example.com/logout"}
(ruby) sp_config.single_logout_services
{"HTTP-Redirect"=>"http://secfense.example.com/logout"}
(ruby) sp_config.single_logout_services.keys
["HTTP-Redirect"]
(ruby) sp_config.single_logout_services.values
["http://secfense.example.com/logout"]

From now

(ruby) sp_config.single_logout_services.first[:binding]

Example:

# IdP initiated SLO request.
If sp_config.single_logout_services.first[:binding] == "HTTP-Redirect"
    redirect_to sp_config.single_logout_services.first[:localtion] + {"SAMLResponse: slo_request}
end

@Zogoo Zogoo changed the title Feature/update slo endpoint parser Parse SLO endpoint as array from SP metadata Jul 16, 2022
Copy link
Collaborator

@jphenow jphenow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar comment to another PR; this seems like enough of a change to expected APIs that it might warrant putting into a new major version release.

@mjobin-mdsol
Copy link
Collaborator

seems like a breaking change. maybe we should first cut a release with a deprecation warning. then bump the version.
I don't now how much this method use used and expected to be a hash...

@Zogoo Zogoo marked this pull request as draft April 9, 2023 21:59
@Zogoo Zogoo changed the base branch from master to v0.2.0-saml-lib January 3, 2024 18:16
@Zogoo Zogoo added the breaking-change Changes should be treated with major version label Jan 3, 2024
@Zogoo Zogoo self-assigned this Jan 3, 2024
@Zogoo Zogoo marked this pull request as ready for review January 3, 2024 21:23
@Zogoo
Copy link
Collaborator Author

Zogoo commented Jan 3, 2024

@jphenow, @mjobin-mdsol I have changed the destination to the v0.2.0 branch.
I'm not familiar with maintaining multiple versions of a gem. However, I suggest we leave v0.15.XX as the older version, which only fixes bugs. What do you guys think about it?

@Zogoo Zogoo added this to the Simplify gem for SAML milestone Jan 4, 2024
@mjobin-mdsol
Copy link
Collaborator

@Zogoo

v0.2.0 appears to be below v0.15+
Maybe you meant to be v1.0 and v2.0 but after reviewing those changes again.
I don't have any objection targeting v0.17.0 or cutting v1.0.0 with those changes in.

@Zogoo Zogoo changed the base branch from v0.2.0-saml-lib to v1.0.0-saml_lib November 1, 2024 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Changes should be treated with major version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants