Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 0 additions & 15 deletions app/controllers/api/projects_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,12 @@

module Api
class ProjectsController < ApiController
include ActionController::Cookies

before_action :authorize_user, only: %i[create update index destroy]
before_action :load_project, only: %i[show update destroy show_context]
before_action :load_projects, only: %i[index]
load_and_authorize_resource
before_action :verify_lesson_belongs_to_school, only: :create
after_action :pagination_link_header, only: %i[index]
before_action :set_auth_cookie_for_scratch, only: %i[show]

def index
@paginated_projects = @projects.page(params[:page])
Expand Down Expand Up @@ -62,18 +59,6 @@ def show_context

private

def set_auth_cookie_for_scratch
return unless @project.scratch_project?
return unless Flipper.enabled?(:cat_mode, school)

cookies[:scratch_auth] = {
value: request.headers['Authorization'],
secure: Rails.env.production?,
same_site: :strict,
http_only: true
}
end

def verify_lesson_belongs_to_school
return if base_params[:lesson_id].blank?
return if school&.lessons&.pluck(:id)&.include?(base_params[:lesson_id])
Expand Down
2 changes: 0 additions & 2 deletions app/controllers/api/scratch/scratch_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
module Api
module Scratch
class ScratchController < ApiController
include IdentifiableByCookie

before_action :authorize_user
before_action :check_scratch_feature

Expand Down
16 changes: 0 additions & 16 deletions app/controllers/concerns/identifiable_by_cookie.rb

This file was deleted.

10 changes: 5 additions & 5 deletions spec/features/scratch/creating_a_scratch_asset_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@
RSpec.describe 'Creating a Scratch asset', type: :request do
let(:school) { create(:school) }
let(:teacher) { create(:teacher, school:) }
let(:cookie_headers) { { 'Cookie' => "scratch_auth=#{UserProfileMock::TOKEN}" } }
let(:auth_headers) { { 'Authorization' => UserProfileMock::TOKEN } }

before do
Flipper.disable :cat_mode
Flipper.disable_actor :cat_mode, school
end

it 'responds 401 Unauthorized when no cookie is provided' do
it 'responds 401 Unauthorized when no Authorization header is provided' do
post '/api/scratch/assets/example.svg'

expect(response).to have_http_status(:unauthorized)
Expand All @@ -21,16 +21,16 @@
it 'responds 404 Not Found when cat_mode is not enabled' do
authenticated_in_hydra_as(teacher)

post '/api/scratch/assets/example.svg', headers: cookie_headers
post '/api/scratch/assets/example.svg', headers: auth_headers

expect(response).to have_http_status(:not_found)
end

it 'creates an asset when cat_mode is enabled and a cookie is provided' do
it 'creates an asset when cat_mode is enabled and the required headers are provided' do
authenticated_in_hydra_as(teacher)
Flipper.enable_actor :cat_mode, school

post '/api/scratch/assets/example.svg', headers: cookie_headers
post '/api/scratch/assets/example.svg', headers: auth_headers

expect(response).to have_http_status(:created)

Expand Down
4 changes: 2 additions & 2 deletions spec/features/scratch/creating_a_scratch_project_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
let(:teacher) { create(:teacher, school:) }
let(:headers) do
{
'Cookie' => "scratch_auth=#{UserProfileMock::TOKEN}",
'Authorization' => UserProfileMock::TOKEN,
'Origin' => 'editor.com'
}
end
Expand Down Expand Up @@ -49,7 +49,7 @@ def make_request(query: request_query, request_headers: headers, request_params:
)
end

it 'responds 401 Unauthorized when no cookie is provided' do
it 'responds 401 Unauthorized when no Authorization header is provided' do
make_request(request_headers: {})

expect(response).to have_http_status(:unauthorized)
Expand Down
34 changes: 5 additions & 29 deletions spec/features/scratch/creating_and_showing_a_scratch_asset_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,16 @@
let(:filename) { "#{basename}.#{format}" }
let(:school) { create(:school) }
let(:teacher) { create(:teacher, school:) }
let(:cookie_headers) { { 'Cookie' => "scratch_auth=#{UserProfileMock::TOKEN}" } }
let(:auth_headers) { { 'Authorization' => UserProfileMock::TOKEN } }

describe 'GET #show' do
context 'when the asset is PNG' do
before do
create(:scratch_asset, :with_file, filename:, asset_path: file_fixture(filename))
end

let(:make_request) { get '/api/scratch/assets/internalapi/asset/test_image_1.png/get/' }

it 'serves the file with png content type' do
make_request
get '/api/scratch/assets/internalapi/asset/test_image_1.png/get/'

follow_redirect! while response.redirect?

Expand All @@ -33,10 +31,8 @@
create(:scratch_asset, :with_file, filename: svg_filename, asset_path: file_fixture(svg_filename))
end

let(:make_request) { get '/api/scratch/assets/internalapi/asset/test_svg_image.svg/get/' }

it 'serves the file with image/svg+xml content type' do
make_request
get '/api/scratch/assets/internalapi/asset/test_svg_image.svg/get/'

follow_redirect! while response.redirect?

Expand All @@ -48,7 +44,7 @@
describe 'POST #create' do
let(:upload) { File.binread(file_fixture(filename)) }
let(:make_request) do
post '/api/scratch/assets/test_image_1.png', headers: { 'Content-Type' => 'application/octet-stream' }.merge(cookie_headers), params: upload
post '/api/scratch/assets/test_image_1.png', headers: { 'Content-Type' => 'application/octet-stream' }.merge(auth_headers), params: upload
end

context 'when user is logged in and cat_mode is enabled' do
Expand All @@ -59,54 +55,42 @@
end

