From 6a31cbb12169e9e59a4045f102c1ca2887220f2c Mon Sep 17 00:00:00 2001 From: Steve Laing Date: Thu, 7 Sep 2023 16:10:22 +0100 Subject: [PATCH] Check the role info from DSI API when user logs in We don't do this if bypassing DSI. The first role to match the list of authorised roles will be recorded in the DsiUserSession along with org info. The absence of a valid role takes the user to the 'Not authorised' page. --- .../omniauth_callbacks_controller.rb | 13 ++++++++- .../check_records/sign_in_controller.rb | 3 ++ .../sign_in/not_authorised.html.erb | 5 ++++ config/routes/check_records.rb | 1 + .../check_records/authentication_steps.rb | 29 +++++++++++++++++-- .../unauthorised_user_signs_in_spec.rb | 21 ++++++++++++++ 6 files changed, 68 insertions(+), 4 deletions(-) create mode 100644 app/views/check_records/sign_in/not_authorised.html.erb create mode 100644 spec/system/check_records/unauthorised_user_signs_in_spec.rb diff --git a/app/controllers/check_records/omniauth_callbacks_controller.rb b/app/controllers/check_records/omniauth_callbacks_controller.rb index b79ff8ba..d3d3da32 100644 --- a/app/controllers/check_records/omniauth_callbacks_controller.rb +++ b/app/controllers/check_records/omniauth_callbacks_controller.rb @@ -4,7 +4,18 @@ class CheckRecords::OmniauthCallbacksController < ApplicationController protect_from_forgery except: :dfe_bypass def dfe - @dsi_user = DsiUser.create_or_update_from_dsi(request.env["omniauth.auth"]) + auth = request.env["omniauth.auth"] + + unless CheckRecords::DfESignIn.bypass? + role = DfESignInApi::GetUserAccessToService.new( + org_id: auth.extra.raw_info.organisation.id, + user_id: auth.uid, + ).call + + return redirect_to check_records_not_authorised_path unless role + end + + @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 diff --git a/app/controllers/check_records/sign_in_controller.rb b/app/controllers/check_records/sign_in_controller.rb index 6c42fd82..6f1aa5b9 100644 --- a/app/controllers/check_records/sign_in_controller.rb +++ b/app/controllers/check_records/sign_in_controller.rb @@ -5,5 +5,8 @@ class SignInController < CheckRecordsController def new end + + def not_authorised + end end end diff --git a/app/views/check_records/sign_in/not_authorised.html.erb b/app/views/check_records/sign_in/not_authorised.html.erb new file mode 100644 index 00000000..4a0a2df4 --- /dev/null +++ b/app/views/check_records/sign_in/not_authorised.html.erb @@ -0,0 +1,5 @@ +

Authorisation required

+

+ You are not authorised to access this service. +

+ diff --git a/config/routes/check_records.rb b/config/routes/check_records.rb index aead2f67..fefd52b8 100644 --- a/config/routes/check_records.rb +++ b/config/routes/check_records.rb @@ -6,6 +6,7 @@ get "/privacy", to: "static#privacy" get "/sign-in", to: "sign_in#new" + get "/not-authorised", to: "sign_in#not_authorised" get "/sign-out", to: "sign_out#new" get "/auth/dfe/callback", to: "omniauth_callbacks#dfe" diff --git a/spec/support/system/check_records/authentication_steps.rb b/spec/support/system/check_records/authentication_steps.rb index 399d039d..040d997f 100644 --- a/spec/support/system/check_records/authentication_steps.rb +++ b/spec/support/system/check_records/authentication_steps.rb @@ -1,13 +1,13 @@ module CheckRecords module AuthenticationSteps - def when_i_sign_in_via_dsi - given_dsi_auth_is_mocked + def when_i_sign_in_via_dsi(authorised: true) + given_dsi_auth_is_mocked(authorised:) when_i_visit_the_sign_in_page and_click_the_dsi_sign_in_button end alias_method :and_i_am_signed_in_via_dsi, :when_i_sign_in_via_dsi - def given_dsi_auth_is_mocked + def given_dsi_auth_is_mocked(authorised:) OmniAuth.config.mock_auth[:dfe] = OmniAuth::AuthHash.new( { provider: "dfe", @@ -16,9 +16,24 @@ def given_dsi_auth_is_mocked email: "test@example.com", first_name: "Test", last_name: "User" + }, + extra: { + raw_info: { + organisation: { + id: org_id, + } + } } } ) + + stub_request( + :get, + "#{ENV.fetch("DFE_SIGN_IN_API_BASE_URL")}/services/checkrecordteacher/organisations/#{org_id}/users/123456", + ).to_return_json( + status: 200, + body: { "roles" => [{ "code" => (authorised ? role_code : "Unauthorised_Role") }] }, + ) end def when_i_visit_the_sign_in_page @@ -28,5 +43,13 @@ def when_i_visit_the_sign_in_page def and_click_the_dsi_sign_in_button click_button "Sign in with DSI" end + + def org_id + "12345678-1234-1234-1234-123456789012" + end + + def role_code + ENV.fetch("DFE_SIGN_IN_API_ROLE_CODES").split(",").first + end end end diff --git a/spec/system/check_records/unauthorised_user_signs_in_spec.rb b/spec/system/check_records/unauthorised_user_signs_in_spec.rb new file mode 100644 index 00000000..f7ef199b --- /dev/null +++ b/spec/system/check_records/unauthorised_user_signs_in_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe "DSI authentication", host: :check_records do + include AuthorizationSteps + include CheckRecords::AuthenticationSteps + + scenario "Unauthorised user signs in via DfE Sign In", test: :with_stubbed_auth do + when_i_am_authorized_with_basic_auth + when_i_sign_in_via_dsi(authorised: false) + then_i_am_redirected_to_the_unauthorised_page + end + + private + + def then_i_am_redirected_to_the_unauthorised_page + expect(page).to have_current_path("/check-records/not-authorised") + expect(page).to have_content("You are not authorised to access this service") + end +end