Allow appending git username / password to dependency

[setu4993]

Resolves: python-poetry/poetry#2062, python-poetry/poetry#2348. Builds on top of #96.

Changes in #96 (copied from here):

This PR allows appending deployment keys to usernames as used by gitlab.

Furthermore a new property is_unsafe is introduced for ParsedUrl, which can be used for cli command like poetry add to easily return whether the git dependency contains a password.

According to gitlab's docs a + is allowed in usernames. This is fixed as well.

Fixes: python-poetry/poetry#2062

(This was initially submitted to the poetry repository: python-poetry/poetry#2169)

Changes I made on top of @finswimmer's work in #96 (rebased to abe9fe5):

• Add tests for when x-oauth-basic GitHub URL is used in package link (Add Personal Access Token Authentication Support poetry#2348).

• Add warning and log for when a dependency stores a password in plain text by using the is_unsafe property (based on discussion in allow appending deployment key to username (#2062) poetry#2169 (comment)).

• Added tests for changed code.

• Updated documentation for changed code. NA

Hoping this is at a good enough point to review and consider merging into main, to be included for a soon-ish release.

[setu4993]

@python-poetry/triage: This PR is ready for review.

[slavaatsig]

So sad it’s not yet available. We recently decided to switch GitHub and Poetry but now realized we cannot use the personal access token to add dependencies from private repositories so migration to Poetry is postponned until it will be supported.
😞

[setu4993]

Thanks for requesting a review from the team. Looking forward to the feedback so we can move to get this fixed and into poetry.

[setu4993]

@finswimmer, @python-poetry/triage: Any updates on this? I'll rebase to the latest master to resolve conflicts later today, but would be good to get a review on this so that this can be merged in for a future release.

[setu4993]

@python-poetry/triage, @finswimmer, @abn, @sdispater : Noticed @jedie's review here, and rebased to the latest master. Can I please get some feedback here? This PR is complete from the changes and test coverage perspective.

I also see that the workflow output requires approval, can one of you please help with that as well?

[betaboon]

it looks like this has been reviewed, approved and is working. can we get this merged ?

[jimmywan]

@setu4993 Does this support passing username/personal access token as env variables? I'd like to use this, but it seems like I'd have to hardcode my creds into the file.

[setu4993]

@setu4993 Does this support passing username/personal access token as env variables? I'd like to use this, but it seems like I'd have to hardcode my creds into the file.

@jimmywan : Good question, I don't think that is possible in pyproject.toml with poetry generally yet. See python-poetry/poetry#208. It works with custom repositories, but I don't think with git dependencies, though.

[steven-dicristofaro]

Reopening this thread - now that this has happened, I cannot get poetry to work with my private dependencies. I receive the following error:

Updating dependencies
Resolving dependencies... (5.2s)

Writing lock file

  AttributeError

  'EmptyConstraint' object has no attribute 'allows'

  at ~/.poetry/lib/poetry/_vendor/py3.8/poetry/core/version/markers.py:291 in validate
      287│ 
      288│         if self._name not in environment:
      289│             return True
      290│ 
    → 291│         return self._constraint.allows(self._parser(environment[self._name]))
      292│ 
      293│     def without_extras(self):  # type: () -> MarkerTypes
      294│         return self.exclude("extra")
      295│ 

Please advise on any updates to support PATs in poetry; my team is currently blocked by this not existing.

[finswimmer]

@steven-dicristofaro: The error you receive had probably nothing to do with this PR/linked issue. It's related to python-poetry/poetry#4402 which is already fixed in poetry 1.1.8.

The reasons why we haven't merge this PR are massive security concerns. Providing username and password would mean, that the credentials are in plain text in your pyproject.toml, poetry.lock file and in the sdist or wheel if you build one. pyproject.toml and poetry.lock will be checked in into git, the packages might end up in a (private) pypi repository. If you install the package the credential are available in the site-packages folder. TL;DR: It's very easy to loose control over your credentials. It's sometimes ok if a program allows the users to shoot themselves into the food, but should it allow them to should themselves into the head?

[setu4993]

@finswimmer : Thanks for responding and engaging (albeit after a long time).

I agree with your concerns, but I disagree with your conclusion. The choice for tools shouldn't be between choosing to support an existing feature and encouraging better hygiene.

poetry not supporting PATs doesn't mean they won't be used. pip supports them and since they are one of the few ways to use private packages straight from GitHub (and likely GitLab) repos, so they'll continue to be used. However, poetry not supporting PATs does mean that for people and teams that rely on PATs, they cannot reliably switch to using poetry because a critical functionality that is required is not supported.

Most people and teams understand the drawbacks of checking in credentials, however, it is sometimes necessary. When I first responded on python-poetry/poetry#2348 and created this PR, it was the only choice we had for hosting private packages, and we used PATs in requirements.txt, knowing completely well what it meant. Now, we've transitioned out of using PATs but I understand why someone might need to and acknowledge that.

Should a tool warn and heavily discourage this behavior? Yes. A 100 times yes. But should a tool that builds on top of another package manager not support functionality that is already supported by the default package manager? I think no. By focusing on this one aspect, it keeps all of the other benefits that poetry affords to potential users.

Choosing to not engage the community and not communicating on an issue that affects users for months at a stretch, sets the wrong standard for a project: It makes users feel unheard and contributors feel unwelcome.

[steven-dicristofaro]

@finswimmer thanks for the quick response - the link you provided solved my problem (some versioning nonsense with an update to sqlalchemy that happened this week).

I do also share your concerns about keeping control over creds, but I also agree with @setu4993 in that it would be nice to have the option of using PAT-based auth (with a profuse set of warnings to the client) to at least give users the comfortability of knowing that poetry supports (or is at least attempting to support) all functions 1-1 of the underlying pip layer.

Either way, I am now unblocked so many thanks for that 😄

[finswimmer]

At the end @sdispater has to decide which way poetry should follow here, because it's a matter of philosophy. I'm fine with or without supporting username/passwords.

to at least give users the comfortability of knowing that poetry supports (or is at least attempting to support) all functions 1-1 of the underlying pip layer.

There is only a loose coupling to pip. At the moment pip is only used for the final installation step and even this will be replaced by a generic installation method in the future, meaning that poetry will be "pip-free". Also it is not the goal by poetry to provide all functions available by pip and/or setuptools. We decide feature-by-feature.

[jkgenser]

@steven-dicristofaro: The error you receive had probably nothing to do with this PR/linked issue. It's related to python-poetry/poetry#4402 which is already fixed in poetry 1.1.8.

The reasons why we haven't merge this PR are massive security concerns. Providing username and password would mean, that the credentials are in plain text in your pyproject.toml, poetry.lock file and in the sdist or wheel if you build one. pyproject.toml and poetry.lock will be checked in into git, the packages might end up in a (private) pypi repository. If you install the package the credential are available in the site-packages folder. TL;DR: It's very easy to loose control over your credentials. It's sometimes ok if a program allows the users to shoot themselves into the food, but should it allow them to should themselves into the head?

Would they be in the poetry.lock file if they are used like the following?"

"https://${GITHUB_ACCESS_TOKEN}@github.com/organization/repo.git"