From 01accdae58ceb651820430d4a7a1d4b8fa1656e3 Mon Sep 17 00:00:00 2001 From: kiragrammel Date: Tue, 22 Aug 2023 09:23:20 +0200 Subject: [PATCH] Use author_in_programming_group? policy for files & RfCs * Allow all members of a programming group to list and solve RfCs * Also adjust policy specs to respect programming groups --- .../request_for_comments_controller.rb | 4 ++ app/models/submission.rb | 4 +- app/policies/application_policy.rb | 16 ++++- app/policies/code_ocean/file_policy.rb | 2 +- app/policies/request_for_comment_policy.rb | 6 +- spec/policies/code_ocean/file_policy_spec.rb | 72 ++++++++++++++++--- .../request_for_comment_policy_spec.rb | 16 +++++ 7 files changed, 101 insertions(+), 19 deletions(-) diff --git a/app/controllers/request_for_comments_controller.rb b/app/controllers/request_for_comments_controller.rb index a9cb4414..9cc182e5 100644 --- a/app/controllers/request_for_comments_controller.rb +++ b/app/controllers/request_for_comments_controller.rb @@ -44,7 +44,11 @@ class RequestForCommentsController < ApplicationController # GET /my_request_for_comments def my_comment_requests @search = policy_scope(RequestForComment) + .joins(:submission) .where(user: current_user) + .or(policy_scope(RequestForComment) + .joins(:submission) + .where(submission: {contributor: current_user.programming_groups})) .order(created_at: :desc) # Order for the LIMIT part of the query .ransack(params[:q]) diff --git a/app/models/submission.rb b/app/models/submission.rb index fd192bf6..368eb507 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -124,8 +124,8 @@ class Submission < ApplicationRecord (contributor_id + exercise.created_at.to_i) % 10 == 1 end - def own_unsolved_rfc(user = self.user) - Pundit.policy_scope(user, RequestForComment).unsolved.find_by(exercise:, user:) + def own_unsolved_rfc(user) + Pundit.policy_scope(user, RequestForComment).joins(:submission).where(submission: {contributor:}).unsolved.find_by(exercise:) end def unsolved_rfc(user = self.user) diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb index 77cab4a9..ad0d760b 100644 --- a/app/policies/application_policy.rb +++ b/app/policies/application_policy.rb @@ -50,9 +50,21 @@ class ApplicationPolicy private :teacher_in_study_group? def author_in_programming_group? - return false unless @record.contributor.programming_group? + if @record.respond_to? :contributor # e.g. submission + possible_programming_group = @record.contributor - @record.contributor.users.include?(@user) + elsif @record.respond_to? :context # e.g. file + possible_programming_group = @record.context.contributor + + elsif @record.respond_to? :submission # e.g. request_for_comment + possible_programming_group = @record.submission.contributor + else + return false + end + + return false unless possible_programming_group.programming_group? + + possible_programming_group.users.include?(@user) end private :author_in_programming_group? diff --git a/app/policies/code_ocean/file_policy.rb b/app/policies/code_ocean/file_policy.rb index 3ea9e2f6..298f6f9c 100644 --- a/app/policies/code_ocean/file_policy.rb +++ b/app/policies/code_ocean/file_policy.rb @@ -38,7 +38,7 @@ module CodeOcean if @record.context.is_a?(Exercise) admin? || author? elsif @record.context.is_a?(Submission) && @record.context.exercise.allow_file_creation - author? + author? || author_in_programming_group? else no_one end diff --git a/app/policies/request_for_comment_policy.rb b/app/policies/request_for_comment_policy.rb index a5e863e2..90879425 100644 --- a/app/policies/request_for_comment_policy.rb +++ b/app/policies/request_for_comment_policy.rb @@ -6,7 +6,7 @@ class RequestForCommentPolicy < ApplicationPolicy end def show? - admin? || author? || rfc_visibility + admin? || author? || author_in_programming_group? || rfc_visibility end def destroy? @@ -14,11 +14,11 @@ class RequestForCommentPolicy < ApplicationPolicy end def mark_as_solved? - admin? || author? + admin? || author? || author_in_programming_group? end def set_thank_you_note? - admin? || author? + admin? || author? || author_in_programming_group? end def clear_question? diff --git a/spec/policies/code_ocean/file_policy_spec.rb b/spec/policies/code_ocean/file_policy_spec.rb index c3b2d774..3bb3e530 100644 --- a/spec/policies/code_ocean/file_policy_spec.rb +++ b/spec/policies/code_ocean/file_policy_spec.rb @@ -30,31 +30,81 @@ describe CodeOcean::FilePolicy do context 'when being part of a submission' do let(:file) { submission.files.first } - context 'when file creation is allowed' do + shared_context 'when file creation is allowed' do before do submission.exercise.update(allow_file_creation: true) end - - it 'grants access to authors' do - expect(policy).to permit(submission.author, file) - end end - context 'when file creation is not allowed' do + shared_context 'when file creation is not allowed' do before do submission.exercise.update(allow_file_creation: false) end + end - it 'grants access to authors' do - expect(policy).not_to permit(submission.author, file) + shared_examples 'no other user allowed to access' do + it 'does not grant access to all other users' do + %i[admin external_user teacher].each do |factory_name| + expect(policy).not_to permit(create(factory_name), file) + end end end - it 'does not grant access to all other users' do - %i[admin external_user teacher].each do |factory_name| - expect(policy).not_to permit(create(factory_name), file) + context 'when a single user authored' do + context 'when file creation is allowed' do + include_context 'when file creation is allowed' + + it 'grants access to authors' do + expect(policy).to permit(submission.author, file) + end + + it_behaves_like 'no other user allowed to access' + end + + context 'when file creation is not allowed' do + include_context 'when file creation is not allowed' + + it 'does not grant access to authors' do + expect(policy).not_to permit(submission.author, file) + end + + it_behaves_like 'no other user allowed to access' end end + + context 'when a programming group authored' do + let(:group_author) { create(:external_user) } + let(:other_group_author) { create(:external_user) } + let(:programming_group) { create(:programming_group, exercise: submission.exercise, users: [group_author, other_group_author]) } + + before do + submission.update(contributor: programming_group) + end + + context 'when file creation is allowed' do + include_context 'when file creation is allowed' + + it 'grants access to authors' do + expect(policy).to permit(group_author, file) + expect(policy).to permit(other_group_author, file) + end + + it_behaves_like 'no other user allowed to access' + end + + context 'when file creation is not allowed' do + include_context 'when file creation is not allowed' + + it 'does not grant access to authors' do + expect(policy).not_to permit(group_author, file) + expect(policy).not_to permit(other_group_author, file) + end + + it_behaves_like 'no other user allowed to access' + end + end + + it_behaves_like 'no other user allowed to access' end end diff --git a/spec/policies/request_for_comment_policy_spec.rb b/spec/policies/request_for_comment_policy_spec.rb index 707066be..88745ead 100644 --- a/spec/policies/request_for_comment_policy_spec.rb +++ b/spec/policies/request_for_comment_policy_spec.rb @@ -59,6 +59,12 @@ describe RequestForCommentPolicy do it 'grants access to authors' do expect(policy).to permit(rfc.author, rfc) end + + it 'grant access to other authors of the programming group' do + rfc.submission.update(contributor: programming_group) + expect(policy).to permit(author_other_group_member, rfc) + expect(policy).to permit(viewer_other_group_member, rfc) + end end shared_examples 'grants access to admins and authors only' do @@ -70,6 +76,12 @@ describe RequestForCommentPolicy do expect(policy).to permit(rfc.author, rfc) end + it 'grant access to other authors of the programming group' do + rfc.submission.update(contributor: programming_group) + expect(policy).to permit(author_other_group_member, rfc) + expect(policy).to permit(viewer_other_group_member, 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) @@ -81,6 +93,10 @@ describe RequestForCommentPolicy do let(:author_study_groups) { create_list(:study_group, 1, consumer: author_consumer) } let(:rfc) { create(:rfc, user: rfc_author) } + let(:author_other_group_member) { create(:external_user, consumer: author_consumer) } + let(:viewer_other_group_member) { create(:external_user, consumer: viewer_consumer) } + let(:programming_group) { create(:programming_group, exercise: rfc.submission.exercise, users: [rfc.author, author_other_group_member, viewer_other_group_member]) } + context "when the author's rfc_visibility is set to all" do let(:author_consumer) { create(:consumer, rfc_visibility: 'all') }