Compare View

switch
from
...
to
 
Commits (4)
Showing 13 changed files   Show diff stats
app/controllers/public/account_controller.rb
  Diff comments   View file @ef5d596
@@ -154,7 +154,7 @@ class AccountController < ApplicationController @@ -154,7 +154,7 @@ class AccountController < ApplicationController
154 154
155 # action to perform logout from the application 155 # action to perform logout from the application
156 def logout 156 def logout
157 - if logged_in? 157 + if logged_in? && self.current_user.id.present?
158 self.current_user.forget_me 158 self.current_user.forget_me
159 end 159 end
160 reset_session 160 reset_session
plugins/oauth_client/controllers/public/oauth_client_plugin_public_controller.rb
  Diff comments   View file @ef5d596
@@ -3,14 +3,14 @@ class OauthClientPluginPublicController < PublicController @@ -3,14 +3,14 @@ class OauthClientPluginPublicController < PublicController
3 skip_before_filter :login_required 3 skip_before_filter :login_required
4 4
5 def callback 5 def callback
6 - auth = request.env["omniauth.auth"]  
7 - 6 + auth_data = request.env["omniauth.auth"]
8 oauth_params = request.env["omniauth.params"] 7 oauth_params = request.env["omniauth.params"]
  8 +
9 if oauth_params && oauth_params["action"] == "external_login" 9 if oauth_params && oauth_params["action"] == "external_login"
10 - external_person_login(auth) 10 + external_person_login(auth_data)
11 else 11 else
12 - auth_user = environment.users.where(email: auth.info.email).first  
13 - if auth_user then login(auth_user.person) else signup(auth) end 12 + auth_user = environment.users.where(email: auth_data.info.email).first
  13 + if auth_user then login(auth_user.person) else signup(auth_data) end
14 end 14 end
15 end 15 end
16 16
@@ -26,48 +26,45 @@ class OauthClientPluginPublicController < PublicController @@ -26,48 +26,45 @@ class OauthClientPluginPublicController < PublicController
26 26
27 protected 27 protected
28 28
29 - def external_person_login(auth) 29 + def external_person_login(auth_data)
30 provider = OauthClientPlugin::Provider.find(session[:provider_id]) 30 provider = OauthClientPlugin::Provider.find(session[:provider_id])
31 31
32 - if provider.enabled?  
33 - user = User.new(email: auth.info.email, login: auth.info.name.to_slug)  
34 - webfinger = OpenStruct.new(  
35 - identifier: auth.info.nickname || user.login,  
36 - name: auth.info.name,  
37 - created_at: Time.now,  
38 - domain: provider.site || auth.provider,  
39 - email: user.email)  
40 - person = ExternalPerson.get_or_create(webfinger)  
41 - user.external_person_id = person.id 32 + user = User.new(email: auth_data.info.email, login: auth_data.info.name.to_slug)
  33 + person_data = OpenStruct.new(
  34 + identifier: auth_data.info.nickname || user.login,
  35 + name: auth_data.info.name,
  36 + created_at: Time.now,
  37 + domain: provider.site || auth_data.provider,
  38 + email: user.email)
  39 + person = ExternalPerson.get_or_create(person_data)
  40 + user.external_person_id = person.id
42 41
43 - oauth_auth = person.oauth_auth  
44 - if oauth_auth.nil?  
45 - auth_data = { external_person: person, provider: provider, enabled: true,  
46 - external_person_uid: auth.uid, external_person_image_url: auth.info.image }  
47 - oauth_auth = OauthClientPlugin::Auth.create_for_strategy(provider.strategy, auth_data)  
48 - end  
49 - self.current_user = user if oauth_auth.enabled?  
50 - else  
51 - session[:notice] = _("Can't login with %s") % provider.name  
52 - end 42 + oauth_auth = person.oauth_auth
  43 + oauth_data = { profile: person, provider: provider, enabled: true,
  44 + external_person_uid: auth_data.uid, external_person_image_url: auth_data.info.image }
  45 + oauth_auth ||= OauthClientPlugin::Auth.create_for_strategy(provider.strategy, oauth_data)
  46 + create_session(user, oauth_auth)
  47 + end
