diff --git a/Gemfile b/Gemfile index 61ad48ceb7..b650176cf0 100644 --- a/Gemfile +++ b/Gemfile @@ -25,6 +25,7 @@ gem "libreconv" gem "matrix" gem "okcomputer" gem "omniauth-azure-activedirectory-v2" +gem "omniauth_openid_connect" gem "omniauth-rails_csrf_protection" gem "pagy" gem "pg" @@ -77,6 +78,7 @@ group :test do gem "capybara" gem "climate_control" gem "cuprite" + gem "pry" gem "rspec" gem "rspec-rails" gem "shoulda-matchers" diff --git a/Gemfile.lock b/Gemfile.lock index 32b5b5c5cf..2683ed45e4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -109,11 +109,13 @@ GEM tzinfo (~> 2.0, >= 2.0.5) addressable (2.8.7) public_suffix (>= 2.0.2, < 7.0) + aes_key_wrap (1.1.0) amazing_print (1.6.0) annotate (3.2.0) activerecord (>= 3.2, < 8.0) rake (>= 10.4, < 14.0) ast (2.4.2) + attr_required (1.0.2) azure-storage-blob (2.0.3) azure-storage-common (~> 2.0) nokogiri (~> 1, >= 1.10.8) @@ -125,6 +127,7 @@ GEM base64 (0.2.0) bcrypt (3.1.20) bigdecimal (3.1.8) + bindata (2.5.0) bindex (0.8.1) bootsnap (1.18.4) msgpack (~> 1.2) @@ -142,6 +145,7 @@ GEM cgi (0.4.1) choice (0.2.0) climate_control (1.2.0) + coderay (1.1.3) combine_pdf (1.0.26) matrix ruby-rc4 (>= 0.1.5) @@ -295,6 +299,11 @@ GEM jsbundling-rails (1.3.1) railties (>= 6.0.0) json (2.7.2) + json-jwt (1.15.3.1) + activesupport (>= 4.2) + aes_key_wrap + bindata + httpclient jwt (2.9.0) base64 language_server-protocol (3.17.0.3) @@ -366,6 +375,20 @@ GEM omniauth-rails_csrf_protection (1.0.2) actionpack (>= 4.2) omniauth (~> 2.0) + omniauth_openid_connect (0.6.1) + omniauth (>= 1.9, < 3) + openid_connect (~> 1.1) + openid_connect (1.4.2) + activemodel + attr_required (>= 1.0.0) + json-jwt (>= 1.15.0) + net-smtp + rack-oauth2 (~> 1.21) + swd (~> 1.3) + tzinfo + validate_email + validate_url + webfinger (~> 1.2) orm_adapter (0.5.0) os (1.1.4) pagy (9.0.9) @@ -386,6 +409,9 @@ GEM activesupport (>= 7.0.0) rack railties (>= 7.0.0) + pry (0.14.2) + coderay (~> 1.1) + method_source (~> 1.0) psych (5.1.2) stringio public_suffix (6.0.1) @@ -398,6 +424,12 @@ GEM rack (2.2.9) rack-attack (6.7.0) rack (>= 1.0, < 4) + rack-oauth2 (1.21.3) + activesupport + attr_required + httpclient + json-jwt (>= 1.11.0) + rack (>= 2.1.0) rack-protection (3.2.0) base64 (>= 0.1.0) rack (~> 2.2, >= 2.2.4) @@ -570,6 +602,10 @@ GEM version_gem (~> 1.1, >= 1.1.1) stringio (3.1.1) strscan (3.1.0) + swd (1.3.0) + activesupport (>= 3) + attr_required (>= 0.0.5) + httpclient (>= 2.4) syntax_tree (6.2.0) prettier_print (>= 1.2.0) syntax_tree-haml (4.0.3) @@ -595,6 +631,9 @@ GEM unf_ext (0.0.9.1) unicode-display_width (2.5.0) useragent (0.16.10) + validate_email (0.1.6) + activemodel (>= 3.0) + mail (>= 2.2.5) validate_url (1.0.15) activemodel (>= 3.0.0) public_suffix @@ -610,6 +649,9 @@ GEM activemodel (>= 6.0.0) bindex (>= 0.4.0) railties (>= 6.0.0) + webfinger (1.2.0) + activesupport + httpclient (>= 2.4) webmock (3.23.1) addressable (>= 2.8.0) crack (>= 0.3.2) @@ -666,11 +708,13 @@ DEPENDENCIES okcomputer omniauth-azure-activedirectory-v2 omniauth-rails_csrf_protection + omniauth_openid_connect pagy pg prawn prettier_print propshaft + pry puma pundit rack-attack diff --git a/adr/00010-govuk-one-login.md b/adr/00010-govuk-one-login.md index a8838ba028..ecfd84e1f4 100644 --- a/adr/00010-govuk-one-login.md +++ b/adr/00010-govuk-one-login.md @@ -24,9 +24,10 @@ Before we can make the technical changes to adopt GOV.UK One Login we will need ## Consequences -We will adopt GOV.UK One Login using OAuth via the [Omniauth Gem]. +We will adopt GOV.UK One Login using OAuth via the [Omniauth Gem] & [Omiauth OpenId Connect Gem]. We will use the shared DfE [sector identifier]. [Omniauth Gem]: https://github.com/omniauth/omniauth +[Omiauth OpenId Connect Gem]: https://github.com/omniauth/omniauth_openid_connect [sector identifier]: https://docs.sign-in.service.gov.uk/before-integrating/choose-your-sector-identifier/ diff --git a/app/controllers/teachers/omniauth_callbacks_controller.rb b/app/controllers/teachers/omniauth_callbacks_controller.rb new file mode 100644 index 0000000000..9055365c65 --- /dev/null +++ b/app/controllers/teachers/omniauth_callbacks_controller.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +class Teachers::OmniauthCallbacksController < ApplicationController + def gov_one + auth = request.env["omniauth.auth"] + email = auth&.info&.email + gov_one_id = auth&.uid + + session[:id_token] = auth&.credentials&.id_token + + teacher = + FindOrCreateTeacherFromGovOne.call( + email:, + gov_one_id:, + eligibility_check_id: session[:eligibility_check_id], + ) + + return error_redirect unless teacher + + sign_in_and_redirect teacher + end + + def failure + error_redirect + end + + private + + def error_redirect + return if teacher_signed_in? + + flash[:alert] = "There was a problem signing in. Please try again." + redirect_to new_teacher_session_path + end + + def after_sign_in_path_for(resource) + stored_location_for(resource) || teacher_interface_root_path + end +end diff --git a/app/helpers/gov_one_helper.rb b/app/helpers/gov_one_helper.rb new file mode 100644 index 0000000000..e8afaf2e74 --- /dev/null +++ b/app/helpers/gov_one_helper.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module GovOneHelper + def logout_uri + params = { + post_logout_redirect_uri: destroy_teacher_session_url, + id_token_hint: session[:id_token], + } + + uri = URI.parse("#{Rails.configuration.gov_one.base_uri}logout") + uri.query = URI.encode_www_form(params) + + uri.to_s + end +end diff --git a/app/models/teacher.rb b/app/models/teacher.rb index 883041f114..784d4c8fc1 100644 --- a/app/models/teacher.rb +++ b/app/models/teacher.rb @@ -18,11 +18,13 @@ # uuid :uuid not null # created_at :datetime not null # updated_at :datetime not null +# gov_one_id :string # # Indexes # # index_teacher_on_lower_email (lower((email)::text)) UNIQUE # index_teachers_on_canonical_email (canonical_email) +# index_teachers_on_gov_one_id (gov_one_id) UNIQUE # index_teachers_on_uuid (uuid) UNIQUE # class Teacher < ApplicationRecord diff --git a/app/services/find_or_create_teacher_from_gov_one.rb b/app/services/find_or_create_teacher_from_gov_one.rb new file mode 100644 index 0000000000..9507c173a0 --- /dev/null +++ b/app/services/find_or_create_teacher_from_gov_one.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +class FindOrCreateTeacherFromGovOne + include ServicePattern + + attr_reader :teacher + + def initialize(email:, gov_one_id:, eligibility_check_id:) + @email = email + @gov_one_id = gov_one_id + @eligibility_check_id = eligibility_check_id + end + + def call + ActiveRecord::Base.transaction do + find_or_create_teacher! + + create_application_form! if teacher_requires_application_form? + end + + teacher + rescue StandardError => e + Sentry.capture_exception(e) + + nil + end + + private + + attr_reader :email, :gov_one_id, :eligibility_check_id + + def find_or_create_teacher! + @teacher = + Teacher.find_by(gov_one_id:) || Teacher.find_by(email:) || + Teacher.create!(email:) + + teacher.update!(gov_one_id:) if teacher.gov_one_id.nil? + end + + def create_application_form! + if valid_eligibility_check? + ApplicationFormFactory.call(teacher:, region: eligibility_check.region) + end + end + + def valid_eligibility_check? + eligibility_check.present? && eligibility_check.region.present? && + eligibility_check.country.eligibility_enabled? + end + + def eligibility_check + @eligibility_check ||= EligibilityCheck.find_by(id: eligibility_check_id) + end + + def teacher_requires_application_form? + teacher.persisted? && teacher.application_form.nil? + end +end diff --git a/app/views/shared/_header.html.erb b/app/views/shared/_header.html.erb index 5d47ffeeb4..32d87b1b9c 100644 --- a/app/views/shared/_header.html.erb +++ b/app/views/shared/_header.html.erb @@ -49,6 +49,9 @@ <% when "teacher" %> <% if current_teacher %> <% header.with_navigation_item(text: "Sign out", href: main_app.destroy_teacher_session_path) %> + <% if FeatureFlags::FeatureFlag.active?(:gov_one_applicant_login) %> + <% header.with_navigation_item(text: "Sign out with GovOne", href: logout_uri) %> + <% end %> <% end %> <% end %> <% end %> diff --git a/app/views/teachers/registrations/new.html.erb b/app/views/teachers/registrations/new.html.erb index cb44b6110f..fc715cde1a 100644 --- a/app/views/teachers/registrations/new.html.erb +++ b/app/views/teachers/registrations/new.html.erb @@ -10,3 +10,9 @@ <%= f.govuk_email_field :email, autocomplete: "email", label: { size: "m" } %> <%= f.govuk_submit %> <% end %> + +<% if FeatureFlags::FeatureFlag.active?(:gov_one_applicant_login) %> +

