SAML-Toolkits / ruby-saml

SAML SSO for Ruby
MIT License
924 stars 566 forks source link

Allow SP certificates to be OpenSSL::X509::Certificate #726

Open tobiasamft opened 1 month ago

tobiasamft commented 1 month ago

This allows settings to accept instances of OpenSSL::X509::Certificate as service provider (SP) certificates.

Solves #723

Version 1.16.0 was, at least partially, able to handle OpenSSL::X509::Certificate as input for settings.certificate (e.g. when using OneLogin::RubySaml::Response).

Since settings.get_sp_certs is the only interface that is used to access certificates, it should be enough to test that interface with instances of OpenSSL::X509::Certificate. There are 3 ways to insert certs, all of them have been tested:

Note that both deprecated interfaces settings.get_sp_cert and settings.get_sp_cert_new use settings.get_sp_certs internally. Thus, they are covered as well.

Same approach could be used for SP private key to accept OpenSSL::PKey. Maybe it's a good idea to make all certificates from settings attr_writer for public and attr_accessor for private access to ensure that certs are accessed via settings.get_sp_certs only (but that would break current interface).

johnnyshields commented 1 month ago

@tobiasamft can you check if this is solved on the v2.x branch? I think it might be already. If it is, we can close this PR b/c we are releasing v2.x soon.

tobiasamft commented 1 month ago

@johnnyshields unfortunately v2.x does not solve this. Using OpenSSL::X509::Certificate as SP certificate still crashes with the following: git/ruby-saml/lib/ruby_saml/settings.rb:377:in 'validate_sp_certs_params!': undefined method 'empty?' for an instance of OpenSSL::X509::Certificate (NoMethodError)

johnnyshields commented 1 month ago

ok. Can you raise the PR to the v2.x branch then please? I will review it.

tobiasamft commented 1 month ago

@johnnyshields I've rebased the branch onto v2.x

johnnyshields commented 1 month ago

@tobiasamft see comment