From da1311e124fd7a517a0cd9043d97b26ab15aa372 Mon Sep 17 00:00:00 2001 From: Andrew Duthie <1779930+aduth@users.noreply.github.com> Date: Tue, 30 Jan 2024 12:36:32 -0500 Subject: [PATCH] Consolidate SAML/OIDC redirect JavaScript (#9985) * Consolidate SAML/OIDC redirect JavaScript changelog: Internal, Code Quality, Consolidate redirect logic for SAML/OIDC * Use classList#replace for no-js replacement * Use consistent click_immediate attribute value See: https://github.com/18F/identity-idp/pull/9985#discussion_r1468175139 * Remove unnecessary hidden field for tests See: https://github.com/18F/identity-idp/pull/9985#discussion_r1468176277 * Fix helper for OIDC redirect link * Improve spec helper resilience to HTML tag attributes --- app/javascript/packs/click-immediate.ts | 3 +++ app/javascript/packs/openid-connect-redirect.ts | 5 ----- app/javascript/packs/saml-post.js | 4 ---- app/views/layouts/base.html.erb | 2 +- .../openid_connect/shared/redirect_js.html.erb | 11 +++++++---- .../saml_idp/shared/saml_post_binding.html.erb | 14 ++++++++------ app/views/shared/saml_post_form.html.erb | 13 ++++++++----- spec/support/oidc_auth_helper.rb | 3 +-- spec/support/saml_response_doc.rb | 2 +- tsconfig.json | 2 +- 10 files changed, 30 insertions(+), 29 deletions(-) create mode 100644 app/javascript/packs/click-immediate.ts delete mode 100644 app/javascript/packs/openid-connect-redirect.ts delete mode 100644 app/javascript/packs/saml-post.js diff --git a/app/javascript/packs/click-immediate.ts b/app/javascript/packs/click-immediate.ts new file mode 100644 index 00000000000..adc6889ab67 --- /dev/null +++ b/app/javascript/packs/click-immediate.ts @@ -0,0 +1,3 @@ +document + .querySelectorAll('[data-click-immediate]') + .forEach((element) => element.click()); diff --git a/app/javascript/packs/openid-connect-redirect.ts b/app/javascript/packs/openid-connect-redirect.ts deleted file mode 100644 index 42be24f47da..00000000000 --- a/app/javascript/packs/openid-connect-redirect.ts +++ /dev/null @@ -1,5 +0,0 @@ -import { forceRedirect } from '@18f/identity-url'; - -document.body.classList.add('usa-sr-only'); -const link = document.querySelector('#openid-connect-redirect')!; -forceRedirect(link.href); diff --git a/app/javascript/packs/saml-post.js b/app/javascript/packs/saml-post.js deleted file mode 100644 index 6c807265d08..00000000000 --- a/app/javascript/packs/saml-post.js +++ /dev/null @@ -1,4 +0,0 @@ -document.addEventListener('DOMContentLoaded', function () { - document.body.className += ' usa-sr-only'; - document.getElementById('saml-post-binding').submit(); -}); diff --git a/app/views/layouts/base.html.erb b/app/views/layouts/base.html.erb index c6b5251951c..571d6cc8ef5 100644 --- a/app/views/layouts/base.html.erb +++ b/app/views/layouts/base.html.erb @@ -18,7 +18,7 @@ <%= title %> | <%= APP_NAME %> <%= javascript_tag(nonce: true) do %> - document.documentElement.className = document.documentElement.className.replace(/\bno-js\b/, 'js'); + document.documentElement.classList.replace('no-js', 'js'); <% end %> <%= preload_link_tag font_url('public-sans/PublicSans-Bold.woff2') %> <%= preload_link_tag font_url('public-sans/PublicSans-Regular.woff2') %> diff --git a/app/views/openid_connect/shared/redirect_js.html.erb b/app/views/openid_connect/shared/redirect_js.html.erb index 1ef3ce51ab3..79613be8c0b 100644 --- a/app/views/openid_connect/shared/redirect_js.html.erb +++ b/app/views/openid_connect/shared/redirect_js.html.erb @@ -1,13 +1,16 @@ - + <%= t('headings.redirecting') %> | <%= APP_NAME %> + <%= javascript_tag(nonce: true) do %> + document.documentElement.classList.replace('no-js', 'js'); + <% end %> <%= stylesheet_link_tag 'application', media: 'all' %> <%= render_stylesheet_once_tags %> -
+
<%= render PageHeadingComponent.new.with_content(t('saml_idp.shared.saml_post_binding.heading')) %> @@ -16,10 +19,10 @@ <%= t('saml_idp.shared.saml_post_binding.no_js') %>

- <%= link_to(t('forms.buttons.continue'), @oidc_redirect_uri, class: 'usa-button usa-button--wide usa-button--big', id: 'openid-connect-redirect') %> + <%= link_to(t('forms.buttons.continue'), @oidc_redirect_uri, class: 'usa-button usa-button--wide usa-button--big', data: { click_immediate: '' }) %>
- <%= render_javascript_pack_once_tags 'openid-connect-redirect' %> + <%= render_javascript_pack_once_tags 'click-immediate' %> diff --git a/app/views/saml_idp/shared/saml_post_binding.html.erb b/app/views/saml_idp/shared/saml_post_binding.html.erb index eb827e26bf8..ae607743875 100644 --- a/app/views/saml_idp/shared/saml_post_binding.html.erb +++ b/app/views/saml_idp/shared/saml_post_binding.html.erb @@ -1,15 +1,17 @@ - + <%= t('headings.redirecting') %> | <%= APP_NAME %> + <%= javascript_tag(nonce: true) do %> + document.documentElement.classList.replace('no-js', 'js'); + <% end %> <%= csrf_meta_tags %> <%= stylesheet_link_tag 'application', media: 'all' %> <%= render_stylesheet_once_tags %> - <%= render_javascript_pack_once_tags 'saml-post' %> -
+
<%= render PageHeadingComponent.new.with_content(t('.heading')) %> @@ -18,16 +20,16 @@ <%= t('.no_js') %>

- <%= form_tag action_url, id: 'saml-post-binding' do %> - <%= hidden_field_tag('csp_uris', csp_uris) if Rails.env.test? %> + <%= simple_form_for('', url: action_url) do |f| %> <%= hidden_field_tag(type, message) %> <% if params.key?(:RelayState) %> <%= hidden_field_tag('RelayState', params[:RelayState]) %> <% end %> - <%= submit_tag t('forms.buttons.submit.default'), class: 'usa-button usa-button--wide usa-button--big' %> + <%= f.submit t('forms.buttons.submit.default'), data: { click_immediate: '' } %> <% end %>
+ <%= render_javascript_pack_once_tags 'click-immediate' %> diff --git a/app/views/shared/saml_post_form.html.erb b/app/views/shared/saml_post_form.html.erb index d07998cc503..b305dff895d 100644 --- a/app/views/shared/saml_post_form.html.erb +++ b/app/views/shared/saml_post_form.html.erb @@ -1,14 +1,16 @@ - + + <%= javascript_tag(nonce: true) do %> + document.documentElement.classList.replace('no-js', 'js'); + <% end %> <%= csrf_meta_tags %> <%= stylesheet_link_tag 'application', media: 'all' %> <%= render_stylesheet_once_tags %> - <%= render_javascript_pack_once_tags 'saml-post' %> -
+
<%= render PageHeadingComponent.new.with_content(t('saml_idp.shared.saml_post_binding.heading')) %> @@ -17,14 +19,15 @@ <%= t('saml_idp.shared.saml_post_binding.no_js') %>

- <%= form_tag action_url, id: 'saml-post-binding' do %> + <%= simple_form_for('', url: action_url) do |f| %> <% form_params.each do |key, val| %> <%= hidden_field_tag(key, val) %> <% end %> - <%= submit_tag t('forms.buttons.submit.default'), class: 'usa-button usa-button--wide usa-button--big' %> + <%= f.submit t('forms.buttons.submit.default'), data: { click_immediate: '' } %> <% end %>
+ <%= render_javascript_pack_once_tags 'click-immediate' %> diff --git a/spec/support/oidc_auth_helper.rb b/spec/support/oidc_auth_helper.rb index 6613f936174..4e533751983 100644 --- a/spec/support/oidc_auth_helper.rb +++ b/spec/support/oidc_auth_helper.rb @@ -120,8 +120,7 @@ def extract_meta_refresh_url end def extract_redirect_url - content = page.find('a#openid-connect-redirect') - content[:href] + page.find_link(t('forms.buttons.continue'))[:href] end def oidc_redirect_url diff --git a/spec/support/saml_response_doc.rb b/spec/support/saml_response_doc.rb index c5018586c78..a45ebe25e21 100644 --- a/spec/support/saml_response_doc.rb +++ b/spec/support/saml_response_doc.rb @@ -33,7 +33,7 @@ def input_id def raw_xml_response if @test_type == 'feature' xml_response - elsif @response.body.include?('') + elsif @response.body.include?('