Merge pull request #2625 from manyfold3d/public-ids

Change ID numbers in URLs to more secure randomised strings

Gemfile

@@ -147,3 +147,5 @@ gem "devise_zxcvbn", "~> 6.0"
 gem "ransack", "~> 4.2"
 gem "federails", git: "https://gitlab.com/manyfold3d/federails.git", branch: "polymorphic_actor_relationship"
 gem "caber"
+
+gem "nanoid", "~> 2.0"

Gemfile.lock

@@ -388,6 +388,7 @@ GEM
     multi_json (1.15.0)
     mutex_m (0.2.0)
     mysql2 (0.5.6)
+    nanoid (2.0.0)
     nenv (0.3.0)
     net-http (0.4.1)
       uri
@@ -721,6 +722,7 @@ DEPENDENCIES
   memoist (~> 0.16.2)
   mittsu!
   mysql2 (~> 0.5.6)
+  nanoid (~> 2.0)
   pg (~> 1.5)
   public_suffix (~> 6.0)
   puma (~> 6.4)

app/components/model_component.html.erb

@@ -40,7 +40,7 @@
               <% if @model.collection %>
                 <li>
                   <%= helpers.icon "collection", @model.collection.model_name.human %>
-                  <%= link_to @model.collection.name, models_path((@filters || {}).merge(collection: @model.collection.id)),
+                  <%= link_to @model.collection.name, models_path((@filters || {}).merge(collection: @model.collection)),
                         "aria-label": [@model.collection.model_name.human, @model.collection.name].join(": ") %>
                 </li>
               <% end %>

app/controllers/collections_controller.rb

@@ -75,7 +75,7 @@ def get_collection
       authorize Collection
       @title = t(".unknown")
     else
-      @collection = Collection.includes(:links, :caber_relations).find(params[:id])
+      @collection = Collection.includes(:links, :caber_relations).find_by(public_id: params[:id])
       authorize @collection
       @title = @collection.name
     end

app/controllers/concerns/filterable.rb

@@ -24,7 +24,8 @@ def filtered_models(filters)
 
   # Filter by library
   def filter_by_library(models, library)
-    library ? models.where(library_id: library) : models
+    l = Library.find_by(public_id: library)
+    l ? models.where(library: l) : models
   end
 
   # Filter by collection
@@ -35,8 +36,8 @@ def filter_by_collection(models, collection)
     when ""
       models.where(collection_id: nil)
     else
-      @collection = Collection.find(collection)
-      models.where(collection: Collection.tree_down(collection))
+      @collection = Collection.find_by(public_id: collection)
+      models.where(collection: Collection.tree_down(@collection.id))
     end
   end
 
@@ -48,7 +49,7 @@ def filter_by_creator(models, creator)
     when ""
       models.where(creator_id: nil)
     else
-      @creator = Creator.find(creator)
+      @creator = Creator.find_by(public_id: creator)
       models.where(creator: @creator)
     end
   end
@@ -96,7 +97,7 @@ def filter_by_missing_tag(models, missingtag, library)
     # Missing tags (If specific tag is not specified, require library to be set)
     if missingtag.presence || (missingtag && library)
       tag_regex_build = []
-      regexes = ((missingtag != "") ? [missingtag] : Library.find(library).tag_regex)
+      regexes = ((missingtag != "") ? [missingtag] : Library.find_by(public_id: library).tag_regex)
       # Regexp match syntax - postgres is different from MySQL and SQLite
       regact = ApplicationRecord.connection.is_a?(ActiveRecord::ConnectionAdapters::PostgreSQLAdapter) ? "~" : "REGEXP"
       regexes.each do |reg|

app/controllers/creators_controller.rb

@@ -73,7 +73,7 @@ def get_creator
       authorize Creator
       @title = t(".unknown")
     else
-      @creator = Creator.includes(:links, :caber_relations).find(params[:id])
+      @creator = Creator.includes(:links, :caber_relations).find_by(public_id: params[:id])
       authorize @creator
       @title = @creator.name
     end

app/controllers/libraries_controller.rb

