From 1aa0e35a7a3d9a4ff8090f5a618babbd2a452757 Mon Sep 17 00:00:00 2001
From: Frederik Ramm <frederik@remote.org>
Date: Tue, 17 Jul 2018 12:41:49 +0200
Subject: [PATCH 1/2] do not allow anonymous users to comment on notes

---
 app/controllers/notes_controller.rb |  2 +-
 app/views/browse/note.html.erb      | 14 +++++++-------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb
index 853072b7bb..0f0e30f202 100644
--- a/app/controllers/notes_controller.rb
+++ b/app/controllers/notes_controller.rb
@@ -5,7 +5,7 @@ class NotesController < ApplicationController
   before_action :check_api_readable
   before_action :authorize_web, :only => [:mine]
   before_action :setup_user_auth, :only => [:create, :comment]
-  before_action :authorize, :only => [:close, :reopen, :destroy]
+  before_action :authorize, :only => [:close, :reopen, :destroy, :comment]
   before_action :require_moderator, :only => [:destroy]
   before_action :check_api_writable, :only => [:create, :comment, :close, :reopen, :destroy]
   before_action :require_allow_write_notes, :only => [:create, :comment, :close, :reopen, :destroy]
diff --git a/app/views/browse/note.html.erb b/app/views/browse/note.html.erb
index 53ea0759e9..3032d94063 100644
--- a/app/views/browse/note.html.erb
+++ b/app/views/browse/note.html.erb
@@ -41,18 +41,18 @@
   <% end %>
 
   <% if @note.status == "open" %>
-    <form action="#">
-      <textarea class="comment" name="text" cols="40" rows="5"></textarea>
-      <div class="buttons clearfix">
-        <% if current_user and current_user.moderator? -%>
+    <% if current_user -%>
+      <form action="#">
+        <textarea class="comment" name="text" cols="40" rows="5"></textarea>
+        <div class="buttons clearfix">
+        <% if current_user.moderator? -%>
           <input type="submit" name="hide" value="<%= t('javascripts.notes.show.hide') %>" class="deemphasize" data-note-id="<%= @note.id %>" data-method="DELETE" data-url="<%= note_url(@note, 'json') %>">
         <% end -%>
-        <% if current_user -%>
-          <input type="submit" name="close" value="<%= t('javascripts.notes.show.resolve') %>" data-note-id="<%= @note.id %>" data-method="POST" data-url="<%= close_note_url(@note, 'json') %>">
-        <% end -%>
+        <input type="submit" name="close" value="<%= t('javascripts.notes.show.resolve') %>" data-note-id="<%= @note.id %>" data-method="POST" data-url="<%= close_note_url(@note, 'json') %>">
         <input type="submit" name="comment" value="<%= t('javascripts.notes.show.comment') %>" data-note-id="<%= @note.id %>" data-method="POST" data-url="<%= comment_note_url(@note, 'json') %>" disabled="1">
       </div>
     </form>
+    <% end -%>
   <% else %>
     <form action="#">
       <input type="hidden" name="text" value="">

From e71d7e8bea5b4f2b67fe4a6bee94a24bacbcb3e5 Mon Sep 17 00:00:00 2001
From: Frederik Ramm <frederik@remote.org>
Date: Wed, 18 Jul 2018 13:29:16 +0200
Subject: [PATCH 2/2] fixed tests to work with new, non-anonymous note comments

---
 test/controllers/notes_controller_test.rb | 58 ++++++-----------------
 1 file changed, 15 insertions(+), 43 deletions(-)

diff --git a/test/controllers/notes_controller_test.rb b/test/controllers/notes_controller_test.rb
index 4444a2f50b..8fe5d382fc 100644
--- a/test/controllers/notes_controller_test.rb
+++ b/test/controllers/notes_controller_test.rb
@@ -226,6 +226,8 @@ def test_create_fail
 
   def test_comment_success
     open_note_with_comment = create(:note_with_comments)
+    user = create(:user)
+    basic_authorization user.email, "test"
     assert_difference "NoteComment.count", 1 do
       assert_no_difference "ActionMailer::Base.deliveries.size" do
         post :comment, :params => { :id => open_note_with_comment.id, :text => "This is an additional comment", :format => "json" }
@@ -240,7 +242,7 @@ def test_comment_success
     assert_equal 2, js["properties"]["comments"].count
     assert_equal "commented", js["properties"]["comments"].last["action"]
     assert_equal "This is an additional comment", js["properties"]["comments"].last["text"]
