это безопасный подход в ActiveRecords в Rails?

Question

Я использую следующее для своих клиентов, чтобы отписаться от моей рассылки;

  def index
    @user = User.find_by_salt(params[:subscribe_code]) 
    if @user.nil? 
      flash[:notice] = "the link is not valid...."
      render :action => 'index'
    else    
      Notification.delete_all(:user_id => @user.id)
      flash[:notice] = "you have been unsubscribed....."
      redirect_to :controller => 'home'
    end 
  end 

моя ссылка выглядит так; http://site.com/unsubscribe/32hj5h2j33j3h333

, поэтому приведенное выше сравнение случайной строки с полем в моей пользовательской таблице и, соответственно, удаление данных из таблицы уведомлений.

Мой вопрос; этот подход безопасен? Есть ли лучший / более эффективный способ сделать это?

Все предложения приветствуются.

Answer 1

Я не вижу ничего плохого в вашем решении, если действие не требует аутентификации.

Если для действия требуется аутентификация, я бы удостоверился, что salt принадлежит текущему пользователю

@user = User.find_by_id_and_salt(current_user.id, params[:subscribe_code])

Answer 2

Неужели так важно, чтобы пользователь был проинформирован, если его ссылка для отмены подписки была неверной? Каковы шансы на это в любом случае? Разве это не было сгенерировано программно, и эта программа была протестирована? Если ответ «да» (подсказка: так и должно быть), то я предлагаю всегда сообщать пользователю, что он отписался, независимо от того, что произошло.

def index
  begin
    @user = User.find_by_salt!(params[:subscribe_code]) 
  rescue ActiveRecord::RecordNotFound
  ensure
    @user.notifications.delete_all if @user
    flash[:notice] = "You have been unsubscribed."
    redirect_to :action => "index"
  end
end

Answer 3

Ваша ссылка должна быть

http://site.com/unsubscribe/?subscribe_code=32hj5h2j33j3h333

в противном случае «32hj5h2j33j3h333» получит в качестве параметра [: id]

остальное в порядке. Предполагая, что номер подписки будет уникальным.

Answer 4

Я думаю, что это безопаснее:

@user = User.find :all, :conditions => { salt => params[:subscribe_code] }

Таким образом, вы уверены, что Rails знает, что params [: subscribe_code] нужно экранировать. Вероятно, есть много способов написать это, что бы сработало.