@@ -11,7 +11,7 @@ def index
   end
 
   def show
-    redirect_to models_path(library: @library.id)
+    redirect_to models_path(library: @library)
   end
 
   def new
@@ -80,7 +80,7 @@ def library_params
   end
 
   def get_library
-    @library = Library.find(params[:id])
+    @library = Library.find_by(public_id: params[:id])
     authorize @library
     @title = @library.name
   end

app/controllers/model_files_controller.rb

@@ -25,7 +25,7 @@ def show
 
   def create
     if params[:convert]
-      file = ModelFile.find(params[:convert][:id].to_i)
+      file = ModelFile.find_by(public_id: params[:convert][:id])
       Analysis::FileConversionJob.perform_later(file.id, params[:convert][:to].to_sym)
       redirect_back_or_to [@model, file], notice: t(".conversion_started")
     else
@@ -50,7 +50,7 @@ def bulk_update
     hash = bulk_update_params
     params[:model_files].each_pair do |id, selected|
       if selected == "1"
-        file = @model.model_files.find(id)
+        file = @model.model_files.find_by(public_id: id)
         current_user.set_list_state(file, :printed, params[:printed] === "1")
         if file.update(hash)
           file.save
@@ -103,11 +103,11 @@ def file_params
   end
 
   def get_model
-    @model = Model.find(params[:model_id])
+    @model = Model.find_by(public_id: params[:model_id])
   end
 
   def get_file
-    @file = @model.model_files.includes(:unsupported_version, :presupported_version).find(params[:id])
+    @file = @model.model_files.includes(:unsupported_version, :presupported_version).find_by(public_id: params[:id])
     authorize @file
     @title = @file.name
   end

app/controllers/models_controller.rb

@@ -64,7 +64,7 @@ def edit
 
   def create
     authorize :model
-    library = Library.find(params[:library])
+    library = Library.find_by(public_id: params[:library])
     uploads = begin
       JSON.parse(params[:uploads])[0]["successful"]
     rescue
@@ -89,7 +89,7 @@ def update
   end
 
   def merge
-    if params[:target] && (target = (@model.parents.find { |x| x.id == params[:target].to_i }))
+    if params[:target] && (target = (@model.parents.find { |x| x.public_id == params[:target] }))
       @model.merge_into! target
       Scan::CheckModelIntegrityJob.perform_later(target.id)
       redirect_to target, notice: t(".success")
@@ -130,7 +130,7 @@ def bulk_update
     add_tags = Set.new(hash.delete(:add_tags))
     remove_tags = Set.new(hash.delete(:remove_tags))
     ids = params[:models].select { |k, v| v == "1" }.keys
-    policy_scope(Model).find(ids).each do |model|
+    policy_scope(Model).where(public_id: ids).find_each do |model|
       if model&.update(hash)
         existing_tags = Set.new(model.tag_list)
         model.tag_list = existing_tags + add_tags - remove_tags
@@ -186,7 +186,7 @@ def model_params
   end
 
   def get_model
-    @model = Model.includes(:model_files, :creator, :preview_file, :library, :tags, :taggings, :links, :caber_relations).find(params[:id])
+    @model = Model.includes(:model_files, :creator, :preview_file, :library, :tags, :taggings, :links, :caber_relations).find_by(public_id: params[:id])
     authorize @model
     @title = @model.name
   end

app/controllers/problems_controller.rb

@@ -29,7 +29,7 @@ def index
   end
 
   def update
-    @problem = Problem.unscoped.find(params[:id])
+    @problem = Problem.unscoped.find_by(public_id: params[:id])
     authorize @problem
     @problem.update!(permitted_params)
     notice = t(

app/models/collection.rb

@@ -3,6 +3,7 @@ class Collection < ApplicationRecord
   include CaberObject
   include Linkable
   include Sluggable
+  include PublicIDable
 
   acts_as_federails_actor username_field: :slug, name_field: :name, profile_url_method: :url_for, actor_type: "Collection", include_in_user_count: false
 
@@ -66,7 +67,7 @@ class Collection < ApplicationRecord
   #     )  SELECT * FROM search_tree
 
   def self.ransackable_attributes(_auth_object = nil)
-    ["caption", "created_at", "id", "name", "notes", "slug", "updated_at"]
+    ["caption", "created_at", "id", "public_id", "name", "notes", "slug", "updated_at"]
   end
 
   def self.ransackable_associations(_auth_object = nil)

app/models/concerns/public_i_dable.rb

@@ -0,0 +1,23 @@
+module PublicIDable
+  extend ActiveSupport::Concern
+
+  included do
+    before_validation :generate_public_id
+    validates :public_id, presence: true, uniqueness: true
+  end
+
+  def to_param
+    public_id
+  end
+
+  private
+
+  ALPHABET = "bcdfghjklmnpqrstvwxzBCDFGHJKLMNPQRSTVWXZ0123456789"
+
+  def generate_public_id
+    return if public_id
+    self.public_id = begin
+      Nanoid.generate(size: 8, alphabet: ALPHABET)
+    end while public_id.nil? || self.class.exists?(public_id: public_id)
+  end
+end

app/models/creator.rb

@@ -3,6 +3,7 @@ class Creator < ApplicationRecord
   include CaberObject
   include Linkable
   include Sluggable
+  include PublicIDable
 
   acts_as_federails_actor username_field: :slug, name_field: :name, profile_url_method: :url_for, include_in_user_count: false
 
@@ -12,7 +13,7 @@ class Creator < ApplicationRecord
   default_scope { order(:name) }
 
   def self.ransackable_attributes(_auth_object = nil)
-    ["caption", "created_at", "id", "name", "notes", "slug", "updated_at"]
+    ["caption", "created_at", "id", "public_id", "name", "notes", "slug", "updated_at"]
   end
 
   def self.ransackable_associations(_auth_object = nil)

app/models/library.rb

@@ -2,6 +2,8 @@
 
 class Library < ApplicationRecord
   extend Memoist
+  include PublicIDable
+
   STORAGE_SERVICES = [
     "filesystem",
     "s3"
@@ -54,7 +56,7 @@ def all_tags
   end
 
   def self.ransackable_attributes(auth_object = nil)
-    ["caption", "created_at", "icon", "name", "notes", "tag_regex", "updated_at"]
+    ["public_id", "caption", "created_at", "icon", "name", "notes", "tag_regex", "updated_at"]
   end
 
   def self.ransackable_associations(auth_object = nil)

app/models/model.rb

@@ -6,6 +6,7 @@ class Model < ApplicationRecord
   include CaberObject
   include Linkable
   include Sluggable
+  include PublicIDable
 
   acts_as_federails_actor username_field: :slug, name_field: :name, profile_url_method: :url_for, actor_type: "Document", include_in_user_count: false
 
@@ -95,7 +96,7 @@ def needs_organizing?
   end
 
   def self.ransackable_attributes(_auth_object = nil)
-    ["caption", "created_at", "id", "library_id", "name", "notes", "path", "slug", "updated_at", "license_cont", "license"]
+    ["caption", "created_at", "id", "public_id", "library_id", "name", "notes", "path", "slug", "updated_at", "license_cont", "license"]
   end
 
   def self.ransackable_associations(auth_object = nil)
@@ -133,7 +134,7 @@ def strip_separators_from_path
   end
 
   def previous_library
-    library_id_changed? ? Library.find(library_id_was) : library
+    library_id_changed? ? Library.find_by(id: library_id_was) : library
   end
 
   def previous_path

app/models/model_file.rb

@@ -1,6 +1,7 @@
 class ModelFile < ApplicationRecord
   include LibraryUploader::Attachment(:attachment)
   include Listable
+  include PublicIDable
 
   extend Memoist
 
@@ -139,7 +140,7 @@ def delete_from_disk_and_destroy
   end
 
   def self.ransackable_attributes(_auth_object = nil)
-    ["caption", "created_at", "digest", "filename", "id", "notes", "presupported", "size", "updated_at", "y_up", "presupported_version_id", "unsupported_version_id"]
+    ["caption", "created_at", "digest", "filename", "id", "public_id", "notes", "presupported", "size", "updated_at", "y_up", "presupported_version_id", "unsupported_version_id"]
   end
 
   def self.ransackable_associations(_auth_object = nil)

app/models/problem.rb

@@ -1,4 +1,6 @@
 class Problem < ApplicationRecord
+  include PublicIDable
+
   belongs_to :problematic, polymorphic: true
 
   validates :category, uniqueness: {scope: :problematic}, presence: true
@@ -53,7 +55,7 @@ def self.create_or_clear(problematic, cat, present, options = {})
   end
 
   def self.ransackable_attributes(auth_object = nil)
-    ["category", "created_at", "id", "note", "problematic_id", "problematic_type", "updated_at", "ignored"]
+    ["category", "created_at", "id", "public_id", "note", "problematic_id", "problematic_type", "updated_at", "ignored"]
   end
 
   def self.ransackable_associations(auth_object = nil)

app/views/application/_filters_card.html.erb

@@ -11,14 +11,14 @@
     <% if @filters[:collection] %>
       <div class="row">
         <div class="col-auto"><%= icon "collection", Collection.model_name.human %></div>
-        <div class="col" aria-label="<%= Collection.model_name.human %>"><%= @collection ? link_to(@collection.name, {collection: @collection.id}) : "Unknown" %></div>
+        <div class="col" aria-label="<%= Collection.model_name.human %>"><%= @collection ? link_to(@collection.name, {collection: @collection}) : "Unknown" %></div>
         <div class="col-auto"><%= link_to icon("trash", t(".remove_collection_filter")), @filters.except(:collection), {class: "text-danger"} %></div>
       </div>
     <% end %>
     <% if @filters[:library] %>
       <div class="row">
         <div class="col-auto"><%= icon "boxes", Library.model_name.human %></div>
-        <div class="col" aria-label="<%= Library.model_name.human %>"><%= [*@filters[:library]].map { |l| Library.find(l).name }.join(", ") %></div>
+        <div class="col" aria-label="<%= Library.model_name.human %>"><%= [*@filters[:library]].map { |l| Library.find_by(public_id: l).name }.join(", ") %></div>
         <div class="col-auto"><%= link_to icon("trash", t(".remove_library_filter")), @filters.except(:library), {class: "text-danger"} %></div>
       </div>
     <% end %>

app/views/application/_navbar.html.erb

@@ -23,7 +23,7 @@
             <%= nav_link(
                   library.icon.presence || "boxes",
                   library.name,
-                  models_path((@filters&.except(:library) || {}).merge({library: library.id})),
+                  models_path((@filters&.except(:library) || {}).merge({library: library})),
                   title: library.caption
                 )	%>
           </li>

app/views/application/_object_preview.html.erb

@@ -1,6 +1,6 @@
 <% if renderable?(file.extension) %>
   <div class="position-relative">
-    <canvas id="preview-file-<%= file.id %>" class="object-preview position-relative" tabindex="0" data-preview
+    <canvas id="preview-file-<%= file.to_param %>" class="object-preview position-relative" tabindex="0" data-preview
       data-preview-url="<%= model_model_file_path(model, file, file.extension.to_sym) %>"
       data-worker-url="<%= javascript_path "offscreen_renderer.js" %>"
       data-format="<%= file.extension %>"

app/views/collections/_collection.html.erb

@@ -19,7 +19,7 @@
       <div class="col-auto">
         <% if collection.collection %>
           <%= icon "collection", collection.model_name.human %>
-          <%= link_to collection.collection.name, (@filters || {}).merge(collection: collection.collection.id) %><br>
+          <%= link_to collection.collection.name, (@filters || {}).merge(collection: collection.collection) %><br>
         <% end %>
         <% if collection.caption %>
           <h6 class="card-subtitle mb-2 text-muted"><%= sanitize collection.caption %></h6>
@@ -34,8 +34,8 @@
         </ul>
       </div>
       <div>
-        <%= link_to "#{policy_scope(Model).where(collection: collection).count} #{Model.model_name.human count: policy_scope(Model).where(collection: collection).count}", {controller: "models", collection: collection.id}, class: "btn btn-primary", "aria-label": translate(".models_button.label", name: collection.name) if policy(collection).show? %>
-        <%= link_to "#{collection.collections.count} #{Collection.model_name.human count: collection.collections.count}", (@filters || {}).merge(controller: "collections", collection: collection.id), class: "btn btn-primary", "aria-label": translate(".collections_button.label", name: collection.name) if collection.collections.count > 0 && policy(collection).show? %>
+        <%= link_to "#{policy_scope(Model).where(collection: collection).count} #{Model.model_name.human count: policy_scope(Model).where(collection: collection).count}", {controller: "models", collection: collection}, class: "btn btn-primary", "aria-label": translate(".models_button.label", name: collection.name) if policy(collection).show? %>
+        <%= link_to "#{collection.collections.count} #{Collection.model_name.human count: collection.collections.count}", (@filters || {}).merge(controller: "collections", collection: collection), class: "btn btn-primary", "aria-label": translate(".collections_button.label", name: collection.name) if collection.collections.count > 0 && policy(collection).show? %>
         <%= link_to icon("pencil-fill", t(".edit_button.text")), edit_collection_path(collection), {class: "btn btn-outline-secondary", "aria-label": translate(".edit_button.label", name: collection.name)} if policy(collection).edit? %>
       </div>
     </div>

app/views/creators/_creator.html.erb

@@ -13,7 +13,7 @@
           <li><%= link_to t("sites.%{site}" % {site: link.site}), link.url %></li>
         <% end %>
       </ul>
-      <%= link_to "#{policy_scope(Model).where(creator: creator).count} #{Model.model_name.human count: policy_scope(Model).where(creator: creator).count}", models_path(creator: creator.id), {class: "btn btn-primary", "aria-label": translate(".models_button.label", name: creator.name)} if policy(creator).show? %>
+      <%= link_to "#{policy_scope(Model).where(creator: creator).count} #{Model.model_name.human count: policy_scope(Model).where(creator: creator).count}", models_path(creator: creator), {class: "btn btn-primary", "aria-label": translate(".models_button.label", name: creator.name)} if policy(creator).show? %>
       <%= link_to icon("pencil-fill", t(".edit_button.text")), edit_creator_path(creator), {class: "btn btn-outline-secondary", "aria-label": translate(".edit_button.label", name: creator.name)} if policy(creator).edit? %>
     </div>
   </div>

app/views/model_files/bulk_edit.html.erb

@@ -16,7 +16,7 @@
     </tr>
     <% @files.each do |file| %>
       <tr>
-        <td><%= form.check_box "model_files[#{file.id}]", data: {bulk_item: file.id.to_s}, "aria-label": translate(".select", name: file.name) %></td>
+        <td><%= form.check_box "model_files[#{file.to_param}]", data: {bulk_item: file.to_param}, "aria-label": translate(".select", name: file.name) %></td>
         <td><%= link_to file.name, [@model, file], title: file.filename %></td>
         <td><code><%= file.filename %></code></td>
         <td><%= icon "check-circle-fill", ModelFile.human_attribute_name(:printed) if current_user.printed?(file) %></td>

app/views/model_files/show.html.erb

@@ -64,7 +64,7 @@
     <%= card :secondary, t(".actions_heading") do %>
       <%= link_to safe_join([icon("cloud-download", t("general.download")), t("general.download")], " "), model_model_file_path(@model, @file, @file.extension.to_sym), {class: "btn btn-secondary"} %>
       <% if policy(@file).edit? && ["stl", "obj"].include?(@file.extension) %>
-        <%= link_to safe_join([icon("arrow-left-right", t(".convert")), t(".convert")], " "), model_model_files_path(@model, convert: {id: @file.id, to: "threemf"}), method: :post, class: "btn btn-warning" %>
+        <%= link_to safe_join([icon("arrow-left-right", t(".convert")), t(".convert")], " "), model_model_files_path(@model, convert: {id: @file.to_param, to: "threemf"}), method: :post, class: "btn btn-warning" %>
       <% end %>
     <% end %>