Skip to content

Make the library secure agen - #83

Merged
mpdavis merged 1 commit into
mpdavis:masterfrom
pvizeli:patch-1
Mar 6, 2018
Merged

Make the library secure agen#83
mpdavis merged 1 commit into
mpdavis:masterfrom
pvizeli:patch-1

Conversation

@pvizeli

@pvizelipvizeli commented Mar 3, 2018

Copy link
Copy Markdown
Contributor

@codecov-io

codecov-io commented Mar 3, 2018

Copy link
Copy Markdown

Codecov Report

Merging #83 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@ Coverage Diff @@## master #83 +/- ##
=======================================
Coverage 93.69% 93.69% =======================================
Files 12 12 Lines 841 841 =======================================
Hits 788 788 Misses 53 53

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 28cc671...b08a58f. Read the comment docs.

@mpdavis

Copy link
Copy Markdown
Owner

lgtm

@mpdavis
mpdavis merged commit c97f18a into mpdavis:masterMar 6, 2018
@zejn

zejn commented Mar 6, 2018

Copy link
Copy Markdown
Collaborator

Hi,

I don't think having a hard requirement on pycryptodome is ok, if you want to use pycrypto on GAE, because pycryptodome installs in the same namespace as pycrypto.

The only viable solution I currently see (if support for old GAE is still an option) is having a pure-Python RSA signing, with optional C backends, which the developers should choose themselves by specifying python-jose[cryptography] or python-jose[pycryptodome] or python[pycrypto]. Pycrypto and pycryptodome are mutually exclusive since they install in the same namespace, and cryptography might not be an option if installing on GAE.

@mpdavis

Copy link
Copy Markdown
Owner

@zejn I believe you are correct.

I will prioritze your pure python RSA backend, and remove the requirement entirely, allowing for the override you describe.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@pvizeli@codecov-io@mpdavis@zejn