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

Add support for including extra_chain_cert with p12 #802

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

nparker-tc
Copy link

Summary of feature

We ran into an integration where the partner required the full certificate chain included with SSL requests. Adding this support in HTTParty allows the option to include the full certificate chain utilizing the existing data from the p12 certificate. This feature has been included in Net::HTTP since Ruby 3.0. I have added a safety check to prevent compatibility issues to ensure it is only accessible when the extra_chain_cert= method is available on the Net::HTTP instance.

Techinical Details

  • Added option to determine if the full certificate chain should be included. Defaults to false
  • Add support in connection_adapter to include the full certificate chain if the option is enabled, p12 is being used, and Net::HTTP supports including the certificate chain
  • Added specs for both new methods

@jnunemaker
Copy link
Owner

Looks good. I've not used this and know nothing about it but I'll merge after CI passes.

@nparker-tc
Copy link
Author

@jnunemaker Wow thanks for the quick response! I will re-try locally the 2.7 build and get it fixed. Appreciate it!

@nparker-tc
Copy link
Author

@jnunemaker

The previous issue was with the way the specs were written. It would run the expectation regardless of whether it was supported on that ruby version or not. In the new specs, both scenarios are checked for the supported and unsupported ruby versions and validate the behavior for both.

Ruby 3.0 build:

image

Ruby 2.7.8 build:

image

@jnunemaker
Copy link
Owner

@nparker-tc 2.7 is failing. Any ideas why? I didn't look closely.

@nparker-tc
Copy link
Author

nparker-tc commented Jul 17, 2024

@nparker-tc 2.7 is failing. Any ideas why? I didn't look closely.

@jnunemaker Is it still? I think it is currently awaiting on a workflow approval for a new build with the updated specs. It failed previously based on the way the specs were written.
More details are in this comment: #802 (comment)
Fix is in this commit: a636119
Workflow pending: https://github.com/jnunemaker/httparty/actions/runs/9962038185

@nparker-tc
Copy link
Author

@jnunemaker Please let me know if you have any questions on this. It should be good to go

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