From 40b05b1626238596a01f936069783a95069a3b70 Mon Sep 17 00:00:00 2001 From: shawn-higgins1 <23224097+shawn-higgins1@users.noreply.github.com> Date: Tue, 9 Jul 2019 13:06:07 -0400 Subject: [PATCH] GRN2-155: Begin preparing for removal of Twitter accounts (#615) * Add twitter deprecation message * Fix rspec test * Extract room switch to its own method * update method name --- app/controllers/sessions_controller.rb | 14 ++ app/controllers/users_controller.rb | 12 +- app/helpers/application_helper.rb | 6 +- app/helpers/sessions_helper.rb | 22 +++ app/views/shared/_flash_messages.html.erb | 6 +- config/environments/test.rb | 2 + config/locales/en.yml | 6 +- spec/controllers/sessions_controller_spec.rb | 161 ++++++++++++++----- 8 files changed, 180 insertions(+), 49 deletions(-) diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 4f6abdd0..678ea8a0 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -51,6 +51,10 @@ class SessionsController < ApplicationController @auth = request.env['omniauth.auth'] @user_exists = check_user_exists + if !@user_exists && @auth['provider'] == "twitter" + return redirect_to root_path, flash: { alert: I18n.t("registration.deprecated.twitter_signup") } + end + # 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 @@ -70,6 +74,16 @@ class SessionsController < ApplicationController invite_registration && !@user_exists login(user) + + if @auth['provider'] == "twitter" + flash[:alert] = if allow_user_signup? && allow_greenlight_accounts? + I18n.t("registration.deprecated.twitter_signin", + link: signup_path(old_twitter_user_id: user.id)) + else + I18n.t("registration.deprecated.twitter_signin", + link: signin_path(old_twitter_user_id: user.id)) + end + end rescue => e logger.error "Error authenticating via omniauth: #{e}" omniauth_fail diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 4b2ba771..3883a584 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -62,6 +62,10 @@ class UsersController < ApplicationController # GET /signin def signin + unless params[:old_twitter_user_id].nil? && session[:old_twitter_user_id].nil? + flash[:alert] = I18n.t("registration.deprecated.new_signin") + session[:old_twitter_user_id] = params[:old_twitter_user_id] unless params[:old_twitter_user_id].nil? + end end # GET /signup @@ -75,6 +79,12 @@ class UsersController < ApplicationController session[:invite_token] = params[:invite_token] end + unless params[:old_twitter_user_id].nil? && session[:old_twitter_user_id].nil? + logout + flash.now[:alert] = I18n.t("registration.deprecated.new_signin") + session[:old_twitter_user_id] = params[:old_twitter_user_id] unless params[:old_twitter_user_id].nil? + end + @user = User.new end @@ -175,7 +185,7 @@ class UsersController < ApplicationController end def ensure_unauthenticated - redirect_to current_user.main_room if current_user + redirect_to current_user.main_room if current_user && params[:old_twitter_user_id].nil? end def user_params diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index b4c77a83..248ebbf8 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -33,7 +33,11 @@ module ApplicationHelper # Determines which providers can show a login button in the login modal. def iconset_providers - configured_providers & [:google, :twitter, :microsoft_office365] + providers = configured_providers & [:google, :twitter, :microsoft_office365] + + providers.delete(:twitter) if session[:old_twitter_user_id] + + providers end # Generates the login URL for a specific provider. diff --git a/app/helpers/sessions_helper.rb b/app/helpers/sessions_helper.rb index dd4263cf..df1fa19d 100644 --- a/app/helpers/sessions_helper.rb +++ b/app/helpers/sessions_helper.rb @@ -19,6 +19,8 @@ module SessionsHelper # Logs a user into GreenLight. def login(user) + migrate_twitter_user(user) + session[:user_id] = user.id # If there are not terms, or the user has accepted them, check for email verification @@ -97,4 +99,24 @@ module SessionsHelper hd_opts end end + + def migrate_twitter_user(user) + if !session["old_twitter_user_id"].nil? && user.provider != "twitter" + old_user = User.find(session["old_twitter_user_id"]) + + old_user.rooms.each do |room| + room.owner = user + + room.name = "Old " + room.name if room.id == old_user.main_room.id + + room.save! + end + + # Query for the old user again so the migrated rooms don't get deleted + old_user.reload + old_user.destroy! + + session["old_twitter_user_id"] = nil + end + end end diff --git a/app/views/shared/_flash_messages.html.erb b/app/views/shared/_flash_messages.html.erb index 6d410e0c..6e863608 100644 --- a/app/views/shared/_flash_messages.html.erb +++ b/app/views/shared/_flash_messages.html.erb @@ -17,17 +17,17 @@ <% if key.eql? "success" %>
- <%= value %> + <%= value.html_safe %>
<% elsif key.eql? "alert" %>
- <%= value %> + <%= value.html_safe %>
<% elsif key.eql? "info" %>
- <%= value %> + <%= value.html_safe %>
<% end %> <% end %> diff --git a/config/environments/test.rb b/config/environments/test.rb index 5f68c35b..f4dd5186 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -49,4 +49,6 @@ Rails.application.configure do # Use standalone BigBlueButton server. config.bigbluebutton_endpoint = config.bigbluebutton_endpoint_default config.bigbluebutton_secret = config.bigbluebutton_secret_default + + config.loadbalanced_configuration = false end diff --git a/config/locales/en.yml b/config/locales/en.yml index 0a34b25b..a933429c 100755 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -330,7 +330,11 @@ en: fail: Your account has not been approved yet. If multiples days have passed since you signed up, please contact your administrator. signup: Your account was successfully created. It has been sent to an administrator for approval. banned: - fail: You do not have access to this application. If you believe this is a mistake, please contact your administrator. + fail: You do not have access to this application. If you believe this is a mistake, please contact your administrator. + deprecated: + new_signin: Select a new login method for you account. All your rooms from your old account will be migrated to the new account + twitter_signin: Signing in via Twitter has been deprecated and will be removed in the next release. Click here to move your account to a new authentication method + twitter_signup: Sign up via Twitter has been deprecated. Please use a different sign up method invite: fail: Your token is either invalid or has expired. If you believe this is a mistake, please contact your administrator. no_invite: You do not have an invitation to join. Please contact your administrator to receive one. diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index f85e1ac2..3ea499c5 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -156,6 +156,29 @@ describe SessionsController, type: :controller do expect(@request.session[:user_id]).to eql(user.id) expect(response).to redirect_to(admins_path) end + + it "should migrate old rooms from the twitter account to the new user" do + twitter_user = User.create(name: "Twitter User", email: "user@twitter.com", image: "example.png", + username: "twitteruser", email_verified: true, provider: 'twitter', social_uid: "twitter-user") + + room = Room.new(name: "Test") + room.owner = twitter_user + room.save! + + post :create, params: { + session: { + email: @user1.email, + password: 'example', + }, + }, session: { + old_twitter_user_id: twitter_user.id + } + + @user1.reload + expect(@user1.rooms.count).to eq(3) + 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 end describe "GET/POST #omniauth" do @@ -173,6 +196,18 @@ describe SessionsController, type: :controller do }, ) + OmniAuth.config.mock_auth[:google] = OmniAuth::AuthHash.new( + provider: "google", + uid: "google-user", + info: { + email: "user@google.com", + name: "Google User", + nickname: "googleuser", + image: "touch.png", + customer: 'customer1', + } + ) + OmniAuth.config.mock_auth[:bn_launcher] = OmniAuth::AuthHash.new( provider: "bn_launcher", uid: "bn-launcher-user", @@ -190,68 +225,108 @@ describe SessionsController, type: :controller do } end - unless Rails.configuration.omniauth_bn_launcher - it "should create and login user with omniauth twitter" do + it "should create and login user with omniauth google" do + request.env["omniauth.auth"] = OmniAuth.config.mock_auth[:google] + get :omniauth, params: { provider: :google } + + u = User.last + expect(u.provider).to eql("google") + expect(u.email).to eql("user@google.com") + expect(@request.session[:user_id]).to eql(u.id) + end + + it "should create and login user with omniauth bn launcher" do + request.env["omniauth.auth"] = OmniAuth.config.mock_auth[:bn_launcher] + get :omniauth, params: { provider: 'bn_launcher' } + + u = User.last + expect(u.provider).to eql("customer1") + expect(u.email).to eql("user@google.com") + expect(@request.session[:user_id]).to eql(u.id) + end + + it "should redirect to root on invalid omniauth login" do + request.env["omniauth.auth"] = :invalid_credentials + get :omniauth, params: { provider: :google } + + expect(response).to redirect_to(root_path) + end + + it "should not create session without omniauth env set for google" do + get :omniauth, params: { provider: 'google' } + + expect(response).to redirect_to(root_path) + end + + context 'twitter deprecation' do + it "should not allow new user sign up with omniauth twitter" do request.env["omniauth.auth"] = OmniAuth.config.mock_auth[:twitter] get :omniauth, params: { provider: :twitter } - u = User.last - expect(u.provider).to eql("twitter") - expect(u.email).to eql("user@twitter.com") - expect(@request.session[:user_id]).to eql(u.id) + expect(response).to redirect_to(root_path) + expect(flash[:alert]).to eq(I18n.t("registration.deprecated.twitter_signup")) end - it "should create and login user with omniauth bn launcher" do - request.env["omniauth.auth"] = OmniAuth.config.mock_auth[:bn_launcher] - get :omniauth, params: { provider: 'bn_launcher' } + it "should notify twitter users that twitter is deprecated" do + allow(Rails.configuration).to receive(:allow_user_signup).and_return(true) + twitter_user = User.create(name: "Twitter User", email: "user@twitter.com", image: "example.png", + username: "twitteruser", email_verified: true, provider: 'twitter', social_uid: "twitter-user") - u = User.last - expect(u.provider).to eql("customer1") - expect(u.email).to eql("user@google.com") - expect(@request.session[:user_id]).to eql(u.id) - end - - it "should redirect to root on invalid omniauth login" do - request.env["omniauth.auth"] = :invalid_credentials + request.env["omniauth.auth"] = OmniAuth.config.mock_auth[:twitter] get :omniauth, params: { provider: :twitter } - expect(response).to redirect_to(root_path) + expect(flash[:alert]).to eq(I18n.t("registration.deprecated.twitter_signin", + link: signup_path(old_twitter_user_id: twitter_user.id))) end - it "should not create session without omniauth env set for google" do - get :omniauth, params: { provider: 'google' } + it "should migrate rooms from the twitter account to the google account" do + twitter_user = User.create(name: "Twitter User", email: "user@twitter.com", image: "example.png", + username: "twitteruser", email_verified: true, provider: 'twitter', social_uid: "twitter-user") - expect(response).to redirect_to(root_path) + room = Room.new(name: "Test") + room.owner = twitter_user + room.save! + + request.env["omniauth.auth"] = OmniAuth.config.mock_auth[:google] + get :omniauth, params: { provider: :google }, session: { old_twitter_user_id: twitter_user.id } + + u = User.last + expect(u.provider).to eql("google") + expect(u.email).to eql("user@google.com") + expect(@request.session[:user_id]).to eql(u.id) + expect(u.rooms.count).to eq(3) + expect(u.rooms.find { |r| r.name == "Old Home Room" }).to_not be_nil + expect(u.rooms.find { |r| r.name == "Test" }).to_not be_nil + end + end + + context 'registration notification emails' do + before do + allow(Rails.configuration).to receive(:enable_email_verification).and_return(true) + @user = create(:user, provider: "greenlight") + @admin = create(:user, provider: "greenlight", email: "test@example.com") + @admin.add_role :admin end - context 'registration notification emails' do - before do - allow(Rails.configuration).to receive(:enable_email_verification).and_return(true) - @user = create(:user, provider: "greenlight") - @admin = create(:user, provider: "greenlight", email: "test@example.com") - @admin.add_role :admin - end + it "should notify admin on new user signup with approve/reject registration" do + allow_any_instance_of(Registrar).to receive(:approval_registration).and_return(true) - it "should notify admin on new user signup with approve/reject registration" do - allow_any_instance_of(Registrar).to receive(:approval_registration).and_return(true) + request.env["omniauth.auth"] = OmniAuth.config.mock_auth[:bn_launcher] - request.env["omniauth.auth"] = OmniAuth.config.mock_auth[:bn_launcher] + expect { get :omniauth, params: { provider: 'bn_launcher' } } + .to change { ActionMailer::Base.deliveries.count }.by(1) + end - expect { get :omniauth, params: { provider: 'bn_launcher' } } - .to change { ActionMailer::Base.deliveries.count }.by(1) - end + it "should notify admin on new user signup with invite registration" do + allow_any_instance_of(Registrar).to receive(:invite_registration).and_return(true) - it "should notify admin on new user signup with invite registration" do - allow_any_instance_of(Registrar).to receive(:invite_registration).and_return(true) + invite = Invitation.create(email: "user@google.com", provider: "greenlight") + @request.session[:invite_token] = invite.invite_token - invite = Invitation.create(email: "user@google.com", provider: "greenlight") - @request.session[:invite_token] = invite.invite_token + request.env["omniauth.auth"] = OmniAuth.config.mock_auth[:bn_launcher] - request.env["omniauth.auth"] = OmniAuth.config.mock_auth[:bn_launcher] - - expect { get :omniauth, params: { provider: 'bn_launcher' } } - .to change { ActionMailer::Base.deliveries.count }.by(1) - end + expect { get :omniauth, params: { provider: 'bn_launcher' } } + .to change { ActionMailer::Base.deliveries.count }.by(1) end end