Skip to content

Commit

Permalink
Return helpful error message when attempting to login/signup w Twitte…
Browse files Browse the repository at this point in the history 
…r oauth with no email (#8734)

* Attempt a test of Twitter oauth with no email

* Update test.rb

* Update user_sessions_controller_test.rb

* change alerts to prompt for email

* try removing email address from a user

* revert removing email from user

* Update user_sessions_controller_test.rb

* Update user_sessions_controller.rb

* Update user_sessions_controller_test.rb

* Update user_sessions_controller.rb

* Update user_sessions_controller_test.rb

* use assert_nil session[:user_session]
  • Loading branch information
@jywarren
jywarren authored Dec 1, 2020
1 parent 8a59f23 commit a23a3f9
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 102 deletions.
107 changes: 56 additions & 51 deletions app/controllers/user_sessions_controller.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,65 +50,70 @@ def handle_social_login_flow(auth)
redirect_to return_to + hash_params, notice: "Already linked to another account!"
end
else # not signed in
# User U has Provider P linked to U. U has email E1 while P has email E2. So, User table can't find E2 provided
# from auth hash, hence U is found by the user of identity having E2 as email
@user = User.where(email: auth["info"]["email"]) ? User.find_by(email: auth["info"]["email"]) : @identity.user
if @user&.status&.zero?
flash[:error] = I18n.t('user_sessions_controller.user_has_been_banned', username: @user.username).html_safe
redirect_to return_to + hash_params
elsif @user&.status == 5
flash[:error] = I18n.t('user_sessions_controller.user_has_been_moderated', username: @user.username).html_safe
redirect_to return_to + hash_params
elsif @identity&.user.present?
# The identity we found had a user associated with it so let's
# just log them in here
@user = @identity.user
@user_session = UserSession.create(@identity.user)
if session[:openid_return_to] # for openid login, redirects back to openid auth process
return_to = session[:openid_return_to]
session[:openid_return_to] = nil
if auth["info"]["email"].nil?
flash[:error] = "You have tried using a Twitter account with no associated email address. Unfortunately we need an email address; please add one and try again, or sign up a different way. Thank you!"
redirect_to return_to
else
# User U has Provider P linked to U. U has email E1 while P has email E2. So, User table can't find E2 provided
# from auth hash, hence U is found by the user of identity having E2 as email
@user = User.where(email: auth["info"]["email"]) ? User.find_by(email: auth["info"]["email"]) : @identity.user
if @user&.status&.zero?
flash[:error] = I18n.t('user_sessions_controller.user_has_been_banned', username: @user.username).html_safe
redirect_to return_to + hash_params
else
redirect_to return_to + hash_params, notice: I18n.t('user_sessions_controller.logged_in')
end
else # identity does not exist so we need to either create a user with identity OR link identity to existing user
if User.where(email: auth["info"]["email"]).empty?
# Create a new user as email provided is not present in PL database
user = User.create_with_omniauth(auth)
WelcomeMailer.notify_newcomer(user).deliver_now
@identity = UserTag.create_with_omniauth(auth, user.id)
key = user.generate_reset_key
elsif @user&.status == 5
flash[:error] = I18n.t('user_sessions_controller.user_has_been_moderated', username: @user.username).html_safe
redirect_to return_to + hash_params
elsif @identity&.user.present?
# The identity we found had a user associated with it so let's
# just log them in here
@user = @identity.user
@user_session = UserSession.create(@identity.user)
@user = user
# send key to user email
PasswordResetMailer.reset_notify(user, key).deliver_now unless user.nil? # respond the same to both successes and failures; security
if session[:openid_return_to] # for openid login, redirects back to openid auth process
return_to = session[:openid_return_to]
session[:openid_return_to] = nil
redirect_to return_to + hash_params
elsif params[:return_to] && params[:return_to].split('/')[0..3] == ["", "subscribe", "multiple", "tag"]
flash[:notice] = "You are now following '#{params[:return_to].split('/')[4]}'."
subscribe_multiple_tag(params[:return_to].split('/')[4])
redirect_to '/dashboard', notice: "You have successfully signed in. Please change your password using the link sent to you via e-mail."
else
redirect_to "/dashboard", notice: "You have successfully signed in. Please change your password using the link sent to you via e-mail."
redirect_to return_to + hash_params, notice: I18n.t('user_sessions_controller.logged_in')
end
else # email exists so link the identity with existing user and log in the user
user = User.where(email: auth["info"]["email"])
# If no identity was found, create a brand new one here
@identity = UserTag.create_with_omniauth(auth, user.ids.first)
# The identity is not associated with the current_user so lets
# associate the identity
@identity.save
@user = user
# log in them
@user_session = UserSession.create(@identity.user)
if session[:openid_return_to] # for openid login, redirects back to openid auth process
return_to = session[:openid_return_to]
session[:openid_return_to] = nil
redirect_to return_to + hash_params
else
redirect_to return_to + hash_params, notice: "Successfully linked to your account!"
else # identity does not exist so we need to either create a user with identity OR link identity to existing user
if User.where(email: auth["info"]["email"]).empty?
# Create a new user as email provided is not present in PL database
user = User.create_with_omniauth(auth)
WelcomeMailer.notify_newcomer(user).deliver_now
@identity = UserTag.create_with_omniauth(auth, user.id)
key = user.generate_reset_key
@user_session = UserSession.create(@identity.user)
@user = user
# send key to user email
PasswordResetMailer.reset_notify(user, key).deliver_now unless user.nil? # respond the same to both successes and failures; security
if session[:openid_return_to] # for openid login, redirects back to openid auth process
return_to = session[:openid_return_to]
session[:openid_return_to] = nil
redirect_to return_to + hash_params
elsif params[:return_to] && params[:return_to].split('/')[0..3] == ["", "subscribe", "multiple", "tag"]
flash[:notice] = "You are now following '#{params[:return_to].split('/')[4]}'."
subscribe_multiple_tag(params[:return_to].split('/')[4])
redirect_to '/dashboard', notice: "You have successfully signed in. Please change your password using the link sent to you via e-mail."
else
redirect_to '/dashboard', notice: "You have successfully signed in. Please change your password using the link sent to you via e-mail."
end
else # email exists in user db so link the identity with existing user and log in the user
user = User.where(email: auth["info"]["email"])
# If no identity was found, create a brand new one here
@identity = UserTag.create_with_omniauth(auth, user.ids.first)
# The identity is not associated with the current_user so lets
# associate the identity
@identity.save
@user = user
# log in them
@user_session = UserSession.create(@identity.user)
if session[:openid_return_to] # for openid login, redirects back to openid auth process
return_to = session[:openid_return_to]
session[:openid_return_to] = nil
redirect_to return_to + hash_params
else
redirect_to return_to + hash_params, notice: "Successfully linked to your account!"
end
end
end
end
Expand Down
10 changes: 10 additions & 0 deletions config/environments/test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,4 +191,14 @@
'email' => 'bob@publiclab.org'
}
})


