Add support for signed URLs used by the render_file function

Gemfile

@@ -19,6 +19,7 @@ gem 'ims-lti', '< 2.0.0'
 gem 'jbuilder'
 gem 'json_schemer'
 gem 'js-routes'
+gem 'jwt'
 gem 'kramdown'
 gem 'mimemagic'
 gem 'net-http-persistent'

Gemfile.lock

@@ -571,6 +571,7 @@ DEPENDENCIES
   jbuilder
   js-routes
   json_schemer
+  jwt
   kramdown
   letter_opener
   listen

app/assets/javascripts/editor/submissions.js

@@ -134,7 +134,11 @@ CodeOceanEditorSubmissions = {
     event.preventDefault();
     if ($('#render').is(':visible')) {
       this.createSubmission('#render', null, function (response) {
-        var url = response.render_url.replace(this.FILENAME_URL_PLACEHOLDER, CodeOceanEditor.active_file.filename.replace(/#$/,'')); // remove # if it is the last character, this is not part of the filename and just an anchor
+        if (response.render_url === undefined) return;
+
+        const active_file = CodeOceanEditor.active_file.filename.replace(/#$/,''); // remove # if it is the last character, this is not part of the filename and just an anchor
+        const desired_file = response.render_url.filter(hash => hash.filepath === active_file);
+        const url = desired_file[0].url;
         var pop_up_window = window.open(url);
         if (pop_up_window) {
           pop_up_window.onerror = function (message) {

app/controllers/application_controller.rb

@@ -7,6 +7,7 @@ class ApplicationController < ActionController::Base
   MEMBER_ACTIONS = %i[destroy edit show update].freeze
   RENDER_HOST = CodeOcean::Config.new(:code_ocean).read[:render_host]
 
+  before_action :deny_access_from_render_host
   after_action :verify_authorized, except: %i[welcome]
   around_action :mnemosyne_trace
   around_action :switch_locale
@@ -68,7 +69,7 @@ class ApplicationController < ActionController::Base
       id: current_user.id,
       type: current_user.class.name,
       username: current_user.displayname,
-      consumer: current_user.consumer.name
+      consumer: current_user.consumer&.name
     )
   end
   private :set_sentry_context
@@ -95,10 +96,13 @@ class ApplicationController < ActionController::Base
   def render_error(message, status)
     set_sentry_context
     respond_to do |format|
-      format.html do
+      format.any do
         # Prevent redirect loop
         if request.url == request.referer
           redirect_to :root, alert: message
+        # Redirect to main domain if the request originated from our render_host
+        elsif request.path == '/' && request.host == RENDER_HOST
+          redirect_to Rails.application.config.action_mailer.default_url_options
         else
           redirect_back fallback_location: :root, allow_other_host: false, alert: message
         end
@@ -116,6 +120,10 @@ class ApplicationController < ActionController::Base
   end
   private :switch_locale
 
+  def deny_access_from_render_host
+    raise Pundit::NotAuthorizedError if RENDER_HOST.present? && request.host == RENDER_HOST
+  end
+
   def welcome
     # Show root page
   end

app/controllers/code_ocean/files_controller.rb

@@ -5,6 +5,12 @@ module CodeOcean
     include CommonBehavior
     include FileParameters
 
+    # Overwrite the CSP header and some default actions for the :render_protected_upload action
+    content_security_policy false, only: :render_protected_upload
+    skip_before_action :deny_access_from_render_host, only: :render_protected_upload
+    skip_before_action :verify_authenticity_token, only: :render_protected_upload
+    before_action :require_user!, except: :render_protected_upload
+
     def authorize!
       authorize(@file)
     end
@@ -19,6 +25,18 @@ module CodeOcean
       send_file(real_location, type: @file.native_file.content_type, filename: @file.name_with_extension, disposition: 'attachment')
     end
 
+    def render_protected_upload
+      # Set @current_user with a new *learner* for Pundit checks
+      @current_user = ExternalUser.new
+
+      @file = authorize AuthenticatedUrlHelper.retrieve!(CodeOcean::File, request)
+
+      raise Pundit::NotAuthorizedError unless @file.name_with_extension == params[:filename]
+
+      real_location = Pathname(@file.native_file.current_path).realpath
+      send_file(real_location, type: @file.native_file.content_type, filename: @file.name_with_extension)
+    end
+
     def create
       @file = CodeOcean::File.new(file_params)
       if @file.file_template_id

app/controllers/submissions_controller.rb

@@ -7,22 +7,17 @@ class SubmissionsController < ApplicationController
   include SubmissionParameters
   include Tubesock::Hijack
 
-  before_action :require_user!
+  before_action :set_submission, only: %i[download download_file run score show statistics test]
-  before_action :set_submission, only: %i[download download_file render_file run score show statistics test]
   before_action :set_testrun, only: %i[run score test]
   before_action :set_files, only: %i[download show]
-  before_action :set_files_and_specific_file, only: %i[download_file render_file run test]
+  before_action :set_files_and_specific_file, only: %i[download_file run test]
   before_action :set_content_type_nosniff, only: %i[download download_file render_file]
 
-  # Overwrite the CSP header for the :render_file action
+  # Overwrite the CSP header and some default actions for the :render_file action
-  content_security_policy only: :render_file do |policy|
+  content_security_policy false, only: :render_file
-    policy.img_src        :none
+  skip_before_action :deny_access_from_render_host, only: :render_file
-    policy.script_src     :none
+  skip_before_action :verify_authenticity_token, only: :render_file
-    policy.font_src       :none
+  before_action :require_user!, except: :render_file
-    policy.style_src      :none
-    policy.connect_src    :none
-    policy.form_action    :none
-  end
 
   def create
     @submission = Submission.new(submission_params)
@@ -80,10 +75,28 @@ class SubmissionsController < ApplicationController
   end
 
   def render_file
-    # If a file should not be downloaded, it should not be rendered either
+    # Set @current_user with a new *learner* for Pundit checks
-    raise Pundit::NotAuthorizedError if @embed_options[:disable_download]
+    @current_user = ExternalUser.new
 
-    send_data(@file.read, filename: @file.name_with_extension, disposition: 'inline')
+    @submission = authorize AuthenticatedUrlHelper.retrieve!(Submission, request, cookies)
+
+    # Throws an exception if the file is not found
+    set_files_and_specific_file
+
+    # Allows access to other files of the same submission, e.g., a linked JS or CSS file where we cannot expect a token in the URL
+    cookie_name = AuthenticatedUrlHelper.cookie_name_for(:render_file_token)
+    if params[AuthenticatedUrlHelper.query_parameter].present?
+      cookies[cookie_name] = AuthenticatedUrlHelper.prepare_short_living_cookie(request.url)
+      cookies.commit!
+    end
+
+    # Finally grant access and send the file
+    if @file.native_file?
+      url = render_protected_upload_url(id: @file.id, filename: @file.name_with_extension)
+      redirect_to AuthenticatedUrlHelper.sign(url, @file)
+    else
+      send_data(@file.content, filename: @file.name_with_extension, disposition: 'inline')
+    end
   end
 
   # rubocop:disable Metrics/CyclomaticComplexity
@@ -372,7 +385,7 @@ class SubmissionsController < ApplicationController
     # @file contains the specific file requested for run / test / render / ...
     set_files
     @file = @files.detect {|file| file.filepath == sanitize_filename }
-    head :not_found unless @file
+    raise ActiveRecord::RecordNotFound unless @file
   end
 
   def set_files

app/helpers/authenticated_url_helper.rb

@@ -0,0 +1,111 @@
+# frozen_string_literal: true
+
+module AuthenticatedUrlHelper
+  include Pundit::Authorization
+
+  class << self
+    TOKEN_ALGORITHM = 'HS512'
+    TOKEN_EXPIRATION = 10.minutes
+    TOKEN_SECRET = Rails.application.secrets.secret_key_base
+    TOKEN_PARAM = :token
+    COOKIE_EXPIRATION = 30.seconds
+
+    def sign(url, object)
+      payload = {object_id: object.id, object_type: object.class.name, url: url, exp: TOKEN_EXPIRATION.from_now.to_i}
+      token = JWT.encode payload, TOKEN_SECRET, TOKEN_ALGORITHM
+
+      add_query_parameters(url, {TOKEN_PARAM => token})
+    end
+
+    def retrieve!(klass, request, cookies = {})
+      # Don't use the default session mechanism and default cookie
+      request.session_options[:skip] = true
+      # Show errors as JSON format, if any
+      request.format = :json
+
+      # Disallow access from normal domain and show an error instead
+      if ApplicationController::RENDER_HOST.present? && request.host != ApplicationController::RENDER_HOST
+        raise Pundit::NotAuthorizedError
+      end
+
+      cookie_name = AuthenticatedUrlHelper.cookie_name_for(:render_file_token)
+      object = klass.find(request.parameters[:id])
+
+      signed_url = request.parameters[TOKEN_PARAM].present? ? request.url : cookies[cookie_name]
+      # Throws an exception if the token is not matching the object or has expired
+      AuthenticatedUrlHelper.verify!(signed_url, object, klass)
+
+      object
+    end
+
+    def verify!(url, object, klass)
+      original_url, removed_parameters = remove_query_parameters(url, [TOKEN_PARAM])
+      expected_payload = {object_id: object.id, object_type: klass.name, url: original_url}
+      token = removed_parameters[TOKEN_PARAM]
+
+      begin
+        payload, = JWT.decode token, TOKEN_SECRET, true, algorithm: TOKEN_ALGORITHM
+      rescue JWT::DecodeError
+        raise Pundit::NotAuthorizedError
+      end
+
+      raise Pundit::NotAuthorizedError unless payload.symbolize_keys.except(:exp) == expected_payload
+    end
+
+    def prepare_short_living_cookie(value)
+      {
+        value: value,
+        expires: COOKIE_EXPIRATION.from_now,
+        httponly: true,
+        same_site: :strict,
+        secure: Rails.env.production? || Rails.env.staging?,
+        path: Rails.application.config.relative_url_root,
+      }
+    end
+
+    def cookie_name_for(name)
+      if (Rails.env.production? || Rails.env.staging?) \
+        && Rails.application.config.relative_url_root == '/'
+        "__Host-#{name}"
+      elsif Rails.env.production? || Rails.env.staging?
+        "__Secure-#{name}"
+      else
+        name
+      end
+    end
+
+    def query_parameter
+      TOKEN_PARAM
+    end
+
+    private
+
+    def add_query_parameters(url, parameters)
+      parsed_url = URI.parse url
+
+      # Add the given parameters to the query string
+      query_params = CGI.parse(parsed_url.query || '')
+      query_params.merge!(parameters)
+
+      # Add the query string back to the URL
+      parsed_url.query = URI.encode_www_form(query_params)
+
+      # Return the full URL
+      parsed_url.to_s
+    end
+
+    def remove_query_parameters(url, parameters)
+      parsed_url = URI.parse url
+
+      # Remove the given parameters from the query string
+      query_params = Rack::Utils.parse_nested_query(parsed_url.query || '')
+      removed_params = query_params.slice!(parameters)
+
+      # Add the query string back to the URL
+      parsed_url.query = URI.encode_www_form(query_params).presence
+
+      # Return the full URL and removed parameters
+      [parsed_url.to_s, removed_params.symbolize_keys]
+    end
+  end
+end

app/policies/code_ocean/file_policy.rb

@@ -26,6 +26,14 @@ module CodeOcean
       end
     end
 
+    def render_protected_upload?
+      return no_one if @record.native_file? && !@record.native_file_location_valid?
+      return no_one if @record.context.is_a?(Exercise) && (@record.context.unpublished || @record.hidden)
+
+      # The AuthenticatedUrlHelper will check for more details, but we cannot determine a specific user
+      everyone
+    end
+
     def create?
       if @record.context.is_a?(Exercise)
         admin? || author?

app/policies/submission_policy.rb

@@ -6,11 +6,15 @@ class SubmissionPolicy < ApplicationPolicy
   end
 
   # insights? is used in the flowr_controller.rb as we use it to authorize the user for a submission
-  %i[download? download_file? render_file? run? score? show? statistics? stop? test?
+  %i[download? download_file? run? score? show? statistics? stop? test?
      insights?].each do |action|
     define_method(action) { admin? || author? }
   end
 
+  def render_file?
+    everyone
+  end
+
   def index?
     admin?
   end

app/views/submissions/show.json.jbuilder

@@ -5,6 +5,14 @@ json.download_url download_submission_path(@submission, format: :json)
 json.score_url score_submission_path(@submission, format: :json)
 json.download_file_url download_file_submission_path(@submission, 'a.', format: :json).gsub(/a\.\.json$/,
   '{filename}.json')
-json.render_url render_submission_path(@submission, 'a.', format: :json).gsub(/a\.\.json$/, '{filename}.json')
+unless @embed_options[:disable_download]
+  json.render_url @submission.collect_files.select(&:visible) do |files|
+    host = ApplicationController::RENDER_HOST || request.host
+    url = render_submission_url(@submission, files.filepath, host: host)
+
+    json.filepath files.filepath
+    json.url AuthenticatedUrlHelper.sign(url, @submission)
+  end
+end
 json.run_url run_submission_path(@submission, 'a.', format: :json).gsub(/a\.\.json$/, '{filename}.json')
 json.test_url test_submission_path(@submission, 'a.', format: :json).gsub(/a\.\.json$/, '{filename}.json')

config/action_mailer.yml.ci

@@ -3,5 +3,4 @@ test:
     from: codeocean@hpi.de
   default_url_options:
     host: localhost
-    port: 3000
   delivery_method: :test

config/action_mailer.yml.example

@@ -19,5 +19,4 @@ test:
   <<: *default
   default_url_options:
     host: localhost
-    port: 3000
   delivery_method: :test

config/code_ocean.yml.example

@@ -1,4 +1,8 @@
 default: &default
+  # A public-facing host to be used for the render_file function of the SubmissionsController.
+  # User content will be served from this host. If not set, the default host is used (less secure!).
+  # render_host: codeoceanusercontent.com
+
   flowr:
     # When enabled, flowr can assist learners with related search results from
     # StackOverflow.com regarding exceptions that occurred during code execution.

config/initializers/content_security_policy.rb

@@ -38,6 +38,7 @@ Rails.application.config.content_security_policy do |policy|
   # Code executions might return a base64 encoded image as a :data URI
   policy.img_src              :self, :data
   policy.object_src           :none
+  policy.media_src            :self
   policy.script_src           :self, :report_sample
   # Our ACE editor unfortunately requires :unsafe_inline for the code highlighting
   policy.style_src            :self, :unsafe_inline, :report_sample

config/routes.rb

@@ -129,6 +129,7 @@ Rails.application.routes.draw do
     resources :files, only: %i[create destroy]
   end
   get '/uploads/files/:id/:filename', to: 'code_ocean/files#show_protected_upload', as: :protected_upload, constraints: {filename: FILENAME_REGEXP}
+  get '/uploads/render_files/:id/:filename', to: 'code_ocean/files#render_protected_upload', as: :render_protected_upload, constraints: {filename: FILENAME_REGEXP}
 
   resources :file_types
 

spec/controllers/submissions_controller_spec.rb

@@ -40,7 +40,7 @@ describe SubmissionsController do
 
   describe 'GET #download_file' do
     context 'with an invalid filename' do
-      before { get :download_file, params: {filename: SecureRandom.hex, id: submission.id} }
+      before { get :download_file, params: {filename: SecureRandom.hex, id: submission.id, format: :json} }
 
       expect_http_status(:not_found)
     end
@@ -108,28 +108,32 @@ describe SubmissionsController do
 
   describe 'GET #render_file' do
     let(:file) { submission.files.first }
+    let(:signed_url) { AuthenticatedUrlHelper.sign(render_submission_url(submission, filename), submission) }
+    let(:token) { Rack::Utils.parse_nested_query(URI.parse(signed_url).query)['token'] }
 
     context 'with an invalid filename' do
-      before { get :render_file, params: {filename: SecureRandom.hex, id: submission.id} }
+      let(:filename) { SecureRandom.hex }
+
+      before { get :render_file, params: {filename: filename, id: submission.id, token: token} }
 
       expect_http_status(:not_found)
     end
 
     context 'with a valid filename' do
       let(:submission) { create(:submission, exercise: create(:audio_video)) }
+      let(:filename) { file.name_with_extension }
 
-      before { get :render_file, params: {filename: file.name_with_extension, id: submission.id} }
+      before { get :render_file, params: {filename: filename, id: submission.id, token: token} }
 
       context 'with a binary file' do
         let(:file) { submission.collect_files.detect {|file| file.file_type.file_extension == '.mp4' } }
+        let(:signed_url_video) { AuthenticatedUrlHelper.sign(render_protected_upload_url(id: file, filename: file.name_with_extension), file) }
 
         expect_assigns(file: :file)
         expect_assigns(submission: :submission)
-        expect_content_type('video/mp4')
-        expect_http_status(:ok)
 
-        it 'renders the file content' do
+        it 'sets the correct redirect' do
-          expect(response.body).to eq(file.read)
+          expect(response.location).to eq signed_url_video
         end
       end
 
@@ -184,7 +188,7 @@ describe SubmissionsController do
     expect_assigns(submission: :submission)
     expect_http_status(:ok)
 
-    %i[render run test].each do |action|
+    %i[run test].each do |action|
       describe "##{action}_url" do
         let(:url) { JSON.parse(response.body).with_indifferent_access.fetch("#{action}_url") }
 
@@ -199,6 +203,20 @@ describe SubmissionsController do
       end
     end
 
+    describe '#render_url' do
+      let(:supported_urls) { JSON.parse(response.body).with_indifferent_access.fetch('render_url') }
+      let(:file) { submission.collect_files.detect(&:main_file?) }
+      let(:url) { supported_urls.find {|hash| hash[:filepath] == file.filepath }['url'] }
+
+      it 'starts like the render path' do
+        expect(url).to start_with(Rails.application.routes.url_helpers.render_submission_url(submission, file.filepath, host: request.host))
+      end
+
+      it 'includes a token' do
+        expect(url).to include '?token='
+      end
+    end
+
     describe '#score_url' do
       let(:url) { JSON.parse(response.body).with_indifferent_access.fetch('score_url') }