Skip to content

Commit d1f34c2

Browse files
committed
Forbid destroying 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 `destroy` action being used to update a non-public project, because it makes the code easier to reason about.
1 parent 894f684 commit d1f34c2

File tree

3 files changed

+13
-3
lines changed

3 files changed

+13
-3
lines changed

app/controllers/api/public_projects_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +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 destroy]
8-
before_action :restrict_to_public_projects, only: %i[update]
8+
before_action :restrict_to_public_projects, only: %i[update destroy]
99

1010
def create
1111
authorize! :create, :public_project

spec/features/public_project/destroying_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 'Destroying a public project', type: :request do
66
let(:destroyer) { 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

1011
before do
@@ -35,6 +36,15 @@
3536
end
3637
end
3738

39+
context 'when project is not public' do
40+
let(:user_id) { SecureRandom.uuid }
41+
42+
it 'responds 403 Forbidden' do
43+
delete("/api/public_projects/#{project.identifier}?project_type=scratch", headers:)
44+
expect(response).to have_http_status(:forbidden)
45+
end
46+
end
47+
3848
it 'responds 404 Not Found when project is not found' do
3949
delete('/api/public_projects/another-identifier?project_type=scratch', headers:)
4050
expect(response).to have_http_status(:not_found)

spec/requests/public_projects/destroy_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
RSpec.describe 'Destroy 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(:destroyer) { build(:experience_cs_admin_user) }
1010

1111
context 'when auth is correct' do

0 commit comments

Comments
 (0)