search

OCA / server-auth

https://odoo-community.org/psc-teams/tools-30
GNU Affero General Public License v3.0
146 stars 399 forks source link

[15.0][FIX] auth_saml: do not force using vulnerable cryptography module #640

Open vincent-hatakeyama opened 3 months ago

vincent-hatakeyama commented 3 months ago

Without this fix, it is impossible to use an upgraded version of cryptography with this module.

It is still necessary to check for compatibility, so that information is added to the install instructions.

vincent-hatakeyama commented 3 months ago

/ocabot merge patch

OCA-git-bot commented 3 months ago

Sorry @vincent-hatakeyama you are not allowed to merge.

To do so you must either have push permissions on the repository, or be a declared maintainer of all modified addons.

If you wish to adopt an addon and become it's maintainer, open a pull request to add your GitHub login to the maintainers key of its manifest.

thomaspaulb commented 1 month ago

/ocabot rebase

OCA-git-bot commented 1 month ago

@thomaspaulb The rebase process failed, because command git push --force xcgd tmp-pr-640:15.0-auth_saml-fix-cryptography_version failed with output:

remote: Permission to xcgd/server-auth.git denied to OCA-git-bot.
fatal: unable to access 'https://github.com/xcgd/server-auth/': The requested URL returned error: 403
thomaspaulb commented 1 month ago

@vincent-hatakeyama Could you rebase or allow access

vincent-hatakeyama commented 1 month ago

@vincent-hatakeyama Could you rebase or allow access

I’ve juste rebased.

thomaspaulb commented 1 month ago

@sbidoul I need your input here.. this goes against this. Is test-requirements.txt OK to edit directly or is there some setup.py magic that we should do to override the module dependency for the test, but not for the module itself?

Or, come to think of it, could we override it there, to let the requirements.txt include the limit but to leave the module's manifest free for users of the module to decide?

sbidoul commented 1 month ago

I agree we should not place an upper bound on cryptography in the module. My initial approach was not optimal. This PR is better.

Last time I checked, the pin on pyopenssl 19 on Odoo's requirements.txt was not really necassary? So another approach could be putting a minimum pyopenssl version in auth_saml2's external dependencies. I think we have done that somewhere, but I can't find it right now.

Longer term, I would like to try and stop using Odoo's requirements.txt in oca/oca-ci, in favor of a custom set of upper bounds on dependencies that are known not to work with Odoo (things like xlrd<2, etc...).