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

Update dev #3101

Merged
merged 28 commits into from
Jan 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
bc6b470
added 'distinct' to the paginable concern's search function
briri Nov 30, 2021
ad52bfc
fixed issues with a few edge-case errors for plan downloads in csv an…
briri Nov 30, 2021
90069f5
untethered regex allows for sql injection
raycarrick-ed Dec 29, 2021
9c24734
added 'distinct' to the paginable concern's search function
briri Nov 30, 2021
c0c8b5e
Merge branch 'issue3072' of github.com:DMPRoadmap/roadmap into issue3072
raycarrick-ed Jan 27, 2022
ef7f5cd
added 'distinct' to the paginable concern's search function
briri Nov 30, 2021
6f29c05
Merge branch 'issue3072' of github.com:DMPRoadmap/roadmap into issue3072
raycarrick-ed Jan 28, 2022
ebc2da4
Merge pull request #3073 from DMPRoadmap/issue3072
raycarrick-ed Jan 28, 2022
544e77b
fixed issues with a few edge-case errors for plan downloads in csv an…
briri Nov 30, 2021
3d76ff1
formatting changes
raycarrick-ed Jan 28, 2022
27434b5
trying to figure out brakeman error
raycarrick-ed Jan 28, 2022
098b579
Fix brakeman sql injection
raycarrick-ed Jan 28, 2022
0c17d14
Rubocop fixes
raycarrick-ed Jan 28, 2022
d9e26a6
renable rubocop
raycarrick-ed Jan 28, 2022
efc7dd1
Merge pull request #3098 from DMPRoadmap/fix_downloads
raycarrick-ed Jan 28, 2022
18695e9
untethered regex allows for sql injection
raycarrick-ed Dec 29, 2021
3b2859c
Merge branch 'sql_injection_fix' of github.com:DMPRoadmap/roadmap int…
raycarrick-ed Jan 28, 2022
8aa5765
Merge pull request #3083 from DMPRoadmap/sql_injection_fix
raycarrick-ed Jan 28, 2022
957879c
fix authorize
raycarrick-ed Jan 28, 2022
0a32936
Merge pull request #3099 from DMPRoadmap/plan_policy_fix
raycarrick-ed Jan 28, 2022
75c2661
based on scan of authorize statements
raycarrick-ed Jan 28, 2022
c068c30
Fix spacing
raycarrick-ed Jan 28, 2022
5962f2d
rubocop
raycarrick-ed Jan 28, 2022
d42ab02
Unknown local variable
raycarrick-ed Jan 31, 2022
d707bed
Merge pull request #3100 from DMPRoadmap/authorize_scan
raycarrick-ed Jan 31, 2022
53012c3
update Gemfile.lock
raycarrick-ed Jan 31, 2022
816f127
updated PDF coversheet to always show the creator of the DMP
briri Oct 7, 2021
016701e
Merge branch 'development' of github.com:DMPRoadmap/roadmap into deve…
raycarrick-ed Jan 31, 2022
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
10 changes: 5 additions & 5 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ GEM
debug_inspector (>= 0.0.1)
bootsnap (1.10.2)
msgpack (~> 1.2)
brakeman (5.2.0)
brakeman (5.2.1)
builder (3.2.4)
bullet (7.0.1)
activesupport (>= 3.0.0)
Expand Down Expand Up @@ -218,7 +218,7 @@ GEM
httparty (0.20.0)
mime-types (~> 3.0)
multi_xml (>= 0.5.2)
i18n (1.8.11)
i18n (1.9.1)
concurrent-ruby (~> 1.0)
ipaddress (0.8.3)
jbuilder (2.11.5)
Expand Down Expand Up @@ -317,7 +317,7 @@ GEM
coderay (~> 1.1)
method_source (~> 1.0)
public_suffix (4.0.6)
puma (5.6.0)
puma (5.6.1)
nio4r (~> 2.0)
pundit (2.1.1)
activesupport (>= 3.0.0)
Expand Down Expand Up @@ -380,12 +380,12 @@ GEM
rspec-mocks (~> 3.10.0)
rspec-collection_matchers (1.2.0)
rspec-expectations (>= 2.99.0.beta1)
rspec-core (3.10.1)
rspec-core (3.10.2)
rspec-support (~> 3.10.0)
rspec-expectations (3.10.2)
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.10.0)
rspec-mocks (3.10.2)
rspec-mocks (3.10.3)
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.10.0)
rspec-rails (5.1.0)
Expand Down
7 changes: 4 additions & 3 deletions app/controllers/concerns/paginable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ module Paginable

