diff --git a/Gemfile b/Gemfile index eefd680..d86ba4a 100644 --- a/Gemfile +++ b/Gemfile @@ -1,8 +1,7 @@ -source 'https://rubygems.org' - -gem 'rails' +source "https://rubygems.org" group :development, :test do + gem "rails", "~> 6" gem "rspec-rails", "~> 3" gem "pry", "~> 0" end diff --git a/README.md b/README.md index 8575013..724cb4e 100644 --- a/README.md +++ b/README.md @@ -114,11 +114,12 @@ two environment variables to use this protocol: format. This environment variable will be the contents of a `.pub` file, newlines and all. -`CHATOPS_AUTH_BASE_URL` is the base URL of your server as the chatops client +`CHATOPS_AUTH_BASE_URL` is the base URLs of your servers as the chatops client sees it. This is specified as an environment variable since rails will trust client headers about a forwarded hostname. For example, if your chatops client has added the url `https://example.com/_chatops`, you'd set this to -`https://example.com`. +`https://example.com`. You can specify more than one base url divided by comma, +e.g. `https://example.com,https://example2.com` You can also optionally set `CHATOPS_AUTH_ALT_PUBLIC_KEY` to a second public key which will be accepted. This is helpful when rolling keys. diff --git a/chatops-controller.gemspec b/chatops-controller.gemspec index fda8ee0..ce5e50c 100644 --- a/chatops-controller.gemspec +++ b/chatops-controller.gemspec @@ -17,6 +17,7 @@ Gem::Specification.new do |s| s.files = Dir["{app,config,db,lib}/**/*", "README.md"] s.test_files = Dir["spec/**/*"] + s.add_dependency "rails" s.add_dependency "actionpack", ">= 6.0" s.add_dependency "activesupport", ">= 6.0" diff --git a/lib/chatops.rb b/lib/chatops.rb index b95e747..fa7d90c 100644 --- a/lib/chatops.rb +++ b/lib/chatops.rb @@ -23,8 +23,8 @@ def self.alt_public_key ENV["CHATOPS_AUTH_ALT_PUBLIC_KEY"] end - def self.auth_base_url - ENV[auth_base_url_env_var_name] + def self.auth_base_urls + ENV.fetch(auth_base_url_env_var_name, "").split(",").map(&:strip) end def self.auth_base_url_env_var_name diff --git a/lib/chatops/controller.rb b/lib/chatops/controller.rb index 72aa8fe..8280eed 100644 --- a/lib/chatops/controller.rb +++ b/lib/chatops/controller.rb @@ -118,26 +118,29 @@ def ensure_user_given end def ensure_chatops_authenticated - body = request.raw_post || "" - signature_string = [@chatops_url, @chatops_nonce, @chatops_timestamp, body].join("\n") - # We return this just to aid client debugging. - response.headers["Chatops-Signature-String"] = Base64.strict_encode64(signature_string) raise ConfigurationError.new("You need to add a client's public key in .pem format via #{Chatops.public_key_env_var_name}") unless Chatops.public_key.present? - if signature_valid?(Chatops.public_key, @chatops_signature, signature_string) || - signature_valid?(Chatops.alt_public_key, @chatops_signature, signature_string) + + body = request.raw_post || "" + + @chatops_urls.each do |url| + signature_string = [url, @chatops_nonce, @chatops_timestamp, body].join("\n") + # We return this just to aid client debugging. + response.headers["Chatops-Signature-String"] = Base64.strict_encode64(signature_string) + if signature_valid?(Chatops.public_key, @chatops_signature, signature_string) || + signature_valid?(Chatops.alt_public_key, @chatops_signature, signature_string) return true + end end + return jsonrpc_error(-32800, 403, "Not authorized") end def ensure_valid_chatops_url - unless Chatops.auth_base_url.present? + unless Chatops.auth_base_urls.present? raise ConfigurationError.new("You need to set the server's base URL to authenticate chatops RPC via #{Chatops.auth_base_url_env_var_name}") end - if Chatops.auth_base_url[-1] == "/" - raise ConfigurationError.new("Don't include a trailing slash in #{Chatops.auth_base_url_env_var_name}; the rails path will be appended and it must match exactly.") - end - @chatops_url = Chatops.auth_base_url + request.path + + @chatops_urls = Chatops.auth_base_urls.map { |url| url.chomp("/") + request.path } end def ensure_valid_chatops_nonce diff --git a/spec/lib/chatops/controller_spec.rb b/spec/lib/chatops/controller_spec.rb index 6225af3..591c6fc 100644 --- a/spec/lib/chatops/controller_spec.rb +++ b/spec/lib/chatops/controller_spec.rb @@ -56,7 +56,7 @@ def ensure_app_given @private_key = OpenSSL::PKey::RSA.new(2048) ENV["CHATOPS_AUTH_PUBLIC_KEY"] = @private_key.public_key.to_pem - ENV["CHATOPS_AUTH_BASE_URL"] = "http://test.host" + ENV["CHATOPS_AUTH_BASE_URL"] = "http://old.host,http://test.host/" end def rails_flexible_post(path, outer_params, jsonrpc_params = nil) @@ -315,7 +315,7 @@ def rails_flexible_post(path, outer_params, jsonrpc_params = nil) :room_id => "#someroom", :unknown_key => "few" # This should get ignored }, { - "app" => "foo" + "app" => "foo" } expect(json_response).to eq({ "jsonrpc" => "2.0", @@ -323,7 +323,7 @@ def rails_flexible_post(path, outer_params, jsonrpc_params = nil) "result" => "{\"params\":{\"action\":\"proxy_parameters\",\"chatop\":\"proxy_parameters\",\"controller\":\"anonymous\",\"mention_slug\":\"mention_slug_here\",\"message_id\":\"message_id_here\",\"room_id\":\"#someroom\",\"user\":\"foo\"},\"jsonrpc_params\":{\"app\":\"foo\"}}" }) expect(response.status).to eq 200 - end + end it "uses typical controller fun like before_action" do