Skip to content

Commit 2b6f331

Browse files
committed
Forbid updating of non-public projects
This `Api::PublicProjectsController` is focussed on managing "public" projects, i.e. those that have no `Project#user_id` set. I think it's sensible to protect against the `update` action being used to update a non-public project, because it makes the code easier to reason about.
1 parent dcb5050 commit 2b6f331

File tree

3 files changed

+19
-2
lines changed

3 files changed

+19
-2
lines changed

app/controllers/api/public_projects_controller.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ class PublicProjectsController < ApiController
55
before_action :authorize_user
66
before_action :restrict_project_type, only: %i[create]
77
before_action :load_project, only: %i[update]
8+
before_action :restrict_to_public_projects, only: %i[update]
89

910
def create
1011
authorize! :create, :public_project
@@ -52,5 +53,11 @@ def restrict_project_type
5253

5354
raise CanCan::AccessDenied.new("#{project_type} not yet supported", :create, :public_project)
5455
end
56+
57+
def restrict_to_public_projects
58+
return if @project.user_id.blank?
59+
60+
raise CanCan::AccessDenied.new('Cannot update non-public project', :update, :public_project)
61+
end
5562
end
5663
end

spec/features/public_project/updating_a_public_project_spec.rb

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44

55
RSpec.describe 'Updating a public project', type: :request do
66
let(:creator) { build(:experience_cs_admin_user) }
7-
let(:project) { create(:project, locale: 'en', project_type: Project::Types::SCRATCH) }
7+
let(:user_id) { nil }
8+
let(:project) { create(:project, locale: 'en', project_type: Project::Types::SCRATCH, user_id:) }
89
let(:headers) { { Authorization: UserProfileMock::TOKEN } }
910
let(:params) { { project: { name: 'New name' } } }
1011

@@ -43,6 +44,15 @@
4344
expect(response).to have_http_status(:unauthorized)
4445
end
4546

47+
context 'when project is not public' do
48+
let(:user_id) { SecureRandom.uuid }
49+
50+
it 'responds 403 Forbidden' do
51+
put("/api/public_projects/#{project.identifier}?project_type=scratch", headers:, params:)
52+
expect(response).to have_http_status(:forbidden)
53+
end
54+
end
55+
4656
it 'responds 404 Not Found when project is not found' do
4757
put('/api/public_projects/another-identifier?project_type=scratch', headers:, params:)
4858
expect(response).to have_http_status(:not_found)

spec/requests/public_projects/update_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
RSpec.describe 'Update public project requests' do
66
let(:locale) { 'fr' }
77
let(:project_loader) { instance_double(ProjectLoader) }
8-
let(:project) { create(:project, locale: 'en', project_type: Project::Types::SCRATCH) }
8+
let(:project) { create(:project, locale: 'en', project_type: Project::Types::SCRATCH, user_id: nil) }
99
let(:creator) { build(:experience_cs_admin_user) }
1010
let(:params) { { project: { name: 'New name' } } }
1111

0 commit comments

Comments
 (0)