diff --git a/app/controllers/concerns/cross_origin_isolation.rb b/app/controllers/concerns/cross_origin_isolation.rb new file mode 100644 index 0000000000..02a43b4923 --- /dev/null +++ b/app/controllers/concerns/cross_origin_isolation.rb @@ -0,0 +1,30 @@ +# Opts an action in to cross-origin isolation, which is what unlocks +# SharedArrayBuffer (and the Atomics built on it) in the browser. +# +# Both headers are needed - the browser only flips crossOriginIsolated once it +# has seen COOP and COEP together. +# +# We use `credentialless` rather than `require-corp` for COEP because it fails +# gracefully: a cross-origin subresource without Cross-Origin-Resource-Policy +# is fetched without credentials rather than being blocked outright. Under +# `require-corp` every image in a user's markdown, every third-party script, +# and every stylesheet from the assets host would have to carry CORP or be +# CORS-fetched, and anything that didn't would simply not load. +# +# Cross-origin *iframes* are strict under both values though: the embedded +# document must send its own COEP, or the element must carry the +# `credentialless` attribute. See VimeoEmbed.tsx. +# +# IMPORTANT: these headers only do anything on a real browser navigation. A +# Turbo visit swaps the body of the existing document, so the document keeps +# whatever isolation it was loaded with. Any action using this must therefore +# also opt out of Turbo - see Tracks::ExercisesController. +module CrossOriginIsolation + extend ActiveSupport::Concern + + private + def cross_origin_isolate! + response.set_header("Cross-Origin-Opener-Policy", "same-origin") + response.set_header("Cross-Origin-Embedder-Policy", "credentialless") + end +end diff --git a/app/controllers/tracks/exercises_controller.rb b/app/controllers/tracks/exercises_controller.rb index a1276f3a37..34e9e5bc30 100644 --- a/app/controllers/tracks/exercises_controller.rb +++ b/app/controllers/tracks/exercises_controller.rb @@ -1,11 +1,21 @@ class Tracks::ExercisesController < ApplicationController include UseTrackExerciseSolutionConcern + include CrossOriginIsolation + + # The editor needs cross-origin isolation for SharedArrayBuffer, and that only + # applies to a real browser navigation. Rendering the full layout (rather than + # the turbo_frame one) means a frame request receives a whole document, whose + # turbo-visit-control meta tag then tells Turbo to reload the page properly. + # See the meta tag in edit.html.haml. + layout -> { "turbo_frame" if turbo_frame_request? && action_name != "edit" } + before_action :use_track! before_action :use_exercise!, only: %i[show start edit complete tooltip no_test_runner] before_action :use_solution, only: %i[show edit complete tooltip] before_action :cache_public_action!, only: %i[tooltip] before_action :cache_show_action!, only: %i[show] before_action :cache_index_action!, only: %i[index] + before_action :cross_origin_isolate!, only: %i[edit] skip_before_action :authenticate_user!, only: %i[index show tooltip] skip_before_action :rate_limit_for_user!, only: %i[tooltip] # Scanning over this is a lot diff --git a/app/javascript/components/common/VimeoEmbed.tsx b/app/javascript/components/common/VimeoEmbed.tsx index ba3ab3b5c9..aa75de07b5 100644 --- a/app/javascript/components/common/VimeoEmbed.tsx +++ b/app/javascript/components/common/VimeoEmbed.tsx @@ -16,6 +16,10 @@ export default function VimeoEmbed({ width="560" height="315" src={src} + // Vimeo doesn't send COEP, so on a cross-origin isolated page (the + // editor) this iframe would be blocked outright without this. + // Not in @types/react yet, hence the cast. + {...({ credentialless: '' } as Record)} frameBorder="0" allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture" allowFullScreen diff --git a/app/javascript/components/student/OpenEditorButton.tsx b/app/javascript/components/student/OpenEditorButton.tsx index 63a2793159..c09cf9eb42 100644 --- a/app/javascript/components/student/OpenEditorButton.tsx +++ b/app/javascript/components/student/OpenEditorButton.tsx @@ -79,7 +79,11 @@ const Button = (props: Props & { className?: string }) => { case 'published': case 'completed': return props.editorEnabled ? ( - + {t('openEditor.openInEditor')} ) : ( @@ -89,7 +93,11 @@ const Button = (props: Props & { className?: string }) => { ) default: return props.editorEnabled ? ( - + {t('openEditor.continueInEditor')} ) : ( diff --git a/app/views/tracks/exercises/edit.html.haml b/app/views/tracks/exercises/edit.html.haml index 24d6442218..36acff798b 100644 --- a/app/views/tracks/exercises/edit.html.haml +++ b/app/views/tracks/exercises/edit.html.haml @@ -1,3 +1,12 @@ +-# + The editor is cross-origin isolated (see CrossOriginIsolation) so that + SharedArrayBuffer is available. Isolation is a property of the document, so it + is only granted on a real browser navigation - a Turbo visit would swap the + body of a non-isolated document and silently lose it. This tells Turbo to do a + full page load whenever it would otherwise render this page itself. +- content_for :meta_tags do + %meta{ name: "turbo-visit-control", content: "reload" } + - require_stylesheet "editor" - require_stylesheet "track" diff --git a/app/views/tracks/exercises/show/_action_box_solve.html.haml b/app/views/tracks/exercises/show/_action_box_solve.html.haml index c3e532fbfa..3bc31cdf8f 100644 --- a/app/views/tracks/exercises/show/_action_box_solve.html.haml +++ b/app/views/tracks/exercises/show/_action_box_solve.html.haml @@ -13,7 +13,8 @@ - if exercise.has_test_runner? %h4= t('.via_editor_heading') - = link_to edit_track_exercise_path(track, exercise), class: "editor-btn btn-primary btn-m" do + -# data-turbo: false so the editor gets a real navigation (see CrossOriginIsolation) + = link_to edit_track_exercise_path(track, exercise), class: "editor-btn btn-primary btn-m", data: { turbo: false } do = graphical_icon :editor = t('.start_in_editor_button') diff --git a/app/views/tracks/iterations/index.html.haml b/app/views/tracks/iterations/index.html.haml index 63a682f696..93014ffa18 100644 --- a/app/views/tracks/iterations/index.html.haml +++ b/app/views/tracks/iterations/index.html.haml @@ -17,7 +17,8 @@ - if @exercise.has_test_runner? .editor %h4= t('.zero_state.editor_heading') - = link_to edit_track_exercise_path(@track, @exercise), class: 'editor-btn btn-primary btn-m' do + -# data-turbo: false so the editor gets a real navigation (see CrossOriginIsolation) + = link_to edit_track_exercise_path(@track, @exercise), class: 'editor-btn btn-primary btn-m', data: { turbo: false } do = graphical_icon "editor" %span= t('.zero_state.editor_button') diff --git a/test/controllers/tracks/exercises_controller_test.rb b/test/controllers/tracks/exercises_controller_test.rb index 2d8210a914..3e931e3021 100644 --- a/test/controllers/tracks/exercises_controller_test.rb +++ b/test/controllers/tracks/exercises_controller_test.rb @@ -159,6 +159,62 @@ class Tracks::ExercisesControllerTest < ActionDispatch::IntegrationTest assert_template "tracks/exercises/edit" end + test "edit: is cross-origin isolated" do + user = create :user + track = create :track + exercise = create(:practice_exercise, track:, slug: "hello-world") + + create(:user_track, user:, track:) + create(:practice_solution, user:, exercise:) + + sign_in!(user) + + get edit_track_exercise_url(track, exercise) + + # SharedArrayBuffer needs both of these. See CrossOriginIsolation. + assert_equal "same-origin", response.headers["Cross-Origin-Opener-Policy"] + assert_equal "credentialless", response.headers["Cross-Origin-Embedder-Policy"] + end + + test "edit: tells turbo to do a full page load" do + user = create :user + track = create :track + exercise = create(:practice_exercise, track:, slug: "hello-world") + + create(:user_track, user:, track:) + create(:practice_solution, user:, exercise:) + + sign_in!(user) + + # Cross-origin isolation is a property of the document, so the editor is + # only isolated when the browser really navigates to it. Rendering into the + # turbo frame would silently lose it, so a frame request must still get a + # full document carrying the turbo-visit-control meta tag. + get edit_track_exercise_url(track, exercise), headers: { "Turbo-Frame" => "tf-main" } + + assert_response :success + + # A full document, not the bare turbo_frame layout... + assert_select "head" + # ...carrying the tag that makes Turbo hand over to a real page load. + assert_select "meta[name=?][content=?]", "turbo-visit-control", "reload" + end + + test "show: still renders into the turbo frame" do + user = create :user + track = create :track + create(:user_track, user:, track:) + exercise = create(:practice_exercise, track:) + + sign_in!(user) + + get track_exercise_url(track, exercise), headers: { "Turbo-Frame" => "tf-main" } + + assert_response :success + assert_select "turbo-frame#tf-main" + assert_select "head", count: 0 + end + test "edit: creates a solution if one is missing" do user = create :user track = create :track