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

[READY] v2.0 - Consistently format cert/private key PEMs #711

Open
wants to merge 2 commits into
base: v2.x
Choose a base branch
from

Conversation

johnnyshields
Copy link
Collaborator

@johnnyshields johnnyshields commented Jul 11, 2024

Fixes #636

I've tried to make this as compatible as possible with the legacy behavior while still handling the case stripping out text before/after the PEM string.

All existing test cases behave as before (* see comment below--there's one case that needed a minor alteration as the case itself was written incorrectly.)


module RubySaml

# SAML2 Auxiliary class
#
class Utils
module Utils
extend self
Copy link
Collaborator Author

@johnnyshields johnnyshields Jul 11, 2024

Choose a reason for hiding this comment

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

Converted Utils to module. It was a singleton previously so this shouldn't break anything.

settings.certificate = ruby_saml_cert_text
wrong_private_key = ruby_saml_key_text.sub!('A', 'B')
wrong_private_key = ruby_saml_key_text.sub!('Z', 'X')
Copy link
Collaborator Author

@johnnyshields johnnyshields Jul 11, 2024

Choose a reason for hiding this comment

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

This spec was kind of silly before, previously ruby_saml_key_text.sub!('A', 'B') was gsubbing -----RSA PRIVATE KEY----- to RSB, and not actually affecting the pkey body. I've improved it so we are now testing for corrupted key body.

My new logic will actually tolerate the case of ----BEGIN RSB PRIVATE KEY----- and just normalize it to -----BEGIN PRIVATE KEY-----, so the old test actually passes now.

@johnnyshields johnnyshields force-pushed the pem-formatter branch 2 times, most recently from 2f319bf to 93b063a Compare July 11, 2024 17:20
@johnnyshields johnnyshields changed the title v2.0 - Consistently format cert/private key PEMs [PLEASE REVIEW BUT DO NOT MERGE] v2.0 - Consistently format cert/private key PEMs Jul 11, 2024
@johnnyshields johnnyshields force-pushed the pem-formatter branch 7 times, most recently from 3694dea to 822da55 Compare July 12, 2024 20:20
@johnnyshields johnnyshields changed the title [PLEASE REVIEW BUT DO NOT MERGE] v2.0 - Consistently format cert/private key PEMs [READY] v2.0 - Consistently format cert/private key PEMs Jul 12, 2024
…y PEM values, including the `RubySaml::Util#format_cert` and `#format_private_key` methods. Introduces new `RubySaml::PemFormatter` module.
@johnnyshields
Copy link
Collaborator Author

@pitbulk this is ready for your review and merge.

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.

1 participant