Refactor UsersController#create

* Simplify controller action
* Extract service classes

app/controllers/users_controller.rb

@@ -1,6 +1,5 @@
 require_dependency 'discourse_hub'
 require_dependency 'user_name_suggester'
-require_dependency 'user_activator'
 require_dependency 'avatar_upload_service'
 
 class UsersController < ApplicationController
@@ -9,6 +8,7 @@ class UsersController < ApplicationController
   skip_before_filter :check_xhr, only: [:show, :password_reset, :update, :activate_account, :authorize_email, :user_preferences_redirect, :avatar]
 
   before_filter :ensure_logged_in, only: [:username, :update, :change_email, :user_preferences_redirect, :upload_avatar, :toggle_avatar]
+  before_filter :respond_to_suspicious_request, only: [:create]
 
   # we need to allow account creation with bad CSRF tokens, if people are caching, the CSRF token on the
   #  page is going to be empty, this means that server will see an invalid CSRF and blow the session
@@ -120,17 +120,39 @@ class UsersController < ApplicationController
   end
 
   def create
-    return fake_success_response if suspicious? params
+    user = User.new(user_params)
 
-    user = User.new_from_params(params)
-    user.ip_address = request.ip
-    auth = authenticate_user(user, params)
-    register_nickname(user)
+    authentication = UserAuthenticator.new(user, session)
+    authentication.start
 
-    user.save ? user_create_successful(user, auth) : user_create_failed(user)
+    activation = UserActivator.new(user, request, session, cookies)
+    activation.start
 
+    if user.save
+      authentication.finish
+      activation.finish
+
+      render json: {
+        success: true,
+        active: user.active?,
+        message: activation.message
+      }
+    else
+      render json: {
+        success: false,
+        message: I18n.t(
+          'login.errors',
+          errors: user.errors.full_messages.join("\n")
+        ),
+        errors: user.errors.to_hash,
+        values: user.attributes.slice('name', 'username', 'email')
+      }
+    end
   rescue ActiveRecord::StatementInvalid
-    render json: { success: false, message: I18n.t("login.something_already_taken") }
+    render json: {
+      success: false,
+      message: I18n.t("login.something_already_taken")
+    }
   rescue DiscourseHub::NicknameUnavailable => e
     render json: e.response_message
   rescue RestClient::Forbidden
@@ -325,67 +347,6 @@ class UsersController < ApplicationController
       challenge
     end
 
-    def suspicious?(params)
-      honeypot_or_challenge_fails?(params) || SiteSetting.invite_only?
-    end
-
-    def fake_success_response
-      render(
-        json: {
-          success: true,
-          active: false,
-          message: I18n.t("login.activate_email", email: params[:email])
-        }
-      )
-    end
-
-    def honeypot_or_challenge_fails?(params)
-      params[:password_confirmation] != honeypot_value ||
-      params[:challenge] != challenge_value.try(:reverse)
-    end
-
-    def valid_session_authentication?(auth, email)
-      auth && auth[:email] == email && auth[:email_valid]
-    end
-
-    def create_third_party_auth_records(user, auth)
-      return unless auth && auth[:authenticator_name]
-
-      authenticator = Users::OmniauthCallbacksController.find_authenticator(auth[:authenticator_name])
-      authenticator.after_create_account(user, auth)
-    end
-
-    def register_nickname(user)
-      if user.valid? && SiteSetting.call_discourse_hub?
-        DiscourseHub.register_nickname(user.username, user.email)
-      end
-    end
-
-    def user_create_successful(user, auth)
-      activator = UserActivator.new(user, request, session, cookies)
-      create_third_party_auth_records(user, auth)
-
-      # Clear authentication session.
-      session[:authentication] = nil
-      render json: { success: true, active: user.active?, message: activator.activation_message }
-    end
-
-    def user_create_failed(user)
-      render json: {
-        success: false,
-        message: I18n.t("login.errors", errors: user.errors.full_messages.join("\n")),
-        errors: user.errors.to_hash,
-        values: user.attributes.slice("name", "username", "email")
-      }
-    end
-
-    def authenticate_user(user, params)
-      auth = session[:authentication]
-      user.active = true if valid_session_authentication?(auth, params[:email])
-      user.password_required! unless auth
-      auth
-    end
-
     def build_avatar_from(file)
       source = if file.is_a?(String)
                  is_api? ? :url : (raise FastImage::UnknownImageType)
@@ -403,4 +364,33 @@ class UsersController < ApplicationController
       render json: { url: upload.url, width: upload.width, height: upload.height }
     end
 
+    def respond_to_suspicious_request
+      if suspicious?(params)
+        render(
+          json: {
+            success: true,
+            active: false,
+            message: I18n.t("login.activate_email", email: params[:email])
+          }
+        )
+      end
+    end
+
+    def suspicious?(params)
+      honeypot_or_challenge_fails?(params) || SiteSetting.invite_only?
+    end
+
+    def honeypot_or_challenge_fails?(params)
+      params[:password_confirmation] != honeypot_value ||
+        params[:challenge] != challenge_value.try(:reverse)
+    end
+
+    def user_params
+      params.permit(
+        :name,
+        :email,
+        :password,
+        :username
+      ).merge(ip_address: request.ip)
+    end
 end