Merge pull request #4366 from oliverbarnes/4361-breaks-facebook-posting

Fix #4361 twitter access level check breaking facebook addition to user

app/controllers/services_controller.rb

@@ -1,7 +1,6 @@
 #   Copyright (c) 2010-2011, Diaspora Inc.  This file is
 #   licensed under the Affero General Public License version 3 or later.  See 
 #   the COPYRIGHT file.
-
 class ServicesController < ApplicationController
   # We need to take a raw POST from an omniauth provider with no authenticity token.
   # See https://github.com/intridea/omniauth/issues/203
@@ -57,7 +56,7 @@ class ServicesController < ApplicationController
   end
 
   def abort_if_read_only_access
-    if header_hash["x_access_level"] && header_hash["x_access_level"] == 'read'
+    if omniauth_hash['provider'] == 'twitter' && twitter_header['x_access_level'] == 'read'
       flash[:error] =  I18n.t( 'services.create.read_only_access' )
       redirect_to_origin
     end
@@ -87,7 +86,17 @@ class ServicesController < ApplicationController
     request.env['omniauth.auth']
   end
 
-  def header_hash 
+  def extra_hash
-    omniauth_hash['extra'] ? omniauth_hash['extra']['access_token']['response']['header'] : {}
+    omniauth_hash['extra'] ? omniauth_hash['extra'] : {} 
+  end
+
+  def twitter_header 
+    twitter_header_present? ? extra_hash['access_token']['response']['header'] : {}
+  end 
+
+  #https://github.com/intridea/omniauth/wiki/Auth-Hash-Schema #=> normalized hash
+  #https://gist.github.com/oliverbarnes/6096959 #=> hash with twitter specific extra
+  def twitter_header_present?
+    extra_hash['access_token'] && extra_hash['access_token']['response'] && extra_hash['access_token']['response']['header']
   end
 end

app/models/service.rb

@@ -43,10 +43,6 @@ class Service < ActiveRecord::Base
       "Services::#{options[:provider].camelize}"
     end
 
-    def access_level
-      auth['extra']['access_token']['response']['header']['x_access_level'] if auth['extra']
-    end
-
     def options
       { 
         nickname:      auth['info']['nickname'],
@@ -54,11 +50,10 @@ class Service < ActiveRecord::Base
         access_secret: auth['credentials']['secret'],
         uid:           auth['uid'],
         provider:      auth['provider'],
-        info:          auth['info'], 
+        info:          auth['info']
-        access_level:  access_level  
       }
     end
 
-    private :auth, :service_type, :access_level, :options
+    private :auth, :service_type, :options
   end
 end

spec/controllers/services_controller_spec.rb

@@ -62,10 +62,11 @@ describe ServicesController do
       end
     end
 
+    context 'Twitter' do
       context 'when the access-level is read-only' do
         before do 
           access_level_hash = { 'extra' => { 'access_token' => { 'response' => { 'header' => { 'x_access_level' => 'read' }}}}}
-        request.env['omniauth.auth'] = omniauth_auth["info"].merge!( access_level_hash )
+          request.env['omniauth.auth'] = omniauth_auth.merge!( access_level_hash )
         end
 
         it 'doesnt create a new service' do
@@ -79,6 +80,20 @@ describe ServicesController do
           flash[:error].include?( 'Access level is read-only' ).should be_true
         end
       end
+    end
+
+    context 'Facebook' do
+      before do
+        facebook_auth_without_twitter_extras = { 'provider' => 'facebook', 'extras' => { 'someotherkey' => 'lorem'}}
+        request.env['omniauth.auth'] = omniauth_auth.merge!( facebook_auth_without_twitter_extras )
+      end
+
+      it "doesn't break when twitter-specific extras aren't available in omniauth hash" do
+        expect {
+          post :create, :provider => 'facebook'
+        }.to change(user.services, :count).by(1)
+      end
+    end
 
     context 'when fetching a photo' do
       before do

spec/models/service_spec.rb

@@ -40,8 +40,7 @@ describe Service do
       { 'provider' => 'facebook',
         'uid'      => '2',
         'info'   => { 'nickname' => 'grimmin' },
-        'credentials' => { 'token' => 'tokin', 'secret' =>"not_so_much" },
+        'credentials' => { 'token' => 'tokin', 'secret' =>"not_so_much" }
-        'extra' => { 'access_token' => { 'response' => { 'header' => { 'x_access_level' => 'read' }}}}
       }
     end
     let(:subject) { described_class.initialize_from_omniauth( omniauth ) }
@@ -55,7 +54,6 @@ describe Service do
       expect( subject.nickname ).to eql "grimmin"
       expect( subject.access_token ).to eql "tokin"
       expect( subject.access_secret ).to eql "not_so_much"
-      expect( subject.access_level ).to eql "read"
     end
   end
 end