Okay,

Today I fixed a weird bug, that I cannot even fully explain. We rely on CanCanCan to handle authorization of actions in our Ruby on Rails application. When something is not allowed, or the user is simply not logged in, we want to redirect the user back to the root, or to the login page of the application, depending on the situation.

Instead of having to write all of this in every controller, we created a small module to do the work for us, and this is included in the ApplicationController:

def permission_denied(*)
  session[:"#{permission_denied_resource}_permission_denied"] = true
  session[:"#{permission_denied_resource}_return_to"] = request.url
  response.headers['X-Permission-Denied'] = true if Rails.env.test?

  flash[:alert] = I18n.t('devise.failure.access_denied') if current_user

  respond_with do |f|
    f.html do
      redirect_to permission_denied_path and return unless request.xhr?
      render json: flash, status: :unauthorized
    end
    f.js do
      flash.discard
      render json: flash, status: :unauthorized
    end
    f.json do
      flash.discard
      render json: flash, status: :unauthorized
    end
  end
end

Now in certain cases, especially with links opened from Office documents, this would result in a double render error. The line causing the problem being this one:

redirect_to permission_denied_path and return unless request.xhr?

According to the precedence of Ruby this does not make much sense. an unless-modifier has a lower precedence than the and operation. My understanding is that the redirect_to and return should be executed together and then the unless statement is verified to actually do it. Or that is what is intended. But that didn’t match with the error that was happening.

When I wrote the code as follows, this problem disappeared:

f.html do
  if request.xhr?
    render json: flash, status: :unauthorized
  else
    redirect_to(permission_denied_path) and return
  end
end

The behaviour is exactly the same as we wanted to achieve with the unless modifier. However this does not raise and double rendering errors when clicking a link from a Word document.

Lesson Learned:  When combining multiple statements, it’s often better to write them out the long way and not cram it all together into a single line.

Advertisements