Skip to content

Commit 615ef11

Browse files
committed
Safer reviewer tokens
1 parent 368a5f0 commit 615ef11

9 files changed

Lines changed: 20 additions & 28 deletions

File tree

app/controllers/genomes_controller.rb

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ def index
2323

2424
# GET /genomes/1 or /genomes/1.json
2525
def show
26-
@register ||= @genome.names.first&.register
2726
@crumbs = [['Genomes', genomes_path], @genome.title('')]
2827
end
2928

@@ -93,10 +92,9 @@ def set_genome
9392
def set_name
9493
@name = params[:name].present? ?
9594
Name.find(params[:name]) : @genome.names.first
96-
if @name.can_view?(current_user) || cookies[:reviewer_token].present?
95+
96+
if @name&.can_view?(current_user, cookies[:reviewer_token])
9797
@register = @name.try(:register)
98-
@register.current_reviewer_token = cookies[:reviewer_token] if @register
99-
@register = nil unless @name.can_view?(current_user)
10098
end
10199
end
102100

app/controllers/names_controller.rb

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -515,13 +515,8 @@ def unobserve
515515
def set_name
516516
@name = Name.find(params[:id])
517517

518-
if @name.can_view?(current_user) || cookies[:reviewer_token].present?
518+
if @name&.can_view?(current_user, cookies[:reviewer_token])
519519
@register = @name.try(:register)
520-
@register.current_reviewer_token = cookies[:reviewer_token] if @register
521-
@register = nil unless @name.can_view?(current_user)
522-
end
523-
524-
if @name.can_view?(current_user)
525520
current_user
526521
&.unseen_notifications
527522
&.where(notifiable: @name)

app/controllers/registers_controller.rb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -448,14 +448,15 @@ def reviewer_token_delete
448448
def set_register
449449
@no_register_sentinel = true
450450
@register = Register.find_by(accession: params[:accession])
451+
451452
if params[:token]
452453
if params[:token] == 'no'
453454
cookies[:reviewer_token] = nil
454455
elsif @register.reviewer_token == params[:token]
455456
cookies[:reviewer_token] = params[:token]
456457
end
457458
end
458-
@register.current_reviewer_token = cookies[:reviewer_token]
459+
459460
current_user
460461
&.unseen_notifications
461462
&.where(notifiable: @register)
@@ -494,7 +495,7 @@ def register_notify_params
494495
end
495496

496497
def authenticate_can_view!
497-
unless @register&.can_view?(current_user)
498+
unless @register&.can_view?(current_user, cookies[:reviewer_token])
498499
flash[:alert] = 'User cannot access register list'
499500
redirect_to(root_path)
500501
end

app/controllers/strains_controller.rb

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,9 @@ def set_strain
7474
def set_name
7575
@name = params[:name].present? ?
7676
Name.find(params[:name]) : @strain.names.first
77-
if @name.can_view?(current_user) || cookies[:reviewer_token].present?
77+
78+
if @name&.can_view?(current_user, cookies[:reviewer_token])
7879
@register = @name.try(:register)
79-
@register.current_reviewer_token = cookies[:reviewer_token] if @register
80-
@register = nil unless @name.can_view?(current_user)
8180
end
8281
end
8382

app/models/name.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -670,9 +670,9 @@ def not_validly_proposed_in?(publication)
670670

671671
# ============ --- USERS --- ============
672672

673-
def can_view?(user)
673+
def can_view?(user, token = nil)
674674
return true if public?
675-
return true if register.try(:current_reviewer_token?)
675+
return true if token.present? && token == register.try(:reviewer_token)
676676

677677
(!user.nil?) && (user.curator? || user?(user))
678678
end

app/models/register.rb

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ class Register < ApplicationRecord
5353
include Register::SampleSet
5454

5555
attr_accessor :modal_form_id
56-
attr_accessor :current_reviewer_token
5756

5857
def to_param
5958
accession
@@ -122,15 +121,15 @@ def last_submission_date
122121
[notified_at, submitted_at].compact.max || names.pluck(:submitted_at).max
123122
end
124123

124+
def correct_reviewer_token?(token)
125+
token.present? && token == reviewer_token
126+
end
127+
125128
def user?(user)
126129
user && user_id == user.id
127130
end
128131
alias :created_by? :user?
129132

130-
def current_reviewer_token?
131-
current_reviewer_token.present? && current_reviewer_token == reviewer_token
132-
end
133-
134133
def can_edit?(user)
135134
return false if validated?
136135
return false unless user
@@ -139,9 +138,9 @@ def can_edit?(user)
139138
user?(user) # && !submitted
140139
end
141140

142-
def can_view?(user)
141+
def can_view?(user, token = nil)
143142
return true if submitted? || validated? || notified?
144-
return true if current_reviewer_token?
143+
return true if correct_reviewer_token?(token)
145144
return false unless user
146145

147146
user.curator? || user?(user)

app/models/register/status.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ def draft?
5050
end
5151

5252
def public?
53-
can_view? nil
53+
can_view?(nil)
5454
end
5555

5656
def all_endorsed?

app/views/layouts/_sentinel_bar.html.erb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@
1212
sentinel_bar[:register] = @register
1313
end
1414

15-
if !@no_reviewer_sentinel && @register.present? &&
16-
@register.current_reviewer_token?
15+
if !@no_reviewer_sentinel &&
16+
@register&.correct_reviewer_token?(cookies[:reviewer_token])
1717
sentinel_bar[:reviewer] = reviewer_token_register_url(@register)
1818
end
1919
%>

app/views/names/_citation.html.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@
129129
<%= help_message(@name.status_name) { @name.status_help } %>
130130
</dd>
131131

132-
<% if @name.register&.can_view?(current_user) %>
132+
<% if @name.register&.can_view?(current_user, cookies[:reviewer_token]) %>
133133
<dt><%= fa_icon('clipboard-list') %> Register List</dt>
134134
<dd>
135135
<%= link_to(@name.register.acc_url, @name.register) %>

0 commit comments

Comments
 (0)