53 48
54 - redirect_to :controller => :account, :action => :login 49 + def signup(auth_data)
  50 + session[:oauth_data] = auth_data
  51 + username = auth_data.info.email.split('@').first
  52 + name = auth_data.info.name
  53 + name ||= auth_data.extra && auth_data.extra.raw_info ? auth_data.extra.raw_info.name : ''
  54 + redirect_to :controller => :account, :action => :signup, :user => {:login => username, :email => auth_data.info.email}, :profile_data => {:name => name}
55 end 55 end
56 56
57 - def signup(auth)  
58 - login = auth.info.email.split('@').first  
59 - session[:oauth_data] = auth  
60 - name = auth.info.name  
61 - name ||= auth.extra && auth.extra.raw_info ? auth.extra.raw_info.name : ''  
62 - redirect_to :controller => :account, :action => :signup, :user => {:login => login, :email => auth.info.email}, :profile_data => {:name => name} 57 + def login(person)
  58 + auth = person.oauth_auths.find_or_create_by(profile: person,
  59 + provider_id: session[:provider_id])
  60 + create_session(person.user, auth)
63 end 61 end
64 62
65 - def login person 63 + def create_session(user, oauth_auth)
66 provider = OauthClientPlugin::Provider.find(session[:provider_id]) 64 provider = OauthClientPlugin::Provider.find(session[:provider_id])
67 - auth = person.oauth_auths.where(provider_id: provider.id).first  
68 - auth ||= person.oauth_auths.create! profile: person, provider: provider, enabled: true  
69 - if auth.enabled? && provider.enabled?  
70 - self.current_user = person.user 65 +
  66 + if oauth_auth.allow_login?
  67 + self.current_user = user
71 else 68 else
72 session[:notice] = _("Can't login with %s") % provider.name 69 session[:notice] = _("Can't login with %s") % provider.name
73 end 70 end
plugins/oauth_client/db/migrate/20160720165808_add_external_profile_to_oauth_auth.rb
  Diff comments   View file @ef5d596
1 class AddExternalProfileToOauthAuth < ActiveRecord::Migration 1 class AddExternalProfileToOauthAuth < ActiveRecord::Migration
2 def up 2 def up
3 - add_column :oauth_client_plugin_auths, :external_person_id, :integer 3 + add_column :oauth_client_plugin_auths, :profile_type, :string
  4 + add_index :oauth_client_plugin_auths, :profile_type
  5 +
4 add_column :oauth_client_plugin_auths, :external_person_uid, :string 6 add_column :oauth_client_plugin_auths, :external_person_uid, :string
5 add_column :oauth_client_plugin_auths, :external_person_image_url, :string 7 add_column :oauth_client_plugin_auths, :external_person_image_url, :string
6 - add_index :oauth_client_plugin_auths, :external_person_id 8 +
  9 + change_column_default :oauth_client_plugin_auths, :enabled, true
7 end 10 end
8 11
9 def down 12 def down
10 - remove_index :oauth_client_plugin_auths, :external_person_id  
11 - remove_column :oauth_client_plugin_auths, :external_person_id 13 + remove_index :oauth_client_plugin_auths, :profile_type
  14 + remove_column :oauth_client_plugin_auths, :profile_type
  15 +
12 remove_column :oauth_client_plugin_auths, :external_person_uid 16 remove_column :oauth_client_plugin_auths, :external_person_uid
13 remove_column :oauth_client_plugin_auths, :external_person_image_url 17 remove_column :oauth_client_plugin_auths, :external_person_image_url
  18 +
  19 + change_column_default :oauth_client_plugin_auths, :enabled, nil
