htwkmooc/codeocean
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix RfC Visibility to work as intended

Browse Source 
The RfC Visibility of a consumer is intended to restrict who can access which RfCs. So far, those restrictions were only applied one way, for learners of a restricted consumer to view other (external) RfCs. However, the other way around should also work: If a RfC was created as part of a restricted consumer, no other external user should be able to interfere with this RfC. This commit, therefore, adds this direction as well and covers both directions with tests.
This commit is contained in:
Sebastian Serth
2023-07-13 10:51:10 +02:00
parent c6977b6319
commit e097036296
3 changed files with 207 additions and 99 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
app/policies/request_for_comment_policy.rb
View File

@ -6,7 +6,7 @@ class RequestForCommentPolicy < ApplicationPolicy
end
def show?
admin? || rfc_visibility
admin? || author? || rfc_visibility
end
def destroy?
@ -14,11 +14,11 @@ class RequestForCommentPolicy < ApplicationPolicy
end
def mark_as_solved?
admin? || (author? && rfc_visibility)
admin? || author?
end
def set_thank_you_note?
admin? || (author? && rfc_visibility)
admin? || author?
end
def clear_question?
@ -42,12 +42,16 @@ class RequestForCommentPolicy < ApplicationPolicy
end
def rfc_visibility
case @user.consumer.rfc_visibility
when 'all'
# The consumer with the most restricted visibility determines the visibility of the RfC
case [@user.consumer.rfc_visibility, @record.author.consumer.rfc_visibility]
# Only if both consumers allow learners to see all RfCs, the RfC is visible to the learner
when %w[all all]
everyone
when 'consumer'
# At least one consumer limits the visibility to the consumer
when %w[consumer all], %w[all consumer], %w[consumer consumer]
@record.author.consumer == @user.consumer
when 'study_group'
# At least one consumer limits the visibility to the study group
when %w[study_group all], %w[all study_group], %w[study_group consumer], %w[consumer study_group], %w[study_group study_group]
@record.submission.study_group.present? && @record.submission.study_group.id == @user.current_study_group_id
else
raise "Unknown RfC Visibility #{current_user.consumer.rfc_visibility}"
@ -61,8 +65,16 @@ class RequestForCommentPolicy < ApplicationPolicy
else
case @user.consumer.rfc_visibility
when 'all'
@scope.all
# We need to filter those RfCs where the visibility is more restricted than the `all` visibility.
rfcs_with_users = @scope
.joins('LEFT OUTER JOIN external_users ON request_for_comments.user_type = \'ExternalUser\' AND request_for_comments.user_id = external_users.id')
.joins('LEFT OUTER JOIN internal_users ON request_for_comments.user_type = \'InternalUser\' AND request_for_comments.user_id = internal_users.id')
rfcs_with_users.where(external_users: {consumer_id: Consumer.rfc_visibility_all})
.or(rfcs_with_users.where(internal_users: {consumer_id: Consumer.rfc_visibility_all}))
when 'consumer'
# Since the `rfc_visibility` is set on a consumer level, we do not need to consider the `study_group` visibility here.
# Therefore, those RfCs where the author is limited to study group RfCs definitely belong to another consumer.
rfcs_with_users = @scope
.joins('LEFT OUTER JOIN external_users ON request_for_comments.user_type = \'ExternalUser\' AND request_for_comments.user_id = external_users.id')
.joins('LEFT OUTER JOIN internal_users ON request_for_comments.user_type = \'InternalUser\' AND request_for_comments.user_id = internal_users.id')
@ -70,6 +82,7 @@ class RequestForCommentPolicy < ApplicationPolicy
rfcs_with_users.where(external_users: {consumer_id: @user.consumer.id})
.or(rfcs_with_users.where(internal_users: {consumer_id: @user.consumer.id}))
when 'study_group'
# Since the `rfc_visibility` is already the most restricted visibility, we do not need to consider any other visibility here.
@scope
.joins(:submission)
.where(submission: {study_group: @user.current_study_group_id})

2
spec/factories/request_for_comment.rb
View File