##
# Regex to validate sort_field param is safe
SORT_COLUMN_FORMAT = /[\w_]+\.[\w_]/.freeze
SORT_COLUMN_FORMAT = /[\w_]+\.[\w_]+$/.freeze

PAGINATION_QUERY_PARAMS = %i[page sort_field sort_direction
search controller action].freeze
Expand Down Expand Up @@ -129,7 +129,7 @@ def paginable?
# rubocop:disable Metrics/AbcSize, Metrics/MethodLength
def refine_query(scope)
@args = @args.with_indifferent_access
scope = scope.search(@args[:search]) if @args[:search].present?
scope = scope.search(@args[:search]).distinct if @args[:search].present?
# Can raise NoMethodError if the scope does not define a search method
if @args[:sort_field].present?
frmt = @args[:sort_field][SORT_COLUMN_FORMAT]
Expand All @@ -148,8 +148,9 @@ def refine_query(scope)
scope = scope.order(order_field.to_sym => sort_direction.to_s)
else
order_field = ActiveRecord::Base.sanitize_sql(@args[:sort_field])
sd = ActiveRecord::Base.sanitize_sql(sort_direction)
scope = scope.includes(table_part.singularize.to_sym)
.order("#{order_field} #{sort_direction}")
.order("#{order_field} #{sd}")
end
end
if @args[:page] != 'ALL'
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/contributors_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def edit
# rubocop:disable Metrics/AbcSize, Metrics/MethodLength
# POST /plans/:plan_id/contributors
def create
authorize @plan
authorize @plan, :edit?

args = translate_roles(hash: contributor_params)
args = process_org(hash: args)
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/org_admin/phase_versions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class PhaseVersionsController < ApplicationController
# POST /org_admin/templates/:template_id/phases/:phase_id/versions
def create
@phase = Phase.find(params[:phase_id])
authorize @phase, :create?
authorize @phase
@new_phase = get_modifiable(@phase)
flash[:notice] = if @new_phase == @phase
'This template is already a draft'
Expand Down
10 changes: 5 additions & 5 deletions app/controllers/org_admin/plans_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def feedback_complete
# rubocop:enable Metrics/AbcSize

