Skip to content

Commit

Permalink
Make split deletion only accessible to owning app
Browse files Browse the repository at this point in the history
  • Loading branch information
jmileham committed Apr 5, 2017
1 parent 18dd404 commit e9cc2df
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 31 deletions.
2 changes: 1 addition & 1 deletion app/controllers/api/v1/split_configs_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def create
end

def destroy
split = Split.find_by!(name: params[:id])
split = current_app.splits.find_by!(name: params[:id])
split.update!(finished_at: Time.zone.now) unless split.finished?
head :no_content
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/authenticated_api_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ class AuthenticatedApiController < UnauthenticatedApiController
attr_reader :current_app

def authenticate
authenticate_with_http_basic do |app_name, auth_secret|
authenticate_or_request_with_http_basic do |app_name, auth_secret|
app = App.find_by(name: app_name)
if app && ActiveSupport::SecurityUtils.secure_compare(app.auth_secret, auth_secret)
@current_app = app
Expand Down
78 changes: 49 additions & 29 deletions spec/controllers/api/v1/split_configs_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,53 +3,73 @@
RSpec.describe Api::V1::SplitConfigsController, type: :controller do
let(:default_app) { FactoryGirl.create :app, name: "default_app", auth_secret: "6Sd6T7T6Q8hKcoo0t8CTzV0IdN1EEHqXB2Ig4raZsOU" }

before do
http_authenticate username: default_app.name, auth_secret: default_app.auth_secret
end

describe '#create' do
it "creates a new split with desired weightings" do
it "doesn't create when unauthenticated" do
post :create, name: 'foobar', weighting_registry: { foo: 10, bar: 90 }

expect(response).to have_http_status(:no_content)
split = Split.find_by(name: 'foobar', owner_app: default_app)
expect(split).to be_truthy
expect(split.registry).to eq 'foo' => 10, 'bar' => 90
expect(response).to have_http_status(:unauthorized)
expect(Split.where(name: 'foobar')).to be_empty
end
end

it 'returns errors when invalid' do
post :create, name: 'fooBar', weighting_registry: { foo: 10, bar: 90 }
expect(response).to have_http_status(:unprocessable_entity)
expect(response_json['errors']['name'].first).to include("snake_case")
describe "while authenticated" do
before do
http_authenticate username: default_app.name, auth_secret: default_app.auth_secret
end
end

describe '#destroy' do
it "sets the finished_at time on the split" do
split = FactoryGirl.create(:split, name: "old_split", owner_app: default_app, finished_at: nil)
describe '#create' do
it "creates a new split with desired weightings" do
post :create, name: 'foobar', weighting_registry: { foo: 10, bar: 90 }

delete :destroy, id: "old_split"
expect(response).to have_http_status(:no_content)
split = Split.find_by(name: 'foobar', owner_app: default_app)
expect(split).to be_truthy
expect(split.registry).to eq 'foo' => 10, 'bar' => 90
end

expect(response).to have_http_status(:no_content)
expect(split.reload).to be_finished
it 'returns errors when invalid' do
post :create, name: 'fooBar', weighting_registry: { foo: 10, bar: 90 }
expect(response).to have_http_status(:unprocessable_entity)
expect(response_json['errors']['name'].first).to include("snake_case")
end
end

it "is idempotent" do
split = FactoryGirl.create(:split, name: "old_split", owner_app: default_app, finished_at: nil)
describe '#destroy' do
it "sets the finished_at time on the split" do
split = FactoryGirl.create(:split, name: "old_split", owner_app: default_app, finished_at: nil)

Timecop.freeze(Time.zone.parse('2011-01-01')) do
delete :destroy, id: "old_split"

expect(response).to have_http_status(:no_content)
expect(split.reload).to be_finished
end

expect(response).to have_http_status(:no_content)
expect(split.reload.finished_at).to eq Time.zone.parse('2011-01-01')
it "can't delete another app's split" do
other_app = FactoryGirl.create :app, name: "other_app"
split = FactoryGirl.create(:split, name: "other_split", owner_app: other_app, finished_at: nil)

Timecop.freeze(Time.zone.parse('2011-01-02')) do
delete :destroy, id: "old_split"
expect { delete :destroy, id: "other_split" }.to raise_error(ActiveRecord::RecordNotFound)

expect(split.reload).not_to be_finished
end

expect(response).to have_http_status(:no_content)
expect(split.reload.finished_at).to eq Time.zone.parse('2011-01-01')
it "is idempotent" do
split = FactoryGirl.create(:split, name: "old_split", owner_app: default_app, finished_at: nil)

Timecop.freeze(Time.zone.parse('2011-01-01')) do
delete :destroy, id: "old_split"
end

expect(response).to have_http_status(:no_content)
expect(split.reload.finished_at).to eq Time.zone.parse('2011-01-01')

Timecop.freeze(Time.zone.parse('2011-01-02')) do
delete :destroy, id: "old_split"
end

expect(response).to have_http_status(:no_content)
expect(split.reload.finished_at).to eq Time.zone.parse('2011-01-01')
end
end
end
end

0 comments on commit e9cc2df

Please sign in to comment.