From: Tom Hughes Date: Wed, 17 Jan 2024 18:41:16 +0000 (+0000) Subject: Merge remote-tracking branch 'upstream/pull/4462' X-Git-Tag: live~932 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/9387df91414616b9668185b72a9457e53937523d?hp=d1b58fb20e90913a6caffca7f5857527cc26bec4 Merge remote-tracking branch 'upstream/pull/4462' --- diff --git a/.rubocop.yml b/.rubocop.yml index 1e18afd83..ddfb63cae 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -68,6 +68,9 @@ Rails/SkipsModelValidations: - 'db/migrate/*.rb' - 'app/controllers/users_controller.rb' +Style/ArgumentsForwarding: + Enabled: false + Style/Documentation: Enabled: false diff --git a/Gemfile.lock b/Gemfile.lock index 0135ebb87..e0ffd49be 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -4,35 +4,35 @@ GEM aasm (5.5.0) concurrent-ruby (~> 1.0) abbrev (0.1.2) - actioncable (7.1.2) - actionpack (= 7.1.2) - activesupport (= 7.1.2) + actioncable (7.1.3) + actionpack (= 7.1.3) + activesupport (= 7.1.3) nio4r (~> 2.0) websocket-driver (>= 0.6.1) zeitwerk (~> 2.6) - actionmailbox (7.1.2) - actionpack (= 7.1.2) - activejob (= 7.1.2) - activerecord (= 7.1.2) - activestorage (= 7.1.2) - activesupport (= 7.1.2) + actionmailbox (7.1.3) + actionpack (= 7.1.3) + activejob (= 7.1.3) + activerecord (= 7.1.3) + activestorage (= 7.1.3) + activesupport (= 7.1.3) mail (>= 2.7.1) net-imap net-pop net-smtp - actionmailer (7.1.2) - actionpack (= 7.1.2) - actionview (= 7.1.2) - activejob (= 7.1.2) - activesupport (= 7.1.2) + actionmailer (7.1.3) + actionpack (= 7.1.3) + actionview (= 7.1.3) + activejob (= 7.1.3) + activesupport (= 7.1.3) mail (~> 2.5, >= 2.5.4) net-imap net-pop net-smtp rails-dom-testing (~> 2.2) - actionpack (7.1.2) - actionview (= 7.1.2) - activesupport (= 7.1.2) + actionpack (7.1.3) + actionview (= 7.1.3) + activesupport (= 7.1.3) nokogiri (>= 1.8.5) racc rack (>= 2.2.4) @@ -42,39 +42,39 @@ GEM rails-html-sanitizer (~> 1.6) actionpack-page_caching (1.2.4) actionpack (>= 4.0.0) - actiontext (7.1.2) - actionpack (= 7.1.2) - activerecord (= 7.1.2) - activestorage (= 7.1.2) - activesupport (= 7.1.2) + actiontext (7.1.3) + actionpack (= 7.1.3) + activerecord (= 7.1.3) + activestorage (= 7.1.3) + activesupport (= 7.1.3) globalid (>= 0.6.0) nokogiri (>= 1.8.5) - actionview (7.1.2) - activesupport (= 7.1.2) + actionview (7.1.3) + activesupport (= 7.1.3) builder (~> 3.1) erubi (~> 1.11) rails-dom-testing (~> 2.2) rails-html-sanitizer (~> 1.6) active_record_union (1.3.0) activerecord (>= 4.0) - activejob (7.1.2) - activesupport (= 7.1.2) + activejob (7.1.3) + activesupport (= 7.1.3) globalid (>= 0.3.6) - activemodel (7.1.2) - activesupport (= 7.1.2) - activerecord (7.1.2) - activemodel (= 7.1.2) - activesupport (= 7.1.2) + activemodel (7.1.3) + activesupport (= 7.1.3) + activerecord (7.1.3) + activemodel (= 7.1.3) + activesupport (= 7.1.3) timeout (>= 0.4.0) activerecord-import (1.5.1) activerecord (>= 4.2) - activestorage (7.1.2) - actionpack (= 7.1.2) - activejob (= 7.1.2) - activerecord (= 7.1.2) - activesupport (= 7.1.2) + activestorage (7.1.3) + actionpack (= 7.1.3) + activejob (= 7.1.3) + activerecord (= 7.1.3) + activesupport (= 7.1.3) marcel (~> 1.0) - activesupport (7.1.2) + activesupport (7.1.3) base64 bigdecimal concurrent-ruby (~> 1.0, >= 1.0.2) @@ -96,8 +96,8 @@ GEM autoprefixer-rails (10.4.16.0) execjs (~> 2) aws-eventstream (1.3.0) - aws-partitions (1.877.0) - aws-sdk-core (3.190.1) + aws-partitions (1.880.0) + aws-sdk-core (3.190.2) aws-eventstream (~> 1, >= 1.3.0) aws-partitions (~> 1, >= 1.651.0) aws-sigv4 (~> 1.8) @@ -126,7 +126,7 @@ GEM bigdecimal (3.1.5) binding_of_caller (1.0.0) debug_inspector (>= 0.0.1) - bootsnap (1.17.0) + bootsnap (1.17.1) msgpack (~> 1.2) bootstrap (5.3.2) autoprefixer-rails (>= 9.1.0) @@ -153,7 +153,7 @@ GEM rack-test (>= 0.6.3) regexp_parser (>= 1.5, < 3.0) xpath (~> 3.2) - concurrent-ruby (1.2.2) + concurrent-ruby (1.2.3) config (5.1.0) deep_merge (~> 1.2, >= 1.2.1) dry-validation (~> 1.0, >= 1.0.0) @@ -289,7 +289,7 @@ GEM image_processing (1.12.2) mini_magick (>= 4.9.5, < 5) ruby-vips (>= 2.0.17, < 3) - image_size (3.3.0) + image_size (3.4.0) in_threads (1.6.0) io-console (0.7.1) irb (1.11.1) @@ -333,7 +333,7 @@ GEM mini_portile2 (2.8.5) mini_racer (0.8.0) libv8-node (~> 18.16.0.0) - minitest (5.20.0) + minitest (5.21.1) msgpack (1.7.2) multi_json (1.15.0) multi_xml (0.6.0) @@ -401,7 +401,7 @@ GEM omniauth (~> 2.0) openstreetmap-deadlock_retry (1.3.1) parallel (1.24.0) - parser (3.3.0.2) + parser (3.3.0.4) ast (~> 2.4.1) racc pg (1.5.4) @@ -431,20 +431,20 @@ GEM rackup (1.0.0) rack (< 3) webrick - rails (7.1.2) - actioncable (= 7.1.2) - actionmailbox (= 7.1.2) - actionmailer (= 7.1.2) - actionpack (= 7.1.2) - actiontext (= 7.1.2) - actionview (= 7.1.2) - activejob (= 7.1.2) - activemodel (= 7.1.2) - activerecord (= 7.1.2) - activestorage (= 7.1.2) - activesupport (= 7.1.2) + rails (7.1.3) + actioncable (= 7.1.3) + actionmailbox (= 7.1.3) + actionmailer (= 7.1.3) + actionpack (= 7.1.3) + actiontext (= 7.1.3) + actionview (= 7.1.3) + activejob (= 7.1.3) + activemodel (= 7.1.3) + activerecord (= 7.1.3) + activestorage (= 7.1.3) + activesupport (= 7.1.3) bundler (>= 1.15.0) - railties (= 7.1.2) + railties (= 7.1.3) rails-controller-testing (1.0.5) actionpack (>= 5.0.1.rc1) actionview (>= 5.0.1.rc1) @@ -459,9 +459,9 @@ GEM rails-i18n (7.0.8) i18n (>= 0.7, < 2) railties (>= 6.0.0, < 8) - railties (7.1.2) - actionpack (= 7.1.2) - activesupport (= 7.1.2) + railties (7.1.3) + actionpack (= 7.1.3) + activesupport (= 7.1.3) irb rackup (>= 1.0.0) rake (>= 12.2) @@ -485,11 +485,11 @@ GEM rouge (4.2.0) rtlcss (0.2.1) mini_racer (>= 0.6.3) - rubocop (1.59.0) + rubocop (1.60.0) json (~> 2.3) language_server-protocol (>= 3.17.0) parallel (~> 1.10) - parser (>= 3.2.2.4) + parser (>= 3.3.0.2) rainbow (>= 2.2.2, < 4.0) regexp_parser (>= 1.8, < 3.0) rexml (>= 3.2.5, < 4.0) diff --git a/INSTALL.md b/INSTALL.md index ec219b504..63aad6f1a 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -23,7 +23,7 @@ of packages required before you can get the various gems installed. ## Minimum requirements * Ruby 3.0+ -* PostgreSQL 12+ +* PostgreSQL 13+ * Bundler (see note below about [developer Ruby setup](#rbenv)) * Javascript Runtime @@ -226,11 +226,11 @@ After installing this software, you may need to carry out some [configuration st # Ruby development install and versions (optional) -For simplicity, this document explains how to install all the website dependencies as "system" dependencies. While this is simpler, and usually faster, you might want more control over the process or the ability to install multiple different versions of software alongside eachother. For many developers, [`rbenv`](https://github.com/rbenv/rbenv) is the easiest way to manage multiple different Ruby versions on the same computer - with the added advantage that the installs are all in your home directory, so you don't need administrator permissions. +For simplicity, this document explains how to install all the website dependencies as "system" dependencies. While this is simpler, and usually faster, you might want more control over the process or the ability to install multiple different versions of software alongside each other. For many developers, [`rbenv`](https://github.com/rbenv/rbenv) is the easiest way to manage multiple different Ruby versions on the same computer - with the added advantage that the installs are all in your home directory, so you don't need administrator permissions. If you choose to install Ruby and Bundler via `rbenv`, then you do not need to install the system libraries for Ruby: -* For Ubuntu, you do not need to install the following packages: `ruby3.0 libruby3.0 ruby3.0-dev bundler`, +* For Ubuntu, you do not need to install the following packages: `ruby ruby-dev ruby-bundler`, * For Fedora, you do not need to install the following packages: `ruby ruby-devel rubygem-rdoc rubygem-bundler rubygems` * For MacOSX, you do not need to `brew install ruby` - but make sure you've installed a version of Ruby using `rbenv` before running `gem install bundler`! diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb index 4876380d0..0eae46f82 100644 --- a/app/abilities/api_ability.rb +++ b/app/abilities/api_ability.rb @@ -5,7 +5,6 @@ class ApiAbility def initialize(user) can :show, :capability - can :index, :change can :index, :map can :show, :permission can :show, :version @@ -22,17 +21,9 @@ class ApiAbility can [:history, :version], OldWay can [:history, :version], OldRelation can [:show], UserBlock - end - - if user&.active? - 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 + if user&.active? can [:comment, :close, :reopen], Note - can [:new, :create], Report can [:create, :show, :update, :destroy, :data], Trace can [:details, :gpx_files], User can [:index, :show, :update, :update_all, :destroy], UserPreference diff --git a/app/models/user.rb b/app/models/user.rb index bc95f20c1..b04f8e2b9 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -34,12 +34,13 @@ # # Indexes # -# users_auth_idx (auth_provider,auth_uid) UNIQUE -# users_display_name_idx (display_name) UNIQUE -# users_display_name_lower_idx (lower((display_name)::text)) -# users_email_idx (email) UNIQUE -# users_email_lower_idx (lower((email)::text)) -# users_home_idx (home_tile) +# users_auth_idx (auth_provider,auth_uid) UNIQUE +# users_display_name_canonical_idx (lower(NORMALIZE(display_name, NFKC))) +# users_display_name_idx (display_name) UNIQUE +# users_display_name_lower_idx (lower((display_name)::text)) +# users_email_idx (email) UNIQUE +# users_email_lower_idx (lower((email)::text)) +# users_home_idx (home_tile) # class User < ApplicationRecord @@ -95,7 +96,7 @@ class User < ApplicationRecord validates :display_name, :presence => true, :length => 3..255, :exclusion => %w[new terms save confirm confirm-email go_public reset-password forgot-password suspended] validates :display_name, :if => proc { |u| u.display_name_changed? }, - :uniqueness => { :case_sensitive => false } + :normalized_uniqueness => { :case_sensitive => false } validates :display_name, :if => proc { |u| u.display_name_changed? }, :characters => { :url_safe => true }, :whitespace => { :leading => false, :trailing => false } @@ -132,7 +133,7 @@ class User < ApplicationRecord user = find_by("email = ? OR display_name = ?", options[:username].strip, options[:username]) if user.nil? - users = where("LOWER(email) = LOWER(?) OR LOWER(display_name) = LOWER(?)", options[:username].strip, options[:username]) + users = where("LOWER(email) = LOWER(?) OR LOWER(NORMALIZE(display_name, NFKC)) = LOWER(NORMALIZE(?, NFKC))", options[:username].strip, options[:username]) user = users.first if users.count == 1 end diff --git a/app/validators/normalized_uniqueness_validator.rb b/app/validators/normalized_uniqueness_validator.rb new file mode 100644 index 000000000..eb3600c7b --- /dev/null +++ b/app/validators/normalized_uniqueness_validator.rb @@ -0,0 +1,18 @@ +class NormalizedUniquenessValidator < ActiveModel::EachValidator + def validate_each(record, attribute, value) + relation = if options.fetch(:case_sensitive, true) + record.class.where("NORMALIZE(#{attribute}, NFKC) = NORMALIZE(?, NFKC)", value) + else + record.class.where("LOWER(NORMALIZE(#{attribute}, NFKC)) = LOWER(NORMALIZE(?, NFKC))", value) + end + + relation = relation.where.not(record.class.primary_key => [record.id_in_database]) if record.persisted? + + if relation.exists? + error_options = options.except(:case_sensitive) + error_options[:value] = value + + record.errors.add(attribute, :taken, **error_options) + end + end +end diff --git a/db/migrate/20231213182102_add_canonical_user_index.rb b/db/migrate/20231213182102_add_canonical_user_index.rb new file mode 100644 index 000000000..905fb3248 --- /dev/null +++ b/db/migrate/20231213182102_add_canonical_user_index.rb @@ -0,0 +1,7 @@ +class AddCanonicalUserIndex < ActiveRecord::Migration[7.1] + disable_ddl_transaction! + + def change + add_index :users, "LOWER(NORMALIZE(display_name, NFKC))", :name => "users_display_name_canonical_idx", :algorithm => :concurrently + end +end diff --git a/db/structure.sql b/db/structure.sql index 0563417cd..c9fdebbc8 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -2922,6 +2922,13 @@ CREATE INDEX user_tokens_user_id_idx ON public.user_tokens USING btree (user_id) CREATE UNIQUE INDEX users_auth_idx ON public.users USING btree (auth_provider, auth_uid); +-- +-- Name: users_display_name_canonical_idx; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX users_display_name_canonical_idx ON public.users USING btree (lower(NORMALIZE(display_name, NFKC))); + + -- -- Name: users_display_name_idx; Type: INDEX; Schema: public; Owner: - -- @@ -3581,6 +3588,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('23'), ('22'), ('21'), +('20231213182102'), ('20231206141457'), ('20231117170422'), ('20231101222146'), diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 4e2675a2e..d21512f2a 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -27,10 +27,13 @@ class UserTest < ActiveSupport::TestCase end def test_unique_display_name - existing_user = create(:user) - new_user = build(:user, :display_name => existing_user.display_name) - assert_not new_user.save - assert_includes new_user.errors[:display_name], "has already been taken" + create(:user, :display_name => "H\u{e9}nryIV") + + %W[H\u{e9}nryIV he\u{301}nryiv H\u{c9}nry\u2163 he\u{301}nry\u2173].each do |name| + new_user = build(:user, :display_name => name) + assert_not new_user.save + assert_includes new_user.errors[:display_name], "has already been taken" + end end def test_email_valid