Modified to handle both Merb and Passenger, as they pass the File part of a

multipart form upload differently. Added unit tests for this and other
behaviors.

3 files changed, 320 insertions, 19 deletions

diff --git a/lib/mixlib/authentication/signatureverification.rb b/lib/mixlib/authentication/signatureverification.rb
index c010515..4383007 100644
--- a/lib/mixlib/authentication/signatureverification.rb
+++ b/lib/mixlib/authentication/signatureverification.rb
@@ -38,6 +38,7 @@ module Mixlib
       # X-Ops-UserId: <user_id>
       # X-Ops-Timestamp:
       # X-Ops-Content-Hash: 
+      # X-Ops-Authorization-#{line_number}
       def authenticate_user_request(request, user_lookup, time_skew=(15*60))
         Mixlib::Authentication::Log.debug "Initializing header auth : #{request.inspect}"
         
@@ -63,27 +64,38 @@ module Mixlib
           @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 request signature: #{@request_signature}"
           
-          # Any file that's included in the request is hashed if it's there. Otherwise,
-          # we hash the body. Look for files by looking for objects that respond to
-          # the read call.
+          # Pull out any file that was attached to this request, using multipart
+          # form uploads.
+          # Depending on the server we're running in, multipart form uploads are
+          # handed to us differently. 
+          # - In Passenger (Cookbooks Community Site), the File is handed to us 
+          #   directly in the params hash. The name is whatever the client used, 
+          #   its value is therefore a File or Tempfile.
+          # - In Merb (Chef server), the File is wrapped. The original parameter 
+          #   name used for the file is passed in with a Hash value. Within the hash
+          #   is a name/value pair named 'file' which actually contains the Tempfile
+          #   instance.
           file_param = request.params.values.find { |value| value.respond_to?(:read) }
 
-          @hashed_body = if file_param
-                           Mixlib::Authentication::Log.debug "Digesting file_param: '#{file_param.inspect}'"
-                           if file_param.respond_to?(:has_key?)
-                             tempfile = file_param[:tempfile]
-                             digester.hash_file(tempfile)
-                           elsif file_param.respond_to?(:read)
-                             digester.hash_file(file_param)
-                           else
-                             digester.hash_body(file_param)
-                           end
-                         else
-                           body = request.raw_post
-                           Mixlib::Authentication::Log.debug "Digesting body: '#{body}'"
-                           digester.hash_body(body)
-                         end
+          # No file_param; we're running in Merb, or it's just not there..
+          if file_param.nil?
+            hash_param = request.params.values.find { |value| value.respond_to?(:has_key?) }  # Hash responds to :has_key? .
+            if !hash_param.nil?
+              file_param = hash_param.values.find { |value| value.respond_to?(:read) } # File/Tempfile responds to :read.
+            end
+          end
 
+          # Any file that's included in the request is hashed if it's there. Otherwise,
+          # we hash the body.
+          if file_param
+            Mixlib::Authentication::Log.debug "Digesting file_param: '#{file_param.inspect}'"
+            @hashed_body = digester.hash_file(file_param)
+          else
+            body = request.raw_post
+            Mixlib::Authentication::Log.debug "Digesting body: '#{body}'"
+            @hashed_body = digester.hash_body(body)
+          end
+          
           Mixlib::Authentication::Log.debug "Authenticating user : #{user_id}, User secret is : #{@user_secret}, Request signature is :\n#{@request_signature}, Auth HTTP header is :\n#{headers[:authorization]}, Hashed Body is : #{@hashed_body}"
           
           #BUGBUG Not doing anything with the signing description yet [cb]          
@@ -94,7 +106,7 @@ module Mixlib
           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.  Most likely missing a necessary header: #{se.message}"
+          raise StandardError,"Failed to authenticate user request.  Most likely missing a necessary header: #{se.message}", se.backtrace
         end
         
         Mixlib::Authentication::Log.debug "Candidate Block is: '#{candidate_block}'\nRequest decrypted block is: '#{request_decrypted_block}'\nCandidate content hash is: #{hashed_body}\nRequest Content Hash is: '#{@content_hash}'\nSignatures match: #{signatures_match}, Allowed Time Skew: #{timeskew_is_acceptable}, Hashes match?: #{hashes_match}\n"
