From 452301b34d842b3e2c0ebf926d4feb7741155eed Mon Sep 17 00:00:00 2001 From: Oliver Azevedo Barnes Date: Wed, 7 Aug 2013 18:59:49 -0500 Subject: [PATCH] Fix #4361 twitter access level check breaking facebook addition to user --- app/controllers/services_controller.rb | 17 +++++++-- app/models/service.rb | 9 +---- spec/controllers/services_controller_spec.rb | 39 ++++++++++++++------ spec/models/service_spec.rb | 4 +- 4 files changed, 43 insertions(+), 26 deletions(-) diff --git a/app/controllers/services_controller.rb b/app/controllers/services_controller.rb index b6b3e191d..69751e082 100644 --- a/app/controllers/services_controller.rb +++ b/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 - omniauth_hash['extra'] ? omniauth_hash['extra']['access_token']['response']['header'] : {} + def extra_hash + 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 diff --git a/app/models/service.rb b/app/models/service.rb index 2620fecf6..463bd8389 100644 --- a/app/models/service.rb +++ b/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'], - access_level: access_level + info: auth['info'] } end - private :auth, :service_type, :access_level, :options + private :auth, :service_type, :options end end diff --git a/spec/controllers/services_controller_spec.rb b/spec/controllers/services_controller_spec.rb index 70b73e6ec..0705566d6 100644 --- a/spec/controllers/services_controller_spec.rb +++ b/spec/controllers/services_controller_spec.rb @@ -62,21 +62,36 @@ describe ServicesController do end end - 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 ) + 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.merge!( access_level_hash ) + end + + it 'doesnt create a new service' do + expect { + post :create, :provider => 'twitter' + }.to_not change(Service, :count).by(1) + end + + it 'flashes an read-only access error' do + post :create, :provider => 'twitter' + 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 'doesnt create a new service' do + it "doesn't break when twitter-specific extras aren't available in omniauth hash" do expect { - post :create, :provider => 'twitter' - }.to_not change(Service, :count).by(1) - end - - it 'flashes an read-only access error' do - post :create, :provider => 'twitter' - flash[:error].include?( 'Access level is read-only' ).should be_true + post :create, :provider => 'facebook' + }.to change(user.services, :count).by(1) end end diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 93607d923..069c8811a 100644 --- a/spec/models/service_spec.rb +++ b/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" }, - 'extra' => { 'access_token' => { 'response' => { 'header' => { 'x_access_level' => 'read' }}}} + 'credentials' => { 'token' => 'tokin', 'secret' =>"not_so_much" } } 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