-    assert_nil js["properties"]["comments"].last["user"]
+    assert_equal user.display_name, js["properties"]["comments"].last["user"]
 
     get :show, :params => { :id => open_note_with_comment.id, :format => "json" }
     assert_response :success
@@ -252,7 +254,7 @@ def test_comment_success
     assert_equal 2, js["properties"]["comments"].count
     assert_equal "commented", js["properties"]["comments"].last["action"]
     assert_equal "This is an additional comment", js["properties"]["comments"].last["text"]
-    assert_nil js["properties"]["comments"].last["user"]
+    assert_equal user.display_name, js["properties"]["comments"].last["user"]
 
     # Ensure that emails are sent to users
     first_user = create(:user)
@@ -263,45 +265,6 @@ def test_comment_success
       create(:note_comment, :note => note, :author => first_user)
       create(:note_comment, :note => note, :author => second_user)
     end
-    assert_difference "NoteComment.count", 1 do
-      assert_difference "ActionMailer::Base.deliveries.size", 2 do
-        post :comment, :params => { :id => note_with_comments_by_users.id, :text => "This is an additional comment", :format => "json" }
-      end
-    end
-    assert_response :success
-    js = ActiveSupport::JSON.decode(@response.body)
-    assert_not_nil js
-    assert_equal "Feature", js["type"]
-    assert_equal note_with_comments_by_users.id, js["properties"]["id"]
-    assert_equal "open", js["properties"]["status"]
-    assert_equal 3, js["properties"]["comments"].count
-    assert_equal "commented", js["properties"]["comments"].last["action"]
-    assert_equal "This is an additional comment", js["properties"]["comments"].last["text"]
-    assert_nil js["properties"]["comments"].last["user"]
-
-    email = ActionMailer::Base.deliveries.find { |e| e.to.first == first_user.email }
-    assert_not_nil email
-    assert_equal 1, email.to.length
-    assert_equal "[OpenStreetMap] An anonymous user has commented on one of your notes", email.subject
-
-    email = ActionMailer::Base.deliveries.find { |e| e.to.first == second_user.email }
-    assert_not_nil email
-    assert_equal 1, email.to.length
-    assert_equal "[OpenStreetMap] An anonymous user has commented on a note you are interested in", email.subject
-
-    get :show, :params => { :id => note_with_comments_by_users.id, :format => "json" }
-    assert_response :success
-    js = ActiveSupport::JSON.decode(@response.body)
-    assert_not_nil js
-    assert_equal "Feature", js["type"]
-    assert_equal note_with_comments_by_users.id, js["properties"]["id"]
-    assert_equal "open", js["properties"]["status"]
-    assert_equal 3, js["properties"]["comments"].count
-    assert_equal "commented", js["properties"]["comments"].last["action"]
-    assert_equal "This is an additional comment", js["properties"]["comments"].last["text"]
-    assert_nil js["properties"]["comments"].last["user"]
-
-    ActionMailer::Base.deliveries.clear
 
     basic_authorization third_user.email, "test"
 
@@ -316,7 +279,7 @@ def test_comment_success
     assert_equal "Feature", js["type"]
     assert_equal note_with_comments_by_users.id, js["properties"]["id"]
     assert_equal "open", js["properties"]["status"]
-    assert_equal 4, js["properties"]["comments"].count
+    assert_equal 3, js["properties"]["comments"].count
     assert_equal "commented", js["properties"]["comments"].last["action"]
     assert_equal "This is an additional comment", js["properties"]["comments"].last["text"]
     assert_equal third_user.display_name, js["properties"]["comments"].last["user"]
@@ -339,7 +302,7 @@ def test_comment_success
     assert_equal "Feature", js["type"]
     assert_equal note_with_comments_by_users.id, js["properties"]["id"]
     assert_equal "open", js["properties"]["status"]
-    assert_equal 4, js["properties"]["comments"].count
+    assert_equal 3, js["properties"]["comments"].count
     assert_equal "commented", js["properties"]["comments"].last["action"]
     assert_equal "This is an additional comment", js["properties"]["comments"].last["text"]
     assert_equal third_user.display_name, js["properties"]["comments"].last["user"]
@@ -350,6 +313,15 @@ def test_comment_success
   def test_comment_fail
     open_note_with_comment = create(:note_with_comments)
 
+    user = create(:user)
+
+    assert_no_difference "NoteComment.count" do
+      post :comment, :params => { :text => "This is an additional comment" }
+      assert_response :unauthorized
+    end
+
+    basic_authorization user.email, "test"
+
     assert_no_difference "NoteComment.count" do
       post :comment, :params => { :text => "This is an additional comment" }
     end