# Twitter Provider with no email provided
OmniAuth.config.mock_auth[:twitter_no_email] = OmniAuth::AuthHash.new({
'provider' => 'twitter',
'uid' => '135798079602',
'info' => {
'name' => 'jeff with no email',
}
})
end
2 changes: 1 addition & 1 deletion test/fixtures/users.yml
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
bob:
username: Bob
status: 1
email: bob@publiclab.org
email: 'bob@publiclab.org'
id: 1
password_salt: <%= salt = Authlogic::Random.hex_token %>
crypted_password: <%= Authlogic::CryptoProviders::Sha512.encrypt("secretive" + salt) %>
Expand Down
113 changes: 63 additions & 50 deletions test/functional/user_sessions_controller_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,61 +154,74 @@ class UserSessionsControllerTest < ActionController::TestCase
assert_equal "Successfully logged out.", flash[:notice]
end

test 'sign up and login via provider basic flow for twitter' do
assert_not_nil OmniAuth.config.mock_auth[:twitter1]
#Omniauth hash is present
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:twitter1]
assert_not_nil request.env['omniauth.auth']
#Sign Up for a new user
post :create
assert_equal "You have successfully signed in. Please change your password using the link sent to you via e-mail.", flash[:notice]
#Log Out
post :destroy
assert_equal "Successfully logged out.", flash[:notice]
#auth hash is present so login via a provider
post :create
assert_equal "Successfully logged in.", flash[:notice]
end