@ -4,7 +4,7 @@ FactoryBot.define do
factory :rfc, class: 'RequestForComment' do
user factory: :external_user
exercise factory: :math
submission { association :submission, exercise:, user: }
submission { association :submission, exercise:, user:, study_group: user&.study_groups&.first }
file
sequence :question do |n|
"test question #{n}"

275
spec/policies/request_for_comment_policy_spec.rb
View File

@ -49,150 +49,245 @@ describe RequestForCommentPolicy do
end
context 'when the RfC visibility is considered' do
let(:user) { create(:learner, consumer:) }
shared_examples 'grants access to everyone' do
it 'grants access to everyone' do
%i[external_user teacher admin].each do |factory_name|
expect(policy).to permit(create(factory_name, consumer: viewer_consumer, study_groups: viewer_study_groups), rfc)
end
end
let(:rfc_other_consumer) do
rfc = create(:rfc, user:)
rfc.user.update(consumer: create(:consumer, name: 'Other Consumer'))
rfc
it 'grants access to authors' do
expect(policy).to permit(rfc.author, rfc)
end
end
let(:rfc_other_study_group) do
rfc = create(:rfc, user:)
rfc.user.update(consumer: user.consumer)
rfc.submission.update(study_group: create(:study_group))
rfc
shared_examples 'grants access to admins and authors only' do
it 'grants access to admins' do
expect(policy).to permit(create(:admin, consumer: viewer_consumer, study_groups: viewer_study_groups), rfc)
end
it 'grants access to authors' do
expect(policy).to permit(rfc.author, rfc)
end
it 'does not grant access to all other users' do
%i[external_user teacher].each do |factory_name|
expect(policy).not_to permit(create(factory_name, consumer: viewer_consumer, study_groups: viewer_study_groups), rfc)
end
end
end
let(:rfc_peer) do
rfc = create(:rfc, user:)
rfc.user.update(consumer: user.consumer)
rfc.submission.update(study_group: user.study_groups.first)
rfc
end
let(:rfc_author) { create(:learner, consumer: author_consumer, study_groups: author_study_groups) }
let(:author_study_groups) { create_list(:study_group, 1, consumer: author_consumer) }
let(:rfc) { create(:rfc, user: rfc_author) }
let(:all_rfcs) { [rfc_other_consumer, rfc_other_study_group, rfc_peer] }
let(:same_consumer_rfcs) { [rfc_other_study_group, rfc_peer] }
let(:other_study_groups_rfcs) { [rfc_other_consumer, rfc_other_study_group] }
context "when the author's rfc_visibility is set to all" do
let(:author_consumer) { create(:consumer, rfc_visibility: 'all') }
context 'when rfc_visibility is set to all' do
let(:consumer) { create(:consumer, rfc_visibility: 'all') }
context 'when the viewer is from another consumer' do
context "when the viewer's rfc_visibility is set to all" do
let(:viewer_consumer) { create(:consumer, name: 'Other Consumer', rfc_visibility: 'all') }
let(:viewer_study_groups) { create_list(:study_group, 1, consumer: viewer_consumer) }
permissions(:show?) do
it 'grants access to everyone' do
%i[external_user teacher admin].each do |factory_name|
all_rfcs.each do |specific_rfc|
expect(policy).to permit(create(factory_name, consumer:), specific_rfc)
permissions(:show?) do
include_examples 'grants access to everyone'
end
%i[mark_as_solved? set_thank_you_note?].each do |action|
permissions(action) do
include_examples 'grants access to admins and authors only'
end
end
end
context "when the viewer's rfc_visibility is set to consumer" do
let(:viewer_consumer) { create(:consumer, name: 'Other Consumer', rfc_visibility: 'consumer') }
let(:viewer_study_groups) { create_list(:study_group, 1, consumer: viewer_consumer) }
%i[mark_as_solved? set_thank_you_note? show?].each do |action|
permissions(action) do
include_examples 'grants access to admins and authors only'
end
end
end
context "when the viewer's rfc_visibility is set to study_group" do
let(:viewer_consumer) { create(:consumer, name: 'Other Consumer', rfc_visibility: 'study_group') }
let(:viewer_study_groups) { create_list(:study_group, 1, consumer: viewer_consumer) }
%i[mark_as_solved? set_thank_you_note? show?].each do |action|
permissions(action) do
include_examples 'grants access to admins and authors only'
end
end
end
end
%i[mark_as_solved? set_thank_you_note?].each do |action|
permissions(action) do
it 'grants access to admins' do
all_rfcs.each do |specific_rfc|
expect(policy).to permit(create(:admin, consumer:), specific_rfc)
end
context 'when the viewer is from the same consumer' do
let(:viewer_consumer) { author_consumer }
context 'when the viewer is from another study group' do
let(:viewer_study_groups) { create_list(:study_group, 1, consumer: viewer_consumer) }
permissions(:show?) do
include_examples 'grants access to everyone'
end
it 'grants access to authors' do
all_rfcs.each do |specific_rfc|
expect(policy).to permit(specific_rfc.author, specific_rfc)
%i[mark_as_solved? set_thank_you_note?].each do |action|
permissions(action) do
include_examples 'grants access to admins and authors only'
end
end
end
it 'does not grant access to all other users' do
%i[external_user teacher].each do |factory_name|
all_rfcs.each do |specific_rfc|
expect(policy).not_to permit(create(factory_name, consumer:), specific_rfc)
end
context 'when the viewer is from the same study group' do
let(:viewer_study_groups) { author_study_groups }
permissions(:show?) do
include_examples 'grants access to everyone'
end
%i[mark_as_solved? set_thank_you_note?].each do |action|
permissions(action) do
include_examples 'grants access to admins and authors only'
end
end
end
end
end
context 'when rfc_visibility is set to consumer' do
let(:consumer) { create(:consumer, rfc_visibility: 'consumer') }
context "when the author's rfc_visibility is set to consumer" do
let(:author_consumer) { create(:consumer, rfc_visibility: 'consumer') }
permissions(:show?) do
it 'grants access to admins' do
all_rfcs.each do |specific_rfc|
expect(policy).to permit(create(:admin, consumer:), specific_rfc)
end
end
context 'when the viewer is from another consumer' do
context "when the viewer's rfc_visibility is set to all" do
let(:viewer_consumer) { create(:consumer, name: 'Other Consumer', rfc_visibility: 'all') }
let(:viewer_study_groups) { create_list(:study_group, 1, consumer: viewer_consumer) }
it 'grants access to users from the same consumer' do
%i[external_user teacher].each do |factory_name|
same_consumer_rfcs.each do |specific_rfc|
expect(policy).to permit(create(factory_name, consumer:), specific_rfc)
%i[mark_as_solved? set_thank_you_note? show?].each do |action|
permissions(action) do
include_examples 'grants access to admins and authors only'
end
end
end
it 'does not grant access to users from other consumers' do
%i[external_user teacher].each do |factory_name|
expect(policy).not_to permit(create(factory_name, consumer:), rfc_other_consumer)
context "when the viewer's rfc_visibility is set to consumer" do
let(:viewer_consumer) { create(:consumer, name: 'Other Consumer', rfc_visibility: 'consumer') }
let(:viewer_study_groups) { create_list(:study_group, 1, consumer: viewer_consumer) }
%i[mark_as_solved? set_thank_you_note? show?].each do |action|
permissions(action) do
include_examples 'grants access to admins and authors only'
end
end
end
context "when the viewer's rfc_visibility is set to study_group" do
let(:viewer_consumer) { create(:consumer, name: 'Other Consumer', rfc_visibility: 'study_group') }
let(:viewer_study_groups) { create_list(:study_group, 1, consumer: viewer_consumer) }
%i[mark_as_solved? set_thank_you_note? show?].each do |action|
permissions(action) do
include_examples 'grants access to admins and authors only'
end
end
end
end
# Testing `mark_as_solved?` and `set_thank_you_note?` is not necessary here,
# because an author of an RfC can only be a user from the same consumer.
context 'when the viewer is from the same consumer' do
let(:viewer_consumer) { author_consumer }
context 'when the viewer is from another study group' do
let(:viewer_study_groups) { create_list(:study_group, 1, consumer: viewer_consumer) }
permissions(:show?) do
include_examples 'grants access to everyone'
end
%i[mark_as_solved? set_thank_you_note?].each do |action|
permissions(action) do
include_examples 'grants access to admins and authors only'
end
end
end
context 'when the viewer is from the same study group' do
let(:viewer_study_groups) { author_study_groups }
permissions(:show?) do
include_examples 'grants access to everyone'
end
%i[mark_as_solved? set_thank_you_note?].each do |action|
permissions(action) do
include_examples 'grants access to admins and authors only'
end
end
end
end
end
context 'when rfc_visibility is set to study_group' do
let(:consumer) { create(:consumer, rfc_visibility: 'study_group') }
context "when the author's rfc_visibility is set to study_group" do
let(:author_consumer) { create(:consumer, rfc_visibility: 'study_group') }
permissions(:show?) do
it 'grants access to admins' do
all_rfcs.each do |specific_rfc|
expect(policy).to permit(create(:admin, consumer:), specific_rfc)
context 'when the viewer is from another consumer' do
context "when the viewer's rfc_visibility is set to all" do
let(:viewer_consumer) { create(:consumer, name: 'Other Consumer', rfc_visibility: 'all') }
let(:viewer_study_groups) { create_list(:study_group, 1, consumer: viewer_consumer) }
%i[mark_as_solved? set_thank_you_note? show?].each do |action|
permissions(action) do
include_examples 'grants access to admins and authors only'
end
end
end
it 'grants access to users from the same study group' do
%i[external_user teacher].each do |factory_name|
expect(policy).to permit(create(factory_name, consumer:, study_groups: [rfc_peer.submission.study_group]), rfc_peer)
context "when the viewer's rfc_visibility is set to consumer" do
let(:viewer_consumer) { create(:consumer, name: 'Other Consumer', rfc_visibility: 'consumer') }
let(:viewer_study_groups) { create_list(:study_group, 1, consumer: viewer_consumer) }
%i[mark_as_solved? set_thank_you_note? show?].each do |action|
permissions(action) do
include_examples 'grants access to admins and authors only'
end
end
end
it 'does not grant access to users from other consumers' do
%i[external_user teacher].each do |factory_name|
other_study_groups_rfcs.each do |specific_rfc|
expect(policy).not_to permit(create(factory_name, consumer:), specific_rfc)
context "when the viewer's rfc_visibility is set to study_group" do
let(:viewer_consumer) { create(:consumer, name: 'Other Consumer', rfc_visibility: 'study_group') }
let(:viewer_study_groups) { create_list(:study_group, 1, consumer: viewer_consumer) }
%i[mark_as_solved? set_thank_you_note? show?].each do |action|
permissions(action) do
include_examples 'grants access to admins and authors only'
end
end
end
end
%i[mark_as_solved? set_thank_you_note?].each do |action|
permissions(action) do
it 'grants access to admins' do
all_rfcs.each do |specific_rfc|
expect(policy).to permit(create(:admin, consumer:), specific_rfc)
context 'when the viewer is from the same consumer' do
let(:viewer_consumer) { author_consumer }
context 'when the viewer is from another study group' do
let(:viewer_study_groups) { create_list(:study_group, 1, consumer: viewer_consumer) }
%i[mark_as_solved? set_thank_you_note? show?].each do |action|
permissions(action) do
include_examples 'grants access to admins and authors only'
end
end
end
# Testing `mark_as_solved?` and `set_thank_you_note?` with another consumer is not
# necessary here, because an author of an RfC can only be a user from the same consumer.
context 'when the viewer is from the same study group' do
let(:viewer_study_groups) { author_study_groups }
it 'grants access to authors of the same primary study group' do
rfc_peer.author.update(study_groups: [rfc_peer.submission.study_group])
expect(policy).to permit(rfc_peer.author, rfc_peer)
permissions(:show?) do
include_examples 'grants access to everyone'
end
it 'does not grant access to authors of another primary study groups' do
rfc_other_study_group.author.update(study_groups: create_list(:study_group, 1))
expect(policy).not_to permit(rfc_other_study_group.author, rfc_other_study_group)
end
it 'does not grant access to all other users' do
%i[external_user teacher].each do |factory_name|
all_rfcs.each do |specific_rfc|
expect(policy).not_to permit(create(factory_name, consumer:), specific_rfc)
end
%i[mark_as_solved? set_thank_you_note?].each do |action|
permissions(action) do
include_examples 'grants access to admins and authors only'
end
end
end