refactor: simplify basic auth implementation

- Inline before_action condition with a lambda instead of a separate
  private method
- Add .to_s guard on credentials in secure_compare
- Rename validate! to warn_if_basic_auth_misconfigured for clarity
- Add enabled? alias for basic_auth_enabled?
- Merge attr_accessor declarations into a single line
- Fix long lines in controller specs

Author: HassaanSiraj

app/controllers/letter_opener_web/application_controller.rb

@@ -2,21 +2,17 @@
 
 module LetterOpenerWeb
   class ApplicationController < ActionController::Base
-    before_action :enforce_basic_auth, if: :basic_auth_configured?
+    before_action :enforce_basic_auth, if: -> { LetterOpenerWeb.config.enabled? }
 
     protect_from_forgery with: :exception, unless: -> { Rails.configuration.try(:api_only) }
 
     private
 
-    def basic_auth_configured?
-      LetterOpenerWeb.config.basic_auth_enabled?
-    end
-
     def enforce_basic_auth
       authenticate_or_request_with_http_basic('Letter Opener Web') do |username, password|
         config = LetterOpenerWeb.config
-        ActiveSupport::SecurityUtils.secure_compare(username, config.username) &
-          ActiveSupport::SecurityUtils.secure_compare(password, config.password)
+        ActiveSupport::SecurityUtils.secure_compare(username, config.username.to_s) &
+          ActiveSupport::SecurityUtils.secure_compare(password, config.password.to_s)
       end
     end
   end

lib/letter_opener_web.rb

@@ -6,15 +6,14 @@
 
 module LetterOpenerWeb
   class Config
-    attr_accessor :letters_location
-    attr_accessor :authentication_enabled
-    attr_accessor :username, :password
+    attr_accessor :letters_location, :authentication_enabled, :username, :password
 
     def basic_auth_enabled?
       authentication_enabled && username.present? && password.present?
     end
+    alias enabled? basic_auth_enabled?
 
-    def validate!
+    def warn_if_basic_auth_misconfigured
       return unless authentication_enabled && (username.blank? || password.blank?)
 
       Rails.logger.warn('[LetterOpenerWeb] authentication_enabled is true but username or password is blank. ' \
@@ -31,7 +30,7 @@ def self.config
 
   def self.configure
     yield config if block_given?
-    config.validate!
+    config.warn_if_basic_auth_misconfigured
   end
 
   def self.reset!

spec/controllers/letter_opener_web/letters_controller_spec.rb

@@ -25,21 +25,24 @@
 
       it 'returns 200 with correct credentials' do
         allow(LetterOpenerWeb::Letter).to receive(:search)
-        request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials('admin', 'secret')
+        request.env['HTTP_AUTHORIZATION'] =
+          ActionController::HttpAuthentication::Basic.encode_credentials('admin', 'secret')
         get :index
         expect(response.status).to eq(200)
       end
 
       it 'returns 401 with wrong password' do
         allow(LetterOpenerWeb::Letter).to receive(:search)
-        request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials('admin', 'wrong')
+        request.env['HTTP_AUTHORIZATION'] =
+          ActionController::HttpAuthentication::Basic.encode_credentials('admin', 'wrong')
         get :index
         expect(response.status).to eq(401)
       end
 
       it 'returns 401 with wrong username' do
         allow(LetterOpenerWeb::Letter).to receive(:search)
-        request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials('wrong', 'secret')
+        request.env['HTTP_AUTHORIZATION'] =
+          ActionController::HttpAuthentication::Basic.encode_credentials('wrong', 'secret')
         get :index
         expect(response.status).to eq(401)
       end