test 'sign up and login via provider basic flow for twitter' do
assert_not_nil OmniAuth.config.mock_auth[:twitter1]
#Omniauth hash is present
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:twitter1]
assert_not_nil request.env['omniauth.auth']
#Sign Up for a new user
post :create
assert_equal "You have successfully signed in. Please change your password using the link sent to you via e-mail.", flash[:notice]
#Log Out
post :destroy
assert_equal "Successfully logged out.", flash[:notice]
#auth hash is present so login via a provider
test 'sign up and login via provider basic flow for twitter user with no email' do
assert_not_nil OmniAuth.config.mock_auth[:twitter_no_email]
#Omniauth hash is present
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:twitter_no_email]
assert_not_nil request.env['omniauth.auth']
#Sign Up for a new user
assert_nil session[:user_session]
assert_difference 'User.count', 0 do
post :create
assert_equal "Successfully logged in.", flash[:notice]
end
assert_nil session[:user_session]
assert_equal "You have tried using a Twitter account with no associated email address. Unfortunately we need an email address; please add one and try again, or sign up a different way. Thank you!", flash[:error]
end

test 'sign up and login via provider alternative flow for twitter' do
assert_not_nil OmniAuth.config.mock_auth[:twitter2]
#Omniauth hash is present
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:twitter2]
assert_not_nil request.env['omniauth.auth']
#Sign Up for an existing user as email exists in the db
post :create
assert_equal "Successfully linked to your account!", flash[:notice]
#Log Out
post :destroy
assert_equal "Successfully logged out.", flash[:notice]
#auth hash is present so login via a provider
post :create
assert_equal "Successfully logged in.", flash[:notice]
end
test 'sign up and login via provider alternative flow for twitter' do
assert_not_nil OmniAuth.config.mock_auth[:twitter2]
#Omniauth hash is present
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:twitter2]
assert_not_nil request.env['omniauth.auth']
#Sign Up for an existing user as email exists in the db
post :create
assert_equal "Successfully linked to your account!", flash[:notice]
#Log Out
post :destroy
assert_equal "Successfully logged out.", flash[:notice]
#auth hash is present so login via a provider
post :create
assert_equal "Successfully logged in.", flash[:notice]
end

test 'login user with an email and then contwitter provider' do
post :create,
params: {
user_session: {
username: users(:jeff).email,
password: 'secretive'
}
}
assert_redirected_to '/dashboard'
assert_not_nil OmniAuth.config.mock_auth[:twitter2]
#Omniauth hash is present
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:twitter2]
assert_not_nil request.env['omniauth.auth']
#Link a twitter account to an existing user
post :create
assert_equal "Successfully linked to your account!", flash[:notice]
#Link same twitter account to an existing user again
post :create
assert_equal "Already linked to your account!", flash[:notice]
#Log Out
post :destroy
assert_equal "Successfully logged out.", flash[:notice]
test 'login user with an email and then contwitter provider' do
post :create,
params: {
user_session: {
username: users(:jeff).email,
password: 'secretive'
}
}
assert_redirected_to '/dashboard'
assert_not_nil OmniAuth.config.mock_auth[:twitter2]
#Omniauth hash is present
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:twitter2]
assert_not_nil request.env['omniauth.auth']
#Link a twitter account to an existing user
post :create
assert_equal "Successfully linked to your account!", flash[:notice]
#Link same twitter account to an existing user again
post :create
assert_equal "Already linked to your account!", flash[:notice]
#Log Out
post :destroy
assert_equal "Successfully logged out.", flash[:notice]
end

test 'sign up and login via provider basic flow for facebook' do
Expand Down

0 comments on commit a23a3f9

Please sign in to comment.