From 68aac30f664bc8aa0e82b253bc5ba19a843e86b7 Mon Sep 17 00:00:00 2001 From: Imnet Edossa Date: Fri, 20 Dec 2024 14:52:35 +0300 Subject: [PATCH 1/4] PIPE-6553 | Upgrade http-hmac-ruby to Ruby 3.3 --- .rubocop.yml | 28 +++++++++ Gemfile | 2 +- Rakefile | 2 +- acquia-http-hmac.gemspec | 21 ++++--- bin/acq-http-request | 52 +++++++--------- example-sqlite3/setup.rb | 26 ++++---- example/app.rb | 12 ++-- example/config.ru | 4 +- features/step_definitions/common.rb | 60 +++++++++--------- features/support/env.rb | 2 +- lib/acquia-http-hmac.rb | 18 +++--- lib/acquia-http-hmac/rack_authenticate.rb | 59 ++++++++---------- .../sqlite3_password_storage.rb | 23 ++++--- test/acquia_spec_test.rb | 9 +-- test/helpers/rack_app_test_base.rb | 24 ++++---- test/httphmac_test.rb | 46 +++++++------- test/httphmac_verify_test.rb | 61 +++++++++---------- test/rack_simple_app_test.rb | 2 +- test/rack_sqlite3_app_test.rb | 7 +-- 19 files changed, 232 insertions(+), 226 deletions(-) create mode 100644 .rubocop.yml diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 0000000..29f6a3b --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1,28 @@ +AllCops: + DisplayCopNames: true + NewCops: disable + TargetRubyVersion: 3.3.6 + +Metrics/AbcSize: + Max: 50 + +Metrics/BlockLength: + Max: 150 + +Metrics/ClassLength: + Max: 1000 + +Metrics/CyclomaticComplexity: + Max: 12 + +Layout/LineLength: + Max: 200 + +Metrics/MethodLength: + Max: 50 + +Metrics/ModuleLength: + Max: 160 + +Metrics/PerceivedComplexity: + Max: 10 diff --git a/Gemfile b/Gemfile index b4e2a20..fa75df1 100644 --- a/Gemfile +++ b/Gemfile @@ -1,3 +1,3 @@ -source "https://rubygems.org" +source 'https://rubygems.org' gemspec diff --git a/Rakefile b/Rakefile index 70a4928..05e9c9a 100644 --- a/Rakefile +++ b/Rakefile @@ -3,7 +3,7 @@ require 'bundler/setup' begin require 'rake/testtask' Rake::TestTask.new do |t| - t.libs.push "lib" + t.libs.push 'lib' t.test_files = FileList['test/*_test.rb'] t.verbose = true end diff --git a/acquia-http-hmac.gemspec b/acquia-http-hmac.gemspec index a6b02de..9abb4ec 100644 --- a/acquia-http-hmac.gemspec +++ b/acquia-http-hmac.gemspec @@ -1,24 +1,27 @@ Gem::Specification.new do |s| s.name = 'acquia-http-hmac' - s.version = '2.0.2' + s.version = '2.0.3' + s.required_ruby_version = '>= 3.3.0' s.licenses = ['MIT'] - s.summary = "HMAC signing library and rack middleware" + s.summary = 'HMAC signing library and rack middleware' s.description = "HMAC signing library and rack middleware conforming to Acquia's HMAC 2.0 specifications" - s.date = Time.now.strftime("%Y-%m-%d") - s.authors = ["Peter Wolanin", "Marc Seeger"] + s.date = Time.now.strftime('%Y-%m-%d') + s.authors = ['Peter Wolanin', 'Marc Seeger'] s.email = 'engineering@acquia.com' - s.homepage = 'https://www.acquia.com/' - s.files = Dir["[A-Z]*", "{bin,etc,lib,test}/**/*"] + s.homepage = 'https://www.acquia.com/' + s.files = Dir['[A-Z]*', '{bin,etc,lib,test}/**/*'] s.bindir = 'bin' - s.require_paths = ["lib"] + s.require_paths = ['lib'] s.executables << 'acq-http-request' s.add_dependency 'addressable', '~> 2.4' - s.add_development_dependency('rake') s.add_development_dependency('grape') - s.add_development_dependency('rack-test') s.add_development_dependency('multi_json') + s.add_development_dependency 'rack', '~> 2.2.10' + s.add_development_dependency('rack-test') + s.add_development_dependency('rake') + s.add_development_dependency 'rubocop', '~> 1.69' s.add_development_dependency('sqlite3') s.add_development_dependency('webrick') end diff --git a/bin/acq-http-request b/bin/acq-http-request index ab7ff42..ca0c55d 100755 --- a/bin/acq-http-request +++ b/bin/acq-http-request @@ -4,9 +4,7 @@ require 'acquia-http-hmac' require 'optparse' require 'ostruct' -if ARGV[0] - url = ARGV[0] -end +url = ARGV[0] if ARGV[0] # Argument processing options = OpenStruct.new @@ -14,10 +12,10 @@ options.realm = 'Test' options.http_method = 'GET' o = OptionParser.new do |opts| opts.banner = "Usage: #{$0} URL -u ID:PASSWORD" - opts.on("-r", "--realm [REALM]", "Server auth realm. Default 'Test'.") { |v| options.realm = v } - opts.on("-u", "--user [ID:PASSWORD]", "HMAC creds") { |v| options.user = v } - opts.on("-d", "--data [DATA]", "Data to POST.") { |v| options.data = v } - opts.on("-X", "--request [METHOD]", "HTTP method. Defaults to GET, or POST if --data is specified.") { |v| options.http_method = v.upcase } + opts.on('-r', '--realm [REALM]', "Server auth realm. Default 'Test'.") { |v| options.realm = v } + opts.on('-u', '--user [ID:PASSWORD]', 'HMAC creds') { |v| options.user = v } + opts.on('-d', '--data [DATA]', 'Data to POST.') { |v| options.data = v } + opts.on('-X', '--request [METHOD]', 'HTTP method. Defaults to GET, or POST if --data is specified.') { |v| options.http_method = v.upcase } end begin o.parse! @@ -34,46 +32,38 @@ end uri = URI(Addressable::URI.escape.encode(url)) -if uri.path == '' - uri.path = '/' -end +uri.path = '/' if uri.path == '' host = uri.host -if uri.port && uri.port != '443' - host << ':' + uri.port -end +host << ':' + uri.port if uri.port && uri.port != '443' id, password = options.user.split(':', 2) mac = Acquia::HTTPHmac::Auth.new(options.realm, password) - - args = { http_method: options.http_method, host: host, id: id, - path_info: uri.path, + path_info: uri.path } -case -when options.http_method == 'GET' - req = Net::HTTP::GET.new(uri) -when options.http_method == 'HEAD' - req = Net::HTTP::HEAD.new(uri) -when options.http_method == 'POST' - req = Net::HTTP::POST.new(uri) -when options.http_method == 'PUT' - req = Net::HTTP::PUT.new(uri) -when options.http_method == 'DELETE' - req = Net::HTTP::DELETE.new(uri) +if options.http_method == 'GET' + Net::HTTP::GET.new(uri) +elsif options.http_method == 'HEAD' + Net::HTTP::HEAD.new(uri) +elsif options.http_method == 'POST' + Net::HTTP::POST.new(uri) +elsif options.http_method == 'PUT' + Net::HTTP::PUT.new(uri) +elsif options.http_method == 'DELETE' + Net::HTTP::DELETE.new(uri) else - fail("Unsupported HTTP verb #{options.http_method}") + raise("Unsupported HTTP verb #{options.http_method}") end mac.prepare_request_headers(args).each do |name, value| - #header(name, value) + # header(name, value) end net = Net::HTTP.new(uri.host, uri.port) -net.use_ssl= uri.host != 'localhost' - +net.use_ssl = uri.host != 'localhost' diff --git a/example-sqlite3/setup.rb b/example-sqlite3/setup.rb index 283e4d2..5522589 100644 --- a/example-sqlite3/setup.rb +++ b/example-sqlite3/setup.rb @@ -3,9 +3,7 @@ require 'openssl' require 'yaml' - class ExampleSQLite3Setup - def initialize(dbfile, passwords_file) @dbfile = dbfile File.unlink(@dbfile) if File.exist?(@dbfile) @@ -13,7 +11,7 @@ def initialize(dbfile, passwords_file) end def create_t1 - return <<-SQL + <<-SQL CREATE TABLE passwords ( id VARCHAR(50), request_date CHAR(10), @@ -24,7 +22,7 @@ def create_t1 end def create_t2 - return <<-SQL + <<-SQL CREATE TABLE password_data ( id VARCHAR(50), request_method VARCHAR(10), @@ -46,43 +44,43 @@ def write_database dates = [ today.strftime('%F'), - tomorrow.strftime('%F'), + tomorrow.strftime('%F') ] realm = 'Test' creds = YAML.safe_load(File.read(@passwords_file)) passwords = {} - creds.each do |id,data| + creds.each do |id, data| passwords[id] = data['password'] end data = { 'testadmin' => [ ['GET', '/'], - ['POST', '/'], + ['POST', '/'] ], 'testuser' => [], 'curltest' => [ - ['GET', '/'], - ], + ['GET', '/'] + ] } - sha256 = OpenSSL::Digest::SHA256.new + sha256 = OpenSSL::Digest.new('SHA256') - passwords.each do |id,pass| + passwords.each do |id, pass| # Run a 2-step HMAC KDF using date and realm binary_pass = Base64.decode64(pass) dates.each do |date| derived_pass1 = OpenSSL::HMAC.digest(sha256, binary_pass, date) derived_pass2 = OpenSSL::HMAC.digest(sha256, derived_pass1, realm) - db.execute("INSERT INTO passwords (id, request_date, base64_password) VALUES ( ?, ?, ? )", [id, date, Base64.strict_encode64(derived_pass2)]) + db.execute('INSERT INTO passwords (id, request_date, base64_password) VALUES ( ?, ?, ? )', [id, date, Base64.strict_encode64(derived_pass2)]) end end data.each do |id, values| values.each do |row| - row.unshift(id) - db.execute("INSERT INTO password_data VALUES ( ?, ?, ? )", row) + row.unshift(id) + db.execute('INSERT INTO password_data VALUES ( ?, ?, ? )', row) end end diff --git a/example/app.rb b/example/app.rb index ea18329..70265d3 100644 --- a/example/app.rb +++ b/example/app.rb @@ -14,28 +14,28 @@ class App < Grape::API helpers do def hellos # Store data in memory for simple testing. - @@hellos ||= {SecureRandom.uuid => "world"} + @@hellos ||= { SecureRandom.uuid => 'world' } @@hellos end end resource :hello do get do - {hello: hellos} + { hello: hellos } end - desc "Return a single hello." + desc 'Return a single hello.' get ':id' do - {hello: hellos[params[:id]]} + { hello: hellos[params[:id]] } end params do - requires :hello, type: String, desc: "A hello." + requires :hello, type: String, desc: 'A hello.' end post do id = SecureRandom.uuid hellos[id] = params[:hello] - {id => params[:hello]} + { id => params[:hello] } end end diff --git a/example/config.ru b/example/config.ru index 73682bc..e6c0757 100755 --- a/example/config.ru +++ b/example/config.ru @@ -1,6 +1,6 @@ #!/usr/bin/env rackup -p8010 -require "bundler/setup" +require 'bundler/setup' Bundler.require require_relative 'app' @@ -12,7 +12,7 @@ unless ENV['NO_AUTHENTICATION'] password_storage: passwords, realm: 'Test', nonce_checker: Acquia::HTTPHmac::MemoryNonceChecker.new, - excluded_paths: ['/healthcheck'], + excluded_paths: ['/healthcheck'] } use Acquia::HTTPHmac::RackAuthenticate, options end diff --git a/features/step_definitions/common.rb b/features/step_definitions/common.rb index 5a8df97..610c131 100644 --- a/features/step_definitions/common.rb +++ b/features/step_definitions/common.rb @@ -1,40 +1,40 @@ -Given /^the endpoint "(.*?)" "(.*?)"$/ do |method, path| - @method = method - @path = path +Given(/^the endpoint "(.*?)" "(.*?)"$/) do |method, path| + @method = method + @path = path end -Given /^the header "(.*?)" "(.*?)"$/ do |key, value| - @headers = Hash.new unless @headers - @headers[key] = value +Given(/^the header "(.*?)" "(.*?)"$/) do |key, value| + @headers ||= {} + @headers[key] = value end -Given /^the custom header "(.*?)" "(.*?)"$/ do |key, value| - @cheaders = Hash.new unless @cheaders - @cheaders[key] = value - @headers = Hash.new unless @headers - @headers[key] = value +Given(/^the custom header "(.*?)" "(.*?)"$/) do |key, value| + @cheaders ||= {} + @cheaders[key] = value + @headers ||= {} + @headers[key] = value end -Given /^the body "(.*?)"$/ do |body| - @body = body +Given(/^the body "(.*?)"$/) do |body| + @body = body end -When /^I sign the request with the "(.*?)" digest and secret key "(.*?)"$/ do |digest, key| - signer = Acquia::HTTPHmac.new(@method, @body, @headers, @cheaders, @path) - key = key - case digest - when "SHA-1" - digester = Digest::SHA1 - when "SHA1" - digester = Digest::SHA1 - when "SHA-256" - digester = Digest::SHA256 - when "SHA256" - digester = Digest::SHA256 - end - @signature = signer.sign(digester, key) +When(/^I sign the request with the "(.*?)" digest and secret key "(.*?)"$/) do |digest, key| + signer = Acquia::HTTPHmac.new(@method, @body, @headers, @cheaders, @path) + key = key + case digest + when 'SHA-1' + digester = Digest::SHA1 + when 'SHA1' + digester = Digest::SHA1 + when 'SHA-256' + digester = Digest::SHA256 + when 'SHA256' + digester = Digest::SHA256 + end + @signature = signer.sign(digester, key) end -Then /^I should see the signature "(.*?)"$/ do |signature| - expect(@signature).to eq(signature) -end \ No newline at end of file +Then(/^I should see the signature "(.*?)"$/) do |signature| + expect(@signature).to eq(signature) +end diff --git a/features/support/env.rb b/features/support/env.rb index d86c6f3..ec1d2b4 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -1 +1 @@ -require 'acquia/httphmac' \ No newline at end of file +require 'acquia/httphmac' diff --git a/lib/acquia-http-hmac.rb b/lib/acquia-http-hmac.rb index 4b0b4ed..aa8d130 100644 --- a/lib/acquia-http-hmac.rb +++ b/lib/acquia-http-hmac.rb @@ -8,7 +8,6 @@ module HTTPHmac VERSION = '2.0' class Auth - def initialize(realm, base64_secret) @realm = realm @secret = Base64.decode64(base64_secret) @@ -37,7 +36,7 @@ def prepare_request_headers(args = {}) content_type: '', headers: {}, body_hash: nil, - version: VERSION, + version: VERSION }.merge(args) # Replace args so that the calling method gets all the values. args.replace(merged_args) @@ -81,9 +80,11 @@ def prepare_request_headers(args = {}) def request_authenticated?(args = {}) return false unless args[:realm] == @realm return false unless args[:nonce].match(/[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}/) + # Allow up to 900 sec (15 min) of clock skew by default. allowed_skew = args[:allowed_skew] || 900 return false if (Time.now.to_i - args[:timestamp].to_i).abs > allowed_skew + base_string = prepare_base_string(args) signature(base_string) == args[:signature] end @@ -122,9 +123,9 @@ def prepare_base_string(args = {}) base_string_parts = [args[:http_method], args[:host].downcase, args[:path_info]] base_string_parts << args[:query_string] base_string_parts << "id=#{Addressable::URI.escape(args[:id])}&nonce=#{args[:nonce]}&realm=#{Addressable::URI.escape(@realm)}&version=#{args[:version]}" - headers = args[:headers].to_a.sort do |x,y| - (key_x, val_x) = x - (key_y, val_y) = y + headers = args[:headers].to_a.sort do |x, y| + (key_x,) = x + (key_y,) = y key_x.downcase <=> key_y.downcase end headers.each do |h| @@ -146,11 +147,12 @@ def self.parse_auth_header(header) nonce: '', realm: '', signature: '', - version: '', + version: '' } header.to_s.sub(/^acquia-http-hmac\s+/, '').split(/,\s*/).each do |value| - m = value.match(/^(\w+)\=\"([^\"]*)\"$/) + m = value.match(/^(\w+)="([^"]*)"$/) break unless m + attributes[m[1].to_sym] = Addressable::URI.unescape(m[2]) end # Re-format custom headers to hash keys. @@ -161,7 +163,7 @@ def self.parse_auth_header(header) end def signature(base_string) - Base64.strict_encode64(OpenSSL::HMAC.digest(OpenSSL::Digest::SHA256.new, @secret, base_string)) + Base64.strict_encode64(OpenSSL::HMAC.digest(OpenSSL::Digest.new('SHA256'), @secret, base_string)) end end end diff --git a/lib/acquia-http-hmac/rack_authenticate.rb b/lib/acquia-http-hmac/rack_authenticate.rb index 148982d..020d999 100644 --- a/lib/acquia-http-hmac/rack_authenticate.rb +++ b/lib/acquia-http-hmac/rack_authenticate.rb @@ -16,14 +16,14 @@ def initialize(app, options) def call(env) # Skip paths based on a list of prefixes. - if @excluded_paths && env['PATH_INFO'].start_with?(*@excluded_paths) - return @app.call(env) - end + return @app.call(env) if @excluded_paths && env['PATH_INFO'].start_with?(*@excluded_paths) + auth_header = env['HTTP_AUTHORIZATION'].to_s return unauthorized if auth_header.empty? attributes = Acquia::HTTPHmac::Auth.parse_auth_header(auth_header) return denied('Invalid nonce') unless @nonce_checker.valid?(attributes[:id], attributes[:nonce]) + args = args_for_authenticator(env, attributes) mac = message_authenticator(args[:id], args[:timestamp]) return denied('Invalid credentials') unless mac && mac.request_authenticated?(args) @@ -40,31 +40,27 @@ def call(env) private def unauthorized - [ 401, - { - 'Content-Type' => 'text/plain', - 'Content-Length' => '0', - 'WWW-Authenticate' => 'acquia-http-hmac realm="'+ @realm +'"' - }, - [] - ] + [401, + { + 'Content-Type' => 'text/plain', + 'Content-Length' => '0', + 'WWW-Authenticate' => 'acquia-http-hmac realm="' + @realm + '"' + }, + []] end def denied(message) - [ 403, - { - 'Content-Type' => 'text/plain', - 'Connection' => 'close', - }, - [message] - ] + [403, + { + 'Content-Type' => 'text/plain', + 'Connection' => 'close' + }, + [message]] end def message_authenticator(id, timestamp) mac = nil - if @password_storage.valid?(id) - mac = Acquia::HTTPHmac::Auth.new(@realm, @password_storage.password(id, timestamp)) - end + mac = Acquia::HTTPHmac::Auth.new(@realm, @password_storage.password(id, timestamp)) if @password_storage.valid?(id) mac end @@ -77,7 +73,7 @@ def args_for_authenticator(env, attributes) path_info: request.path_info, content_type: request.content_type, body_hash: env['HTTP_X_AUTHORIZATION_CONTENT_SHA256'], - timestamp: env['HTTP_X_AUTHORIZATION_TIMESTAMP'].to_i, + timestamp: env['HTTP_X_AUTHORIZATION_TIMESTAMP'].to_i }.merge(attributes) # Map expected header names to the key that would be in env. attributes[:headers].keys.each do |name| @@ -121,13 +117,11 @@ def sign_response(status, headers, resp_body, nonce, timestamp, mac) headers['Cache-Control'] = 'no-transform, no-cache, no-store, private, max-age=0' [status, headers, [final_body]] end - end ### The classes below are primarily for testing. class SimplePasswordStorage - def initialize(creds = {}) @@creds = creds end @@ -143,13 +137,15 @@ def valid?(id) # @param [Integer] timestamp # A unix timestamp. The returned password may be different based on # the current date or time. - def password(id, timestamp) - fail('Invalid id') unless @@creds[id] && @@creds[id]['password'] + def password(id, _timestamp) + raise('Invalid id') unless @@creds[id] && @@creds[id]['password'] + @@creds[id]['password'] end def data(id) - fail('Invalid id') unless @@creds[id] + raise('Invalid id') unless @@creds[id] + @@creds[id] end @@ -159,18 +155,15 @@ def ids end class FilePasswordStorage < SimplePasswordStorage - def initialize(filename) creds = {} - if File.exist?(filename) - creds = YAML.safe_load(File.read(filename)) - end + creds = YAML.safe_load(File.read(filename)) if File.exist?(filename) super(creds) end end class NoopNonceChecker - def valid?(id, nonce) + def valid?(_id, nonce) nonce.length == 36 end end @@ -183,6 +176,7 @@ def initialize def valid?(id, nonce) # A UUID is 36 characters. return false unless nonce.length == 36 + @@seen[id] ||= {} valid = !@@seen[id][nonce] @@seen[id][nonce] = Time.now.to_i @@ -191,4 +185,3 @@ def valid?(id, nonce) end end end - diff --git a/lib/acquia-http-hmac/sqlite3_password_storage.rb b/lib/acquia-http-hmac/sqlite3_password_storage.rb index 6e57a23..ac7eff6 100644 --- a/lib/acquia-http-hmac/sqlite3_password_storage.rb +++ b/lib/acquia-http-hmac/sqlite3_password_storage.rb @@ -4,9 +4,7 @@ module Acquia module HTTPHmac - class SQLite3PasswordStorage - def initialize(filename) @filename = filename @creds = {} @@ -25,13 +23,15 @@ def valid?(id) # A unix timestamp. The returned password may be different based on # the current date or time. def password(id, timestamp) - fail('Invalid id') unless valid?(id) + raise('Invalid id') unless valid?(id) + load(id, timestamp.to_i) @creds[id]['password'] end def data(id) - fail('Invalid id') unless valid?(id) + raise('Invalid id') unless valid?(id) + result = [] connection.execute('SELECT * FROM password_data WHERE id = ?', [today]) do |row| result << row @@ -51,17 +51,17 @@ def ids def load(id, timestamp = nil) date = timestamp ? Time.at(timestamp).utc.strftime('%F') : today - if @creds[id].nil? || date != today - @creds[id] = false - connection.execute('SELECT base64_password FROM passwords WHERE id = ? AND request_date = ?', [id, date]) do |row| - @creds[id] = {} - @creds[id]['password'] = row['base64_password'] - end + return unless @creds[id].nil? || date != today + + @creds[id] = false + connection.execute('SELECT base64_password FROM passwords WHERE id = ? AND request_date = ?', [id, date]) do |row| + @creds[id] = {} + @creds[id]['password'] = row['base64_password'] end end def connection - @connection ||= SQLite3::Database.new(@filename, { readonly: true, results_as_hash: true }) + @connection ||= SQLite3::Database.new(@filename, { readonly: true, results_as_hash: true }) end def today @@ -70,4 +70,3 @@ def today end end end - diff --git a/test/acquia_spec_test.rb b/test/acquia_spec_test.rb index 86e0324..f7308e7 100644 --- a/test/acquia_spec_test.rb +++ b/test/acquia_spec_test.rb @@ -2,7 +2,6 @@ require_relative '../lib/acquia-http-hmac' class TestAcquiaHmacSpec < Minitest::Test - def test_fixture fixtures_path = File.join(File.dirname(__FILE__), '../fixtures/acquia_spec.json') fixtures_json = File.read(File.realpath(fixtures_path)) @@ -30,7 +29,7 @@ def test_fixture nonce: input['nonce'], timestamp: input['timestamp'], headers: signed_headers, - body_hash: body_hash, + body_hash: body_hash } headers = mac.prepare_request_headers(args) @@ -41,18 +40,16 @@ def test_fixture assert(headers['Authorization'].include?("nonce=\"#{input['nonce']}\"")) expected_headers = input['signed_headers'].join(';') assert(headers['Authorization'].include?("headers=\"#{expected_headers}\"")) - assert(headers['Authorization'].include?("version=\"2.0\"")) + assert(headers['Authorization'].include?('version="2.0"')) assert(headers['Authorization'].include?("signature=\"#{expectations['message_signature']}\"")) # Prove that we can authenticate the request. - attributes = Acquia::HTTPHmac::Auth::parse_auth_header(expectations['authorization_header']) + attributes = Acquia::HTTPHmac::Auth.parse_auth_header(expectations['authorization_header']) auth_args = args.merge(attributes) auth_args[:allowed_skew] = input['timestamp'] + 900 auth_args[:headers] = signed_headers ret = mac.request_authenticated?(auth_args) assert(ret, "request_authenticated? failed for #{input['name']}") end - end - end diff --git a/test/helpers/rack_app_test_base.rb b/test/helpers/rack_app_test_base.rb index df70ca0..f329eff 100644 --- a/test/helpers/rack_app_test_base.rb +++ b/test/helpers/rack_app_test_base.rb @@ -17,7 +17,7 @@ def prepare_get(id, password, args = {}) http_method: 'GET', host: 'example.org', # Default in the Rack test id: id, - path_info: '/hello', + path_info: '/hello' }.merge(args) mac.prepare_request_headers(args).each do |name, value| header(name, value) @@ -34,7 +34,7 @@ def prepare_post(id, password, body, args = {}) id: id, path_info: '/hello', body: body, - content_type: 'application/json', + content_type: 'application/json' }.merge(args) mac.prepare_request_headers(args).each do |name, value| header(name, value) @@ -46,20 +46,20 @@ def prepare_post(id, password, body, args = {}) # Add just the auth middleware def app passwords = get_password_storage - Rack::Builder.new { - map "/" do + Rack::Builder.new do + map '/' do # Need this base middleware so that request.logger is defined. use Rack::NullLogger options = { password_storage: passwords, realm: 'Test', nonce_checker: Acquia::HTTPHmac::MemoryNonceChecker.new, - excluded_paths: ['/healthcheck'], + excluded_paths: ['/healthcheck'] } use Acquia::HTTPHmac::RackAuthenticate, options run Example::App end - }.to_app + end.to_app end def test_401_get @@ -126,7 +126,7 @@ def test_403_missing_header_get passwords = get_password_storage id = passwords.ids.first # Pass a header expected to be signed. - prepare_get(id, get_password(id), headers: {'X-Custom-Foo' => 'nick'}) + prepare_get(id, get_password(id), headers: { 'X-Custom-Foo' => 'nick' }) get '/hello' # The expected header was missing. assert_equal(403, last_response.status) @@ -136,7 +136,7 @@ def test_403_mismatched_header_get passwords = get_password_storage id = passwords.ids.first # Pass a header expected to be signed. - prepare_get(id, get_password(id), headers: {'X-Custom-Foo' => 'nick'}) + prepare_get(id, get_password(id), headers: { 'X-Custom-Foo' => 'nick' }) # The expected header has a different value. header('X-Custom-Foo', 'nack') get '/hello' @@ -171,7 +171,7 @@ def test_get_with_extra_header passwords = get_password_storage id = passwords.ids.first # Pass a header expected to be signed. - prepare_get(id, get_password(id), headers: {'X-Custom-Foo' => 'nick'}) + prepare_get(id, get_password(id), headers: { 'X-Custom-Foo' => 'nick' }) header('X-Custom-Foo', 'nick') get '/hello' assert_equal(200, last_response.status) @@ -181,7 +181,7 @@ def test_get_with_quoted_header passwords = get_password_storage id = passwords.ids.first # Pass a header expected to be signed. - prepare_get(id, get_password(id), headers: {'X-Custom-Foo' => '"nick"'}) + prepare_get(id, get_password(id), headers: { 'X-Custom-Foo' => '"nick"' }) header('X-Custom-Foo', '"nick"') get '/hello' assert_equal(200, last_response.status) @@ -191,7 +191,7 @@ def test_get_with_spaces_in_header passwords = get_password_storage id = passwords.ids.first # Pass a header expected to be signed. - prepare_get(id, get_password(id), headers: {'X-Custom-Foo' => 'a b c '}) + prepare_get(id, get_password(id), headers: { 'X-Custom-Foo' => 'a b c ' }) header('X-Custom-Foo', ' a b c ') get '/hello' assert_equal(200, last_response.status) @@ -201,7 +201,7 @@ def test_get_with_spaces_in_quoted_header passwords = get_password_storage id = passwords.ids.first # Pass a header expected to be signed. - prepare_get(id, get_password(id), headers: {'X-Custom-Foo' => '"hi nick" '}) + prepare_get(id, get_password(id), headers: { 'X-Custom-Foo' => '"hi nick" ' }) header('X-Custom-Foo', ' "hi nick" ') get '/hello' assert_equal(200, last_response.status) diff --git a/test/httphmac_test.rb b/test/httphmac_test.rb index 30f25d5..13bdaf1 100644 --- a/test/httphmac_test.rb +++ b/test/httphmac_test.rb @@ -2,38 +2,37 @@ require_relative '../lib/acquia-http-hmac' class TestHTTPHmac < Minitest::Test - def test_prepare_request_get - mac = Acquia::HTTPHmac::Auth.new('TestRealm', "dGhlc2VjcmV0") + mac = Acquia::HTTPHmac::Auth.new('TestRealm', 'dGhlc2VjcmV0') args = { http_method: 'GET', host: 'www.example.com', id: 'test', - path_info: '/hello', + path_info: '/hello' } headers = mac.prepare_request_headers(args) auth_header = headers['Authorization'] - assert(auth_header.match /acquia-http-hmac realm="TestRealm",id="test",nonce="[0-9a-f-]{36}",version="2\.0",headers="[^"]*",signature="[^"]+"/) + assert(auth_header.match(/acquia-http-hmac realm="TestRealm",id="test",nonce="[0-9a-f-]{36}",version="2\.0",headers="[^"]*",signature="[^"]+"/)) # Repeat with known nonce and timestamp # "dGhlc2VjcmV0" is base64 of 'thesecret' - mac = Acquia::HTTPHmac::Auth.new('TestRealm', "dGhlc2VjcmV0") - args[:nonce] = "f2c91a46-b505-4b50-afa2-21364dc8ff34" - args[:timestamp] = "1432180014" + mac = Acquia::HTTPHmac::Auth.new('TestRealm', 'dGhlc2VjcmV0') + args[:nonce] = 'f2c91a46-b505-4b50-afa2-21364dc8ff34' + args[:timestamp] = '1432180014' headers = mac.prepare_request_headers(args) auth_header = headers['Authorization'] # We expect the following base string: # GET # www.example.com # /hello - # + # # id=test&nonce=f2c91a46-b505-4b50-afa2-21364dc8ff34&realm=TestRealm&version=2.0 # 1432180014 m = auth_header.match(/.*,signature="([^"]+)"$/) assert(m, 'Did not find signature') # Compare to a signature calulated with the base string in PHP. - assert_equal("hKHBXbx9KDirAWpvYKGOqHVSLn6yjD3V5aaQTRklPPA=", m[1]) + assert_equal('hKHBXbx9KDirAWpvYKGOqHVSLn6yjD3V5aaQTRklPPA=', m[1]) # Repeast with a query string that needs to be normalized. args[:query_string] = 'base=foo&all' headers = mac.prepare_request_headers(args) @@ -48,24 +47,24 @@ def test_prepare_request_get m = auth_header.match(/.*,signature="([^"]+)"$/) assert(m, 'Did not find signature') # Compare to a signature calulated with the base string in PHP. - assert_equal("dvl8wLvEcLbAtKfvIYaIGThIXHBpOtOTw7dQX4nBjwM=", m[1]) + assert_equal('dvl8wLvEcLbAtKfvIYaIGThIXHBpOtOTw7dQX4nBjwM=', m[1]) end def test_prepare_request_get_headers - mac = Acquia::HTTPHmac::Auth.new('TestRealm', "dGhlc2VjcmV0") + mac = Acquia::HTTPHmac::Auth.new('TestRealm', 'dGhlc2VjcmV0') args = { http_method: 'GET', host: 'www.example.com', id: 'test', path_info: '/hello', query_string: 'base=foo&all', - nonce: "f2c91a46-b505-4b50-afa2-21364dc8ff34", - timestamp: "1432180014", - headers: {'x-custom-foo' => 'nick'} + nonce: 'f2c91a46-b505-4b50-afa2-21364dc8ff34', + timestamp: '1432180014', + headers: { 'x-custom-foo' => 'nick' } } headers = mac.prepare_request_headers(args) auth_header = headers['Authorization'] - assert(auth_header.match /acquia-http-hmac realm="TestRealm",id="test",nonce="[0-9a-f-]{36}",version="2\.0",headers="[^"]*",signature="[^"]+"/) + assert(auth_header.match(/acquia-http-hmac realm="TestRealm",id="test",nonce="[0-9a-f-]{36}",version="2\.0",headers="[^"]*",signature="[^"]+"/)) # We expect the following base string: # GET @@ -78,37 +77,36 @@ def test_prepare_request_get_headers m = auth_header.match(/.*,signature="([^"]+)"$/) assert(m, 'Did not find signature') # Compare to a signature calulated with the base string in PHP. - assert_equal("RuYnAieiiOOWWAZ0tjZ/+HMebpBCBhGSYEWWBF+lP28=", m[1]) + assert_equal('RuYnAieiiOOWWAZ0tjZ/+HMebpBCBhGSYEWWBF+lP28=', m[1]) end def test_prepare_request_post # Use known nonce and timestamp - mac = Acquia::HTTPHmac::Auth.new('TestRealm', "dGhlc2VjcmV0") + mac = Acquia::HTTPHmac::Auth.new('TestRealm', 'dGhlc2VjcmV0') args = { http_method: 'POST', host: 'www.example.com', id: 'test', path_info: '/hello', - nonce: "f2c91a46-b505-4b50-afa2-21364dc8ab34", - timestamp: "1432180014", + nonce: 'f2c91a46-b505-4b50-afa2-21364dc8ab34', + timestamp: '1432180014' } args[:body] = '{"method":"hi.bob","params":["5","4","8"]}' args[:content_type] = 'application/json' headers = mac.prepare_request_headers(args) auth_header = headers['Authorization'] - assert(auth_header.match /acquia-http-hmac realm="TestRealm",id="test",nonce="[0-9a-f-]{36}",version="2\.0",headers="[^"]*",signature="[^"]+"/) - assert_equal(headers['X-Authorization-Content-SHA256'], "6paRNxUA7WawFxJpRp4cEixDjHq3jfIKX072k9slalo=") + assert(auth_header.match(/acquia-http-hmac realm="TestRealm",id="test",nonce="[0-9a-f-]{36}",version="2\.0",headers="[^"]*",signature="[^"]+"/)) + assert_equal(headers['X-Authorization-Content-SHA256'], '6paRNxUA7WawFxJpRp4cEixDjHq3jfIKX072k9slalo=') # We expect the following base string: # POST # www.example.com # /hello - # + # # id=test&nonce=f2c91a46-b505-4b50-afa2-21364dc8ab34&realm=TestRealm&version=2.0 # 1432180014 # application/json # 6paRNxUA7WawFxJpRp4cEixDjHq3jfIKX072k9slalo= m = auth_header.match(/.*,signature="([^"]+)"$/) - assert_equal("Un7AVsJ80yxzR0Jn+LB6orziDrAistPNm3h33bNZiJ0=", m[1]) + assert_equal('Un7AVsJ80yxzR0Jn+LB6orziDrAistPNm3h33bNZiJ0=', m[1]) end - end diff --git a/test/httphmac_verify_test.rb b/test/httphmac_verify_test.rb index bf2a5dc..f3c259a 100644 --- a/test/httphmac_verify_test.rb +++ b/test/httphmac_verify_test.rb @@ -3,38 +3,37 @@ require_relative '../lib/acquia-http-hmac' class HmacVerifyTest < Minitest::Test - def get_params { - :http_method => 'GET', - :host => 'example.com', - :id => '12345', - :path_info => "/some/path", - :query_string => "foo=bar", - :body => nil, - :content_type => "application/json", - :nonce => '869a8b00-f96f-4a9e-98e6-a6e38b0de316', - :timestamp => Time.now.to_i + http_method: 'GET', + host: 'example.com', + id: '12345', + path_info: '/some/path', + query_string: 'foo=bar', + body: nil, + content_type: 'application/json', + nonce: '869a8b00-f96f-4a9e-98e6-a6e38b0de316', + timestamp: Time.now.to_i } end def post_params { - :http_method => 'POST', - :host => 'example.com', - :id => '54321', - :path_info => "/another/path", - :query_string => "foo=bar", - :body => 'tbd: yes', - :content_type => "application/json", - :nonce => '869a8b00-f96f-4a9e-98e6-a6e38b0de316', - :timestamp => Time.now.to_i + http_method: 'POST', + host: 'example.com', + id: '54321', + path_info: '/another/path', + query_string: 'foo=bar', + body: 'tbd: yes', + content_type: 'application/json', + nonce: '869a8b00-f96f-4a9e-98e6-a6e38b0de316', + timestamp: Time.now.to_i } end def setup # "dGhlc2VjcmV0" is base64 of 'thesecret' - @secret = "dGhlc2VjcmV0" + @secret = 'dGhlc2VjcmV0' @realm = 'TestRealm' hmac = Acquia::HTTPHmac::Auth.new(@realm, @secret) @@ -43,33 +42,33 @@ def setup end def test_get_no_body - attributes = Acquia::HTTPHmac::Auth::parse_auth_header(@req_get['Authorization']) + attributes = Acquia::HTTPHmac::Auth.parse_auth_header(@req_get['Authorization']) hmac = Acquia::HTTPHmac::Auth.new(@realm, @secret) ret = hmac.request_authenticated?(get_params.merge(attributes)) - assert(ret, "request_authenticated? failed for GET") + assert(ret, 'request_authenticated? failed for GET') end def test_it_fails_with_invalid_realm - attributes = Acquia::HTTPHmac::Auth::parse_auth_header(@req_get['Authorization']) + attributes = Acquia::HTTPHmac::Auth.parse_auth_header(@req_get['Authorization']) hmac = Acquia::HTTPHmac::Auth.new('bad_realm', @secret) ret = hmac.request_authenticated?(get_params.merge(attributes)) - assert(!ret, "request_authenticated? accepted invalid realm") + assert(!ret, 'request_authenticated? accepted invalid realm') end def test_it_fails_with_invalid_secret - attributes = Acquia::HTTPHmac::Auth::parse_auth_header(@req_get['Authorization']) + attributes = Acquia::HTTPHmac::Auth.parse_auth_header(@req_get['Authorization']) hmac = Acquia::HTTPHmac::Auth.new(@realm, Base64.strict_encode64('wrong password')) ret = hmac.request_authenticated?(get_params.merge(attributes)) - assert(!ret, "request_authenticated? accepted invalid secret") + assert(!ret, 'request_authenticated? accepted invalid secret') end def test_post_with_body params = post_params params[:body_hash] = @req_post['X-Authorization-Content-SHA256'] - attributes = Acquia::HTTPHmac::Auth::parse_auth_header(@req_post['Authorization']) + attributes = Acquia::HTTPHmac::Auth.parse_auth_header(@req_post['Authorization']) hmac = Acquia::HTTPHmac::Auth.new(@realm, @secret) ret = hmac.request_authenticated?(params.merge(attributes)) - assert(ret, "request_authenticated? failed for POST") + assert(ret, 'request_authenticated? failed for POST') end def test_it_requires_recent_timestamp @@ -79,8 +78,8 @@ def test_it_requires_recent_timestamp params[:timestamp] = params[:timestamp].to_i - 901 hmac = Acquia::HTTPHmac::Auth.new(@realm, @secret) get = hmac.prepare_request_headers(params) - attributes = Acquia::HTTPHmac::Auth::parse_auth_header(get['Authorization']) + attributes = Acquia::HTTPHmac::Auth.parse_auth_header(get['Authorization']) ret = hmac.request_authenticated?(params.merge(attributes)) - assert(!ret, "request_authenticated? accepted old timestamp") + assert(!ret, 'request_authenticated? accepted old timestamp') end -end \ No newline at end of file +end diff --git a/test/rack_simple_app_test.rb b/test/rack_simple_app_test.rb index 55590d6..7297821 100644 --- a/test/rack_simple_app_test.rb +++ b/test/rack_simple_app_test.rb @@ -8,7 +8,7 @@ def get_password_storage @passwords ||= Acquia::HTTPHmac::FilePasswordStorage.new(File.dirname(__FILE__) + '/../fixtures/passwords.yml') end - def get_password(id, timestamp = nil) + def get_password(id, _timestamp = nil) get_password_storage.data(id)['password'] end end diff --git a/test/rack_sqlite3_app_test.rb b/test/rack_sqlite3_app_test.rb index d223333..e6a7d50 100644 --- a/test/rack_sqlite3_app_test.rb +++ b/test/rack_sqlite3_app_test.rb @@ -14,17 +14,17 @@ def setup s = ExampleSQLite3Setup.new(@dbfile, @passwords_file) s.write_database @binary_passwords = {} - YAML.safe_load(File.read(@passwords_file)).each do |id,data| + YAML.safe_load(File.read(@passwords_file)).each do |id, data| @binary_passwords[id] = Base64.decode64(data['password']) end end - def get_password(id, timestamp = nil) + def get_password(id, _timestamp = nil) ts = Time.now.to_i date = Time.at(ts).utc.strftime('%F') realm = 'Test' # Run a 2-step HMAC KDF using date and realm - sha256 = OpenSSL::Digest::SHA256.new + sha256 = OpenSSL::Digest.new('SHA256') derived_pass1 = OpenSSL::HMAC.digest(sha256, @binary_passwords[id], date) derived_pass2 = OpenSSL::HMAC.digest(sha256, derived_pass1, realm) Base64.strict_encode64(derived_pass2) @@ -33,5 +33,4 @@ def get_password(id, timestamp = nil) def get_password_storage @storage ||= Acquia::HTTPHmac::SQLite3PasswordStorage.new(@dbfile) end - end From 1ffc98c33a9e7a1b02cfe4058c8e8660887edd84 Mon Sep 17 00:00:00 2001 From: Imnet Edossa Date: Fri, 20 Dec 2024 15:34:21 +0300 Subject: [PATCH 2/4] Final fixes --- .rubocop.yml | 21 +++++++++++++++++- Gemfile | 2 ++ Rakefile | 13 +++++++++-- acquia-http-hmac.gemspec | 2 ++ bin/acq-http-request | 20 +++++++++-------- example-sqlite3/setup.rb | 5 ++++- example/app.rb | 3 +++ example/config.ru | 3 ++- features/step_definitions/common.rb | 3 ++- features/support/env.rb | 2 ++ lib/acquia-http-hmac/rack_authenticate.rb | 17 +++++++++----- .../sqlite3_password_storage.rb | 3 +++ ...cquia-http-hmac.rb => acquia_http_hmac.rb} | 5 ++++- test/acquia_spec_test.rb | 4 +++- test/helpers/rack_app_test_base.rb | 8 ++++--- test/httphmac_test.rb | 4 +++- test/httphmac_verify_test.rb | 22 ++++++++++--------- test/rack_simple_app_test.rb | 4 +++- test/rack_sqlite3_app_test.rb | 6 +++-- 19 files changed, 107 insertions(+), 40 deletions(-) rename lib/{acquia-http-hmac.rb => acquia_http_hmac.rb} (98%) diff --git a/.rubocop.yml b/.rubocop.yml index 29f6a3b..f2e41f5 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -3,8 +3,27 @@ AllCops: NewCops: disable TargetRubyVersion: 3.3.6 +Style/FrozenStringLiteralComment: + Exclude: + - 'lib/acquia-http-hmac/rack_authenticate.rb' + +Metrics/ParameterLists: + Max: 7 + +Style/ClassVars: + Exclude: + - 'example/app.rb' + - 'lib/acquia-http-hmac/rack_authenticate.rb' + Metrics/AbcSize: Max: 50 + Exclude: + - 'test/acquia_spec_test.rb' + +Naming/AccessorMethodName: + Exclude: + - 'test/rack_simple_app_test.rb' + - 'test/rack_sqlite3_app_test.rb' Metrics/BlockLength: Max: 150 @@ -22,7 +41,7 @@ Metrics/MethodLength: Max: 50 Metrics/ModuleLength: - Max: 160 + Max: 250 Metrics/PerceivedComplexity: Max: 10 diff --git a/Gemfile b/Gemfile index fa75df1..7f4f5e9 100644 --- a/Gemfile +++ b/Gemfile @@ -1,3 +1,5 @@ +# frozen_string_literal: true + source 'https://rubygems.org' gemspec diff --git a/Rakefile b/Rakefile index 05e9c9a..bb52302 100644 --- a/Rakefile +++ b/Rakefile @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'bundler/setup' begin @@ -7,6 +9,13 @@ begin t.test_files = FileList['test/*_test.rb'] t.verbose = true end - task(default: [:test]) -rescue LoadError + + desc 'Run RuboCop' + task :rubocop do + sh 'rubocop' + end + + task default: %i[test rubocop] +rescue LoadError => e + warn "Could not load rake/testtask: #{e}" end diff --git a/acquia-http-hmac.gemspec b/acquia-http-hmac.gemspec index 9abb4ec..25fb051 100644 --- a/acquia-http-hmac.gemspec +++ b/acquia-http-hmac.gemspec @@ -1,3 +1,5 @@ +# frozen_string_literal: true + Gem::Specification.new do |s| s.name = 'acquia-http-hmac' s.version = '2.0.3' diff --git a/bin/acq-http-request b/bin/acq-http-request index ca0c55d..d132b62 100755 --- a/bin/acq-http-request +++ b/bin/acq-http-request @@ -1,4 +1,5 @@ #!/usr/bin/env ruby +# frozen_string_literal: true require 'acquia-http-hmac' require 'optparse' @@ -11,7 +12,7 @@ options = OpenStruct.new options.realm = 'Test' options.http_method = 'GET' o = OptionParser.new do |opts| - opts.banner = "Usage: #{$0} URL -u ID:PASSWORD" + opts.banner = "Usage: #{$PROGRAM_NAME} URL -u ID:PASSWORD" opts.on('-r', '--realm [REALM]', "Server auth realm. Default 'Test'.") { |v| options.realm = v } opts.on('-u', '--user [ID:PASSWORD]', 'HMAC creds') { |v| options.user = v } opts.on('-d', '--data [DATA]', 'Data to POST.') { |v| options.data = v } @@ -19,13 +20,13 @@ o = OptionParser.new do |opts| end begin o.parse! -rescue Exception => e +rescue StandardError => e puts e.message puts o.help exit 1 end -if ARGV.empty? or !options.user +if ARGV.empty? || !options.user puts o.help exit end @@ -35,7 +36,7 @@ uri = URI(Addressable::URI.escape.encode(url)) uri.path = '/' if uri.path == '' host = uri.host -host << ':' + uri.port if uri.port && uri.port != '443' +host << ":#{uri.port}" if uri.port && uri.port != '443' id, password = options.user.split(':', 2) @@ -48,15 +49,16 @@ args = { path_info: uri.path } -if options.http_method == 'GET' +case options.http_method +when 'GET' Net::HTTP::GET.new(uri) -elsif options.http_method == 'HEAD' +when 'HEAD' Net::HTTP::HEAD.new(uri) -elsif options.http_method == 'POST' +when 'POST' Net::HTTP::POST.new(uri) -elsif options.http_method == 'PUT' +when 'PUT' Net::HTTP::PUT.new(uri) -elsif options.http_method == 'DELETE' +when 'DELETE' Net::HTTP::DELETE.new(uri) else raise("Unsupported HTTP verb #{options.http_method}") diff --git a/example-sqlite3/setup.rb b/example-sqlite3/setup.rb index 5522589..89ad837 100644 --- a/example-sqlite3/setup.rb +++ b/example-sqlite3/setup.rb @@ -1,8 +1,11 @@ +# frozen_string_literal: true + require 'sqlite3' require 'base64' require 'openssl' require 'yaml' +# Class: ExampleSQLite3Setup class ExampleSQLite3Setup def initialize(dbfile, passwords_file) @dbfile = dbfile @@ -88,7 +91,7 @@ def write_database end end -if $0 == __FILE__ +if $PROGRAM_NAME == __FILE__ mypath = File.dirname(__FILE__) dbfile = File.join(mypath, 'passwords.sqlite3') diff --git a/example/app.rb b/example/app.rb index 70265d3..7615f8d 100644 --- a/example/app.rb +++ b/example/app.rb @@ -1,9 +1,12 @@ +# frozen_string_literal: true + require 'bundler/setup' require 'securerandom' require 'grape' require 'json' module Example + # Class: App class App < Grape::API version 'v1', using: :header, vendor: 'acquia' diff --git a/example/config.ru b/example/config.ru index e6c0757..42d20fb 100755 --- a/example/config.ru +++ b/example/config.ru @@ -1,4 +1,5 @@ #!/usr/bin/env rackup -p8010 +# frozen_string_literal: true require 'bundler/setup' Bundler.require @@ -7,7 +8,7 @@ require_relative 'app' require 'acquia-http-hmac/rack_authenticate' unless ENV['NO_AUTHENTICATION'] - passwords = Acquia::HTTPHmac::FilePasswordStorage.new(File.dirname(__FILE__) + '/../fixtures/passwords.yml') + passwords = Acquia::HTTPHmac::FilePasswordStorage.new("#{File.dirname(__FILE__)}/../fixtures/passwords.yml") options = { password_storage: passwords, realm: 'Test', diff --git a/features/step_definitions/common.rb b/features/step_definitions/common.rb index 610c131..f2930ca 100644 --- a/features/step_definitions/common.rb +++ b/features/step_definitions/common.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + Given(/^the endpoint "(.*?)" "(.*?)"$/) do |method, path| @method = method @path = path @@ -21,7 +23,6 @@ When(/^I sign the request with the "(.*?)" digest and secret key "(.*?)"$/) do |digest, key| signer = Acquia::HTTPHmac.new(@method, @body, @headers, @cheaders, @path) - key = key case digest when 'SHA-1' digester = Digest::SHA1 diff --git a/features/support/env.rb b/features/support/env.rb index ec1d2b4..462ae14 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -1 +1,3 @@ +# frozen_string_literal: true + require 'acquia/httphmac' diff --git a/lib/acquia-http-hmac/rack_authenticate.rb b/lib/acquia-http-hmac/rack_authenticate.rb index 020d999..e75f0de 100644 --- a/lib/acquia-http-hmac/rack_authenticate.rb +++ b/lib/acquia-http-hmac/rack_authenticate.rb @@ -1,10 +1,11 @@ require 'yaml' require 'openssl' require 'base64' -require_relative '../acquia-http-hmac' +require_relative '../acquia_http_hmac' module Acquia module HTTPHmac + # Class: RackAuthenticate class RackAuthenticate def initialize(app, options) @password_storage = options[:password_storage] @@ -26,7 +27,7 @@ def call(env) args = args_for_authenticator(env, attributes) mac = message_authenticator(args[:id], args[:timestamp]) - return denied('Invalid credentials') unless mac && mac.request_authenticated?(args) + return denied('Invalid credentials') unless mac&.request_authenticated?(args) return denied('Invalid body') unless valid_body?(env) @@ -44,7 +45,7 @@ def unauthorized { 'Content-Type' => 'text/plain', 'Content-Length' => '0', - 'WWW-Authenticate' => 'acquia-http-hmac realm="' + @realm + '"' + 'WWW-Authenticate' => "acquia-http-hmac realm=\"#{@realm}\"" }, []] end @@ -76,8 +77,8 @@ def args_for_authenticator(env, attributes) timestamp: env['HTTP_X_AUTHORIZATION_TIMESTAMP'].to_i }.merge(attributes) # Map expected header names to the key that would be in env. - attributes[:headers].keys.each do |name| - key = 'HTTP_' + name.gsub('-', '_').upcase + attributes[:headers].each_key do |name| + key = "HTTP_#{name.gsub('-', '_').upcase}" args[:headers][name] = env[key] if env[key] end args @@ -112,7 +113,7 @@ def sign_response(status, headers, resp_body, nonce, timestamp, mac) # Rack defines the response body as implementing #each resp_body.each { |part| final_body << part } # Use the request nonce to sign the response. - headers['X-Server-Authorization-HMAC-SHA256'] = mac.signature(nonce + "\n" + timestamp.to_s + "\n" + final_body) + headers['X-Server-Authorization-HMAC-SHA256'] = mac.signature("#{nonce}\n#{timestamp}\n#{final_body}") # Nobody should be changing or caching this response. headers['Cache-Control'] = 'no-transform, no-cache, no-store, private, max-age=0' [status, headers, [final_body]] @@ -121,6 +122,7 @@ def sign_response(status, headers, resp_body, nonce, timestamp, mac) ### The classes below are primarily for testing. + # Class: SimplePasswordStorage class SimplePasswordStorage def initialize(creds = {}) @@creds = creds @@ -154,6 +156,7 @@ def ids end end + # Class: FilePasswordStorage class FilePasswordStorage < SimplePasswordStorage def initialize(filename) creds = {} @@ -162,12 +165,14 @@ def initialize(filename) end end + # Class: NoopNonceChecker class NoopNonceChecker def valid?(_id, nonce) nonce.length == 36 end end + # Class: MemoryNonceChecker class MemoryNonceChecker def initialize @@seen = {} diff --git a/lib/acquia-http-hmac/sqlite3_password_storage.rb b/lib/acquia-http-hmac/sqlite3_password_storage.rb index ac7eff6..219f6ce 100644 --- a/lib/acquia-http-hmac/sqlite3_password_storage.rb +++ b/lib/acquia-http-hmac/sqlite3_password_storage.rb @@ -1,9 +1,12 @@ +# frozen_string_literal: true + require 'openssl' require 'base64' require 'sqlite3' module Acquia module HTTPHmac + # Class: SQLite3PasswordStorage class SQLite3PasswordStorage def initialize(filename) @filename = filename diff --git a/lib/acquia-http-hmac.rb b/lib/acquia_http_hmac.rb similarity index 98% rename from lib/acquia-http-hmac.rb rename to lib/acquia_http_hmac.rb index aa8d130..b8a820a 100644 --- a/lib/acquia-http-hmac.rb +++ b/lib/acquia_http_hmac.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'addressable' require 'openssl' require 'base64' @@ -7,6 +9,7 @@ module Acquia module HTTPHmac VERSION = '2.0' + # Class: Auth class Auth def initialize(realm, base64_secret) @realm = realm @@ -45,7 +48,7 @@ def prepare_request_headers(args = {}) headers = {} headers['X-Authorization-Timestamp'] = args[:timestamp] - unless args[:body].nil? || (args[:body].length == 0) + unless args[:body].nil? || args[:body].empty? args[:body_hash] = Base64.strict_encode64(OpenSSL::Digest::SHA256.digest(args[:body])) headers['X-Authorization-Content-SHA256'] = args[:body_hash] end diff --git a/test/acquia_spec_test.rb b/test/acquia_spec_test.rb index f7308e7..451c847 100644 --- a/test/acquia_spec_test.rb +++ b/test/acquia_spec_test.rb @@ -1,5 +1,7 @@ +# frozen_string_literal: true + require 'minitest/autorun' -require_relative '../lib/acquia-http-hmac' +require_relative '../lib/acquia_http_hmac' class TestAcquiaHmacSpec < Minitest::Test def test_fixture diff --git a/test/helpers/rack_app_test_base.rb b/test/helpers/rack_app_test_base.rb index f329eff..06b2ac2 100644 --- a/test/helpers/rack_app_test_base.rb +++ b/test/helpers/rack_app_test_base.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'rack/test' require 'acquia-http-hmac/rack_authenticate' @@ -86,7 +88,7 @@ def test_403_bad_password_get passwords = get_password_storage id = passwords.ids.first # Use an invalid password by adding a letter. - prepare_get(id, 'a' + get_password(id)) + prepare_get(id, "a#{get_password(id)}") get '/hello' assert_equal(403, last_response.status) end @@ -95,7 +97,7 @@ def test_403_bad_id_get passwords = get_password_storage id = passwords.ids.first # Use an invalid id by adding a letter. - prepare_get(id + 'a', get_password(id)) + prepare_get("#{id}a", get_password(id)) get '/hello' assert_equal(403, last_response.status) end @@ -223,7 +225,7 @@ def test_403_bad_body_post body = '{"hello":"hi.bob","params":["5","4","8"]}' prepare_post(id, get_password(id), body) # Create a mismatch by adding an extra character to the body. - post '/hello', body + 'a' + post '/hello', "#{body}a" assert_equal(403, last_response.status) end end diff --git a/test/httphmac_test.rb b/test/httphmac_test.rb index 13bdaf1..0a409d4 100644 --- a/test/httphmac_test.rb +++ b/test/httphmac_test.rb @@ -1,5 +1,7 @@ +# frozen_string_literal: true + require 'minitest/autorun' -require_relative '../lib/acquia-http-hmac' +require_relative '../lib/acquia_http_hmac' class TestHTTPHmac < Minitest::Test def test_prepare_request_get diff --git a/test/httphmac_verify_test.rb b/test/httphmac_verify_test.rb index f3c259a..9071d84 100644 --- a/test/httphmac_verify_test.rb +++ b/test/httphmac_verify_test.rb @@ -1,9 +1,11 @@ +# frozen_string_literal: true + require 'minitest/autorun' require 'base64' -require_relative '../lib/acquia-http-hmac' +require_relative '../lib/acquia_http_hmac' class HmacVerifyTest < Minitest::Test - def get_params + def params { http_method: 'GET', host: 'example.com', @@ -37,28 +39,28 @@ def setup @realm = 'TestRealm' hmac = Acquia::HTTPHmac::Auth.new(@realm, @secret) - @req_get = hmac.prepare_request_headers(get_params) + @req_get = hmac.prepare_request_headers(params) @req_post = hmac.prepare_request_headers(post_params) end def test_get_no_body attributes = Acquia::HTTPHmac::Auth.parse_auth_header(@req_get['Authorization']) hmac = Acquia::HTTPHmac::Auth.new(@realm, @secret) - ret = hmac.request_authenticated?(get_params.merge(attributes)) + ret = hmac.request_authenticated?(params.merge(attributes)) assert(ret, 'request_authenticated? failed for GET') end def test_it_fails_with_invalid_realm attributes = Acquia::HTTPHmac::Auth.parse_auth_header(@req_get['Authorization']) hmac = Acquia::HTTPHmac::Auth.new('bad_realm', @secret) - ret = hmac.request_authenticated?(get_params.merge(attributes)) + ret = hmac.request_authenticated?(params.merge(attributes)) assert(!ret, 'request_authenticated? accepted invalid realm') end def test_it_fails_with_invalid_secret attributes = Acquia::HTTPHmac::Auth.parse_auth_header(@req_get['Authorization']) hmac = Acquia::HTTPHmac::Auth.new(@realm, Base64.strict_encode64('wrong password')) - ret = hmac.request_authenticated?(get_params.merge(attributes)) + ret = hmac.request_authenticated?(params.merge(attributes)) assert(!ret, 'request_authenticated? accepted invalid secret') end @@ -73,13 +75,13 @@ def test_post_with_body def test_it_requires_recent_timestamp # We need to do our own GET with a wrong timestamp here: - params = get_params + parameters = params # Put it 901 seconds in the past. - params[:timestamp] = params[:timestamp].to_i - 901 + parameters[:timestamp] = parameters[:timestamp].to_i - 901 hmac = Acquia::HTTPHmac::Auth.new(@realm, @secret) - get = hmac.prepare_request_headers(params) + get = hmac.prepare_request_headers(parameters) attributes = Acquia::HTTPHmac::Auth.parse_auth_header(get['Authorization']) - ret = hmac.request_authenticated?(params.merge(attributes)) + ret = hmac.request_authenticated?(parameters.merge(attributes)) assert(!ret, 'request_authenticated? accepted old timestamp') end end diff --git a/test/rack_simple_app_test.rb b/test/rack_simple_app_test.rb index 7297821..9b8fdb8 100644 --- a/test/rack_simple_app_test.rb +++ b/test/rack_simple_app_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'minitest/autorun' require_relative 'helpers/rack_app_test_base' @@ -5,7 +7,7 @@ class TestRackApp < Minitest::Test include TestRackAppBase def get_password_storage - @passwords ||= Acquia::HTTPHmac::FilePasswordStorage.new(File.dirname(__FILE__) + '/../fixtures/passwords.yml') + @get_password_storage ||= Acquia::HTTPHmac::FilePasswordStorage.new("#{File.dirname(__FILE__)}/../fixtures/passwords.yml") end def get_password(id, _timestamp = nil) diff --git a/test/rack_sqlite3_app_test.rb b/test/rack_sqlite3_app_test.rb index e6a7d50..fcf8993 100644 --- a/test/rack_sqlite3_app_test.rb +++ b/test/rack_sqlite3_app_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'minitest/autorun' require 'base64' require 'openssl' @@ -10,7 +12,7 @@ class TestSqlite3RackApp < Minitest::Test def setup @dbfile = File.join(File.dirname(__FILE__), '/../fixtures/passwords.sqlite3') - @passwords_file = File.dirname(__FILE__) + '/../fixtures/passwords.yml' + @passwords_file = "#{File.dirname(__FILE__)}/../fixtures/passwords.yml" s = ExampleSQLite3Setup.new(@dbfile, @passwords_file) s.write_database @binary_passwords = {} @@ -31,6 +33,6 @@ def get_password(id, _timestamp = nil) end def get_password_storage - @storage ||= Acquia::HTTPHmac::SQLite3PasswordStorage.new(@dbfile) + @get_password_storage ||= Acquia::HTTPHmac::SQLite3PasswordStorage.new(@dbfile) end end From 89a63faaebfe19332b7f4ea6142c45d9133f8dee Mon Sep 17 00:00:00 2001 From: Imnet Edossa Date: Fri, 20 Dec 2024 15:44:09 +0300 Subject: [PATCH 3/4] Pin gem versions --- acquia-http-hmac.gemspec | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/acquia-http-hmac.gemspec b/acquia-http-hmac.gemspec index 25fb051..dc911d6 100644 --- a/acquia-http-hmac.gemspec +++ b/acquia-http-hmac.gemspec @@ -16,14 +16,14 @@ Gem::Specification.new do |s| s.require_paths = ['lib'] s.executables << 'acq-http-request' - s.add_dependency 'addressable', '~> 2.4' + s.add_dependency 'addressable', '~> 2.8' - s.add_development_dependency('grape') - s.add_development_dependency('multi_json') + s.add_development_dependency('grape', '~> 2.2') + s.add_development_dependency('multi_json', '~> 1.15') s.add_development_dependency 'rack', '~> 2.2.10' - s.add_development_dependency('rack-test') - s.add_development_dependency('rake') + s.add_development_dependency('rack-test', '~> 2.1') + s.add_development_dependency('rake', '~> 13.2') s.add_development_dependency 'rubocop', '~> 1.69' - s.add_development_dependency('sqlite3') - s.add_development_dependency('webrick') + s.add_development_dependency('sqlite3', '~> 2.4') + s.add_development_dependency('webrick', '~> 1.9') end From 01b407eeec480aed613fc0774f078107fc6df7ac Mon Sep 17 00:00:00 2001 From: Imnet Edossa Date: Fri, 20 Dec 2024 15:46:03 +0300 Subject: [PATCH 4/4] Remove .travis.yml --- .travis.yml | 11 ----------- 1 file changed, 11 deletions(-) delete mode 100644 .travis.yml diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index 71d2c2b..0000000 --- a/.travis.yml +++ /dev/null @@ -1,11 +0,0 @@ -language: ruby -rvm: - - "2.6" - - "2.7" - - "3.0" -before_script: - - "bundle exec rackup -p8010 example/config.ru &" - - "sleep 3" -script: - - "bundle exec rake test" - - "example/test.sh"