Skip to content

Commit

Permalink
Merge branch 'development' into bug-3214-vulnerability_no_rate_limit_…
Browse files Browse the repository at this point in the history
…on_reset_password_link
  • Loading branch information
briri committed Mar 23, 2023
2 parents 057496a + 385e884 commit d067a9f
Show file tree
Hide file tree
Showing 11 changed files with 126 additions and 26 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/eslint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
with:
cache: 'yarn'
node-version: 16

# Run yarn install for JS dependencies
- name: 'Yarn Install'
run: yarn install
Expand Down
11 changes: 9 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ With the removal of the webpacker gem, the DartSass package has been installed t
- Sass variables are no longer declared globally and have to be included in files where they are used.
For more detailed explanation, please refer to this video : https://www.youtube.com/watch?v=CR-a8upNjJ0

### Introduction of RackAttack
[Rack Attack](https://github.com/rack/rack-attack) is middleware that can be used to help protect the application from malicious activity. You can establish white/black lists for specific IP addresses and also define rate limits.

- Using Rack-attack address vulnerabilities pointed out in password reset and login: there was no request rate limit.[#3214](https://github.com/DMPRoadmap/roadmap/issues/3214)

### Cleanup of Capybara configuration
- Cleaned up Gemfile by:
- removing gems that were already commented out
Expand All @@ -58,13 +63,15 @@ For more detailed explanation, please refer to this video : https://www.youtube.
### GitHub actions updates
- Added node version specification (v16) to eslint, PostgreSQL and MySQL github action to eliminate `digital routine enveloped` error [#319](https://github.com/portagenetwork/roadmap/issues/319)

### Enhancements
- Added enum to the funding status attribute of plan model to make the dropdown of 'funding status' being translatable
- Allow users to download both single phase and in PDF, TEXT and DOCX format. CSV file can only download single phase instead of all phases.

### Bug Fixes
- Using Rack-attack address vulnerabilities pointed out in password reset and login: there was no request rate limit.[#3214](https://github.com/DMPRoadmap/roadmap/issues/3214)

## v4.0.2

### Added

- Added CHANGELOG.md and Danger Github Action [#3257](https://github.com/DMPRoadmap/roadmap/issues/3257)
- Added validation with custom error message in research_output.rb to ensure a user does not enter a very large value as 'Anticipated file size'. [#3161](https://github.com/DMPRoadmap/roadmap/issues/3161)
- Added popover for org profile page and added explanation for public plan
Expand Down
2 changes: 0 additions & 2 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -297,8 +297,6 @@ GEM
no_proxy_fix (0.1.2)
nokogiri (1.14.2-arm64-darwin)
racc (~> 1.4)
nokogiri (1.14.2-x86_64-linux)
racc (~> 1.4)
notiffany (0.1.3)
nenv (~> 0.1)
shellany (~> 0.0)
Expand Down
18 changes: 12 additions & 6 deletions app/controllers/plan_exports_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,18 @@ def show

@hash = @plan.as_pdf(current_user, @show_coversheet)
@formatting = export_params[:formatting] || @plan.settings(:export).formatting
@selected_phase = if params.key?(:phase_id)
@plan.phases.find(params[:phase_id])
else
@plan.phases.order('phases.updated_at DESC')
if params.key?(:phase_id) && params[:phase_id].length.positive?
# order phases by phase number asc
@hash[:phases] = @hash[:phases].sort_by { |phase| phase[:number] }
if params[:phase_id] == 'All'
@hash[:all_phases] = true
else
@selected_phase = @plan.phases.find(params[:phase_id])
end
else
@selected_phase = @plan.phases.order('phases.updated_at DESC')
.detect { |p| p.visibility_allowed?(@plan) }
end
end

# Added contributors to coverage of plans.
# Users will see both roles and contributor names if the role is filled
Expand Down Expand Up @@ -102,7 +108,7 @@ def show_pdf
date: l(@plan.updated_at.to_date, format: :readable)),
font_size: 8,
spacing: (Integer(@formatting[:margin][:bottom]) / 2) - 4,
right: '[page] of [topage]',
right: _('[page] of [topage]'),
encoding: 'utf8'
}
end
Expand Down
1 change: 1 addition & 0 deletions app/controllers/plans_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,7 @@ def download
@plan = Plan.find(params[:id])
authorize @plan
@phase_options = @plan.phases.order(:number).pluck(:title, :id)
@phase_options.insert(0, ['All phases', 'All']) if @phase_options.length > 1
@export_settings = @plan.settings(:export)
render 'download'
end
Expand Down
12 changes: 11 additions & 1 deletion app/javascript/src/plans/download.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,15 @@ $(() => {
} else {
$('#download-settings').show();
}
});

if (frmt === 'csv') {
$('#phase_id').find('option[value="All"').hide();
$('#phase_id option:eq(1)').attr('selected', 'selected');
$('#phase_id').val($('#phase_id option:eq(1)').val());
} else if (frmt === 'pdf' || frmt === 'html' || frmt === 'docx' || frmt === 'text') {
$('#phase_id').find('option[value="All"').show();
$('#phase_id').val($('#phase_id option:first').val());
$('#phase_id option:first').attr('selected', 'selected');
}
}).trigger('change');
});
6 changes: 6 additions & 0 deletions app/models/plan.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ class Plan < ApplicationRecord
privately_visible: _('private')
}.freeze

