Add coverage for UnreservedUsernameValidator (#25590)

app/validators/unreserved_username_validator.rb

@@ -11,16 +11,31 @@ class UnreservedUsernameValidator < ActiveModel::Validator
 
   private
 
-  def pam_controlled?
+  def reserved_username?
-    return false unless Devise.pam_authentication && Devise.pam_controlled_service
+    pam_username_reserved? || settings_username_reserved?
-
-    Rpam2.account(Devise.pam_controlled_service, @username).present?
   end
 
-  def reserved_username?
+  def pam_username_reserved?
-    return true if pam_controlled?
+    pam_controlled? && pam_reserves_username?
-    return false unless Setting.reserved_usernames
+  end
 
+  def pam_controlled?
+    Devise.pam_authentication && Devise.pam_controlled_service
+  end
+
+  def pam_reserves_username?
+    Rpam2.account(Devise.pam_controlled_service, @username)
+  end
+
+  def settings_username_reserved?
+    settings_has_reserved_usernames? && settings_reserves_username?
+  end
+
+  def settings_has_reserved_usernames?
+    Setting.reserved_usernames.present?
+  end
+
+  def settings_reserves_username?
     Setting.reserved_usernames.include?(@username.downcase)
   end
 end

spec/validators/unreserved_username_validator_spec.rb

@@ -2,41 +2,118 @@
 
 require 'rails_helper'
 
-RSpec.describe UnreservedUsernameValidator, type: :validator do
+describe UnreservedUsernameValidator do
-  describe '#validate' do
+  let(:record_class) do
-    before do
+    Class.new do
-      allow(validator).to receive(:reserved_username?) { reserved_username }
+      include ActiveModel::Validations
-      validator.validate(account)
+      attr_accessor :username
+
+      validates_with UnreservedUsernameValidator
     end
+  end
+  let(:record) { record_class.new }
 
-    let(:validator) { described_class.new }
+  describe '#validate' do
-    let(:account)   { instance_double(Account, username: username, errors: errors) }
+    context 'when username is nil' do
-    let(:errors) { instance_double(ActiveModel::Errors, add: nil) }
+      it 'does not add errors' do
+        record.username = nil
 
-    context 'when @username is blank?' do
+        expect(record).to be_valid
-      let(:username) { nil }
+        expect(record.errors).to be_empty
-
-      it 'not calls errors.add' do
-        expect(errors).to_not have_received(:add).with(:username, any_args)
       end
     end
 
-    context 'when @username is not blank?' do
+    context 'when PAM is enabled' do
-      let(:username) { 'f' }
+      before do
+        allow(Devise).to receive(:pam_authentication).and_return(true)
+      end
 
-      context 'with reserved_username?' do
+      context 'with a pam service available' do
-        let(:reserved_username) { true }
+        let(:service) { double }
+        let(:pam_class) do
+          Class.new do
+            def self.account(service, username); end
+          end
+        end
 
-        it 'calls errors.add' do
+        before do
-          expect(errors).to have_received(:add).with(:username, :reserved)
+          stub_const('Rpam2', pam_class)
+          allow(Devise).to receive(:pam_controlled_service).and_return(service)
+        end
+
+        context 'when the account exists' do
+          before do
+            allow(Rpam2).to receive(:account).with(service, 'username').and_return(true)
+          end
+
+          it 'adds errors to the record' do
+            record.username = 'username'
+
+            expect(record).to_not be_valid
+            expect(record.errors.first.attribute).to eq(:username)
+            expect(record.errors.first.type).to eq(:reserved)
+          end
+        end
+
+        context 'when the account does not exist' do
+          before do
+            allow(Rpam2).to receive(:account).with(service, 'username').and_return(false)
+          end
+
+          it 'does not add errors to the record' do
+            record.username = 'username'
+
+            expect(record).to be_valid
+            expect(record.errors).to be_empty
+          end
         end
       end
 
-      context 'when username is not reserved' do
+      context 'without a pam service' do
-        let(:reserved_username) { false }
+        before do
+          allow(Devise).to receive(:pam_controlled_service).and_return(false)
+        end
 
-        it 'not calls errors.add' do
+        context 'when there are not any reserved usernames' do
-          expect(errors).to_not have_received(:add).with(:username, any_args)
+          before do
+            stub_reserved_usernames(nil)
+          end
+
+          it 'does not add errors to the record' do
+            record.username = 'username'
+
+            expect(record).to be_valid
+            expect(record.errors).to be_empty
+          end
+        end
+
+        context 'when there are reserved usernames' do
+          before do
+            stub_reserved_usernames(%w(alice bob))
+          end
+
+          context 'when the username is reserved' do
+            it 'adds errors to the record' do
+              record.username = 'alice'
+
+              expect(record).to_not be_valid
+              expect(record.errors.first.attribute).to eq(:username)
+              expect(record.errors.first.type).to eq(:reserved)
+            end
+          end
+
+          context 'when the username is not reserved' do
+            it 'does not add errors to the record' do
+              record.username = 'chris'
+
+              expect(record).to be_valid
+              expect(record.errors).to be_empty
+            end
+          end
+        end
+
+        def stub_reserved_usernames(value)
+          allow(Setting).to receive(:[]).with('reserved_usernames').and_return(value)
         end
       end
     end