diff --git a/app/controllers/public/account_controller.rb b/app/controllers/public/account_controller.rb index 20c4564..0f61266 100644 --- a/app/controllers/public/account_controller.rb +++ b/app/controllers/public/account_controller.rb @@ -154,7 +154,7 @@ class AccountController < ApplicationController # action to perform logout from the application def logout - if logged_in? + if logged_in? && self.current_user.id.present? self.current_user.forget_me end reset_session diff --git a/app/models/external_person.rb b/app/models/external_person.rb index 7e9a11e..7387e2a 100644 --- a/app/models/external_person.rb +++ b/app/models/external_person.rb @@ -122,6 +122,10 @@ class ExternalPerson < ActiveRecord::Base "#{jid(options)}/#{self.name}" end + def name + "#{self[:name]}@#{self.source}" + end + class ExternalPerson::Image def initialize(path) @path = path @@ -198,7 +202,10 @@ class ExternalPerson < ActiveRecord::Base relationships_cache_key: '', is_member_of?: false, follows?: false, each_friend: nil, is_last_admin?: false, is_last_admin_leaving?: false, leave: nil, last_notification: nil, notification_time: 0, notifier: nil, - remove_suggestion: nil, allow_invitation_from?: false + remove_suggestion: nil, allow_invitation_from?: false, + tracked_actions: ActionTracker::Record.none, follow: [], + update_profile_circles: ProfileFollower.none, unfollow: ProfileFollower.none, + remove_profile_from_circle: ProfileFollower.none, followed_profiles: Profile.none } derivated_methods = generate_derivated_methods(methods_and_responses) @@ -255,7 +262,8 @@ class ExternalPerson < ActiveRecord::Base {}, followed_by?: false, display_private_info_to?: true, can_view_field?: true, remove_from_suggestion_list: nil, layout_template: 'default', is_admin?: false, add_friend: false, follows?: false, is_a_friend?: false, - already_request_friendship?: false + already_request_friendship?: false, allow_followers: false, + in_social_circle: false, in_circle: false } derivated_methods = generate_derivated_methods(methods_and_responses) diff --git a/features/external_login.feature b/features/external_login.feature index 8236b10..a0ca9be 100644 --- a/features/external_login.feature +++ b/features/external_login.feature @@ -7,8 +7,8 @@ Feature: external login Scenario: login from portal homepage Given feature "allow_change_of_redirection_after_login" is disabled on environment And the following external environments - | identifier | name | url | - | test | Test | http://federated.noosfero.org | + | identifier | name | url | + | test | Test | http://federated.noosfero.org/ | And the following external users | login | | joaosilva@federated.noosfero.org | @@ -26,8 +26,8 @@ Feature: external login Scenario: not login from portal homepage Given feature "allow_change_of_redirection_after_login" is disabled on environment And the following external environments - | identifier | name | url | - | test | Test | http://federated.noosfero.org | + | identifier | name | url | + | test | Test | http://federated.noosfero.org/ | And I am not logged in And I go to the homepage And I follow "Login" diff --git a/plugins/oauth_client/controllers/oauth_client_plugin_admin_controller.rb b/plugins/oauth_client/controllers/oauth_client_plugin_admin_controller.rb index 3e4e370..b1394fe 100644 --- a/plugins/oauth_client/controllers/oauth_client_plugin_admin_controller.rb +++ b/plugins/oauth_client/controllers/oauth_client_plugin_admin_controller.rb @@ -1,6 +1,7 @@ class OauthClientPluginAdminController < AdminController def index + @config = OauthClientPlugin::Config.instance end def new @@ -13,6 +14,11 @@ class OauthClientPluginAdminController < AdminController redirect_to :action => 'index' end + def update_configs + OauthClientPlugin::Config.instance.update_attributes(params[:oauth_client_config]) + redirect_to :action => 'index' + end + def edit @provider = params[:id] ? environment.oauth_providers.find(params[:id]) : environment.oauth_providers.new if request.post? @@ -24,4 +30,8 @@ class OauthClientPluginAdminController < AdminController end end + def edit_login_option + option = params['oauth_client_plugin_option'] + end + end diff --git a/plugins/oauth_client/controllers/public/oauth_client_plugin_public_controller.rb b/plugins/oauth_client/controllers/public/oauth_client_plugin_public_controller.rb index 16bf9bc..35a3837 100644 --- a/plugins/oauth_client/controllers/public/oauth_client_plugin_public_controller.rb +++ b/plugins/oauth_client/controllers/public/oauth_client_plugin_public_controller.rb @@ -3,9 +3,15 @@ class OauthClientPluginPublicController < PublicController skip_before_filter :login_required def callback - auth = request.env["omniauth.auth"] - auth_user = environment.users.where(email: auth.info.email).first - if auth_user then login auth_user.person else signup auth end + auth_data = request.env["omniauth.auth"] + oauth_params = request.env["omniauth.params"] + + if oauth_params && oauth_params["action"] == "external_login" + external_person_login(auth_data) + else + auth_user = environment.users.where(email: auth_data.info.email).first + if auth_user then login(auth_user.person) else signup(auth_data) end + end end def failure @@ -20,12 +26,45 @@ class OauthClientPluginPublicController < PublicController protected - def login person + def external_person_login(auth_data) provider = OauthClientPlugin::Provider.find(session[:provider_id]) - auth = person.oauth_auths.where(provider_id: provider.id).first - auth ||= person.oauth_auths.create! profile: person, provider: provider, enabled: true - if auth.enabled? && provider.enabled? - self.current_user = person.user + + user = User.new(email: auth_data.info.email, login: auth_data.info.name.to_slug) + person = OauthClientPlugin::OauthExternalPerson.find_or_create_by( + identifier: auth_data.info.nickname || user.login, + name: auth_data.info.name, + created_at: Time.now, + source: provider.site || auth_data.provider, + email: user.email + ) + user.external_person_id = person.id + + oauth_auth = person.oauth_auth + oauth_data = { profile: person, provider: provider, enabled: true, + external_person_uid: auth_data.uid, external_person_image_url: auth_data.info.image } + oauth_auth ||= OauthClientPlugin::Auth.create_for_strategy(provider.strategy, oauth_data) + create_session(user, oauth_auth) + end + + def signup(auth_data) + session[:oauth_data] = auth_data + username = auth_data.info.email.split('@').first + name = auth_data.info.name + name ||= auth_data.extra && auth_data.extra.raw_info ? auth_data.extra.raw_info.name : '' + redirect_to :controller => :account, :action => :signup, :user => {:login => username, :email => auth_data.info.email}, :profile_data => {:name => name} + end + + def login(person) + auth = person.oauth_auths.find_or_create_by(profile: person, + provider_id: session[:provider_id]) + create_session(person.user, auth) + end + + def create_session(user, oauth_auth) + provider = OauthClientPlugin::Provider.find(session[:provider_id]) + + if oauth_auth.allow_login? + self.current_user = user else session[:notice] = _("Can't login with %s") % provider.name end @@ -33,12 +72,4 @@ class OauthClientPluginPublicController < PublicController redirect_to :controller => :account, :action => :login end - def signup(auth) - login = auth.info.email.split('@').first - session[:oauth_data] = auth - name = auth.info.name - name ||= auth.extra && auth.extra.raw_info ? auth.extra.raw_info.name : '' - redirect_to :controller => :account, :action => :signup, :user => {:login => login, :email => auth.info.email}, :profile_data => {:name => name} - end - end diff --git a/plugins/oauth_client/db/migrate/20160714113820_create_oauth_client_plugin_config.rb b/plugins/oauth_client/db/migrate/20160714113820_create_oauth_client_plugin_config.rb new file mode 100644 index 0000000..f321a05 --- /dev/null +++ b/plugins/oauth_client/db/migrate/20160714113820_create_oauth_client_plugin_config.rb @@ -0,0 +1,9 @@ +class CreateOauthClientPluginConfig < ActiveRecord::Migration + + def change + create_table :oauth_client_plugin_configs do |t| + t.belongs_to :environment + t.boolean :allow_external_login, :default => false + end + end +end diff --git a/plugins/oauth_client/db/migrate/20160720165808_add_external_profile_to_oauth_auth.rb b/plugins/oauth_client/db/migrate/20160720165808_add_external_profile_to_oauth_auth.rb new file mode 100644 index 0000000..72d21b5 --- /dev/null +++ b/plugins/oauth_client/db/migrate/20160720165808_add_external_profile_to_oauth_auth.rb @@ -0,0 +1,21 @@ +class AddExternalProfileToOauthAuth < ActiveRecord::Migration + def up + add_column :oauth_client_plugin_auths, :profile_type, :string + add_index :oauth_client_plugin_auths, :profile_type + + add_column :oauth_client_plugin_auths, :external_person_uid, :string + add_column :oauth_client_plugin_auths, :external_person_image_url, :string + + change_column_default :oauth_client_plugin_auths, :enabled, true + end + + def down + remove_index :oauth_client_plugin_auths, :profile_type + remove_column :oauth_client_plugin_auths, :profile_type + + remove_column :oauth_client_plugin_auths, :external_person_uid + remove_column :oauth_client_plugin_auths, :external_person_image_url + + change_column_default :oauth_client_plugin_auths, :enabled, nil + end +end diff --git a/plugins/oauth_client/db/migrate/20160809181708_adds_type_to_external_person.rb b/plugins/oauth_client/db/migrate/20160809181708_adds_type_to_external_person.rb new file mode 100644 index 0000000..402c1b6 --- /dev/null +++ b/plugins/oauth_client/db/migrate/20160809181708_adds_type_to_external_person.rb @@ -0,0 +1,9 @@ +class AddsTypeToExternalPerson < ActiveRecord::Migration + def up + add_column :external_people, :type, :string + end + + def down + remove_column :external_people, :type + end +end diff --git a/plugins/oauth_client/lib/ext/environment.rb b/plugins/oauth_client/lib/ext/environment.rb index 725221e..995809a 100644 --- a/plugins/oauth_client/lib/ext/environment.rb +++ b/plugins/oauth_client/lib/ext/environment.rb @@ -1,7 +1,6 @@ require_dependency 'environment' class Environment - + has_one :oauth_client_plugin_configs, :class_name => 'OauthClientPlugin::Config' has_many :oauth_providers, :class_name => 'OauthClientPlugin::Provider' - end diff --git a/plugins/oauth_client/lib/ext/profile.rb b/plugins/oauth_client/lib/ext/profile.rb index f0c1c65..d5eb948 100644 --- a/plugins/oauth_client/lib/ext/profile.rb +++ b/plugins/oauth_client/lib/ext/profile.rb @@ -2,7 +2,7 @@ require_dependency 'profile' class Profile - has_many :oauth_auths, foreign_key: :profile_id, class_name: 'OauthClientPlugin::Auth', dependent: :destroy + has_many :oauth_auths, as: :profile, class_name: 'OauthClientPlugin::Auth', dependent: :destroy has_many :oauth_providers, through: :oauth_auths, source: :provider end diff --git a/plugins/oauth_client/lib/oauth_client_plugin.rb b/plugins/oauth_client/lib/oauth_client_plugin.rb index eb7dff0..980cff2 100644 --- a/plugins/oauth_client/lib/oauth_client_plugin.rb +++ b/plugins/oauth_client/lib/oauth_client_plugin.rb @@ -90,6 +90,7 @@ class OauthClientPlugin < Noosfero::Plugin def account_controller_filters { :type => 'before_filter', :method_name => 'signup', + :options => { :only => 'signup' }, :block => proc { auth = session[:oauth_data] @@ -104,7 +105,7 @@ class OauthClientPlugin < Noosfero::Plugin end def js_files - ["script.js"] + ["script.js", "provider.js"] end end diff --git a/plugins/oauth_client/models/oauth_client_plugin/auth.rb b/plugins/oauth_client/models/oauth_client_plugin/auth.rb index 3260909..f332527 100644 --- a/plugins/oauth_client/models/oauth_client_plugin/auth.rb +++ b/plugins/oauth_client/models/oauth_client_plugin/auth.rb @@ -1,13 +1,14 @@ class OauthClientPlugin::Auth < ApplicationRecord - attr_accessible :profile, :provider, :enabled, - :access_token, :expires_in + attr_accessible :profile, :provider, :provider_id, :enabled, + :access_token, :expires_in, :type, :external_person_uid, + :external_person_image_url - belongs_to :profile, class_name: 'Profile' + belongs_to :profile, polymorphic: true belongs_to :provider, class_name: 'OauthClientPlugin::Provider' - validates_presence_of :profile validates_presence_of :provider + validates_presence_of :profile validates_uniqueness_of :profile_id, scope: :provider_id extend ActsAsHavingSettings::ClassMethods @@ -27,4 +28,34 @@ class OauthClientPlugin::Auth < ApplicationRecord not self.expired? end + def allow_login? + self.enabled? && self.provider.enabled? + end + + def self.create_for_strategy(strategy, args = {}) + namespace = self.name.split("::")[0] + class_name = "#{namespace}::#{strategy.camelize}Auth" + OauthClientPlugin::Auth.create!(args.merge(type: class_name)) + end + + IMAGE_SIZES = { + :big => "150", + :thumb => "100", + :portrait => "64", + :minor => "50", + :icon => "18" + } + + # The following methods should be implemented by + # the Provider specific Auth classes + def image_url(size = nil) + nil + end + def profile_url + nil + end + def settings_url + nil + end + end diff --git a/plugins/oauth_client/models/oauth_client_plugin/config.rb b/plugins/oauth_client/models/oauth_client_plugin/config.rb new file mode 100644 index 0000000..cb11742 --- /dev/null +++ b/plugins/oauth_client/models/oauth_client_plugin/config.rb @@ -0,0 +1,15 @@ +class OauthClientPlugin::Config < ApplicationRecord + + belongs_to :environment + attr_accessible :allow_external_login, :environment_id + + class << self + def instance + environment = Environment.default + environment.oauth_client_plugin_configs || create(environment_id: environment.id) + end + + private :new + end + +end diff --git a/plugins/oauth_client/models/oauth_client_plugin/facebook_auth.rb b/plugins/oauth_client/models/oauth_client_plugin/facebook_auth.rb new file mode 100644 index 0000000..24314ae --- /dev/null +++ b/plugins/oauth_client/models/oauth_client_plugin/facebook_auth.rb @@ -0,0 +1,16 @@ +class OauthClientPlugin::FacebookAuth < OauthClientPlugin::Auth + + def image_url(size = nil) + size = IMAGE_SIZES[size] || IMAGE_SIZES[:icon] + "#{self.external_person_image_url}?width=#{size}" + end + + def profile_url + "https://www.facebook.com/#{self.external_person_uid}" + end + + def settings_url + "https://www.facebook.com/settings" + end + +end diff --git a/plugins/oauth_client/models/oauth_client_plugin/github_auth.rb b/plugins/oauth_client/models/oauth_client_plugin/github_auth.rb new file mode 100644 index 0000000..758d223 --- /dev/null +++ b/plugins/oauth_client/models/oauth_client_plugin/github_auth.rb @@ -0,0 +1,15 @@ +class OauthClientPlugin::GithubAuth < OauthClientPlugin::Auth + + def image_url(size = nil) + size = IMAGE_SIZES[size] || IMAGE_SIZES[:icon] + "#{self.external_person_image_url}&size=#{size}" + end + + def profile_url + "https://www.github.com/#{self.profile.identifier}" + end + + def settings_url + "https://www.github.com/settings" + end +end diff --git a/plugins/oauth_client/models/oauth_client_plugin/google_oauth2_auth.rb b/plugins/oauth_client/models/oauth_client_plugin/google_oauth2_auth.rb new file mode 100644 index 0000000..e024495 --- /dev/null +++ b/plugins/oauth_client/models/oauth_client_plugin/google_oauth2_auth.rb @@ -0,0 +1,15 @@ +class OauthClientPlugin::GoogleOauth2Auth < OauthClientPlugin::Auth + + def image_url(size = nil) + size = IMAGE_SIZES[size] || IMAGE_SIZES[:icon] + "#{self.external_person_image_url}?sz=#{size}" + end + + def profile_url + "https://plus.google.com/#{self.external_person_uid}" + end + + def settings_url + "https://plus.google.com/u/0/settings" + end +end diff --git a/plugins/oauth_client/models/oauth_client_plugin/noosfero_oauth2_auth.rb b/plugins/oauth_client/models/oauth_client_plugin/noosfero_oauth2_auth.rb new file mode 100644 index 0000000..dddf676 --- /dev/null +++ b/plugins/oauth_client/models/oauth_client_plugin/noosfero_oauth2_auth.rb @@ -0,0 +1,14 @@ +class OauthClientPlugin::NoosferoOauth2Auth < OauthClientPlugin::Auth + + def image_url(size = "") + URI.join("http://#{self.provider.client_options[:site]}/profile/#{self.profile.identifier}/icon/", size) + end + + def profile_url + "http://#{self.profile.source}/profile/#{self.profile.identifier}" + end + + def settings_url + "http://#{self.profile.source}/myprofile/#{self.profile.identifier}" + end +end diff --git a/plugins/oauth_client/models/oauth_client_plugin/oauth_external_person.rb b/plugins/oauth_client/models/oauth_client_plugin/oauth_external_person.rb new file mode 100644 index 0000000..5e885e3 --- /dev/null +++ b/plugins/oauth_client/models/oauth_client_plugin/oauth_external_person.rb @@ -0,0 +1,35 @@ +class OauthClientPlugin::OauthExternalPerson < ExternalPerson + + has_one :oauth_auth, as: :profile, class_name: 'OauthClientPlugin::Auth', dependent: :destroy + has_one :oauth_provider, through: :oauth_auth, source: :provider + + def avatar + self.oauth_auth.image_url + end + + def image + OauthClientPlugin::OauthExternalPerson::Image.new(self.oauth_auth) + end + + def public_profile_url + self.oauth_auth.profile_url + end + + def url + self.oauth_auth.profile_url + end + + def admin_url + self.oauth_auth.settings_url + end + + class OauthClientPlugin::OauthExternalPerson::Image < ExternalPerson::Image + def initialize(oauth_auth) + @oauth_auth = oauth_auth + end + + def public_filename(size = nil) + URI(@oauth_auth.image_url(size)) + end + end +end diff --git a/plugins/oauth_client/models/oauth_client_plugin/provider.rb b/plugins/oauth_client/models/oauth_client_plugin/provider.rb index 38f7256..5372bf0 100644 --- a/plugins/oauth_client/models/oauth_client_plugin/provider.rb +++ b/plugins/oauth_client/models/oauth_client_plugin/provider.rb @@ -4,6 +4,8 @@ class OauthClientPlugin::Provider < ApplicationRecord validates_presence_of :name, :strategy + validate :noosfero_provider_must_have_a_site + extend ActsAsHavingImage::ClassMethods acts_as_having_image @@ -19,4 +21,9 @@ class OauthClientPlugin::Provider < ApplicationRecord scope :enabled, -> { where enabled: true } + def noosfero_provider_must_have_a_site + if self.strategy == 'noosfero_oauth2' && (self.client_options.nil? || self.client_options[:site].blank?) + self.errors.add(:site, "A Noosfero provider must have a site") + end + end end diff --git a/plugins/oauth_client/models/oauth_client_plugin/twitter_auth.rb b/plugins/oauth_client/models/oauth_client_plugin/twitter_auth.rb new file mode 100644 index 0000000..14d08fa --- /dev/null +++ b/plugins/oauth_client/models/oauth_client_plugin/twitter_auth.rb @@ -0,0 +1,23 @@ +class OauthClientPlugin::TwitterAuth < OauthClientPlugin::Auth + + IMAGE_SIZES = { + :big => "", + :thumb => "_bigger", + :portrait => "_normal", + :minor => "_normal", + :icon => "_mini" + } + + def image_url(size = nil) + size = IMAGE_SIZES[size] || IMAGE_SIZES[:icon] + self.external_person_image_url.gsub("_normal", size) + end + + def profile_url + "https://twitter.com/#{self.profile.identifier}" + end + + def settings_url + "https://twitter.com/settings" + end +end diff --git a/plugins/oauth_client/public/provider.js b/plugins/oauth_client/public/provider.js new file mode 100644 index 0000000..1a6cacd --- /dev/null +++ b/plugins/oauth_client/public/provider.js @@ -0,0 +1,12 @@ +function toggle_strategy(strategyName) { + if (strategyName == "noosfero_oauth2") { + $(".client-url").addClass("required-field"); + } else { + $(".client-url").removeClass("required-field"); + } +} + +$(document).on("change", "select#oauth_client_plugin_provider_strategy", function() { + var selectedOption = $(this).find(":selected").text(); + toggle_strategy(selectedOption); +}); diff --git a/plugins/oauth_client/test/functional/account_controller_test.rb b/plugins/oauth_client/test/functional/account_controller_test.rb new file mode 100644 index 0000000..0ff02f4 --- /dev/null +++ b/plugins/oauth_client/test/functional/account_controller_test.rb @@ -0,0 +1,19 @@ +require 'test_helper' + +class AccountControllerTest < ActionController::TestCase + def setup + external_person = ExternalPerson.create!(identifier: 'johnlock', + name: 'John Locke', + source: 'anerenvironment.org', + email: 'john@locke.org', + created_at: Date.yesterday + ) + session[:external] = external_person.id + end + + should "not create an User when logging out" do + assert_no_difference 'User.count' do + get :logout + end + end +end diff --git a/plugins/oauth_client/test/functional/oauth_client_plugin_public_controller_test.rb b/plugins/oauth_client/test/functional/oauth_client_plugin_public_controller_test.rb index d85b2b8..c339b09 100644 --- a/plugins/oauth_client/test/functional/oauth_client_plugin_public_controller_test.rb +++ b/plugins/oauth_client/test/functional/oauth_client_plugin_public_controller_test.rb @@ -5,76 +5,97 @@ class OauthClientPluginPublicControllerTest < ActionController::TestCase def setup @auth = mock @auth.stubs(:info).returns(mock) + @auth.info.stubs(:email).returns("user@email.com") + @auth.info.stubs(:name).returns("User") + @auth.info.stubs(:nickname).returns("user") + @auth.info.stubs(:image).returns("url.to.image.com") + @auth.stubs(:provider).returns("testprovider") + @auth.stubs(:uid).returns("jh12j3h12kjh312") + request.env["omniauth.auth"] = @auth @environment = Environment.default - @provider = OauthClientPlugin::Provider.create!(:name => 'provider', :strategy => 'provider', :enabled => true) + @provider = OauthClientPlugin::Provider.create!(:name => 'provider', :strategy => 'github', :enabled => true) + + session[:provider_id] = provider.id end attr_reader :auth, :environment, :provider should 'redirect to signup when user is not found' do - auth.info.stubs(:email).returns("xyz123@noosfero.org") - auth.info.stubs(:name).returns('xyz123') - session[:provider_id] = provider.id - get :callback assert_match /.*\/account\/signup/, @response.redirect_url end - should 'redirect to login when user is found' do - user = create_user - auth.info.stubs(:email).returns(user.email) - auth.info.stubs(:name).returns(user.name) - session[:provider_id] = provider.id + should 'login when user already signed up' do + create_user(@auth.info.name, email: @auth.info.email) get :callback - assert_redirected_to :controller => :account, :action => :login - assert_equal user.id, session[:user] + assert session[:user].present? end - should 'do not login when the provider is disabled' do - user = create_user - auth.info.stubs(:email).returns(user.email) - auth.info.stubs(:name).returns(user.name) - session[:provider_id] = provider.id + should 'not login when user already signed up and the provider is disabled' do + create_user(@auth.info.name, email: @auth.info.email) provider.update_attribute(:enabled, false) get :callback - assert_redirected_to :controller => :account, :action => :login - assert_equal nil, session[:user] + assert session[:user].nil? end - should 'do not login when the provider is disabled for a user' do - user = create_user - auth.info.stubs(:email).returns(user.email) - auth.info.stubs(:name).returns(user.name) - session[:provider_id] = provider.id - user.person.oauth_auths.create!(profile: user.person, provider: provider, enabled: false) + should 'not login when user already signed up and the provider is disabled for him' do + create_user(@auth.info.name, email: @auth.info.email) + OauthClientPlugin::Auth.any_instance.stubs(:enabled?).returns(false) get :callback - assert_redirected_to :controller => :account, :action => :login - assert_equal nil, session[:user] + assert session[:user].nil? end - should 'save provider when an user login with it' do - user = create_user - auth.info.stubs(:email).returns(user.email) - auth.info.stubs(:name).returns(user.name) - session[:provider_id] = provider.id + should 'not duplicate oauth_auths when the same provider is used several times' do + user = create_user(@auth.info.name, email: @auth.info.email) get :callback - assert_equal [provider], user.oauth_providers + assert_no_difference 'user.oauth_auths.count' do + 3.times { get :callback } + end end - should 'do not duplicate relations between an user and a provider when the same provider was used again in a login' do - user = create_user - auth.info.stubs(:email).returns(user.email) - auth.info.stubs(:name).returns(user.name) - session[:provider_id] = provider.id + should 'perform external login using provider when url param is present' do + request.env["omniauth.params"] = {"action" => "external_login"} get :callback - assert_no_difference 'user.oauth_auths.count' do - 3.times { get :callback } + assert_redirected_to :controller => :account, :action => :login + assert session[:external].present? + end + + should 'not create an user when performing external login' do + request.env["omniauth.params"] = {"action" => "external_login"} + + assert_no_difference 'User.count' do + get :callback end end + should 'not perform external login when the provider is disabled' do + request.env["omniauth.params"] = {"action" => "external_login"} + provider.update_attribute(:enabled, false) + + get :callback + assert_redirected_to :controller => :account, :action => :login + assert session[:external].nil? + end + + should 'not perform external login when the provider is disabled for a user' do + request.env["omniauth.params"] = {"action" => "external_login"} + OauthClientPlugin::GithubAuth.any_instance.stubs(:enabled?).returns(false) + + get :callback + assert_redirected_to :controller => :account, :action => :login + assert session[:external].nil? + end + + should 'save provider when an external person logs in with it' do + request.env["omniauth.params"] = {"action" => "external_login"} + + get :callback + external_person = OauthClientPlugin::OauthExternalPerson.find_by(identifier: auth.info.nickname) + assert_equal provider, external_person.oauth_auth.provider + end end diff --git a/plugins/oauth_client/test/unit/auth_test.rb b/plugins/oauth_client/test/unit/auth_test.rb new file mode 100644 index 0000000..5e71608 --- /dev/null +++ b/plugins/oauth_client/test/unit/auth_test.rb @@ -0,0 +1,60 @@ +require 'test_helper' + +class AuthTest < ActiveSupport::TestCase + + def setup + @person = fast_create(Person) + @provider = fast_create(OauthClientPlugin::Provider, name: "GitHub") + @external_person = fast_create(ExternalPerson, name: "testuser", email: "test@email.com", + identifier: "testuser") + + OauthClientPlugin::Auth.any_instance.stubs(:external_person_image_url).returns("http://some.host/image") + OauthClientPlugin::Auth.any_instance.stubs(:external_person_uid).returns("j4b25cj234hb5n235") + OauthClientPlugin::Provider.any_instance.stubs(:client_options).returns({site: "http://host.com"}) + end + + should "not create an auth without a related profile or external person" do + auth = OauthClientPlugin::Auth.new(provider: @provider) + assert_not auth.valid? + end + + should "create an auth with an external person" do + auth = OauthClientPlugin::Auth.create!(profile: @external_person, + provider: @provider) + assert auth.id.present? + end + + should "create an auth with a profile" do + auth = OauthClientPlugin::Auth.create!(profile: @person, provider: @provider) + assert auth.id.present? + end + + should "create an auth for a custom provider" do + auth = OauthClientPlugin::Auth.create_for_strategy("github", provider: @provider, + profile: @person) + assert auth.id.present? + assert auth.is_a? OauthClientPlugin::GithubAuth + end + + STRATEGIES = %w[facebook github google_oauth2 noosfero_oauth2 twitter] + STRATEGIES.each do |strategy| + should "override the external person's image url for #{strategy} strategy" do + auth = OauthClientPlugin::Auth.create_for_strategy(strategy, provider: @provider, + profile: @external_person) + assert_not auth.image_url.nil? + end + + should "override the external person's profile url for #{strategy} strategy" do + auth = OauthClientPlugin::Auth.create_for_strategy(strategy, provider: @provider, + profile: @external_person) + assert_not auth.profile_url.nil? + end + + should "override the external person's profile settings url for #{strategy} strategy" do + auth = OauthClientPlugin::Auth.create_for_strategy(strategy, provider: @provider, + profile: @external_person) + assert_not auth.settings_url.nil? + end + end + +end diff --git a/plugins/oauth_client/test/unit/oauth_external_person_test.rb b/plugins/oauth_client/test/unit/oauth_external_person_test.rb new file mode 100644 index 0000000..2b32b0e --- /dev/null +++ b/plugins/oauth_client/test/unit/oauth_external_person_test.rb @@ -0,0 +1,31 @@ + +require 'test_helper' + +class OauthExternalPersonTest < ActiveSupport::TestCase + + def setup + provider = fast_create(OauthClientPlugin::Provider, name: "GitHub") + @external_person = fast_create(ExternalPerson, name: "testuser", email: "test@email.com", + identifier: "testuser") + OauthClientPlugin::GithubAuth.create!(profile: @external_person, provider: provider) + + @oauth_external_person = fast_create(OauthClientPlugin::OauthExternalPerson, + name: "testuser", email: "test@email.com", + identifier: "testuser") + OauthClientPlugin::GithubAuth.create!(profile: @oauth_external_person, + provider: provider) + end + + should "not orverride info from a regular external person" do + assert_not_equal @external_person.avatar, @oauth_external_person.avatar + assert_not_equal @external_person.url, @oauth_external_person.url + assert_not_equal @external_person.admin_url, @oauth_external_person.admin_url + assert_not_equal @external_person.public_profile_url, + @oauth_external_person.public_profile_url + end + + should "not override the Image class from a regular external person" do + assert @external_person.image.is_a? ExternalPerson::Image + assert @oauth_external_person.image.is_a? OauthClientPlugin::OauthExternalPerson::Image + end +end diff --git a/plugins/oauth_client/test/unit/provider_test.rb b/plugins/oauth_client/test/unit/provider_test.rb new file mode 100644 index 0000000..c2948fc --- /dev/null +++ b/plugins/oauth_client/test/unit/provider_test.rb @@ -0,0 +1,17 @@ +require 'test_helper' + +class ProviderTest < ActiveSupport::TestCase + + should "only create a noosfero provider with a site" do + provider = OauthClientPlugin::Provider.new(:name => 'noosfero', :strategy => 'noosfero_oauth2') + assert_not provider.valid? + + provider.client_options = { :site => "http://noosfero.org" } + assert provider.valid? + end + + should "create a regular provider without a site" do + provider = OauthClientPlugin::Provider.new(:name => 'github', :strategy => 'github') + assert provider.valid? + end +end diff --git a/plugins/oauth_client/views/account/_oauth_signup.html.erb b/plugins/oauth_client/views/account/_oauth_signup.html.erb index 346c0dd..2365bcb 100644 --- a/plugins/oauth_client/views/account/_oauth_signup.html.erb +++ b/plugins/oauth_client/views/account/_oauth_signup.html.erb @@ -1,7 +1,7 @@ <%= hidden_field_tag 'return_to', '/' %> diff --git a/plugins/oauth_client/views/auth/_generate_providers_links.html.erb b/plugins/oauth_client/views/auth/_generate_providers_links.html.erb new file mode 100644 index 0000000..015e427 --- /dev/null +++ b/plugins/oauth_client/views/auth/_generate_providers_links.html.erb @@ -0,0 +1,5 @@ +<% providers.each do |provider| %> + + <%= link_to provider.image ? image_tag(provider.image.public_filename) : provider.name, "/plugin/oauth_client/#{provider.strategy}?id=#{provider.id}&action=#{action}", :class => provider.strategy, :title => provider.name %> + +<% end %> diff --git a/plugins/oauth_client/views/auth/_oauth_login.html.erb b/plugins/oauth_client/views/auth/_oauth_login.html.erb index 209b21a..f57fa74 100644 --- a/plugins/oauth_client/views/auth/_oauth_login.html.erb +++ b/plugins/oauth_client/views/auth/_oauth_login.html.erb @@ -1,11 +1,17 @@
<% unless providers.empty? %> - <%= _('Login with:') %> - <% end %> - <% providers.each do |provider| %> - - <%= link_to provider.image ? image_tag(provider.image.public_filename) : provider.name, "/plugin/oauth_client/#{provider.strategy}?id=#{provider.id}", :class => provider.strategy, :title => provider.name %> - + <% end %> diff --git a/plugins/oauth_client/views/oauth_client_plugin_admin/_noosfero_oauth2.html.erb b/plugins/oauth_client/views/oauth_client_plugin_admin/_noosfero_oauth2.html.erb deleted file mode 100644 index 6928d7a..0000000 --- a/plugins/oauth_client/views/oauth_client_plugin_admin/_noosfero_oauth2.html.erb +++ /dev/null @@ -1,5 +0,0 @@ -<%= f.fields_for :client_options, OpenStruct.new(provider.options[:client_options]) do |c| %> -
- <%= labelled_form_field _('Client Url'), c.text_field(:site) %> -
-<% end %> diff --git a/plugins/oauth_client/views/oauth_client_plugin_admin/edit.html.erb b/plugins/oauth_client/views/oauth_client_plugin_admin/edit.html.erb index a028185..0b18e0c 100644 --- a/plugins/oauth_client/views/oauth_client_plugin_admin/edit.html.erb +++ b/plugins/oauth_client/views/oauth_client_plugin_admin/edit.html.erb @@ -8,7 +8,7 @@
- <%= labelled_form_field _('Name'), f.text_field(:name) %> + <%= required labelled_form_field _('Name'), f.text_field(:name) %>
@@ -27,8 +27,10 @@ <%= labelled_form_field _('Client Secret'), f.text_field(:client_secret) %>
- <% if File.exists?(File.join(File.dirname(__FILE__), "_#{@provider.strategy}.html.erb")) %> - <%= render :partial => "#{@provider.strategy}", :locals => {:f => f, :provider => @provider} %> + <%= f.fields_for :client_options, OpenStruct.new(@provider.options[:client_options]) do |c| %> +
" title="<%= _("The service URL. If this value is informed, it will be used to identify the users, replacing the provider name") %>"> + <%= labelled_form_field _('Client Url'), c.text_field(:site) %> +
<% end %>
diff --git a/plugins/oauth_client/views/oauth_client_plugin_admin/index.html.erb b/plugins/oauth_client/views/oauth_client_plugin_admin/index.html.erb index 98b3ce1..98d69a9 100644 --- a/plugins/oauth_client/views/oauth_client_plugin_admin/index.html.erb +++ b/plugins/oauth_client/views/oauth_client_plugin_admin/index.html.erb @@ -1,4 +1,22 @@

<%= _('Oauth Client Settings') %>

+ +<%= labelled_form_for @config, url: { action: "update_configs" } do |f| %> + + + <%= hidden_field_tag "oauth_client_config[allow_external_login]", false %> + + + +
<%= _('Allow external login') %><%= check_box_tag "oauth_client_config[allow_external_login]", true, @config.allow_external_login %>
+ +
+ <%= button_bar do %> + <%= submit_button('save', _('Save changes')) %> + <%= button :back, _('Back to plugins panel'), :controller => 'plugins', :action => 'index' %> + <% end %> +
+<% end %> +

<%= _('Providers') %>

<%= button :add, _('New'), {:action => 'new'} %> diff --git a/test/unit/external_person_test.rb b/test/unit/external_person_test.rb index b181b59..9601c59 100644 --- a/test/unit/external_person_test.rb +++ b/test/unit/external_person_test.rb @@ -31,7 +31,9 @@ class ExternalPersonTest < ActiveSupport::TestCase should 'not be a member of any communities' do community = fast_create(Community) - refute community.add_member(@external_person) + assert_raise do + community.add_member(@external_person) + end assert_equivalent [], @external_person.memberships end -- libgit2 0.21.2