[CHEF-761] extract header handling into its own class

7 files changed, 288 insertions, 47 deletions

diff --git a/lib/mixlib/authentication.rb b/lib/mixlib/authentication.rb
index 38b466e..ab10db7 100644
--- a/lib/mixlib/authentication.rb
+++ b/lib/mixlib/authentication.rb
@@ -20,6 +20,12 @@ require 'mixlib/log'
 
 module Mixlib
   module Authentication
+    class AuthenticationError < StandardError
+    end
+
+    class MissingAuthenticationHeader < AuthenticationError
+    end
+
     class Log
       extend  Mixlib::Log      
     end
diff --git a/lib/mixlib/authentication/http_authentication_request.rb b/lib/mixlib/authentication/http_authentication_request.rb
new file mode 100644
index 0000000..68a2611
--- /dev/null
+++ b/lib/mixlib/authentication/http_authentication_request.rb
@@ -0,0 +1,85 @@
+#
+# Author:: Daniel DeLeo (<dan@opscode.com>)
+# Copyright:: Copyright (c) 2010 Opscode, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+# 
+#     http://www.apache.org/licenses/LICENSE-2.0
+# 
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require 'mixlib/authentication'
+
+module Mixlib
+  module Authentication
+    class HTTPAuthenticationRequest
+
+      MANDATORY_HEADERS = [:x_ops_sign, :x_ops_userid, :x_ops_timestamp, :host, :x_ops_content_hash]
+
+      def initialize(request)
+        @request = request
+        @request_signature = nil
+        assert_required_headers_present
+      end
+
+      def headers
+        @headers ||= @request.env.inject({ }) { |memo, kv| memo[$2.gsub(/\-/,"_").downcase.to_sym] = kv[1] if kv[0] =~ /^(HTTP_)(.*)/; memo }
+      end
+
+      def http_method
+        @request.method.to_s
+      end
+
+      def path
+        @request.path.to_s
+      end
+
+      def signing_description
+        headers[:x_ops_sign].chomp
+      end
+
+      def user_id
+        headers[:x_ops_userid].chomp
+      end
+
+      def timestamp
+        headers[:x_ops_timestamp].chomp
+      end
+
+      def host
+        headers[:host].chomp
+      end
+
+      def content_hash
+        headers[:x_ops_content_hash].chomp
+      end
+
+      def request_signature
+        unless @request_signature
+          @request_signature = headers.find_all { |h| h[0].to_s =~ /^x_ops_authorization_/ }.sort { |x,y| x.to_s <=> y.to_s}.map { |i| i[1] }.join("\n")
+          Mixlib::Authentication::Log.debug "Reconstituted (user-supplied) request signature: #{@request_signature}"
+        end
+        @request_signature
+      end
+
+      private
+
+      def assert_required_headers_present
+        missing_headers = MANDATORY_HEADERS - headers.keys
+        unless missing_headers.empty?
+          missing_headers.map! { |h| h.to_s.upcase }
+          raise MissingAuthenticationHeader, "missing required authentication header(s) '#{missing_headers.join("', '")}'"
+        end
+      end
+
+    end
+  end
+end
diff --git a/lib/mixlib/authentication/signatureverification.rb b/lib/mixlib/authentication/signatureverification.rb
index 239a92e..ec09f12 100644
--- a/lib/mixlib/authentication/signatureverification.rb
+++ b/lib/mixlib/authentication/signatureverification.rb
@@ -17,9 +17,9 @@
 # limitations under the License.
 #
 
-require 'ostruct'
 require 'net/http'
 require 'mixlib/authentication'
+require 'mixlib/authentication/http_authentication_request'
 require 'mixlib/authentication/signedheaderauth'
 
 module Mixlib
@@ -27,26 +27,19 @@ module Mixlib
     class SignatureResponse < Struct.new(:name)
     end
 
