feat(ssid): migrate to rails credentials, ssid multipart upload api

Author: adi-herwana-nus

.gitignore

@@ -14,6 +14,12 @@
 # Ignore user-specific VSCode project files
 .vscode
 
+# Ignore credentials files which may contain sensitive information
+/config/credentials/*.key
+/config/credentials/*.yml.enc
+!/config/credentials/test.key
+!/config/credentials/test.yml.enc
+
 # Ignore the default SQLite database.
 /db/*.sqlite3
 /db/*.sqlite3-journal

app/services/course/assessment/submission/ssid_plagiarism_service.rb

@@ -32,7 +32,7 @@ def fetch_plagiarism_result(limit, offset)
   end
 
   def download_submission_pair_result(submission_pair_id)
-    ssid_api_service = SsidAsyncApiService.new(
+    ssid_api_service = SsidApiService.new(
       "submission-pairs/#{submission_pair_id}/report", {}
     )
     response_status, response_body = ssid_api_service.get
@@ -52,7 +52,7 @@ def share_assessment_result
   end
 
   def fetch_plagiarism_check_result
-    ssid_api_service = SsidAsyncApiService.new("folders/#{@main_assessment.ssid_folder_id}/plagiarism-checks", {})
+    ssid_api_service = SsidApiService.new("folders/#{@main_assessment.ssid_folder_id}/plagiarism-checks", {})
     response_status, response_body = ssid_api_service.get
     raise SsidError, { status: response_status, body: response_body } unless response_status == 200
 
@@ -67,13 +67,28 @@ def create_ssid_folders
     end
   end
 
+  def prepare_ssid_presigned_request(assessment)
+    ssid_api_service = SsidApiService.new("folders/#{assessment.ssid_folder_id}/uploads", {})
+    response_status, response_body = ssid_api_service.post
+    raise SsidError, { status: response_status, body: response_body } unless response_status == 200
+
+    upload_url = response_body['payload']['data']['url']
+    unless SsidApiService.upload_whitelist.match?(upload_url)
+      raise SsidError, { status: 500, body: 'Invalid presigned URL' }
+    end
+
+    [upload_url, response_body['payload']['data']['fields']]
+  end
+
   def run_upload_answers
     @linked_assessments.each do |assessment|
+      upload_url, upload_fields = prepare_ssid_presigned_request(assessment)
       service = Course::Assessment::Submission::SsidZipDownloadService.new(assessment)
       zip_files = service.download_and_zip
-      ssid_api_service = SsidAsyncApiService.new("folders/#{assessment.ssid_folder_id}/submissions", {})
+      ssid_api_service = SsidApiService.new('', {}, upload_url)
       zip_files.each do |zip_file|
-        response_status, response_body = ssid_api_service.post_multipart(zip_file)
+        form_data = upload_fields.merge('file' => Faraday::Multipart::FilePart.new(zip_file, 'application/zip'))
+        response_status, response_body = ssid_api_service.post_multipart(form_data)
         raise SsidError, { status: response_status, body: response_body } unless response_status == 204
       end
     ensure
@@ -82,7 +97,7 @@ def run_upload_answers
   end
 
   def send_plagiarism_check_request
-    ssid_api_service = SsidAsyncApiService.new("folders/#{@main_assessment.ssid_folder_id}/plagiarism-checks", {
+    ssid_api_service = SsidApiService.new("folders/#{@main_assessment.ssid_folder_id}/plagiarism-checks", {
       comparedFolderIds: @linked_assessments.pluck(:ssid_folder_id)
     })
     response_status, response_body = ssid_api_service.post
@@ -94,7 +109,7 @@ def ssid_submission_to_submission_id(ssid_submission)
   end
 
   def fetch_ssid_submission_pair_data(limit, offset)
-    ssid_api_service = SsidAsyncApiService.new(
+    ssid_api_service = SsidApiService.new(
       "folders/#{@main_assessment.ssid_folder_id}/plagiarism-checks/latest/submission-pairs",
       { limit: limit, offset: offset }
     )
@@ -105,7 +120,7 @@ def fetch_ssid_submission_pair_data(limit, offset)
   end
 
   def create_ssid_shared_resource_link(resource_type, resource_id)
-    ssid_api_service = SsidAsyncApiService.new('shared-resources', {
+    ssid_api_service = SsidApiService.new('shared-resources', {
       resourceType: resource_type,
       resourceId: resource_id
     })

app/services/course/ssid_folder_service.rb

@@ -5,7 +5,7 @@ def initialize(folder_name, parent_folder_id = nil)
   end
 
   def run_create_ssid_folder_service
-    ssid_api_service = SsidAsyncApiService.new('folders', @folder_object)
+    ssid_api_service = SsidApiService.new('folders', @folder_object)
     response_status, response_body = ssid_api_service.post
 
     # If id is lost in our DB somehow, we can recover it if SSID returns a 409

app/services/ssid_async_api_service.rb app/services/ssid_api_service.rbapp/services/ssid_async_api_service.rb renamed to app/services/ssid_api_service.rb

@@ -1,13 +1,20 @@
 # frozen_string_literal: true
 
-class SsidAsyncApiService
-  def config
-    ENV.fetch('SSID_URL')
+class SsidApiService
+  def self.api_url
+    Rails.application.credentials.ssid.url
   end
 
-  def initialize(api_namespace, payload)
+  # Validate that the URL is in the whitelist before allowing uploads to it.
+  # This is to prevent leaking data in case the SSID response is intercepted.
+  def self.upload_whitelist
+    Regexp.new(Rails.application.credentials.ssid.upload_whitelist_pattern)
+  end
+
+  def initialize(api_namespace, payload, url = nil)
     @api_namespace = api_namespace
     @payload = payload
+    @url = url || self.class.api_url
   end
 
   def post
@@ -20,8 +27,7 @@ def post
     [500, nil]
   end
 
-  def post_multipart(file_path)
-    form_data = { 'file' => Faraday::Multipart::FilePart.new(file_path, 'application/zip') }
+  def post_multipart(form_data)
     response = connection.post(@api_namespace) do |req|
       req.body = form_data
     end
@@ -42,8 +48,10 @@ def get
   private
 
   def connection
-    @connection ||= Faraday.new(url: config) do |builder|
-      builder.request :authorization, 'Bearer', -> { ENV.fetch('SSID_API_KEY', nil) }
+    @connection ||= Faraday.new(url: @url) do |builder|
+      if @url == self.class.api_url
+        builder.request :authorization, 'Bearer', -> { Rails.application.credentials.ssid.api_key }
+      end
       builder.request :multipart
     end
   end

config/credentials/README.md

@@ -11,7 +11,7 @@ Each environment has two files:
 - `<environment>.key`, the decryption key that allows reading the `.yml.enc` file. **Keys from deployed environments (staging and production) must NEVER BE COMMITTED.** Once leaked, all secrets in the environment will be compromised.
 - `<environment>.yml.enc`, an encrypted YAML file containing sensitive environment data (e.g. API keys). While theoretically safe to commit because the data is unreadable without the decryption key, we currently do not commit files containing real sensitive data as an additional safety measure.
 
-The `development` and `test` key files are checked into this repository, so local development works out of the box. The sample credentials use the same structure as staging/production but with redacted values, so external API integrations (e.g. Codaveri, AWS) will not function.
+The `test` key files are checked into this repository. The sample credentials use the same structure as other environments but with redacted values, so external API integrations (e.g. Codaveri, AWS) will not function.
 
 If you are a Coursemology team member who needs working credentials, contact current staff for the appropriate credentials file.
 

config/credentials/development.key

@@ -1 +1 @@
-22UauTnIrniRSCH/x4uVoj8HBcZJHb1ET04fxlhsYVsXPZ5vXG5mr+V5D5+F5iKv4pHNpAA7Osg6b/jWiG6t5uKPvlCwRL5j4aBi03bMIc5AMBH/8s2gEfvpWSZUC0viSNT4zwfJgKdgwHs4QhBg3EfkYxLe6S3R7W/RwX/hgW8pQB5Hw3bZhpbOtDuJeP/4Of4xg51PWP2hEz4K4IF9kTMKtjiGpprIoUQ0WFhe7l8cDBWsXBZdusCH4azQHkKo0A6YtuABe+Yaq5gBb7J2GqnVss89IJRp3TILfjgpFy8CrjGx1DpiY6csHFlvqbiOxJUjk+B0oqp2qIFOrGabcNEaGCT7tvNUEJwKb9WPYLTwfNW2EW4VgXAO7hM=--bb5c6OI9HMr/h3Jd--0p6ur3vWAdKRI3SYxGArog==
+ONmuV4CWoclCzxsWAbR+B0P9g7iT9wGrBGZBzNLfxp0S6lQ+kiJJrw8wV66vKDYxiODYdTFA4W+vL//7Dd3HAdMy1s+3qFxtEu0GvbFbHJDbu46uCQd/4UiVEpjHa0KeBwd2n6SpIUAZDmBbrtHoGKzxz1lP3M6vyd1g9D2vZguiExuH4JVDbF4jMYd4DQdlqLZL2Poa3r1Gwp/70mt4u2J+rNI1JshK0LFov9P3sAqWIDDbodaQXRDmp92eSNtdui5AmaeGtQjYryygE8njKE4OWhWAGmoRAk20hpiOZvvIc8DMtFxQQemrXZWFRdOkq2g/Lyy2MOMMSSM9dhoAOcPEjCEcSNDmkXddrefJPkEfWp+sXWC6590pbyx7Lvterl+u/bHMALhnUGdVkkKxrNVWAJ9xt1TOfR4TM6ThBZOPF8/x+VEnd0arhh3zW1pe8FJ2mZqFxGbL8VuOA/VPRzAszEq3u3bW5WY36AMIg7xgBplyBqQXDsTW7BCbSFHpX7Y+VwpEOj1lSyhoVGd11s/oWXXHZD/W9C/QbBiWHOe5cYJ/Ugvryhkob/6gkR1B5u6DWLOsQrokF5yMN33h3MwrKbhFYUcaH/D9hECJweVcW+zKUKBpPbI7smlaigBCFP1kUn4D4XoV4Lmr1A7Na+Cn2c3p7aVarPC7GJic4/pA0ZQKDL3/CKsTv24z44j0G4mDpT7Rp6SSYuIkb2mqufp3lqSgC/e7MkDHJ1lx5g3BmCCga0SeNaXxXvRNs05eDCewP69m58rFTS+JubhA8XrXl5Az8xovn3W9QgYR5Zp60gC7cVWigzCZTqpNBGranw3HecQ1N12hxZ/41u3ZLLz9WSIgWPI6WBQ5wXhyuFUBAD261Q==--RTxTAhlW3voVXY8u--+DWAPJUB5NgmSl6FAyhUYg==

config/credentials/development.yml.enc

@@ -13,7 +13,6 @@
 # We use a dummy value since leaving it as null raises an error, and potentially
 # in the future we can use it in stub logic to differentiate from other external APIs.
 ENV['CODAVERI_URL'] ||= 'http://localhost:53896'
-ENV['SSID_URL'] ||= 'http://localhost:53897'
 
 require 'spec_helper'
 require 'rspec/rails'

config/credentials/test.yml.enc

@@ -20,7 +20,8 @@
     end
 
     before do
-      allow_any_instance_of(SsidAsyncApiService).to receive(:connection).and_return(connection)
+      allow(SsidApiService).to receive(:api_url).and_return('http://localhost:53897')
+      allow_any_instance_of(SsidApiService).to receive(:connection).and_return(connection)
       allow_any_instance_of(Course::Assessment::Submission::SsidZipDownloadService).to receive(:download_and_zip).
         and_return([File.join(Rails.root, 'spec/fixtures/course/ssid/submissions.zip')])
     end
@@ -38,36 +39,66 @@
     end
 
     describe '#start_plagiarism_check' do
-      before do
-        stubs.post('/folders') do
-          [Ssid::ApiStubs::CREATE_FOLDER_SUCCESS[:status],
-           { 'Content-Type': 'application/json' },
-           Ssid::ApiStubs::CREATE_FOLDER_SUCCESS[:body]]
-        end
-        stubs.post(/folders\/.*\/submissions/) do |env|
-          expect(env[:url].to_s).to include("/folders/#{assessment.ssid_folder_id}/submissions")
-          [Ssid::ApiStubs::UPLOAD_ANSWERS_SUCCESS[:status],
-           { 'Content-Type': 'application/json' },
-           Ssid::ApiStubs::UPLOAD_ANSWERS_SUCCESS[:body]]
+      context 'when presigned URL matches whitelist' do
+        before do
+          allow(SsidApiService).to receive(:upload_whitelist).and_return(/.*/)
+          stubs.post('/folders') do
+            [Ssid::ApiStubs::CREATE_FOLDER_SUCCESS[:status],
+             { 'Content-Type': 'application/json' },
+             Ssid::ApiStubs::CREATE_FOLDER_SUCCESS[:body]]
+          end
+          stubs.post(/folders\/.*\/uploads/) do |env|
+            expect(env[:url].to_s).to include("/folders/#{assessment.ssid_folder_id}/uploads")
+            [Ssid::ApiStubs::GET_PRESIGNED_UPLOAD_URL_SUCCESS[:status],
+             { 'Content-Type': 'application/json' },
+             Ssid::ApiStubs::GET_PRESIGNED_UPLOAD_URL_SUCCESS[:body]]
+          end
+          stubs.post('/') do
+            [Ssid::ApiStubs::MULTIPART_UPLOAD_SUCCESS[:status],
+             {},
+             Ssid::ApiStubs::MULTIPART_UPLOAD_SUCCESS[:body]]
+          end
+          stubs.post(/folders\/.*\/plagiarism-checks/) do |env|
+            expect(env[:url].to_s).to include("/folders/#{assessment.ssid_folder_id}/plagiarism-checks")
+            [Ssid::ApiStubs::SEND_PLAGIARISM_CHECK_SUCCESS[:status],
+             { 'Content-Type': 'application/json' },
+             Ssid::ApiStubs::SEND_PLAGIARISM_CHECK_SUCCESS[:body]]
+          end
+          stubs.get(/folders\/.*\/plagiarism-checks/) do |env|
+            expect(env[:url].to_s).to include("/folders/#{assessment.ssid_folder_id}/plagiarism-checks")
+            [Ssid::ApiStubs::FETCH_PLAGIARISM_CHECK_SUCCESSFUL[:status],
+             { 'Content-Type': 'application/json' },
+             Ssid::ApiStubs::FETCH_PLAGIARISM_CHECK_SUCCESSFUL[:body]]
+          end
         end
