Skip to content

[feat] Thread safe configurator#26

Merged
Zogoo merged 18 commits intomasterfrom
feature/thread_safe_config
Jan 22, 2025
Merged

[feat] Thread safe configurator#26
Zogoo merged 18 commits intomasterfrom
feature/thread_safe_config

Conversation

@Zogoo
Copy link
Owner

@Zogoo Zogoo commented Dec 12, 2024

This PR removes public and private key dependency from low-layer logic. This will allow the application to directly pass the certificates to the signing logic instead of using a config that is shared between threads.

At the controller level, I am still passing config values if the value is not provided, which makes it safer for existing gem users.

Additionally, I'm fixing Ruby syntax and hash arguments to make it developer-friendly.

This PR will revert the Proc approach for setting the certificate and private key because I think those approaches are basically trying to achieve the same thing. They need to set up their certificate for each request (not persist the entire app).

@Zogoo Zogoo self-assigned this Dec 12, 2024
@Zogoo Zogoo added the enhancement New feature or request label Jan 20, 2025
Zogoo and others added 2 commits January 21, 2025 18:29
* Squash commits for saml_idp gem

* [feat] Allow SP config force signature validation (#16)

* Allow SP config force signature validation

* Allow SP config force signature validation

Tested with Slack with Authn request signature option
---------

Co-authored-by: zogoo <ch.tsogbadrakh@gmail.com>

* [feat] Don’t ignore certificates without usage (#17)

I have tested with live SAML SP apps and it works fine

* Unspecified certifciate from SP metadata

---------

Co-authored-by: zogoo <ch.tsogbadrakh@gmail.com>

* Try with proper way to update helper method (#19)

* Set minimum test coverage (saml-idp#207)

* Set minimum test coverage to a very high value for testing

* Update minimum coverage to actual current value

* Try with proper way to update helper method

* Correctly decode and mock with correct REXML class

* Drop the min coverage

---------

Co-authored-by: Mathieu Jobin <majobin@mdsol.com>
Co-authored-by: zogoo <ch.tsogbadrakh@gmail.com>

* [feat] Collect request validation errors (#18)

* wip add error collector

* Fix type and rewrite request with proper validation test cases

* Lead error render decision to gem user

* Validate the certificate's existence before verifying the signature.

---------

Co-authored-by: zogoo <ch.tsogbadrakh@gmail.com>

* Support lowercase percent-encoded sequences for URL encoding (#20)

Co-authored-by: zogoo <ch.tsogbadrakh@gmail.com>

* [fix] Gem CI updates for latest versions (#22)

* Remove duplications

* Pre-conditions need to be defined in before section

* Le's not test logger in here

---------

Co-authored-by: zogoo <ch.tsogbadrakh@gmail.com>

* [fix] Allow IdP set reference ID for SAML response (#21)

* Pass ref id as Session Index

* Official Rails 8 is not released yet to RubyGem until that let's stick official older version

---------

Co-authored-by: zogoo <ch.tsogbadrakh@gmail.com>

* Support rails 8 for dev env (#23)

Co-authored-by: zogoo <ch.tsogbadrakh@gmail.com>

* Assertion flag should able switchable by application (#24)

Co-authored-by: zogoo <ch.tsogbadrakh@gmail.com>

* concurrent-ruby v1.3.5 has removed the dependency on logger

---------

Co-authored-by: zogoo <ch.tsogbadrakh@gmail.com>
Co-authored-by: Mathieu Jobin <majobin@mdsol.com>
@Zogoo Zogoo merged commit bda1269 into master Jan 22, 2025
25 checks passed
@hamaron
Copy link

hamaron commented Jan 23, 2025

Great. Will this be merged into saml-idp/saml_idp?

@Zogoo
Copy link
Owner Author

Zogoo commented Jan 24, 2025

@hamaron, it's nice to see you here. It’s not merged yet. I would like to test the behavior of my project, and if everything works well, let me merge it with the "saml_idp" side. Meanwhile, if you could also test it somewhere, that would be a great help for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants