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 154  
155 155 # action to perform logout from the application
156 156 def logout
157   - if logged_in?
  157 + if logged_in? && self.current_user.id.present?
158 158 self.current_user.forget_me
159 159 end
160 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 3 skip_before_filter :login_required
4 4  
5 5 def callback
6   - auth = request.env["omniauth.auth"]
7   -
  6 + auth_data = request.env["omniauth.auth"]
8 7 oauth_params = request.env["omniauth.params"]
  8 +
9 9 if oauth_params && oauth_params["action"] == "external_login"
10   - external_person_login(auth)
  10 + external_person_login(auth_data)
11 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 14 end
15 15 end
16 16  
... ... @@ -26,48 +26,45 @@ class OauthClientPluginPublicController < PublicController
26 26  
27 27 protected
28 28  
29   - def external_person_login(auth)
  29 + def external_person_login(auth_data)
30 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 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 61 end
64 62  
65   - def login person
  63 + def create_session(user, oauth_auth)
66 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 68 else
72 69 session[:notice] = _("Can't login with %s") % provider.name
73 70 end
... ...
plugins/oauth_client/db/migrate/20160720165808_add_external_profile_to_oauth_auth.rb
  Diff comments   View file @ef5d596
1 1 class AddExternalProfileToOauthAuth < ActiveRecord::Migration
2 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 6 add_column :oauth_client_plugin_auths, :external_person_uid, :string
5 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 10 end
8 11  
9 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 16 remove_column :oauth_client_plugin_auths, :external_person_uid
13 17 remove_column :oauth_client_plugin_auths, :external_person_image_url
  18 +
  19 + change_column_default :oauth_client_plugin_auths, :enabled, nil
14 20 end
15 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 2  
3 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 6 has_one :oauth_provider, through: :oauth_auth, source: :provider
7 7  
8 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 2  
3 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 6 has_many :oauth_providers, through: :oauth_auths, source: :provider
7 7  
8 8 end
... ...
plugins/oauth_client/models/oauth_client_plugin/auth.rb
  Diff comments   View file @ef5d596
1 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 8 belongs_to :provider, class_name: 'OauthClientPlugin::Provider'
10 9  
11 10 validates_presence_of :provider
  11 + validates_presence_of :profile
12 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 14 acts_as_having_settings field: :data
18 15  
... ... @@ -30,18 +27,16 @@ class OauthClientPlugin::Auth &lt; ApplicationRecord
30 27 not self.expired?
31 28 end
32 29  
  30 + def allow_login?
  31 + self.enabled? && self.provider.enabled?
  32 + end
  33 +
33 34 def self.create_for_strategy(strategy, args = {})
34 35 namespace = self.name.split("::")[0]
35 36 class_name = "#{namespace}::#{strategy.camelize}Auth"
36 37 OauthClientPlugin::Auth.create!(args.merge(type: class_name))
37 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 40 IMAGE_SIZES = {
46 41 :big => "150",
47 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 6 end
7 7  
8 8 def profile_url
9   - "https://www.github.com/#{self.external_person.identifier}"
  9 + "https://www.github.com/#{self.profile.identifier}"
10 10 end
11 11  
12 12 def settings_url
... ...
plugins/oauth_client/models/oauth_client_plugin/noosfero_oauth2_auth.rb
  Diff comments   View file @ef5d596
1 1 class OauthClientPlugin::NoosferoOauth2Auth < OauthClientPlugin::Auth
2 2  
3 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 5 end
6 6  
7 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 9 end
10 10  
11 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 13 end
14 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 14 end
15 15  
16 16 def profile_url
17   - "https://twitter.com/#{self.external_person.identifier}"
  17 + "https://twitter.com/#{self.profile.identifier}"
18 18 end
19 19  
20 20 def settings_url
... ...
plugins/oauth_client/test/functional/account_controller_test.rb 0 → 100644
  Diff comments   View file @ef5d596
... ... @@ -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 25 assert_match /.*\/account\/signup/, @response.redirect_url
26 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 61 request.env["omniauth.params"] = {"action" => "external_login"}
30 62  
31 63 get :callback
... ... @@ -33,7 +65,15 @@ class OauthClientPluginPublicControllerTest &lt; ActionController::TestCase
33 65 assert session[:external].present?
34 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 77 request.env["omniauth.params"] = {"action" => "external_login"}
38 78 provider.update_attribute(:enabled, false)
39 79  
... ... @@ -42,7 +82,7 @@ class OauthClientPluginPublicControllerTest &lt; ActionController::TestCase
42 82 assert session[:external].nil?
43 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 86 request.env["omniauth.params"] = {"action" => "external_login"}
47 87 OauthClientPlugin::GithubAuth.any_instance.stubs(:enabled?).returns(false)
48 88  
... ... @@ -58,17 +98,4 @@ class OauthClientPluginPublicControllerTest &lt; ActionController::TestCase
58 98 external_person = ExternalPerson.find_by(identifier: auth.info.nickname)
59 99 assert_equal provider, external_person.oauth_auth.provider
60 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 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 19 end
20 20  
21 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 23 provider: @provider)
24 24 assert auth.id.present?
25 25 end
... ... @@ -40,19 +40,19 @@ class AuthTest &lt; ActiveSupport::TestCase
40 40 STRATEGIES.each do |strategy|
41 41 should "override the external person's image url for #{strategy} strategy" do
42 42 auth = OauthClientPlugin::Auth.create_for_strategy(strategy, provider: @provider,
43   - external_person: @external_person)
  43 + profile: @external_person)
44 44 assert_not auth.image_url.nil?
45 45 end
46 46  
47 47 should "override the external person's profile url for #{strategy} strategy" do
48 48 auth = OauthClientPlugin::Auth.create_for_strategy(strategy, provider: @provider,
49   - external_person: @external_person)
  49 + profile: @external_person)
50 50 assert_not auth.profile_url.nil?
51 51 end
52 52  
53 53 should "override the external person's profile settings url for #{strategy} strategy" do
54 54 auth = OauthClientPlugin::Auth.create_for_strategy(strategy, provider: @provider,
55   - external_person: @external_person)
  55 + profile: @external_person)
56 56 assert_not auth.settings_url.nil?
57 57 end
58 58 end
... ...
plugins/oauth_client/views/auth/_oauth_login.html.erb
  Diff comments   View file @ef5d596
... ... @@ -8,7 +8,7 @@
8 8 <% if OauthClientPlugin::Config.instance.allow_external_login %>
9 9 <%= _('Login with:') %>
10 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 12 </div>
13 13 <% end %>
14 14 </div>
... ...