Merge pull request #267 from DFE-Digital/fix/csrf-patch-for-omniauth

CSRF patch for Omniauth vulnerability

Gemfile

@@ -18,6 +18,7 @@ gem "pg"
 gem "mini_racer"
 gem "omniauth"
 gem "omniauth_openid_connect"
+gem "omniauth-rails_csrf_protection"
 gem "puma", "~> 5.2"
 gem "redcarpet", "~> 3.5"
 gem "redis", "~> 4.2"

Gemfile.lock

@@ -203,6 +203,9 @@ GEM
     omniauth (1.9.1)
       hashie (>= 3.4.6)
       rack (>= 1.6.2, < 3)
+    omniauth-rails_csrf_protection (0.1.2)
+      actionpack (>= 4.2)
+      omniauth (>= 1.3.1)
     omniauth_openid_connect (0.3.5)
       addressable (~> 2.5)
       omniauth (~> 1.9)
@@ -426,6 +429,7 @@ DEPENDENCIES
   mini_racer
   mock_redis
   omniauth
+  omniauth-rails_csrf_protection
   omniauth_openid_connect
   pg
   pry

app/controllers/application_controller.rb

@@ -27,7 +27,7 @@ def authenticate_user!
     return if current_user
 
     session.delete(:dfe_sign_in_uid)
-    redirect_to new_dfe_path
+    redirect_to root_path
   end
 
   def current_journey

app/controllers/sessions_controller.rb

@@ -4,11 +4,6 @@ class SessionsController < ApplicationController
   skip_before_action :authenticate_user!
   protect_from_forgery except: :bypass_callback
 
-  def new
-    # This is defined by the class name of our Omniauth strategy
-    redirect_to "/auth/dfe"
-  end
-
   def create
     UserSession.new(session: session)
       .persist_successful_dfe_sign_in_claim!(omniauth_hash: auth_hash)

app/views/pages/specifying_start_page.html.erb

@@ -37,7 +37,7 @@
 <p class="govuk-body"><%= I18n.t("specifying.start_page.pause_and_resume_body") %></p>
 
 <% if DfESignIn.bypass? %>
-  <%= link_to "#{I18n.t("generic.button.start")} (bypass DSI)", "/auth/developer", class: "govuk-button" %>
+  <%= button_to "#{I18n.t("generic.button.start")} (bypass DSI)", "/auth/developer", class: "govuk-button" %>
 <% else %>
-  <%= link_to I18n.t("generic.button.start"), new_dfe_path, class: "govuk-button" %>
+  <%= button_to I18n.t("generic.button.start"), "/auth/dfe", class: "govuk-button" %>
 <% end %>

config/routes.rb

@@ -8,12 +8,6 @@
   post "/api/contentful/entry_updated" => "api/contentful/entries#changed"
 
   # DfE Sign In
-  resource :sessions,
-    only: %i[create new],
-    as: :dfe,
-    path: "/dfe/sessions",
-    controller: "sessions"
-
   get "/auth/dfe/callback", to: "sessions#create"
   get "/auth/dfe/signout", to: "sessions#destroy"
   get "/auth/failure", to: "sessions#failure"

spec/features/visitors/anyone_can_sign_in_with_dfe_sign_in_spec.rb

@@ -58,7 +58,8 @@
       .with("Sign in failed unexpectedly")
       .and_call_original
 
-    visit dashboard_path
+    visit root_path
+    click_button I18n.t("generic.button.start")
 
     expect(page).to have_content(I18n.t("errors.sign_in.unexpected_failure.page_title"))
     expect(page).to have_content(I18n.t("errors.sign_in.unexpected_failure.page_body"))

spec/requests/authentication_spec.rb

@@ -18,7 +18,7 @@
     end
 
     it "users can access the new session endpoint" do
-      get new_dfe_path
+      post "/auth/dfe"
       expect(response).to have_http_status(:found)
     end
 
@@ -36,29 +36,29 @@
   describe "Endpoints that do require authentication" do
     it "users cannot access the new journey path" do
       get new_journey_path
-      expect(response).to redirect_to(new_dfe_path)
+      expect(response).to redirect_to(root_path)
     end
 
     it "users cannot access an existing journey" do
       journey = create(:journey)
       get journey_path(journey)
-      expect(response).to redirect_to(new_dfe_path)
+      expect(response).to redirect_to(root_path)
     end
 
     it "users cannot edit an answer" do
       answer = create(:radio_answer)
       get edit_journey_step_path(answer.step.journey, answer.step)
-      expect(response).to redirect_to(new_dfe_path)
+      expect(response).to redirect_to(root_path)
     end
 
     it "users cannot see the journey map" do
       get new_journey_map_path
-      expect(response).to redirect_to(new_dfe_path)
+      expect(response).to redirect_to(root_path)
     end
 
     it "users cannot see the preview endpoints" do
       get preview_entry_path("an-entry-id")
-      expect(response).to redirect_to(new_dfe_path)
+      expect(response).to redirect_to(root_path)
     end
   end
 

spec/support/sign_in_helpers.rb

@@ -13,7 +13,8 @@ def user_sign_in_attempt_fails(dsi_uid: SecureRandom.uuid)
   end
 
   def user_starts_the_journey
-    visit dashboard_path
+    visit root_path
+    click_link I18n.t("generic.button.start")
     click_link I18n.t("dashboard.create.link")
   end