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

[SDK-4546] Add orgs in client credentials support #540

Merged
merged 4 commits into from
Nov 13, 2023
Merged

Conversation

adamjmcgrath
Copy link
Contributor

Changes

  • Add optional organization parameter to client credentials grant
  • Add Management API endpoints to /client-grants and /organizations

page: options.fetch(:page, nil),
include_totals: options.fetch(:include_totals, nil)
}
path = "#{organizations_client_grants_path(organization_id)}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it necessary to interpolate the return value, if organizations_client_grants_path(organization_id) returns a string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

body = {}
body[:grant_id] = grant_id

path = "#{organizations_client_grants_path(organization_id)}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as before, is the string interpolation necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as before, just being consistent...

@@ -11,13 +11,15 @@ module ClientGrants
# @param audience [string] The audience of the client grant to retrieve.
# @param page [int] Page number to get, 0-based.
# @param per_page [int] Results per page if also passing a page number.
# @param allow_any_organization [bool] Optional filter on allow_any_organization.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we expand a bit on what allow_any_organization does?

Copy link
Contributor Author

@adamjmcgrath adamjmcgrath Nov 10, 2023

Choose a reason for hiding this comment

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

I'd agree if you were getting/setting the allow_any_organization field, but this is a filter on the value

@adamjmcgrath adamjmcgrath changed the title [SDK-4544] Add orgs in client credentials support [SDK-4546] Add orgs in client credentials support Nov 10, 2023
@adamjmcgrath adamjmcgrath merged commit 74eaac9 into master Nov 13, 2023
13 checks passed
@adamjmcgrath adamjmcgrath deleted the orgs-cc branch November 13, 2023 14:48
@adamjmcgrath adamjmcgrath mentioned this pull request Nov 13, 2023
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.

2 participants