Use secure token in path instead of reg. identifier (#579)

We spotted when working on implementing the ability to order copy cards in the back-office that we would face an issue trying to reuse forms where the expectation was to find a transient registration using just the registration identifier.

For example, CBDU100 is an active registration. I then start a renewal so create a `RenewingRegistration` which sits in the `transient_registrations` collection. But then I need to take a copy card order for the existing active registration. That would create an `OrderCopyCardsRegistration` transient registration with the same registration identifier of CBDU100.

So how would the renewing journey or the order copy cards journey know which transient registration to pull if we only have the reg_identifier as an ID?

We solved this in WEX by having all transient registrations automatically creating a `token` (which is a Base58 24 character string) when first saved to the DB, and then using that in the URL to identify the record.

So this change, which touches loads of files in the codebase, actually is about switching to using the token rather than the registration identifier in our journeys.

It also brings the URL pattern inline with WEX, so `/{token}/stage/`. This resolves an issue we had in WEX and WCR with validations losing the ID (because it was at the end of the URL rather than the beginning) which then broke going back after a validation error.

app/controllers/waste_carriers_engine/address_forms_controller.rb

@@ -3,7 +3,7 @@
 module WasteCarriersEngine
   class AddressFormsController < FormsController
     def skip_to_manual_address
-      find_or_initialize_transient_registration(params[:reg_identifier])
+      find_or_initialize_transient_registration(params[:token])
 
       @transient_registration.skip_to_manual_address! if form_matches_state?
       redirect_to_correct_form

app/controllers/waste_carriers_engine/bank_transfer_forms_controller.rb

@@ -17,11 +17,5 @@ def create
     def set_up_finance_details
       @transient_registration.prepare_for_payment(:bank_transfer, current_user)
     end
-
-    def transient_registration_attributes
-      # TODO: Remvoe when default empty params
-      # Nothing to submit
-      params.fetch(:bank_transfer_form).permit
-    end
   end
 end

app/controllers/waste_carriers_engine/business_type_forms_controller.rb

@@ -13,7 +13,7 @@ def create
     private
 
     def transient_registration_attributes
-      params.fetch(:business_type_form, {}).permit(:business_type, :reg_identifier)
+      params.fetch(:business_type_form, {}).permit(:business_type, :token)
     end
   end
 end

app/controllers/waste_carriers_engine/copy_cards_forms_controller.rb

@@ -16,9 +16,9 @@ def transient_registration_attributes
       params.fetch(:copy_cards_form).permit(:temp_cards)
     end
 
-    def find_or_initialize_transient_registration(reg_identifier)
-      @transient_registration = OrderCopyCardsRegistration.where(reg_identifier: reg_identifier).first ||
-                                OrderCopyCardsRegistration.new(reg_identifier: reg_identifier)
+    def find_or_initialize_transient_registration(token)
+      @transient_registration = OrderCopyCardsRegistration.where(token: token).first ||
+                                OrderCopyCardsRegistration.new(reg_identifier: token)
     end
   end
 end

app/controllers/waste_carriers_engine/forms_controller.rb

@@ -6,21 +6,22 @@ class FormsController < ApplicationController
 
     before_action :authenticate_user!
     before_action :back_button_cache_buster
+    before_action :validate_token
 
     # Expects a form class name (eg BusinessTypeForm) and a snake_case name for the form (eg business_type_form)
     def new(form_class, form)
-      set_up_form(form_class, form, params[:reg_identifier], true)
+      set_up_form(form_class, form, params[:token], true)
     end
 
     # Expects a form class name (eg BusinessTypeForm) and a snake_case name for the form (eg business_type_form)
     def create(form_class, form)
-      return false unless set_up_form(form_class, form, params[form][:reg_identifier])
+      return false unless set_up_form(form_class, form, params[:token])
 
       submit_form(instance_variable_get("@#{form}"), transient_registration_attributes)
     end
 
     def go_back
-      find_or_initialize_transient_registration(params[:reg_identifier])
+      find_or_initialize_transient_registration(params[:token])
 
       @transient_registration.back! if form_matches_state?
       redirect_to_correct_form
@@ -34,15 +35,23 @@ def transient_registration_attributes
       params.permit
     end
 
-    def find_or_initialize_transient_registration(reg_identifier)
-      @transient_registration = TransientRegistration.where(reg_identifier: reg_identifier).first ||
-                                RenewingRegistration.new(reg_identifier: reg_identifier)
+    def validate_token
+      return redirect_to(page_path("invalid")) unless find_or_initialize_transient_registration(params[:token])
     end
 
+    # We're not really memoizing this instance variable here, so we don't think
+    # this cop is valid in this context
+    # rubocop:disable Naming/MemoizedInstanceVariableName
+    def find_or_initialize_transient_registration(token)
+      @transient_registration ||= TransientRegistration.where(token: token).first
+    end
+    # rubocop:enable Naming/MemoizedInstanceVariableName
+
     # Expects a form class name (eg BusinessTypeForm), a snake_case name for the form (eg business_type_form),
-    # and the reg_identifier param
-    def set_up_form(form_class, form, reg_identifier, get_request = false)
-      find_or_initialize_transient_registration(reg_identifier)
+    # and the token param
+    def set_up_form(form_class, form, token, get_request = false)
+      find_or_initialize_transient_registration(token)
+
       set_workflow_state if get_request
 
       return false unless setup_checks_pass?
@@ -68,10 +77,10 @@ def redirect_to_correct_form
       redirect_to form_path
     end
 
-    # Get the path based on the workflow state, with reg_identifier as params, ie:
-    # new_state_name_path/:reg_identifier
+    # Get the path based on the workflow state, with token as params, ie:
+    # new_state_name_path/:token
     def form_path
-      send("new_#{@transient_registration.workflow_state}_path".to_sym, @transient_registration.reg_identifier)
+      send("new_#{@transient_registration.workflow_state}_path".to_sym, @transient_registration.token)
     end
 
     def setup_checks_pass?

app/controllers/waste_carriers_engine/person_forms_controller.rb

@@ -11,7 +11,7 @@ def create(form_class, form)
     end
 
     def submit_and_add_another(form_class, form)
-      return unless set_up_form(form_class, form, params[form][:reg_identifier])
+      return unless set_up_form(form_class, form, params[:token])
 
       form_instance_variable = instance_variable_get("@#{form}")
 
@@ -25,7 +25,7 @@ def submit_and_add_another(form_class, form)
     end
 
     def delete_person(form_class, form)
-      return unless set_up_form(form_class, form, params[:reg_identifier])
+      return unless set_up_form(form_class, form, params[:token])
 
       respond_to do |format|
         # Check if there are any matches first, to avoid a Mongoid error

app/controllers/waste_carriers_engine/postcode_forms_controller.rb

@@ -3,7 +3,7 @@
 module WasteCarriersEngine
   class PostcodeFormsController < FormsController
     def skip_to_manual_address
-      find_or_initialize_transient_registration(params[:reg_identifier])
+      find_or_initialize_transient_registration(params[:token])
 
       @transient_registration.skip_to_manual_address! if form_matches_state?
       redirect_to_correct_form

app/controllers/waste_carriers_engine/renewal_start_forms_controller.rb

@@ -9,5 +9,12 @@ def new
     def create
       super(RenewalStartForm, "renewal_start_form")
     end
+
+    private
+
+    def find_or_initialize_transient_registration(token)
+      @transient_registration = RenewingRegistration.where(token: token).first ||
+                                RenewingRegistration.new(reg_identifier: token)
+    end
   end
 end

app/controllers/waste_carriers_engine/tier_check_forms_controller.rb

@@ -13,7 +13,7 @@ def create
     private
 
     def transient_registration_attributes
-      params.require(:tier_check_form).permit(:temp_tier_check)
+      params.fetch(:tier_check_form, {}).permit(:temp_tier_check)
     end
   end
 end

app/controllers/waste_carriers_engine/worldpay_forms_controller.rb

@@ -46,7 +46,7 @@ def prepare_for_payment
     end
 
     def respond_to_acceptable_payment(action)
-      return unless valid_transient_registration?(params[:reg_identifier])
+      return unless valid_transient_registration?(params[:token])
 
       if response_is_valid?(action, params)
         log_and_send_worldpay_response(true, action)
@@ -60,7 +60,7 @@ def respond_to_acceptable_payment(action)
     end
 
     def respond_to_unsuccessful_payment(action)
-      return unless valid_transient_registration?(params[:reg_identifier])
+      return unless valid_transient_registration?(params[:token])
 
       if response_is_valid?(action, params)
         log_and_send_worldpay_response(true, action)
@@ -73,8 +73,8 @@ def respond_to_unsuccessful_payment(action)
       go_back
     end
 
-    def valid_transient_registration?(reg_identifier)
-      find_or_initialize_transient_registration(reg_identifier)
+    def valid_transient_registration?(token)
+      find_or_initialize_transient_registration(token)
       setup_checks_pass?
     end
 

app/forms/waste_carriers_engine/base_form.rb

@@ -9,9 +9,17 @@ class BaseForm
 
     attr_reader :transient_registration
 
-    delegate :reg_identifier, to: :transient_registration
+    delegate :token, :reg_identifier, to: :transient_registration
+
+    # If the record is new, and not yet persisted (which it is when the start
+    # page is first submitted) then we have nothing to validate hence the check
+    validates :token, presence: true, if: -> { transient_registration&.persisted? }
+    validates(
+      :reg_identifier,
+      "waste_carriers_engine/reg_identifier": true,
+      if: -> { transient_registration&.persisted? }
+    )
 
-    validates :reg_identifier, "waste_carriers_engine/reg_identifier": true
     validate :transient_registration_valid?
 
     define_model_callbacks :initialize

app/forms/waste_carriers_engine/renewal_complete_form.rb

@@ -22,7 +22,7 @@ def initialize(transient_registration)
     private
 
     def build_certificate_link
-      registration = Registration.where(reg_identifier: reg_identifier).first
+      registration = @transient_registration.registration
       return unless registration.present?
 
       id = registration.id

app/forms/waste_carriers_engine/renewal_start_form.rb

@@ -11,5 +11,11 @@ def submit(_params)
 
       super(attributes)
     end
+
+    def find_or_initialize_transient_registration(token)
+      # TODO: Downtime at deploy when releasing token?
+      @transient_registration = RenewingRegistration.where(token: token).first ||
+                                RenewingRegistration.new(reg_identifier: token)
+    end
   end
 end

app/models/concerns/waste_carriers_engine/can_have_secure_token.rb

@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+module WasteCarriersEngine
+  module CanHaveSecureToken
+    extend ActiveSupport::Concern
+    include Mongoid::Document
+
+    included do
+      field :token, type: String
+
+      before_create :generate_unique_secure_token
+
+      def generate_unique_secure_token
+        self.token = SecureTokenService.run
+      end
+    end
+  end
+end

app/models/waste_carriers_engine/transient_registration.rb

@@ -7,6 +7,7 @@ class TransientRegistration
     include CanCheckRegistrationStatus
     include CanFilterConvictionStatus
     include CanHaveRegistrationAttributes
+    include CanHaveSecureToken
     include CanStripWhitespace
 
     store_in collection: "transient_registrations"

app/services/waste_carriers_engine/renewal_completion_service.rb

@@ -78,6 +78,7 @@ def send_confirmation_email
     def copy_data_from_transient_registration
       registration_attributes = @registration.attributes.except("_id", "financeDetails", "past_registrations")
       renewal_attributes = @transient_registration.attributes.except("_id",
+                                                                     "token",
                                                                      "financeDetails",
                                                                      "temp_cards",
                                                                      "temp_company_postcode",

app/services/waste_carriers_engine/worldpay_service.rb

@@ -19,7 +19,7 @@ def prepare_for_payment
       reference = parse_response(response)
 
       if reference.present?
-        worldpay_url_service = WorldpayUrlService.new(@transient_registration.reg_identifier, reference[:link])
+        worldpay_url_service = WorldpayUrlService.new(@transient_registration.token, reference[:link])
         url = worldpay_url_service.format_link
 
         {

app/services/waste_carriers_engine/worldpay_url_service.rb

@@ -4,8 +4,8 @@ module WasteCarriersEngine
   # Builds the URL we use to redirect users to Worldpay.
   # This should include URLs for our own service, which Worldpay will redirect users to after payment.
   class WorldpayUrlService
-    def initialize(reg_identifier, base_url)
-      @reg_identifier = reg_identifier
+    def initialize(token, base_url)
+      @token = token
       @base_url = base_url
     end
 
@@ -29,7 +29,7 @@ def redirect_url_for(action)
     def build_path_for(action)
       path = "#{action}_worldpay_forms_path"
       url = [Rails.configuration.host,
-             WasteCarriersEngine::Engine.routes.url_helpers.public_send(path, @reg_identifier)]
+             WasteCarriersEngine::Engine.routes.url_helpers.public_send(path, token: @token)]
       CGI.escape(url.join)
     end
   end

app/views/waste_carriers_engine/bank_transfer_forms/new.html.erb

@@ -1,4 +1,4 @@
-<%= render("waste_carriers_engine/shared/back", back_path: back_bank_transfer_forms_path(@bank_transfer_form.reg_identifier)) %>
+<%= render("waste_carriers_engine/shared/back", back_path: back_bank_transfer_forms_path(@bank_transfer_form.token)) %>
 
 <div class="text">
   <%= form_for(@bank_transfer_form) do |f| %>
@@ -105,7 +105,6 @@
       </strong>
     </div>
 
-    <%= f.hidden_field :reg_identifier, value: @bank_transfer_form.reg_identifier %>
     <div class="form-group">
       <%= f.submit t(".next_button"), class: "button" %>
     </div>

app/views/waste_carriers_engine/business_type_forms/new.html.erb

@@ -1,4 +1,4 @@
-<%= render("waste_carriers_engine/shared/back", back_path: back_business_type_forms_path(@business_type_form.reg_identifier)) %>
+<%= render("waste_carriers_engine/shared/back", back_path: back_business_type_forms_path(@business_type_form.token)) %>
 
 <div class="text">
   <%= form_for(@business_type_form) do |f| %>
@@ -45,7 +45,6 @@
 
     <p><%= t(".help") %></p>
 
-    <%= f.hidden_field :reg_identifier, value: @business_type_form.reg_identifier %>
     <div class="form-group">
       <%= f.submit t(".next_button"), class: "button" %>
     </div>

app/views/waste_carriers_engine/cannot_renew_company_no_change_forms/new.html.erb

@@ -1,4 +1,4 @@
-<%= render("waste_carriers_engine/shared/back", back_path: back_cannot_renew_company_no_change_forms_path(@cannot_renew_company_no_change_form.reg_identifier)) %>
+<%= render("waste_carriers_engine/shared/back", back_path: back_cannot_renew_company_no_change_forms_path(@cannot_renew_company_no_change_form.token)) %>
 
 <div class="text">
   <h1 class="heading-large"><%= t(".heading", reg_identifier: @cannot_renew_company_no_change_form.reg_identifier) %></h1>

app/views/waste_carriers_engine/cannot_renew_lower_tier_forms/new.html.erb

@@ -1,4 +1,4 @@
-<%= render("waste_carriers_engine/shared/back", back_path: back_cannot_renew_lower_tier_forms_path(@cannot_renew_lower_tier_form.reg_identifier)) %>
+<%= render("waste_carriers_engine/shared/back", back_path: back_cannot_renew_lower_tier_forms_path(@cannot_renew_lower_tier_form.token)) %>
 
 <div class="text">
   <h1 class="heading-large"><%= t(".heading", reg_identifier: @cannot_renew_lower_tier_form.reg_identifier) %></h1>

app/views/waste_carriers_engine/cannot_renew_type_change_forms/new.html.erb

@@ -1,4 +1,4 @@
-<%= render("waste_carriers_engine/shared/back", back_path: back_cannot_renew_type_change_forms_path(@cannot_renew_type_change_form.reg_identifier)) %>
+<%= render("waste_carriers_engine/shared/back", back_path: back_cannot_renew_type_change_forms_path(@cannot_renew_type_change_form.token)) %>
 
 <div class="text">
   <h1 class="heading-large"><%= t(".heading", reg_identifier: @cannot_renew_type_change_form.reg_identifier) %></h1>

app/views/waste_carriers_engine/cards_forms/new.html.erb

@@ -1,4 +1,4 @@
-<%= render("waste_carriers_engine/shared/back", back_path: back_cards_forms_path(@cards_form.reg_identifier)) %>
+<%= render("waste_carriers_engine/shared/back", back_path: back_cards_forms_path(@cards_form.token)) %>
 
 <div class="text">
   <%= form_for(@cards_form) do |f| %>
@@ -30,8 +30,6 @@
       </fieldset>
     </div>
 
-    <%= f.hidden_field :reg_identifier, value: @cards_form.reg_identifier %>
-
     <div class="form-group">
       <%= f.submit t(".next_button"), class: "button" %>
     </div>

app/views/waste_carriers_engine/cbd_type_forms/new.html.erb

@@ -1,4 +1,4 @@
-<%= render("waste_carriers_engine/shared/back", back_path: back_cbd_type_forms_path(@cbd_type_form.reg_identifier)) %>
+<%= render("waste_carriers_engine/shared/back", back_path: back_cbd_type_forms_path(@cbd_type_form.token)) %>
 
 <div class="text">
   <%= form_for(@cbd_type_form) do |f| %>
@@ -49,7 +49,6 @@
       </details>
     </div>
 
-    <%= f.hidden_field :reg_identifier, value: @cbd_type_form.reg_identifier %>
     <div class="form-group">
       <%= f.submit t(".next_button"), class: "button" %>
     </div>

app/views/waste_carriers_engine/check_your_answers_forms/new.html.erb

@@ -1,4 +1,4 @@
-<%= render("waste_carriers_engine/shared/back", back_path: back_check_your_answers_forms_path(@check_your_answers_form.reg_identifier)) %>
+<%= render("waste_carriers_engine/shared/back", back_path: back_check_your_answers_forms_path(@check_your_answers_form.token)) %>
 
 <div class="text">
   <% if @check_your_answers_form.errors.any? %>
@@ -114,7 +114,6 @@
 
       <hr>
 
-      <%= f.hidden_field :reg_identifier, value: @check_your_answers_form.reg_identifier %>
       <div class="form-group">
         <%= f.submit t(".next_button"), class: "button" %>
       </div>

app/views/waste_carriers_engine/company_address_forms/new.html.erb

@@ -1,4 +1,4 @@
-<%= render("waste_carriers_engine/shared/back", back_path: back_company_address_forms_path(@company_address_form.reg_identifier)) %>
+<%= render("waste_carriers_engine/shared/back", back_path: back_company_address_forms_path(@company_address_form.token)) %>
 
 <div class="text">
   <%= form_for(@company_address_form) do |f| %>
@@ -20,7 +20,6 @@
       <%= link_to(t(".manual_address_link"), skip_to_manual_address_company_address_forms_path(@company_address_form.reg_identifier)) %>
     </div>
 
-    <%= f.hidden_field :reg_identifier, value: @company_address_form.reg_identifier %>
     <div class="form-group">
       <%= f.submit t(".next_button"), class: "button" %>
     </div>