# GET /org_admin/download_plans
# rubocop:disable Metrics/AbcSize, Metrics/MethodLength
# rubocop:disable Metrics/AbcSize, Metrics/MethodLength, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
def download_plans
# Test auth directly and throw Pundit error sincePundit
# is unaware of namespacing
Expand All @@ -68,9 +68,9 @@ def download_plans
csv << [
plan.title.to_s,
plan.template.title.to_s,
(plan.owner.org.present? ? plan.owner.org.name : '').to_s,
plan.owner.name(false).to_s,
plan.owner.email.to_s,
(plan.owner&.org&.present? ? plan.owner.org.name : '').to_s,
plan.owner&.name(false)&.to_s,
plan.owner&.email&.to_s,
l(plan.latest_update.to_date, format: :csv).to_s,
Plan::VISIBILITY_MESSAGE[plan.visibility.to_sym].capitalize.to_s
]
Expand All @@ -81,6 +81,6 @@ def download_plans
format.csv { send_data plans, filename: "#{file_name}.csv" }
end
end
# rubocop:enable Metrics/AbcSize, Metrics/MethodLength
# rubocop:enable Metrics/AbcSize, Metrics/MethodLength, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
end
end
2 changes: 1 addition & 1 deletion app/controllers/paginable/contributors_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class ContributorsController < ApplicationController
# GET /paginable/plans/:plan_id/contributors/index/:page
def index
@plan = Plan.find_by(id: params[:plan_id])
authorize @plan
authorize @plan, :show?
paginable_renderise(
partial: 'index',
scope: Contributor.where(plan_id: @plan.id),
Expand Down
6 changes: 2 additions & 4 deletions app/controllers/paginable/plans_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class PlansController < ApplicationController

# /paginable/plans/privately_visible/:page
def privately_visible
raise Pundit::NotAuthorizedError unless Paginable::PlanPolicy.new(current_user).privately_visible?
authorize Plan

paginable_renderise(
partial: 'privately_visible',
Expand All @@ -19,9 +19,7 @@ def privately_visible

# GET /paginable/plans/organisationally_or_publicly_visible/:page
def organisationally_or_publicly_visible
unless Paginable::PlanPolicy.new(current_user).organisationally_or_publicly_visible?
raise Pundit::NotAuthorizedError
end
authorize Plan

paginable_renderise(
partial: 'organisationally_or_publicly_visible',
Expand Down
5 changes: 5 additions & 0 deletions app/policies/department_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@
class DepartmentPolicy < ApplicationPolicy
# NOTE: @user is the signed_in_user and @record is an instance of Department

def index?
(@user.can_org_admin? && @user.org.id == @department.org_id) ||
@user.can_super_admin?
end

def new?
@user.can_org_admin? || @user.can_super_admin?
end
Expand Down
18 changes: 11 additions & 7 deletions app/policies/phase_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,30 +12,34 @@ class PhasePolicy < ApplicationPolicy
# - The template which they are modifying belongs to their org

def show?
@user.can_modify_templates? && (@record.template.org_id == @user.org_id)
@user.can_modify_templates? && (@record.template.org_id == @user.org_id)
end

def preview?
@user.can_modify_templates? && (@record.template.org_id == @user.org_id)
@user.can_modify_templates? && (@record.template.org_id == @user.org_id)
end

def edit?
user.can_modify_templates? && (@record.template.org_id == user.org_id)
end

def update?
@user.can_modify_templates? && (@record.template.org_id == @user.org_id)
@user.can_modify_templates? && (@record.template.org_id == @user.org_id)
end

def new?
@user.can_modify_templates? && (@record.template.org_id == @user.org_id)
@user.can_modify_templates? && (@record.template.org_id == @user.org_id)
end

def create?
@user.can_modify_templates? && (@record.template.org_id == @user.org_id)
@user.can_modify_templates? && (@record.template.org_id == @user.org_id)
end

def destroy?
@user.can_modify_templates? && (@record.template.org_id == @user.org_id)
@user.can_modify_templates? && (@record.template.org_id == @user.org_id)
end

def sort?
@user.can_modify_templates? && (@record.template.org_id == @user.org_id)
@user.can_modify_templates? && (@record.template.org_id == @user.org_id)
end
end
12 changes: 12 additions & 0 deletions app/policies/plan_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
class PlanPolicy < ApplicationPolicy
# NOTE: @user is the signed_in_user and @record is an instance of Plan

def index?
@user.present?
end

def show?
@record.readable_by?(@user.id)
end
Expand Down Expand Up @@ -70,4 +74,12 @@ def select_guidances_list?
def update_guidances_list?
@record.editable_by?(@user.id)
end

def privately_visible?
@user.present?
end

def organisationally_or_publicly_visible?
@user.present?
end
end
2 changes: 1 addition & 1 deletion app/views/shared/export/_plan.erb
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
<br>
<%# case for displaying comments OR text %>
<% elsif !blank %>
<%= sanitize answer.text %>
<%= sanitize answer&.text %>
<br><br>
<% end %>
<% end %>
Expand Down
4 changes: 2 additions & 2 deletions app/views/shared/export/_plan_txt.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<%= "#{@plan.title}" %>
<%= "----------------------------------------------------------\n" %>
<% if @show_coversheet %>
<%= @hash[:attribution].many? ? _("Creators: ") : _('Creator:') %> <%= @hash[:attribution].join(', ') %>
<%= @hash[:attribution].length > 1 ? _("Creators: ") : _('Creator:') %> <%= @hash[:attribution].join(', ') %>
<%= _("Affiliation: ") + @hash[:affiliation] %>
<% if @hash[:funder].present? %>
<%= _("Template: ") + @hash[:funder] %>
Expand All @@ -24,7 +24,7 @@
<% @hash[:phases].each do |phase| %>
<%# Only render selected phase %>
<% if phase[:title] == @selected_phase.title %>
<%= (@hash[:phases].many? ? "#{phase[:title]}" : "") %>
<%= (@hash[:phases].length > 1 ? "#{phase[:title]}" : "") %>
<% phase[:sections].each do |section| %>
<% if display_section?(@hash[:customization], section, @show_custom_sections) && num_section_questions(@plan, section, phase) > 0 %>
<% if @show_sections_questions %>
Expand Down