Refactor CustomerToken model for improved token management; streamline token generation and expiration handling, and enhance association with issues

app/controllers/customers_controller.rb

@@ -134,38 +134,44 @@ class CustomersController < ApplicationController
 
   # creates new customer view tokens, removes expired tokens & redirects to newly created customer view with new token.
   def share
+    issue = Issue.find(params[:id])
 
-    Thread.new do
-      logger.info "Removing expired customer tokens"
-      CustomerToken.remove_expired_tokens
-      ActiveRecord::Base.connection.close
-    end
+    token = issue.share_token
+    redirect_to view_path(token.token)
 
-    begin
-      issue = Issue.find_by_id(params[:id])
-      redirect_to view_path issue.share_token.token
-    rescue
-      flash[:error] = t :notice_issue_not_found
+  rescue ActiveRecord::RecordNotFound
+    flash[:error] = t(:notice_issue_not_found)
     render_404
   end
-  end
   
   # displays an issue for a customer with a provided security CustomerToken
   def view
+    User.current = User.anonymous
 
-    User.current = User.find_by lastname: 'Anonymous'
+    # Load only active, non-expired token
+    @token = CustomerToken.active.find_by(token: params[:token])
+    return render_403 unless @token
 
-    @token = CustomerToken.find_by token: params[:token]
-    begin
-      @token.destroy if @token.expired?
-      raise "Token Expired" if @token.destroyed
+    # Load associated issue
+    @issue = @token.issue
+    return render_403 unless @issue
 
+    # Optional: enforce token belongs to the issue's customer
+    return render_403 unless @issue.customer_id == @token.issue.customer_id
+
+    # Store token in session for subsequent requests if needed
     session[:token] = @token.token
-      @issue = Issue.find @token.issue_id
-      @journals = @issue.journals.
-        preload(:details).
-        preload(user: :email_address).
-        reorder(:created_on, :id).to_a
+
+    load_issue_data
+  rescue ActiveRecord::RecordNotFound
+    render_403
+  end
+
+  private
+
+  def load_issue_data
+    @journals = @issue.journals.preload(:details).preload(user: :email_address).reorder(:created_on, :id).to_a
+
     @journals.each_with_index { |j, i| j.indice = i + 1 }
     @journals.reject!(&:private_notes?) unless User.current.allowed_to?(:view_private_notes, @issue.project)
     Journal.preload_journals_details_custom_fields(@journals)
@@ -175,18 +181,12 @@ class CustomersController < ApplicationController
     @changesets = @issue.changesets.visible.preload(:repository, :user).to_a
     @changesets.reverse! if User.current.wants_comments_in_reverse_order?
 
-      @relations = @issue.relations.select {|r| r.other_issue(@issue) && r.other_issue(@issue).visible? }
+    @relations = @issue.relations.select { |r| r.other_issue(@issue)&.visible? }
     @allowed_statuses = @issue.new_statuses_allowed_to(User.current)
     @priorities = IssuePriority.active
     @time_entry = TimeEntry.new(issue: @issue, project: @issue.project)
     @relation   = IssueRelation.new
-    rescue
-      flash[:error] = t :notice_forbidden
-      render_403
   end
-  end
-
-  private
 
   # redmine permission - add customers
   def add_customer

app/models/customer_token.rb

@@ -8,54 +8,44 @@
 #
 #THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
-class CustomerToken < ActiveRecord::Base
+class CustomerToken < ApplicationRecord
+  belongs_to :issue
 
-  has_many :issues
-  validates_presence_of :issue_id
-  before_create :generate_token, :generate_expire_date
-  attr_accessor :destroyed
-  after_destroy :mark_as_destroyed
+  validates :issue_id, presence: true
+  validates :token, presence: true, uniqueness: true
 
-  OAUTH_CONSUMER_SECRET = Setting.plugin_redmine_qbo['settingsOAuthConsumerSecret'] || 'CONFIGURE__' + SecureRandom.uuid
+  before_validation :generate_token, on: :create
+  before_validation :generate_expire_date, on: :create
 
-  # generates a random token using the plugin setting settingsOAuthConsumerSecret for salt
-  def generate_token
-    self.token = SecureRandom.base64(15).tr('+/=lIO0', OAUTH_CONSUMER_SECRET)
-  end
+  scope :active, -> { where("expires_at > ?", Time.current) }
 
-  # generates an expiring date
-  def generate_expire_date
-    self.expires_at = Time.now + 1.month
-  end
+  TOKEN_EXPIRATION = 1.month
 
-  # set destroyed flag
-  def mark_as_destroyed
-    self.destroyed = true
-  end
-
-  # purge expired tokens
-  def self.remove_expired_tokens
-    where("expires_at < ?", Time.now).destroy_all
-  end
-  
-  # has the token expired?
   def expired?
-    self.expires_at < Time.now
+    expires_at.present? && expires_at <= Time.current
+  end
+
+  def self.remove_expired_tokens
+    where("expires_at <= ?", Time.current).delete_all
   end
 
-  # Getter convenience method for tokens
   def self.get_token(issue)
+    return unless issue
+    return unless User.current.allowed_to?(:view_issues, issue.project)
 
-    # check to see if token exists & if it is expired
-    token = find_by_issue_id issue.id
-    unless token.nil?
-      return token unless token.expired?
-      # remove expired tokens
-      token.destroy
+    token = active.find_by(issue_id: issue.id)
+    return token if token
+
+    create!(issue: issue)
   end
 
-    # only create new token if we have an issue to attach it to
-    return create(issue_id: issue.id) if User.current.logged?
+  private
+
+  def generate_token
+    self.token ||= SecureRandom.urlsafe_base64(32)
   end
 
+  def generate_expire_date
+    self.expires_at ||= Time.current + TOKEN_EXPIRATION
+  end
 end