Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support to render widgets partial without any layout furniture #3989

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions apps/dashboard/app/controllers/widgets_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# frozen_string_literal: true

# The Controller to render widget templates without any layout furniture
class WidgetsController < ApplicationController

def show
widget_path = File.join('/widgets', params[:widget_path])

unless valid_path?(widget_path)
render plain: "400 Bad Request. Invalid widget path: #{widget_path}", status: :bad_request
return
end


widget_exists = lookup_context.exists?(widget_path, [], true)
unless widget_exists
render plain: "404 Widget not found: #{widget_path}", status: :not_found
return
end

render partial: widget_path, layout: false
end

private

# Checks if the widget path contains only allowed characters
def valid_path?(widget_path)
widget_path.match?(/\A[a-zA-Z0-9_\-\/]+\z/)
end
end

2 changes: 2 additions & 0 deletions apps/dashboard/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@

post 'settings', :to => 'settings#update'

match '/widgets/*widget_path', to: 'widgets#show', via: [:get, :post], as: 'widgets'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay on this. Why do we need post here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for completeness. There might be complex widgets that might require to submit data from the frontend to the backend for an update of the state/data of the widget after the initial render. Adding a POST will provide a more REST friendly way of getting the data.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for completeness...

We don't seem to be passing the body or query params to the partial. That seems super risky to do so, because we don't know them up front and so can't filter them. I have to admit that scares me a bit.

Copy link
Contributor Author

@abujeda abujeda Dec 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, there is no need to pass them and this is not adding something that is not already possible. The body and query params are available in any template through the params object.

This is a valid and working widget template (tested locally to confirm)
/pun/sys/dashboard?parameter=Aday

# /pun/sys/dashboard?parameter=Aday 
# widget template
<%
  value = params[:parameter]
%>
<%= "Query Param Value: #{value}" %>

Doing this with a POST will be more REST friendly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To mitigate the security concerns, we could make this a experimental feature for now to give us time to try and discover any issues.

# Experimental Feature
# Allows widget partials to be rendered without any page furniture.
# It can be use to extend OOD functionality.
  if Configuration.widget_partials_enabled?
     match '/widgets/*widget_path', to: 'widgets#show', via: [:get, :post], as: 'widgets'
  end

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To mitigate the security concerns, we could make this a experimental feature

Yes please. I just tagged 4.0.0 but this can make it into 4.0.1.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes completed


# Support ticket routes
if Configuration.support_ticket_enabled?
get '/support', to: 'support_ticket#new'
Expand Down
45 changes: 45 additions & 0 deletions apps/dashboard/test/controllers/widgets_controller_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
require "test_helper"

class WidgetsControllerTest < ActiveSupport::TestCase

def setup
@controller = WidgetsController.new
end

test 'valid_path? validates widget paths' do
refute @controller.send(:valid_path?, '/test!')
refute @controller.send(:valid_path?, '/test/../../outside_dir')
refute @controller.send(:valid_path?, '@user:pwd/dir')

assert @controller.send(:valid_path?, 'test')
assert @controller.send(:valid_path?, '/test')
assert @controller.send(:valid_path?, '/test/path/widget')
assert @controller.send(:valid_path?, '/test_path/widget')
assert @controller.send(:valid_path?, '/test-path/widget_under/name')
end

test 'show should return HTTP 400 when invalid widget path is used' do
@params = ActionController::Parameters.new({ widget_path: '!!invalid' })
@controller.stubs(:params).returns(@params)
@controller.expects(:render).with(plain: '400 Bad Request. Invalid widget path: /widgets/!!invalid', status: :bad_request)

@controller.show
end

test 'show should return HTTP 404 when valid widget path is not found in the system' do
@params = ActionController::Parameters.new({ widget_path: '/valid/path' })
@controller.stubs(:params).returns(@params)
@controller.expects(:render).with(plain: '404 Widget not found: /widgets/valid/path', status: :not_found)

@controller.show
end

test 'show should render widget when valid widget path is found in the system' do
@params = ActionController::Parameters.new({ widget_path: '/valid/path' })
@controller.stubs(:params).returns(@params)
@controller.lookup_context.stubs(:exists?).returns(true)
@controller.expects(:render).with(partial: '/widgets/valid/path', layout: false)

@controller.show
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<h3>test response from widget partial</h3>
18 changes: 18 additions & 0 deletions apps/dashboard/test/integration/widgets_partial_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
require 'html_helper'
require 'test_helper'

class WidgetsPartialTest < ActionDispatch::IntegrationTest

test 'should render widget partial without any layout furniture' do
get widgets_url('widgets_partial_test')

assert_response :ok
assert_equal '<h3>test response from widget partial</h3>', @response.body
end

test 'should render return 404 response when widget is missing' do
get widgets_url('missing_widget')

assert_response :not_found
end
end
Loading