From: Tom Hughes Date: Thu, 28 Mar 2019 08:24:04 +0000 (+0000) Subject: Merge remote-tracking branch 'upstream/pull/2190' X-Git-Tag: live~2667 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/0c79686f3cef86330d0ab6bbef2a9e2275ac93ac?hp=c44d689b313d11720aa528cb904db57a1e93143a Merge remote-tracking branch 'upstream/pull/2190' --- diff --git a/.erb-lint.yml b/.erb-lint.yml index 29057feb2..c78bcd178 100644 --- a/.erb-lint.yml +++ b/.erb-lint.yml @@ -3,7 +3,7 @@ linters: SelfClosingTag: enabled: false SpaceInHtmlTag: - enabled: false # TODO + enabled: true Rubocop: enabled: true rubocop_config: @@ -25,33 +25,5 @@ linters: Enabled: false Rails/OutputSafety: Enabled: false - Style/StringLiterals: - Enabled: false # TODO - Style/BracesAroundHashParameters: - Enabled: false # TODO - Rails/DynamicFindBy: - Enabled: false # TODO - Style/AndOr: - Enabled: false # TODO - Style/WordArray: - Enabled: false # TODO - Style/MethodCallWithoutArgsParentheses: - Enabled: false # TODO - Layout/LeadingBlankLines: - Enabled: false # TODO - Style/NestedParenthesizedCalls: - Enabled: false # TODO - Rails/LinkToBlank: - Enabled: false # TODO - Style/HashSyntax: - Enabled: false # TODO - Style/SymbolProc: - Enabled: false # TODO - Style/Not: - Enabled: false # TODO - Style/NegatedIf: - Enabled: false # TODO - Style/ConditionalAssignment: - Enabled: false # TODO exclude: - '**/vendor/**' diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb index 7e8e921a2..d2864e452 100644 --- a/app/abilities/ability.rb +++ b/app/abilities/ability.rb @@ -6,28 +6,21 @@ class Ability def initialize(user) can [:relation, :relation_history, :way, :way_history, :node, :node_history, :changeset, :note, :new_note, :query], :browse - can :show, :capability - can :index, :change can :search, :direction can [:index, :permalink, :edit, :help, :fixthemap, :offline, :export, :about, :preview, :copyright, :key, :id], :site can [:finish, :embed], :export can [:search, :search_latlon, :search_ca_postcode, :search_osm_nominatim, :search_geonames, :search_osm_nominatim_reverse, :search_geonames_reverse], :geocoder - can :index, :map can [:token, :request_token, :access_token, :test_request], :oauth - can :show, :permission - can [:search_all, :search_nodes, :search_ways, :search_relations], :search - can [:trackpoints], :swf if Settings.status != "database_offline" - can [:index, :feed, :show, :download, :query], Changeset + can [:index, :feed], Changeset can :index, ChangesetComment can [:index, :rss, :show, :comments], DiaryEntry - can [:index, :create, :comment, :feed, :show, :search, :mine], Note + can [:mine], Note can [:index, :show], Redaction can [:index, :show, :data, :georss, :picture, :icon], Trace - can :index, Tracepoint - 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 [:terms, :login, :logout, :new, :create, :save, :confirm, :confirm_resend, :confirm_email, :lost_password, :reset_password, :show, :auth_success, :auth_failure], User can [:index, :show, :blocks_on, :blocks_by], UserBlock can [:index, :show], Node can [:index, :show, :full, :ways_for_node], Way @@ -47,31 +40,14 @@ class Ability can [:new, :create, :reply, :show, :inbox, :outbox, :mark, :destroy], Message can [:close, :reopen], Note can [:new, :create], Report - can [:mine, :new, :create, :edit, :update, :delete, :api_create, :api_read, :api_update, :api_delete, :api_data], Trace - can [:account, :go_public, :make_friend, :remove_friend, :api_details, :api_gpx_files], User - can [:read, :read_one, :update, :update_one, :delete_one], UserPreference - - if user.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 + can [:mine, :new, :create, :edit, :update, :delete], Trace + can [:account, :go_public, :make_friend, :remove_friend], User if user.moderator? - can [:destroy, :restore], ChangesetComment can [:index, :show, :resolve, :ignore, :reopen], Issue can :create, IssueComment - can :destroy, Note can [:new, :create, :edit, :update, :destroy], Redaction can [:new, :edit, :create, :update, :revoke], UserBlock - - if user.terms_agreed? - can :redact, OldNode - can :redact, OldWay - can :redact, OldRelation - end end if user.administrator? diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb new file mode 100644 index 000000000..54bc8fb4c --- /dev/null +++ b/app/abilities/api_ability.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +class ApiAbility + include CanCan::Ability + + def initialize(user) + can :show, :capability + can :index, :change + can :index, :map + can :show, :permission + can [:search_all, :search_nodes, :search_ways, :search_relations], :search + can [:trackpoints], :swf + + if Settings.status != "database_offline" + can [:show, :download, :query], Changeset + can [:index, :create, :comment, :feed, :show, :search], Note + can :index, Tracepoint + can [:api_users, :api_read], User + can [:index, :show], Node + can [:index, :show, :full, :ways_for_node], Way + can [:index, :show, :full, :relations_for_node, :relations_for_way, :relations_for_relation], Relation + can [:history, :version], OldNode + can [:history, :version], OldWay + can [:history, :version], OldRelation + end + + if user + can :welcome, :site + can [:revoke, :authorize], :oauth + + if Settings.status != "database_offline" + can [:index, :new, :create, :show, :edit, :update, :destroy], ClientApplication + can [:new, :create, :reply, :show, :inbox, :outbox, :mark, :destroy], Message + can [:close, :reopen], Note + can [:new, :create], Report + can [:api_create, :api_read, :api_update, :api_delete, :api_data], Trace + can [:api_details, :api_gpx_files], User + can [:read, :read_one, :update, :update_one, :delete_one], UserPreference + + if user.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? + can [:destroy, :restore], ChangesetComment + can :destroy, Note + + if user.terms_agreed? + can :redact, OldNode + can :redact, OldWay + can :redact, OldRelation + end + end + end + end + + # Define abilities for the passed in user here. For example: + # + # user ||= User.new # guest user (not logged in) + # if user.admin? + # can :manage, :all + # else + # can :read, :all + # end + # + # The first argument to `can` is the action you are giving the user + # permission to do. + # If you pass :manage it will apply to every action. Other common actions + # here are :read, :create, :update and :destroy. + # + # The second argument is the resource the user can perform the action on. + # If you pass :all it will apply to every resource. Otherwise pass a Ruby + # class of the resource. + # + # The third argument is an optional hash of conditions to further filter the + # objects. + # For example, here the user can only update published articles. + # + # can :update, Article, :published => true + # + # See the wiki for details: + # https://github.com/CanCanCommunity/cancancan/wiki/Defining-Abilities + end +end diff --git a/app/abilities/capability.rb b/app/abilities/api_capability.rb similarity index 98% rename from app/abilities/capability.rb rename to app/abilities/api_capability.rb index f4c24e97d..9f59b1f24 100644 --- a/app/abilities/capability.rb +++ b/app/abilities/api_capability.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class Capability +class ApiCapability include CanCan::Ability def initialize(token) diff --git a/app/controllers/api/traces_controller.rb b/app/controllers/api/traces_controller.rb index 47dd152a3..5e95892e1 100644 --- a/app/controllers/api/traces_controller.rb +++ b/app/controllers/api/traces_controller.rb @@ -44,6 +44,7 @@ module Api if trace.user == current_user trace.visible = false trace.save! + TraceDestroyerJob.perform_later(trace) if Settings.trace_use_job_queue head :ok else @@ -84,6 +85,7 @@ module Api trace = do_create(params[:file], tags, description, visibility) if trace.id + TraceImporterJob.perform_later(@trace) if Settings.trace_use_job_queue render :plain => trace.id.to_s elsif trace.valid? head :internal_server_error diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index 579af27cf..df7cfe93b 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -16,6 +16,15 @@ class ApiController < ApplicationController end end + def current_ability + # Use capabilities from the oauth token if it exists and is a valid access token + if Authenticator.new(self, [:token]).allow? + ApiAbility.new(nil).merge(ApiCapability.new(current_token)) + else + ApiAbility.new(current_user) + end + end + def deny_access(_exception) if current_token set_locale diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 3ab09b63d..c880e6add 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -329,12 +329,7 @@ class ApplicationController < ActionController::Base end def current_ability - # Use capabilities from the oauth token if it exists and is a valid access token - if Authenticator.new(self, [:token]).allow? - Ability.new(nil).merge(Capability.new(current_token)) - else - Ability.new(current_user) - end + Ability.new(current_user) end def deny_access(_exception) diff --git a/app/controllers/changesets_controller.rb b/app/controllers/changesets_controller.rb index 32d9fd733..3b20130d0 100644 --- a/app/controllers/changesets_controller.rb +++ b/app/controllers/changesets_controller.rb @@ -4,7 +4,6 @@ class ChangesetsController < ApplicationController layout "site" require "xml/libxml" - skip_before_action :verify_authenticity_token, :except => [:index] before_action :authorize_web before_action :set_locale before_action -> { check_database_readable(true) }, :only => [:index, :feed] diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb index 54fdb2810..7c6b033ca 100644 --- a/app/controllers/notes_controller.rb +++ b/app/controllers/notes_controller.rb @@ -16,13 +16,11 @@ class NotesController < ApplicationController if @user = User.active.find_by(:display_name => params[:display_name]) @params = params.permit(:display_name) @title = t "notes.mine.title", :user => @user.display_name - @heading = t "notes.mine.heading", :user => @user.display_name - @description = t "notes.mine.subheading", :user => render_to_string(:partial => "user", :object => @user) @page = (params[:page] || 1).to_i @page_size = 10 @notes = @user.notes @notes = @notes.visible unless current_user&.moderator? - @notes = @notes.order("updated_at DESC, id").distinct.offset((@page - 1) * @page_size).limit(@page_size).preload(:comments => :author).to_a + @notes = @notes.order("updated_at DESC, id").distinct.offset((@page - 1) * @page_size).limit(@page_size).preload(:comments => :author) else @title = t "users.no_such_user.title" @not_found_user = params[:display_name] diff --git a/app/views/browse/_common_details.html.erb b/app/views/browse/_common_details.html.erb index 3b3030abf..44bbe04f8 100644 --- a/app/views/browse/_common_details.html.erb +++ b/app/views/browse/_common_details.html.erb @@ -1,32 +1,30 @@

<% if common_details.changeset.tags['comment'].present? %> - <%= linkify(h(common_details.changeset.tags['comment'])) %> + <%= linkify(h(common_details.changeset.tags["comment"])) %> <% else %> - <%= t 'browse.no_comment' %> + <%= t "browse.no_comment" %> <% end %>

- <%= - t "browse.#{common_details.visible? ? :edited : :deleted}_by_html", + <%= t "browse.#{common_details.visible? ? :edited : :deleted}_by_html", :time => distance_of_time_in_words_to_now(common_details.timestamp), :user => changeset_user_link(common_details.changeset), - :title => l(common_details.timestamp) - %> + :title => l(common_details.timestamp) %>
- <%= t 'browse.version' %> + <%= t "browse.version" %> #<%= h(common_details.version) %> · - <%= t 'browse.in_changeset' %> + <%= t "browse.in_changeset" %> #<%= link_to common_details.changeset_id, :action => :changeset, :id => common_details.changeset_id %>
<% if @type == "node" and common_details.visible? %>
- <%= t 'browse.location' %> - <%= link_to(content_tag(:span, number_with_delimiter(common_details.lat), :class => "latitude") + ", " + content_tag(:span, number_with_delimiter(common_details.lon), :class => "longitude"), { :controller => 'site', :action => 'index', :anchor => "map=18/#{common_details.lat}/#{common_details.lon}" }) %> + <%= t "browse.location" %> + <%= link_to(content_tag(:span, number_with_delimiter(common_details.lat), :class => "latitude") + ", " + content_tag(:span, number_with_delimiter(common_details.lon), :class => "longitude"), :controller => "site", :action => "index", :anchor => "map=18/#{common_details.lat}/#{common_details.lon}") %>
<% end %> diff --git a/app/views/browse/_containing_relation.html.erb b/app/views/browse/_containing_relation.html.erb index 391462d92..b3cb90cec 100644 --- a/app/views/browse/_containing_relation.html.erb +++ b/app/views/browse/_containing_relation.html.erb @@ -1,9 +1,7 @@ -
  • <%= - linked_name = link_to h(printable_name(containing_relation.relation)), :action => "relation", :id => containing_relation.relation.id.to_s - if containing_relation.member_role.blank? - raw t '.entry', :relation_name => linked_name - else - raw t '.entry_role', :relation_name => linked_name, :relation_role => h(containing_relation.member_role) - end - %> +
  • <%= linked_name = link_to h(printable_name(containing_relation.relation)), :action => "relation", :id => containing_relation.relation.id.to_s + if containing_relation.member_role.blank? + raw t ".entry", :relation_name => linked_name + else + raw t ".entry_role", :relation_name => linked_name, :relation_role => h(containing_relation.member_role) + end %>
    • diff --git a/app/views/browse/_node.html.erb b/app/views/browse/_node.html.erb index 873360bb2..52502ad4b 100644 --- a/app/views/browse/_node.html.erb +++ b/app/views/browse/_node.html.erb @@ -1,9 +1,9 @@ <% if node.redacted? %>
    - <%= t 'browse.redacted.message_html', - :type => t('browse.redacted.type.node'), + <%= t "browse.redacted.message_html", + :type => t("browse.redacted.type.node"), :version => node.version, - :redaction_link => link_to(t('browse.redacted.redaction', + :redaction_link => link_to(t("browse.redacted.redaction", :id => node.redaction.id), node.redaction) %>
    <% else %> @@ -11,10 +11,10 @@ <%= render :partial => "common_details", :object => node %> <% unless node.ways.empty? and node.containing_relation_members.empty? %> -

    <%= t 'browse.part_of' %>

    +

    <%= t "browse.part_of" %>

    diff --git a/app/views/browse/_relation.html.erb b/app/views/browse/_relation.html.erb index e7c4817b0..452556364 100644 --- a/app/views/browse/_relation.html.erb +++ b/app/views/browse/_relation.html.erb @@ -1,9 +1,9 @@ <% if relation.redacted? %>
    - <%= t 'browse.redacted.message_html', - :type => t('browse.redacted.type.relation'), + <%= t "browse.redacted.message_html", + :type => t("browse.redacted.type.relation"), :version => relation.version, - :redaction_link => link_to(t('browse.redacted.redaction', + :redaction_link => link_to(t("browse.redacted.redaction", :id => relation.redaction.id), relation.redaction) %>
    <% else %> @@ -11,12 +11,12 @@ <%= render :partial => "common_details", :object => relation %> <% unless relation.containing_relation_members.empty? %> -

    <%= t 'browse.part_of' %>

    +

    <%= t "browse.part_of" %>

    <% end %> <% unless relation.relation_members.empty? %> -

    <%= t '.members' %>

    +

    <%= t ".members" %>

    <% end %> diff --git a/app/views/browse/_relation_member.html.erb b/app/views/browse/_relation_member.html.erb index 5b6b5c365..bb37bdf21 100644 --- a/app/views/browse/_relation_member.html.erb +++ b/app/views/browse/_relation_member.html.erb @@ -1,14 +1,10 @@ -<% - member_class = link_class(relation_member.member_type.downcase, relation_member.member) - linked_name = link_to printable_name(relation_member.member), { :action => relation_member.member_type.downcase, :id => relation_member.member_id.to_s }, :title => link_title(relation_member.member), :rel => link_follow(relation_member.member) - type_str = t '.type.' + relation_member.member_type.downcase -%> +<% member_class = link_class(relation_member.member_type.downcase, relation_member.member) + linked_name = link_to printable_name(relation_member.member), { :action => relation_member.member_type.downcase, :id => relation_member.member_id.to_s }, { :title => link_title(relation_member.member), :rel => link_follow(relation_member.member) } + type_str = t ".type." + relation_member.member_type.downcase %>
  • - <%= - if relation_member.member_role.blank? - raw t '.entry', :type => type_str, :name => linked_name - else - raw t '.entry_role', :type => type_str, :name => linked_name, :role => h(relation_member.member_role) - end - %> + <%= if relation_member.member_role.blank? + raw t ".entry", :type => type_str, :name => linked_name + else + raw t ".entry_role", :type => type_str, :name => linked_name, :role => h(relation_member.member_role) + end %>
    • diff --git a/app/views/browse/_tag_details.html.erb b/app/views/browse/_tag_details.html.erb index 9c3fbbfc2..0e1a3fbef 100644 --- a/app/views/browse/_tag_details.html.erb +++ b/app/views/browse/_tag_details.html.erb @@ -1,5 +1,5 @@ <% unless tag_details.empty? %> -

    <%= t '.tags' %>

    +

    <%= t ".tags" %>

    <%= render :partial => "tag", :collection => tag_details.sort %>
    diff --git a/app/views/browse/_way.html.erb b/app/views/browse/_way.html.erb index 7099f015a..ed206c59b 100644 --- a/app/views/browse/_way.html.erb +++ b/app/views/browse/_way.html.erb @@ -1,9 +1,9 @@ <% if way.redacted? %>
    - <%= t 'browse.redacted.message_html', - :type => t('browse.redacted.type.way'), + <%= t "browse.redacted.message_html", + :type => t("browse.redacted.type.way"), :version => way.version, - :redaction_link => link_to(t('browse.redacted.redaction', + :redaction_link => link_to(t("browse.redacted.redaction", :id => way.redaction.id), way.redaction) %>
    <% else %> @@ -11,21 +11,21 @@ <%= render :partial => "common_details", :object => way %> <% unless way.containing_relation_members.empty? %> -

    <%= t 'browse.part_of' %>

    +

    <%= t "browse.part_of" %>

    <% end %> <% unless way.way_nodes.empty? %> -

    <%= t '.nodes' %>

    +

    <%= t ".nodes" %>