-
Notifications
You must be signed in to change notification settings - Fork 897
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
Authentications Read and Delete api #13780
Authentications Read and Delete api #13780
Conversation
Wouldn't it be better to set the endpoint to the higher-level concept (Authentications) and allow the requester to filter by type? |
@bdunne I considered that, but that was out of the scope of my pivotal ticket so I thought I would narrow it. Also, create / update of these credentials (ie #13773) will work differently than some of the other thoughts @abellotti @gmcculloug @lgalis ? |
Please rebase this PR to incorporate #13846 in order to ensure api.yml integrity ❤️ ❤️ ❤️ |
Checked commits jntullo/manageiq@58c837d~...f6d106e with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
:collection_actions: | ||
:get: | ||
- :name: read | ||
:identifier: embedded_automation_manager_credentials_view |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@abellotti how should we handle this identifier? We are using the Authentication
model, but only have this identifier currently. Is it fine as is, and as needed we will add additional ones via #13827 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The thought behind #13827 is that higher level collections are authorizable if the user has at least one of the related/relevant roles, so if a user has embedded_automation_manager_credentials_view or later automation_manager_credentials_view, etc. he'd be able to access this collection.
Signature above is fine, once #13827 is merged, you'd also use the array equivalent:
:get:
- :name: read
:identifier:
- embedded_automation_manager_credentials_view
- automation_manager_credentials_view (adding later whatever additional ones materialize as )
@miq-bot remove_label wip |
Was a bit concerned that this was returning everything (was running as admin), but ran as a non-admin user without the embedded_automation_manager_credentials_view role, and got the standard 403. So, I'd 👍 with this. |
This API is to read and delete
Authentication
resources.Need correct product features for this PR (currently just has a placeholder).
Links
@miq-bot add_label wip, enhancement, api, euwe/no
@miq-bot assign @abellotti