From 3e42ea0397cd5064240294043850c3385f917a55 Mon Sep 17 00:00:00 2001 From: Artem Egorov Date: Fri, 5 Jan 2024 14:45:08 +0000 Subject: [PATCH 1/7] Allows to set multiple base auth urls --- README.md | 5 +++-- lib/chatops.rb | 4 ++-- lib/chatops/controller.rb | 28 ++++++++++++++++------------ spec/lib/chatops/controller_spec.rb | 6 +++--- 4 files changed, 24 insertions(+), 19 deletions(-) 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/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..8f95150 100644 --- a/lib/chatops/controller.rb +++ b/lib/chatops/controller.rb @@ -118,26 +118,30 @@ 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) - return true + + body = request.raw_post || "" + + valid = @chatops_urls.any? 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 true if valid 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 From 9c1c630eb0229dc27b3588b972c502c6b0501505 Mon Sep 17 00:00:00 2001 From: Artem Egorov Date: Fri, 5 Jan 2024 16:52:53 +0000 Subject: [PATCH 2/7] stick rails version for tests --- Gemfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile b/Gemfile index eefd680..9d9ce9c 100644 --- a/Gemfile +++ b/Gemfile @@ -1,6 +1,6 @@ -source 'https://rubygems.org' +source "https://rubygems.org" -gem 'rails' +gem "rails", "~> 6" group :development, :test do gem "rspec-rails", "~> 3" From f7cd321515cfa51a37d0a365946e13d95759d63a Mon Sep 17 00:00:00 2001 From: Artem Egorov Date: Fri, 5 Jan 2024 16:57:04 +0000 Subject: [PATCH 3/7] can't stick rails actually --- Gemfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index 9d9ce9c..a9f7234 100644 --- a/Gemfile +++ b/Gemfile @@ -1,6 +1,6 @@ source "https://rubygems.org" -gem "rails", "~> 6" +gem "rails" group :development, :test do gem "rspec-rails", "~> 3" From 1c937719f84a8602cf5a5d217e69fdd615b28a36 Mon Sep 17 00:00:00 2001 From: Artem Egorov Date: Fri, 5 Jan 2024 16:59:15 +0000 Subject: [PATCH 4/7] install rails 6 for tests only --- .github/workflows/ruby.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ruby.yml b/.github/workflows/ruby.yml index 16a50a7..0da8023 100644 --- a/.github/workflows/ruby.yml +++ b/.github/workflows/ruby.yml @@ -15,5 +15,6 @@ jobs: ruby-version: 2.7.2 - name: Build and test run: | + gem install rails -v "~> 6" bundle install --binstubs bin/rspec From 508fbb0fe2034cd2addf9111153885133c1c3e88 Mon Sep 17 00:00:00 2001 From: Artem Egorov Date: Fri, 5 Jan 2024 17:40:48 +0000 Subject: [PATCH 5/7] revert --- .github/workflows/ruby.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/ruby.yml b/.github/workflows/ruby.yml index 0da8023..16a50a7 100644 --- a/.github/workflows/ruby.yml +++ b/.github/workflows/ruby.yml @@ -15,6 +15,5 @@ jobs: ruby-version: 2.7.2 - name: Build and test run: | - gem install rails -v "~> 6" bundle install --binstubs bin/rspec From eabf05f9726d5238db11d6d56eef6765112f8f5d Mon Sep 17 00:00:00 2001 From: Artem Egorov Date: Mon, 8 Jan 2024 11:58:38 +0000 Subject: [PATCH 6/7] fix tests --- Gemfile | 3 +-- chatops-controller.gemspec | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile b/Gemfile index a9f7234..d86ba4a 100644 --- a/Gemfile +++ b/Gemfile @@ -1,8 +1,7 @@ source "https://rubygems.org" -gem "rails" - group :development, :test do + gem "rails", "~> 6" gem "rspec-rails", "~> 3" gem "pry", "~> 0" end 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" From e60af165bac95da42c8fe392d19d2445d13dd38f Mon Sep 17 00:00:00 2001 From: Artem Egorov Date: Mon, 8 Jan 2024 13:01:54 +0000 Subject: [PATCH 7/7] fix ensure_chatops_authenticated return --- lib/chatops/controller.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/chatops/controller.rb b/lib/chatops/controller.rb index 8f95150..8280eed 100644 --- a/lib/chatops/controller.rb +++ b/lib/chatops/controller.rb @@ -122,17 +122,16 @@ def ensure_chatops_authenticated body = request.raw_post || "" - valid = @chatops_urls.any? do |url| + @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 + return true end end - return true if valid return jsonrpc_error(-32800, 403, "Not authorized") end