tested and fixed Campfire strategy

oa-oauth/lib/omniauth/strategies/basecamp.rb

@@ -68,7 +68,7 @@ module OmniAuth
           },
           'extra' => {
             'access_token' => @access_token
-          },
+          }
         })
       end
       

oa-oauth/lib/omniauth/strategies/campfire.rb

@@ -20,12 +20,28 @@ module OmniAuth
       
       protected
       
-      def request_phase
-        if env['REQUEST_METHOD'] == 'GET'
-          ask_for_campfire_subdomain
-        else
-          super(options.merge(:site => campfire_url))
+      def client
+        ::OAuth2::Client.new(@client.id, @client.secret, :site => campfire_url)
       end
+      
+      def request_phase
+        if subdomain
+          super
+        else
+          ask_for_campfire_subdomain
+        end
+      end
+      
+      def callback_phase
+        if subdomain
+          super
+        else
+          ask_for_campfire_subdomain
+        end
+      end
+      
+      def subdomain
+        ((request.session[:oauth] ||= {})[:campfire] ||= {})[:subdomain] ||= request.params[CAMPFIRE_SUBDOMAIN_PARAMETER]
       end
       
       def user_data
@@ -33,23 +49,25 @@ module OmniAuth
       end
       
       def ask_for_campfire_subdomain
-        OmniAuth::Form.build(title) do
-          text_field 'Campfire Subdomain', CAMPFIRE_SUBDOMAIN_PARAMETER
+        OmniAuth::Form.build('Campfire Subdomain Required') do
+          text_field 'Campfire Subdomain', ::OmniAuth::Strategies::Campfire::CAMPFIRE_SUBDOMAIN_PARAMETER
         end.to_response
       end
       
       def campfire_url
-        subdomain = request.params[CAMPFIRE_SUBDOMAIN_PARAMETER]
-        'http://#{subdomain}.campfirenow.com'
+        "https://#{subdomain}.campfirenow.com"
       end
       
       def auth_hash
-        user_hash = MultiJson.decode(@response.body)['user']
+        data = self.user_data
         OmniAuth::Utils.deep_merge(super, {
-          'uid' => user_hash['id'],
-          'user_info' => user_info(user_hash),
+          'uid' => data['user']['id'].to_s,
+          'user_info' => user_info(data),
           'credentials' => {
-            'token' => user_hash['api_auth_token']
+            'token' => data['api_auth_token']
+          },
+          'extra' => {
+            'access_token' => @access_token
           }
         })
       end

oa-oauth/spec/fixtures/campfire_200.json

@@ -0,0 +1,10 @@
+{
+  "user": {
+    "id": 92718,
+    "name": "Kenneth Szell",
+    "email_address": "kens@example.org",
+    "admin": true,
+    "created_at": "2009-07-20T09:21:34Z",
+    "type": "Member"
+  }
+}

oa-oauth/spec/omniauth/strategies/campfire_spec.rb

@@ -1,7 +1,82 @@
 require File.dirname(__FILE__) + '/../../spec_helper'
 
 describe OmniAuth::Strategies::Campfire do
-  it 'should exist' do
-    # do nothing
+  
+  def app
+    Rack::Builder.new {
+      use OmniAuth::Test::PhonySession
+      use OmniAuth::Strategies::Campfire, 'abc', 'def'
+      run lambda { |env| [200, {'Content-Type' => 'text/plain'}, [Rack::Request.new(env).params.key?('auth').to_s]] }
+    }.to_app
+  end
+
+  def session
+    last_request.env['rack.session']
+  end
+  
+  describe '/auth/campfire without a subdomain' do
+    before do
+      get '/auth/campfire'
+    end
+    
+    it 'should respond with OK' do
+      last_response.should be_ok
+    end
+    
+    it 'should respond with HTML' do
+      last_response.content_type.should == 'text/html'
+    end
+    
+    it 'should render a subdomain input' do
+      last_response.body.should =~ %r{<input[^>]*subdomain}
+    end
+  end
+  
+  describe 'POST /auth/campfire with a subdomain' do
+    before do
+      # the middleware doesn't actually care that it's a POST,
+      # but it makes the "redirect_to" calculation down below easier
+      # since the params are passed in the body rather than the URL.
+      post '/auth/campfire', {OmniAuth::Strategies::Campfire::CAMPFIRE_SUBDOMAIN_PARAMETER => 'flugle'}
+    end
+    
+    it 'should redirect to the proper authorize_url' do
+      last_response.should be_redirect
+      redirect_to = CGI.escape(last_request.url + '/callback')
+      last_response.headers['Location'].should == "https://flugle.campfirenow.com/oauth/authorize?client_id=abc&redirect_uri=#{redirect_to}&type=web_server"
+    end
+    
+    it 'should set the campfire subdomain in the session' do
+      session[:oauth][:campfire][:subdomain].should == 'flugle'
+    end
+    
+  end
+  
+  describe 'followed by GET /auth/campfire/callback' do
+    before do
+      stub_request(:post, 'https://flugle.campfirenow.com/oauth/access_token').
+         to_return(:body => %q{{"access_token": "your_token"}})
+      stub_request(:get, 'https://flugle.campfirenow.com/users/me.json?access_token=your_token').
+         to_return(:body => File.read(File.join(File.dirname(__FILE__), '..', '..', 'fixtures', 'campfire_200.json')))
+      get '/auth/campfire/callback?code=plums', {}, {'rack.session' => {:oauth => {:campfire => {:subdomain => 'flugle'}}}}
+    end
+  
+    it 'should set the provider to "campfire"' do
+      last_request['auth']['provider'].should == 'campfire'
+    end
+  
+    it 'should set the UID to "92718"' do
+      last_request['auth']['uid'].should == '92718'
+    end
+  
+    it 'should exchange the request token for an access token' do
+      token = last_request['auth']['extra']['access_token']
+      token.should be_kind_of(OAuth2::AccessToken)
+      token.token.should == 'your_token'
+    end
+  
+    it 'should call through to the master app' do
+      last_response.body.should == 'true'
+    end
   end
 end