Merge pull request #93 from Shopify/adding_signature_valid_tests

moved validate_params into request_token and updated readme and tests

CHANGELOG

@@ -1,6 +1,10 @@
+== Version 3.1.9
+
+* in Session::request_token params is no longer optional, you must pass all the params and the method will now extract the code
+
 == Version 3.1.8
 
-* Expose `index` and `show` actions of `Location` 
+* Expose `index` and `show` actions of `Location`
 * Added create_permission_url and request_token helper methods
 * Edited the readme to better describe the getting started procedure
 
@@ -114,7 +118,7 @@ requests/responses
 
 == Version 1.1.1 (October 5, 2010)
 
-* Remove hard coded xml formatting in API calls 
+* Remove hard coded xml formatting in API calls
 * Remove jeweler stuff
 * Ruby 1.9 encoding fix
 

README.rdoc

@@ -13,7 +13,7 @@ The API is implemented as JSON over HTTP using all four verbs (GET/POST/PUT/DELE
 All API usage happens through Shopify applications, created by either shop owners for their own shops, or by Shopify Partners for use by other shop owners:
 
 * Shop owners can create applications for themselves through their own admin: http://docs.shopify.com/api/tutorials/creating-a-private-app
-* Shopify Partners create applications through their admin: http://app.shopify.com/services/partners 
+* Shopify Partners create applications through their admin: http://app.shopify.com/services/partners
 
 For more information and detailed documentation about the API visit http://api.shopify.com
 
@@ -32,7 +32,7 @@ ShopifyAPI uses ActiveResource to communicate with the REST web service. ActiveR
 1. First create a new application in either the partners admin or your store admin. For a private App you'll need the API_KEY and the PASSWORD otherwise you'll need the API_KEY and SHARED_SECRET.
 
 2. For a private App you just need to set the base site url as follows:
-    
+
     shop_url = "https://#{API_KEY}:#{PASSWORD}@SHOP_NAME.myshopify.com/admin"
     ShopifyAPI::Base.site = shop_url
 
@@ -53,7 +53,7 @@ ShopifyAPI uses ActiveResource to communicate with the REST web service. ActiveR
    * redirect_uri – Optional – The URL that the merchant will be sent to once authentication is complete. Defaults to the URL specified in the application settings and must be the same host as that URL.
 
    We've added the create_permision_url method to make this easier, first instantiate your session object:
-    
+
     session = ShopifyAPI::Session.new("SHOP_NAME.myshopify.com")
 
    Then call:
@@ -70,18 +70,20 @@ ShopifyAPI uses ActiveResource to communicate with the REST web service. ActiveR
     POST https://SHOP_NAME.myshopify.com/admin/oauth/access_token
 
    with the following parameters:
-   
+
    * client_id – Required – The API key for your app
    * client_secret – Required – The shared secret for your app
    * code – Required – The token you received in step 3
 
    and you'll get your permanent access token back in the response.
 
-   There is a method to make the request and get the token for you:
+   There is a method to make the request and get the token for you. Pass
+   all the params received from the previous call and the method will verify
+   the params, extract the temp code and then request your token:
 
-    token = session.request_token(code)
+    token = session.request_token(params)
 
-   Which will request the token, save it to the session object and return it. For future sessions simply pass the token in when creating the session object:
+   This method will save the token to the session object and return it. For future sessions simply pass the token in when creating the session object:
 
     session = ShopifyAPI::Session.new("SHOP_NAME.myshopify.com", token)
 
@@ -140,7 +142,7 @@ This package also includes the +shopify+ executable to make it easy to open up a
 == Using Development Version
 
 Download the source code and run:
-    
+
     rake install
 
 == Additional Resources

lib/shopify_api/session.rb

@@ -1,20 +1,20 @@
 
 module ShopifyAPI
-  
+
   class Session
     cattr_accessor :api_key
     cattr_accessor :secret
-    cattr_accessor :protocol 
+    cattr_accessor :protocol
     self.protocol = 'https'
 
     attr_accessor :url, :token, :name
-    
+
     class << self
-    
+
       def setup(params)
         params.each { |k,value| send("#{k}=", value) }
       end
-      
+
       def temp(domain, token, &block)
         session = new(domain, token)
         begin
@@ -31,7 +31,7 @@ module ShopifyAPI
           ShopifyAPI::Base.activate_session(original_session)
         end
       end
-      
+
       def prepare_url(url)
         return nil if url.blank?
         url.gsub!(/https?:\/\//, '')                            # remove http:// or https://
@@ -57,16 +57,10 @@ module ShopifyAPI
       end
 
     end
-    
-    def initialize(url, token = nil, params = nil)
+
+    def initialize(url, token = nil)
       self.url, self.token = url, token
       self.class.prepare_url(self.url)
-
-      if params
-        unless self.class.validate_signature(params) && params[:timestamp].to_i > 24.hours.ago.utc.to_i
-          raise "Invalid Signature: Possible malicious login" 
-        end
-      end
     end
 
     def create_permission_url(scope, redirect_uri = nil)
@@ -75,9 +69,15 @@ module ShopifyAPI
       "#{protocol}://#{url}/admin/oauth/authorize?#{parameterize(params)}"
     end
 
-    def request_token(code)
+    def request_token(params)
       return token if token
-      
+
+      unless self.class.validate_signature(params) && params[:timestamp].to_i > 24.hours.ago.utc.to_i
+        raise "Invalid Signature: Possible malicious login"
+      end
+
+      code = params['code']
+
       response = access_token_request(code)
 
       if response.code == "200"
@@ -98,14 +98,14 @@ module ShopifyAPI
     def valid?
       url.present? && token.present?
     end
-  
+
     private
       def parameterize(params)
         URI.escape(params.collect{|k,v| "#{k}=#{v}"}.join('&'))
       end
 
       def access_token_request(code)
-        uri = URI.parse("#{protocol}://#{url}/admin/oauth/access_token")      
+        uri = URI.parse("#{protocol}://#{url}/admin/oauth/access_token")
         https = Net::HTTP.new(uri.host, uri.port)
         https.use_ssl = true
         request = Net::HTTP::Post.new(uri.request_uri)

test/session_test.rb

@@ -1,13 +1,13 @@
 require 'test_helper'
 
 class SessionTest < Test::Unit::TestCase
-  
+
   context "Session" do
     should "not be valid without a url" do
       session = ShopifyAPI::Session.new(nil, "any-token")
       assert_not session.valid?
     end
-    
+
     should "not be valid without token" do
       session = ShopifyAPI::Session.new("testshop.myshopify.com")
       assert_not session.valid?
@@ -17,7 +17,7 @@ class SessionTest < Test::Unit::TestCase
       session = ShopifyAPI::Session.new("testshop.myshopify.com", "any-token")
       assert session.valid?
     end
-    
+
     should "not raise error without params" do
       assert_nothing_raised do
         session = ShopifyAPI::Session.new("testshop.myshopify.com", "any-token")
@@ -26,7 +26,8 @@ class SessionTest < Test::Unit::TestCase
 
     should "raise error if params passed but signature omitted" do
       assert_raises(RuntimeError) do
-        session = ShopifyAPI::Session.new("testshop.myshopify.com", "any-token", {'foo' => 'bar'})
+        session = ShopifyAPI::Session.new("testshop.myshopify.com")
+        session.request_token({'code' => 'any-code'})
       end
     end
 
@@ -35,13 +36,13 @@ class SessionTest < Test::Unit::TestCase
       assert_equal "My test key", ShopifyAPI::Session.api_key
       assert_equal "My test secret", ShopifyAPI::Session.secret
     end
-    
+
     should "use 'https' protocol by default for all sessions" do
       assert_equal 'https', ShopifyAPI::Session.protocol
     end
 
     should "#temp reset ShopifyAPI::Base.site to original value" do
-      
+
       ShopifyAPI::Session.setup(:api_key => "key", :secret => "secret")
       session1 = ShopifyAPI::Session.new('fakeshop.myshopify.com', 'token1')
       ShopifyAPI::Base.activate_session(session1)
@@ -85,19 +86,12 @@ class SessionTest < Test::Unit::TestCase
       assert_equal "https://localhost.myshopify.com/admin/oauth/authorize?client_id=My_test_key&scope=", permission_url
     end
 
-    should "request token should get token" do
-      ShopifyAPI::Session.setup(:api_key => "My test key", :secret => "My test secret")
-      session = ShopifyAPI::Session.new('http://localhost.myshopify.com')
-      fake nil, :url => 'https://localhost.myshopify.com/admin/oauth/access_token',:method => :post, :body => '{"access_token" : "token"}'
-      assert_equal "token", session.request_token("code")
-    end
-
     should "raise exception if code invalid in request token" do
       ShopifyAPI::Session.setup(:api_key => "My test key", :secret => "My test secret")
       session = ShopifyAPI::Session.new('http://localhost.myshopify.com')
       fake nil, :url => 'https://localhost.myshopify.com/admin/oauth/access_token',:method => :post, :status => 404, :body => '{"error" : "invalid_request"}'
       assert_raises(RuntimeError) do
-        session.request_token("bad_code")
+        session.request_token(params={:code => "bad-code"})
       end
       assert_equal false, session.valid?
     end
@@ -119,13 +113,46 @@ class SessionTest < Test::Unit::TestCase
       assert_equal "https://testshop.myshopify.com/admin", session.site
     end
 
+    should "return_token_if_signature_is_valid" do
+      ShopifyAPI::Session.secret = 'secret'
+      params = {:code => 'any-code', :timestamp => Time.now}
+      sorted_params = make_sorted_params(params)
+      signature = Digest::MD5.hexdigest(ShopifyAPI::Session.secret + sorted_params)
+      fake nil, :url => 'https://testshop.myshopify.com/admin/oauth/access_token',:method => :post, :body => '{"access_token" : "any-token"}'
+      session = ShopifyAPI::Session.new("testshop.myshopify.com")
+      token = session.request_token(params.merge(:signature => signature))
+      assert_equal "any-token", token
+    end
+
     should "raise error if signature does not match expected" do
       ShopifyAPI::Session.secret = 'secret'
-      params = {:foo => 'hello', :foo => 'world', :timestamp => Time.now}
-      sorted_params = params.except(:signature, :action, :controller).collect{|k,v|"#{k}=#{v}"}.sort.join
+      params = {:code => "any-code", :timestamp => Time.now}
+      sorted_params = make_sorted_params(params)
       signature = Digest::MD5.hexdigest(ShopifyAPI::Session.secret + sorted_params)
+      params[:foo] = 'world'
+      assert_raises(RuntimeError) do
+        session = ShopifyAPI::Session.new("testshop.myshopify.com")
+        session.request_token(params.merge(:signature => signature))
+      end
+    end
 
-      session = ShopifyAPI::Session.new("testshop.myshopify.com", "any-token", params.merge(:signature => signature))
+    should "raise error if timestamp is too old" do
+      ShopifyAPI::Session.secret = 'secret'
+      params = {:code => "any-code", :timestamp => Time.now - 2.days}
+      sorted_params = make_sorted_params(params)
+      signature = Digest::MD5.hexdigest(ShopifyAPI::Session.secret + sorted_params)
+      params[:foo] = 'world'
+      assert_raises(RuntimeError) do
+        session = ShopifyAPI::Session.new("testshop.myshopify.com")
+        session.request_token(params.merge(:signature => signature))
+      end
+    end
+
+    private
+
+    def make_sorted_params(params)
+      sorted_params = params.except(:signature, :action, :controller).collect{|k,v|"#{k}=#{v}"}.sort.join
     end
+
   end
 end