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

feat: add ability to pass certs #519

Conversation

NigelGreenway
Copy link
Contributor

A key feature that is missing from what I can tell is the ability to pass a cert/key. This change request is to add a cert/key via configuration based on the host. If it matches that host, then the file path for the certs/keys will be added to the request.

Changes:

  • Add --cert and --key flag with file path to docs
  • Add a configuration setup so the keys are only sent with the corresponding request

A key feature that is missing from what I can tell is the ability to
pass a cert/key. This change request is to add a cert/key via
configuration based on the host. If it matches that host, then the
file path for the certs/keys will be added to the request.

Changes:
 - Add `--cert` and `--key` flag with file path to docs
 - Add a configuration setup so the keys are only sent with the
   corresponding request
@NigelGreenway
Copy link
Contributor Author

I still need to add tests and update the docs, and sort out some code that I've amended in error with formatting. Is this something that you would be interesting in?

Copy link
Contributor

@boltlessengineer boltlessengineer left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution! Yes, we definitely need this feature. I can merge this PR when it is ready.
Please avoid including unnecessary code reformats ;)

@NigelGreenway NigelGreenway marked this pull request as ready for review February 10, 2025 22:25
@NigelGreenway
Copy link
Contributor Author

Please find all comments resolved, and I've added docs and tests.

Linting, formatting and tests are all green 💪

for domain, _ in pairs(config.clients.curl.opts.certificates) do
local target = req.url

local s, _ = string.find(target, domain, 1, true)
Copy link
Contributor

Choose a reason for hiding this comment

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

We already have domain matching functionality in cookie_jar.lua so we can use same logic for both, but for now, I think this is fine. I might add a TODO comment here though.

local function match_cookie(url, cookie)
local req_domain, req_path = utils.parse_url(url)
if not req_domain then
return false
end
local domain_matches = ("." .. req_domain):match(vim.pesc(cookie.domain) .. "$")
local path_matches = req_path:sub(1, #cookie.path) == cookie.path
if domain_matches and path_matches then
logger.debug(
("cookie %s with domain %s and path %s matched to url: %s"):format(
cookie.name,
cookie.domain,
cookie.path,
url
)
)
else
logger.debug(
("cookie %s with domain %s and path %s NOT matched to url: %s"):format(
cookie.name,
cookie.domain,
cookie.path,
url
)
)
end
return domain_matches and path_matches
end

Copy link
Contributor

@boltlessengineer boltlessengineer left a comment

Choose a reason for hiding this comment

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

LGTM. I’ll squash-merge it after adding a commit with some minor fixes.

- add some todo comments
- remove unnecessary `Certificates` type
- add short explanation to new config field
- fix wrong method execution on test code
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