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

Make request APIs consistent by restricting access to automation/provision requests to admin/requester #15186

Merged

Conversation

imtayadeway
Copy link
Contributor

Brings consistency to the automation/provision requests API so that it filters/restricts access to requests in the same way as the generic requests API

@miq-bot add-label bug, api
@miq-bot assign @abellotti

@imtayadeway imtayadeway force-pushed the api/fix/request-access-consistency branch from 2e6efdb to 618a550 Compare May 22, 2017 18:07
@miq-bot
Copy link
Member

miq-bot commented May 22, 2017

Checked commits imtayadeway/manageiq@1ae66cc~...618a550 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
5 files checked, 0 offenses detected
Everything looks fine. ⭐

@@ -41,5 +41,16 @@ def deny_resource(type, id, data)
rescue => err
action_result(false, err.to_s)
end

def find_automation_requests(id)
Copy link
Member

Choose a reason for hiding this comment

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

seem similar to the find_provision_requests, anyway we can have common code ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It might be good to do this as a follow up since there might be more opportunities to do this outside the scope of fixing these two endpoints

klass.find_by!(:requester => User.current_user, :id => id)
end

def automation_requests_search_conditions
Copy link
Member

Choose a reason for hiding this comment

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

same here

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 above - this issue is broader than this PR, so might be best done in a follow up

@chessbyte
Copy link
Member

@imtayadeway bump

@imtayadeway
Copy link
Contributor Author

@abellotti bump

@abellotti
Copy link
Member

LGTM 🎹

@imtayadeway please have a follow up PR to eliminate the common code (find_*_requests). Thanks.

@abellotti abellotti merged commit 9c808a7 into ManageIQ:master Jul 5, 2017
@abellotti abellotti added this to the Sprint 64 Ending Jul 10, 2017 milestone Jul 5, 2017
imtayadeway added a commit to imtayadeway/manageiq that referenced this pull request Jul 5, 2017
imtayadeway added a commit to imtayadeway/manageiq-providers-amazon that referenced this pull request Jul 5, 2017
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.

4 participants