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
This commit is contained in:
Krzysztof Kotlarek 2023-03-31 16:26:56 +11:00 committed by GitHub
parent ef1b781ced
commit c86d772277
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 3 additions and 12 deletions

View File

@ -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,

View File

@ -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",