14 end 20 end
15 end 21 end
plugins/oauth_client/lib/ext/external_person.rb
  Diff comments   View file @ef5d596
@@ -2,7 +2,7 @@ require_dependency &#39;external_person&#39; @@ -2,7 +2,7 @@ require_dependency &#39;external_person&#39;
2 2
3 class ExternalPerson 3 class ExternalPerson
4 4
5 - has_one :oauth_auth, foreign_key: :external_person_id, class_name: 'OauthClientPlugin::Auth', dependent: :destroy 5 + has_one :oauth_auth, as: :profile, class_name: 'OauthClientPlugin::Auth', dependent: :destroy
6 has_one :oauth_provider, through: :oauth_auth, source: :provider 6 has_one :oauth_provider, through: :oauth_auth, source: :provider
7 7
8 def avatar 8 def avatar
plugins/oauth_client/lib/ext/profile.rb
  Diff comments   View file @ef5d596
@@ -2,7 +2,7 @@ require_dependency &#39;profile&#39; @@ -2,7 +2,7 @@ require_dependency &#39;profile&#39;
2 2
3 class Profile 3 class Profile
4 4
5 - has_many :oauth_auths, foreign_key: :profile_id, class_name: 'OauthClientPlugin::Auth', dependent: :destroy 5 + has_many :oauth_auths, as: :profile, class_name: 'OauthClientPlugin::Auth', dependent: :destroy
6 has_many :oauth_providers, through: :oauth_auths, source: :provider 6 has_many :oauth_providers, through: :oauth_auths, source: :provider
7 7
8 end 8 end
plugins/oauth_client/models/oauth_client_plugin/auth.rb
  Diff comments   View file @ef5d596
1 class OauthClientPlugin::Auth < ApplicationRecord 1 class OauthClientPlugin::Auth < ApplicationRecord
2 2
3 - attr_accessible :profile, :external_person, :provider,  
4 - :enabled, :access_token, :expires_in, :type,  
5 - :external_person_uid, :external_person_image_url 3 + attr_accessible :profile, :provider, :provider_id, :enabled,
  4 + :access_token, :expires_in, :type, :external_person_uid,
  5 + :external_person_image_url
6 6
7 - belongs_to :profile, class_name: 'Profile'  
8 - belongs_to :external_person, class_name: 'ExternalPerson' 7 + belongs_to :profile, polymorphic: true
9 belongs_to :provider, class_name: 'OauthClientPlugin::Provider' 8 belongs_to :provider, class_name: 'OauthClientPlugin::Provider'
10 9
11 validates_presence_of :provider 10 validates_presence_of :provider
  11 + validates_presence_of :profile
12 validates_uniqueness_of :profile_id, scope: :provider_id 12 validates_uniqueness_of :profile_id, scope: :provider_id
13 - validates_uniqueness_of :external_person_id, scope: :provider_id  
14 -  
15 - validate :must_be_related_to_profile  
16 13
17 acts_as_having_settings field: :data 14 acts_as_having_settings field: :data
18 15
@@ -30,18 +27,16 @@ class OauthClientPlugin::Auth &lt; ApplicationRecord @@ -30,18 +27,16 @@ class OauthClientPlugin::Auth &lt; ApplicationRecord
30 not self.expired? 27 not self.expired?
31 end 28 end
32 29
  30 + def allow_login?
  31 + self.enabled? && self.provider.enabled?
  32 + end
  33 +
