From c86d772277bec82d643862999205caac5326580d Mon Sep 17 00:00:00 2001 From: Krzysztof Kotlarek Date: Fri, 31 Mar 2023 16:26:56 +1100 Subject: [PATCH] FIX: Drop internal URL validation for paths in sidebar (#20891) `Rails.application.routes.recognize_path(value)` was not working for /admin paths because StaffConstraint.new requires user to check permission. This validation is not bringing much value, and the easiest way is to drop it. In the worse case scenario, a user will have an incorrect link in their sidebar. Bug reported: https://meta.discourse.org/t/custom-sidebar-sections-being-tested-on-meta/255303/66 --- app/models/sidebar_url.rb | 7 ++----- spec/models/sidebar_url_spec.rb | 8 +------- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/app/models/sidebar_url.rb b/app/models/sidebar_url.rb index ad94de39a02..a969d2eee0d 100644 --- a/app/models/sidebar_url.rb +++ b/app/models/sidebar_url.rb @@ -15,11 +15,8 @@ class SidebarUrl < ActiveRecord::Base before_validation :remove_internal_hostname, :set_external def path_validator - if external? - raise ActionController::RoutingError.new("Not Found") if value !~ Discourse::Utils::URI_REGEXP - else - Rails.application.routes.recognize_path(value) - end + return true if !external? + raise ActionController::RoutingError.new("Not Found") if value !~ Discourse::Utils::URI_REGEXP rescue ActionController::RoutingError errors.add( :value, diff --git a/spec/models/sidebar_url_spec.rb b/spec/models/sidebar_url_spec.rb index 37d5218b0cd..4542f399456 100644 --- a/spec/models/sidebar_url_spec.rb +++ b/spec/models/sidebar_url_spec.rb @@ -1,13 +1,7 @@ # frozen_string_literal: true RSpec.describe SidebarUrl do - it "validates path" do - expect(SidebarUrl.new(icon: "link", name: "categories", value: "/categories").valid?).to eq( - true, - ) - expect(SidebarUrl.new(icon: "link", name: "categories", value: "/invalid_path").valid?).to eq( - false, - ) + it "validates external URLs" do expect( SidebarUrl.new( icon: "link",