diff --git a/.env.development b/.env.development
index 501b8e66..e607f870 100644
--- a/.env.development
+++ b/.env.development
@@ -1,4 +1,5 @@
BYPASS_DSI=true
+HOSTING_DOMAIN=http://localhost:3000
HOSTING_ENVIRONMENT=local
DFE_SIGN_IN_API_BASE_URL=https://dev-api.signin.education.gov.uk
diff --git a/app/components/flash_message_component.html.erb b/app/components/flash_message_component.html.erb
new file mode 100644
index 00000000..62227cae
--- /dev/null
+++ b/app/components/flash_message_component.html.erb
@@ -0,0 +1,8 @@
+<%= govuk_notification_banner(
+ title_text: title,
+ classes: classes,
+ html_attributes: { role: role },
+) do |notification_banner| %>
+ <% notification_banner.with_heading(text: heading) %>
+ <%= body %>
+<% end %>
diff --git a/app/components/flash_message_component.rb b/app/components/flash_message_component.rb
new file mode 100644
index 00000000..2d79f42b
--- /dev/null
+++ b/app/components/flash_message_component.rb
@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+class FlashMessageComponent < ViewComponent::Base
+ ALLOWED_PRIMARY_KEYS = %i[info warning success].freeze
+ DEVISE_PRIMARY_KEYS = { alert: :warning, notice: :info }.freeze
+
+ def initialize(flash:)
+ super
+ @flash = flash.to_hash.symbolize_keys!
+ end
+
+ def message_key
+ key =
+ flash.keys.detect do |k|
+ ALLOWED_PRIMARY_KEYS.include?(k) || DEVISE_PRIMARY_KEYS.keys.include?(k)
+ end
+ DEVISE_PRIMARY_KEYS[key] || key
+ end
+
+ def title
+ I18n.t(message_key, scope: :notification_banner)
+ end
+
+ def classes
+ "govuk-notification-banner--#{message_key}"
+ end
+
+ def role
+ %i[warning success].include?(message_key) ? "alert" : "region"
+ end
+
+ def heading
+ messages.is_a?(Array) ? messages[0] : messages
+ end
+
+ def body
+ tag.p(messages[1], class: "govuk-body") if messages.is_a?(Array) && messages.count >= 2
+ end
+
+ def render?
+ !flash.empty? && message_key
+ end
+
+ private
+
+ def messages
+ flash[message_key] || flash[DEVISE_PRIMARY_KEYS.key(message_key)]
+ end
+
+ attr_reader :flash
+end
diff --git a/app/controllers/auth_failures_controller.rb b/app/controllers/auth_failures_controller.rb
new file mode 100644
index 00000000..c1e62560
--- /dev/null
+++ b/app/controllers/auth_failures_controller.rb
@@ -0,0 +1,31 @@
+class AuthFailuresController < ApplicationController
+ skip_before_action :authenticate_dsi_user!
+ skip_before_action :handle_expired_session!
+
+ class OpenIdConnectProtocolError < StandardError; end
+
+ def failure
+ return redirect_to(dsi_sign_out_path(id_token_hint: session[:id_token])) if session_expired?
+ handle_failure_then_redirect_to sign_in_path(oauth_failure: true)
+ end
+
+ private
+
+ def handle_failure_then_redirect_to(path)
+ oidc_error = OpenIdConnectProtocolError.new(error_message)
+ unless Rails.env.development?
+ Sentry.capture_exception(oidc_error)
+ redirect_to(path) and return
+ end
+
+ raise oidc_error
+ end
+
+ def error_message
+ @error_message ||= request.env["omniauth.error"]&.message
+ end
+
+ def session_expired?
+ error_message == "sessionexpired"
+ end
+end
diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb
index aa623806..c16df395 100644
--- a/app/controllers/omniauth_callbacks_controller.rb
+++ b/app/controllers/omniauth_callbacks_controller.rb
@@ -5,24 +5,41 @@ class OmniauthCallbacksController < ApplicationController
skip_before_action :authenticate_dsi_user!
skip_before_action :handle_expired_session!
- def dfe
- auth = request.env["omniauth.auth"]
-
- unless DfESignIn.bypass?
- role = DfESignInApi::GetUserAccessToService.new(
- org_id: auth.extra.raw_info.organisation.id,
- user_id: auth.uid,
- ).call
+ before_action :add_auth_attributes_to_session, only: :dfe
+ def dfe
+ if DfESignIn.bypass?
+ create_or_update_dsi_user
+ else
return redirect_to not_authorised_path unless role
+ create_or_update_dsi_user(role)
end
- @dsi_user = DsiUser.create_or_update_from_dsi(auth, role)
+ redirect_to root_path
+ end
+ alias_method :dfe_bypass, :dfe
+
+ private
+ def auth
+ request.env["omniauth.auth"]
+ end
+
+ def add_auth_attributes_to_session
+ session[:id_token] = auth.credentials.id_token
+ session[:organisation_name] = auth.extra.raw_info.organisation.name
+ end
+
+ def create_or_update_dsi_user(role = nil)
+ @dsi_user = DsiUser.create_or_update_from_dsi(auth, role)
session[:dsi_user_id] = @dsi_user.id
session[:dsi_user_session_expiry] = 2.hours.from_now.to_i
+ end
- redirect_to root_path
+ def role
+ @role ||= DfESignInApi::GetUserAccessToService.new(
+ org_id: auth.extra.raw_info.organisation.id,
+ user_id: auth.uid,
+ ).call
end
- alias_method :dfe_bypass, :dfe
end
diff --git a/app/controllers/sign_in_controller.rb b/app/controllers/sign_in_controller.rb
index 4408cb36..5e0687a8 100644
--- a/app/controllers/sign_in_controller.rb
+++ b/app/controllers/sign_in_controller.rb
@@ -1,7 +1,15 @@
class SignInController < ApplicationController
skip_before_action :authenticate_dsi_user!
skip_before_action :handle_expired_session!
+ before_action :reset_session
+ before_action :handle_failed_sign_in, if: -> { params[:oauth_failure] == "true" }
def new
end
+
+ private
+
+ def handle_failed_sign_in
+ flash.now[:warning] = I18n.t("generic_oauth_failure")
+ end
end
diff --git a/app/controllers/sign_out_controller.rb b/app/controllers/sign_out_controller.rb
index f2b9172a..c87b9608 100644
--- a/app/controllers/sign_out_controller.rb
+++ b/app/controllers/sign_out_controller.rb
@@ -1,7 +1,7 @@
class SignOutController < ApplicationController
skip_before_action :handle_expired_session!
+ before_action :reset_session
def new
- session[:dsi_user_id] = nil if dsi_user_signed_in?
end
end
diff --git a/app/views/errors/not_authorised.html.erb b/app/views/errors/not_authorised.html.erb
index f262fe6a..a571b48b 100644
--- a/app/views/errors/not_authorised.html.erb
+++ b/app/views/errors/not_authorised.html.erb
@@ -2,9 +2,10 @@
-
Authorisation required
+
You cannot use the DfE Sign-in account for <%= session[:organisation_name] %> to check the children's barred list.
- You are not authorised to access this service.
+ If you have access to another organisation in DfE Sign-in, you can <%= govuk_link_to("sign out and start again", dsi_sign_out_path(id_token_hint: session[:id_token])) %>.
+
diff --git a/app/views/layouts/base.html.erb b/app/views/layouts/base.html.erb
index eb952b1c..d7273698 100644
--- a/app/views/layouts/base.html.erb
+++ b/app/views/layouts/base.html.erb
@@ -28,10 +28,12 @@
<%= govuk_skip_link %>
<%= govuk_header(service_name: t("service.name")) do |header|
- if current_dsi_user
- header.with_navigation_item(href: "/sign-out", text: "Sign out")
- else
- header.with_navigation_item(href: "/sign-in", text: "Sign in")
+ if request.path != not_authorised_path
+ if current_dsi_user
+ header.with_navigation_item(href: dsi_sign_out_path(id_token_hint: session[:id_token]), text: "Sign out")
+ else
+ header.with_navigation_item(href: sign_in_path, text: "Sign in")
+ end
end
if support_interface?
@@ -49,6 +51,7 @@
<%= govuk_back_link(href: yield(:back_link_url)) if content_for?(:back_link_url) %>
<%= yield(:breadcrumbs) if content_for?(:breadcrumbs) %>
+ <%= render(FlashMessageComponent.new(flash: flash)) %>
<%= yield :content %>
diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb
index ea2a6673..afaf47b5 100644
--- a/config/initializers/omniauth.rb
+++ b/config/initializers/omniauth.rb
@@ -1,6 +1,9 @@
require "dfe_sign_in"
+require "omniauth/strategies/dfe_openid_connect"
OmniAuth.config.logger = Rails.logger
+OmniAuth.config.add_camelization('dfe_openid_connect', 'DfEOpenIDConnect')
+OmniAuth.config.on_failure = proc { |env| AuthFailuresController.action(:failure).call(env) }
if DfESignIn.bypass?
Rails.application.config.middleware.use OmniAuth::Builder do
@@ -12,9 +15,11 @@
dfe_sign_in_issuer_uri = URI(ENV.fetch("DFE_SIGN_IN_ISSUER", "example"))
Rails.application.config.middleware.use OmniAuth::Builder do
- provider :openid_connect,
+ provider :dfe_openid_connect,
name: :dfe,
callback_path: "/auth/dfe/callback",
+ logout_path: "/sign-out",
+ post_logout_redirect_uri: "#{ENV['HOSTING_DOMAIN']}/sign-out",
client_options: {
host: dfe_sign_in_issuer_uri&.host,
identifier: ENV["DFE_SIGN_IN_CLIENT_ID"],
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 41c10705..b008f7b4 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -3,6 +3,7 @@ en:
name: Check the Children’s Barred List
email: employer.access@education.gov.uk
dbs_email: dbscost@dbs.gov.uk
+ generic_oauth_failure: There was a problem signing you in. Please try again.
activemodel:
errors:
models:
diff --git a/config/routes.rb b/config/routes.rb
index bbe5d0b6..5af572f6 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -6,6 +6,7 @@
get "/sign-in", to: "sign_in#new"
get "/sign-out", to: "sign_out#new"
+ get "/auth/dfe/sign-out", to: "sign_out#new", as: :dsi_sign_out
get "/auth/dfe/callback", to: "omniauth_callbacks#dfe"
post "/auth/developer/callback", to: "omniauth_callbacks#dfe_bypass"
diff --git a/lib/omniauth/strategies/dfe_openid_connect.rb b/lib/omniauth/strategies/dfe_openid_connect.rb
new file mode 100644
index 00000000..5adbfded
--- /dev/null
+++ b/lib/omniauth/strategies/dfe_openid_connect.rb
@@ -0,0 +1,27 @@
+# This strategy ensures that the id_token_hint param is included in the post_logout_redirect_uri.
+# The node-oidc-provider library requires this param to be present in order for the redirect to work.
+# See: https://github.com/panva/node-oidc-provider/blob/03c9bc513860e68ee7be84f99bfc9dc930b224e8/lib/actions/end_session.js#L27
+# See: https://github.com/omniauth/omniauth_openid_connect/blob/34370d655d39fe7980f89f55715888e0ebd7270e/lib/omniauth/strategies/openid_connect.rb#L423
+#
+module OmniAuth
+ module Strategies
+ class DfEOpenIDConnect < OmniAuth::Strategies::OpenIDConnect
+ TOKEN_KEY = "id_token_hint".freeze
+
+ def encoded_post_logout_redirect_uri
+ return unless options.post_logout_redirect_uri
+
+ logout_uri_params = {
+ "post_logout_redirect_uri" => options.post_logout_redirect_uri
+ }
+
+ if query_string.present?
+ query_params = CGI.parse(query_string[1..])
+ logout_uri_params[TOKEN_KEY] = query_params[TOKEN_KEY].first if query_params.key?(TOKEN_KEY)
+ end
+
+ URI.encode_www_form(logout_uri_params)
+ end
+ end
+ end
+end
diff --git a/spec/support/system/authentication_steps.rb b/spec/support/system/authentication_steps.rb
index d6bdaf33..94028300 100644
--- a/spec/support/system/authentication_steps.rb
+++ b/spec/support/system/authentication_steps.rb
@@ -11,6 +11,9 @@ def given_dsi_auth_is_mocked(authorised:)
{
provider: "dfe",
uid: "123456",
+ credentials: {
+ id_token: "abc123",
+ },
info: {
email: "test@example.com",
first_name: "Test",
@@ -20,6 +23,7 @@ def given_dsi_auth_is_mocked(authorised:)
raw_info: {
organisation: {
id: org_id,
+ name: "Test School",
}
}
}
@@ -34,6 +38,21 @@ def given_dsi_auth_is_mocked(authorised:)
)
end
+ def given_dsi_auth_is_mocked_with_a_failure(message)
+ allow(Sentry).to receive(:capture_exception)
+ OmniAuth.config.mock_auth[:dfe] = message.to_sym
+ global_failure_handler = OmniAuth.config.on_failure
+ local_failure_handler = proc do |env|
+ env["omniauth.error"] = OmniAuth::Strategies::OpenIDConnect::CallbackError.new(error: message)
+ env
+ end
+ OmniAuth.config.on_failure = global_failure_handler << local_failure_handler
+
+ yield if block_given?
+
+ OmniAuth.config.on_failure = global_failure_handler
+ end
+
def user_roles_endpoint
"#{ENV.fetch("DFE_SIGN_IN_API_BASE_URL")}/services/checkchildrensbarredlist/organisations/#{org_id}/users/123456"
end
diff --git a/spec/system/unauthorised_user_signs_in_spec.rb b/spec/system/unauthorised_user_signs_in_spec.rb
index 1dac09b8..94971a3f 100644
--- a/spec/system/unauthorised_user_signs_in_spec.rb
+++ b/spec/system/unauthorised_user_signs_in_spec.rb
@@ -16,6 +16,14 @@
def then_i_am_not_authorised
expect(page.status_code).to eq 401
- expect(page).to have_content("You are not authorised to access this service")
+ expect(page).to have_content(
+ "You cannot use the DfE Sign-in account for Test School to check the children's barred list"
+ )
+ expect(page).to have_link("sign out and start again", href: "/auth/dfe/sign-out?id_token_hint=abc123")
+
+ within(".govuk-header__content") do
+ expect(page).not_to have_link("Sign in")
+ expect(page).not_to have_link("Sign out")
+ end
end
end
diff --git a/spec/system/user_has_oauth_error_signing_in_spec.rb b/spec/system/user_has_oauth_error_signing_in_spec.rb
new file mode 100644
index 00000000..233d6ccd
--- /dev/null
+++ b/spec/system/user_has_oauth_error_signing_in_spec.rb
@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+RSpec.describe "DSI authentication", type: :system do
+ include ActivateFeaturesSteps
+ include AuthenticationSteps
+
+ before do
+ given_the_service_is_open
+ allow(Sentry).to receive(:capture_exception)
+ end
+
+ scenario "User has oauth error when signing in", test: :with_stubbed_auth do
+ given_dsi_auth_is_mocked_with_a_failure("invalid_credentials") do
+ when_i_visit_the_sign_in_page
+ and_click_the_dsi_sign_in_button
+ then_i_see_a_sign_in_error
+ end
+ end
+
+ scenario "User has sessionexpiry oauth error", test: :with_stubbed_auth do
+ given_dsi_auth_is_mocked_with_a_failure("sessionexpired") do
+ when_i_visit_the_sign_in_page
+ and_click_the_dsi_sign_in_button
+ then_i_am_redirected_to_sign_in
+ end
+ end
+
+ private
+
+ def then_i_see_a_sign_in_error
+ expect(page).to have_content "There was a problem signing you in. Please try again."
+ end
+
+ def then_i_am_redirected_to_sign_in
+ expect(page).to have_current_path(sign_in_path)
+ end
+end
diff --git a/spec/system/user_signs_in_spec.rb b/spec/system/user_signs_in_spec.rb
index 2c7da6ab..2be4667e 100644
--- a/spec/system/user_signs_in_spec.rb
+++ b/spec/system/user_signs_in_spec.rb
@@ -15,7 +15,12 @@
private
def then_i_am_signed_in
- within("header") { expect(page).to have_content "Sign out" }
+ within("header") do
+ expect(page).to have_link("Sign out")
+ sign_out_link = find_link("Sign out")
+ # Expect the token from mocked auth to be in the sign out link
+ expect(sign_out_link[:href]).to include "id_token_hint=abc123"
+ end
expect(DsiUser.count).to eq 1
expect(DsiUserSession.count).to eq 1
end
diff --git a/spec/system/user_signs_out_spec.rb b/spec/system/user_signs_out_spec.rb
new file mode 100644
index 00000000..3c2f2425
--- /dev/null
+++ b/spec/system/user_signs_out_spec.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+RSpec.describe "DSI authentication", type: :system do
+ include ActivateFeaturesSteps
+ include AuthenticationSteps
+
+ scenario "User signs out", test: :with_stubbed_auth do
+ given_the_service_is_open
+ and_i_am_signed_in_via_dsi
+ when_i_sign_out
+ then_i_am_redirected_to_the_sign_in_page
+ end
+
+ private
+
+ def when_i_sign_out
+ click_on "Sign out"
+ end
+
+ def then_i_am_redirected_to_the_sign_in_page
+ expect(page).to have_content "You are now signed out"
+ end
+end