Merge pull request #2311 from alphagov/fix-use-of-instance-name

Fix use of INSTANCE_NAME to determine environment-specific behaviour

app/helpers/two_step_verification_helper.rb

@@ -28,8 +28,8 @@ def on_2sv_setup_journey
 
   def otp_secret_key_uri(user:, otp_secret_key:)
     issuer = I18n.t("devise.issuer")
-    if Rails.application.config.instance_name
-      issuer = "#{Rails.application.config.instance_name.titleize} #{issuer}"
+    unless GovukEnvironment.production?
+      issuer = "#{GovukEnvironment.name.titleize} #{issuer}"
     end
 
     issuer = ERB::Util.url_encode(issuer)

app/mailers/mailer_helper.rb

@@ -4,18 +4,18 @@ def email_from
   end
 
   def app_name
-    I18n.t("mailer.app_name", instance_name:)
+    if GovukEnvironment.production?
+      I18n.t("mailer.app_name.no_instance")
+    else
+      I18n.t("mailer.app_name.instance", instance_name: GovukEnvironment.name)
+    end
   end
 
   def email_from_address
-    if instance_name.present?
-      I18n.t("mailer.email_from_address.instance", instance_name: instance_name.parameterize)
+    if GovukEnvironment.production?
+      I18n.t("mailer.email_from_address.no_instance")
     else
-      I18n.t("mailer.email_from_address.no-instance")
+      I18n.t("mailer.email_from_address.instance", instance_name: GovukEnvironment.name.parameterize)
     end
   end
-
-  def instance_name
-    Rails.application.config.instance_name
-  end
 end

app/mailers/noisy_batch_invitation.rb

@@ -10,7 +10,7 @@ def make_noise(batch_invitation)
 
     user_count = batch_invitation.batch_invitation_users.count
     subject = "[SIGNON] #{@user.name} created a batch of #{user_count} users"
-    subject << " in #{instance_name}" if instance_name
+    subject << " in #{GovukEnvironment.name}" unless GovukEnvironment.production?
     view_mail(template_id, subject:)
   end
 end

app/mailers/user_mailer.rb

@@ -4,7 +4,7 @@ class UserMailer < Devise::Mailer
 
   default from: proc { email_from }
 
-  helper_method :suspension_time, :account_name, :instance_name, :locked_time, :unlock_time, :production?
+  helper_method :suspension_time, :account_name, :locked_time, :unlock_time
 
   def two_step_reset(user)
     @user = user
@@ -17,7 +17,7 @@ def two_step_changed(user)
   end
 
   def two_step_enabled(user)
-    prefix = "[#{Rails.application.config.instance_name.titleize}] " unless production?
+    prefix = "[#{GovukEnvironment.name.titleize}] " unless GovukEnvironment.production?
     @user = user
     view_mail(template_id, to: @user.email, subject: "#{prefix}2-step verification set up")
   end
@@ -119,10 +119,10 @@ def unlock_time
   end
 
   def account_name
-    if instance_name.present?
-      "#{instance_name} account"
-    else
+    if GovukEnvironment.production?
       "account"
+    else
+      "#{GovukEnvironment.name} account"
     end
   end
 
@@ -134,8 +134,4 @@ def subject_for(key)
       app_name:,
     )
   end
-
-  def production?
-    Rails.application.config.instance_name.blank?
-  end
 end

app/models/govuk_environment.rb

@@ -0,0 +1,13 @@
+class GovukEnvironment
+  def self.name
+    if Rails.env.development? || Rails.env.test?
+      "development"
+    else
+      ENV["INSTANCE_NAME"]
+    end
+  end
+
+  def self.production?
+    name == "production"
+  end
+end

app/models/user.rb

@@ -258,7 +258,7 @@ def need_two_step_verification?
   end
 
   def set_2sv_for_admin_roles
-    return if Rails.application.config.instance_name.present?
+    return unless GovukEnvironment.production?
 
     self.require_2sv = true if role_changed? && (admin? || superadmin? || organisation_admin? || super_organisation_admin?)
   end

app/views/user_mailer/notify_reset_password_disallowed_due_to_suspension.text.erb

@@ -1,7 +1,7 @@
 
 Your <%= t('department.name') %> Signon <%= account_name %>, for <%= @user.email %>, is suspended. You can't request a password reset on a suspended account.
 
-<% if instance_name.present? %>
+<% unless GovukEnvironment.production? %>
 <%= account_name.humanize %> suspensions do not suspend production accounts.
 <% end %>
 

app/views/user_mailer/suspension_notification.text.erb

@@ -2,7 +2,7 @@
 
 Your <%= t('department.name') %> Signon <%= account_name %>, for <%= @user.email %>, has now been suspended and you won't be able to access any <%= t('department.name') %> <%= account_name %> publishing applications.
 
-<% if instance_name.present? %>
+<% unless GovukEnvironment.production? %>
 <%= account_name.humanize %> suspensions do not suspend production accounts or remove access to <%= t('department.name') %> production applications.
 <% end %>
 

app/views/user_mailer/suspension_reminder.text.erb

@@ -2,7 +2,7 @@
 
 Your <%= t('department.name') %> Signon <%= account_name %>, for <%= @user.email %>, will be suspended <%= suspension_time %>. After suspension you won't be able to access any <%= t('department.name') %> <%= account_name %> publishing applications.
 
-<% if instance_name.present? %>
+<% unless GovukEnvironment.production? %>
 <%= account_name.humanize %> suspensions do not suspend production accounts or remove access to <%= t('department.name') %> production applications.
 <% end %>
 

app/views/user_mailer/two_step_enabled.text.erb

@@ -1,5 +1,5 @@
 You’ve successfully set up 2-step verification. This will make your Signon account more secure. Next time you sign in, you’ll need the phone you used during set up.
 
-<% unless production? %>
+<% unless GovukEnvironment.production? %>
 You’ll have to set up 2-step verification separately for your Production account.
 <% end %>

app/views/user_mailer/unlock_instructions.text.erb

@@ -1,6 +1,6 @@
 Your <%= t('department.name') %> Signon <%= account_name %>, for <%= @user.email %>, was locked at <%= locked_time %> because the wrong sign on details were entered too many times.
 
-<% if instance_name.present? %>
+<% unless GovukEnvironment.production? %>
   <%= account_name.humanize %> locks do not lock production accounts.
 <% end %>
 

config/initializers/devise.rb

@@ -1,5 +1,4 @@
 require "govuk_app_config"
-require_relative "instance_name"
 
 devise_config = Rails.application.config_for(:devise).symbolize_keys
 

config/locales/department_specific.en.yml

@@ -6,10 +6,12 @@ en:
   devise:
     issuer: "GOV.UK Signon"
   mailer:
-    app_name: "GOV.UK Signon %{instance_name}"
+    app_name:
+      instance: "GOV.UK Signon %{instance_name}"
+      no_instance: "GOV.UK Signon"
     email_from_address:
       instance: "noreply-signon-%{instance_name}@digital.cabinet-office.gov.uk"
-      no-instance: "noreply-signon@digital.cabinet-office.gov.uk"
+      no_instance: "noreply-signon@digital.cabinet-office.gov.uk"
 
   noisy_batch_invitation_mailer:
     to: "signon-alerts@digital.cabinet-office.gov.uk"

test/helpers/two_step_verification_helper_test.rb

@@ -18,12 +18,7 @@ class TwoStepVerificationHelperTest < ActionView::TestCase
 
     context "in production" do
       setup do
-        @old_instance_name = Rails.application.config.instance_name
-        Rails.application.config.instance_name = nil
-      end
-
-      teardown do
-        Rails.application.config.instance_name = @old_instance_name
+        GovukEnvironment.stubs(:production?).returns(true)
       end
 
       should "not include the environment name" do

test/models/govuk_environment_test.rb

@@ -0,0 +1,61 @@
+require "test_helper"
+
+class GovukEnvironmentTest < ActionMailer::TestCase
+  context ".name" do
+    context "when in Rails development environment" do
+      setup do
+        Rails.env.stubs(:development?).returns(true)
+        Rails.env.stubs(:test?).returns(false)
+      end
+
+      should "return 'development'" do
+        assert_equal "development", GovukEnvironment.name
+      end
+    end
+
+    context "when in Rails test environment" do
+      setup do
+        Rails.env.stubs(:development?).returns(false)
+        Rails.env.stubs(:test?).returns(true)
+      end
+
+      should "return 'development'" do
+        assert_equal "development", GovukEnvironment.name
+      end
+    end
+
+    context "when not in Rails development or test environment" do
+      setup do
+        Rails.env.stubs(:development?).returns(false)
+        Rails.env.stubs(:test?).returns(false)
+        ENV.stubs(:[]).with("INSTANCE_NAME").returns("instance-name")
+      end
+
+      should "return value of INSTANCE_NAME env var" do
+        assert_equal "instance-name", GovukEnvironment.name
+      end
+    end
+  end
+
+  context ".production?" do
+    context "when name is 'production'" do
+      setup do
+        GovukEnvironment.stubs(:name).returns("production")
+      end
+
+      should "return truthy" do
+        assert GovukEnvironment.production?
+      end
+    end
+
+    context "when name is not 'production'" do
+      setup do
+        GovukEnvironment.stubs(:name).returns("not-production")
+      end
+
+      should "return falsey" do
+        assert_not GovukEnvironment.production?
+      end
+    end
+  end
+end

