Add support for custom certificate authority and client certificates#1325
Add support for custom certificate authority and client certificates#1325sdispater merged 9 commits intopython-poetry:masterfrom Caligatio:cert-auth
Conversation
Caligatio
changed the title
|
@brycedrennan I realize this is a substantial PR but I just successfully used my branch to both push and pull to a mutual TLS authenticated repository. This unfortunately is a blocker for my major use case of Poetry so please let me know I can help this along. |
|
Thanks for your work on this feature! Unfortunately, we're rather behind on merges so it may be a while till we get to this. |
poetry/console/commands/publish.py
Outdated
| option("username", "u", "The username to access the repository.", flag=False), | ||
| option("password", "p", "The password to access the repository.", flag=False), | ||
| option( | ||
| "custom-ca", |
There was a problem hiding this comment.
minor -- consider using curl or twine's syntax
--cacert, --cert, --key
or
--cert, --client-cert with client-cert being a combined pem
There was a problem hiding this comment.
I probably struggled the most with naming the silly parameters compared to anything else about this PR. I started with custom-ca, hated it, but then figured we could come to consensus in the PR itself :)
Since I'm interfacing with both requests and pip under the hood, I need to use a combined client cert/key file as that's the only thing both take (pip is the picky one). That being said, I liked pip's --client-cert and went with it.
For CA:
- Twine (as you noted) uses
--cacert - pip (as you noted) uses
--cert - cURL uses
--cacert - wget uses
--ca-certificate requestsuses the parameterverify(that's terrible)- Python's built-in
sslmodule uses the parametercafile urllib3uses the parameterca_certs
Since I did --client-cert (with the hyphen in between), I'm thinking it should be consistent and be --ca-<SOMETHING> but that looks weird too. I'm happy to change it to anything that the maintainers will accept; I fully recognize --custom-ca is not great.
There was a problem hiding this comment.
I think we can follow what's already used in both pip and twine, i.e. --cert and --client-cert.
|
I changed the name and was very confident everything would continue working, however, there were major changes to |
|
Checked it today and it both pulls packages using certs and will publish. Confidence restored! |
|
@Caligatio , @brycedrennan, any updates on this PR? Thank you. |
|
I consider it done and am just waiting on it to be merged. |
b0g3r
left a comment
b0g3r
left a comment
There was a problem hiding this comment.
LGTM
Awesome feature, we in team waited a very long time for.
|
@Caligatio Thanks for your contribution! I am ready to merge this to include it in the |
|
Rebased to master and merged in the changes. Should be good to go! |
|
Thanks! |
…ython-poetry#1325) * Add custom certificate authority and client certificate support * Add documentation on certificates * Add cli options to publish for cert and custom ca * Fix requests+pathlib problem and logic hole when BasicAuth+certs are used * Rename custom-ca to cert * Make black happy
…ython-poetry#1325) * Add custom certificate authority and client certificate support * Add documentation on certificates * Add cli options to publish for cert and custom ca * Fix requests+pathlib problem and logic hole when BasicAuth+certs are used * Rename custom-ca to cert * Make black happy
|
Is this released in version 1.0? EDIT: it is, no need to answer 🙂 |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
7 participants
This PR adds support for specifying a custom certificate authority to validate custom repositories as well as using client certificate-based authentication for custom repos. This was quite a bit bigger than I anticipated but I think I plumbed everything that is needed.
This should address #1012 and #297