Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
5 changes: 5 additions & 0 deletions .changeset/heavy-radios-confess.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/view-components": patch
---

Clean up Lookbook-related security alerts.
20 changes: 16 additions & 4 deletions app/components/primer/alpha/toggle_switch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,6 @@ def initialize(src: nil, csrf_token: nil, checked: false, enabled: true, size: S
}

@system_arguments[:src] = @src if @src

return unless @src && @csrf_token

@system_arguments[:csrf] = @csrf_token
end

def on?
Expand All @@ -73,6 +69,22 @@ def enabled?
def disabled?
!enabled?
end

private

def before_render
@csrf_token ||= view_context.form_authenticity_token(
form_options: {
method: :post,
action: @src
}
)

@system_arguments[:data] = merge_data(
@system_arguments,
{ data: { csrf: @csrf_token } }
)
end
end
end
end
2 changes: 1 addition & 1 deletion app/components/primer/alpha/toggle_switch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class ToggleSwitchElement extends HTMLElement {

get csrf(): string | null {
const csrfElement = this.querySelector('[data-csrf]')
return this.getAttribute('csrf') || (csrfElement instanceof HTMLInputElement && csrfElement.value) || null
return this.getAttribute('data-csrf') || (csrfElement instanceof HTMLInputElement && csrfElement.value) || null
}

get csrfField(): string {
Expand Down
2 changes: 1 addition & 1 deletion app/lib/primer/attributes_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def merge_aria(*hashes)
# It's designed to be used to normalize and merge data information from system_arguments
# hashes. Consider using this pattern in component initializers:
#
# @system_arguments[:data] = merge_aria(
# @system_arguments[:data] = merge_data(
# @system_arguments,
# { data: { foo: "bar" } }
# )
Expand Down
2 changes: 1 addition & 1 deletion demo/app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@

# :nodoc:
class ApplicationController < ActionController::Base
protect_from_forgery
protect_from_forgery with: :exception
end
2 changes: 0 additions & 2 deletions demo/app/controllers/auto_check_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
# For auto-check previews
# :nocov:
class AutoCheckController < ApplicationController
skip_before_action :verify_authenticity_token

def error
render partial: "auto_check/error_message",
locals: { input_value: params[:value] },
Expand Down
22 changes: 10 additions & 12 deletions demo/app/controllers/toggle_switch_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,39 +7,37 @@ class << self
attr_accessor :last_request
end

skip_before_action :verify_authenticity_token
rescue_from ActionController::InvalidAuthenticityToken, with: :handle_invalid_authenticity_token

before_action :reject_non_ajax_request
before_action :verify_artificial_authenticity_token

def create
# lol this is so not threadsafe
self.class.last_request = request

sleep 1 unless Rails.env.test?

if params[:fail] == "true"
render status: :internal_server_error, plain: "Something went wrong."
return
end

head :accepted
end

private

def handle_invalid_authenticity_token
render status: :unauthorized, plain: "Bad CSRF token."
end

# this mimics dotcom behavior
def reject_non_ajax_request
return if request.headers["HTTP_REQUESTED_WITH"] == "XMLHttpRequest"

head :unprocessable_entity
end

def verify_artificial_authenticity_token
# don't check token if not provided
return unless form_params[:authenticity_token]

# if provided, check token
return if form_params[:authenticity_token] == "let_me_in"

render status: :unauthorized, plain: "Bad CSRF token"
end

def form_params
params.permit(:value, :authenticity_token)
end
Expand Down
2 changes: 1 addition & 1 deletion demo/app/views/lookbook/panels/_assets.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
<h4 class="asset-title font-mono"><%= asset[:path].relative_path_from(Primer::ViewComponents.root) %></h4>
</header>
<div class="asset-contents">
<%= code language: asset[:language], line_numbers: true do %><%== File.read(asset[:path]) %><% end %>
<%= code language: asset[:language], line_numbers: true do %><%= File.read(asset[:path]) %><% end %>
</div>
</div>
<% end %>
Expand Down
4 changes: 1 addition & 3 deletions demo/config/environments/test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
config.action_dispatch.show_exceptions = false

# Disable request forgery protection in test environment.
config.action_controller.allow_forgery_protection = false
config.action_controller.allow_forgery_protection = true

# Print deprecation notices to the stderr.
config.active_support.deprecation = :stderr
Expand All @@ -48,9 +48,7 @@
config.autoload_paths << Rails.root.join("..", "test", "forms")
config.view_component.preview_paths << Rails.root.join("..", "test", "previews")

# rubocop:disable Style/IfUnlessModifier
if ENV.fetch("VC_COMPAT_PATCH_ENABLED", "false") == "true"
config.view_component.capture_compatibility_patch_enabled = true
end
# rubocop:enable Style/IfUnlessModifier
end
10 changes: 1 addition & 9 deletions lib/primer/forms/toggle_switch.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,5 @@

<div><%= render(Caption.new(input: @input)) %></div>
</span>
<%
csrf = @input.csrf || @view_context.form_authenticity_token(
form_options: {
method: :post,
action: @input.src
}
)
%>
<%= render(Primer::Alpha::ToggleSwitch.new(src: @input.src, csrf: csrf, **@input.input_arguments)) %>
<%= render(Primer::Alpha::ToggleSwitch.new(src: @input.src, csrf_token: @input.csrf, **@input.input_arguments)) %>
<% end %>
2 changes: 1 addition & 1 deletion previews/primer/alpha/toggle_switch_preview.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def with_no_src
end

def with_csrf_token
render(Primer::Alpha::ToggleSwitch.new(src: UrlHelpers.toggle_switch_index_path, csrf_token: "let_me_in"))
render(Primer::Alpha::ToggleSwitch.new(src: UrlHelpers.toggle_switch_index_path))
end

def with_bad_csrf_token
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<%= render(ExampleToggleSwitchForm.new(csrf: "let_me_in", label: "Good example", src: toggle_switch_index_path, id: "success-toggle")) %>
<%= render(ExampleToggleSwitchForm.new(label: "Good example", src: toggle_switch_index_path, id: "success-toggle")) %>
<hr>
<%= render(ExampleToggleSwitchForm.new(csrf: "a_bad_value", label: "Bad example", src: toggle_switch_index_path, id: "error-toggle")) %>
<%= render(ExampleToggleSwitchForm.new(label: "Bad example", src: toggle_switch_index_path(fail: true), id: "error-toggle")) %>
2 changes: 1 addition & 1 deletion test/components/alpha/toggle_switch_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def test_small
def test_csrf_token
render_inline(Primer::Alpha::ToggleSwitch.new(src: "/foo", csrf_token: "abc123"))

assert_selector("[csrf]")
assert_selector("[data-csrf]")
end
end
end
Expand Down
13 changes: 3 additions & 10 deletions test/lib/primer/forms/integration_forms_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,19 +47,12 @@ def test_toggle_switch_form_errors
wait_for_toggle_switch_spinner

assert_selector("#error-toggle [data-target='toggle-switch.errorIcon']")
assert_selector(".FormControl-inlineValidation", text: "Bad CSRF token")
assert_selector(".FormControl-inlineValidation", text: "Something went wrong.")

page.evaluate_script(<<~JAVASCRIPT)
document
.querySelector('toggle-switch#error-toggle')
.setAttribute('csrf', 'let_me_in');
JAVASCRIPT

find("#error-toggle").click
find("#success-toggle").click
wait_for_toggle_switch_spinner

refute_selector("#error-toggle [data-target='toggle-switch.errorIcon']")
refute_selector("#error-toggle", text: "Bad CSRF token")
refute_selector("#success-toggle [data-target='toggle-switch.errorIcon']")
end

def test_action_menu_form_input
Expand Down
13 changes: 9 additions & 4 deletions test/lib/primer/forms/toggle_switch_form_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,19 @@
class Primer::Forms::ToggleSwitchFormTest < Minitest::Test
include Primer::ComponentTestHelpers

def test_it_renders_with_a_name
bogus_csrf = "let me in"
render_inline(ExampleToggleSwitchForm.new(csrf: bogus_csrf, src: "/toggle_switch"))
def test_it_renders
render_inline(ExampleToggleSwitchForm.new(src: "/toggle_switch"))

assert_selector "toggle-switch[src='/toggle_switch'][csrf='#{bogus_csrf}']"
assert_selector "toggle-switch[src='/toggle_switch']"
assert_selector "em", text: "favorite"
end

def test_accepts_custom_csrf_token
bogus_csrf = "let me in"
render_inline(ExampleToggleSwitchForm.new(csrf: bogus_csrf, src: "/toggle_switch"))
assert_selector "toggle-switch[data-csrf='#{bogus_csrf}']"
end

def test_can_render_without_subclass
render_inline(
Primer::Forms::ToggleSwitchForm.new(
Expand Down