excid3
My first User's ID and your first User's ID will be the same
I hope this issue can be the start of a conversation that will eventually lead to me opening a PR :smiley:
Reading https://github.com/excid3/prefixed_ids/blob/master/lib/prefixed_ids/prefix_id.rb#L10 it looks like the only salt for the IDs is the table name, which means that if we both call our Users model "User" and go with the Rails default of a table called "users" that will give us both the same prefixed IDs for 1, 2, 3, etc. Specifically:
> 1.upto(5) { |i| puts "#{i}: #{User.new(id: i).prefix_id}" }
1: user_zWNY2gOL1xwrYuRqXQnjJZyP
2: user_dpN6gn4RBqyD1cM87QeJzk2j
3: user_P6LrYRp4kb2kYSL8v3XgwO9E
4: user_dgy1jVRA4bAmYtObwMYWLm3Q
5: user_QkV4oMPl98aK7hOx3R1KAe6J
This means that a savvy attacker could potentially use Insecure Direct Object References to enumerate through our users. Or from the other side, a project using the prefixed_ids gem without reading the source could believe they have secure IDs when they don't.
I think a good solution would be to change the salt from model.table_name to something like "#{model.table_name}--#{Rails.application.credentials.secret_key_base}".
This could be something configurable via an option passed to has_prefix_id to maintain backwards compatibility... although for security I wonder if this should be the default and turning it off could be possible for people who need backwards compatibility.
The best way to support backwards compatibility while avoiding unexpected ID changes could be to use a different variable from Rails.application.credentials like prefixed_ids_secret_key and throw a loud exception if that doesn't exist, but use just the table name if it is set to the empty string. That way anybody upgrading the gem will know there's been a breaking change and be able to decide which route makes the most sense for them from there.
Let me know what you think :smile:
This is all based on hashid-rails which doesn't have a salt by default. We can add an option to include a configurable salt and a configurable pepper like they do? That would keep it consistent with that gem.
https://github.com/jcypret/hashid-rails#configuration-optional
I like the idea of adding an initializer-based configuration block, but it feels like quite a big departure from how prefixed_ids are configured per-model at the moment. Were you thinking that the initializer/configure block would set up defaults for all prefixed IDs which could then be overridden by passing arguments to has_prefix_id, like like override_find and minimum_length are now?