IdentityPython / pyjwkest

Implementation of JWT, JWS, JWE and JWK
Apache License 2.0
92 stars 55 forks source link

Return validation results from verify_compact #73

Closed jschlyter closed 7 years ago

rohe commented 7 years ago

I think that would be the way to go.

7 okt. 2016 kl. 16:31 skrev Jakob Schlyter notifications@github.com:

@jschlyter commented on this pull request.

In src/jwkest/jws.py:

@@ -566,7 +578,7 @@ def verify_compact(self, jws, keys=None, allow_none=False, sigalg=None): logger.debug( "Verified message using key with kid=%s" % key.kid) self.msg = jwt.payload()

  • return self.msg
  • return (self.msg, {'key': key})

One could also consider returning only a dictionary with both msg and key.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

-- Roland "Education is the path from cocky ignorance to miserable uncertainty.” - Mark Twain

jschlyter commented 7 years ago

We now return a dict.

rohe commented 7 years ago

Have you looked at the tests ?

7 okt. 2016 kl. 22:39 skrev Jakob Schlyter notifications@github.com:

We now return a dict.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rohe/pyjwkest/pull/73#issuecomment-252355616, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGGPAw9shuKLvprsPr4mrsSPpJXOa8Rks5qxq4SgaJpZM4KRF_U.

jschlyter commented 7 years ago

I have now. Also changed so that JWS self.key contains the last key.

rohe commented 7 years ago

So, if self.key contains the key that was used to verify the signature is there still a need for getting it return ? Having the method return a dict changes the API in a not backward compatible way so if it can be avoided I’d prefer that.

8 okt. 2016 kl. 19:13 skrev Jakob Schlyter notifications@github.com:

I have now. Also changed to that JWS self.key contains the last key.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

-- Roland "Education is the path from cocky ignorance to miserable uncertainty.” - Mark Twain

jschlyter commented 7 years ago

I'm good with only setting self.key. Want a PR with only that?

Sent from my iPhone, hence this mail might be briefer than normal.

10 okt. 2016 kl. 20:01 skrev Roland Hedberg notifications@github.com:

So, if self.key contains the key that was used to verify the signature is there still a need for getting it return ? Having the method return a dict changes the API in a not backward compatible way so if it can be avoided I’d prefer that.

8 okt. 2016 kl. 19:13 skrev Jakob Schlyter notifications@github.com:

I have now. Also changed to that JWS self.key contains the last key.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

-- Roland "Education is the path from cocky ignorance to miserable uncertainty.” - Mark Twain

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

jschlyter commented 7 years ago

Although do note that the API was not changed, the new verify_compact_verbose() returns the dict and the old verify_compact() still returns the msg only.

But, isn't storing results in the object itself might be dangerous for concurrency? If two validations happens simultanetously with the same JWS object, they will overwrite each others results.

rohe commented 7 years ago

Ah, right ! Sorry about that !

10 okt. 2016 kl. 20:33 skrev Jakob Schlyter notifications@github.com:

Although do note that the API was not changed, the new verify_compact_verbose() returns the dict and the old verify_compact() still returns the msg only.

But, isn't storing results in the object itself might be dangerous for concurrency? If two validations happens simultanetously with the same JWS object, they will overwrite each others results.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

-- Roland "Education is the path from cocky ignorance to miserable uncertainty.” - Mark Twain