while at a security review we noticed the following design/ security flaw in rails/ activerecord's named finders.
Named finders (ex. find_by_confirmation_token) are used when no or exactly one result is desired. Consider the following code:
account = Account.find_by_confirmation_token(params[:secret_token])
Confirmed accounts have their confirmation_token set to nil. Unconfirmed accounts have some digest there. Now when the controller action is called without secret_token set in the params hash a random confirmed account is returned. This is because there's no check if the underlying query returns more than one row. Of course this is not desired.
To help finding those subtile bugs/ design flaws, I'd suggest to remove the "LIMIT 1" from the generated sql by the named finders and instead raise an exception if the query returns more than one row. Suggestion: "named finders expect the sql to return at most one result, got xx".
This change should not have any backwards compatibility issues because a named finder with a query which selects more than one row is never desired and a sign for bad design.
What do you think?