From: Andy Allan Date: Wed, 15 May 2024 16:43:04 +0000 (+0100) Subject: Merge pull request #4680 from tomhughes/validate-page-numbers X-Git-Tag: live~695 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/ffda8d7ac5ca4f40a1211225dd3e1c898fc232a8?ds=inline;hp=-c Merge pull request #4680 from tomhughes/validate-page-numbers Add parameter validation to pagination --- ffda8d7ac5ca4f40a1211225dd3e1c898fc232a8 diff --combined Gemfile index 04395b49a,253b685a7..75387b5d5 --- a/Gemfile +++ b/Gemfile @@@ -28,7 -28,6 +28,7 @@@ gem "jbuilder", "~> 2.7 gem "bootsnap", ">= 1.4.2", :require => false # Use rtlcss for RTL conversion +gem "mini_racer", "~> 0.9.0" gem "rtlcss" # Use autoprefixer to generate CSS prefixes @@@ -63,6 -62,7 +63,7 @@@ gem "oauth-plugin", ">= 0.5.1 gem "openstreetmap-deadlock_retry", ">= 1.3.1", :require => "deadlock_retry" gem "rack-cors" gem "rails-i18n", "~> 7.0.0" + gem "rails_param" gem "rinku", ">= 2.0.6", :require => "rails_rinku" gem "strong_migrations" gem "validates_email_format_of", ">= 1.5.1" @@@ -131,7 -131,7 +132,7 @@@ gem "gd2-ffij", ">= 0.4.0 gem "marcel" # Used for browser detection -gem "browser" +gem "browser", "< 6" # for ruby 3.0 support # Used for S3 object storage gem "aws-sdk-s3" @@@ -142,9 -142,6 +143,9 @@@ gem "image_processing # Used to validate widths gem "unicode-display_width" +# Keep ruby 3.0 compatibility +gem "multi_xml", "~> 0.6.0" + # Gems useful for development group :development do gem "better_errors" diff --combined Gemfile.lock index 087e19f45,5406ba2ff..14beee830 --- a/Gemfile.lock +++ b/Gemfile.lock @@@ -95,17 -95,17 +95,17 @@@ GE autoprefixer-rails (10.4.16.0) execjs (~> 2) aws-eventstream (1.3.0) - aws-partitions (1.910.0) - aws-sdk-core (3.191.6) + aws-partitions (1.928.0) + aws-sdk-core (3.196.0) aws-eventstream (~> 1, >= 1.3.0) aws-partitions (~> 1, >= 1.651.0) aws-sigv4 (~> 1.8) jmespath (~> 1, >= 1.6.1) - aws-sdk-kms (1.78.0) - aws-sdk-core (~> 3, >= 3.191.0) + aws-sdk-kms (1.81.0) + aws-sdk-core (~> 3, >= 3.193.0) aws-sigv4 (~> 1.1) - aws-sdk-s3 (1.146.1) - aws-sdk-core (~> 3, >= 3.191.0) + aws-sdk-s3 (1.150.0) + aws-sdk-core (~> 3, >= 3.194.0) aws-sdk-kms (~> 1) aws-sigv4 (~> 1.8) aws-sigv4 (1.8.0) @@@ -122,12 -122,12 +122,12 @@@ erubi (~> 1.4) parser (>= 2.4) smart_properties - bigdecimal (3.1.7) + bigdecimal (3.1.8) binding_of_caller (1.0.1) debug_inspector (>= 1.2.0) bootsnap (1.18.3) msgpack (~> 1.2) - bootstrap (5.3.2) + bootstrap (5.3.3) autoprefixer-rails (>= 9.1.0) popper_js (>= 2.11.8, < 3) bootstrap_form (5.4.0) @@@ -178,12 -178,12 +178,12 @@@ activerecord (>= 3.0, < 8.0) delayed_job (>= 3.0, < 5) docile (1.4.0) - doorkeeper (5.6.9) + doorkeeper (5.7.0) railties (>= 5) doorkeeper-i18n (5.2.7) doorkeeper (>= 5.2) - doorkeeper-openid_connect (1.8.8) - doorkeeper (>= 5.5, < 5.7) + doorkeeper-openid_connect (1.8.9) + doorkeeper (>= 5.5, < 5.8) jwt (>= 2.5) drb (2.2.1) dry-configurable (1.1.0) @@@ -258,13 -258,14 +258,13 @@@ highline (3.0.1) htmlentities (4.3.4) http_accept_language (2.1.1) - i18n (1.14.4) + i18n (1.14.5) concurrent-ruby (~> 1.0) i18n-js (3.9.2) i18n (>= 0.6.6) - i18n-tasks (1.0.13) + i18n-tasks (1.0.14) activesupport (>= 4.0.2) ast (>= 2.1.0) - better_html (>= 1.0, < 3.0) erubi highline (>= 2.0.0) i18n @@@ -288,10 -289,10 +288,10 @@@ image_size (3.4.0) in_threads (1.6.0) io-console (0.7.2) - irb (1.12.0) - rdoc + irb (1.13.1) + rdoc (>= 4.0.0) reline (>= 0.4.2) - jbuilder (2.11.5) + jbuilder (2.12.0) actionview (>= 5.0.0) activesupport (>= 5.0.0) jmespath (1.6.2) @@@ -327,7 -328,7 +327,7 @@@ maxminddb (0.1.22) mini_magick (4.12.0) mini_mime (1.1.5) - mini_portile2 (2.8.5) + mini_portile2 (2.8.6) mini_racer (0.9.0) libv8-node (~> 18.19.0.0) minitest (5.22.3) @@@ -339,7 -340,7 +339,7 @@@ mutex_m (0.2.0) net-http (0.4.1) uri - net-imap (0.4.10) + net-imap (0.4.11) date net-protocol net-pop (0.1.2) @@@ -348,8 -349,8 +348,8 @@@ timeout net-smtp (0.5.0) net-protocol - nio4r (2.7.1) - nokogiri (1.16.3) + nio4r (2.7.3) + nokogiri (1.16.5) mini_portile2 (~> 2.8.2) racc (~> 1.4) oauth (0.4.7) @@@ -395,12 -396,12 +395,12 @@@ omniauth-openid (2.0.1) omniauth (>= 1.0, < 3.0) rack-openid (~> 1.4.0) - omniauth-rails_csrf_protection (1.0.1) + omniauth-rails_csrf_protection (1.0.2) actionpack (>= 4.2) omniauth (~> 2.0) openstreetmap-deadlock_retry (1.3.1) parallel (1.24.0) - parser (3.3.0.5) + parser (3.3.1.0) ast (~> 2.4.1) racc pg (1.5.6) @@@ -458,6 -459,9 +458,9 @@@ rails-i18n (7.0.9) i18n (>= 0.7, < 2) railties (>= 6.0.0, < 8) + rails_param (1.3.1) + actionpack (>= 3.2.0) + activesupport (>= 3.2.0) railties (7.1.3.2) actionpack (= 7.1.3.2) activesupport (= 7.1.3.2) @@@ -473,10 -477,10 +476,10 @@@ ffi (~> 1.0) rdoc (6.6.3.1) psych (>= 4.0.0) - regexp_parser (2.9.0) - reline (0.5.1) + regexp_parser (2.9.1) + reline (0.5.7) io-console (~> 0.5) - request_store (1.6.0) + request_store (1.7.0) rack (>= 1.4) rexml (3.2.6) rinku (2.0.6) @@@ -484,7 -488,7 +487,7 @@@ rouge (4.2.1) rtlcss (0.2.1) mini_racer (>= 0.6.3) - rubocop (1.63.0) + rubocop (1.63.5) json (~> 2.3) language_server-protocol (>= 3.17.0) parallel (~> 1.10) @@@ -495,8 -499,8 +498,8 @@@ rubocop-ast (>= 1.31.1, < 2.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 2.4.0, < 3.0) - rubocop-ast (1.31.2) - parser (>= 3.3.0.4) + rubocop-ast (1.31.3) + parser (>= 3.3.1.0) rubocop-capybara (2.20.0) rubocop (~> 1.41) rubocop-factory_bot (2.25.1) @@@ -526,7 -530,7 +529,7 @@@ google-protobuf (~> 3.23) rake (>= 13.0.0) secure_headers (6.5.0) - selenium-webdriver (4.19.0) + selenium-webdriver (4.20.1) base64 (~> 0.2) rexml (~> 3.2, >= 3.2.5) rubyzip (>= 1.2.2, < 3.0) @@@ -538,8 -542,6 +541,8 @@@ simplecov-html (0.12.3) simplecov-lcov (0.8.0) simplecov_json_formatter (0.1.4) + simpleidn (0.2.2) + unf (~> 0.1.4) smart_properties (1.17.0) snaky_hash (2.0.1) hashie @@@ -570,14 -572,10 +573,14 @@@ railties (>= 6.0.0) tzinfo (2.0.6) concurrent-ruby (~> 1.0) + unf (0.1.4) + unf_ext + unf_ext (0.0.9.1) unicode-display_width (2.5.0) uri (0.13.0) - validates_email_format_of (1.7.2) - i18n + validates_email_format_of (1.8.2) + i18n (>= 0.8.0) + simpleidn vendorer (0.2.0) version_gem (1.1.4) webmock (3.23.0) @@@ -591,7 -589,7 +594,7 @@@ websocket-extensions (0.1.5) xpath (3.2.0) nokogiri (~> 1.8) - zeitwerk (2.6.13) + zeitwerk (2.6.14) PLATFORMS ruby @@@ -611,7 -609,7 +614,7 @@@ DEPENDENCIE bootstrap (~> 5.3.2) bootstrap_form (~> 5.0) brakeman - browser + browser (< 6) bzip2-ffi cancancan canonical-rails @@@ -650,10 -648,8 +653,10 @@@ logstasher marcel maxminddb + mini_racer (~> 0.9.0) minitest (~> 5.1) minitest-focus + multi_xml (~> 0.6.0) oauth-plugin (>= 0.5.1) omniauth (~> 2.0.2) omniauth-facebook @@@ -672,6 -668,7 +675,7 @@@ rails (~> 7.1.0) rails-controller-testing rails-i18n (~> 7.0.0) + rails_param rinku (>= 2.0.6) rotp rtlcss diff --combined app/controllers/application_controller.rb index 5d69a5fc8,d80b286d6..f5accc3c4 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@@ -10,6 -10,8 +10,8 @@@ class ApplicationController < ActionCon rescue_from CanCan::AccessDenied, :with => :deny_access check_authorization + rescue_from RailsParam::InvalidParameterError, :with => :invalid_parameter + before_action :fetch_body around_action :better_errors_allow_inline, :if => proc { Rails.env.development? } @@@ -67,10 -69,6 +69,10 @@@ @oauth_token = current_user.oauth_token(Settings.oauth_application) if current_user && Settings.key?(:oauth_application) end + def require_oauth_10a_support + report_error t("application.oauth_10a_disabled", :link => t("application.auth_disabled_link")), :forbidden unless Settings.oauth_10a_support + end + ## # require the user to have cookies enabled in their browser def require_cookies @@@ -310,6 -308,17 +312,17 @@@ end end + def invalid_parameter(_exception) + if request.get? + respond_to do |format| + format.html { redirect_to :controller => "/errors", :action => "bad_request" } + format.any { head :bad_request } + end + else + head :bad_request + end + end + # extract authorisation credentials from headers, returns user = nil if none def auth_data if request.env.key? "X-HTTP_AUTHORIZATION" # where mod_rewrite might have put it diff --combined app/controllers/changesets_controller.rb index 6a80f260a,757664b58..19ec9c91e --- a/app/controllers/changesets_controller.rb +++ b/app/controllers/changesets_controller.rb @@@ -18,6 -18,8 +18,8 @@@ class ChangesetsController < Applicatio ## # list non-empty changesets in reverse chronological order def index + param! :max_id, Integer, :min => 1 + @params = params.permit(:display_name, :bbox, :friends, :nearby, :max_id, :list) if request.format == :atom && @params[:max_id] @@@ -84,9 -86,8 +86,9 @@@ @way_pages, @ways = paginate(:old_ways, :conditions => { :changeset_id => @changeset.id }, :per_page => 20, :parameter => "way_page") @relation_pages, @relations = paginate(:old_relations, :conditions => { :changeset_id => @changeset.id }, :per_page => 20, :parameter => "relation_page") if @changeset.user.active? && @changeset.user.data_public? - @next_by_user = @changeset.user.changesets.where("id > ?", @changeset.id).reorder(:id => :asc).first - @prev_by_user = @changeset.user.changesets.where("id < ?", @changeset.id).reorder(:id => :desc).first + changesets = conditions_nonempty(@changeset.user.changesets) + @next_by_user = changesets.where("id > ?", @changeset.id).reorder(:id => :asc).first + @prev_by_user = changesets.where("id < ?", @changeset.id).reorder(:id => :desc).first end render :layout => map_layout rescue ActiveRecord::RecordNotFound diff --combined config/locales/en.yml index fdd3077ac,b035cccc3..647cf66f2 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@@ -142,6 -142,7 +142,6 @@@ en auth_provider: Authentication Provider auth_uid: Authentication UID email: "Email" - email_confirmation: "Email Confirmation" new_email: "New Email Address" active: "Active" display_name: "Display Name" @@@ -633,6 -634,9 +633,9 @@@ contact_url_title: Various contact channels explained contact: contact contact_the_community_html: Feel free to %{contact_link} the OpenStreetMap community if you have found a broken link / bug. Make a note of the exact URL of your request. + bad_request: + title: Bad request + description: The operation you requested on the OpenStreetMap server is not valid (HTTP 400) forbidden: title: Forbidden description: The operation you requested on the OpenStreetMap server is only available to administrators (HTTP 403) @@@ -1847,17 -1851,43 +1850,17 @@@ sessions: new: title: "Log in" - heading: "Log in" + tab_title: "Log in" + login_to_authorize_html: "Log in to OpenStreetMap to access %{client_app_name}." email or username: "Email Address or Username" password: "Password" - openid_html: "%{logo} OpenID" remember: "Remember me" lost password link: "Lost your password?" login_button: "Log in" register now: Register now - with external: "Alternatively, use a third party to log in:" - no account: Don't have an account? + with external: "or log in with a third party" + or: "or" auth failure: "Sorry, could not log in with those details." - openid_logo_alt: "Log in with an OpenID" - auth_providers: - openid: - title: Log in with OpenID - alt: Log in with an OpenID URL - google: - title: Log in with Google - alt: Log in with a Google OpenID - facebook: - title: Log in with Facebook - alt: Log in with a Facebook Account - microsoft: - title: Log in with Microsoft - alt: Log in with a Microsoft Account - github: - title: Log in with GitHub - alt: Log in with a GitHub Account - wikipedia: - title: Log in with Wikipedia - alt: Log in with a Wikipedia Account - wordpress: - title: Log in with Wordpress - alt: Log in with a Wordpress OpenID - aol: - title: Log in with AOL - alt: Log in with an AOL OpenID destroy: title: "Logout" heading: "Logout from OpenStreetMap" @@@ -2071,14 -2101,6 +2074,14 @@@ %{france}: Contains data sourced from Direction Générale des Impôts. contributors_fr_france: France + contributors_hr_credit_html: | + %{croatia}: Contains data from the %{dgu_link} and %{open_data_portal} + (public information of Croatia). + contributors_hr_croatia: Croatia + contributors_hr_dgu: State Geodetic Administration of Croatia + contributors_hr_dgu_url: https://dgu.gov.hr/ + contributors_hr_open_data_portal: National Open Data Portal + contributors_hr_open_data_portal_url: https://data.gov.hr/ contributors_nl_credit_html: | %{netherlands}: Contains © AND data, 2007 (%{and_link}) contributors_nl_netherlands: Netherlands @@@ -2557,9 -2579,6 +2560,9 @@@ other: "GPX file with %{count} points from %{user}" description_without_count: "GPX file from %{user}" application: + basic_auth_disabled: "HTTP Basic Authentication is disabled: %{link}" + oauth_10a_disabled: "OAuth 1.0 and 1.0a are disabled: %{link}" + auth_disabled_link: "https://wiki.openstreetmap.org/wiki/2024_authentication_update" permission_denied: You do not have permission to access that action require_cookies: cookies_needed: "You appear to have cookies disabled - please enable cookies in your browser before continuing." @@@ -2575,34 -2594,6 +2578,34 @@@ oauth2_applications: OAuth 2 applications oauth2_authorizations: OAuth 2 authorizations muted_users: Muted Users + auth_providers: + openid_logo_alt: "Log in with an OpenID" + openid_html: "%{logo} OpenID" + openid_login_button: "Continue" + openid: + title: Log in with OpenID + alt: Log in with an OpenID URL + google: + title: Log in with Google + alt: Log in with a Google OpenID + facebook: + title: Log in with Facebook + alt: Log in with a Facebook Account + microsoft: + title: Log in with Microsoft + alt: Log in with a Microsoft Account + github: + title: Log in with GitHub + alt: Log in with a GitHub Account + wikipedia: + title: Log in with Wikipedia + alt: Log in with a Wikipedia Account + wordpress: + title: Log in with Wordpress + alt: Log in with a Wordpress OpenID + aol: + title: Log in with AOL + alt: Log in with an AOL OpenID oauth: authorize: title: "Authorize access to your account" @@@ -2640,8 -2631,6 +2643,8 @@@ write_redactions: Redact map data read_email: Read user email address skip_authorization: Auto approve application + for_roles: + moderator: This permission is for actions available only to moderators oauth_clients: new: title: "Register a new application" @@@ -2733,34 -2722,23 +2736,34 @@@ users: new: title: "Sign Up" + tab_title: "Sign up" + signup_to_authorize_html: "Sign up with OpenStreetMap to access %{client_app_name}." no_auto_account_create: "Unfortunately we are not currently able to create an account for you automatically." please_contact_support_html: 'Please contact %{support_link} to arrange for an account to be created - we will try and deal with the request as quickly as possible.' support: support about: - header: Free and editable + header: Free and editable. paragraph_1: Unlike other maps, OpenStreetMap is completely created by people like you, and it's free for anyone to fix, update, download and use. - paragraph_2: Sign up to get started contributing. We'll send an email to confirm your account. + paragraph_2: Sign up to get started contributing. + welcome: "Welcome to OpenStreetMap" + duplicate_social_email: "If you already have an OpenStreetMap account and wish to use a 3rd party identity provider, please log in using your password and modify the settings of your account." display name description: "Your publicly displayed username. You can change this later in the preferences." + by_signing_up_html: "By signing up, you agree to our %{tou_link}, %{privacy_policy_link} and %{contributor_terms_link}." + tou: "terms of use" + contributor_terms_url: "https://wiki.osmfoundation.org/wiki/Licence/Contributor_Terms" + contributor_terms: "contributor terms" external auth: "Third Party Authentication:" - use external auth: "Alternatively, use a third party to log in" - auth no password: "With third party authentication a password is not required, but some extra tools or server may still need one." continue: Sign Up terms accepted: "Thanks for accepting the new contributor terms!" - email_confirmation_help_html: 'Your address is not displayed publicly, see our %{privacy_policy_link} for more information.' + email_help_html: 'Your address is not displayed publicly, see our %{privacy_policy_link} for more information.' privacy_policy: privacy policy privacy_policy_url: https://wiki.osmfoundation.org/wiki/Privacy_Policy privacy_policy_title: OSMF privacy policy including section on email addresses + consider_pd_html: "I consider my contributions to be in the %{consider_pd_link}." + consider_pd: "public domain" + consider_pd_url: https://wiki.osmfoundation.org/wiki/Licence_and_Legal_FAQ/Why_would_I_want_my_contributions_to_be_public_domain + or: "or" + use external auth: "or sign up with a third party" terms: title: "Terms" heading: "Terms" @@@ -3084,9 -3062,6 +3087,9 @@@ new: title: "New Note" intro: "Spotted a mistake or something missing? Let other mappers know so we can fix it. Move the marker to the correct position and type a note to explain the problem." + anonymous_warning_html: "You are not logged in. Please %{log_in} or %{sign_up} if you want to receive updates for your note." + anonymous_warning_log_in: "log in" + anonymous_warning_sign_up: "sign up" advice: "Your note is public and may be used to update the map, so don't enter personal information, or information from copyrighted maps or directory listings." add: Add Note javascripts: diff --combined config/routes.rb index 8271e7e4d,1a229b365..c44064ba3 --- a/config/routes.rb +++ b/config/routes.rb @@@ -122,7 -122,7 +122,7 @@@ OpenStreetMap::Application.routes.draw match :subscribe, :unsubscribe, :on => :member, :via => [:get, :post] end get "/changeset/:id/comments/feed" => "changeset_comments#index", :as => :changeset_comments_feed, :id => /\d*/, :defaults => { :format => "rss" } - resources :notes, :path => "note", :only => [:show, :new] + resources :notes, :path => "note", :id => /\d+/, :only => [:show, :new] get "/user/:display_name/history" => "changesets#index" get "/user/:display_name/history/feed" => "changesets#feed", :defaults => { :format => :atom } @@@ -347,6 -347,7 +347,7 @@@ resources :redactions # errors + match "/400", :to => "errors#bad_request", :via => :all match "/403", :to => "errors#forbidden", :via => :all match "/404", :to => "errors#not_found", :via => :all match "/500", :to => "errors#internal_server_error", :via => :all diff --combined test/controllers/changesets_controller_test.rb index 3d264181c,44022ba20..a486e4b5e --- a/test/controllers/changesets_controller_test.rb +++ b/test/controllers/changesets_controller_test.rb @@@ -92,6 -92,15 +92,15 @@@ class ChangesetsControllerTest < Action check_index_result(changesets.last(20)) end + ## + # This should report an error + def test_index_invalid_xhr + %w[-1 0 fred].each do |id| + get history_path(:format => "html", :list => "1", :max_id => id) + assert_redirected_to :controller => :errors, :action => :bad_request + end + end + ## # This should display the last 20 changesets closed in a specific area def test_index_bbox @@@ -321,26 -330,13 +330,26 @@@ def test_show_adjacent_changesets user = create(:user) - changesets = create_list(:changeset, 3, :user => user) + changesets = create_list(:changeset, 3, :user => user, :num_changes => 1) sidebar_browse_check :changeset_path, changesets[1].id, "changesets/show" assert_dom "a[href='#{changeset_path changesets[0]}']", :count => 1 assert_dom "a[href='#{changeset_path changesets[2]}']", :count => 1 end + def test_show_adjacent_nonempty_changesets + user = create(:user) + changeset1 = create(:changeset, :user => user, :num_changes => 1) + create(:changeset, :user => user, :num_changes => 0) + changeset3 = create(:changeset, :user => user, :num_changes => 1) + create(:changeset, :user => user, :num_changes => 0) + changeset5 = create(:changeset, :user => user, :num_changes => 1) + + sidebar_browse_check :changeset_path, changeset3.id, "changesets/show" + assert_dom "a[href='#{changeset_path changeset1}']", :count => 1 + assert_dom "a[href='#{changeset_path changeset5}']", :count => 1 + end + ## # This should display the last 20 non-empty changesets def test_feed diff --combined test/controllers/notes_controller_test.rb index a54334269,c778f41c2..4092ad732 --- a/test/controllers/notes_controller_test.rb +++ b/test/controllers/notes_controller_test.rb @@@ -83,6 -83,15 +83,15 @@@ class NotesControllerTest < ActionDispa assert_select "table.note_list tbody tr", :count => 10 end + def test_index_invalid_paged + user = create(:user) + + %w[-1 0 fred].each do |page| + get user_notes_path(user, :page => page) + assert_redirected_to :controller => :errors, :action => :bad_request + end + end + def test_empty_page user = create(:user) get user_notes_path(user) @@@ -93,7 -102,7 +102,7 @@@ def test_read_note open_note = create(:note_with_comments) - browse_check :note_path, open_note.id, "notes/show" + sidebar_browse_check :note_path, open_note.id, "notes/show" end def test_read_hidden_note @@@ -111,7 -120,7 +120,7 @@@ session_for(create(:moderator_user)) - browse_check :note_path, hidden_note_with_comment.id, "notes/show" + sidebar_browse_check :note_path, hidden_note_with_comment.id, "notes/show" end def test_read_note_hidden_comments @@@ -119,12 -128,12 +128,12 @@@ create(:note_comment, :note => note, :visible => false) end - browse_check :note_path, note_with_hidden_comment.id, "notes/show" + sidebar_browse_check :note_path, note_with_hidden_comment.id, "notes/show" assert_select "div.note-comments ul li", :count => 1 session_for(create(:moderator_user)) - browse_check :note_path, note_with_hidden_comment.id, "notes/show" + sidebar_browse_check :note_path, note_with_hidden_comment.id, "notes/show" assert_select "div.note-comments ul li", :count => 2 end @@@ -134,12 -143,12 +143,12 @@@ create(:note_comment, :note => note, :author => hidden_user) end - browse_check :note_path, note_with_hidden_user_comment.id, "notes/show" + sidebar_browse_check :note_path, note_with_hidden_user_comment.id, "notes/show" assert_select "div.note-comments ul li", :count => 1 session_for(create(:moderator_user)) - browse_check :note_path, note_with_hidden_user_comment.id, "notes/show" + sidebar_browse_check :note_path, note_with_hidden_user_comment.id, "notes/show" assert_select "div.note-comments ul li", :count => 1 end @@@ -147,7 -156,7 +156,7 @@@ user = create(:user) closed_note = create(:note_with_comments, :closed, :closed_by => user, :comments_count => 2) - browse_check :note_path, closed_note.id, "notes/show" + sidebar_browse_check :note_path, closed_note.id, "notes/show" assert_select "div.note-comments ul li", :count => 2 assert_select "div.details", /Resolved by #{user.display_name}/ @@@ -155,24 -164,52 +164,24 @@@ reset! - browse_check :note_path, closed_note.id, "notes/show" + sidebar_browse_check :note_path, closed_note.id, "notes/show" assert_select "div.note-comments ul li", :count => 1 assert_select "div.details", /Resolved by deleted/ end - def test_new_note + def test_new_note_anonymous get new_note_path assert_response :success assert_template "notes/new" + assert_select "#sidebar_content a[href='#{login_path(:referer => new_note_path)}']", :count => 1 end - private - - # This is a convenience method for most of the above checks - # First we check that when we don't have an id, it will correctly return a 404 - # then we check that we get the correct 404 when a non-existant id is passed - # then we check that it will get a successful response, when we do pass an id - def browse_check(path, id, template) - path_method = method(path) - - assert_raise ActionController::UrlGenerationError do - get path_method.call - end - - # assert_raise ActionController::UrlGenerationError do - # get path_method.call(:id => -10) # we won't have an id that's negative - # end - - get path_method.call(:id => 0) - assert_response :not_found - assert_template "browse/not_found" - assert_template :layout => "map" - - get path_method.call(:id => 0), :xhr => true - assert_response :not_found - assert_template "browse/not_found" - assert_template :layout => "xhr" - - get path_method.call(:id => id) - assert_response :success - assert_template template - assert_template :layout => "map" + def test_new_note + session_for(create(:user)) - get path_method.call(:id => id), :xhr => true + get new_note_path assert_response :success - assert_template template - assert_template :layout => "xhr" + assert_template "notes/new" + assert_select "#sidebar_content a[href='#{login_path(:referer => new_note_path)}']", :count => 0 end end diff --combined test/controllers/users_controller_test.rb index c5566e65d,02cc2eb58..cff52cff2 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@@ -82,6 -82,7 +82,6 @@@ class UsersControllerTest < ActionDispa assert_select "div#content", :count => 1 do assert_select "form[action='/user/new'][method='post']", :count => 1 do assert_select "input[id='user_email']", :count => 1 - assert_select "input[id='user_email_confirmation']", :count => 1 assert_select "input[id='user_display_name']", :count => 1 assert_select "input[id='user_pass_crypt'][type='password']", :count => 1 assert_select "input[id='user_pass_crypt_confirmation'][type='password']", :count => 1 @@@ -105,10 -106,18 +105,10 @@@ def test_new_success user = build(:user, :pending) - assert_no_difference "User.count" do - assert_no_difference "ActionMailer::Base.deliveries.size" do - perform_enqueued_jobs do - post user_new_path, :params => { :user => user.attributes } - end - end - end - assert_difference "User.count", 1 do assert_difference "ActionMailer::Base.deliveries.size", 1 do perform_enqueued_jobs do - post user_save_path, :params => { :read_ct => 1, :read_tou => 1 } + post user_new_path, :params => { :user => user.attributes } end end end @@@ -142,14 -151,55 +142,14 @@@ assert_select "form > div > input.is-invalid#user_email" end - def test_save_duplicate_email - user = build(:user, :pending) - - # Set up our user as being half-way through registration - assert_no_difference "User.count" do - assert_no_difference "ActionMailer::Base.deliveries.size" do - perform_enqueued_jobs do - post user_new_path, :params => { :user => user.attributes } - end - end - end - - # Now create another user with that email - create(:user, :email => user.email) - - # Check that the second half of registration fails - assert_no_difference "User.count" do - assert_no_difference "ActionMailer::Base.deliveries.size" do - perform_enqueued_jobs do - post user_save_path, :params => { :read_ct => 1, :read_tou => 1 } - end - end - end - - assert_response :success - assert_template "new" - assert_select "form > div > input.is-invalid#user_email" - end - - def test_save_duplicate_email_uppercase + def test_new_duplicate_email_uppercase user = build(:user, :pending) - - # Set up our user as being half-way through registration - assert_no_difference "User.count" do - assert_no_difference "ActionMailer::Base.deliveries.size" do - perform_enqueued_jobs do - post user_new_path, :params => { :user => user.attributes } - end - end - end - - # Now create another user with that email, but uppercased create(:user, :email => user.email.upcase) - # Check that the second half of registration fails assert_no_difference "User.count" do assert_no_difference "ActionMailer::Base.deliveries.size" do perform_enqueued_jobs do - post user_save_path, :params => { :read_ct => 1, :read_tou => 1 } + post user_new_path, :params => { :user => user.attributes } end end end @@@ -159,14 -209,26 +159,14 @@@ assert_select "form > div > input.is-invalid#user_email" end - def test_save_duplicate_name + def test_new_duplicate_name user = build(:user, :pending) - - # Set up our user as being half-way through registration - assert_no_difference "User.count" do - assert_no_difference "ActionMailer::Base.deliveries.size" do - perform_enqueued_jobs do - post user_new_path, :params => { :user => user.attributes } - end - end - end - - # Now create another user with that display name create(:user, :display_name => user.display_name) - # Check that the second half of registration fails assert_no_difference "User.count" do assert_no_difference "ActionMailer::Base.deliveries.size" do perform_enqueued_jobs do - post user_save_path, :params => { :read_ct => 1, :read_tou => 1 } + post user_new_path, :params => { :user => user.attributes } end end end @@@ -176,14 -238,26 +176,14 @@@ assert_select "form > div > input.is-invalid#user_display_name" end - def test_save_duplicate_name_uppercase + def test_new_duplicate_name_uppercase user = build(:user, :pending) - - # Set up our user as being half-way through registration - assert_no_difference "User.count" do - assert_no_difference "ActionMailer::Base.deliveries.size" do - perform_enqueued_jobs do - post user_new_path, :params => { :user => user.attributes } - end - end - end - - # Now create another user with that display_name, but uppercased create(:user, :display_name => user.display_name.upcase) - # Check that the second half of registration fails assert_no_difference "User.count" do assert_no_difference "ActionMailer::Base.deliveries.size" do perform_enqueued_jobs do - post user_save_path, :params => { :read_ct => 1, :read_tou => 1 } + post user_new_path, :params => { :user => user.attributes } end end end @@@ -193,9 -267,18 +193,9 @@@ assert_select "form > div > input.is-invalid#user_display_name" end - def test_save_blocked_domain + def test_new_blocked_domain user = build(:user, :pending, :email => "user@example.net") - # Set up our user as being half-way through registration - assert_no_difference "User.count" do - assert_no_difference "ActionMailer::Base.deliveries.size" do - perform_enqueued_jobs do - post user_new_path, :params => { :user => user.attributes } - end - end - end - # Now block that domain create(:acl, :domain => "example.net", :k => "no_account_creation") @@@ -203,7 -286,7 +203,7 @@@ assert_no_difference "User.count" do assert_no_difference "ActionMailer::Base.deliveries.size" do perform_enqueued_jobs do - post user_save_path, :params => { :read_ct => 1, :read_tou => 1 } + post user_new_path, :params => { :user => user.attributes } end end end @@@ -215,9 -298,18 +215,9 @@@ def test_save_referer_params user = build(:user, :pending) - # Set up our user as being half-way through registration - assert_no_difference "User.count" do - assert_no_difference "ActionMailer::Base.deliveries.size" do - perform_enqueued_jobs do - post user_new_path, :params => { :user => user.attributes, :referer => "/edit?editor=id#map=1/2/3" } - end - end - end - assert_difference "User.count", 1 do assert_difference "ActionMailer::Base.deliveries.size", 1 do - post user_save_path, :params => { :read_ct => 1, :read_tou => 1 } + post user_new_path, :params => { :user => user.attributes, :referer => "/edit?editor=id#map=1/2/3" } assert_enqueued_with :job => ActionMailer::MailDeliveryJob, :args => proc { |args| args[3][:args][2] == welcome_path(:editor => "id", :zoom => 1, :lat => 2, :lon => 3) } perform_enqueued_jobs @@@ -227,6 -319,24 +227,6 @@@ ActionMailer::Base.deliveries.clear end - def test_terms_new_user - user = build(:user, :pending) - - # Set up our user as being half-way through registration - assert_no_difference "User.count" do - assert_no_difference "ActionMailer::Base.deliveries.size" do - perform_enqueued_jobs do - post user_new_path, :params => { :user => user.attributes } - end - end - end - - get user_terms_path - - assert_response :success - assert_template :terms - end - def test_terms_agreed user = create(:user, :terms_seen => true, :terms_agreed => Date.yesterday) @@@ -558,6 -668,18 +558,18 @@@ check_no_page_link "Older Users" end + def test_index_get_invalid_paginated + session_for(create(:administrator_user)) + + %w[-1 0 fred].each do |id| + get users_path(:before => id) + assert_redirected_to :controller => :errors, :action => :bad_request + + get users_path(:after => id) + assert_redirected_to :controller => :errors, :action => :bad_request + end + end + private def check_no_page_link(name)