Creating user accounts as an administrative user I have a site where user accounts can be created in two ways: by new users (sign up) and by admin users. Since the creation is handled differently in each case, I decided to split the logic into two separate functions. I have a working implementation but I'm pretty new to Rails so I don't know if it is robust and secure. I am looking for feedback on how to improve it.

users_controller.rb

before_action :signed_in_user, only: [:home, :edit, :update, :destroy]
before_action :correct_user, only: [:edit, :update]
before_action :admin_user, only: [:create_by_admin, :destroy]

.
.
.

def new
    if !signed_in?
        @user = User.new
        @user.confirmation_code = User.new_confirmation_code
    elsif signed_in? &amp;&amp; current_user.admin?
        @user = User.new
        render 'new_by_admin' # basically a new.html.erb but with a dropdown for the admin setting
    else
        redirect_to root_url
    end
end

def create
    if signed_in? &amp;&amp; !current_user.admin?
        redirect_to root_url
    elsif signed_in? &amp;&amp; current_user.admin?
        create_by_admin
    else
        create_by_user
    end
end

def create_by_user
    @user = User.new(user_params)
    if @user.save
        UserMailer.confirmation_email(@user).deliver
        flash[:success] = "Great! Now check your email for activation instructions."
        redirect_to root_url
    else
        render 'new'
    end
end

def create_by_admin
    @user = User.new(admin_params)
    if @user.save
        @user.update_attributes(confirmed: true)
        flash[:success] = "User for " + @user.name + " created."
        redirect_to users_path
    else
        render 'new_by_admin'
    end
end

.
.
.

private
    def user_params
        params.require(:user).permit(:name, :email, :password, :password_confirmation)
    end

    def admin_params
        params.require(:user).permit(:name, :email, :password, :password_confirmation, :admin)
    end


Essentially, all create requests are routed to the create action, and from there sent to the appropriate method (depending on whether the requesting user is a new user or an existing user who is an admin). The differences between the methods is that new users receive an email to activate their accounts upon signup and cannot set themselves as admin, whereas admin users create accounts that are already confirmed and can set them as admin.

Is this a silly way to do it? Should I create an admin_controller that handles all administrative requests?
It's fine to create 2 methods to break up the logic a bit - however, there are other things I'd look into before tackling the user vs admin stuff:

Firstly: Ruby style prescribes 2-spaces indentation - not tabs, not 4 spaces.

Second, if you're going to use them, make those create_by* methods private. Don't mix 'em with the regular controller actions.

Third: String interpolation:

flash[:success] = "User for #{@user.name} created."


But success is an uncommon flash key. You can use the default notice message type directly in your redirect

redirect_to root_url, notice: "User for #{@user.name} created."


If you absolutely want to use success, you can use add_flash_types to add it, and use redirect_to ..., success: '...'.

And then you have this:

render 'new_by_admin' # basically a new.html.erb but with a dropdown for the admin setting


Don't use two templates - that's a giant hassle to maintain. Use a conditional in the view itself to show or not show the dropdown. I'd also add an admin? helper like this

# in application_controller.rb
def admin?
  current_user.try(:admin?)
end
helper_method :admin?


(I'm assuming that current_user is nil if you're not signed in).

So now you can check for admins in one go rather than both check signed_in? and then check current_user.admin? 

In the new view you get this:

&lt;% if admin? %&gt;
  your dropdown
&lt;% end %&gt;


And you don't need to 2 templates. Your if..elsif..else in #create can also become the more readable:

if admin?
  ...
elsif signed_in? # regular user
  ...
else
  ...
end


Your code also makes me wonder how you handle passwords. If an admin creates a user, I imagine you'll want to send login instructions and password to the new user.

Right now, it looks like the admin picks a password for other people, but you should already have a "reset password" method (for users who've forgotten their password) that will create a random password and email it to the user. It shouldn't be the admin's job to make up passwords.