From: Milan Cvetkovic Date: Fri, 26 Apr 2024 06:08:04 +0000 (+0000) Subject: Re-introduce additional round trip for verifying auth_provider X-Git-Tag: live~577^2~1 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/4965c19b7a8c96ab87b543af1fc36a1ad7514c09?ds=inline;hp=--cc Re-introduce additional round trip for verifying auth_provider --- 4965c19b7a8c96ab87b543af1fc36a1ad7514c09 diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 3156497a4..c3c4dcfb3 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -107,8 +107,13 @@ class UsersController < ApplicationController if current_user.invalid? # Something is wrong with a new user, so rerender the form render :action => "new" + elsif current_user.auth_provider.present? + # Verify external authenticator before moving on + session[:new_user] = current_user.slice("email", "display_name", "pass_crypt", "pass_crypt_confirmation") + redirect_to auth_url(current_user.auth_provider, current_user.auth_uid), :status => :temporary_redirect else # Save the user record + session[:new_user] = current_user.slice("email", "display_name", "pass_crypt", "pass_crypt_confirmation") save_new_user params[:email_hmac] end end @@ -222,6 +227,12 @@ class UsersController < ApplicationController session[:user_errors] = current_user.errors.as_json redirect_to edit_account_path + elsif session[:new_user] + session[:new_user]["auth_provider"] = provider + session[:new_user]["auth_uid"] = uid + + email_hmac = UsersController.message_hmac(email) if email_verified && email + save_new_user email_hmac else user = User.find_by(:auth_provider => provider, :auth_uid => uid) @@ -270,6 +281,8 @@ class UsersController < ApplicationController private def save_new_user(email_hmac) + new_user = session.delete(:new_user) + self.current_user = User.new(new_user) if check_signup_allowed(current_user.email) current_user.data_public = true current_user.description = "" if current_user.description.nil? diff --git a/test/integration/user_creation_test.rb b/test/integration/user_creation_test.rb index 59eb9ae30..4611860d0 100644 --- a/test/integration/user_creation_test.rb +++ b/test/integration/user_creation_test.rb @@ -45,6 +45,47 @@ class UserCreationTest < ActionDispatch::IntegrationTest assert_select "form > div > input.is-invalid#user_email" end + def test_user_create_association_bad_auth_provider + assert_difference("User.count", 0) do + assert_no_difference("ActionMailer::Base.deliveries.size") do + perform_enqueued_jobs do + post "/user/new", + :params => { :user => { :email => "test@example.com", + :display_name => "new_tester", + :pass_crypt => "testtest", + :pass_crypt_confirmation => "testtest", + :auth_provider => "noprovider", + :auth_uid => "123454321", + :consider_pd => "1" } } + assert_redirected_to auth_path(:provider => "noprovider", :origin => "/user/new") + post response.location + end + end + end + assert_response :not_found + end + + def test_user_create_association_no_auth_uid + OmniAuth.config.mock_auth[:google] = :invalid_credentials + assert_difference("User.count", 0) do + assert_no_difference("ActionMailer::Base.deliveries.size") do + perform_enqueued_jobs do + post "/user/new", + :params => { :user => { :email => "test@example.com", + :display_name => "new_tester", + :pass_crypt => "testtest", + :pass_crypt_confirmation => "testtest", + :auth_provider => "google", + :consider_pd => "1" } } + assert_redirected_to auth_path(:provider => "google", :origin => "/user/new") + post response.location + end + end + end + follow_redirect! + assert_redirected_to auth_failure_path(:strategy => "google", :message => "invalid_credentials", :origin => "/user/new") + end + def test_user_create_association_submit_duplicate_email dup_email = create(:user).email display_name = "new_tester" @@ -56,7 +97,7 @@ class UserCreationTest < ActionDispatch::IntegrationTest :display_name => display_name, :pass_crypt => "testtest", :pass_crypt_confirmation => "testtest", - :auth_provider => "auth_provider", + :auth_provider => "google", :auth_uid => "123454321", :consider_pd => "1" } } end @@ -116,7 +157,7 @@ class UserCreationTest < ActionDispatch::IntegrationTest post "/user/new", :params => { :user => { :email => email, :display_name => dup_display_name, - :auth_provider => "provider", + :auth_provider => "google", :auth_uid => "123454321", :consider_pd => "1" } } end @@ -255,6 +296,9 @@ class UserCreationTest < ActionDispatch::IntegrationTest :auth_provider => "openid", :auth_uid => "http://localhost:1123/new.tester", :consider_pd => "1" } } + assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new") + post response.location + follow_redirect! end end end @@ -291,10 +335,22 @@ class UserCreationTest < ActionDispatch::IntegrationTest def test_user_create_openid_failure OmniAuth.config.mock_auth[:openid] = :connection_failed + new_email = "newtester-openid2@osm.org" + display_name = "new_tester-openid2" assert_difference("User.count", 0) do assert_difference("ActionMailer::Base.deliveries.size", 0) do perform_enqueued_jobs do - post auth_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new") + post "/user/new", + :params => { :user => { :email => new_email, + :email_confirmation => new_email, + :display_name => display_name, + :auth_provider => "openid", + :auth_uid => "http://localhost:1123/new.tester", + :pass_crypt => "", + :pass_crypt_confirmation => "" } } + assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new") + post response.location + assert_redirected_to auth_success_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new") follow_redirect! assert_redirected_to auth_failure_path(:strategy => "openid", :message => "connection_failed", :origin => "/user/new") follow_redirect! @@ -307,7 +363,7 @@ class UserCreationTest < ActionDispatch::IntegrationTest end def test_user_create_openid_redirect - auth_uid = "12345654321" + auth_uid = "http://localhost:1123/new.tester" new_email = "redirect_tester_openid@osm.org" display_name = "redirect_tester_openid" @@ -330,6 +386,11 @@ class UserCreationTest < ActionDispatch::IntegrationTest :auth_provider => "openid", :auth_uid => auth_uid, :consider_pd => "1" } } + assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new") + post response.location + assert_redirected_to auth_success_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new") + follow_redirect! + assert_redirected_to :controller => :confirmations, :action => :confirm, :display_name => display_name follow_redirect! end end @@ -393,6 +454,10 @@ class UserCreationTest < ActionDispatch::IntegrationTest :auth_uid => auth_uid, :consider_pd => "1" }, :email_hmac => email_hmac } + assert_redirected_to auth_path(:provider => "google", :origin => "/user/new") + post response.location + assert_redirected_to auth_success_path(:provider => "google") + follow_redirect! assert_redirected_to welcome_path follow_redirect! end @@ -434,11 +499,22 @@ class UserCreationTest < ActionDispatch::IntegrationTest def test_user_create_google_failure OmniAuth.config.mock_auth[:google] = :connection_failed + new_email = "newtester-google2@osm.org" + display_name = "new_tester-google2" assert_difference("User.count", 0) do assert_difference("ActionMailer::Base.deliveries.size", 0) do perform_enqueued_jobs do - post auth_path(:provider => "google", :origin => "/user/new") - assert_response :redirect + post "/user/new", + :params => { :user => { :email => new_email, + :email_confirmation => new_email, + :display_name => display_name, + :auth_provider => "google", + :auth_uid => "123454321", + :pass_crypt => "", + :pass_crypt_confirmation => "" } } + assert_redirected_to auth_path(:provider => "google", :origin => "/user/new") + post response.location + assert_redirected_to auth_success_path(:provider => "google") follow_redirect! assert_redirected_to auth_failure_path(:strategy => "google", :message => "connection_failed", :origin => "/user/new") follow_redirect! @@ -479,7 +555,11 @@ class UserCreationTest < ActionDispatch::IntegrationTest :auth_provider => "google", :auth_uid => auth_uid, :consider_pd => "1" } } - assert_response :redirect + assert_redirected_to auth_path(:provider => "google", :origin => "/user/new") + post response.location + assert_redirected_to auth_success_path(:provider => "google") + follow_redirect! + assert_redirected_to :controller => :confirmations, :action => :confirm, :display_name => display_name follow_redirect! end end @@ -542,6 +622,10 @@ class UserCreationTest < ActionDispatch::IntegrationTest :auth_uid => auth_uid, :consider_pd => "1" }, :email_hmac => email_hmac } + assert_redirected_to auth_path(:provider => "facebook", :origin => "/user/new") + post response.location + assert_redirected_to auth_success_path(:provider => "facebook") + follow_redirect! assert_redirected_to welcome_path follow_redirect! end @@ -582,11 +666,22 @@ class UserCreationTest < ActionDispatch::IntegrationTest def test_user_create_facebook_failure OmniAuth.config.mock_auth[:facebook] = :connection_failed + new_email = "newtester-facebook2@osm.org" + display_name = "new_tester-facebook2" assert_difference("User.count", 0) do assert_difference("ActionMailer::Base.deliveries.size", 0) do perform_enqueued_jobs do - post auth_path(:provider => "facebook", :origin => "/user/new") - assert_response :redirect + post "/user/new", + :params => { :user => { :email => new_email, + :email_confirmation => new_email, + :display_name => display_name, + :auth_provider => "facebook", + :auth_uid => "123454321", + :pass_crypt => "", + :pass_crypt_confirmation => "" } } + assert_redirected_to auth_path(:provider => "facebook", :origin => "/user/new") + post response.location + assert_redirected_to auth_success_path(:provider => "facebook") follow_redirect! assert_redirected_to auth_failure_path(:strategy => "facebook", :message => "connection_failed", :origin => "/user/new") follow_redirect! @@ -628,6 +723,11 @@ class UserCreationTest < ActionDispatch::IntegrationTest :auth_provider => "facebook", :auth_uid => auth_uid, :consider_pd => "1" } } + assert_redirected_to auth_path(:provider => "facebook", :origin => "/user/new") + post response.location + assert_redirected_to auth_success_path(:provider => "facebook") + follow_redirect! + assert_redirected_to :controller => :confirmations, :action => :confirm, :display_name => display_name assert_response :redirect follow_redirect! end @@ -690,6 +790,10 @@ class UserCreationTest < ActionDispatch::IntegrationTest :auth_uid => auth_uid, :consider_pd => "1" }, :email_hmac => email_hmac } + assert_redirected_to auth_path(:provider => "microsoft", :origin => "/user/new") + post response.location + assert_redirected_to auth_success_path(:provider => "microsoft") + follow_redirect! assert_redirected_to welcome_path follow_redirect! end @@ -730,11 +834,22 @@ class UserCreationTest < ActionDispatch::IntegrationTest def test_user_create_microsoft_failure OmniAuth.config.mock_auth[:microsoft] = :connection_failed + new_email = "newtester-microsoft2@osm.org" + display_name = "new_tester-microsoft2" assert_difference("User.count", 0) do assert_difference("ActionMailer::Base.deliveries.size", 0) do perform_enqueued_jobs do - post auth_path(:provider => "microsoft", :origin => "/user/new") - assert_response :redirect + post "/user/new", + :params => { :user => { :email => new_email, + :email_confirmation => new_email, + :display_name => display_name, + :auth_provider => "microsoft", + :auth_uid => "123454321", + :pass_crypt => "", + :pass_crypt_confirmation => "" } } + assert_redirected_to auth_path(:provider => "microsoft", :origin => "/user/new") + post response.location + assert_redirected_to auth_success_path(:provider => "microsoft") follow_redirect! assert_redirected_to auth_failure_path(:strategy => "microsoft", :message => "connection_failed", :origin => "/user/new") follow_redirect! @@ -775,6 +890,11 @@ class UserCreationTest < ActionDispatch::IntegrationTest :auth_provider => "microsoft", :auth_uid => auth_uid, :consider_pd => "1" } } + assert_redirected_to auth_path(:provider => "microsoft", :origin => "/user/new") + post response.location + assert_redirected_to auth_success_path(:provider => "microsoft") + follow_redirect! + assert_redirected_to :controller => :confirmations, :action => :confirm, :display_name => display_name assert_response :redirect follow_redirect! end @@ -842,6 +962,10 @@ class UserCreationTest < ActionDispatch::IntegrationTest :read_ct => 1, :read_tou => 1, :email_hmac => email_hmac } + assert_redirected_to auth_path(:provider => "github", :origin => "/user/new") + post response.location + assert_redirected_to auth_success_path(:provider => "github") + follow_redirect! assert_redirected_to welcome_path follow_redirect! end @@ -883,10 +1007,22 @@ class UserCreationTest < ActionDispatch::IntegrationTest def test_user_create_github_failure OmniAuth.config.mock_auth[:github] = :connection_failed + new_email = "newtester-github2@osm.org" + display_name = "new_tester-github2" assert_difference("User.count", 0) do assert_difference("ActionMailer::Base.deliveries.size", 0) do perform_enqueued_jobs do - post auth_path(:provider => "github", :origin => "/user/new") + post "/user/new", + :params => { :user => { :email => new_email, + :email_confirmation => new_email, + :display_name => display_name, + :auth_provider => "github", + :auth_uid => "123454321", + :pass_crypt => "", + :pass_crypt_confirmation => "" } } + assert_redirected_to auth_path(:provider => "github", :origin => "/user/new") + post response.location + assert_redirected_to auth_success_path(:provider => "github") follow_redirect! assert_redirected_to auth_failure_path(:strategy => "github", :message => "connection_failed", :origin => "/user/new") follow_redirect! @@ -926,6 +1062,11 @@ class UserCreationTest < ActionDispatch::IntegrationTest :auth_provider => "github", :auth_uid => auth_uid, :consider_pd => "1" } } + assert_redirected_to auth_path(:provider => "github", :origin => "/user/new") + post response.location + assert_redirected_to auth_success_path(:provider => "github") + follow_redirect! + assert_redirected_to :controller => :confirmations, :action => :confirm, :display_name => display_name assert_response :redirect follow_redirect! end @@ -992,6 +1133,10 @@ class UserCreationTest < ActionDispatch::IntegrationTest :read_ct => 1, :read_tou => 1, :email_hmac => email_hmac } + assert_redirected_to auth_path(:provider => "wikipedia", :origin => "/user/new") + post response.location + assert_redirected_to auth_success_path(:provider => "wikipedia", :origin => "/user/new") + follow_redirect! assert_redirected_to welcome_path follow_redirect! end @@ -1030,11 +1175,22 @@ class UserCreationTest < ActionDispatch::IntegrationTest def test_user_create_wikipedia_failure OmniAuth.config.mock_auth[:wikipedia] = :connection_failed + new_email = "newtester-wikipedia2@osm.org" + display_name = "new_tester-wikipedia2" assert_difference("User.count", 0) do assert_difference("ActionMailer::Base.deliveries.size", 0) do perform_enqueued_jobs do - post auth_path(:provider => "wikipedia", :origin => "/user/new") - assert_response :redirect + post "/user/new", + :params => { :user => { :email => new_email, + :email_confirmation => new_email, + :display_name => display_name, + :auth_provider => "wikipedia", + :auth_uid => "123454321", + :pass_crypt => "", + :pass_crypt_confirmation => "" } } + assert_redirected_to auth_path(:provider => "wikipedia", :origin => "/user/new") + post response.location + assert_redirected_to auth_success_path(:provider => "wikipedia", :origin => "/user/new") follow_redirect! assert_redirected_to auth_failure_path(:strategy => "wikipedia", :message => "connection_failed", :origin => "/user/new") follow_redirect! @@ -1076,6 +1232,11 @@ class UserCreationTest < ActionDispatch::IntegrationTest :auth_provider => "wikipedia", :auth_uid => auth_uid, :consider_pd => "1" } } + assert_redirected_to auth_path(:provider => "wikipedia", :origin => "/user/new") + post response.location + assert_redirected_to auth_success_path(:provider => "wikipedia", :origin => "/user/new") + follow_redirect! + assert_redirected_to :controller => :confirmations, :action => :confirm, :display_name => display_name assert_response :redirect follow_redirect! end