views:

73

answers:

2

Hello,

My Rails application have a User model and a Group model, where User belongs to a Group. Thanks to this, a user can be a admin, a manager, a subscriber, etc.

Until recently, when for example a new admin need to be create on the app, the process is just to create a new normal account, and then an admin sets the new normal account's group_id attribute as the group id of the admin... using some condition in my User controller. But it's not very clean, I think. Because for security, I need to add this kind of code in (for example) User#update:

class UsersController < ApplicationController
  # ...
  def update
    @user = User.find(params[:id])
    # I need to add some lines here, just as on the bottom of the post.
    # I think it's ugly... in my controller. But I can not put this
    # control in the model, because of current_user is not accessible
    # into User model, I think.
    if @user.update_attributes(params[:user])
      flash[:notice] = "yea"
      redirect_to root_path
    else
      render :action => 'edit'
    end
  end
  # ...
end

Is there a clean way to do it, with a Rails plugin? Or without...

By more clean, I think it could be better if those lines from User#update:

if current_user.try(:group).try(:level).to_i > @user.try(:group).try(:level).to_i
  if Group.exists?(params[:user][:group_id].to_i)
    if Group.find(params[:user][:group_id].to_i).level < current_user.group.level
      @user.group.id = params[:user][:group_id]
    end
  end
end

...was removed from the controller and the application was able to set the group only if a the current user's group's level is better then the edited user. But maybe I'm wrong, maybe my code is yet perfect :)

Note: in my User model, there is this code:

class User < ActiveRecord::Base
  belongs_to :group
  attr_readonly :group_id
  before_create :first_user
  private
  def first_user
    self.group_id = Group.all.max {|a,b| a.level <=> b.level }.id unless User.exists?
  end
end

Do you think it's a good way? Or do you process differently?

Thank you.

A: 

Well if you have a User/Groups (or User/Roles) model there is no other way to go than that you have underlined.

If it is a one-to-many association you can choose to store the user group as a string and if it is a many-to-many association you can go for a bitmask but nonetheless either through business logic or admin choice you need to set the User/Group relation.

You can have several choices on how to set this relationship in a view.

To expand your model's capability I advice you to use CanCan, a very good authorization gem which makes it super easy to allow fine grain access to each resource in your rails app.

tommasop
A: 

Hi, i prefer the controller methods to be lean and small, and to put actual model logic inside your model (where it belongs).

In your controller i would write something along the lines of

def update
  @user = User.find(params[:id]
  if @user.can_be_updated_by? current_user
    @user.set_group params[:user][:group_id], current_user.group.level
  end
  # remove group_id from hash
  params[:user].remove_key(:group_id)
  if @user.update_attributes(params[:user])
     ... as before
end

and in your model you would have

def can_be_updated_by? (other_user)
  other_user.try(:group).try(:level).to_i > self.try(:group).try(:level).to_i
end

def set_group(group_id, allowed_level)
  group = Group.find(group_id.to_i)
  self.group = group if group.present? && group.level < allowed_level
end

Does that help?

nathanvda
Thank you, nathanvda. Yea that help me! I think so too, model logic is better inside models ;)
moshimoshi