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

delete authentication in provider #14307

Merged
merged 3 commits into from
Mar 16, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions app/controllers/api/authentications_controller.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,18 @@
module Api
class AuthenticationsController < BaseController
def delete_resource(type, id, _data = {})
auth = resource_search(id, type, collection_class(:authentications))
raise 'type not currently supported' unless auth.respond_to?(:delete_in_provider_queue)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe update the message. maybe something like "Delete not supported for #{authentication_ident(auth)}"

task_id = auth.delete_in_provider_queue
action_result(true, "Deleting #{authentication_ident(auth)}", :task_id => task_id)
rescue => err
action_result(false, err.to_s)
end

private

def authentication_ident(auth)
"Authentication id:#{auth.id} name: '#{auth.name}'"
end
end
end
5 changes: 1 addition & 4 deletions config/api.yml
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@
:options:
- :collection
- :subcollection
:verbs: *gpd
:verbs: *gp
Copy link
Member

Choose a reason for hiding this comment

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

Why is the DELETE action removed on resource. You can add that back. We support DELETE on resources that are zapped async, i.e. /api/providers, if user sends a DELETE HTTP action, it's for fire and forget, they won't care about the task_id. If they care, they'd simply use the delete POST action and leverage our action result signature.

:klass: Authentication
:collection_actions:
:get:
Expand All @@ -249,9 +249,6 @@
:post:
- :name: delete
:identifier: embedded_automation_manager_credentials_delete
:delete:
- :name: delete
:identifier: embedded_automation_manager_credentials_delete
:subcollection_actions:
:get:
- :name: read
Expand Down
85 changes: 56 additions & 29 deletions spec/requests/api/authentications_spec.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
RSpec.describe 'Authentications API' do
let(:provider) { FactoryGirl.create(:provider_ansible_tower) }
let(:auth) { FactoryGirl.create(:ansible_cloud_credential, :resource => provider) }
let(:auth_2) { FactoryGirl.create(:ansible_cloud_credential, :resource => provider) }

describe 'GET/api/authentications' do
it 'lists all the authentication configuration script bases with an appropriate role' do
auth = FactoryGirl.create(:authentication)
Expand Down Expand Up @@ -51,66 +55,89 @@

describe 'POST /api/authentications' do
it 'will delete an authentication' do
auth = FactoryGirl.create(:authentication)
api_basic_authorize collection_action_identifier(:authentications, :delete, :post)

expected = {
'results' => [a_hash_including('success' => true)]
'results' => [
a_hash_including(
'success' => true,
'message' => a_string_including('Deleting Authentication'),
'task_id' => a_kind_of(Numeric)
)
]
}
expect do
run_post(authentications_url, :action => 'delete', :resources => [{ 'id' => auth.id }])
end.to change(Authentication, :count).by(-1)
run_post(authentications_url, :action => 'delete', :resources => [{ 'id' => auth.id }])

expect(response.parsed_body).to include(expected)
expect(response).to have_http_status(:ok)
end

it 'will forbid deletion to an authentication without appropriate role' do
it 'verifies that the type is supported' do
api_basic_authorize collection_action_identifier(:authentications, :delete, :post)
auth = FactoryGirl.create(:authentication)
api_basic_authorize

run_post(authentications_url, :action => 'delete', :resources => [{ 'id' => auth.id }])
expect(response).to have_http_status(:forbidden)

expected = {
'results' => [
{ 'success' => false, 'message' => 'type not currently supported' }
]
}
expect(response).to have_http_status(:ok)
expect(response.parsed_body).to include(expected)
end
end

describe 'POST /api/authentications/:id' do
it 'will delete an authentication' do
auth = FactoryGirl.create(:authentication)
api_basic_authorize action_identifier(:authentications, :delete, :resource_actions, :post)
it 'will delete multiple authentications' do
api_basic_authorize collection_action_identifier(:authentications, :delete, :post)

expect do
run_post(authentications_url(auth.id), :action => 'delete')
end.to change(Authentication, :count).by(-1)
expect(response.parsed_body).to include('success' => true)
expected = {
'results' => [
a_hash_including(
'success' => true,
'message' => a_string_including('Deleting Authentication'),
'task_id' => a_kind_of(Numeric)
),
a_hash_including(
'success' => true,
'message' => a_string_including('Deleting Authentication'),
'task_id' => a_kind_of(Numeric)
)
]
}
run_post(authentications_url, :action => 'delete', :resources => [{ 'id' => auth.id }, { 'id' => auth_2.id }])

expect(response.parsed_body).to include(expected)
expect(response).to have_http_status(:ok)
end

it 'will not delete an authentication without an appropriate role' do
it 'will forbid deletion to an authentication without appropriate role' do
auth = FactoryGirl.create(:authentication)
api_basic_authorize

run_post(authentications_url(auth.id), :action => 'delete')

run_post(authentications_url, :action => 'delete', :resources => [{ 'id' => auth.id }])
expect(response).to have_http_status(:forbidden)
end
end

describe 'DELETE /api/authentications/:id' do
describe 'POST /api/authentications/:id' do
it 'will delete an authentication' do
auth = FactoryGirl.create(:authentication)
api_basic_authorize action_identifier(:authentications, :delete, :resource_actions, :delete)
api_basic_authorize action_identifier(:authentications, :delete, :resource_actions, :post)

run_post(authentications_url(auth.id), :action => 'delete')

expect do
run_delete(authentications_url(auth.id))
end.to change(Authentication, :count).by(-1)
expect(response).to have_http_status(:no_content)
expected = {
'success' => true,
'message' => a_string_including('Deleting Authentication'),
'task_id' => a_kind_of(Numeric)
}
expect(response.parsed_body).to include(expected)
expect(response).to have_http_status(:ok)
end

it 'will not delete an authentication without an appropriate role' do
auth = FactoryGirl.create(:authentication)
api_basic_authorize

run_delete(authentications_url(auth.id))
run_post(authentications_url(auth.id), :action => 'delete')

expect(response).to have_http_status(:forbidden)
end
Expand Down