diff --git a/lib/sorcery/model/submodules/external.rb b/lib/sorcery/model/submodules/external.rb index 7c33d8a0..96268742 100644 --- a/lib/sorcery/model/submodules/external.rb +++ b/lib/sorcery/model/submodules/external.rb @@ -95,15 +95,13 @@ module InstanceMethods def add_provider_to_user(provider, uid) authentications = sorcery_config.authentications_class.name.demodulize.underscore.pluralize # first check to see if user has a particular authentication already - if sorcery_adapter.find_authentication_by_oauth_credentials(authentications, provider, uid).nil? - user = send(authentications).build(sorcery_config.provider_uid_attribute_name => uid, - sorcery_config.provider_attribute_name => provider) - user.sorcery_adapter.save(validate: false) - else - user = false - end + return false if sorcery_adapter.find_authentication_by_oauth_credentials(authentications, provider, uid) - user + authentication = send(authentications).build(sorcery_config.provider_uid_attribute_name => uid, + sorcery_config.provider_attribute_name => provider) + authentication.sorcery_adapter.save(validate: false) + + authentication end end end diff --git a/spec/rails_app/app/controllers/sorcery_controller.rb b/spec/rails_app/app/controllers/sorcery_controller.rb index 1f843c37..a4ffbcf0 100644 --- a/spec/rails_app/app/controllers/sorcery_controller.rb +++ b/spec/rails_app/app/controllers/sorcery_controller.rb @@ -445,10 +445,10 @@ def test_add_second_provider return unless logged_in? - if (@user = add_provider_to_user(provider)) - redirect_to 'bla', notice: 'Success!' + if (@authentication = add_provider_to_user(provider)) + redirect_to "bla", notice: "Success!" else - redirect_to 'blu', alert: 'Failed!' + redirect_to "blu", alert: "Failed!" end end diff --git a/spec/shared_examples/user_oauth_shared_examples.rb b/spec/shared_examples/user_oauth_shared_examples.rb index 3e6ec302..14ee34fc 100644 --- a/spec/shared_examples/user_oauth_shared_examples.rb +++ b/spec/shared_examples/user_oauth_shared_examples.rb @@ -29,5 +29,28 @@ external_user expect(User.load_from_provider(:twitter, 980_342)).to be_nil end + + describe "#add_provider_to_user" do + let!(:user) { create_new_user } + + subject { user.add_provider_to_user(:twitter, 123) } + + context "when a provider is successfully added" do + it "returns an instance of authentication" do + expect { + expect(subject).to be_kind_of(Authentication) + }.to change(Authentication, :count).by(1) + end + end + + context "when a provider already exists" do + let(:user) { create_new_external_user :twitter } + + it "does not create a new authentication and returns false" do + expect { subject }.not_to change(Authentication, :count) + expect(subject).to be false + end + end + end end end