FUNDING_STATUS = {
planned: _('Planned'),
funded: _('Funded'),
denied: _('Denied')
}.freeze

# ==============
# = Attributes =
# ==============
Expand Down
5 changes: 2 additions & 3 deletions app/views/plans/_project_details.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -167,15 +167,14 @@ ethics_report_tooltip = _("Link to a protocol from a meeting with an ethics comm
<%= form.label(:funding_status, _("Funding status"), class: "control-label") %>
</div>
<div class="col-md-8">
<% funding_statuses = Plan.funding_statuses.map { |status| [status[0].capitalize, status[0]] } %>
<%= form.select :funding_status, options_for_select(funding_statuses, form.object.funding_status),
<% funding_statuses = Plan::FUNDING_STATUS.map { |status| [_(status[0].to_s.capitalize), _(status[0].to_s)] } %>
<%= form.select :funding_status, options_for_select(funding_statuses, form.object.funding_status),
{
include_blank: _("- Please select one -"),
selected: form.object.funding_status
},
{ class: "form-control" } %>
</div>

<%= form.fields_for :grant, plan.grant do |grant_fields| %>
<div class="col-md-12">
<%= grant_fields.label(:value, _("Grant number/url"), class: "control-label") %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/shared/export/_plan.erb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

<% @hash[:phases].each do |phase| %>
<%# Only render selected phase %>
<% if phase[:title] == @selected_phase.title %>
<% if @hash[:all_phases] || (@selected_phase.present? && phase[:title] == @selected_phase.title) %>
<%# Page break before each phase %>
<div style="page-break-before:always;"></div>
<h1><%= download_plan_page_title(@plan, phase, @hash) %></h1>
Expand Down
2 changes: 1 addition & 1 deletion app/views/shared/export/_plan_txt.erb
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@

<% @hash[:phases].each do |phase| %>
<%# Only render selected phase %>
<% if phase[:title] == @selected_phase.title %>
<% if @hash[:all_phases] || (@selected_phase.present? && phase[:title] == @selected_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 %>
Expand Down
91 changes: 82 additions & 9 deletions spec/features/plans/exports_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,35 @@
expect(page).not_to have_text(new_plan.title)
end

# Separate code to test all-phase-download for html since it requires operation in new window
scenario 'User downloads their plan as HTML' do
within("#plan_#{plan.id}") do
click_button('Actions')
click_link 'Download'
end
select('html')
new_window = window_opened_by { click_button 'Download Plan' }
within_window new_window do
expect(page.source).to have_text(plan.title)
if plan.phases.present?
new_window = window_opened_by do
_select_option('phase_id', 'All')
click_button 'Download Plan'
end
within_window new_window do
expect(page.source).to have_text(plan.title)
plan.phases.each do |phase|
expect(page.source).to have_text(phase.title)
end
end
new_window = window_opened_by do
_select_option('phase_id', plan.phases[1].id)
click_button 'Download Plan'
end
within_window new_window do
expect(page.source).to have_text(plan.title)
expect(page.source).to have_text(plan.phases[1].title)
expect(page.source).not_to have_text(plan.phases[2].title) if plan.phases.length > 2
end
else
_regular_download('html')
end
end

Expand All @@ -96,8 +116,12 @@
click_link 'Download'
end
select('pdf')
click_button 'Download Plan'
expect(page.source).to have_text(plan.title)
if plan.phases.present?
_all_phase_download
_single_phase_download
else
_regular_download('pdf')
end
end

scenario 'User downloads their plan as CSV' do
Expand All @@ -106,8 +130,7 @@
click_link 'Download'
end
select('csv')
click_button 'Download Plan'
expect(page.source).to have_text(plan.title)
_regular_download('csv')
end

scenario 'User downloads their plan as text' do
Expand All @@ -116,8 +139,12 @@
click_link 'Download'
end
select('text')
click_button 'Download Plan'
expect(page.source).to have_text(plan.title)
if plan.phases.present?
_all_phase_download
_single_phase_download
else
_regular_download('text')
end
end

scenario 'User downloads their plan as docx' do
Expand All @@ -126,7 +153,53 @@
click_link 'Download'
end
select('docx')
if plan.phases.present?
_all_phase_download
_single_phase_download
else
_regular_download('docx')
end
end

# ===========================
# = Helper methods =
# ===========================

# rubocop:disable Metrics/AbcSize
# disable Rubocup metrics check to confirm both plan title and phase title on downloaded file
def _regular_download(format)
if format == 'html'
new_window = window_opened_by do
click_button 'Download Plan'
end
within_window new_window do
expect(page.source).to have_text(plan.title)
end
else
click_button 'Download Plan'
expect(page.source).to have_text(plan.title)
end
end

def _all_phase_download
_select_option('phase_id', 'All')
click_button 'Download Plan'
expect(page.source).to have_text(plan.title)
plan.phases.each do |phase| # All phase titles should be included in output
expect(page.source).to have_text(phase.title)
end
end

def _single_phase_download
_select_option('phase_id', plan.phases[1].id)
click_button 'Download Plan'
expect(page.source).to have_text(plan.title)
expect(page.source).to have_text(plan.phases[1].title)
expect(page.source).not_to have_text(plan.phases[2].title) if plan.phases.length > 2
end

# rubocop:enable Metrics/AbcSize
def _select_option(select_id, option_value)
find(:id, select_id).find("option[value='#{option_value}']").select_option
end
end

0 comments on commit d067a9f

Please sign in to comment.