From 680eb019f700b6d109bfd8dc5d552dea72ecb8c7 Mon Sep 17 00:00:00 2001 From: Chris Oliver Date: Wed, 22 Mar 2023 09:03:54 -0500 Subject: [PATCH] Add fallback: true option --- CHANGELOG.md | 4 ++++ Gemfile.lock | 4 ++-- README.md | 6 ++++-- gemfiles/rails_6.gemfile.lock | 4 ++-- gemfiles/rails_6_1.gemfile.lock | 4 ++-- gemfiles/rails_7_0.gemfile.lock | 4 ++-- gemfiles/rails_master.gemfile.lock | 4 ++-- lib/prefixed_ids.rb | 13 ++++++++++--- lib/prefixed_ids/prefix_id.rb | 8 +++++--- lib/prefixed_ids/version.rb | 2 +- test/dummy/app/models/team.rb | 3 +++ .../db/migrate/20230322131821_create_teams.rb | 8 ++++++++ test/dummy/db/schema.rb | 19 ++++++++++++------- test/dummy/test/models/team_test.rb | 7 +++++++ test/fixtures/teams.yml | 8 ++++++++ test/prefixed_ids_test.rb | 15 +++++++++++++++ 16 files changed, 87 insertions(+), 26 deletions(-) create mode 100644 test/dummy/app/models/team.rb create mode 100644 test/dummy/db/migrate/20230322131821_create_teams.rb create mode 100644 test/dummy/test/models/team_test.rb create mode 100644 test/fixtures/teams.yml diff --git a/CHANGELOG.md b/CHANGELOG.md index 74dc9c5..d7680e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ ### Unreleased +### 1.5.0 + +* Add `has_prefix_id fallback: false` option to disable lookup by regular ID - @excid3 + ### 1.4.0 * Add `decode_prefix_id` and `decode_prefix_ids` class methods - @TastyPi diff --git a/Gemfile.lock b/Gemfile.lock index 6acdde5..6de3d98 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - prefixed_ids (1.4.0) + prefixed_ids (1.5.0) hashids (>= 1.0.0, < 2.0.0) rails (>= 6.0.0) @@ -203,4 +203,4 @@ DEPENDENCIES standard BUNDLED WITH - 2.4.7 + 2.4.9 diff --git a/README.md b/README.md index 1bb9bc3..c81dd89 100644 --- a/README.md +++ b/README.md @@ -59,7 +59,7 @@ User.find("user_5vJjbzXq9KrLEMm32iAnOP0xGDYk6dpe") User.find_by_prefix_id("user_5vJjbzXq9KrLEMm32iAnOP0xGDYk6dpe") ``` -⚠️ Note that `find` still finds records by the primary key. Eg. `localhost/users/1` still works. +⚠️ Note that `find` still finds records by the primary key. Eg. `localhost/users/1` still works. If you're targeting security issues by masking the ID, make sure to use `find_by_prefix_id` and [add a salt](#salt). We also override `to_param` by default so it'll be used in URLs automatically. @@ -108,10 +108,12 @@ You can customize the prefix, length, and attribute name for PrefixedIds. ```ruby class Account < ApplicationRecord - has_prefix_id :acct, minimum_length: 32, override_find: false, override_param: false, salt: "" + has_prefix_id :acct, minimum_length: 32, override_find: false, override_param: false, salt: "", fallback: false end ``` +By default, `find` will accept both Prefix IDs and regular IDs. Setting `fallback: false` will disable finding by regular IDs and will only allow Prefix IDs. + ## Development After checking out the repo, run `bin/setup` to install dependencies. Then, run `rake test` to run the tests. You can also run `bin/console` for an interactive prompt that will allow you to experiment. diff --git a/gemfiles/rails_6.gemfile.lock b/gemfiles/rails_6.gemfile.lock index 9e0a9e6..37296cf 100644 --- a/gemfiles/rails_6.gemfile.lock +++ b/gemfiles/rails_6.gemfile.lock @@ -1,7 +1,7 @@ PATH remote: .. specs: - prefixed_ids (1.4.0) + prefixed_ids (1.5.0) hashids (>= 1.0.0, < 2.0.0) rails (>= 6.0.0) @@ -202,4 +202,4 @@ DEPENDENCIES standard BUNDLED WITH - 2.4.7 + 2.4.9 diff --git a/gemfiles/rails_6_1.gemfile.lock b/gemfiles/rails_6_1.gemfile.lock index a1ce768..adc7e0b 100644 --- a/gemfiles/rails_6_1.gemfile.lock +++ b/gemfiles/rails_6_1.gemfile.lock @@ -1,7 +1,7 @@ PATH remote: .. specs: - prefixed_ids (1.4.0) + prefixed_ids (1.5.0) hashids (>= 1.0.0, < 2.0.0) rails (>= 6.0.0) @@ -205,4 +205,4 @@ DEPENDENCIES standard BUNDLED WITH - 2.4.7 + 2.4.9 diff --git a/gemfiles/rails_7_0.gemfile.lock b/gemfiles/rails_7_0.gemfile.lock index 4fd2f3b..cd14421 100644 --- a/gemfiles/rails_7_0.gemfile.lock +++ b/gemfiles/rails_7_0.gemfile.lock @@ -1,7 +1,7 @@ PATH remote: .. specs: - prefixed_ids (1.4.0) + prefixed_ids (1.5.0) hashids (>= 1.0.0, < 2.0.0) rails (>= 6.0.0) @@ -202,4 +202,4 @@ DEPENDENCIES standard BUNDLED WITH - 2.4.7 + 2.4.9 diff --git a/gemfiles/rails_master.gemfile.lock b/gemfiles/rails_master.gemfile.lock index 2016f6a..e336d4b 100644 --- a/gemfiles/rails_master.gemfile.lock +++ b/gemfiles/rails_master.gemfile.lock @@ -94,7 +94,7 @@ GIT PATH remote: .. specs: - prefixed_ids (1.4.0) + prefixed_ids (1.5.0) hashids (>= 1.0.0, < 2.0.0) rails (>= 6.0.0) @@ -211,4 +211,4 @@ DEPENDENCIES standard BUNDLED WITH - 2.4.7 + 2.4.9 diff --git a/lib/prefixed_ids.rb b/lib/prefixed_ids.rb index df47114..5d1f6b5 100644 --- a/lib/prefixed_ids.rb +++ b/lib/prefixed_ids.rb @@ -34,14 +34,16 @@ module Rails included do class_attribute :_prefix_id + class_attribute :_prefix_id_fallback end class_methods do - def has_prefix_id(prefix, override_find: true, override_param: true, **options) + def has_prefix_id(prefix, override_find: true, override_param: true, fallback: true, **options) include Attribute include Finder if override_find include ToParam if override_param self._prefix_id = PrefixId.new(self, prefix, **options) + self._prefix_id_fallback = fallback # Register with PrefixedIds to support PrefixedIds#find PrefixedIds.models[prefix.to_s] = self @@ -72,7 +74,7 @@ def decode_prefix_ids(ids) end def prefix_id - self.class._prefix_id.encode(id) + _prefix_id.encode(id) end end @@ -81,7 +83,12 @@ module Finder class_methods do def find(*ids) - super(*ids.map { |id| _prefix_id.decode(id, fallback: true) }) + prefix_ids = *ids.map do |id| + prefix_id = _prefix_id.decode(id, fallback: _prefix_id_fallback) + raise Error, "#{id} is not a valid prefix_id" if !_prefix_id_fallback && prefix_id.nil? + prefix_id + end + super(*prefix_ids) end def relation diff --git a/lib/prefixed_ids/prefix_id.rb b/lib/prefixed_ids/prefix_id.rb index 6466d44..6daf766 100644 --- a/lib/prefixed_ids/prefix_id.rb +++ b/lib/prefixed_ids/prefix_id.rb @@ -19,9 +19,11 @@ def decode(id, fallback: false) fallback_value = fallback ? id : nil _, id_without_prefix = PrefixedIds.split_id(id, @delimiter) decoded_hashid = @hashids.decode(id_without_prefix) - return fallback_value unless valid?(decoded_hashid) - - decoded_hashid.last || fallback_value + if fallback && !valid?(decoded_hashid) + fallback_value + else + decoded_hashid.last + end end private diff --git a/lib/prefixed_ids/version.rb b/lib/prefixed_ids/version.rb index 5d1a979..4c9265e 100644 --- a/lib/prefixed_ids/version.rb +++ b/lib/prefixed_ids/version.rb @@ -1,3 +1,3 @@ module PrefixedIds - VERSION = "1.4.0" + VERSION = "1.5.0" end diff --git a/test/dummy/app/models/team.rb b/test/dummy/app/models/team.rb new file mode 100644 index 0000000..244f9e0 --- /dev/null +++ b/test/dummy/app/models/team.rb @@ -0,0 +1,3 @@ +class Team < ApplicationRecord + has_prefix_id :team, fallback: false +end diff --git a/test/dummy/db/migrate/20230322131821_create_teams.rb b/test/dummy/db/migrate/20230322131821_create_teams.rb new file mode 100644 index 0000000..fe53f41 --- /dev/null +++ b/test/dummy/db/migrate/20230322131821_create_teams.rb @@ -0,0 +1,8 @@ +class CreateTeams < ActiveRecord::Migration[7.0] + def change + create_table :teams do |t| + + t.timestamps + end + end +end diff --git a/test/dummy/db/schema.rb b/test/dummy/db/schema.rb index cbc5068..09932f9 100644 --- a/test/dummy/db/schema.rb +++ b/test/dummy/db/schema.rb @@ -10,23 +10,28 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_05_03_145247) do +ActiveRecord::Schema[7.0].define(version: 2023_03_22_131821) do create_table "accounts", force: :cascade do |t| t.integer "user_id" - t.datetime "created_at", precision: 6, null: false - t.datetime "updated_at", precision: 6, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false end create_table "posts", force: :cascade do |t| t.integer "user_id", null: false - t.datetime "created_at", precision: 6, null: false - t.datetime "updated_at", precision: 6, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false t.index ["user_id"], name: "index_posts_on_user_id" end + create_table "teams", force: :cascade do |t| + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + create_table "users", force: :cascade do |t| - t.datetime "created_at", precision: 6, null: false - t.datetime "updated_at", precision: 6, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false end add_foreign_key "posts", "users" diff --git a/test/dummy/test/models/team_test.rb b/test/dummy/test/models/team_test.rb new file mode 100644 index 0000000..c6cf23d --- /dev/null +++ b/test/dummy/test/models/team_test.rb @@ -0,0 +1,7 @@ +require "test_helper" + +class TeamTest < ActiveSupport::TestCase + # test "the truth" do + # assert true + # end +end diff --git a/test/fixtures/teams.yml b/test/fixtures/teams.yml new file mode 100644 index 0000000..c14ecbe --- /dev/null +++ b/test/fixtures/teams.yml @@ -0,0 +1,8 @@ +# Read about fixtures at https://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html + +# This model initially had no columns defined. If you add columns to the +# model remove the "{}" from the fixture names and add the columns immediately +# below each fixture, per the syntax in the comments below +# +one: {} +two: {} diff --git a/test/prefixed_ids_test.rb b/test/prefixed_ids_test.rb index ce1080f..4a521d3 100644 --- a/test/prefixed_ids_test.rb +++ b/test/prefixed_ids_test.rb @@ -148,4 +148,19 @@ class PrefixedIdsTest < ActiveSupport::TestCase test "can override salt on model" do assert_equal "accountsabcd", Account._prefix_id.hashids.salt end + + test "decode with fallback false returns nil for regular ID" do + assert_nil Team._prefix_id.decode(1) + end + + test "disabled fallback allows find by prefix id" do + team = Team.find_by(id: ActiveRecord::FixtureSet.identify(:one)) + assert_equal team, Team.find(team.prefix_id) + end + + test "disabled fallback raises an error if not prefix_id" do + assert_raises PrefixedIds::Error do + Team.find(ActiveRecord::FixtureSet.identify(:one)) + end + end end