From 7579f8d3675c3ccc2e6ad48c7ecea9ed56ca18b6 Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Sat, 8 Jul 2017 23:24:49 +0530 Subject: [PATCH 01/14] Start working on a function to add commenting feature through a token --- app/controllers/comment_controller.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/app/controllers/comment_controller.rb b/app/controllers/comment_controller.rb index a31fc18dbc..99347a3ed9 100644 --- a/app/controllers/comment_controller.rb +++ b/app/controllers/comment_controller.rb @@ -36,6 +36,22 @@ def create end end + def create_by_token + # Parse the user's token from the headers + + # Use logic similar to the above create controller but with the user that + # the token belongs to instead of the `current_user` + + # In order to avoid looking up the entire database for a user having a + # matching token, we could instead ask the user to pass username/id in the + # params along with the node id and comment body. + # This would also mean that if someone tries to bruteforce a token, they + # would to do so for each user individually. + + # If the username/id and token do not match, send an error in the same + # format as the request (JSON/XML) + end + # create answer comments def answer_create @answer_id = params[:aid] From 829fae92d338ea01ebe8f4bc4913d51dc52ca889 Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Sun, 9 Jul 2017 01:06:55 +0530 Subject: [PATCH 02/14] Add authorization --- app/controllers/comment_controller.rb | 29 ++++++++++++++++----------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/app/controllers/comment_controller.rb b/app/controllers/comment_controller.rb index 99347a3ed9..47ae79fd39 100644 --- a/app/controllers/comment_controller.rb +++ b/app/controllers/comment_controller.rb @@ -37,19 +37,24 @@ def create end def create_by_token - # Parse the user's token from the headers + @username = params[:username] + @node_id = params[:id] + @body = pararms[:body] + @token = request.headers["token"] - # Use logic similar to the above create controller but with the user that - # the token belongs to instead of the `current_user` - - # In order to avoid looking up the entire database for a user having a - # matching token, we could instead ask the user to pass username/id in the - # params along with the node id and comment body. - # This would also mean that if someone tries to bruteforce a token, they - # would to do so for each user individually. - - # If the username/id and token do not match, send an error in the same - # format as the request (JSON/XML) + @user = Users.find_by_username @username + if @user && @user.token == @token + # TODO: Do the usual commenting stuff + else + msg = { + :status => :unauthorized + :message => "Unauthorized" + } + respond_to do |format| + format.xml { render :xml => msg.to_xml } + format.json { render :json => msg.to_json } + end + end end # create answer comments From a095e8d03acfafa8ef8a53724e42a856bab47e3f Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Sun, 9 Jul 2017 01:20:35 +0530 Subject: [PATCH 03/14] Avoid ruby hash rocket syntax --- app/controllers/comment_controller.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/comment_controller.rb b/app/controllers/comment_controller.rb index 47ae79fd39..301482f108 100644 --- a/app/controllers/comment_controller.rb +++ b/app/controllers/comment_controller.rb @@ -47,12 +47,12 @@ def create_by_token # TODO: Do the usual commenting stuff else msg = { - :status => :unauthorized - :message => "Unauthorized" + status: :unauthorized + message: "Unauthorized" } respond_to do |format| - format.xml { render :xml => msg.to_xml } - format.json { render :json => msg.to_json } + format.xml { render xml: msg.to_xml } + format.json { render json: msg.to_json } end end end From 0ff8697bdcce4cbf840ac9c088ae85d2ab14c126 Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Sun, 9 Jul 2017 01:40:08 +0530 Subject: [PATCH 04/14] Facepalm --- app/controllers/comment_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/comment_controller.rb b/app/controllers/comment_controller.rb index 301482f108..f190c8b5bc 100644 --- a/app/controllers/comment_controller.rb +++ b/app/controllers/comment_controller.rb @@ -47,7 +47,7 @@ def create_by_token # TODO: Do the usual commenting stuff else msg = { - status: :unauthorized + status: :unauthorized, message: "Unauthorized" } respond_to do |format| From 195bc07eea2a502eab4f3cd86839f8f701a05d07 Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Sun, 9 Jul 2017 16:18:43 +0530 Subject: [PATCH 05/14] Start working on a private function for adding comments --- app/controllers/comment_controller.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/app/controllers/comment_controller.rb b/app/controllers/comment_controller.rb index f190c8b5bc..a9b072faaf 100644 --- a/app/controllers/comment_controller.rb +++ b/app/controllers/comment_controller.rb @@ -1,3 +1,12 @@ +def create_comment(node_id, user, body) + @node = Node.find node_id + @comment = @node.add_comment(uid: user.uid, body: body) + if user && @comment.save + @comment.notify user + return @comment + end +end + class CommentController < ApplicationController respond_to :html, :xml, :json before_filter :require_user, only: %i[create update delete] From 787134c487cce3a717491e64332943b4113dbfeb Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Sun, 9 Jul 2017 18:15:23 +0530 Subject: [PATCH 06/14] Finalize the original function using the private helper --- app/controllers/comment_controller.rb | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/app/controllers/comment_controller.rb b/app/controllers/comment_controller.rb index a9b072faaf..532b9c252d 100644 --- a/app/controllers/comment_controller.rb +++ b/app/controllers/comment_controller.rb @@ -1,9 +1,13 @@ -def create_comment(node_id, user, body) - @node = Node.find node_id - @comment = @node.add_comment(uid: user.uid, body: body) +class CommentError < ArgumentError +end + +def create_comment(node, user, body) + @comment = node.add_comment(uid: user.uid, body: body) if user && @comment.save @comment.notify user return @comment + else + raise CommentError.new() end end @@ -21,9 +25,11 @@ def index # create node comments def create @node = Node.find params[:id] - @comment = @node.add_comment(uid: current_user.uid, body: params[:body]) - if current_user && @comment.save - @comment.notify(current_user) + @body = params[:body] + @user = current_user + + begin + @comment = create_comment(@node, @user, @body) respond_with do |format| if params[:type] && params[:type] == 'question' @answer_id = 0 @@ -39,7 +45,7 @@ def create end end end - else + rescue CommentError flash[:error] = 'The comment could not be saved.' render text: 'failure' end From a5834f16d3549b0a3f6dcee89f6248cf1f1a2da7 Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Sun, 9 Jul 2017 22:41:34 +0530 Subject: [PATCH 07/14] Make comment_by_token method using the private function --- app/controllers/comment_controller.rb | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/app/controllers/comment_controller.rb b/app/controllers/comment_controller.rb index 532b9c252d..9a0625d53c 100644 --- a/app/controllers/comment_controller.rb +++ b/app/controllers/comment_controller.rb @@ -52,14 +52,32 @@ def create end def create_by_token - @username = params[:username] - @node_id = params[:id] + @node = Node.find params[:id] + @user = Users.find_by_username params[:username] @body = pararms[:body] @token = request.headers["token"] - @user = Users.find_by_username @username if @user && @user.token == @token - # TODO: Do the usual commenting stuff + begin + @comment = create_comment(@node, @user, @body) + msg = { + status: :created, + message: "Created" + } + respond_to do |format| + format.xml { render xml: msg.to_xml } + format.json { render json: msg.to_json } + end + rescue CommentError + msg = { + status: :bad_request, + message: "Bad Request" + } + respond_to do |format| + format.xml { render xml: msg.to_xml } + format.json { render json: msg.to_json } + end + end else msg = { status: :unauthorized, From d2846ac59febfbfb970c3c49e415f27c7085c608 Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Mon, 10 Jul 2017 01:08:47 +0530 Subject: [PATCH 08/14] Move CommentError and create_comment to CommentHelper --- app/controllers/comment_controller.rb | 15 ++------------- app/helpers/comment_helper.rb | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 13 deletions(-) create mode 100644 app/helpers/comment_helper.rb diff --git a/app/controllers/comment_controller.rb b/app/controllers/comment_controller.rb index 9a0625d53c..0cb4864b84 100644 --- a/app/controllers/comment_controller.rb +++ b/app/controllers/comment_controller.rb @@ -1,17 +1,6 @@ -class CommentError < ArgumentError -end - -def create_comment(node, user, body) - @comment = node.add_comment(uid: user.uid, body: body) - if user && @comment.save - @comment.notify user - return @comment - else - raise CommentError.new() - end -end - class CommentController < ApplicationController + include CommentHelper + respond_to :html, :xml, :json before_filter :require_user, only: %i[create update delete] diff --git a/app/helpers/comment_helper.rb b/app/helpers/comment_helper.rb new file mode 100644 index 0000000000..9d53134fc1 --- /dev/null +++ b/app/helpers/comment_helper.rb @@ -0,0 +1,14 @@ +module CommentHelper + class CommentError < ArgumentError + end + + def create_comment(node, user, body) + @comment = node.add_comment(uid: user.uid, body: body) + if user && @comment.save + @comment.notify user + return @comment + else + raise CommentError.new() + end + end +end From 917c90ebfadcc331178ab14fa32af8c3d5b973b0 Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Mon, 10 Jul 2017 02:09:24 +0530 Subject: [PATCH 09/14] Write first functional test; Introduce wierd 406 error --- app/controllers/comment_controller.rb | 6 +++--- config/routes.rb | 2 ++ test/functional/comment_controller_test.rb | 16 +++++++++++++++- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/app/controllers/comment_controller.rb b/app/controllers/comment_controller.rb index 0cb4864b84..fc3bcd3268 100644 --- a/app/controllers/comment_controller.rb +++ b/app/controllers/comment_controller.rb @@ -42,9 +42,9 @@ def create def create_by_token @node = Node.find params[:id] - @user = Users.find_by_username params[:username] - @body = pararms[:body] - @token = request.headers["token"] + @user = User.find_by_username params[:username] + @body = params[:body] + @token = request.headers['Token'] if @user && @user.token == @token begin diff --git a/config/routes.rb b/config/routes.rb index 4da07155c8..f3fc8012d1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -9,6 +9,8 @@ mount GrapeSwaggerRails::Engine => '/api/d1ocs' #end + # Manually written API functions + get '/api/comment', to: 'comment#create_by_token' resources :rusers resources :user_sessions diff --git a/test/functional/comment_controller_test.rb b/test/functional/comment_controller_test.rb index 3d809c34a0..c99b551d86 100644 --- a/test/functional/comment_controller_test.rb +++ b/test/functional/comment_controller_test.rb @@ -53,7 +53,7 @@ def teardown end assert_response :success assert_not_nil :comment - #assert_template partial: 'wiki/_comment' + #assert_template partial: 'wiki/_comment' #should uncomment above once comment displaying partial is implemented for wikis. end @@ -251,4 +251,18 @@ def teardown assert ActionMailer::Base.deliveries.collect(&:to).include?([rusers(:moderator).email]) # tag followers can be found in tag_selection.yml end + + test 'should post comment through token successfully' do + get :create_by_token, + { + id: 1, + body: 'Test Comment', + username: "Bob" + },{ + 'Accept' => 'application/json', + 'Content-Type' => 'application/json', + 'Token' => 'abcdefg12345' + } + assert_response :success + end end From 48ecaae38b436d00167108b0b8722ab94b05b894 Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Mon, 10 Jul 2017 19:31:49 +0530 Subject: [PATCH 10/14] Fix 406 issue --- app/controllers/comment_controller.rb | 8 ++++---- config/routes.rb | 6 +++--- test/functional/comment_controller_test.rb | 22 ++++++++++++---------- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/app/controllers/comment_controller.rb b/app/controllers/comment_controller.rb index fc3bcd3268..c1047c9347 100644 --- a/app/controllers/comment_controller.rb +++ b/app/controllers/comment_controller.rb @@ -44,14 +44,14 @@ def create_by_token @node = Node.find params[:id] @user = User.find_by_username params[:username] @body = params[:body] - @token = request.headers['Token'] + @token = request.env['rack.session']['token'] if @user && @user.token == @token begin @comment = create_comment(@node, @user, @body) msg = { status: :created, - message: "Created" + message: 'Created' } respond_to do |format| format.xml { render xml: msg.to_xml } @@ -60,7 +60,7 @@ def create_by_token rescue CommentError msg = { status: :bad_request, - message: "Bad Request" + message: 'Bad Request' } respond_to do |format| format.xml { render xml: msg.to_xml } @@ -70,7 +70,7 @@ def create_by_token else msg = { status: :unauthorized, - message: "Unauthorized" + message: 'Unauthorized' } respond_to do |format| format.xml { render xml: msg.to_xml } diff --git a/config/routes.rb b/config/routes.rb index f3fc8012d1..a530e74653 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -3,15 +3,15 @@ mount JasmineRails::Engine => '/specs' if defined?(JasmineRails) mount JasmineFixtureServer => '/spec/javascripts/fixtures' if defined?(Jasmine::Jquery::Rails::Engine) + # Manually written API functions + post '/bot/comment/id.:format', to: 'comment#create_by_token' + #Search RESTful endpoints #constraints(subdomain: 'api') do mount Srch::API => '/api' mount GrapeSwaggerRails::Engine => '/api/d1ocs' #end - # Manually written API functions - get '/api/comment', to: 'comment#create_by_token' - resources :rusers resources :user_sessions resources :images diff --git a/test/functional/comment_controller_test.rb b/test/functional/comment_controller_test.rb index c99b551d86..d200547559 100644 --- a/test/functional/comment_controller_test.rb +++ b/test/functional/comment_controller_test.rb @@ -253,16 +253,18 @@ def teardown end test 'should post comment through token successfully' do - get :create_by_token, - { - id: 1, - body: 'Test Comment', - username: "Bob" - },{ - 'Accept' => 'application/json', - 'Content-Type' => 'application/json', - 'Token' => 'abcdefg12345' - } + @params = { + id: 1, + body: "Test Comment", + username: "Bob", + format: 'json' + } + + @headers = { + "token" => "abcdefg12345" + } + + post :create_by_token, @params, @headers assert_response :success end end From f80878b333ff4289fd90a43179f4dd42540207c0 Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Mon, 10 Jul 2017 20:23:18 +0530 Subject: [PATCH 11/14] Add second test and improve responses --- app/controllers/comment_controller.rb | 21 +++--------------- test/functional/comment_controller_test.rb | 25 ++++++++++++++++++---- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/app/controllers/comment_controller.rb b/app/controllers/comment_controller.rb index c1047c9347..b394230a32 100644 --- a/app/controllers/comment_controller.rb +++ b/app/controllers/comment_controller.rb @@ -49,32 +49,17 @@ def create_by_token if @user && @user.token == @token begin @comment = create_comment(@node, @user, @body) - msg = { - status: :created, - message: 'Created' - } respond_to do |format| - format.xml { render xml: msg.to_xml } - format.json { render json: msg.to_json } + format.all { render :nothing => true, :status => :created } end rescue CommentError - msg = { - status: :bad_request, - message: 'Bad Request' - } respond_to do |format| - format.xml { render xml: msg.to_xml } - format.json { render json: msg.to_json } + format.all { render :nothing => true, :status => :bad_request } end end else - msg = { - status: :unauthorized, - message: 'Unauthorized' - } respond_to do |format| - format.xml { render xml: msg.to_xml } - format.json { render json: msg.to_json } + format.all { render :nothing => true, :status => :unauthorized } end end end diff --git a/test/functional/comment_controller_test.rb b/test/functional/comment_controller_test.rb index d200547559..3411ccbcd5 100644 --- a/test/functional/comment_controller_test.rb +++ b/test/functional/comment_controller_test.rb @@ -252,19 +252,36 @@ def teardown # tag followers can be found in tag_selection.yml end - test 'should post comment through token successfully' do + test 'should post comment through valid token successfully' do @params = { id: 1, - body: "Test Comment", - username: "Bob", + body: 'Test Comment', + username: 'Bob', format: 'json' } @headers = { - "token" => "abcdefg12345" + 'token' => 'invalid-token' + } + + post :create_by_token, @params, @headers + assert_response :unauthorized + end + + test 'should not post comment through invalid token successfully' do + @params = { + id: 1, + body: 'Test Comment', + username: 'Bob', + format: 'json' + } + + @headers = { + 'token' => 'abcdefg12345' } post :create_by_token, @params, @headers assert_response :success end + end From 61bca004d1cf5b25e1d58f149bb3fc8d8483edc6 Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Mon, 10 Jul 2017 20:25:54 +0530 Subject: [PATCH 12/14] Fix test names --- test/functional/comment_controller_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/comment_controller_test.rb b/test/functional/comment_controller_test.rb index 3411ccbcd5..e0db35ccfc 100644 --- a/test/functional/comment_controller_test.rb +++ b/test/functional/comment_controller_test.rb @@ -252,7 +252,7 @@ def teardown # tag followers can be found in tag_selection.yml end - test 'should post comment through valid token successfully' do + test 'should not post comment through invalid token successfully' do @params = { id: 1, body: 'Test Comment', @@ -268,7 +268,7 @@ def teardown assert_response :unauthorized end - test 'should not post comment through invalid token successfully' do + test 'should post comment through valid token successfully' do @params = { id: 1, body: 'Test Comment', From e513cdd1145c3e616133d5d1135394de92699411 Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Mon, 10 Jul 2017 20:36:28 +0530 Subject: [PATCH 13/14] Add test for invalid comment --- test/functional/comment_controller_test.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/functional/comment_controller_test.rb b/test/functional/comment_controller_test.rb index e0db35ccfc..0168465aa9 100644 --- a/test/functional/comment_controller_test.rb +++ b/test/functional/comment_controller_test.rb @@ -268,6 +268,22 @@ def teardown assert_response :unauthorized end + test 'should not post an invalid comment successfully' do + @params = { + id: 1, + body: '', + username: 'Bob', + format: 'json' + } + + @headers = { + 'token' => 'abcdefg12345' + } + + post :create_by_token, @params, @headers + assert_response :bad_request + end + test 'should post comment through valid token successfully' do @params = { id: 1, From 1a9c61e1243a077c23e2e1ab1e530b6d61200139 Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Tue, 11 Jul 2017 01:24:52 +0530 Subject: [PATCH 14/14] Add recommendations --- app/controllers/comment_controller.rb | 3 +++ config/routes.rb | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app/controllers/comment_controller.rb b/app/controllers/comment_controller.rb index b394230a32..d21b86796b 100644 --- a/app/controllers/comment_controller.rb +++ b/app/controllers/comment_controller.rb @@ -48,6 +48,9 @@ def create_by_token if @user && @user.token == @token begin + # The create_comment is a function that has been defined inside the + # CommentHelper module inside app/helpers/comment_helper.rb and can be + # used in here because the module was `include`d right at the beginning @comment = create_comment(@node, @user, @body) respond_to do |format| format.all { render :nothing => true, :status => :created } diff --git a/config/routes.rb b/config/routes.rb index a530e74653..7a27b0345d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -4,7 +4,7 @@ mount JasmineFixtureServer => '/spec/javascripts/fixtures' if defined?(Jasmine::Jquery::Rails::Engine) # Manually written API functions - post '/bot/comment/id.:format', to: 'comment#create_by_token' + post '/comment/create/token/id.:format', to: 'comment#create_by_token' #Search RESTful endpoints #constraints(subdomain: 'api') do