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 extra headers option to enhance HTTP requests #8030

Closed
wants to merge 3 commits into from

Conversation

amancevice
Copy link

Users can supply --extra-headers='{...}' option to pip commands that enhances the PipSession object with custom headers.

This enables use of private PyPI servers that use token-based authentication.

Example:

pip install \
  --extra-headers='{"Authorization": "..."}' \
  --index-url https://secure.pypi.example.com/simple \
  --trusted-host secure.pypi.example.com \
  fizz==1.2.3

Users can supply --extra-headers='{...}' option to pip commands that enhances the
PipSession object with custom headers.

This enables use of private PyPI servers that use token-based authentication.

Example:

```
pip install \
  --extra-headers='{"Authorization": "..."}' \
  --index-url https://secure.pypi.example.com/simple \
  --trusted-host secure.pypi.example.com \
  fizz==1.2.3
```
tests/unit/test_network_session.py Show resolved Hide resolved
@@ -335,6 +335,17 @@ def exists_action():
) # type: Callable[..., Option]


def extra_headers():
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to add it to the install and download (and to probably more ones that I don't remember) CLI. You should add it as a req file option

Copy link
Author

Choose a reason for hiding this comment

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

thanks for the hints, I wasn't quite sure where to drop this code.

Copy link
Author

Choose a reason for hiding this comment

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

Would it be okay to put it in the general_group?

Copy link
Author

Choose a reason for hiding this comment

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

Forgive my ignorance, but what do you mean "You should add it as a req file option"?

Copy link
Contributor

Choose a reason for hiding this comment

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

For the req file- see SUPPORTED_VERSIONS in req_file.py

About the general- it does not makes sense as there are pip commands which do not make any requests at all, you can add to install cmd, download cmd, etc...

src/pip/_internal/cli/req_command.py Outdated Show resolved Hide resolved
src/pip/_internal/cli/req_command.py Outdated Show resolved Hide resolved
@uranusjr
Copy link
Member

I like the idea, but definitely don’t agree with the syntax. A syntax more similar to cURL’s -H flag would make much more sense.

@sbidoul
Copy link
Member

sbidoul commented Apr 13, 2020

Does this create a risk of sending extra headers to unwanted hosts? I'm thinking of cases where there are multiple indexes, --find-links, direct urls. Should there be a mechanism to associate additional headers to specific hosts?

@amancevice
Copy link
Author

I like the idea, but definitely don’t agree with the syntax. A syntax more similar to cURL’s -H flag would make much more sense.

I can update the syntax to be more like cURL's for sure. I may need guidance on how to add it to the CLI, but I'll take a whack at it.

@amancevice
Copy link
Author

amancevice commented Apr 13, 2020

Does this create a risk of sending extra headers to unwanted hosts? I'm thinking of cases where there are multiple indexes, --find-links, direct urls. Should there be a mechanism to associate additional headers to specific hosts?

Yes that is probably a valid concern—I'm not an expert on pip's internals, so I'm not sure how safeguarding against that would work. Would restricting --extra-headers or -H options (as proposed above) to only acting on --extra-index-urls be a path forward?

In my use case, I have some code that sets up a private PyPI index on AWS that uses API Gateway to front an S3 bucket, where I keep my pips. I can secure it with some custom code to implement Basic Auth, but I want to use AWS' built-in Cognito. Of course in order to do that I'd need to pass an Authorizer token in the header.

Since I expect to be using a combination of public and private pips, I might set up a requirements.txt file with all my project's public dependencies and a requirements-private.txt with all the project's private dependencies. In my setup scripts I might then have something like:

pip install -r requirements.txt
pip install -r requirements-private.txt \
  --index-url https://pypi.mine.com/simple ]
  [header options] \
  [and maybe even --no-deps]

And that way I'm only ever sending the auth token to my private index (I think!).

@pfmoore
Copy link
Member

pfmoore commented Apr 13, 2020

One other option to address this use case would be to use a local unauthenticated proxy to the authenticated backend. Having to start the proxy before using pip is inconvenient, conceded, but it gives you much greater control over precisely what you send, as well as likely supporting a much greater range of situations.

You could even write a small wrapper script:

  • Start the proxy
  • pip --extra-index-url=proxy
  • Stop the proxy

if you don't want the overhead/admin/risk of having a proxy running any longer than necessary.

Having said all of this, I don't object to the original idea, I just wonder how complex it could become (different headers for different indexes, etc.)

@amancevice
Copy link
Author

A companion option like --trusted-hosts (or even trusted hosts itself) could be used to discriminate which hosts get the header(s) as well. Would that be a sufficient way to keep complexity down?

@pradyunsg pradyunsg added state: needs discussion This needs some more discussion type: feature request Request for a new feature labels Apr 13, 2020
@pradyunsg
Copy link
Member

puts on release manager hat

To set expectations early, this is definitely not going to be on pip 20.1 release, since there's definitely a lot of discussion needed here and this is basically a PR for adding a new feature to pip.

takes off release manager hat

A companion option like --trusted-hosts (or even trusted hosts itself) could be used to discriminate which hosts get the header(s) as well. Would that be a sufficient way to keep complexity down?

What about hosts that want different headers, due to different Authorization on different index servers? I'm not suggesting this to bog down this PR, but because I anticipate that feature request coming in the future.

@amancevice
Copy link
Author

@pradyunsg that sounds perfectly reasonable. I think all the feedback on is PR is definitely warranted & I appreciate the discussion!

Maybe I should close this PR and open an issue where different implementation options can be discussed. Sound good?

@pradyunsg
Copy link
Member

That sounds great!

@amancevice
Copy link
Author

Pushed discussion to #8042

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label May 20, 2020
@lock lock bot locked as resolved and limited conversation to collaborators May 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation state: needs discussion This needs some more discussion type: feature request Request for a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants