Refactor appeal partial to avoid brakeman XSS warning (#25880)
This commit is contained in:
parent
bcd0171e5e
commit
9f218c9924
5 changed files with 47 additions and 36 deletions
19
app/helpers/admin/disputes_helper.rb
Normal file
19
app/helpers/admin/disputes_helper.rb
Normal file
|
@ -0,0 +1,19 @@
|
||||||
|
# frozen_string_literal: true
|
||||||
|
|
||||||
|
module Admin
|
||||||
|
module DisputesHelper
|
||||||
|
def strike_action_label(appeal)
|
||||||
|
t(key_for_action(appeal),
|
||||||
|
scope: 'admin.strikes.actions',
|
||||||
|
name: content_tag(:span, appeal.strike.account.username, class: 'username'),
|
||||||
|
target: content_tag(:span, appeal.account.username, class: 'target'))
|
||||||
|
.html_safe
|
||||||
|
end
|
||||||
|
|
||||||
|
private
|
||||||
|
|
||||||
|
def key_for_action(appeal)
|
||||||
|
AccountWarning.actions.slice(appeal.strike.action).keys.first
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
|
@ -4,7 +4,7 @@
|
||||||
= image_tag appeal.account.avatar.url(:original), alt: '', width: 40, height: 40, class: 'avatar'
|
= image_tag appeal.account.avatar.url(:original), alt: '', width: 40, height: 40, class: 'avatar'
|
||||||
.log-entry__content
|
.log-entry__content
|
||||||
.log-entry__title
|
.log-entry__title
|
||||||
= t(appeal.strike.action, scope: 'admin.strikes.actions', name: content_tag(:span, appeal.strike.account.username, class: 'username'), target: content_tag(:span, appeal.account.username, class: 'target')).html_safe
|
= strike_action_label(appeal)
|
||||||
.log-entry__timestamp
|
.log-entry__timestamp
|
||||||
%time.formatted{ datetime: appeal.strike.created_at.iso8601 }
|
%time.formatted{ datetime: appeal.strike.created_at.iso8601 }
|
||||||
= l(appeal.strike.created_at)
|
= l(appeal.strike.created_at)
|
||||||
|
|
|
@ -1,38 +1,5 @@
|
||||||
{
|
{
|
||||||
"ignored_warnings": [
|
"ignored_warnings": [
|
||||||
{
|
|
||||||
"warning_type": "Cross-Site Scripting",
|
|
||||||
"warning_code": 2,
|
|
||||||
"fingerprint": "71cf98c8235b5cfa9946b5db8fdc1a2f3a862566abb34e4542be6f3acae78233",
|
|
||||||
"check_name": "CrossSiteScripting",
|
|
||||||
"message": "Unescaped model attribute",
|
|
||||||
"file": "app/views/admin/disputes/appeals/_appeal.html.haml",
|
|
||||||
"line": 7,
|
|
||||||
"link": "https://brakemanscanner.org/docs/warning_types/cross_site_scripting",
|
|
||||||
"code": "t((Unresolved Model).new.strike.action, :scope => \"admin.strikes.actions\", :name => content_tag(:span, (Unresolved Model).new.strike.account.username, :class => \"username\"), :target => content_tag(:span, (Unresolved Model).new.account.username, :class => \"target\"))",
|
|
||||||
"render_path": [
|
|
||||||
{
|
|
||||||
"type": "template",
|
|
||||||
"name": "admin/disputes/appeals/index",
|
|
||||||
"line": 20,
|
|
||||||
"file": "app/views/admin/disputes/appeals/index.html.haml",
|
|
||||||
"rendered": {
|
|
||||||
"name": "admin/disputes/appeals/_appeal",
|
|
||||||
"file": "app/views/admin/disputes/appeals/_appeal.html.haml"
|
|
||||||
}
|
|
||||||
}
|
|
||||||
],
|
|
||||||
"location": {
|
|
||||||
"type": "template",
|
|
||||||
"template": "admin/disputes/appeals/_appeal"
|
|
||||||
},
|
|
||||||
"user_input": "(Unresolved Model).new.strike",
|
|
||||||
"confidence": "Weak",
|
|
||||||
"cwe_id": [
|
|
||||||
79
|
|
||||||
],
|
|
||||||
"note": ""
|
|
||||||
},
|
|
||||||
{
|
{
|
||||||
"warning_type": "Cross-Site Scripting",
|
"warning_type": "Cross-Site Scripting",
|
||||||
"warning_code": 4,
|
"warning_code": 4,
|
||||||
|
|
|
@ -18,10 +18,14 @@ RSpec.describe Admin::Disputes::AppealsController do
|
||||||
describe 'GET #index' do
|
describe 'GET #index' do
|
||||||
let(:current_user) { Fabricate(:user, role: UserRole.find_by(name: 'Admin')) }
|
let(:current_user) { Fabricate(:user, role: UserRole.find_by(name: 'Admin')) }
|
||||||
|
|
||||||
it 'lists appeals' do
|
before { appeal }
|
||||||
|
|
||||||
|
it 'returns a page that lists details of appeals' do
|
||||||
get :index
|
get :index
|
||||||
|
|
||||||
expect(response).to have_http_status(200)
|
expect(response).to have_http_status(:success)
|
||||||
|
expect(response.body).to include("<span class=\"username\">#{strike.account.username}</span>")
|
||||||
|
expect(response.body).to include("<span class=\"target\">#{appeal.account.username}</span>")
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
21
spec/helpers/admin/disputes_helper_spec.rb
Normal file
21
spec/helpers/admin/disputes_helper_spec.rb
Normal file
|
@ -0,0 +1,21 @@
|
||||||
|
# frozen_string_literal: true
|
||||||
|
|
||||||
|
require 'rails_helper'
|
||||||
|
|
||||||
|
describe Admin::DisputesHelper do
|
||||||
|
describe 'strike_action_label' do
|
||||||
|
it 'returns html describing the appeal' do
|
||||||
|
adam = Account.new(username: 'Adam')
|
||||||
|
becky = Account.new(username: 'Becky')
|
||||||
|
strike = AccountWarning.new(account: adam, action: :suspend)
|
||||||
|
appeal = Appeal.new(strike: strike, account: becky)
|
||||||
|
|
||||||
|
expected = <<~OUTPUT.strip
|
||||||
|
<span class="username">Adam</span> suspended <span class="target">Becky</span>'s account
|
||||||
|
OUTPUT
|
||||||
|
result = helper.strike_action_label(appeal)
|
||||||
|
|
||||||
|
expect(result).to eq(expected)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
Loading…
Reference in a new issue