[openstreetmap/openstreetmap-website] Re-arrange login and signup screens as discussed in #4128 (PR #4455)

[Milan Cvetkovic]

@milan-cvetkovic commented on this pull request.



> @@ -290,8 +241,23 @@ def auth_success
         else
           failed_login t("sessions.new.auth failure")
         end
+      elsif user.nil? && user = User.find_by(:email => email)
+        user[:auth_uid] = uid
+        user[:auth_provider] = provider
+        user.save!
+
+        user.deactivate! if user.status == "active" && !email_verified
+
+        if user.status == "active"
+          successful_login(user)
+        else
+          session[:token] = user.tokens.create.token
+          UserMailer.signup_confirm(user, user.tokens.create(:referer => session[:referer])).deliver_later
+          redirect_to :controller => :confirmations, :action => :confirm, :display_name => user.display_name
+        end

I agree with you that this is indeed a tricky scenario. I am not really a fan of hijacking the existing account once you signin successfully with social account with same email address.

> There's a definite problem here, that I had a case of only this week, where somebody tries a social signup and they already have an account but I think we need to send a confirmation email in all cases and probably make it a different one saying that we need to confirm the social signup link and just use it to confirm the link rather than doing a deactivate first.

I suspect user already had an OSM account and tried logging in with social account, with desire to add an alternate method of signing in. The functionality to do this already exists, by modifying in the "External Authentication" in settings.

> There's a definite problem here, that I had a case of only this week, where somebody tries a social signup and they already have an account but I think we need to send a confirmation email in all cases and probably make it a different one saying that we need to confirm the social signup link and just use it to confirm the link rather than doing a deactivate first.

This seems like a complex approach to me, since the confirmation email process currently practically only activates the account.

I suggest we step back and simply detect this scenario, and suggest the user to actually login with their email account/password, and modify their settings to include social sign in? This would also eliminate the scenarios where the user already uses a different social signup.


-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4455#discussion_r1479872612
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/pull/4455/review/1865305146 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20240206/d2277515/attachment.htm>