Resolved issue #205, Allow a list of valid audiences to be configured #246
Conversation
…nfigured Added test cases to check if audience is provided a list.
CHANGELOG.md
Outdated
| @@ -12,6 +12,12 @@ This project adheres to [Semantic Versioning](http://semver.org/). | |||
| - Better error messages when using an algorithm that requires the cryptography package, but it isn't available [#230][230] | |||
|
|
|||
| ### Fixed | |||
| - Allow a list of valid audiences to be configured [#205][205] | |||
|
|
|||
| [v1.4.3][1.4.3] | |||
There was a problem hiding this comment.
No need to add this version section here. We do that whenever we make new releases.
jwt/api_jwt.py
Outdated
| raise TypeError('audience must be a string or None') | ||
| if not isinstance(audience, (string_types, type(None), list)): | ||
| if not isinstance(audience, list): | ||
| raise TypeError('audience must be a string or None') |
There was a problem hiding this comment.
What do you think about moving this all into self._validate_aud()?
That would probably simplify the implementation a little bit.
There was a problem hiding this comment.
Actually, it looks like you basically already did that for the most part. Maybe we should just get rid of the redundant check here completely?
CHANGELOG.md
Outdated
| @@ -12,6 +12,12 @@ This project adheres to [Semantic Versioning](http://semver.org/). | |||
| - Better error messages when using an algorithm that requires the cryptography package, but it isn't available [#230][230] | |||
|
|
|||
| ### Fixed | |||
| - Allow a list of valid audiences to be configured [#205][205] | |||
|
|
|||
| [v1.4.3][1.4.3] | |||
There was a problem hiding this comment.
We typically add the new version section when we do a release. We can probably delete these lines for now and leave it up to the maintainers at release time.
|
Hi Mark Adams, Thanks for review. I took care of your comments. Could you please review? Thanks. |
| if isinstance(audience, string_types): | ||
| audience = [audience] | ||
|
|
||
| if not isinstance(audience, list): |
There was a problem hiding this comment.
I think it's more idiomatic to try to iterate over the object and handle the exception if it's not iterable.
| raise InvalidAudienceError('Invalid audience format') | ||
|
|
||
| for aud in audience: | ||
| if aud in audience_claims: |
There was a problem hiding this comment.
Since we are now checking multiple values to see if they are in audience_claims do you think it might make more sense to make audience_claims a set?
|
Closed by #306 |
Added test cases to check if audience is provided a list.