GET params dans le projet ruby-on-rails - meilleures pratiques?

Question

J'ai hérité d'une petite application rails et j'ai besoin de l'étendre légèrement. C'est en fait assez simple, mais je veux m'assurer que je le fais de la bonne manière ...

Si je visite myapp: 3000/api/personnes il me donne une liste complète des personnes au format XML. Je veux passer param dans l'URL afin que je puisse retourner les utilisateurs qui correspondent à la connexion ou à l'e-mail, par exemple. yapp: 3000/api/persons? login = jmais donnait la personne avec le login correspondant. Voici le code:

def index 
    if params.size > 2 # We have 'action' & 'controller' by default 
    if params['login'] 
     @person = [Person.find(:first, :conditions => { :login => params['login'] })] 
    elsif params['email'] 
     @persons = [Person.find(:first, :conditions => { :email => params['email'] })] 
    end 
    else 
    @persons = Person.find(:all) 
    end 
end 

Deux questions ...

1. Est-il sécuritaire? Est-ce que ActiveRecord me protège des attaques par injection SQL (remarquez que je fais confiance aux params qui arrivent)?
2. Est-ce la meilleure façon de le faire, ou existe-t-il des rails automagiques que je ne connais pas?

Answer 1

Voici la section du guide de sécurité ruby ​​on rails sur SQL Injection. Il semble que ce que vous avez, en utilisant des conditions de hachage, soit assez sûr. On dirait que l'utilisation Person.find_by_login ou Person.find_by_email pourrait être un peu mieux.

Answer 2

1. Oui, le code que vous avez indiqué doit être sûr de l'injection SQL.
2. Oui, ce qui est généralement le code de rails acceptable ... mais

Il y a quelques bizarreries.

Votre action d'index parle de @person et de @persons. Par convention, @persons est attendu, et @person est inhabituel. Je soupçonne que vous pouvez éliminer @person, et effacer les choses d'un seul coup. Quelque chose comme ça (non testé):

def index 
    @persons = if params[:email] 
    Person.find_all_by_email(params[:email]) 
    elsif params[:login] 
    Person.find_all_by_login(params[:login]) 
    else 
    Person.all 
    end 
end 

Ne pas oublier de mettre à jour votre point de vue - je suppose qu'il est toujours à la recherche @person. Si votre vue fait quelque chose d'intéressant avec @person, vous voudrez probablement la déplacer vers votre show show.