Coding is like gardening...

Archive for the ‘under the hood’ Category

redirect_to :back not reliable

Suppose you want to allow someone to make a comment and then return to the page they were on. An easy way to do this in Rails is to set the CommentsController method to redirect_to :back

def create
  comment = Comment.new(params[:comment])
  if comment.save
    redirect_to :back
  end
end

But beware! The trouble with this is that Firefox has an option to disable referrers, so you can’t rely upon the HTTP_REFERER being set.

So you could set up a session variable when you display your comment form, to later be used in the controller. The current url is found in request.path

<% session[:return_to] = request.path -%>
def create
  comment = Comment.new(params[:comment])
  if comment.save
    redirect_to session[:return_to]
  end
end

Sure, people can turn off session cookies, but if they do that then you have all sorts of other Rails problems to do with authenticity tokens and user authentication, so I think this is a fairly safe method.

Of course, an even better option is to add the comment by AJAX and avoid the need for a page refresh at all … but that can be the topic of a different blog post!

Under the hood: Not-so-basic authentication

Recently I worked on a project that required a single login to access administration options. There was no need for a full-blown RESTful authentication solution – I was advised to “Just use basic auth!”

Rails makes it easy. You probably know the standard example. You put a before_filter :authenticate in the controllers that require it, and set it up in the Application Controller.

def authenticate
  authenticate_or_request_with_http_basic do |user, password|
    user == 'admin' && password == 'pass'
  end
end

It’s all well and good … until you want to add a log out button. The browser stores the successful login credentials in a sort of cookie, and applies them to every page which requests basic authentication. Once you’ve logged in, it’s actually quite hard to make the browser forget you until you quit and restart the browser. It’s hard, but not impossible. If you can force basic authentication to fail, the browser will throw away the credentials.

So the solution is to add a session variable that says “NO SRSLY, LOG ME OUT PLS!” This is the logout action (a destroy method in a Sessions Controller)

def destroy
  session[:logout_requested] = true
  flash[:notice] = "You have logged out successfully"
  redirect_to(root_path)
end

Now for the tricky bit. The way this works is subtle and takes a moment to figure out each time I think about it. We change the authenticate method in the Application Controller so that as well as checking the username and password, it also ensures that this flag has not been set. Meaning we can cause basic authentication to fail when we want it to.

def authenticate
  authenticate_or_request_with_http_basic do |user, password|
    user == 'admin' && password == 'pass' && session[:logout_requested] != true
  end
  session[:logout_requested] = nil
end

Next time our user goes to a page which requires authentication, the browser still provides the correct username and password, but the flag causes the basic authentication to fail. Obviously we then have to clear the flag straight away, otherwise the user would not be able to get back in again even with the correct credentials. The user must type in the correct login name and password again to be able to get back in.

Perhaps we want to know whether the user is logged in or not, so that we know whether to display an edit button. We can set another session variable. Conveniently, authenticate_or_request_with_http_basic returns a boolean value.

def authenticate
  session[:logged_in] = authenticate_or_request_with_http_basic do |user, password|
    user == 'admin' && password == 'pass' && session[:logout_requested] != true
  end
  session[:logout_requested] = nil
end

Remember to set the flag to false when you log out. Also remember that this flag could be true, false or nil so a check in the Application Controller looks like this:

def logged_in?
  session[:logged_in] == true
end

Finally it’s worth noting that the username and password do not have to be hard-coded like this. It’s simple for an example, but don’t think that’s all there is to basic authentication. There’s nothing to stop you comparing against values in a settings table or even doing a user lookup à la RESTful authentication.

def authenticate
  session[:logged_in] = authenticate_or_request_with_http_basic do |email, password|
    user = User.authenticate(email, password)
    if user && session[:logout_requested] != true
      self.current_user = user
      true
    else
      self.current_user = nil
      false
    end
  end
  session[:logout_requested] = nil
end

Thanks to Richard and Tris for their help in figuring out the not-so-basic aspects of basic authentication! :)

Under the hood: RSpec as your early warning system

This article is part of our ‘under the hood’ series, where we examine coding concepts and techniques related to how we build our products and services for our clients. If you’re of a technical bent, then do check out other articles as they appear in our ‘under the hood’ category.

One of the most useful aspects of RSpec and BDD is how it can highlight areas of your code that are obvious targets for improvement.

When we write our specs before our code we are effectively writing the API, the contract that our code must fulfill. It’s often the case that if this contract is hard to write, or just looks clumsy, then there may be a better way to express it.  This makes RSpec our first way of detecting code smells.

Perhaps the easiest type of code smell to detect with RSpec is Law of Demeter violations. Imagine a simple Ruby on Rails task management application. Tasks can be owned by Users. There may be many places where we would want to show who owns what Task. The simplest way to do this in Rails would be to use a belongs_to association to expose the attributes of the User to the Task. This makes it trivial to get the name of the user who is assigned to a task:

class Task < ActiveRecord::Base
  belongs_to :owner, :class_name => "User"
end

# to get the owner name:
task.owner.name

While this is easy to write, it would make our specs more complex. Anywhere we wished to mock the name of the owner we would have to write something like the following:

task.stub!(:owner).and_return(mock_model(User, :name => "Bob Smith"))

There’s a lot of noise in this line. We’re creating a mock object just to produce the name of the user. Typing and maintaining test code like the above just seems unnecessary — we’re mocking more than we need. Perhaps worse, it doesn’t express our intentions clearly. We’re asking the task to return its owner, then asking that owner to give us its name. Instead of this approach, we can define the contract as we go and we can actually express our intention:

task.stub!(:owner_name).and_return("Bob Smith")

This makes our tests look tidier, but it actually has a more beneficial side effect. Even though we now have to write more code (speccing then adding the owner_name method to the Task class), it’s now the Task’s job to find the name of its owner. This makes our code more maintainable. For instance, imagine that not every task has an owner. With our original code, anywhere we explicitly used the owner association we would have to change the code to:

(task.owner.nil? ? "No Owner" : task.owner.name)

If this was used many times throughout our code base, we’ve suddenly got a large number of places to find and test this code. By defining the owner_name method early we avoided this problem because there’s only one place that now has to deal with the messy business of finding the name of the owner. Our other specs and stories aren’t broken because they can still happily call the owner_name method without worrying about how this is actually handled.

By using RSpec to help us separate out what information we want from how we get this information, we’ve made our codebase just a little bit cleaner and more maintainable.