X Tutup
Skip to content

Support multiple issuers #246#288

Merged
lbalmaceda merged 10 commits intoauth0:masterfrom
itdevelopmentapps:master
Mar 11, 2019
Merged

Support multiple issuers #246#288
lbalmaceda merged 10 commits intoauth0:masterfrom
itdevelopmentapps:master

Conversation

@itdevelopmentapps
Copy link
Copy Markdown
Contributor

Added support for multiple issuers.

I wanted to change the previous pull request so I closed it, maybe if you guys can remove that one in favor for this one.

Many thanks.

Copy link
Copy Markdown
Contributor

@lbalmaceda lbalmaceda left a comment

Choose a reason for hiding this comment

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

Thanks for the PR and sorry for the delay. I've left you some comments. Please remember to rebase the branch.

@lbalmaceda lbalmaceda closed this Jan 28, 2019
@lbalmaceda lbalmaceda reopened this Jan 28, 2019
Copy link
Copy Markdown
Contributor

@lbalmaceda lbalmaceda left a comment

Choose a reason for hiding this comment

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

Sorry for the confusion. I've left a few changes I need you to accept before merging this.

@lbalmaceda
Copy link
Copy Markdown
Contributor

@itdevelopmentapps are you still interesting in merging this PR? Please, don't hesitate to ask any questions.

@itdevelopmentapps
Copy link
Copy Markdown
Contributor Author

@lbalmaceda please advise in what test you would like to see to increase coverage so that we can get this pull request accepted.

@lbalmaceda
Copy link
Copy Markdown
Contributor

By checking the coverage report, there are 2 missing branches.

  • withAudience needs to be called both with a valid audience and a null string value. Then you need to call verify. Tests are only present for a "valid audience value" here.
  • for withIssuer tests are present for "both valid issuer and null string values" here but you need to add another one that verifies against a JWT payload that doesn't have an issuer claim defined.

I hope that's clear enough. It should help make the check green.

@itdevelopmentapps
Copy link
Copy Markdown
Contributor Author

@lbalmaceda we've added the suggested tests.

@lbalmaceda lbalmaceda added this to the v3-Next milestone Mar 11, 2019
@lbalmaceda lbalmaceda merged commit 6ffa703 into auth0:master Mar 11, 2019
@lbalmaceda
Copy link
Copy Markdown
Contributor

Thank you. Will release before eow.

@itdevelopmentapps
Copy link
Copy Markdown
Contributor Author

Many thanks.

@lbalmaceda lbalmaceda modified the milestones: v3-Next, 3.8.0 Mar 14, 2019
@vikeri
Copy link
Copy Markdown

vikeri commented Oct 4, 2019

Using Clojure this was a breaking change, got the following error:

java.lang.ClassCastException: java.lang.String cannot be cast to [Ljava.lang.String;

When trying to call withIssuer.
To fix:
Before

(.withIssuer Verification "example")

After

(.withIssuer Verification (into-array String ["example]))

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

X Tutup