Various refinements (#1260)

- make cookie expiration 2 days - consistent backlinks helper used in layouts - GovOne docs and Yard updates - remove duplication and leftovers in controllers and routes - update locales - add spec around re-caching on timeout - change interruption page layout - Convert error pages to CMS resources - Remove early_years_emails code - Lint / fix / drop field - Brakeman security issues for response/feedback strong params - yarn upgrade - npm package upgrade - retry after CMS error - refactor answering logic into concern - widen interruption page - clean layouts into partials - Use column names as headers for feedback data exports - Remove public beta redirect
DFE-Digital · Jul 15, 2024 · eb5b739 · eb5b739
1 parent f552dbf
commit eb5b739
Show file tree

Hide file tree

Showing 94 changed files with 2,033 additions and 2,107 deletions.
diff --git a/.dockerignore b/.dockerignore
@@ -3,7 +3,7 @@
 .github
 .yardoc
 .yarn
-!.yarn/releases/yarn-3.2.1.cjs
+!.yarn/releases/yarn-4.0.2.cjs
 adr
 app/assets/builds
 config/*.key

diff --git a/.yarn/releases/yarn-3.2.1.cjs b/.yarn/releases/yarn-3.2.1.cjs
diff --git a/.yarn/releases/yarn-4.0.2.cjs b/.yarn/releases/yarn-4.0.2.cjs
diff --git a/.yarnrc.yml b/.yarnrc.yml
@@ -1,3 +1,7 @@
+compressionLevel: mixed
+
+enableGlobalCache: false
+
 enableTelemetry: 0
 
 nodeLinker: node-modules
@@ -13,4 +17,4 @@ supportedArchitectures:
     - darwin
     - linux
 
-yarnPath: .yarn/releases/yarn-3.2.1.cjs
+yarnPath: .yarn/releases/yarn-4.0.2.cjs
diff --git a/GOV_ONE.md b/GOV_ONE.md
@@ -1,6 +1,12 @@
 # GOV.UK One Login
 
+- [Service page](https://www.sign-in.service.gov.uk)
+- [Status updates](https://status.account.gov.uk)
+- [Admin tool](https://admin.sign-in.service.gov.uk/sign-in)
+- [Prototype]()
+
 ## Integration Environment
+
 - __Base URI__: https://oidc.integration.account.gov.uk
 - __Redirect URLs__:
     - http://localhost:3000/users/auth/openid_connect/callback
@@ -11,44 +17,50 @@
 ## Technical Checklist
 
 ### Authentication request requirements
-| Requirement | Response |
-| --- | --- |
-| __Describe how you’re using the state parameter to prevent CSRF attacks__ | The state is generated as a random uuid, which is then stored in the Rails session storage. The state parameter for authorisation request responses must match the state stored in the session before the user is authenticated. 
-| __Describe how you’re generating the nonce parameter__ | The nonce is a randomly generated alphanumeric string of 25 characters, it is used to verify the `id_token`. |
-| __Describe how you handle authorise endpoint errors__  | The errors are logged and the user is redirected to the homepage with an alert informing them of a problem  |
-| __Describe how you’re handling access_denied errors where session state is also missing__ | The user is redirected to the homepage with an alert informing them of a problem and encouraging them to try again |
+
+| Requirement                                                                               | Response                                                                                                                                                                                                                         |
+| ---                                                                                       | ---                                                                                                                                                                                                                              |
+| __Describe how you’re using the state parameter to prevent CSRF attacks__                 | The state is generated as a random uuid, which is then stored in the Rails session storage. The state parameter for authorisation request responses must match the state stored in the session before the user is authenticated. |
+| __Describe how you’re generating the nonce parameter__                                    | The nonce is a randomly generated alphanumeric string of 25 characters, it is used to verify the `id_token`.                                                                                                                     |
+| __Describe how you handle authorise endpoint errors__                                     | The errors are logged and the user is redirected to the homepage with an alert informing them of a problem                                                                                                                       |
+| __Describe how you’re handling access_denied errors where session state is also missing__ | The user is redirected to the homepage with an alert informing them of a problem and encouraging them to try again                                                                                                               |
 
 ### Token request requirements 
-| Requirement | Response |
-| --- | --- |
+
+| Requirement                                                                                              | Response                                            |
+| ---                                                                                                      | ---                                                 |
 | __Describe how you ensure that your client secret / private key is not exposed to unauthorised parties__ | These are encrypted and stored in Rails credentials |
-| __For the private_key_jwt confirm that each jti claim value in the JWT assertion is used once.__ | &check; |
+| __For the private_key_jwt confirm that each jti claim value in the JWT assertion is used once.__         | &check;                                             |
 
 ### Token validation requirements 
-| Requirement | Response |
-| --- | --- |
-| __Confirm you validate the iss claim is https://oidc.account.gov.uk/__ | &check; |
-| __Confirm you validate the aud claim matches your client_id__ | &check; |
-| __Confirm you validate the nonce claim matches the your application generated__ | &check; |
-| __Confirm you validate the current time is before the time in the exp claim__  | &check; |
-| __Confirm you validate the current time is between the time in the auth_time claim and the exp claim__ | &check; |
-| __Confirm you validate the signature on the id-token__ | &check; |
-| __Describe how you handle token endpoint errors__ | The error is logged and the user is redirected to the homepage with an alert informing them of a problem  |
-| __Describe how you ensure that the GOV.UK One Login Access Token is not exposed to unauthorised parties outside of your trusted backend server__  | The access token is not exposed to the user and is only used to make requests to the UserInfo endpoint during the user session. Communication with GOV.UK One Login is over HTTPS. |
-
-### UserInfo request requirements  
-| Requirement | Response |
-| --- | --- |
-| __Confirm you validate the sub claim in the UserInfo response matches the id-token sub claim__  | &check; |
-| __Describe how you handle UserInfo endpoint errors__ | The error is logged and the user is redirected to the homepage with an alert informing them of a problem |
-| __If you’re using the email address scope, confirm that you’re aware this represents the GOV.UK One Login username and may not be the user’s preferred contact email address__ | &check; |
+
+| Requirement                                                                                                                                      | Response                                                                                                                                                                           |
+| ---                                                                                                                                              | ---                                                                                                                                                                                |
+| __Confirm you validate the iss claim is https://oidc.account.gov.uk/__                                                                           | &check;                                                                                                                                                                            |
+| __Confirm you validate the aud claim matches your client_id__                                                                                    | &check;                                                                                                                                                                            |
+| __Confirm you validate the nonce claim matches the your application generated__                                                                  | &check;                                                                                                                                                                            |
+| __Confirm you validate the current time is before the time in the exp claim__                                                                    | &check;                                                                                                                                                                            |
+| __Confirm you validate the current time is between the time in the auth_time claim and the exp claim__                                           | &check;                                                                                                                                                                            |
+| __Confirm you validate the signature on the id-token__                                                                                           | &check;                                                                                                                                                                            |
+| __Describe how you handle token endpoint errors__                                                                                                | The error is logged and the user is redirected to the homepage with an alert informing them of a problem                                                                           |
+| __Describe how you ensure that the GOV.UK One Login Access Token is not exposed to unauthorised parties outside of your trusted backend server__ | The access token is not exposed to the user and is only used to make requests to the UserInfo endpoint during the user session. Communication with GOV.UK One Login is over HTTPS. |
+
+### UserInfo request requirements
+
+| Requirement                                                                                                                                                                    | Response                                                                                                 |
+| ---                                                                                                                                                                            | ---                                                                                                      |
+| __Confirm you validate the sub claim in the UserInfo response matches the id-token sub claim__                                                                                 | &check;                                                                                                  |
+| __Describe how you handle UserInfo endpoint errors__                                                                                                                           | The error is logged and the user is redirected to the homepage with an alert informing them of a problem |
+| __If you’re using the email address scope, confirm that you’re aware this represents the GOV.UK One Login username and may not be the user’s preferred contact email address__ | &check;                                                                                                  |
 
 ### Key management requirements 
-| Requirement | Response |
-| --- | --- |
+
+| Requirement                                                                                                                     | Response                                                 |
+| ---                                                                                                                             | ---                                                      |
 | __If using the GOV.UK One Login OpenID Provider JWKS Endpoint for signature validation describe your approach to key rotation__ | The keys are cached and the cache expires every 24 hours |
 
-### Session management requirements  
-| Requirement | Response |
-| --- | --- |
-| __Confirm that you’ve implemented logout functionality and that your service calls the GOV.UK One Login logout endpoint__ | &check; |
+### Session management requirements
+
+| Requirement                                                                                                               | Response |
+| ---                                                                                                                       | ---      |
+| __Confirm that you’ve implemented logout functionality and that your service calls the GOV.UK One Login logout endpoint__ | &check;  |
diff --git a/app/components/header_component.rb b/app/components/header_component.rb
@@ -50,7 +50,7 @@ def call
       tag.li(**html_attributes) do
         if link?
           link_to(href, class: 'dfe-header__navigation-link', **options) do
-            text.concat(chevron).html_safe
+            (text + chevron).html_safe
           end
         else
           text

diff --git a/app/controllers/concerns/answering.rb b/app/controllers/concerns/answering.rb
@@ -0,0 +1,45 @@
+# Form processing for training and feedback
+#
+module Answering
+  extend ActiveSupport::Concern
+
+private
+
+  # @return [Boolean]
+  def multiple_choice?
+    params[:response][:answers].is_a?(Array)
+  end
+
+  # @return [Boolean]
+  def correct?
+    content.opinion_question? ? true : content.correct_answers.eql?(response_answers)
+  end
+
+  # @return [ActionController::Parameters]
+  def response_params
+    if multiple_choice?
+      params.require(:response).permit(:text_input, answers: [])
+    else
+      params.require(:response).permit(:text_input, :answers)
+    end
+  end
+
+  # @return [String]
+  def response_text_input
+    response_params[:text_input]
+  end
+
+  # @return [Array<Integer>]
+  def response_answers
+    Array(response_params[:answers]).compact_blank.map(&:to_i)
+  end
+
+  # @return [Boolean]
+  def save_response!
+    current_user_response.update(
+      answers: response_answers,
+      correct: correct?,
+      text_input: response_text_input,
+    )
+  end
+end
diff --git a/app/controllers/concerns/learning.rb b/app/controllers/concerns/learning.rb
@@ -1,3 +1,5 @@
+# Common methods for training controllers
+#
 module Learning
   # ----------------------------------------------------------------------------
   # Request Params
@@ -71,6 +73,11 @@ def section_bar
   # @note memoization ensures validation errors work
   # @return [Response]
   def current_user_response
-    @current_user_response ||= content.feedback_question? ? current_user.response_for_shared(content, mod) : current_user.response_for(content)
+    @current_user_response ||=
+      if content.feedback_question?
+        current_user.response_for_shared(content, mod)
+      else
+        current_user.response_for(content)
+      end
   end
 end
diff --git a/app/controllers/concerns/logging.rb b/app/controllers/concerns/logging.rb
@@ -8,7 +8,7 @@ module Logging
   # @yield [nil]
   def log_caching
     yield
-  rescue HTTP::TimeoutError
+  rescue HTTP::TimeoutError, Contentful::Error
     message = "Repopulating cache #{ENV['ENVIRONMENT']} #{self.class.name}"
     Sentry.capture_message message, level: :info
     yield

diff --git a/app/controllers/feedback_controller.rb b/app/controllers/feedback_controller.rb
@@ -1,4 +1,6 @@
 class FeedbackController < ApplicationController
+  include Answering
+
   before_action :research_participation, only: :show
 
   helper_method :content,
@@ -40,15 +42,6 @@ def research_participation
     end
   end
 
-  # @return [Boolean]
-  def save_response!
-    current_user_response.update(
-      answers: user_answers,
-      correct: true,
-      text_input: response_params[:text_input],
-    )
-  end
-
   # @return [Course]
   def mod
     Course.config
@@ -75,19 +68,12 @@ def content_name
     params[:id]
   end
 
-  # OPTIMIZE: duplicated from ResponsesController
-  def response_params
-    params.require(:response).permit!
-  end
-
-  # OPTIMIZE: duplicated from ResponsesController
-  def user_answers
-    Array(response_params[:answers]).compact_blank.map(&:to_i)
-  end
-
   # @return [Hash]
   def feedback_cookie
-    cookies[:course_feedback] = { value: current_user.visit_token }
+    cookies[:course_feedback] = {
+      value: current_user.visit_token,
+      expires: 2.days.from_now,
+    }
   end
 
   # @return [Boolean, nil]

diff --git a/app/controllers/learning_controller.rb b/app/controllers/learning_controller.rb
@@ -1,5 +1,3 @@
-# frozen_string_literal: true
-
 class LearningController < ApplicationController
   before_action :authenticate_registered_user!
 

diff --git a/app/controllers/registration/base_controller.rb b/app/controllers/registration/base_controller.rb
@@ -1,3 +1,14 @@
+#
+# Registration journey order:
+#
+#   1. terms_and_conditions (one-off)
+#   2. name
+#   3. setting_type / setting_type_other
+#   4. local_authority
+#   5. role_type / role_type_other
+#   6. early_years_experience
+#   7. training_emails
+#
 module Registration
   class BaseController < ApplicationController
     before_action :authenticate_user!

diff --git a/app/controllers/registration/early_years_emails_controller.rb b/app/controllers/registration/early_years_emails_controller.rb
diff --git a/app/controllers/registration/early_years_experiences_controller.rb b/app/controllers/registration/early_years_experiences_controller.rb
@@ -20,13 +20,18 @@ def update
 
   private
 
+    # @return [ActionController::Parameters]
     def user_params
       params.require(:user).permit(:early_years_experience)
     end
 
     # @return [Registration::EarlyYearsExperiencesForm]
     def form
-      @form ||= EarlyYearsExperiencesForm.new(user: current_user, early_years_experience: current_user.early_years_experience)
+      @form ||=
+        EarlyYearsExperiencesForm.new(
+          user: current_user,
+          early_years_experience: current_user.early_years_experience,
+        )
     end
   end
 end
diff --git a/app/controllers/registration/local_authorities_controller.rb b/app/controllers/registration/local_authorities_controller.rb
@@ -16,7 +16,7 @@ def update
 
   private
 
-    # @return [Hash]
+    # @return [ActionController::Parameters]
     def user_params
       params.require(:user).permit(:local_authority)
     end

diff --git a/app/controllers/registration/names_controller.rb b/app/controllers/registration/names_controller.rb
@@ -21,6 +21,7 @@ def update
 
   private
 
+    # @return [ActionController::Parameters]
     def user_params
       params.require(:user).permit(:first_name, :last_name)
     end

diff --git a/app/controllers/registration/role_type_others_controller.rb b/app/controllers/registration/role_type_others_controller.rb
@@ -16,13 +16,18 @@ def update
 
   private
 
+    # @return [ActionController::Parameters]
     def user_params
       params.require(:user).permit(:role_type_other)
     end
 
     # @return [Registration::RoleTypeOtherForm]
     def form
-      @form ||= RoleTypeOtherForm.new(user: current_user, role_type_other: current_user.role_type_other)
+      @form ||=
+        RoleTypeOtherForm.new(
+          user: current_user,
+          role_type_other: current_user.role_type_other,
+        )
     end
   end
 end
diff --git a/app/controllers/registration/role_types_controller.rb b/app/controllers/registration/role_types_controller.rb
@@ -16,13 +16,18 @@ def update
 
   private
 
+    # @return [ActionController::Parameters]
     def user_params
       params.require(:user).permit(:role_type)
     end
 
     # @return [Registration::RoleTypeForm]
     def form
-      @form ||= RoleTypeForm.new(user: current_user, role_type: current_user.role_type)
+      @form ||=
+        RoleTypeForm.new(
+          user: current_user,
+          role_type: current_user.role_type,
+        )
     end
   end
 end
diff --git a/app/controllers/registration/setting_type_others_controller.rb b/app/controllers/registration/setting_type_others_controller.rb
@@ -16,13 +16,18 @@ def update
 
   private
 
+    # @return [ActionController::Parameters]
     def user_params
       params.require(:user).permit(:setting_type_other)
     end
 
     # @return [Registration::SettingTypeOtherForm]
     def form
-      @form ||= SettingTypeOtherForm.new(user: current_user, setting_type_other: current_user.setting_type_other)
+      @form ||=
+        SettingTypeOtherForm.new(
+          user: current_user,
+          setting_type_other: current_user.setting_type_other,
+        )
     end
   end
 end