diff --git a/app/helpers/defer_script_tag_helper.rb b/app/helpers/defer_script_tag_helper.rb deleted file mode 100644 index be927c67aaa551d5a75ef5962a7d5cbedc70bc1f..0000000000000000000000000000000000000000 --- a/app/helpers/defer_script_tag_helper.rb +++ /dev/null @@ -1,10 +0,0 @@ -# frozen_string_literal: true - -module DeferScriptTagHelper - # Override the default ActionView `javascript_include_tag` helper to support page specific deferred loading. - # PLEASE NOTE: `defer` is also critical so that we don't run JavaScript entrypoints before the DOM is ready. - # Please see https://gitlab.com/groups/gitlab-org/-/epics/4538#note_432159769. - def javascript_include_tag(*sources) - super(*sources, defer: true) - end -end diff --git a/app/helpers/gitlab_script_tag_helper.rb b/app/helpers/gitlab_script_tag_helper.rb new file mode 100644 index 0000000000000000000000000000000000000000..467f3f7305b44b5f214067ea90b59946c83957b6 --- /dev/null +++ b/app/helpers/gitlab_script_tag_helper.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module GitlabScriptTagHelper + # Override the default ActionView `javascript_include_tag` helper to support page specific deferred loading. + # PLEASE NOTE: `defer` is also critical so that we don't run JavaScript entrypoints before the DOM is ready. + # Please see https://gitlab.com/groups/gitlab-org/-/epics/4538#note_432159769. + # The helper also makes sure the `nonce` attribute is included in every script when the content security + # policy is enabled. + def javascript_include_tag(*sources) + super(*sources, defer: true, nonce: true) + end + + # The helper makes sure the `nonce` attribute is included in every script when the content security + # policy is enabled. + def javascript_tag(content_or_options_with_block = nil, html_options = {}) + if content_or_options_with_block.is_a?(Hash) + content_or_options_with_block[:nonce] = true + else + html_options[:nonce] = true + end + + super + end +end diff --git a/app/views/layouts/_google_analytics.html.haml b/app/views/layouts/_google_analytics.html.haml index e8a5359e79126cadbb1340c8e407141af4d67f19..759e9ef36b944959af032beb9f66122647e06774 100644 --- a/app/views/layouts/_google_analytics.html.haml +++ b/app/views/layouts/_google_analytics.html.haml @@ -1,4 +1,4 @@ -= javascript_tag nonce: true do += javascript_tag do :plain var _gaq = _gaq || []; _gaq.push(['_setAccount', '#{extra_config.google_analytics_id}']); diff --git a/app/views/layouts/_google_tag_manager_head.html.haml b/app/views/layouts/_google_tag_manager_head.html.haml index ab03f1e76704a56406f98da86c8b067e1ed76ef2..48eb9e40cc49a6238f48ffc7bfa8b37d30d8c5ac 100644 --- a/app/views/layouts/_google_tag_manager_head.html.haml +++ b/app/views/layouts/_google_tag_manager_head.html.haml @@ -1,5 +1,5 @@ - if google_tag_manager_enabled? - = javascript_tag nonce: true do + = javascript_tag do :plain (function(w,d,s,l,i){w[l]=w[l]||[];w[l].push({'gtm.start': new Date().getTime(),event:'gtm.js'});var f=d.getElementsByTagName(s)[0], diff --git a/app/views/layouts/_img_loader.html.haml b/app/views/layouts/_img_loader.html.haml index cddcd6e0af6ee024066d4a3ef7332533c00e64c6..f6d7d163e6f9bc91774c0c61fd164c8729c52224 100644 --- a/app/views/layouts/_img_loader.html.haml +++ b/app/views/layouts/_img_loader.html.haml @@ -1,4 +1,4 @@ -= javascript_tag nonce: true do += javascript_tag do :plain if ('loading' in HTMLImageElement.prototype) { document.querySelectorAll('img.lazy').forEach(img => { diff --git a/app/views/layouts/_init_auto_complete.html.haml b/app/views/layouts/_init_auto_complete.html.haml index 82ec92988eb4fcfcc34828a2d1d0c26ca6d22b91..509f5be809787de7906a6ce0bffb9828d8bd14c7 100644 --- a/app/views/layouts/_init_auto_complete.html.haml +++ b/app/views/layouts/_init_auto_complete.html.haml @@ -4,7 +4,7 @@ - datasources = autocomplete_data_sources(object, noteable_type) - if object - = javascript_tag nonce: true do + = javascript_tag do :plain gl = window.gl || {}; gl.GfmAutoComplete = gl.GfmAutoComplete || {}; diff --git a/app/views/layouts/_init_client_detection_flags.html.haml b/app/views/layouts/_init_client_detection_flags.html.haml index 6537b86085fb6aa07a54003ba30bf3b04599f593..03967bbbfcf91f5d1ec9fb165b8466f3fee1a0f8 100644 --- a/app/views/layouts/_init_client_detection_flags.html.haml +++ b/app/views/layouts/_init_client_detection_flags.html.haml @@ -1,7 +1,7 @@ - client = client_js_flags - if client - = javascript_tag nonce: true do + = javascript_tag do :plain gl = window.gl || {}; gl.client = #{client.to_json}; diff --git a/app/views/layouts/_matomo.html.haml b/app/views/layouts/_matomo.html.haml index 1297d120683473c930aa847add490c379039e5cc..fcd3156a1628b8d1e639a503f05c4fccd2936bf8 100644 --- a/app/views/layouts/_matomo.html.haml +++ b/app/views/layouts/_matomo.html.haml @@ -1,5 +1,5 @@ -= javascript_tag nonce: true do += javascript_tag do :plain var _paq = window._paq = window._paq || []; _paq.push(['trackPageView']); diff --git a/app/views/layouts/_snowplow.html.haml b/app/views/layouts/_snowplow.html.haml index d7ff5ad109411964e57cc4c988bbe11d8a1c1e56..9d14dfb378605a8267f67de929778d54ef893733 100644 --- a/app/views/layouts/_snowplow.html.haml +++ b/app/views/layouts/_snowplow.html.haml @@ -1,6 +1,6 @@ - return unless Gitlab::CurrentSettings.snowplow_enabled? -= javascript_tag nonce: true do += javascript_tag do :plain ;(function(p,l,o,w,i,n,g){if(!p[i]){p.GlobalSnowplowNamespace=p.GlobalSnowplowNamespace||[]; p.GlobalSnowplowNamespace.push(i);p[i]=function(){(p[i].q=p[i].q||[]).push(arguments) diff --git a/app/views/layouts/_startup_css_activation.haml b/app/views/layouts/_startup_css_activation.haml index a426d686c34633397407a3bca31e8240f1458895..5fb53385acc9b7c84a014f9c801777eb00a2adc9 100644 --- a/app/views/layouts/_startup_css_activation.haml +++ b/app/views/layouts/_startup_css_activation.haml @@ -1,6 +1,6 @@ - return unless use_startup_css? -= javascript_tag nonce: true do += javascript_tag do :plain document.querySelectorAll('link[media="print"]').forEach(linkTag => { linkTag.setAttribute('data-startupcss', 'loading'); diff --git a/app/views/layouts/_startup_js.html.haml b/app/views/layouts/_startup_js.html.haml index 9c488e4f40d0a1a1c2be75ed4bfa43a860362ee9..35cd191c60040783c3eeb789c84266d40cb93d89 100644 --- a/app/views/layouts/_startup_js.html.haml +++ b/app/views/layouts/_startup_js.html.haml @@ -1,6 +1,6 @@ - return unless page_startup_api_calls.present? || page_startup_graphql_calls.present? -= javascript_tag nonce: true do += javascript_tag do :plain var gl = window.gl || {}; gl.startup_calls = #{page_startup_api_calls.to_json}; diff --git a/app/views/layouts/errors.html.haml b/app/views/layouts/errors.html.haml index dc924a0e25dc2518bc10acee76731a10f5aff1f7..25fe4c898caa9bd6c26d529acd74f2fdada664ae 100644 --- a/app/views/layouts/errors.html.haml +++ b/app/views/layouts/errors.html.haml @@ -8,7 +8,7 @@ %body .page-container = yield - = javascript_tag nonce: true do + = javascript_tag do :plain (function(){ var goBackElement = document.querySelector('.js-go-back'); diff --git a/app/views/layouts/group.html.haml b/app/views/layouts/group.html.haml index 6d2c5870e4367eb8f1cf6ea908dddf2d110f5bdb..58fed89dfe768b8b1e7bd293a8cd6aab7061a175 100644 --- a/app/views/layouts/group.html.haml +++ b/app/views/layouts/group.html.haml @@ -8,7 +8,7 @@ - content_for :page_specific_javascripts do - if current_user - = javascript_tag nonce: true do + = javascript_tag do :plain window.uploads_path = "#{group_uploads_path(@group)}"; diff --git a/app/views/layouts/project.html.haml b/app/views/layouts/project.html.haml index 62e5431e2902e6bb11dd3a038e347bd5f71ccc5b..2df502d28991530a87e926ca730cabed722a91e3 100644 --- a/app/views/layouts/project.html.haml +++ b/app/views/layouts/project.html.haml @@ -10,7 +10,7 @@ - content_for :project_javascripts do - project = @target_project || @project - if current_user - = javascript_tag nonce: true do + = javascript_tag do :plain window.uploads_path = "#{project_uploads_path(project)}"; diff --git a/app/views/layouts/snippets.html.haml b/app/views/layouts/snippets.html.haml index 6cc53ba3342a7e7ed507a5416998fde8187fe3c0..54b5ec85cccff7979d31c0cd4a207e499666aaf1 100644 --- a/app/views/layouts/snippets.html.haml +++ b/app/views/layouts/snippets.html.haml @@ -4,7 +4,7 @@ - content_for :page_specific_javascripts do - if snippets_upload_path - = javascript_tag nonce: true do + = javascript_tag do :plain window.uploads_path = "#{snippets_upload_path}"; diff --git a/app/views/projects/merge_requests/_widget.html.haml b/app/views/projects/merge_requests/_widget.html.haml index 9736071b03f34285d733156b039b5606af1e600b..123affeb5d694b3b1c9de10d4add5bde5b86d412 100644 --- a/app/views/projects/merge_requests/_widget.html.haml +++ b/app/views/projects/merge_requests/_widget.html.haml @@ -1,4 +1,4 @@ -= javascript_tag nonce: true do += javascript_tag do :plain window.gl = window.gl || {}; window.gl.mrWidgetData = #{serialize_issuable(@merge_request, serializer: 'widget', issues_links: true)} diff --git a/ee/app/views/projects/merge_requests/show.html.haml b/ee/app/views/projects/merge_requests/show.html.haml index 54683c2282d7111c063c7f6de906ce9c5ead3115..a1e8a9f45f6a129a505d58be7a7ce62919fec9b6 100644 --- a/ee/app/views/projects/merge_requests/show.html.haml +++ b/ee/app/views/projects/merge_requests/show.html.haml @@ -1,6 +1,6 @@ = render_ce "projects/merge_requests/show" -= javascript_tag nonce: true do += javascript_tag do :plain // Append static, server-generated data not included in merge request entity (EE-Only) // Object.assign would be useful here, but it blows up Phantom.js in tests diff --git a/spec/helpers/defer_script_tag_helper_spec.rb b/spec/helpers/defer_script_tag_helper_spec.rb deleted file mode 100644 index 14317e353ab5fef8c21a087873633d004193cefe..0000000000000000000000000000000000000000 --- a/spec/helpers/defer_script_tag_helper_spec.rb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe DeferScriptTagHelper do - describe 'script tag' do - script_url = 'test.js' - - it 'returns an script tag with defer=true' do - expect(javascript_include_tag(script_url).to_s) - .to eq "" - end - end -end diff --git a/spec/helpers/gitlab_script_tag_helper_spec.rb b/spec/helpers/gitlab_script_tag_helper_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..37413b9b1c273a25386aaa8ec074d9c383bba9c0 --- /dev/null +++ b/spec/helpers/gitlab_script_tag_helper_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GitlabScriptTagHelper do + before do + allow(helper).to receive(:content_security_policy_nonce).and_return('noncevalue') + end + + describe 'external script tag' do + let(:script_url) { 'test.js' } + + it 'returns a script tag with defer=true and a nonce' do + expect(helper.javascript_include_tag(script_url).to_s) + .to eq "" + end + end + + describe 'inline script tag' do + let(:tag_with_nonce) {""} + let(:tag_with_nonce_and_type) {""} + + it 'returns a script tag with a nonce using block syntax' do + expect(helper.javascript_tag { 'alert(1)' }.to_s).to eq tag_with_nonce + end + + it 'returns a script tag with a nonce using block syntax with options' do + expect(helper.javascript_tag(type: 'application/javascript') { 'alert(1)' }.to_s).to eq tag_with_nonce_and_type + end + + it 'returns a script tag with a nonce using argument syntax' do + expect(helper.javascript_tag('alert(1)').to_s).to eq tag_with_nonce + end + + it 'returns a script tag with a nonce using argument syntax with options' do + expect(helper.javascript_tag( 'alert(1)', type: 'application/javascript').to_s).to eq tag_with_nonce_and_type + end + + # This scenario does not really make sense, but it's supported so we test it + it 'returns a script tag with a nonce using argument and block syntax with options' do + expect(helper.javascript_tag( '// ignored', type: 'application/javascript') { 'alert(1)' }.to_s).to eq tag_with_nonce_and_type + end + end +end