2009-05-23 4 views
1

Dans un accès d'originalité, j'écris une application de blog en utilisant Ruby on Rails. Mon PostsController contient du code qui garantit que l'utilisateur connecté peut uniquement modifier ou supprimer ses propres messages.Rails: Garder les contrôles d'usurpation d'utilisateur DRY

J'ai essayé de factoriser ce code dans une méthode privée avec un seul argument pour le message flash à afficher, mais quand je l'ai fait et testé en éditant le post d'un autre auteur, j'ai un ActionController::DoubleRenderError - "Peut seulement rendre ou rediriger une fois par action ".

Comment puis-je conserver ces vérifications DRY? L'approche évidente consiste à utiliser un filtre avant, mais la méthode destroy doit afficher un flash différent.

Voici le code du contrôleur concerné:

before_filter :find_post_by_slug!, :only => [:edit, :show] 

def edit 

    # FIXME Refactor this into a separate method 
    if @post.user != current_user 
    flash[:notice] = "You cannot edit another author’s posts." 
    redirect_to root_path and return 
    end 
    ... 
end 

def update 
    @post = Post.find(params[:id]) 

    # FIXME Refactor this into a separate method 
    if @post.user != current_user 
    flash[:notice] = "You cannot edit another author’s posts." 
    redirect_to root_path and return 
    end 
    ... 
end 

def destroy 
    @post = Post.find_by_slug(params[:slug]) 

    # FIXME Refactor this into a separate method 
    if @post.user != current_user 
    flash[:notice] = "You cannot delete another author’s posts." 
    redirect_to root_path and return 
    end 
    ... 
end 

private 
def find_post_by_slug! 
    slug = params[:slug] 
    @post = Post.find_by_slug(slug) if slug 
    raise ActiveRecord::RecordNotFound if @post.nil? 
end 

Répondre

2

L'approche du filtre avant est toujours une bonne option. Vous pouvez accéder à l'action demandée en utilisant la méthode action_name du contrôleur. Désolé pour cet opérateur ternaire dans le milieu ici. :) Naturellement, vous pouvez faire n'importe quelle logique que vous aimez.

Vous pouvez également utiliser une méthode si vous le souhaitez, et éviter le double rendu en renvoyant explicitement en cas d'échec. La clé ici est de retourner afin que vous ne doublez pas.

def destroy 
    @post = Post.find_by_slug(params[:slug]) 
    return unless authorized_to('delete') 
    ... 
end 

protected 

def authorized_to(mess_with) 
    if @post.user != current_user 
    flash[:notice] = "You cannot #{mess_with} another author’s posts." 
    redirect_to root_path and return false 
    end 
    return true 
end 

Vous pouvez simplifier plus (à mon avis) en séparant les différentes parties du comportement (autorisation, manipulation autorisation mauvaise) comme ceci:

def destroy 
    @post = Post.find_by_slug(params[:slug]) 
    punt("You cannot mess with another author's post") and return unless author_of(@post) 
    ... 
end 

protected 

def author_of(post) 
    post.user == current_user 
end 

def punt(message) 
    flash[:notice] = message 
    redirect_to root_path 
end 

Personnellement, je préfère délester tous ce travail de routine à un plugin. Mon plugin d'autorisation favori personnel est Authorization. Je l'ai utilisé avec beaucoup de succès ces dernières années.

Ce serait factoriser votre contrôleur à utiliser des variations sur:

permit "author of :post" 
+0

Ne faites pas de requête avant la vérification d'authentification! –

+0

@Pedro Vous allez devoir m'expliquer comment vous pouvez vérifier l'autorisation (pas l'authentification) en fonction d'un modèle spécifique avant d'avoir une copie de ce modèle. :) –

+0

Votre méthode d'autorisation fait 0 requêtes. Vous effectuez une recherche avant de vérifier si l'utilisateur est autorisé ou non. –

1

La réponse simple est de changer le message à quelque chose qui correspond à la fois: « Vous ne pouvez pas jouer avec les messages d'un autre auteur. »

+0

Oui, mais je ne veux pas vraiment faire ça. –

1

Si vous n'aimez pas le laid * retour dans cette dernière solution, vous pouvez utiliser un filtre autour et donné sous condition que si l'utilisateur est autorisé.

around_filter :check_authorization, :only => [:destroy, :update] 

private 
def check_authorization 
    @post = Post.find_by_slug(params[:slug]) 
    if @post.user == current_user 
     yield 
    else 
     flash[:notice] = case action_name 
     when "destroy" 
      "You cannot delete another author's posts." 
     when "update" 
      "You cannot edit another author's posts." 
     end 
     redirect_to root_path 
    end 
end 

* - c'est ma préférence, bien que le code soit parfaitement valide. Je trouve juste que, dans le style, ça a tendance à ne pas convenir.

Je dois également ajouter que je n'ai pas testé cela et je ne suis pas sûr à 100% que cela fonctionnerait, même si cela devrait être assez facile à essayer.

Questions connexes