GovOne authentication integration changes (#1020)

* WIP * tweak logout path logic * Existing session logic * Opt in WCAG logic * add check for session id token in logout link * QA debug * Avoid duplicate hits on about page * Sanity check * Fix memory issue for QA locally and in pipeline? * Trigger full WCAG check on demand if already deployed and includes changes * Factory and count * Test feature in pipeline * Integrate GovOne to replace existing sessions and skip deprecated specs * Update docs and fix specs * Increase coverage * Fix pa11y and plan to retire old devise functionality * Tidy * Delete old GPaaS debugging var --------- Co-authored-by: jack.coggin <[email protected]>
DFE-Digital · Jan 5, 2024 · 263000d · 263000d
1 parent 4af564b
commit 263000d
Show file tree

Hide file tree

Showing 49 changed files with 398 additions and 390 deletions.
diff --git a/.github/labeler.yml b/.github/labeler.yml
@@ -17,6 +17,7 @@ terraform:
 frontend:
   - changed-files:
     - any-glob-to-any-file:
+      - app/views/**/*.html.slim
       - app/assets/images/*
       - app/assets/stylesheets/**/*
       - app/javascript/**/*

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -83,8 +83,15 @@ jobs:
       -
         name: Run rubocop
         run: bundle exec rubocop
+      # Answer migration feature test
       # -
       #   name: Run test suite
       #   run: bundle exec rspec
       #   env:
       #     DISABLE_USER_ANSWER: true
+      # Gov One feature test
+      -
+        name: Run test suite
+        run: bundle exec rspec
+        env:
+          GOV_ONE_LOGIN: true
diff --git a/.github/workflows/pa11y.yml b/.github/workflows/pa11y.yml
@@ -17,7 +17,7 @@ on:
 
 jobs:
   test:
-    if: contains(github.event.pull_request.labels.*.name, 'review')
+    if: contains(github.event.pull_request.labels.*.name, 'review') && contains(github.event.pull_request.labels.*.name, 'frontend') && contains(github.event.pull_request.labels.*.name, 'a11y')
     runs-on: ubuntu-latest
     environment: development
     env:

diff --git a/.github/workflows/qa.yml b/.github/workflows/qa.yml
@@ -38,10 +38,14 @@ jobs:
         image: selenium/standalone-chrome:latest
         ports:
           - 4441:4444
+        volumes:
+          - /dev/shm:/dev/shm
       firefox:
         image: selenium/standalone-firefox:latest
         ports:
           - 4442:4444
+        volumes:
+          - /dev/shm:/dev/shm
 
     steps:
       -
@@ -67,4 +71,4 @@ jobs:
         name: Use Firefox
         env:
           BROWSER: standalone_firefox
-        run: bundle exec rspec --default-path ui
+        run: bundle exec rspec --default-path ui
diff --git a/.yardopts b/.yardopts
@@ -5,9 +5,9 @@
 --markup markdown
 -
 cms/*.md
-gov_one_login/*.md
 data/*.md
 uml/*.md
 adr/*.md
 RELEASE.md
+GOV_ONE.md
 LICENSE
diff --git a/gov_one_login/GOV-ONE-LOGIN.md → GOV_ONE.md b/gov_one_login/GOV-ONE-LOGIN.md → GOV_ONE.md
diff --git a/README.md b/README.md
@@ -163,7 +163,7 @@ Manually adding the **"deployed"** label to a pull request in Github will cause
 This supports manual testing and content review in a production environment.
 
 When a feature branch review application is deployed, the URL to access it is added as a comment
-in the PR conversation in the format: <https://ey-recovery-pr-##.london.cloudapps.digital/>
+in the PR conversation in the format: <https://eyrecovery-review-pr-##.azurewebsites.net>
 
 Review applications are deployed with 3 seeded user accounts that share a restricted password.
 This facilitates team members demoing content and functionality, so registration is not required.
@@ -315,9 +315,10 @@ or in the UK Government digital slack workspace in the `#govuk-notify` channel.
 
 ---
 
-# One Login
+# GovOne Login
 
-<https://signin.integration.account.gov.uk/sign-in-or-create>
+Register an account on the integration OIDC used in development <https://signin.integration.account.gov.uk/sign-in-or-create>.
+Using this authentication method also requires basic HTTP auth credentials.
 
 ---
 

diff --git a/RELEASE.md b/RELEASE.md
@@ -22,7 +22,7 @@ We use 9 swim lanes on the [Jira][jira] board.
 
 - `content` deployment
 - `ER-456` branch
-- <https://ey-recovery-pr-123.london.cloudapps.digital>
+- <https://eyrecovery-review-pr-123.azurewebsites.net>
 - [feature deployments][deployments]
 - meets [definition of done](#definition-of-done)
 - mark PR as `ready` and [request review](#review-process)
@@ -40,22 +40,22 @@ We use 9 swim lanes on the [Jira][jira] board.
 
 - `development` deployment
 - `main` branch
-- <https://ey-recovery-dev.london.cloudapps.digital>
+- <https://eyrecovery-dev.azurewebsites.net>
 - update accessibility and quality checks as required
 
 **7. Approval**
 
 - `staging` deployment
 - `rcx.x.x` release candidate tag
-- <https://ey-recovery-staging.london.cloudapps.digital>
+- <https://staging.child-development-training.education.gov.uk>
 - [open milestones (release candidates)][release-candidates]
 - seek product owner sign-off
 
 **8. Release**
 
 - `production` deployment
 - `vx.x.x` version tag
-- <https://ey-recovery.london.cloudapps.digital>
+- <https://child-development-training.education.gov.uk>
 - [close milestone][released-versions] and rename from `rc` to `v`
 - periodically publish a [release][releases] CHANGELOG
 

diff --git a/app/controllers/gov_one_controller.rb b/app/controllers/gov_one_controller.rb
diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb
@@ -1,6 +1,16 @@
 # frozen_string_literal: true
 
 class Users::SessionsController < Devise::SessionsController
+  layout 'hero' if Rails.application.gov_one_login?
+
+  def new
+    if Rails.application.gov_one_login?
+      render 'gov_one'
+    else
+      super
+    end
+  end
+
 protected
 
   def after_sign_in_path_for(resource)

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
@@ -11,14 +11,14 @@ def navigation
       header.with_navigation_item(text: 'Home', href: root_path, classes: %w[dfe-header__navigation-item])
       if user_signed_in?
         header.with_action_link(text: 'My Account', href: user_path, options: { inverse: true })
-        header.with_action_link(text: 'Sign out', href: logout_path, options: { id: 'sign-out-desktop', data: { turbo_method: :get }, inverse: true })
+        header.with_action_link(text: 'Sign out', href: destroy_user_session_path, options: { id: 'sign-out-desktop', data: { turbo_method: :get }, inverse: true })
         header.with_navigation_item(text: 'My modules', href: my_modules_path, classes: %w[dfe-header__navigation-item])
         header.with_navigation_item(text: 'Learning log', href: user_notes_path, classes: %w[dfe-header__navigation-items]) if current_user.course_started?
         header.with_navigation_item(text: 'My account', href: user_path, classes: %w[dfe-header__navigation-item dfe-header-f-mob])
-        header.with_navigation_item(text: 'Sign out', href: logout_path, options: { data: { turbo_method: :get } }, classes: %w[dfe-header__navigation-item dfe-header-f-mob], html_attributes: { id: 'sign-out-f-mob' })
+        header.with_navigation_item(text: 'Sign out', href: destroy_user_session_path, options: { data: { turbo_method: :get } }, classes: %w[dfe-header__navigation-item dfe-header-f-mob], html_attributes: { id: 'sign-out-f-mob' })
       else
-        header.with_action_link(text: 'Sign in', href: login_path, options: { inverse: true })
-        header.with_navigation_item(text: 'Sign in', href: login_path, classes: %w[dfe-header__navigation-item dfe-header-f-mob])
+        header.with_action_link(text: 'Sign in', href: new_user_session_path, options: { inverse: true })
+        header.with_navigation_item(text: 'Sign in', href: new_user_session_path, classes: %w[dfe-header__navigation-item dfe-header-f-mob])
       end
     end
   end
@@ -59,14 +59,4 @@ def html_title(*parts)
   def calculate_module_state
     CalculateModuleState.new(user: current_user).call
   end
-
-  # @return [String]
-  def login_path
-    Rails.application.gov_one_login? ? gov_one_info_path : new_user_session_path
-  end
-
-  # @return [String]
-  def logout_path
-    Rails.application.gov_one_login? ? logout_uri.to_s : destroy_user_session_path
-  end
 end
diff --git a/app/helpers/link_helper.rb b/app/helpers/link_helper.rb
@@ -1,4 +1,10 @@
 module LinkHelper
+  # @note Handle active sessions for feature flag Rails.application.gov_one_login?
+  # @return [String]
+  def destroy_user_session_path
+    session[:id_token].present? ? logout_uri.to_s : super
+  end
+
   # OPTIMIZE: use this helper for all back link logic and consistent location
   #
   # @return [String, nil]

diff --git a/app/models/data_analysis/user_overview.rb b/app/models/data_analysis/user_overview.rb
@@ -19,6 +19,7 @@ def column_names
           'Locked Out',
           'Confirmed',
           'Unconfirmed',
+          'GovOne',
           'User Defined Roles',
           'Started Learning',
           'Not Started Learning',
@@ -49,13 +50,14 @@ def dashboard
           locked_out: User.locked_out.count,
           confirmed: User.confirmed.count,
           unconfirmed: User.unconfirmed.count,
+          gov_one: User.gov_one.count,
           user_defined_roles: User.all.collect(&:role_type_other).uniq.count,
           started_learning: started_learning,
           not_started_learning: not_started_learning,
           with_notes: with_notes_count,
-          with_notes_percentage: with_notes_percentage,
+          with_notes_percentage: with_notes_percentage.round(2),
           without_notes: without_notes_count,
-          without_notes_percentage: 1 - with_notes_percentage,
+          without_notes_percentage: (1 - with_notes_percentage).round(2),
           complete_registration_mail_recipients: CompleteRegistrationMailJob.recipients.count,
           start_training_mail_recipients: StartTrainingMailJob.recipients.count,
           continue_training_mail_recipients: ContinueTrainingMailJob.recipients.count,

diff --git a/app/models/user.rb b/app/models/user.rb
@@ -31,18 +31,27 @@ def self.find_or_create_from_gov_one(email:, gov_one_id:)
       user.save!
     else
       user = new(email: email, gov_one_id: gov_one_id, confirmed_at: Time.zone.now)
-      user.save!(validate: false)
+      user.save!(validate: false) # TODO: validate despite blank password
     end
     user
   end
 
-  # Include default devise modules. Others available are:
-  # :timeoutable, :trackable, :recoverable and :omniauthable
   attr_accessor :context
 
   devise :database_authenticatable, :registerable, :recoverable,
          :validatable, :rememberable, :confirmable, :lockable, :timeoutable,
          :secure_validatable, :omniauthable, omniauth_providers: [:openid_connect]
+
+  # FIXME: retire old devise functionality
+  # if Rails.application.gov_one_login?
+  #   devise :database_authenticatable, :rememberable, :lockable, :timeoutable,
+  #          :omniauthable, omniauth_providers: [:openid_connect]
+  # else
+  #   devise :database_authenticatable, :registerable, :recoverable,
+  #          :validatable, :rememberable, :confirmable, :lockable, :timeoutable,
+  #          :secure_validatable
+  # end
+
   devise :pwned_password unless Rails.env.test?
 
   has_many :responses
@@ -53,6 +62,8 @@ def self.find_or_create_from_gov_one(email:, gov_one_id:)
   has_many :events, class_name: 'Ahoy::Event'
   has_many :notes
 
+  scope :gov_one, -> { where.not(gov_one_id: nil) }
+
   # account status
   scope :public_beta_only_registration_complete, -> { registered_since_private_beta.registration_complete }
   scope :since_public_beta, -> { where(created_at: Rails.application.public_beta_launch_date..Time.zone.now) }

diff --git a/app/views/about/_enrol.html.slim b/app/views/about/_enrol.html.slim
@@ -1,4 +1,13 @@
 - unless current_user
   .light-grey-box class='govuk-!-margin-top-8 govuk-!-margin-bottom-4'
     = m('about.enrol')
-    = render 'home/enrol_buttons'
+
+    .govuk-button-group
+      - if Rails.application.gov_one_login?
+        = govuk_link_to 'Create an account or sign in', new_user_session_path
+
+      - else
+        = govuk_button_link_to 'Create an account', new_user_registration_path
+        .white-space-pre-wrap= ' or '
+        = govuk_link_to 'sign in', new_user_session_path
+
diff --git a/app/views/gov_one/show.html.slim → app/views/devise/sessions/gov_one.html.slim b/app/views/gov_one/show.html.slim → app/views/devise/sessions/gov_one.html.slim
@@ -1,20 +1,19 @@
 - content_for :page_title do
-  = html_title t('home.title')
+  = html_title t('account_login.title')
 
 - content_for :hero do
   .govuk-grid-row class='govuk-!-padding-top-9'
     .govuk-grid-column-three-quarters
       h1.dfe-heading-xl class='govuk-!-margin-bottom-4'
         = t('gov_one_info.hero.header')
-      p.govuk-body-l = t('gov_one_info.hero.body')
+      p.govuk-body-l
+        = t('gov_one_info.hero.body')
 
 
 .govuk-grid-row
   .govuk-grid-column-full
-    . class='govuk-!-margin-bottom-5'
-        = m('gov_one_info.body')
-        hr
-
+    = m('gov_one_info.body')
+    hr.govuk-section-break.govuk-section-break--visible.govuk-section-break--l
     = login_button
 
 .govuk-grid-row

diff --git a/app/views/home/_enrol_buttons.html.slim b/app/views/home/_enrol_buttons.html.slim
diff --git a/app/views/home/_prompt.html.slim b/app/views/home/_prompt.html.slim
@@ -0,0 +1,7 @@
+.prompt.prompt-home
+  .govuk-grid-row
+    .govuk-grid-column-one-quarter
+      i.fa-2x.fa-solid.fa-circle-info aria-describedby='info icon'
+
+    .govuk-grid-column-three-quarters
+      = m('home.prompt', headings_start_with: 'xl')
diff --git a/app/views/home/index.html.slim b/app/views/home/index.html.slim
@@ -1,41 +1,28 @@
-- if Rails.application.gov_one_login?
-  - content_for :page_title do
-    = html_title t('home.title')
+- content_for :page_title do
+  = html_title t('home.title')
 
-  - content_for :hero do
-    = render 'hero'
+- content_for :hero do
+  = render 'hero'
 
-  = render 'learning/cms_debug'
-  = render 'debug'
+= render 'learning/cms_debug'
+= render 'debug'
+
+- if Rails.application.gov_one_login? # ----------------------------------------
 
   .govuk-grid-row
     .govuk-grid-column-one-half
       = m('home.about', headings_start_with: 'xl')
 
-  .prompt.prompt-home
-    .govuk-grid-row
-      .govuk-grid-column-one-quarter
-        i.fa-2x.fa-solid.fa-circle-info aria-describedby='info icon'
-
-      .govuk-grid-column-three-quarters
-        = m('home.prompt', headings_start_with: 'xl')
+  = render 'prompt'
 
   - unless current_user
-    .govuk-grid-row class="govuk-!-margin-top-9"
+    .govuk-grid-row class='govuk-!-margin-top-9'
       .govuk-grid-column-full
-        = govuk_button_link_to gov_one_info_path, class: "govuk-button--start" do
-          | #{t('home.gov_one_button')}
+        = govuk_button_link_to new_user_session_path, class: 'govuk-button--start' do
+          = t('home.gov_one_button')
           = render 'chevron'
 
-- else
-  - content_for :page_title do
-    = html_title t('home.title')
-
-  - content_for :hero do
-    = render 'hero'
-
-  = render 'learning/cms_debug'
-  = render 'debug'
+- else # -----------------------------------------------------------------------
 
   .govuk-grid-row
     .govuk-grid-column-one-half
@@ -51,10 +38,4 @@
             .white-space-pre-wrap= ' or '
             = govuk_link_to 'create an account', new_user_registration_path
 
-  .prompt.prompt-home
-    .govuk-grid-row
-      .govuk-grid-column-one-quarter
-        i.fa-2x.fa-solid.fa-circle-info aria-describedby='info icon'
-
-      .govuk-grid-column-three-quarters
-        = m('home.prompt', headings_start_with: 'xl')
+  = render 'prompt'
diff --git a/app/views/learning/_cms_debug.html.slim b/app/views/learning/_cms_debug.html.slim
@@ -1,7 +1,5 @@
 - if debug?
   pre.debug_dump
-    h4 Deployment
-    | instance: #{ENV['CF_INSTANCE_INDEX']}
     h4 Contentful
     | env: #{ContentfulRails.configuration.environment}
     br