-    class AuthenticationError < StandardError
-    end
-
-    class MissingAuthenticationHeader < AuthenticationError
-    end
-
     class SignatureVerification
       MANDATORY_HEADERS = [:x_ops_sign, :x_ops_userid, :x_ops_timestamp, :host, :x_ops_content_hash]
 
       include Mixlib::Authentication::SignedHeaderAuth
-      
-      attr_reader :timestamp
-      attr_reader :http_method
-      attr_reader :path
-      attr_reader :user_id
+
       attr_reader :request
 
+      attr_reader :auth_request
+
       def initialize
         @valid_signature, @valid_timestamp, @valid_content_hash = false, false, false
         @hashed_body = nil
+        @request, @auth_request = nil, nil
       end
 
       # Takes the request, boils down the pieces we are interested in,
@@ -62,16 +55,12 @@ module Mixlib
       def authenticate_user_request(request, user_lookup, time_skew=(15*60))
         Mixlib::Authentication::Log.debug "Initializing header auth : #{request.inspect}"
 
-        @request, @user_secret  = request, user_lookup
-        @allowed_time_skew      = time_skew # in seconds
-
-        digester = Mixlib::Authentication::Digester
+        @request           = request
+        @user_secret       = user_lookup
+        @allowed_time_skew = time_skew # in seconds
 
         begin
-
-          assert_required_headers_present
-          extract_auth_params_from_request
-          build_request_signature
+          @auth_request = HTTPAuthenticationRequest.new(request)
           
           #BUGBUG Not doing anything with the signing description yet [cb]          
           parse_signing_description
@@ -80,14 +69,12 @@ module Mixlib
           verify_timestamp
           verify_content_hash
 
-          #timeskew_is_acceptable = timestamp_within_bounds?(Time.parse(timestamp), Time.now)
-          #hashes_match = (@content_hash == hashed_body)
         rescue StandardError=>se
-          raise StandardError,"Failed to authenticate user request. Check your client key and clock: #{se.message}", se.backtrace
+          raise AuthenticationError,"Failed to authenticate user request. Check your client key and clock: #{se.message}", se.backtrace
         end
 
         if valid_request?
-          SignatureResponse.new(:name=>user_id)
+          SignatureResponse.new(user_id)
         else
           nil
         end
@@ -119,17 +106,6 @@ module Mixlib
 
       private
 
-      def extract_auth_params_from_request
-        @http_method         = request.method.to_s
-        @path                = request.path.to_s
-        @signing_description = headers[:x_ops_sign].chomp
-        @user_id             = headers[:x_ops_userid].chomp
-        @timestamp           = headers[:x_ops_timestamp].chomp
-        @host                = headers[:host].chomp
-        @content_hash        = headers[:x_ops_content_hash].chomp
-        Mixlib::Authentication::Log.debug "Authenticating user : #{user_id}, User secret is : \n#{@user_secret}"
-      end
-
       def assert_required_headers_present
         MANDATORY_HEADERS.each do |header|
           unless headers.key?(header)
@@ -138,15 +114,41 @@ module Mixlib
         end
       end
 
-      def build_request_signature
-        # if there are 11 headers, the sort breaks - it becomes lexicographic sort rather than numeric [cb]
-        @request_signature = headers.find_all { |h| h[0].to_s =~ /^x_ops_authorization_/ }.sort { |x,y| x.to_s <=> y.to_s}.map { |i| i[1] }.join("\n")
-        Mixlib::Authentication::Log.debug "Reconstituted (user-supplied) request signature: #{@request_signature}"
+      def http_method
+        auth_request.http_method
+      end
+
+      def path
+        auth_request.path
+      end
+
+      def signing_description
+        auth_request.signing_description
+      end
+
+      def user_id
+        auth_request.user_id
+      end
+
+      def timestamp
+        auth_request.timestamp
+      end
+
+      def host
+        auth_request.host
+      end
+
+      def request_signature
+        auth_request.request_signature
+      end
+
+      def content_hash
+        auth_request.content_hash
       end
 
       def verify_signature
         candidate_block = canonicalize_request
