Fixes #20425 - Handle OtpManager password access better

lib/foreman_tasks_core/otp_manager.rb

@@ -6,12 +6,15 @@ class OtpManager
     class << self
       def generate_otp(username)
         otp = SecureRandom.hex
-        @passwords ||= {}
-        @passwords[username] = otp.to_s
+        passwords[username] = otp.to_s
       end
 
       def drop_otp(username, password)
-        @passwords.delete(username) if @passwords[username] == password
+        passwords.delete(username) if passwords[username] == password
+      end
+
+      def passwords
+        @password ||= {}
       end
 
       def authenticate(hash)

test/unit/otp_manager_test.rb

@@ -3,54 +3,68 @@
 
 module ForemanTasksCore
   class OtpManagerTest < ActiveSupport::TestCase
+    class TestOtpManager < OtpManager
+      def self.reset!
+        @passwords = nil
+      end
+    end
+
+    before do
+      TestOtpManager.reset!
+    end
+
     let(:username) { 'myuser' }
     let(:password) { '123456789' }
     let(:base64)   { 'bXl1c2VyOjEyMzQ1Njc4OQ==' }
 
+    it 'it doesn\'t raise when no passwords were generated yet' do
+      assert_nil TestOtpManager.drop_otp('missing', 'password')
+    end
+
     it 'generates OTPs using SecureRandom.hex and converts them to strings' do
       otp = 4
       SecureRandom.stubs(:hex).returns(otp)
-      OtpManager.generate_otp(username).must_equal otp.to_s
+      TestOtpManager.generate_otp(username).must_equal otp.to_s
     end
 
     it 'removes OTP only when correct username and password is provided' do
-      otp = OtpManager.generate_otp(username)
-      OtpManager.drop_otp('wrong_username', 'wrong_password').must_be :nil?
-      OtpManager.drop_otp(username, 'wrong_password').must_be :nil?
-      OtpManager.drop_otp('wrong_username', otp).must_be :nil?
-      OtpManager.drop_otp(username, otp).must_equal otp
+      otp = TestOtpManager.generate_otp(username)
+      assert_nil TestOtpManager.drop_otp('wrong_username', 'wrong_password')
+      assert_nil TestOtpManager.drop_otp(username, 'wrong_password')
+      assert_nil TestOtpManager.drop_otp('wrong_username', otp)
+      TestOtpManager.drop_otp(username, otp).must_equal otp
     end
 
     it 'parses the hash correctly' do
       SecureRandom.stubs(:hex).returns(password)
-      OtpManager.expects(:drop_otp).with(username, password.to_s)
-      OtpManager.authenticate(base64)
+      TestOtpManager.expects(:drop_otp).with(username, password.to_s)
+      TestOtpManager.authenticate(base64)
     end
 
     it 'authenticates correctly' do
       SecureRandom.stubs(:hex).returns(password)
-      generated = OtpManager.generate_otp(username)
+      generated = TestOtpManager.generate_otp(username)
 
-      OtpManager.authenticate(base64).must_equal generated
+      TestOtpManager.authenticate(base64).must_equal generated
     end
 
     it 'OTPs can be used only once' do
       SecureRandom.stubs(:hex).returns(password)
-      generated = OtpManager.generate_otp(username)
+      generated = TestOtpManager.generate_otp(username)
 
-      OtpManager.authenticate(base64).must_equal generated
-      OtpManager.authenticate(base64).must_be :nil?
+      TestOtpManager.authenticate(base64).must_equal generated
+      assert_nil TestOtpManager.authenticate(base64)
     end
 
     it 'creates token from username and password correctly' do
-      OtpManager.tokenize(username, password).must_equal base64
+      TestOtpManager.tokenize(username, password).must_equal base64
     end
 
     it 'overwrites old OTP when generating a new one for the same username' do
-      old = OtpManager.generate_otp(username)
-      new = OtpManager.generate_otp(username)
-      OtpManager.drop_otp(username, old).must_be :nil?
-      OtpManager.drop_otp(username, new).must_equal new
+      old = TestOtpManager.generate_otp(username)
+      new = TestOtpManager.generate_otp(username)
+      assert_nil TestOtpManager.drop_otp(username, old)
+      TestOtpManager.drop_otp(username, new).must_equal new
     end
   end
 end