Skip to content

Commit

Permalink
FIX: Fix user_id validation (#312)
Browse files Browse the repository at this point in the history
Validation of `user_id` parameter will throw a 500 error because
`User.find_by_username_or_email` does not throw
`ActiveRecord::RecordNotFound`, but silently returns `nil`.
This results in a `NoMethodError` in `object.id` on the next line
  • Loading branch information
Lhcfl authored Aug 21, 2024
1 parent 760667d commit 6bf3ac7
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 6 deletions.
9 changes: 3 additions & 6 deletions lib/discourse_data_explorer/parameter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -205,12 +205,9 @@ def cast_to_ruby(string)
invalid_format string, "The specified #{klass_name} was not found"
end
elsif type == :user_id
begin
object = User.find_by_username_or_email(string)
value = object.id
rescue ActiveRecord::RecordNotFound
invalid_format string, "The user named #{string} was not found"
end
object = User.find_by_username_or_email(string)
invalid_format string, "The user named #{string} was not found" if object.blank?
value = object.id
elsif type == :post_id
if string =~ %r{/t/[^/]+/(\d+)(\?u=.*)?$}
object = Post.with_deleted.find_by(topic_id: $1, post_number: 1)
Expand Down
17 changes: 17 additions & 0 deletions spec/lib/parameter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,23 @@ def param(identifier, type, default, nullable)
end
end
end

describe "user_id type" do
fab!(:user)
it "raises an error if no such user exists" do
expect {
param("user_id", :user_id, nil, false).cast_to_ruby("user_not_exist")
}.to raise_error(::DiscourseDataExplorer::ValidationError)
expect {
param("user_id", :user_id, nil, false).cast_to_ruby("user_not_exist@fake.email")
}.to raise_error(::DiscourseDataExplorer::ValidationError)
end

it "returns the user id if the user exists" do
expect(param("user_id", :user_id, nil, false).cast_to_ruby(user.username)).to eq(user.id)
expect(param("user_id", :user_id, nil, false).cast_to_ruby(user.email)).to eq(user.id)
end
end
end

describe ".create_from_sql" do
Expand Down

0 comments on commit 6bf3ac7

Please sign in to comment.