Rails Security: redirect if not current_user

Question

I've been using authlogic and it works really well. One thing I've noticed is that as a hacker I could easily type in this address:

localhost::3000/users/a_user_that_is_not_me/edit

Since the form in the edit form is for @user which is set to current_user and requires an authenticity token, even if I tried to put in details for the other user I end up changing my own account instead of the other users.

That's nice and good, but I'd like it so that these hackers get redirected before they even see the form.

I tried this in the users_controller:

def edit
  if admin?
    @user = params[:user]
  elsif User.find_by_username(params[:id]) != current_user
    @user = current_user
    @not_user = User.find_by_username(params[:id])
    redirect_to user_path(@not_user)
  else
    @user = current_user
  end
end

The redirect works if I type in an address with another user's name but I get a 404 error when trying to access the edit page for the current user.

Any ideas why this doesn't work?

Answer 1

It looks like you are assigning @user to a string if the current user is an admin. This is simpler (less typo-prone):

def edit
  u = User.find_by_username!(params[:id])
  if admin? or current_user.username == params[:id]
    @user = u
  else
    redirect_to user_path(u)
  end
end

Also, don't you want to use find_by_username! (with bang on end) so that a 404 page is rendered when the user is not found? I'm not sure how you're getting the 404 page now...

Answer 2

If you're going to be doing this kind of thing a lot, check out an authorization plugin like authorization-san.

Authorization differs from authentication in that authentication is logging in, but authorization pertains to the authenticated (or un-authenticated) user's rights to perform actions.

With authentication-san, you could define this rule with this piece of code in your controller:

# this assumes you've got some way to set @user to the user you're looking up, 
# e.g. in a before_filter 
allow_access(:authenticated, :only => [:edit, :update]) { current_user == @user }