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

feat: support multiple audiences #44

Merged
merged 4 commits into from
Jan 23, 2024

Conversation

peterphanouvong
Copy link
Contributor

Explain your changes

  • allow passing in audiences as string of audiences separated by whitespace
  • set the url params in a way to allow for more than 1 audience

Suppose there is a related issue with enough detail for a reviewer to understand your changes fully. In that case, you can omit an explanation and instead include either “Fixes #XX” or “Updates #XX” where “XX” is the issue number.

Checklist

🛟 If you need help, consider asking for advice over in the Kinde community.

lib/sdk/oauth2-flows/AuthCodeAbstract.ts Outdated Show resolved Hide resolved
lib/sdk/oauth2-flows/ClientCredentials.ts Outdated Show resolved Hide resolved
@coel
Copy link
Member

coel commented Jan 18, 2024

@peterphanouvong, I'd suggest instead of doing a string split convert the property config.audience to be an array. It could be string or string[] to maintain backwards compatibility.

So for OAuth2FlowOptions, the property becomes:

audience?: string | string[];

Then check later if it is an array or not. Could be a condition or do something to make an array if not like:

const audienceArray = Array.isArray(this.config.audience) ? this.config.audience : [this.config.audience];

@DaveOrDead
Copy link
Member

@coel I guess a lot of the time the audience is being passed in via a .env file which would mean declaring

AUDIENCE=['a','b']

and then having to do some nastiness to convert that environment variable string into an array?

@coel
Copy link
Member

coel commented Jan 18, 2024

@DaveOrDead , I think if it is nasty we shouldn't force it onto users in the base SDK. It seems like whitespace is valid in an audience, though we don't seem to handle it. We might need to patch that up.

If we have helpers to deal with .env I think it should happen in the layer .env is introduced.

@DaveOrDead
Copy link
Member

@coel I'm ok with that @peterphanouvong are you ok to update?

@DaveOrDead DaveOrDead merged commit db341e1 into main Jan 23, 2024
1 check passed
@DaveOrDead DaveOrDead deleted the peter/feat/support-multiple-audiences branch January 23, 2024 21:56
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.

3 participants