Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Audience String array #238

Closed
wants to merge 14 commits into from
Closed

Audience String array #238

wants to merge 14 commits into from

Conversation

ricveal
Copy link

@ricveal ricveal commented Jul 15, 2017

Added suport to Array of String for Audience #77

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 99.809% when pulling dfbc3df on ricveal:audience_array into ac73b1c on jwtk:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling d45fa8d on ricveal:audience_array into ac73b1c on jwtk:master.

@ricveal
Copy link
Author

ricveal commented Jul 15, 2017

It seems like Travis CI configuration for JDK7 is broken :(

In previous versions, you can only set the complete value of the audience field. Now you can set as mandatory one or multiple audiences when you are parsing the token and, internally the library will check if the parsed audiences contains all of them.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling d452c2c on ricveal:audience_array into ac73b1c on jwtk:master.

@dogeared
Copy link
Contributor

Please merge master to your branch. the jdk7 issue is fixed. Thanks!

Merge Travis configuration fixes in forked master
Updated Travis configuration
@coveralls
Copy link

coveralls commented Aug 2, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling f608faf on ricveal:audience_array into 44faaca on jwtk:master.

@dogeared
Copy link
Contributor

For reference: https://tools.ietf.org/html/rfc7519#section-4.1.3

Since this is an interface change and in its current form is not backwards compatible, it would have to be released with a major version release, probably 1.0.

Since the spec indicates that for a single audience a string is acceptable, perhaps it should be able to handle both a simple String and a String array?

@mtttcgcg
Copy link

mtttcgcg commented Mar 8, 2018

We are really looking forward to this feature because we want to share a web service amongst multiple clients, which have tokens containing different aud claims.

@ricveal
Copy link
Author

ricveal commented Mar 17, 2018

@dogeared I've made the changes requested.

The changes keep the API to handle String in the audience attribute, adding support to handle String array.

@ferlemes
Copy link

Hi all! Is there a plan to have this merged to master? Do you need help on this?

@ricveal
Copy link
Author

ricveal commented Oct 23, 2018

I have updated again the branch with the latest changes in master.

I have to check why tests are now failing.

I will update the PR as soon as I can!

@ricveal
Copy link
Author

ricveal commented Mar 25, 2020

I cannot not find time to correctly review this PR I started almost three years ago. Sorry.

The project has evolved during this time and my proposed implementation is practically invalid with the current codebase so I close this PR.

I'm also going to archive my fork project since I will not continue working on this feature.

Please feel free to reopen the PR or use the repository as a reference in case someone wants to continue this functionality.

Thank you for your time @dogeared !

@ricveal ricveal closed this Mar 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants