FIX: Validate duration minutes values for topic timer (#12040)

Add server and client side validations to ensure topic timer durations cannot exceed 2 years and cannot be less than or equal to 0.

app/assets/javascripts/discourse/app/controllers/edit-topic-timer.js

@@ -121,6 +121,9 @@ export default Controller.extend(ModalFunctionality, {
 
   actions: {
     onChangeStatusType(value) {
+      if (value !== CLOSE_STATUS_TYPE) {
+        this.set("topicTimer.based_on_last_post", false);
+      }
       this.set("topicTimer.status_type", value);
     },
 
@@ -145,14 +148,24 @@ export default Controller.extend(ModalFunctionality, {
 
       if (
         this.get("topicTimer.duration_minutes") &&
-        !this.get("topicTimer.updateTime") &&
-        this.get("topicTimer.duration_minutes") <= 0
+        !this.get("topicTimer.updateTime")
       ) {
-        this.flash(
-          I18n.t("topic.topic_status_update.min_duration"),
-          "alert-error"
-        );
-        return;
+        if (this.get("topicTimer.duration_minutes") <= 0) {
+          this.flash(
+            I18n.t("topic.topic_status_update.min_duration"),
+            "alert-error"
+          );
+          return;
+        }
+
+        // cannot be more than 2 years
+        if (this.get("topicTimer.duration_minutes") > 2 * 365 * 1440) {
+          this.flash(
+            I18n.t("topic.topic_status_update.max_duration"),
+            "alert-error"
+          );
+          return;
+        }
       }
 
       this._setTimer(

app/controllers/topics_controller.rb

@@ -475,19 +475,23 @@ class TopicsController < ApplicationController
     options.merge!(duration_minutes: params[:duration_minutes].to_i) if params[:duration_minutes].present?
     options.merge!(duration: params[:duration].to_i) if params[:duration].present?
 
-    topic_status_update = topic.set_or_create_timer(
-      status_type,
-      params[:time],
-      **options
-    )
+    begin
+      topic_timer = topic.set_or_create_timer(
+        status_type,
+        params[:time],
+        **options
+      )
+    rescue ActiveRecord::RecordInvalid => e
+      return render_json_error(e.message)
+    end
 
     if topic.save
       render json: success_json.merge!(
-        execute_at: topic_status_update&.execute_at,
-        duration_minutes: topic_status_update&.duration_minutes,
-        based_on_last_post: topic_status_update&.based_on_last_post,
+        execute_at: topic_timer&.execute_at,
+        duration_minutes: topic_timer&.duration_minutes,
+        based_on_last_post: topic_timer&.based_on_last_post,
         closed: topic.closed,
-        category_id: topic_status_update&.category_id
+        category_id: topic_timer&.category_id
       )
     else
       render_json_error(topic)

app/models/topic_timer.rb

@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 class TopicTimer < ActiveRecord::Base
+  MAX_DURATION_MINUTES = 2.years.to_i / 60
+
   self.ignored_columns = [
     "duration" # TODO(2021-06-01): remove
   ]
@@ -20,6 +22,7 @@ class TopicTimer < ActiveRecord::Base
   validates :category_id, presence: true, if: :publishing_to_category?
 
   validate :executed_at_in_future?
+  validate :duration_in_range?
 
   scope :scheduled_bump_topics, -> { where(status_type: TopicTimer.types[:bump], deleted_at: nil).pluck(:topic_id) }
   scope :pending_timers, ->(before_time = Time.now.utc) do
@@ -134,6 +137,22 @@ class TopicTimer < ActiveRecord::Base
 
   private
 
+  def duration_in_range?
+    return if duration_minutes.blank?
+
+    if duration_minutes.to_i <= 0
+      errors.add(:duration_minutes, I18n.t(
+        'activerecord.errors.models.topic_timer.attributes.duration_minutes.cannot_be_zero'
+      ))
+    end
+
+    if duration_minutes.to_i > MAX_DURATION_MINUTES
+      errors.add(:duration_minutes, I18n.t(
+        'activerecord.errors.models.topic_timer.attributes.duration_minutes.exceeds_maximum'
+      ))
+    end
+  end
+
   def executed_at_in_future?
     return if created_at.blank? || (execute_at > created_at)
 

config/locales/client.en.yml

@@ -2431,6 +2431,7 @@ en:
         when: "When:"
         time_frame_required: "Please select a time frame"
         min_duration: "Duration must be greater than 0"
+        max_duration: "Duration must be less than 2 years"
       auto_update_input:
         none: "Select a timeframe"
         now: "Now"

config/locales/server.en.yml

@@ -616,6 +616,9 @@ en:
           attributes:
             execute_at:
               in_the_past: "must be in the future."
+            duration_minutes:
+              cannot_be_zero: "must be greater than 0."
+              exceeds_maximum: "cannot be more than 2 years."
         translation_overrides:
           attributes:
             value:

spec/models/topic_timer_spec.rb

@@ -23,6 +23,20 @@ RSpec.describe TopicTimer, type: :model do
         topic_timer.update!(execute_at: 1.minute.ago, created_at: 10.minutes.ago)
         expect(TopicTimer.pending_timers.pluck(:id)).to include(topic_timer.id)
       end
+
+      describe "duration values" do
+        it "does not allow durations <= 0" do
+          topic_timer.duration_minutes = -1
+          topic_timer.save
+          expect(topic_timer.errors.full_messages.first).to include("Duration minutes must be greater than 0.")
+        end
+
+        it "does not allow crazy big durations (2 years in minutes)" do
+          topic_timer.duration_minutes = 3.years.to_i / 60
+          topic_timer.save
+          expect(topic_timer.errors.full_messages.first).to include("Duration minutes cannot be more than 2 years.")
+        end
+      end
     end
     describe '#status_type' do
       it 'should ensure that only one active public topic status update exists' do