-        stubs.post(/folders\/.*\/plagiarism-checks/) do |env|
-          expect(env[:url].to_s).to include("/folders/#{assessment.ssid_folder_id}/plagiarism-checks")
-          [Ssid::ApiStubs::SEND_PLAGIARISM_CHECK_SUCCESS[:status],
-           { 'Content-Type': 'application/json' },
-           Ssid::ApiStubs::SEND_PLAGIARISM_CHECK_SUCCESS[:body]]
-        end
-        stubs.get(/folders\/.*\/plagiarism-checks/) do |env|
-          expect(env[:url].to_s).to include("/folders/#{assessment.ssid_folder_id}/plagiarism-checks")
-          [Ssid::ApiStubs::FETCH_PLAGIARISM_CHECK_SUCCESSFUL[:status],
-           { 'Content-Type': 'application/json' },
-           Ssid::ApiStubs::FETCH_PLAGIARISM_CHECK_SUCCESSFUL[:body]]
+
+        it 'completes similarity check successfully' do
+          subject.start_plagiarism_check
+          response = subject.fetch_plagiarism_check_result
+          expect(response['status']).to eq('successful')
         end
       end
 
-      it 'completes similarity check successfully' do
-        subject.start_plagiarism_check
-        response = subject.fetch_plagiarism_check_result
-        expect(response['status']).to eq('successful')
+      context 'when presigned URL does not match whitelist' do
+        before do
+          allow(SsidApiService).to receive(:upload_whitelist).and_return(/^https:\/\/trusted\.s3\.amazonaws\.com/)
+          stubs.post('/folders') do
+            [Ssid::ApiStubs::CREATE_FOLDER_SUCCESS[:status],
+             { 'Content-Type': 'application/json' },
+             Ssid::ApiStubs::CREATE_FOLDER_SUCCESS[:body]]
+          end
+          stubs.post(/folders\/.*\/uploads/) do
+            [Ssid::ApiStubs::GET_PRESIGNED_UPLOAD_URL_SUCCESS[:status],
+             { 'Content-Type': 'application/json' },
+             Ssid::ApiStubs::GET_PRESIGNED_UPLOAD_URL_SUCCESS[:body]]
+          end
+        end
+
+        it 'raises SsidError before uploading' do
+          expect_any_instance_of(Course::Assessment::Submission::SsidZipDownloadService).
+            not_to receive(:download_and_zip)
+          expect { subject.start_plagiarism_check }.to raise_error(SsidError)
+        end
       end
     end