Skip to content
This repository has been archived by the owner on Oct 3, 2019. It is now read-only.

Allow audience verification override #7

Merged
merged 2 commits into from
Jan 9, 2018

Conversation

dursk
Copy link
Contributor

@dursk dursk commented Jan 9, 2018

Allow for skipping audience verification when decoding id_tokens forwarded from BFFs

Copy link

@kinney kinney left a comment

Choose a reason for hiding this comment

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

lgtm 🏄

@@ -31,7 +31,7 @@ def __init__(self, certificate=None, allowed_audiences=None):

self._allowed_audiences = set(allowed_audiences)

def decode(self, token):
def decode(self, token, verify_audience=True):
Copy link
Contributor

Choose a reason for hiding this comment

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

There's an argument here that we shouldn't allow users of mbq.tokens to shoot themselves in the foot or knowingly avoid checking the audience because it's "easier."

Another approach would be to provide a separate method Decoder.decode_foreign_token (with a better name) that has the same functionality but doesn't give the caller the impression that they can use it whenever they want a more relaxed decoder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, totally open to suggestions on the name!

Copy link
Contributor

@mattbriancon mattbriancon left a comment

Choose a reason for hiding this comment

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

Looks great!

@@ -57,6 +57,16 @@ def test_decode_bad_audience(self):
with self.assertRaises(tokens.TokenError):
decoder.decode(make_jwt(audience='different_audience'))

def test_decode_foreign_token(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: function name?

@dursk dursk force-pushed the allow-to-skip-aud-verification branch from 8b8b8e2 to 1815b6f Compare January 9, 2018 19:06
@dursk dursk merged commit 1f480f3 into master Jan 9, 2018
@dursk dursk deleted the allow-to-skip-aud-verification branch January 9, 2018 19:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants