From: Andy Allan Date: Wed, 10 Oct 2018 09:26:30 +0000 (+0200) Subject: Merge branch 'authz' of https://github.com/rubyforgood/openstreetmap-website into... X-Git-Tag: live~2824^2~15 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/420a7289a0b08eee091f6650c2e83166df3fbe69?hp=-c Merge branch 'authz' of https://github.com/rubyforgood/openstreetmap-website into rubyforgood-authz --- 420a7289a0b08eee091f6650c2e83166df3fbe69 diff --combined Gemfile index 9ba270313,a385851ff..b559027c2 --- a/Gemfile +++ b/Gemfile @@@ -1,7 -1,7 +1,7 @@@ source "https://rubygems.org" # Require rails -gem "rails", "5.1.5" +gem "rails", "5.2.0" # Require things which have moved to gems in ruby 1.9 gem "bigdecimal", "~> 1.1.0", :platforms => :ruby_19 @@@ -16,7 -16,7 +16,7 @@@ gem "json gem "pg", "~> 0.18" # Use SCSS for stylesheets -gem "sass-rails", "~> 5.0" +gem "sassc-rails" # Use Uglifier as compressor for JavaScript assets gem "uglifier", ">= 1.3.0" @@@ -31,21 -31,19 +31,22 @@@ gem "jquery-rails # gem 'jbuilder', '~> 2.0' gem "jsonify-rails" +# Reduces boot times through caching; required in config/boot.rb +gem "bootsnap", ">= 1.1.0", :require => false + # Use R2 for RTL conversion gem "r2", "~> 0.2.7" # Use autoprefixer to generate CSS prefixes -gem "autoprefixer-rails" +gem "autoprefixer-rails", "~> 8.6.3" # Use image_optim to optimise images gem "image_optim_rails" # Load rails plugins gem "actionpack-page_caching" + gem "cancancan" -gem "composite_primary_keys", "~> 10.0.0" +gem "composite_primary_keys", "~> 11.0.0" gem "dynamic_form" gem "http_accept_language", "~> 2.0.0" gem "i18n-js", ">= 3.0.0" diff --combined Gemfile.lock index 76a31e169,06ddc0fe2..cd94df5e1 --- a/Gemfile.lock +++ b/Gemfile.lock @@@ -2,71 -2,65 +2,72 @@@ GE remote: https://rubygems.org/ specs: SystemTimer (1.2.3) - aasm (4.1.0) - actioncable (5.1.5) - actionpack (= 5.1.5) + aasm (5.0.1) + concurrent-ruby (~> 1.0) + actioncable (5.2.0) + actionpack (= 5.2.0) nio4r (~> 2.0) - websocket-driver (~> 0.6.1) - actionmailer (5.1.5) - actionpack (= 5.1.5) - actionview (= 5.1.5) - activejob (= 5.1.5) + websocket-driver (>= 0.6.1) + actionmailer (5.2.0) + actionpack (= 5.2.0) + actionview (= 5.2.0) + activejob (= 5.2.0) mail (~> 2.5, >= 2.5.4) rails-dom-testing (~> 2.0) - actionpack (5.1.5) - actionview (= 5.1.5) - activesupport (= 5.1.5) + actionpack (5.2.0) + actionview (= 5.2.0) + activesupport (= 5.2.0) rack (~> 2.0) rack-test (>= 0.6.3) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.0.2) actionpack-page_caching (1.1.0) actionpack (>= 4.0.0, < 6) - actionview (5.1.5) - activesupport (= 5.1.5) + actionview (5.2.0) + activesupport (= 5.2.0) builder (~> 3.1) erubi (~> 1.4) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.0.3) - activejob (5.1.5) - activesupport (= 5.1.5) + activejob (5.2.0) + activesupport (= 5.2.0) globalid (>= 0.3.6) - activemodel (5.1.5) - activesupport (= 5.1.5) - activerecord (5.1.5) - activemodel (= 5.1.5) - activesupport (= 5.1.5) - arel (~> 8.0) - activesupport (5.1.5) + activemodel (5.2.0) + activesupport (= 5.2.0) + activerecord (5.2.0) + activemodel (= 5.2.0) + activesupport (= 5.2.0) + arel (>= 9.0) + activestorage (5.2.0) + actionpack (= 5.2.0) + activerecord (= 5.2.0) + marcel (~> 0.3.1) + activesupport (5.2.0) concurrent-ruby (~> 1.0, >= 1.0.2) - i18n (~> 0.7) + i18n (>= 0.7, < 2) minitest (~> 5.1) tzinfo (~> 1.1) addressable (2.5.2) public_suffix (>= 2.0.2, < 4.0) - annotate (2.7.3) + annotate (2.7.4) activerecord (>= 3.2, < 6.0) rake (>= 10.4, < 13.0) - arel (8.0.0) + arel (9.0.0) ast (2.4.0) - autoprefixer-rails (8.5.1) + autoprefixer-rails (8.6.5) execjs - better_errors (2.4.0) + better_errors (2.5.0) coderay (>= 1.0.0) erubi (>= 1.0.0) rack (>= 0.9.0) bigdecimal (1.1.0) binding_of_caller (0.8.0) debug_inspector (>= 0.0.1) + bootsnap (1.3.2) + msgpack (~> 1.0) builder (3.2.3) + cancancan (2.1.3) - canonical-rails (0.2.3) + canonical-rails (0.2.4) rails (>= 4.1, < 5.3) capybara (2.18.0) addressable @@@ -85,12 -79,12 +86,12 @@@ coffee-script-source execjs coffee-script-source (1.12.2) - composite_primary_keys (10.0.3) - activerecord (~> 5.1.0, >= 5.1.5) + composite_primary_keys (11.0.3) + activerecord (~> 5.2.0) concurrent-ruby (1.0.5) - coveralls (0.8.21) + coveralls (0.8.22) json (>= 1.8, < 3) - simplecov (~> 0.14.1) + simplecov (~> 0.16.1) term-ansicolor (~> 1.3) thor (~> 0.19.4) tins (~> 1.6) @@@ -99,19 -93,19 +100,19 @@@ crass (1.0.4) dalli (2.7.8) debug_inspector (0.0.3) - docile (1.1.5) + docile (1.3.1) dynamic_form (1.1.4) erubi (1.7.1) execjs (2.7.0) exifr (1.3.4) - factory_bot (4.10.0) + factory_bot (4.11.1) activesupport (>= 3.0.0) - factory_bot_rails (4.10.0) - factory_bot (~> 4.10.0) + factory_bot_rails (4.11.1) + factory_bot (~> 4.11.1) railties (>= 3.0.0) faraday (0.12.2) multipart-post (>= 1.2, < 3) - ffi (1.9.23) + ffi (1.9.25) fspath (3.1.0) geoip (1.6.4) globalid (0.4.1) @@@ -122,21 -116,20 +123,21 @@@ http_accept_language (2.0.5) i18n (0.9.5) concurrent-ruby (~> 1.0) - i18n-js (3.0.5) + i18n-js (3.0.11) i18n (>= 0.6.6, < 2) - image_optim (0.26.1) + image_optim (0.26.2) exifr (~> 1.2, >= 1.2.2) fspath (~> 3.0) - image_size (~> 1.5) + image_size (>= 1.5, < 3) in_threads (~> 1.3) progress (~> 3.0, >= 3.0.1) image_optim_rails (0.4.1) image_optim (~> 0.24) rails sprockets - image_size (1.5.0) + image_size (2.0.0) in_threads (1.5.0) + jaro_winkler (1.5.1) jquery-rails (4.3.3) rails-dom-testing (>= 1, < 3) railties (>= 4.2.0) @@@ -169,22 -162,19 +170,22 @@@ nokogiri (>= 1.5.9) mail (2.7.0) mini_mime (>= 0.1.1) + marcel (0.3.3) + mimemagic (~> 0.3.2) method_source (0.9.0) - mime-types (3.1) + mime-types (3.2.2) mime-types-data (~> 3.2015) - mime-types-data (3.2016.0521) + mime-types-data (3.2018.0812) mimemagic (0.3.2) - mini_mime (1.0.0) + mini_mime (1.0.1) mini_portile2 (2.3.0) minitest (5.11.3) + msgpack (1.2.4) multi_json (1.13.1) multi_xml (0.6.0) multipart-post (2.0.0) nio4r (2.3.1) - nokogiri (1.8.2) + nokogiri (1.8.4) mini_portile2 (~> 2.3.0) nokogumbo (1.5.0) nokogiri @@@ -235,18 -225,18 +236,18 @@@ mimemagic (~> 0.3.0) terrapin (~> 0.6.0) parallel (1.12.1) - parser (2.5.1.0) + parser (2.5.1.2) ast (~> 2.4.0) pg (0.21.0) poltergeist (1.18.1) capybara (>= 2.1, < 4) cliver (~> 0.3.1) websocket-driver (>= 0.2.0) - powerpack (0.1.1) + powerpack (0.1.2) progress (3.4.0) psych (3.0.2) - public_suffix (3.0.2) - puma (3.11.4) + public_suffix (3.0.3) + puma (3.12.0) quad_tile (1.0.1) r2 (0.2.7) rack (2.0.5) @@@ -254,21 -244,20 +255,21 @@@ rack-openid (1.3.1) rack (>= 1.1.0) ruby-openid (>= 2.1.8) - rack-test (1.0.0) + rack-test (1.1.0) rack (>= 1.0, < 3) rack-uri_sanitizer (0.0.2) - rails (5.1.5) - actioncable (= 5.1.5) - actionmailer (= 5.1.5) - actionpack (= 5.1.5) - actionview (= 5.1.5) - activejob (= 5.1.5) - activemodel (= 5.1.5) - activerecord (= 5.1.5) - activesupport (= 5.1.5) + rails (5.2.0) + actioncable (= 5.2.0) + actionmailer (= 5.2.0) + actionpack (= 5.2.0) + actionview (= 5.2.0) + activejob (= 5.2.0) + activemodel (= 5.2.0) + activerecord (= 5.2.0) + activestorage (= 5.2.0) + activesupport (= 5.2.0) bundler (>= 1.3.0) - railties (= 5.1.5) + railties (= 5.2.0) sprockets-rails (>= 2.0.0) rails-controller-testing (1.0.2) actionpack (~> 5.x, >= 5.0.1) @@@ -282,9 -271,9 +283,9 @@@ rails-i18n (4.0.2) i18n (~> 0.6) rails (>= 4.0) - railties (5.1.5) - actionpack (= 5.1.5) - activesupport (= 5.1.5) + railties (5.2.0) + actionpack (= 5.2.0) + activesupport (= 5.2.0) method_source rake (>= 0.8.7) thor (>= 0.18.1, < 2.0) @@@ -301,44 -290,39 +302,44 @@@ rack (>= 1.4) rinku (2.0.4) rotp (3.3.1) - rubocop (0.56.0) + rubocop (0.59.1) + jaro_winkler (~> 1.5.1) parallel (~> 1.10) - parser (>= 2.5) + parser (>= 2.5, != 2.5.1.1) powerpack (~> 0.1) rainbow (>= 2.2.2, < 4.0) ruby-progressbar (~> 1.7) unicode-display_width (~> 1.0, >= 1.0.1) ruby-openid (2.7.0) - ruby-progressbar (1.9.0) + ruby-progressbar (1.10.0) ruby_dep (1.5.0) safe_yaml (1.0.4) - sanitize (4.6.5) + sanitize (4.6.6) crass (~> 1.0.2) nokogiri (>= 1.4.4) nokogumbo (~> 1.4) - sass (3.5.6) + sass (3.6.0) sass-listen (~> 4.0.0) sass-listen (4.0.0) rb-fsevent (~> 0.9, >= 0.9.4) rb-inotify (~> 0.9, >= 0.9.7) - sass-rails (5.0.7) - railties (>= 4.0.0, < 6) - sass (~> 3.1) - sprockets (>= 2.8, < 4.0) - sprockets-rails (>= 2.0, < 4.0) - tilt (>= 1.1, < 3) + sassc (1.12.1) + ffi (~> 1.9.6) + sass (>= 3.3.0) + sassc-rails (1.3.0) + railties (>= 4.0.0) + sass + sassc (~> 1.9) + sprockets (> 2.11) + sprockets-rails + tilt secure_headers (6.0.0) - simplecov (0.14.1) - docile (~> 1.1.0) + simplecov (0.16.1) + docile (~> 1.1) json (>= 1.8, < 3) simplecov-html (~> 0.10.0) simplecov-html (0.10.2) - sprockets (3.7.1) + sprockets (3.7.2) concurrent-ruby (~> 1.0) rack (> 1, < 3) sprockets-rails (3.2.1) @@@ -358,17 -342,17 +359,17 @@@ tins (1.16.3) tzinfo (1.2.5) thread_safe (~> 0.1) - uglifier (4.1.10) + uglifier (4.1.19) execjs (>= 0.3.0, < 3) - unicode-display_width (1.3.2) + unicode-display_width (1.4.0) validates_email_format_of (1.6.3) i18n vendorer (0.1.16) - webmock (3.4.1) + webmock (3.4.2) addressable (>= 2.3.6) crack (>= 0.3.2) hashdiff - websocket-driver (0.6.5) + websocket-driver (0.7.0) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.3) xpath (3.1.0) @@@ -382,15 -366,15 +383,16 @@@ DEPENDENCIE aasm actionpack-page_caching annotate - autoprefixer-rails + autoprefixer-rails (~> 8.6.3) better_errors bigdecimal (~> 1.1.0) binding_of_caller + bootsnap (>= 1.1.0) + cancancan canonical-rails capybara (~> 2.13) coffee-rails (~> 4.2) - composite_primary_keys (~> 10.0.0) + composite_primary_keys (~> 11.0.0) coveralls dalli dynamic_form @@@ -428,7 -412,7 +430,7 @@@ r2 (~> 0.2.7) rack-cors rack-uri_sanitizer - rails (= 5.1.5) + rails (= 5.2.0) rails-controller-testing rails-i18n (~> 4.0.0) record_tag_helper @@@ -437,7 -421,7 +439,7 @@@ rotp rubocop sanitize - sass-rails (~> 5.0) + sassc-rails secure_headers uglifier (>= 1.3.0) validates_email_format_of (>= 1.5.1) diff --combined app/controllers/application_controller.rb index bd1995014,eed183893..2c5fbe51d --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@@ -1,8 -1,11 +1,11 @@@ class ApplicationController < ActionController::Base include SessionPersistence + # check_authorization protect_from_forgery :with => :exception + rescue_from CanCan::AccessDenied, :with => :deny_access + before_action :fetch_body around_action :better_errors_allow_inline, :if => proc { Rails.env.development? } @@@ -17,16 -20,16 +20,16 @@@ session.delete(:user) session_expires_automatically - redirect_to :controller => "user", :action => "suspended" + redirect_to :controller => "users", :action => "suspended" # don't allow access to any auth-requiring part of the site unless # the new CTs have been seen (and accept/decline chosen). elsif !current_user.terms_seen && flash[:skip_terms].nil? flash[:notice] = t "user.terms.you need to accept or decline" if params[:referer] - redirect_to :controller => "user", :action => "terms", :referer => params[:referer] + redirect_to :controller => "users", :action => "terms", :referer => params[:referer] else - redirect_to :controller => "user", :action => "terms", :referer => request.fullpath + redirect_to :controller => "users", :action => "terms", :referer => request.fullpath end end elsif session[:token] @@@ -41,7 -44,7 +44,7 @@@ def require_user unless current_user if request.get? - redirect_to :controller => "user", :action => "login", :referer => request.fullpath + redirect_to :controller => "users", :action => "login", :referer => request.fullpath else head :forbidden end @@@ -283,7 -286,8 +286,7 @@@ # TODO: some sort of escaping of problem characters in the message response.headers["Error"] = message - if request.headers["X-Error-Format"] && - request.headers["X-Error-Format"].casecmp("xml").zero? + if request.headers["X-Error-Format"]&.casecmp("xml")&.zero? result = OSM::API.new.get_xml_doc result.root.name = "osmError" result.root << (XML::Node.new("status") << "#{Rack::Utils.status_code(status)} #{Rack::Utils::HTTP_STATUS_CODES[status]}") @@@ -309,7 -313,7 +312,7 @@@ helper_method :preferred_languages def set_locale(reset = false) - if current_user && current_user.languages.empty? && !http_accept_language.user_preferred_languages.empty? + if current_user&.languages&.empty? && !http_accept_language.user_preferred_languages.empty? current_user.languages = http_accept_language.user_preferred_languages current_user.save end @@@ -386,11 -390,11 +389,11 @@@ ## # render a "no such user" page def render_unknown_user(name) - @title = t "user.no_such_user.title" + @title = t "users.no_such_user.title" @not_found_user = name respond_to do |format| - format.html { render :template => "user/no_such_user", :status => :not_found } + format.html { render :template => "users/no_such_user", :status => :not_found } format.all { head :not_found } end end @@@ -434,7 -438,7 +437,7 @@@ def preferred_editor editor = if params[:editor] params[:editor] - elsif current_user && current_user.preferred_editor + elsif current_user&.preferred_editor current_user.preferred_editor else DEFAULT_EDITOR @@@ -466,6 -470,23 +469,23 @@@ raise end + def current_ability + Ability.new(current_user).merge(granted_capabily) + end + + def granted_capabily + Capability.new(current_user, current_token) + end + + def deny_access(_exception) + if current_user + set_locale + report_error t("oauth.permissions.missing"), :forbidden + else + require_user + end + end + private # extract authorisation credentials from headers, returns user = nil if none diff --combined app/controllers/diary_entry_controller.rb index 723fff17e,6e9268008..d3d7f6a7c --- a/app/controllers/diary_entry_controller.rb +++ b/app/controllers/diary_entry_controller.rb @@@ -3,12 -3,13 +3,13 @@@ class DiaryEntryController < Applicatio before_action :authorize_web before_action :set_locale - before_action :require_user, :only => [:new, :edit, :comment, :hide, :hidecomment, :subscribe, :unsubscribe] + + authorize_resource + - before_action :lookup_user, :only => [:view, :comments] + before_action :lookup_user, :only => [:show, :comments] before_action :check_database_readable before_action :check_database_writable, :only => [:new, :edit, :comment, :hide, :hidecomment, :subscribe, :unsubscribe] - before_action :require_administrator, :only => [:hide, :hidecomment] - before_action :allow_thirdparty_images, :only => [:new, :edit, :list, :view, :comments] + before_action :allow_thirdparty_images, :only => [:new, :edit, :index, :show, :comments] def new @title = t "diary_entry.new.title" @@@ -29,7 -30,7 +30,7 @@@ # Subscribe user to diary comments @diary_entry.subscriptions.create(:user => current_user) - redirect_to :action => "list", :display_name => current_user.display_name + redirect_to :action => "index", :display_name => current_user.display_name else render :action => "edit" end @@@ -47,9 -48,9 +48,9 @@@ @diary_entry = DiaryEntry.find(params[:id]) if current_user != @diary_entry.user - redirect_to :action => "view", :id => params[:id] + redirect_to diary_entry_path(@diary_entry.user, @diary_entry) elsif params[:diary_entry] && @diary_entry.update(entry_params) - redirect_to :action => "view", :id => params[:id] + redirect_to diary_entry_path(@diary_entry.user, @diary_entry) end set_map_location @@@ -71,9 -72,9 +72,9 @@@ # Add the commenter to the subscribers if necessary @entry.subscriptions.create(:user => current_user) unless @entry.subscribers.exists?(current_user.id) - redirect_to :action => "view", :display_name => @entry.user.display_name, :id => @entry.id + redirect_to diary_entry_path(@entry.user, @entry) else - render :action => "view" + render :action => "show" end rescue ActiveRecord::RecordNotFound render :action => "no_such_entry", :status => :not_found @@@ -84,7 -85,7 +85,7 @@@ diary_entry.subscriptions.create(:user => current_user) unless diary_entry.subscribers.exists?(current_user.id) - redirect_to :action => "view", :display_name => diary_entry.user.display_name, :id => diary_entry.id + redirect_to diary_entry_path(diary_entry.user, diary_entry) rescue ActiveRecord::RecordNotFound render :action => "no_such_entry", :status => :not_found end @@@ -94,17 -95,17 +95,17 @@@ diary_entry.subscriptions.where(:user => current_user).delete_all if diary_entry.subscribers.exists?(current_user.id) - redirect_to :action => "view", :display_name => diary_entry.user.display_name, :id => diary_entry.id + redirect_to diary_entry_path(diary_entry.user, diary_entry) rescue ActiveRecord::RecordNotFound render :action => "no_such_entry", :status => :not_found end - def list + def index if params[:display_name] @user = User.active.find_by(:display_name => params[:display_name]) if @user - @title = t "diary_entry.list.user_title", :user => @user.display_name + @title = t "diary_entry.index.user_title", :user => @user.display_name @entries = @user.diary_entries else render_unknown_user params[:display_name] @@@ -112,7 -113,7 +113,7 @@@ end elsif params[:friends] if current_user - @title = t "diary_entry.list.title_friends" + @title = t "diary_entry.index.title_friends" @entries = DiaryEntry.where(:user_id => current_user.friend_users) else require_user @@@ -120,7 -121,7 +121,7 @@@ end elsif params[:nearby] if current_user - @title = t "diary_entry.list.title_nearby" + @title = t "diary_entry.index.title_nearby" @entries = DiaryEntry.where(:user_id => current_user.nearby) else require_user @@@ -130,10 -131,10 +131,10 @@@ @entries = DiaryEntry.joins(:user).where(:users => { :status => %w[active confirmed] }) if params[:language] - @title = t "diary_entry.list.in_language_title", :language => Language.find(params[:language]).english_name + @title = t "diary_entry.index.in_language_title", :language => Language.find(params[:language]).english_name @entries = @entries.where(:language_code => params[:language]) else - @title = t "diary_entry.list.title" + @title = t "diary_entry.index.title" end end @@@ -157,7 -158,7 +158,7 @@@ @entries = user.diary_entries @title = t("diary_entry.feed.user.title", :user => user.display_name) @description = t("diary_entry.feed.user.description", :user => user.display_name) - @link = url_for :controller => "diary_entry", :action => "list", :display_name => user.display_name, :host => SERVER_URL, :protocol => SERVER_PROTOCOL + @link = url_for :controller => "diary_entry", :action => "index", :display_name => user.display_name, :host => SERVER_URL, :protocol => SERVER_PROTOCOL else head :not_found return @@@ -169,21 -170,21 +170,21 @@@ @entries = @entries.where(:language_code => params[:language]) @title = t("diary_entry.feed.language.title", :language_name => Language.find(params[:language]).english_name) @description = t("diary_entry.feed.language.description", :language_name => Language.find(params[:language]).english_name) - @link = url_for :controller => "diary_entry", :action => "list", :language => params[:language], :host => SERVER_URL, :protocol => SERVER_PROTOCOL + @link = url_for :controller => "diary_entry", :action => "index", :language => params[:language], :host => SERVER_URL, :protocol => SERVER_PROTOCOL else @title = t("diary_entry.feed.all.title") @description = t("diary_entry.feed.all.description") - @link = url_for :controller => "diary_entry", :action => "list", :host => SERVER_URL, :protocol => SERVER_PROTOCOL + @link = url_for :controller => "diary_entry", :action => "index", :host => SERVER_URL, :protocol => SERVER_PROTOCOL end end @entries = @entries.visible.includes(:user).order("created_at DESC").limit(20) end - def view + def show @entry = @user.diary_entries.visible.where(:id => params[:id]).first if @entry - @title = t "diary_entry.view.title", :user => params[:display_name], :title => @entry.title + @title = t "diary_entry.show.title", :user => params[:display_name], :title => @entry.title else @title = t "diary_entry.no_such_entry.title", :id => params[:id] render :action => "no_such_entry", :status => :not_found @@@ -193,13 -194,13 +194,13 @@@ def hide entry = DiaryEntry.find(params[:id]) entry.update(:visible => false) - redirect_to :action => "list", :display_name => entry.user.display_name + redirect_to :action => "index", :display_name => entry.user.display_name end def hidecomment comment = DiaryComment.find(params[:comment]) comment.update(:visible => false) - redirect_to :action => "view", :display_name => comment.diary_entry.user.display_name, :id => comment.diary_entry.id + redirect_to diary_entry_path(comment.diary_entry.user, comment.diary_entry) end def comments @@@ -215,6 -216,22 +216,22 @@@ private + # This is required because, being a default-deny system, cancancan + # _cannot_ tell you the reason you were denied access; and so + # the "nice" feedback presenting next steps can't be gleaned from + # the exception + ## + # for the hide actions, require that the user is a administrator, or fill out + # a helpful error message and return them to the user page. + def deny_access(exception) + if current_user && exception.action.in?([:hide, :hidecomment]) + flash[:error] = t("user.filter.not_an_administrator") + redirect_to :action => "view" + else + super + end + end + ## # return permitted diary entry parameters def entry_params @@@ -229,16 -246,6 +246,6 @@@ params.require(:diary_comment).permit(:body) end - ## - # require that the user is a administrator, or fill out a helpful error message - # and return them to the user page. - def require_administrator - unless current_user.administrator? - flash[:error] = t("user.filter.not_an_administrator") - redirect_to :action => "show" - end - end - ## # decide on a location for the diary entry map def set_map_location diff --combined app/controllers/site_controller.rb index efb77e2f5,8f4aafa44..2fa91256e --- a/app/controllers/site_controller.rb +++ b/app/controllers/site_controller.rb @@@ -6,10 -6,11 +6,11 @@@ class SiteController < ApplicationContr before_action :set_locale before_action :redirect_browse_params, :only => :index before_action :redirect_map_params, :only => [:index, :edit, :export] - before_action :require_user, :only => [:welcome] before_action :require_oauth, :only => [:index] before_action :update_totp, :only => [:index] + authorize_resource :class => false + def index session[:location] ||= OSM.ip_location(request.env["REMOTE_ADDR"]) unless STATUS == :database_readonly || STATUS == :database_offline end @@@ -102,7 -103,9 +103,9 @@@ @locale = params[:copyright_locale] || I18n.locale end - def welcome; end + def welcome + require_user + end def help; end @@@ -120,7 -123,7 +123,7 @@@ append_content_security_policy_directives( :connect_src => %w[*], :img_src => %w[* blob:], - :script_src => %w[dev.virtualearth.net *.wikipedia.org www.wikidata.org services.arcgisonline.com serviceslab.arcgisonline.com 'unsafe-eval'], + :script_src => %w[dev.virtualearth.net 'unsafe-eval'], :style_src => %w['unsafe-inline'] ) diff --combined app/controllers/users_controller.rb index d18cf188c,d853d4822..09bdd6d3e --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@@ -1,24 -1,26 +1,26 @@@ -class UserController < ApplicationController +class UsersController < ApplicationController layout "site", :except => [:api_details] + skip_authorization_check :only => [:login, :logout] + - skip_before_action :verify_authenticity_token, :only => [:api_read, :api_details, :api_gpx_files, :auth_success] + skip_before_action :verify_authenticity_token, :only => [:api_read, :api_users, :api_details, :api_gpx_files, :auth_success] before_action :disable_terms_redirect, :only => [:terms, :save, :logout, :api_details] before_action :authorize, :only => [:api_details, :api_gpx_files] - before_action :authorize_web, :except => [:api_read, :api_details, :api_gpx_files] - before_action :set_locale, :except => [:api_read, :api_details, :api_gpx_files] + before_action :authorize_web, :except => [:api_read, :api_users, :api_details, :api_gpx_files] + before_action :set_locale, :except => [:api_read, :api_users, :api_details, :api_gpx_files] before_action :require_user, :only => [:account, :go_public, :make_friend, :remove_friend] before_action :require_self, :only => [:account] - before_action :check_database_readable, :except => [:login, :api_read, :api_details, :api_gpx_files] + before_action :check_database_readable, :except => [:login, :api_read, :api_users, :api_details, :api_gpx_files] before_action :check_database_writable, :only => [:new, :account, :confirm, :confirm_email, :lost_password, :reset_password, :go_public, :make_friend, :remove_friend] - before_action :check_api_readable, :only => [:api_read, :api_details, :api_gpx_files] + before_action :check_api_readable, :only => [:api_read, :api_users, :api_details, :api_gpx_files] before_action :require_allow_read_prefs, :only => [:api_details] before_action :require_allow_read_gpx, :only => [:api_gpx_files] before_action :require_cookies, :only => [:new, :login, :confirm] - before_action :require_administrator, :only => [:set_status, :delete, :list] - around_action :api_call_handle_error, :only => [:api_read, :api_details, :api_gpx_files] + before_action :require_administrator, :only => [:set_status, :delete, :index] + around_action :api_call_handle_error, :only => [:api_read, :api_users, :api_details, :api_gpx_files] before_action :lookup_user_by_id, :only => [:api_read] before_action :lookup_user_by_name, :only => [:set_status, :delete] - before_action :allow_thirdparty_images, :only => [:view, :account] + before_action :allow_thirdparty_images, :only => [:show, :account] def terms @legale = params[:legale] || OSM.ip_to_country(request.remote_ip) || DEFAULT_LEGALE @@@ -27,9 -29,9 +29,9 @@@ if request.xhr? render :partial => "terms" else - @title = t "user.terms.title" + @title = t "users.terms.title" - if current_user && current_user.terms_agreed? + if current_user&.terms_agreed? # Already agreed to terms, so just show settings redirect_to :action => :account, :display_name => current_user.display_name elsif current_user.nil? && session[:new_user].nil? @@@ -39,13 -41,13 +41,13 @@@ end def save - @title = t "user.new.title" + @title = t "users.new.title" if params[:decline] if current_user current_user.terms_seen = true - flash[:notice] = t("user.new.terms declined", :url => t("user.new.terms declined url")).html_safe if current_user.save + flash[:notice] = t("users.new.terms declined", :url => t("users.new.terms declined url")).html_safe if current_user.save if params[:referer] redirect_to params[:referer] @@@ -53,7 -55,7 +55,7 @@@ redirect_to :action => :account, :display_name => current_user.display_name end else - redirect_to t("user.terms.declined") + redirect_to t("users.terms.declined") end elsif current_user unless current_user.terms_agreed? @@@ -61,7 -63,7 +63,7 @@@ current_user.terms_agreed = Time.now.getutc current_user.terms_seen = true - flash[:notice] = t "user.new.terms accepted" if current_user.save + flash[:notice] = t "users.new.terms accepted" if current_user.save end if params[:referer] @@@ -134,18 -136,18 +136,18 @@@ current_user.errors.add(attribute, error) end end - @title = t "user.account.title" + @title = t "users.account.title" end def go_public current_user.data_public = true current_user.save - flash[:notice] = t "user.go_public.flash success" + flash[:notice] = t "users.go_public.flash success" redirect_to :action => "account", :display_name => current_user.display_name end def lost_password - @title = t "user.lost_password.title" + @title = t "users.lost_password.title" if params[:user] && params[:user][:email] user = User.visible.find_by(:email => params[:user][:email]) @@@ -159,16 -161,16 +161,16 @@@ if user token = user.tokens.create Notifier.lost_password(user, token).deliver_now - flash[:notice] = t "user.lost_password.notice email on way" + flash[:notice] = t "users.lost_password.notice email on way" redirect_to :action => "login" else - flash.now[:error] = t "user.lost_password.notice email cannot find" + flash.now[:error] = t "users.lost_password.notice email cannot find" end end end def reset_password - @title = t "user.reset_password.title" + @title = t "users.reset_password.title" if params[:token] token = UserToken.find_by(:token => params[:token]) @@@ -184,12 -186,12 +186,12 @@@ if current_user.save token.destroy - flash[:notice] = t "user.reset_password.flash changed" + flash[:notice] = t "users.reset_password.flash changed" successful_login(current_user) end end else - flash[:error] = t "user.reset_password.flash token bad" + flash[:error] = t "users.reset_password.flash token bad" redirect_to :action => "lost_password" end else @@@ -198,7 -200,7 +200,7 @@@ end def new - @title = t "user.new.title" + @title = t "users.new.title" @referer = params[:referer] || session[:referer] append_content_security_policy_directives( @@@ -271,12 -273,12 +273,12 @@@ end def logout - @title = t "user.logout.title" + @title = t "users.logout.title" if params[:session] == session.id if session[:token] token = UserToken.find_by(:token => session[:token]) - token.destroy if token + token&.destroy session.delete(:token) end session.delete(:user) @@@ -292,11 -294,11 +294,11 @@@ def confirm if request.post? token = UserToken.find_by(:token => params[:confirm_string]) - if token && token.user.active? - flash[:error] = t("user.confirm.already active") + if token&.user&.active? + flash[:error] = t("users.confirm.already active") redirect_to :action => "login" elsif !token || token.expired? - flash[:error] = t("user.confirm.unknown token") + flash[:error] = t("users.confirm.unknown token") redirect_to :action => "confirm" else user = token.user @@@ -315,7 -317,7 +317,7 @@@ end if token.nil? || token.user != user - flash[:notice] = t("user.confirm.success") + flash[:notice] = t("users.confirm.success") redirect_to :action => :login, :referer => referer else token.destroy @@@ -337,10 -339,10 +339,10 @@@ token = UserToken.find_by(:token => session[:token]) if user.nil? || token.nil? || token.user != user - flash[:error] = t "user.confirm_resend.failure", :name => params[:display_name] + flash[:error] = t "users.confirm_resend.failure", :name => params[:display_name] else Notifier.signup_confirm(user, user.tokens.create).deliver_now - flash[:notice] = t("user.confirm_resend.success", :email => user.email, :sender => SUPPORT_EMAIL).html_safe + flash[:notice] = t("users.confirm_resend.success", :email => user.email, :sender => SUPPORT_EMAIL).html_safe end redirect_to :action => "login" @@@ -349,7 -351,7 +351,7 @@@ def confirm_email if request.post? token = UserToken.find_by(:token => params[:confirm_string]) - if token && token.user.new_email? + if token&.user&.new_email? self.current_user = token.user current_user.email = current_user.new_email current_user.new_email = nil @@@ -357,9 -359,9 +359,9 @@@ gravatar_enabled = gravatar_enable(current_user) if current_user.save flash[:notice] = if gravatar_enabled - t("user.confirm_email.success") + " " + gravatar_status_message(current_user) + t("users.confirm_email.success") + " " + gravatar_status_message(current_user) else - t("user.confirm_email.success") + t("users.confirm_email.success") end else flash[:errors] = current_user.errors @@@ -368,10 -370,10 +370,10 @@@ session[:user] = current_user.id redirect_to :action => "account", :display_name => current_user.display_name elsif token - flash[:error] = t "user.confirm_email.failure" + flash[:error] = t "users.confirm_email.failure" redirect_to :action => "account", :display_name => token.user.display_name else - flash[:error] = t "user.confirm_email.unknown_token" + flash[:error] = t "users.confirm_email.unknown_token" end end end @@@ -389,18 -391,6 +391,18 @@@ render :action => :api_read, :content_type => "text/xml" end + def api_users + raise OSM::APIBadUserInput, "The parameter users is required, and must be of the form users=id[,id[,id...]]" unless params["users"] + + ids = params["users"].split(",").collect(&:to_i) + + raise OSM::APIBadUserInput, "No users were given to search for" if ids.empty? + + @users = User.visible.find(ids) + + render :action => :api_users, :content_type => "text/xml" + end + def api_gpx_files doc = OSM::API.new.get_xml_doc current_user.traces.reload.each do |trace| @@@ -409,11 -399,11 +411,11 @@@ render :xml => doc.to_s end - def view + def show @user = User.find_by(:display_name => params[:display_name]) if @user && - (@user.visible? || (current_user && current_user.administrator?)) + (@user.visible? || (current_user&.administrator?)) @title = @user.display_name else render_unknown_user params[:display_name] @@@ -429,18 -419,18 +431,18 @@@ friend.befriender = current_user friend.befriendee = @new_friend if current_user.is_friends_with?(@new_friend) - flash[:warning] = t "user.make_friend.already_a_friend", :name => @new_friend.display_name + flash[:warning] = t "users.make_friend.already_a_friend", :name => @new_friend.display_name elsif friend.save - flash[:notice] = t "user.make_friend.success", :name => @new_friend.display_name + flash[:notice] = t "users.make_friend.success", :name => @new_friend.display_name Notifier.friend_notification(friend).deliver_now else - friend.add_error(t("user.make_friend.failed", :name => @new_friend.display_name)) + friend.add_error(t("users.make_friend.failed", :name => @new_friend.display_name)) end if params[:referer] redirect_to params[:referer] else - redirect_to :action => "view" + redirect_to :action => "show" end end else @@@ -455,15 -445,15 +457,15 @@@ if request.post? if current_user.is_friends_with?(@friend) Friend.where(:user_id => current_user.id, :friend_user_id => @friend.id).delete_all - flash[:notice] = t "user.remove_friend.success", :name => @friend.display_name + flash[:notice] = t "users.remove_friend.success", :name => @friend.display_name else - flash[:error] = t "user.remove_friend.not_a_friend", :name => @friend.display_name + flash[:error] = t "users.remove_friend.not_a_friend", :name => @friend.display_name end if params[:referer] redirect_to params[:referer] else - redirect_to :action => "view" + redirect_to :action => "show" end end else @@@ -476,19 -466,19 +478,19 @@@ def set_status @user.status = params[:status] @user.save - redirect_to :action => "view", :display_name => params[:display_name] + redirect_to user_path(:display_name => params[:display_name]) end ## # delete a user, marking them as deleted and removing personal data def delete @user.delete - redirect_to :action => "view", :display_name => params[:display_name] + redirect_to user_path(:display_name => params[:display_name]) end ## # display a list of users matching specified criteria - def list + def index if request.post? ids = params[:user].keys.collect(&:to_i) @@@ -552,7 -542,7 +554,7 @@@ if user.nil? && provider == "google" openid_url = auth_info[:extra][:id_info]["openid_id"] user = User.find_by(:auth_provider => "openid", :auth_uid => openid_url) if openid_url - user.update(:auth_provider => provider, :auth_uid => uid) if user + user&.update(:auth_provider => provider, :auth_uid => uid) end if user @@@ -562,9 -552,9 +564,9 @@@ when "active", "confirmed" then successful_login(user, request.env["omniauth.params"]["referer"]) when "suspended" then - failed_login t("user.login.account is suspended", :webmaster => "mailto:#{SUPPORT_EMAIL}").html_safe + failed_login t("users.login.account is suspended", :webmaster => "mailto:#{SUPPORT_EMAIL}").html_safe else - failed_login t("user.login.auth failure") + failed_login t("users.login.auth failure") end else redirect_to :action => "new", :nickname => name, :email => email, @@@ -576,7 -566,7 +578,7 @@@ ## # omniauth failure callback def auth_failure - flash[:error] = t("user.auth_failure." + params[:message]) + flash[:error] = t("users.auth_failure." + params[:message]) redirect_to params[:origin] || login_url end @@@ -590,9 -580,9 +592,9 @@@ elsif user = User.authenticate(:username => username, :password => password, :pending => true) unconfirmed_login(user) elsif User.authenticate(:username => username, :password => password, :suspended => true) - failed_login t("user.login.account is suspended", :webmaster => "mailto:#{SUPPORT_EMAIL}").html_safe, username + failed_login t("users.login.account is suspended", :webmaster => "mailto:#{SUPPORT_EMAIL}").html_safe, username else - failed_login t("user.login.auth failure"), username + failed_login t("users.login.auth failure"), username end end @@@ -727,12 -717,12 +729,12 @@@ set_locale(true) if user.new_email.blank? || user.new_email == user.email - flash.now[:notice] = t "user.account.flash update success" + flash.now[:notice] = t "users.account.flash update success" else user.email = user.new_email if user.valid? - flash.now[:notice] = t "user.account.flash update success confirm needed" + flash.now[:notice] = t "users.account.flash update success confirm needed" begin Notifier.email_confirm(user, user.tokens.create).deliver_now @@@ -754,10 -744,10 +756,10 @@@ # and return them to the user page. def require_administrator if current_user && !current_user.administrator? - flash[:error] = t("user.filter.not_an_administrator") + flash[:error] = t("users.filter.not_an_administrator") if params[:display_name] - redirect_to :action => "view", :display_name => params[:display_name] + redirect_to user_path(:display_name => params[:display_name]) else redirect_to :action => "login", :referer => request.fullpath end @@@ -826,7 -816,6 +828,7 @@@ def gravatar_enable(user) # code from example https://en.gravatar.com/site/implement/images/ruby/ return false if user.image.present? + hash = Digest::MD5.hexdigest(user.email.downcase) url = "https://www.gravatar.com/avatar/#{hash}?d=404" # without d=404 we will always get an image back response = OSM.http_client.get(URI.parse(url)) @@@ -839,9 -828,9 +841,9 @@@ # display a message about th current status of the gravatar setting def gravatar_status_message(user) if user.image_use_gravatar - t "user.account.gravatar.enabled" + t "users.account.gravatar.enabled" else - t "user.account.gravatar.disabled" + t "users.account.gravatar.disabled" end end end diff --combined test/test_helper.rb index 83cf909dd,df88a6ee0..b7b934552 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@@ -2,12 -2,10 +2,12 @@@ require "coveralls Coveralls.wear!("rails") ENV["RAILS_ENV"] = "test" -require File.expand_path("../config/environment", __dir__) +require_relative "../config/environment" require "rails/test_help" require "webmock/minitest" +WebMock.disable_net_connect!(:allow_localhost => true) + module ActiveSupport class TestCase include FactoryBot::Syntax::Methods @@@ -87,6 -85,16 +87,16 @@@ @request.env["HTTP_AUTHORIZATION"] = format("Basic %{auth}", :auth => Base64.encode64("#{user}:#{pass}")) end + ## + # set oauth token permissions + def grant_oauth_token(*tokens) + request.env["oauth.token"] = AccessToken.new do |token| + tokens.each do |t| + token.public_send("#{t}=", true) + end + end + end + ## # set request readers to ask for a particular error format def error_format(format) @@@ -132,15 -140,15 +142,15 @@@ end def stub_gravatar_request(email, status = 200, body = nil) - hash = Digest::MD5.hexdigest(email.downcase) + hash = ::Digest::MD5.hexdigest(email.downcase) url = "https://www.gravatar.com/avatar/#{hash}?d=404" stub_request(:get, url).and_return(:status => status, :body => body) end def stub_hostip_requests # Controller tests and integration tests use different IPs - stub_request(:get, "http://api.hostip.info/country.php?ip=0.0.0.0") - stub_request(:get, "http://api.hostip.info/country.php?ip=127.0.0.1") + stub_request(:get, "https://api.hostip.info/country.php?ip=0.0.0.0") + stub_request(:get, "https://api.hostip.info/country.php?ip=127.0.0.1") end def email_text_parts(message)