From 6a97985c443c5cab9ee75351b9b3c53924a1504e Mon Sep 17 00:00:00 2001 From: phil Date: Wed, 18 Jun 2014 12:00:05 +0200 Subject: [PATCH 1/3] raise an error if api/talks#update fails to set state --- app/controllers/api/talks_controller.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/app/controllers/api/talks_controller.rb b/app/controllers/api/talks_controller.rb index 015b60c71..062ee3319 100644 --- a/app/controllers/api/talks_controller.rb +++ b/app/controllers/api/talks_controller.rb @@ -26,6 +26,13 @@ def update # TODO check for security issue (whitelist methods) msg = send @method, msg if respond_to? @method + # this is critical, so raise an error if it fails + if state && state != @talk.reload.session[current_user.id][:state] + raise "Critical: Failed to set state #{state} " + + "for user #{current_user.id} " + + "on talk #{@talk.id}" + end + publish msg.to_hash head :ok end From e45e2ab7d5236cdf5213e2b80606e1923c672f68 Mon Sep 17 00:00:00 2001 From: "Munen Alain M. Lafon" Date: Thu, 19 Jun 2014 16:58:25 +0200 Subject: [PATCH 2/3] make it testable --- app/controllers/api/talks_controller.rb | 6 +++++- spec/controllers/api/talks_controller_spec.rb | 7 +++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/app/controllers/api/talks_controller.rb b/app/controllers/api/talks_controller.rb index 062ee3319..8d07560ef 100644 --- a/app/controllers/api/talks_controller.rb +++ b/app/controllers/api/talks_controller.rb @@ -27,7 +27,7 @@ def update msg = send @method, msg if respond_to? @method # this is critical, so raise an error if it fails - if state && state != @talk.reload.session[current_user.id][:state] + if validate_state(state) raise "Critical: Failed to set state #{state} " + "for user #{current_user.id} " + "on talk #{@talk.id}" @@ -102,4 +102,8 @@ def verified_request? super || form_authenticity_token == request.headers['X-XSRF-TOKEN'] end + def validate_state(state) + state && state != @talk.reload.session[current_user.id][:state] + end + end diff --git a/spec/controllers/api/talks_controller_spec.rb b/spec/controllers/api/talks_controller_spec.rb index 78f0e3788..353bfaa7a 100644 --- a/spec/controllers/api/talks_controller_spec.rb +++ b/spec/controllers/api/talks_controller_spec.rb @@ -30,6 +30,13 @@ response.status.should be(422) end + it 'raises error when state cannot be set' do + Api::TalksController.any_instance.stub(:validate_state).and_raise + expect { + put :update, id: @talk.id, msg: { state: 'WaitingForPromotion' } + }.to raise_error + end + end # as host sending events for other user From c43109c96a3d6d19784f49ef4478a4395c213c90 Mon Sep 17 00:00:00 2001 From: "Munen Alain M. Lafon" Date: Thu, 19 Jun 2014 17:01:02 +0200 Subject: [PATCH 3/3] add method to error log --- app/controllers/api/talks_controller.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/api/talks_controller.rb b/app/controllers/api/talks_controller.rb index 8d07560ef..c27899fb9 100644 --- a/app/controllers/api/talks_controller.rb +++ b/app/controllers/api/talks_controller.rb @@ -30,7 +30,8 @@ def update if validate_state(state) raise "Critical: Failed to set state #{state} " + "for user #{current_user.id} " + - "on talk #{@talk.id}" + "on talk #{@talk.id} " + + "with method #{@method}" end publish msg.to_hash