Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added PyJWS #141

Merged
merged 4 commits into from
Apr 21, 2015
Merged

Added PyJWS #141

merged 4 commits into from
Apr 21, 2015

Conversation

mark-adams
Copy link
Contributor

So, eventually, I'd like to see us do more with JWS / JWE / JWK objects as well. (Probably as part of a 2.0 release).

At this point though, it makes sense to split up the JWS logic (signing and verification) from the JWT logic (JSON-encoding / -decoding of the payload and verification of the lcaims) into separate classes. That way it will be easier to make fixes to both branches (if we have bugs crop up) as we work on a 2.0.0 release branch over the next few months.

  • Split PyJWT into PyJWS and PyJWT (inheriting from PyJWS)
  • Split tests into JWS tests and JWT tests
  • Split PyJWT._validate_claims into multiple PyJWT._validate_<claim-name> methods to simplify the code complexity a little bit

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 59ed367 on mark-adams:added-pyjws into c17c6d4 on jpadilla:master.

@mark-adams mark-adams added this to the v2.0.0 milestone Apr 19, 2015
@jpadilla
Copy link
Owner

@mark-adams oh yeah, this is a nice first step. I'm totally down for that. How about creating a new branch for v2 so we can instead merge all this work there in parallel to master?

@mark-adams
Copy link
Contributor Author

I think having a 2.0-develop branch would be a great idea! However, I think it might be a good idea to merge this PR into master, that way the JWS vs JWT logic will already be split out in master and any bugfixes that we need to make to the existing logic will be much less complicated to handle.

@jpadilla
Copy link
Owner

@mark-adams gotcha, yea makes sense.

Sign up for free to 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.

3 participants