diff --git a/spec/mixlib/authentication/mixlib_authentication_spec.rb b/spec/mixlib/authentication/mixlib_authentication_spec.rb
new file mode 100644
index 0000000..a21d7dd
--- /dev/null
+++ b/spec/mixlib/authentication/mixlib_authentication_spec.rb
@@ -0,0 +1,285 @@
+#
+# Author:: Tim Hinderliter (<tim@opscode.com>)
+# Copyright:: Copyright (c) 2009 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.
+#
+
+$:.push File.expand_path(File.join(File.dirname(__FILE__), "..", "..", "..", "lib")) # lib in mixlib-authentication
+$:.push File.expand_path(File.join(File.dirname(__FILE__), "..", "..", "..", "..", "mixlib-log", "lib")) # mixlib-log/log
+
+require 'rubygems'
+
+require 'ostruct'
+require 'openssl'
+require 'mixlib/authentication/signatureverification'
+require 'time'
+
+# TODO: should make these regular spec-based mock objects.
+class MockRequest
+  attr_accessor :env, :params, :raw_post
+
+  def initialize(headers, params, raw_post)
+    @env = headers
+    @params = params
+    @raw_post = raw_post
+  end
+
+  def method
+    "POST"
+  end
+end
+
+class MockFile
+  def initialize
+    @have_read = nil
+  end
+
+  def self.length
+    BODY.length
+  end
+
+  def read(len, out_str)
+    if @have_read.nil?
+      @have_read = 1
+      out_str[0..-1] = BODY
+      BODY
+    else
+      nil
+    end
+  end
+end
+
+# Uncomment this to get some more info from the methods we're testing.
+#Mixlib::Authentication::Log.level :debug
+
+describe "Mixlib::Authentication::SignedHeaderAuth" do
+  it "should sign headers" do
+    # fix the timestamp, private key and body so we get the same answer back
+    # every time.
+    args = {
+      :body => BODY, 
+      :user_id => USER_ID,
+      :http_method => :post,
+      :timestamp => TIMESTAMP_ISO8601,    # fixed timestamp so we get back the same answer each time.
+      :file => MockFile.new,
+    }
+
+    private_key = OpenSSL::PKey::RSA.new(PRIVATE_KEY)
+      
+    signing_obj = Mixlib::Authentication::SignedHeaderAuth.signing_object(args)
+
+    # If you need to regenerate the constants in this test spec, print out
+    # the results of res.inspect and copy them as appropriate into the 
+    # the constants in this file.
+    res = signing_obj.sign(private_key)
+    res.should == EXPECTED_SIGN_RESULT
+  end
+end
+
+describe "Mixlib::Authentication::SignatureVerification" do
+  
+  before(:each) do
+    @user_private_key = OpenSSL::PKey::RSA.new(PRIVATE_KEY)
+  end
+
+  it "should authenticate a File-containing request - Merb" do
+    request_params = MERB_REQUEST_PARAMS.clone
+    request_params["file"] =
+      { "size"=>MockFile.length, "content_type"=>"application/octet-stream", "filename"=>"zsh.tar.gz", "tempfile"=>MockFile.new }
+
+    mock_request = MockRequest.new(MERB_HEADERS, request_params, "")
+    Time.should_receive(:now).at_least(:once).and_return(TIMESTAMP_OBJ)
+
+    service = Mixlib::Authentication::SignatureVerification.new
+    res = service.authenticate_user_request(mock_request, @user_private_key)
+    res.should_not be_nil
+  end
+
+  it "should authenticate a normal (post body) request - Merb" do
+    mock_request = MockRequest.new(MERB_HEADERS, MERB_REQUEST_PARAMS, BODY)
+    Time.should_receive(:now).at_least(:once).and_return(TIMESTAMP_OBJ)
+
+    service = Mixlib::Authentication::SignatureVerification.new
+    res = service.authenticate_user_request(mock_request, @user_private_key)
+    res.should_not be_nil
+  end
+
+  it "should authenticate a File-containing request - Passenger" do
+    request_params = PASSENGER_REQUEST_PARAMS.clone
+    request_params["tarball"] = MockFile.new
+
+    mock_request = MockRequest.new(PASSENGER_HEADERS, request_params, "")
+    Time.should_receive(:now).at_least(:once).and_return(TIMESTAMP_OBJ)
+
+    service = Mixlib::Authentication::SignatureVerification.new
+    res = service.authenticate_user_request(mock_request, @user_private_key)
+    res.should_not be_nil
+  end
+
+  it "shouldn't authenticate if Authorization header is wrong" do
+    headers = MERB_HEADERS.clone
+    headers["HTTP_X_OPS_CONTENT_HASH"] += "_"
+
+    mock_request = MockRequest.new(headers, MERB_REQUEST_PARAMS, BODY)
+    Time.should_receive(:now).at_least(:once).and_return(TIMESTAMP_OBJ)
+
+    service = Mixlib::Authentication::SignatureVerification.new
+    res = service.authenticate_user_request(mock_request, @user_private_key)
+    res.should be_nil
+  end
+
+end
+
+USER_ID = "spec-user"
+BODY = "Spec Body"
+TIMESTAMP_ISO8601 = "2009-01-01T12:00:00Z"
+TIMESTAMP_OBJ = Time.parse("Thu Jan 01 12:00:00 -0000 2009")
+
+REQUESTING_ACTOR_ID = "c0f8a68c52bffa1020222a56b23cccfa"
+
+# Content hash is ???TODO
+X_OPS_CONTENT_HASH = "DFteJZPVv6WKdQmMqZUQUumUyRs="
+X_OPS_AUTHORIZATION_LINES = [
+  "GtDNsSpYDNOqN10Y+Xc28EMwAw5os931TKbJ6XmhaGi5xB12gN4Gw2atjeQr", 
+  "cZW3z3YGds+MlFZmQIoxgmQmYcUw176HQkMkP1e2Nd0674/ZiPprAAi1ak2s", 
+  "CHBlYQWkBHRg6s1rLNhI1nPwjF6m8g9fZ6b0gdwRkSKvS2m5l0es8p7zI7GT", 
+  "++etikvDfp7AXDtWkO8CeQObd4EKvZKBp/bOPSyl0u6/sFE753b6SvH1qOt+", 
+  "nMGavVyDHLHMvjnbBPif5o+VufYYcdKz9IgUDEs7Ul81NAWAwjOdpVxJnY2A", 
+  "0cgt6ij6eEk/I3CvXlo1rsW1+pZ7AJLPgISY2JeVOw==", 
+]
+
+# We expect Mixlib::Authentication::SignedHeaderAuth#sign to return this
+# if passed the BODY above.
+EXPECTED_SIGN_RESULT = {
+  "X-Ops-Content-Hash"=>X_OPS_CONTENT_HASH,
+  "X-Ops-Userid"=>USER_ID,
+  "X-Ops-Sign"=>"version=1.0",
+  "X-Ops-Authorization-1"=>X_OPS_AUTHORIZATION_LINES[0],
+  "X-Ops-Authorization-2"=>X_OPS_AUTHORIZATION_LINES[1],
+  "X-Ops-Authorization-3"=>X_OPS_AUTHORIZATION_LINES[2],
+  "X-Ops-Authorization-4"=>X_OPS_AUTHORIZATION_LINES[3],
+  "X-Ops-Authorization-5"=>X_OPS_AUTHORIZATION_LINES[4],
+  "X-Ops-Authorization-6"=>X_OPS_AUTHORIZATION_LINES[5],
+  "X-Ops-Timestamp"=>TIMESTAMP_ISO8601
+}
+
+# This is what will be in request.params for the Merb case.
+MERB_REQUEST_PARAMS = {
+  "name"=>"zsh", "action"=>"create", "controller"=>"chef_server_api/cookbooks", 
+  "organization_id"=>"local-test-org", "requesting_actor_id"=>REQUESTING_ACTOR_ID,
+}
+
+# Tis is what will be in request.env for the Merb case.
+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"=>X_OPS_AUTHORIZATION_LINES[0], 
+  "HTTP_X_OPS_AUTHORIZATION_2"=>X_OPS_AUTHORIZATION_LINES[1], 
+  "HTTP_X_OPS_AUTHORIZATION_3"=>X_OPS_AUTHORIZATION_LINES[2], 
+  "HTTP_X_OPS_AUTHORIZATION_4"=>X_OPS_AUTHORIZATION_LINES[3], 
+  "HTTP_X_OPS_AUTHORIZATION_5"=>X_OPS_AUTHORIZATION_LINES[4], 
+  "HTTP_X_OPS_AUTHORIZATION_6"=>X_OPS_AUTHORIZATION_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", 
+}
+
+PASSENGER_REQUEST_PARAMS = {
+  "action"=>"create",
+  #"tarball"=>#<File:/tmp/RackMultipart20091120-25570-mgq2sa-0>,
+  "controller"=>"api/v1/cookbooks",
+  "cookbook"=>"{\"category\":\"databases\"}",
+}
+
+PASSENGER_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"=>X_OPS_AUTHORIZATION_LINES[0], 
+  "HTTP_X_OPS_AUTHORIZATION_2"=>X_OPS_AUTHORIZATION_LINES[1], 
+  "HTTP_X_OPS_AUTHORIZATION_3"=>X_OPS_AUTHORIZATION_LINES[2], 
+  "HTTP_X_OPS_AUTHORIZATION_4"=>X_OPS_AUTHORIZATION_LINES[3], 
+  "HTTP_X_OPS_AUTHORIZATION_5"=>X_OPS_AUTHORIZATION_LINES[4], 
+  "HTTP_X_OPS_AUTHORIZATION_6"=>X_OPS_AUTHORIZATION_LINES[5], 
+
+  # Random set of other headers to exercirse the non- HTTP_ code path
+  "HTTP_ACCEPT"=>"application/json",
+  "SERVER_SOFTWARE"=>"Apache",
+  "SCRIPT_URI"=>"http://com-stg.opscode.com/api/v1/cookbooks",
+  "SCRIPT_NAME"=>"",
+  "SERVER_ADDR"=>"10.242.197.174",
+  "SERVER_NAME"=>"com-stg.opscode.com",
+  "DOCUMENT_ROOT"=>"/srv/opscode-community/current/public",
+}
+
+# generated with
+#   openssl genrsa -out private.pem 2048
+#   openssl rsa -in private.pem -out public.pem -pubout
+PUBLIC_KEY = <<EOS
+-----BEGIN PUBLIC KEY-----
+MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA0ueqo76MXuP6XqZBILFz
+iH/9AI7C6PaN5W0dSvkr9yInyGHSz/IR1+4tqvP2qlfKVKI4CP6BFH251Ft9qMUB
+uAsnlAVQ1z0exDtIFFOyQCdR7iXmjBIWMSS4buBwRQXwDK7id1OxtU23qVJv+xwE
+V0IzaaSJmaGLIbvRBD+qatfUuQJBMU/04DdJIwvLtZBYdC2219m5dUBQaa4bimL+
+YN9EcsDzD9h9UxQo5ReK7b3cNMzJBKJWLzFBcJuePMzAnLFktr/RufX4wpXe6XJx
+oVPaHo72GorLkwnQ0HYMTY8rehT4mDi1FI969LHCFFaFHSAaRnwdXaQkJmSfcxzC
+YQIDAQAB
+-----END PUBLIC KEY-----
+EOS
+
+PRIVATE_KEY = <<EOS
+-----BEGIN RSA PRIVATE KEY-----
+MIIEpAIBAAKCAQEA0ueqo76MXuP6XqZBILFziH/9AI7C6PaN5W0dSvkr9yInyGHS
+z/IR1+4tqvP2qlfKVKI4CP6BFH251Ft9qMUBuAsnlAVQ1z0exDtIFFOyQCdR7iXm
+jBIWMSS4buBwRQXwDK7id1OxtU23qVJv+xwEV0IzaaSJmaGLIbvRBD+qatfUuQJB
+MU/04DdJIwvLtZBYdC2219m5dUBQaa4bimL+YN9EcsDzD9h9UxQo5ReK7b3cNMzJ
+BKJWLzFBcJuePMzAnLFktr/RufX4wpXe6XJxoVPaHo72GorLkwnQ0HYMTY8rehT4
+mDi1FI969LHCFFaFHSAaRnwdXaQkJmSfcxzCYQIDAQABAoIBAQCW3I4sKN5B9jOe
+xq/pkeWBq4OvhW8Ys1yW0zFT8t6nHbB1XrwscQygd8gE9BPqj3e0iIEqtdphbPmj
+VHqTYbC0FI6QDClifV7noTwTBjeIOlgZ0NSUN0/WgVzIOxUz2mZ2vBZUovKILPqG
+TOi7J7RXMoySMdcXpP1f+PgvYNcnKsT72UcWaSXEV8/zo+Zm/qdGPVWwJonri5Mp
+DVm5EQSENBiRyt028rU6ElXORNmoQpVjDVqZ1gipzXkifdjGyENw2rt4V/iKYD7V
+5iqXOsvP6Cemf4gbrjunAgDG08S00kiUgvVWcdXW+dlsR2nCvH4DOEe3AYYh/aH8
+DxEE7FbtAoGBAPcNO8fJ56mNw0ow4Qg38C+Zss/afhBOCfX4O/SZKv/roRn5+gRM
+KRJYSVXNnsjPI1plzqR4OCyOrjAhtuvL4a0DinDzf1+fiztyNohwYsW1vYmqn3ti
+EN0GhSgE7ppZjqvLQ3f3LUTxynhA0U+k9wflb4irIlViTUlCsOPkrNJDAoGBANqL
+Q+vvuGSsmRLU/Cenjy+Mjj6+QENg51dz34o8JKuVKIPKU8pNnyeLa5fat0qD2MHm
+OB9opeQOcw0dStodxr6DB3wi83bpjeU6BWUGITNiWEaZEBrQ0aiqNJJKrrHm8fAZ
+9o4l4oHc4hI0kYVYYDuxtKuVJrzZiEapTwoOcYiLAoGBAI/EWbeIHZIj9zOjgjEA
+LHvm25HtulLOtyk2jd1njQhlHNk7CW2azIPqcLLH99EwCYi/miNH+pijZ2aHGCXb
+/bZrSxM0ADmrZKDxdB6uGCyp+GS2sBxjEyEsfCyvwhJ8b3Q100tqwiNO+d5FCglp
+HICx2dgUjuRVUliBwOK93nx1AoGAUI8RhIEjOYkeDAESyhNMBr0LGjnLOosX+/as
+qiotYkpjWuFULbibOFp+WMW41vDvD9qrSXir3fstkeIAW5KqVkO6mJnRoT3Knnra
+zjiKOITCAZQeiaP8BO5o3pxE9TMqb9VCO3ffnPstIoTaN4syPg7tiGo8k1SklVeH
+2S8lzq0CgYAKG2fljIYWQvGH628rp4ZcXS4hWmYohOxsnl1YrszbJ+hzR+IQOhGl
+YlkUQYXhy9JixmUUKtH+NXkKX7Lyc8XYw5ETr7JBT3ifs+G7HruDjVG78EJVojbd
+8uLA+DdQm5mg4vd1GTiSK65q/3EeoBlUaVor3HhLFki+i9qpT8CBsg==
+-----END RSA PRIVATE KEY-----
+EOS
diff --git a/spec/spec.opts b/spec/spec.opts
new file mode 100644
index 0000000..7636dbe
--- /dev/null
+++ b/spec/spec.opts
@@ -0,0 +1,4 @@
+--colour
+--format specdoc
+--loadby mtime
+--reverse