diff --git a/app/controllers/api/projects_controller.rb b/app/controllers/api/projects_controller.rb index 52ab64663..6837c9403 100644 --- a/app/controllers/api/projects_controller.rb +++ b/app/controllers/api/projects_controller.rb @@ -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]) @@ -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]) diff --git a/app/controllers/api/scratch/scratch_controller.rb b/app/controllers/api/scratch/scratch_controller.rb index 9b1d1b187..863ab9928 100644 --- a/app/controllers/api/scratch/scratch_controller.rb +++ b/app/controllers/api/scratch/scratch_controller.rb @@ -3,8 +3,6 @@ module Api module Scratch class ScratchController < ApiController - include IdentifiableByCookie - before_action :authorize_user before_action :check_scratch_feature diff --git a/app/controllers/concerns/identifiable_by_cookie.rb b/app/controllers/concerns/identifiable_by_cookie.rb deleted file mode 100644 index f77a598bc..000000000 --- a/app/controllers/concerns/identifiable_by_cookie.rb +++ /dev/null @@ -1,16 +0,0 @@ -# frozen_string_literal: true - -module IdentifiableByCookie - extend ActiveSupport::Concern - include ActionController::Cookies - - included do - before_action :load_current_user - attr_reader :current_user - end - - def load_current_user - token = cookies[:scratch_auth] - @current_user = User.from_token(token:) if token - end -end diff --git a/spec/features/scratch/creating_a_scratch_asset_spec.rb b/spec/features/scratch/creating_a_scratch_asset_spec.rb index 5d1ce8679..8b4e386a4 100644 --- a/spec/features/scratch/creating_a_scratch_asset_spec.rb +++ b/spec/features/scratch/creating_a_scratch_asset_spec.rb @@ -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) @@ -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) diff --git a/spec/features/scratch/creating_a_scratch_project_spec.rb b/spec/features/scratch/creating_a_scratch_project_spec.rb index d3e17f6ce..8a72c8757 100644 --- a/spec/features/scratch/creating_a_scratch_project_spec.rb +++ b/spec/features/scratch/creating_a_scratch_project_spec.rb @@ -7,7 +7,7 @@ let(:teacher) { create(:teacher, school:) } let(:headers) do { - 'Cookie' => "scratch_auth=#{UserProfileMock::TOKEN}", + 'Authorization' => UserProfileMock::TOKEN, 'Origin' => 'editor.com' } end @@ -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) diff --git a/spec/features/scratch/creating_and_showing_a_scratch_asset_spec.rb b/spec/features/scratch/creating_and_showing_a_scratch_asset_spec.rb index 0eac23747..ce3e2ef74 100644 --- a/spec/features/scratch/creating_and_showing_a_scratch_asset_spec.rb +++ b/spec/features/scratch/creating_and_showing_a_scratch_asset_spec.rb @@ -9,7 +9,7 @@ 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 @@ -17,10 +17,8 @@ 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? @@ -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? @@ -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 @@ -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', @@ -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', @@ -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 diff --git a/spec/features/scratch/updating_a_scratch_project_spec.rb b/spec/features/scratch/updating_a_scratch_project_spec.rb index 2ada2ce06..5c32a885e 100644 --- a/spec/features/scratch/updating_a_scratch_project_spec.rb +++ b/spec/features/scratch/updating_a_scratch_project_spec.rb @@ -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) @@ -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( @@ -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) diff --git a/spec/requests/projects/show_spec.rb b/spec/requests/projects/show_spec.rb index b154fc8c6..74c515392 100644 --- a/spec/requests/projects/show_spec.rb +++ b/spec/requests/projects/show_spec.rb @@ -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') }