-        request_decrypted_block = @user_secret.public_decrypt(Base64.decode64(@request_signature))
+        request_decrypted_block = @user_secret.public_decrypt(Base64.decode64(request_signature))
         @valid_signature = (request_decrypted_block == candidate_block)
 
         # Keep the debug messages lined up so it's easy to scan them
@@ -166,11 +168,11 @@ module Mixlib
       end
 
       def verify_content_hash
-        @valid_content_hash = (@content_hash == hashed_body)
+        @valid_content_hash = (content_hash == hashed_body)
 
         # Keep the debug messages lined up so it's easy to scan them
         Mixlib::Authentication::Log.debug("Expected content hash is: '#{hashed_body}'")
-        Mixlib::Authentication::Log.debug(" Request Content Hash is: '#{@content_hash}'")
+        Mixlib::Authentication::Log.debug(" Request Content Hash is: '#{content_hash}'")
         Mixlib::Authentication::Log.debug("           Hashes match?: #{@valid_content_hash}")
 
         @valid_content_hash
diff --git a/lib/mixlib/authentication/signedheaderauth.rb b/lib/mixlib/authentication/signedheaderauth.rb
index 63fd352..7cc3234 100644
--- a/lib/mixlib/authentication/signedheaderauth.rb
+++ b/lib/mixlib/authentication/signedheaderauth.rb
@@ -19,7 +19,6 @@
 
 require 'time'
 require 'base64'
-require 'ostruct'
 require 'digest/sha1'
 require 'mixlib/authentication'
 require 'mixlib/authentication/digester'
@@ -107,7 +106,7 @@ module Mixlib
       # ====Parameters
       #
       def parse_signing_description
-        parts = @signing_description.strip.split(";").inject({ }) do |memo, part|
+        parts = signing_description.strip.split(";").inject({ }) do |memo, part|
           field_name, field_value = part.split("=")
           memo[field_name.to_sym] = field_value.strip
           memo
