Skip to content

Commit ec60963

Browse files
authored
Fix missing authorization check before REST project remix creation (#864)
## Status - Closes RaspberryPiFoundation/digital-editor-issues#1468 ### Why The REST project remix endpoint authenticated the caller but did not verify that the caller could view the original project before cloning it. That meant a user who knew a project identifier could create a remix from a project they were not authorized to read. This now aligns the REST remix flow with the existing Scratch and GraphQL remix authorization behavior. ### What Changed - Added an explicit authorize! :show, project check before Project::CreateRemix.call. - Added regression coverage for: - remixing another user’s private project - a student remixing a teacher-only lesson project they cannot view - The regression specs assert the request is forbidden, no remix is created, and the clone operation is not called. _Follow-up: consider enabling CanCanCan check_authorization at the API controller layer so missing authorization becomes fail-closed by default. This should be handled separately because it requires auditing existing controllers and adding explicit skip_authorization_check for intentionally public or non-CanCan endpoints._
1 parent bb1347f commit ec60963

2 files changed

Lines changed: 45 additions & 0 deletions

File tree

app/controllers/api/projects/remixes_controller.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ def show_identifier
2525
end
2626

2727
def create
28+
authorize! :show, project
29+
2830
# Ensure we have a fallback value to prevent bad requests
2931
remix_origin = request.origin || request.referer
3032
result = Project::CreateRemix.call(params: remix_params,

spec/requests/projects/remix_spec.rb

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,49 @@
168168
expect(response).to have_http_status(:not_found)
169169
end
170170

171+
context 'when the original project belongs to another user' do
172+
let!(:original_project) { create(:project, user_id: create(:user).id) }
173+
174+
it 'returns forbidden without creating a remix' do
175+
allow(Project::CreateRemix).to receive(:call).and_call_original
176+
177+
expect do
178+
post("/api/projects/#{original_project.identifier}/remix", params: { project: project_params }, headers:)
179+
end.not_to change(Project, :count)
180+
181+
expect(response).to have_http_status(:forbidden)
182+
expect(Project::CreateRemix).not_to have_received(:call)
183+
end
184+
end
185+
186+
context 'when a student cannot view the teacher-only original project' do
187+
let(:student) { create(:student, school:) }
188+
let(:teacher) { create(:teacher, school:) }
189+
let(:school_class) { create(:school_class, school:, teacher_ids: [teacher.id]) }
190+
let(:lesson) { create(:lesson, school:, school_class:, user_id: teacher.id, visibility: 'teachers') }
191+
let!(:original_project) do
192+
lesson.project.tap do |project|
193+
project.update!(school:, user_id: teacher.id, instructions: 'Teacher-only instructions')
194+
end
195+
end
196+
197+
before do
198+
create(:class_student, school_class:, student_id: student.id)
199+
authenticated_in_hydra_as(student)
200+
end
201+
202+
it 'returns forbidden without creating a remix' do
203+
allow(Project::CreateRemix).to receive(:call).and_call_original
204+
205+
expect do
206+
post("/api/projects/#{original_project.identifier}/remix", params: { project: project_params }, headers:)
207+
end.not_to change(Project, :count)
208+
209+
expect(response).to have_http_status(:forbidden)
210+
expect(Project::CreateRemix).not_to have_received(:call)
211+
end
212+
end
213+
171214
context 'when project cannot be saved' do
172215
before do
173216
authenticated_in_hydra_as(owner)

0 commit comments

Comments
 (0)