test/models/noisy_batch_invitation_test.rb

@@ -34,32 +34,33 @@ class NoisyBatchInvitationTest < ActionMailer::TestCase
     end
   end
 
-  context "make_noise when instance_name has been set" do
+  context "make_noise in non-production environment" do
     setup do
-      Rails.application.config.stubs(:instance_name).returns("Test Fools")
+      GovukEnvironment.stubs(:production?).returns(false)
+      GovukEnvironment.stubs(:name).returns("Test Fools")
 
       user = create(:user, name: "Bob Loblaw")
       @batch_invitation = create(:batch_invitation, user:)
       create(:batch_invitation_user, batch_invitation: @batch_invitation)
       @email = NoisyBatchInvitation.make_noise(@batch_invitation).deliver_now
     end
 
-    should "from address should include the instance name" do
+    should "from address should include the environment name" do
       assert_match(/".* Signon Test Fools" <noreply-signon-test-fools@.*\.gov\.uk>/, @email[:from].to_s)
     end
   end
 
-  context "work correctly when no instance name is set" do
+  context "work correctly in production environment" do
     setup do
-      Rails.application.config.stubs(:instance_name).returns(nil)
+      GovukEnvironment.stubs(:production?).returns(true)
 
       user = create(:user, name: "Bob Loblaw")
       @batch_invitation = create(:batch_invitation, user:)
       create(:batch_invitation_user, batch_invitation: @batch_invitation)
       @email = NoisyBatchInvitation.make_noise(@batch_invitation).deliver_now
     end
 
-    should "from address should include the instance name" do
+    should "from address should not include the environment name" do
       assert_match(/".* Signon" <noreply-signon@.*\.gov\.uk>/, @email[:from].to_s)
     end
   end

test/models/user_mailer_test.rb

@@ -22,7 +22,8 @@ def assert_support_present_in_text(link_text, email = @email)
 
     context "in a non-production environment" do
       setup do
-        Rails.application.config.stubs(instance_name: "foobar")
+        GovukEnvironment.stubs(:production?).returns(false)
+        GovukEnvironment.stubs(:name).returns("foobar")
       end
 
       should "include the environment in the subject" do
@@ -36,7 +37,7 @@ def assert_support_present_in_text(link_text, email = @email)
 
     context "in the production environment" do
       setup do
-        Rails.application.config.stubs(instance_name: nil)
+        GovukEnvironment.stubs(:production?).returns(true)
       end
 
       should "not include the environment in the subject" do
@@ -119,7 +120,8 @@ def assert_support_present_in_text(link_text, email = @email)
 
   context "email a user to notify of suspension" do
     setup do
-      Rails.application.config.stubs(:instance_name).returns("test")
+      GovukEnvironment.stubs(:production?).returns(false)
+      GovukEnvironment.stubs(:name).returns("test")
       stub_user = stub(name: "User", email: "user@example.com")
       @email = UserMailer.suspension_notification(stub_user)
     end
@@ -137,9 +139,10 @@ def assert_support_present_in_text(link_text, email = @email)
     end
   end
 
-  context "on a named Signon instance" do
+  context "on a non-production Signon instance" do
     setup do
-      Rails.application.config.stubs(:instance_name).returns("test")
+      GovukEnvironment.stubs(:production?).returns(false)
+      GovukEnvironment.stubs(:name).returns("test")
       stub_user = stub(name: "User", email: "user@example.com")
       @email = UserMailer.suspension_reminder(stub_user, 3)
     end
@@ -155,7 +158,8 @@ def assert_support_present_in_text(link_text, email = @email)
 
   context "emailing a user to explain why their account is locked" do
     setup do
-      Rails.application.config.stubs(:instance_name).returns("test")
+      GovukEnvironment.stubs(:production?).returns(false)
+      GovukEnvironment.stubs(:name).returns("test")
       @the_time = Time.zone.now
       user = User.new(name: "User", email: "user@example.com", locked_at: @the_time)
       @email = UserMailer.unlock_instructions(user, "afaketoken")

test/models/user_test.rb

@@ -5,7 +5,7 @@ class UserTest < ActiveSupport::TestCase
   include ActiveJob::TestHelper
 
   def setup
-    Rails.application.config.stubs(instance_name: nil)
+    GovukEnvironment.stubs(:production?).returns(true)
 
     @user = create(:user)
   end
@@ -21,7 +21,8 @@ def setup
     end
 
     should "default to false for admins and superadmins in non-production" do
-      Rails.application.config.stubs(instance_name: "foobar")
+      GovukEnvironment.stubs(:production?).returns(false)
+      GovukEnvironment.stubs(:name).returns("foobar")
 
       assert_not create(:admin_user).require_2sv?
       assert_not create(:superadmin_user).require_2sv?