Skip to content
This repository has been archived by the owner on Jan 25, 2018. It is now read-only.

Upgrade to mainline PyJWT (part of bug 1145024) #620

Merged
merged 1 commit into from Mar 30, 2015

Conversation

kumar303
Copy link
Contributor

This switches us from PyJWT-mozilla to PyJWT which
is more maintained and has more features now.
This doesn't introduce any new functionality but
to use the new PyJWT we also needed a mozpay-py upgrade.

@@ -87,7 +96,8 @@ def req_ok(req):
eq_(dd['typ'], payload['typ'])
eq_(dd['response']['price']['amount'], 1)
eq_(dd['response']['price']['currency'], u'USD')
jwt.decode(req['notice'], 'f', verify=True)
jwt.decode(req['notice'], 'f', verify=True,
audience=self.payment_issuer)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a little paranoid so I'd rather see audience='some-apps-public-id' here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think as long as it's different from the default than it should be pretty robust

@mstriemer
Copy link
Contributor

I don't remember why we were using PyJWT-mozilla before but seemingly that has been addressed? Hopefully this fixes the pip install without --no-deps gotcha (or however that happened).

I'd like to the the actual string that's being used in the tests rather than self.payment_issuer in several places but I might be overly paranoid.

I'm not sure if this catches all the places it needs to, and I don't think there are any integration tests here. Is there some way integration tests could be added? (In a separate patch).

@mstriemer
Copy link
Contributor

r+wc

@kumar303
Copy link
Contributor Author

The mozwebQA team has integration tests for the payment flows. When it lands on dev I will check with them.

There never was a great reason for why we used PyJWT-mozilla. We needed some security patches but didn't have luck integrating them upstream so we forked it. However, PyJWT has since got a new maintainer and it is very active now.

This switches us from PyJWT-mozilla to PyJWT which
is more maintained and has more features now.
This doesn't introduce any new functionality but
to use the new PyJWT we also needed a mozpay-py upgrade.
kumar303 added a commit that referenced this pull request Mar 30, 2015
Upgrade to mainline PyJWT (part of bug 1145024)
@kumar303 kumar303 merged commit cc17105 into mozilla:master Mar 30, 2015
@kumar303 kumar303 deleted the upgrade-jwt branch March 30, 2015 20:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants