From caa4d0d441d857191210ddd9c95dfcd31e6139f2 Mon Sep 17 00:00:00 2001 From: Ruslan Khaertdinov Date: Tue, 13 Oct 2015 11:20:23 +0300 Subject: [PATCH 01/10] [#325] Authentication via Social Networks --- CHANGELOG.md | 2 + Gemfile | 4 +- Gemfile.lock | 35 +++++- app/controllers/social_profiles_controller.rb | 21 ++++ .../users/omniauth_callbacks_controller.rb | 80 ++++++++++++ .../users/registrations_controller.rb | 17 +++ app/models/social_profile.rb | 10 ++ app/models/user.rb | 15 ++- app/services/check_omniauth.rb | 12 ++ .../application/_navigation_user.html.slim | 8 ++ app/views/social_profiles/_list.html.slim | 21 ++++ app/views/users/registrations/edit.html.slim | 2 + config/initializers/devise.rb | 2 + config/locales/flash.en.yml | 7 ++ config/locales/models/social_profile.en.yml | 7 ++ config/rails_best_practices.yml | 3 + config/routes.rb | 4 +- .../20151013081630_create_social_profiles.rb | 9 ++ db/schema.rb | 12 +- spec/factories/social_profiles.rb | 7 ++ .../social_profiles/add_remove_spec.rb | 98 +++++++++++++++ .../visitor/social_profiles/sign_up_spec.rb | 116 ++++++++++++++++++ spec/models/social_profiles_spec.rb | 11 ++ spec/models/user_spec.rb | 1 + spec/services/check_omniauth_spec.rb | 42 +++++++ spec/support/features/feature_helpers.rb | 15 +++ 26 files changed, 555 insertions(+), 6 deletions(-) create mode 100644 app/controllers/social_profiles_controller.rb create mode 100644 app/controllers/users/omniauth_callbacks_controller.rb create mode 100644 app/controllers/users/registrations_controller.rb create mode 100644 app/models/social_profile.rb create mode 100644 app/services/check_omniauth.rb create mode 100644 app/views/social_profiles/_list.html.slim create mode 100644 config/locales/models/social_profile.en.yml create mode 100644 db/migrate/20151013081630_create_social_profiles.rb create mode 100644 spec/factories/social_profiles.rb create mode 100644 spec/features/user/account/social_profiles/add_remove_spec.rb create mode 100644 spec/features/visitor/social_profiles/sign_up_spec.rb create mode 100644 spec/models/social_profiles_spec.rb create mode 100644 spec/services/check_omniauth_spec.rb create mode 100644 spec/support/features/feature_helpers.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index ad1047a9..bc879e8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Add ability to sign in/sign up via Social networks. And connect/disconnect network with current account from the profile page. +- Update [uglifier](https://github.com/lautis/uglifier) gem up to 2.7.2 - Move Rack::CanonicalHost and Rack::Auth::Basic configuration to initializers - Support [Heroku Review Apps](https://devcenter.heroku.com/articles/github-integration#review-apps) - Update [rails](https://github.com/rails/rails) version up to 4.2.3 diff --git a/Gemfile b/Gemfile index f5443231..b21bb7c7 100644 --- a/Gemfile +++ b/Gemfile @@ -14,7 +14,7 @@ gem "jquery-rails" gem "sass-rails", "~> 5.0.0" gem "skim" gem "therubyracer", platforms: :ruby -gem "uglifier", ">= 1.3.0" +gem "uglifier" # views gem "active_link_to" @@ -28,6 +28,8 @@ gem "devise" gem "google-analytics-rails" gem "interactor" gem "kaminari" +gem "omniauth-facebook" +gem "omniauth-google-oauth2" gem "responders" gem "rollbar", "~> 0.10.3" gem "seedbank" diff --git a/Gemfile.lock b/Gemfile.lock index 3fd62acb..12235294 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -129,6 +129,8 @@ GEM railties (>= 3.0.0) faker (1.5.0) i18n (~> 0.5) + faraday (0.9.2) + multipart-post (>= 1.2, < 3) fastercsv (1.5.5) foreman (0.63.0) dotenv (>= 0.7) @@ -151,6 +153,7 @@ GEM google-analytics-rails (0.0.6) haml (4.0.6) tilt + hashie (3.4.2) highline (1.6.21) i18n (0.7.0) interactor (3.1.0) @@ -166,6 +169,7 @@ GEM railties (>= 4.2.0) thor (>= 0.14, < 2.0) json (1.8.3) + jwt (1.5.1) kaminari (0.16.3) actionpack (>= 3.0.0) activesupport (>= 3.0.0) @@ -185,8 +189,30 @@ GEM mini_portile (0.6.2) minitest (5.8.0) multi_json (1.11.0) + multi_xml (0.5.5) + multipart-post (2.0.0) nokogiri (1.6.6.2) mini_portile (~> 0.6.0) + oauth2 (1.0.0) + faraday (>= 0.8, < 0.10) + jwt (~> 1.0) + multi_json (~> 1.3) + multi_xml (~> 0.5) + rack (~> 1.2) + omniauth (1.2.2) + hashie (>= 1.2, < 4) + rack (~> 1.0) + omniauth-facebook (2.0.1) + omniauth-oauth2 (~> 1.2) + omniauth-google-oauth2 (0.2.8) + addressable (~> 2.3) + jwt (~> 1.0) + multi_json (~> 1.3) + omniauth (>= 1.1.1) + omniauth-oauth2 (>= 1.1.1) + omniauth-oauth2 (1.3.1) + oauth2 (~> 1.0) + omniauth (~> 1.2) orm_adapter (0.5.0) parser (2.2.2.3) ast (>= 1.1, < 3.0) @@ -350,7 +376,7 @@ GEM tilt (1.4.1) tzinfo (1.2.2) thread_safe (~> 0.1) - uglifier (2.4.0) + uglifier (2.7.2) execjs (>= 0.3.0) json (>= 1.8.0) uniform_notifier (1.9.0) @@ -404,6 +430,8 @@ DEPENDENCIES launchy letter_opener metamagic + omniauth-facebook + omniauth-google-oauth2 pg pry-rails pundit @@ -429,6 +457,9 @@ DEPENDENCIES spring-commands-rspec therubyracer thin - uglifier (>= 1.3.0) + uglifier web-console (~> 2.0) webmock + +BUNDLED WITH + 1.10.6 diff --git a/app/controllers/social_profiles_controller.rb b/app/controllers/social_profiles_controller.rb new file mode 100644 index 00000000..eac940b2 --- /dev/null +++ b/app/controllers/social_profiles_controller.rb @@ -0,0 +1,21 @@ +class SocialProfilesController < ApplicationController + before_action :authenticate_user! + + expose(:social_profiles) { current_user.social_profiles } + expose(:social_profile) + + def destroy + if social_profile.destroy + flash[:notice] = t "flash.actions.destroy.notice", resource_name: resource_name + else + flash[:alert] = t "flash.actions.destroy.alert", resource_name: resource_name + end + redirect_to edit_user_registration_url + end + + private + + def resource_name + SocialProfile.model_name.human + end +end diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb new file mode 100644 index 00000000..29a1cb7e --- /dev/null +++ b/app/controllers/users/omniauth_callbacks_controller.rb @@ -0,0 +1,80 @@ +module Users + class OmniauthCallbacksController < Devise::OmniauthCallbacksController + def google_oauth2 + if !CheckOmniauth.verified?(auth) + when_social_profile_not_verified + else + handle_callback + end + end + + def facebook + if !CheckOmniauth.verified?(auth) + when_social_profile_not_verified + else + handle_callback + end + end + + private + + def handle_callback + if current_user && social_profile + when_current_user_and_social_profile + elsif current_user + when_current_user + elsif social_profile + when_social_profile + else + when_first_visit + end + end + + def social_profile + @social_profile ||= SocialProfile.from_omniauth(auth) + end + + def auth + request.env["omniauth.auth"] + end + + def when_social_profile_not_verified + flash[:notice] = t "flash.omniauth.not_verified" + redirect_to root_url + end + + def when_current_user_and_social_profile + flash[:notice] = t "flash.omniauth.already_linked" + redirect_to edit_user_registration_url + end + + def when_current_user + current_user.apply_omniauth(auth) + current_user.save! + flash[:notice] = t "flash.omniauth.social_profile_created" + redirect_to edit_user_registration_url + end + + def when_social_profile + flash[:notice] = t "flash.omniauth.already_linked" + sign_in_and_redirect(:user, social_profile.user) + end + + # rubocop:disable Metrics/AbcSize + def when_first_visit + user.apply_omniauth(auth) + if user.save + flash[:notice] = t "devise.registrations.signed_up" + sign_in_and_redirect(:user, user) + else + session[:omniauth] = auth.except("extra") + redirect_to new_user_registration_url + end + end + # rubocop:enable Metrics/AbcSize + + def user + @user ||= User.from_omniauth(auth) + end + end +end diff --git a/app/controllers/users/registrations_controller.rb b/app/controllers/users/registrations_controller.rb new file mode 100644 index 00000000..f3626d34 --- /dev/null +++ b/app/controllers/users/registrations_controller.rb @@ -0,0 +1,17 @@ +module Users + class RegistrationsController < Devise::RegistrationsController + expose(:social_profiles) { current_user.social_profiles if current_user } + + def create + super + session[:omniauth] = nil unless @user.new_record? + end + + private + + def build_resource(*args) + super + @user.apply_omniauth(session[:omniauth]) if session[:omniauth] + end + end +end diff --git a/app/models/social_profile.rb b/app/models/social_profile.rb new file mode 100644 index 00000000..8f51a1e9 --- /dev/null +++ b/app/models/social_profile.rb @@ -0,0 +1,10 @@ +class SocialProfile < ActiveRecord::Base + belongs_to :user + + validates :user, :provider, :uid, presence: true + validates :uid, uniqueness: { scope: :provider } + + def self.from_omniauth(auth) + find_by(provider: auth.provider, uid: auth.uid) + end +end diff --git a/app/models/user.rb b/app/models/user.rb index 521dad51..9656edd9 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,9 +1,12 @@ class User < ActiveRecord::Base devise :database_authenticatable, :registerable, :confirmable, - :recoverable, :rememberable, :trackable, :validatable + :recoverable, :rememberable, :trackable, :validatable, + :omniauthable, omniauth_providers: %i(google_oauth2 facebook) validates :full_name, presence: true + has_many :social_profiles, dependent: :destroy + def to_s full_name end @@ -11,4 +14,14 @@ def to_s def full_name_with_email "#{self[:full_name]} (#{email})" end + + def self.from_omniauth(auth) + where(email: auth["info"]["email"]).first_or_initialize + end + + def apply_omniauth(auth) + self.email = auth["info"]["email"] if email.blank? + self.full_name = auth["info"]["name"] if full_name.blank? + social_profiles.build(provider: auth["provider"], uid: auth["uid"]) + end end diff --git a/app/services/check_omniauth.rb b/app/services/check_omniauth.rb new file mode 100644 index 00000000..cb54a799 --- /dev/null +++ b/app/services/check_omniauth.rb @@ -0,0 +1,12 @@ +class CheckOmniauth + def self.verified?(auth) + case auth.provider + when "facebook" + auth.info.verified? + when "google_oauth2" + auth.extra.raw_info.email_verified? + else + fail ArgumentError, "Verification checking is not implemented for provider: '#{auth.provider}'" + end + end +end diff --git a/app/views/application/_navigation_user.html.slim b/app/views/application/_navigation_user.html.slim index f3fe4746..28bf04a4 100644 --- a/app/views/application/_navigation_user.html.slim +++ b/app/views/application/_navigation_user.html.slim @@ -13,3 +13,11 @@ ul.right - else = active_link_to 'Sign in', new_user_session_path, active: :exclusive, wrap_tag: :li = active_link_to 'Sign up', new_user_registration_path, active: :exclusive, wrap_tag: :li + = active_link_to "Sign in with Google", + user_omniauth_authorize_path(:google_oauth2), + active: :exclusive, + wrap_tag: :li + = active_link_to "Sign in with Facebook", + user_omniauth_authorize_path(:facebook), + active: :exclusive, + wrap_tag: :li diff --git a/app/views/social_profiles/_list.html.slim b/app/views/social_profiles/_list.html.slim new file mode 100644 index 00000000..972b1b3f --- /dev/null +++ b/app/views/social_profiles/_list.html.slim @@ -0,0 +1,21 @@ +h6 + - if social_profiles.any? + b Linked social networks: + ul.js-social-profiles + - social_profiles.each do |social_profile| + li + = t "active_record.attributes.social_profile.provider_name.#{social_profile.provider}" + | (#{social_profile.uid}) + |   + = link_to "x", + social_profile, + data: { confirm: "Are you sure you want to remove this social profile?" }, + method: :delete + b Add another service to sign in with: + - else + b Sign in through one of these services: + + p + = link_to "Google", user_omniauth_authorize_path(:google_oauth2) + |   + = link_to "Facebook", user_omniauth_authorize_path(:facebook) diff --git a/app/views/users/registrations/edit.html.slim b/app/views/users/registrations/edit.html.slim index 0decc952..a18583d0 100644 --- a/app/views/users/registrations/edit.html.slim +++ b/app/views/users/registrations/edit.html.slim @@ -24,6 +24,8 @@ = f.button :submit, 'Update' .medium-6.columns.end + = render "social_profiles/list", social_profiles: social_profiles + h6 b Cancel my account p diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index e5b9fb2c..0f57869c 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -230,6 +230,8 @@ # Add a new OmniAuth provider. Check the wiki for more information on setting # up on your models and hooks. # config.omniauth :github, 'APP_ID', 'APP_SECRET', scope: 'user,public_repo' + config.omniauth :google_oauth2, ENV["GOOGLE_CLIENT_ID"], ENV["GOOGLE_CLIENT_SECRET"] + config.omniauth :facebook, ENV["FACEBOOK_APP_ID"], ENV["FACEBOOK_APP_SECRET"], info_fields: "email, name, verified" # ==> Warden configuration # If you want to use other strategies, that are not supported by Devise, or diff --git a/config/locales/flash.en.yml b/config/locales/flash.en.yml index 2b319d66..ae860c4a 100644 --- a/config/locales/flash.en.yml +++ b/config/locales/flash.en.yml @@ -14,3 +14,10 @@ en: feedbacks: create: notice: "Email was successfully sent." + + omniauth: + social_profile_created: 'Social profile was successfully created.' + already_linked: 'Social profile already linked.' + sign_in: 'Signed in successfully.' + sign_up: 'Signed up successfully.' + not_verified: 'Social account is not verified.' diff --git a/config/locales/models/social_profile.en.yml b/config/locales/models/social_profile.en.yml new file mode 100644 index 00000000..9a464125 --- /dev/null +++ b/config/locales/models/social_profile.en.yml @@ -0,0 +1,7 @@ +en: + active_record: + attributes: + social_profile: + provider_name: + google_oauth2: Google + facebook: Facebook diff --git a/config/rails_best_practices.yml b/config/rails_best_practices.yml index e7ed6505..a9ebd07e 100644 --- a/config/rails_best_practices.yml +++ b/config/rails_best_practices.yml @@ -28,6 +28,9 @@ RemoveTrailingWhitespaceCheck: { } RemoveUnusedMethodsInControllersCheck: except_methods: - ApplicationController#devise_parameter_sanitizer + - Users::RegistrationsController#create + - Users::RegistrationsController#build_resource + - Users::RegistrationsController#devise_parameter_sanitizer RemoveUnusedMethodsInHelpersCheck: { except_methods: [] } RemoveUnusedMethodsInModelsCheck: { except_methods: [] } ReplaceComplexCreationWithFactoryMethodCheck: { attribute_assignment_count: 2 } diff --git a/config/routes.rb b/config/routes.rb index 2053e363..19b0936c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,7 +1,9 @@ Rails.application.routes.draw do - devise_for :users + devise_for :users, + controllers: { omniauth_callbacks: "users/omniauth_callbacks", registrations: "users/registrations" } resource :feedback, only: %i(new create) + resources :social_profiles, only: :destroy with_options controller: :pages do get :about diff --git a/db/migrate/20151013081630_create_social_profiles.rb b/db/migrate/20151013081630_create_social_profiles.rb new file mode 100644 index 00000000..1278f4d8 --- /dev/null +++ b/db/migrate/20151013081630_create_social_profiles.rb @@ -0,0 +1,9 @@ +class CreateSocialProfiles < ActiveRecord::Migration + def change + create_table :social_profiles do |t| + t.references :user, index: true + t.string :provider, index: true, null: false, default: "" + t.string :uid, index: true, null: false, default: "" + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 423f9aa7..bf4d9a46 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,11 +11,21 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20100713113845) do +ActiveRecord::Schema.define(version: 20151013081630) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" + create_table "social_profiles", force: :cascade do |t| + t.integer "user_id" + t.string "provider", default: "", null: false + t.string "uid", default: "", null: false + end + + add_index "social_profiles", ["provider"], name: "index_social_profiles_on_provider", using: :btree + add_index "social_profiles", ["uid"], name: "index_social_profiles_on_uid", using: :btree + add_index "social_profiles", ["user_id"], name: "index_social_profiles_on_user_id", using: :btree + create_table "users", force: :cascade do |t| t.string "email", limit: 255, default: "", null: false t.string "encrypted_password", limit: 255, default: "", null: false diff --git a/spec/factories/social_profiles.rb b/spec/factories/social_profiles.rb new file mode 100644 index 00000000..45755b4d --- /dev/null +++ b/spec/factories/social_profiles.rb @@ -0,0 +1,7 @@ +FactoryGirl.define do + factory :social_profile do + user + provider "facebook" + uid "12345" + end +end diff --git a/spec/features/user/account/social_profiles/add_remove_spec.rb b/spec/features/user/account/social_profiles/add_remove_spec.rb new file mode 100644 index 00000000..9329492b --- /dev/null +++ b/spec/features/user/account/social_profiles/add_remove_spec.rb @@ -0,0 +1,98 @@ +require "rails_helper" + +feature "Add/Remove social profiles" do + let(:uid) { "12345" } + let(:user) { create(:user, :confirmed) } + let(:user_attributes) { user.attributes.slice(:full_name, :email) } + let(:omniauth_params) { omniauth_mock(provider, uid, user_attributes) } + + before do + stub_omniauth(provider, omniauth_params) + allow(CheckOmniauth).to receive(:verified?).and_return(verified) + + login_as user + visit edit_user_registration_path + end + + context "when social account is verified" do + let(:verified) { true } + + context "when provider is Facebook" do + let(:provider) { :facebook } + + scenario "user adds social profile" do + expect(page).to have_link("Facebook") + expect(page).not_to have_content("Linked social networks") + + click_link "Facebook" + expect(page).to have_content(I18n.t "flash.omniauth.social_profile_created") + expect(page).to have_content("Linked social networks") + expect(page).to have_css(".js-social-profiles", text: "Facebook") + + click_link "Facebook" + expect(page).to have_content(I18n.t "flash.omniauth.already_linked") + + click_link "x" + expect(page).to have_content(I18n.t "flash.actions.destroy.notice", resource_name: social_profile_name) + expect(page).not_to have_content("Linked social networks") + end + end + + context "when provider is Google" do + let(:provider) { :google_oauth2 } + + scenario "user adds social profile" do + expect(page).to have_link("Google") + expect(page).not_to have_content("Linked social networks") + + click_link "Google" + expect(page).to have_content(I18n.t "flash.omniauth.social_profile_created") + expect(page).to have_content("Linked social networks") + expect(page).to have_css(".js-social-profiles", text: "Google") + + click_link "Google" + expect(page).to have_content(I18n.t "flash.omniauth.already_linked") + + click_link "x" + expect(page).to have_content(I18n.t "flash.actions.destroy.notice", resource_name: social_profile_name) + expect(page).not_to have_content("Linked social networks") + end + end + end + + context "when social account is not verified" do + let(:verified) { false } + + context "when provider is Facebook" do + let(:provider) { :facebook } + + scenario "user not able to add social profile" do + expect(page).to have_link("Facebook") + expect(page).not_to have_content("Linked social networks") + + click_link "Facebook" + expect(page).to have_content(I18n.t "flash.omniauth.not_verified") + expect(page).not_to have_content("Linked social networks") + expect(page).not_to have_css(".js-social-profiles", text: "Facebook") + end + end + + context "when provider is Google" do + let(:provider) { :google_oauth2 } + + scenario "user not able to add social profile" do + expect(page).to have_link("Google") + expect(page).not_to have_content("Linked social networks") + + click_link "Google" + expect(page).to have_content(I18n.t "flash.omniauth.not_verified") + expect(page).not_to have_content("Linked social networks") + expect(page).not_to have_css(".js-social-profiles", text: "Google") + end + end + end + + def social_profile_name + SocialProfile.model_name.human + end +end diff --git a/spec/features/visitor/social_profiles/sign_up_spec.rb b/spec/features/visitor/social_profiles/sign_up_spec.rb new file mode 100644 index 00000000..0cac99d4 --- /dev/null +++ b/spec/features/visitor/social_profiles/sign_up_spec.rb @@ -0,0 +1,116 @@ +require "rails_helper" + +feature "Sign Up" do + let(:uid) { "12345" } + let(:registered_user) { User.find_by_email(user_attributes[:email]) } + let(:omniauth_params) { omniauth_mock(provider, uid, user_attributes) } + + let(:user_attributes) do + ActiveSupport::HashWithIndifferentAccess.new( + attributes_for(:user).slice(:full_name, :email, :password, :password_confirmation) + ) + end + + before do + stub_omniauth(provider, omniauth_params) + allow(CheckOmniauth).to receive(:verified?).and_return(verified) + + visit root_path + end + + context "when social account is verified" do + let(:verified) { true } + + context "when user and social profile not exist" do + context "when provider is Google" do + let(:provider) { :google_oauth2 } + + scenario "Visitor signs up through provider" do + click_link "Sign in with Google" + expect(page).to have_field("user_full_name", with: user_attributes[:full_name]) + expect(page).to have_field("user_email", with: user_attributes[:email]) + + fill_in "user_password", with: user_attributes[:password] + fill_in "user_password_confirmation", with: user_attributes[:password_confirmation] + click_button "Sign up" + + expect(page).to have_content(I18n.t "devise.registrations.signed_up") + expect(page).to have_text(registered_user.email) + end + end + + context "when provider is Facebook" do + let(:provider) { :facebook } + + scenario "Visitor signs up through provider" do + click_link "Sign in with Facebook" + expect(page).to have_field("user_full_name", with: user_attributes[:full_name]) + expect(page).to have_field("user_email", with: user_attributes[:email]) + + fill_in "user_password", with: user_attributes[:password] + fill_in "user_password_confirmation", with: user_attributes[:password_confirmation] + click_button "Sign up" + + expect(page).to have_content(I18n.t "devise.registrations.signed_up") + expect(page).to have_text(registered_user.email) + end + end + end + + context "when user and social profile exist" do + let(:user) { create(:user, :confirmed, user_attributes) } + + before do + user.social_profiles.create(provider: provider, uid: uid) + end + + context "when provider is Google" do + let(:provider) { :google_oauth2 } + + scenario "Visitor signs in through provider" do + click_link "Sign in with Google" + + expect(page).to have_content(I18n.t "flash.omniauth.already_linked") + expect(page).to have_text(registered_user.email) + end + end + + context "when provider is Facebook" do + let(:provider) { :facebook } + + scenario "Visitor signs in through provider" do + click_link "Sign in with Facebook" + + expect(page).to have_content(I18n.t "flash.omniauth.already_linked") + expect(page).to have_text(registered_user.email) + end + end + end + end + + context "when social account is not verified" do + let(:verified) { false } + + context "when provider is Google" do + let(:provider) { :google_oauth2 } + + scenario "Visitor not able to sign up through provider" do + click_link "Sign in with Google" + + expect(page).to have_content(I18n.t "flash.omniauth.not_verified") + expect(page).not_to have_text(user_attributes[:email]) + end + end + + context "when provider is Facebook" do + let(:provider) { :facebook } + + scenario "Visitor not able to sign up through provider" do + click_link "Sign in with Facebook" + + expect(page).to have_content(I18n.t "flash.omniauth.not_verified") + expect(page).not_to have_text(user_attributes[:email]) + end + end + end +end diff --git a/spec/models/social_profiles_spec.rb b/spec/models/social_profiles_spec.rb new file mode 100644 index 00000000..79c30cf3 --- /dev/null +++ b/spec/models/social_profiles_spec.rb @@ -0,0 +1,11 @@ +require "rails_helper" + +describe SocialProfile do + subject { create :social_profile } + + it { should belong_to :user } + it { is_expected.to validate_presence_of :user } + it { is_expected.to validate_presence_of :uid } + it { is_expected.to validate_presence_of :provider } + it { is_expected.to validate_uniqueness_of(:uid).scoped_to(:provider) } +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 0d1f5ef0..5ca43167 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2,4 +2,5 @@ describe User do it { is_expected.to validate_presence_of :full_name } + it { is_expected.to have_many(:social_profiles).dependent(:destroy) } end diff --git a/spec/services/check_omniauth_spec.rb b/spec/services/check_omniauth_spec.rb new file mode 100644 index 00000000..f998d262 --- /dev/null +++ b/spec/services/check_omniauth_spec.rb @@ -0,0 +1,42 @@ +require "rails_helper" + +describe CheckOmniauth do + let(:auth) { double(:omniauth, provider: provider) } + + describe ".verified?" do + subject { described_class.verified?(auth) } + + context "when provider is Facebook" do + let(:provider) { "facebook" } + + before do + allow(auth).to receive_message_chain(:info, :verified?).and_return(true) + end + + it "returns corresponding value" do + expect(subject).to eq(true) + end + end + + context "when provider is Google" do + let(:provider) { "google_oauth2" } + + before do + allow(auth).to receive_message_chain(:extra, :raw_info, :email_verified?).and_return(true) + end + + it "returns corresponding value" do + expect(subject).to eq(true) + end + end + + context "when provider is not in the case statement" do + let(:provider) { "another" } + + it "raises Exception" do + expect { subject } + .to raise_error(ArgumentError, "Verification checking is not implemented for provider: '#{auth.provider}'") + end + end + end +end diff --git a/spec/support/features/feature_helpers.rb b/spec/support/features/feature_helpers.rb new file mode 100644 index 00000000..f387c246 --- /dev/null +++ b/spec/support/features/feature_helpers.rb @@ -0,0 +1,15 @@ +module FeatureHelpers + def stub_omniauth(provider, omniauth_mock) + OmniAuth.config.test_mode = true + OmniAuth.config.mock_auth[provider] = omniauth_mock + Rails.application.env_config["omniauth.auth"] = omniauth_mock + end + + def omniauth_mock(provider, uid, user_attrs = {}) + OmniAuth::AuthHash.new( + provider: provider, + uid: uid, + info: { name: user_attrs[:full_name], email: user_attrs[:email] } + ) + end +end From fd23292a217ea672df717a484c712f9a1fb5b4c5 Mon Sep 17 00:00:00 2001 From: Ruslan Khaertdinov Date: Tue, 13 Oct 2015 12:24:28 +0300 Subject: [PATCH 02/10] update .ruby-verison --- .ruby-version | 2 +- Gemfile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.ruby-version b/.ruby-version index b1b25a5f..58594069 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -2.2.2 +2.2.3 diff --git a/Gemfile b/Gemfile index b21bb7c7..e7f7f3ee 100644 --- a/Gemfile +++ b/Gemfile @@ -1,6 +1,6 @@ source "https://rubygems.org" -ruby "2.2.2" +ruby "2.2.3" gem "rails", "4.2.3" gem "pg" From 8d6d669f0bd50e2a0aec3c781f44ae0a835353ac Mon Sep 17 00:00:00 2001 From: Ruslan Khaertdinov Date: Tue, 13 Oct 2015 22:19:00 +0300 Subject: [PATCH 03/10] apply review comments: move check_omniauth content into policy object, use before filter and use expose instead of private method --- .../users/omniauth_callbacks_controller.rb | 28 ++++++++----------- .../omniauth_verification_policy.rb} | 10 +++++-- .../social_profiles/add_remove_spec.rb | 2 +- .../visitor/social_profiles/sign_up_spec.rb | 2 +- .../omniauth_verification_policy_spec.rb} | 6 ++-- 5 files changed, 24 insertions(+), 24 deletions(-) rename app/{services/check_omniauth.rb => policies/omniauth_verification_policy.rb} (69%) rename spec/{services/check_omniauth_spec.rb => policies/omniauth_verification_policy_spec.rb} (89%) diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index 29a1cb7e..6756daab 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/app/controllers/users/omniauth_callbacks_controller.rb @@ -1,19 +1,15 @@ module Users class OmniauthCallbacksController < Devise::OmniauthCallbacksController + before_action :check_omniauth_verification + + expose(:user) { User.from_omniauth(auth) } + def google_oauth2 - if !CheckOmniauth.verified?(auth) - when_social_profile_not_verified - else - handle_callback - end + handle_callback end def facebook - if !CheckOmniauth.verified?(auth) - when_social_profile_not_verified - else - handle_callback - end + handle_callback end private @@ -38,11 +34,6 @@ def auth request.env["omniauth.auth"] end - def when_social_profile_not_verified - flash[:notice] = t "flash.omniauth.not_verified" - redirect_to root_url - end - def when_current_user_and_social_profile flash[:notice] = t "flash.omniauth.already_linked" redirect_to edit_user_registration_url @@ -73,8 +64,11 @@ def when_first_visit end # rubocop:enable Metrics/AbcSize - def user - @user ||= User.from_omniauth(auth) + def check_omniauth_verification + return if OmniauthVerificationPolicy.new(auth).verified? + + flash[:notice] = t "flash.omniauth.not_verified" + redirect_to root_url end end end diff --git a/app/services/check_omniauth.rb b/app/policies/omniauth_verification_policy.rb similarity index 69% rename from app/services/check_omniauth.rb rename to app/policies/omniauth_verification_policy.rb index cb54a799..5980e59b 100644 --- a/app/services/check_omniauth.rb +++ b/app/policies/omniauth_verification_policy.rb @@ -1,5 +1,11 @@ -class CheckOmniauth - def self.verified?(auth) +class OmniauthVerificationPolicy + attr_reader :auth + + def initialize(auth) + @auth = auth + end + + def verified? case auth.provider when "facebook" auth.info.verified? diff --git a/spec/features/user/account/social_profiles/add_remove_spec.rb b/spec/features/user/account/social_profiles/add_remove_spec.rb index 9329492b..61e0b9e7 100644 --- a/spec/features/user/account/social_profiles/add_remove_spec.rb +++ b/spec/features/user/account/social_profiles/add_remove_spec.rb @@ -8,7 +8,7 @@ before do stub_omniauth(provider, omniauth_params) - allow(CheckOmniauth).to receive(:verified?).and_return(verified) + allow(OmniauthVerificationPolicy).to receive_message_chain(:new, :verified?).and_return(verified) login_as user visit edit_user_registration_path diff --git a/spec/features/visitor/social_profiles/sign_up_spec.rb b/spec/features/visitor/social_profiles/sign_up_spec.rb index 0cac99d4..f8bf1990 100644 --- a/spec/features/visitor/social_profiles/sign_up_spec.rb +++ b/spec/features/visitor/social_profiles/sign_up_spec.rb @@ -13,7 +13,7 @@ before do stub_omniauth(provider, omniauth_params) - allow(CheckOmniauth).to receive(:verified?).and_return(verified) + allow(OmniauthVerificationPolicy).to receive_message_chain(:new, :verified?).and_return(verified) visit root_path end diff --git a/spec/services/check_omniauth_spec.rb b/spec/policies/omniauth_verification_policy_spec.rb similarity index 89% rename from spec/services/check_omniauth_spec.rb rename to spec/policies/omniauth_verification_policy_spec.rb index f998d262..0845e0f9 100644 --- a/spec/services/check_omniauth_spec.rb +++ b/spec/policies/omniauth_verification_policy_spec.rb @@ -1,10 +1,10 @@ require "rails_helper" -describe CheckOmniauth do +describe OmniauthVerificationPolicy do let(:auth) { double(:omniauth, provider: provider) } - describe ".verified?" do - subject { described_class.verified?(auth) } + describe "#verified?" do + subject { described_class.new(auth).verified? } context "when provider is Facebook" do let(:provider) { "facebook" } From e4e85497cac61031a56817dc4e50bb0bc0132586 Mon Sep 17 00:00:00 2001 From: Ruslan Khaertdinov Date: Thu, 22 Oct 2015 22:58:51 +0300 Subject: [PATCH 04/10] try to add services --- .../users/omniauth_callbacks_controller.rb | 81 ++++++------------- ...sting_user_email_authentication_service.rb | 17 ++++ ...ng_user_identity_authentication_service.rb | 13 +++ .../new_user_registration_service.rb | 35 ++++++++ app/interactors/oauth_organizer.rb | 57 +++++++++++++ app/models/social_profile.rb | 6 ++ 6 files changed, 154 insertions(+), 55 deletions(-) create mode 100644 app/interactors/existing_user_email_authentication_service.rb create mode 100644 app/interactors/existing_user_identity_authentication_service.rb create mode 100644 app/interactors/new_user_registration_service.rb create mode 100644 app/interactors/oauth_organizer.rb diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index 6756daab..88874a77 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/app/controllers/users/omniauth_callbacks_controller.rb @@ -1,74 +1,45 @@ module Users class OmniauthCallbacksController < Devise::OmniauthCallbacksController - before_action :check_omniauth_verification - - expose(:user) { User.from_omniauth(auth) } - - def google_oauth2 - handle_callback - end - - def facebook - handle_callback + expose(:user) { OauthOrganizer.new(auth, current_user).call } + expose(:social_profile) { SocialProfile.from_omniauth(auth) } + + SocialProfile::PROVIDERS.each do |provider| + define_method "#{provider}" do + begin + handle_user(user_from_oauth, provider) + rescue OauthOrganizer::OauthError => e + handle_error(e, provider) + end + end end private - def handle_callback - if current_user && social_profile - when_current_user_and_social_profile - elsif current_user - when_current_user - elsif social_profile - when_social_profile + def handle_user(user, provider) + if user.persisted? + sign_in_and_redirect user, event: :authentication + set_flash_message(:notice, :success, kind: "#{provider.titleize}") if is_navigational_format? else - when_first_visit + session["devise.#{provider}_data"] = auth + redirect_to new_user_registration_url end end - def social_profile - @social_profile ||= SocialProfile.from_omniauth(auth) + def handle_error(e, provider) + if user_signed_in? + redirect_to root_path, notice: e.message + else + redirect_to new_user_session_path, + notice: "Your #{provider.titleize} account can't be used to sign in. Please verify it via profile page." + end end def auth request.env["omniauth.auth"] end - def when_current_user_and_social_profile - flash[:notice] = t "flash.omniauth.already_linked" - redirect_to edit_user_registration_url - end - - def when_current_user - current_user.apply_omniauth(auth) - current_user.save! - flash[:notice] = t "flash.omniauth.social_profile_created" - redirect_to edit_user_registration_url - end - - def when_social_profile - flash[:notice] = t "flash.omniauth.already_linked" - sign_in_and_redirect(:user, social_profile.user) - end - - # rubocop:disable Metrics/AbcSize - def when_first_visit - user.apply_omniauth(auth) - if user.save - flash[:notice] = t "devise.registrations.signed_up" - sign_in_and_redirect(:user, user) - else - session[:omniauth] = auth.except("extra") - redirect_to new_user_registration_url - end - end - # rubocop:enable Metrics/AbcSize - - def check_omniauth_verification - return if OmniauthVerificationPolicy.new(auth).verified? - - flash[:notice] = t "flash.omniauth.not_verified" - redirect_to root_url + def auth_verified? + OmniauthVerificationPolicy.new(auth).verified? end end end diff --git a/app/interactors/existing_user_email_authentication_service.rb b/app/interactors/existing_user_email_authentication_service.rb new file mode 100644 index 00000000..20d71eff --- /dev/null +++ b/app/interactors/existing_user_email_authentication_service.rb @@ -0,0 +1,17 @@ +class ExistingUserEmailAuthenticationService + def initialize(auth) + @auth = auth + end + + def call + user = find_user_by_email + + user + end + + def find_user_by_email + email = @auth.extra.raw_info.email || @auth.info.email + + User.find_by_email(email) + end +end diff --git a/app/interactors/existing_user_identity_authentication_service.rb b/app/interactors/existing_user_identity_authentication_service.rb new file mode 100644 index 00000000..64db047b --- /dev/null +++ b/app/interactors/existing_user_identity_authentication_service.rb @@ -0,0 +1,13 @@ +class ExistingUserIdentityAuthenticationService + def initialize(auth, signed_in_resource = nil) + @auth = auth + @signed_in_resource = signed_in_resource + @identity = SocialProfile.find_by(uid: @auth.uid, provider: @auth.provider) + end + + def call + user = @signed_in_resource || @identity.try(:user) + + user + end +end diff --git a/app/interactors/new_user_registration_service.rb b/app/interactors/new_user_registration_service.rb new file mode 100644 index 00000000..eaacb94a --- /dev/null +++ b/app/interactors/new_user_registration_service.rb @@ -0,0 +1,35 @@ +class NewUserRegistrationService + TEMP_EMAIL_PREFIX = "change@me" + + def initialize(auth) + @auth = auth + end + + def call + user = create_user_from_auth! + + user + end + + private + + def create_user_from_auth! + user = User.new(user_params) + user.skip_confirmation_notification! + user.save! + + user + end + + def user_params + { + full_name: @auth.extra.raw_info.name, + email: @auth.extra.raw_info.email.presence || temp_email, + password: Devise.friendly_token[0, 20] + } + end + + def temp_email + "#{TEMP_EMAIL_PREFIX}-#{@auth.uid}-#{@auth.provider}.com" + end +end diff --git a/app/interactors/oauth_organizer.rb b/app/interactors/oauth_organizer.rb new file mode 100644 index 00000000..04926156 --- /dev/null +++ b/app/interactors/oauth_organizer.rb @@ -0,0 +1,57 @@ +class OauthOrganizer + class OauthError < StandardError + end + + def initialize(auth, current_user) + @auth = auth + @current_user = current_user + end + + def call + user = build_response + user.present? ? connect_accounts!(user) : fail_oauth + + user + end + + private + + def build_response + response = find_user_by_identity || find_user_by_email || sign_up_with_oauth + + response + end + + def find_user_by_identity + ExistingUserIdentityAuthenticationService.new(@auth, @current_user).call if auth_verified? + end + + def find_user_by_email + ExistingUserEmailAuthenticationService.new(@auth).call if auth_verified? + end + + def found_user_by_email? + ExistingUserEmailAuthenticationService.new(@auth).call.present? + end + + def sign_up_with_oauth + NewUserRegistrationService.new(@auth).call if !found_user_by_email? && auth_verified? + end + + def fail_oauth + fail OauthError, + "Sorry, but yours #{@auth.provider.titleize} failed verification. + Seems like yours #{@auth.provider.titleize} account hasn't been verified." + end + + def connect_accounts!(user) + identity = Identity.find_for_oauth(@auth) + return false if identity.user == user + + identity.update_attribute(:user, user) + end + + def auth_verified? + OmniauthVerificationPolicy.new(@auth).verified? + end +end diff --git a/app/models/social_profile.rb b/app/models/social_profile.rb index 8f51a1e9..966a522c 100644 --- a/app/models/social_profile.rb +++ b/app/models/social_profile.rb @@ -1,4 +1,6 @@ class SocialProfile < ActiveRecord::Base + PROVIDERS = %i(facebook google_oauth2) + belongs_to :user validates :user, :provider, :uid, presence: true @@ -7,4 +9,8 @@ class SocialProfile < ActiveRecord::Base def self.from_omniauth(auth) find_by(provider: auth.provider, uid: auth.uid) end + + def self.find_for_oauth(auth) + find_or_create_by(uid: auth.uid, provider: auth.provider) + end end From cf698130f9ee2a56a8756eafe6703b5f875cde51 Mon Sep 17 00:00:00 2001 From: Ruslan Khaertdinov Date: Fri, 30 Oct 2015 15:27:54 +0300 Subject: [PATCH 05/10] apply review comments: extract omniauth logic into separate services --- .../omniauth_callbacks_controller.rb | 42 +++++++ app/controllers/registrations_controller.rb | 13 +++ .../users/omniauth_callbacks_controller.rb | 45 -------- .../users/registrations_controller.rb | 17 --- app/helpers/omniauth_helper.rb | 5 + ...sting_user_email_authentication_service.rb | 17 --- ...ng_user_identity_authentication_service.rb | 13 --- app/interactors/fetch_oauth_user.rb | 38 +++++++ .../new_user_registration_service.rb | 35 ------ app/interactors/oauth_organizer.rb | 49 ++------ app/models/social_profile.rb | 4 - app/models/user.rb | 10 +- app/policies/auth_verification_policy.rb | 12 ++ app/policies/omniauth_verification_policy.rb | 18 --- .../application/_navigation_user.html.slim | 14 +-- app/views/social_profiles/_list.html.slim | 33 +++--- app/views/users/registrations/edit.html.slim | 2 +- config/locales/flash.en.yml | 7 -- config/locales/oauth.en.yml | 5 + config/rails_best_practices.yml | 6 +- config/routes.rb | 2 +- .../social_profiles/add_remove_spec.rb | 97 +++++----------- .../visitor/social_profiles/sign_up_spec.rb | 106 ++++++++---------- spec/interactors/fetch_oauth_user_spec.rb | 55 +++++++++ spec/interactors/oauth_organizer_spec.rb | 30 +++++ ...ec.rb => auth_verification_policy_spec.rb} | 9 +- spec/rails_helper.rb | 2 + .../feature_helpers.rb => oauth_helpers.rb} | 4 +- 28 files changed, 325 insertions(+), 365 deletions(-) create mode 100644 app/controllers/omniauth_callbacks_controller.rb create mode 100644 app/controllers/registrations_controller.rb delete mode 100644 app/controllers/users/omniauth_callbacks_controller.rb delete mode 100644 app/controllers/users/registrations_controller.rb create mode 100644 app/helpers/omniauth_helper.rb delete mode 100644 app/interactors/existing_user_email_authentication_service.rb delete mode 100644 app/interactors/existing_user_identity_authentication_service.rb create mode 100644 app/interactors/fetch_oauth_user.rb delete mode 100644 app/interactors/new_user_registration_service.rb create mode 100644 app/policies/auth_verification_policy.rb delete mode 100644 app/policies/omniauth_verification_policy.rb create mode 100644 config/locales/oauth.en.yml create mode 100644 spec/interactors/fetch_oauth_user_spec.rb create mode 100644 spec/interactors/oauth_organizer_spec.rb rename spec/policies/{omniauth_verification_policy_spec.rb => auth_verification_policy_spec.rb} (73%) rename spec/support/{features/feature_helpers.rb => oauth_helpers.rb} (89%) diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb new file mode 100644 index 00000000..2d7e457a --- /dev/null +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -0,0 +1,42 @@ +class OmniauthCallbacksController < Devise::OmniauthCallbacksController + include OmniauthHelper + + expose(:user) { OauthOrganizer.new(current_user, auth_hash).call } + + SocialProfile::PROVIDERS.each do |provider| + define_method(provider.to_s) do + begin + handle_user + rescue OauthOrganizer::OauthError + handle_error + end + end + end + + def after_sign_in_path_for(_resource) + edit_user_registration_path + end + + private + + def auth_hash + request.env["omniauth.auth"] + end + + # rubocop:disable Metrics/AbcSize + def handle_user + if user.persisted? + sign_in_and_redirect user, event: :authentication + set_flash_message(:notice, :success, kind: "#{provider_name(auth_hash.provider)}") if is_navigational_format? + else + session[:omniauth] = auth_hash.except("extra") + redirect_to new_user_registration_url + end + end + # rubocop:enable Metrics/AbcSize + + def handle_error + redirect_to new_user_session_path, + notice: t("omniauth.verification.failure", kind: provider_name(auth_hash.provider)) + end +end diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb new file mode 100644 index 00000000..27eab5a6 --- /dev/null +++ b/app/controllers/registrations_controller.rb @@ -0,0 +1,13 @@ +class RegistrationsController < Devise::RegistrationsController + def create + super + session[:omniauth] = nil unless @user.new_record? + end + + private + + def build_resource(*args) + super + @user.apply_omniauth(session[:omniauth]) if session[:omniauth] + end +end diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb deleted file mode 100644 index 88874a77..00000000 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ /dev/null @@ -1,45 +0,0 @@ -module Users - class OmniauthCallbacksController < Devise::OmniauthCallbacksController - expose(:user) { OauthOrganizer.new(auth, current_user).call } - expose(:social_profile) { SocialProfile.from_omniauth(auth) } - - SocialProfile::PROVIDERS.each do |provider| - define_method "#{provider}" do - begin - handle_user(user_from_oauth, provider) - rescue OauthOrganizer::OauthError => e - handle_error(e, provider) - end - end - end - - private - - def handle_user(user, provider) - if user.persisted? - sign_in_and_redirect user, event: :authentication - set_flash_message(:notice, :success, kind: "#{provider.titleize}") if is_navigational_format? - else - session["devise.#{provider}_data"] = auth - redirect_to new_user_registration_url - end - end - - def handle_error(e, provider) - if user_signed_in? - redirect_to root_path, notice: e.message - else - redirect_to new_user_session_path, - notice: "Your #{provider.titleize} account can't be used to sign in. Please verify it via profile page." - end - end - - def auth - request.env["omniauth.auth"] - end - - def auth_verified? - OmniauthVerificationPolicy.new(auth).verified? - end - end -end diff --git a/app/controllers/users/registrations_controller.rb b/app/controllers/users/registrations_controller.rb deleted file mode 100644 index f3626d34..00000000 --- a/app/controllers/users/registrations_controller.rb +++ /dev/null @@ -1,17 +0,0 @@ -module Users - class RegistrationsController < Devise::RegistrationsController - expose(:social_profiles) { current_user.social_profiles if current_user } - - def create - super - session[:omniauth] = nil unless @user.new_record? - end - - private - - def build_resource(*args) - super - @user.apply_omniauth(session[:omniauth]) if session[:omniauth] - end - end -end diff --git a/app/helpers/omniauth_helper.rb b/app/helpers/omniauth_helper.rb new file mode 100644 index 00000000..88471a14 --- /dev/null +++ b/app/helpers/omniauth_helper.rb @@ -0,0 +1,5 @@ +module OmniauthHelper + def provider_name(provider) + t "active_record.attributes.social_profile.provider_name.#{provider}" + end +end diff --git a/app/interactors/existing_user_email_authentication_service.rb b/app/interactors/existing_user_email_authentication_service.rb deleted file mode 100644 index 20d71eff..00000000 --- a/app/interactors/existing_user_email_authentication_service.rb +++ /dev/null @@ -1,17 +0,0 @@ -class ExistingUserEmailAuthenticationService - def initialize(auth) - @auth = auth - end - - def call - user = find_user_by_email - - user - end - - def find_user_by_email - email = @auth.extra.raw_info.email || @auth.info.email - - User.find_by_email(email) - end -end diff --git a/app/interactors/existing_user_identity_authentication_service.rb b/app/interactors/existing_user_identity_authentication_service.rb deleted file mode 100644 index 64db047b..00000000 --- a/app/interactors/existing_user_identity_authentication_service.rb +++ /dev/null @@ -1,13 +0,0 @@ -class ExistingUserIdentityAuthenticationService - def initialize(auth, signed_in_resource = nil) - @auth = auth - @signed_in_resource = signed_in_resource - @identity = SocialProfile.find_by(uid: @auth.uid, provider: @auth.provider) - end - - def call - user = @signed_in_resource || @identity.try(:user) - - user - end -end diff --git a/app/interactors/fetch_oauth_user.rb b/app/interactors/fetch_oauth_user.rb new file mode 100644 index 00000000..e680d23a --- /dev/null +++ b/app/interactors/fetch_oauth_user.rb @@ -0,0 +1,38 @@ +class FetchOauthUser + attr_reader :auth + private :auth + + def initialize(auth) + @auth = auth + end + + def call + find_user_by_email || find_social_profile_user || build_user + end + + private + + def find_user_by_email + user_by_email if auth_verified? + end + + def user_by_email + @user_by_email ||= User.find_by(email: auth["info"]["email"]) + end + + def auth_verified? + @auth_verified ||= AuthVerificationPolicy.verified?(auth) + end + + def find_social_profile_user + social_profile.try(:user) + end + + def social_profile + SocialProfile.from_omniauth(auth) + end + + def build_user + User.build_from_omniauth(auth) if auth_verified? && user_by_email.nil? + end +end diff --git a/app/interactors/new_user_registration_service.rb b/app/interactors/new_user_registration_service.rb deleted file mode 100644 index eaacb94a..00000000 --- a/app/interactors/new_user_registration_service.rb +++ /dev/null @@ -1,35 +0,0 @@ -class NewUserRegistrationService - TEMP_EMAIL_PREFIX = "change@me" - - def initialize(auth) - @auth = auth - end - - def call - user = create_user_from_auth! - - user - end - - private - - def create_user_from_auth! - user = User.new(user_params) - user.skip_confirmation_notification! - user.save! - - user - end - - def user_params - { - full_name: @auth.extra.raw_info.name, - email: @auth.extra.raw_info.email.presence || temp_email, - password: Devise.friendly_token[0, 20] - } - end - - def temp_email - "#{TEMP_EMAIL_PREFIX}-#{@auth.uid}-#{@auth.provider}.com" - end -end diff --git a/app/interactors/oauth_organizer.rb b/app/interactors/oauth_organizer.rb index 04926156..7c275714 100644 --- a/app/interactors/oauth_organizer.rb +++ b/app/interactors/oauth_organizer.rb @@ -2,56 +2,27 @@ class OauthOrganizer class OauthError < StandardError end - def initialize(auth, current_user) - @auth = auth + attr_reader :auth, :current_user + private :auth, :current_user + + def initialize(current_user, auth) @current_user = current_user + @auth = auth end def call - user = build_response - user.present? ? connect_accounts!(user) : fail_oauth - + user.present? ? user.connect_social_profile(auth) : fail_oauth user end private - def build_response - response = find_user_by_identity || find_user_by_email || sign_up_with_oauth - - response - end - - def find_user_by_identity - ExistingUserIdentityAuthenticationService.new(@auth, @current_user).call if auth_verified? - end - - def find_user_by_email - ExistingUserEmailAuthenticationService.new(@auth).call if auth_verified? - end - - def found_user_by_email? - ExistingUserEmailAuthenticationService.new(@auth).call.present? - end - - def sign_up_with_oauth - NewUserRegistrationService.new(@auth).call if !found_user_by_email? && auth_verified? + def user + @user ||= current_user || FetchOauthUser.new(auth).call end def fail_oauth - fail OauthError, - "Sorry, but yours #{@auth.provider.titleize} failed verification. - Seems like yours #{@auth.provider.titleize} account hasn't been verified." - end - - def connect_accounts!(user) - identity = Identity.find_for_oauth(@auth) - return false if identity.user == user - - identity.update_attribute(:user, user) - end - - def auth_verified? - OmniauthVerificationPolicy.new(@auth).verified? + fail OauthError, "Sorry, but yours #{auth.provider.titleize} failed verification. + Seems like yours #{auth.provider.titleize} account hasn't been verified." end end diff --git a/app/models/social_profile.rb b/app/models/social_profile.rb index 966a522c..87e1087c 100644 --- a/app/models/social_profile.rb +++ b/app/models/social_profile.rb @@ -9,8 +9,4 @@ class SocialProfile < ActiveRecord::Base def self.from_omniauth(auth) find_by(provider: auth.provider, uid: auth.uid) end - - def self.find_for_oauth(auth) - find_or_create_by(uid: auth.uid, provider: auth.provider) - end end diff --git a/app/models/user.rb b/app/models/user.rb index 9656edd9..f3132181 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,7 +1,7 @@ class User < ActiveRecord::Base devise :database_authenticatable, :registerable, :confirmable, :recoverable, :rememberable, :trackable, :validatable, - :omniauthable, omniauth_providers: %i(google_oauth2 facebook) + :omniauthable, omniauth_providers: SocialProfile::PROVIDERS validates :full_name, presence: true @@ -15,8 +15,8 @@ def full_name_with_email "#{self[:full_name]} (#{email})" end - def self.from_omniauth(auth) - where(email: auth["info"]["email"]).first_or_initialize + def self.build_from_omniauth(auth) + new(email: auth["info"]["email"], full_name: auth["info"]["name"]) end def apply_omniauth(auth) @@ -24,4 +24,8 @@ def apply_omniauth(auth) self.full_name = auth["info"]["name"] if full_name.blank? social_profiles.build(provider: auth["provider"], uid: auth["uid"]) end + + def connect_social_profile(auth) + apply_omniauth(auth) && save unless SocialProfile.from_omniauth(auth) + end end diff --git a/app/policies/auth_verification_policy.rb b/app/policies/auth_verification_policy.rb new file mode 100644 index 00000000..ae9d7418 --- /dev/null +++ b/app/policies/auth_verification_policy.rb @@ -0,0 +1,12 @@ +class AuthVerificationPolicy + def self.verified?(auth) + case auth.provider + when "facebook" + auth.info.verified? || auth.extra.raw_info.verified? + when "google_oauth2" + auth.extra.raw_info.email_verified? + else + fail ArgumentError, I18n.t("omniauth.verification.not_implemented", kind: auth.provider) + end + end +end diff --git a/app/policies/omniauth_verification_policy.rb b/app/policies/omniauth_verification_policy.rb deleted file mode 100644 index 5980e59b..00000000 --- a/app/policies/omniauth_verification_policy.rb +++ /dev/null @@ -1,18 +0,0 @@ -class OmniauthVerificationPolicy - attr_reader :auth - - def initialize(auth) - @auth = auth - end - - def verified? - case auth.provider - when "facebook" - auth.info.verified? - when "google_oauth2" - auth.extra.raw_info.email_verified? - else - fail ArgumentError, "Verification checking is not implemented for provider: '#{auth.provider}'" - end - end -end diff --git a/app/views/application/_navigation_user.html.slim b/app/views/application/_navigation_user.html.slim index 28bf04a4..6c5d619d 100644 --- a/app/views/application/_navigation_user.html.slim +++ b/app/views/application/_navigation_user.html.slim @@ -13,11 +13,9 @@ ul.right - else = active_link_to 'Sign in', new_user_session_path, active: :exclusive, wrap_tag: :li = active_link_to 'Sign up', new_user_registration_path, active: :exclusive, wrap_tag: :li - = active_link_to "Sign in with Google", - user_omniauth_authorize_path(:google_oauth2), - active: :exclusive, - wrap_tag: :li - = active_link_to "Sign in with Facebook", - user_omniauth_authorize_path(:facebook), - active: :exclusive, - wrap_tag: :li + + - SocialProfile::PROVIDERS.each do |provider| + = active_link_to "Sign in with #{provider_name(provider)}", + user_omniauth_authorize_path(provider), + active: :exclusive, + wrap_tag: :li diff --git a/app/views/social_profiles/_list.html.slim b/app/views/social_profiles/_list.html.slim index 972b1b3f..69571677 100644 --- a/app/views/social_profiles/_list.html.slim +++ b/app/views/social_profiles/_list.html.slim @@ -1,21 +1,14 @@ -h6 - - if social_profiles.any? - b Linked social networks: - ul.js-social-profiles - - social_profiles.each do |social_profile| - li - = t "active_record.attributes.social_profile.provider_name.#{social_profile.provider}" - | (#{social_profile.uid}) - |   - = link_to "x", - social_profile, - data: { confirm: "Are you sure you want to remove this social profile?" }, - method: :delete - b Add another service to sign in with: - - else - b Sign in through one of these services: +- if social_profiles.any? + b Successfully authorized via: + ul.js-social-profiles + - social_profiles.each do |social_profile| + li = link_to "#{provider_name(social_profile.provider)} (#{social_profile.uid.truncate(9)}). Unauthorize?", + social_profile, + data: { confirm: "Are you sure you want to remove this social profile?" }, + method: :delete, + class: "js-unauthorize" - p - = link_to "Google", user_omniauth_authorize_path(:google_oauth2) - |   - = link_to "Facebook", user_omniauth_authorize_path(:facebook) +b Add service to sign in with: +ul + - SocialProfile::PROVIDERS.each do |provider| + li = link_to provider_name(provider), user_omniauth_authorize_path(provider) diff --git a/app/views/users/registrations/edit.html.slim b/app/views/users/registrations/edit.html.slim index a18583d0..dcd25388 100644 --- a/app/views/users/registrations/edit.html.slim +++ b/app/views/users/registrations/edit.html.slim @@ -24,7 +24,7 @@ = f.button :submit, 'Update' .medium-6.columns.end - = render "social_profiles/list", social_profiles: social_profiles + = render "social_profiles/list", social_profiles: current_user.social_profiles h6 b Cancel my account diff --git a/config/locales/flash.en.yml b/config/locales/flash.en.yml index ae860c4a..2b319d66 100644 --- a/config/locales/flash.en.yml +++ b/config/locales/flash.en.yml @@ -14,10 +14,3 @@ en: feedbacks: create: notice: "Email was successfully sent." - - omniauth: - social_profile_created: 'Social profile was successfully created.' - already_linked: 'Social profile already linked.' - sign_in: 'Signed in successfully.' - sign_up: 'Signed up successfully.' - not_verified: 'Social account is not verified.' diff --git a/config/locales/oauth.en.yml b/config/locales/oauth.en.yml new file mode 100644 index 00000000..9f69b348 --- /dev/null +++ b/config/locales/oauth.en.yml @@ -0,0 +1,5 @@ +en: + omniauth: + verification: + failure: Your %{kind} account can't be used to sign in. Please verify it via profile page. + not_implemented: Verification checking is not implemented for %{kind}. diff --git a/config/rails_best_practices.yml b/config/rails_best_practices.yml index a9ebd07e..a2db146b 100644 --- a/config/rails_best_practices.yml +++ b/config/rails_best_practices.yml @@ -28,9 +28,9 @@ RemoveTrailingWhitespaceCheck: { } RemoveUnusedMethodsInControllersCheck: except_methods: - ApplicationController#devise_parameter_sanitizer - - Users::RegistrationsController#create - - Users::RegistrationsController#build_resource - - Users::RegistrationsController#devise_parameter_sanitizer + - RegistrationsController#create + - RegistrationsController#build_resource + - RegistrationsController#devise_parameter_sanitizer RemoveUnusedMethodsInHelpersCheck: { except_methods: [] } RemoveUnusedMethodsInModelsCheck: { except_methods: [] } ReplaceComplexCreationWithFactoryMethodCheck: { attribute_assignment_count: 2 } diff --git a/config/routes.rb b/config/routes.rb index 19b0936c..791f8643 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,6 +1,6 @@ Rails.application.routes.draw do devise_for :users, - controllers: { omniauth_callbacks: "users/omniauth_callbacks", registrations: "users/registrations" } + controllers: { omniauth_callbacks: "omniauth_callbacks", registrations: "registrations" } resource :feedback, only: %i(new create) resources :social_profiles, only: :destroy diff --git a/spec/features/user/account/social_profiles/add_remove_spec.rb b/spec/features/user/account/social_profiles/add_remove_spec.rb index 61e0b9e7..e7cb7f0e 100644 --- a/spec/features/user/account/social_profiles/add_remove_spec.rb +++ b/spec/features/user/account/social_profiles/add_remove_spec.rb @@ -1,94 +1,51 @@ require "rails_helper" feature "Add/Remove social profiles" do - let(:uid) { "12345" } let(:user) { create(:user, :confirmed) } let(:user_attributes) { user.attributes.slice(:full_name, :email) } - let(:omniauth_params) { omniauth_mock(provider, uid, user_attributes) } + let(:oauth) { omniauth_mock(provider, "12345", user_attributes) } before do - stub_omniauth(provider, omniauth_params) - allow(OmniauthVerificationPolicy).to receive_message_chain(:new, :verified?).and_return(verified) + stub_omniauth(provider, oauth) + allow(AuthVerificationPolicy).to receive(:verified?) login_as user visit edit_user_registration_path end - context "when social account is verified" do - let(:verified) { true } + context "when provider is Facebook" do + let(:provider) { :facebook } - context "when provider is Facebook" do - let(:provider) { :facebook } + scenario "user adds social profile" do + expect(page).to have_link("Facebook") + expect(page).not_to have_content("Successfully authorized via:") - scenario "user adds social profile" do - expect(page).to have_link("Facebook") - expect(page).not_to have_content("Linked social networks") + click_link "Facebook" + expect(page).to have_content(I18n.t "devise.omniauth_callbacks.success", kind: "Facebook") + expect(page).to have_content("Successfully authorized via:") + expect(page).to have_css(".js-social-profiles", text: "Facebook") - click_link "Facebook" - expect(page).to have_content(I18n.t "flash.omniauth.social_profile_created") - expect(page).to have_content("Linked social networks") - expect(page).to have_css(".js-social-profiles", text: "Facebook") - - click_link "Facebook" - expect(page).to have_content(I18n.t "flash.omniauth.already_linked") - - click_link "x" - expect(page).to have_content(I18n.t "flash.actions.destroy.notice", resource_name: social_profile_name) - expect(page).not_to have_content("Linked social networks") - end - end - - context "when provider is Google" do - let(:provider) { :google_oauth2 } - - scenario "user adds social profile" do - expect(page).to have_link("Google") - expect(page).not_to have_content("Linked social networks") - - click_link "Google" - expect(page).to have_content(I18n.t "flash.omniauth.social_profile_created") - expect(page).to have_content("Linked social networks") - expect(page).to have_css(".js-social-profiles", text: "Google") - - click_link "Google" - expect(page).to have_content(I18n.t "flash.omniauth.already_linked") - - click_link "x" - expect(page).to have_content(I18n.t "flash.actions.destroy.notice", resource_name: social_profile_name) - expect(page).not_to have_content("Linked social networks") - end + find(:css, ".js-unauthorize").click + expect(page).to have_content(I18n.t "flash.actions.destroy.notice", resource_name: social_profile_name) + expect(page).not_to have_content("Successfully authorized via:") end end - context "when social account is not verified" do - let(:verified) { false } - - context "when provider is Facebook" do - let(:provider) { :facebook } - - scenario "user not able to add social profile" do - expect(page).to have_link("Facebook") - expect(page).not_to have_content("Linked social networks") - - click_link "Facebook" - expect(page).to have_content(I18n.t "flash.omniauth.not_verified") - expect(page).not_to have_content("Linked social networks") - expect(page).not_to have_css(".js-social-profiles", text: "Facebook") - end - end + context "when provider is Google" do + let(:provider) { :google_oauth2 } - context "when provider is Google" do - let(:provider) { :google_oauth2 } + scenario "user adds social profile" do + expect(page).to have_link("Google") + expect(page).not_to have_content("Successfully authorized via:") - scenario "user not able to add social profile" do - expect(page).to have_link("Google") - expect(page).not_to have_content("Linked social networks") + click_link "Google" + expect(page).to have_content(I18n.t "devise.omniauth_callbacks.success", kind: "Google") + expect(page).to have_content("Successfully authorized via:") + expect(page).to have_css(".js-social-profiles", text: "Google") - click_link "Google" - expect(page).to have_content(I18n.t "flash.omniauth.not_verified") - expect(page).not_to have_content("Linked social networks") - expect(page).not_to have_css(".js-social-profiles", text: "Google") - end + find(:css, ".js-unauthorize").click + expect(page).to have_content(I18n.t "flash.actions.destroy.notice", resource_name: social_profile_name) + expect(page).not_to have_content("Successfully authorized via:") end end diff --git a/spec/features/visitor/social_profiles/sign_up_spec.rb b/spec/features/visitor/social_profiles/sign_up_spec.rb index f8bf1990..2ac441e0 100644 --- a/spec/features/visitor/social_profiles/sign_up_spec.rb +++ b/spec/features/visitor/social_profiles/sign_up_spec.rb @@ -1,94 +1,84 @@ require "rails_helper" feature "Sign Up" do - let(:uid) { "12345" } let(:registered_user) { User.find_by_email(user_attributes[:email]) } - let(:omniauth_params) { omniauth_mock(provider, uid, user_attributes) } - - let(:user_attributes) do - ActiveSupport::HashWithIndifferentAccess.new( - attributes_for(:user).slice(:full_name, :email, :password, :password_confirmation) - ) - end + let(:oauth) { omniauth_mock(provider, "12345", user_attributes) } + let(:user_attributes) { attributes_for(:user).slice(:full_name, :email, :password, :password_confirmation) } before do - stub_omniauth(provider, omniauth_params) - allow(OmniauthVerificationPolicy).to receive_message_chain(:new, :verified?).and_return(verified) + stub_omniauth(provider, oauth) + allow(AuthVerificationPolicy).to receive(:verified?).and_return(verified) visit root_path end - context "when social account is verified" do + context "when user is not persisted" do let(:verified) { true } - context "when user and social profile not exist" do - context "when provider is Google" do - let(:provider) { :google_oauth2 } + context "when provider is Google" do + let(:provider) { :google_oauth2 } - scenario "Visitor signs up through provider" do - click_link "Sign in with Google" - expect(page).to have_field("user_full_name", with: user_attributes[:full_name]) - expect(page).to have_field("user_email", with: user_attributes[:email]) + scenario "Visitor signs up through provider" do + click_link "Sign in with Google" + expect(page).to have_field("user_full_name", with: user_attributes[:full_name]) + expect(page).to have_field("user_email", with: user_attributes[:email]) - fill_in "user_password", with: user_attributes[:password] - fill_in "user_password_confirmation", with: user_attributes[:password_confirmation] - click_button "Sign up" + fill_in "user_password", with: user_attributes[:password] + fill_in "user_password_confirmation", with: user_attributes[:password_confirmation] + click_button "Sign up" - expect(page).to have_content(I18n.t "devise.registrations.signed_up") - expect(page).to have_text(registered_user.email) - end + expect(page).to have_content(I18n.t "devise.registrations.signed_up") + expect(page).to have_text(registered_user.email) end + end - context "when provider is Facebook" do - let(:provider) { :facebook } + context "when provider is Facebook" do + let(:provider) { :facebook } - scenario "Visitor signs up through provider" do - click_link "Sign in with Facebook" - expect(page).to have_field("user_full_name", with: user_attributes[:full_name]) - expect(page).to have_field("user_email", with: user_attributes[:email]) + scenario "Visitor signs up through provider" do + click_link "Sign in with Facebook" + expect(page).to have_field("user_full_name", with: user_attributes[:full_name]) + expect(page).to have_field("user_email", with: user_attributes[:email]) - fill_in "user_password", with: user_attributes[:password] - fill_in "user_password_confirmation", with: user_attributes[:password_confirmation] - click_button "Sign up" + fill_in "user_password", with: user_attributes[:password] + fill_in "user_password_confirmation", with: user_attributes[:password_confirmation] + click_button "Sign up" - expect(page).to have_content(I18n.t "devise.registrations.signed_up") - expect(page).to have_text(registered_user.email) - end + expect(page).to have_content(I18n.t "devise.registrations.signed_up") + expect(page).to have_text(registered_user.email) end end + end - context "when user and social profile exist" do - let(:user) { create(:user, :confirmed, user_attributes) } + context "when user is persisted" do + let(:verified) { true } - before do - user.social_profiles.create(provider: provider, uid: uid) - end + before { create(:user, user_attributes) } - context "when provider is Google" do - let(:provider) { :google_oauth2 } + context "when provider is Google" do + let(:provider) { :google_oauth2 } - scenario "Visitor signs in through provider" do - click_link "Sign in with Google" + scenario "Visitor signs in through provider" do + click_link "Sign in with Google" - expect(page).to have_content(I18n.t "flash.omniauth.already_linked") - expect(page).to have_text(registered_user.email) - end + expect(page).to have_content(I18n.t "devise.omniauth_callbacks.success", kind: "Google") + expect(page).to have_text(registered_user.email) end + end - context "when provider is Facebook" do - let(:provider) { :facebook } + context "when provider is Facebook" do + let(:provider) { :facebook } - scenario "Visitor signs in through provider" do - click_link "Sign in with Facebook" + scenario "Visitor signs in through provider" do + click_link "Sign in with Facebook" - expect(page).to have_content(I18n.t "flash.omniauth.already_linked") - expect(page).to have_text(registered_user.email) - end + expect(page).to have_content(I18n.t "devise.omniauth_callbacks.success", kind: "Facebook") + expect(page).to have_text(registered_user.email) end end end - context "when social account is not verified" do + context "when oauth can't be used for authentication" do let(:verified) { false } context "when provider is Google" do @@ -97,7 +87,7 @@ scenario "Visitor not able to sign up through provider" do click_link "Sign in with Google" - expect(page).to have_content(I18n.t "flash.omniauth.not_verified") + expect(page).to have_content(I18n.t "omniauth.verification.failure", kind: "Google") expect(page).not_to have_text(user_attributes[:email]) end end @@ -108,7 +98,7 @@ scenario "Visitor not able to sign up through provider" do click_link "Sign in with Facebook" - expect(page).to have_content(I18n.t "flash.omniauth.not_verified") + expect(page).to have_content(I18n.t "omniauth.verification.failure", kind: "Facebook") expect(page).not_to have_text(user_attributes[:email]) end end diff --git a/spec/interactors/fetch_oauth_user_spec.rb b/spec/interactors/fetch_oauth_user_spec.rb new file mode 100644 index 00000000..6af1daa8 --- /dev/null +++ b/spec/interactors/fetch_oauth_user_spec.rb @@ -0,0 +1,55 @@ +require "rails_helper" + +describe FetchOauthUser do + let(:auth) { omniauth_mock("facebook", "12345", user_attributes) } + let(:service) { described_class.new(auth) } + let(:user_attributes) { attributes_for(:user).slice(:full_name, :email) } + + subject { service.call } + + before do + allow(AuthVerificationPolicy).to receive(:verified?).and_return(verified) + end + + context "when user is persisted" do + let!(:user) { create(:user, user_attributes) } + + context "when auth verified" do + let(:verified) { true } + + it { is_expected.to eq(user) } + end + + context "when auth is not verified" do + let(:verified) { false } + + it { is_expected.not_to eq(user) } + end + end + + context "when social_profile persisted" do + let!(:social_profile) { create(:social_profile, user: user) } + + let(:user) { create(:user, user_attributes) } + let(:verified) { false } + + it { is_expected.to eq(social_profile.user) } + end + + context "when user and social profile are not persisted" do + context "when auth is verified" do + let(:verified) { true } + + it "returns new user" do + expect(subject).to be_a_new_record + expect(subject).to be_kind_of(User) + end + end + + context "when auth is not verified" do + let(:verified) { false } + + it { is_expected.to eq(nil) } + end + end +end diff --git a/spec/interactors/oauth_organizer_spec.rb b/spec/interactors/oauth_organizer_spec.rb new file mode 100644 index 00000000..30891448 --- /dev/null +++ b/spec/interactors/oauth_organizer_spec.rb @@ -0,0 +1,30 @@ +require "rails_helper" + +describe OauthOrganizer do + let(:oauth) { omniauth_mock("facebook", "12345") } + let(:service) { described_class.new(current_user, oauth) } + + subject { service.call } + + context "when user present" do + let(:current_user) { create(:user) } + + it "connects new social_profile" do + expect { subject }.to change { current_user.social_profiles.count }.by(1) + expect(subject).to eq(current_user) + end + end + + context "when user not present" do + before do + allow(FetchOauthUser).to receive_message_chain(:new, :call) + allow(AuthVerificationPolicy).to receive(:verified?) + end + + let(:current_user) { nil } + + it "fails OauthError" do + expect { subject }.to raise_error(OauthOrganizer::OauthError) + end + end +end diff --git a/spec/policies/omniauth_verification_policy_spec.rb b/spec/policies/auth_verification_policy_spec.rb similarity index 73% rename from spec/policies/omniauth_verification_policy_spec.rb rename to spec/policies/auth_verification_policy_spec.rb index 0845e0f9..b70aca0e 100644 --- a/spec/policies/omniauth_verification_policy_spec.rb +++ b/spec/policies/auth_verification_policy_spec.rb @@ -1,16 +1,17 @@ require "rails_helper" -describe OmniauthVerificationPolicy do +describe AuthVerificationPolicy do let(:auth) { double(:omniauth, provider: provider) } - describe "#verified?" do - subject { described_class.new(auth).verified? } + describe ".verified?" do + subject { described_class.verified?(auth) } context "when provider is Facebook" do let(:provider) { "facebook" } before do allow(auth).to receive_message_chain(:info, :verified?).and_return(true) + allow(auth).to receive_message_chain(:extra, :raw_info, :verified?).and_return(true) end it "returns corresponding value" do @@ -35,7 +36,7 @@ it "raises Exception" do expect { subject } - .to raise_error(ArgumentError, "Verification checking is not implemented for provider: '#{auth.provider}'") + .to raise_error(ArgumentError, I18n.t("omniauth.verification.not_implemented", kind: provider)) end end end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 7177eac9..c1f2cbd8 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -10,4 +10,6 @@ RSpec.configure do |config| config.use_transactional_fixtures = false config.infer_spec_type_from_file_location! + + config.include OauthHelpers end diff --git a/spec/support/features/feature_helpers.rb b/spec/support/oauth_helpers.rb similarity index 89% rename from spec/support/features/feature_helpers.rb rename to spec/support/oauth_helpers.rb index f387c246..478599ae 100644 --- a/spec/support/features/feature_helpers.rb +++ b/spec/support/oauth_helpers.rb @@ -1,4 +1,4 @@ -module FeatureHelpers +module OauthHelpers def stub_omniauth(provider, omniauth_mock) OmniAuth.config.test_mode = true OmniAuth.config.mock_auth[provider] = omniauth_mock @@ -7,7 +7,7 @@ def stub_omniauth(provider, omniauth_mock) def omniauth_mock(provider, uid, user_attrs = {}) OmniAuth::AuthHash.new( - provider: provider, + provider: provider.to_s, uid: uid, info: { name: user_attrs[:full_name], email: user_attrs[:email] } ) From 14072010f06a6779bf7823569df691e167c16058 Mon Sep 17 00:00:00 2001 From: Ruslan Khaertdinov Date: Fri, 30 Oct 2015 22:46:56 +0300 Subject: [PATCH 06/10] rework oauth_organizer and fetch_oauth_user classes --- app/interactors/fetch_oauth_user.rb | 22 +++------------- app/interactors/oauth_organizer.rb | 18 ++++++++++++- spec/interactors/fetch_oauth_user_spec.rb | 32 ++--------------------- 3 files changed, 22 insertions(+), 50 deletions(-) diff --git a/app/interactors/fetch_oauth_user.rb b/app/interactors/fetch_oauth_user.rb index e680d23a..49c2a34e 100644 --- a/app/interactors/fetch_oauth_user.rb +++ b/app/interactors/fetch_oauth_user.rb @@ -7,32 +7,16 @@ def initialize(auth) end def call - find_user_by_email || find_social_profile_user || build_user + find_user_by_email || find_social_profile_user end private def find_user_by_email - user_by_email if auth_verified? - end - - def user_by_email - @user_by_email ||= User.find_by(email: auth["info"]["email"]) - end - - def auth_verified? - @auth_verified ||= AuthVerificationPolicy.verified?(auth) + User.find_by(email: auth["info"]["email"]) end def find_social_profile_user - social_profile.try(:user) - end - - def social_profile - SocialProfile.from_omniauth(auth) - end - - def build_user - User.build_from_omniauth(auth) if auth_verified? && user_by_email.nil? + SocialProfile.from_omniauth(auth).try(:user) end end diff --git a/app/interactors/oauth_organizer.rb b/app/interactors/oauth_organizer.rb index 7c275714..662e0b51 100644 --- a/app/interactors/oauth_organizer.rb +++ b/app/interactors/oauth_organizer.rb @@ -18,7 +18,23 @@ def call private def user - @user ||= current_user || FetchOauthUser.new(auth).call + @user ||= current_user || fetch_oauth_user || build_user + end + + def fetch_oauth_user + FetchOauthUser.new(auth).call if auth_verified? + end + + def auth_verified? + AuthVerificationPolicy.verified?(auth) + end + + def build_user + User.build_from_omniauth(auth) if auth_verified? && !found_user_by_email? + end + + def found_user_by_email? + User.find_by(email: auth["info"]["email"]).present? end def fail_oauth diff --git a/spec/interactors/fetch_oauth_user_spec.rb b/spec/interactors/fetch_oauth_user_spec.rb index 6af1daa8..61809179 100644 --- a/spec/interactors/fetch_oauth_user_spec.rb +++ b/spec/interactors/fetch_oauth_user_spec.rb @@ -7,49 +7,21 @@ subject { service.call } - before do - allow(AuthVerificationPolicy).to receive(:verified?).and_return(verified) - end - context "when user is persisted" do let!(:user) { create(:user, user_attributes) } - context "when auth verified" do - let(:verified) { true } - - it { is_expected.to eq(user) } - end - - context "when auth is not verified" do - let(:verified) { false } - - it { is_expected.not_to eq(user) } - end + it { is_expected.to eq(user) } end context "when social_profile persisted" do let!(:social_profile) { create(:social_profile, user: user) } let(:user) { create(:user, user_attributes) } - let(:verified) { false } it { is_expected.to eq(social_profile.user) } end context "when user and social profile are not persisted" do - context "when auth is verified" do - let(:verified) { true } - - it "returns new user" do - expect(subject).to be_a_new_record - expect(subject).to be_kind_of(User) - end - end - - context "when auth is not verified" do - let(:verified) { false } - - it { is_expected.to eq(nil) } - end + it { is_expected.to eq(nil) } end end From 2690d1135b0d095c62daf24a6bd082a686f2c868 Mon Sep 17 00:00:00 2001 From: Ruslan Khaertdinov Date: Fri, 30 Oct 2015 23:06:52 +0300 Subject: [PATCH 07/10] extend user_spec --- app/models/user.rb | 8 +++++++- spec/models/user_spec.rb | 25 +++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/app/models/user.rb b/app/models/user.rb index f3132181..237694dc 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -26,6 +26,12 @@ def apply_omniauth(auth) end def connect_social_profile(auth) - apply_omniauth(auth) && save unless SocialProfile.from_omniauth(auth) + social_profile = SocialProfile.from_omniauth(auth) + + if social_profile + social_profile.update_attribute(:user, self) + else + apply_omniauth(auth) && save + end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 5ca43167..49d7c839 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -3,4 +3,29 @@ describe User do it { is_expected.to validate_presence_of :full_name } it { is_expected.to have_many(:social_profiles).dependent(:destroy) } + + describe "#connect_social_profile" do + let(:user) { create(:user) } + let(:provider) { "facebook" } + let(:uid) { "12345" } + let(:auth) { omniauth_mock(provider, uid) } + + subject { user.connect_social_profile(auth) } + + context "when social_profile not exists" do + it "creates related social_profile" do + expect { subject }.to change { user.social_profiles.count }.by(1) + end + end + + context "when disconnected social profile exists" do + let!(:social_profile) { create(:social_profile, provider: provider, uid: uid, user: another_user) } + + let(:another_user) { create(:user) } + + it "changes user for given social_profile" do + expect { subject }.to change { social_profile.reload.user }.from(another_user).to(user) + end + end + end end From 95fa19c5fd7ef26960f42f6e3ef4ec485ccbb1ba Mon Sep 17 00:00:00 2001 From: Ruslan Khaertdinov Date: Mon, 9 Nov 2015 14:42:10 +0300 Subject: [PATCH 08/10] apply review comments: revise code --- app/interactors/fetch_oauth_user.rb | 10 ++--- app/interactors/oauth_organizer.rb | 2 +- app/policies/auth_verification_policy.rb | 38 ++++++++++++++----- .../social_profiles/add_remove_spec.rb | 6 +-- .../visitor/social_profiles/sign_up_spec.rb | 2 +- spec/interactors/oauth_organizer_spec.rb | 2 +- .../policies/auth_verification_policy_spec.rb | 2 +- 7 files changed, 41 insertions(+), 21 deletions(-) diff --git a/app/interactors/fetch_oauth_user.rb b/app/interactors/fetch_oauth_user.rb index 49c2a34e..199fc735 100644 --- a/app/interactors/fetch_oauth_user.rb +++ b/app/interactors/fetch_oauth_user.rb @@ -7,16 +7,16 @@ def initialize(auth) end def call - find_user_by_email || find_social_profile_user + find_social_profile_user || find_user_by_email end private - def find_user_by_email - User.find_by(email: auth["info"]["email"]) - end - def find_social_profile_user SocialProfile.from_omniauth(auth).try(:user) end + + def find_user_by_email + User.find_by(email: auth["info"]["email"]) + end end diff --git a/app/interactors/oauth_organizer.rb b/app/interactors/oauth_organizer.rb index 662e0b51..a2cf1e16 100644 --- a/app/interactors/oauth_organizer.rb +++ b/app/interactors/oauth_organizer.rb @@ -26,7 +26,7 @@ def fetch_oauth_user end def auth_verified? - AuthVerificationPolicy.verified?(auth) + AuthVerificationPolicy.new(auth).verified? end def build_user diff --git a/app/policies/auth_verification_policy.rb b/app/policies/auth_verification_policy.rb index ae9d7418..a113c04e 100644 --- a/app/policies/auth_verification_policy.rb +++ b/app/policies/auth_verification_policy.rb @@ -1,12 +1,32 @@ class AuthVerificationPolicy - def self.verified?(auth) - case auth.provider - when "facebook" - auth.info.verified? || auth.extra.raw_info.verified? - when "google_oauth2" - auth.extra.raw_info.email_verified? - else - fail ArgumentError, I18n.t("omniauth.verification.not_implemented", kind: auth.provider) - end + attr_reader :auth + private :auth + + def initialize(auth) + @auth = auth + end + + def verified? + request_verification_for + rescue NoMethodError + fail_with_error + end + + private + + def request_verification_for + send(auth.provider) + end + + def fail_with_error + fail ArgumentError, I18n.t("omniauth.verification.not_implemented", kind: auth.provider) + end + + def facebook + auth.info.verified? || auth.extra.raw_info.verified? + end + + def google_oauth2 + auth.extra.raw_info.email_verified? end end diff --git a/spec/features/user/account/social_profiles/add_remove_spec.rb b/spec/features/user/account/social_profiles/add_remove_spec.rb index e7cb7f0e..7a5ed84e 100644 --- a/spec/features/user/account/social_profiles/add_remove_spec.rb +++ b/spec/features/user/account/social_profiles/add_remove_spec.rb @@ -7,7 +7,7 @@ before do stub_omniauth(provider, oauth) - allow(AuthVerificationPolicy).to receive(:verified?) + allow_any_instance_of(AuthVerificationPolicy).to receive(:verified?) login_as user visit edit_user_registration_path @@ -21,7 +21,7 @@ expect(page).not_to have_content("Successfully authorized via:") click_link "Facebook" - expect(page).to have_content(I18n.t "devise.omniauth_callbacks.success", kind: "Facebook") + expect(page).to have_content("Successfully authenticated from Facebook account.") expect(page).to have_content("Successfully authorized via:") expect(page).to have_css(".js-social-profiles", text: "Facebook") @@ -39,7 +39,7 @@ expect(page).not_to have_content("Successfully authorized via:") click_link "Google" - expect(page).to have_content(I18n.t "devise.omniauth_callbacks.success", kind: "Google") + expect(page).to have_content("Successfully authenticated from Google account.") expect(page).to have_content("Successfully authorized via:") expect(page).to have_css(".js-social-profiles", text: "Google") diff --git a/spec/features/visitor/social_profiles/sign_up_spec.rb b/spec/features/visitor/social_profiles/sign_up_spec.rb index 2ac441e0..fcbdbb76 100644 --- a/spec/features/visitor/social_profiles/sign_up_spec.rb +++ b/spec/features/visitor/social_profiles/sign_up_spec.rb @@ -7,7 +7,7 @@ before do stub_omniauth(provider, oauth) - allow(AuthVerificationPolicy).to receive(:verified?).and_return(verified) + allow_any_instance_of(AuthVerificationPolicy).to receive(:verified?).and_return(verified) visit root_path end diff --git a/spec/interactors/oauth_organizer_spec.rb b/spec/interactors/oauth_organizer_spec.rb index 30891448..1e21f845 100644 --- a/spec/interactors/oauth_organizer_spec.rb +++ b/spec/interactors/oauth_organizer_spec.rb @@ -18,7 +18,7 @@ context "when user not present" do before do allow(FetchOauthUser).to receive_message_chain(:new, :call) - allow(AuthVerificationPolicy).to receive(:verified?) + allow_any_instance_of(AuthVerificationPolicy).to receive(:verified?) end let(:current_user) { nil } diff --git a/spec/policies/auth_verification_policy_spec.rb b/spec/policies/auth_verification_policy_spec.rb index b70aca0e..fb4f6fdf 100644 --- a/spec/policies/auth_verification_policy_spec.rb +++ b/spec/policies/auth_verification_policy_spec.rb @@ -4,7 +4,7 @@ let(:auth) { double(:omniauth, provider: provider) } describe ".verified?" do - subject { described_class.verified?(auth) } + subject { described_class.new(auth).verified? } context "when provider is Facebook" do let(:provider) { "facebook" } From 11a7531b30ed9597156f59543f6174c7b532e52b Mon Sep 17 00:00:00 2001 From: Ruslan Khaertdinov Date: Wed, 11 Nov 2015 13:19:48 +0300 Subject: [PATCH 09/10] apply review comments: refactor feature specs --- .../social_profiles/add_remove_spec.rb | 43 +++++++++--------- .../visitor/social_profiles/sign_up_spec.rb | 45 +++++++++++-------- 2 files changed, 50 insertions(+), 38 deletions(-) diff --git a/spec/features/user/account/social_profiles/add_remove_spec.rb b/spec/features/user/account/social_profiles/add_remove_spec.rb index 7a5ed84e..18c0470e 100644 --- a/spec/features/user/account/social_profiles/add_remove_spec.rb +++ b/spec/features/user/account/social_profiles/add_remove_spec.rb @@ -4,6 +4,10 @@ let(:user) { create(:user, :confirmed) } let(:user_attributes) { user.attributes.slice(:full_name, :email) } let(:oauth) { omniauth_mock(provider, "12345", user_attributes) } + let(:authentication_message) { "Successfully authenticated from #{provider_name} account." } + let(:unlink_message) { "Social profile was successfully destroyed." } + let(:unauthorize_link) { find(:css, ".js-unauthorize") } + let(:provider_name) { I18n.t("active_record.attributes.social_profile.provider_name.#{provider}") } before do stub_omniauth(provider, oauth) @@ -17,17 +21,15 @@ let(:provider) { :facebook } scenario "user adds social profile" do - expect(page).to have_link("Facebook") - expect(page).not_to have_content("Successfully authorized via:") + expect(page).not_to have_providers_list(provider_name) - click_link "Facebook" - expect(page).to have_content("Successfully authenticated from Facebook account.") - expect(page).to have_content("Successfully authorized via:") - expect(page).to have_css(".js-social-profiles", text: "Facebook") + click_link provider_name + expect(page).to have_text(authentication_message) + expect(page).to have_providers_list(provider_name) - find(:css, ".js-unauthorize").click - expect(page).to have_content(I18n.t "flash.actions.destroy.notice", resource_name: social_profile_name) - expect(page).not_to have_content("Successfully authorized via:") + unauthorize_link.click + expect(page).to have_text(unlink_message) + expect(page).not_to have_providers_list(provider_name) end end @@ -35,21 +37,22 @@ let(:provider) { :google_oauth2 } scenario "user adds social profile" do - expect(page).to have_link("Google") - expect(page).not_to have_content("Successfully authorized via:") + expect(page).not_to have_providers_list(provider_name) - click_link "Google" - expect(page).to have_content("Successfully authenticated from Google account.") - expect(page).to have_content("Successfully authorized via:") - expect(page).to have_css(".js-social-profiles", text: "Google") + click_link provider_name + expect(page).to have_text(authentication_message) + expect(page).to have_providers_list(provider_name) - find(:css, ".js-unauthorize").click - expect(page).to have_content(I18n.t "flash.actions.destroy.notice", resource_name: social_profile_name) - expect(page).not_to have_content("Successfully authorized via:") + unauthorize_link.click + expect(page).to have_text(unlink_message) + expect(page).not_to have_providers_list(provider_name) end end - def social_profile_name - SocialProfile.model_name.human + # rubocop:disable Style/PredicateName + def have_providers_list(provider) + have_text("Successfully authorized via:") + have_css(".js-social-profiles", text: provider) end + # rubocop:enable Style/PredicateName end diff --git a/spec/features/visitor/social_profiles/sign_up_spec.rb b/spec/features/visitor/social_profiles/sign_up_spec.rb index fcbdbb76..c2d4354d 100644 --- a/spec/features/visitor/social_profiles/sign_up_spec.rb +++ b/spec/features/visitor/social_profiles/sign_up_spec.rb @@ -4,6 +4,7 @@ let(:registered_user) { User.find_by_email(user_attributes[:email]) } let(:oauth) { omniauth_mock(provider, "12345", user_attributes) } let(:user_attributes) { attributes_for(:user).slice(:full_name, :email, :password, :password_confirmation) } + let(:provider_name) { I18n.t("active_record.attributes.social_profile.provider_name.#{provider}") } before do stub_omniauth(provider, oauth) @@ -14,20 +15,17 @@ context "when user is not persisted" do let(:verified) { true } + let(:success_message) { "You have signed up successfully." } context "when provider is Google" do let(:provider) { :google_oauth2 } scenario "Visitor signs up through provider" do click_link "Sign in with Google" - expect(page).to have_field("user_full_name", with: user_attributes[:full_name]) - expect(page).to have_field("user_email", with: user_attributes[:email]) + expect(page).to have_prefilled_fields(user_attributes) - fill_in "user_password", with: user_attributes[:password] - fill_in "user_password_confirmation", with: user_attributes[:password_confirmation] - click_button "Sign up" - - expect(page).to have_content(I18n.t "devise.registrations.signed_up") + fill_profile_with(user_attributes) + expect(page).to have_text(success_message) expect(page).to have_text(registered_user.email) end end @@ -37,14 +35,10 @@ scenario "Visitor signs up through provider" do click_link "Sign in with Facebook" - expect(page).to have_field("user_full_name", with: user_attributes[:full_name]) - expect(page).to have_field("user_email", with: user_attributes[:email]) - - fill_in "user_password", with: user_attributes[:password] - fill_in "user_password_confirmation", with: user_attributes[:password_confirmation] - click_button "Sign up" + expect(page).to have_prefilled_fields(user_attributes) - expect(page).to have_content(I18n.t "devise.registrations.signed_up") + fill_profile_with(user_attributes) + expect(page).to have_text(success_message) expect(page).to have_text(registered_user.email) end end @@ -52,6 +46,7 @@ context "when user is persisted" do let(:verified) { true } + let(:success_message) { "Successfully authenticated from #{provider_name} account." } before { create(:user, user_attributes) } @@ -61,7 +56,7 @@ scenario "Visitor signs in through provider" do click_link "Sign in with Google" - expect(page).to have_content(I18n.t "devise.omniauth_callbacks.success", kind: "Google") + expect(page).to have_text(success_message) expect(page).to have_text(registered_user.email) end end @@ -72,7 +67,7 @@ scenario "Visitor signs in through provider" do click_link "Sign in with Facebook" - expect(page).to have_content(I18n.t "devise.omniauth_callbacks.success", kind: "Facebook") + expect(page).to have_text(success_message) expect(page).to have_text(registered_user.email) end end @@ -80,6 +75,7 @@ context "when oauth can't be used for authentication" do let(:verified) { false } + let(:error_msg) { "Your #{provider_name} account can't be used to sign in. Please verify it via profile page." } context "when provider is Google" do let(:provider) { :google_oauth2 } @@ -87,7 +83,7 @@ scenario "Visitor not able to sign up through provider" do click_link "Sign in with Google" - expect(page).to have_content(I18n.t "omniauth.verification.failure", kind: "Google") + expect(page).to have_text(error_msg) expect(page).not_to have_text(user_attributes[:email]) end end @@ -98,9 +94,22 @@ scenario "Visitor not able to sign up through provider" do click_link "Sign in with Facebook" - expect(page).to have_content(I18n.t "omniauth.verification.failure", kind: "Facebook") + expect(page).to have_text(error_msg) expect(page).not_to have_text(user_attributes[:email]) end end end + + # rubocop:disable Style/PredicateName + def have_prefilled_fields(opts = {}) + have_field("user_full_name", with: opts[:full_name]) + have_field("user_email", with: opts[:email]) + end + # rubocop:enable Style/PredicateName + + def fill_profile_with(opts = {}) + fill_in "user_password", with: opts[:password] + fill_in "user_password_confirmation", with: opts[:password_confirmation] + click_button "Sign up" + end end From 55525949a1174bc1e5e4dac7693a46f5b1af89d2 Mon Sep 17 00:00:00 2001 From: Ruslan Khaertdinov Date: Thu, 19 Nov 2015 22:36:46 +0300 Subject: [PATCH 10/10] apply review comments: extract user#connect_social_profile into service, remove unused checks from OauthOrganizer --- app/interactors/connect_social_profile.rb | 23 ++++++++++++++++ app/interactors/oauth_organizer.rb | 8 +++--- app/models/user.rb | 10 ------- .../connect_social_profile_spec.rb | 27 +++++++++++++++++++ spec/models/user_spec.rb | 25 ----------------- 5 files changed, 54 insertions(+), 39 deletions(-) create mode 100644 app/interactors/connect_social_profile.rb create mode 100644 spec/interactors/connect_social_profile_spec.rb diff --git a/app/interactors/connect_social_profile.rb b/app/interactors/connect_social_profile.rb new file mode 100644 index 00000000..de94d79a --- /dev/null +++ b/app/interactors/connect_social_profile.rb @@ -0,0 +1,23 @@ +class ConnectSocialProfile + attr_reader :user, :auth + private :user, :auth + + def initialize(user, auth) + @user = user + @auth = auth + end + + def call + if social_profile + social_profile.update_attribute(:user, user) + else + user.apply_omniauth(auth) && user.save + end + end + + private + + def social_profile + @social_profile ||= SocialProfile.from_omniauth(auth) + end +end diff --git a/app/interactors/oauth_organizer.rb b/app/interactors/oauth_organizer.rb index a2cf1e16..dfd3d36c 100644 --- a/app/interactors/oauth_organizer.rb +++ b/app/interactors/oauth_organizer.rb @@ -11,7 +11,7 @@ def initialize(current_user, auth) end def call - user.present? ? user.connect_social_profile(auth) : fail_oauth + user.present? ? connect_social_profile : fail_oauth user end @@ -30,11 +30,11 @@ def auth_verified? end def build_user - User.build_from_omniauth(auth) if auth_verified? && !found_user_by_email? + User.build_from_omniauth(auth) if auth_verified? end - def found_user_by_email? - User.find_by(email: auth["info"]["email"]).present? + def connect_social_profile + ConnectSocialProfile.new(user, auth).call end def fail_oauth diff --git a/app/models/user.rb b/app/models/user.rb index 237694dc..d823ebba 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -24,14 +24,4 @@ def apply_omniauth(auth) self.full_name = auth["info"]["name"] if full_name.blank? social_profiles.build(provider: auth["provider"], uid: auth["uid"]) end - - def connect_social_profile(auth) - social_profile = SocialProfile.from_omniauth(auth) - - if social_profile - social_profile.update_attribute(:user, self) - else - apply_omniauth(auth) && save - end - end end diff --git a/spec/interactors/connect_social_profile_spec.rb b/spec/interactors/connect_social_profile_spec.rb new file mode 100644 index 00000000..f127d6b9 --- /dev/null +++ b/spec/interactors/connect_social_profile_spec.rb @@ -0,0 +1,27 @@ +require "rails_helper" + +describe ConnectSocialProfile do + let(:user) { create(:user) } + let(:provider) { "facebook" } + let(:uid) { "12345" } + let(:auth) { omniauth_mock(provider, uid) } + let(:service) { described_class.new(user, auth) } + + subject { service.call } + + context "when social_profile not exists" do + it "creates related social_profile" do + expect { subject }.to change { user.social_profiles.count }.by(1) + end + end + + context "when disconnected social profile exists" do + let!(:social_profile) { create(:social_profile, provider: provider, uid: uid, user: another_user) } + + let(:another_user) { create(:user) } + + it "changes user for given social_profile" do + expect { subject }.to change { social_profile.reload.user }.from(another_user).to(user) + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 49d7c839..5ca43167 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -3,29 +3,4 @@ describe User do it { is_expected.to validate_presence_of :full_name } it { is_expected.to have_many(:social_profiles).dependent(:destroy) } - - describe "#connect_social_profile" do - let(:user) { create(:user) } - let(:provider) { "facebook" } - let(:uid) { "12345" } - let(:auth) { omniauth_mock(provider, uid) } - - subject { user.connect_social_profile(auth) } - - context "when social_profile not exists" do - it "creates related social_profile" do - expect { subject }.to change { user.social_profiles.count }.by(1) - end - end - - context "when disconnected social profile exists" do - let!(:social_profile) { create(:social_profile, provider: provider, uid: uid, user: another_user) } - - let(:another_user) { create(:user) } - - it "changes user for given social_profile" do - expect { subject }.to change { social_profile.reload.user }.from(another_user).to(user) - end - end - end end