33 def self.create_for_strategy(strategy, args = {}) 34 def self.create_for_strategy(strategy, args = {})
34 namespace = self.name.split("::")[0] 35 namespace = self.name.split("::")[0]
35 class_name = "#{namespace}::#{strategy.camelize}Auth" 36 class_name = "#{namespace}::#{strategy.camelize}Auth"
36 OauthClientPlugin::Auth.create!(args.merge(type: class_name)) 37 OauthClientPlugin::Auth.create!(args.merge(type: class_name))
37 end 38 end
38 39
39 - def must_be_related_to_profile  
40 - if self.profile.nil? && self.external_person.nil?  
41 - self.errors.add(:base, "Must be related to a profile or an external person")  
42 - end  
43 - end  
44 -  
45 IMAGE_SIZES = { 40 IMAGE_SIZES = {
46 :big => "150", 41 :big => "150",
47 :thumb => "100", 42 :thumb => "100",
plugins/oauth_client/models/oauth_client_plugin/github_auth.rb
  Diff comments   View file @ef5d596
@@ -6,7 +6,7 @@ class OauthClientPlugin::GithubAuth &lt; OauthClientPlugin::Auth @@ -6,7 +6,7 @@ class OauthClientPlugin::GithubAuth &lt; OauthClientPlugin::Auth
6 end 6 end
7 7
8 def profile_url 8 def profile_url
9 - "https://www.github.com/#{self.external_person.identifier}" 9 + "https://www.github.com/#{self.profile.identifier}"
10 end 10 end
11 11
12 def settings_url 12 def settings_url
plugins/oauth_client/models/oauth_client_plugin/noosfero_oauth2_auth.rb
  Diff comments   View file @ef5d596
1 class OauthClientPlugin::NoosferoOauth2Auth < OauthClientPlugin::Auth 1 class OauthClientPlugin::NoosferoOauth2Auth < OauthClientPlugin::Auth
2 2
3 def image_url(size = "") 3 def image_url(size = "")
4 - URI.join("http://#{self.provider.client_options[:site]}/profile/#{self.external_person.identifier}/icon/", size) 4 + URI.join("http://#{self.provider.client_options[:site]}/profile/#{self.profile.identifier}/icon/", size)
5 end 5 end
6 6
7 def profile_url 7 def profile_url
8 - "http://#{self.external_person.source}/profile/#{self.external_person.identifier}" 8 + "http://#{self.profile.source}/profile/#{self.profile.identifier}"
9 end 9 end
10 10
11 def settings_url 11 def settings_url
12 - "http://#{self.external_person.source}/myprofile/#{self.external_person.identifier}" 12 + "http://#{self.profile.source}/myprofile/#{self.profile.identifier}"
13 end 13 end
14 end 14 end
plugins/oauth_client/models/oauth_client_plugin/twitter_auth.rb
  Diff comments   View file @ef5d596
@@ -14,7 +14,7 @@ class OauthClientPlugin::TwitterAuth &lt; OauthClientPlugin::Auth @@ -14,7 +14,7 @@ class OauthClientPlugin::TwitterAuth &lt; OauthClientPlugin::Auth
14 end 14 end
15 15
16 def profile_url 16 def profile_url
17 - "https://twitter.com/#{self.external_person.identifier}" 17 + "https://twitter.com/#{self.profile.identifier}"
18 end 18 end
19 19
20 def settings_url 20 def settings_url
plugins/oauth_client/test/functional/account_controller_test.rb 0 → 100644
  Diff comments   View file @ef5d596
@@ -0,0 +1,19 @@ @@ -0,0 +1,19 @@
  1 +require 'test_helper'
  2 +
  3 +class AccountControllerTest < ActionController::TestCase
  4 + def setup
  5 + external_person = ExternalPerson.create!(identifier: 'johnlock',
  6 + name: 'John Locke',
  7 + source: 'anerenvironment.org',
  8 + email: 'john@locke.org',
  9 + created_at: Date.yesterday
  10 + )
  11 + session[:external] = external_person.id
  12 + end
  13 +
  14 + should "not create an User when logging out" do
  15 + assert_no_difference 'User.count' do
  16 + get :logout
  17 + end
  18 + end
  19 +end
plugins/oauth_client/test/functional/oauth_client_plugin_public_controller_test.rb
  Diff comments   View file @ef5d596
@@ -25,7 +25,39 @@ class OauthClientPluginPublicControllerTest &lt; ActionController::TestCase @@ -25,7 +25,39 @@ class OauthClientPluginPublicControllerTest &lt; ActionController::TestCase
25 assert_match /.*\/account\/signup/, @response.redirect_url 25 assert_match /.*\/account\/signup/, @response.redirect_url
26 end 26 end
27 27
28 - should 'login using provider when url param is present' do 28 + should 'login when user already signed up' do
  29 + create_user(@auth.info.name, email: @auth.info.email)
  30 +
  31 + get :callback
  32 + assert session[:user].present?
  33 + end
  34 +
  35 + should 'not login when user already signed up and the provider is disabled' do
  36 + create_user(@auth.info.name, email: @auth.info.email)
  37 + provider.update_attribute(:enabled, false)
  38 +
  39 + get :callback
  40 + assert session[:user].nil?
  41 + end
  42 +
  43 + should 'not login when user already signed up and the provider is disabled for him' do
  44 + create_user(@auth.info.name, email: @auth.info.email)
  45 + OauthClientPlugin::Auth.any_instance.stubs(:enabled?).returns(false)
  46 +
  47 + get :callback
  48 + assert session[:user].nil?
  49 + end
  50 +
  51 + should 'not duplicate oauth_auths when the same provider is used several times' do
  52 + user = create_user(@auth.info.name, email: @auth.info.email)
  53 +
  54 + get :callback
  55 + assert_no_difference 'user.oauth_auths.count' do
  56 + 3.times { get :callback }
  57 + end
  58 + end
  59 +
  60 + should 'perform external login using provider when url param is present' do
29 request.env["omniauth.params"] = {"action" => "external_login"} 61 request.env["omniauth.params"] = {"action" => "external_login"}
30 62
31 get :callback 63 get :callback
@@ -33,7 +65,15 @@ class OauthClientPluginPublicControllerTest &lt; ActionController::TestCase @@ -33,7 +65,15 @@ class OauthClientPluginPublicControllerTest &lt; ActionController::TestCase
33 assert session[:external].present? 65 assert session[:external].present?
34 end 66 end
35 67
36 - should 'not login when the provider is disabled' do 68 + should 'not create an user when performing external login' do
  69 + request.env["omniauth.params"] = {"action" => "external_login"}
  70 +
  71 + assert_no_difference 'User.count' do
  72 + get :callback
  73 + end
  74 + end
  75 +
  76 + should 'not perform external login when the provider is disabled' do
37 request.env["omniauth.params"] = {"action" => "external_login"} 77 request.env["omniauth.params"] = {"action" => "external_login"}
38 provider.update_attribute(:enabled, false) 78 provider.update_attribute(:enabled, false)
39 79
@@ -42,7 +82,7 @@ class OauthClientPluginPublicControllerTest &lt; ActionController::TestCase @@ -42,7 +82,7 @@ class OauthClientPluginPublicControllerTest &lt; ActionController::TestCase
42 assert session[:external].nil? 82 assert session[:external].nil?
43 end 83 end
44 84
45 - should 'not login when the provider is disabled for a user' do 85 + should 'not perform external login when the provider is disabled for a user' do
46 request.env["omniauth.params"] = {"action" => "external_login"} 86 request.env["omniauth.params"] = {"action" => "external_login"}
47 OauthClientPlugin::GithubAuth.any_instance.stubs(:enabled?).returns(false) 87 OauthClientPlugin::GithubAuth.any_instance.stubs(:enabled?).returns(false)
48 88
@@ -58,17 +98,4 @@ class OauthClientPluginPublicControllerTest &lt; ActionController::TestCase @@ -58,17 +98,4 @@ class OauthClientPluginPublicControllerTest &lt; ActionController::TestCase
58 external_person = ExternalPerson.find_by(identifier: auth.info.nickname) 98 external_person = ExternalPerson.find_by(identifier: auth.info.nickname)
59 assert_equal provider, external_person.oauth_auth.provider 99 assert_equal provider, external_person.oauth_auth.provider
60 end 100 end
61 -  
62 -# should 'do not duplicate relations between an user and a provider when the same provider was used again in a login' do  
63 -# user = create_user  
64 -# auth.info.stubs(:email).returns(user.email)  
65 -# auth.info.stubs(:name).returns(user.name)  
66 -# session[:provider_id] = provider.id  
67 -#  
68 -# get :callback  
69 -# assert_no_difference 'user.oauth_auths.count' do  
70 -# 3.times { get :callback }  
71 -# end  
72 -# end  
73 -#  
74 end 101 end
plugins/oauth_client/test/unit/auth_test.rb
  Diff comments   View file @ef5d596
@@ -19,7 +19,7 @@ class AuthTest &lt; ActiveSupport::TestCase @@ -19,7 +19,7 @@ class AuthTest &lt; ActiveSupport::TestCase
19 end 19 end
20 20
21 should "create an auth with an external person" do 21 should "create an auth with an external person" do
22 - auth = OauthClientPlugin::Auth.create!(external_person: @external_person, 22 + auth = OauthClientPlugin::Auth.create!(profile: @external_person,
23 provider: @provider) 23 provider: @provider)
24 assert auth.id.present? 24 assert auth.id.present?
25 end 25 end
@@ -40,19 +40,19 @@ class AuthTest &lt; ActiveSupport::TestCase @@ -40,19 +40,19 @@ class AuthTest &lt; ActiveSupport::TestCase
40 STRATEGIES.each do |strategy| 40 STRATEGIES.each do |strategy|
41 should "override the external person's image url for #{strategy} strategy" do 41 should "override the external person's image url for #{strategy} strategy" do
42 auth = OauthClientPlugin::Auth.create_for_strategy(strategy, provider: @provider, 42 auth = OauthClientPlugin::Auth.create_for_strategy(strategy, provider: @provider,
43 - external_person: @external_person) 43 + profile: @external_person)
44 assert_not auth.image_url.nil? 44 assert_not auth.image_url.nil?
45 end 45 end
46 46
47 should "override the external person's profile url for #{strategy} strategy" do 47 should "override the external person's profile url for #{strategy} strategy" do
48 auth = OauthClientPlugin::Auth.create_for_strategy(strategy, provider: @provider, 48 auth = OauthClientPlugin::Auth.create_for_strategy(strategy, provider: @provider,
49 - external_person: @external_person) 49 + profile: @external_person)
50 assert_not auth.profile_url.nil? 50 assert_not auth.profile_url.nil?
51 end 51 end
52 52
53 should "override the external person's profile settings url for #{strategy} strategy" do 53 should "override the external person's profile settings url for #{strategy} strategy" do
54 auth = OauthClientPlugin::Auth.create_for_strategy(strategy, provider: @provider, 54 auth = OauthClientPlugin::Auth.create_for_strategy(strategy, provider: @provider,
55 - external_person: @external_person) 55 + profile: @external_person)
56 assert_not auth.settings_url.nil? 56 assert_not auth.settings_url.nil?
57 end 57 end
58 end 58 end
plugins/oauth_client/views/auth/_oauth_login.html.erb
  Diff comments   View file @ef5d596
@@ -8,7 +8,7 @@ @@ -8,7 +8,7 @@
8 <% if OauthClientPlugin::Config.instance.allow_external_login %> 8 <% if OauthClientPlugin::Config.instance.allow_external_login %>
9 <%= _('Login with:') %> 9 <%= _('Login with:') %>
10 <div> 10 <div>
11 - <%= render :partial => 'auth/generate_providers_links', :locals => {:providers => providers, :action => "external_login"} %> 11 + <%= render :partial => 'auth/generate_providers_links', :locals => {:providers => providers, :action => "external_login"} %>
12 </div> 12 </div>
13 <% end %> 13 <% end %>
14 </div> 14 </div>