Gov One Login

+ + <%= button_to "Sign up in with Gov One", "/teacher/auth/gov_one", class: "govuk-button" %> +<% end %> diff --git a/app/views/teachers/sessions/new.html.erb b/app/views/teachers/sessions/new.html.erb index 8a57e5ed91..6726986a02 100644 --- a/app/views/teachers/sessions/new.html.erb +++ b/app/views/teachers/sessions/new.html.erb @@ -10,3 +10,9 @@ <%= f.govuk_submit %> <% end %> + +<% if FeatureFlags::FeatureFlag.active?(:gov_one_applicant_login) %> +

Gov One Login

+ + <%= button_to "Sign in with Gov One", "/teacher/auth/gov_one", class: "govuk-button" %> +<% end %> diff --git a/config/analytics_blocklist.yml b/config/analytics_blocklist.yml index 929b22b009..3e26aeb27a 100644 --- a/config/analytics_blocklist.yml +++ b/config/analytics_blocklist.yml @@ -88,6 +88,7 @@ - note :teachers: - access_your_teaching_qualifications_url + - gov_one_id :timeline_events: - age_range_max - age_range_min diff --git a/config/application.rb b/config/application.rb index f53e6b9896..22bfa2ecfe 100644 --- a/config/application.rb +++ b/config/application.rb @@ -54,5 +54,6 @@ class Application < Rails::Application config.action_mailer.deliver_later_queue_name = "mailer" config.dqt = config_for(:dqt) + config.gov_one = config_for(:gov_one) end end diff --git a/config/feature_flags.yml b/config/feature_flags.yml index 43ec01d362..d8c0d6b9cf 100644 --- a/config/feature_flags.yml +++ b/config/feature_flags.yml @@ -25,6 +25,10 @@ feature_flags: author: Thomas Leese description: Enable suitability records and matching. + gov_one_applicant_login: + author: Hassan Mir + description: Allow teacher applicants to sign in using GovOne Login + teacher_applications: author: Thomas Leese description: > diff --git a/config/gov_one.yml b/config/gov_one.yml new file mode 100644 index 0000000000..72ab8db4ed --- /dev/null +++ b/config/gov_one.yml @@ -0,0 +1,14 @@ +development: + base_uri: <%= ENV.fetch("GOV_ONE_OAUTH_BASE_URI", "https://oidc.integration.account.gov.uk/") %> + client_id: <%= ENV["GOV_ONE_OAUTH_CLIENT_ID"] %> + base_62_private_key: <%= ENV["GOV_ONE_OAUTH_BASE64_PRIVATE_KEY"] %> + +production: + base_uri: <%= ENV["GOV_ONE_OAUTH_BASE_URI"] %> + client_id: <%= ENV["GOV_ONE_OAUTH_CLIENT_ID"] %> + base_62_private_key: <%= ENV["GOV_ONE_OAUTH_BASE64_PRIVATE_KEY"] %> + +test: + base_uri: https://oidc.integration.account.gov.uk/ + client_id: test + base_62_private_key: test diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index eb765be8d9..32f9bbb57e 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -2,3 +2,74 @@ OmniAuth.config.allowed_request_methods = %i[post get] OmniAuth.config.silence_get_warning = true + +# We had to place the GovOne OmniAuth setup in this way +# since devise does not allow multi model omniauth by default. +# Source: https://github.com/heartcombo/devise/wiki/OmniAuth-with-multiple-models +# +# This configuration ensures that a POST request to /teacher/auth/gov_one +# redirects the user to Gov One authorization endpoint and on success returns +# them back to our callback URL with all auth details accessible via request.env["omniauth.auth"] +# inside the controller action (teachers/omniauth_callbacks#gov_one). +onelogin_sign_in_issuer_uri = + if Rails.configuration.gov_one.base_uri.present? + URI(Rails.configuration.gov_one.base_uri) + end + +callback_path = "/teacher/auth/gov_one/callback" + +jwks_uri = + if onelogin_sign_in_issuer_uri.present? + "#{onelogin_sign_in_issuer_uri&.to_s}.well-known/jwks.json" + end + +gov_one_redirect_uri = + if HostingEnvironment.development? + "http://localhost:3000#{callback_path}" + else + "https://#{HostingEnvironment.host}#{callback_path}" + end + +end_session_endpoint = + ( + if onelogin_sign_in_issuer_uri.present? + "#{onelogin_sign_in_issuer_uri&.to_s}logout" + end + ) + +client_secret = + if Rails.configuration.gov_one.base_62_private_key.present? && + !Rails.env.test? + OpenSSL::PKey::RSA.new( + Base64.decode64(Rails.configuration.gov_one.base_62_private_key), + ) + end + +Rails.application.config.middleware.use OmniAuth::Builder do + provider :openid_connect, + { + name: :gov_one, + callback_path:, + scope: %i[openid email], + response_type: :code, + issuer: onelogin_sign_in_issuer_uri&.to_s, + path_prefix: "/teacher/auth", + client_auth_method: "jwt_bearer", + client_options: { + port: nil, + scheme: "https", + host: onelogin_sign_in_issuer_uri&.host, + identifier: Rails.configuration.gov_one.client_id, + secret: client_secret, + redirect_uri: gov_one_redirect_uri, + authorization_endpoint: "/authorize", + jwks_uri:, + userinfo_endpoint: "/userinfo", + end_session_endpoint:, + }, + } + + on_failure do |env| + Teachers::OmniauthCallbacksController.action(:failure).call(env) + end +end diff --git a/config/routes.rb b/config/routes.rb index a0b017258d..cd8543cd0c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -447,6 +447,8 @@ } devise_scope :teacher do + get "teacher/auth/gov_one/callback" => "teachers/omniauth_callbacks#gov_one" + get "/teacher/sign_in_or_sign_up", to: "teachers/sessions#new_or_create", as: "create_or_new_teacher_session" diff --git a/db/migrate/20240910153044_add_gov_one_id_to_teacher_record.rb b/db/migrate/20240910153044_add_gov_one_id_to_teacher_record.rb new file mode 100644 index 0000000000..78733d7516 --- /dev/null +++ b/db/migrate/20240910153044_add_gov_one_id_to_teacher_record.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class AddGovOneIdToTeacherRecord < ActiveRecord::Migration[7.2] + def change + add_column :teachers, :gov_one_id, :string + add_index :teachers, :gov_one_id, unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index eb159c3077..88b8e23634 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_07_17_141207) do +ActiveRecord::Schema[7.2].define(version: 2024_09_10_153044) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -533,8 +533,10 @@ t.text "canonical_email", default: "", null: false t.string "access_your_teaching_qualifications_url" t.text "email_domain", default: "", null: false + t.string "gov_one_id" t.index "lower((email)::text)", name: "index_teacher_on_lower_email", unique: true t.index ["canonical_email"], name: "index_teachers_on_canonical_email" + t.index ["gov_one_id"], name: "index_teachers_on_gov_one_id", unique: true t.index ["uuid"], name: "index_teachers_on_uuid", unique: true end diff --git a/docs/diagram.pdf b/docs/diagram.pdf index e56f197cb1..37f7fed9c0 100644 Binary files a/docs/diagram.pdf and b/docs/diagram.pdf differ diff --git a/spec/factories/teachers.rb b/spec/factories/teachers.rb index 97e76343a1..aaa59b3454 100644 --- a/spec/factories/teachers.rb +++ b/spec/factories/teachers.rb @@ -18,11 +18,13 @@ # uuid :uuid not null # created_at :datetime not null # updated_at :datetime not null +# gov_one_id :string # # Indexes # # index_teacher_on_lower_email (lower((email)::text)) UNIQUE # index_teachers_on_canonical_email (canonical_email) +# index_teachers_on_gov_one_id (gov_one_id) UNIQUE # index_teachers_on_uuid (uuid) UNIQUE # FactoryBot.define do diff --git a/spec/models/teacher_spec.rb b/spec/models/teacher_spec.rb index dc40cd39c1..69ca5b6cd7 100644 --- a/spec/models/teacher_spec.rb +++ b/spec/models/teacher_spec.rb @@ -18,11 +18,13 @@ # uuid :uuid not null # created_at :datetime not null # updated_at :datetime not null +# gov_one_id :string # # Indexes # # index_teacher_on_lower_email (lower((email)::text)) UNIQUE # index_teachers_on_canonical_email (canonical_email) +# index_teachers_on_gov_one_id (gov_one_id) UNIQUE # index_teachers_on_uuid (uuid) UNIQUE # require "rails_helper" diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 227f31a46b..d2fd7cc613 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -87,3 +87,5 @@ end Dir[Rails.root.join("spec/support/**/*.rb")].sort.each { |f| require f } + +OmniAuth.config.test_mode = true diff --git a/spec/requests/teachers/omniauth_callbacks/gov_one_spec.rb b/spec/requests/teachers/omniauth_callbacks/gov_one_spec.rb new file mode 100644 index 0000000000..93e04def5a --- /dev/null +++ b/spec/requests/teachers/omniauth_callbacks/gov_one_spec.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe "/teacher/auth/gov_one/callback", type: :request do + subject(:gov_one_callback) { get "/teacher/auth/gov_one/callback" } + + let(:omniauth_hash) do + OmniAuth::AuthHash.new( + "uid" => gov_one_id, + "info" => { + "email" => email, + }, + "credentials" => { + "id_token" => "99999", + }, + ) + end + + let(:gov_one_id) { "123456789" } + let(:email) { "test@example.com" } + + before { OmniAuth.config.mock_auth[:default] = omniauth_hash } + + it "generates a new teacher record" do + expect { gov_one_callback }.to change(Teacher, :count).by(1) + end + + it "redirects the teacher to the teacher interface" do + gov_one_callback + + expect(response).to redirect_to(teacher_interface_root_path) + end + + it "sets the id_token session" do + gov_one_callback + + expect(request.session[:id_token]).to eq("99999") + end + + context "when there is an existing teacher for the same identity" do + before { create :teacher, email:, gov_one_id: } + + it "does not generate a new teacher record with an application" do + expect { gov_one_callback }.not_to change(Teacher, :count) + end + end + + context "when there is an existing eligibility_check_id in session" do + let(:eligibility_check) { create :eligibility_check, :eligible } + + before do + allow_any_instance_of(ActionDispatch::Request).to receive(:session) do + OpenStruct.new(id: 1, eligibility_check_id: eligibility_check.id) + end + end + + it "generates a new teacher record with an application" do + expect { gov_one_callback }.to change(Teacher, :count).by(1) + + application = Teacher.last.application_forms.first + expect(application).not_to be_nil + expect(application.region.country.code).to eq( + eligibility_check.country_code, + ) + end + end + + context "when no auth info is provided" do + let(:omniauth_hash) { OmniAuth::AuthHash.new({}) } + + it "does not generate a new teacher record with an application" do + expect { gov_one_callback }.not_to change(Teacher, :count) + end + + it "redirects the user back to the sign in page" do + gov_one_callback + + expect(response).to redirect_to(new_teacher_session_path) + expect(request.flash[:alert]).to eq( + "There was a problem signing in. Please try again.", + ) + end + end +end diff --git a/spec/services/find_or_create_teacher_from_gov_one_spec.rb b/spec/services/find_or_create_teacher_from_gov_one_spec.rb new file mode 100644 index 0000000000..7b3379b590 --- /dev/null +++ b/spec/services/find_or_create_teacher_from_gov_one_spec.rb @@ -0,0 +1,109 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe FindOrCreateTeacherFromGovOne do + subject(:call) do + described_class.call(email:, gov_one_id:, eligibility_check_id:) + end + + let(:email) { "test@example.com" } + let(:gov_one_id) { "123456789" } + let(:eligibility_check) { nil } + let(:eligibility_check_id) { eligibility_check&.id } + + it "creates a teacher record" do + expect { call }.to change(Teacher, :count).by(1) + end + + it "ensures the teacher record has the email and gov_one_id provided" do + teacher = call + + expect(teacher).to have_attributes(email:, gov_one_id:) + end + + context "when eligibility_check_id is provided" do + context "with it being eligible" do + let(:eligibility_check) { create :eligibility_check, :eligible } + + it "generates an application form for the teacher record" do + teacher = call + + expect(teacher.application_forms).not_to be_empty + expect(teacher.application_forms.first.region.country.code).to eq( + eligibility_check.country_code, + ) + end + end + + context "with it not being eligible" do + let(:eligibility_check) { create :eligibility_check, :ineligible } + + it "does not generate an application form for the teacher record" do + teacher = call + + expect(teacher.application_forms).to be_empty + end + end + end + + context "when a teacher record with the same email and gov_one_id exists" do + let!(:existing_teacher) { create :teacher, email:, gov_one_id: } + + it "does not generate a new teacher record" do + expect { call }.not_to change(Teacher, :count) + end + + it "returns the existing teacher record" do + expect(call).to eq(existing_teacher) + end + end + + context "when a teacher record with the same email exists without a gov_one_id" do + let!(:existing_teacher) { create :teacher, email: } + + it "does not generate a new teacher record" do + expect { call }.not_to change(Teacher, :count) + end + + it "returns the existing teacher record" do + expect(call).to eq(existing_teacher) + end + + it "sets the gov_one_id on the existing teacher record" do + call + + expect(existing_teacher.reload.gov_one_id).to eq(gov_one_id) + end + end + + context "when a teacher record exists with gov_one_id but a different email" do + let!(:existing_teacher) do + create :teacher, email: "differentemail@example.com", gov_one_id: + end + + it "does not generate a new teacher record" do + expect { call }.not_to change(Teacher, :count) + end + + it "returns the teacher record matching the gov one id" do + expect(call).to eq(existing_teacher) + end + end + + context "when two teacher records exist, one matching gov_one_id and another the email" do + let!(:existing_teacher_matching_id) do + create :teacher, email: "differentemail@example.com", gov_one_id: + end + + before { create :teacher, email:, gov_one_id: "other-id" } + + it "does not generate a new teacher record" do + expect { call }.not_to change(Teacher, :count) + end + + it "returns the teacher record matching the gov one id as priority" do + expect(call).to eq(existing_teacher_matching_id) + end + end +end diff --git a/spec/system/support_interface/staff_spec.rb b/spec/system/support_interface/staff_spec.rb index 605fcbbe9e..6d53cbe4fe 100644 --- a/spec/system/support_interface/staff_spec.rb +++ b/spec/system/support_interface/staff_spec.rb @@ -27,9 +27,7 @@ then_i_see_the_invited_staff_user then_i_sign_out - when_i_visit_the_invitation_email_with_azure_ad_enabled - # TODO: Add back in once Azure Login Page working on tests again. - # then_i_am_taken_to_the_azure_login_page + when_i_visit_the_invitation_email_with_azure_ad_enabled_i_see_the_link_to_access end it "allows inviting a user with Azure active directory deactivated" do @@ -90,12 +88,10 @@ def when_i_visit_the_staff_page visit support_interface_staff_index_path end - def when_i_visit_the_invitation_email_with_azure_ad_enabled + def when_i_visit_the_invitation_email_with_azure_ad_enabled_i_see_the_link_to_access message = ActionMailer::Base.deliveries.first uri = URI.parse(URI.extract(message.body.raw_source).second) expect(uri.path).to eq("/staff/auth/azure_activedirectory_v2") - # TODO: Add back in once Azure Login Page working on tests again. - # visit uri.path.to_s end def when_i_visit_the_invitation_email_with_azure_ad_disabled @@ -187,8 +183,4 @@ def and_i_submit_the_edit_form def then_i_see_the_changed_permission expect(page).not_to have_content("Support console access\tNo") end - - def then_i_am_taken_to_the_azure_login_page - expect(page.current_url).to include("https://login.microsoftonline.com/") - end end