Commit 64b4bd83f6c008af03442235e7cda064e5bed230
1 parent
d60663d3
Exists in
staging
and in
4 other branches
oauth_client: fix client
Showing
8 changed files
with
74 additions
and
52 deletions
Show diff stats
plugins/oauth_client/controllers/public/oauth_client_plugin_public_controller.rb
| @@ -2,18 +2,8 @@ class OauthClientPluginPublicController < PublicController | @@ -2,18 +2,8 @@ class OauthClientPluginPublicController < PublicController | ||
| 2 | 2 | ||
| 3 | def callback | 3 | def callback |
| 4 | auth = request.env["omniauth.auth"] | 4 | auth = request.env["omniauth.auth"] |
| 5 | - login = auth.info.email.split('@').first | ||
| 6 | - user = environment.users.find_with_omniauth(auth) | ||
| 7 | - | ||
| 8 | - if user | ||
| 9 | - session[:user] = user | ||
| 10 | - redirect_to :controller => :account, :action => :login | ||
| 11 | - else | ||
| 12 | - session[:oauth_data] = auth | ||
| 13 | - name = auth.info.name | ||
| 14 | - name ||= auth.extra && auth.extra.raw_info ? auth.extra.raw_info.name : '' | ||
| 15 | - redirect_to :controller => :account, :action => :signup, :user => {:login => login, :email => auth.info.email}, :profile_data => {:name => name} | ||
| 16 | - end | 5 | + user = environment.users.find_by_email(auth.info.email) |
| 6 | + user ? login(user) : signup(auth) | ||
| 17 | end | 7 | end |
| 18 | 8 | ||
| 19 | def failure | 9 | def failure |
| @@ -26,4 +16,29 @@ class OauthClientPluginPublicController < PublicController | @@ -26,4 +16,29 @@ class OauthClientPluginPublicController < PublicController | ||
| 26 | redirect_to root_url | 16 | redirect_to root_url |
| 27 | end | 17 | end |
| 28 | 18 | ||
| 19 | + protected | ||
| 20 | + | ||
| 21 | + def login(user) | ||
| 22 | + provider = OauthClientPlugin::Provider.find(session[:provider_id]) | ||
| 23 | + user_provider = user.oauth_user_providers.find_by_provider_id(provider.id) | ||
| 24 | + unless user_provider | ||
| 25 | + user_provider = user.oauth_user_providers.create(:user => user, :provider => provider, :enabled => true) | ||
| 26 | + end | ||
| 27 | + if user_provider.enabled? | ||
| 28 | + session[:user] = user.id | ||
| 29 | + else | ||
| 30 | + session[:notice] = _("Can't login with #{provider.name}") | ||
| 31 | + end | ||
| 32 | + | ||
| 33 | + redirect_to :controller => :account, :action => :login | ||
| 34 | + end | ||
| 35 | + | ||
| 36 | + def signup(auth) | ||
| 37 | + login = auth.info.email.split('@').first | ||
| 38 | + session[:oauth_data] = auth | ||
| 39 | + name = auth.info.name | ||
| 40 | + name ||= auth.extra && auth.extra.raw_info ? auth.extra.raw_info.name : '' | ||
| 41 | + redirect_to :controller => :account, :action => :signup, :user => {:login => login, :email => auth.info.email}, :profile_data => {:name => name} | ||
| 42 | + end | ||
| 43 | + | ||
| 29 | end | 44 | end |
plugins/oauth_client/db/migrate/20140828184930_add_settings_to_users.rb
plugins/oauth_client/db/migrate/20141014162710_create_oauth_client_user_providers.rb
0 → 100644
| @@ -0,0 +1,14 @@ | @@ -0,0 +1,14 @@ | ||
| 1 | +class CreateOauthClientUserProviders < ActiveRecord::Migration | ||
| 2 | + def self.up | ||
| 3 | + create_table :oauth_client_plugin_user_providers do |t| | ||
| 4 | + t.references :user | ||
| 5 | + t.references :provider | ||
| 6 | + t.boolean :enabled | ||
| 7 | + t.timestamps | ||
| 8 | + end | ||
| 9 | + end | ||
| 10 | + | ||
| 11 | + def self.down | ||
| 12 | + drop_table :oauth_client_plugin_user_providers | ||
| 13 | + end | ||
| 14 | +end |
plugins/oauth_client/lib/ext/user.rb
| @@ -2,21 +2,11 @@ require_dependency 'user' | @@ -2,21 +2,11 @@ require_dependency 'user' | ||
| 2 | 2 | ||
| 3 | class User | 3 | class User |
| 4 | 4 | ||
| 5 | - acts_as_having_settings :field => :settings | ||
| 6 | - | ||
| 7 | - settings_items :oauth_providers, :type => Array, :default => [] | ||
| 8 | - | ||
| 9 | - def self.find_with_omniauth(auth) | ||
| 10 | - user = self.find_by_email(auth.info.email) | ||
| 11 | - if user# && !user.oauth_providers.empty? #FIXME save new oauth providers | ||
| 12 | - user | ||
| 13 | - else | ||
| 14 | - nil | ||
| 15 | - end | ||
| 16 | - end | 5 | + has_many :oauth_user_providers, :class_name => 'OauthClientPlugin::UserProvider' |
| 6 | + has_many :oauth_providers, :through => :oauth_user_providers, :source => :provider | ||
| 17 | 7 | ||
| 18 | def password_required_with_oauth? | 8 | def password_required_with_oauth? |
| 19 | - password_required_without_oauth? && oauth_providers.blank? | 9 | + password_required_without_oauth? && oauth_providers.empty? |
| 20 | end | 10 | end |
| 21 | 11 | ||
| 22 | alias_method_chain :password_required?, :oauth | 12 | alias_method_chain :password_required?, :oauth |
| @@ -24,7 +14,12 @@ class User | @@ -24,7 +14,12 @@ class User | ||
| 24 | after_create :activate_oauth_user | 14 | after_create :activate_oauth_user |
| 25 | 15 | ||
| 26 | def activate_oauth_user | 16 | def activate_oauth_user |
| 27 | - activate unless oauth_providers.empty? | 17 | + unless oauth_providers.empty? |
| 18 | + activate | ||
| 19 | + oauth_providers.each do |provider| | ||
| 20 | + OauthClientPlugin::UserProvider.create!(:user => self, :provider => provider, :enabled => true) | ||
| 21 | + end | ||
| 22 | + end | ||
| 28 | end | 23 | end |
| 29 | 24 | ||
| 30 | def make_activation_code_with_oauth | 25 | def make_activation_code_with_oauth |
plugins/oauth_client/lib/oauth_client_plugin.rb
| @@ -60,6 +60,8 @@ class OauthClientPlugin < Noosfero::Plugin | @@ -60,6 +60,8 @@ class OauthClientPlugin < Noosfero::Plugin | ||
| 60 | provider_id = request.session['omniauth.params'] ? request.session['omniauth.params']['id'] : request.params['id'] | 60 | provider_id = request.session['omniauth.params'] ? request.session['omniauth.params']['id'] : request.params['id'] |
| 61 | provider = environment.oauth_providers.find(provider_id) | 61 | provider = environment.oauth_providers.find(provider_id) |
| 62 | strategy.options.merge!(provider.options.symbolize_keys) | 62 | strategy.options.merge!(provider.options.symbolize_keys) |
| 63 | + | ||
| 64 | + request.session[:provider_id] = provider_id | ||
| 63 | } | 65 | } |
| 64 | 66 | ||
| 65 | provider provider, :setup => setup, | 67 | provider provider, :setup => setup, |
| @@ -80,7 +82,7 @@ class OauthClientPlugin < Noosfero::Plugin | @@ -80,7 +82,7 @@ class OauthClientPlugin < Noosfero::Plugin | ||
| 80 | auth = session[:oauth_data] | 82 | auth = session[:oauth_data] |
| 81 | 83 | ||
| 82 | if auth.present? && params[:user].present? | 84 | if auth.present? && params[:user].present? |
| 83 | - params[:user][:oauth_providers] = [{:provider => auth.provider, :uid => auth.uid}] | 85 | + params[:user][:oauth_providers] = [OauthClientPlugin::Provider.find(session[:provider_id])] |
| 84 | if request.post? && auth.info.email != params[:user][:email] | 86 | if request.post? && auth.info.email != params[:user][:email] |
| 85 | raise "Wrong email for oauth signup" | 87 | raise "Wrong email for oauth signup" |
| 86 | end | 88 | end |
plugins/oauth_client/lib/oauth_client_plugin/user_provider.rb
0 → 100644
| @@ -0,0 +1,10 @@ | @@ -0,0 +1,10 @@ | ||
| 1 | +class OauthClientPlugin::UserProvider < Noosfero::Plugin::ActiveRecord | ||
| 2 | + | ||
| 3 | + belongs_to :user, :class_name => 'User' | ||
| 4 | + belongs_to :provider, :class_name => 'OauthClientPlugin::Provider' | ||
| 5 | + | ||
| 6 | + set_table_name :oauth_client_plugin_user_providers | ||
| 7 | + | ||
| 8 | + attr_accessible :user, :provider, :enabled | ||
| 9 | + | ||
| 10 | +end |
plugins/oauth_client/test/unit/oauth_client_plugin_test.rb
| @@ -9,9 +9,10 @@ class OauthClientPluginTest < ActiveSupport::TestCase | @@ -9,9 +9,10 @@ class OauthClientPluginTest < ActiveSupport::TestCase | ||
| 9 | @environment = Environment.default | 9 | @environment = Environment.default |
| 10 | @session = {} | 10 | @session = {} |
| 11 | @request = mock | 11 | @request = mock |
| 12 | + @provider = OauthClientPlugin::Provider.create!(:name => 'name', :identifier => 'identifier', :strategy => 'strategy') | ||
| 12 | end | 13 | end |
| 13 | 14 | ||
| 14 | - attr_reader :params, :plugin, :environment, :session, :request | 15 | + attr_reader :params, :plugin, :environment, :session, :request, :provider |
| 15 | 16 | ||
| 16 | should 'has extra contents for login' do | 17 | should 'has extra contents for login' do |
| 17 | assert plugin.login_extra_contents | 18 | assert plugin.login_extra_contents |
| @@ -41,6 +42,7 @@ class OauthClientPluginTest < ActiveSupport::TestCase | @@ -41,6 +42,7 @@ class OauthClientPluginTest < ActiveSupport::TestCase | ||
| 41 | oauth_data.stubs(:provider).returns('provider') | 42 | oauth_data.stubs(:provider).returns('provider') |
| 42 | info.stubs(:email).returns('test@example.com') | 43 | info.stubs(:email).returns('test@example.com') |
| 43 | session[:oauth_data] = oauth_data | 44 | session[:oauth_data] = oauth_data |
| 45 | + session[:provider_id] = provider.id | ||
| 44 | 46 | ||
| 45 | params[:user] = {:email => 'test2@example.com'} | 47 | params[:user] = {:email => 'test2@example.com'} |
| 46 | assert_raises RuntimeError do | 48 | assert_raises RuntimeError do |
| @@ -58,6 +60,7 @@ class OauthClientPluginTest < ActiveSupport::TestCase | @@ -58,6 +60,7 @@ class OauthClientPluginTest < ActiveSupport::TestCase | ||
| 58 | oauth_data.stubs(:provider).returns('provider') | 60 | oauth_data.stubs(:provider).returns('provider') |
| 59 | info.stubs(:email).returns('test@example.com') | 61 | info.stubs(:email).returns('test@example.com') |
| 60 | session[:oauth_data] = oauth_data | 62 | session[:oauth_data] = oauth_data |
| 63 | + session[:provider_id] = provider.id | ||
| 61 | 64 | ||
| 62 | params[:user] = {:email => 'test@example.com'} | 65 | params[:user] = {:email => 'test@example.com'} |
| 63 | instance_eval(&plugin.account_controller_filters[:block]) | 66 | instance_eval(&plugin.account_controller_filters[:block]) |
| @@ -74,6 +77,7 @@ class OauthClientPluginTest < ActiveSupport::TestCase | @@ -74,6 +77,7 @@ class OauthClientPluginTest < ActiveSupport::TestCase | ||
| 74 | oauth_data = mock | 77 | oauth_data = mock |
| 75 | oauth_data.stubs(:uid).returns('uid') | 78 | oauth_data.stubs(:uid).returns('uid') |
| 76 | oauth_data.stubs(:provider).returns('provider') | 79 | oauth_data.stubs(:provider).returns('provider') |
| 80 | + session[:provider_id] = provider.id | ||
| 77 | 81 | ||
| 78 | session[:oauth_data] = oauth_data | 82 | session[:oauth_data] = oauth_data |
| 79 | instance_eval(&plugin.account_controller_filters[:block]) | 83 | instance_eval(&plugin.account_controller_filters[:block]) |
plugins/oauth_client/test/unit/user_test.rb
| @@ -2,22 +2,13 @@ require File.dirname(__FILE__) + '/../test_helper' | @@ -2,22 +2,13 @@ require File.dirname(__FILE__) + '/../test_helper' | ||
| 2 | 2 | ||
| 3 | class UserTest < ActiveSupport::TestCase | 3 | class UserTest < ActiveSupport::TestCase |
| 4 | 4 | ||
| 5 | - should 'find with omniauth params' do | ||
| 6 | - user = fast_create(User) | ||
| 7 | - user.settings[:oauth_providers] = [:test => {}] | ||
| 8 | - user.save! | ||
| 9 | - auth = {:info => OpenStruct.new({:email => user.email})} | ||
| 10 | - assert_equal user, User.find_with_omniauth(OpenStruct.new(auth)) | ||
| 11 | - end | ||
| 12 | - | ||
| 13 | - should 'do not return user if there is no provider' do | ||
| 14 | - user = fast_create(User) | ||
| 15 | - auth = {:info => OpenStruct.new({:email => user.email})} | ||
| 16 | - assert_equal nil, User.find_with_omniauth(OpenStruct.new(auth)) | 5 | + def setup |
| 6 | + @provider = OauthClientPlugin::Provider.create!(:name => 'name', :identifier => 'identifier', :strategy => 'strategy') | ||
| 17 | end | 7 | end |
| 8 | + attr_reader :provider | ||
| 18 | 9 | ||
| 19 | should 'password is not required if there is a oauth provider' do | 10 | should 'password is not required if there is a oauth provider' do |
| 20 | - User.create!(:email => 'testoauth@example.com', :login => 'testoauth', :oauth_providers => [:test]) | 11 | + User.create!(:email => 'testoauth@example.com', :login => 'testoauth', :oauth_providers => [provider]) |
| 21 | end | 12 | end |
| 22 | 13 | ||
| 23 | should 'password is required if there is a oauth provider' do | 14 | should 'password is required if there is a oauth provider' do |
| @@ -27,7 +18,7 @@ class UserTest < ActiveSupport::TestCase | @@ -27,7 +18,7 @@ class UserTest < ActiveSupport::TestCase | ||
| 27 | end | 18 | end |
| 28 | 19 | ||
| 29 | should 'activate user when created with oauth' do | 20 | should 'activate user when created with oauth' do |
| 30 | - user = User.create!(:email => 'testoauth@example.com', :login => 'testoauth', :oauth_providers => [:test]) | 21 | + user = User.create!(:email => 'testoauth@example.com', :login => 'testoauth', :oauth_providers => [provider]) |
| 31 | assert user.activated? | 22 | assert user.activated? |
| 32 | end | 23 | end |
| 33 | 24 | ||
| @@ -37,7 +28,7 @@ class UserTest < ActiveSupport::TestCase | @@ -37,7 +28,7 @@ class UserTest < ActiveSupport::TestCase | ||
| 37 | end | 28 | end |
| 38 | 29 | ||
| 39 | should 'not make activation code when created with oauth' do | 30 | should 'not make activation code when created with oauth' do |
| 40 | - user = User.create!(:email => 'testoauth@example.com', :login => 'testoauth', :oauth_providers => [:test]) | 31 | + user = User.create!(:email => 'testoauth@example.com', :login => 'testoauth', :oauth_providers => [provider]) |
| 41 | assert !user.activation_code | 32 | assert !user.activation_code |
| 42 | end | 33 | end |
| 43 | 34 |