Use headers instead of cookies for authorization (#771)

## Status

- Closes
[1256](RaspberryPiFoundation/digital-editor-issues#1256)
- Related to
[1239](RaspberryPiFoundation/digital-editor-issues#1239)
and
[1255](RaspberryPiFoundation/digital-editor-issues#1255)

## What's changed?

- Force endpoints to use authentication headers instead of Cookies
- Removed cookie helpers since not used anymore

## Steps to perform after deploying to production

_If the production environment requires any extra work after this PR has
been deployed detail it here. This could be running a Rake task, a
migration, or upgrading a Gem. That kind of thing._

Author: abcampo-iry

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])

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
 

app/controllers/concerns/identifiable_by_cookie.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)
 

spec/features/scratch/creating_a_scratch_asset_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)

spec/features/scratch/creating_a_scratch_project_spec.rb

@@ -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?
 
@@ -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

spec/features/scratch/creating_and_showing_a_scratch_asset_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)

spec/features/scratch/updating_a_scratch_project_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') }