From 005ec84c73ec8a658202f3dc9dc6e8e083490849 Mon Sep 17 00:00:00 2001 From: Ahmad Farhat Date: Wed, 22 Jan 2020 16:32:56 -0500 Subject: [PATCH] GRN2-252: Change to how sign ins are processed (#869) * Social to local * Social/Local to Social * Rubocop fixes * Added test cases * Added the ability to clear social uids * Update admins_controller.rb * Update admins_controller.rb --- app/controllers/admins_controller.rb | 7 ++ app/controllers/concerns/authenticator.rb | 12 +++ app/controllers/password_resets_controller.rb | 4 +- app/controllers/sessions_controller.rb | 31 +++++++ .../admins/components/_settings.html.erb | 10 +++ config/locales/en.yml | 5 ++ config/routes.rb | 1 + spec/controllers/admins_controller_spec.rb | 30 +++++++ .../password_resets_controller_spec.rb | 2 +- spec/controllers/sessions_controller_spec.rb | 82 ++++++++++++++++++- 10 files changed, 181 insertions(+), 3 deletions(-) diff --git a/app/controllers/admins_controller.rb b/app/controllers/admins_controller.rb index 3ae5e28c..84395d70 100644 --- a/app/controllers/admins_controller.rb +++ b/app/controllers/admins_controller.rb @@ -175,6 +175,13 @@ class AdminsController < ApplicationController end end + # POST /admins/clear_auth + def clear_auth + User.include_deleted.where(provider: @user_domain).update_all(social_uid: nil) + + redirect_to admin_site_settings_path, flash: { success: I18n.t("administrator.flash.settings") } + end + # POST /admins/clear_cache def clear_cache Rails.cache.delete("#{@user_domain}/getUser") diff --git a/app/controllers/concerns/authenticator.rb b/app/controllers/concerns/authenticator.rb index f4dc3799..f935d6b4 100644 --- a/app/controllers/concerns/authenticator.rb +++ b/app/controllers/concerns/authenticator.rb @@ -68,6 +68,18 @@ module Authenticator session.delete(:user_id) if current_user end + # Check if the user is using local accounts + def auth_changed_to_local?(user) + Rails.configuration.loadbalanced_configuration && user.social_uid.present? && allow_greenlight_accounts? + end + + # Check if the user exists under the same email with no social uid and that social accounts are allowed + def auth_changed_to_social?(email) + Rails.configuration.loadbalanced_configuration && + User.exists?(email: email, provider: @user_domain, social_uid: nil) && + !allow_greenlight_accounts? + end + private # Migrates all of the twitter users rooms to the new account diff --git a/app/controllers/password_resets_controller.rb b/app/controllers/password_resets_controller.rb index 3c22a43e..1164e5b8 100644 --- a/app/controllers/password_resets_controller.rb +++ b/app/controllers/password_resets_controller.rb @@ -56,6 +56,8 @@ class PasswordResetsController < ApplicationController # Password does not match password confirmation flash.now[:alert] = I18n.t("password_different_notice") elsif @user.update_attributes(user_params) + # Clear the user's social uid if they are switching from a social to a local account + @user.update_attribute(:social_uid, nil) if @user.social_uid.present? # Successfully reset password return redirect_to root_path, flash: { success: I18n.t("password_reset_success") } end @@ -66,7 +68,7 @@ class PasswordResetsController < ApplicationController private def find_user - @user = User.find_by(email: params[:email]) + @user = User.find_by(email: params[:email], provider: @user_domain) end def user_params diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 11e7d0fe..90272c46 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -74,6 +74,10 @@ class SessionsController < ApplicationController # Check user with that email exists return redirect_to(signin_path, alert: I18n.t("invalid_credentials")) unless user + + # Check if authenticators have switched + return switch_account_to_local(user) if !is_super_admin && auth_changed_to_local?(user) + # Check correct password was entered return redirect_to(signin_path, alert: I18n.t("invalid_credentials")) unless user.try(:authenticate, session_params[:password]) @@ -199,6 +203,9 @@ class SessionsController < ApplicationController # If using invitation registration method, make sure user is invited return redirect_to root_path, flash: { alert: I18n.t("registration.invite.no_invite") } unless passes_invite_reqs + # Switch the user to a social account if they exist under the same email with no social uid + switch_account_to_social if !@user_exists && auth_changed_to_social?(@auth['info']['email']) + user = User.from_omniauth(@auth) logger.info "Support: Auth user #{user.email} is attempting to login." @@ -225,4 +232,28 @@ class SessionsController < ApplicationController end end end + + # Send the user a password reset email to allow them to set their password + def switch_account_to_local(user) + logger.info "Switching social account to local account for #{user.uid}" + + # Send the user a reset password email + user.create_reset_digest + send_password_reset_email(user) + + # Overwrite the flash with a more descriptive message if successful + flash[:success] = I18n.t("reset_password.auth_change") if flash[:success].present? + + redirect_to signin_path + end + + # Set the user's social id to the new id being passed + def switch_account_to_social + user = User.find_by(email: @auth['info']['email'], provider: @user_domain, social_uid: nil) + + logger.info "Switching account to social account for #{user.uid}" + + # Set the user's social id to the one being returned from auth + user.update_attribute(:social_uid, @auth['uid']) + end end diff --git a/app/views/admins/components/_settings.html.erb b/app/views/admins/components/_settings.html.erb index 0b2fd482..fa36afe1 100644 --- a/app/views/admins/components/_settings.html.erb +++ b/app/views/admins/components/_settings.html.erb @@ -155,6 +155,7 @@ <% if current_user.has_role? :super_admin%> +
@@ -164,4 +165,13 @@
+
+
+
+ + + <%= button_to t("administrator.site_settings.clear_auth.button"), admin_clear_auth_path, class: "btn btn-primary" %> +
+
+
<% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 06533a02..9f248bcf 100755 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -46,6 +46,10 @@ en: info: Clears the stored provider cache which forces a new request for the updated info title: Clear Provider Cache button: Clear Cache + clear_auth: + info: Clears the current authenticator for users allowing them to sign back in using a different authentication method + title: Clear Current Authenticator + button: Clear Auth color: info: Changing the Regular Color will change both Lighten and Darken. Lighten and Darken can then be changed individually title: Primary Color @@ -415,6 +419,7 @@ en: password: New Password confirm: New Password Confirmation update: Update Password + auth_change: The authentication method has changed. Please check your email to set your password. roles: active: Active admin: Admin diff --git a/config/routes.rb b/config/routes.rb index 47b8fd69..8dd8b80c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -55,6 +55,7 @@ Rails.application.routes.draw do post '/registration_method', to: 'admins#registration_method', as: :admin_change_registration post '/coloring', to: 'admins#coloring', as: :admin_coloring post '/clear_cache', to: 'admins#clear_cache', as: :admin_clear_cache + post '/clear_auth', to: 'admins#clear_auth', as: :admin_clear_auth # Roles post '/role', to: 'admins#new_role', as: :admin_new_role patch 'roles/order', to: 'admins#change_role_order', as: :admin_roles_order diff --git a/spec/controllers/admins_controller_spec.rb b/spec/controllers/admins_controller_spec.rb index 78069e69..26d91681 100644 --- a/spec/controllers/admins_controller_spec.rb +++ b/spec/controllers/admins_controller_spec.rb @@ -344,6 +344,36 @@ describe AdminsController, type: :controller do expect(response).to redirect_to(admin_site_settings_path) end end + + it "clears all users social uids if clear auth button is clicked" do + allow_any_instance_of(ApplicationController).to receive(:set_user_domain).and_return("provider1") + controller.instance_variable_set(:@user_domain, "provider1") + + @request.session[:user_id] = @admin.id + + @admin.add_role :super_admin + @admin.update_attribute(:provider, "greenlight") + @user2 = create(:user, provider: "provider1") + @user3 = create(:user, provider: "provider1") + + @user.update_attribute(:social_uid, Faker::Internet.password) + @user2.update_attribute(:social_uid, Faker::Internet.password) + @user3.update_attribute(:social_uid, Faker::Internet.password) + + expect(@user.social_uid).not_to be(nil) + expect(@user2.social_uid).not_to be(nil) + expect(@user3.social_uid).not_to be(nil) + + post :clear_auth + + @user.reload + @user2.reload + @user3.reload + + expect(@user.social_uid).to be(nil) + expect(@user2.social_uid).to be(nil) + expect(@user3.social_uid).to be(nil) + end end describe "Roles" do diff --git a/spec/controllers/password_resets_controller_spec.rb b/spec/controllers/password_resets_controller_spec.rb index c8b87664..2853ac8c 100644 --- a/spec/controllers/password_resets_controller_spec.rb +++ b/spec/controllers/password_resets_controller_spec.rb @@ -115,7 +115,7 @@ describe PasswordResetsController, type: :controller do end it "updates attributes if the password update is a success" do - user = create(:user) + user = create(:user, provider: "greenlight") token = "reset_token" cost = ActiveModel::SecurePassword.min_cost ? BCrypt::Engine::MIN_COST : BCrypt::Engine.cost diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index c0db4d12..c802bf67 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -88,7 +88,11 @@ describe SessionsController, type: :controller do end describe "POST #create" do - before { allow(Rails.configuration).to receive(:enable_email_verification).and_return(true) } + before do + allow(Rails.configuration).to receive(:enable_email_verification).and_return(true) + allow_any_instance_of(SessionsController).to receive(:auth_changed_to_local?).and_return(false) + end + before(:each) do @user1 = create(:user, provider: 'greenlight', password: 'example', password_confirmation: 'example') @user2 = create(:user, password: 'example', password_confirmation: "example") @@ -251,6 +255,22 @@ describe SessionsController, type: :controller do expect(@user1.rooms.find { |r| r.name == "Old Home Room" }).to_not be_nil expect(@user1.rooms.find { |r| r.name == "Test" }).to_not be_nil end + + it "sends the user a reset password email if the authentication method is changing to local" do + allow_any_instance_of(SessionsController).to receive(:auth_changed_to_local?).and_return(true) + email = Faker::Internet.email + + create(:user, email: email, provider: "greenlight", social_uid: "google-user") + + expect { + post :create, params: { + session: { + email: email, + password: 'example', + }, + } + }.to change { ActionMailer::Base.deliveries.count }.by(1) + end end describe "GET/POST #omniauth" do @@ -428,6 +448,66 @@ describe SessionsController, type: :controller do expect(response).to redirect_to(root_path) end + + it "switches a social account to a different social account if the authentication method changed" do + request.env["omniauth.auth"] = OmniAuth.config.mock_auth[:bn_launcher] + get :omniauth, params: { provider: 'bn_launcher' } + + u = User.find_by(social_uid: "bn-launcher-user") + u.social_uid = nil + users_old_uid = u.uid + u.save! + + new_user = OmniAuth::AuthHash.new( + provider: "bn_launcher", + uid: "bn-launcher-user-new", + info: { + email: "user@google.com", + name: "Office User", + nickname: "googleuser", + image: "touch.png", + customer: 'customer1', + } + ) + + allow_any_instance_of(SessionsController).to receive(:auth_changed_to_social?).and_return(true) + allow_any_instance_of(ApplicationController).to receive(:set_user_domain).and_return("customer1") + controller.instance_variable_set(:@user_domain, "customer1") + + request.env["omniauth.auth"] = new_user + get :omniauth, params: { provider: 'bn_launcher' } + + new_u = User.find_by(social_uid: "bn-launcher-user-new") + expect(users_old_uid).to eq(new_u.uid) + end + + it "switches a local account to a different social account if the authentication method changed" do + email = Faker::Internet.email + user = create(:user, email: email, provider: "customer1") + users_old_uid = user.uid + + new_user = OmniAuth::AuthHash.new( + provider: "bn_launcher", + uid: "bn-launcher-user-new", + info: { + email: email, + name: "Office User", + nickname: "googleuser", + image: "touch.png", + customer: 'customer1', + } + ) + + allow_any_instance_of(SessionsController).to receive(:auth_changed_to_social?).and_return(true) + allow_any_instance_of(ApplicationController).to receive(:set_user_domain).and_return("customer1") + controller.instance_variable_set(:@user_domain, "customer1") + + request.env["omniauth.auth"] = new_user + get :omniauth, params: { provider: 'bn_launcher' } + + new_u = User.find_by(social_uid: "bn-launcher-user-new") + expect(users_old_uid).to eq(new_u.uid) + end end describe "POST #ldap" do