Skip to content

Commit

Permalink
Fix ToggleSwitch CSRF behavior
Browse files Browse the repository at this point in the history
  • Loading branch information
camertron committed May 17, 2024
1 parent 8158992 commit 773bd6e
Show file tree
Hide file tree
Showing 8 changed files with 34 additions and 30 deletions.
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
22 changes: 11 additions & 11 deletions demo/app/controllers/toggle_switch_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,37 +7,37 @@ class << self
attr_accessor :last_request
end

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/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 Down
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")) %>

0 comments on commit 773bd6e

Please sign in to comment.