Commit e7ae25458160e1bbeda3b0070c67fa16af1bb189
1 parent
c5776ba3
Exists in
fix_gallery_image_url
and in
3 other branches
oauth_client: deserialize fields and improve authentication
Showing
14 changed files
with
136 additions
and
66 deletions
Show diff stats
plugins/oauth_client/controllers/public/oauth_client_plugin_public_controller.rb
@@ -4,8 +4,8 @@ class OauthClientPluginPublicController < PublicController | @@ -4,8 +4,8 @@ class OauthClientPluginPublicController < PublicController | ||
4 | 4 | ||
5 | def callback | 5 | def callback |
6 | auth = request.env["omniauth.auth"] | 6 | auth = request.env["omniauth.auth"] |
7 | - user = environment.users.find_by_email(auth.info.email) | ||
8 | - user ? login(user) : signup(auth) | 7 | + auth_user = environment.users.where(email: auth.info.email).first |
8 | + if auth_user then login auth_user.person else signup auth end | ||
9 | end | 9 | end |
10 | 10 | ||
11 | def failure | 11 | def failure |
@@ -20,16 +20,14 @@ class OauthClientPluginPublicController < PublicController | @@ -20,16 +20,14 @@ class OauthClientPluginPublicController < PublicController | ||
20 | 20 | ||
21 | protected | 21 | protected |
22 | 22 | ||
23 | - def login(user) | 23 | + def login person |
24 | provider = OauthClientPlugin::Provider.find(session[:provider_id]) | 24 | provider = OauthClientPlugin::Provider.find(session[:provider_id]) |
25 | - user_provider = user.oauth_user_providers.find_by_provider_id(provider.id) | ||
26 | - unless user_provider | ||
27 | - user_provider = user.oauth_user_providers.create(:user => user, :provider => provider, :enabled => true) | ||
28 | - end | ||
29 | - if user_provider.enabled? && provider.enabled? | ||
30 | - session[:user] = user.id | 25 | + auth = person.oauth_auths.where(provider_id: provider.id).first |
26 | + auth ||= person.oauth_auths.create! profile: person, provider: provider, enabled: true | ||
27 | + if auth.enabled? && provider.enabled? | ||
28 | + self.current_user = person.user | ||
31 | else | 29 | else |
32 | - session[:notice] = _("Can't login with %s") % provider.name | 30 | + session[:notice] = _("Can't login with #{provider.name}") |
33 | end | 31 | end |
34 | 32 | ||
35 | redirect_to :controller => :account, :action => :login | 33 | redirect_to :controller => :account, :action => :login |
plugins/oauth_client/db/migrate/20150815170000_deserialize_fields_on_oauth_client_plugin_provider.rb
0 → 100644
@@ -0,0 +1,20 @@ | @@ -0,0 +1,20 @@ | ||
1 | +class DeserializeFieldsOnOauthClientPluginProvider < ActiveRecord::Migration | ||
2 | + | ||
3 | + def up | ||
4 | + add_column :oauth_client_plugin_providers, :client_id, :text | ||
5 | + add_column :oauth_client_plugin_providers, :client_secret, :text | ||
6 | + | ||
7 | + OauthClientPlugin::Provider.find_each batch_size: 50 do |provider| | ||
8 | + provider.client_id = provider.options.delete :client_id | ||
9 | + provider.client_secret = provider.options.delete :client_secret | ||
10 | + provider.save! | ||
11 | + end | ||
12 | + | ||
13 | + add_index :oauth_client_plugin_providers, :client_id | ||
14 | + end | ||
15 | + | ||
16 | + def down | ||
17 | + say "this migration can't be reverted" | ||
18 | + end | ||
19 | + | ||
20 | +end |
plugins/oauth_client/db/migrate/20150815173209_add_authorization_data_to_oauth_client_user_provider.rb
0 → 100644
@@ -0,0 +1,26 @@ | @@ -0,0 +1,26 @@ | ||
1 | +class AddAuthorizationDataToOauthClientUserProvider < ActiveRecord::Migration | ||
2 | + | ||
3 | + def change | ||
4 | + rename_table :oauth_client_plugin_user_providers, :oauth_client_plugin_auths | ||
5 | + | ||
6 | + add_column :oauth_client_plugin_auths, :type, :string | ||
7 | + add_column :oauth_client_plugin_auths, :provider_user_id, :string | ||
8 | + add_column :oauth_client_plugin_auths, :access_token, :text | ||
9 | + add_column :oauth_client_plugin_auths, :expires_at, :datetime | ||
10 | + add_column :oauth_client_plugin_auths, :scope, :text | ||
11 | + add_column :oauth_client_plugin_auths, :data, :text, default: {}.to_yaml | ||
12 | + | ||
13 | + add_column :oauth_client_plugin_auths, :profile_id, :integer | ||
14 | + OauthClientPlugin::Auth.find_each batch_size: 50 do |auth| | ||
15 | + auth.profile = User.find(auth.user_id).person | ||
16 | + auth.save! | ||
17 | + end | ||
18 | + remove_column :oauth_client_plugin_auths, :user_id | ||
19 | + | ||
20 | + add_index :oauth_client_plugin_auths, :profile_id | ||
21 | + add_index :oauth_client_plugin_auths, :provider_id | ||
22 | + add_index :oauth_client_plugin_auths, :provider_user_id | ||
23 | + add_index :oauth_client_plugin_auths, :type | ||
24 | + end | ||
25 | + | ||
26 | +end |
plugins/oauth_client/lib/ext/user.rb
@@ -2,8 +2,14 @@ require_dependency 'user' | @@ -2,8 +2,14 @@ require_dependency 'user' | ||
2 | 2 | ||
3 | class User | 3 | class User |
4 | 4 | ||
5 | - has_many :oauth_user_providers, :class_name => 'OauthClientPlugin::UserProvider' | ||
6 | - has_many :oauth_providers, :through => :oauth_user_providers, :source => :provider | 5 | + has_many :oauth_auths, through: :person |
6 | + has_many :oauth_providers, through: :oauth_auths, source: :provider | ||
7 | + | ||
8 | + after_create :activate_oauth_user | ||
9 | + | ||
10 | + def activate_oauth_user | ||
11 | + self.activate if oauth_providers.present? | ||
12 | + end | ||
7 | 13 | ||
8 | def password_required_with_oauth? | 14 | def password_required_with_oauth? |
9 | password_required_without_oauth? && oauth_providers.empty? | 15 | password_required_without_oauth? && oauth_providers.empty? |
@@ -11,17 +17,6 @@ class User | @@ -11,17 +17,6 @@ class User | ||
11 | 17 | ||
12 | alias_method_chain :password_required?, :oauth | 18 | alias_method_chain :password_required?, :oauth |
13 | 19 | ||
14 | - after_create :activate_oauth_user | ||
15 | - | ||
16 | - def activate_oauth_user | ||
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 | ||
23 | - end | ||
24 | - | ||
25 | def make_activation_code_with_oauth | 20 | def make_activation_code_with_oauth |
26 | oauth_providers.blank? ? make_activation_code_without_oauth : nil | 21 | oauth_providers.blank? ? make_activation_code_without_oauth : nil |
27 | end | 22 | end |
plugins/oauth_client/lib/oauth_client_plugin.rb
@@ -45,7 +45,9 @@ class OauthClientPlugin < Noosfero::Plugin | @@ -45,7 +45,9 @@ class OauthClientPlugin < Noosfero::Plugin | ||
45 | true | 45 | true |
46 | end | 46 | end |
47 | 47 | ||
48 | - OmniAuth.config.on_failure = OauthClientPluginPublicController.action(:failure) | 48 | + Rails.configuration.to_prepare do |
49 | + OmniAuth.config.on_failure = OauthClientPluginPublicController.action(:failure) | ||
50 | + end | ||
49 | 51 | ||
50 | Rails.application.config.middleware.use OmniAuth::Builder do | 52 | Rails.application.config.middleware.use OmniAuth::Builder do |
51 | PROVIDERS.each do |provider, options| | 53 | PROVIDERS.each do |provider, options| |
@@ -60,7 +62,8 @@ class OauthClientPlugin < Noosfero::Plugin | @@ -60,7 +62,8 @@ class OauthClientPlugin < Noosfero::Plugin | ||
60 | provider_id = request.params['id'] | 62 | provider_id = request.params['id'] |
61 | provider_id ||= request.session['omniauth.params']['id'] if request.session['omniauth.params'] | 63 | provider_id ||= request.session['omniauth.params']['id'] if request.session['omniauth.params'] |
62 | provider = environment.oauth_providers.find(provider_id) | 64 | provider = environment.oauth_providers.find(provider_id) |
63 | - strategy.options.merge!(provider.options.symbolize_keys) | 65 | + strategy.options.merge! client_id: provider.client_id, client_secret: provider.client_secret |
66 | + strategy.options.merge! provider.options.symbolize_keys | ||
64 | 67 | ||
65 | request.session[:provider_id] = provider_id | 68 | request.session[:provider_id] = provider_id |
66 | } | 69 | } |
plugins/oauth_client/lib/oauth_client_plugin/provider.rb
@@ -1,20 +0,0 @@ | @@ -1,20 +0,0 @@ | ||
1 | -class OauthClientPlugin::Provider < Noosfero::Plugin::ActiveRecord | ||
2 | - | ||
3 | - belongs_to :environment | ||
4 | - | ||
5 | - validates_presence_of :name, :strategy | ||
6 | - | ||
7 | - acts_as_having_image | ||
8 | - acts_as_having_settings :field => :options | ||
9 | - | ||
10 | - settings_items :client_id, :type => :string | ||
11 | - settings_items :client_secret, :type => :string | ||
12 | - settings_items :client_options, :type => Hash | ||
13 | - | ||
14 | - attr_accessible :name, :environment, :strategy, :client_id, :client_secret, :enabled, :client_options, :image_builder | ||
15 | - | ||
16 | - scope :enabled, :conditions => {:enabled => true} | ||
17 | - | ||
18 | - acts_as_having_image | ||
19 | - | ||
20 | -end |
plugins/oauth_client/lib/oauth_client_plugin/user_provider.rb
@@ -1,10 +0,0 @@ | @@ -1,10 +0,0 @@ | ||
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 |
@@ -0,0 +1,29 @@ | @@ -0,0 +1,29 @@ | ||
1 | +class OauthClientPlugin::Auth < ActiveRecord::Base | ||
2 | + | ||
3 | + attr_accessible :profile, :provider, :enabled, | ||
4 | + :access_token, :expires_in | ||
5 | + | ||
6 | + belongs_to :profile, class_name: 'Profile' | ||
7 | + belongs_to :provider, class_name: 'OauthClientPlugin::Provider' | ||
8 | + | ||
9 | + validates_presence_of :profile | ||
10 | + validates_presence_of :provider | ||
11 | + validates_uniqueness_of :profile_id, scope: :provider_id | ||
12 | + | ||
13 | + acts_as_having_settings field: :data | ||
14 | + | ||
15 | + def expires_in | ||
16 | + self.expires_at - Time.now | ||
17 | + end | ||
18 | + def expires_in= value | ||
19 | + self.expires_at = Time.now + value.to_i | ||
20 | + end | ||
21 | + | ||
22 | + def expired? | ||
23 | + Time.now > self.expires_at rescue true | ||
24 | + end | ||
25 | + def not_expired? | ||
26 | + not self.expired? | ||
27 | + end | ||
28 | + | ||
29 | +end |
plugins/oauth_client/models/oauth_client_plugin/provider.rb
0 → 100644
@@ -0,0 +1,21 @@ | @@ -0,0 +1,21 @@ | ||
1 | +class OauthClientPlugin::Provider < ActiveRecord::Base | ||
2 | + | ||
3 | + belongs_to :environment | ||
4 | + | ||
5 | + validates_presence_of :name, :strategy | ||
6 | + | ||
7 | + acts_as_having_image | ||
8 | + acts_as_having_settings field: :options | ||
9 | + | ||
10 | + settings_items :site, type: String | ||
11 | + settings_items :client_options, type: Hash | ||
12 | + | ||
13 | + attr_accessible :name, :strategy, :enabled, :site, :image_builder, | ||
14 | + :environment, :environment_id, | ||
15 | + :client_id, :client_secret, :client_options | ||
16 | + | ||
17 | + scope :enabled, -> { where enabled: true } | ||
18 | + | ||
19 | + acts_as_having_image | ||
20 | + | ||
21 | +end |
plugins/oauth_client/test/functional/oauth_client_plugin_public_controller_test.rb
1 | -require File.dirname(__FILE__) + '/../test_helper' | 1 | +require 'test_helper' |
2 | 2 | ||
3 | class OauthClientPluginPublicControllerTest < ActionController::TestCase | 3 | class OauthClientPluginPublicControllerTest < ActionController::TestCase |
4 | 4 | ||
@@ -21,7 +21,7 @@ class OauthClientPluginPublicControllerTest < ActionController::TestCase | @@ -21,7 +21,7 @@ class OauthClientPluginPublicControllerTest < ActionController::TestCase | ||
21 | end | 21 | end |
22 | 22 | ||
23 | should 'redirect to login when user is found' do | 23 | should 'redirect to login when user is found' do |
24 | - user = fast_create(User, :environment_id => environment.id) | 24 | + user = create_user |
25 | auth.info.stubs(:email).returns(user.email) | 25 | auth.info.stubs(:email).returns(user.email) |
26 | auth.info.stubs(:name).returns(user.name) | 26 | auth.info.stubs(:name).returns(user.name) |
27 | session[:provider_id] = provider.id | 27 | session[:provider_id] = provider.id |
@@ -32,7 +32,7 @@ class OauthClientPluginPublicControllerTest < ActionController::TestCase | @@ -32,7 +32,7 @@ class OauthClientPluginPublicControllerTest < ActionController::TestCase | ||
32 | end | 32 | end |
33 | 33 | ||
34 | should 'do not login when the provider is disabled' do | 34 | should 'do not login when the provider is disabled' do |
35 | - user = fast_create(User, :environment_id => environment.id) | 35 | + user = create_user |
36 | auth.info.stubs(:email).returns(user.email) | 36 | auth.info.stubs(:email).returns(user.email) |
37 | auth.info.stubs(:name).returns(user.name) | 37 | auth.info.stubs(:name).returns(user.name) |
38 | session[:provider_id] = provider.id | 38 | session[:provider_id] = provider.id |
@@ -44,11 +44,11 @@ class OauthClientPluginPublicControllerTest < ActionController::TestCase | @@ -44,11 +44,11 @@ class OauthClientPluginPublicControllerTest < ActionController::TestCase | ||
44 | end | 44 | end |
45 | 45 | ||
46 | should 'do not login when the provider is disabled for a user' do | 46 | should 'do not login when the provider is disabled for a user' do |
47 | - user = fast_create(User, :environment_id => environment.id) | 47 | + user = create_user |
48 | auth.info.stubs(:email).returns(user.email) | 48 | auth.info.stubs(:email).returns(user.email) |
49 | auth.info.stubs(:name).returns(user.name) | 49 | auth.info.stubs(:name).returns(user.name) |
50 | session[:provider_id] = provider.id | 50 | session[:provider_id] = provider.id |
51 | - user.oauth_user_providers.create(:user => user, :provider => provider, :enabled => false) | 51 | + user.person.oauth_auths.create!(profile: user.person, provider: provider, enabled: false) |
52 | 52 | ||
53 | get :callback | 53 | get :callback |
54 | assert_redirected_to :controller => :account, :action => :login | 54 | assert_redirected_to :controller => :account, :action => :login |
@@ -56,7 +56,7 @@ class OauthClientPluginPublicControllerTest < ActionController::TestCase | @@ -56,7 +56,7 @@ class OauthClientPluginPublicControllerTest < ActionController::TestCase | ||
56 | end | 56 | end |
57 | 57 | ||
58 | should 'save provider when an user login with it' do | 58 | should 'save provider when an user login with it' do |
59 | - user = fast_create(User, :environment_id => environment.id) | 59 | + user = create_user |
60 | auth.info.stubs(:email).returns(user.email) | 60 | auth.info.stubs(:email).returns(user.email) |
61 | auth.info.stubs(:name).returns(user.name) | 61 | auth.info.stubs(:name).returns(user.name) |
62 | session[:provider_id] = provider.id | 62 | session[:provider_id] = provider.id |
@@ -66,13 +66,13 @@ class OauthClientPluginPublicControllerTest < ActionController::TestCase | @@ -66,13 +66,13 @@ class OauthClientPluginPublicControllerTest < ActionController::TestCase | ||
66 | end | 66 | end |
67 | 67 | ||
68 | should 'do not duplicate relations between an user and a provider when the same provider was used again in a login' do | 68 | should 'do not duplicate relations between an user and a provider when the same provider was used again in a login' do |
69 | - user = fast_create(User, :environment_id => environment.id) | 69 | + user = create_user |
70 | auth.info.stubs(:email).returns(user.email) | 70 | auth.info.stubs(:email).returns(user.email) |
71 | auth.info.stubs(:name).returns(user.name) | 71 | auth.info.stubs(:name).returns(user.name) |
72 | session[:provider_id] = provider.id | 72 | session[:provider_id] = provider.id |
73 | 73 | ||
74 | get :callback | 74 | get :callback |
75 | - assert_no_difference 'user.oauth_user_providers.count' do | 75 | + assert_no_difference 'user.oauth_auths.count' do |
76 | 3.times { get :callback } | 76 | 3.times { get :callback } |
77 | end | 77 | end |
78 | end | 78 | end |
plugins/oauth_client/test/unit/environment_test.rb
plugins/oauth_client/test/unit/oauth_client_plugin_test.rb
plugins/oauth_client/test/unit/user_test.rb