nocebo/diaspora
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Issue #4124 Check write access before authorizing Twitter

Browse source
This commit is contained in:
Oliver Azevedo Barnes 2013-08-02 19:01:20 -05:00
parent 7057f77924
commit 55a58713a5
8 changed files with 199 additions and 81 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
Changelog.md
View file

@ -3,12 +3,14 @@
**Attention:** This release includes a potentially long running migration! However it should be safe to run this while keeping your application servers on.
## Refactor
* Service and ServiceController, general code reorg to make it cleaner/+ testable/+ extensible
* Background actual mailing when sending invitations [#4069](https://github.com/diaspora/diaspora/issues/4069)
* Set the current user on the client side through gon [#4028](https://github.com/diaspora/diaspora/issues/4028)
* Update sign out route to a DELETE request [#4068](https://github.com/diaspora/diaspora/issues/4068)
* Convert all ActivityStreams::Photo to StatusMessages and drop ActivityStreams::Photo [#4144](https://github.com/diaspora/diaspora/issues/4144)
## Bug fixes
* Check twitter write access before adding/authorizing it for a user.
* Don't focus comment form on 'show n more comments' [#4265](https://github.com/diaspora/diaspora/issues/4265)
* Do not render mobile photo view for none-existing photos [#4194](https://github.com/diaspora/diaspora/issues/4194)
* Render markdown content for prettier email subjects and titles [#4182](https://github.com/diaspora/diaspora/issues/4182)

82
app/controllers/services_controller.rb
View file

@ -7,8 +7,8 @@ class ServicesController < ApplicationController
# See https://github.com/intridea/omniauth/issues/203
# See also http://www.communityguides.eu/articles/16
skip_before_filter :verify_authenticity_token, :only => :create
before_filter :authenticate_user!
before_filter :abort_if_already_authorized, :abort_if_read_only_access, :only => :create
respond_to :html
respond_to :json, :only => :inviter
@ -18,42 +18,18 @@ class ServicesController < ApplicationController
end
def create
auth = request.env['omniauth.auth']
service = Service.initialize_from_omniauth( omniauth_hash )
toke = auth['credentials']['token']
secret = auth['credentials']['secret']
if current_user.services << service
current_user.update_profile_with_omniauth( service.info )
provider = auth['provider']
user = auth['info']
service = "Services::#{provider.camelize}".constantize.new(:nickname => user['nickname'],
:access_token => toke,
:access_secret => secret,
:uid => auth['uid'])
current_user.services << service
if service.persisted?
fetch_photo = current_user.profile[:image_url].blank?
current_user.update_profile(current_user.profile.from_omniauth_hash(user))
Workers::FetchProfilePhoto.perform_async(current_user.id, service.id, user["image"]) if fetch_photo
fetch_photo(service) if no_profile_image?
flash[:notice] = I18n.t 'services.create.success'
else
flash[:error] = I18n.t 'services.create.failure'
if existing_service = Service.where(:type => service.type.to_s, :uid => service.uid).first
flash[:error] << I18n.t('services.create.already_authorized',
:diaspora_id => existing_service.user.profile.diaspora_handle,
:service_name => provider.camelize )
end
end
if request.env['omniauth.origin'].nil?
render :text => ("<script>window.close()</script>")
else
redirect_to request.env['omniauth.origin']
end
redirect_to_origin
end
def failure
@ -68,4 +44,50 @@ class ServicesController < ApplicationController
flash[:notice] = I18n.t 'services.destroy.success'
redirect_to services_url
end
private
def abort_if_already_authorized
if service = Service.where(uid: omniauth_hash['uid']).first
flash[:error] = I18n.t( 'services.create.already_authorized',
diaspora_id: service.user.profile.diaspora_handle,
service_name: service.provider.camelize )
redirect_to_origin
end
end
def abort_if_read_only_access
if header_hash["x_access_level"] && header_hash["x_access_level"] == 'read'
flash[:error] = I18n.t( 'services.create.read_only_access' )
redirect_to_origin
end
end
def redirect_to_origin
if origin
redirect_to origin
else
render(text: "<script>window.close()</script>")
end
end
def no_profile_image?
current_user.profile[:image_url].blank?
end
def fetch_photo(service)
Workers::FetchProfilePhoto.perform_async(current_user.id, service.id, service.info["image"])
end
def origin
request.env['omniauth.origin']
end
def omniauth_hash
request.env['omniauth.auth']
end
def header_hash
omniauth_hash['extra'] ? omniauth_hash['extra']['access_token']['response']['header'] : {}
end
end

48
app/models/service.rb
View file

@ -6,13 +6,11 @@ class Service < ActiveRecord::Base
include ActionView::Helpers::TextHelper
include MarkdownifyHelper
attr_accessor :provider, :info, :access_level
belongs_to :user
validates_uniqueness_of :uid, :scope => :type
def self.titles(service_strings)
service_strings.map{|s| "Services::#{s.titleize}"}
end
def profile_photo_url
nil
end
@ -21,4 +19,46 @@ class Service < ActiveRecord::Base
#don't do anything (should be overriden by service extensions)
end
class << self
def titles(service_strings)
service_strings.map {|s| "Services::#{s.titleize}"}
end
def first_from_omniauth( auth_hash )
@@auth = auth_hash
where( type: service_type, uid: options[:uid] ).first
end
def initialize_from_omniauth( auth_hash )
@@auth = auth_hash
service_type.constantize.new( options )
end
def auth
@@auth
end
def service_type
"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'],
access_token: auth['credentials']['token'],
access_secret: auth['credentials']['secret'],
uid: auth['uid'],
provider: auth['provider'],
info: auth['info'],
access_level: access_level
}
end
private :auth, :service_type, :access_level, :options
end
end

4
app/models/user.rb
View file

@ -350,6 +350,10 @@ class User < ActiveRecord::Base
end
end
def update_profile_with_omniauth( user_info )
update_profile( self.profile.from_omniauth_hash( user_info ) )
end
def deliver_profile_update
Postzord::Dispatcher.build(self, profile).post
end

1
config/locales/diaspora/en.yml
View file

@ -760,6 +760,7 @@ en:
success: "Authentication successful."
failure: "Authentication failed."
already_authorized: "A user with diaspora id %{diaspora_id} already authorized that %{service_name} account."
read_only_access: "Access level is read-only, please try to authorize again later"
destroy:
success: "Successfully deleted authentication."
failure:

87
spec/controllers/services_controller_spec.rb
View file

@ -5,61 +5,78 @@
require 'spec_helper'
describe ServicesController do
let(:mock_access_token) { Object.new }
let(:omniauth_auth) {
let(:omniauth_auth) do
{ 'provider' => 'twitter',
'uid' => '2',
'info' => { 'nickname' => 'grimmin' },
'credentials' => { 'token' => 'tokin', 'secret' =>"not_so_much" }
}
}
'credentials' => { 'token' => 'tokin', 'secret' =>"not_so_much" }}
end
let(:user) { alice }
before do
@user = alice
sign_in :user, @user
@controller.stub!(:current_user).and_return(@user)
mock_access_token.stub!(:token => "12345", :secret => "56789")
sign_in :user, user
@controller.stub!(:current_user).and_return(user)
end
describe '#index' do
it 'displays all connected serivices for a user' do
4.times do
FactoryGirl.create(:service, :user => @user)
before do
FactoryGirl.create(:service, user: user)
end
it "displays user's connected services" do
get :index
assigns[:services].should == @user.services
assigns[:services].should == user.services
end
end
describe '#create' do
context 'when not fetching a photo' do
before do
request.env['omniauth.auth'] = omniauth_auth
request.env['omniauth.origin'] = root_url
end
it 'creates a new OmniauthService' do
it 'creates a new service and associates it with the current user' do
expect {
post :create, :provider => 'twitter'
}.to change(@user.services, :count).by(1)
}.to change(user.services, :count).by(1)
end
it 'creates a twitter service' do
Service.delete_all
@user.getting_started = false
it 'saves the provider' do
post :create, :provider => 'twitter'
@user.reload.services.first.class.name.should == "Services::Twitter"
user.reload.services.first.class.name.should == "Services::Twitter"
end
it 'returns error if the user already a service with that uid' do
Services::Twitter.create!(:nickname => omniauth_auth["info"]['nickname'],
:access_token => omniauth_auth['credentials']['token'],
:access_secret => omniauth_auth['credentials']['secret'],
:uid => omniauth_auth['uid'],
:user => bob)
context 'when service exists with the same uid' do
before { Services::Twitter.create!(uid: omniauth_auth['uid'], user_id: user.id) }
it 'doesnt create a new service' do
expect {
post :create, :provider => 'twitter'
flash[:error].include?(bob.person.profile.diaspora_handle).should be_true
}.to_not change(Service, :count).by(1)
end
it 'flashes an already_authorized error with the diaspora handle for the user' do
post :create, :provider => 'twitter'
flash[:error].include?(user.profile.diaspora_handle).should be_true
flash[:error].include?( 'already authorized' ).should be_true
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 )
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
@ -72,9 +89,7 @@ describe ServicesController do
end
it 'does not queue a job if the profile photo is set' do
profile = @user.person.profile
profile[:image_url] = "/non/default/image.jpg"
profile.save
@controller.stub!(:no_profile_image?).and_return false
Workers::FetchProfilePhoto.should_not_receive(:perform_async)
@ -82,11 +97,9 @@ describe ServicesController do
end
it 'queues a job to save user photo if the photo does not exist' do
profile = @user.person.profile
profile[:image_url] = nil
profile.save
@controller.stub!(:no_profile_image?).and_return true
Workers::FetchProfilePhoto.should_receive(:perform_async).with(@user.id, anything(), "https://service.com/fallback_lowres.jpg")
Workers::FetchProfilePhoto.should_receive(:perform_async).with(user.id, anything(), "https://service.com/fallback_lowres.jpg")
post :create, :provider => 'twitter'
end
@ -95,13 +108,13 @@ describe ServicesController do
describe '#destroy' do
before do
@service1 = FactoryGirl.create(:service, :user => @user)
@service1 = FactoryGirl.create(:service, :user => user)
end
it 'destroys a service selected by id' do
lambda{
delete :destroy, :id => @service1.id
}.should change(@user.services, :count).by(-1)
}.should change(user.services, :count).by(-1)
end
end
end

38
spec/models/service_spec.rb
View file

@ -19,7 +19,43 @@ describe Service do
end
it 'by default has no profile photo url' do
Service.new.profile_photo_url.should be_nil
expect( described_class.new.profile_photo_url ).to be_nil
end
describe '.titles' do
it "converts passed service titles into service constants" do
expect( described_class.titles( ['twitter'] ) ).to eql ['Services::Twitter']
end
end
describe '.first_from_omniauth' do
let(:omniauth) { { 'provider' => 'facebook', 'uid' => '1', 'credentials' => {}, 'info' => {} } }
it 'first service by provider and uid' do
expect( described_class.first_from_omniauth( omniauth ) ).to eql @service
end
end
describe '.initialize_from_omniauth' do
let(:omniauth) do
{ 'provider' => 'facebook',
'uid' => '2',
'info' => { 'nickname' => 'grimmin' },
'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 ) }
it 'new service obj of type Services::Facebook' do
expect( subject.type ).to eql "Services::Facebook"
end
it 'new service obj with oauth options populated' do
expect( subject.uid ).to eql "2"
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