From 7755f9b381c007ce98e0858473a9f29f1cd25311 Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Mon, 5 Nov 2018 09:11:01 -0500 Subject: Read ingress passwords/API keys from encrypted credentials Fall back to ENV for people who prefer that approach. --- app/controllers/action_mailbox/base_controller.rb | 24 ++++++++++++-- .../ingresses/mailgun/inbound_emails_controller.rb | 38 ++++++++++++---------- .../mandrill/inbound_emails_controller.rb | 29 +++++++++-------- .../ingresses/postfix/inbound_emails_controller.rb | 5 +-- .../sendgrid/inbound_emails_controller.rb | 5 +-- lib/action_mailbox.rb | 1 + lib/action_mailbox/engine.rb | 1 + .../amazon/inbound_emails_controller_test.rb | 2 ++ .../mailgun/inbound_emails_controller_test.rb | 10 +++--- .../mandrill/inbound_emails_controller_test.rb | 9 +++-- .../postfix/inbound_emails_controller_test.rb | 18 ++-------- .../sendgrid/inbound_emails_controller_test.rb | 15 ++------- test/test_helper.rb | 19 ++++++++--- 13 files changed, 91 insertions(+), 85 deletions(-) diff --git a/app/controllers/action_mailbox/base_controller.rb b/app/controllers/action_mailbox/base_controller.rb index d3846b06e1..a2f7eb4b61 100644 --- a/app/controllers/action_mailbox/base_controller.rb +++ b/app/controllers/action_mailbox/base_controller.rb @@ -1,15 +1,33 @@ class ActionMailbox::BaseController < ActionController::Base skip_forgery_protection + before_action :ensure_configured + private - def authenticate - if username.present? && password.present? - http_basic_authenticate_or_request_with username: username, password: password, realm: "Action Mailbox" + def ensure_configured + unless ActionMailbox.ingress == ingress_name + head :not_found + end + end + + def ingress_name + self.class.name[/^ActionMailbox::Ingresses::(.*?)::/, 1].underscore.to_sym + end + + + def authenticate_by_password + if password.present? + http_basic_authenticate_or_request_with username: "actionmailbox", password: password, realm: "Action Mailbox" else raise ArgumentError, "Missing required ingress credentials" end end + def password + Rails.application.credentials.dig(:action_mailbox, :ingress_password) || ENV["RAILS_INBOUND_EMAIL_PASSWORD"] + end + + # TODO: Extract to ActionController::HttpAuthentication def http_basic_authenticate_or_request_with(username:, password:, realm: nil) authenticate_or_request_with_http_basic(realm || "Application") do |given_username, given_password| diff --git a/app/controllers/action_mailbox/ingresses/mailgun/inbound_emails_controller.rb b/app/controllers/action_mailbox/ingresses/mailgun/inbound_emails_controller.rb index c7e53b07f4..0b763dcf18 100644 --- a/app/controllers/action_mailbox/ingresses/mailgun/inbound_emails_controller.rb +++ b/app/controllers/action_mailbox/ingresses/mailgun/inbound_emails_controller.rb @@ -11,21 +11,30 @@ class ActionMailbox::Ingresses::Mailgun::InboundEmailsController < ActionMailbox end def authenticated? - Authenticator.new( - timestamp: params.require(:timestamp), - token: params.require(:token), - signature: params.require(:signature) - ).authenticated? + if key.present? + Authenticator.new( + key: key, + timestamp: params.require(:timestamp), + token: params.require(:token), + signature: params.require(:signature) + ).authenticated? + else + raise ArgumentError, <<~MESSAGE.squish + Missing required Mailgun API key. Set action_mailbox.mailgun_api_key in your application's + encrypted credentials or provide the MAILGUN_INGRESS_API_KEY environment variable. + MESSAGE + end end - class Authenticator - cattr_accessor :key - attr_reader :timestamp, :token, :signature + def key + Rails.application.credentials.dig(:action_mailbox, :mailgun_api_key) || ENV["MAILGUN_INGRESS_API_KEY"] + end - def initialize(timestamp:, token:, signature:) - @timestamp, @token, @signature = Integer(timestamp), token, signature + class Authenticator + attr_reader :key, :timestamp, :token, :signature - ensure_presence_of_key + def initialize(key:, timestamp:, token:, signature:) + @key, @timestamp, @token, @signature = key, Integer(timestamp), token, signature end def authenticated? @@ -33,13 +42,6 @@ class ActionMailbox::Ingresses::Mailgun::InboundEmailsController < ActionMailbox end private - def ensure_presence_of_key - unless key.present? - raise ArgumentError, "Missing required Mailgun API key" - end - end - - def signed? ActiveSupport::SecurityUtils.secure_compare signature, expected_signature end diff --git a/app/controllers/action_mailbox/ingresses/mandrill/inbound_emails_controller.rb b/app/controllers/action_mailbox/ingresses/mandrill/inbound_emails_controller.rb index bcaa5faf23..0601125cdb 100644 --- a/app/controllers/action_mailbox/ingresses/mandrill/inbound_emails_controller.rb +++ b/app/controllers/action_mailbox/ingresses/mandrill/inbound_emails_controller.rb @@ -24,17 +24,25 @@ class ActionMailbox::Ingresses::Mandrill::InboundEmailsController < ActionMailbo end def authenticated? - Authenticator.new(request).authenticated? + if key.present? + Authenticator.new(request, key).authenticated? + else + raise ArgumentError, <<~MESSAGE.squish + Missing required Mandrill API key. Set action_mailbox.mandrill_api_key in your application's + encrypted credentials or provide the MANDRILL_INGRESS_API_KEY environment variable. + MESSAGE + end end - class Authenticator - cattr_accessor :key - attr_reader :request + def key + Rails.application.credentials.dig(:action_mailbox, :mandrill_api_key) || ENV["MANDRILL_INGRESS_API_KEY"] + end - def initialize(request) - @request = request + class Authenticator + attr_reader :request, :key - ensure_presence_of_key + def initialize(request, key) + @request, @key = request, key end def authenticated? @@ -42,13 +50,6 @@ class ActionMailbox::Ingresses::Mandrill::InboundEmailsController < ActionMailbo end private - def ensure_presence_of_key - unless key.present? - raise ArgumentError, "Missing required Mandrill API key" - end - end - - def given_signature request.headers["X-Mandrill-Signature"] end diff --git a/app/controllers/action_mailbox/ingresses/postfix/inbound_emails_controller.rb b/app/controllers/action_mailbox/ingresses/postfix/inbound_emails_controller.rb index 72303378a9..133accf651 100644 --- a/app/controllers/action_mailbox/ingresses/postfix/inbound_emails_controller.rb +++ b/app/controllers/action_mailbox/ingresses/postfix/inbound_emails_controller.rb @@ -1,8 +1,5 @@ class ActionMailbox::Ingresses::Postfix::InboundEmailsController < ActionMailbox::BaseController - cattr_accessor :username, default: "actionmailbox" - cattr_accessor :password - - before_action :authenticate, :require_valid_rfc822_message + before_action :authenticate_by_password, :require_valid_rfc822_message def create ActionMailbox::InboundEmail.create_and_extract_message_id! request.body.read diff --git a/app/controllers/action_mailbox/ingresses/sendgrid/inbound_emails_controller.rb b/app/controllers/action_mailbox/ingresses/sendgrid/inbound_emails_controller.rb index f31845d8cd..b856eb5b94 100644 --- a/app/controllers/action_mailbox/ingresses/sendgrid/inbound_emails_controller.rb +++ b/app/controllers/action_mailbox/ingresses/sendgrid/inbound_emails_controller.rb @@ -1,8 +1,5 @@ class ActionMailbox::Ingresses::Sendgrid::InboundEmailsController < ActionMailbox::BaseController - cattr_accessor :username, default: "actionmailbox" - cattr_accessor :password - - before_action :authenticate + before_action :authenticate_by_password def create ActionMailbox::InboundEmail.create_and_extract_message_id! params.require(:email) diff --git a/lib/action_mailbox.rb b/lib/action_mailbox.rb index ae37cb84ed..fbc8122d9d 100644 --- a/lib/action_mailbox.rb +++ b/lib/action_mailbox.rb @@ -6,6 +6,7 @@ module ActionMailbox autoload :Base autoload :Router + mattr_accessor :ingress mattr_accessor :logger mattr_accessor :incinerate_after, default: 30.days end diff --git a/lib/action_mailbox/engine.rb b/lib/action_mailbox/engine.rb index b4758aacb5..6a0312c65f 100644 --- a/lib/action_mailbox/engine.rb +++ b/lib/action_mailbox/engine.rb @@ -10,6 +10,7 @@ module ActionMailbox initializer "action_mailbox.config" do config.after_initialize do |app| + ActionMailbox.ingress = app.config.action_mailbox.ingress ActionMailbox.logger = app.config.action_mailbox.logger || Rails.logger ActionMailbox.incinerate_after = app.config.action_mailbox.incinerate_after || 30.days end diff --git a/test/controllers/ingresses/amazon/inbound_emails_controller_test.rb b/test/controllers/ingresses/amazon/inbound_emails_controller_test.rb index 5eda6d8d65..c36c500cbe 100644 --- a/test/controllers/ingresses/amazon/inbound_emails_controller_test.rb +++ b/test/controllers/ingresses/amazon/inbound_emails_controller_test.rb @@ -4,6 +4,8 @@ ActionMailbox::Ingresses::Amazon::InboundEmailsController.verifier = Module.new { def self.authentic?(message); true; end } class ActionMailbox::Ingresses::Amazon::InboundEmailsControllerTest < ActionDispatch::IntegrationTest + setup { ActionMailbox.ingress = :amazon } + test "receiving an inbound email from Amazon" do assert_difference -> { ActionMailbox::InboundEmail.count }, +1 do post rails_amazon_inbound_emails_url, params: { content: file_fixture("../files/welcome.eml").read }, as: :json diff --git a/test/controllers/ingresses/mailgun/inbound_emails_controller_test.rb b/test/controllers/ingresses/mailgun/inbound_emails_controller_test.rb index 8fb3dd28c6..c5ec71013e 100644 --- a/test/controllers/ingresses/mailgun/inbound_emails_controller_test.rb +++ b/test/controllers/ingresses/mailgun/inbound_emails_controller_test.rb @@ -1,8 +1,10 @@ require "test_helper" -ActionMailbox::Ingresses::Mailgun::InboundEmailsController::Authenticator.key = "tbsy84uSV1Kt3ZJZELY2TmShPRs91E3yL4tzf96297vBCkDWgL" +ENV["MAILGUN_INGRESS_API_KEY"] = "tbsy84uSV1Kt3ZJZELY2TmShPRs91E3yL4tzf96297vBCkDWgL" class ActionMailbox::Ingresses::Mailgun::InboundEmailsControllerTest < ActionDispatch::IntegrationTest + setup { ActionMailbox.ingress = :mailgun } + test "receiving an inbound email from Mailgun" do assert_difference -> { ActionMailbox::InboundEmail.count }, +1 do travel_to "2018-10-09 15:15:00 EDT" @@ -78,12 +80,10 @@ class ActionMailbox::Ingresses::Mailgun::InboundEmailsControllerTest < ActionDis end private - delegate :key, :key=, to: ActionMailbox::Ingresses::Mailgun::InboundEmailsController::Authenticator - def switch_key_to(new_key) - previous_key, self.key = key, new_key + previous_key, ENV["MAILGUN_INGRESS_API_KEY"] = ENV["MAILGUN_INGRESS_API_KEY"], new_key yield ensure - self.key = previous_key + ENV["MAILGUN_INGRESS_API_KEY"] = previous_key end end diff --git a/test/controllers/ingresses/mandrill/inbound_emails_controller_test.rb b/test/controllers/ingresses/mandrill/inbound_emails_controller_test.rb index 1658d85104..c8a8e731d6 100644 --- a/test/controllers/ingresses/mandrill/inbound_emails_controller_test.rb +++ b/test/controllers/ingresses/mandrill/inbound_emails_controller_test.rb @@ -1,9 +1,10 @@ require "test_helper" -ActionMailbox::Ingresses::Mandrill::InboundEmailsController::Authenticator.key = "1l9Qf7lutEf7h73VXfBwhw" +ENV["MANDRILL_INGRESS_API_KEY"] = "1l9Qf7lutEf7h73VXfBwhw" class ActionMailbox::Ingresses::Mandrill::InboundEmailsControllerTest < ActionDispatch::IntegrationTest setup do + ActionMailbox.ingress = :mandrill @events = JSON.generate([{ event: "inbound", msg: { raw_msg: file_fixture("../files/welcome.eml").read } }]) end @@ -48,12 +49,10 @@ class ActionMailbox::Ingresses::Mandrill::InboundEmailsControllerTest < ActionDi end private - delegate :key, :key=, to: ActionMailbox::Ingresses::Mandrill::InboundEmailsController::Authenticator - def switch_key_to(new_key) - previous_key, self.key = key, new_key + previous_key, ENV["MANDRILL_INGRESS_API_KEY"] = ENV["MANDRILL_INGRESS_API_KEY"], new_key yield ensure - self.key = previous_key + ENV["MANDRILL_INGRESS_API_KEY"] = previous_key end end diff --git a/test/controllers/ingresses/postfix/inbound_emails_controller_test.rb b/test/controllers/ingresses/postfix/inbound_emails_controller_test.rb index a9588791b9..5e0777aa30 100644 --- a/test/controllers/ingresses/postfix/inbound_emails_controller_test.rb +++ b/test/controllers/ingresses/postfix/inbound_emails_controller_test.rb @@ -1,8 +1,8 @@ require "test_helper" -ActionMailbox::Ingresses::Postfix::InboundEmailsController.password = "tbsy84uSV1Kt3ZJZELY2TmShPRs91E3yL4tzf96297vBCkDWgL" - class ActionMailbox::Ingresses::Postfix::InboundEmailsControllerTest < ActionDispatch::IntegrationTest + setup { ActionMailbox.ingress = :postfix } + test "receiving an inbound email from Postfix" do assert_difference -> { ActionMailbox::InboundEmail.count }, +1 do post rails_postfix_inbound_emails_url, headers: { "Authorization" => credentials, "Content-Type" => "message/rfc822" }, @@ -51,18 +51,4 @@ class ActionMailbox::Ingresses::Postfix::InboundEmailsControllerTest < ActionDis end end end - - private - delegate :username, :password, :password=, to: ActionMailbox::Ingresses::Postfix::InboundEmailsController - - def credentials - ActionController::HttpAuthentication::Basic.encode_credentials username, password - end - - def switch_password_to(new_password) - previous_password, self.password = password, new_password - yield - ensure - self.password = previous_password - end end diff --git a/test/controllers/ingresses/sendgrid/inbound_emails_controller_test.rb b/test/controllers/ingresses/sendgrid/inbound_emails_controller_test.rb index 759a532087..bf6ccd8f03 100644 --- a/test/controllers/ingresses/sendgrid/inbound_emails_controller_test.rb +++ b/test/controllers/ingresses/sendgrid/inbound_emails_controller_test.rb @@ -1,8 +1,8 @@ require "test_helper" -ActionMailbox::Ingresses::Sendgrid::InboundEmailsController.password = "tbsy84uSV1Kt3ZJZELY2TmShPRs91E3yL4tzf96297vBCkDWgL" - class ActionMailbox::Ingresses::Sendgrid::InboundEmailsControllerTest < ActionDispatch::IntegrationTest + setup { ActionMailbox.ingress = :sendgrid } + test "receiving an inbound email from Sendgrid" do assert_difference -> { ActionMailbox::InboundEmail.count }, +1 do post rails_sendgrid_inbound_emails_url, @@ -43,16 +43,7 @@ class ActionMailbox::Ingresses::Sendgrid::InboundEmailsControllerTest < ActionDi end private - delegate :username, :password, :password=, to: ActionMailbox::Ingresses::Sendgrid::InboundEmailsController - def credentials - ActionController::HttpAuthentication::Basic.encode_credentials username, password - end - - def switch_password_to(new_password) - previous_password, self.password = password, new_password - yield - ensure - self.password = previous_password + ActionController::HttpAuthentication::Basic.encode_credentials "actionmailbox", ENV["RAILS_INBOUND_EMAIL_PASSWORD"] end end diff --git a/test/test_helper.rb b/test/test_helper.rb index 264f9e8482..b4459f3feb 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -1,5 +1,5 @@ -# Configure Rails Environment ENV["RAILS_ENV"] = "test" +ENV["RAILS_INBOUND_EMAIL_PASSWORD"] = "tbsy84uSV1Kt3ZJZELY2TmShPRs91E3yL4tzf96297vBCkDWgL" require_relative "../test/dummy/config/environment" ActiveRecord::Migrator.migrations_paths = [File.expand_path("../test/dummy/db/migrate", __dir__)] @@ -7,14 +7,11 @@ require "rails/test_help" require "byebug" -# Filter out Minitest backtrace while allowing backtrace from other libraries -# to be shown. Minitest.backtrace_filter = Minitest::BacktraceFilter.new require "rails/test_unit/reporter" Rails::TestUnitReporter.executable = 'bin/test' -# Load fixtures from the engine if ActiveSupport::TestCase.respond_to?(:fixture_path=) ActiveSupport::TestCase.fixture_path = File.expand_path("fixtures", __dir__) ActionDispatch::IntegrationTest.fixture_path = ActiveSupport::TestCase.fixture_path @@ -28,6 +25,20 @@ class ActiveSupport::TestCase include ActionMailbox::TestHelper, ActiveJob::TestHelper end +class ActionDispatch::IntegrationTest + private + def credentials + ActionController::HttpAuthentication::Basic.encode_credentials "actionmailbox", ENV["RAILS_INBOUND_EMAIL_PASSWORD"] + end + + def switch_password_to(new_password) + previous_password, ENV["RAILS_INBOUND_EMAIL_PASSWORD"] = ENV["RAILS_INBOUND_EMAIL_PASSWORD"], new_password + yield + ensure + ENV["RAILS_INBOUND_EMAIL_PASSWORD"] = previous_password + end +end + if ARGV.include?("-v") ActiveRecord::Base.logger = Logger.new(STDOUT) ActiveJob::Base.logger = Logger.new(STDOUT) -- cgit v1.2.3