From 1f3624e86edd9fadabdf18a60fec46daf83719b4 Mon Sep 17 00:00:00 2001 From: Joseph Kempster Date: Thu, 7 Oct 2021 13:44:18 +0100 Subject: [PATCH] Move authenticate step into wizard This is mostly shared between the GiT app and TTA app, so moving it into the gem. --- Gemfile.lock | 2 +- app/models/callbacks/wizard.rb | 2 +- app/models/events/wizard.rb | 2 +- app/models/mailing_list/wizard.rb | 2 +- app/models/wizard/steps/authenticate.rb | 56 --------- config/locales/en.yml | 7 -- spec/models/callbacks/wizard_spec.rb | 2 +- spec/models/events/wizard_spec.rb | 2 +- spec/models/mailing_list/wizard_spec.rb | 2 +- spec/models/wizard/steps/authenticate_spec.rb | 113 ------------------ spec/requests/event_steps_controller_spec.rb | 2 +- 11 files changed, 8 insertions(+), 184 deletions(-) delete mode 100644 app/models/wizard/steps/authenticate.rb delete mode 100644 spec/models/wizard/steps/authenticate_spec.rb diff --git a/Gemfile.lock b/Gemfile.lock index fdff8a7ef2..f582d0c347 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,6 +1,6 @@ GIT remote: https://github.com/DFE-Digital/dfe_wizard.git - revision: 4ab1ef8f0cf5c09d64ac0329001595f8bf4520e7 + revision: 3b8fb44c760a0e342d89ed7a4824733a224646b0 branch: move-extra-controller-code specs: dfe_wizard (0.1.0) diff --git a/app/models/callbacks/wizard.rb b/app/models/callbacks/wizard.rb index c75c84427b..f99beb35f7 100644 --- a/app/models/callbacks/wizard.rb +++ b/app/models/callbacks/wizard.rb @@ -7,7 +7,7 @@ class Wizard < ::DFEWizard::Base self.steps = [ Steps::PersonalDetails, Steps::MatchbackFailed, - ::Wizard::Steps::Authenticate, + ::DFEWizard::Steps::Authenticate, Steps::Callback, Steps::TalkingPoints, Steps::PrivacyPolicy, diff --git a/app/models/events/wizard.rb b/app/models/events/wizard.rb index f6019e34f1..e9a829d061 100644 --- a/app/models/events/wizard.rb +++ b/app/models/events/wizard.rb @@ -6,7 +6,7 @@ class Wizard < ::DFEWizard::Base self.steps = [ Steps::PersonalDetails, - ::Wizard::Steps::Authenticate, + ::DFEWizard::Steps::Authenticate, Steps::ContactDetails, Steps::FurtherDetails, Steps::PersonalisedUpdates, diff --git a/app/models/mailing_list/wizard.rb b/app/models/mailing_list/wizard.rb index 93573827b9..eeacf26f9e 100644 --- a/app/models/mailing_list/wizard.rb +++ b/app/models/mailing_list/wizard.rb @@ -6,7 +6,7 @@ class Wizard < ::DFEWizard::Base self.steps = [ Steps::Name, - ::Wizard::Steps::Authenticate, + ::DFEWizard::Steps::Authenticate, Steps::AlreadySubscribed, Steps::DegreeStatus, Steps::TeacherTraining, diff --git a/app/models/wizard/steps/authenticate.rb b/app/models/wizard/steps/authenticate.rb deleted file mode 100644 index cc3a35691d..0000000000 --- a/app/models/wizard/steps/authenticate.rb +++ /dev/null @@ -1,56 +0,0 @@ -module Wizard - module Steps - class Authenticate < ::DFEWizard::Step - include ActiveModel::Dirty - - IDENTITY_ATTRS = %i[email first_name last_name date_of_birth].freeze - - attribute :timed_one_time_password - - validates :timed_one_time_password, presence: true, length: { is: 6, message: :invalid }, - format: { with: /\A[0-9]*\z/, message: :invalid } - validate :timed_one_time_password_is_correct, if: :perform_api_check? - - before_validation if: :timed_one_time_password do - self.timed_one_time_password = timed_one_time_password.to_s.strip - end - - def skipped? - @store["authenticate"] != true - end - - def export - {} - end - - def candidate_identity_data - @store.fetch(IDENTITY_ATTRS).compact.transform_keys do |k| - k.camelize(:lower).to_sym - end - end - - private - - def perform_api_check? - timed_one_time_password_valid? && !@wizard.access_token_used? - end - - def timed_one_time_password_valid? - self.class.validators_on(:timed_one_time_password).each do |validator| - validator.validate_each(self, :timed_one_time_password, timed_one_time_password) - end - errors.none? - end - - def timed_one_time_password_is_correct - request = GetIntoTeachingApiClient::ExistingCandidateRequest.new(candidate_identity_data) - if timed_one_time_password_changed? - clear_attribute_changes(%i[timed_one_time_password]) - @wizard.process_access_token(timed_one_time_password, request) - end - rescue GetIntoTeachingApiClient::ApiError - errors.add(:timed_one_time_password, :wrong_code) - end - end - end -end diff --git a/config/locales/en.yml b/config/locales/en.yml index 55af79fc57..d8091684f1 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -288,13 +288,6 @@ en: subject: blank: Select a subject - wizard/steps/authenticate: - attributes: - timed_one_time_password: - blank: Enter the verification code sent to your email address - invalid: The verification code should be 6 digits - wrong_code: Please enter the latest verification code sent to your email address - events/search: attributes: type: diff --git a/spec/models/callbacks/wizard_spec.rb b/spec/models/callbacks/wizard_spec.rb index d41c663cde..6a2519f845 100644 --- a/spec/models/callbacks/wizard_spec.rb +++ b/spec/models/callbacks/wizard_spec.rb @@ -23,7 +23,7 @@ is_expected.to eql [ Callbacks::Steps::PersonalDetails, Callbacks::Steps::MatchbackFailed, - ::Wizard::Steps::Authenticate, + ::DFEWizard::Steps::Authenticate, Callbacks::Steps::Callback, Callbacks::Steps::TalkingPoints, Callbacks::Steps::PrivacyPolicy, diff --git a/spec/models/events/wizard_spec.rb b/spec/models/events/wizard_spec.rb index 7733474175..830c235193 100644 --- a/spec/models/events/wizard_spec.rb +++ b/spec/models/events/wizard_spec.rb @@ -20,7 +20,7 @@ it do is_expected.to eql [ Events::Steps::PersonalDetails, - ::Wizard::Steps::Authenticate, + ::DFEWizard::Steps::Authenticate, Events::Steps::ContactDetails, Events::Steps::FurtherDetails, Events::Steps::PersonalisedUpdates, diff --git a/spec/models/mailing_list/wizard_spec.rb b/spec/models/mailing_list/wizard_spec.rb index a205e908b0..351602d69c 100644 --- a/spec/models/mailing_list/wizard_spec.rb +++ b/spec/models/mailing_list/wizard_spec.rb @@ -19,7 +19,7 @@ it do is_expected.to eql [ MailingList::Steps::Name, - ::Wizard::Steps::Authenticate, + ::DFEWizard::Steps::Authenticate, MailingList::Steps::AlreadySubscribed, MailingList::Steps::DegreeStatus, MailingList::Steps::TeacherTraining, diff --git a/spec/models/wizard/steps/authenticate_spec.rb b/spec/models/wizard/steps/authenticate_spec.rb deleted file mode 100644 index 038ef17572..0000000000 --- a/spec/models/wizard/steps/authenticate_spec.rb +++ /dev/null @@ -1,113 +0,0 @@ -require "rails_helper" - -describe Wizard::Steps::Authenticate do - include_context "with wizard step" - it_behaves_like "a with wizard step" - - before { allow(wizard).to receive(:exchange_access_token) { {} } } - - it { is_expected.to respond_to :timed_one_time_password } - - context "validations" do - before { instance.valid? } - subject { instance.errors.messages } - it { is_expected.to include(:timed_one_time_password) } - end - - context "timed one time password" do - it { is_expected.to allow_value("000000").for :timed_one_time_password } - it { is_expected.to allow_value(" 123456").for :timed_one_time_password } - it { is_expected.not_to allow_value("abc123").for :timed_one_time_password } - it { is_expected.not_to allow_value("1234567").for :timed_one_time_password } - it { is_expected.not_to allow_value("12345").for :timed_one_time_password } - end - - describe "#skipped?" do - it "returns true if authenticate is false or nil" do - wizardstore["authenticate"] = false - expect(subject).to be_skipped - wizardstore["authenticate"] = nil - expect(subject).to be_skipped - end - - it "returns false if authenticate is true" do - wizardstore["authenticate"] = true - expect(subject).not_to be_skipped - end - end - - describe "#export" do - it "returns an empty hash" do - allow_any_instance_of(described_class).to receive(:skipped?) { false } - subject.timed_one_time_password = "123456" - expect(subject.export).to be_empty - end - end - - describe "#save" do - before do - subject.timed_one_time_password = totp - wizardstore["email"] = "email@address.com" - wizardstore["first_name"] = "First" - wizardstore["last_name"] = "Last" - end - - let(:totp) { "123456" } - let(:request) do - GetIntoTeachingApiClient::ExistingCandidateRequest.new( - email: wizardstore["email"], - firstName: wizardstore["first_name"], - lastName: wizardstore["last_name"], - ) - end - - context "when invalid" do - it "does not attempt to call the API" do - subject.timed_one_time_password = nil - subject.save - expect { subject.save }.not_to raise_error - end - end - - context "when valid" do - it "attempts to call the API exactly once for each valid timed_one_time_password" do - expect(wizard).to receive(:exchange_access_token).with(totp, request).and_raise(GetIntoTeachingApiClient::ApiError).once - expect(wizard).to receive(:exchange_access_token).with("000000", request).once - subject.timed_one_time_password = totp - subject.save - subject.save - subject.timed_one_time_password = "000000" - subject.save - end - - it "does not call the API on validation if already authenticated" do - expect(wizard).to receive(:access_token_used?) { true } - expect(wizard).not_to receive(:exchange_access_token) - subject.timed_one_time_password = totp - subject.valid? - end - - it "throws an error if #exchange_access_token is not defined" do - expect(wizard).to receive(:exchange_access_token).and_call_original - expect { subject.save }.to raise_error(DFEWizard::AccessTokenNotSupportedError) - end - end - - context "when TOTP is correct" do - it "updates the store with the response" do - response = GetIntoTeachingApiClient::TeachingEventAddAttendee.new(candidateId: "abc123") - expect(wizard).to receive(:exchange_access_token).with(totp, request) { response } - subject.save - expect(wizardstore["candidate_id"]).to eq(response.candidate_id) - end - end - - context "when TOTP is incorrect" do - it "is marked as invalid" do - expect(wizard).to receive(:exchange_access_token).with(totp, request) - .and_raise(GetIntoTeachingApiClient::ApiError) - expect(subject).to be_invalid - end - end - end -end diff --git a/spec/requests/event_steps_controller_spec.rb b/spec/requests/event_steps_controller_spec.rb index 8714e7293b..a4305b7985 100644 --- a/spec/requests/event_steps_controller_spec.rb +++ b/spec/requests/event_steps_controller_spec.rb @@ -147,7 +147,7 @@ def perform_request allow_any_instance_of(Events::Steps::PersonalDetails).to \ receive(:valid?).and_return true - allow_any_instance_of(Wizard::Steps::Authenticate).to \ + allow_any_instance_of(::DFEWizard::Steps::Authenticate).to \ receive(:valid?).and_return true allow_any_instance_of(Events::Steps::ContactDetails).to \