diff --git a/spec/mixlib/authentication/http_authentication_request_spec.rb b/spec/mixlib/authentication/http_authentication_request_spec.rb
new file mode 100644
index 0000000..bd5fe9c
--- /dev/null
+++ b/spec/mixlib/authentication/http_authentication_request_spec.rb
@@ -0,0 +1,128 @@
+# Author:: Daniel DeLeo (<dan@opscode.com>)
+# Copyright:: Copyright (c) 2010 Opscode, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+# 
+#     http://www.apache.org/licenses/LICENSE-2.0
+# 
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require File.expand_path(File.join(File.dirname(__FILE__), '..','..','spec_helper'))
+
+require 'mixlib/authentication'
+require 'mixlib/authentication/http_authentication_request'
+require 'ostruct'
+require 'pp'
+
+describe Mixlib::Authentication::HTTPAuthenticationRequest do
+  before do
+    request = Struct.new(:env, :method, :path)
+
+    @timestamp_iso8601 = "2009-01-01T12:00:00Z"
+    @x_ops_content_hash = "DFteJZPVv6WKdQmMqZUQUumUyRs="
+    @user_id = "spec-user"
+    @http_x_ops_lines = [
+      "jVHrNniWzpbez/eGWjFnO6lINRIuKOg40ZTIQudcFe47Z9e/HvrszfVXlKG4",
+      "NMzYZgyooSvU85qkIUmKuCqgG2AIlvYa2Q/2ctrMhoaHhLOCWWoqYNMaEqPc",
+      "3tKHE+CfvP+WuPdWk4jv4wpIkAz6ZLxToxcGhXmZbXpk56YTmqgBW2cbbw4O",
+      "IWPZDHSiPcw//AYNgW1CCDptt+UFuaFYbtqZegcBd2n/jzcWODA7zL4KWEUy",
+      "9q4rlh/+1tBReg60QdsmDRsw/cdO1GZrKtuCwbuD4+nbRdVBKv72rqHX9cu0",
+      "utju9jzczCyB+sSAQWrxSsXB/b8vV2qs0l4VD2ML+w=="]
+    @merb_headers = {
+      # These are used by signatureverification. An arbitrary sampling of non-HTTP_*
+      # headers are in here to exercise that code path.
+      "HTTP_HOST"=>"127.0.0.1", 
+      "HTTP_X_OPS_SIGN"=>"version=1.0",
+      "HTTP_X_OPS_REQUESTID"=>"127.0.0.1 1258566194.85386", 
+      "HTTP_X_OPS_TIMESTAMP"=>@timestamp_iso8601, 
+      "HTTP_X_OPS_CONTENT_HASH"=>@x_ops_content_hash, 
+      "HTTP_X_OPS_USERID"=>@user_id, 
+      "HTTP_X_OPS_AUTHORIZATION_1"=>@http_x_ops_lines[0], 
+      "HTTP_X_OPS_AUTHORIZATION_2"=>@http_x_ops_lines[1], 
+      "HTTP_X_OPS_AUTHORIZATION_3"=>@http_x_ops_lines[2], 
+      "HTTP_X_OPS_AUTHORIZATION_4"=>@http_x_ops_lines[3], 
+      "HTTP_X_OPS_AUTHORIZATION_5"=>@http_x_ops_lines[4], 
+      "HTTP_X_OPS_AUTHORIZATION_6"=>@http_x_ops_lines[5], 
+
+      # Random sampling
+      "REMOTE_ADDR"=>"127.0.0.1", 
+      "PATH_INFO"=>"/organizations/local-test-org/cookbooks", 
+      "REQUEST_PATH"=>"/organizations/local-test-org/cookbooks", 
+      "CONTENT_TYPE"=>"multipart/form-data; boundary=----RubyMultipartClient6792ZZZZZ",
+      "CONTENT_LENGTH"=>"394", 
+    }
+    @request = request.new(@merb_headers, "POST", '/nodes')
+    @http_authentication_request = Mixlib::Authentication::HTTPAuthenticationRequest.new(@request)
+  end
+
+  it "normalizes the headers to lowercase symbols" do
+    expected = {:host=>"127.0.0.1",
+                :x_ops_sign=>"version=1.0",
+                :x_ops_requestid=>"127.0.0.1 1258566194.85386",
+                :x_ops_timestamp=>"2009-01-01T12:00:00Z",
+                :x_ops_content_hash=>"DFteJZPVv6WKdQmMqZUQUumUyRs=",
+                :x_ops_userid=>"spec-user",
+                :x_ops_authorization_1=>"jVHrNniWzpbez/eGWjFnO6lINRIuKOg40ZTIQudcFe47Z9e/HvrszfVXlKG4",
+                :x_ops_authorization_2=>"NMzYZgyooSvU85qkIUmKuCqgG2AIlvYa2Q/2ctrMhoaHhLOCWWoqYNMaEqPc",
+                :x_ops_authorization_3=>"3tKHE+CfvP+WuPdWk4jv4wpIkAz6ZLxToxcGhXmZbXpk56YTmqgBW2cbbw4O",
+                :x_ops_authorization_4=>"IWPZDHSiPcw//AYNgW1CCDptt+UFuaFYbtqZegcBd2n/jzcWODA7zL4KWEUy",
+                :x_ops_authorization_5=>"9q4rlh/+1tBReg60QdsmDRsw/cdO1GZrKtuCwbuD4+nbRdVBKv72rqHX9cu0",
+                :x_ops_authorization_6=>"utju9jzczCyB+sSAQWrxSsXB/b8vV2qs0l4VD2ML+w=="}
+    @http_authentication_request.headers.should == expected
+  end
+
+  it "raises an error when not all required headers are given" do
+    @merb_headers.delete("HTTP_X_OPS_SIGN")
+    exception = Mixlib::Authentication::MissingAuthenticationHeader
+    lambda {Mixlib::Authentication::HTTPAuthenticationRequest.new(@request)}.should raise_error(exception)
+  end
+
+  it "extracts the path from the request" do
+    @http_authentication_request.path.should == '/nodes'
+  end
+
+  it "extracts the request method from the request" do
+    @http_authentication_request.http_method.should == 'POST'
+  end
+
+  it "extracts the signing description from the request headers" do
+    @http_authentication_request.signing_description.should == 'version=1.0'
+  end
+
+  it "extracts the user_id from the request headers" do
+    @http_authentication_request.user_id.should == 'spec-user'
+  end
+
+  it "extracts the timestamp from the request headers" do
+    @http_authentication_request.timestamp.should == "2009-01-01T12:00:00Z"
+  end
+
+  it "extracts the host from the request headers" do
+    @http_authentication_request.host.should == "127.0.0.1"
+  end
+
+  it "extracts the content hash from the request headers" do
+    @http_authentication_request.content_hash.should == "DFteJZPVv6WKdQmMqZUQUumUyRs="
+  end
+
+  it "rebuilds the request signature from the headers" do
+    expected=<<-SIG
+jVHrNniWzpbez/eGWjFnO6lINRIuKOg40ZTIQudcFe47Z9e/HvrszfVXlKG4
+NMzYZgyooSvU85qkIUmKuCqgG2AIlvYa2Q/2ctrMhoaHhLOCWWoqYNMaEqPc
+3tKHE+CfvP+WuPdWk4jv4wpIkAz6ZLxToxcGhXmZbXpk56YTmqgBW2cbbw4O
+IWPZDHSiPcw//AYNgW1CCDptt+UFuaFYbtqZegcBd2n/jzcWODA7zL4KWEUy
+9q4rlh/+1tBReg60QdsmDRsw/cdO1GZrKtuCwbuD4+nbRdVBKv72rqHX9cu0
+utju9jzczCyB+sSAQWrxSsXB/b8vV2qs0l4VD2ML+w==
+SIG
+    @http_authentication_request.request_signature.should == expected.chomp
+  end
+
+end
\ No newline at end of file
diff --git a/spec/mixlib/authentication/mixlib_authentication_spec.rb b/spec/mixlib/authentication/mixlib_authentication_spec.rb
index 4b16e93..4a64580 100644
--- a/spec/mixlib/authentication/mixlib_authentication_spec.rb
+++ b/spec/mixlib/authentication/mixlib_authentication_spec.rb
@@ -17,9 +17,7 @@
 # limitations under the License.
 #
 
-$:.unshift File.expand_path(File.join(File.dirname(__FILE__), "..", "..", "..", "lib")) # lib in mixlib-authentication
-$:.unshift File.expand_path(File.join(File.dirname(__FILE__), "..", "..", "..", "..", "mixlib-log", "lib")) # mixlib-log/log
-
+require File.expand_path(File.join(File.dirname(__FILE__), '..','..','spec_helper'))
 require 'rubygems'
 
 require 'ostruct'
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
new file mode 100644
index 0000000..3185a12
--- /dev/null
+++ b/spec/spec_helper.rb
@@ -0,0 +1,23 @@
+#
+# Author:: Tim Hinderliter (<tim@opscode.com>)
+# Author:: Christopher Walters (<cw@opscode.com>)
+# Copyright:: Copyright (c) 2009, 2010 Opscode, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+# 
+#     http://www.apache.org/licenses/LICENSE-2.0
+# 
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+$:.unshift File.expand_path(File.join(File.dirname(__FILE__), "..", "lib")) # lib in mixlib-authentication
+$:.unshift File.expand_path(File.join(File.dirname(__FILE__), "..", "..", "mixlib-log", "lib")) # mixlib-log/log
+
+require 'rubygems'