diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bd886dc..80e4caf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,43 +4,38 @@ on: pull_request: push: branches: - - master - main jobs: build: runs-on: ubuntu-latest - strategy: - matrix: - ruby: - - 2.5 - - 2.6 - - 2.7 - - 3.0 - steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Setup ruby uses: ruby/setup-ruby@v1 with: - ruby-version: ${{ matrix.ruby }} + ruby-version: "3.2" bundler-cache: true - - name: Lint + - name: Rubocop run: bundle exec rubocop + - name: Syntax Tree + run: | + set -E + bundle exec stree check Gemfile discourse_mail_receiver.gemspec $(git ls-files '*.rb') - name: Tests run: bundle exec rake test publish: - if: github.event_name == 'push' && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/master') + if: github.event_name == 'push' && github.ref == 'refs/heads/main' needs: build runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Release Gem uses: discourse/publish-rubygems-action@v2-beta diff --git a/.rubocop.yml b/.rubocop.yml index d46296c..0c850b0 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,2 +1,5 @@ inherit_gem: - rubocop-discourse: default.yml + rubocop-discourse: stree-compat.yml + +Discourse: + Enabled: false diff --git a/.streerc b/.streerc new file mode 100644 index 0000000..cc0be49 --- /dev/null +++ b/.streerc @@ -0,0 +1,2 @@ +--print-width=100 +--plugins=plugin/trailing_comma,disable_ternary diff --git a/Gemfile b/Gemfile index d6d1038..7fe1f21 100644 --- a/Gemfile +++ b/Gemfile @@ -1,6 +1,6 @@ # frozen_string_literal: true -source 'https://rubygems.org' +source "https://rubygems.org" # Specify your gem's dependencies in onebox.gemspec gemspec diff --git a/Gemfile.lock b/Gemfile.lock index 34083a2..04d6e1a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,68 +1,117 @@ PATH remote: . specs: - discourse_mail_receiver (4.0.7) + discourse_mail_receiver (4.0.8) mail (~> 2.7.1) GEM remote: https://rubygems.org/ specs: + activesupport (7.1.3.4) + base64 + bigdecimal + concurrent-ruby (~> 1.0, >= 1.0.2) + connection_pool (>= 2.2.5) + drb + i18n (>= 1.6, < 2) + minitest (>= 5.1) + mutex_m + tzinfo (~> 2.0) ast (2.4.2) - diff-lcs (1.4.4) + base64 (0.2.0) + bigdecimal (3.1.8) + concurrent-ruby (1.3.3) + connection_pool (2.4.1) + diff-lcs (1.5.1) + drb (2.2.1) + i18n (1.14.5) + concurrent-ruby (~> 1.0) + json (2.7.2) + language_server-protocol (3.17.0.3) mail (2.7.1) mini_mime (>= 0.1.1) - mini_mime (1.1.0) - parallel (1.20.1) - parser (3.0.1.1) + mini_mime (1.1.5) + minitest (5.24.1) + mutex_m (0.2.0) + parallel (1.25.1) + parser (3.3.4.0) ast (~> 2.4.1) - rainbow (3.0.0) - rake (13.0.3) - regexp_parser (2.1.1) - rexml (3.2.8) - strscan (>= 3.0.9) - rspec (3.10.0) - rspec-core (~> 3.10.0) - rspec-expectations (~> 3.10.0) - rspec-mocks (~> 3.10.0) - rspec-core (3.10.1) - rspec-support (~> 3.10.0) - rspec-expectations (3.10.1) + racc + prettier_print (1.2.1) + racc (1.8.0) + rack (3.1.7) + rainbow (3.1.1) + rake (13.2.1) + regexp_parser (2.9.2) + rexml (3.3.2) + strscan + rspec (3.13.0) + rspec-core (~> 3.13.0) + rspec-expectations (~> 3.13.0) + rspec-mocks (~> 3.13.0) + rspec-core (3.13.0) + rspec-support (~> 3.13.0) + rspec-expectations (3.13.1) diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.10.0) - rspec-mocks (3.10.2) + rspec-support (~> 3.13.0) + rspec-mocks (3.13.1) diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.10.0) - rspec-support (3.10.2) - rubocop (1.15.0) + rspec-support (~> 3.13.0) + rspec-support (3.13.1) + rubocop (1.65.0) + json (~> 2.3) + language_server-protocol (>= 3.17.0) parallel (~> 1.10) - parser (>= 3.0.0.0) + parser (>= 3.3.0.2) rainbow (>= 2.2.2, < 4.0) - regexp_parser (>= 1.8, < 3.0) - rexml - rubocop-ast (>= 1.5.0, < 2.0) + regexp_parser (>= 2.4, < 3.0) + rexml (>= 3.2.5, < 4.0) + rubocop-ast (>= 1.31.1, < 2.0) ruby-progressbar (~> 1.7) - unicode-display_width (>= 1.4.0, < 3.0) - rubocop-ast (1.5.0) - parser (>= 3.0.1.1) - rubocop-discourse (2.4.1) - rubocop (>= 1.1.0) - rubocop-rspec (>= 2.0.0) - rubocop-rspec (2.3.0) - rubocop (~> 1.0) - rubocop-ast (>= 1.1.0) - ruby-progressbar (1.11.0) + unicode-display_width (>= 2.4.0, < 3.0) + rubocop-ast (1.31.3) + parser (>= 3.3.1.0) + rubocop-capybara (2.21.0) + rubocop (~> 1.41) + rubocop-discourse (3.8.1) + activesupport (>= 6.1) + rubocop (>= 1.59.0) + rubocop-capybara (>= 2.0.0) + rubocop-factory_bot (>= 2.0.0) + rubocop-rails (>= 2.25.0) + rubocop-rspec (>= 3.0.1) + rubocop-rspec_rails (>= 2.30.0) + rubocop-factory_bot (2.26.1) + rubocop (~> 1.61) + rubocop-rails (2.25.1) + activesupport (>= 4.2.0) + rack (>= 1.1) + rubocop (>= 1.33.0, < 2.0) + rubocop-ast (>= 1.31.1, < 2.0) + rubocop-rspec (3.0.3) + rubocop (~> 1.61) + rubocop-rspec_rails (2.30.0) + rubocop (~> 1.61) + rubocop-rspec (~> 3, >= 3.0.1) + ruby-progressbar (1.13.0) strscan (3.1.0) - unicode-display_width (2.0.0) + syntax_tree (6.2.0) + prettier_print (>= 1.2.0) + syntax_tree-disable_ternary (1.0.0) + tzinfo (2.0.6) + concurrent-ruby (~> 1.0) + unicode-display_width (2.5.0) PLATFORMS ruby DEPENDENCIES - bundler (~> 2.0) discourse_mail_receiver! - rake (~> 13.0) - rspec (~> 3.10) - rubocop-discourse (~> 2.4.1) + rake + rspec + rubocop-discourse + syntax_tree + syntax_tree-disable_ternary BUNDLED WITH 2.1.4 diff --git a/discourse_mail_receiver.gemspec b/discourse_mail_receiver.gemspec index 3cef40f..28bef13 100644 --- a/discourse_mail_receiver.gemspec +++ b/discourse_mail_receiver.gemspec @@ -1,27 +1,28 @@ # frozen-string-literal: true # coding: utf-8 -lib = File.expand_path('../lib', __FILE__) -$LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) +lib = File.expand_path("../lib", __FILE__) +$LOAD_PATH.unshift(lib) if !$LOAD_PATH.include?(lib) Gem::Specification.new do |spec| - spec.name = 'discourse_mail_receiver' - spec.version = '4.0.7' - spec.authors = ['Discourse Team'] - spec.email = ['team@discourse.org'] - spec.description = %q{A gem used to package the core .rb files of the mail-receiver.} - spec.summary = spec.description - spec.homepage = 'https://github.com/discourse/mail-receiver' - spec.license = 'MIT' + spec.name = "discourse_mail_receiver" + spec.version = "4.0.8" + spec.authors = ["Discourse Team"] + spec.email = ["team@discourse.org"] + spec.description = "A gem used to package the core .rb files of the mail-receiver." + spec.summary = spec.description + spec.homepage = "https://github.com/discourse/mail-receiver" + spec.license = "MIT" - spec.required_ruby_version = '>= 2.5.0' + spec.required_ruby_version = Gem::Requirement.new(">= 3.0.0") - spec.files = Dir['lib/**/*.rb'] - spec.require_paths = ['lib'] + spec.files = Dir["lib/**/*.rb"] + spec.require_paths = ["lib"] - spec.add_runtime_dependency 'mail', '~> 2.7.1' - spec.add_development_dependency 'bundler', '~> 2.0' - spec.add_development_dependency 'rake', '~> 13.0' - spec.add_development_dependency 'rspec', '~> 3.10' - spec.add_development_dependency 'rubocop-discourse', '~> 2.4.1' + spec.add_runtime_dependency "mail", "~> 2.7.1" + spec.add_development_dependency "rake" + spec.add_development_dependency "rspec" + spec.add_development_dependency "rubocop-discourse" + spec.add_development_dependency "syntax_tree" + spec.add_development_dependency "syntax_tree-disable_ternary" end diff --git a/lib/mail_receiver/discourse_mail_receiver.rb b/lib/mail_receiver/discourse_mail_receiver.rb index 89796d6..1a67b98 100644 --- a/lib/mail_receiver/discourse_mail_receiver.rb +++ b/lib/mail_receiver/discourse_mail_receiver.rb @@ -1,12 +1,11 @@ # frozen_string_literal: true -require 'syslog' -require 'json' +require "syslog" +require "json" require "uri" require "net/http" -require_relative 'mail_receiver_base' +require_relative "mail_receiver_base" class DiscourseMailReceiver < MailReceiverBase - def initialize(env_file = nil, recipient = nil, mail = nil) super(env_file) @@ -23,8 +22,8 @@ def endpoint @endpoint = @env["DISCOURSE_MAIL_ENDPOINT"] - if @env['DISCOURSE_BASE_URL'] - @endpoint = "#{@env['DISCOURSE_BASE_URL']}/admin/email/handle_mail" + if @env["DISCOURSE_BASE_URL"] + @endpoint = "#{@env["DISCOURSE_BASE_URL"]}/admin/email/handle_mail" end @endpoint end @@ -55,5 +54,4 @@ def process logger.err "Failed to POST the e-mail to %s: %s", endpoint, response.code :failure end - end diff --git a/lib/mail_receiver/fast_rejection.rb b/lib/mail_receiver/fast_rejection.rb index d857c4f..d5d5c39 100644 --- a/lib/mail_receiver/fast_rejection.rb +++ b/lib/mail_receiver/fast_rejection.rb @@ -1,22 +1,21 @@ # frozen_string_literal: true -require 'set' -require 'syslog' -require 'json' -require 'uri' -require 'cgi' -require 'net/http' +require "syslog" +require "json" +require "uri" +require "cgi" +require "net/http" -require_relative 'mail' -require_relative 'mail_receiver_base' +require_relative "mail" +require_relative "mail_receiver_base" class FastRejection < MailReceiverBase - def initialize(env_file) super(env_file) - @disabled = @env['DISCOURSE_FAST_REJECTION_DISABLED'] || !@env['DISCOURSE_BASE_URL'] + @disabled = @env["DISCOURSE_FAST_REJECTION_DISABLED"] || !@env["DISCOURSE_BASE_URL"] - @blacklisted_sender_domains = @env.fetch('BLACKLISTED_SENDER_DOMAINS', "").split(" ").map(&:downcase).to_set + @blacklisted_sender_domains = + @env.fetch("BLACKLISTED_SENDER_DOMAINS", "").split(" ").map(&:downcase).to_set end def disabled? @@ -24,7 +23,7 @@ def disabled? end def process - $stdout.sync = true # unbuffered output + $stdout.sync = true # unbuffered output args = {} while line = gets @@ -32,29 +31,29 @@ def process line = line.chomp if line.empty? puts "action=#{process_single_request(args)}" - puts '' + puts "" - args = {} # reset for next request. + args = {} # reset for next request. else - k, v = line.chomp.split('=', 2) + k, v = line.chomp.split("=", 2) args[k] = v end end end def process_single_request(args) - return 'dunno' if disabled? + return "dunno" if disabled? - if args['request'] != 'smtpd_access_policy' - return 'defer_if_permit Internal error, Request type invalid' - elsif args['protocol_state'] != 'RCPT' - return 'dunno' - elsif args['sender'].nil? + if args["request"] != "smtpd_access_policy" + return "defer_if_permit Internal error, Request type invalid" + elsif args["protocol_state"] != "RCPT" + return "dunno" + elsif args["sender"].nil? # Note that while this key should always exist, its value may be the empty # string. Postfix will convert the "<>" null sender to "". - return 'defer_if_permit No sender specified' - elsif args['recipient'].nil? - return 'defer_if_permit No recipient specified' + return "defer_if_permit No sender specified" + elsif args["recipient"].nil? + return "defer_if_permit No recipient specified" end run_filters(args) @@ -62,8 +61,8 @@ def process_single_request(args) def maybe_reject_email(from, to) uri = URI.parse(endpoint) - fromarg = CGI::escape(from) - toarg = CGI::escape(to) + fromarg = CGI.escape(from) + toarg = CGI.escape(to) qs = "from=#{fromarg}&to=#{toarg}" if uri.query && !uri.query.empty? @@ -80,7 +79,10 @@ def maybe_reject_email(from, to) get["Api-Key"] = key response = http.request(get) rescue StandardError => ex - logger.err "Failed to GET smtp_should_reject answer from %s: %s (%s)", endpoint, ex.message, ex.class + logger.err "Failed to GET smtp_should_reject answer from %s: %s (%s)", + endpoint, + ex.message, + ex.class logger.err ex.backtrace.map { |l| " #{l}" }.join("\n") return "defer_if_permit Internal error, API request preparation failed" ensure @@ -89,28 +91,23 @@ def maybe_reject_email(from, to) if Net::HTTPSuccess === response reply = JSON.parse(response.body) - if reply['reject'] - return "reject #{reply['reason']}" - end + return "reject #{reply["reason"]}" if reply["reject"] else logger.err "Failed to GET smtp_should_reject answer from %s: %s", endpoint, response.code return "defer_if_permit Internal error, API request failed" end - "dunno" # let future tests also be allowed to reject this one. + "dunno" # let future tests also be allowed to reject this one. end def endpoint - "#{@env['DISCOURSE_BASE_URL']}/admin/email/smtp_should_reject.json" + "#{@env["DISCOURSE_BASE_URL"]}/admin/email/smtp_should_reject.json" end private def run_filters(args) - filters = [ - :maybe_reject_by_sender_domain, - :maybe_reject_by_api, - ] + filters = %i[maybe_reject_by_sender_domain maybe_reject_by_api] filters.each do |f| action = send(f, args) @@ -121,25 +118,24 @@ def run_filters(args) end def maybe_reject_by_sender_domain(args) - sender = args['sender'] + sender = args["sender"] return "dunno" if sender.empty? domain = domain_from_addrspec(sender) if domain.empty? logger.info("deferred mail with domainless sender #{sender}") - return 'defer_if_permit Invalid sender' + return "defer_if_permit Invalid sender" end if @blacklisted_sender_domains.include? domain logger.info("rejected mail from blacklisted sender domain #{domain} (from #{sender})") - return 'reject Invalid sender' + return "reject Invalid sender" end "dunno" end def maybe_reject_by_api(args) - maybe_reject_email(args['sender'], args['recipient']) + maybe_reject_email(args["sender"], args["recipient"]) end - end diff --git a/lib/mail_receiver/mail_receiver_base.rb b/lib/mail_receiver/mail_receiver_base.rb index e5a9816..98a3b08 100644 --- a/lib/mail_receiver/mail_receiver_base.rb +++ b/lib/mail_receiver/mail_receiver_base.rb @@ -1,21 +1,20 @@ # frozen_string_literal: true class MailReceiverBase - class ReceiverException < StandardError; end + class ReceiverException < StandardError + end attr_reader :env def initialize(env_file) - unless File.exists?(env_file) - fatal "Config file %s does not exist. Aborting.", env_file - end + fatal "Config file %s does not exist. Aborting.", env_file unless File.exist?(env_file) @env = JSON.parse(File.read(env_file)) - %w{DISCOURSE_API_KEY DISCOURSE_API_USERNAME}.each do |kw| + %w[DISCOURSE_API_KEY DISCOURSE_API_USERNAME].each do |kw| fatal "env var %s is required", kw unless @env[kw] end - if @env['DISCOURSE_MAIL_ENDPOINT'].nil? && @env['DISCOURSE_BASE_URL'].nil? + if @env["DISCOURSE_MAIL_ENDPOINT"].nil? && @env["DISCOURSE_BASE_URL"].nil? fatal "DISCOURSE_MAIL_ENDPOINT and DISCOURSE_BASE_URL env var missing" end end @@ -29,11 +28,11 @@ def logger end def key - @env['DISCOURSE_API_KEY'] + @env["DISCOURSE_API_KEY"] end def username - @env['DISCOURSE_API_USERNAME'] + @env["DISCOURSE_API_USERNAME"] end def fatal(*args) diff --git a/spec/lib/discourse_mail_receiver_spec.rb b/spec/lib/discourse_mail_receiver_spec.rb index a37d01e..213ed9c 100644 --- a/spec/lib/discourse_mail_receiver_spec.rb +++ b/spec/lib/discourse_mail_receiver_spec.rb @@ -1,25 +1,24 @@ # frozen_string_literal: true -require_relative '../../lib/mail_receiver/discourse_mail_receiver' +require_relative "../../lib/mail_receiver/discourse_mail_receiver" RSpec.describe DiscourseMailReceiver do - let(:recipient) { "eviltrout@example.com" } let(:mail) { "some body" } it "raises an error without a recipient" do - expect { - described_class.new(file_for(:standard), nil, mail) - }.to raise_error(MailReceiverBase::ReceiverException) + expect { described_class.new(file_for(:standard), nil, mail) }.to raise_error( + MailReceiverBase::ReceiverException, + ) end it "raises an error without mail" do - expect { - described_class.new(file_for(:standard), recipient, nil) - }.to raise_error(MailReceiverBase::ReceiverException) + expect { described_class.new(file_for(:standard), recipient, nil) }.to raise_error( + MailReceiverBase::ReceiverException, + ) - expect { - described_class.new(file_for(:standard), recipient, "") - }.to raise_error(MailReceiverBase::ReceiverException) + expect { described_class.new(file_for(:standard), recipient, "") }.to raise_error( + MailReceiverBase::ReceiverException, + ) end it "has a backwards compatible endpoint" do @@ -28,26 +27,25 @@ end it "has the correct endpoint" do - receiver = described_class.new(file_for(:standard), 'eviltrout@example.com', 'test mail') + receiver = described_class.new(file_for(:standard), "eviltrout@example.com", "test mail") expect(receiver.endpoint).to eq("https://localhost:8080/admin/email/handle_mail") end it "can process mail" do expect_any_instance_of(Net::HTTP).to receive(:request) do |http| - Net::HTTPSuccess.new(http, 200, 'OK') + Net::HTTPSuccess.new(http, 200, "OK") end - receiver = described_class.new(file_for(:standard), 'eviltrout@example.com', 'test mail') + receiver = described_class.new(file_for(:standard), "eviltrout@example.com", "test mail") expect(receiver.process).to eq(:success) end it "returns failure on HTTP error" do expect_any_instance_of(Net::HTTP).to receive(:request) do |http| - Net::HTTPServerError.new(http, 500, 'Error') + Net::HTTPServerError.new(http, 500, "Error") end - receiver = described_class.new(file_for(:standard), 'eviltrout@example.com', 'test mail') + receiver = described_class.new(file_for(:standard), "eviltrout@example.com", "test mail") expect(receiver.process).to eq(:failure) end - end diff --git a/spec/lib/fast_reject_spec.rb b/spec/lib/fast_reject_spec.rb deleted file mode 100644 index a757c09..0000000 --- a/spec/lib/fast_reject_spec.rb +++ /dev/null @@ -1,149 +0,0 @@ -# frozen_string_literal: true -require_relative '../../lib/mail_receiver/fast_rejection' - -describe FastRejection do - - it "is enabled if BASE_URL is present" do - receiver = described_class.new(file_for(:standard)) - expect(receiver).not_to be_disabled - end - - it "is disabled if FAST_REJECTION_DISABLED is set" do - receiver = described_class.new(file_for(:fast_disabled)) - expect(receiver).to be_disabled - end - - it "is disabled if missing the base URL" do - receiver = described_class.new(file_for(:standard_deprecated)) - expect(receiver).to be_disabled - end - - it "has the correct endpoint" do - receiver = described_class.new(file_for(:standard)) - expect(receiver.endpoint).to eq('https://localhost:8080/admin/email/smtp_should_reject.json') - end - - context "process_single_request" do - let(:receiver) { described_class.new(file_for(:standard)) } - - it "returns defer_if_permit if not smtpd_access_policy" do - response = receiver.process_single_request('request' => 'unexpected') - expect(response).to start_with('defer_if_permit') - end - - it "returns dunno if the protocol state is not RCPT" do - response = receiver.process_single_request( - 'request' => 'smtpd_access_policy', - 'protocol_state' => 'NOT_RCPT' - ) - expect(response).to eq('dunno') - end - - it "returns defer_if_permit if no sender" do - response = receiver.process_single_request( - 'request' => 'smtpd_access_policy', - 'protocol_state' => 'RCPT', - 'recipient' => 'discourse@example.com' - ) - expect(response).to start_with('defer_if_permit') - end - - it "returns dunno if from the null sender" do - expect_any_instance_of(Net::HTTP).to receive(:request) do |http| - response = Net::HTTPSuccess.new(http, 200, "OK") - allow(response).to receive(:body).and_return("{}") - response - end - response = receiver.process_single_request( - 'request' => 'smtpd_access_policy', - 'protocol_state' => 'RCPT', - 'sender' => '', - 'recipient' => 'discourse@example.com' - ) - expect(response).to eq('dunno') - end - - it "returns defer_if_permit if no recipient" do - response = receiver.process_single_request( - 'request' => 'smtpd_access_policy', - 'protocol_state' => 'RCPT', - 'sender' => 'eviltrout@example.com' - ) - expect(response).to start_with('defer_if_permit') - end - - it "returns defer_if_permit if sender addr-spec contains no domain" do - response = receiver.process_single_request( - 'request' => 'smtpd_access_policy', - 'protocol_state' => 'RCPT', - 'sender' => 'miscreant', - 'recipient' => 'discourse@example.com' - ) - expect(response).to start_with('defer_if_permit') - end - - it "returns reject if sender domain is blacklisted" do - response = receiver.process_single_request( - 'request' => 'smtpd_access_policy', - 'protocol_state' => 'RCPT', - 'sender' => 'miscreant@bad.com', - 'recipient' => 'discourse@example.com' - ) - expect(response).to start_with('reject') - end - - it "returns reject if sender domain is blacklisted with differing case" do - response = receiver.process_single_request( - 'request' => 'smtpd_access_policy', - 'protocol_state' => 'RCPT', - 'sender' => 'miscreant@SaD.NeT', - 'recipient' => 'discourse@example.com' - ) - expect(response).to start_with('reject') - end - - it "returns dunno if everything looks good" do - expect_any_instance_of(Net::HTTP).to receive(:request) do |http| - response = Net::HTTPSuccess.new(http, 200, "OK") - allow(response).to receive(:body).and_return("{}") - response - end - response = receiver.process_single_request( - 'request' => 'smtpd_access_policy', - 'protocol_state' => 'RCPT', - 'sender' => 'eviltrout@example.com', - 'recipient' => 'discourse@example.com' - ) - expect(response).to eq("dunno") - end - - it "rejects if there's an HTTP error" do - expect_any_instance_of(Net::HTTP).to receive(:request) do |http| - Net::HTTPServerError.new(http, 500, 'Error') - end - response = receiver.process_single_request( - 'request' => 'smtpd_access_policy', - 'protocol_state' => 'RCPT', - 'sender' => 'eviltrout@example.com', - 'recipient' => 'discourse@example.com' - ) - expect(response).to start_with('defer_if_permit') - end - - it "rejects if the HTTP response has reject in the JSON" do - expect_any_instance_of(Net::HTTP).to receive(:request) do |http| - response = Net::HTTPSuccess.new(http, 200, "OK") - allow(response).to receive(:body).and_return('{"reject": true, "reason": "because I said so"}') - response - end - response = receiver.process_single_request( - 'request' => 'smtpd_access_policy', - 'protocol_state' => 'RCPT', - 'sender' => 'eviltrout@example.com', - 'recipient' => 'discourse@example.com' - ) - expect(response).to eq("reject because I said so") - end - end - -end diff --git a/spec/lib/fast_rejection_spec.rb b/spec/lib/fast_rejection_spec.rb new file mode 100644 index 0000000..8969499 --- /dev/null +++ b/spec/lib/fast_rejection_spec.rb @@ -0,0 +1,159 @@ +# frozen_string_literal: true +require_relative "../../lib/mail_receiver/fast_rejection" + +describe FastRejection do + it "is enabled if BASE_URL is present" do + receiver = described_class.new(file_for(:standard)) + expect(receiver).not_to be_disabled + end + + it "is disabled if FAST_REJECTION_DISABLED is set" do + receiver = described_class.new(file_for(:fast_disabled)) + expect(receiver).to be_disabled + end + + it "is disabled if missing the base URL" do + receiver = described_class.new(file_for(:standard_deprecated)) + expect(receiver).to be_disabled + end + + it "has the correct endpoint" do + receiver = described_class.new(file_for(:standard)) + expect(receiver.endpoint).to eq("https://localhost:8080/admin/email/smtp_should_reject.json") + end + + describe "#process_single_request" do + let(:receiver) { described_class.new(file_for(:standard)) } + + it "returns defer_if_permit if not smtpd_access_policy" do + response = receiver.process_single_request("request" => "unexpected") + expect(response).to start_with("defer_if_permit") + end + + it "returns dunno if the protocol state is not RCPT" do + response = + receiver.process_single_request( + "request" => "smtpd_access_policy", + "protocol_state" => "NOT_RCPT", + ) + expect(response).to eq("dunno") + end + + it "returns defer_if_permit if no sender" do + response = + receiver.process_single_request( + "request" => "smtpd_access_policy", + "protocol_state" => "RCPT", + "recipient" => "discourse@example.com", + ) + expect(response).to start_with("defer_if_permit") + end + + it "returns dunno if from the null sender" do + expect_any_instance_of(Net::HTTP).to receive(:request) do |http| + response = Net::HTTPSuccess.new(http, 200, "OK") + allow(response).to receive(:body).and_return("{}") + response + end + response = + receiver.process_single_request( + "request" => "smtpd_access_policy", + "protocol_state" => "RCPT", + "sender" => "", + "recipient" => "discourse@example.com", + ) + expect(response).to eq("dunno") + end + + it "returns defer_if_permit if no recipient" do + response = + receiver.process_single_request( + "request" => "smtpd_access_policy", + "protocol_state" => "RCPT", + "sender" => "eviltrout@example.com", + ) + expect(response).to start_with("defer_if_permit") + end + + it "returns defer_if_permit if sender addr-spec contains no domain" do + response = + receiver.process_single_request( + "request" => "smtpd_access_policy", + "protocol_state" => "RCPT", + "sender" => "miscreant", + "recipient" => "discourse@example.com", + ) + expect(response).to start_with("defer_if_permit") + end + + it "returns reject if sender domain is blacklisted" do + response = + receiver.process_single_request( + "request" => "smtpd_access_policy", + "protocol_state" => "RCPT", + "sender" => "miscreant@bad.com", + "recipient" => "discourse@example.com", + ) + expect(response).to start_with("reject") + end + + it "returns reject if sender domain is blacklisted with differing case" do + response = + receiver.process_single_request( + "request" => "smtpd_access_policy", + "protocol_state" => "RCPT", + "sender" => "miscreant@SaD.NeT", + "recipient" => "discourse@example.com", + ) + expect(response).to start_with("reject") + end + + it "returns dunno if everything looks good" do + expect_any_instance_of(Net::HTTP).to receive(:request) do |http| + response = Net::HTTPSuccess.new(http, 200, "OK") + allow(response).to receive(:body).and_return("{}") + response + end + response = + receiver.process_single_request( + "request" => "smtpd_access_policy", + "protocol_state" => "RCPT", + "sender" => "eviltrout@example.com", + "recipient" => "discourse@example.com", + ) + expect(response).to eq("dunno") + end + + it "rejects if there's an HTTP error" do + expect_any_instance_of(Net::HTTP).to receive(:request) do |http| + Net::HTTPServerError.new(http, 500, "Error") + end + response = + receiver.process_single_request( + "request" => "smtpd_access_policy", + "protocol_state" => "RCPT", + "sender" => "eviltrout@example.com", + "recipient" => "discourse@example.com", + ) + expect(response).to start_with("defer_if_permit") + end + + it "rejects if the HTTP response has reject in the JSON" do + expect_any_instance_of(Net::HTTP).to receive(:request) do |http| + response = Net::HTTPSuccess.new(http, 200, "OK") + allow(response).to receive(:body).and_return( + '{"reject": true, "reason": "because I said so"}', + ) + response + end + response = + receiver.process_single_request( + "request" => "smtpd_access_policy", + "protocol_state" => "RCPT", + "sender" => "eviltrout@example.com", + "recipient" => "discourse@example.com", + ) + expect(response).to eq("reject because I said so") + end + end +end diff --git a/spec/lib/mail_receiver_base_spec.rb b/spec/lib/mail_receiver_base_spec.rb index fc187dd..b7b3a2f 100644 --- a/spec/lib/mail_receiver_base_spec.rb +++ b/spec/lib/mail_receiver_base_spec.rb @@ -1,44 +1,42 @@ # frozen_string_literal: true -require_relative '../../lib/mail_receiver/discourse_mail_receiver' +require_relative "../../lib/mail_receiver/discourse_mail_receiver" RSpec.describe MailReceiverBase do - it "raises an error with an a non-existant env file" do - expect { - described_class.new("does-not-exist.json") - }.to raise_error(MailReceiverBase::ReceiverException) + expect { described_class.new("does-not-exist.json") }.to raise_error( + MailReceiverBase::ReceiverException, + ) end it "parses the env file" do receiver = described_class.new(file_for(:standard)) - expect(receiver.key).to eq('EXAMPLE_KEY') - expect(receiver.username).to eq('eviltrout') - expect(receiver.env['DISCOURSE_BASE_URL']).to eq('https://localhost:8080') + expect(receiver.key).to eq("EXAMPLE_KEY") + expect(receiver.username).to eq("eviltrout") + expect(receiver.env["DISCOURSE_BASE_URL"]).to eq("https://localhost:8080") end it "works with the deprecated format" do receiver = described_class.new(file_for(:standard_deprecated)) - expect(receiver.key).to eq('EXAMPLE_KEY') - expect(receiver.username).to eq('eviltrout') - expect(receiver.env['DISCOURSE_MAIL_ENDPOINT']).to eq('https://localhost:8080/mail-me') + expect(receiver.key).to eq("EXAMPLE_KEY") + expect(receiver.username).to eq("eviltrout") + expect(receiver.env["DISCOURSE_MAIL_ENDPOINT"]).to eq("https://localhost:8080/mail-me") end it "raises an error if the env file doesn't have the api key" do - expect { - described_class.new(file_for(:missing_api_key)) - }.to raise_error(MailReceiverBase::ReceiverException) + expect { described_class.new(file_for(:missing_api_key)) }.to raise_error( + MailReceiverBase::ReceiverException, + ) end it "raises an error if the env file doesn't have the username" do - expect { - described_class.new(file_for(:missing_username)) - }.to raise_error(MailReceiverBase::ReceiverException) + expect { described_class.new(file_for(:missing_username)) }.to raise_error( + MailReceiverBase::ReceiverException, + ) end it "raises an error if MAIL_ENDPOINT or BASE_URL are missing" do - expect { - described_class.new(file_for(:missing_host)) - }.to raise_error(MailReceiverBase::ReceiverException) + expect { described_class.new(file_for(:missing_host)) }.to raise_error( + MailReceiverBase::ReceiverException, + ) end - end diff --git a/spec/lib/mail_spec.rb b/spec/lib/mail_spec.rb index f9d191e..5f18ab1 100644 --- a/spec/lib/mail_spec.rb +++ b/spec/lib/mail_spec.rb @@ -1,8 +1,9 @@ # frozen_string_literal: true -require_relative '../../lib/mail_receiver/mail' - -RSpec.describe 'domain_from_addrspec' do +require_relative "../../lib/mail_receiver/mail" +# rubocop:disable RSpec/DescribeClass +# mail.rb is not implemented as a class or module +RSpec.describe "domain_from_addrspec" do it "normalises domains to lowercase" do expect(domain_from_addrspec("local-part@DOMAIN.NET")).to eq "domain.net" end @@ -14,5 +15,4 @@ it "returns an empty string if a domain was not found" do expect(domain_from_addrspec("local-part")).to be_empty end - end