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

fix: auth requests use org and user names if present #22272

Merged
merged 4 commits into from
Aug 23, 2021

Conversation

williamhbaker
Copy link
Contributor

Closes #22261

Fixes requests to /api/v2/authorizations so that the query parameters of user and/or org will be used to populate the filter with the respective ID's if user/org are present but userID/orgID are not in the query.

Giving precedence to the ID parameters if they are present over the "name" parameters seems to be consistent with how other resource endpoints work. For example, if you provide both a user and a userID, the userID will be used to filter the authorizations.

Copy link
Member

@gwossum gwossum left a comment

Choose a reason for hiding this comment

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

Looks like it should work as described. Are there test cases that test if an invalid user or org are in the query? Couldn't tell if there were any tests cases that did that.

CHANGELOG.md Outdated Show resolved Hide resolved
@williamhbaker
Copy link
Contributor Author

Looks like it should work as described. Are there test cases that test if an invalid user or org are in the query? Couldn't tell if there were any tests cases that did that.

It goes through the tenant service, which will cause the handler to return an error (404 HTTP response) if the request user or org name doesn't exist. There aren't tests directly here for that, but the tenant service does have tests for org not found and user not found.

Copy link
Member

@gwossum gwossum left a comment

Choose a reason for hiding this comment

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

I've seen enough. Approved.

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.

/api/v2/authorizations/?user query parameter does not filter authorizations
3 participants