Skip to content

Allows multiple values for 'iss'#31

Merged
mpdavis merged 3 commits into
mpdavis:masterfrom
bjmc:multiple_iss
Jul 27, 2016
Merged

Allows multiple values for 'iss'#31
mpdavis merged 3 commits into
mpdavis:masterfrom
bjmc:multiple_iss

Conversation

@bjmc

@bjmc bjmc commented Jul 27, 2016

Copy link
Copy Markdown
Contributor

Apparently Google may send either https://accounts.google.com or accounts.google.com for their 'iss' value, so it's necessary to check both.

This pull request allows passing a list or tuple to jwt.decode(issuer= so, for example, you can check against both valid Google values, instead of having to catch an exception and re-validate.

@codecov-io

codecov-io commented Jul 27, 2016

Copy link
Copy Markdown

Current coverage is 95.78% (diff: 100%)

Merging #31 into master will increase coverage by 0.01%

@@             master        #31   diff @@
==========================================
  Files             7          7          
  Lines           496        498     +2   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits            475        477     +2   
  Misses           21         21          
  Partials          0          0          

Powered by Codecov. Last update 8766f13...2cf888f

Comment thread jose/jwt.py Outdated
if issuer is not None:
if claims.get('iss') != issuer:
if isinstance(issuer, string_types):
issuer = [issuer]

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Since we don't need to mutate this, can we construct this as a tuple instead of a list?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point. I changed jws._get_keys() to use a tuple as well, for consistency. (I do think 1-element tuples are kind of ugly, though)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I can't disagree that they are ugly.

@mpdavis

mpdavis commented Jul 27, 2016

Copy link
Copy Markdown
Owner

lgtm

@mpdavis mpdavis merged commit 2055f0a into mpdavis:master Jul 27, 2016
@mpdavis

mpdavis commented Jul 27, 2016

Copy link
Copy Markdown
Owner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants