Added trusted repository option

[maayanbar13]

Upstream fix for python-poetry/poetry#2912.

Resolves upstream issue python-poetry/poetry#1556.

Pull Request Check List

• Updated documentation for changed code.
• Added tests for changed code.

Both not applicable, docs and tests are added in upstream PR.

[tomgrin10]

@abn Is there a reason this is not getting merged?

[absassi]

Why is the option added to the schema of pyproject.toml? How to validate TLS connection is already part of the user configuration (certificates.<repo>.cert), it does not make sense to define one option in each place. Also, some users might prefer to validate their connections, while other don't want it for all sorts of reasons.

At least, I think this deserves more discussion before merging (otherwise it might have to be changed again, causing yet more breaking changes regarding certificates, like when certificates config was created, breaking the previous configuration options).

[maayanbar13]

Why is the option added to the schema of pyproject.toml? How to validate TLS connection is already part of the user configuration (certificates.<repo>.cert), it does not make sense to define one option in each place.

I don't agree that this option defines how to validate the TLS connection but rather if.
It woudn't make a lot of sense to define that you don't want to use TLS inside a .cert file...
(And I think it would make sense to disallow using HTTP repositories unless this option is enabled.)

Also, some users might prefer to validate their connections, while other don't want it for all sorts of reasons.

While I can see why this might be an interesting feature, the main usage of this option is in scenarios where you use a self-hosted private repository and can't be bothered setting up certificates (usually in some sort of enterprise environment).
In that case all the users will want to have the same option (TLS disabled).

At least, I think this deserves more discussion before merging (otherwise it might have to be changed again, causing yet more breaking changes regarding certificates, like when certificates config was created, breaking the previous configuration options).

I don't agree that this is a breaking change: this option field is optional, and could default to the current behavior.
If, down the line, the decision will be made to make this option user-specific, it could be implemented in a way that overrides the current option instead of replacing it. (I.E both options available, with the user-specific one having priority.)

[absassi]

I don't agree that this option defines how to validate the TLS connection but rather if.
It woudn't make a lot of sense to define that you don't want to use TLS inside a .cert file...
(And I think it would make sense to disallow using HTTP repositories unless this option is enabled.)

@maayanbar13 sorry if I wasn't clear, I don't mean to define it in the certificate file, but in a config option like certificates.<repo>.trusted = true. This way, all options related to TLS validation (i.e., the option of if and the option of how) are grouped together.

It is also makes more sense to me there than in pyproject.toml for privately hosted repos, because it is something that is related to the repository, not the project.

Obviously, having both options available would be fine too.

[maayanbar13]

Obviously, having both options available would be fine too.

Sure, that makes sense.
I've implemented said feature in the downstream PR, and its pending the original author to merge it.

That feature is unrelated to the poetry-core repo, so I think PR should be merged.
Waiting on @abn 's response :)

[maayanbar13]

@abn @absassi Is there any reason this is not getting merged?

[mathbou]

Is there any reason this PR is still pending after all this time?

[therewillbeblood]

@maayanbar13 can you please solve merge conflicts and push again ?

[maayanbar13]

@therewillbeblood Sure :)

[therewillbeblood]

@abn it seems you are assigned as a reviewer, would you have an ETA to share to us when you could do the review ?

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
1.5% Duplication

[abn]

First off; apologies for not getting to this earlier.

Second, I think (along the lines of what @absassi was suggesting) that this should be a poetry level option for repository that ends up being configured as poetry config certificates.foo.verify False. Not something that lands in pyproject.toml but rather user specfic config for hte repository. The reason for this is partly consistency, and partly security. Wheter or not a host's certificate is verified is upto the end user and not the project, you do not want the insecure behaviour to be default across everyone.

For the above reason, I am closing this issue. But will review any downstream PR for the new verify option alongside current certificate configurations.