it 'creates a new asset' do
# Arrange
Flipper.enable_actor :cat_mode, school

# Act & Assert
expect { make_request }.to change(ScratchAsset, :count).by(1)
end

it 'sets the filename on the asset' do
# Arrange
Flipper.enable_actor :cat_mode, school

# Act & Assert
make_request
expect(ScratchAsset.last.filename).to eq(filename)
end

it 'attaches the uploaded file to the asset' do
# Arrange
Flipper.enable_actor :cat_mode, school

# Act & Assert
make_request
expect(ScratchAsset.last.file).to be_attached
end

it 'stores the content of the file in the attachment' do
# Arrange
Flipper.enable_actor :cat_mode, school

# Act & Assert
make_request
expect(ScratchAsset.last.file.download).to eq(upload)
end

it 'responds with 201 Created' do
# Arrange
Flipper.enable_actor :cat_mode, school

# Act & Assert
make_request
expect(response).to have_http_status(:created)
end

it 'includes the status and filename (without extension) in the response' do
# Arrange
Flipper.enable_actor :cat_mode, school

# Act & Assert
make_request
expect(response.parsed_body).to include(
'status' => 'ok',
Expand All @@ -124,28 +108,22 @@
end

it 'does not update the content of the file in the attachment' do
# Arrange
Flipper.enable_actor :cat_mode, school

# Act & Assert
make_request
expect(ScratchAsset.last.file.download).to eq(original_upload)
end

it 'responds with 201 Created' do
# Arrange
Flipper.enable_actor :cat_mode, school

# Act & Assert
make_request
expect(response).to have_http_status(:created)
end

it 'includes the status and filename (without extension) in the response' do
# Arrange
Flipper.enable_actor :cat_mode, school

# Act & Assert
make_request
expect(response.parsed_body).to include(
'status' => 'ok',
Expand All @@ -163,10 +141,8 @@
end

it 'responds 404 Not Found when cat_mode is not enabled' do
# Act
post '/api/scratch/assets/example.svg', headers: cookie_headers
post '/api/scratch/assets/example.svg', headers: auth_headers

# Assert
expect(response).to have_http_status(:not_found)
end
end
Expand Down
13 changes: 5 additions & 8 deletions spec/features/scratch/updating_a_scratch_project_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@
RSpec.describe 'Updating a Scratch project', type: :request do
let(:school) { create(:school) }
let(:teacher) { create(:teacher, school:) }
let(:cookie_headers) { { 'Cookie' => "scratch_auth=#{UserProfileMock::TOKEN}" } }
let(:auth_headers) { { 'Authorization' => UserProfileMock::TOKEN } }

before do
Flipper.disable :cat_mode
Flipper.disable_actor :cat_mode, school
end

it 'responds 401 Unauthorized when no cookie is provided' do
it 'responds 401 Unauthorized when no Authorization header is provided' do
put '/api/scratch/projects/any-identifier', params: { project: { targets: [] } }

expect(response).to have_http_status(:unauthorized)
Expand All @@ -21,13 +21,12 @@
it 'responds 404 Not Found when cat_mode is not enabled' do
authenticated_in_hydra_as(teacher)

put '/api/scratch/projects/any-identifier', params: { content: { targets: [] } }, headers: cookie_headers
put '/api/scratch/projects/any-identifier', params: { content: { targets: [] } }, headers: auth_headers

expect(response).to have_http_status(:not_found)
end

it 'updates a project when cat_mode is enabled and a cookie is provided' do
# Arrange
it 'updates a project when cat_mode is enabled and an Authorization header is provided' do
authenticated_in_hydra_as(teacher)
Flipper.enable_actor :cat_mode, school
project = create(
Expand All @@ -37,10 +36,8 @@
)
create(:scratch_component, project: project)

# Act
put "/api/scratch/projects/#{project.identifier}", params: { targets: ['some update'] }, headers: cookie_headers
put "/api/scratch/projects/#{project.identifier}", params: { targets: ['some update'] }, headers: auth_headers

# Assert
expect(response).to have_http_status(:ok)

data = JSON.parse(response.body, symbolize_names: true)
Expand Down
37 changes: 0 additions & 37 deletions spec/requests/projects/show_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,43 +54,6 @@
end
end

context 'when setting scratch auth cookie' do
let(:project_type) { Project::Types::PYTHON }
let!(:project) { create(:project, school:, user_id: teacher.id, locale: nil, project_type:) }

before do
Flipper.disable :cat_mode
Flipper.disable_actor :cat_mode, school
end

it 'does not set auth cookie when project is not scratch' do
get("/api/projects/#{project.identifier}", headers:)

expect(response).to have_http_status(:ok)
expect(response.cookies['scratch_auth']).to be_nil
end

context 'when project is code editor scratch' do
let(:project_type) { Project::Types::CODE_EDITOR_SCRATCH }

it 'does not set auth cookie when cat_mode is not enabled' do
get("/api/projects/#{project.identifier}", headers:)

expect(response).to have_http_status(:ok)
expect(response.cookies['scratch_auth']).to be_nil
end

it 'sets auth cookie to auth header' do
Flipper.enable_actor :cat_mode, school

get("/api/projects/#{project.identifier}", headers:)

expect(response).to have_http_status(:ok)
expect(cookies['scratch_auth']).to eq(UserProfileMock::TOKEN)
end
end
end

context 'when loading a student\'s project' do
let(:school_class) { create(:school_class, school:, teacher_ids: [teacher.id]) }
let(:lesson) { create(:lesson, school:, school_class:, user_id: teacher.id, visibility: 'students') }
Expand Down
Loading