From: Tom Hughes Date: Wed, 16 Jan 2019 10:23:27 +0000 (+0000) Subject: Merge remote-tracking branch 'upstream/pull/2116' X-Git-Tag: live~3635 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/11806a676fb88f51ece004b7c05bde38f1e77706?hp=3e49e4a62ad9ccce7a193ab0393a7722896455aa Merge remote-tracking branch 'upstream/pull/2116' --- diff --git a/Gemfile b/Gemfile index f75921f12..3cf075045 100644 --- a/Gemfile +++ b/Gemfile @@ -77,7 +77,7 @@ gem "omniauth-openid" gem "omniauth-windowslive" # Markdown formatting support -gem "redcarpet" +gem "kramdown" # For status transitions of Issues gem "aasm" diff --git a/Gemfile.lock b/Gemfile.lock index 2aba9c21b..3b70d157a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -161,6 +161,7 @@ GEM jsonify (< 0.4.0) jwt (2.1.0) kgio (2.11.2) + kramdown (1.17.0) libv8 (3.16.14.19) libxml-ruby (3.1.0) listen (3.1.5) @@ -303,7 +304,6 @@ GEM ffi (~> 1.0) record_tag_helper (1.0.0) actionview (~> 5.x) - redcarpet (3.4.0) ref (2.0.0) request_store (1.4.1) rack (>= 1.4) @@ -412,6 +412,7 @@ DEPENDENCIES json jsonify-rails kgio + kramdown libxml-ruby (>= 2.0.5) listen logstasher @@ -438,7 +439,6 @@ DEPENDENCIES rails-controller-testing rails-i18n (~> 4.0.0) record_tag_helper - redcarpet rinku (>= 1.2.2) rotp rubocop diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb index a3700b305..90f377931 100644 --- a/app/abilities/ability.rb +++ b/app/abilities/ability.rb @@ -4,6 +4,7 @@ class Ability include CanCan::Ability def initialize(user) + can [:trackpoints, :map, :changes, :capabilities, :permissions], :api can [:relation, :relation_history, :way, :way_history, :node, :node_history, :changeset, :note, :new_note, :query], :browse can [:index, :feed, :read, :download, :query], Changeset @@ -22,6 +23,12 @@ class Ability can [:index, :show, :data, :georss, :picture, :icon], Trace can [:terms, :api_users, :login, :logout, :new, :create, :save, :confirm, :confirm_resend, :confirm_email, :lost_password, :reset_password, :show, :api_read, :auth_success, :auth_failure], User can [:index, :show, :blocks_on, :blocks_by], UserBlock + can [:read, :nodes], Node + can [:read, :full, :ways, :ways_for_node], Way + can [:read, :full, :relations, :relations_for_node, :relations_for_way, :relations_for_relation], Relation + can [:history, :version], OldNode + can [:history, :version], OldWay + can [:history, :version], OldRelation if user can :welcome, :site @@ -38,6 +45,9 @@ class Ability if user.terms_agreed? || !REQUIRE_TERMS_AGREED can [:create, :update, :upload, :close, :subscribe, :unsubscribe, :expand_bbox], Changeset can :create, ChangesetComment + can [:create, :update, :delete], Node + can [:create, :update, :delete], Way + can [:create, :update, :delete], Relation end if user.moderator? @@ -47,6 +57,11 @@ class Ability can :destroy, Note can [:new, :create, :edit, :update, :destroy], Redaction can [:new, :edit, :create, :update, :revoke], UserBlock + if user.terms_agreed? || !REQUIRE_TERMS_AGREED + can :redact, OldNode + can :redact, OldWay + can :redact, OldRelation + end end if user.administrator? diff --git a/app/abilities/capability.rb b/app/abilities/capability.rb index 556d4036c..3d951900b 100644 --- a/app/abilities/capability.rb +++ b/app/abilities/capability.rb @@ -15,11 +15,19 @@ class Capability if token&.user&.terms_agreed? || !REQUIRE_TERMS_AGREED can [:create, :update, :upload, :close, :subscribe, :unsubscribe, :expand_bbox], Changeset if capability?(token, :allow_write_api) can :create, ChangesetComment if capability?(token, :allow_write_api) + can [:create, :update, :delete], Node if capability?(token, :allow_write_api) + can [:create, :update, :delete], Way if capability?(token, :allow_write_api) + can [:create, :update, :delete], Relation if capability?(token, :allow_write_api) end if token&.user&.moderator? can [:destroy, :restore], ChangesetComment if capability?(token, :allow_write_api) can :destroy, Note if capability?(token, :allow_write_notes) + if token&.user&.terms_agreed? || !REQUIRE_TERMS_AGREED + can :redact, OldNode if capability?(token, :allow_write_api) + can :redact, OldWay if capability?(token, :allow_write_api) + can :redact, OldRelation if capability?(token, :allow_write_api) + end end end diff --git a/app/assets/stylesheets/common.scss b/app/assets/stylesheets/common.scss index d36e77285..d725cc287 100644 --- a/app/assets/stylesheets/common.scss +++ b/app/assets/stylesheets/common.scss @@ -2342,11 +2342,11 @@ a.button { margin-left: $lineheight; } - ul li { + ul > li { list-style: disc; } - ol li { + ol > li { list-style: decimal; } } diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index 90883376c..3273665d2 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -1,5 +1,9 @@ class ApiController < ApplicationController skip_before_action :verify_authenticity_token + before_action :api_deny_access_handler + + authorize_resource :class => false + before_action :check_api_readable, :except => [:capabilities] before_action :setup_user_auth, :only => [:permissions] around_action :api_call_handle_error, :api_call_timeout diff --git a/app/controllers/nodes_controller.rb b/app/controllers/nodes_controller.rb index baa6d8195..65f9b27ed 100644 --- a/app/controllers/nodes_controller.rb +++ b/app/controllers/nodes_controller.rb @@ -5,7 +5,10 @@ class NodesController < ApplicationController skip_before_action :verify_authenticity_token before_action :authorize, :only => [:create, :update, :delete] - before_action :require_allow_write_api, :only => [:create, :update, :delete] + before_action :api_deny_access_handler + + authorize_resource + before_action :require_public_data, :only => [:create, :update, :delete] before_action :check_api_writable, :only => [:create, :update, :delete] before_action :check_api_readable, :except => [:create, :update, :delete] diff --git a/app/controllers/old_controller.rb b/app/controllers/old_controller.rb index 4f01b1e2a..74fe0883b 100644 --- a/app/controllers/old_controller.rb +++ b/app/controllers/old_controller.rb @@ -6,9 +6,11 @@ class OldController < ApplicationController skip_before_action :verify_authenticity_token before_action :setup_user_auth, :only => [:history, :version] + before_action :api_deny_access_handler before_action :authorize, :only => [:redact] - before_action :authorize_moderator, :only => [:redact] - before_action :require_allow_write_api, :only => [:redact] + + authorize_resource + before_action :check_api_readable before_action :check_api_writable, :only => [:redact] around_action :api_call_handle_error, :api_call_timeout diff --git a/app/controllers/relations_controller.rb b/app/controllers/relations_controller.rb index b9108cea1..27bf5cdf8 100644 --- a/app/controllers/relations_controller.rb +++ b/app/controllers/relations_controller.rb @@ -3,7 +3,10 @@ class RelationsController < ApplicationController skip_before_action :verify_authenticity_token before_action :authorize, :only => [:create, :update, :delete] - before_action :require_allow_write_api, :only => [:create, :update, :delete] + before_action :api_deny_access_handler + + authorize_resource + before_action :require_public_data, :only => [:create, :update, :delete] before_action :check_api_writable, :only => [:create, :update, :delete] before_action :check_api_readable, :except => [:create, :update, :delete] @@ -148,6 +151,8 @@ class RelationsController < ApplicationController relations_for_object("Relation") end + private + def relations_for_object(objtype) relationids = RelationMember.where(:member_type => objtype, :member_id => params[:id]).collect(&:relation_id).uniq diff --git a/app/controllers/ways_controller.rb b/app/controllers/ways_controller.rb index 39129ebf3..85d9b5a5b 100644 --- a/app/controllers/ways_controller.rb +++ b/app/controllers/ways_controller.rb @@ -3,7 +3,10 @@ class WaysController < ApplicationController skip_before_action :verify_authenticity_token before_action :authorize, :only => [:create, :update, :delete] - before_action :require_allow_write_api, :only => [:create, :update, :delete] + before_action :api_deny_access_handler + + authorize_resource + before_action :require_public_data, :only => [:create, :update, :delete] before_action :check_api_writable, :only => [:create, :update, :delete] before_action :check_api_readable, :except => [:create, :update, :delete] diff --git a/config/initializers/sanitize.rb b/config/initializers/sanitize.rb index 240f1e315..c7b7b3326 100644 --- a/config/initializers/sanitize.rb +++ b/config/initializers/sanitize.rb @@ -1,5 +1,5 @@ Sanitize::Config::OSM = Sanitize::Config::RELAXED.dup Sanitize::Config::OSM[:elements] -= %w[div style] -Sanitize::Config::OSM[:add_attributes] = { "a" => { "rel" => "nofollow" } } +Sanitize::Config::OSM[:add_attributes] = { "a" => { "rel" => "nofollow noopener noreferer" } } Sanitize::Config::OSM[:remove_contents] = %w[script style] diff --git a/config/locales/en.yml b/config/locales/en.yml index e87e8f8ee..bbcba0f2c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1626,7 +1626,7 @@ en: edit: Edit preview: Preview markdown_help: - title_html: Parsed with Markdown + title_html: Parsed with kramdown headings: Headings heading: Heading subheading: Subheading diff --git a/lib/rich_text.rb b/lib/rich_text.rb index d0539b2b0..2b3e07d6a 100644 --- a/lib/rich_text.rb +++ b/lib/rich_text.rb @@ -55,11 +55,15 @@ module RichText SimpleFormat.new.simple_format(text) end - def linkify(text) + def sanitize(text) + Sanitize.clean(text, Sanitize::Config::OSM).html_safe + end + + def linkify(text, mode = :urls) if text.html_safe? - Rinku.auto_link(text, :urls, tag_builder.tag_options(:rel => "nofollow")).html_safe + Rinku.auto_link(text, mode, tag_builder.tag_options(:rel => "nofollow noopener noreferer")).html_safe else - Rinku.auto_link(text, :urls, tag_builder.tag_options(:rel => "nofollow")) + Rinku.auto_link(text, mode, tag_builder.tag_options(:rel => "nofollow noopener noreferer")) end end end @@ -72,30 +76,16 @@ module RichText def to_text to_s end - - private - - def sanitize(text) - Sanitize.clean(text, Sanitize::Config::OSM).html_safe - end end class Markdown < Base def to_html - Markdown.html_parser.render(self).html_safe + linkify(sanitize(Kramdown::Document.new(self).to_html), :all) end def to_text to_s end - - def self.html_renderer - @html_renderer ||= Redcarpet::Render::XHTML.new(:filter_html => true, :safe_links_only => true, :link_attributes => { :rel => "nofollow" }) - end - - def self.html_parser - @html_parser ||= Redcarpet::Markdown.new(html_renderer, :no_intra_emphasis => true, :autolink => true, :space_after_headers => true) - end end class Text < Base diff --git a/test/lib/rich_text_test.rb b/test/lib/rich_text_test.rb index 74d396b68..e1603fb09 100644 --- a/test/lib/rich_text_test.rb +++ b/test/lib/rich_text_test.rb @@ -8,14 +8,14 @@ class RichTextTest < ActiveSupport::TestCase assert_html r do assert_select "a", 1 assert_select "a[href='http://example.com/']", 1 - assert_select "a[rel='nofollow']", 1 + assert_select "a[rel='nofollow noopener noreferer']", 1 end r = RichText.new("html", "foo bar baz") assert_html r do assert_select "a", 1 assert_select "a[href='http://example.com/']", 1 - assert_select "a[rel='nofollow']", 1 + assert_select "a[rel='nofollow noopener noreferer']", 1 end r = RichText.new("html", "foo example@example.com bar") @@ -27,7 +27,7 @@ class RichTextTest < ActiveSupport::TestCase assert_html r do assert_select "a", 1 assert_select "a[href='mailto:example@example.com']", 1 - assert_select "a[rel='nofollow']", 1 + assert_select "a[rel='nofollow noopener noreferer']", 1 end r = RichText.new("html", "foo
bar
baz") @@ -64,28 +64,28 @@ class RichTextTest < ActiveSupport::TestCase assert_html r do assert_select "a", 1 assert_select "a[href='http://example.com/']", 1 - assert_select "a[rel='nofollow']", 1 + assert_select "a[rel='nofollow noopener noreferer']", 1 end r = RichText.new("markdown", "foo [bar](http://example.com/) baz") assert_html r do assert_select "a", 1 assert_select "a[href='http://example.com/']", 1 - assert_select "a[rel='nofollow']", 1 + assert_select "a[rel='nofollow noopener noreferer']", 1 end r = RichText.new("markdown", "foo example@example.com bar") assert_html r do assert_select "a", 1 assert_select "a[href='mailto:example@example.com']", 1 - assert_select "a[rel='nofollow']", 1 + assert_select "a[rel='nofollow noopener noreferer']", 1 end r = RichText.new("markdown", "foo [bar](mailto:example@example.com) bar") assert_html r do assert_select "a", 1 assert_select "a[href='mailto:example@example.com']", 1 - assert_select "a[rel='nofollow']", 1 + assert_select "a[rel='nofollow noopener noreferer']", 1 end r = RichText.new("markdown", "foo ![bar](http://example.com/example.png) bar") @@ -162,7 +162,7 @@ class RichTextTest < ActiveSupport::TestCase assert_html r do assert_select "a", 1 assert_select "a[href='http://example.com/']", 1 - assert_select "a[rel='nofollow']", 1 + assert_select "a[rel='nofollow noopener noreferer']", 1 end r = RichText.new("text", "foo example@example.com bar")